fix(gateway/state): fix the truncate ordinal address space and make a mis-aimed rewind recoverable (#82756)

[JoaoMarcos44]

Fixes #82756.

Two commits, two independent findings, each verified against the real code path before it was written:

#	Commit	Claim	How it was verified
1	keep the personality pivot out of the ordinal space	An untagged role=user marker shifts every later rewind	Failing test on the real injection point: the cut landed at 3 instead of 5
2	make a rewind truncation recoverable	The rewind write is a hard DELETE with no archive	Storage test: dropped turns readable via include_inactive=True after the rewrite

1. The addressing defect

truncate_before_user_ordinal is an index into the list of real user turns. The gateway builds that list with role == "user" and not display_kind, and the repo already states the invariant, in test_prompt_submit_truncate_ordinal_skips_display_kind_rows:

display_kind timeline rows (model_switch, async_delegation_complete, …) are role=user but no client counts them as user turns. Without the filter, a trailing marker shifts the ordinal so the wrong message is targeted for truncation.

_apply_personality_to_session broke that invariant at the producer. Its pivot rides as role=user on purpose — strict OpenAI-compatible providers reject system messages that are not first in the list (#48338), the same reason _append_model_switch_marker does it — but unlike the model-switch marker it carried no display_kind:

# tui_gateway/server.py — before
session["history"].append({"role": "user", "content": marker})              # counted as a real turn
# tui_gateway/server.py — _append_model_switch_marker, for comparison
entry = {"role": "user", "content": marker, "display_kind": "model_switch"} # correctly excluded

After a personality change the gateway's address space contains a phantom turn no client can see, so every later rewind / edit / regenerate resolves one slot too early.

I checked whether any other producer has the same hole, and none does. The synthetic-turn path (auto_continue, async_delegation_complete) stamps the kind on the live in-memory message, not just the persisted row — agent/turn_context.py:

# "Stamp that on the live message […] The model still receives role/content
#  unchanged; the api_messages build strips both fields from every outgoing copy."
if persist_user_display_kind:
    user_msg["display_kind"] = persist_user_display_kind
messages.append(user_msg)

The personality pivot was the one producer writing straight into session["history"] and bypassing that contract. The two direct session["history"].append injection sites in the gateway are now both tagged.

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#8b0000', 'mainBkg': '#0a0204', 'primaryTextColor': '#ffccd5', 'primaryBorderColor': '#ff0038', 'lineColor': '#ff0038'}}}%%
graph TD
    A[Client Rewind Request<br/>ordinal = 2] --> B{Gateway Address Space<br/>role=user AND NOT display_kind}

    subgraph BEFORE[Before - Untagged Pivot]
        C[Slot 0: first turn] --> D[PHANTOM SLOT<br/>personality pivot]
        D --> E[Slot 1: second turn]
        E --> F[Slot 2: third turn]
        G[Resolved Slot 2 = second turn] --> H[Cut Lands Too Early]
        H --> I[Unasked Turns Dropped]
    end

    subgraph AFTER[After - Tagged Pivot]
        J[Slot 0: first turn] --> K[EXCLUDED<br/>display_kind personality_switch]
        K --> L[Slot 1: second turn]
        L --> M[Slot 2: third turn]
        N[Resolved Slot 2 = third turn] --> O[Cut Lands On Target]
        O --> P[Only Intended Turns Dropped]
    end

    B --> BEFORE
    B --> AFTER

    style D fill:#8b0000,stroke:#ff0038,color:#ffccd5
    style I fill:#8b0000,stroke:#ff0038,color:#ffccd5
    style K fill:#1a3a1a,stroke:#4ade80,color:#d1fae5
    style P fill:#1a3a1a,stroke:#4ade80,color:#d1fae5

Loading

Infographic :

2. Recoverability

Guarding the aim of a rewind still leaves every other way of aiming it wrong terminal. All three incidents ended at the same write, and all three were unrecoverable for the same reason: replace_messages() DELETEs the rows, which also evicts them from the FTS index — no active=0 archive, nothing to restore from.

The codebase already draws this distinction and already ships the safe half of it:

• archive_and_compact is documented as "the durability-preserving alternative to replace_messages" (active=0, compacted=1 — summarized away, still discoverable in search).
• rewind_to_message — the /undo path — soft-deletes to active=0, compacted=0 and keeps the rows "on disk for audit / forensic inspection".

The desktop rewind is the same user-facing operation as /undo, and it was the one taking the destructive branch. replace_messages(..., archive_dropped=True) flips the DELETE to a content-preserving UPDATE messages SET active = 0, reusing the existing transaction and the existing compacted=0 marking, so the dropped turns stay readable via get_messages(..., include_inactive=True) and stay out of session search.

The live transcript is byte-identical either way — only the durability of the dropped turns changes. The parameter defaults to False, so the fork handler, the ACP adapter and gateway/session.py are untouched; a test pins that.

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#00f0ff', 'mainBkg': '#0a0a16', 'primaryTextColor': '#ffffff', 'primaryBorderColor': '#ff007f', 'lineColor': '#00f0ff'}}}%%
graph LR
    A[Rewind Write] --> B{replace_messages}
    B -->|"before: DELETE"| C[Rows Removed]
    C --> D[FTS Entry Dropped]
    D --> E[Permanent Loss]
    B -->|"after: archive_dropped"| F["UPDATE active = 0"]
    F --> G[Rows Kept On Disk]
    G --> H[Readable via include_inactive]
    H --> I[Mis-aimed Cut Is Reversible]

    style E fill:#4a0d2e,stroke:#ff007f,color:#ffffff
    style I fill:#0d3a4a,stroke:#00f0ff,color:#ffffff

Loading

Why the shipped guards let the original report through

Incident	Guard	Why it does not fire
#70516 → #70895	confirm_empty_truncate	Only covers ordinal 0; a mid-session cut leaves a non-empty transcript.
#80763 → #80802	confirm_truncate required	Proves intent, not agreement about the target. The desktop legitimately sets it — this was a rewind, it just landed on the wrong turn.
#82514 (open)	out-of-range ordinals → 4018	The ordinal was in range. It was in range and wrong.

Both existing guards are correct and are unchanged. They are consent gates; commit 1 is the addressing bug underneath them and commit 2 is the blast radius when any of them is wrong.

Test plan

• python -m pytest tests/hermes_state/test_replace_messages_archive_siblings.py -q — 8 passed (5 existing [Bug]: /retry permanently deletes archived compaction history on messaging platforms #80216 tests + 3 new: dropped turns recoverable, rewind marking not compaction marking, default stays destructive).
• python -m pytest tests/test_tui_gateway_server.py -k "truncat or personality_marker" -q — 10 passed.
• python -m pytest tests/test_tui_gateway_server.py -q — 514 passed, 15 failed. All 15 are save_cfg / config.set / persist_model_switch YAML-persistence tests that fail identically on an unmodified tree (verified by stashing and re-running): pre-existing local Windows noise.
  • One real-threading test (…requeues_all_unstarted_notifications_with_real_threading) failed once under a combined run and passed on re-run and in isolation — load-sensitive, not related to these changes.
• Commit 1 was confirmed to fail without its fix: AssertionError: the pivot shifted the ordinal: the cut landed at 3 instead of 5 — the second exchange is deleted although the user asked to rewind to third.
• Test doubles for replace_messages in the gateway suite were widened to the real signature — a double that does not accept what production passes silently converts this write into a 5008.
• apps/desktop/src/lib/chat-messages.test.ts extends the existing projects durable timeline kinds without inspecting text contract case with personality_switch. Not executed locally — this checkout's desktop node_modules cannot resolve @testing-library/react, @assistant-ui/react or @tanstack/react-query, so vitest and npm run typecheck fail at import resolution across the whole app (605 pre-existing errors, none on the edited lines). CI is the real signal for that file.

Deliberately out of scope

The cross-surface split-brain reported in the fourth occurrence (Telegram app and Desktop driving one DM). No marker fix reaches a second writer. Any general "attest the address space" mechanism also has to compose with #82514's compression-prefix mapping — auto-compression legitimately rewrites session["history"] to a suffix, so a naive client/gateway turn-count check would refuse every rewind after a compaction. That belongs in one design, not bolted onto this.

Commit 2 does reduce that case from permanent loss to a recoverable one.

Relation to #82766

#82766 targets the same issue from a different layer — stable-id addressing, which is the direction the issue itself asks for. There is no overlap with this PR: no new wire field, no new error code, no change to either consent gate. The two compose (correct address space + recoverable write here, stable addressing there), and I have offered there to rebase, split, or drop whatever its author prefers so reviewers are not handed duplicate work.

One note left on that PR for the author to confirm: every writer of session["history"] stores provider-format messages — list(agent_messages), result["messages"], or get_messages_as_conversation(), whose docstring reads "Load messages in the OpenAI conversation format (role + content dicts)" — none of which carry a client message id, while the id its desktop side sends is the renderer's ephemeral one (`${timestamp}-${index}-${displayRole}`, documented as ephemeral in SessionMessage). I may be misreading that flow; it is raised there as a question, not a blocker.

🤖 Generated with Claude Code

[kshitijk4poor]

Salvaged into #82950 with your authorship preserved — both commits cherry-picked verbatim (address-space fix + recoverable write), plus the boolean-ordinal guard credited to @StanleyStetson as you suggested, and two review follow-ups on top: the session.branch / _persist_branch_seed history copies were dropping display_kind (re-planting the untagged-marker class into branched sessions after a restart), and the ui-tui renderer needed the personality_switch case for parity with your desktop change.

Your producer-level invariant test and the archive-vs-delete storage contract tests were exactly the right shape — they transplanted cleanly and caught nothing but green. Thanks for the rigorous root-cause work AND the exemplary collaboration on #82766. Closing this one since #82950 carries it forward.