fix(metadata-fs): confirm absence on disk before publishing an external delete

[claude]

Fixes#7369

The fork this card set, and which side the code decided

The reopened card gave two branches and said the dev must decide by reading the watcher's event-normalization code, not by picking the cheaper edit:

• if the watcher does not promise that an external in-place edit is never surfaced as delete, fix the test;
• if it does, today's queue log is a real watcher defect and the fix belongs in metadata-fs, with the test left strict.

It promises it, so this is the watcher fix. watch-dot-root.test.ts is not touched.

The exact code that decided it

handleFsChange's removal face published on the observer's word alone:

if(kind==='unlink'){awaitthis.publishExternalDelete(ref,key);return;}

The only suppression inside publishExternalDelete is !currentHead — a comparison against the index, which is exactly what a spurious unlink leaves intact. It answers "is this removal ours"; nothing answered "did a removal happen".

The repository already held the opposite discipline one method away. The reconciliation sweep's delete pass re-checks disk truth under the same per-key lock before retiring a key, and says why in place:

awaitthis.mutex.run(key,async()=>{// Re-checked UNDER the lock. The enumeration above ran outside it, so// a `put()` that created this file in between would otherwise be// reported as an external delete.if(existsSync(file))return;awaitthis.publishExternalDelete(ref,key);});

and test/external-write-resync.test.ts pins that contract for the sweep — when a false absence is injected, expect(events.filter((e) => e.op === 'delete')).toHaveLength(0), with the comment "every item of that type would otherwise be retired as an external delete". The watcher face was the one path in the package that skipped the check.

Why the queue log is that defect and not a slow runner

From the job log itself (run 33057527457, Test Core (2/6)): the failure is at watch-dot-root.test.ts:268, so the exact-count assertion on line 267 passed. Exactly one event arrived, and it was typed delete, for a file the case creates with put() and then rewrites in place with fs.writeFile and never removes. The file ran in 2511ms, so this was not a late delivery — the phantom removal arrived promptly. A delete published there means the repository retired an item that was on disk.

Why chokidar's unlink is not evidence of removal

chokidar reaches its removal path from failed stats as well as from real removals, and says so at both sites (chokidar 5 handler.js):

• _handleFile's poll listener runs when fs.watchFile reports a zeroed stat, re-stats the file, and calls _remove from the catch — under the comment "Fix issues where mtime is null but file is still present" — with no discrimination on errno, so EMFILE/ENFILE retires a file that is there;
• _handleRead's snapshot diff _removes every previously tracked entry its readdirp pass did not enumerate, which includes entries whose per-entry lstat failed rather than only the ones that are gone.

Both faults are load-shaped, which is why this surfaces in the merge queue and nowhere else: the queue is the only context that runs the FULL suite, and PR-side CI runs the affected subset.

What the bug cost, beyond one red test

A delete is not a droppable notification. It is appended to the JSONL change log and broadcast to every subscriber, and MetadataManager drops the item from the registry and the list() cache on receipt. The sweep then found the file still on disk and republished it as a create. So a failed stat produced a durable delete/create pair for an item nobody removed, with a window in between where live metadata had disappeared — in the shipped .objectstack/metadata layout, not only under test.

The change

One guard in handleFsChange, plus the doc block that now states the contract:

if(kind==='unlink'&&!existsSync(absPath)){awaitthis.publishExternalDelete(ref,key);return;}

Falling through is the repair, not just a skip: when the path is still there the honest reading of the event is "something happened to this file", which is the content path's question. It answers with the same currentHead === hash comparison used everywhere else, so a spurious unlink that arrived alongside a real in-place edit surfaces as the update it always was, in the same tick, instead of the delete + create pair the index-only check produced. Genuine external removals are unaffected — for those the file really is gone, so they are still published on the first delivery — and delete()'s own unlink is still suppressed by !currentHead.

The pin

test/external-delete-requires-absence.test.ts, three cases, entered at the seam immediately below chokidar — the discipline self-write-suppression.test.ts set in this package for this exact reason ("this package has been ejected from the merge queue twice already by wall-clock watcher assertions"). The upstream trigger is a failed stat under resource pressure, which cannot be summoned on demand and would be a wall-clock race to wait for; the boundary condition all of its causes share is "chokidar calls back with unlink for a path that is still there", and that is what the cases deliver. This mirrors how external-write-resync.test.ts reproduces its own family (detach the listeners) rather than forcing one named gate.

Both directions are asserted, because each alone has a trivial wrong fix: a repository that published nothing on unlink would pass the first two cases and fail the third.

1. an unlink for a path that still exists, edited externally → one update, identity intact, item still readable;
2. a purely spurious unlink, nothing changed on disk → no event at all, and the change log still reads ['create'];
3. a genuine external removal → still one delete, hash: null, parentHash = the old head.

Verification

Head of this branch: 6ff0c6b0. Every reading below is from that tree.

Reverse verification — packages/metadata-fs/src/repository.ts restored from origin/main (the fix committed first, so the restore leg has a real reference point), mutation confirmed on disk by counting both anchors before measuring (fix guard 0 occurrences, pre-fix branch 1), restore proved by blob hash against the HEAD blob plus an empty git diff HEAD. No rebuild is owed on either leg: the tests resolve the subject from source (../src/index.js, transformed by vitest), and the only dist in play is @objectstack/metadata-core's, which the mutation does not touch.

Predicted direction: cases 1 and 2 red, case 3 green. Measured, pre-fix:

FAIL test/external-delete-requires-absence.test.ts > ... > an unlink for a path that still exists, edited externally, is the update it always was
AssertionError: expected 'delete' to be 'update' // Object.is equality
FAIL test/external-delete-requires-absence.test.ts > ... > a purely spurious unlink — nothing on disk changed — publishes nothing at all
AssertionError: expected [ { seq: 2, op: 'delete', …(6) } ] to deeply equal []
Tests 2 failed | 1 passed (3)

The first line is byte-identical to the assertion that ejected PR #12684 from the merge queue.

Suite — pnpm --filter @objectstack/metadata-fs test → Test Files 9 passed (9), Tests 68 passed (68) (CI on the failing run had 8 files / 65 tests; this adds one file and three cases).

Typecheck — pnpm --filter @objectstack/metadata-fs typecheck (tsc --noEmit && tsc --noEmit -p tsconfig.test.json) clean. Confirmed the new file is genuinely in the test program rather than silently excluded: tsc -p tsconfig.test.json --listFiles lists it (1 occurrence, not 0).

Repetition under load — 20 iterations of watch-dot-root.test.ts + the new pin, with 4 CPU hogs on a 4-core box (100% oversubscription): 20 passed / 0 failed, Tests 5 passed (5) in every iteration, zero AssertionErrors.

Gates — the family derived by scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the real change set (23 runs, exit codes captured before any pipe), all green. Their own verdict lines include check-nul-bytes: OK (scanned 7045 text file(s) ...; no raw ASCII control bytes), check-cross-package-test-inputs — OK: 20 package(s) read outside themselves, all declared, check-engine-double-contract: OK — 689 pinned, 134 in the DEBT ledger, 3 exempt, check-test-source-alias OK — 72 packages with tests scanned.

What I did not establish

I did not observe a spurious chokidar unlink end-to-end on this container, and I am not claiming to have reproduced the queue's proximate trigger. I tried: a scratch probe drove case 1's exact shape against the real watcher with a tap counting every raw unlink for the item path, classifying each by whether the file still existed at that instant. 24 iterations with the process pushed to its file-descriptor ceiling, then 24 more with the same fd pressure plus 6 CPU hogs on 4 cores — both runs: spurious_unlink=0 real_unlink=0 delete_events=0 update_events=24. The probe was scratch and is not in this branch.

So the mechanism above is read from chokidar's source and from what the job log proves must have happened (one event, typed delete, for a file that existed) — not from a local reproduction. Which of the two chokidar branches fired on the runner is unknown, and per the reasoning external-write-resync.test.ts already records for its own family, that is precisely the detail a pin should not depend on: the fix and its cases are keyed to the boundary condition, so they hold for either branch and for a third nobody has found.

Scope

One package. packages/spec untouched; #12684 remains open and is unrelated to this branch. content/docs/releases/ untouched. A changeset is included because published runtime code changed.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-fs, touching 2 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/concepts/metadata-lifecycle.mdx(via FileSystemRepository (symbol))

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 284fc22d8834f2c9a3530592d6614e0eb6a28590 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 19e2fcf1a5d22ad57e443a69543ce313e00ef437 — the merge of head 6ff0c6b0526210dcf3694a8c5256f9b3ae429af6 into base 284fc22d8834f2c9a3530592d6614e0eb6a28590, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 19e2fcf1a5d22ad57e443a69543ce313e00ef437 && git checkout 19e2fcf1a5d22ad57e443a69543ce313e00ef437
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 284fc22d8834f2c9a3530592d6614e0eb6a28590 6ff0c6b0526210dcf3694a8c5256f9b3ae429af6 && git checkout -B drift-repro 284fc22d8834f2c9a3530592d6614e0eb6a28590 && git merge --no-ff 6ff0c6b0526210dcf3694a8c5256f9b3ae429af6
node scripts/docs-audit/affected-docs.mjs --json 284fc22d8834f2c9a3530592d6614e0eb6a28590

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 284fc22d8834f2c9a3530592d6614e0eb6a28590 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-zhuang]

PM verification record (dispatching seat for #7369; this seat shares the PR's author identity, so this is a COMMENT review — packages/metadata-fs is not a governed surface and the queue does not require an approval here).

Read the full diff. The dispatch asked the dev to decide a fork by reading code — watcher contract vs. test tolerance — and the evidence for the branch taken is decisive:

• This was never test flakiness; it was a product defect the queue's full-suite load surfaced twice. The removal face published a delete on chokidar's word alone, and chokidar reaches its removal path from failed stats (both cited sites in chokidar 5 handler.js). The blast radius was durable: a phantom delete appended to the change log and broadcast, registry/list() eviction, then a sweep-republished create — a permanent delete/create pair for a file nobody removed. Exactly the silent-corruption shape the repo's loud-failure discipline exists to catch.
• The fix imports the discipline the reconciliation sweep already had (absence confirmed on disk under the same per-key lock) and falls through so a spurious unlink riding a real edit surfaces as the update it always was. watch-dot-root.test.ts untouched, as the reopened card demanded.
• The new pin asserts both directions at the handler seam — spurious-unlink-with-edit ⇒ update; pure spurious unlink ⇒ nothing, history stays clean; genuine removal ⇒ delete on first delivery — which forecloses both trivial wrong fixes (ignore-all-unlinks and status quo). No wall-clock races introduced.
• Reverse verification is the strongest in this batch: pre-fix code restored by blob hash, predicted failure set measured exactly, and the failing assertion byte-identical to queue run 33057527457's — the defect, the log, and the fix agree.
• Docs-drift's flagged page (content/docs/concepts/metadata-lifecycle.mdx) re-read against this diff: it documents the event vocabulary and SSE bridge, makes no claim this change falsifies. No doc edit owed.
• Finding #12696 (chokidar atomic: true via dead-code default — an unchosen behavior in the delivery path) correctly left out of this PR and filed for triage.

Landing via the normal queue. This also removes the standing cause of the two queue ejections recorded on #7369.

---

Generated by Claude Code