fix(codex-bridge): give the identity lease a start token on Windows - #978
fix(codex-bridge): give the identity lease a start token on Windows#978joelmitz wants to merge 3 commits into
Conversation
The codex bridge could not start on Windows at all. Every launch died in
writeLease() with "cannot determine process start token for identity lease",
20+ times in a row in run/codex-bridge.<team>.<agent>.log.
startToken() had exactly two sources and Windows satisfies neither:
/proc/<pid>/stat ENOENT -- Windows has no /proc
ps -o lstart= "ps: unknown option -- o" -- the only ps likely to be on
PATH there is MSYS's, and it rejects -o outright rather
than degrading
Both yield an empty token, and writeLease() fails closed on an empty token by
design (fujibee#906: a bridge that cannot publish an enumerable lease must not go on
to arm its network, or it becomes exactly the unreapable orphan the lease
exists to prevent). So the fail-closed was correct; the missing source was not.
Adds Process.StartTime.Ticks via PowerShell as the Windows source, in both
codex-bridge.js startToken() and codex-bridge-launcher.sh _start_token, and
admits "pwsh" in the lease schema.
ONE source, not a preference list
---------------------------------
WMIC's CreationDate is ~2x cheaper (0.36s vs 0.83s measured) and was the first
implementation, but it is deprecated and already absent from Windows 11 installs
that have dropped the Feature-on-Demand.
The problem is not that WMIC may be missing. It is that a per-side "WMIC, else
PowerShell" order lets the writer and the reaper resolve DIFFERENT sources for
the SAME process whenever only one of the two can reach wmic.exe. Their tokens
then differ by FORMAT, so the reaper reads a live bridge's lease as some other
process's and never collects the orphan. This was reproduced, not theorised:
with wmic shadowed on one side only, the two sides returned
node: src=wmic token=20260824045224.102163+540
bash: src=pwsh token=639231439441021632
for one pid. Ticks is the one value both sides can always agree on, so WMIC's
speed is not worth the divergence and it is not used at all.
Falling back from powershell.exe to pwsh is safe for a reason that does not
apply to WMIC: both return the SAME Ticks for a given pid (measured), so the
src label names the format, not the executable. Verified that when
powershell.exe is unreachable, both sides fall through to pwsh and still agree.
Windows must not take the /proc branch
--------------------------------------
The Windows branch is taken INSTEAD of the /proc branch, not merely before it.
MSYS and Cygwin do expose a working /proc -- field 22 and all -- but it is keyed
by the emulation layer's own pid space, while a lease records the Windows pid
codex-bridge.js sees as process.pid. Measured on MINGW64: the same shell is MSYS
pid 3065729 and winpid 1456. Letting /proc win would silently return the start
time of whatever unrelated MSYS process sits at that number, which is the
recycled-pid confusion the token exists to prevent, only harder to notice
because nothing errors.
Verification
------------
Windows 11 26200, MINGW64, node 22 (win32), codex-cli 0.149.0.
- node startToken() and shell _start_token return byte-identical
"pwsh<TAB>639231441791462826" for the same live Windows pid; likewise when
powershell.exe is unreachable and both fall through to pwsh
- _read_lease: 8 cases pass -- pwsh integer accepted, pwsh non-integer and
empty rejected, wmic-format rejected, proc/ps regressions still accepted,
proc non-numeric and an unknown src still rejected
- end to end: bridge starts (alive, armed), publishes
start=639231452158110466 startsrc=pwsh, and an agmsg message sent to the role
drives a turn and gets an answer back. Before this change the same sequence
produced only the token error
- tests/test_codex_bridge_launcher.bats gives byte-identical results with and
without this change on this machine (10 of 12 fail either way; the suite does
not run on MSYS). NOT validated on Linux or macOS -- the POSIX branches are
untouched, but that is an argument, not a measurement
…rim the rationale to the PR Adds the regression tests the previous commit described but did not commit, and moves the evidence behind the WMIC decision out of the code and into the PR. Tests ----- `_read_lease` is the reaper's only gate on a lease, so its accept/reject set is the contract that widening `proc|ps` to `proc|ps|pwsh` changes. The six cases exercise it directly -- the pattern tests/test_remote.bats already uses for `_remote_endpoint_display` -- rather than through the reaper, which needs a spawnable bridge and a live pid. That is exactly what does not work on Git Bash (fujibee#567), and the schema question depends on neither. Only the case that actually runs PowerShell carries `windows-native` in its name, so it lands on the Windows leg; the schema cases carry no such marker and run on every leg. What each case is for, stated plainly rather than as "6 tests pass": | case | with the change | without it | |---|---|---| | a pwsh lease parses | ok | **fails** | | windows-native: a live pid yields an integer pwsh token | ok | **fails** | | a pwsh token that is not an integer is rejected | ok | ok | | an unrecognised startsrc is rejected | ok | ok | | proc and ps still parse; proc non-numeric still rejected | ok | ok | | POSIX: a live pid yields a proc or ps token | skip on Windows | skip | Two detect the change; four are guards. The `wmic` label is in the rejection case deliberately and not as an arbitrary bad value: it pins the decision not to adopt WMIC, so reintroducing it fails loudly. Comments -------- The WMIC rationale ran 22 lines in codex-bridge.js and 20 in the launcher. The conclusion belongs in the code; the measurements behind it (the reproduced divergence, MSYS pid 3065729 vs winpid 1456) belong in the PR, where a reader looking for *why* will be. Trimmed to 10 and 14 lines with no fact dropped from the argument itself. Verification ------------ Windows 11 26200, MINGW64, bats-core 1.14.0 from a clone. - the six cases: all pass; re-run against `dbb9c2c2`'s codex/ tree, the two change-detecting cases fail as tabled above - NOT `bats tests/` green on this machine, and that is not achievable here: .github/workflows/tests.yml shards the suite over `[ubuntu-latest, macos-latest]` only, and the Windows leg runs `filter: "windows-native"`. The full file has never been expected to pass under Git Bash - `launcher: windows-native starts the bridge (fujibee#567)` fails here with and without this change. It asserts the half of fujibee#567 that is not fixed: on Git Bash the parent-liveness probe asks `tasklist` about an MSYS pid. Measured on this machine -- the same shell is MSYS pid 3994449 and winpid 19568; `tasklist` finds only the latter, `kill -0` only the former. The workflow itself lists `windows runtime (fujibee#567)` among its known intermittent reds - ubuntu/macos results are NOT from this machine and are not claimed here
Verified on Windows — #977 resolvedTested on Applied this PR on top of upstream/main ( startToken unit
Both return valid integers via bats ( PR #978 added tests 30–35: 5 pass + 1 skip (test 34 is POSIX-only, correctly skipped). Existing tests: 14 pass / 19 fail / 2 skip — matches the upstream/main baseline (12 pass / 16 fail / 1 skip). The 3 flipped tests ( Bridge real startup
Multi-agent E2E (monitor mode) Ran a 3-seat codex fan-out (implementer / reviewer / verifier) with this PR applied. All 3 bridges started cleanly, leases acquired with |
|
Additional Windows measurement, one code-level note, and a scope remark — from an independent local fix of the same failure (#977), written before we found this PR. Environment. Windows 11, Git for Windows 2.55 (MSYSTEM=MINGW64), node 24.19, codex-cli 0.153.4, agmsg Reproduction of #977 on this base. Every bridge spawn exited at startup with Measured with a local patch of the same shape as this PR (PowerShell
Separately, after adding our local Windows reaper changes (see the scope remark below), three cleanup runs with Test condition worth stating: the project already had a role-session record from an earlier session, so the launcher first bound the bridge to that stale thread. Because One untested concern on the token expression. Scope remark. With this PR merged, a Windows bridge can start, but Related discussion I read while doing this: #458 (pid spaces), #996 (liveness / absence / unknown contract), #149 (TUI exit leaves the bridge). I am not treating any of them as a blocker for this PR; they are context for the follow-up. |
|
Thanks for this, and for the detailed measurements — and thanks to @masa6161 and @NNK-github for the independent Windows verification. The three macOS failures on the CI run ( Moving the pattern out of the command substitution fixes it on both bash 3.2 and 5.x: diff --git a/tests/test_codex_bridge_launcher.bats b/tests/test_codex_bridge_launcher.bats
index 5c2761e..7973669 100644
--- a/tests/test_codex_bridge_launcher.bats
+++ b/tests/test_codex_bridge_launcher.bats
@@ -715,7 +715,8 @@ _lease_verdict() { # <startsrc> <start> -> prints accept|reject
printf 'v=1\nproject=%s\npairs=%s\nhost=h\npid=123\nstart=%s\nstartsrc=%s\n' \
"$h40" "$h40" "$2" "$1" > "$TEST_SKILL_DIR/lease-under-test"
bash -c '
- eval "$(sed -n "/^_read_lease() {/,/^}/p" "$1")"
+ pattern="/^_read_lease() {/,/^}/p"
+ eval "$(sed -n "$pattern" "$1")"
_read_lease "$2" && echo accept || echo reject
' _ "$LAUNCHER" "$TEST_SKILL_DIR/lease-under-test" 2>/dev/null
}
@@ -751,7 +752,8 @@ _lease_verdict() { # <startsrc> <start> -> prints accept|reject
_run_start_token() { # <pid> -> runs _start_token in a subshell
run bash -c '
- eval "$(sed -n "/^_agmsg_is_windows() {/,/^}/p;/^_start_token() {/,/^}/p" "$1")"
+ pattern="/^_agmsg_is_windows() {/,/^}/p;/^_start_token() {/,/^}/p"
+ eval "$(sed -n "$pattern" "$1")"
_start_token "$2"
' _ "$LAUNCHER" "$1"
}If you can fold that into the branch, the remaining reds on the run are the known intermittent ones (#937, #567) and we can re-run CI. The Windows-side change itself looks right to us. |
|
One more thing from a closer read: the new |
|
Addressed both review comments in
Verification on Linux:
The full launcher Bats file also exercised all 34 cases: 30 passed, 2 Windows-only cases skipped, and 2 known intermittent |
Fixes #977.
The codex bridge could not start on Windows at all:
startToken()had twosources and Windows satisfies neither, so
writeLease()failed closed on anempty token every time and the launcher respawned it into the same wall.
The fail-closed is correct (#906) — the missing Windows source is what this adds:
Process.StartTime.Ticksthrough PowerShell, in bothcodex-bridge.jsstartToken()andcodex-bridge-launcher.sh_start_token, withpwshadmittedto the lease schema.
One source, not a preference list
WMIC's
CreationDateis ~2x cheaper (0.36 s vs 0.83 s measured) and was thefirst implementation. It is dropped, and not because WMIC may be missing: a
per-side "WMIC, else PowerShell" order lets the writer and the reaper resolve
different sources for the same process whenever only one of them can reach
wmic.exe, and their tokens differ by format — so the reaper reads a livebridge's lease as another process's. Reproduced with
wmicshadowed on one side:powershell.exe→pwshfallback is safe where WMIC was not: both returnidentical Ticks for a pid, so the
srclabel names the format, not the binary.The Windows branch replaces the
/procbranch rather than preceding itMSYS/Cygwin expose a working
/proc, but it is keyed by their own pid spacewhile the lease records the Windows
process.pid, so preferring it would returnan unrelated process's start time with nothing to signal it. Measurements in #977.
Tests
Six cases against
_read_lease— the reaper's only gate on a lease, so itsaccept/reject set is the contract that widening
proc|pstoproc|ps|pwshchanges. Exercised directly, the way
tests/test_remote.batsalready does for_remote_endpoint_display, rather than through the reaper: that path needs aspawnable bridge and a live pid, which is what does not work under Git Bash
(#567) and has nothing to do with the schema.
Two detect the change; four are guards. Stated as a table rather than a pass
count so a reviewer checking it finds the same thing:
pwshlease parseswindows-native: a live pid yields an integerpwshtokenpwshtoken that is not an integer is rejectedstartsrcis rejected (wmicincluded)proc/psstill parse;procnon-numeric still rejectedprocorpstokenwmicis in the rejection case deliberately, not as an arbitrary bad value: itpins the decision above, so reintroducing WMIC fails loudly.
Only the case that runs PowerShell carries
windows-nativein its name, so itlands on that leg; the rest carry no marker and run everywhere. The two
platform-specific cases complement each other — on either leg one runs and the
other skips.
Verification
pass; re-run against the parent commit's
codex/tree, the twochange-detecting cases fail as tabled
windows-nativecases pass andthe mutation check reproduces. Run independently on a separate machine by a
second person, not a re-run of the Windows box
start=639231452158110466 startsrc=pwsh, and a message sent to the role drivesa turn and gets an answer. Before this change the same sequence produced only
the token error
Not verified
bats tests/green on Windows. That is not achievable and is not new: theworkflow shards the suite over
[ubuntu-latest, macos-latest]only, and theWindows leg runs
filter: "windows-native". The full file has never beenexpected to pass under Git Bash
launcher: windows-native starts the bridge (#567)fails on our Windowsmachine with and without this change. It asserts the half of codex bridge on Windows: port-detection liveness probe asks tasklist about an MSYS pid, so it always aborts on iteration 1 #567 that is not
fixed: the parent-liveness probe asks
tasklistabout an MSYS pid, and the twopid spaces are disjoint (
tasklistfinds winpid 19568 only;kill -0findsMSYS pid 3994449 only). The workflow already lists
windows runtime (#567)among its known intermittent reds
that is an argument, not a measurement
Unrelated flake, for context
tests/test_codex_bridge_launcher.batsflakes under full-suite load on Linuxindependently of this change: baseline produced the same
not ok 17on one ofthree full runs, and the failing test moves between runs, while single-test runs
were green 3/3 on both. We are not claiming equal rates — the run count is far
too small — only that the same failure occurs without this change.