Skip to content

fix(acp): preserve assistant reasoning metadata in session persistence#13575

Closed
Aslaaen wants to merge 1 commit into
NousResearch:mainfrom
Aslaaen:fix/acp-reasoning-persistence
Closed

fix(acp): preserve assistant reasoning metadata in session persistence#13575
Aslaaen wants to merge 1 commit into
NousResearch:mainfrom
Aslaaen:fix/acp-reasoning-persistence

Conversation

@Aslaaen

@Aslaaen Aslaaen commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

ACP session persistence was dropping assistant reasoning metadata during save/restore.

This change forwards assistant reasoning, reasoning_details, and
codex_reasoning_items into SessionDB.append_message() so restored ACP
sessions retain the same assistant context they had before persistence.

Why this is a bug

hermes_state.SessionDB already supports storing and restoring these fields,
but acp_adapter.session.SessionManager._persist() only wrote the basic
message fields. As a result, ACP session restore silently degraded assistant
history by removing reasoning-related metadata.

That created an inconsistent round-trip:

  • in-memory ACP history kept reasoning fields
  • persisted ACP history lost them after restore

Changes

  • update ACP session persistence to pass:
    • reasoning
    • reasoning_details
    • codex_reasoning_items
  • add a regression test covering assistant reasoning round-trip persistence

Tests

Ran:

  • uv run --with pytest --with pytest-xdist pytest tests/acp/test_session.py -q -n 4

Result:

  • 31 passed

@Aslaaen Aslaaen marked this pull request as draft April 21, 2026 15:06
@Aslaaen Aslaaen marked this pull request as ready for review April 21, 2026 15:06
@alt-glitch alt-glitch added type/bug Something isn't working comp/acp Agent Communication Protocol adapter labels Apr 21, 2026
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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants