fix(state): preserve soft-archived rows in transcript rewrites

[Rival]

What does this PR do?

replace_messages() deletes every row for the session by default — including the active = 0 soft-archived turns that archive_and_compact() deliberately keeps on disk for durability (#38763). The ACP adapter already guards against this (acp_adapter/session.py: probe has_archived_messages(), then pass active_only=True), but two other transcript-rewrite call sites don't:

• gateway/session.py — the /retry, /undo and /compress rewrite path;
• tui_gateway/server.py — history truncation in prompt.submit.

Any transcript rewrite on a session that has been in-place-compacted silently destroys the archived pre-compaction history. We hit this in production: an archived transcript segment vanished after a bulk rewrite, with nothing in the logs.

This PR makes the default safe instead of patching call sites one by one: active_only becomes Optional[bool] = None, where None means "preserve soft-archived rows when the session has any" (an active = 0 existence probe). The probe runs inside the same write transaction as the delete, so the decision cannot race a concurrent archive. Passing active_only=False explicitly keeps the old full-wipe semantics for callers that really mean it; explicit True is unchanged.

Effects on existing callers:

• acp_adapter/session.py — its manual probe becomes redundant (kept as-is here to minimise the diff; can be simplified in a follow-up).
• gateway/session.py, tui_gateway/server.py — fixed by the new default.
• gateway/platforms/api_server.py (session fork) — unaffected: a freshly created fork session has no archived rows, so the probe short-circuits and behaviour is byte-identical.

If changing the default is undesirable, the fallback is to replicate the ACP guard at the two unprotected call sites — happy to rework the PR that way, but the auto-default also covers future callers.

Related Issue

Same durability concern as #38763 (which introduced the soft-archive).

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• hermes_state.py — replace_messages(active_only: Optional[bool] = None); None resolves via an active = 0 existence probe inside the write transaction; docstring rewritten to document all three modes.
• tests/test_hermes_state.py — three new tests: rewrite with archived rows present preserves them (new default); explicit active_only=False still wipes; rewrite on a session without archived rows matches old behaviour (message_count included).

How to Test

uv run --extra dev pytest tests/test_hermes_state.py -q

306 passed locally (303 pre-existing + 3 new). Also ran every other test file referencing replace_messages (tests/test_tui_gateway_server.py, tests/run_agent/test_in_place_compaction.py, tests/gateway/test_session_api.py, tests/gateway/test_session.py, tests/hermes_cli/test_web_server.py, tests/acp/test_session.py) before and after the change — pass/fail/skip counts are identical (the failures in my environment are missing optional deps, present on both runs).

Manual: create a session, force in-place compaction (soft-archived rows appear), run /undo or /retry, verify the archived rows survive (SELECT count(*) FROM messages WHERE session_id=? AND active=0).

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(state):, one squashed commit)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix (2 files)
• I've run pytest on the affected modules and all tests pass (306 passed in tests/test_hermes_state.py); full pytest tests/ not re-run end-to-end (optional-dep env)
• I've added tests for my changes
• I've tested on my platform: Linux

Documentation & Housekeeping

• I've updated relevant documentation — the docstring is the documentation surface for this internal method
• I've updated cli-config.yaml.example if I added/changed config keys — N/A: no config keys
• I've considered cross-platform impact — pure SQLite/stdlib change
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

🤖 Generated with Claude Code

[alt-glitch]

This was generated by AI during triage.

Related: same soft-archived-transcript-preservation family as open #50306 (which added active_only=True at the ACP persist site) and the merged compaction fixes #56342 / #52658. Different mechanism: this PR flips replace_messages()'s default to active_only=None (preserve soft-archived rows via an in-transaction active=0 probe), making the safe behavior the default for the /retry, /undo, /compress and TUI-truncation call sites rather than guarding each one. Related, not a duplicate.

[teknium1]

Thanks for tracing the surviving rewrite paths; the current TUI truncation path still calls the destructive default at tui_gateway/server.py:8509, and gateway retry reaches it through gateway/session.py:2470.

Problems

• hermes_state.py:3288 changes every implicit rewrite, including Yuanbao recall redaction. That path loads only active rows and rewrites them at gateway/platforms/yuanbao.py:1420-1427 and :1459-1487; preserving inactive rows would leave recalled content in compaction-archived history.
• The added unit tests do not exercise those call paths or distinguish compaction archives from recall-redaction semantics.

Suggested changes

• Preserve the destructive default and pass an explicit active-only choice only from transcript rewrite paths that are meant to retain archived history.
• Add real-SessionDB regressions for the protected rewrite path and the Yuanbao recall path.

This is an automated hermes-sweeper review.

[teknium1]

Changing this default also affects Yuanbao recall redaction through SessionStore.rewrite_transcript(): it loads only active rows, edits the recalled message, then rewrites. If inactive rows are retained automatically, a recalled message already in a compaction archive remains stored unchanged. Keep the destructive default and make archive preservation explicit at the safe call sites.

[Rival]

Closing this — the review was right, and in the meantime most of what it was for has been fixed upstream by other means. Writing up what I found so it is not lost, since a real gap does remain.

All line references are against main @ cbecd72e97.

The review's objection holds, and the approach here is wrong

Flipping replace_messages's default to auto-preserve cannot work, because SessionStore.rewrite_transcript (gateway/session.py:3174) is a shared funnel and one of its callers needs the destructive semantics on purpose.

Yuanbao recall redaction (gateway/platforms/yuanbao.py:1427, :1487) loads the live transcript, swaps the recalled message's content for _REDACTED, and rewrites. Under today's destructive default the archived rows are deleted too, so the recalled text is purged everywhere. Under this PR's auto-preserve default the active = 0 rows survive still holding the original, un-redacted text — an unsent message left on disk. One blanket default cannot serve both that and the archive-preserving callers, so the opt-in has to be per-caller, exactly as suggested.

What upstream has since closed

/compress — closed, by removing the call rather than making it safe. Both gateway/slash_commands.py:4095 and gateway/run.py:14058 now gate rewrite_transcript on if rotated:. After in-place compaction archive_and_compact has already done the right thing, so the rewrite was simply unnecessary — and that redundant rewrite was what destroyed the archive. Comments there cite #61145 for this exact loss.

ACP persist — closed, using precisely the pattern the review asked for. acp_adapter/session.py:485-493 probes and passes the choice explicitly:

has_archived = db.has_archived_messages(state.session_id)
db.replace_messages(state.session_id, state.history, active_only=has_archived)

with SessionDB.has_archived_messages (hermes_state.py:7379) added as a cheap existence probe documented for exactly this decision. So the idiom already exists in-tree — anyone picking up the remainder has something to copy rather than invent.

/undo was never affected. I had this wrong, and so did this PR. _handle_undo_command goes through rewind_session → SessionDB.rewind_to_message, which sets active = 0 — it is the soft-archive mechanism, so it has nothing to lose. It appears in the list only because rewrite_transcript's docstring (gateway/session.py:3177) still names "/retry, /undo, and /compress". That docstring is now stale twice over: /undo never routed here, and /compress only does so on the rotated path. Cheap free fix if someone is in the file.

What still looks open

Two callers still reach the destructive default, so a session that was compacted in place can still lose its pre-compaction archive:

• /retry — gateway/slash_commands.py:2624 truncates history to before the last real user turn and calls rewrite_transcript, which hits replace_messages with the default at gateway/session.py:3191.
• TUI truncation — tui_gateway/server.py:11220 calls replace_messages directly. The guard added just above it covers a different bug (refusing the degenerate history[:0] total-wipe edge); it does not touch archived rows.

If that is worth fixing, the shape the review asked for is now mechanical: thread an explicit choice through rewrite_transcript instead of changing replace_messages's default, have /retry and the TUI path pass has_archived_messages(...), and leave yuanbao destructive — ideally passing active_only=False explicitly so the intent is visible at the call site rather than inherited from a default. Regressions against a real SessionDB for both the protected path and the recall path, per the review.

Why closing rather than reworking

Once the default flip goes — and it should — nothing in this diff survives: both tests here exercise that hunk specifically. The remainder does not overlap this branch at all, and the title no longer describes the fix, which would only mislead a reviewer. Cleaner as a fresh narrow PR if and when someone wants it.

One caveat I want to be explicit about: the two remaining gaps are from reading the code, not from a reproduction. I did not confirm that /retry actually loses archived rows in practice — it needs a session compacted in place (the rotated path behaves differently) whose archive is still present at /retry time, and I have not verified that combination is reachable on live paths. Worth confirming before anyone spends work on it.

Thanks for the review — the yuanbao point was the one I had missed, and it is the reason the whole approach was wrong rather than just incomplete.

[Rival]

Closing per the write-up above: the default flip is the wrong approach (yuanbao recall needs the destructive semantics), /compress and the ACP path have since been fixed upstream by other means, and /undo was never affected. The residual /retry + TUI-truncation gap is documented above for whoever wants it — it shares no code with this branch, so a fresh narrow PR would be cleaner than reworking this one.