fix(tui): prompt.submit's truncate_before_user_ordinal reads fresh history

[Crong-Gabia]

Summary

Fixes #69107 — prompt.submit's truncate_before_user_ordinal (the Desktop/TUI edit-and-regenerate flow) computed the truncation point from session["history"], which can remain stale while another REST-gateway-backed client writes new turns to the same durable session.

An ordinal valid against the current SessionDB state was therefore rejected with 4018 "target user message is no longer in session history". Because the same path performs a destructive replace_messages(...), using the stale snapshot could also discard newer rows.

Root cause

The gateway's in-memory history is refreshed on resume and after its own turns, but it is not authoritative for writes made by another client.

Compressed sessions add an identity wrinkle: Desktop ordinals are based on the full displayed lineage, while the model-fed working history contains only the current continuation segment. Loading ancestors into the replacement history would duplicate compressed rows in the tip session.

Fix

• Re-read the current session segment from SessionDB with repair_alternation=True, matching the model-fed resume projection, immediately before validating and applying the truncation.
• Translate the full-display user ordinal to the current-segment ordinal using display_history_prefix, without copying ancestor rows into the tip.
• Fail closed without mutating in-memory history or starting a turn if either the authoritative DB read or destructive replacement fails.
• Persist the truncation before committing it to session["history"], so a SQLite/FTS/ENOSPC failure cannot leave memory and durable history diverged.
• Preserve the current main predicate that excludes display_kind timeline rows from both current-segment and ancestor-prefix user ordinals.
• Follow the latest prompt-handler split by applying the change in tui_gateway/methods_prompt.py without restoring the mechanically extracted handler in server.py.
• Preserve the existing 4018 behavior for negative, ancestor-only, or otherwise out-of-range targets, and preserve the newer confirm_empty_truncate guard for intentional first-turn rewinds.

Scope

This PR intentionally fixes the concrete edit/regenerate correctness and data-loss path only. It does not add a live transcript subscription or choose new cross-client UI behavior; that broader UX remains a separate follow-up in #69107.

Relationship to #72876

#72876 independently proposes write-before-memory failure handling for replace_messages(). The current head of this PR already includes that behavior and its regression coverage, in addition to the authoritative history refresh, compressed-lineage ordinal translation, resume-projection preservation, and fail-closed DB-read path. The implementations therefore substantially overlap; this PR covers #72876's failure mode as part of the broader stale cross-client history fix.

Testing

• Regression test for a completed write from another client that previously produced 4018.
• Real-SessionDB compression-lineage test proving only the current segment is truncated while ancestor rows remain unchanged.
• Failure-path tests proving DB read and replacement errors leave memory/version unchanged and do not start a turn.
• Existing empty-truncation and ordinary truncation tests updated to exercise the authoritative read contract.
• Rebased onto current main (5a23e3c52): python -m pytest tests/test_tui_gateway_server.py -q → 498 passed.
• python -m py_compile tui_gateway/methods_prompt.py tests/test_tui_gateway_server.py — clean.
• git diff --check — clean.

Review focus

The highest-risk invariant is that the displayed full-lineage ordinal must resolve to the current segment without ever copying ancestor rows into the destructive replacement. The real-SessionDB lineage regression test pins that behavior. Both authoritative-read and replacement failures are fail-closed before the turn starts.

[PRATHAMESH75]

Reviewed against #69107. The root cause is confirmed: prompt.submit's truncate_before_user_ordinal branch computes user_indices from session["history"] — the TUI process's in-memory copy refreshed only on resume — so a turn appended by another client (REST gateway) sharing the same session_key is invisible, and a valid ordinal is rejected with 4018. Re-reading from the DB right before the bounds check is the right direction, and because the same history then feeds replace_messages, this also closes the more serious failure mode: persisting the stale (shorter) truncation would have silently dropped the concurrent writer's rows — cross-client data loss, not just a stale display. The regression test faithfully models stale-in-memory vs fresh-DB and the fail-safe fallback is a good touch.

One edge case worth confirming before merge — the re-read hardcodes include_ancestors=True (and drops repair_alternation), which diverges from how session["history"] is actually built on the child/subagent resume path. tui_gateway/server.py:6356 resumes a delegated child with get_messages_as_conversation(target, repair_alternation=True) and deliberately without include_ancestors, with the comment that "include_ancestors would prepend the parent's transcript onto the subagent's branch"; its display projection (:6385) is likewise child-only. For a resumed child session that has a parent transcript, re-reading with include_ancestors=True would prepend the parent's user turns, shifting every entry in user_indices — so the same truncate_before_user_ordinal (which the client computed against the child-only display) would resolve to a different, earlier turn and truncate at the wrong point. That's the same truncation/data-loss class this PR is closing, just triggered on child sessions rather than by staleness.

