PLAN.md

@@ -1045,6 +1045,43 @@ has to be its own commit with its own re-recorded scorecard.
   `$out`/`$merge` durability semantics, whether the expression evaluator is
   shared with M3's pipeline updates, and whether `allowDiskUse` has to stop
   being a lie.
+- **`drop` unlocked the collection it had just freed** — *fixed.* Found while
+  writing the positional-refusal tests and not caused by them; it reproduced
+  at `3c5eee2` with that work stashed.
+
+  A use-after-free, not an allocator quirk. Dispatch held `drop`'s collection
+  lock across the handler, the handler freed the `Collection` the lock lives
+  in, and dispatch then ran `unlock_collection` on freed memory — an atomic
+  read-modify-write inside `Io.RwLock.unlock`. One insert and one drop was
+  enough.
+
+  The wire path not faulting was luck, not safety: 25 insert/drop cycles
+  against a live server pass because the general allocator leaves the freed
+  page mapped, so the atomic write lands somewhere harmless. It was the same
+  undefined behaviour either way, and `testing.allocator` is what made it
+  visible. **`drop` had no unit test at all**, which is why it went unseen.
+
+  Fixed by giving `drop` no collection lock. The catalog lock is what actually
+  excludes: every collection lock in the engine — dispatch, the TTL sweep,
+  `compact`'s rebuild, `write_catalog`, `slab_stats`, reclamation — is taken
+  while holding the catalog at least shared, so `drop` holding it exclusively
+  already keeps all of them out. The collection lock bought exclusion that was
+  already there and paid for it by locking an object about to cease existing.
+
+  That change also made the dispatch epilogue's predicate wrong, and it turned
+  out to have been wrong already: it fired on `locks.coll == .exclusive` as a
+  stand-in for "this was a write", and **`dropDatabase` is the one write that
+  never held a collection lock**, so it had never reached the commit and
+  checkpoint epilogue at all. Now keyed on `kind == .write`.
+
+  Still open, and deliberately not fixed here: `drop_collection` writes no log
+  record, so a dropped collection resurrects on reopen unless a checkpoint
+  happened to run — a pre-existing limitation with its own test at
+  `db.zig:5542`. Separately, `apply_pending_write` (`$out`/`$merge`) calls
+  `drop_collection` under `engine.rwlock` rather than the catalog lock, so it
+  is not excluded by the reasoning above; it holds no collection lock, so it
+  is not this crash, but the two drops disagree about which lock protects the
+  namespace and that wants one answer.
 - **M3 update operators** — open. `distinct` landed first because it was a
   whole missing command with no dependencies, and measuring it turned up three
   things worth keeping, none of which are `distinct`'s to fix:

docs/M3_ARRAYFILTERS_DESIGN_REVIEW.md

@@ -0,0 +1,334 @@
+# M3 design review — `arrayFilters`, and the positional operators
+
+Written before any code, for the same reason the M2 review was: the previous
+two milestones both began with a gate that pointed somewhere other than the
+work, and both times the measurement said so before the implementation did.
+This one is no exception, but it fails in a new direction — the plan names a
+*missing feature*, and the measurement found a *data-loss bug*.
+
+Everything below was measured on 2026-08-10 against mongod 8.3.7 on
+`:27099` and this server at `5942f5e` on `:27020`, running the identical
+probe against both.
+
+---
+
+## 1. The plan names a feature; the measurement found data loss
+
+PLAN §3 lists `arrayFilters` in M3's scope, and PLAN §6 sizes it at "14 cases".
+That framing says: a feature is absent, and implementing it converts 14
+failures into passes.
+
+What is actually happening is worse and cheaper to describe. Take the corpus's
+own document and its own update:
+
+```js
+// { _id: 1, y: [ {b: 3}, {b: 1} ] }
+updateOne({}, {$set: {'y.$[i].b': 2}}, {arrayFilters: [{'i.b': 3}]})
+```
+
+| | result |
+|---|---|
+| mongod 8.3.7 | `{_id: 1, y: [{b: 2}, {b: 1}]}` |
+| this server | `{_id: 1, y: {"$[i]": {"b": 2}}}` |
+
+The array is gone. Not "not updated" — **replaced by a document whose single
+key is the literal text `$[i]`**, with every element and everything in them
+discarded. The server answers `ok: 1`, `matchedCount: 1`, `modifiedCount: 1`.
+No error, no warning, nothing in the log.
+
+This is the class M2 spent a milestone eliminating — a construct the engine
+does not implement answering confidently instead of refusing — except that
+here the wrong answer is not a `0` in a report, it is destruction of the
+client's stored data, and it is reachable by any unauthenticated client
+issuing a completely ordinary MongoDB update.
+
+### It is not an `arrayFilters` bug
+
+`arrayFilters` is not consulted at all. The string `arrayFilters` appears
+**nowhere in `src/`** — the option is accepted off the wire and dropped. The
+destruction comes from the *path*, and every path form that MongoDB gives a
+non-numeric array segment triggers it:
+
+| update | mongod | this server |
+|---|---|---|
+| `{$set: {'y.$[i].b': 2}}` + filters | `y: [{b:2},{b:1}]` | `y: {"$[i]": {"b": 2}}` |
+| `{$set: {'y.$[].b': 9}}` | `y: [{b:9},{b:9}]` | `y: {"$[]": {"b": 9}}` |
+| `{$set: {'y.$.b': 7}}` after `{'y.b': 3}` | `y: [{b:7},{b:1}]` | `y: {"$": {"b": 7}}` |
+| `{$inc: {'y.$[i].b': 10}}` + filters | `y: [{b:13},{b:1}]` | `y: {"$[i]": 10}` |
+
+The last row is worth a second look: `$inc` did not increment anything. It
+created a field and gave it the *operand*. So this is not `$set`-specific
+either — it is below the operator, in the path walk that every operator uses.
+
+**Three operators, one root cause.** `$` (positional, MongoDB 2.x),
+`$[]` (all-positional, 3.6) and `$[<ident>]` (filtered-positional, 3.6) all
+land in the same branch.
+
+### The branch
+
+`src/update.zig:283`, inside `set_path`, reached when a path segment under an
+array is not a number:
+
+```zig
+.array => |arr| {
+    const index = parse_index(segs[1]) orelse {
+        // treat as non-array: replace with a doc
+        var sub_pairs: std.ArrayListUnmanaged(bson.Pair) = .empty;
+        ...
+        pairs.items[idx].value = .{ .doc = try sub_pairs.toOwnedSlice(arena) };
+        return;
+    };
+```
+
+The comment states the behaviour plainly and is not wrong about what the code
+does. `parse_index("$[i]")` fails, so the array is treated as a thing that
+should have been a document, and is overwritten with one. For a genuinely
+unknown field name under an array — `y.nope.b` — that is arguably a defensible
+reading of MongoDB's create-intermediate-paths rule. For `$`, `$[]` and
+`$[i]`, which are the three ways MongoDB spells "descend into this array", it
+is the exact inversion of the intent.
+
+Out of 17 probe cases covering the operators, their edge cases and their
+refusals, **16 diverge from mongod and 1 agrees**. The one that agrees is
+`arrayFilters` supplied alongside a *replacement* document, where both servers
+ignore it.
+
+---
+
+## 2. Where the implementation actually is
+
+| | state |
+|---|---|
+| `arrayFilters` parsing | absent from `src/` entirely |
+| `$[<ident>]` | destroys the array |
+| `$[]` | destroys the array |
+| `$` positional | destroys the array |
+| `update.apply` signature | `(doc, update)` — no filter, no arrayFilters |
+| path model | `split_path` → `[]const []const u8`, max 16 segments |
+| target model | exactly one target per path, resolved while writing |
+| `modifiedCount` | byte comparison in `engine.replace` |
+
+Two of these rows are load-bearing for the design, and both are good news.
+
+**`modifiedCount` comes free.** `n_modified` is incremented only when
+`engine.replace` returns `.modified`, and that is decided by whether the
+serialized bytes changed — "a write that would store the same bytes is neither
+logged nor counted". So the moment the corruption stops, an update whose
+filters match nothing leaves the document byte-identical and is correctly
+reported as `modifiedCount: 0`. **5 of the 14 corpus failures are
+`modifiedCount` assertions and need no counting work at all.** (The runner
+reports the first failing assertion, so some of those 5 have a wrong outcome
+behind them too; the point is that none of them needs a *counter*.)
+
+**`update.apply` has no access to the filter.** This is the one real
+architectural fork, and it separates the three operators:
+
+- `$[]` and `$[<ident>]` need the document and the filter *list*. Both can be
+  passed down; nothing else is required.
+- `$` needs **which element the query matched** — information that exists only
+  during query evaluation and is thrown away before `apply` is called. Three
+  call sites (`cmd_update` at `commands.zig:1976`, `cmd_find_and_modify` at
+  `:2099`, and `:4131`) would each have to carry a matched-index out of the
+  match and into the update.
+
+So `$` is not "one more case of the same fix". It is the same *refusal*, but a
+different *implementation*, and it can be sequenced separately.
+
+---
+
+## 3. The gate named in the plan cannot see the worst of this
+
+M3's gate in PLAN §3 reads: *"remaining crud coverage; e2e3/e2e4 green"*.
+
+- **`e2e3.js` and `e2e4.js` contain zero occurrences** of `arrayFilters`, `$[`,
+  or a positional path. They are green today, they would stay green through
+  every version of this bug, and they will stay green after it is fixed. As a
+  gate for this work they measure nothing.
+- **The crud corpus covers `$[<ident>]` only.** Five files use it; four
+  contribute the 14 failures, and `client-bulkWrite-update-options.json`
+  is skipped entirely (needs server ≥ 8.0).
+- **The corpus contains no `$[]` case and no bare `$` case at all.** Grepped
+  the whole pinned unified corpus: zero.
+
+That last line is the finding. A fix scoped to what the gate measures would
+leave `$` and `$[]` still overwriting arrays with `{"$": {...}}`, and the
+scorecard would go green on 14 new passes while two of the three ways to
+destroy a client's data remained. The gate is not merely pointing away from
+the work — **it would certify the bug as fixed.**
+
+The 14, for the record:
+
+| file | cases | first failing assertion |
+|---|---|---|
+| `updateOne-arrayFilters.json` | 5 | 3 outcome, 2 modifiedCount |
+| `bulkWrite-arrayFilters.json` | 3 | 2 outcome, 1 modifiedCount |
+| `findOneAndUpdate-arrayFilters.json` | 3 | 3 outcome |
+| `updateMany-arrayFilters.json` | 3 | 1 outcome, 2 modifiedCount |
+
+---
+
+## 4. What the measurement says the fix has to look like
+
+Recorded because none of it is guessable, and because two of these directly
+contradict how `set_path` behaves today.
+
+### One path can name many targets
+
+```
+y.$[].c.$[].d   on   y: [ {c: [{d:1},{d:2}]}, {c: [{d:3}]} ]
+  -> y: [ {c: [{d:0},{d:0}]}, {c: [{d:0}]} ]
+```
+
+A genuine cross-product: three leaves, all written. `set_path`'s contract —
+walk to one place, write there — cannot express this. The path model has to
+become *enumerate the matching targets, then apply the operator to each*, which
+also means the resolution must sit **below** the operator dispatch, since
+`$unset` through `$[i]` works the same way (measured).
+
+### A positional segment never creates anything
+
+This is the sharp reversal. `$set: {'a.b': 1}` on a document without `a`
+creates the intermediate document — that is MongoDB's rule and `set_path`
+implements it. But:
+
+| situation | mongod |
+|---|---|
+| `y.$[i].b` where `y` is missing | error 2, "The path 'y' must exist in the document in order to apply array update" |
+| `y.$[i].b` where `y` is `5` | error 2, "Cannot apply array updates to non-array element y: 5" |
+| upsert with `$[i]`, nothing matched | error 2, same "path must exist" — the upsert does *not* get a special case |
+
+So a positional segment is a *filter over what is there*, never a constructor.
+The upsert row means no separate upsert path is needed: it falls out.
+
+### Identifiers are matched as a query against each element
+
+`arrayFilters: [{'i.b.q': 2}]` selects elements whose nested `b.q` is 2, and
+operators work (`{'i.b': {$gte: 0}}` matched every element). So each filter is
+an ordinary query document, rooted at the identifier, evaluated against each
+array element — `query.matches` should apply directly with the identifier
+stripped from the front of each key.
+
+### The refusals, measured
+
+| condition | code | message |
+|---|---|---|
+| identifier in path with no matching filter (including no `arrayFilters` at all) | 2 | `No array filter found for identifier 'k' in path 'y.$[k].b'` |
+| identifier is not lowercase-alphanumeric | 2 | `Error parsing array filter :: caused by :: The top-level field name must be an alphanumeric string beginning with a lowercase letter, found '1x'` |
+| the path's array does not exist | 2 | `The path 'y' must exist in the document in order to apply array update` |
+| the path's element is not an array | 2 | `Cannot apply array updates to non-array element y: 5` |
+| `$` when the query did not match the array | 2 | `Plan executor error during update :: caused by :: The positional operator did not find the match needed from the query.` |
+| a filter entry the update never uses | 9 | `The array filter for identifier 'i' was not used in the update { $set: { y.b: 2 } }` |
+| two filters with the same identifier | 9 | `Found multiple array filters with the same top-level field name i` |
+| a filter with no top-level field | 9 | `Cannot use an expression without a top-level field name in arrayFilters` |
+| a filter with two top-level fields | 9 | `Error parsing array filter :: caused by :: Expected a single top-level field name, found 'i' and 'j'` |
+| `arrayFilters` not an array | 14 | `BSON field 'update.updates.arrayFilters' is the wrong type 'string', expected type 'array'` |
+
+Mostly this splits as *9 = the `arrayFilters` array judged on its own*, *2 =
+anything needing the update document or the stored document*. **It is not a
+clean rule**, and it should not be tidied into one: "identifier is not
+lowercase-alphanumeric" carries the same `Error parsing array filter` prefix as
+the 9s and is nevertheless a 2. Recorded as measured. Inventing the rule and
+deriving the codes from it would get that row wrong, which is precisely the
+failure mode this repo has hit three times.
+
+`UpdateError` is a closed set consumed at three call sites that currently
+collapse everything to `bad_value(reply, "bad update")`. Ten distinct
+refusals with two codes and specific messages will not fit that shape; the
+error → reply mapping has to move or widen.
+
+---
+
+## 5. Gate options
+
+### Option A — the 14 corpus cases
+
+*"`updateOne`/`updateMany`/`findOneAndUpdate`/`bulkWrite`-arrayFilters: 0 fail."*
+
+Cheap, external, already written. And, per §3, it certifies the bug fixed
+while `$` and `$[]` still destroy arrays. It is the plan's implied gate and it
+is the one option the measurement rules out on its own.
+
+### Option B — the 14, plus a purpose-built positional corpus
+
+Option A plus a recorded corpus for the two operators the pinned suite forgot,
+built exactly like `tests/spec/aggregate/`: inputs authored in `sources/`,
+every expectation recorded from mongod 8.3.7 by a `record.js`, run through the
+shared runner with `--suite-dir`.
+
+That machinery exists and was built for this situation. The M2.5 corpus caught
+`$avg` over a no-numeric group returning `0` instead of `null` — a bug no
+hand-written expectation would have contained, because the author would have
+written down what they believed. The same argument applies here with more
+force: the beliefs about `$[]`, missing paths and upserts in §4 were wrong
+before I measured them.
+
+Cost: one `sources/positional.json`, one recorder run. The recorder is ~120
+lines and already written for aggregation; the update shape needs its own but
+the pattern transfers.
+
+### Option C — differential fuzzing against mongod
+
+Generate random documents and random positional updates, apply to both servers,
+compare. Strongest possible coverage of the target-enumeration logic, which is
+where the cross-product and the create-nothing rule will actually break.
+
+Rejected as *the gate* for the same reason M2 rejected it: it is not
+reproducible as a committed artifact, a red run names a random seed rather than
+a behaviour, and it needs a live mongod in the loop. Worth having as a
+one-off during development; not worth being the thing a milestone is judged on.
+
+### Option D — refuse, do not implement
+
+Detect `$`, `$[]` and `$[<ident>]` and answer BadValue instead of corrupting.
+Gate: the 14 cases fail *with the right code* rather than passing.
+
+This is not a joke option and it should not be dismissed quickly. It stops the
+data loss in roughly a day, it is the M2 doctrine applied exactly ("refuse what
+the engine does not implement rather than answering confidently"), and it can
+ship before the real implementation is designed. It leaves 14 cases red.
+
+---
+
+## 6. Recommendation
+
+**Option D first, as its own commit, then Option B.**
+
+The reasoning is the split between the two halves of this problem. They have
+very different urgency and very different sizes:
+
+- The data loss is a bug, it is remotely reachable, it needs no feature work to
+  fix, and every day it stays in `main` is a day the server can silently
+  destroy an array. Refusing costs a path scan and a `BadValue`.
+- The feature is a genuine redesign of the path model — one path to many
+  targets, resolution below operator dispatch, a create-nothing rule that
+  inverts the current one. It deserves to be designed against a corpus rather
+  than rushed to make a bug stop.
+
+Shipping D first also makes B's corpus honest: recorded against mongod while
+this server refuses cleanly, so every expectation in it is measured before a
+line of the implementation exists — the discipline that made M2.5's corpus
+worth having, where each tier was recorded red and then driven to green.
+
+Within B, sequence `$[]` and `$[<ident>]` together (same plumbing, both need
+only document + filters), and `$` after (needs the matched index carried out of
+query evaluation, three call sites).
+
+---
+
+## 7. What this review does not cover
+
+- **The other M3 operators.** `$setOnInsert`, `$addToSet`, `$mul`, `$min`/`$max`,
+  `$pop`, `$pullAll`, `$currentDate`, pipeline-form updates. Independent of this
+  work, and the ~10 pipeline-update failures are a separate design question.
+- **Index correctness after a positional update.** A multikey index over `y.b`
+  must be regenerated when an element changes. The normal `engine.replace` path
+  presumably handles it since it re-derives entries from the new document, but
+  that is an assumption stated, not a measurement — it needs a test in whichever
+  option is chosen.
+- **`max_path_segments = 16`.** Whether a positional segment costs one or two,
+  and what mongod's own limit is. Not measured.
+- **`$` in a projection** (`{'y.$': 1}`), which is the same sigil in a different
+  position and is not in scope here.
+- **The unknown-operator gap** recorded in PLAN §6 during the `distinct` work
+  (`{x: {$bogus: 1}}` matches nothing instead of erroring). Same silent-wrong-
+  answer class, different code path, still open.

src/commands.zig

@@ -81,6 +81,10 @@ pub const ErrorCode = enum(i32) {
     query_plan_killed = 175,
     unauthorized = 13,
     type_mismatch = 14,
+    /// `PathNotViable`, measured on mongod 8.3.7: what an update answers when
+    /// a path segment names a field inside something that cannot hold one --
+    /// in practice, a non-numeric segment applied to an array.
+    path_not_viable = 28,
     operation_failed = 96,
     // Session and transaction codes, measured against mongod 8.3.7 with a raw
     // OP_MSG probe -- the driver rewrites `lsid` with its own session, so a
@@ -194,7 +198,19 @@ const command_table = [_]Command{
     // Writes: the target collection exclusively; create/drop take the
     // catalog exclusively (they mutate the maps).
     .{ .name = "create", .kind = .write, .locks = .{ .catalog = .exclusive, .coll = .exclusive }, .handler = cmd_create },
-    .{ .name = "drop", .kind = .write, .locks = .{ .catalog = .exclusive, .coll = .exclusive }, .handler = cmd_drop },
+    // `drop` takes the catalog exclusively and **no collection lock**: it frees
+    // the very Collection a lock would live in, and dispatch then unlocked the
+    // freed memory. That was a use-after-free on an `Io.RwLock`, and under
+    // testing.allocator it is a hard SIGSEGV on the first insert-then-drop.
+    //
+    // Nothing is lost by dropping the lock, because the catalog lock is what
+    // actually excludes here: every collection lock in this engine -- dispatch,
+    // the TTL sweep, `compact`'s rebuild, `write_catalog`, `slab_stats`,
+    // reclamation -- is taken while holding the catalog at least shared, so
+    // holding it exclusively already keeps every one of them out. The
+    // collection lock was buying exclusion that was already there, and paying
+    // for it by locking an object about to cease existing.
+    .{ .name = "drop", .kind = .write, .locks = .{ .catalog = .exclusive }, .handler = cmd_drop },
     .{ .name = "dropDatabase", .kind = .write, .locks = .{ .catalog = .exclusive }, .handler = cmd_drop_database },
     .{ .name = "createIndexes", .kind = .write, .locks = .{ .catalog = .shared, .coll = .exclusive }, .handler = cmd_create_indexes },
     .{ .name = "dropIndexes", .kind = .write, .locks = .{ .catalog = .shared, .coll = .exclusive }, .handler = cmd_drop_indexes },
@@ -309,7 +325,15 @@ pub fn dispatch(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
         };
         try ctx.engine.commit();
     }
-    if (cmd.locks.coll == .exclusive) {
+    // Keyed on what the command *is*, not on which lock it happened to take.
+    // These two agreed for every command until `drop` gave up its collection
+    // lock, at which point the old `locks.coll == .exclusive` would have
+    // silently stopped committing and checkpointing after a drop. It was
+    // already wrong for `dropDatabase`, the one write that never held a
+    // collection lock: it has been skipping this epilogue all along, so a
+    // dropped database waited for some later write to trigger a checkpoint
+    // before the catalog recording it was written.
+    if (cmd.kind == .write) {
         // Durability (seal + fsync) coalesces across concurrent writers. A
         // commit error deliberately wins over the handler's captured `result`:
         // whether the write reached disk matters more to the client than why
@@ -1951,7 +1975,9 @@ fn cmd_update(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
 
         if (matched.items.len == 0) {
             if (upsert) {
-                const new_doc = try build_upsert_doc(reply, q, u_doc);
+                var up_diag: update.Diagnostic = .{};
+                const new_doc = build_upsert_doc(reply, q, u_doc, &up_diag) catch |err|
+                    return update_refusal(reply, err, up_diag);
                 ctx.engine.insert(db_name, coll_name, new_doc, ctx.oid_gen) catch |err| switch (err) {
                     error.DuplicateKey, error.DuplicateKeyIndex => return duplicate_key_error(ctx, reply, db_name, coll_name, new_doc),
                     else => return err,
@@ -1973,10 +1999,9 @@ fn cmd_update(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
             // and a rejected update must not corrupt the stored document.
             const doc = try doc_tree(reply.arena_alloc(), coll, off);
             const copy = try clone_doc(reply, doc);
-            update.apply(copy, &.{ .arena = undefined, .pairs = u_doc }) catch |err| switch (err) {
+            var diag: update.Diagnostic = .{};
-                error.ImmutableId, error.InvalidUpdate => return bad_value(reply, "bad update"),
+            update.apply(copy, &.{ .arena = undefined, .pairs = u_doc }, &diag) catch |err|
-                else => return err,
+                return update_refusal(reply, err, diag);
-            };
             const written = ctx.engine.replace(db_name, coll_name, copy, ctx.oid_gen) catch |err| switch (err) {
                 error.DuplicateKey, error.DuplicateKeyIndex => {
                     const e = try reply.arena_alloc().alloc(bson.Pair, 3);
@@ -2079,7 +2104,9 @@ fn cmd_find_and_modify(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !v
 
     if (target == null and do_update and upsert) {
         const u_doc = doc_arg(msg.body.get("update")) orelse return bad_value(reply, "update must be a document");
-        const new_doc = try build_upsert_doc(reply, q, u_doc);
+        var up_diag: update.Diagnostic = .{};
+        const new_doc = build_upsert_doc(reply, q, u_doc, &up_diag) catch |err|
+            return update_refusal(reply, err, up_diag);
         ctx.engine.insert(db_name, coll_name, new_doc, ctx.oid_gen) catch |err| switch (err) {
             error.DuplicateKey, error.DuplicateKeyIndex => return duplicate_key_error(ctx, reply, db_name, coll_name, new_doc),
             else => return err,
@@ -2096,10 +2123,9 @@ fn cmd_find_and_modify(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !v
         const u_doc = doc_arg(msg.body.get("update")) orelse return bad_value(reply, "update must be a document");
         const before = try bson.copy_pairs(arena, target.?.pairs);
         const copy = try clone_doc(reply, target.?);
-        update.apply(copy, &.{ .arena = undefined, .pairs = u_doc }) catch |err| switch (err) {
+        var diag: update.Diagnostic = .{};
-            error.ImmutableId, error.InvalidUpdate => return bad_value(reply, "bad update"),
+        update.apply(copy, &.{ .arena = undefined, .pairs = u_doc }, &diag) catch |err|
-            else => return err,
+            return update_refusal(reply, err, diag);
-        };
         // findAndModify reports `n` (matched) and `updatedExisting`, neither of
         // which distinguishes a no-op, so whether it wrote is not needed here.
         _ = try ctx.engine.replace(db_name, coll_name, copy, ctx.oid_gen);
@@ -4108,12 +4134,48 @@ fn clone_doc(reply: *wire.Reply, doc: *const bson.Document) !*bson.Document {
     return owned;
 }
 
+/// The reply an update refusal turns into.
+///
+/// Shared by all four places that apply an update, because a refusal wired
+/// into three of them would be a silent divergence between `update`,
+/// `findAndModify` and the upsert path -- and the two new ones exist to stop
+/// a silent divergence in the first place.
+///
+/// The messages are this server's own words. mongod's `PathNotViable` text
+/// embeds a shell-syntax rendering of the offending element (`Cannot create
+/// field 'nope' in element {y: [ { b: 3 }, { b: 1 } ]}`), and there is no BSON
+/// formatter here that produces it. The code is what the corpus asserts and
+/// the code is exact; a half-copy of the text would be worse than a clear
+/// sentence that does not pretend.
+fn update_refusal(reply: *wire.Reply, err: anyerror, diag: update.Diagnostic) !void {
+    const arena = reply.arena_alloc();
+    switch (err) {
+        error.PositionalUnsupported => return bad_value(reply, try std.fmt.allocPrint(
+            arena,
+            "the positional operator '{s}' in path '{s}' is not implemented by this server",
+            .{ diag.segment, diag.path },
+        )),
+        error.PathNotViable => return reply.put_error(
+            @intFromEnum(ErrorCode.path_not_viable),
+            "PathNotViable",
+            try std.fmt.allocPrint(
+                arena,
+                "Cannot create field '{s}' in an array, at path '{s}'",
+                .{ diag.segment, diag.path },
+            ),
+        ),
+        error.ImmutableId, error.InvalidUpdate => return bad_value(reply, "bad update"),
+        else => return err,
+    }
+}
+
 /// Build the document for an upsert: equality fields from the filter, then
 /// the update operators applied. Owned by the reply arena.
 fn build_upsert_doc(
     reply: *wire.Reply,
     q: []const bson.Pair,
     u_doc: []const bson.Pair,
+    diag: *update.Diagnostic,
 ) !*bson.Document {
     const arena = reply.arena_alloc();
     var pairs: std.ArrayListUnmanaged(bson.Pair) = .empty;
@@ -4128,10 +4190,7 @@ fn build_upsert_doc(
     const owned = try arena.create(bson.Document);
     owned.* = bson.Document{ .arena = std.heap.ArenaAllocator.init(arena), .pairs = try pairs.toOwnedSlice(arena) };
     // Apply update operators to build the final doc; _id handled by insert.
-    update.apply(owned, &.{ .arena = undefined, .pairs = u_doc }) catch |err| switch (err) {
+    try update.apply(owned, &.{ .arena = undefined, .pairs = u_doc }, diag);
-        error.ImmutableId, error.InvalidUpdate => return error.InvalidUpdate,
-        else => return err,
-    };
     return owned;
 }
 
@@ -5494,6 +5553,122 @@ test "distinct applies its filter before collecting" {
     try testing.expectEqual(@as(i32, 33), values[1].int32);
 }
 
+test "drop does not unlock the collection it just freed" {
+    // Regression test for a use-after-free: dispatch held `drop`'s collection
+    // lock across the handler, the handler freed the Collection the lock lives
+    // in, and dispatch then ran `unlock_collection` on freed memory. One
+    // insert and one drop was enough -- SIGSEGV inside `Io.RwLock.unlock`.
+    //
+    // Two things kept it hidden. `drop` had no unit test at all: before this
+    // one, every `parse_fake_msg("drop", ...)` in the tree was inside a test
+    // written to hunt it. And over the wire it did not fault -- 25
+    // insert/drop cycles against a live server pass -- because the general
+    // allocator leaves the freed page mapped, so the atomic write lands
+    // somewhere harmless. testing.allocator is what makes it visible, which
+    // is exactly why this test belongs here rather than in an e2e script.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+    var ctx = tdb.ctx(io);
+
+    try dispatch_insert(&tdb, io, "arr", &.{
+        .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 1 } }} },
+    });
+    try testing.expectEqual(@as(?i32, null), try run_for_code(&ctx, "drop", .{ .string = "arr" }, &.{}));
+    try testing.expect(ctx.engine.get_collection("test", "arr") == null);
+
+    // The namespace is reusable afterwards, and dropping it again is
+    // NamespaceNotFound rather than a second free.
+    try dispatch_insert(&tdb, io, "arr", &.{
+        .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 2 } }} },
+    });
+    try testing.expectEqual(@as(i32, 1), try doc_count(&ctx, "arr"));
+    try testing.expectEqual(@as(?i32, null), try run_for_code(&ctx, "drop", .{ .string = "arr" }, &.{}));
+    try testing.expectEqual(
+        @as(?i32, @intFromEnum(ErrorCode.namespace_not_found)),
+        try run_for_code(&ctx, "drop", .{ .string = "arr" }, &.{}),
+    );
+}
+
+test "dropDatabase frees its collections without unlocking them" {
+    // The same shape one level up, and the reason the epilogue is now keyed on
+    // `kind == .write`: dropDatabase is the one write that never held a
+    // collection lock, so it never reached the commit/checkpoint epilogue.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+    var ctx = tdb.ctx(io);
+
+    try dispatch_insert(&tdb, io, "one", &.{
+        .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 1 } }} },
+    });
+    try dispatch_insert(&tdb, io, "two", &.{
+        .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 2 } }} },
+    });
+    try testing.expectEqual(@as(?i32, null), try run_for_code(&ctx, "dropDatabase", .{ .int32 = 1 }, &.{}));
+    try testing.expect(ctx.engine.get_collection("test", "one") == null);
+    try testing.expect(ctx.engine.get_collection("test", "two") == null);
+}
+
+test "a positional update is refused on the wire and stores nothing" {
+    // The end of the chain the unit tests start: the refusal has to reach the
+    // client as a code, and the stored document -- not just the working copy
+    // -- has to be the one that was there before.
+    //
+    // Every case here previously answered ok: 1 with nModified: 1, having
+    // replaced `y` with a document keyed by the path segment's text.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+    var ctx = tdb.ctx(io);
+
+    const cases = [_]struct { coll: []const u8, path: []const u8, code: i32 }{
+        .{ .coll = "a1", .path = "y.$[i].b", .code = 2 }, // filtered positional
+        .{ .coll = "a2", .path = "y.$[].b", .code = 2 }, // all-positional
+        .{ .coll = "a3", .path = "y.$.b", .code = 2 }, // positional
+        .{ .coll = "a4", .path = "y.nope.b", .code = 28 }, // PathNotViable, same branch
+    };
+    for (cases) |c| {
+        try dispatch_insert(&tdb, io, c.coll, &.{
+            .{ .doc = &.{
+                .{ .key = "_id", .value = .{ .int32 = 1 } },
+                .{ .key = "y", .value = .{ .array = &.{
+                    .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 3 } }} },
+                } } },
+            } },
+        });
+
+        const updates = [_]bson.Value{.{ .doc = &.{
+            .{ .key = "q", .value = .{ .doc = &.{} } },
+            .{ .key = "u", .value = .{ .doc = &.{
+                .{ .key = "$set", .value = .{ .doc = &.{
+                    .{ .key = c.path, .value = .{ .int32 = 2 } },
+                } } },
+            } } },
+        } }};
+        try testing.expectEqual(@as(?i32, c.code), try run_for_code(&ctx, "update", .{ .string = c.coll }, &.{
+            .{ .key = "updates", .value = .{ .array = &updates } },
+        }));
+
+        // Read it back through `distinct`: if the array survived it still has
+        // an element with `b: 3`, and if it was overwritten by a document
+        // there is nothing at `y.b` at all.
+        var reply = wire.Reply.init(testing.allocator);
+        defer reply.deinit();
+        const values = try distinct_values(&tdb, io, &reply, c.coll, &.{
+            .{ .key = "key", .value = .{ .string = "y.b" } },
+        });
+        try testing.expectEqual(@as(usize, 1), values.len);
+        try testing.expectEqual(@as(i32, 3), values[0].int32);
+    }
+}
+
 test "aggregate $sort without a preceding $group sorts and frees correctly" {
     // Regression test for a remote, client-triggerable invalid free: the
     // $sort stage materialized its document list from the reply arena and

src/update.zig

@@ -6,7 +6,42 @@ const std = @import("std");
 const bson = @import("bson.zig");
 const query = @import("query.zig");
 
-pub const UpdateError = error{ ImmutableId, InvalidUpdate, OutOfMemory };
+pub const UpdateError = error{
+    ImmutableId,
+    InvalidUpdate,
+    /// A `$`, `$[]` or `$[<identifier>]` segment: MongoDB's three ways of
+    /// saying "descend into this array", none of them implemented here.
+    PositionalUnsupported,
+    /// A non-numeric segment applied to an array, which is never a field to
+    /// create. mongod's `PathNotViable`.
+    PathNotViable,
+    OutOfMemory,
+};
+
+/// Which path a refusal was about, so the reply can name it instead of saying
+/// "bad update". Borrowed from the update document, which outlives the call.
+pub const Diagnostic = struct {
+    path: []const u8 = "",
+    segment: []const u8 = "",
+};
+
+fn note(diag: ?*Diagnostic, path: []const u8, segment: []const u8) void {
+    if (diag) |d| d.* = .{ .path = path, .segment = segment };
+}
+
+/// `$`, `$[]` and `$[<identifier>]` -- the three spellings of "descend into
+/// this array".
+///
+/// None is implemented, and until they are each has to be refused rather than
+/// walked. `set_path` used to reach the array, fail to read the segment as an
+/// index, and overwrite the array with a document keyed by the segment's
+/// literal text: `{$set: {"y.$[i].b": 2}}` turned `y: [{b: 3}, {b: 1}]` into
+/// `y: {"$[i]": {"b": 2}}` and answered ok: 1. Every element was discarded,
+/// under every operator -- `$inc` stored its operand rather than incrementing.
+fn is_positional(seg: []const u8) bool {
+    if (std.mem.eql(u8, seg, "$")) return true;
+    return seg.len >= 3 and std.mem.startsWith(u8, seg, "$[") and seg[seg.len - 1] == ']';
+}
 
 const max_path_segments = 16;
 
@@ -28,8 +63,16 @@ fn is_operator_key(key: []const u8) bool {
 }
 
 /// Apply an update document to `doc`: a replacement, or a set of operators.
-pub fn apply(doc: *bson.Document, update: *const bson.Document) UpdateError!void {
+pub fn apply(
+    doc: *bson.Document,
+    update: *const bson.Document,
+    diag: ?*Diagnostic,
+) UpdateError!void {
+    // A replacement carries data, not paths, so nothing below applies to it --
+    // which is also why mongod ignores `arrayFilters` alongside one, the single
+    // case out of seventeen where this server already agreed with it.
     if (is_replacement(update.pairs)) return apply_replacement(doc, update.pairs);
+    try reject_positional(update.pairs, diag);
     const arena = doc.arena.allocator();
     var pairs = try copy_to_list(bson.Pair, arena, doc.pairs);
     for (update.pairs) |op| {
@@ -37,11 +80,40 @@ pub fn apply(doc: *bson.Document, update: *const bson.Document) UpdateError!void
         // is not one is a mixed document -- which MongoDB rejects rather than
         // guessing at.
         if (!is_operator_key(op.key)) return error.InvalidUpdate;
-        try apply_operator(arena, &pairs, op.key, op.value);
+        try apply_operator(arena, &pairs, op.key, op.value, diag);
     }
     doc.pairs = try pairs.toOwnedSlice(arena);
 }
 
+/// Refuse every positional path in the update before any of it is applied.
+///
+/// Up front rather than at the point of use, because one update names several
+/// paths: checking as we walk would refuse the third path having already
+/// rewritten what the first two named. The caller discards its copy on error
+/// either way, so this is not what makes the refusal safe -- it is what makes
+/// the refusal *about the update* rather than about however far a walk got.
+fn reject_positional(update: []const bson.Pair, diag: ?*Diagnostic) UpdateError!void {
+    for (update) |op| {
+        const ops = doc_pairs(op.value) orelse continue;
+        for (ops) |p| {
+            try reject_positional_path(p.key, diag);
+            // `$rename`'s destination is a path too, and it is the *value*.
+            if (std.mem.eql(u8, op.key, "$rename") and p.value == .string) {
+                try reject_positional_path(p.value.string, diag);
+            }
+        }
+    }
+}
+
+fn reject_positional_path(path: []const u8, diag: ?*Diagnostic) UpdateError!void {
+    var it = std.mem.splitScalar(u8, path, '.');
+    while (it.next()) |seg| {
+        if (!is_positional(seg)) continue;
+        note(diag, path, seg);
+        return error.PositionalUnsupported;
+    }
+}
+
 /// Replace every field of `doc` with `replacement`'s, except `_id`.
 ///
 /// `_id` is immutable, so it survives and keeps its position at the front (which
@@ -97,6 +169,7 @@ fn apply_operator(
     pairs: *std.ArrayListUnmanaged(bson.Pair),
     op: []const u8,
     value: bson.Value,
+    diag: ?*Diagnostic,
 ) UpdateError!void {
     if (std.mem.eql(u8, op, "$set")) {
         const ops = doc_pairs(value) orelse return error.InvalidUpdate;
@@ -104,7 +177,7 @@ fn apply_operator(
             if (std.mem.eql(u8, p.key, "_id")) return error.ImmutableId;
             var segs: [max_path_segments][]const u8 = undefined;
             const n = split_path(p.key, &segs) orelse return error.InvalidUpdate;
-            try set_path(arena, pairs, segs[0..n], try bson.copy_value(arena, p.value));
+            try set_path(arena, pairs, segs[0..n], try bson.copy_value(arena, p.value), p.key, diag);
         }
         return;
     }
@@ -125,7 +198,7 @@ fn apply_operator(
             const current = get_value(pairs.items, segs[0..n]) orelse bson.Value{ .int32 = 0 };
             if (!current.is_number() or !p.value.is_number()) return error.InvalidUpdate;
             const sum = try numeric_add(current, p.value);
-            try set_path(arena, pairs, segs[0..n], sum);
+            try set_path(arena, pairs, segs[0..n], sum, p.key, diag);
         }
         return;
     }
@@ -151,12 +224,12 @@ fn apply_operator(
                         else => return error.InvalidUpdate,
                     };
                     for (arr) |item| try items.append(arena, try bson.copy_value(arena, item));
-                    try set_path(arena, pairs, segs[0..n], .{ .array = try items.toOwnedSlice(arena) });
+                    try set_path(arena, pairs, segs[0..n], .{ .array = try items.toOwnedSlice(arena) }, p.key, diag);
                     continue;
                 }
             }
             try items.append(arena, try bson.copy_value(arena, p.value));
