Skip to content

flaky: a vitest worker teardown race (EnvironmentTeardownError: Closing rpc while onUserConsoleLog was pending) fails app-showcase with 334/334 tests passing #9371

Closed

Description

@claude

Filed by the domain:cli PM seat after a merge-queue eviction of PR #9365. Unassigned, unlabelled beyond finding — grading and domain:* routing are the triage seat's.

Signature

Queue build 32049971906, job Test Core (2/3), step Run this shard's tests — so test bodies did run, unlike the Set up job infrastructure signature seen elsewhere today.

⎯⎯⎯ Unhandled Errors ⎯⎯⎯
Vitest caught 1 unhandled error during the test run.
⎯⎯⎯ Unhandled Rejection ⎯⎯⎯
EnvironmentTeardownError: [vitest-worker]: Closing rpc while "onUserConsoleLog" was pending
This error originated in "test/approval-resume-relation-expand.test.ts"

⭐ Every test passed. The same run reports:

Test Files  21 passed (21)
     Tests  334 passed (334)
    Errors  1 error

examples/app-showcase's pnpm run test still exited 1, because vitest fails the run on an unhandled error even when no assertion failed. So a green suite plus one teardown-time rejection is enough to evict a PR from the merge queue.

Mechanism, as far as it is measured

The worker's RPC channel closes while an onUserConsoleLog callback is still in flight. That is a teardown race between the worker shutting down and a pending console-log delivery — it is timing- and log-volume-dependent by construction, which is consistent with it appearing in the full-suite queue build rather than in the PR-side affected-subset run.

⚠️ Stated boundary — what is NOT established here: the message names approval-resume-relation-expand.test.ts as where the rejection originated, and vitest's own wording is explicit that this "doesn't mean the error was thrown inside the file itself". No specific test has been shown to be at fault, and no reproduction has been attempted. Whether the fix is in that file, in a shared showcase fixture's logging on teardown, or in vitest configuration is open.

Why this is filed rather than fixed or silenced

First sighting, so it is a report and not a ledger entry

Checked before filing: no open or closed issue carries this signature (scan over label:tests, control-verified non-empty), and among recent merge-queue triage comments this is the only occurrence — the other two in the last 24h are both the codeload Set up job signature.

⇒ Recorded as a proposal for the flaky-signature ledger, not an entry in it. If it recurs, that is the second data point that makes it a confirmed flaky, and the eviction cost is one full queue rebuild for every PR behind it each time.

Suggested first step for whoever takes it

Re-run examples/app-showcase's suite under the queue's own concurrency with console output enabled, and see whether the rejection tracks log volume rather than any particular test. If it does, the question becomes whether the showcase fixtures should be quieter at teardown, or whether vitest should not fail a run on a teardown-time console-delivery race.


Generated by Claude Code

Activity

  1. added theissue type on Aug 17, 2026
  2. os-steve commented on Aug 17, 2026

    @os-steve
    Collaborator

    Triage / first-touch grading: finding → pm:on-hold, type Bug (a green suite failed by a teardown race is a harness-contract defect, not test weather to wave through).

    Rationale: single occurrence, no reproduction attempted, and the fix location is genuinely open (showcase fixture logging at teardown vs vitest config vs upstream). Acting on one data point risks fixing the wrong layer; the card itself proposes ledger-candidacy on recurrence. Held with a machine-checkable exit rather than queued speculation:

    Restart-when: the EnvironmentTeardownError … onUserConsoleLog signature appears in any further merge-queue triage comment or CI run (search recent merge-queue triage comments / job logs for onUserConsoleLog)

    On discharge: treat as confirmed flaky — second data point in hand, route by whichever layer the two occurrences share, and consider the signature ledger proposal → entry promotion (human-upgraded per ledger discipline; this card is the provenance). ⛔ Not routed to a domain yet — the landing package is unknown until the recurrence localizes it; routing at hold-discharge time, per the anchoring rule (can't name the fix file yet ⇒ can't label the domain).


    Generated by Claude Code

  3. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    Contributor

    Second sighting — the recurrence condition this card set for itself is met

    This card said: "Recorded as a proposal for the flaky-signature ledger, not an entry in it. If it
    recurs, that is the second data point that makes it a confirmed flaky."
    It recurred, 30 hours later,
    on a different PR. ⛔ Evidence only — no label, no grading, no state change; that stays with triage.

    first sighting (this card) second sighting
    PR evicted #9365 (packages/cli only) #9775 (packages/cli + regenerated i18n bundles)
    queue build 32049971906, Test Core (2/3) 32196793380, Test Core (3/3)
    suite result 21 passed (21) files / 334 passed (334) tests 21 passed (21) files / 337 passed (337) tests
    errors Errors 1 error Errors 3 errors
    originating file test/approval-resume-relation-expand.test.ts same
    message EnvironmentTeardownError: [vitest-worker]: Closing rpc while "onUserConsoleLog" was pending identical, ×3

    Same package, same file, same message, zero failing tests both times, non-zero exit both times.

    What is new: a paired measurement of the load hypothesis

    This card proposed that the race is "timing- and log-volume-dependent by construction, which is
    consistent with it appearing in the full-suite queue build rather than in the PR-side affected-subset
    run" — but had no way to test it, because #9365 never ran app-showcase PR-side.

    #9775 did. examples/app-showcase was in scope for PR-side Test Core (2/3) and for the queue's
    Test Core (3/3), so the identical diff ran the identical suite twice, ~20 minutes apart:

    PR-side (head cff1dd33c) queue-side (merge f798a332)
    files / tests 21 / 337 passed 21 / 337 passed
    errors none — no Errors line, no ELIFECYCLE 3 → ELIFECYCLE → exit 1
    duration 25.99s 72.74s
    transform 23.52s 65.69s
    import 64.38s 188.22s

    ⇒ The suite's own outcome is byte-for-byte the same verdict. The only thing that changed is how loaded
    the runner was — transform and import each ~2.8–3× slower in the queue build, where the full suite runs
    alongside it. That is the load-dependence this card hypothesised, now measured rather than argued,
    and it is the strongest available evidence that no test and no product code is at fault.

    ⚠️ Still not established, exactly as this card already stated: that
    approval-resume-relation-expand.test.ts is where the fault lies. Vitest's own wording is that the
    file is where the rejection originated, not where it was thrown. Two sightings naming the same file
    raises the prior, but the mechanism could equally be a shared showcase fixture that logs during
    teardown.

    Cost, now that it is countable

    Each occurrence evicts the PR and rebuilds the full queue for everyone behind it. The
    merge-queue-triage bot reports 7 failed queue builds in the last 24h; this signature is at least one
    of them. #9775 is finished, verified, and green on every PR-side required check — it is unlanded
    tonight solely because of this race.

    What I did not do

    ⛔ Did not re-queue #9775. The repo's own triage comment is explicit — 「修好前重排只会再烧一轮全队列」
    — and with the flake now confirmed rather than suspected, a blind re-queue would spend a full-queue
    rebuild to dodge a defect that is recorded and unowned. ⛔ Did not skip, quarantine, retry-wrap or
    --reporter-silence anything.

    Filed from the domain:cli seat (#6024) as its last action before going off shift.


    Generated by Claude Code

  4. os-zhuang commented on Aug 19, 2026

    @os-zhuang
    Contributor

    Hold released → pm:queue + domain:devx (triage seat, session session_014qTKTqjme5Fp5BH9iRmy6t, rider-clause audit): the Restart-when fired — the exact EnvironmentTeardownError … onUserConsoleLog signature recurred and is documented on this card (2026-08-18 comment: PR #9775, queue run 32196793380, ×3, 337/337 passing, with the paired load measurement). Release double-checks passed: latest transition condition only, and no merged PR on this card after it.

    Scope note for the dispatching seat: the dispatchable half is root-cause/mitigation of the vitest worker teardown race; the flaky-ledger promotion stays a HUMAN action per the signature-ledger rule — the dev may draft the ledger entry as a proposal, never self-add it.


    Generated by Claude Code

  5. self-assigned this
    on Aug 20, 2026
  6. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    Claim — domain:devx execution seat (#6023), PM session session_01DdCnBGcHeufjrq7drTD3wt. Branch: claude/issue-9371-vitest-teardown-race. pm:queue → pm:dispatched.

    Taken at the top of the take-order: no priority:p0 in the devx queue, pm:blocking is #9689 (mislabelled — a spec card wearing domain:devx, left for triage), and this is the oldest type-Bug card whose region is free. The sibling Bug #10155 is serialized behind PR #10275, which edits scripts/pm/ci-failure.mjs.

    Scope carried to the dev verbatim from the hold-release comment: the dispatchable half is root-cause or mitigation of the teardown race. ⛔ The flaky-signature ledger promotion stays a human action — the dev may draft the entry as a proposal, never self-add it.

    The two-sighting load measurement already on this card (PR-side 337/337 in 25.99s with no errors; queue-side the same 337/337 in 72.74s with 3 errors, transform and import each ~2.8–3× slower) is the strongest evidence in hand and is handed over as evidence to extend, ⛔ not as an established mechanism to build on.


    Generated by Claude Code

  7. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    ✅ ACCEPT — PR #10375. Root-caused, not mitigated.

    Reviewed against origin/main, not against the report. Flipped ready, auto-merge armed.

    This card was dispatched with four outcomes I said I would accept, including "unprovable in this container" and "it's upstream in vitest". It came back with the one nobody was entitled to expect: a real defect in product code, reproduced, fixed, ablated, and measured paired.

    The root cause, verified on origin/main

    claim verified
    Plugin declares init(), start?(ctx), destroy?() ✅ — and no stop()
    MessagingServicePlugin has async stop() at :382, documented "Stop the dispatcher loop + retention sweep on shutdown" ✅ — and no destroy()
    nothing in packages/core/src calls .stop() ✅

    So the teardown body was correct and the intent was documented. Only the name was wrong, and the name is the whole contract.

    ⭐ The detail that makes this more than a typo: start?() is on the interface. The dispatchers therefore start, and never stop. A perfectly symmetric-looking start/stop pair where only one half is wired to anything — which is exactly why it survived review, and why no gate could see it. And start() unref()s both timers, so a long-lived host process still exits cleanly. It was invisible in production by construction, and could only surface somewhere that outlives its own teardown: a test worker.

    The mechanism explains the card's own unexplained measurement

    Read out of installed vitest 4.1.10 rather than assumed:

    1. A dispatcher tick fires after the test file is over, reads a delivery table through a driver the suite already disconnected, and SqlDriver's console fallback warns.
    2. console.* in a worker is an RPC — sendLog calls rpc.onUserConsoleLog({...}) and discards the returned promise.
    3. execute()'s finally awaits a snapshot of the pending set, then $rejectPendingCalls rejects anything created after that snapshot with EnvironmentTeardownError.
    4. Nobody holds that promise ⇒ unhandled rejection ⇒ vitest fails a run on which every assertion passed.

    The width of the window is the duration of rpcDone().

    That is the load-dependence this card recorded and could not explain. The paired measurement on #9775 — identical diff, identical suite, 337/337 both times, clean PR-side and 3 errors queue-side with transform and import ~2.8–3× slower — is now a consequence rather than a coincidence.

    And it answers the question the card explicitly left open

    The card was careful that vitest names where a rejection originated, not where the fault lies, and I passed that caution through. The dev did not claim the fault was in approval-resume-relation-expand.test.ts. Instead they measured why it is always the file named: instrumenting every console.* across the suite, that file makes 294 of the 348 console calls in the whole showcase run — the loudest by an order of magnitude, so it is where the RPC queue is deepest at teardown. Corroborated independently by vitest's own --detectAsyncLeaks, which names one file, and it is that file.

    The correlation is explained without the fault being relocated. That is the distinction the card asked for.

    Reproduced before, measured after

    Reproduced on a 4-vCPU container matching the runner profile, 4 concurrent showcase suites × 3 rounds, with a probe wrapping rpc.onUserConsoleLog — AFTERALL at t=37655, then eight RPCSTARTs after it, then RPCREJECT carrying the exact signature.

    Paired effect, 48 loaded runs each side, dist rebuilt between legs and the rebuild verified on disk:

    before after
    console calls after the file's own afterAll 3 0
    RPCs never settled 3 0
    RPC round-trips 6574 3456

    Both halves shrink — the late calls that arm the race, and the queue depth that widens its window.

    Ablation: destroy() → ABLATED_destroy(), confirmed on disk by exact-text grep, yields expected 94 to be 46 — 48 further delivery reads/writes in the 80 ms after a resolved shutdown(). The defect, quantified, on a pin that can go red.

    Judgement calls I want on the record

    • stop() retained as a delegating alias. It is public API of an exported class, and an embedder may have learned to call it directly precisely because the kernel never did. Removing it would have been the tidy choice and the wrong one.
    • The pin's pre-shutdown leg is a positive control, called out as load-bearing: without it, a dispatcher that never started would pass vacuously.
    • A vitest.config.ts with one anchored @objectstack/core alias, because a pin asserting a property of the kernel's teardown contract must run against this checkout's source, not a dist artifact.
    • Changeset, not skip-changeset — @objectstack/service-messaging is published and its runtime behaviour changes. Correct, and the opposite call from the sibling cards this seat landed today.

    ⛔ Ledger promotion is yours, not mine

    Per the signature-ledger rule this is a human action. The dev drafted a proposal and did not self-add it, exactly as instructed. It is in the PR body ready to promote, and it correctly records the residual risk: the upstream amplifier is unfixed (#10374), so a recurrence in a different package is a new instance of the class, not a regression of this fix.

    Filed, not fixed here — all three correctly scoped out

    ⚠️ #10371 is the one to look at next. It is the same defect in another published plugin, found by the same reasoning, and it is unfixed on main right now.


    Generated by Claude Code

  8. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor
    {
      "issue": 9371,
      "status": "done",
      "branch": "claude/issue-9371-vitest-teardown-race",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/10375",
      "premise_still_valid": true,
      "summary": "The race still reproduces on current main, and it is load-dependent exactly as the card measured — but the fault is not in test/approval-resume-relation-expand.test.ts, not in a showcase fixture, and not in vitest configuration. MessagingServicePlugin starts two setInterval dispatchers at kernel:ready and released them from a method named stop(); the kernel's only plugin teardown hook is destroy() (Plugin.destroy?() in core types.ts, the only one ObjectKernel.performShutdown() and LiteKernel.destroy() invoke), so nothing ever called it and both dispatchers kept ticking after await kernel.shutdown() resolved. start() unref()s the timers, so a long-lived process still exits and the leak is silent in production; under vitest the worker is alive throughout teardown, so a tick fires after the test file is over, reads a delivery table through a driver the suite already disconnected, and SqlDriver's console fallback warns. console.* in a worker is an unawaited RPC (sendLog discards the promise); one created after rpcDone() snapshots the pending set is rejected by $rejectPendingCalls as EnvironmentTeardownError, lands as an unhandled rejection, and fails a run in which every test passed. The window is the duration of rpcDone() — ~1ms idle, long on a saturated runner — which is the whole of the load-dependence and why the identical diff ran clean PR-side and red queue-side. Fix: the teardown body moves to destroy(); stop() is retained as a delegating alias (public API of an exported class). Landed with a deterministic pin and a changeset; @objectstack/service-messaging is published so this is NOT skip-changeset.",
      "tests": "All at head bd5293d83, working tree byte-identical to HEAD (git status --porcelain empty); exit codes captured before any pipe. REPRODUCTION (positive control, pre-fix): 4 concurrent full app-showcase suites x 3 rounds on this 4-vCPU box with a probe wrapping rpc.onUserConsoleLog produced the exact CI signature — 'RPCREJECT 30847 t=38057 id=137 err=EnvironmentTeardownError: [vitest-worker]: Closing rpc while \"onUserConsoleLog\" was pending' x6 in one worker, one $rejectPendingCalls sweep, with AFTERALL at t=37655 and eight console RPCs starting after it. Corroborated by vitest's own detector on the unmodified suite: 'Async Leaks 4 / PROMISE leaking in test/approval-resume-relation-expand.test.ts' (x4) — one file, the one both sightings name — alongside 'Test Files 21 passed (21) / Tests 342 passed (342)'. Instrumenting every console.* across the suite: 348 calls per run, 294 of them (85%) from that one file, from SchemaRegistry.log (222) and driver-sql's console fallback warn (120). Confirmed the calls are invisible: a probe's console.log never appeared in output while its process.stdout.write did — vitest 4's non-TTY default is MinimalReporter with silent:'passed-only', so the RPC is paid and the output discarded. PAIRED BEFORE/AFTER, identical harness, same box, N=48 loaded runs each side, dist rebuilt between legs and the rebuild verified on disk (grep 'async destroy()' in dist/index.js: 0 before, 1 after): console calls after the file's own afterAll 3 -> 0; console RPC round-trips 6574 -> 3456; RPCs never settled (RPCSTART-RPCOK) 3 -> 0; total console calls 14251 -> 11136. ABLATION: renamed destroy() -> ABLATED_destroy(), mutation confirmed on disk by grep anchored on the exact text ('async destroy(' 0 occurrences, 'async ABLATED_destroy(' 1) — no rebuild needed, the subject is imported by relative path so it resolves to source not dist. Ablated leg RED for the right reason: 'AssertionError: expected 94 to be 46' / 'Test Files 1 failed (1) / Tests 2 failed (2)' — 48 further delivery reads/writes in the 80ms after a resolved shutdown(). Restored from the commit (grep back to 1 and 0) and re-run GREEN: 'Test Files 1 passed (1) / Tests 2 passed (2)'. SUITES, EXIT=0 each: service-messaging 'Test Files 23 passed (23) / Tests 244 passed (244)' (23 test files on disk = 23 reported, so the new pin is in the run); service-messaging typecheck; app-showcase 'Test Files 21 passed (21) / Tests 342 passed (342)'; app-showcase typecheck. GATES: set derived with node scripts/pm/dispatch-gates.mjs with NO path args (it derives its own change set from the merge base), re-derived after the final commit; all 16 EXIT=0, quoting each gate's own verdict line — slot-lookup 'ratchet holds: 107 unswept site(s) in 25 file(s), none new'; check-test-source-alias 'OK — 72 packages with tests scanned'; check-type-source-resolution 'OK — 76 packages with a tsconfig.json scanned'; check-type-check-coverage 'OK — 64/77 workspace packages type-checked'; check-type-check-coverage --re-measure 'OK — 33 ledger entr(ies) re-measured, none above its recorded number'; check-engine-double-contract 'OK — 331 pinned, 133 in the DEBT ledger, 2 exempt'; where-matcher '0 silently-wrong and 0 unjudged matcher(s), none new'; query-options-erasure 'at the ceiling, no files added'; check-i18n-bundles 'OK (9 package(s))'; check-nul-bytes 'OK (scanned 6093 text file(s), no raw ASCII control bytes)'; empty-changeset 'No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)'; changeset-no-major 'introduces no major bump'; adr-0087-registration 'adds no declared-breaking changeset'; objectui-changeset, changeset-gate-self-tests, affected-docs all OK. Two gates went RED on the first pass and were fixed properly, never by widening a baseline: slot-lookup (my pin erased two getService lookups to any — now typed ObjectQL / IDataEngine) and check-test-source-alias (my pin imported @objectstack/core unaliased, i.e. from dist — now one anchored alias in a new packages/services/service-messaging/vitest.config.ts, because a pin on the KERNEL's teardown contract must judge this checkout's source).",
      "open_questions": [
        {
          "question": "Non-blocking, recorded rather than asked: the trigger is fixed but the upstream amplifier is not — vitest turns any late console.* into an unhandled rejection that fails a green run, so any other package whose harness leaves work running past a test file is exposed. Filed as #10374. Does the repo want to act on the class?",
          "options": [
            "A — leave it: #10374 records the mechanism and the diagnostic (vitest --detectAsyncLeaks names the exposed file), so the next sighting is minutes not days; act only on recurrence.",
            "B — sweep the class now: fix #10371 (plugin-reports has 5 timer sites behind an uncalled stop()) and add a guard failing any Plugin implementation that declares stop() and no destroy().",
            "C — raise the unawaited-rpc half upstream with vitest (flush console logs before $rejectPendingCalls, or do not reject a fire-and-forget notification into an unhandled rejection)."
          ],
          "recommendation": "A now, B when the devx queue has room. The measured business need is a queue that does not evict green PRs, and this card's occurrence is closed at the source; B is the durable version and cheap (one guard closes the whole class structurally, which is the declared=enforced shape this repo prefers), but it is a second card's worth of blast radius across six packages and should not ride here. C is real but slow and buys nothing we cannot get from B — and at startup scope, waiting on an upstream fix for a defect we can make unreachable ourselves is the worse trade."
        }
      ],
      "out_of_scope_findings": [
        "filed as #10371: plugin-reports releases 5 timer sites from stop(), which the kernel never calls — same defect as this card, different package; plus five further plugins with an uncalled stop() but no timers",
        "filed as #10373: examples/app-showcase approval-resume-relation-expand.test.ts disconnects the driver before shutting the kernel down, so every kernel drains its teardown against a dead driver",
        "filed as #10374 (finding): the upstream vitest amplifier is still live — any late console.* in any package can fail a fully green suite, and the non-TTY default reporter never prints that output, so suites pay RPC round-trips for logs nobody reads"
      ]
    }

    Generated by Claude Code

  9. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    os-dev-report

    Reposting the dev report verbatim: the previous comment carried the marker as an HTML comment and GitHub's body sanitizer stripped it on store, leaving that copy invisible to a marker scan. Same JSON, marker now as literal first-line text. Supersedes #9371 (comment) — no content differs.

    {
      "issue": 9371,
      "status": "done",
      "branch": "claude/issue-9371-vitest-teardown-race",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/10375",
      "premise_still_valid": true,
      "summary": "The race still reproduces on current main, and it is load-dependent exactly as the card measured — but the fault is not in test/approval-resume-relation-expand.test.ts, not in a showcase fixture, and not in vitest configuration. MessagingServicePlugin starts two setInterval dispatchers at kernel:ready and released them from a method named stop(); the kernel's only plugin teardown hook is destroy() (Plugin.destroy?() in core types.ts, the only one ObjectKernel.performShutdown() and LiteKernel.destroy() invoke), so nothing ever called it and both dispatchers kept ticking after await kernel.shutdown() resolved. start() unref()s the timers, so a long-lived process still exits and the leak is silent in production; under vitest the worker is alive throughout teardown, so a tick fires after the test file is over, reads a delivery table through a driver the suite already disconnected, and SqlDriver's console fallback warns. console.* in a worker is an unawaited RPC (sendLog discards the promise); one created after rpcDone() snapshots the pending set is rejected by $rejectPendingCalls as EnvironmentTeardownError, lands as an unhandled rejection, and fails a run in which every test passed. The window is the duration of rpcDone() — ~1ms idle, long on a saturated runner — which is the whole of the load-dependence and why the identical diff ran clean PR-side and red queue-side. Fix: the teardown body moves to destroy(); stop() is retained as a delegating alias (public API of an exported class). Landed with a deterministic pin and a changeset; @objectstack/service-messaging is published so this is NOT skip-changeset.",
      "tests": "All at head bd5293d83, working tree byte-identical to HEAD (git status --porcelain empty); exit codes captured before any pipe. REPRODUCTION (positive control, pre-fix): 4 concurrent full app-showcase suites x 3 rounds on this 4-vCPU box with a probe wrapping rpc.onUserConsoleLog produced the exact CI signature — 'RPCREJECT 30847 t=38057 id=137 err=EnvironmentTeardownError: [vitest-worker]: Closing rpc while onUserConsoleLog was pending' x6 in one worker, one $rejectPendingCalls sweep, with AFTERALL at t=37655 and eight console RPCs starting after it. Corroborated by vitest's own detector on the unmodified suite: 'Async Leaks 4 / PROMISE leaking in test/approval-resume-relation-expand.test.ts' (x4) — one file, the one both sightings name — alongside 'Test Files 21 passed (21) / Tests 342 passed (342)'. Instrumenting every console.* across the suite: 348 calls per run, 294 of them (85%) from that one file, from SchemaRegistry.log (222) and driver-sql's console fallback warn (120). Confirmed the calls are invisible: a probe's console.log never appeared in output while its process.stdout.write did — vitest 4's non-TTY default is MinimalReporter with silent:'passed-only', so the RPC is paid and the output discarded. PAIRED BEFORE/AFTER, identical harness, same box, N=48 loaded runs each side, dist rebuilt between legs and the rebuild verified on disk (grep 'async destroy()' in dist/index.js: 0 before, 1 after): console calls after the file's own afterAll 3 to 0; console RPC round-trips 6574 to 3456; RPCs never settled (RPCSTART-RPCOK) 3 to 0; total console calls 14251 to 11136. ABLATION: renamed destroy() to ABLATED_destroy(), mutation confirmed on disk by grep anchored on the exact text ('async destroy(' 0 occurrences, 'async ABLATED_destroy(' 1) — no rebuild needed, the subject is imported by relative path so it resolves to source not dist. Ablated leg RED for the right reason: 'AssertionError: expected 94 to be 46' / 'Test Files 1 failed (1) / Tests 2 failed (2)' — 48 further delivery reads/writes in the 80ms after a resolved shutdown(). Restored from the commit (grep back to 1 and 0) and re-run GREEN: 'Test Files 1 passed (1) / Tests 2 passed (2)'. SUITES, EXIT=0 each: service-messaging 'Test Files 23 passed (23) / Tests 244 passed (244)' (23 test files on disk = 23 reported, so the new pin is in the run); service-messaging typecheck; app-showcase 'Test Files 21 passed (21) / Tests 342 passed (342)'; app-showcase typecheck. GATES: set derived with node scripts/pm/dispatch-gates.mjs with NO path args (it derives its own change set from the merge base), re-derived after the final commit; all 16 EXIT=0, quoting each gate's own verdict line — slot-lookup 'ratchet holds: 107 unswept site(s) in 25 file(s), none new'; check-test-source-alias 'OK — 72 packages with tests scanned'; check-type-source-resolution 'OK — 76 packages with a tsconfig.json scanned'; check-type-check-coverage 'OK — 64/77 workspace packages type-checked'; check-type-check-coverage --re-measure 'OK — 33 ledger entr(ies) re-measured, none above its recorded number'; check-engine-double-contract 'OK — 331 pinned, 133 in the DEBT ledger, 2 exempt'; where-matcher '0 silently-wrong and 0 unjudged matcher(s), none new'; query-options-erasure 'at the ceiling, no files added'; check-i18n-bundles 'OK (9 package(s))'; check-nul-bytes 'OK (scanned 6093 text file(s), no raw ASCII control bytes)'; empty-changeset 'No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)'; changeset-no-major 'introduces no major bump'; adr-0087-registration 'adds no declared-breaking changeset'; objectui-changeset, changeset-gate-self-tests, affected-docs all OK. Two gates went RED on the first pass and were fixed properly, never by widening a baseline: slot-lookup (my pin erased two getService lookups to any — now typed ObjectQL / IDataEngine) and check-test-source-alias (my pin imported @objectstack/core unaliased, i.e. from dist — now one anchored alias in a new packages/services/service-messaging/vitest.config.ts, because a pin on the KERNEL's teardown contract must judge this checkout's source).",
      "open_questions": [
        {
          "question": "Non-blocking, recorded rather than asked: the trigger is fixed but the upstream amplifier is not — vitest turns any late console.* into an unhandled rejection that fails a green run, so any other package whose harness leaves work running past a test file is exposed. Filed as #10374. Does the repo want to act on the class?",
          "options": [
            "A — leave it: #10374 records the mechanism and the diagnostic (vitest --detectAsyncLeaks names the exposed file), so the next sighting is minutes not days; act only on recurrence.",
            "B — sweep the class now: fix #10371 (plugin-reports has 5 timer sites behind an uncalled stop()) and add a guard failing any Plugin implementation that declares stop() and no destroy().",
            "C — raise the unawaited-rpc half upstream with vitest (flush console logs before $rejectPendingCalls, or do not reject a fire-and-forget notification into an unhandled rejection)."
          ],
          "recommendation": "A now, B when the devx queue has room. The measured business need is a queue that does not evict green PRs, and this card's occurrence is closed at the source; B is the durable version and cheap (one guard closes the whole class structurally, which is the declared=enforced shape this repo prefers), but it is a second card's worth of blast radius across six packages and should not ride here. C is real but slow and buys nothing we cannot get from B — and at startup scope, waiting on an upstream fix for a defect we can make unreachable ourselves is the worse trade."
        }
      ],
      "out_of_scope_findings": [
        "filed as #10371: plugin-reports releases 5 timer sites from stop(), which the kernel never calls — same defect as this card, different package; plus five further plugins with an uncalled stop() but no timers",
        "filed as #10373: examples/app-showcase approval-resume-relation-expand.test.ts disconnects the driver before shutting the kernel down, so every kernel drains its teardown against a dead driver",
        "filed as #10374 (finding): the upstream vitest amplifier is still live — any late console.* in any package can fail a fully green suite, and the non-TTY default reporter never prints that output, so suites pay RPC round-trips for logs nobody reads"
      ]
    }

    Generated by Claude Code

  10. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    PM ruling on the open question — B is the destination, sequenced through #10371. ⛔ Not a rider here.

    Your recommendation was "A now, B when the devx queue has room." I am taking B, and moving it sooner than "when there is room", for one reason your own report supplies: #10371 is not hypothetical. plugin-reports has 5 timer sites behind an uncalled stop(), in a published package, on main, right now. That is the same defect that just cost the merge queue two full rebuilds — it has simply not been unlucky yet.

    Option A is what you do when the class is theoretical. It is not; you measured six instances.

    The half that makes B worth more than six fixes

    add a guard failing any Plugin implementation that declares stop() and no destroy()

    ⭐ That guard is the whole value, and it should not be treated as the optional half. The six instances are the symptom; the cause is that Plugin has an optional destroy?() and nothing anywhere objects to a class that implements a plausible-looking teardown method the interface never declares. Fix six plugins and the seventh arrives next month spelled shutdown() or close(). Add the guard and the class becomes unreachable — which is the declared-equals-enforced shape this repo reaches for everywhere else, and exactly the argument that carried #10086 (one predicate, 33 call sites, plus a gate that refuses the other spellings).

    Note the guard is also cheap to make non-vacuous, which is not always true of a class-closing gate: its positive control writes itself, because MessagingServicePlugin on main before this PR is a real fixture that must red.

    ⛔ Correctly not ridden on this PR. Six packages of blast radius does not belong in a card that root-causes a queue flake, and you were right to keep it out.

    Option C — declined, for your reason

    at startup scope, waiting on an upstream fix for a defect we can make unreachable ourselves is the worse trade

    Agreed. C is real (#10374 records it) but it is slow, it is not ours to schedule, and it buys nothing B does not. ⚠️ Keep #10374 open regardless — it is the reason a recurrence in a different package is a new instance of the class rather than a regression of this fix, and the ledger proposal you drafted says exactly that.

    What happens next

    #10371 is filed, open, and ungraded — no domain:*, no type, no pm:*. ⛔ Grading is triage's single-producer field, not this seat's, so I am not setting it. I have noted the urgency and the guard there so it is levelled with the right information rather than as an ordinary tidy-up.


    Everything else in the report matches what I verified independently before accepting: Plugin declares init() / start?(ctx) / destroy?() and no stop(); MessagingServicePlugin had async stop() at :382 documented as shutdown, and no destroy(); nothing in packages/core/src calls .stop().

    Two things I want to name because they are easy to skip past:

    • The two gates that went red first were fixed properly, not baselined. slot-lookup (your pin erased two getService lookups to any — now typed ObjectQL / IDataEngine) and check-test-source-alias (your pin imported @objectstack/core from dist — now one anchored alias, because a pin on the kernel's teardown contract must judge this checkout's source). Both were your own pin's defects, caught by the repo's gates, and repaired at the cause.
    • You proved the console output is invisible, not merely noisy: a probe's console.log never appeared while its process.stdout.write did — vitest 4's non-TTY default is MinimalReporter with silent: 'passed-only'. So the suite pays 6574 RPC round-trips for output nobody reads. That is what makes finding: any late console.* in a vitest worker can fail a fully green suite — the #9371 amplifier is upstream and still live #10374 a real finding rather than a stylistic complaint.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions