fix: bound turn/interrupt with a timeout so cancellation can't hang the companion by axisrow · Pull Request #48 · axisrow/codex-plugin-cc

axisrow · 2026-08-02T12:37:58Z

Summary

interruptAppServerTurn (plugins/codex/scripts/lib/codex.mjs:1206) called client.request("turn/interrupt", ...) with no timeoutMs. If a broker accepted the connection and then went silent, the companion would block on that await until the host's own SIGKILL — leaving the original write-capable turn running on the broker with nobody able to reach it.
Adds TURN_INTERRUPT_TIMEOUT_MS = 5000 and passes timeoutMs + onTimeout (force-close via client.close()) to the request, mirroring the existing pattern used for the initialize handshake (app-server.mjs:380-389).
This is fix Add model and effort flags to review commands #1 from the issue's proposed list — the cheapest, highest-value item, closing the unbounded-cleanup hole described as "Finding 2."

Finding 1 (deadline anchored after an unbounded setup phase) and the remaining proposed items are a larger design change (moving where the ceiling is armed) and are left for a follow-up, as the issue itself scopes them separately.

Test plan

New fixture behavior silent-turn-interrupt in tests/fake-codex-fixture.mjs: accepts turn/interrupt but never answers it.
New test in tests/runtime.test.mjs: turn/interrupt itself is bounded: a broker that accepts it but never answers does not hang the companion (#47) — asserts the companion exits within ceiling + interrupt timeout (~9s) rather than hanging on the unanswered request.
Full suite: npm test — 185/185 passing, 0 regressions.

Fixes #47 (finding 2).

🤖 Generated with Claude Code

https://claude.ai/code/session_013hBHjrDA8DzzY2SEq4ieiS

…he companion client.request("turn/interrupt", ...) had no timeoutMs, so a broker that accepted the connection and then went silent would block the companion until the host's own SIGKILL, leaving the original write-capable turn running with nobody able to reach it. Pass a 5s timeoutMs (with onTimeout force-closing the connection), mirroring the pattern already used for the initialize handshake. Fixes #47 (finding 2; the cheapest, highest-value item from the proposed fix list). Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_013hBHjrDA8DzzY2SEq4ieiS

axisrow · 2026-08-02T18:34:52Z

🔍 Local review (cycle 1) — round 744d840d-f015-48ca-802b-fe7fe950b2a3

Reviewed locally (/review only — Codex companion was silent this round due to a usage-limit error; excluded for this run per user decision, config unchanged for future runs).

Verdict	Reviewer	Finding	Location
(n/a)	claude	turn/interrupt request now carries a bounded timeout with force-close on expiry, matching the initialize-handshake pattern	plugins/codex/scripts/lib/codex.mjs:1215
SKIP	claude	test asserts a generous 15s wall-clock bound rather than pinning close to the actual ~7.5s budget, so a future regression widening the timeout would not be caught precisely	tests/runtime.test.mjs

No critical issues found. One non-blocking suggestion noted, left as-is (does not affect correctness).

axisrow · 2026-08-02T18:35:28Z

📋 Review summary — all cycles

Cycle	Reviewer	Finding	Verdict	Resolution
(pre-review)	author	turn/interrupt lacked a request timeout, allowing an unbounded hang if the broker went silent after accepting the connection	FIX	Fixed in `1e41e55` (this PR)
1	claude	test bound is generous (15s) relative to the actual ~7.5s expected duration, reducing precision of the regression guard	SKIP	Left as-is (non-blocking, does not affect correctness)

Totals: 1 FIX (resolved before review, in the PR itself), 1 SKIP.

Note: Codex companion was silent this round (usage-limit error, resets ~Aug 8 2026) and was excluded from this run per explicit user decision; the review-only reviewer for this cycle was /review (Claude). Saved cycle-review config is unchanged — future runs will include Codex again.

Codex review of PR #48 (issue #47 finding 2) caught that interruptAppServerTurn's onTimeout handler called client.close(), but BrokerCodexAppServerClient.close() sent socket.end() and awaited exitPromise with no fallback. A peer that accepts the FIN but never reciprocates leaves exitPromise unresolved forever, so close() could hang indefinitely — defeating the request-level timeoutMs the previous commit added, since the cleanup path it triggers wasn't itself bounded. Add a 50ms grace period before force-destroying the socket, mirroring the pattern the spawned client's close() already uses (stdin.end() + killChildNow() fallback). New fixture spawnStubbornBroker() completes the handshake and answers requests, but (via allowHalfOpen: true) never reciprocates the FIN -- reproducing the exact wedge Codex described. Verified red without the fix (hangs ~3s) and green with it (~56ms, matching the grace period). Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_013hBHjrDA8DzzY2SEq4ieiS

axisrow · 2026-08-02T19:10:18Z

Thanks — this is a real bug, fixed in 2320230.

interruptAppServerTurn's onTimeout called client.close(), but BrokerCodexAppServerClient.close() only sent socket.end() (graceful FIN) and awaited exitPromise, which resolves solely from the socket's "close" event. A peer that accepts the FIN but never reciprocates leaves that promise unresolved forever — so close() itself was unbounded, which meant the request-level timeoutMs from the previous commit didn't actually bound the cleanup path it triggered.

Fix: close() now force-destroy()s the socket after a 50ms grace period if exitResolved hasn't flipped by then — mirroring the pattern the spawned-client's close() already used (stdin.end() + a killChildNow() fallback timer).

Added a regression test (tests/broker-client-close-hang.test.mjs) with a new spawnStubbornBroker() fixture that completes the handshake, answers requests, but (via allowHalfOpen: true) never reciprocates the client's FIN. Verified it's red without the fix (hangs ~3s, correctly caught) and green with it (~56ms, matching the grace period) — confirmed by temporarily reverting the fix and re-running.

axisrow · 2026-08-02T19:12:19Z

🔍 Local review (cycle 2) — Codex re-review

Re-reviewed by Codex against the fix commit (2320230): verdict approve, no material findings. (Codex's own sandbox couldn't execute the test suite — EPERM on temp dirs — but the fix was independently verified locally: red without it (~3s hang), green with it (~56ms), plus a full npm test pass, 0 failures.)

📋 Review summary — all cycles

Cycle	Reviewer	Finding	Verdict	Resolution
1	claude (`/review`)	test's elapsed-time bound (15s) is looser than the actual expected ~7.5s budget	SKIP	Left as-is, non-blocking
1	codex	`onTimeout`'s `client.close()` on the broker transport can hang forever if the peer never reciprocates the FIN, defeating the request-level timeout	FIX	Fixed in `2320230`
2	codex	re-review of the fix	approve	No material findings

Totals: 1 FIX (resolved), 1 SKIP, cycle 2 clean (approve, no new findings). CI: no checks configured on this repo/branch.

axisrow merged commit 44b5172 into main Aug 3, 2026

axisrow deleted the ao/codex-plugin-cc-8/fix-issue-47-interrupt-timeout branch August 3, 2026 01:13

Navigation Menu

Repository navigation

fix: bound turn/interrupt with a timeout so cancellation can't hang the companion - #48

fix: bound turn/interrupt with a timeout so cancellation can't hang the companion#48
axisrow merged 2 commits into
mainfrom
ao/codex-plugin-cc-8/fix-issue-47-interrupt-timeout

axisrow commented Aug 2, 2026

Uh oh!

axisrow commented Aug 2, 2026

Uh oh!

axisrow commented Aug 2, 2026

Uh oh!

axisrow commented Aug 2, 2026

Uh oh!

axisrow commented Aug 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

axisrow commented Aug 2, 2026

Summary

Test plan

Uh oh!

axisrow commented Aug 2, 2026

🔍 Local review (cycle 1) — round 744d840d-f015-48ca-802b-fe7fe950b2a3

Uh oh!

axisrow commented Aug 2, 2026

📋 Review summary — all cycles

Uh oh!

axisrow commented Aug 2, 2026

Uh oh!

axisrow commented Aug 2, 2026

🔍 Local review (cycle 2) — Codex re-review

📋 Review summary — all cycles

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant