FileSystemRepository.close() never reaches its event broker — a parked watch() iterator stays parked after shutdown

Found while implementing #11021 (SysMetadataRepository.close() cannot drain a filtered or numeric-since watcher). Same defect class, different implementation and different package, so it is filed rather than fixed in that PR.

The mechanism

FileSystemRepository.close() (packages/metadata-fs/src/repository.ts:198) does exactly two things:

async close(): Promise<void> {
  this.stopResync();
  if (this.watcher) {
    await this.watcher.close();   // chokidar
    this.watcher = null;
  }
  this.started = false;
}

It never touches this.broker (packages/metadata-fs/src/repository.ts:117, createBroker in packages/metadata-fs/src/sync.ts:49). The broker has no teardown of its own — subscribe/unsubscribe add to and delete from a plain Set, and nothing else empties it.

Each watch() iterator (packages/metadata-fs/src/watch-iterable.ts) parks its pending next() on a waiter callback that only two things can settle: a broker push, or the iterator's own local close() — which runs from iterator.return() / iterator.throw() and from nowhere else. So after repo.close():

the chokidar source is gone, so no further push can arrive;
the subscriber is still registered, and nothing calls its terminator.

A consumer holding a for await over fsRepo.watch(...) at shutdown therefore never sees the loop end. Unlike the #11021 shape this is not filter-dependent — there is no drain attempt at all, so it applies to every subscription shape including watch({}).

Confidence

Code read, not a runtime probe. #11021's scope is packages/metadata-protocol, and building/booting metadata-fs to confirm was out of that card's surface. The read is unambiguous (there is no path from close() to the broker or to any iterator terminator), but the matrix that #11021 carries for its own repository has not been produced here. Whoever picks this up should measure it first.

Related contract statement

#11021 adds invariant 8 to the MetadataRepository invariant table in packages/metadata-core/src/repository.ts — "shutdown terminates; it does not emit" — stating what a repository-level close() owes a pending iterator. That row names this gap as the one measured non-conformance among today's implementations. Fixing it is the same one-line shape the #11021 repair uses: hold each subscription's terminator alongside its event sink, and run every terminator from close().

InMemoryRepository is unaffected: it exposes no repository-level shutdown at all, so its iterators end only through return().

Generated by Claude Code

fix(metadata-protocol): close() terminates watch iterators instead of…

Triage: pm:queue, domain:engine, Bug. FileSystemRepository.close() never reaches its event broker, so a parked watch() iterator survives shutdown — lifecycle invariant restore, no contract change.

Generated by Claude Code

test(metadata-fs): pin the invariant-8 shutdown contract (red baselin…

fix(metadata-fs): close() terminates every live watch() iterator (#11127

os-dev-report

{
  "issue": 11127,
  "status": "done",
  "branch": "claude/issue-11127-filesystem-close-terminates",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/11198",
  "premise_still_valid": true,
  "summary": "The premise held and was measured, not just re-read: FileSystemRepository.close() reached stopResync() and watcher.close() and stopped, never touching the broker, so a parked watch() iterator stayed parked for EVERY subscription shape including watch({}) — nine cases red on unmodified source. The broker now holds each subscription's terminator next to its event sink (BrokerSubscriber.terminate + EventBroker.terminateAll), and close() runs every terminator — the same routine iterator.return() runs, snapshot-then-clear with a per-terminator try/catch, mirroring SysMetadataRepository's post-#11021 shape. Termination runs BEFORE the await on watcher.close(), so a rejecting watcher.close() cannot strand a consumer. Invariant 8's exception text DID need updating and is updated in the same PR: it named FileSystemRepository as the one non-conformance, and now records no declared exceptions plus a line binding any future close()-offering implementation to the same row — no semantics touched. One bounded in-place repair beyond the card's text, declared in the PR body: watch() returns a deferred iterable whose subscriber registers only after the eager log read resolves, so a close() inside that window swept a broker the subscription had not yet joined; watch() now carries the close generation it was opened under (a counter, not a boolean, because start() may follow close()) and a late-arriving subscription terminates on arrival.",
  "tests": "All at 609147d8 (head of the branch, after the final commit). BASELINE, same committed pin (3e545f41) against unmodified source: `pnpm --filter @objectstack/metadata-fs test` → 'Test Files 1 failed | 7 passed (8) · Tests 9 failed | 56 passed (65)' — all nine new cases returned Symbol(still-pending) 2s after close(). AFTER: same command → 'Test Files 8 passed (8) · Tests 65 passed (65)'. TYPECHECK: `pnpm --filter @objectstack/metadata-fs --filter @objectstack/metadata-core typecheck` → both 'Done' (script name echoed: `tsc --noEmit && tsc --noEmit -p tsconfig.test.json`, so not a zero-match silent pass). DOWNSTREAM (`...` prefix = dependents): the FS-repo consumers in @objectstack/metadata, incl. the real-LiteKernel shutdown test driving close() through MetadataPlugin.destroy() → 'Test Files 3 passed (3) · Tests 10 passed (10)'. ABLATION of the deferred-window guard (arrivesClosed wiring deleted, terminateAll kept): mutation confirmed ON DISK — 'BEFORE: arrivesClosed occurrences = 1 / AFTER: 0' plus the git diff showing the single deleted line, with an abort guard had it not landed; NO rebuild needed and none claimed — the pin imports ../src/index.js and the one package-name import (@objectstack/metadata-core) is untouched by the mutation; result 'Tests 1 failed | 64 passed', exactly the deferred-window case and no other, i.e. the guard is load-bearing; restore leg confirmed ('RESTORED: arrivesClosed count = 1', then `git status --porcelain` empty). PUBLISHED-TYPE REVERSE CHECK: dist/index.d.ts built at HEAD vs at origin/main's packages/metadata-fs/src — only non-comment diff line is 'private closeGeneration;'. GATES (set derived by `node scripts/pm/dispatch-gates.mjs` with no paths passed, each read from the gate's own verdict line, all exit 0): check:nul-bytes ('OK (scanned 6489 text file(s) … no raw ASCII control bytes)'), check:changeset-gate-self-tests, check:objectui-changeset, check:slot-lookup ('ratchet holds … none new'), check:test-source-alias, check:type-source-resolution, check-adr-0087-registration, check-changeset-no-major ('introduces no `major` bump'), check-ci-filter-parity, check-empty-changeset ('1 declaring changeset(s) added'), check-plugin-teardown-shape, check-affected-docs, plus the convention-triggered set for a new test file: check:query-options-erasure, check:engine-double-contract ('OK — 384 pinned'), check:where-matcher, check:type-check-coverage. TWO DECLARED NARROWINGS, both argued in the PR body: (1) repo-wide `pnpm lint` is CI's — locally eslint --no-inline-config over the 5 changed source files → 0 errors / 0 warnings, 5 of 5 actually linted (count read from --format json), and untouched files cannot move because this repo's single eslint.config.mjs never enables type-aware linting for any file (its own words, line 328); (2) check:type-check-debt --re-measure needs the whole workspace built (does not fit the ~10min foreground ceiling) — its population is the DEBT/TEST_DEBT ledger literals in scripts/check-type-check-coverage.mjs (lines 495/709) and neither touched package appears in either, while the one drift route into a ledgered dependent (@objectstack/metadata) is the published .d.ts, measured unchanged above; its structural half ran green. CI has NOT been waited on — gate status at report time is in_progress by the dispatch contract.",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

ACCEPT — PR #11198, 31/31 checks green, enqueued

Verified by name: every run success or skipped, Lint & Repo Gates and Temporal Conformance (live PG + MySQL) green, Console Pin Gate skipped. Flipped to ready, auto-merge enabled.

⭐ You found a defect the card's own acceptance shape could not reach

watch() returns a deferred iterable — the subscriber registers only once the eager log read resolves. A close() landing inside that window sweeps a broker the subscription has not yet joined, so it arrives afterwards and parks forever. Same defect, different route, same function, and the card's repro could never have produced it.

Two things make that repair trustworthy rather than speculative:

A counter, not a boolean — because start() may follow close(), and a restart must not poison watchers opened after it. That is the failure mode a boolean would have shipped.
You ablated it. Deleting the arrivesClosed wiring while keeping terminateAll() reddened exactly one case — the deferred-window one, no others. So its load-bearingness is measured, not argued. ⭐ That is now this lane's standard (recorded as lesson 8 after the SORT axis' runtime premise is unpinned: neither door refuses an ORDER BY over an unprovisioned injected anchor, and nothing asserts it #10744): a guard defended by argument gets deleted by the next reader who finds the argument unconvincing.

The rest, briefly

The pin was committed RED first (3e545f41) and the fix after (609147d8), so the baseline is in git history rather than asserted. Nine cases returning Symbol(still-pending) 2s after close() is an unambiguous failure signature.
Invariant 8's exception text updated, as the dispatch required. It named FileSystemRepository as the one measured non-conformance; that is now stale and the row records no declared exceptions, plus a line binding any future close()-offering implementation. ⭐ Adding that forward-binding line was not asked for and is the difference between fixing a list and fixing the thing that lets the list rot.
Deliberate symmetry with SysMetadataRepository's post-[finding] SysMetadataRepository.close() cannot drain a filtered or numeric-since watcher — the pending next() never settles and the consumer's for-await hangs #11021 shape — {dispatch, terminate} vs {push, terminate}, snapshot-then-clear on both, per-terminator try/catch on both. "That symmetry is the whole reason the invariant exists." Correct.
Termination runs before await watcher.close(), so a rejecting watcher.close() cannot strand a consumer. Subtle and right.
Published type surface measured, not asserted: dist/index.d.ts built at HEAD vs at origin/main — only non-comment diff is private closeGeneration;.

On the two declared narrowings

Both are the right way to handle a check you cannot fully run, and I am accepting them as written:

repo-wide pnpm lint left to CI — with the invariance argument (this repo's single eslint.config.mjs never enables type-aware linting for any file, quoted at its line 328), so no untouched file's verdict can be a function of this diff. CI has now run it: green.
check:type-check-debt --re-measure not run locally, because it needs the whole workspace built and that does not fit the container's ~10-minute foreground ceiling. ⭐ Rather than banking that as a pass you named the population (the DEBT/TEST_DEBT ledger literals at check-type-check-coverage.mjs:495/:709), showed neither touched package appears in either, and closed the one real drift route into a ledgered dependent by measuring the published .d.ts. That is a reasoned invariance argument, not a skipped gate — the distinction this lane cares about, and CI's Type Check · debt ledger has now confirmed it green.

Docs advisory — checked, no action

The drift check flagged content/docs/concepts/metadata-lifecycle.mdx via the FileSystemRepository symbol. All three mentions are about what it is, which package ships it, and what it appends to the change log; no page describes close(), shutdown, or watch-termination semantics (zero hits for close()|shutdown|terminat, with watch appearing 8 times in the same file as the positive control, so the zero is a reading). This change is invisible to the docs.

No out-of-scope findings, and none forced.

Generated by Claude Code

fix(metadata-protocol): close() terminates watch iterators instead of…

fix(metadata-fs): FileSystemRepository.close() terminates every live …

docs(metadata-fs): re-anchor the dead tracker citations to the commit…

Navigation Menu

Repository navigation

`FileSystemRepository.close()` never reaches its event broker — a parked `watch()` iterator stays parked after shutdown #11127

The mechanism

Confidence

Related contract statement

ACCEPT — PR #11198, 31/31 checks green, enqueued

⭐ You found a defect the card's own acceptance shape could not reach

The rest, briefly

On the two declared narrowings

Docs advisory — checked, no action

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

FileSystemRepository.close() never reaches its event broker — a parked watch() iterator stays parked after shutdown #11127

Description

The mechanism

Confidence

Related contract statement

Activity

os-zhuang commented on Aug 23, 2026

os-zhuang commented on Aug 23, 2026

os-zhuang commented on Aug 23, 2026

ACCEPT — PR #11198, 31/31 checks green, enqueued

⭐ You found a defect the card's own acceptance shape could not reach

The rest, briefly

On the two declared narrowings

Docs advisory — checked, no action

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`FileSystemRepository.close()` never reaches its event broker — a parked `watch()` iterator stays parked after shutdown #11127