Skip to content

Harden rescue task handoff failures - #296

Open
pengyou200902 wants to merge 1 commit into
openai:mainfrom
pengyou200902:fix/rescue-duplicate-task-failure-paths
Open

Harden rescue task handoff failures#296
pengyou200902 wants to merge 1 commit into
openai:mainfrom
pengyou200902:fix/rescue-duplicate-task-failure-paths

Conversation

@pengyou200902

@pengyou200902 pengyou200902 commented May 5, 2026

Copy link
Copy Markdown
Contributor

Issue

Follow-up to #234 and #235. PR #235 fixed the original /codex:rescue recursion by routing the command through the Agent tool instead of re-entering Skill(codex:rescue). That fixed the missing-Agent/fork routing bug, but it left adjacent duplicate-handoff paths outside its scope.

Those remaining paths happen when the rescue forwarder's first task attempt fails before returning a useful terminal result. In particular, shell-quoted prompt forwarding is fragile for multiline or shell-sensitive task text, ambiguous model handling can pass an unintended model name, and the forwarder contract previously said to return nothing on failure, which gives the caller no clear terminal result.

Fix

  • Add explicit task --stdin support so the companion script reads forwarded task text directly instead of receiving it as a shell-quoted CLI argument.
  • Reject mixed prompt sources with --stdin (--prompt-file or positional prompt text) so prompt transport is deterministic.
  • Allow empty --stdin only for resume runs, preserving resume-only fallback to the default continue prompt.
  • Tighten rescue model forwarding: pass only explicit user-requested model strings, with the documented spark -> gpt-5.3-codex-spark alias exception.
  • Replace the empty failure response contract with a non-retry contract: return failure output exactly once and stop.
  • Drop the stale experimentalRawEvents app-server request field so the build matches the current generated Codex protocol types.

Tests

  • npm run build
  • node --test tests/commands.test.mjs
  • node --test tests/runtime.test.mjs -t "task --stdin"
  • npm test
  • PR CI passes on latest commit

@pengyou200902
pengyou200902 force-pushed the fix/rescue-duplicate-task-failure-paths branch 2 times, most recently from c4606d7 to a00fa06 Compare May 5, 2026 00:39
@pengyou200902
pengyou200902 force-pushed the fix/rescue-duplicate-task-failure-paths branch from a00fa06 to ac8690d Compare May 5, 2026 01:10
@pengyou200902
pengyou200902 marked this pull request as ready for review May 5, 2026 01:32
@pengyou200902
pengyou200902 requested a review from a team May 5, 2026 01:32
@lzrong0203

Copy link
Copy Markdown

Independent confirmation of the failure mode this PR targets, from a field incident we then verified forensically and reproduced mechanically. Sharing because it directly exercises both halves of this fix (the non-retry contract and --stdin), and it isolates which part of the problem remains out of scope.

Field incident (v1.0.4; the rescue instruction files and broker/tracked-jobs code are byte-identical through 1.0.6 and current main)

One rescue request → two concurrent server-side jobs:

UTC Event
07:57:08 Forwarder runs foreground task "$(cat promptfile)" (multiline prompt forced the temp-file detour this PR's --stdin eliminates)
07:57:10 Server-side job A created (task-mrona8sx-…)
08:07:10 Host Bash tool kills the CLI at its 600 s ceiling (exit 143, "Command timed out after 10m 0s") — the timeout output contains no job id
08:08:01 Forwarder retries with task --background → job B (task-mrono8y5-…), same prompt
08:12:47 Manual cancel of job A: interrupt reports "thread not found" — the orphaned turn had already completed unattended; only the manual cancel transitioned its record out of running

Why the forwarder retried instead of stopping: the only failure rule it has is "If the Bash call fails or Codex cannot be invoked, return nothing" (agents/codex-rescue.md:42), and it is explicitly forbidden to call cancel/status/result (codex-rescue.md:27–28). A timeout-after-job-creation therefore leaves it with no cleanup path and no terminal result to return — exactly the gap the "return failure output exactly once and stop" contract closes.

Mechanical reproduction (no model in the loop)

timeout 20 node scripts/codex-companion.mjs task "<any prompt that runs >20s>" ; echo $?   # → 124

The killed client's job survives (still running ~44 s later, ≈3× the task's natural duration), the server-side turn completes unattended, and the record leaves running only via manual cancel ("no active turn to interrupt"). So the orphan requires nothing model-driven — any client death does it.

The residual half, for scoping

Code-wise the freeze happens because app-server-broker.mjs:225–233 socket close/error handlers clear ownership but never interrupt the turn, and lib/tracked-jobs.mjs:142–204 transitions a record only from the client's own try/catch — there is no pid-liveness or heartbeat check. This PR prevents the duplicate submission; a dead client still wedges its job record at running (the #122 / #372 / #432 / #486 family). The two fixes compose well — just noting the boundary so this PR isn't expected to solve that half.

Full forensic timeline (transcript + job JSONs) available if useful.

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.

2 participants