-            try set_path(arena, pairs, segs[0..n], .{ .array = try items.toOwnedSlice(arena) });
+            try set_path(arena, pairs, segs[0..n], .{ .array = try items.toOwnedSlice(arena) }, p.key, diag);
         }
         return;
     }
@@ -177,7 +250,7 @@ fn apply_operator(
                     try items.append(arena, elem);
                 }
             }
-            try set_path(arena, pairs, segs[0..n], .{ .array = try items.toOwnedSlice(arena) });
+            try set_path(arena, pairs, segs[0..n], .{ .array = try items.toOwnedSlice(arena) }, p.key, diag);
         }
         return;
     }
@@ -192,7 +265,7 @@ fn apply_operator(
             unset_path(arena, pairs, old_segs[0..old_n]);
             var new_segs: [max_path_segments][]const u8 = undefined;
             const new_n = split_path(p.value.string, &new_segs) orelse return error.InvalidUpdate;
-            try set_path(arena, pairs, new_segs[0..new_n], v);
+            try set_path(arena, pairs, new_segs[0..new_n], v, p.value.string, diag);
         }
         return;
     }
@@ -258,6 +331,8 @@ fn set_path(
     pairs: *std.ArrayListUnmanaged(bson.Pair),
     segs: []const []const u8,
     value: bson.Value,
+    path: []const u8,
+    diag: ?*Diagnostic,
 ) UpdateError!void {
     if (segs.len == 1) {
         if (find_pair(pairs.items, segs[0])) |idx| {
@@ -270,23 +345,31 @@ fn set_path(
     const idx = find_pair(pairs.items, segs[0]) orelse {
         const is_array = parse_index(segs[1]) != null;
         try pairs.append(arena, .{ .key = try arena.dupe(u8, segs[0]), .value = if (is_array) .{ .array = &.{} } else .{ .doc = &.{} } });
-        return set_path(arena, pairs, segs, value);
+        return set_path(arena, pairs, segs, value, path, diag);
     };
     switch (pairs.items[idx].value) {
         .doc => |sub| {
             var sub_pairs = try copy_to_list(bson.Pair, arena, sub);
             defer sub_pairs.deinit(arena);
-            try set_path(arena, &sub_pairs, segs[1..], value);
+            try set_path(arena, &sub_pairs, segs[1..], value, path, diag);
             pairs.items[idx].value = .{ .doc = try sub_pairs.toOwnedSlice(arena) };
         },
         .array => |arr| {
             const index = parse_index(segs[1]) orelse {
-                // treat as non-array: replace with a doc
+                // A non-numeric segment under an array is never a field to
-                var sub_pairs: std.ArrayListUnmanaged(bson.Pair) = .empty;
+                // create. This branch used to read "treat as non-array:
-                defer sub_pairs.deinit(arena);
+                // replace with a doc" and did exactly that -- `y.nope.b`
-                try set_path(arena, &sub_pairs, segs[1..], value);
+                // turned `y: [{b: 3}]` into `y: {nope: {b: 2}}`, discarding
-                pairs.items[idx].value = .{ .doc = try sub_pairs.toOwnedSlice(arena) };
+                // every element and answering ok: 1. mongod refuses with
-                return;
+                // PathNotViable and leaves the document alone.
+                //
+                // The positional spellings took this same branch and are the
+                // reason it was found; they are refused earlier, by
+                // `reject_positional`. What reaches here is the rest of the
+                // class: a plain field name, and a `$`-prefixed one that is
+                // not positional.
+                note(diag, path, segs[1]);
+                return error.PathNotViable;
             };
             var items = try copy_to_list(bson.Value, arena, arr);
             defer items.deinit(arena);
@@ -300,13 +383,13 @@ fn set_path(
                     .doc => |sub| {
                         var sub_pairs = try copy_to_list(bson.Pair, arena, sub);
                         defer sub_pairs.deinit(arena);
-                        try set_path(arena, &sub_pairs, segs[2..], value);
+                        try set_path(arena, &sub_pairs, segs[2..], value, path, diag);
                         items.items[index] = .{ .doc = try sub_pairs.toOwnedSlice(arena) };
                     },
                     else => {
                         var sub_pairs: std.ArrayListUnmanaged(bson.Pair) = .empty;
                         defer sub_pairs.deinit(arena);
-                        try set_path(arena, &sub_pairs, segs[2..], value);
+                        try set_path(arena, &sub_pairs, segs[2..], value, path, diag);
                         items.items[index] = .{ .doc = try sub_pairs.toOwnedSlice(arena) };
                     },
                 }
@@ -316,7 +399,7 @@ fn set_path(
         else => {
             var sub_pairs: std.ArrayListUnmanaged(bson.Pair) = .empty;
             defer sub_pairs.deinit(arena);
-            try set_path(arena, &sub_pairs, segs[1..], value);
+            try set_path(arena, &sub_pairs, segs[1..], value, path, diag);
             pairs.items[idx].value = .{ .doc = try sub_pairs.toOwnedSlice(arena) };
         },
     }
@@ -425,7 +508,7 @@ test "$set, $inc, $unset, $rename" {
         .{ .key = "$inc", .value = .{ .doc = &.{.{ .key = "user.age", .value = .{ .int32 = 2 } }} } },
         .{ .key = "$unset", .value = .{ .doc = &.{.{ .key = "gone", .value = .{ .string = "" } }} } },
         .{ .key = "$rename", .value = .{ .doc = &.{.{ .key = "new", .value = .{ .string = "renamed" } }} } },
-    }));
+    }), null);
 
     const user = bson.get_pair(doc.pairs, "user").?;
     try testing.expectEqualStrings("alice", user.doc[0].value.string);
@@ -445,13 +528,13 @@ test "$push and $pull" {
 
     try apply(&doc, &doc_of(&.{
         .{ .key = "$push", .value = .{ .doc = &.{.{ .key = "tags", .value = .{ .string = "c" } }} } },
-    }));
+    }), null);
     try testing.expectEqual(@as(usize, 3), bson.get_pair(doc.pairs, "tags").?.array.len);
     try testing.expectEqualStrings("c", bson.get_pair(doc.pairs, "tags").?.array[2].string);
 
     try apply(&doc, &doc_of(&.{
         .{ .key = "$pull", .value = .{ .doc = &.{.{ .key = "tags", .value = .{ .string = "b" } }} } },
-    }));
+    }), null);
     const tags = bson.get_pair(doc.pairs, "tags").?.array;
     try testing.expectEqual(@as(usize, 2), tags.len);
     try testing.expectEqualStrings("a", tags[0].string);
@@ -459,7 +542,7 @@ test "$push and $pull" {
 
     try apply(&doc, &doc_of(&.{
         .{ .key = "$push", .value = .{ .doc = &.{.{ .key = "tags", .value = .{ .doc = &.{.{ .key = "$each", .value = .{ .array = &.{ .{ .string = "x" }, .{ .string = "y" } } } }} } }} } },
-    }));
+    }), null);
     try testing.expectEqual(@as(usize, 4), bson.get_pair(doc.pairs, "tags").?.array.len);
 }
 
@@ -475,7 +558,7 @@ test "$set nested creation and _id protection" {
             .{ .key = "a.b.c", .value = .{ .int32 = 42 } },
             .{ .key = "arr.1", .value = .{ .string = "x" } },
         } } },
-    }));
+    }), null);
     const a = bson.get_pair(doc.pairs, "a").?;
     const b = bson.get_pair(a.doc, "b").?;
     try testing.expectEqual(@as(i64, 42), bson.get_pair(b.doc, "c").?.int32);
@@ -485,7 +568,7 @@ test "$set nested creation and _id protection" {
 
     try testing.expectError(error.ImmutableId, apply(&doc, &doc_of(&.{
         .{ .key = "$set", .value = .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 2 } }} } },
-    })));
+    }), null));
 }
 
 test "a replacement keeps _id and drops every other field" {
@@ -502,7 +585,7 @@ test "a replacement keeps _id and drops every other field" {
 
     try apply(&doc, &doc_of(&.{
         .{ .key = "fresh", .value = .{ .int32 = 42 } },
-    }));
+    }), null);
 
     try testing.expectEqual(@as(usize, 2), doc.pairs.len);
     // _id survives, and stays at the front where it is stored.
@@ -523,7 +606,7 @@ test "an empty replacement leaves a document holding only its _id" {
 
     // Legal, and the reason `is_replacement` treats an empty document as one
     // rather than as a no-op set of operators.
-    try apply(&doc, &doc_of(&.{}));
+    try apply(&doc, &doc_of(&.{}), null);
     try testing.expectEqual(@as(usize, 1), doc.pairs.len);
     try testing.expectEqualStrings("_id", doc.pairs[0].key);
 }
@@ -545,7 +628,7 @@ test "a replacement may repeat the _id it is replacing, but not change it" {
     try apply(&doc, &doc_of(&.{
         .{ .key = "_id", .value = .{ .int32 = 7 } },
         .{ .key = "b", .value = .{ .int32 = 2 } },
-    }));
+    }), null);
     try testing.expectEqual(@as(i64, 7), doc.pairs[0].value.int32);
     try testing.expectEqual(@as(i64, 2), bson.get_pair(doc.pairs, "b").?.int32);
     // And only once, not twice.
@@ -554,14 +637,14 @@ test "a replacement may repeat the _id it is replacing, but not change it" {
     // A different _id: refused.
     try testing.expectError(error.ImmutableId, apply(&doc, &doc_of(&.{
         .{ .key = "_id", .value = .{ .int32 = 8 } },
-    })));
+    }), null));
 
     // Equal across numeric types is the same _id, matching the canonical key
     // encoding the _id_ index descends on.
     try apply(&doc, &doc_of(&.{
         .{ .key = "_id", .value = .{ .double = 7.0 } },
         .{ .key = "c", .value = .{ .int32 = 3 } },
-    }));
+    }), null);
     try testing.expectEqual(@as(i64, 3), bson.get_pair(doc.pairs, "c").?.int32);
 }
 
@@ -578,7 +661,7 @@ test "a replacement supplies the _id when the document has none" {
     try apply(&doc, &doc_of(&.{
         .{ .key = "a", .value = .{ .int32 = 1 } },
         .{ .key = "_id", .value = .{ .int32 = 99 } },
-    }));
+    }), null);
 
     try testing.expectEqual(@as(usize, 2), doc.pairs.len);
     try testing.expectEqualStrings("_id", doc.pairs[0].key);
@@ -596,13 +679,13 @@ test "a mixed update document is refused from either side" {
     try testing.expectError(error.InvalidUpdate, apply(&doc, &doc_of(&.{
         .{ .key = "$set", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
         .{ .key = "plain", .value = .{ .int32 = 1 } },
-    })));
+    }), null));
 
     // Starts with data, so it is a replacement and an operator has no meaning.
     try testing.expectError(error.InvalidUpdate, apply(&doc, &doc_of(&.{
         .{ .key = "plain", .value = .{ .int32 = 1 } },
         .{ .key = "$set", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
-    })));
+    }), null));
 }
 
 test "is_replacement decides on the first field only" {
@@ -613,3 +696,153 @@ test "is_replacement decides on the first field only" {
     // replacement path then stores it, which is what MongoDB does with it.
     try testing.expect(is_replacement(&.{.{ .key = "", .value = .{ .int32 = 1 } }}));
 }
+
+/// A document with one array field, rebuilt per case so a refusal can be
+/// checked against untouched bytes.
+fn array_doc(arena: std.mem.Allocator) !bson.Document {
+    var doc = bson.Document{ .arena = std.heap.ArenaAllocator.init(arena), .pairs = &.{} };
+    doc.pairs = try doc.arena.allocator().dupe(bson.Pair, &.{
+        .{ .key = "y", .value = .{ .array = &.{
+            .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 3 } }} },
+            .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 1 } }} },
+        } } },
+    });
+    return doc;
+}
+
+fn expect_y_untouched(doc: *const bson.Document) !void {
+    const y = doc.get("y") orelse return error.TestUnexpectedResult;
+    // The array is still an array. Before this refusal existed it was a
+    // document keyed by the path segment's literal text, and everything in it
+    // was gone.
+    try testing.expect(y == .array);
+    try testing.expectEqual(@as(usize, 2), y.array.len);
+    try testing.expectEqual(@as(i32, 3), y.array[0].doc[0].value.int32);
+    try testing.expectEqual(@as(i32, 1), y.array[1].doc[0].value.int32);
+}
+
+test "a positional path is refused and the array survives" {
+    // The load-bearing test of the refusal. Each of these used to answer
+    // success having replaced `y` with `{"<segment>": ...}` -- the array and
+    // both its elements discarded, `ok: 1`, `modifiedCount: 1`.
+    //
+    // Mutation check: delete the `reject_positional` call in `apply` and every
+    // case here goes red on `expect_y_untouched`, not on the error.
+    const cases = [_][]const u8{
+        "y.$[i].b", // filtered positional
+        "y.$[].b", // all-positional
+        "y.$.b", // positional
+        "y.$[i]", // as the leaf
+        "$", // in first position
+        "y.$[i].c.$[j].d", // nested, two identifiers
+    };
+    for (cases) |path| {
+        var doc = try array_doc(testing.allocator);
+        defer doc.arena.deinit();
+        var diag: Diagnostic = .{};
+        try testing.expectError(error.PositionalUnsupported, apply(&doc, &doc_of(&.{
+            .{ .key = "$set", .value = .{ .doc = &.{.{ .key = path, .value = .{ .int32 = 2 } }} } },
+        }), &diag));
+        try testing.expectEqualStrings(path, diag.path);
+        try expect_y_untouched(&doc);
+    }
+}
+
+test "every operator refuses a positional path, not just $set" {
+    // The destruction was below the operator, in the shared path walk: `$inc`
+    // through `$[i]` stored its operand instead of incrementing. So the
+    // refusal has to be below the operator too.
+    const ops = [_]struct { op: []const u8, value: bson.Value }{
+        .{ .op = "$set", .value = .{ .int32 = 2 } },
+        .{ .op = "$inc", .value = .{ .int32 = 10 } },
+        .{ .op = "$unset", .value = .{ .string = "" } },
+        .{ .op = "$push", .value = .{ .int32 = 1 } },
+        .{ .op = "$pull", .value = .{ .int32 = 1 } },
+    };
+    for (ops) |o| {
+        var doc = try array_doc(testing.allocator);
+        defer doc.arena.deinit();
+        try testing.expectError(error.PositionalUnsupported, apply(&doc, &doc_of(&.{
+            .{ .key = o.op, .value = .{ .doc = &.{.{ .key = "y.$[i].b", .value = o.value }} } },
+        }), null));
+        try expect_y_untouched(&doc);
+    }
+}
+
+test "$rename checks its destination, which is the value not the key" {
+    var doc = try array_doc(testing.allocator);
+    defer doc.arena.deinit();
+    var diag: Diagnostic = .{};
+    try testing.expectError(error.PositionalUnsupported, apply(&doc, &doc_of(&.{
+        .{ .key = "$rename", .value = .{ .doc = &.{
+            .{ .key = "y", .value = .{ .string = "z.$[i]" } },
+        } } },
+    }), &diag));
+    try testing.expectEqualStrings("z.$[i]", diag.path);
+    try expect_y_untouched(&doc);
+}
+
+test "nothing in the update is applied when one of its paths is refused" {
+    // The refusal is taken before the first write, so the good path in this
+    // update does not land either. Mutation check: move `reject_positional`
+    // inside the operator loop and `ok` appears on the document.
+    var doc = try array_doc(testing.allocator);
+    defer doc.arena.deinit();
+    try testing.expectError(error.PositionalUnsupported, apply(&doc, &doc_of(&.{
+        .{ .key = "$set", .value = .{ .doc = &.{
+            .{ .key = "ok", .value = .{ .int32 = 1 } },
+            .{ .key = "y.$[i].b", .value = .{ .int32 = 2 } },
+        } } },
+    }), null));
+    try testing.expect(doc.get("ok") == null);
+    try expect_y_untouched(&doc);
+}
+
+test "a non-numeric segment under an array is PathNotViable, not a new field" {
+    // The rest of the class the positional forms belonged to. `y.nope.b` is
+    // not a positional operator and is refused for a different reason with a
+    // different code -- measured on mongod 8.3.7, which answers 28 here and 2
+    // for the positional forms.
+    const cases = [_][]const u8{ "y.nope.b", "y.nope", "y.$x.b" };
+    for (cases) |path| {
+        var doc = try array_doc(testing.allocator);
+        defer doc.arena.deinit();
+        var diag: Diagnostic = .{};
+        try testing.expectError(error.PathNotViable, apply(&doc, &doc_of(&.{
+            .{ .key = "$set", .value = .{ .doc = &.{.{ .key = path, .value = .{ .int32 = 2 } }} } },
+        }), &diag));
+        try expect_y_untouched(&doc);
+    }
+}
+
+test "a numeric segment still addresses an array element" {
+    // The regression guard for the refusal above: indexed paths are the one
+    // way into an array that does work, and they must keep working, including
+    // the null padding past the end.
+    var doc = try array_doc(testing.allocator);
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$set", .value = .{ .doc = &.{
+            .{ .key = "y.0.b", .value = .{ .int32 = 7 } },
+            .{ .key = "y.3.b", .value = .{ .int32 = 8 } },
+        } } },
+    }), null);
+    const y = doc.get("y").?;
+    try testing.expectEqual(@as(usize, 4), y.array.len);
+    try testing.expectEqual(@as(i32, 7), y.array[0].doc[0].value.int32);
+    try testing.expect(y.array[2] == .null);
+    try testing.expectEqual(@as(i32, 8), y.array[3].doc[0].value.int32);
+}
+
+test "a replacement is not a path, so it is not refused" {
+    // The one case out of seventeen where this server already agreed with
+    // mongod: `arrayFilters` alongside a replacement is ignored by both, and
+    // a replacement field named like a path is data, not a path.
+    var doc = try array_doc(testing.allocator);
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "z", .value = .{ .int32 = 1 } },
+    }), null);
+    try testing.expect(doc.get("y") == null);
+    try testing.expectEqual(@as(i32, 1), doc.get("z").?.int32);
+}

