mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(gateway): harden hidden-incomplete detection against the sentinel 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).
This commit is contained in:
parent
08015c3a8f
commit
09b6d22dfe
2 changed files with 57 additions and 8 deletions
|
|
@ -2697,17 +2697,27 @@ def _normalize_empty_agent_response(
|
|||
|
||||
|
||||
def _is_gateway_hidden_reasoning_incomplete_turn(agent_result: dict) -> bool:
|
||||
"""Detect retry-exhausted turns with hidden reasoning but no visible answer."""
|
||||
"""Detect retry-exhausted turns with hidden reasoning but no visible answer.
|
||||
|
||||
The conversation loop returns the retry-exhaustion sentinel as BOTH
|
||||
``final_response`` and ``error`` ("Codex response remained incomplete
|
||||
after 3 continuation attempts"), so ``final_response`` being non-empty
|
||||
does not mean the model produced a visible answer. Treat the turn as
|
||||
hidden when the error sentinel is present and ``final_response`` is
|
||||
either empty or merely echoes that sentinel — any genuinely different
|
||||
final text means the model DID answer and must be delivered.
|
||||
"""
|
||||
if not isinstance(agent_result, dict):
|
||||
return False
|
||||
if agent_result.get("failed") or agent_result.get("interrupted"):
|
||||
return False
|
||||
if agent_result.get("final_response"):
|
||||
return False
|
||||
if not agent_result.get("partial"):
|
||||
return False
|
||||
error_text = str(agent_result.get("error", "") or "").lower()
|
||||
return "remained incomplete after" in error_text
|
||||
error_text = str(agent_result.get("error", "") or "").strip()
|
||||
if "remained incomplete after" not in error_text.lower():
|
||||
return False
|
||||
final_response = str(agent_result.get("final_response") or "").strip()
|
||||
return not final_response or final_response == error_text
|
||||
|
||||
|
||||
def _should_clear_resume_pending_after_turn(agent_result: dict) -> bool:
|
||||
|
|
@ -11825,6 +11835,14 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
return None
|
||||
|
||||
response = agent_result.get("final_response") or ""
|
||||
# Hidden-reasoning-only retry exhaustion: the loop's sentinel text
|
||||
# ("Codex response remained incomplete after 3 continuation
|
||||
# attempts") doubles as final_response, so it would be delivered
|
||||
# verbatim into the channel — where peer agents can ingest it as a
|
||||
# completed assistant turn (#51628). Blank it here so the normal
|
||||
# empty-response handling (and the suppression below) applies.
|
||||
if _is_gateway_hidden_reasoning_incomplete_turn(agent_result):
|
||||
response = ""
|
||||
try:
|
||||
from gateway.response_filters import is_intentional_silence_agent_result
|
||||
_intentional_silence = is_intentional_silence_agent_result(
|
||||
|
|
|
|||
|
|
@ -50,8 +50,12 @@ class CaptureSlackAdapter(BasePlatformAdapter):
|
|||
|
||||
|
||||
def _make_incomplete_result() -> dict:
|
||||
# Mirror the REAL conversation-loop exhaustion shape: the sentinel text is
|
||||
# returned as BOTH final_response and error (agent/conversation_loop.py's
|
||||
# "remained incomplete after 3 continuation attempts" return).
|
||||
_sentinel = "Codex response remained incomplete after 3 continuation attempts"
|
||||
return {
|
||||
"final_response": None,
|
||||
"final_response": _sentinel,
|
||||
"messages": [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "assistant", "content": ""},
|
||||
|
|
@ -62,7 +66,7 @@ def _make_incomplete_result() -> dict:
|
|||
"partial": True,
|
||||
"completed": False,
|
||||
"interrupted": False,
|
||||
"error": "Codex response remained incomplete after 3 continuation attempts",
|
||||
"error": _sentinel,
|
||||
"last_prompt_tokens": 0,
|
||||
}
|
||||
|
||||
|
|
@ -89,6 +93,10 @@ def _make_runner(adapter: CaptureSlackAdapter) -> gateway_run.GatewayRunner:
|
|||
runner.session_store.rewrite_transcript = MagicMock()
|
||||
runner.session_store.append_to_transcript = MagicMock()
|
||||
runner.session_store.update_session = MagicMock()
|
||||
# The transient-failure persistence path dedupes on platform message_id
|
||||
# (#47237). A bare MagicMock returns a truthy mock, which would wrongly
|
||||
# mark the user turn as a duplicate and skip persisting it.
|
||||
runner.session_store.has_platform_message_id = MagicMock(return_value=False)
|
||||
runner._running_agents = {}
|
||||
runner._pending_messages = {}
|
||||
runner._pending_approvals = {}
|
||||
|
|
@ -116,15 +124,38 @@ def _make_event() -> MessageEvent:
|
|||
def test_incomplete_codex_warning_is_not_surfaced_as_chat_text():
|
||||
agent_result = _make_incomplete_result()
|
||||
|
||||
# Mirror the gateway pipeline: the hidden-turn detector blanks the
|
||||
# sentinel final_response BEFORE empty-response normalization runs.
|
||||
response = agent_result.get("final_response") or ""
|
||||
assert gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result)
|
||||
response = ""
|
||||
|
||||
response = gateway_run._normalize_empty_agent_response(
|
||||
agent_result,
|
||||
agent_result.get("final_response") or "",
|
||||
response,
|
||||
history_len=4,
|
||||
)
|
||||
|
||||
assert response == ""
|
||||
|
||||
|
||||
def test_real_answer_alongside_incomplete_error_is_never_suppressed():
|
||||
"""A turn whose final_response is genuine model text (not the sentinel
|
||||
echo) must be delivered even when the error field carries the
|
||||
retry-exhaustion sentinel — suppression is only for hidden turns."""
|
||||
agent_result = _make_incomplete_result()
|
||||
agent_result["final_response"] = "Here is the actual answer."
|
||||
|
||||
assert not gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result)
|
||||
|
||||
|
||||
def test_interrupted_or_failed_turns_are_not_classified_hidden():
|
||||
for key in ("interrupted", "failed"):
|
||||
agent_result = _make_incomplete_result()
|
||||
agent_result[key] = True
|
||||
assert not gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_incomplete_codex_turn_stays_out_of_slack_transcript(monkeypatch, tmp_path):
|
||||
adapter = CaptureSlackAdapter()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue