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/desktop): consent-first truncation precedence + dedup (simplify pass) 

Final-diff simplify/review pass findings on #83785:

- Consent gate (confirm_truncate -> 4029) now checked BEFORE target
  resolution, restoring the pre-PR precedence: an unconfirmed submit
  carrying truncation params refuses without paying the durable-transcript
  read or heal-stamping live history dicts, and an unconfirmed out-of-range
  ordinal returns 4029 (not 4018). Malformed params still refuse first
  with 4004. Regression test added (spy DB asserts zero reads pre-consent;
  mutation-checked against the previous commit).
- _coerce_truncate_ordinal generalized to _coerce_truncate_int(param_name):
  the row_id branch was inlining the exact bool-guard + int() -> 4004
  pattern the helper had just extracted.
- Deleted the dead user_indices re-read after _resolve_truncate_row_id
  (heal mutates dicts in place; the filter output is identical) and the
  duplicate range check that had deadened the pre-existing guard.
- Desktop: exported isVisibleUserMessage from use-prompt-actions/utils and
  used it in visibleUserOrdinal / visibleUserIndexAtOrdinal /
  rebindSurvivorRowIds — one predicate for the ordinal parity all three
  depend on instead of three verbatim copies.
- Docs: programmatic-integration.md documents survivor_user_row_ids.
This commit is contained in:
kshitij 2026-08-13 13:26:55 +05:30
parent f6081b6647
commit a4f468e832
5 changed files with 151 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.ts
View File

@ -18,6 +18,7 @@ import { branchGroupForUser, type ChatMessage, chatMessageText, textPart } from
import {
appendText,
isSessionBusyError,
isVisibleUserMessage,
visibleUserIndexAtOrdinal,
visibleUserOrdinal,
withSessionBusyRetry,
@ -66,7 +67,7 @@ export function rebindSurvivorRowIds(messages: ChatMessage[], survivorRowIds: Su
let ordinal = 0
return messages.map(message => {
if (message.role !== 'user' || message.hidden) {
if (!isVisibleUserMessage(message)) {
return message
}

11
apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts
View File

@ -495,8 +495,15 @@ export function appendText(message: AppendMessage): string {
.trim()
}
/** The one visible-user filter every user-ordinal computation must share —
* truncate ordinals, ordinal→index resolution, and survivor-rowId rebinding
* all rely on counting exactly the same turns. */
export function isVisibleUserMessage(message: ChatMessage): boolean {
return message.role === 'user' && !message.hidden
}
export function visibleUserOrdinal(messages: readonly ChatMessage[], end: number): number {
return messages.slice(0, end).filter(m => m.role === 'user' && !m.hidden).length
return messages.slice(0, end).filter(isVisibleUserMessage).length
}
export function visibleUserIndexAtOrdinal(messages: readonly ChatMessage[], targetOrdinal: number): number {
@ -505,7 +512,7 @@ export function visibleUserIndexAtOrdinal(messages: readonly ChatMessage[], targ
for (let index = 0; index < messages.length; index += 1) {
const message = messages[index]
if (message.role !== 'user' || message.hidden) {
if (!isVisibleUserMessage(message)) {
continue
}

77
tests/test_tui_gateway_server.py
View File

@ -17922,3 +17922,80 @@ def test_prompt_submit_consecutive_rewinds_with_returned_survivor_row_ids(
assert isinstance(survivors2, list) and len(survivors2) == 1
finally:
server._sessions.pop(sid, None)
def test_prompt_submit_unconfirmed_truncation_refuses_before_target_resolution(
monkeypatch,
):
"""Consent (4029) is checked BEFORE target resolution: an unconfirmed
submit carrying truncation params must not pay the durable-transcript
read or heal-stamp live history (simplify review on #83785), and an
out-of-range unconfirmed ordinal refuses 4029, not 4018 — the baseline
precedence before the row-id feature.
"""
db_reads = []
class _SpyDB:
def get_messages_as_conversation(self, *a, **k):
db_reads.append(1)
return []
def replace_messages(self, *a, **k):
pytest.fail("must not write")
hist = [
{"role": "user", "content": "first"},
{"role": "assistant", "content": "r1"},
{"role": "user", "content": "second"},
{"role": "assistant", "content": "r2"},
]
sess = _session(history=list(hist), session_key="consent-precedence-key")
sid = "consent-precedence-sid"
server._sessions[sid] = sess
monkeypatch.setattr(server, "_get_db", lambda: _SpyDB())
monkeypatch.setattr(
server, "_start_agent_build", lambda *a, **k: pytest.fail("must not start a turn")
)
monkeypatch.setattr(
server, "_start_inflight_turn", lambda *a, **k: pytest.fail("must not start a turn")
)
try:
# Unconfirmed row_id: 4029, no DB read, no heal stamps on history.
resp = server.handle_request(
{
"id": "1",
"method": "prompt.submit",
"params": {"session_id": sid, "text": "x", "truncate_before_row_id": 3},
}
)
assert (resp.get("error") or {}).get("code") == 4029, resp
assert db_reads == []
assert all("_row_id" not in m for m in sess["history"])
# Malformed param still beats consent: bool row_id is 4004.
resp = server.handle_request(
{
"id": "2",
"method": "prompt.submit",
"params": {"session_id": sid, "text": "x", "truncate_before_row_id": True},
}
)
assert (resp.get("error") or {}).get("code") == 4004, resp
# Unconfirmed out-of-range ordinal: 4029 (consent), not 4018 (range).
resp = server.handle_request(
{
"id": "3",
"method": "prompt.submit",
"params": {
"session_id": sid,
"text": "x",
"truncate_before_user_ordinal": 99,
},
}
)
assert (resp.get("error") or {}).get("code") == 4029, resp
assert len(sess["history"]) == 4
finally:
server._sessions.pop(sid, None)

125
tui_gateway/methods_prompt.py
View File

@ -155,19 +155,19 @@ 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.
def _coerce_truncate_int(rid, value, param_name="truncate_before_user_ordinal"):
"""Return ``(int_value, error_response)`` for a client-supplied integer param.
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.
1 and aim a confirmed rewind at the wrong 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")
return None, _err(rid, 4004, f"{param_name} 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")
return None, _err(rid, 4004, f"{param_name} must be an integer")
def _reconcile_client_ordinal(rid, sid, client_ordinal, msg_ordinal, param_name, target_repr):
@ -180,7 +180,7 @@ def _reconcile_client_ordinal(rid, sid, client_ordinal, msg_ordinal, param_name,
"""
if client_ordinal is None:
return msg_ordinal, None
ordinal, err = _coerce_truncate_ordinal(rid, client_ordinal)
ordinal, err = _coerce_truncate_int(rid, client_ordinal)
if err is not None:
return None, err
if ordinal != msg_ordinal:
@ -380,26 +380,60 @@ def _(rid, params: dict) -> dict:
or truncate_row_id is not None
):
history = session.get("history", [])
# Malformed params refuse first (4004), regardless of consent —
# the historical ordinal-path precedence.
target_row_id = None
if truncate_row_id is not None:
target_row_id, err = _coerce_truncate_int(
rid, truncate_row_id, "truncate_before_row_id"
)
if err is not None:
return err
client_ordinal = None
if truncate_user_ordinal is not None:
client_ordinal, err = _coerce_truncate_int(rid, truncate_user_ordinal)
if err is not None:
return err
# An ordinal/id alone is not consent. A client that carries a leftover
# ordinal into an ORDINARY submit sends a request that is
# indistinguishable, field by field, from a real rewind — same
# method, same shape, an in-range target — and the cut it asks for
# is a destructive replace_messages() the user never requested
# (#80763: 296 -> 52 messages, 244 durable rows gone). Only the
# client knows whether this submit is a rewind/edit/regenerate, so
# it has to say so; refuse the cut when it doesn't. Consent is
# checked BEFORE target resolution: an unconfirmed (leaked-state)
# request must refuse with 4029 without paying the durable
# transcript read or heal-stamping live history dicts that
# row-id resolution performs.
if not is_truthy_value(params.get("confirm_truncate")):
logger.warning(
"prompt.submit: REFUSED unconfirmed truncation of session %s "
"(%d messages held; ordinal=%s, row_id=%s, message_id=%s). "
"The client attached truncation parameters without "
"confirm_truncate — likely stale truncation parameters on "
"an ordinary submit.",
sid,
len(history),
client_ordinal,
target_row_id,
truncate_message_id,
)
return _err(
rid,
4029,
"truncation parameters require confirm_truncate=true; "
"an ordinary prompt.submit must not drop session history "
"(update your Hermes client if a rewind was intended)",
)
user_indices = _history_user_indices(history)
ordinal = None
if truncate_row_id is not None:
if isinstance(truncate_row_id, bool):
return _err(
rid,
4004,
"truncate_before_row_id must be an integer",
)
try:
target_row_id = int(truncate_row_id)
except (TypeError, ValueError):
return _err(
rid,
4004,
"truncate_before_row_id must be an integer",
)
if target_row_id is not None:
# Durable address first — never degrade a missing row_id into a
# client ordinal cut (#82959 / #82766 review). Unknown id refuses
# without touching data; stale ordinal with a *resolved* row_id
@ -407,8 +441,6 @@ def _(rid, params: dict) -> dict:
found_match = _resolve_truncate_row_id(
session, history, target_row_id
)
# user_indices may have been healed with _row_id stamps
user_indices = _history_user_indices(history)
if found_match is None:
logger.warning(
@ -425,7 +457,7 @@ def _(rid, params: dict) -> dict:
msg_ordinal, _ = found_match
ordinal, err = _reconcile_client_ordinal(
rid, sid, truncate_user_ordinal, msg_ordinal,
rid, sid, client_ordinal, msg_ordinal,
"truncate_before_row_id", target_row_id,
)
if err is not None:
@ -457,49 +489,14 @@ def _(rid, params: dict) -> dict:
msg_ordinal, _ = found_match
ordinal, err = _reconcile_client_ordinal(
rid, sid, truncate_user_ordinal, msg_ordinal,
rid, sid, client_ordinal, msg_ordinal,
"truncate_before_message_id", msg_id_str,
)
if err is not None:
return err
else:
ordinal, err = _coerce_truncate_ordinal(rid, truncate_user_ordinal)
if err is not None:
return err
ordinal = client_ordinal
if ordinal < 0 or ordinal >= len(user_indices):
return _err(
rid,
4018,
"target user message is no longer in session history",
)
# An ordinal/id alone is not consent. A client that carries a leftover
# ordinal into an ORDINARY submit sends a request that is
# indistinguishable, field by field, from a real rewind — same
# method, same shape, an in-range target — and the cut it asks for
# is a destructive replace_messages() the user never requested
# (#80763: 296 -> 52 messages, 244 durable rows gone). Only the
# client knows whether this submit is a rewind/edit/regenerate, so
# it has to say so; refuse the cut when it doesn't.
if not is_truthy_value(params.get("confirm_truncate")):
logger.warning(
"prompt.submit: REFUSED unconfirmed truncation of session %s "
"(%d messages held; ordinal=%d). The client attached "
"truncation parameter without confirm_truncate — "
"likely stale truncation parameters on an ordinary submit.",
sid,
len(history),
ordinal,
)
return _err(
rid,
4029,
"truncation parameters require confirm_truncate=true; "
"an ordinary prompt.submit must not drop session history "
"(update your Hermes client if a rewind was intended)",
)
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
@ -1350,7 +1347,7 @@ def register(server) -> None:
_mem_db_pair_agrees,
_find_user_turn_by_row_id,
_resolve_truncate_row_id,
_coerce_truncate_ordinal,
_coerce_truncate_int,
_reconcile_client_ordinal,
_pending_reaction_notes,
):

2
website/docs/developer-guide/programmatic-integration.md
View File

@ -70,6 +70,8 @@ A rewind / edit / regenerate is a `prompt.submit` that drops part of the stored
A truncation parameter without `confirm_truncate` is refused with code `4004` or `4029` and nothing is written. Hosts that implement rewind must set the flag at the moment the user asks for it, and must never keep truncation parameters in state across ordinary submits. Prefer `truncate_before_row_id` (from resume `row_id` / `_row_id`) over ordinals; keep the ordinal as a back-compat / optimistic-row path only when no durable id is available yet.
On a successful truncating submit against a durable session, the `prompt.submit` result additionally carries `survivor_user_row_ids` — the fresh post-rewrite row IDs of the surviving user turns, in visible-user-ordinal order. The rewrite re-inserts the kept prefix as new rows, so every row ID the host cached before the rewind is stale afterward; rebind cached IDs from this list (a `null` entry means that turn has no durable ID — drop the cached one) or the next rewind targeting an older surviving turn will be refused with `4018`.
### Events streamed back
`message.delta`, `message.complete`, `tool.start`, `tool.progress`, `tool.complete`, `approval.request`, `clarify.request`, `sudo.request`, `sudo.expire`, `secret.request`, `secret.expire`, `gateway.ready`, plus session lifecycle and error events. Expiry events carry the original `{ request_id }`; external hosts should clear only the matching pending prompt.