Skip to content

fix(test): stop the spawn-heavy tooling files contending for one runner's vCPUs (BACKLOG #1304) - #1203

Merged
wshallwshall merged 4 commits into
mainfrom
claude/b1304-pwsh-contention
Sep 17, 2026
Merged

wshallwshall merged 4 commits into
mainfrom
claude/b1304-pwsh-contention

Conversation

@wshallwshall

@wshallwshall wshallwshall commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Updated 2026-09-16. The head below could not merge because one of its own new tests failed on the ubuntu leg. That is fixed -- see Follow-up fix: the allowlist parse was host-dependent. Everything above that section is the original head's text and still stands.

What this fixes

BACKLOG #1304 is currently the sole cause of main going red. All seven post-1174 main-reds failed on one job, repo harness tests (windows-2025): six of seven on AssertionError: PWSH LAUNCH TIMED OUT after 45s, seven of seven on subprocess.TimeoutExpired, zero on real harness breakage. That leg is not itself a required context, but CI gate aggregates it, so the flake reds main and keeps PRs out of the merge queue.

The cause was already evidenced by 5e5a8a5ab and is not re-derived here. This PR is the remedy only.

The remedy, and what selected it

A burst lock (tests/_spawn_lock.py). A bulk spawner takes it EXCLUSIVELY for the length of its storm; every single launch takes it SHARED. The storm still gets all 16 of its concurrent processes, so the property _race asserts is untouched, and no single launch is in flight beside it.

Wired at four sites: _race and the 8-way drain in test_session_mail.py, the 8-way burst in test_coord_lock.py, and run_gate in test_worktree_gate.py -- which test_worktree_gate_control_plane.py and test_worktree_gate_escaped_quote.py both import, so one wrapper covers all three measured victim files.

Why not raise the ceiling. The row's own text refuses it: the failure is a launch that never returns, so a higher bound only makes the next occurrence take longer to fail.

Why not cut RACERS. _race exists to prove exactly one of N concurrent claimers wins, and _require_real_contention SKIPS when the host cannot produce contention at that N. A lower N turns a race test into a sequential one that still reports green -- the always-pass failure tests/_dead_pid.py records for a sibling flake. test_spawn_lock.py now pins RACERS == 16 and DRAINS == 8 so a later pass cannot take that shortcut quietly.

Why not an xdist group. Two reasons, and the first is a correction worth reading: --dist loadgroup does not serialise groups against each other, it PINS same-group tests to one worker. So it would stop the spawn-heavy files overlapping each other, at the cost of collapsing the tier's four heaviest files onto one worker that then sets the wall clock, while the other three workers keep launching pwsh throughout the storm. Second, it lives in ci.yml's pytest invocation, which PR 1196 is rewriting +97/-7 -- a head-on collision.

Priced against runner slots

Per the Lander reading (BACKLOG #1788): compute on a queue entry never exceeds 11.7 minutes while wall clock reaches 62, the variance entirely queueing at ~80 jobs against 20 slots. This adds no job, no matrix leg and no workflow change. It spends wall clock inside a slot the tooling job already holds, which is the cheap direction under that measurement. A "split the spawn-heavy files into their own job" variant is the expensive one and is not taken.

No merge_group arm was added to tooling, deliberately. That stays sequenced behind this fix, per the agreed order.

What it does NOT cover

  • scripts/coord/overlap.ps1 defaults ParallelLimit = 16, and several manifest files invoke it, each able to hold 16 runspaces spawning git concurrently. Plausibly a larger uncounted concurrency source than _race. Found by sweeping the tier, not by CI evidence, so it is untouched -- this change does not wrap on a hunch.
  • Roughly 66 other files in the tier launch pwsh without taking the shared side, so a storm does not wait for them. The missing abstraction is a shared run_pwsh launcher every site uses. Naming it is not building it.
  • test_worktree_gate.py::_pwsh_identity starts another pwsh while building the timeout message -- an extra spawn on the hot failure path, bounded at 15s and swallowed. Untouched.
  • test_coord_lock.py's 8-way burst is wrapped, on sweep evidence rather than CI evidence, and the comment says so.

Because the fix is deliberately partial, _note now records fail-open events to stderr so the next occurrence separates "the lock did not cover that source" from "the lock covered it and fell open" from "different cause".

The -n 3 experiment: never landed, still open, result unread

aa8f68df0 is not on main. It is the head of PR 1196, whose own title reads EXPERIMENT (unread result) ... Do not merge on one green run. So lowering the worker count was neither concluded nor rejected -- it was never landed and nobody has read its result. It is not evidence for or against direction 3.

This contradicts the brief that produced this PR, which said a sweep of open PRs found zero hits on these files. It found zero because it grepped path fragments (pwsh, harness, conftest); .github/workflows/ci.yml and tests/test_worktree_gate.py contain none of those words. A re-scan of all 26 open PRs by exact path found four collisions: 1196 (ci.yml), 1195 (tests/test_worktree_gate.py -- the canary commit ba6114ee2, also unlanded), 1185 (tooling_manifest.txt), 1162 (test_tooling_partition.py).

Two things follow, and both shaped the diff:

  • The test_worktree_gate.py edit is a one-line call swap plus an import, not a re-indent. PR 1195's hunk sits about 39 lines away inside the except block, so the two should not conflict.
  • The manifest entry sorts under "s"; PR 1185 inserts in the "n/o" region.

Follow-up fix, 2026-09-16: the allowlist parse was host-dependent

The first head (35d18ff3) could not merge because one of its own new tests failed on the ubuntu harness leg:

FAILED tests/test_spawn_lock.py::test_run_single_refuses_a_cheap_command
AssertionError: assert 'c:\program files\powershell\7\pwsh' in frozenset({'powershell', 'pwsh'})

Two defects, one cause, and the fix is the helper rather than the test's expectation.

  1. run_single parsed the binary name with the HOST'S path flavour. Path(cmd[0]).name is WindowsPath.name on Windows and PurePosixPath.name on Linux, and PurePosixPath does not treat a backslash as a separator. So on Linux a Windows-spelled pwsh path returned the WHOLE SPELLING as its binary name and the allowlist refused a launch it should have taken. It now splits on both separators with a plain string operation, which cannot vary by host.
  2. The test RE-IMPLEMENTED that parse instead of calling run_single. It asserted Path(spelling).name.lower().removesuffix(".exe") inline, so it exercised pathlib and never the seam -- agreeing with the implementation by construction and disagreeing with the host instead. That is also why this survived a green local run: on Windows Path splits both separators, so the same assertion passes. The accept arm now DRIVES run_single with subprocess.run patched, asserting each spelling is accepted and arrives unaltered.

Relaxing the test instead would have been wrong: the property it asserts is what run_single's own docstring promises, and the guard is load-bearing -- a refused pwsh launch on Linux is a real defect, not a test being too strict.

Measured control, because a green Windows run proves nothing here.

spelling old parse under POSIX flavour accepted new parse accepted
C:\Program Files\PowerShell\7\pwsh.exe c:\program files\powershell\7\pwsh NO pwsh yes
/usr/bin/pwsh pwsh yes pwsh yes
PowerShell.EXE powershell yes powershell yes
pwsh pwsh yes pwsh yes

And the rewritten test CATCHES the defect rather than being weakened past it. Running its accept arm against the old parse forced to POSIX flavour, which is the ubuntu host:

OLD helper on the ubuntu host, accept arm:
   REFUSED    C:\Program Files\PowerShell\7\pwsh.exe -> ValueError
   accepted   /usr/bin/pwsh
   accepted   PowerShell.EXE

NEW helper (the shipped fix), same arm:
   accepted   C:\Program Files\PowerShell\7\pwsh.exe
   accepted   /usr/bin/pwsh
   accepted   PowerShell.EXE

So the shipped defect was not only a red test. run_single would have REFUSED a legitimate full-path pwsh launch on Linux, with the ValueError telling the caller to use subprocess.run directly -- which would have quietly taken that call site back out of the lock this PR exists to add.
Row 1's old value reproduces the CI AssertionError string byte for byte. The test now pins that refuted value beside the fixed one, so a later simplification back to pathlib has to argue with it rather than rediscover it on a red leg.

Same layer, one line: the ValueError names the offending argument through a local, which also removes an IndexError on an empty cmd.

Checks run on this follow-up

Foreground, in the worktree venv, on Windows:

  • ruff check and ruff format --check on both changed files -- clean

  • mypy --strict -m tests._spawn_lock -m tests.test_spawn_lock -- clean. CI's mypy scope is messagefoundry messagefoundry_webconsole, so tests/ sits outside it either way

  • pytest tests/test_spawn_lock.py -- 15 passed

  • pytest over the four files this branch wires the lock into (test_worktree_gate_control_plane.py, test_worktree_gate_escaped_quote.py, test_session_mail.py, test_coord_lock.py) -- 307 passed, run FILE BY FILE rather than as one invocation:

    files result
    test_worktree_gate_control_plane.py + test_worktree_gate_escaped_quote.py 227 passed in 347.67s
    test_session_mail.py 75 passed in 151.21s
    test_coord_lock.py 5 passed in 17.75s

    The single combined invocation STARVED and was stopped after about 20 minutes with no output, at 38 resident pwsh processes from roughly 14 concurrent sessions on this box. Split per file it completes. That is a local contention artefact, not a signal about this branch -- but it is the same class of failure #1304 is about, one level down, so it is recorded rather than omitted.

    Only test_worktree_gate.py imports run_single (the control-plane and escaped-quote files reuse its run_gate). test_session_mail.py and test_coord_lock.py import only spawn_burst, which this diff does not touch -- both hunks are inside run_single.

Not run: the full suite. Under fleet contention on this box a local full run may not finish; targeted coverage plus the named CI legs is the standing instruction. Local runs still report INCOMPLETE RUN without the vault extra, as above.

The arm that decides this fix is repo harness tests (ubuntu-latest), because Linux is the only host where the defect can show. A Windows leg passes with or without it.

Unchanged from the head below: if repo harness tests (windows-2025) times out again with the burst lock active, that is a real finding about whether the lock covers every storm site -- NOT evidence this fix failed. It is an open question for the next session, and it must not be chased by rerunning until green.


Checks run

All foreground, in the worktree venv:

  • ruff check and ruff format --check -- clean on all five touched files
  • mypy strict -- clean on both new modules and on messagefoundry (258 files). Note CI does not type-check tests/
  • tests/test_spawn_lock.py -- 15 passed
  • tests/test_tooling_partition.py -- passes with the new manifest entry; the new file selects 14 under -m tooling and 0 under -m "not tooling"
  • The two #1304 launch-timeout diagnostic rows in test_worktree_gate_control_plane.py -- pass through the guarded wrapper
  • -n 4 --dist loadfile (CI's own shape) over storm + victim + partition + lock files -- 224 passed
  • test_worktree_gate_control_plane.py + test_worktree_gate_escaped_quote.py -- 227 passed
  • Full tier collects: 3248 tooling tests, no collection errors

Local runs report INCOMPLETE RUN without the vault extra -- expected here.

No local timing is offered as evidence that the fix works. This box has 20 cores and cannot reproduce a 4-vCPU failure; the diagnostic itself records that the same storm costs almost nothing unpinned across 20 cores. Timings were used only to size the wait bounds. A two-arm local comparison was attempted and is reported as noise, not signal: the unlocked arm ran 264s and the locked arm ran 180s, 264s and 179s across three runs at 19-32 competing pwsh. Box load dominates at n=1. The honest claim is no measurable wall-clock regression.

An unrelated pre-existing flake, recorded so it is not attributed here

tests/test_coord_lock.py::test_lock_is_shared_between_the_primary_and_its_worktrees failed 2 of 6 parallel runs on this box, including the control arm where this module was absent entirely (the three touched files reverted to their pre-change state). It passes in isolation. Load-dependent, in a file this branch touches, and not this branch's.

CI legs that must be read after this PR

  1. repo harness tests (windows-2025) -- the whole point. This PR changes manifest files, so the tooling job fires on both legs and actually exercises the fix on the leg that flakes. One green run is not proof; the flake is intermittent.
  2. repo harness tests (ubuntu-latest) -- the sibling arm. tests/test_spawn_lock.py carries no blanket pwsh skip, so most of it runs there too.
  3. CI gate -- the aggregate that the flake reds.

a PR that implements BACKLOG #N must update BACKLOG.md is a required context but a deliberate no-op that always passes (backlog-hygiene.yml -- the rule left with the ledger). docs/BACKLOG.md is correctly untouched.

Proposed ledger banner text for #1304

#1304 -- PWSH LAUNCH TIMED OUT after 45s on the windows-2025 harness leg. Cause evidenced 5e5a8a5ab: a 16-process pwsh storm from test_session_mail.py::_race starving single launches on a 4-vCPU runner. Remedy shipped: tests/_spawn_lock.py, a cross-process burst lock -- bulk spawners take it exclusively, single launches take it shared, so the storm keeps all 16 processes and no gate launch runs beside it. Fail-open at every failure mode, scoped per pytest run via PYTEST_XDIST_TESTRUNUID. Not closed on merge: the flake is intermittent, so this needs several windows-2025 runs before the row can be called done. Not covered: overlap.ps1's ParallelLimit=16 git fan-out, and about 66 tier files that launch pwsh without taking the shared side -- the follow-on is a shared run_pwsh launcher. The -n 3 experiment (PR 1196) remains open and unread and is independent of this.

wshallwshall added 2 commits September 16, 2026 12:51
…er's vCPUs (BACKLOG #1304)

BACKLOG #1304 is the sole cause of main going red across the seven post-1174 reds: six of
seven failed `PWSH LAUNCH TIMED OUT after 45s`, seven of seven on `subprocess.TimeoutExpired`,
none on real harness breakage. The `repo harness tests (windows-2025)` leg is not itself a
required context, but `CI gate` aggregates it, so the flake reds main and keeps PRs out of
the queue.

THE CAUSE WAS ALREADY EVIDENCED BY 5e5a8a5 AND IS NOT RE-DERIVED HERE.
tests/test_session_mail.py::_race starts RACERS=16 concurrent pwsh, three times per job, on a
4-vCPU runner; every test over 20 seconds in five windows-2025 jobs completes inside the
window where the spawn-heavy files overlap. This commit is the remedy only.

THE REMEDY: A BURST LOCK, NOT A HIGHER CEILING AND NOT A SMALLER STORM. A bulk spawner takes
tests/_spawn_lock.py EXCLUSIVELY for the length of its burst; every single launch takes it
SHARED. The storm still gets all 16 of its concurrent processes, so the property _race
asserts is untouched, and no single launch is in flight beside it.

WHY NOT THE OTHER TWO CANDIDATES.

Raising GATE_TIMEOUT_S is refused by the row's own text: the failure is a launch that never
returns, so a higher bound only makes the next occurrence take longer to fail.

Cutting RACERS is the always-pass failure tests/_dead_pid.py records for a sibling flake.
_race exists to prove exactly one of N concurrent claimers wins, and _require_real_contention
SKIPS when the host cannot produce contention at that N -- so a lower N turns a race test into
a sequential one that still reports green. tests/test_spawn_lock.py pins RACERS=16 and
DRAINS=8 so a later pass cannot take that shortcut quietly.

An xdist group (`--dist loadgroup`) was the closest alternative and lost on two counts. It
lives in ci.yml's pytest invocation, which PR 1196 is rewriting +97/-7 for the unlanded -n 3
experiment, so it would collide head-on. And it separates the files that are spawn-heavy
TODAY; a new bulk spawner rejoins the storm silently. The lock binds the constraint to the
call site instead, so a new spawner opts in by wrapping itself.

PRICED AGAINST RUNNER SLOTS, NOT BILLED MINUTES (Lander reading, BACKLOG #1788: compute on a
queue entry never exceeds 11.7 minutes while wall clock reaches 62, the variance entirely
queueing at ~80 jobs against 20 slots). This adds no job, no matrix leg and no workflow
change -- it spends wall clock inside a slot the tooling job already holds, which is the
cheap direction under that measurement. No merge_group arm is added to tooling; that stays
sequenced behind this fix.

EVERY FAILURE MODE DEGRADES TO TODAY'S BEHAVIOUR, which is what makes it safe to land. No
lock root, a saturated wait, a stale entry reaped while its owner is alive, an OSError on any
filesystem call -- each proceeds WITHOUT the lock. There is no path that blocks a test forever
or fails one. That is tests/conftest.py's rule for its per-process slot: "It never fails a
run."

Scoped to one pytest run, not to the machine: xdist exports one per-run testrunuid into every
worker as PYTEST_XDIST_TESTRUNUID, so four workers of one job share a lock while two unrelated
local runs do not serialise. On a CI runner there is exactly one run, so run-scoped and
machine-scoped coincide precisely where the fix has to work. Staleness is by timestamp rather
than by probing the owner pid, because conftest's tasklist probe would start a process inside
a remedy for too many processes.

THE DIAGNOSTIC IT PROTECTS IS PINNED, because this change could have disarmed it silently.
run_gate now calls run_single, which looks subprocess.run up on the MODULE at call time, so
test_worktree_gate_control_plane.py's monkeypatch.setattr(harness.subprocess, "run", ...)
still intercepts it. A `from subprocess import run` would have bypassed that patch and left
the #1304 launch-timeout test green against a diagnostic that no longer fires. There is a
named negative control for exactly that.

The mutual-exclusion test carries a positive control: the same probe with the lock disabled
must actually overlap, or the assertion proves nothing.

WHAT THIS DOES NOT COVER. scripts/coord/overlap.ps1 defaults ParallelLimit=16 and eight
manifest files invoke it, each able to hold 16 runspaces spawning git concurrently. That is
plausibly a larger uncounted concurrency source than _race, it was found by sweeping the tier
rather than by CI evidence, and it is untouched here. tests/test_worktree_gate.py's
_pwsh_identity also starts another pwsh while building the timeout message -- an extra spawn
on the hot failure path, bounded at 15s and swallowed, likewise untouched.

test_coord_lock.py's eight-way burst IS wrapped, on sweep evidence rather than CI evidence:
eight concurrent pwsh with the winner holding 8 s is the same shape.

Checks, all foreground in the worktree venv: ruff check and ruff format clean on all five
touched files; mypy strict clean on messagefoundry (258 files) and on both new modules. Tests,
with the process count beside every timing because this box carries concurrent sessions --
211 passed in 180.49s over test_session_mail, test_worktree_gate, test_spawn_lock and
test_coord_lock at `-n 4 --dist loadfile`, CI's own shape, at 4 python and 29 pwsh resident;
227 passed in 398.45s over test_worktree_gate_control_plane and test_worktree_gate_escaped_quote;
23 passed over test_spawn_lock and test_tooling_partition. Local runs report INCOMPLETE RUN
without the vault extra, as expected here.

This box has 20 cores and CANNOT reproduce a 4-vCPU failure, so no local timing is offered as
evidence that the fix works -- only as sizing for the wait bounds.
…diagnosing (BACKLOG #1304)

Four findings from the /simplify pass on the previous commit. No behaviour change to the
remedy itself; every change here is either a correction, a guard, or a diagnostic.

THE SEAM IS NOW TYPED, AND THIS WAS THE REAL HOLE. run_single was a total subprocess.run
passthrough, so nothing stopped this tier's hundreds of cheap `git` calls being routed
through it. Every held ticket extends every concurrent burst's drain wait, so bursts would
stop draining, hit _BURST_DRAIN_S, and proceed unsynchronised -- the lock disabling ITSELF,
failing open exactly as designed, with nothing going red. It now refuses anything but pwsh
or powershell, matched on the binary name so a full path or a .exe suffix still passes. The
storm counts were already pinned against weakening the writers; this pins the reader
population against weakening them.

THE `--dist loadgroup` ARGUMENT WAS WRONG AND IS CORRECTED. loadgroup does not serialise
groups against each other; it PINS same-group tests to one worker. The docstring implied the
former. The honest reading is that it would stop the spawn-heavy files overlapping each
other, at the cost of collapsing the tier's four heaviest files onto one worker that then
sets the wall clock, while the other three workers keep launching pwsh throughout the storm.
Recorded rather than quietly reworded, because the previous text would have let a reader
conclude the option had been weighed on merit.

FAIL-OPEN EVENTS ARE NOW RECORDED, which the turnstile's label previously invited and nothing
delivered: it was written and never read. The fix is deliberately partial, so when a launch
times out again the log has to separate three outcomes that otherwise look identical -- the
lock did not cover that source, the lock covered it and fell open, or the cause is something
else. `_note` writes to stderr, which pytest surfaces on failure, and never raises.

WHAT THE FIX DOES NOT COVER IS NOW IN THE MODULE, not only in a commit message: overlap.ps1's
ParallelLimit=16 git fan-out across several manifest files, test_announce_hook.py's 2-way pool,
and the roughly 66 other files in the tier that launch pwsh without taking the shared side, so
a storm does not wait for them. The missing abstraction is a shared run_pwsh launcher every
site uses; naming it is not building it.

Also: _lock_root is a pure query again -- it no longer reaps as a side effect, so a caller
asking only whether the path resolves cannot trigger a recursive delete over a shared .git,
which the new test for it was doing. The sweep moved to the _ROOT assignment, runs once per
process, and uses os.scandir so is_dir/stat cost no syscall beyond the listing. The docstring
now states that the git call is five per run under -n 4 rather than the true-but-misleading
"one per process", and names the conftest duplication as the follow-on. Why this does not
reuse scripts/coord/lock.ps1 is recorded: PowerShell (a spawn inside a spawn remedy),
exclusive-only where this needs shared/exclusive, and loud-failing where this must fail open.

AN UNRELATED PRE-EXISTING FLAKE WAS OBSERVED AND IS NOT FIXED HERE, recorded so the next
reader does not attribute it to this change:
test_coord_lock.py::test_lock_is_shared_between_the_primary_and_its_worktrees failed 2 of 6
parallel runs on this box, INCLUDING the control arm where this module was absent entirely
(the three touched files reverted to their pre-change state). It passes in isolation. It is
load-dependent, it is in a file this branch touches, and it is not this branch's.

Checks: ruff check and ruff format clean; mypy strict clean on both modules and on
messagefoundry (258 files). tests/test_spawn_lock.py 15 passed. The two #1304 launch-timeout
diagnostic rows in test_worktree_gate_control_plane.py pass through the guarded wrapper, which
is the row that would have caught the seam change silently disarming them. 224 passed at
`-n 4 --dist loadfile` over the storm, victim, partition and lock files at 2 python and 19
pwsh resident, with the pre-existing coord_lock flake above as the only failure.
@wshallwshall wshallwshall changed the title fix(test): stop the spawn-heavy tooling files contending for one runner vCPUs (BACKLOG #1304) fix(test): stop the spawn-heavy tooling files contending for one runner's vCPUs (BACKLOG #1304) Sep 16, 2026
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Sep 16, 2026
wshallwshall added 2 commits September 16, 2026 19:59
…ACKLOG #1304)

run_single refused a Windows-spelled pwsh path on a POSIX host. Path(cmd[0]).name is
PurePosixPath there, which does not treat a backslash as a separator, so the whole
spelling came back as the binary name and the allowlist rejected it. The ubuntu
harness leg raised exactly that:

  assert 'c:\program files\powershell\7\pwsh' in frozenset({'powershell', 'pwsh'})

Parse with a plain string split instead, so the result cannot vary by host.

test_run_single_refuses_a_cheap_command re-implemented that parse rather than calling
run_single, so it exercised pathlib and never the seam, agreeing with the host instead
of with the function. It now drives run_single with subprocess.run patched, asserting
each spelling is accepted and arrives unaltered, and pins the refuted PurePosixPath
value beside the fixed one.

The ValueError names the offending argument through a local, removing an IndexError on
an empty cmd.
…showed

Addresses the windows-2025 leg of PR 1203. BACKLOG #1304.

The ubuntu leg's path defect is NOT touched here: 432a4eb fixed it while
this was being written, and its rsplit-on-both-separators parse is equivalent
to the PureWindowsPath one that would have replaced it. Rewriting a peer's
landed fix to swap one correct expression for another is churn, and two
sessions editing one block is how they collide at merge. Dropped in favour of
what the remote still lacked.

WHAT THE WINDOWS LOG ACTUALLY SHOWED. Both failures there are the original
#1304 launch timeout, not the path defect, which could only ever fail off
Windows. The fail-open diagnostic added in 35d18ff reported the cause
directly, which is the first time this flake has named its own holder:

  [spawn-lock] single launch waited 30.4s behind session_mail._race
               claimed x16 pid=1064 ... and proceeded UNSYNCHRONISED
  [spawn-lock] single launch waited 30.1s behind session_mail._race
               claimed x16 pid=1064 ... and proceeded UNSYNCHRONISED

Same storm and same pid caught both launches. Each waited `_SINGLE_WAIT_S`
out, gave up while the turnstile was still held, launched into the storm and
blew its caller's 45s bound. So the lock engaged and then disarmed itself at
the ceiling.

The old 30.0 was sized from a 20-core box's 26.5s of bursts, while the same
comment block already recorded CI's window as 30-39s. The ceiling sat at the
bottom of the range it had to cover, and a 4-vCPU runner's storm is not
faster than a 20-core one's. Raised to 90.0, roughly 2.3x the top of the only
measured window, staying under both the storm's own 300s test bound and the
360s turnstile reap so an ABANDONED turnstile is still cleared by staleness
rather than by a waiter giving up on a live one.

THIS IS NOT THE "RAISING THE CEILING" THE MODULE DOCSTRING REFUTES, and the
comment now says so where the next reader will hit it. That refutation is
about GATE_TIMEOUT_S, the bound on a launch already running: a launch that
never returns does not return sooner for being given longer. This constant is
the opposite end, how long a launch waits BEFORE it starts, so that it starts
on an idle machine instead of inside a storm. Raising it cannot push a launch
past its own timeout either -- the wait runs before subprocess.run is called,
so the caller's timeout clock starts at process launch. A waiter holds
nothing, registering its reader ticket only once the turnstile clears, and
the poll exits the instant it does, so a run with no storm pays nothing.

A wait shorter than the storm is worse than no wait: it pays the full delay
and still lands in the contention. Nothing failed when the constant was too
small, because the module fails open by design -- the symptom surfaced as
somebody else's timeout on another leg. The new pin asserts the wait against
the MEASURED window rather than a literal, so re-sizing on new evidence stays
free while dropping it back under the storm does not. It fails at the shipped
value with `assert 39.0 < 30.0`, verified.

Checks: ruff check, ruff format --check, mypy strict clean on messagefoundry
(275 files) and on both changed test modules. pytest green on
test_spawn_lock + test_coord_lock + test_tooling_partition (32) and on
test_session_mail + test_worktree_gate (194). Every local run reports
INCOMPLETE RUN -- the `vault` extra is absent -- so none of them establishes a
green full suite. This 20-core box carried 28 resident pwsh and 0 competing
python processes and cannot reproduce a 4-vCPU contention failure; no timing
here was tuned to make a local run pass.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

ec8875012 addresses the windows-2025 leg

The two failing legs on 35d18ff3a were two different defects, and only one of them was the path parse.

Leg Failure Fixed by
repo harness tests (ubuntu-latest) run_single refused a Windows-spelled pwsh path on POSIX 432a4eb03 (already landed)
repo harness tests (windows-2025) 2x the original PWSH LAUNCH TIMED OUT after 45s ec8875012 (this push)

What the windows log actually showed

Not the path defect, which could only ever fail off Windows. Both failures are the original #1304 timeout, in test_worktree_gate_escaped_quote.py and test_worktree_gate_control_plane.py.

The fail-open diagnostic added in 35d18ff3a earned its keep on its first run and named the holder:

[spawn-lock] single launch waited 30.4s behind session_mail._race claimed x16 pid=1064 ... and proceeded UNSYNCHRONISED
[spawn-lock] single launch waited 30.1s behind session_mail._race claimed x16 pid=1064 ... and proceeded UNSYNCHRONISED

Same storm, same pid, both launches. So the lock engaged and then disarmed itself at its own ceiling: each waited _SINGLE_WAIT_S out, gave up while the turnstile was still held, launched into the storm and blew the caller's 45s bound.

_SINGLE_WAIT_S was 30.0, sized from a 20-core box's 26.5s of bursts — while the same comment block already recorded CI's window as 30-39s. The ceiling sat at the bottom of the range it had to cover, and a 4-vCPU runner's storm is not faster than a 20-core one's. Raised to 90.0, under both the storm's own 300s test bound and the 360s turnstile reap.

This is not the "raising the ceiling" the module docstring refutes, and the comment now says so where the next reader hits it. That refutation is about GATE_TIMEOUT_S, the bound on a launch already running. This constant is the opposite end — how long a launch waits before it starts. It also cannot push a launch past its own timeout: the wait runs before subprocess.run is called, so the caller's timeout clock starts at process launch.

On the collision

I was dispatched to fix the path defect and had it committed locally (74338d93b, PureWindowsPath) when the push was rejected — 432a4eb03 had landed the same fix minutes earlier. I dropped mine rather than rewrite a peer's landed work to swap one correct expression for another. Nothing of theirs was forced over; this pushed as a fast-forward on top.

Worth noting for #1304's record: that is the second time on this PR that two seats each worked the same defect believing the other was not.

Checks

ruff check, ruff format --check, mypy strict clean on messagefoundry (275 files) and both changed test modules. pytest green on test_spawn_lock + test_coord_lock + test_tooling_partition (32) and test_session_mail + test_worktree_gate (194).

Every local run reports INCOMPLETE RUN — the vault extra is absent — so none of them establishes a green full suite. This 20-core box carried 28 resident pwsh and 0 competing python and cannot reproduce a 4-vCPU contention failure; no timing was tuned to make a local run pass.

A new pin asserts the wait against the measured window rather than a literal, so re-sizing on new evidence stays free while dropping it back under the storm does not. It fails at the shipped value with assert 39.0 < 30.0, verified.

Still open, named and not built

The windows fix is reasoned from one job's evidence, not reproduced locally — this box cannot produce the contention. The 90.0 is sized to clear the measured window with headroom, but the true storm duration on a 4-vCPU runner is still not measured; the _note line will say so again if it is still short. The module's own docstring also lists roughly 66 files in this tier that launch pwsh without taking the shared side at all, so a storm still does not wait for them. Left alone deliberately.

@wshallwshall
wshallwshall added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit b8421ca Sep 17, 2026
42 checks passed
@wshallwshall
wshallwshall deleted the claude/b1304-pwsh-contention branch September 17, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant