Skip to content

test(core): measure ref'd-timer leaks on the subject's own handles, not on the process - #10786

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10685-refd-timer-window
Aug 21, 2026
Merged

test(core): measure ref'd-timer leaks on the subject's own handles, not on the process#10786
os-zhuang merged 1 commit into
mainfrom
claude/issue-10685-refd-timer-window

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10685

Nothing was red. This is an unstated invariant, not a live failure — and the remedy is chosen so the pins get sounder, not quieter.

The defect

Four @objectstack/core pins counted ref'd timers with a process-global probe:

constrefdTimers=()=>process.getActiveResourcesInfo().filter((r)=>r==='Timeout').length;

getActiveResourcesInfo() reports the whole process and a Test Core shard runs ~37 core files in one worker, so the absolute count is ambient: it belongs to every co-tenant file, not to the test reading it. Scoring a subject against it with toBe is sound only while the window contains no event-loop turn — timer callbacks run in the timers phase, and a microtask drain never reaches it.

health-monitor.test.ts stated that requirement and held itself to it. Three others depended on it silently, with an await inside the measured window (kernel.bootstrap() twice, manager.reloadPlugin() once). They passed only because those hooks settle on microtasks — a property of code they do not own, written down nowhere.

The remedy: name the guards instead of counting the process

New packages/core/src/refd-timer-probe.testkit.ts:

  • recordGuards(delay, body) captures the Timeout handles the subject arms, told apart from every other timer on the shared loop by the very timeout they were configured with.
  • stillPinningTheLoop(handles) reports how many of those are still holding the loop open. It is deliberately synchronous, which is what makes the invariant structural rather than a comment: its two samples are adjacent statements, no await can be inserted between them without turning it into a different and visibly async function, and between two adjacent synchronous statements no timer callback can run at all. The ambient value cancels; what survives is the subject's own delta.
  • It throws on an empty handle list, because "nothing was armed" and "nothing was left behind" are different facts and a zero must never stand for both.

Each site now reads:

constguards=awaitrecordGuards(STARTUP_TIMEOUT,()=>kernel.bootstrap());expect(guards).toHaveLength(2);// the guards were really armedexpect(stillPinningTheLoop(guards)).toBe(0);// and neither outlived its race

health-monitor.test.ts already did this by hand; its assertions are unchanged, its two local helper definitions now come from the testkit so the package holds one documented instrument rather than four hand-rolled copies. The module docblock is where the invariant, and the two measured incidents behind it (#10661's expected 2 to be 4, #6329's @vitest/runner 100 ms throttle timer), now live.

Does this weaken what the tests assert?

Net stronger, with one narrowing stated plainly.

Stronger: the old absolute count could not distinguish "eight guards armed and all eight reclaimed" from "nothing was ever armed" — both read as a difference of zero. Every site now pins the arming count (2, 8, 1) as well as the reclaim, so a subject that stops arming guards reds instead of passing vacuously.

Narrowed: the new probe watches only handles armed with the guard's own timeout, so a leak of some other timer inside bootstrap() no longer shows up here. That coverage is not lost — the fake-timer siblings in the same files (vi.getTimerCount()) already assert that no timer of any delay is pending after the subject settles, and they see unref()'d timers too, which getActiveResourcesInfo() cannot. The old assertion could only have caught such a timer by luck anyway, since ambient noise moved it just as freely.

Deliberately not done: widening the comparison (toBeLessThanOrEqual, a tolerance). That direction converts a latent false-red into a permanent false-green — with a process-global reading it also lets a real subject leak of +1 hide behind two foreign expiries of -2 in the same window. Filed as #10785.

Evidence

Exposure demonstrated before fixing, then closed. Ambient noise = 4000 ref'd timers armed at 1 ms spacing via a scratch setupFiles (the #10661 technique); "future change" = a simulated retry backoff in bootstrap() and debounce in reloadPlugin(), i.e. a real-timer await inside the measured window, applied to production source only.

legtreeconditionsresult
Apre-fixno noise54 passed (54)
Bpre-fixnoise54 passed (54) — exposure latent, as the card says
Cpre-fixnoise + future change3 failed: expected 3744 to be 3755, expected 2037 to be 2047, expected 2021 to be 2037
Dpost-fixnoise + future change3 passed

The count goes down in leg C — foreign timers expiring mid-window — the same signature as the CI red in #10661.

Ablations, each applied by anchor and confirmed on disk by anchor count (never by an editor's exit code), predicted before running:

ablationmutationpredictedobserved
A1drop clearTimeout from TimeoutGuard.reclaim()2 of 3 red (both kernel sites)2 red — expected 2 to be +0, expected 8 to be +0
A2drop clearTimeout from hot-reload's raceShutdownTimeout1 of 3 red (hot-reload)1 red — expected 1 to be +0
Btestkit stops recognising the subject's guards4 red across the 3 files4 red (kernel x2, hot-reload, health-monitor), 50 passed

A1/A2 are the "green for the right reason" legs: a real leak still reds, and the message now names how many guards leaked instead of quoting an ambient count. B confirms the testkit is what supplies the data, health-monitor's de-duplicated site included. Every mutation was restored with git restore --source=HEAD and the tree proven byte-identical with git hash-object against the HEAD blob. No rebuild is involved anywhere: these tests import their subjects through relative source specifiers, so vitest resolves src/*.ts directly and never dist/ — legs C and D differ only by an edit to src/kernel.ts with no build step between them, which is that fact demonstrated rather than assumed.

Suite: pnpm --filter @objectstack/core testTest Files 37 passed (37), Tests 887 passed (887).

Gates

Derived with node scripts/pm/dispatch-gates.mjs from the real changeset and run at e303311cdf; exit codes captured by redirect-then-capture, never after a pipe.

gateexitverdict line
check:kernel-hook-pairs04 dispatched kernel:* hook(s), each pinned in both kernel.test.ts and lite-kernel.test.ts
check:slot-lookup0slot-lookup ratchet holds: 107 unswept site(s) ... none new
check:test-source-alias072 packages with tests scanned
check:type-source-resolution076 packages with a tsconfig.json scanned
scripts/docs-audit/check-affected-docs.mjs0(no affected hand-written docs)
check:nul-bytes0scanned 6238 text file(s) ... no raw ASCII control bytes
check:engine-double-contract0371 pinned, 133 in the DEBT ledger, 2 exempt
check:where-matcher0271 matcher(s) discovered ... 0 silently-wrong
check:type-check-coverage064/77 workspace packages type-checked

check:type-check-debt --re-measure was narrowed and declared: it refuses an unbuilt worktree and needs the whole workspace closure built. Instead the ratchet's own measurement was reproduced for the only package this diff touches — pnpm exec tsc --noEmit in packages/core reports 98 raw errors, exactly the DEBT['@objectstack/core'] entry, with zero of them in the new testkit. CI runs the farm regardless.

No changeset: test-only, nothing publishes. skip-changeset applied additively.

Filed separately, both domain:devx, unassigned: #10783 (a fifth site with the same shape, in service-automation, outside this card's list) and #10785 (a gate that would close the class, plus the toBeLessThanOrEqual masking arithmetic).


Generated by Claude Code

Four core pins counted ref'd timers with a PROCESS-global probe
(`process.getActiveResourcesInfo().filter(r => r === 'Timeout').length`)
and scored a subject against the absolute value. A `Test Core` shard runs
~37 core files in one worker, so that value is ambient: it belongs to
every co-tenant file, not to the test reading it. Scoring it with `toBe`
is sound only while the window crosses no event-loop turn.
`health-monitor.test.ts` said so and held itself to it. Three others
relied on the same property silently, with an `await` inside the measured
window — green only because the plugin hooks they awaited settle on
microtasks, a property of code they do not own and written down nowhere.
Add a retry backoff to `bootstrap()` or a debounce to `reloadPlugin()`
and the pins go intermittently red, pointing at the timer count instead
of at the change.
Rather than loosen the comparison, name the guards. The new
`refd-timer-probe.testkit.ts` records the `Timeout` handles the subject
arms (told apart by the timeout they were configured with) and reports
how many are still holding the loop open — `stillPinningTheLoop()`,
deliberately synchronous, so the invariant is structural instead of a
comment. Every site now also pins how many guards were armed, which the
absolute count could never distinguish from "nothing was measured".
Refs #10685
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@github-actionsgithub-actionsBot added size/m and removed skip-changeset PR has no user-facing published change; bypasses the changeset gate labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 53428b8c9dc6ba3aa79b6b00a1a3a83877be7f81packageMentionDocs.

Which tree this was computed on

This run read content/docs from 377b083cec4a703d1b2287539edae67badbbeee3 — the merge of head e303311cdf629f5a65d2370b26b1c0ef15a79081 into base 53428b8c9dc6ba3aa79b6b00a1a3a83877be7f81, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 377b083cec4a703d1b2287539edae67badbbeee3 && git checkout 377b083cec4a703d1b2287539edae67badbbeee3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 53428b8c9dc6ba3aa79b6b00a1a3a83877be7f81 e303311cdf629f5a65d2370b26b1c0ef15a79081 && git checkout -B drift-repro 53428b8c9dc6ba3aa79b6b00a1a3a83877be7f81 && git merge --no-ff e303311cdf629f5a65d2370b26b1c0ef15a79081
node scripts/docs-audit/affected-docs.mjs --json 53428b8c9dc6ba3aa79b6b00a1a3a83877be7f81

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 11:33
@os-zhuang
os-zhuang enabled auto-merge August 21, 2026 11:34
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit f4e5d91Aug 21, 2026
38 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10685-refd-timer-window branch August 21, 2026 12:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Three ref'd-timer pins score a process-global count across an await — green only because their windows happen not to cross a timer phase

2 participants

@os-zhuang@claude