fix(gateway): suppress hidden-only incomplete Codex turns - #51657
fix(gateway): suppress hidden-only incomplete Codex turns#51657LeonSGP43 wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real gateway/session-safety failure. The current patch needs rework against current main.
Problems
agent/conversation_loop.py:4436returns retry exhaustion with a non-emptyfinal_response.gateway/run.py:2604returns any non-empty response before the new classifier can run, and the proposed helper also rejects non-emptyagent_result["final_response"]. The suppression path therefore cannot match the production result shape.tests/gateway/test_incomplete_gateway_turns.pyfabricatesfinal_response=Noneand mocks_run_agent, so it does not exercise that current path.- The later #51628 reproduction reports
Codex Responses stream did not emit a terminal response; the proposed exact matcher only recognizesremained incomplete after.
Suggested changes
- Classify this structured incomplete/no-visible-answer state before the non-empty fallback is returned, then add an end-to-end gateway persistence/delivery test using the current result shape.
- Cover the no-terminal-stream reproduction or use a provider-agnostic structured criterion.
Automated hermes-sweeper review.
| if not agent_result.get("partial"): | ||
| return False | ||
| error_text = str(agent_result.get("error", "") or "").lower() | ||
| return "remained incomplete after" in error_text |
There was a problem hiding this comment.
This predicate cannot match the current retry-exhaustion result: agent/conversation_loop.py:4436 sets this same diagnostic as a non-empty final_response, while the helper rejects non-empty final_response and the normalizer returns non-empty response at gateway/run.py:2604 before reaching this code. Please classify the structured incomplete/no-visible-output state before that early return and test the real result shape.
… final_response Follow-up to the salvaged #51657: the conversation loop returns the retry-exhaustion sentinel as BOTH final_response and error, so the original detector (which required final_response to be falsy) never fired on real exhaustion turns — the sentinel text was delivered verbatim into the channel, exactly the #51628 poisoning vector. Detect the sentinel echo, blank it before empty-response normalization, and never suppress a turn whose final_response is genuine model text. Also: dedupe-guard mock fix in the test fixture (has_platform_message_id must return False, not a truthy MagicMock) and two guard tests (real answer never suppressed; interrupted/failed never classified).
|
Merged via PR #65062 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge). One substantive follow-up on top: the detector required |
… final_response Follow-up to the salvaged NousResearch#51657: the conversation loop returns the retry-exhaustion sentinel as BOTH final_response and error, so the original detector (which required final_response to be falsy) never fired on real exhaustion turns — the sentinel text was delivered verbatim into the channel, exactly the NousResearch#51628 poisoning vector. Detect the sentinel echo, blank it before empty-response normalization, and never suppress a turn whose final_response is genuine model text. Also: dedupe-guard mock fix in the test fixture (has_platform_message_id must return False, not a truthy MagicMock) and two guard tests (real answer never suppressed; interrupted/failed never classified).
… final_response Follow-up to the salvaged NousResearch#51657: the conversation loop returns the retry-exhaustion sentinel as BOTH final_response and error, so the original detector (which required final_response to be falsy) never fired on real exhaustion turns — the sentinel text was delivered verbatim into the channel, exactly the NousResearch#51628 poisoning vector. Detect the sentinel echo, blank it before empty-response normalization, and never suppress a turn whose final_response is genuine model text. Also: dedupe-guard mock fix in the test fixture (has_platform_message_id must return False, not a truthy MagicMock) and two guard tests (real answer never suppressed; interrupted/failed never classified).
Closes #51628
Summary
Testing