The reported top-level shared-session scenario has no ancestors, so include_ancestors=True is harmless there and the fix works as tested. But since the branch can run on any session, consider re-reading with the same projection the session was resumed with (mirror the resume flags / the display projection the ordinal was computed against) rather than unconditionally adding ancestors, so the ordinal stays anchored to the rows the client actually saw. Dropping repair_alternation is a smaller sibling of the same point — the resume path heals a durable user;user once, and the re-read no longer does.

Otherwise this correctly fixes the confirmed bug for the scenario in the issue.

[Crong-Gabia]

Thanks — this review was recorded against the initial head ed5b275, and the ancestor-projection concern was valid.

The latest commits address both points:

• The truncation re-read is now tip/current-segment only; it no longer passes include_ancestors=True.
• Full-display ordinals are translated with display_history_prefix, so compressed-session ancestors are never copied into or persisted on the tip.
• The re-read now passes repair_alternation=True, matching the model-fed resume projection.
• DB read and replacement failures both fail closed without mutating in-memory history or starting a turn.

A real-SessionDB compression-lineage regression test verifies that the current segment is truncated while the parent transcript remains unchanged. The latest GitHub-generated merge result against current main passes the full gateway file: 398 passed.

[Crong-Gabia]

Rebased onto current main and revalidated the truncation path against the changes that landed after this PR opened. The resolution preserves the newer confirm_empty_truncate full-transcript guard, performs the authoritative DB refresh before ordinal validation, keeps display-lineage ordinal translation tip-local, retains repair_alternation=True, and fails closed before mutating in-memory state when persistence fails. I also updated the regression fixtures for the current persist_user_message call contract. Current result: the branch is GitHub-mergeable and tests/test_tui_gateway_server.py passes in full: 472 passed.

[teknium1]

Thanks for the focused stale-history investigation. The stale-snapshot premise is still present on current main at tui_gateway/server.py:11214-11226, but this branch needs a careful port rather than a direct salvage.

Problems

• tui_gateway/methods_prompt.py:135-136 refreshes the DB snapshot, then :190 performs a separate destructive replace_messages. SessionDB.replace_messages starts its delete/reinsert transaction at hermes_state.py:5453-5480 with no expected-version check. A cross-client append between those operations can still be deleted.
• Current main no longer has tui_gateway/methods_prompt.py; the active handler is tui_gateway/server.py:11116. Its resume path uses get_resume_conversations() and sanitize_replay_history() at tui_gateway/server.py:7750-7759, so the proposed refresh must preserve that current model-history projection.

Suggested changes

• Port the handler change to tui_gateway/server.py and derive the fresh tip model history using the current resume projection.
• Add an atomic/conditional rewrite or conflict path, with a regression for an append occurring after the refresh but before replacement.

Automated hermes-sweeper review.

[teknium1]

This refresh and the later replace_messages at line 190 are separate DB operations. SessionDB.replace_messages deletes and reinserts in its own transaction without an expected-version predicate, so another client can append after this read and have that new row erased. Please make the rewrite conditional/atomic against this snapshot, or fail with a conflict, and cover that interleaving.

[Crong-Gabia]

Addressed the latest stale-snapshot review in c70eb92b3 and rebased the branch onto current main.

• Refresh now uses the current resume projection: get_resume_conversations() followed by sanitize_replay_history().
• Added SessionDB.replace_active_messages_if_unchanged(), which compares the expected active tip projection and performs the rewrite inside the same BEGIN IMMEDIATE transaction.
• A cross-client append between refresh and rewrite now fails with conflict 4091 instead of deleting the new row.
• Added real-SessionDB coverage for both matching conditional rewrites and the append-after-refresh interleaving; the TUI regression verifies the concurrent message remains durable and no turn starts.
• Preserved current-main module layout (prompt.submit is in tui_gateway/methods_prompt.py after the handler split).

Validation: tests/test_hermes_state.py 139 passed; tests/test_tui_gateway_server.py 499 passed; py_compile and git diff --check clean.

[Crong-Gabia]

Rebased onto current main (f07f47fe7) and reconciled the intervening truncation/storage changes. The updated branch preserves upstream error code 5008 for persistence failures, matches the current resume projection including opt-in _row_id identity, and retains the conditional atomic rewrite/conflict path for cross-client appends. Validation: tests/test_hermes_state.py 164 passed; tests/test_tui_gateway_server.py 522 passed; py_compile and git diff --check clean.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address or reference #69107: #69109 fixes the stale cross-client ordinal path and protects the destructive rewrite against concurrent appends, while #72876 and its merged salvage #76634 cover the narrower persistence-failure path.

Related pull requests

• fix(tui): prompt.submit's truncate_before_user_ordinal reads fresh history #69109 best fix — (+590/-52) — n/a: Re-reads the current resume projection before resolving the ordinal, translates display-lineage ordinals without copying ancestors, and adds an atomic conditional active-tip rewrite that returns a conflict if another client appends. The diff addresses the contributor's keep_open review concern by comparing the expected projection inside the same write transaction and testing that an interleaved append remains durable.
• fix(tui): fail closed when prompt.submit cannot persist history truncation #72876 [closed] partial — (+75/-3) — n/a: Moves truncation persistence before in-memory mutation and refuses to start the turn when replacement fails, preventing durable zombie history. It remains relevant as the closed predecessor whose fix was adapted and merged through fix(tui): fail closed when prompt.submit cannot persist history truncation (salvage #72876) #76634, with authorship preserved.
• fix(tui): fail closed when prompt.submit cannot persist history truncation (salvage #72876) #76634 [merged] partial — (+77/-3) — n/a: Ports fix(tui): fail closed when prompt.submit cannot persist history truncation #72876's write-before-memory, fail-closed behavior to the active methods_prompt.py handler and adds the replacement-failure regression test. It is the merged reference implementation for that narrower failure mode, but it does not add fix(tui): prompt.submit's truncate_before_user_ordinal reads fresh history #69109's fresh-history read or conditional rewrite against concurrent appends.

Duplicates

#72876 and #76634 implement substantially the same persistence-failure fix, with #76634 being the merged current-handler salvage; that subset is also incorporated into the broader #69109 diff.

Suggested consolidation

Keep #69109 open with a salvage path: retain its authoritative refresh, lineage-aware ordinal translation, and atomic conditional rewrite/conflict tests. This is consistent with the contributor keep_open review, and the visible diff addresses its race objection; #72876 is already closed as superseded by merged #76634, while #76634 should remain the merged reference for the narrower fail-closed subset.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I69107(["issue #69107 (open)"])
    P69109["PR #69109 (open)"]
    P69109 -->|best fix| I69107
    class I69107 open
    class P69109 open
    class P69109 best
    class P69109 target
    click I69107 "https://github.com/NousResearch/hermes-agent/issues/69107"
    click P69109 "https://github.com/NousResearch/hermes-agent/pull/69109"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 42 kB of PR diffs, 13 kB of issue/PR text, 12 kB of discussion (14 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[Crong-Gabia]

Rebased the accepted salvage onto current main (a991dfc25) with no additional behavior changes. The authoritative refresh, lineage-aware ordinal translation, and atomic conditional rewrite/conflict path remain intact. Validation: tests/test_hermes_state.py + tests/test_tui_gateway_server.py 702 passed; py_compile and git diff --check clean.

[teknium1]

Thanks for the thorough work here — this PR correctly identified the stale-ordinal mis-aim class and the destructive-write risks well before the incident that finally forced the issue (#87059).

Closing as superseded: the problem was ultimately resolved by stronger means than fresher ordinal math.

• fix(gateway): stop Desktop edit-resend from truncating the wrong turn #87150 made ordinal-only truncation fail closed for any durable session — the gateway now refuses the fragile addressing outright instead of re-reading history to make it safer.
• fix(desktop): always send a durable truncation address on rewind/edit/regenerate #87294 made the Desktop always send a durable truncate_before_row_id (resolved by content against the stamped transcript when the bubble never learned its id), degrading to a plain resubmit when no durable address can be proven.

Together those remove the ordinal-only path this PR was hardening, so most of its diff no longer has a target. Your fail-closed-on-DB-read-failure instinct and the write-before-memory ordering both landed on main through that lineage as well.

One piece of your work still stands on its own: the compare-and-swap replace_active_messages_if_unchanged() guard against a cross-process writer appending between the history read and the truncation write. It's a narrow race with a recoverable consequence (truncation writes soft-archive via archive_dropped=True), but it's legitimate defense-in-depth — if you'd like to rebase just that hermes_state half as a standalone PR against current main, we'd be glad to review it.

Appreciate the contribution and the tests.

[Crong-Gabia]

@teknium1 Following your suggestion, I split the standalone hermes_state CAS portion out of this superseded PR and opened #88799 against current main.

It contains only:

• SessionDB.replace_active_messages_if_unchanged() with the conditional active-transcript check and rewrite in one transaction\n- real-SessionDB coverage for both stale-snapshot rejection and matching soft-archive rewrite
• Validation on the latest-main-based branch: py_compile and the focused tests passed (2 passed). I also tagged you on the new PR since GitHub did not allow me to add you directly as a reviewer.