Skip to content

Commit bd2fc8b

Browse files
os-samclaude
andauthored
fix(metadata-fs): give an external write more than one delivery attempt — content-keyed reconciliation behind the watcher poll (#9339) (#9656)
* fix(metadata-fs): give an external write more than one delivery attempt — content-keyed reconciliation behind the watcher poll (#9339) The watcher gave an externally-written file exactly ONE chance to be noticed, and losing it was permanent and silent. Under `usePolling`, chokidar re-reads a directory only when its stat strictly advances; an external write advances the type directory's mtime once, so every later poll compares an unchanged stat and can never rediscover the file. At least six independent one-shot gates sit on that single attempt, spanning three layers, and all six produce a byte-identical observable — which is why #7282 was closed on one member of the family and reopened as #9339. The fix never asks which gate fired. A bounded, content-keyed reconciliation sweep compares what is on disk against `heads` and publishes the divergence through the same handler the watcher feeds, so it is robust across all six by construction. `put()`'s direct registration (#7336) and every production watcher constant are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F * fix(metadata-fs): the reconciliation sweep discriminates its read failures instead of inventing an empty listing (#8895) Both `readdir` seams in `resync()` swallowed every errno and answered with "there is nothing here". ENOENT is the one truthful empty answer — a path that does not exist holds no items — and it stays silent. Every other code means the read could not RUN: EACCES, EIO, and above all EMFILE/ENFILE, which degrade this read and chokidar's own `fs.watchFile` polling at the same time and for the same reason. Silence there made the backstop absent for the life of the process exactly under the load-dependent conditions it exists to catch. Reported at `error` per AGENTS.md's judgement question — the system keeps looking healthy while its index drifts from disk — naming the consequence and the fix, and latched per path+errno so a standing fault is said once rather than every 2s. It deliberately does not throw: this runs on a background timer. An unreadable type directory is also excluded from the delete pass, which would otherwise read "could not look" as "the files are gone" and retire every item of that type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F * docs(metadata-fs): put the sweep's rationale back above resync() rather than stacked on reportResyncFault (#9339) Comment-only. The read-seam commit inserted the new helpers above resync() and left its doc block attached to the first of them, so two doc comments stacked on reportResyncFault and the sweep itself read undocumented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 440439f commit bd2fc8b

3 files changed

Lines changed: 658 additions & 16 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/metadata-fs": patch
3+
---
4+
5+
fix(metadata-fs): an external write reaches subscribers even when the watcher's single delivery attempt is lost — content-keyed reconciliation behind the poll (#9339)
6+
7+
`FileSystemRepository`'s watcher gave an externally-written file **exactly one**
8+
chance to be noticed, and losing it was permanent and silent. Under
9+
`usePolling`, chokidar re-reads a directory only when its stat *strictly*
10+
advances; an external write advances the type directory's mtime once, so poll
11+
#2..#N compare an unchanged stat and can never rediscover the file. Measured on
12+
#9339 with a fault-injection harness: with that single read suppressed, fifteen
13+
further poll ticks never find the file — a 20s deadline and a 200s deadline buy
14+
the same one attempt. That is the structural reason behind #7282's empirical
15+
finding that the event is *"never delivered, not slow"*, and why widening the
16+
deadline (#7208) and lowering `interval` were both spent before they were tried.
17+
18+
**At least six independent one-shot gates sit on that attempt**, spanning three
19+
layers — the kernel timestamp (the directory mtime does not strictly advance),
20+
chokidar's readdir throttle and readdir snapshot, and chokidar's emit gates
21+
(`_throttle('add')`, a stale `_pendingWrites` entry, the `awaitWriteFinish`
22+
ENOENT early return). Each produces a byte-identical observable: no event, ever,
23+
for that path. They are indistinguishable at the point of failure, which is why
24+
#7282's close — picked from that family — covered one member and reopened.
25+
26+
**The fix does not name a member.** A bounded, content-keyed reconciliation
27+
sweep runs alongside the watcher and compares what is on disk against `heads`,
28+
the index that already defines what the repository believes it holds, publishing
29+
any divergence through the *same* handler the watcher feeds. Its only premise is
30+
that the bytes on disk stopped matching the index, so it is robust across all six
31+
by construction — and equally across a seventh nobody has found.
32+
33+
- **Cadence** — one pass over `<root>/<type>/*.json` every 2s (twice the poll
34+
interval), the same walk `start()` already performs once. Sweeps are chained
35+
rather than intervalled, so they can never overlap or stack behind a slow
36+
disk; the timer is `unref`ed and is retired by `close()`; and it is armed only
37+
alongside the watcher, so a `disableWatch` repository pays nothing.
38+
- **Exactly-once is preserved.** Suppression stays content-keyed (`#7335`): the
39+
sweep republishes nothing the watcher already delivered, and recognises this
40+
repository's own `put()` by content rather than by a clock.
41+
- **Events are indistinguishable from the fast path** — same `op`,
42+
`parentHash`, `source: 'fs'` and actor, because they are produced by the same
43+
code. A subscriber cannot be made to care which path noticed.
44+
- **A recovered path is re-armed** with the watcher through the seam `put()`
45+
already uses, so a loss upstream of chokidar's `_handleFile` does not leave
46+
the file dependent on the sweep forever.
47+
- `put()`'s existing direct registration (#7336) is unchanged, as are
48+
`usePolling`, `interval`, and `awaitWriteFinish`.
49+
50+
⚠️ **Bound on the claim.** The six gates are *forced fault injections*, not the
51+
CI mechanism, which was never identified and may be a seventh. What is measured
52+
is that the fix converts **six of six** forced one-shot gates from permanent
53+
loss to delivery (3/3 runs each), where all six returned an empty event list
54+
before it. That is not the same statement as "the flake is fixed".

0 commit comments

Comments
 (0)