RomanNum3ral
/
hermes-agent
mirror of https://github.com/NousResearch/hermes-agent.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor(gateway): dedupe truncation-target validation; drop dead state and redundant test 

Review cleanup on the #83202 salvage (findings from the 4-angle + 3-reviewer
passes, all verified against the diff):

- Extract _coerce_truncate_ordinal() and _reconcile_client_ordinal(): the
  bool-check/int-coercion block was duplicated verbatim 3x and the 4030
  ordinal-mismatch block 2x across the row-id/message-id/ordinal branches
  (~90 lines of copy-paste with drift risk between the two durable branches).
- Delete target_idx (4 assignments, 0 reads — the cut uses
  user_indices[ordinal]) and replace the stale inline user-indices
  comprehension with the _history_user_indices helper it duplicated.
- Drop test_reproduce_row_id_truncation: a strict subset of
  test_prompt_submit_truncates_by_row_id +
  test_prompt_submit_refuses_ordinal_and_row_id_mismatch with weaker asserts.
- Collapse PR-introduced blank-line runs in the test file.

Behavior-preserving: error codes, messages, and log fields unchanged
(4004/4018/4029/4030 wording identical); full test_tui_gateway_server.py
suite green (549 passed).
This commit is contained in:
kshitij 2026-08-11 14:15:42 +05:30
parent 16de3c3f1b
commit 040420bd11
2 changed files with 69 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
tests/test_tui_gateway_server.py
View File

@ -4622,9 +4622,6 @@ def test_prompt_submit_refuses_ordinal_and_message_id_mismatch(monkeypatch):
server._sessions.pop("mismatch-trunc-sid", None)
def test_prompt_submit_truncates_by_row_id(monkeypatch):
"""#82959: prompt.submit with truncate_before_row_id must cut at the target row id."""
replaced = []
@ -4708,57 +4705,6 @@ def test_prompt_submit_truncates_by_string_row_id(monkeypatch):
server._sessions.pop("str-row-id-trunc-sid", None)
def test_reproduce_row_id_truncation(monkeypatch):
"""#82959: Reproduction test for durable row_id truncation and 4030 ordinal mismatch validation."""
replaced = []
class _FakeDB:
def replace_messages(self, key, messages, active_only=False, archive_dropped=False):
replaced.append((key, list(messages)))
history = [
{"_row_id": 101, "role": "user", "content": "first"},
{"_row_id": 102, "role": "assistant", "content": "reply 1"},
{"_row_id": 103, "role": "user", "content": "second"},
{"_row_id": 104, "role": "assistant", "content": "reply 2"},
]
server._sessions["repro-sid"] = _session(history=list(history))
monkeypatch.setattr(server, "_get_db", lambda: _FakeDB())
monkeypatch.setattr(server, "_start_agent_build", lambda *a, **k: None)
try:
# 1. Target turn 2 via row_id 103
resp = server.handle_request({
"id": "1",
"method": "prompt.submit",
"params": {
"session_id": "repro-sid",
"text": "edited second turn",
"truncate_before_row_id": 103,
"confirm_truncate": True,
}
})
assert resp.get("error") is None
# 2. Refuse ordinal & row_id mismatch with code 4030
server._sessions["mismatch-sid"] = _session(history=list(history))
resp_mismatch = server.handle_request({
"id": "2",
"method": "prompt.submit",
"params": {
"session_id": "mismatch-sid",
"text": "stale rewind",
"truncate_before_row_id": 103,
"truncate_before_user_ordinal": 0,
"confirm_truncate": True,
}
})
assert resp_mismatch["error"]["code"] == 4030
finally:
server._sessions.pop("repro-sid", None)
server._sessions.pop("mismatch-sid", None)
def test_prompt_submit_refuses_ordinal_and_row_id_mismatch(monkeypatch):
"""#82959: A mismatch between truncate_before_user_ordinal and truncate_before_row_id must return 4030."""
history = [
@ -4870,8 +4816,6 @@ def test_prompt_submit_row_id_ignores_platform_id_fallback(monkeypatch):
server._sessions.pop("string-id-sid", None)
def test_prompt_submit_refuses_empty_truncation_without_confirm(monkeypatch):
"""A confirmed rewind still must not wipe a non-empty transcript by accident.

159
tui_gateway/methods_prompt.py
View File

@ -155,6 +155,55 @@ def _resolve_truncate_row_id(session: dict, history: list, target_row_id: int):
return db_ord, mem_idx
def _coerce_truncate_ordinal(rid, value):
"""Return ``(ordinal, error_response)`` for a client-supplied ordinal.
bool is an int subclass: a JSON ``true`` would coerce via int() to
ordinal 1 and aim a confirmed rewind at the second user turn — refuse it
like any other non-integer.
"""
if isinstance(value, bool):
return None, _err(rid, 4004, "truncate_before_user_ordinal must be an integer")
try:
return int(value), None
except (TypeError, ValueError):
return None, _err(rid, 4004, "truncate_before_user_ordinal must be an integer")
def _reconcile_client_ordinal(rid, sid, client_ordinal, msg_ordinal, param_name, target_repr):
"""Cross-check a client ordinal against a resolved durable target.
Returns ``(ordinal, error_response)``: the target's ordinal when the
client sent none or agreed, else the 4004/4030 refusal. A stale ordinal
alongside a *resolved* durable id is the #82756 drift class — refuse
rather than guess which address the user meant.
"""
if client_ordinal is None:
return msg_ordinal, None
ordinal, err = _coerce_truncate_ordinal(rid, client_ordinal)
if err is not None:
return None, err
if ordinal != msg_ordinal:
logger.warning(
"prompt.submit: REFUSED truncation due to ordinal mismatch for session %s "
"(ordinal=%d, %s_ordinal=%d, %s=%s). "
"Stale truncate_before_user_ordinal detected.",
sid,
ordinal,
param_name,
msg_ordinal,
param_name,
target_repr,
)
return None, _err(
rid,
4030,
f"truncate_before_user_ordinal ({ordinal}) does not match "
f"{param_name} target turn ({msg_ordinal})",
)
return ordinal, None
def _pending_reaction_notes(session: dict) -> str:
"""Note block describing reactions the user added since the last turn, or "".
@ -329,7 +378,6 @@ def _(rid, params: dict) -> dict:
history = session.get("history", [])
user_indices = _history_user_indices(history)
target_idx = None
ordinal = None
if truncate_row_id is not None:
@ -371,41 +419,13 @@ def _(rid, params: dict) -> dict:
"target user message is no longer in session history",
)
msg_ordinal, target_idx = found_match
if truncate_user_ordinal is not None:
if isinstance(truncate_user_ordinal, bool):
return _err(
rid,
4004,
"truncate_before_user_ordinal must be an integer",
)
try:
ordinal = int(truncate_user_ordinal)
except (TypeError, ValueError):
return _err(
rid,
4004,
"truncate_before_user_ordinal must be an integer",
)
if ordinal != msg_ordinal:
logger.warning(
"prompt.submit: REFUSED truncation due to ordinal mismatch for session %s "
"(ordinal=%d, row_id_ordinal=%d, row_id=%d). "
"Stale truncate_before_user_ordinal detected.",
sid,
ordinal,
msg_ordinal,
target_row_id,
)
return _err(
rid,
4030,
f"truncate_before_user_ordinal ({ordinal}) does not match "
f"truncate_before_row_id target turn ({msg_ordinal})",
)
else:
ordinal = msg_ordinal
msg_ordinal, _ = found_match
ordinal, err = _reconcile_client_ordinal(
rid, sid, truncate_user_ordinal, msg_ordinal,
"truncate_before_row_id", target_row_id,
)
if err is not None:
return err
elif truncate_message_id is not None:
msg_id_str = str(truncate_message_id)
found_match = None
@ -431,56 +451,17 @@ def _(rid, params: dict) -> dict:
"target user message is no longer in session history",
)
msg_ordinal, target_idx = found_match
if truncate_user_ordinal is not None:
if isinstance(truncate_user_ordinal, bool):
return _err(
rid,
4004,
"truncate_before_user_ordinal must be an integer",
)
try:
ordinal = int(truncate_user_ordinal)
except (TypeError, ValueError):
return _err(
rid,
4004,
"truncate_before_user_ordinal must be an integer",
)
if ordinal != msg_ordinal:
logger.warning(
"prompt.submit: REFUSED truncation due to ordinal mismatch for session %s "
"(ordinal=%d, message_id_ordinal=%d, message_id=%s). "
"Stale truncate_before_user_ordinal detected.",
sid,
ordinal,
msg_ordinal,
msg_id_str,
)
return _err(
rid,
4030,
f"truncate_before_user_ordinal ({ordinal}) does not match "
f"truncate_before_message_id target turn ({msg_ordinal})",
)
else:
ordinal = msg_ordinal
msg_ordinal, _ = found_match
ordinal, err = _reconcile_client_ordinal(
rid, sid, truncate_user_ordinal, msg_ordinal,
"truncate_before_message_id", msg_id_str,
)
if err is not None:
return err
else:
if isinstance(truncate_user_ordinal, bool):
return _err(
rid,
4004,
"truncate_before_user_ordinal must be an integer",
)
try:
ordinal = int(truncate_user_ordinal)
except (TypeError, ValueError):
return _err(
rid,
4004,
"truncate_before_user_ordinal must be an integer",
)
ordinal, err = _coerce_truncate_ordinal(rid, truncate_user_ordinal)
if err is not None:
return err
if ordinal < 0 or ordinal >= len(user_indices):
return _err(
@ -488,7 +469,6 @@ def _(rid, params: dict) -> dict:
4018,
"target user message is no longer in session history",
)
target_idx = user_indices[ordinal]
# An ordinal/id alone is not consent. A client that carries a leftover
# ordinal into an ORDINARY submit sends a request that is
@ -515,10 +495,7 @@ def _(rid, params: dict) -> dict:
"an ordinary prompt.submit must not drop session history "
"(update your Hermes client if a rewind was intended)",
)
user_indices = [
i for i, m in enumerate(history)
if m.get("role") == "user" and not m.get("display_kind")
]
user_indices = _history_user_indices(history)
# Reject out-of-range ordinals on BOTH ends. A negative value would
# otherwise sail past the upper-bound check and hit Python's negative
# indexing below (user_indices[-1] -> the LAST user turn), silently
@ -1336,6 +1313,8 @@ def register(server) -> None:
_mem_db_pair_agrees,
_find_user_turn_by_row_id,
_resolve_truncate_row_id,
_coerce_truncate_ordinal,
_reconcile_client_ordinal,
_pending_reaction_notes,
):
setattr(