Uh oh!
There was an error while loading. Please reload this page.
fix(app-showcase): shut down kernels before disconnecting drivers in approval-resume-relation-expand.test.ts - #10767
Merged
Conversation
…approval-resume-relation-expand.test.ts The afterEach in this test disconnected the drivers before shutting down the kernels, so every kernel's own teardown ran against a driver that was already gone. Swap the order: stop the producers, then remove the resource they depend on. packages/services/service-messaging/src/plugin-shutdown-stops-dispatchers.test.ts (#9371) already does it in the right order; the comment explaining why is carried across verbatim. Measured (not assumed): on current main this file's own DATABASE_ERROR console lines are all "no such table" probes against sys_organization/ sys_user/sys_member/sys_user_position/sys_user_permission_set/sys_position (tables this harness's bootShowcaseApprovals() never provisions) -- not post-disconnect reads. Before/after/ablation-restore all measure identically (31 DATABASE_ERROR lines in isolated runs, 36 in the full 24-file suite, byte-for-byte identical breakdown each time), so the reordering is a no-op for this file's console volume today. The fix still stands on the ordering principle itself, not on savings: draining a kernel against an already-disconnected driver is the wrong order regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0f14f70b-575c-5f2b-a235-4000a55db042
The comment landed in the previous commit asserted that reversing the kernel/driver teardown order is what makes this file's DATABASE_ERROR lines appear -- but the PR's own before/after measurement shows the opposite: those lines are "no such table" probes bootShowcaseApprovals() never provisions, unaffected by the swap (36/31, identical before and after). Restate the comment as two separate claims: the ordering rule (why the change is correct regardless of today's cost) and the measured fact about this file's own DATABASE_ERROR lines (why the swap didn't move them). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
os-warren
marked this pull request as ready for review
August 21, 2026 11:33
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#10373
The defect
examples/app-showcase/test/approval-resume-relation-expand.test.ts'safterEachdisconnected the drivers before shutting down the kernels. Every kernel's own teardown — hook dispatch, each plugin'sdestroy(), each service's drain of in-flight work — then ran against a driver that was already gone. The correct order is the reverse: stop the producers, then remove the resource they depend on.packages/services/service-messaging/src/plugin-shutdown-stops-dispatchers.test.ts(added in #9371) already does it in the right order. This file's comment states the same ordering rule, plus what this PR's own measurement found about this file specifically (see below — the rule and the measurement are two different kinds of claim, kept separate on purpose):(An earlier revision of this comment borrowed #9371's parenthetical verbatim — "reversing these is what makes a suite shout DATABASE_ERROR" — which is true of
service-messaging, where it was measured, but not true of this file, where the measurement below shows the opposite. Fixed so the comment doesn't re-plant the same misattribution the card exists to correct.)Measured console volume — three numbers, and why the first two disagree by an order of magnitude
main, this PR's parent commit)The 348/294/120 vs. 36/31 gap is the #9371 effect the card itself warned about, not a methodology disagreement between the two readings. The card's numbers predate #9371 landing (
fix(service-messaging): stop the delivery dispatchers on the kernel's own teardown hook, merged as #10375) — that PR already removed the dominant source of this traffic across the whole suite. Both readings are correct measurements; they are measurements of two different trees, taken ~30 hours apart with a merge in between. Neither number is wrong.On today's
main, the reordering changes nothing measurable for this file. Before and after are byte-for-byte identical: full-suite console output is 341 lines / 36DATABASE_ERRORcalls in both runs (identical per-table breakdown: 11sys_user, 9sys_organization, 7sys_member, 7sys_user_position, 1sys_position, 1sys_user_permission_set), and this file in isolation is 31DATABASE_ERRORcalls in both runs.Why: every
DATABASE_ERRORline this file currently produces isno such table: sys_*— a probe against system tables (sys_organization,sys_user,sys_member,sys_user_position,sys_user_permission_set,sys_position) thatbootShowcaseApprovals()never provisions (it registersAccount/Project/Task/Product/Invoice/InvoiceLineonly), hit mid-test during approval/position resolution — not a post-disconnect teardown read. The teardown-race class this card targets has already been eliminated for this file's traffic by #9371; what's left is an unrelated, pre-existing gap unaffected by driver/kernel ordering either way.This is still the correct fix. Per the card: draining a kernel against an already-disconnected driver is the wrong order even on a day when it's quiet. The reorder is justified on the ordering principle, not on savings realized today — and now the in-code comment says that too, instead of implying a saving that this file never had.
Ablation
Predicted before running it (written down first): reverting the order should restore the file to byte-identical to its pre-PR blob, and — because before/after already measured identically — the DATABASE_ERROR count on revert should also stay ~31 for the isolated run, not "come back" from some lower number.
Observed:
git hash-objecton the reverted file =a9fd1ae5c5fc52ee59ba81e4d7f35157ece1027c, exactly the pre-PR blob. Isolated DATABASE_ERROR count on the reverted file: 31, identical per-table breakdown to both the before and after runs. Prediction confirmed.Tests
pnpm --filter @objectstack/example-showcase test -- --maxWorkers=2, one run, both before and after the swap: 24/24 files, 364/364 tests passing, exit 0. Not a correctness fix — the assertions passed before this PR too — this is teardown-order hygiene plus an honest re-measurement.Gates
Local gates run per
node scripts/pm/dispatch-gates.mjs(no path args, off the final commit) — see the PR report comment on #10373 for the full table with quoted verdict lines.Generated by Claude Code