Skip to content

fix(acp): preserve compaction-archived transcript on per-turn persist - #50306

Closed
mrparker0980 wants to merge 1 commit into
NousResearch:mainfrom
mrparker0980:fix/acp-preserve-archived-transcript
Closed

mrparker0980 wants to merge 1 commit into
NousResearch:mainfrom
mrparker0980:fix/acp-preserve-archived-transcript

Conversation

@mrparker0980

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops every long-running ACP session from silently losing its entire
pre-compaction transcript. The ACP AIAgent is created with the same
session_id and SessionDB as the adapter (_make_agent), so once its
context grows past the compression threshold it compacts in place via
SessionDB.archive_and_compact(), which deliberately soft-archives the
pre-compaction turns (active=0, compacted=1) to keep them on disk and
discoverable by session_search (the #38763 durability guarantee). The
problem: as soon as that turn ends, SessionManager._persist() called
replace_messages(), whose unconditional DELETE FROM messages WHERE session_id = ? wiped all rows for the id — including the ones the agent
had just archived. Net effect: auto-compaction fires, the turn returns, and
the archived history is gone.

Why this approach: the per-turn persist only ever needs to refresh the live
(active) set — the agent already owns archival. So _persist now replaces
only active=1 rows and leaves soft-archived rows untouched, rather than
trying to teach the adapter about compaction state. This keeps the existing
atomic delete+reinsert contract (the #13675 rollback-on-failure guarantee)
fully intact for the live set.

Why not change replace_messages outright: it's also used by destructive
rewrite flows (/retry, /undo, /compress, forks) that genuinely want every
row gone, so the new behavior is opt-in via an active_only flag and all
other callers are unchanged.

Related Issue

N/A

Type of Change

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

Changes Made

  • hermes_state.py: add an active_only: bool = False parameter to
    SessionDB.replace_messages(). When set, the DELETE is scoped with
    AND active = 1 so soft-archived rows survive; message_count/
    tool_call_count then track the live set, matching archive_and_compact().
    Default is unchanged, so existing callers keep their destructive semantics.
  • acp_adapter/session.py: SessionManager._persist() now calls
    replace_messages(..., active_only=True), and the method docstrings are
    updated to reflect that only the live rows are replaced.
  • tests/acp/test_session.py: add
    test_save_session_preserves_compaction_archived_history, a regression
    test that compacts a session in place and asserts the archived turns remain
    retrievable via get_messages(include_inactive=True) after a subsequent
    save_session().

How to Test

  1. Reproduce: with the fix reverted, run the new test — it fails because the
    per-turn persist deletes the archived rows.
  2. Verify the fix: scripts/run_tests.sh tests/acp/test_session.py — the new
    test_save_session_preserves_compaction_archived_history passes, and the
    existing fix(acp): preserve persisted session history on save failures #13675 rollback test still passes.
  3. Regression-check the shared method: the replace_messages /
    archive_and_compact SessionDB tests in tests/hermes_state/ and
    tests/test_hermes_state.py all pass, confirming default callers are
    unaffected.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 25.5)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

## What does this PR do?

Stops every long-running ACP session from silently losing its entire
pre-compaction transcript. The ACP `AIAgent` is created with the same
`session_id` and `SessionDB` as the adapter (`_make_agent`), so once its
context grows past the compression threshold it compacts *in place* via
`SessionDB.archive_and_compact()`, which deliberately soft-archives the
pre-compaction turns (`active=0, compacted=1`) to keep them on disk and
discoverable by `session_search` (the NousResearch#38763 durability guarantee). The
problem: as soon as that turn ends, `SessionManager._persist()` called
`replace_messages()`, whose unconditional `DELETE FROM messages WHERE
session_id = ?` wiped *all* rows for the id — including the ones the agent
had just archived. Net effect: auto-compaction fires, the turn returns, and
the archived history is gone.

Why this approach: the per-turn persist only ever needs to refresh the live
(active) set — the agent already owns archival. So `_persist` now replaces
only `active=1` rows and leaves soft-archived rows untouched, rather than
trying to teach the adapter about compaction state. This keeps the existing
atomic delete+reinsert contract (the NousResearch#13675 rollback-on-failure guarantee)
fully intact for the live set.

Why not change `replace_messages` outright: it's also used by destructive
rewrite flows (/retry, /undo, /compress, forks) that genuinely want every
row gone, so the new behavior is opt-in via an `active_only` flag and all
other callers are unchanged.

## Related Issue

N/A

## Type of Change

- [x] 🐛 Bug fix (non-breaking change that fixes an issue)

## Changes Made

- `hermes_state.py`: add an `active_only: bool = False` parameter to
  `SessionDB.replace_messages()`. When set, the `DELETE` is scoped with
  `AND active = 1` so soft-archived rows survive; `message_count`/
  `tool_call_count` then track the live set, matching `archive_and_compact()`.
  Default is unchanged, so existing callers keep their destructive semantics.
- `acp_adapter/session.py`: `SessionManager._persist()` now calls
  `replace_messages(..., active_only=True)`, and the method docstrings are
  updated to reflect that only the live rows are replaced.
- `tests/acp/test_session.py`: add
  `test_save_session_preserves_compaction_archived_history`, a regression
  test that compacts a session in place and asserts the archived turns remain
  retrievable via `get_messages(include_inactive=True)` after a subsequent
  `save_session()`.

## How to Test

1. Reproduce: with the fix reverted, run the new test — it fails because the
   per-turn persist deletes the archived rows.
2. Verify the fix: `scripts/run_tests.sh tests/acp/test_session.py` — the new
   `test_save_session_preserves_compaction_archived_history` passes, and the
   existing NousResearch#13675 rollback test still passes.
3. Regression-check the shared method: the `replace_messages` /
   `archive_and_compact` SessionDB tests in `tests/hermes_state/` and
   `tests/test_hermes_state.py` all pass, confirming default callers are
   unaffected.

## Checklist

### Code

- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: macOS 15 (Darwin 25.5)

### Documentation & Housekeeping

- [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A
- [x] I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jun 21, 2026
kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Jul 1, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
kshitijk4poor added a commit that referenced this pull request Jul 1, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR #50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
@kshitijk4poor

Copy link
Copy Markdown
Contributor

Closing as a duplicate of #50405, merged via #56342. Both PRs fixed the same ACP archived-history data loss. #50405's owned-agent skip also handles the compression id-rotation case; your active_only=True primitive was adopted (with credit) in the follow-up commit 723ccda to cover the model-switch / restore save paths. Thanks @mrparker0980 — your approach directly shaped the final fix.

waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
Jasper6439 pushed a commit to Jasper6439/hermes-agent that referenced this pull request Jul 5, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Follow-up widening the archived-history fix to the sibling save paths the
original PR did not cover. Model switches (_cmd_model, set_session_model) and
_restore mint a fresh AIAgent with _session_db_created=False, so the
agent-owns-persistence guard evaluates False and the blind full-history
replace_messages() fired — DELETEing the durable active=0/compacted=1 rows on
any compressed ACP session (same data-loss class the PR fixes, different
trigger).

- hermes_state.replace_messages: add active_only=True to delete/reinsert only
  the live (active=1) rows, leaving soft-archived rows untouched (idea adopted
  from the competing PR NousResearch#50306 by @mrparker0980, credited).
- hermes_state.has_archived_messages: cheap existence probe for active=0 rows.
- acp_adapter._persist: when the agent doesn't own persistence but the session
  already has archived rows on disk, replace active-only; otherwise the
  destructive full replace stays (fresh create/fork has nothing to lose).
- Regression test: model-switch save on a compacted session keeps the archived
  turn discoverable via get_messages(include_inactive=True) + search_messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants