Skip to content

fix(acp): resume compression continuations - #80423

Open
valo wants to merge 1 commit into
NousResearch:mainfrom
valo:fix/acp-compression-resume
Open

fix(acp): resume compression continuations#80423
valo wants to merge 1 commit into
NousResearch:mainfrom
valo:fix/acp-compression-resume

Conversation

@valo

@valo valo commented Aug 6, 2026

Copy link
Copy Markdown

Summary

  • Resolve the current compression-continuation session before restoring an ACP session.
  • Build the restored agent and transcript from that active storage segment, while retaining the caller's ACP ID as the in-memory lookup key.
  • Add a regression covering an ACP root session that was rotated by compression.

Test plan

  • scripts/run_tests.sh tests/acp/test_session.py tests/hermes_state/test_resolve_resume_session_id.py
  • .venv/bin/ruff check acp_adapter/session.py tests/acp/test_session.py
  • git diff --check

This prevents resumed ACP tasks from attempting to persist to a compression-ended parent session.

@alt-glitch alt-glitch added type/bug Something isn't working comp/acp Agent Communication Protocol adapter area/compression Context compression and continuation sessions P4 Best-effort: we will get to it when we get to it (no commitment) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 6, 2026
@ayushnangia

Copy link
Copy Markdown
Contributor

Reviewed as part of the rotation-seam map on #80337: this is the boundary version of the fix — resolve_resume_session_id is the canonical resolver for resume semantics, and restoring agent + history + SessionState under the resolved storage id (rather than patching one lookup) closes the whole member. The heartbeat sibling of this class is the #80225/#80208 pair; the recovery member merged as #80487.

@spfcraze

spfcraze commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
Restoring a compressed ACP session resolves the storage id into SessionState.session_id while the in-memory map stays keyed by the caller's id, so the session listing emits the continuation id twice and the one lookup documented as in-memory-only misses the restored state.

Problems:

  • _restore (acp_adapter/session.py) keeps self._sessions[session_id] = state under the caller's id while SessionState is built with session_id=storage_session_id — the state's own id is not a key in the map.
  • SessionManager.list_sessions emits s.session_id and skips persisted rows only when their id is an in-memory map key (seen_ids = set(self._sessions.keys())); the compression continuation row carries source=agent.platform ("acp"), so for a restored session the continuation id is listed twice — once from the in-memory state, once from the list_sessions_rich(source="acp") merge.
  • _schedule_mcp_late_refresh (acp_adapter/server.py) reads self.session_manager._sessions.get(state.session_id) as an in-memory-only lookup; for a restored session that key is the continuation id, which is not in the map, so the pre-first-turn tool refresh no-ops.

Solution:
Register the resolved id as an alias in the map alongside the caller's id (self._sessions[storage_session_id] = state), and dedupe list_sessions on s.session_id; get_session(caller_id) keeps working and state.session_id-keyed lookups hit the same state.


Checked against 17e701c — the tip of fix/acp-compression-resume when this was written — and b3aa561, main at the same moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions comp/acp Agent Communication Protocol adapter P4 Best-effort: we will get to it when we get to it (no commitment) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants