Skip to content

fix(codex-bridge): make the failure cap reach and bound the re-arm rate (#906 link 3) - #941

Merged
fujibee merged 2 commits into
mainfrom
fix/bridge-rearm-hotspin
Aug 22, 2026
Merged

fix(codex-bridge): make the failure cap reach and bound the re-arm rate (#906 link 3)#941
fujibee merged 2 commits into
mainfrom
fix/bridge-rearm-hotspin

Conversation

@fujibee

@fujibee fujibee commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Part of #906 (link 3). On main.

The measurement first

#906 lists "re-arm loop has no backoff" as a root cause. Measured against the real bridge (a fake WS app-server whose process/spawn exit codes are scripted), that is only half true -- the consecutive-failure cap (--max-failures, default 3) already exists (#209) and fires. Four scenarios, before this PR:

scenario watch-once returns before
(a) every cycle exit 124 cap fires at 3 arms, stopping after 3 consecutive, exit 1
(b) fail, fail, wake (distinct max_id), repeating never stops -- the wake reset the failure counter to 0, so a bridge that kept delivering also kept failing, forever
(c) a wake (distinct max_id) every cycle 2094 arms in 56 s (~37/s), no throttle
(d) a wake with one repeated max_id stale-wake guard stops it at 2 arms (correct)

So the incident's "~80 armed from one PID that never gives up" is (b): the cap is real but never reaches, because any wake resets it. And (c) is a latent second hole: the wake -> re-arm path has no rate floor at all. The fork storm the reporter saw (800 PIDs/s) is each armed watch-once forking its own library sourcing under pressure; the re-arm loop does not create that rate, it just restarts it forever -- which is why making the cap reach (b) is the direct fix, and bounding the rate (c) is the belt-and-suspenders.

The change (codex-bridge.js), two ceilings in one place each

(1) A wake decays the failure counter by one instead of resetting it to zero. A wake is progress, not proof the host recovered -- amid failures it is exactly the (b) churn. Forgiving one failure per wake lets a genuinely-recovered bridge (mostly wakes) fall to zero, while a fail/fail/wake churn nets +1 per cycle and reaches the cap. A clean deadline (exit 2 -- a full --timeout that ran end to end) is the stronger signal and still resets outright.

(2) A minimum interval between arms (MIN_ARM_INTERVAL_MS, 1 s), enforced in armWatch -- the one path every re-arm goes through (a clean deadline, a wake and its turn, an idle transition). watch-once's own deadline paces the healthy case at one arm per --timeout (300 s), so the floor is only ever felt by a degenerate loop; a deferred arm still runs, nothing is dropped.

Both are ceilings on "how fast / how forgiving," kept together so the next reader sees (b) and (c) as one subject rather than reading "the cap was fixed" and forgetting (c).

Not done here: widening the stale-wake guard from "one repeated max_id" to "N wakes in a window" -- (d) shows the guard works as specified, and (2) already bounds the distinct-wake flood, so it is a separate judgment, not this PR.

After the fix, re-measured (same harness)

scenario before after
(a) every fail cap fires, 3 arms, exit 1 unchanged -- 3 arms, exit 1 (no regression)
(b) fail,fail,wake never stops cap reaches -- stops at ~7 arms, exit 1
(c) wake flood 2094 arms / 56 s (37/s) ~1 arm/s -- 26 arms / 25 s
(d) same-id flood guard fires, 2 arms unchanged -- guard fires, exit 1

Verification

  • tests/test_codex_bridge.bats +2, driving the real bridge against the scripted fake: (b) the failure cap reaches even when wakes interleave (the bridge stops itself), and (c) a flood of distinct wakes is rate-limited (arm count over ~6 s stays in single digits, where the unbounded loop produced hundreds). The two unchanged scenarios (a)/(d) are already covered by existing tests and the failure-cap test above.
  • Full test_codex_bridge.bats 44/44 local.

The consecutive-watch-once-failure cap (default 3) already exists (#209),
so #906's "re-arm has no backoff" is not the whole story. Reproduced
against the real bridge with a fake app-server, four scenarios:

  (a) every watch-once fails            -> cap fires at 3 (already works)
  (b) fail, fail, wake, repeating       -> cap NEVER reached: the wake
                                           reset the counter to 0, so a
                                           bridge that kept delivering also
                                           kept failing, forever
  (c) a flood of distinct wakes         -> 2094 arms in 56 s (37/s), no
                                           throttle -- each arm forks
                                           watch-once's library sourcing,
                                           the fork pressure #906 saturated
  (d) a flood of one repeated wake      -> stale-wake guard stops it (ok)

Two ceilings, so both (b) and (c) are bounded and the next reader sees
them as one subject rather than assuming the cap "was fixed":

(1) A wake decays the failure counter by one instead of resetting it to
zero. A wake is progress, not proof the host recovered; forgiving one
failure per wake lets a genuinely-recovered bridge fall to zero while a
fail/fail/wake churn still climbs to the cap. A clean deadline (exit 2) --
a full timeout that ran end to end -- is the stronger signal and still
resets outright.

(2) A minimum interval between arms (MIN_ARM_INTERVAL_MS), on armWatch,
the one path every re-arm goes through. watch-once's own deadline paces
the healthy case at one arm per --timeout, so the floor is only ever felt
by a degenerate loop.

After the fix, re-measured: (a) still 3 arms/stop, (b) now stops at ~7
arms, (c) ~1 arm/s (26 in 25 s, was 37/s), (d) still guarded. Tests pin
(b) and (c) against the real bridge.

Part of #906 (link 3).
…st [[ ]]

The two message checks were `[[ "$output" =~ ... ]]` in a non-last
position, which cannot fail on macOS bash 3.2 -- the enforced-assertions
guard (#670) flags exactly that. Use `grep -q ... <<<"$output"`, a plain
command that trips errexit on both shells.
@fujibee
fujibee merged commit f6d44d3 into main Aug 22, 2026
22 checks passed
fujibee added a commit that referenced this pull request Aug 22, 2026
Rebasing this branch onto main after #941 landed put #941's two #936
tests just before this branch's #906 test in the same file; resolving
that conflict dropped the closing brace of #941's second test, leaving
the file unparseable. Restore it. Tests only; no logic change.
fujibee added a commit that referenced this pull request Aug 22, 2026
…k 1) (#935)

* fix(codex-bridge): exit when another writer owns the thread, don't proceed

The thread/resume catch was written to tolerate a benign failure -- a
Codex 0.142+ --remote session that never created a rollout, where
turn/start still works from the threadId alone, so the bridge stays
alive idle. But it swallowed a deterministic one too: "already has an
active writer" (-32600) means another writer -- a co-resident Codex
Desktop, or a second bridge -- owns this thread, and resume cannot
succeed while that holds. A bridge that proceeds anyway still arms
watchers and holds ~10 threads, so on a host with a per-user pid limit
duplicates accumulate until the slice is saturated and every fork of
every process of the user fails (#906, two incidents in one day on an
HPC login node).

Now the two are told apart. The JSON-RPC clients carried only
message.error.message and dropped the code; both now attach it. The
catch matches the "already has an active writer" message -- the
condition itself, not the generic -32600 code, which is also returned
for other invalid requests -- and die()s, so a bridge that cannot own
its thread stops instead of lingering. Every other resume failure keeps
the benign proceed-without-resume, unchanged (the #276 test still
passes).

Part of #906 (link 1 of the reported chain; the launcher orphan scan,
re-arm backoff, and cross-host PID qualification are separate).

* docs(codex-bridge): the resume-error code is diagnostic, not the gate

Address a review nit: the client comment implied ensureThread needs the
code to distinguish the active-writer case, but it decides on the
message. Say the code is carried for diagnostics and future callers, not
for that branch.

* test(codex-bridge): assert the active-writer case with forms that fail on 3.2

The three checks were `[[ ]]` in non-last positions, which report ok with
a false claim inside on macOS bash 3.2 -- the shell CI runs. So on the
macOS shards those assertions were no-ops. Use grep for the positive and
`[ "$(grep -c ...)" -eq 0 ]` for the negatives, which fail on both shells.
No logic change.

* test(codex-bridge): restore the brace dropped rebasing onto #941's tests

Rebasing this branch onto main after #941 landed put #941's two #936
tests just before this branch's #906 test in the same file; resolving
that conflict dropped the closing brace of #941's second test, leaving
the file unparseable. Restore it. Tests only; no logic change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant