Skip to content

test(metadata-fs): triage snapshot on the #9339 anchor-event failure message - #9400

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-9339-failure-message-watched-set
Aug 17, 2026
Merged

test(metadata-fs): triage snapshot on the #9339 anchor-event failure message#9400
os-zhuang merged 1 commit into
mainfrom
claude/issue-9339-failure-message-watched-set

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Part of #9339 — the small Part of the PM review ruled in on Q1 of the investigation report. Out of scope: #9339's own decision (fix-at-the-coupling vs. quarantine) — it remains open and is not addressed here.

What changed

packages/metadata-fs/test/watch-write-registration.test.ts's line-166 assertion —

expect(events.map((e)=>e.ref.name)).toContain('anchor');

— cannot currently tell apart the six independently-measured one-shot delivery gates (mtime-tie, readdir-throttle, readdirp-miss, add-throttle, pending-write, awf-enoent) that the investigation found all reproduce the CI signature byte-identically. This PR adds a getWatched()[viewDir] snapshot to that assertion's failure message only:

constanchorWatched=watchedIn(repo,viewDir);expect(events.map((e)=>e.ref.name),`#9339 triage — getWatched()[${JSON.stringify(viewDir)}] = ${JSON.stringify(anchorWatched)}. `+(anchorWatched.includes('anchor.json')
? `'anchor.json' IS present in the watched set: the loss is at or after the emit gate (_handleFile ran, no event reached the iterator).`
: `'anchor.json' is ABSENT from the watched set: the loss is upstream of _handleFile (the directory scan/poll layer never registered the path).`),).toContain('anchor');

That single datum splits the six candidate mechanisms into two disjoint classes, so the next CI ejection arrives pre-triaged instead of reopening the whole question.

What did NOT change (byte-identical)

  • Assertion semantics — still expect(events.map((e) => e.ref.name)).toContain('anchor'), same condition, same pass/fail boundary. watchedIn(repo, viewDir) is a synchronous read of chokidar's already-armed watcher state; capturing it adds no await and does not touch timing.
  • EVENT_WAIT_MS — untouched at 20_000. The investigation established the anchor phase has exactly one delivery attempt, so widening buys nothing; this PR does not reopen that.
  • packages/metadata-fs/src/repository.ts and every other production file — untouched. No awaitWriteFinish / pollInterval / interval / usePolling tuning.
  • No skip/.skip/.todo/retry-wrap of the test.
  • No retry or content-keyed re-check added — that is the candidate fix and it is still the maintainer's open decision.

Proof the message is real

Forced the assertion to fail with a throwaway local edit (events.length = 0; right before the snapshot, immediately reverted — never committed), then ran the real test and captured the rendered output verbatim:

FAIL test/watch-write-registration.test.ts > FileSystemRepository watcher — writes register their own path (#7282) > registers a put() path with the watcher without waiting for a poll
AssertionError: #9339 triage — getWatched()["/tmp/objectstack-fs7282-Pb2jid/view"] = ["anchor.json","seed.json"]. 'anchor.json' IS present in the watched set: the loss is at or after the emit gate (_handleFile ran, no event reached the iterator).: expected [] to include 'anchor'
❯ test/watch-write-registration.test.ts:180:7

Confirmed both branches read correctly in code (present → "at or after the emit gate"; absent → "upstream of _handleFile"); only the present branch was forced and observed directly, since forcing the absent branch would require faulting chokidar's internals rather than the test's own event array.

Tests

At final commit 466299153 (worktree off origin/maine1bb0cad6):

  • pnpm --filter '@objectstack/metadata-fs^...' build — dependency closure (spec, metadata-core) built clean.
  • pnpm --filter '@objectstack/metadata-fs' test -- --maxWorkers=26 files / 51 tests passed, including the target test, both before staging the change and again at the final commit head.
  • pnpm --filter '@objectstack/metadata-fs' typecheck — clean (tsc --noEmit + tsc --noEmit -p tsconfig.test.json).
  • node scripts/check-nul-bytes.mjs — OK, 6107 files scanned.
  • Gate list re-derived against the actual changed path via node scripts/pm/dispatch-gates.mjs packages/metadata-fs/test/watch-write-registration.test.ts:
    • node scripts/docs-audit/check-affected-docs.mjs — OK, 155 self-test cases pass.
    • pnpm check:query-options-erasure — ratchet holds, 67 unswept non-test sites unchanged, test surface 240/47 unchanged (no new query-options sites added).
    • pnpm check:engine-double-contract — OK, 317 pinned / 133 DEBT / 2 exempt, none new (this test declares no engine double).
    • node scripts/check-where-matcher-conformance.mjs — OK, 253/253 conform, none new (this test declares no WHERE matcher).
    • pnpm check:type-check-coverage / check:type-check-debt (--re-measure) — not run to full re-measure: it requires the entire ~53-package workspace closure built (pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'), which is disproportionate for this change. Confirmed instead that @objectstack/metadata-fs is not a DEBT/TEST_DEBT-ledgered package (it graduated in [finding] The 3 test layers #7353 made visible are repair candidates, not permanent debt — metadata-fs (6) and example-showcase (4) look like same-day graduations #7923 via the sibling-tsconfig.test.json route) and that its own typecheck script — which covers both src/ and test/ under that route — passes cleanly at this commit, so the ratchet is not implicated by a test-file-only, cleanly-typechecking diff.

Generated by Claude Code

…message
watch-write-registration.test.ts's line-166 assertion (`expect(events...).toContain('anchor')`)
cannot currently distinguish six independently-measured one-shot delivery gates that all
produce the same CI signature (see #9339's investigation). Add getWatched()[viewDir] to the
assertion's failure message only: absent means the loss is upstream of `_handleFile`, present
means the loss is at or after the emit gate. Assertion semantics and EVENT_WAIT_MS are
byte-identical -- this only changes what a failure says when it happens.
Part of #9339
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj
@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 17, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — the diagnostic was rendered, and the two things it could not show are named

⛔ Not flipping yet; CI started at 22:55Z. Holding until each job's own conclusion is green. ⚠️ The queue runs the full suite as a required check (#9283 Option A), so landing is slower than earlier today.

⭐ The message exists, and you proved it by making it render

I asked for this specifically — "a diagnostic nobody has seen rendered is a diagnostic that might not render" — and you forced the failure with a throwaway edit, captured the output verbatim, and reverted without committing. The rendered line is the deliverable:

AssertionError: #9339 triage — getWatched()["/tmp/objectstack-fs7282-Pb2jid/view"] = ["anchor.json","seed.json"].
'anchor.json' IS present in the watched set: the loss is at or after the emit gate …: expected [] to include 'anchor'

It reads standalone in a CI log, which was the bar.

⭐ The limitation you named is the one that mattered

only the present branch was forced and observed directly, since forcing the absent branch would require faulting chokidar's internals rather than the test's own event array.

Correct, and correctly disclosed. The forced case emptied eventsafter a healthy delivery, so the message's diagnosis in that specific render ("loss is at or after the emit gate") describes the artificial fault rather than a real one — inherent to the demonstration, not a defect. The mapping itself is sound: the investigation measured the six modes splitting cleanly no/no/no and yes/yes/yes on anchorInWatchedSet, which is exactly the two-class split the message encodes.

The timing question, which this test of all tests deserves

watchedIn(repo, viewDir) is a synchronous read of already-armed watcher state, evaluated after the 20s wait has elapsed. So it cannot perturb the delivery window it is diagnosing. ⭐ On a card whose entire subject is a timing-sensitive one-shot delivery, a diagnostic that moved the timing would be self-defeating — you addressed it rather than leaving me to wonder.

⭐ The gate you did NOT run, stated as such

check:type-check-debt --re-measure needs the whole ~53-package closure built, and you skipped it with evidence rather than silently: @objectstack/metadata-fs is not a DEBT/TEST_DEBT-ledgered package (it graduated in #7923 via the sibling-tsconfig.test.json route), and its own typecheck — which covers src/ and test/ under that route — passes clean at this commit.

That is the right shape. A skipped gate named with its reason is a measurement; a skipped gate left unmentioned is a hole. This shift has produced eleven instances of the second kind, so I want the first kind on the record.

Scope held

Assertion semantics byte-identical (same expect(...).toContain('anchor'), same boundary — only vitest's message argument added). EVENT_WAIT_MS untouched at 20_000. repository.ts untouched. No skip, no retry-wrap, no content-keyed re-check. Part of #9339, ⛔ not Fixes.

#9339's decision is untouched and remains the maintainer's: fix at the coupling (a retry or content-keyed re-check on the external-write path) versus quarantine with a named owner and a date. The investigation eliminated constant-tuning from that menu on measured grounds; nothing here narrows it further.

Holding for CI.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Fixed the Part-of PR must not also close its card gate (job 9553577926495536039338, now green): the body's disclaimer sentence contained the literal string Fixes #9339, which the closing-keyword parser reads regardless of surrounding prose or negation. Reworded to "Out of scope: #9339's own decision … it remains open and is not addressed here" with no closing keyword anywhere in the body. No other content changed — diff, commit, and every other claim in the PR body stand as reviewed.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 17, 2026 23:20
@os-zhuang
os-zhuang added this pull request to the merge queueAug 17, 2026
Merged via the queue into main with commit e0ff5b7Aug 17, 2026
30 of 31 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-9339-failure-message-watched-set branch August 17, 2026 23:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-zhuang@claude