session-manager.test.ts reports its own timeout as a product failure

[pseudoseed]

session-manager.test.ts:1331 reports its own timeout as a product failure. It is the defect this program keeps hitting at every layer — porch verdicts, check output selection, skip reasons, signal codes — and here it is inside the test suite.

The core defect: "I could not tell" is spelled the same way as "no"

const deadline2 = Date.now() + 15000;
while (manager.getSessionInfo('cleanexit-test') && Date.now() < deadline2) {
 await new Promise((resolve) => setTimeout(resolve, 100));
}
expect(manager.getSessionInfo('cleanexit-test')).toBeNull();

When the deadline expires, that assertion fails identically to the case where the
fast-clean-exit valve genuinely never tripped. A reader sees expected null, got a session and
concludes the valve is broken. That is a real bug's signature, produced by a clock running out.

So the fix is not a bigger number. On deadline expiry it must fail with its own message
naming the timeout and the observed state — how many spawns were seen, what the session looked
like — so a future reader can tell "the valve did not trip" from "the valve had not tripped
YET when I stopped looking". A larger or adaptive bound is worth doing as well, but a bound
that cannot say why it expired will mislead someone again at whatever value we pick.

What was observed

During spec 146 phase 9, this test failed twice on a branch that had been green minutes earlier
with only a markdown commit between:

× bounds a harness that exits 0 immediately, without applying crashLoopFallback 15484ms
 packages/codev/src/terminal/__tests__/session-manager.test.ts:1331

15484ms against a 15000ms deadline. It did not assert something false — it ran out the poll
and then asserted. Its vitest timeout is 30000ms and was never approached.

A later full-suite run on a quiet machine passed: 6669 passed | 50 skipped, exit 0. Same
commit, same tree, no code change between them.

Measured, so the numbers are checkable

Three isolated runs of this test alone on a quiet machine:

Run	Duration
1	486ms
2	490ms
3	485ms

Mean 487ms, spread 5ms (1.0% of the mean). Against the 15000ms bound that is ~30x
headroom — the test uses about 3.2% of its own deadline when nothing else is running. This is
not a marginal bound on a quiet machine; it is a very stable one.

Decomposed from source rather than inferred:

• FAST_CLEAN_EXIT_MS = 2_000 and MAX_FAST_CLEAN_EXITS = 5 (session-manager.ts:118-119).
  The valve trips on the fifth consecutive clean exit within 2s of launch (:1163).
• The sequence is therefore 5 sequential shellper spawns, with restartDelay: 50 between
  them — 200ms of deliberate delay across the four gaps.
• 487ms total, minus ~200ms of delay, minus 100ms poll granularity, leaves roughly
  35–45ms per spawn.

The arithmetic anyone can check: to blow a 15000ms deadline, 5 spawns must cost about
2960ms each — roughly 70x the quiet cost.

What is NOT known, stated rather than papered over

Nothing measured explains a 70x per-spawn slowdown. Four mechanisms were tested. None
reproduces it.

Mechanism	Result	vs 490ms baseline
Bulk sequential write pressure (4 parallel dd)	537 / 492 / 946ms	1.9x worst
Metadata pressure (6 workers creating small files)	515 / 515 / 508ms	1.05x
A real cold pnpm install, link phase sampled	586 / 493ms	1.2x
Repeated git worktree add --detach + remove	491 / 489 / 486 / 487ms	1.0x

The install run was phase-attributed rather than sampled blind: each measurement recorded the
node_modules entry count before and after itself, so a sample taken while entries were actually
being created is distinguishable from one taken during resolve or fetch. Two samples had real
growth (deltas of 1010 and 377 entries) and came in at 586ms and 493ms. The phase that would
matter was measured, not missed.

A concurrent-build hypothesis was also checked and killed: packages/codev's build is
pnpm clean && tsc with clean = rm -rf dist, so a build running under the test would delete
the binary it spawns. dist/terminal/shellper-main.js was written once at 11:39:03, and a find
over packages/codev/dist for modifications during the failing run's window returns nothing.

Two limits on the negatives, which are properties of the reproduction rather than of the
hypotheses: the install ran 24.7s against the original's ~2 minutes, and used an isolated store
(--store-dir) rather than the shared one, so it is a harsher-but-shorter and not-identical load.

The honest state: the cause is unknown. Quiet cost 487ms with 1% spread, a bound of 15000ms,
a failure requiring ~30x, two failures inside one window, four mechanisms tested, none of which
explains it.

Three fixes, independent of today's cause

1. Distinguish expiry from failure. On deadline expiry, fail with a message naming the
   timeout and the observed state. This is the one that matters; the rest are hardening.

2. it.skipIf(!!process.env.CI) excludes it from the only environment with predictable load.
   A timing-sensitive test runs exclusively on developer machines, where load is whatever happens
   to be running — validated only in the least controlled place available. Either make it
   CI-runnable, or accept that its bound is being validated nowhere.

3. Assert the spawned artifact exists before polling. The test resolves
   path.resolve(dirname, '../../../dist/terminal/shellper-main.js') — a build output — and
   spawns it, with nothing checking it is there. A missing binary produces 15 seconds of silence
   followed by the same misleading assertion. One statSync before the loop turns that into an
   immediate, named failure. This holds whether or not a missing binary caused today's failure:
   the test currently cannot distinguish "the valve did not trip" from "I never had a binary to
   spawn".

Reproduction

Not reliably reproducible on a quiet machine — 3/3 passes at 487ms ±5ms. Observed twice under
concurrent activity on 2026-08-29 during spec 146 phase 9. That irreproducibility is part of the
report, not a reason to close it: the failure mode is that the test cannot tell you why it failed,
and that is present on every run, passing or not.

[pseudoseed]

Independent confirmation from bugfix-196's worktree, and it is the opposite mechanism to the
one disproved here — which is what makes the pairing useful.

spir-146 killed the "a concurrent build deleted it mid-test" hypothesis with mtime evidence: its
dist was written once at 11:39:03 and untouched throughout its run. That kill stands.

My run hit the same missing file by a different route. packages/codev/dist/terminal/shellper-main.js
has mtime 12:13:35, which is the pnpm build I ran after the failing suite. So in my case the
file was never there at all — absent from the start, rather than present-then-deleted. Cause: I ran
the suite before building, and dist/ is a build artifact, not a tracked file.

Two different mechanisms, one indistinguishable symptom:

Error: Invalid shellper info JSON
stderr log (…/shellper-int-test-1.log):
 Error: Cannot find module '…/packages/codev/dist/terminal/shellper-main.js'

The test spawns the artifact by path and asserts nothing about the artifact existing, so both
failures present as a poll timeout followed by a message about JSON — when the actual fault is
a missing file, and the real explanation is only visible by opening a temp-directory log the
failure output happens to name.

This is the evidence that part 4 is not theoretical. One statSync on the artifact before the
poll turns both cases into an immediate, named failure, and it distinguishes them for free: "never
built" and "deleted while running" are both ENOENT at spawn time, but only one of them can happen
before the first poll.

Evidence: src/terminal/__tests__/session-manager.test.ts, two failing cases, from a full-suite run
in .builders/bugfix-196 at 12:07 on 2026-08-29.

[pseudoseed]

A second unmeasured bound, in a different test, failed today — and comparing the two sharpens what this issue is actually about.

	session-manager.test.ts:1331	spec-146-phase-9-porch-engine.test.ts pack test
Bound	15000ms, self-imposed poll deadline	5000ms, vitest's default
Real cost	487ms ±5ms quiet	~2.3s quiet (2725/2275/2017), ~4.6s loaded (4419/4635/4728)
Headroom	~30x	~2x quiet, negative under load
Outcome	Failed anyway; four mechanisms measured, none reproduces it	Failed predictably whenever the machine was busy

Neither number was ever sized against anything. That is the finding, and it is larger than "5000ms was too small".

One bound was absurdly generous and one was impossible, and nothing in either test tells a reader which. Both look equally deliberate in source. A reader debugging either one has no way to know whether the number represents a measured budget or a value someone typed once — so the 30x bound got hours of investigation into what could possibly have exceeded it, while the 2x bound got a builder reporting a correct test as a failure.

The pack bound is now measured, with the measurement written into the comment beside it so the next person raising it knows what it was sized against. The session-manager bound is still unmeasured — its 15000ms has no stated relationship to the ~487ms the test actually costs, and part 1 of this issue stands: on expiry it fails identically to a genuine product failure, so nobody can tell an expired clock from a broken valve.

Suggested addition to this issue's scope: a timing-bounded test should carry its measurement. Not a convention anybody has to remember — a comment beside the number saying what it was sized against, so 15000 and 5000 stop being indistinguishable from each other and from a guess. Every bound in this report that caused a problem today was a bare integer with no provenance.

[pseudoseed]

The pack test has never executed in CI — and the mechanism is open, not closed

Found while porch done on #197 failed twice on this test. The raised bound is the right fix;
this comment is about why a bound below the loaded cost survived long enough to need one.

Two misconfigurations, in opposite directions, meeting in one test

spec-146-phase-9-porch-engine.test.ts runs in neither CI job:

• Unit Tests runs vitest with the default config, which does collect the file. But that
  job builds sdk, core, types, artifact-canvas and porch-driver — never packages/codev.
  So dist is absent and it.skipIf(!distBuilt) skips the assertion.
• Tower Integration does build packages/codev, so dist exists. But it runs
  vitest.e2e.config.ts, whose include is only src/commands/porch/__tests__/e2e/** and
  src/**/*.e2e.test.ts. This filename matches neither, so it is never collected.

Skipped in the job that includes it; excluded from the job that has dist. Each choice is
individually defensible. They meet in a test that has never run.

So porch is its only executor anywhere

porch check / porch done run the build criterion first, which creates
packages/codev/dist — which is precisely what arms the skipIf gate. Nothing else on a
developer machine builds packages/codev before running the default suite.

That reframes the survival. It is not that the bound was under-tested and nobody measured it:
there was no signal it could ever have contradicted. No green run reached it and no red
run reached it. A bound that no run can reach is not under-tested, it is unobserved — and it
became observable only when a gate that always builds first started running it.

It also gives the failure its nasty shape: invisible to anyone running vitest directly,
near-deterministic for anyone going through porch, which is the one path that can block work.

The test was honest; nobody was reading it

Worth saying in the author's favour. The file carries the inverse gate too:

it.skipIf(distBuilt)('records why pack relative imports could not check', () => {
 expect(packSkipReason).toBe('could not check: packages/codev/dist not built');

So in CI the only branch that has ever run is the one that says "could not check". The
signal existed and was correctly worded — it just landed as a skip-reason in a passing run,
where nobody looks. This is the repo's own lesson in a new costume: "I could not tell" must
never be spelled the same way as "no" — here it was spelled differently, and still read as
"fine", because a green suite absorbs both.

Sizing: one test today, but the mechanism stays open

skipIf(!distBuilt) appears exactly once across packages/codev/src — this file. The
other dist-dependent tests are named *.e2e.test.ts (install.e2e.test.ts and the rest of
src/__tests__/cli/), so they land in Tower Integration and do run.

So today the gap is one test. But nothing prevents the next one: any dist-gated test added
to the default config will silently never run, and nothing will report it.

Recommendation (spir-146's call, not mine)

Rename it spec-146-phase-9-porch-engine.e2e.test.ts.

That puts it in Tower Integration, which already builds packages/codev, and it matches the
convention vitest.config.ts:25 states for itself — "server-spawning / integration tests". A
test that shells out to npm pack is exactly that. It is cheaper than teaching the Unit Tests
job to build codev, and it makes the skipIf gate unnecessary rather than merely
satisfied — which is the difference between removing the failure mode and moving it.

Measurements are spir-146's (2.3s quiet, 4.6s under four parallel disk writers, against the
5000ms default). Filed from #197; I have not touched the file.

[pseudoseed]

A fifth part, and it changes why the bounds in this issue survived

spec-146-phase-9-porch-engine.test.ts's pack test never executes in CI at all. Verified in both configs:

• Unit Tests runs the default config, which includes the file — but that job never builds packages/codev, so it.skipIf(!distBuilt) skips it.
• Tower Integration does build packages/codev — but it runs the e2e config, whose include is src/commands/porch/__tests__/e2e/**/*.test.ts and src/**/*.e2e.test.ts. The filename matches neither.

Skipped in the job that includes it; excluded from the job that has dist. porch is not its usual executor — porch is its only one.

That reframes this issue's central point. The 5000ms bound did not survive because nobody measured it. It survived because there was no signal it could ever have contradicted. A bound that only executes on a path nobody watches is not under-tested; it is untested, and it will read as deliberate to every future reader.

skipIf(!distBuilt) appears exactly once in packages/codev/src today, so the gap is one test — but the mechanism is open for the next one.

The fix is a SPLIT, not a rename

Renaming the file to *.e2e.test.ts moves it into Tower Integration, which already builds packages/codev, and makes the distBuilt gate unnecessary rather than merely satisfied. That reasoning is correct about the test.

But the rename acts on the file, and the file holds 8 tests. Only one is the pack test. The other seven cover createPorchThreadEngine — the first turn carrying the builder's prompt, the role joined onto it, the control proving a prompt-less create starts no turn, and activeTurnId tracking. Those are unit tests of live logic, they run in the default suite today, and they are fast.

Renaming would silently move all seven into a job that runs sequentially and only in Tower Integration — fixing one test that never runs in CI by taking seven that do and putting them in the same category.

So: extract the pack test into its own *.e2e.test.ts file and leave the engine tests where they are. The e2e config's 20-minute testTimeout then covers it, which makes the explicit 60s bound redundant — keep the measurement comment anyway, since it is the record of what the cost actually is.

Why this is a follow-up rather than part of the timeout fix

The timeout raise (#208) is one constant and unblocks a builder now. The split is a structural change to which CI job owns a test, and it deserves its own review rather than riding along with a one-line fix or with an 88-file feature PR.

[pseudoseed]

Third instance today, and the count changes what this issue is

A third failure from this family, on 2026-08-29:

× no stderr tail logged for file-based stderr (Bugfix #324) 30208ms
 packages/codev/src/terminal/__tests__/session-manager.test.ts:2187

Every marker of the original: it.skipIf(!!process.env.CI), so CI never runs it; 30208ms against
a 30s bound, so it ran out the clock rather than asserting something false; and the same file as this
issue's original failure.

The count

I grepped the package rather than estimating:

$ grep -rn "skipIf(!!process.env.CI)" --include="*.test.ts" packages/codev/src | awk -F: '{print $1}' | sort | uniq -c
 8 packages/codev/src/terminal/__tests__/session-manager.test.ts

Eight, all in one file, and nowhere else in the package. So this is not a scattered habit — it is
a single bounded cluster, and today two of those eight have failed on a loaded machine while CI
stayed green throughout, because CI runs none of them.

That reframes the issue. It is not "a flaky test with an unmeasured bound". It is a file containing
eight timing-bounded tests that execute only in the least controlled environment available, with no
CI signal that can ever contradict any of them — the same structural point already recorded here
about the pack test, now with a population attached.

The load was real and worth stating: four builders, several concurrent suites, and a CI-triggering
push on one machine.

What it does not affect

CI was green across all eight jobs on the commit in question, including the two that had been red.
This failure is in a test CI does not run, so it says nothing about that verification either way.

The consequence the builder named, which is the operationally useful part

My local suite is no longer a clean signal on this machine under this load, and I would not want a
future local red read as necessarily meaningful without checking whether it is one of those eight.

That is the cost of the cluster: it degrades the trustworthiness of every local run, because a red
now requires triage before it means anything.

Ask, extending the existing one

The three parts already filed here stand — a distinct timeout message naming the observed state, the
skipIf(CI) exclusion, the spawn-cost measurement. Add a fourth:

4. Treat the eight as one decision, not eight. Either they belong in CI (and their bounds must be
   sized against a loaded runner), or they are genuinely local-only (and should say so loudly enough
   that a red from one is immediately identifiable as such). Eight tests silently in the second
   category while everyone treats a local red as signal is the actual defect.

Related: #212 (a suite-wide declared prerequisite would surface this class), #199.

[pseudoseed]

Another data point, from builder aspir-236 (spec 236) on 2026-08-30.

One instance, under full parallel load. The round-5 full packages/codev run: 7143 passed, 1 failed.

FAIL src/terminal/__tests__/session-manager.test.ts > stderr tail logging (integration)
 > no stderr tail logged for file-based stderr (Bugfix #324)
Error: Test timed out in 30000ms.
 ❯ src/terminal/__tests__/session-manager.test.ts:2187:29

Suite duration 199s wall, 594s of test time — so it was running heavily in parallel when it went.

It passes alone. Immediately re-running that file by itself: 91/91 passed, no changes in between.

Nothing on this branch touches it. Spec 236 is the t3code snapshot provider, asynchronous gate approval and afx pair; src/terminal/ is untouched by the branch.

Consistent with this issue's shape. The failure surfaced as a bare 30s timeout at the assertion, which says nothing about whether the product behaviour under test is right — the timeout and a real regression are spelled the same way, which is what this issue is about. it.skipIf(!!process.env.CI) means CI never runs it, so it can only ever be seen locally, and only under load.

Recorded here rather than annotated in my project's review, on the architect's steer: a data point on a known load-dependent failure is worth more on the issue than in a review nobody will grep.