tests/spec/scorecard.txt

@@ -209,9 +209,9 @@ aggregate-rawdata.json	SKIP	Aggregate with rawData option	needs server >= 8.2.0
 aggregate-write-readPreference.json	SKIP	*	needs topology replicaset/sharded/load-balanced
 aggregate.json	SKIP	aggregate with a document comment - pre 4.4	needs server <= 4.2.99
 aggregate.json	SKIP	aggregate with comment does not set comment on getMore - pre 4.4	needs server <= 4.3.99
-bulkWrite-arrayFilters.json	FAIL	BulkWrite updateOne with arrayFilters	outcome crud-tests.test[0].y: expected an array, got {"$[i]":{"b":2}}
+bulkWrite-arrayFilters.json	FAIL	BulkWrite updateOne with arrayFilters	MongoBulkWriteError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-bulkWrite-arrayFilters.json	FAIL	BulkWrite updateMany with arrayFilters	outcome crud-tests.test[0].y: expected an array, got {"$[i]":{"b":2}}
+bulkWrite-arrayFilters.json	FAIL	BulkWrite updateMany with arrayFilters	MongoBulkWriteError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-bulkWrite-arrayFilters.json	FAIL	BulkWrite with arrayFilters	bulkWrite.modifiedCount: expected 3, got 2
+bulkWrite-arrayFilters.json	FAIL	BulkWrite with arrayFilters	MongoBulkWriteError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
 bulkWrite-collation.json	FAIL	BulkWrite with delete operations and collation	bulkWrite.deletedCount: expected 4, got 0
 bulkWrite-collation.json	FAIL	BulkWrite with update operations and collation	bulkWrite.matchedCount: expected 6, got 2
 bulkWrite-comment.json	SKIP	BulkWrite with comment - pre 4.4	needs server <= 4.2.99
@@ -361,9 +361,9 @@ findOneAndReplace-upsert.json	FAIL	FindOneAndReplace when no documents match wit
 findOneAndReplace-upsert.json	FAIL	FindOneAndReplace when no documents match with id specified with upsert returning the document after modification	findOneAndReplace: expected a document, got null
 findOneAndReplace.json	FAIL	FindOneAndReplace when many documents match returning the document after modification	findOneAndReplace.x: expected 32, got 22
 findOneAndReplace.json	FAIL	FindOneAndReplace when one document matches returning the document after modification	findOneAndReplace.x: expected 32, got 22
-findOneAndUpdate-arrayFilters.json	FAIL	FindOneAndUpdate when no document matches arrayFilters	outcome crud-v1.coll[0].y: expected an array, got {"$[i]":{"b":2}}
+findOneAndUpdate-arrayFilters.json	FAIL	FindOneAndUpdate when no document matches arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-findOneAndUpdate-arrayFilters.json	FAIL	FindOneAndUpdate when one document matches arrayFilters	outcome crud-v1.coll[0].y: expected an array, got {"$[i]":{"b":2}}
+findOneAndUpdate-arrayFilters.json	FAIL	FindOneAndUpdate when one document matches arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-findOneAndUpdate-arrayFilters.json	FAIL	FindOneAndUpdate when multiple documents match arrayFilters	outcome crud-v1.coll[0].y: expected an array, got {"$[i]":{"b":2}}
+findOneAndUpdate-arrayFilters.json	FAIL	FindOneAndUpdate when multiple documents match arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
 findOneAndUpdate-collation.json	FAIL	FindOneAndUpdate when many documents match with collation returning the document before modification	findOneAndUpdate: expected a document, got null
 findOneAndUpdate-comment.json	FAIL	findOneAndUpdate with string comment	MongoServerError: update must be a document
 findOneAndUpdate-comment.json	FAIL	findOneAndUpdate with document comment	MongoServerError: update must be a document
@@ -403,9 +403,9 @@ replaceOne-let.json	SKIP	ReplaceOne with let option	needs server >= 5.0
 replaceOne-let.json	FAIL	ReplaceOne with let option unsupported (server-side error)	replaceOne: expected an error, the operation succeeded
 replaceOne-rawdata.json	SKIP	ReplaceOne with rawData option	needs server >= 8.2.0
 replaceOne-sort.json	SKIP	ReplaceOne with sort option	needs server >= 8.0
-updateMany-arrayFilters.json	FAIL	UpdateMany when no documents match arrayFilters	updateMany.modifiedCount: expected 0, got 2
+updateMany-arrayFilters.json	FAIL	UpdateMany when no documents match arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-updateMany-arrayFilters.json	FAIL	UpdateMany when one document matches arrayFilters	updateMany.modifiedCount: expected 1, got 2
+updateMany-arrayFilters.json	FAIL	UpdateMany when one document matches arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-updateMany-arrayFilters.json	FAIL	UpdateMany when multiple documents match arrayFilters	outcome crud-v1.coll[0].y: expected an array, got {"$[i]":{"b":2}}
+updateMany-arrayFilters.json	FAIL	UpdateMany when multiple documents match arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
 updateMany-collation.json	FAIL	UpdateMany when many documents match with collation	updateMany.matchedCount: expected 2, got 1
 updateMany-comment.json	SKIP	UpdateMany with comment - pre 4.4	needs server <= 4.2.99
 updateMany-dots_and_dollars.json	SKIP	Updating document to set top-level dollar-prefixed key on 5.0+ server	needs server >= 5.0
@@ -416,11 +416,11 @@ updateMany-let.json	SKIP	updateMany with let option	needs server >= 5.0
 updateMany-let.json	FAIL	updateMany with let option unsupported (server-side error)	updateMany: error message "update spec requires u" does not contain "'update.let' is an unknown field"
 updateMany-pipeline.json	FAIL	UpdateMany using pipelines	MongoServerError: update spec requires u
 updateMany-rawdata.json	SKIP	updateMany with rawData option	needs server >= 8.2.0
-updateOne-arrayFilters.json	FAIL	UpdateOne when no document matches arrayFilters	updateOne.modifiedCount: expected 0, got 1
+updateOne-arrayFilters.json	FAIL	UpdateOne when no document matches arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-updateOne-arrayFilters.json	FAIL	UpdateOne when one document matches arrayFilters	outcome crud-v1.coll[0].y: expected an array, got {"$[i]":{"b":2}}
+updateOne-arrayFilters.json	FAIL	UpdateOne when one document matches arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-updateOne-arrayFilters.json	FAIL	UpdateOne when multiple documents match arrayFilters	outcome crud-v1.coll[0].y: expected an array, got {"$[i]":{"b":2}}
+updateOne-arrayFilters.json	FAIL	UpdateOne when multiple documents match arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].b' is not implemented by this server
-updateOne-arrayFilters.json	FAIL	UpdateOne when no documents match multiple arrayFilters	updateOne.modifiedCount: expected 0, got 1
+updateOne-arrayFilters.json	FAIL	UpdateOne when no documents match multiple arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].c.$[j].d' is not implemented by this server
-updateOne-arrayFilters.json	FAIL	UpdateOne when one document matches multiple arrayFilters	outcome crud-v1.coll[2].y: expected an array, got {"$[i]":{"c":{"$[j]":{"d":0}}}}
+updateOne-arrayFilters.json	FAIL	UpdateOne when one document matches multiple arrayFilters	MongoServerError: the positional operator '$[i]' in path 'y.$[i].c.$[j].d' is not implemented by this server
 updateOne-collation.json	FAIL	UpdateOne when one document matches with collation	updateOne.matchedCount: expected 1, got 0
 updateOne-comment.json	SKIP	UpdateOne with comment - pre 4.4	needs server <= 4.2.99
 updateOne-dots_and_dollars.json	SKIP	Updating document to set top-level dollar-prefixed key on 5.0+ server	needs server >= 5.0