[jbeckwith-oai]

Summary

This pull request fixes completed tool outputs disappearing when another tool in the same turn fails. Client-managed runs retain accepted call/output pairs, required reasoning and replay ancestry in Session history, error details, streamed results, and existing RunState recovery. The run still reports the failure, and rejected input or tool output is not published.

Accepted output is recorded after tool output guardrails and provider conversion, before optional custom-data extraction and end hooks. Failed runs may therefore retain an output with incomplete optional metadata. Local recovery snapshots detach supported mutable output values, tool-output guardrail diagnostics, replay payloads and SDK metadata from late callbacks while preserving stream occurrence markers and provider-owned response identities. Live copies preserve built-in container and SDK structured-output model types so callbacks on same-state retries still receive typed outputs. Built-in copies also preserve recursive backlinks and shared references, so accepted cyclic values cannot replace the selected failure during snapshotting. Opaque application values retain identity; conversion to plain data remains at JSON serialization. Successful runs retain their existing metadata behavior. The non-streaming selected-error path waits for pending input admission only when there is completed history to publish; otherwise it cancels and drains the pending guardrail. Parent cancellation remains distinct from a cancelled guardrail child. Existing Session reconciliation handles failed writes; there is no new public API or persisted schema.

Scope and limitations:

• Recovery when on_llm_end raises before tool execution is separate work. The baseline already loses completed provider results at that earlier response-hook boundary; this PR does not change provider-result acceptance timing or claim to fix it.
• Server-managed continuation (conversation_id / response chaining) keeps its existing recovery path. The known local-output gap on follow-up failures is separate work.
• Cleanup remains bounded. Application callbacks that suppress cancellation cannot be forcibly terminated by asyncio; this change does not guarantee their termination or bounded aggregate resource use.
• Retained history does not guarantee exactly-once external side effects across crashes, failed persistence, or new model-issued calls. Application idempotency remains necessary where required.

Test plan

• Required verification stack passed: format, lint, Mypy, Pyright, and 12,320 tests (69 skipped).
• 569 focused tests covering completed siblings, admission, metadata, native/function tools, approval resume, Session reconciliation, hosted events, ToolSearch and Program replay.
• 10 focused Python 3.10 metadata/cancellation regressions passed.
• Two consecutive clean fresh-context adversarial review rounds on the final content.
• All 23 CI checks passed on 7bb7c7a3, including native macOS sandbox coverage, both Windows jobs, and Python 3.10–3.14. The two conditional release-refresh jobs were skipped.

Issue number

Closes #5327

Checks

• Added regression coverage
• Ran .agents/skills/code-change-verification/scripts/run.sh
• All local verification steps pass
• Independent adversarial review completed on the final content

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T00:10:42.729084Z	7bb7c7a	New commits
🔒 Security Review	✅ Completed 2026-10-14T00:04:53.886649Z	7bb7c7a	New commits

Security findings

Advisory findings (3)

• 🟡 Cancel native finalizers after the drain timeout · Medium · Resolved · From earlier review · 8493f58
• 🟡 Ensure canceled native finalizers terminate before returning · Medium · Resolved · From earlier review · 5a2bc69
• 🟡 Raise late tripwires after tool-local cancellation · Medium · Resolved · From earlier review · 57a2c71

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af69e3739e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[markstuart-oai]

Reviewed 39324dd. I found a passing-input timing case that still loses completed side effects; the inline comment describes the trigger and regression test.

I also confirmed two existing findings against this commit:

• Supplied non-streamed RunState is not synchronized. After an approved interruption plus accepted state.add_input(...), the generated/session arrays are copied before the next turn. This handler appends completed pairs and the response only to local arrays, so serializing the supplied state after the tool error loses them. Even without added input, the session and raw-response copies lag. Synchronize the recovery state before rethrowing.
• Streamed replay loses the tool-use snapshot. The new error branch bypasses the normal tracker snapshot. A first turn with tool_choice="required" can retain completed outputs but restore an empty tracker, forcing another tool call on resume instead of resetting tool choice.

Please make partial-turn acceptance update the complete recovery checkpoint, including transcript, response and tool-use state, through the existing state owners. The two runners currently update different subsets of that state. The earlier non-streamed input-admission comment does not reproduce here: resumed turns set all_input_guardrails to an empty list.

Validation: source review of the four-file diff, lifecycle/state owners and regression tests. GitHub reports 23 successful checks and two skipped readiness checks at this head. I did not run repository workloads. The added tests cover the basic failure modes, but not the delayed passing input verdict or these resume-state cases.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e334fbc96

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 8493f588e1

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8493f588e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 959ebbb16a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dpiet-oai]

The non-streaming runner still loses completed tool history when a default parallel input guardrail is pending; see the inline finding.

[chatgpt-codex-connector]

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 5a2bc6993d

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c177ae685

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dpiet-oai]

A completed side effect can still be forgotten when custom-data finalization exceeds the bounded wait; see the inline finding.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57a2c7122e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 57a2c7122e

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

[jbeckwith-oai]

Reviewed all 16 assertions in the October 13 body-only review (review 5501972744) against b281469. No additional in-scope code change is warranted:

• Callback exception handling (2 assertions): on_tool_error_selected is an internal runner-owned signal, not an application callback. The production implementations in run.py and run_internal/run_loop.py only set the selected-error flag. The function executor invokes it before sibling draining; category handling sets its failure event and invokes the same signal. No supported throwing producer was identified. Adding catch-and-log fallback for an arbitrary replacement callback would introduce behavior outside this contract.
• Initialization and field documentation (2): partial_tool_results is initialized before the owning try (run.py:1767–1774), so the claimed unbound-local path is absent. _tool_error_selected already has an attribute docstring immediately after its declaration (result.py:711–712).
• Nesting, finalizer splitting, and parameter-object suggestions (4): these are optional refactors without a demonstrated incorrect outcome. The actual executor is _FunctionToolBatchExecutor; adding configuration dataclasses or separating coupled finalization steps is not needed for this fix.
• Failure-selection/guardrail test claims (5): test_input_verdict_during_tool_drain_preserves_selected_tool_error covers function and native categories, both Runner APIs, delayed pass/reject/error, and tool-local cancellation. test_streamed_partial_history_waits_for_input_verdict, its non-streamed counterpart, and test_selected_tool_error_survives_completed_task_guardrail_race assert caller-visible errors and retained/withheld history. These exercise signal forwarding and guardrail arbitration rather than merely asserting a private callback was called. The guardrail condition controls cancellation of the run-loop task, not cancellation of the guardrail task itself.
• Sibling-category lifecycle test claims (2): test_native_finalization_survives_sibling_failure controls the category-failure boundary and verifies finalized metadata, original error, Session history, events and serialized state for computer/custom/patch/shell/local-shell cases. test_native_finalization_preserves_output_without_delaying_parent covers timeout and parent cancellation. These establish the lifecycle outcome without coupling tests to a private drain-helper call.
• Cancellation-predicate test suggestion (1): test_completed_history_at_failure_boundaries distinguishes ordinary failure, function/native tool-local cancellation and genuine parent cancellation through both public Runner APIs. test_asyncio_tasks.py also verifies child-failure notification versus parent cancellation. No additional caller-visible gap is established by the proposed helper-only boolean assertions.

Validation on this head: formatting, lint, Mypy and Pyright passed; 12,295 tests passed and 69 skipped locally. CI is green, including native macOS and both Windows test jobs. The server-managed follow-up recovery gap and hard termination of cancellation-resistant application callbacks remain explicitly deferred; this response does not describe either as fixed.

[markstuart-oai]

Reviewed b2814699c7572a1b25f49d1045e8f8b6de643cb7. The earlier delayed-input, supplied-state synchronization, and tool-use snapshot findings are addressed. I found two remaining issues: late finalizers can still mutate serializable tool output held by a failed-run snapshot, and a cancelled guardrail child can replace the selected tool error in the non-streaming runner. The inline comments describe the supported triggers.

I checked the client-managed recovery boundary, accepted-output filtering, resume diagnostics, and Session publication boundary. I kept server-managed follow-up recovery and forced termination of cancellation-resistant callbacks outside this review, as scoped in the description.

All non-skipped current-head CI checks passed, including Python 3.10–3.14, Windows, and native macOS. Readiness-refresh jobs were skipped. This review used source, existing test cases, and hosted CI results; I did not run local workloads or reproduce the timing cases.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6bdf4d1b6e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jbeckwith-oai]

@dpiet-oai could you re-review the latest head, d5a02c2? Your earlier requested-changes review is still active. Completed accepted outputs now survive sibling failure even when optional metadata is unfinished, with detached recovery snapshots so late finalizers cannot change published history. The follow-up guardrail cancellation and failure-reporting issues are also addressed. All 23 CI checks pass, and all review threads are resolved. We stepped back and reassessed the full design with two fresh reviewers; neither found a remaining blocker. The PR description records the deliberately deferred server-managed recovery and cancellation-resistant callback limitations.

[markstuart-oai]

Reviewed d5a02c2fbf2dbf099554e52dff5643959eb01b54. The earlier mutable-output and cancelled-guardrail findings are addressed. One P2 remains: the new snapshot converts supported Python output objects to plain data before a live-state retry. The inline comment traces this with ToolOutputText and a tool_use_behavior callback. My earlier suggestion should have distinguished detaching mutable values from JSON conversion.

I reviewed the four-file follow-up against the earlier head and retained the complete 16-file review context. All 23 non-skipped checks passed on this head. This was source and hosted-result review only; I did not run a local reproduction or test suite.

[markstuart-oai]

Reviewed the full change through 8d451e43d486656123c58ef93d0937a1729747bb, with a focused independent pass on the new snapshot code. The earlier live ToolOutputText retry finding is fixed.

One P2 remains in the new copier: a supported recursive plain tool result can replace the selected sibling error with RecursionError and prevent completed outputs from reaching Session history. The inline comment gives the public Runner path and expected regression coverage.

All non-skipped checks on this head passed. This finding comes from source tracing; I did not run repository tests or workloads.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7bb7c7a36e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[markstuart-oai]

Reviewed 7bb7c7a36e93e69d541217700586852942214a24. The recursive-output fix addresses my previous finding. The copier records mutable containers before descending and preserves tuple backlinks. A cyclic accepted output therefore no longer replaces the selected sibling error. The new regression checks both Runner modes and verifies detached error history and saved Session history. No remaining in-scope findings.

I also checked the provider/LLM-end hook finding against the merge base and the author's reply. That loss exists before this change: LLM-end hooks run before tool execution and its recovery checkpoint. It is a separate response-hook recovery gap, outside this PR's tool-batch and post-tool failure scope. This approval does not claim that gap is fixed.

The complete increment changes two files; the remaining PR source matches the previous review. All 23 non-skipped checks pass on this head, including Python 3.10–3.14 and Windows tests. I reviewed source and hosted CI status. I did not run local tests or the author's reproduction.

[marcuswood-oai]

Approved. The client-managed recovery path retains accepted tool outputs while preserving admission checks, selected errors, and the existing RunState format.

Checked

• Focused Runner tests pass for sibling failures, late input verdicts, output privacy, snapshot isolation, and approval resume.
• Reviewed native-tool cleanup and the existing security findings; the current head's CI checks pass.

Caveat: This approval covers the documented scope. It does not claim recovery at the earlier on_llm_end boundary, server-managed follow-up recovery, or termination of callbacks that suppress cancellation.