Skip to content

metadata-fs: selfWrites suppression is time-keyed, so a poll tick landing inside the 200 ms window can swallow an external edit #7335

Description

@os-zhuang

Found while working #7282. Unassigned — recording, not claiming. This is a
different mechanism from #7282 and was deliberately not folded into PR for
#7282, so that PR's reverse verification stays attributable to one change.

What it is

FileSystemRepository.put() / .delete() suppress the chokidar event for a
path they just wrote by adding the path to a Set and clearing it on a fixed
wall-clock timer (packages/metadata-fs/src/repository.ts, selfWrites and the
two setTimeout(..., 200) calls). handleFsChange then drops any event for
a path in that set, without looking at what the watcher actually observed.

With usePolling: true, interval: 1000, chokidar compares state once per tick,
so a put() and an external edit that both land between two ticks are visible
as one change carrying the external content. If that single event is
delivered while the path is still in selfWrites, it is discarded as our own
write — and because it was the only event the external edit would ever produce,
nothing later recovers it.

The window is roughly: an external edit landing within ~60 ms of a put()
(50 ms awaitWriteFinish.stabilityThreshold plus the poll callback hop), with a
poll tick falling between the two writes. Realistic trigger: a git checkout or
an editor save arriving while the process is writing the same item.

Status of the evidence — please grade accordingly

Honest about what is and is not measured:

  • Derived, not observed. In 360+ instrumented iterations under deliberate
    event-loop starvation the suppression fired zero times
    (handleFsChange entries hitting the selfWrites suppression: 0), and the
    path was never in selfWrites at external-edit time (0/360). Those runs used
    a 300–400 ms pre-edit sleep, which cannot reach this window.
  • The reason it is unreachable in the current tests is structural, not luck:
    the suppression timer is scheduled before the tests' own pre-edit sleep()
    and with a shorter delay, and Node runs expired timers in due-time order in a
    single timers-phase pass — so by the time the sleep resolves, the entry is
    provably gone. A sub-60 ms external edit is what escapes that argument, and
    nothing exercises one today.
  • So this may well be observation-class rather than a live defect. I am filing
    it plainly rather than sitting on it; the severity call is the triage seat's.

Note on the obvious fix

Making the suppression content-keyed (record the hash of what we wrote, clear
the entry when the content the watcher observes matches) removes the race
outright. Worth knowing before anyone implements it: handleFsChange already
has a content-keyed suppression one step further down —
if (currentHead === hash) return; compares the content the watcher actually
read against the head index, which put() updates in the same microtask
continuation as the write. That downstream check is what suppresses self-writes
in practice today. So the change may be closer to "delete the time-keyed
pre-check" than to "add a hash to it" — but that needs measuring, particularly
for the unlink path and for specs whose in-memory form does not round-trip
through JSON identically.

Not related to

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions