fix(codex): avoid orphaned Responses message IDs - #97445
salehelsayed wants to merge 1 commit into
Conversation
ehz0ah
left a comment
There was a problem hiding this comment.
Reviewed exact head e3a6ba38287c. The change restores the Responses replay invariant without reintroducing store-false reasoning ID lookups: linked msg_* IDs are removed whenever their rs_* identity cannot be replayed, while message content, phase, status, and tool-call pairing remain intact. The same rule is applied to conversion, preflight, normalization, stale-replay pruning, and invalid-encrypted-content recovery.
I reproduced the invalid graph on current main and confirmed that this head produces the dependency-closed graph. I also ran the focused 10-file suite through scripts/run_tests.sh: 672 passed. Ruff, compileall, and git diff --check passed. I found no correctness blockers in the submitted implementation. The branch currently conflicts with main, so it will need a rebase before merge.
Enforce dependency closure at the final Responses provider-call boundary so stateless reasoning replay cannot send a native msg_* identity after its required rs_* identity has been stripped. Preserve independent message IDs and degrade all typed message IDs when encrypted reasoning replay is disabled. Add focused regression coverage.
e3a6ba3 to
1fecbd3
Compare
|
Thanks @salehelsayed — the work in this PR has landed on
Your contribution is credited there (cherry-picked authorship / co-author trailer or credit in the PR body; see the linked PR for what was kept and what was trimmed). Closing this one as landed / superseded so the backlog reflects reality. If something in your original diff is still missing on current |
What does this PR do?
Fixes #97442.
Related to #97427, which appears to report the same GPT-5.6 Responses replay failure.
Hermes intentionally strips replayed reasoning-item IDs when
store=falsebecause some Responses-compatible endpoints otherwise try to look them up server-side. A dependent native assistantmsg_*ID can nevertheless survive, producing an invalid partial item graph such as:GPT-5.6 rejects that request because the message identity depends on the stripped
rs_*reasoning identity.This refreshed implementation is rebased onto current
mainand enforces the invariant at the final provider-call boundary, after request middleware and Responses preflight:reasoningitem starts an output group, dependent typed assistantmessageIDs are removed until a real group boundary;function_calldoes not prematurely close the reasoning group;function_call_outputand ordinary role messages close the group;Current-main refresh
The original PR head had become conflicted after substantial upstream refactoring. On 2026-09-11 the branch was rebuilt as a single focused commit on current
main(ad03f20dd61919ca2135d6904e787a94284aacaf) rather than mechanically resolving the old diff.Current changes:
agent/responses_replay_identity.pyagent/turn_api_call.pytests/agent/test_responses_replay_identity.pycontributors/emails/saleh.fekry@gmail.comsalehelsayed.Type of Change
Validation
The refreshed head is conflict-free and GitHub reports it as mergeable against current
main.Hosted CI, Docker, and Nix workflows were triggered for the refreshed head but currently show
action_required, so they require maintainer approval before they can execute for this external-contributor PR.Checklist
mainmsg_*IDs