fix(metadata-fs): suppress self-writes by observed content, not by a 200ms clock (#7335)

[huangyiirene]

Fixes #7335

The defect

handleFsChange opened with if (this.selfWrites.has(absPath)) return; — a Set that put()/delete() added the path to and a setTimeout(…, 200) cleared. It discarded every event for a recently-written path without ever reading what the watcher had observed.

Under usePolling: true, interval: 1000 chokidar compares state once per tick, so our own write and an external edit landing between two ticks are delivered as a single event carrying the external content. Dropped on the timer — and it was the only event that edit would ever produce.

The card said "measure first". Here is the measurement.

The dispatch left two things open. Both are now answered, and the first answer contradicts the obvious fix.

(a) or (b): does the pre-check carry real load?

(a) — it does not. Delete it. The content-keyed suppression the filer spotted one step down already covers both faces:

face	covering check	verified
put() → add/change	currentHead === hash	head set in the same continuation as the rename; awaitWriteFinish holds the event a further stabilityThreshold
delete() → unlink	!currentHead	head null at delete-resolve and at event delivery (measured)

Instrumented handleFsChange on origin/main @ 69fde55:

• 6 put/put/delete cycles → 6 unlink events, 0 hit the pre-check, all 6 correctly suppressed downstream.
• 25 self-write change events → 0 inside the window (delivery lag 519–585 ms).
• 40 randomised-phase runs → the pre-check fired 7 times, and all 7 were data loss.

The pre-check was never once observed suppressing a genuine self-write. Every observed firing was a swallowed external edit — so its only distinguishable effect was the bug.

The window is not "derived" any more — it is observed

The filing recorded 0/360 and called it structural. It was structural, but the structure was in the harness, not the defect. Delivery lag is (interval - (writeTime mod interval)) + awaitWriteFinish, so a fixed pre-edit sleep phase-locks the poll and pins the lag outside the window. Randomising the sleep so the lag samples [0, interval) uniformly, 40 runs on 69fde55:

delivery lag	runs	external edit
< 200 ms	7	SWALLOWED
> 200 ms	33	delivered

Zero exceptions on either side of the boundary. The nearest miss was 202 ms — delivered.

The fix

1. Remove the time-keyed pre-check and the selfWrites Set with it.
2. delete() retires the head before it unlinks, not after. awaitWriteFinish debounces only add/change; chokidar emits unlink with no stability delay, so that face has no cushion between the disk mutation and its event. Ordering the index update first makes !currentHead a total suppression rather than a race against the poll callback. A failed unlink restores the head before rethrowing, so the error path is byte-for-byte the old semantics.

Verification record

Reverse-verified, direction predicted before running. With the old behaviour restored on the same tree:

• put(): an external change coalesced into our own write is NOT swallowed → FAIL (expected [] to have a length of 1 — nothing published)
• delete(): …NOT swallowed → FAIL (same)
• both …undisturbed, is still not republished → PASS (the old pre-check suppresses those too)

Exactly as predicted. The undisturbed cases passing in both directions is the point: they are what stops "publish everything" from being a passing fix.

End-to-end, same randomised harness, after the fix: 40/40 delivered, 0 swallowed (pre-fix: 33/40, 7 swallowed). Five post-fix runs landed at 76, 142, 143, 147, 193 ms — inside the old window, where every pre-fix swallow happened — and all five delivered the edit. The window was genuinely sampled, not merely avoided.

Gates

gate	result
packages/metadata-fs suite	✅ 6 files, 35 tests
packages/metadata (downstream consumer)	✅ 31 files, 603 tests
build closure (turbo --filter=...@objectstack/metadata-fs)	✅ 67/67
eslint on changed files	✅ clean
check:nul-bytes	✅ 7167 files

CI owns the full lint.yml farm.

Card clauses

1. The unlink path has no content to hash. Answered structurally, not by hashing: for a removal the absence is the content, and !currentHead is the identity comparison. Measured — head is null both when delete() resolves and when the unlink event is delivered. The reordering above is what makes that total rather than probabilistic, and the delete face is pinned with its own external-interference case (an external actor recreating the path inside the window — the symmetric data loss).

2. Specs that do not round-trip through JSON. Real, and entirely pre-existing — measured, not assumed:

• hashSpec canonicalises a Date to {} in memory, but JSON.stringify writes an ISO string, so the disk hash differs.
• Such a spec already fails put().version === get().hash today with no watcher involved (measured: 2978612c vs e5abe659).
• The 200 ms window never covered it anyway: on the pre-fix tree, a Date-bearing second put() already publishes a spurious {op: update, actor: 'fs'} and already corrupts the head index — because the event arrives ~560 ms in, roughly 360 ms after the window expired.

So this class cannot regress: the pre-check was not holding it. Documented in handleFsChange rather than silently changed. Worth its own card if anyone wants put() to reject or normalise non-round-tripping specs.

3. Shared consistency coverage. test/self-write-suppression.test.ts is table-driven over a FACES array (put, delete), each contributing both directions. A future change that fixes one face and regresses the other fails the table.

4. Same-day churn. Anchored on symbol names against origin/main @ 69fde55 (includes ab07b5382 / #7282 and #7150 via #7208), not against the card's line numbers. trackWrittenPath (#7282) and isIgnoredWatchPath (#7150) are untouched — three distinct mechanisms in one file, and only the suppression moves.

Why the assertions sit at the handler seam

The end-to-end reproduction is ~17% per iteration and costs ~100 s for 40 runs, because the thing being sampled is a poll phase. Committing it would buy a probabilistic test for a property that can be stated exactly — and this package has been ejected from the merge queue twice on wall-clock watcher assertions (#7208, #7255). So the cases drive handleFsChange directly, entered with exactly the arguments a sub-200 ms delivery produces, and assert the contract: an event whose observed content differs from the index must be published, however recently we wrote that path. The probabilistic harness is the evidence above, run in both directions; the committed pin is deterministic. Same split, and same reasoning, as watch-write-registration.test.ts.

Repricing #7408

Completely unaffected — and already closed. #7408 is a negative-assertion soundness problem in watch-dot-root.test.ts case 2 (a 4 s quiet window shorter than measured delivery latency, so an empty array proves nothing under load). It was fixed by PR #7472 on 2026-08-11 and the issue is closed. Different file, different direction of assertion, no shared mechanism: this change touches neither watch-dot-root.test.ts nor the ignore matcher it guards, and adds no new negative-by-timeout assertion — the undisturbed cases assert an unchanged change log after a synchronously delivered event, so they have no quiet window to be too short.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 11, 2026 8:32pm

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-fs.

1 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-fs)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.