ci: isolate Bun test shards into fresh-process batches - #1469
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughLinux CI replaces direct Bun shard execution with deterministic fresh-process batching. The helper validates configuration, assigns tests to shards, classifies failures, and retries runtime crashes or timeouts per file. Workflow tests verify API and storage test inclusion. ChangesBun CI batching
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant BatchHelper
participant BunTestProcess
CIWorkflow->>BatchHelper: pass TEST_SHARD and batch settings
BatchHelper->>BunTestProcess: run a bounded test batch
BunTestProcess-->>BatchHelper: return status and log output
BatchHelper->>BunTestProcess: retry affected files individually after timeout or runtime crash
BatchHelper-->>CIWorkflow: return shard status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
Wibias
marked this pull request as ready for review
August 11, 2026 10:00
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 236-239: Update the workflow comment describing
scripts/ci/run-bun-test-batches.sh to document both one-time Bun runtime-crash
recovery and the one-time retry for singleton files after a timeout, while
preserving the statement that ordinary test failures are not retried.
In `@tests/zz-ci-api-usage-isolation.test.ts`:
- Around line 28-31: Update the assertions in
tests/zz-ci-api-usage-isolation.test.ts lines 28-31 to verify that the
tests/api-usage.test.ts matching branch returns 1, rather than only checking
filename text. Update tests/zz-ci-storage-policy-isolation.test.ts lines 28-32
to verify that its storage-policy patterns are within the exclusion branch
returning 1; both sites require direct assertion changes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b9fca9d-3ef0-40fb-91c5-11b128cb21db
📒 Files selected for processing (4)
.github/workflows/ci.ymlscripts/ci/run-bun-test-batches.shtests/zz-ci-api-usage-isolation.test.tstests/zz-ci-storage-policy-isolation.test.ts
Comment on lines
+236
to
+239
| # `scripts/ci/run-bun-test-batches.sh` mirrors Bun's sorted round-robin shard | ||
| # assignment, then runs each shard in small batches so every batch gets a fresh | ||
| # Bun process. The helper prints the exact files before each batch and retries | ||
| # only a Bun runtime crash once; ordinary test failures are never retried. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document timeout retries.
Lines 238-239 state that the helper retries only a Bun runtime crash. scripts/ci/run-bun-test-batches.sh also retries a singleton file once after a timeout. Update this text to describe both runtime-crash and timeout recovery. Otherwise, CI operators can misdiagnose an expected retry as unexpected behavior.
Proposed fix
- # Bun process. The helper prints the exact files before each batch and retries
- # only a Bun runtime crash once; ordinary test failures are never retried.
+ # Bun process. The helper prints the exact files before each batch and isolates
+ # runtime crashes and timeouts per file. Ordinary test failures are never retried.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # `scripts/ci/run-bun-test-batches.sh` mirrors Bun's sorted round-robin shard | |
| # assignment, then runs each shard in small batches so every batch gets a fresh | |
| # Bun process. The helper prints the exact files before each batch and retries | |
| # only a Bun runtime crash once; ordinary test failures are never retried. | |
| # `scripts/ci/run-bun-test-batches.sh` mirrors Bun's sorted round-robin shard | |
| # assignment, then runs each shard in small batches so every batch gets a fresh | |
| # Bun process. The helper prints the exact files before each batch and isolates | |
| # runtime crashes and timeouts per file. Ordinary test failures are never retried. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 236 - 239, Update the workflow comment
describing scripts/ci/run-bun-test-batches.sh to document both one-time Bun
runtime-crash recovery and the one-time retry for singleton files after a
timeout, while preserving the statement that ordinary test failures are not
retried.
Comment on lines
+28
to
+31
| const batchHelper = await Bun.file( | ||
| new URL("../scripts/ci/run-bun-test-batches.sh", import.meta.url), | ||
| ).text(); | ||
| expect(batchHelper).toContain("tests/api-usage.test.ts)"); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the exclusion result, not only filename text.
These assertions pass if the filename appears in a comment or if the matching case branch returns 0. A regression can then run dedicated tests in general shards and remove the intended isolation.
tests/zz-ci-api-usage-isolation.test.ts#L28-L31: assert that thetests/api-usage.test.tsbranch returns1.tests/zz-ci-storage-policy-isolation.test.ts#L28-L32: assert that the storage-policy patterns are in the exclusion branch that returns1.
Proposed fix
- expect(batchHelper).toContain("tests/api-usage.test.ts)");
+ expect(batchHelper).toMatch(
+ /tests\/api-usage\.test\.ts\)\s*\n\s*return 1/,
+ );- expect(batchHelper).toContain("tests/api-storage-policy*.test.ts");
- expect(batchHelper).toContain("tests/api-storage.test.ts");
+ expect(batchHelper).toMatch(
+ /tests\/api-storage-policy\*\.test\.ts\|tests\/api-storage\.test\.ts\|tests\/api-usage\.test\.ts\)\s*\n\s*return 1/,
+ );📍 Affects 2 files
tests/zz-ci-api-usage-isolation.test.ts#L28-L31(this comment)tests/zz-ci-storage-policy-isolation.test.ts#L28-L32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/zz-ci-api-usage-isolation.test.ts` around lines 28 - 31, Update the
assertions in tests/zz-ci-api-usage-isolation.test.ts lines 28-31 to verify that
the tests/api-usage.test.ts matching branch returns 1, rather than only checking
filename text. Update tests/zz-ci-storage-policy-isolation.test.ts lines 28-32
to verify that its storage-policy patterns are within the exclusion branch
returning 1; both sites require direct assertion changes.
3 tasks
3 tasks
lidge-jun
added a commit
that referenced
this pull request
Aug 14, 2026
…eline Local: tsc exit 0; 14 shutdown-hook + 8 linker + 16 CL-09 + 52 lab-automation + 11 repo-hygiene all green. Module graph, runtime imports only: lifecycle.ts 69 -> 0 reachable src/lab modules. router.ts and responses/core.ts remain at 24 through a single traced chain (router -> assemble -> catalog -> lab/query/catalog), which is phase 3. Both boundary guards were driven red before being trusted: reintroducing the Lab import into responses/core.ts failed the new boundary test and the inverted CL-09 guard, and the phase-1 scheduler leak showed runningAfter=true before its fix. The remote Linux full suite reported 127 failures. Recorded as a pre-existing full-suite condition rather than absorbed silently, on three independent grounds: no Lab or boundary test is among them; every sampled failing file passes standalone on the same host and commit; and a clean dev baseline at c6688c7 on the same runner accumulated failures on the same trajectory (19 -> 112) and converges toward the branch count. That is cross-file interference in an unsharded run -- which is why CI shards the suite into four fresh-process batches (#1469). The authoritative full-suite signal for this branch is CI on the PR, not one unsharded host run.
lidge-jun
added a commit
that referenced
this pull request
Aug 14, 2026
…who never opted in (#1681) * docs(devlog): Lab/core decoupling roadmap + owner-only core CODEOWNERS The Compatibility Lab reaches the proxy core on the mandatory path: a user with no routing profile still executes Lab code on every request and loads the Lab module graph at startup. The measured cost is small; the architectural cost is not, and there is no way to decline it. Roadmap unit devlog/_plan/260814_lab_core_decoupling/ records six verified coupling points and the import cycle that makes them expensive: routing/compatibility/assemble -> routing/quota -> providers/quota -> codex/auth-api -> codex/native-main-admission -> server/lifecycle -> lab/automation/orchestrator That cycle pulls ~69 src/lab files into the graph, so cutting the lifecycle edge is phase 1 rather than the most visible symptom. Design is registration, not dynamic import: core declares slots, Lab registers into them at activation. Routing stays synchronous because making it async to relocate an import would touch ~283 call sites and break the sync subagent-fallback API. 060 records an independent adversarial audit (FAIL, 5 blockers) and the amendments. The most serious: the planned deferred-activation window was fail-closed, not degrade-open -- compatibility unknownEvidence defaults to exclude, so a policy request in that window throws NoEligiblePolicyCandidateError rather than losing evidence. Phase 3 now awaits activation before policy routing. CODEOWNERS adds an owner-only block for the four core files, placed after /src/server/ so last-match-wins takes effect. Branch protection on dev now requires code-owner review. No runtime code changes in this commit. * docs(devlog): audit rounds 2-3 — remove the activation window entirely Round 2 (FAIL, 4 blockers) found that the round-1 readiness-gate fix introduced worse defects than it closed. The reviewer proved three by executing the real modules: during a deferred-activation window a policy alias in a subagent fallback chain is silently skipped and the subagent runs on a different model than the operator configured -- no error, no log, only a skipped entry. The reviewer's key observation: that window does not exist today. Static imports make compatibility evidence load synchronously. My own deferral created it. 070 removes deactivation from scope (reconcile only ever activates; a create-then-delete user keeps Lab resident until restart, which does not affect users who never opted in). 080 is a self-correction made while round 3 was in flight. I verified that readinessGate is NOT admission control -- it appears three times in server/index.ts and the only read is inside the /ready response body at :852, a status report for external supervisors. So 070's A1 rested on a false premise. The resolution is to never make activation asynchronous. server/index.ts already imports Lab statically and already runs its startup block synchronously; the protected set is corrected to three files, with server/index.ts treated as an unprotected composition root. A composition root is supposed to know which optional subsystems exist. Bun.serve binds at :1638 and the Lab block sits at :1738, so I verified the gap: the three awaits in that range are inside the server.stop closure, the one .then is fire-and-forget quota priming, and scheduleStartupRun is documented as never blocking listen. The path is synchronous, so no request can be handled before activation. That ordering is now stated as an invariant and phase 4 asserts it. No runtime code changes in this commit. * docs(devlog): audit round 3 close-out — GO-WITH-FIXES, no High blockers Reviewer verified the corrected design empirically: with phase-1/2 cuts simulated, router.ts, lifecycle.ts, and responses/core.ts each reach zero src/lab modules, and none transitively reaches server/index.ts. Two independent confirmations. Startup ordering: the only awaits between Bun.serve (:1638) and return server (:1752) are inside the server.stop closure, and startServer is non-async, so the sync subagent-fallback path cannot observe an unregistered slot. Loaded vs executed: Lab modules do no import-time work -- ensureLabDirs is called from function bodies, never top level, and the single module-level allocation is an empty Map. A static import in the composition root creates no directory, opens no SQLite handle, and starts no timer. Two findings folded in. R3-1 (Medium): the dry-run endpoint assembles candidate evidence outside the activation gate, so an operator preview would disagree with production AND the behavioral guard would pass while the property is violated -- fixed by activating on the create and dry-run paths, and by extending the guard. R3-2 (Low): runtime activation must resolve configDir the way startup does. Also recorded: the reviewer disclosed that its probe started a real server and triggered the #1610 model-rename migration on the live user config. Assessed and left as-is -- idempotent, the same change any ocx start applies, user data outside the repo, working tree unaffected. The missing isolation instruction was a defect in my dispatch packet and is now part of phase 4 test guidance. No runtime code changes in this commit. * refactor(server): break the Lab import cycle at the shutdown path server/lifecycle.ts imported lab/automation/orchestrator for two teardown calls. That single edge closed a cycle: routing/compatibility/assemble -> routing/quota -> providers/quota -> codex/auth-api -> codex/native-main-admission -> server/lifecycle -> lab/automation/orchestrator so any module reaching lifecycle.ts dragged the Lab graph in with it, including for installs with no routing profile. Core now owns a shutdown-hook registry and never names Lab. Lab registers its teardown inside setLabAutomationDispatchDeps, next to the existing registerCurrentServerResourceCleanup lease, so activation and teardown registration cannot drift apart. A process that never activates Lab registers nothing and shutdown does no work. Measured on the module graph, runtime imports only: src/server/lifecycle.ts 69 -> 0 reachable src/lab modules src/router.ts 24 (phase 2/3 scope, via compatibility/subject) src/server/responses/core.ts 24 (phase 2/3 scope, same path) One deliberate behavior change: teardown is now scoped to the activated configDir, where the previous shutdown call passed none and keyed on the default. setLabAutomationDispatchDeps is per-configDir, so those disagreed in multi-config test processes. Verification: bun x tsc --noEmit exit 0; 7 new hook tests pass; 52 lab-automation tests across 7 files pass; repo-hygiene and lab-passive-production-evidence pass. Phase 1 of devlog/_plan/260814_lab_core_decoupling/. * fix(lab): register scheduler teardown where the scheduler is started Phase 1 registered the shutdown hook only inside setLabAutomationDispatchDeps. That covers the startup path, but the management API and the CLI can start a scheduler without ever installing dispatch deps: server/management/lab-automation-routes.ts:69 applySchedulerPolicy cli/lab.ts:399 In those cases no hook existed, and since core no longer imports the orchestrator to stop it, the scheduler survived drainAndShutdown -- a live interval leaking past shutdown. Enabling Lab automation from the dashboard was enough to hit it. Proven before fixing: a probe asserting the scheduler stops after runOptionalShutdownHooks failed with runningAfter=true, then passed after the change. That probe is now a permanent regression test. startLabAutomationScheduler registers its own keyed teardown, so the hook exists whenever a timer exists regardless of which entry point created it. The two keys are distinct (lab-automation: vs lab-automation-scheduler:), and the registry replaces by key, so repeated starts cannot accumulate hooks. Verification: bun x tsc --noEmit exit 0; 60 tests pass across 8 files (52 lab-automation + 8 shutdown-hook). Found by my own pre-audit of phase 1 while the reviewer was running. * test(lab): cover the reviewer-reproduced shutdown cases at outcome level Phase 1's tests exercised the registry with inline closures only, so none of them imported the orchestrator and none asserted that a REAL Lab scheduler is stopped. 010:168-169 had called for exactly that outcome-level assertion and it was missing from the diff -- which is why the scheduler leak reached review. Adds the four cases an independent audit reproduced in an isolated configDir: case D startup activates, its lease is released, then PUT /api/lab/automation restarts the scheduler with no deps -- the production trigger, needing no unusual setup case B setLabAutomationDispatchDeps({}) early-returns a no-op release without registering anything case C ordinary activation still torn down double shutdown running twice is safe and idempotent Case D is the one I missed: my own probe covered only the never-activated path. All four fail against the pre-fix orchestrator and pass against 00a345b. Verification: bun x tsc --noEmit exit 0; 75 tests pass across 9 files. * test(lab): pin the two-key scheduler hook interaction An activated config now carries two hooks -- lab-automation: from the deps lease and lab-automation-scheduler: from the timer. Independent review probed the interactions (cases E-H) and found them sound; these tests keep them that way. repeated starts three startLabAutomationScheduler calls leave one working hook, guarded twice: the existing-timer early return skips re-registration, and the registry replaces by key restart a scheduler restarted after a completed shutdown re-arms, so the registry is not one-shot Verification: bun x tsc --noEmit exit 0; 77 tests pass across 9 files. * refactor(responses): move Lab route linkage off the per-request path responses/core.ts called resolveProductionRouteSubject on every request attempt -- streaming and non-streaming, once per attempt and once per combo child -- with no way to turn it off. The call is not free even when it produces nothing: resolveCompatibilitySubjectsForInboundWire builds a Lab protocol subject and digests it before it checks for an installation salt, so an install with no routing profile ran Lab digest work per request and discarded it. Core now holds a nullable slot. It resolves to null unless an opt-in subsystem registered a linker, and the non-throwing guarantee lives in the slot helper rather than being restated at the call site. The Lab implementation moves to src/lib/lab-passive-linker-registration.ts, alongside the other lab-* host integrations, to be installed at activation in phase 3. CL-09 keeps working for installs that opt in: labRouteSubjectId is unchanged in usage/log.ts, and passive-production, the management route, the CLI, and the Compatibility Matrix all read the same field. Nothing is retired. The CL-09 architecture guard is inverted rather than deleted, as 020 planned: it asserted core CONTAINS resolveProductionRouteSubject, and now asserts core contains neither it nor any routing/compatibility import, with the positive assertion moved onto the registration module. responses/core.ts still reaches Lab transitively through router.ts -> routing/compatibility/assemble.ts -> catalog.ts -> lab/query/catalog.ts. That is phase 3 scope and is why the remaining count is unchanged at 24. Verification: bun x tsc --noEmit exit 0; 49 tests pass across 4 files. Phase 2 of devlog/_plan/260814_lab_core_decoupling/. * docs(devlog): record WP1 verification evidence and the full-suite baseline Local: tsc exit 0; 14 shutdown-hook + 8 linker + 16 CL-09 + 52 lab-automation + 11 repo-hygiene all green. Module graph, runtime imports only: lifecycle.ts 69 -> 0 reachable src/lab modules. router.ts and responses/core.ts remain at 24 through a single traced chain (router -> assemble -> catalog -> lab/query/catalog), which is phase 3. Both boundary guards were driven red before being trusted: reintroducing the Lab import into responses/core.ts failed the new boundary test and the inverted CL-09 guard, and the phase-1 scheduler leak showed runningAfter=true before its fix. The remote Linux full suite reported 127 failures. Recorded as a pre-existing full-suite condition rather than absorbed silently, on three independent grounds: no Lab or boundary test is among them; every sampled failing file passes standalone on the same host and commit; and a clean dev baseline at c6688c7 on the same runner accumulated failures on the same trajectory (19 -> 112) and converges toward the branch count. That is cross-file interference in an unsharded run -- which is why CI shards the suite into four fresh-process batches (#1469). The authoritative full-suite signal for this branch is CI on the PR, not one unsharded host run. * refactor(routing): complete the Lab/core boundary with a provider slot All three protected core files now reach ZERO src/lab modules: src/router.ts 24 -> 0 src/server/lifecycle.ts 69 -> 0 (phase 1) src/server/responses/core.ts 24 -> 0 routeModelInternal stays synchronous. Making it async to permit a dynamic import would touch ~283 call sites and break the sync subagent-fallback API (isNativeModelQuotaExhausted, isModelHealthBlocked, selectAvailableSubagentModel and friends have nowhere to await), so the seam is a nullable provider slot instead. assemble.ts keeps capability, health, quota, and cost -- the evidence routing always needs -- and consults the slot for compatibility. The Lab-reaching half (subject resolution, catalog snapshot, projection read, attachCompatibilityEvidence) moves verbatim into lab-evidence-provider.ts; its state already arrived entirely through arguments, so this is a relocation, not a rewrite. With no provider registered the evaluator sees no compatibility evidence and scores exactly as it did before compatibility policy existed. Per audit B5 the options type splits along the same seam: CoreEvidenceOptions carries configDir and routedProviderConfig, while the three Lab test seams belong to LabCompatibilityProviderOptions. AssemblePolicyEvidenceOptions remains as an alias so existing callers keep compiling. Activation is synchronous and gated. server/index.ts calls activateLab only when labActivationRequired -- any routing profile, or automation enabled on disk -- and does so in the same synchronous turn as Bun.serve, so no request can observe an unregistered slot. Three audit rounds established that a deferred window is unpatchable: the sync fallback chain would silently drop a policy alias and run the subagent on a different model than the operator configured. labAutomationEnabledOnDisk mirrors loadLabAutomationConfig precedence (automation-config.json first, automation-policy.json legacy) with plain node:fs, so the detector never imports Lab to decide whether to import Lab. Audit R3-1 and R3-2 are closed: the dry-run endpoint and the profile-create path both activate first, so an operator preview cannot disagree with production, and both resolve configDir the way the startup block does. tests/core-lab-boundary.test.ts enforces the property with a transitive import-graph walk, not a regex -- the original defect hid in a six-hop chain where no single file looked wrong. Driven red: reintroducing a direct Lab import into router.ts failed both guards and printed the chain 'src/router.ts -> src/lab/paths.ts'. Verification: bun x tsc --noEmit exit 0; 84 tests pass across 6 boundary/Lab files; 34 routing tests pass; 6 boundary guards pass. Phase 3 of devlog/_plan/260814_lab_core_decoupling/. * test(boundary): close a real hole in the guard and pin it against attack I attacked my own guard instead of trusting it, and defeated it: a top-level `void import("./lab/paths")` in a protected file passed cleanly while loading Lab at runtime. The walker matched static imports, side-effect imports, and runtime re-exports, but not dynamic import(). The regex now covers all four forms, and the four attacks plus a type-only negative case are permanent tests that synthesize each import shape against a temporary probe file. A guard that only matches the shapes which happen to exist today would rot silently -- these fail if the walker regresses. Type-only imports stay excluded: they are erased at build time, so they are not runtime edges, and the negative test pins that distinction rather than leaving it implicit. Verification: bun x tsc --noEmit exit 0; 11 tests pass. Each attack was confirmed red before the fix (dynamic import: 0 failures before, 2 after). * fix(lab): an invalid automation config must not take startup down startLabAutomationScheduler runs the full automation normalizer, which throws LabAutomationError on any field violation. labAutomationEnabledOnDisk only checks policy.enabled, so a parseable file with enabled:true but missing optional fields passed the gate and then threw -- out of activateLab, out of startServer, after Bun.serve had already bound. This sat on the startup path of every install with a routing profile, because activateLab reaches the scheduler branch regardless of why activation was required. Reproduced in an isolated configDir: threw 'invalid policy layers', provider slot registered, activation record absent -- so the receipts were orphaned and a later activateLab would register the slots a second time. A partially written automation file now disables Lab automation for the run and logs what to fix; routing, evidence, and everything else keep working. The activation record is stored before the scheduler call so a throw cannot leave slots and record inconsistent. The pre-phase-3 code read the legacy file through loadLabAutomationPolicy and never ran the combined-config normalizer, so this was a regression introduced by the boundary work, not a pre-existing condition. Also from the audit: the activation-key invariant is now written down (activation is all-or-nothing and reason-independent, which is what makes configDir a safe key -- if a registration ever becomes conditional, the key must include the reason), and the guard's known limits are stated rather than implied (a static walker cannot resolve computed specifiers; require() is unavailable in an ESM package). Adds tests/lab-activation.test.ts (10) covering the crash regression, the bare install registering nothing, idempotence, the automation-only-then-profile ordering trap, and all six detection-precedence cases; plus tests/compatibility-provider-equivalence.test.ts pinning that every candidate gets a compatibility object when requirements exist -- including unresolvable ones -- and none when no provider is registered. Verification: bun x tsc --noEmit exit 0; 57 tests pass across 4 files. * fix(lab): distinguish a busy state lock from an invalid automation config Independent review reproduced a 5018ms startup stall by holding the automation state lock with a live PID: activation waited out the 5s lock timeout, then logged 'automation config is invalid' and continued. The file was fine -- the cause was contention -- so the message sent the operator to fix the wrong thing. The two causes now get different messages because they need different actions, and the lock case says it will retry on the next start rather than implying corruption. Also records why the failure asymmetry is deliberate: startup degrades with a warning, while the management API and CLI let LabAutomationError surface (the route maps it to a 400). Someone toggling automation should see the validation error; someone merely starting the proxy should not lose unrelated traffic. Adds coverage for a failed scheduler start leaving nothing dangling: the shutdown hook is registered before the throw, so a hook exists with no timer behind it -- verified harmless (no running scheduler, hooks run without throwing). Test isolation fix: the provider and linker slots are process-global, so a sibling test file that registered one leaked into the bare-install assertion. beforeEach now resets the slots themselves, not just the activation record. Verification: bun x tsc --noEmit exit 0; 80 tests pass across 6 files. * docs(agents): record the optional-subsystem boundary invariant A prose rule would not have caught the original violation -- CL-01 through CL-09 each passed CI and automated review -- so the enforcement is tests/core-lab-boundary.test.ts. This entry exists so a contributor meets the rule before CI does, and understands why it is not a style preference: the violation hid in a six-hop chain where no single file looked wrong. Also records the two obligations that are easy to break by accident: activation must stay behind labActivationRequired, and it must stay synchronous. The second is load-bearing -- everything between Bun.serve and the return of startServer runs in one synchronous turn, and the subagent-fallback chain has nowhere to await, so an await added before the activation block would silently reroute subagents to a different model than the operator configured. server/index.ts is exempt by design: a composition root is supposed to know which optional subsystems exist. Its obligation is the gate, not the import. * docs(devlog): record the sharded verification for phase 3 Four shards, matching how CI runs the suite: 11,916 tests, 0 failures, every shard exit 0. The new boundary suites were picked up by the shards rather than only by focused local runs. This also settles the earlier 127-failure unsharded result: the same tree passes clean when run the way CI runs it, which confirms that number was cross-file interference rather than a defect in this work. Records the guard red-runs including the dynamic-import hole -- the one attack that passed until it was fixed. * docs(devlog): record PR #1681 and the CI result 24 checks pass, 0 failures. All four CI test shards green, which independently confirms the lidge run and closes out the earlier unsharded 127-failure observation. Records why the release is deferred rather than skipped: MAINTAINERS.md makes promotion maintainer-controlled and the release runs from dev after the PR lands. Releasing from a feature branch would violate the branch policy this unit just tightened.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
bun testprocess with deterministic fresh-process batches.storage-policyandapi-usagejobs, plus the unsharded macOS control and dispatch-only Windows lane.BUN_TEST_BATCH_SIZE; the timeout is configurable throughBUN_TEST_BATCH_TIMEOUT_SECONDS.Verification
bash -n scripts/ci/run-bun-test-batches.sh4fb67f8supplied the root-cause evidence for the adaptive fallback:tests/cli-help.test.tsand then hung until the outer 120-second timeout on both attemptstests/cli-status-json.test.tsand then hung until the same timeout on both attemptsbun testinvocation still runs its selected files inside one Bun processfb38f4breplaces the repeated whole-batch retry with singleton process isolation after a runtime crash or timeout, preserving fast 12-file batches on the healthy path.Checklist
Summary by CodeRabbit