fix(test): isolate smithery-env-trust first-spawn cold-start under CI contention - #3970
Conversation
… contention Root cause: the first Bun probe child absorbs cold compile of the probe + env module graph. Under coding-agent shard contention that cold start can exceed the 60s per-test budget (observed 60001ms on #3969 exact-head) while the four sibling cases complete in ~300ms once the module graph is warm. Fix (harness isolation, assertions unchanged): - beforeAll warmup spawn (120s suite budget) moves cold-start outside it() - resolveIn kills stalled children at 45s with a diagnostic instead of leaking pipes until the outer it() timeout Not a third blind per-test timeout bump; the 60s it() budgets stay. Lore-id: smithery-spawn-isolation-1 Confidence: high Scope-risk: narrow Reversibility: trivial Tested: not fully offline (local natives missing); exact-head CI verifies Not-tested: full parallel shard matrix offline
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c1aac25f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // finish in ~300ms; under extreme shard contention they may take a few seconds. | ||
| // Kill rather than wait for the outer it() timeout so a stalled child cannot | ||
| // pin the suite for the full 60s and leak pipes. | ||
| const PROBE_SPAWN_BUDGET_MS = 45_000; |
There was a problem hiding this comment.
When the first Bun probe needs 45–120 seconds under the same shard contention this patch is intended to tolerate, this constant kills it at 45 seconds and resolveIn throws, so the 120-second beforeAll timeout can never absorb the observed >60-second cold start; the suite instead fails earlier than before. The warmup needs a separate longer spawn budget, while the shorter kill budget can remain for already-warmed per-test probes.
Useful? React with 👍 / 👎.
1 participant
Problem
After #3967 raised the per-test timeout to 60s, #3969 exact-head CI still failed:
The four sibling child-process cases in the same file complete in ~290–350ms. Only the first spawn stalls.
Root cause
resolveInspawns a Bun child that loadssmithery-env-probe.ts→smithery-auth→@gajae-code/utils/env(eager parse of project/agent/home env + shell rc). Under coding-agent shard contention, the first child pays cold compile of that graph and can exceed 60s. Later spawns hit a warm Bun/module/disk cache and finish in ~300ms.A third blind per-test timeout bump would not fix isolation and would still leave a stalled child holding pipes until the outer
it()budget expires.Fix (harness only — assertions unchanged)
beforeAllwarmup spawn (120s suite budget, matches other child-process suite budgets) moves cold-start outside per-test timeouts.resolveInkill-on-timeout at 45s with a diagnostic so a stalled child is reaped instead of leaking until the outer 60sit()timeout.Non-goals
Verification