Skip to content

fix(agent): adopt live compression child on flush - #79763

Open
KCAYAAI wants to merge 1 commit into
NousResearch:mainfrom
KCAYAAI:fix/adopt-compression-continuation-on-flush
Open

KCAYAAI wants to merge 1 commit into
NousResearch:mainfrom
KCAYAAI:fix/adopt-compression-continuation-on-flush

Conversation

@KCAYAAI

@KCAYAAI KCAYAAI commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

A transcript flush that loses a context-compression race now follows the session's unique live continuation instead of aborting the turn against the closed parent.

If Hermes cannot select exactly one live continuation, it still fails closed, preserves the unpersisted markers for a safe retry, and reports a compression-lineage failure rather than suggesting that the disk is full.

Problem

SessionDB.append_messages_batch() correctly rejects writes to sessions closed by compression. AIAgent._flush_messages_to_session_db_unlocked() treated that specific rejection like any other persistence failure, so an otherwise healthy turn could stop after another compression path rotated the session.

The resulting session_persistence_failed message pointed operators toward disk capacity or state.db permissions even when both were healthy.

Changes

  • Catch CompressionSessionClosedError at the agent's atomic batch-flush boundary.
  • Resolve exactly one live compression child and append the unchanged batch inside one BEGIN IMMEDIATE transaction, preventing a second publisher from making the lineage ambiguous between selection and commit.
  • Update the agent/session logging binding only after the atomic reroute commits.
  • Preserve fail-closed behavior when the child is missing or ambiguous.
  • Propagate unresolved cases as compression_session_closed with actionable, non-disk guidance.
  • Add regression coverage for successful adoption, idempotent repeat flushes, ambiguous-child refusal, and turn-finalizer classification.

Relationship to existing work

This is distinct from #77386 / #77410 / #77799, which address writers waiting on a compression lock, and from #57895 / #79322, which route queued gateway follow-ups after a completed turn. This patch handles the lower-level agent transcript flush after the parent has already been closed and a continuation published.

Validation

  • pytest tests/run_agent/test_compression_closed_flush_reroute.py tests/run_agent/test_860_dedup.py tests/run_agent/test_tool_call_incremental_persistence.py tests/run_agent/test_turn_completion_explainer.py -q -o addopts= — 25 passed
  • pytest tests/agent/test_compression_concurrent_fork.py tests/test_hermes_state_compression_busy_retry.py tests/hermes_state/test_append_messages_batch.py tests/hermes_state -q -o addopts= — 99 passed
  • pytest tests/gateway/test_session.py tests/agent/test_turn_finalizer_final_response_persistence.py -q -o addopts= — 64 passed
  • ruff check hermes_state.py run_agent.py agent/conversation_loop.py agent/turn_finalizer.py tests/run_agent/test_compression_closed_flush_reroute.py tests/run_agent/test_tool_call_incremental_persistence.py tests/hermes_state/test_append_messages_batch.py
  • git diff --cached --check

Checklist

Code

  • I've read the Contributing Guide
  • My commit message follows Conventional Commits
  • I searched existing PRs for overlap
  • The PR contains only changes related to this fix
  • Tests cover the new behavior
  • Tested on Linux

Documentation & Housekeeping

  • Documentation update is not required for this internal persistence behavior
  • No configuration keys or tool schemas changed
  • Cross-platform behavior was considered; the change is SQLite/session logic only

Agent Disclosure

Created with Hermes Agent under direct human instruction and reviewed before submission.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/compression Context compression and continuation sessions P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 5, 2026
@KCAYAAI
KCAYAAI force-pushed the fix/adopt-compression-continuation-on-flush branch from e2ef308 to fb30dae Compare August 8, 2026 01:28
@KCAYAAI

KCAYAAI commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this PR onto current main (b3aa561faffd64f05436e429a6415d175e534ec9) with no conflicts; the branch is now one commit ahead and zero behind.

Local verification on head fb30dae1b2dcb794321ff065166272c3d3604721:

  • Focused compression/persistence suite: 25 passed, 0 failed
  • Ruff, py_compile, and git diff --check: passed
  • Patch ID is unchanged from the prior reviewed head

CI is red only on tests/test_tui_gateway_server.py::test_write_json_serializes_concurrent_writes in slice 5/12. The failing test and tui_gateway/server.py are byte-identical to current main, and the exact test passed 10/10 local reruns on this PR head.

I attempted to rerun the failed upstream job, but GitHub returned 403 Must have admin rights to Repository; fork authors cannot trigger that rerun.

Could a maintainer please rerun the failed slice 5/12 job?

@spfcraze

spfcraze commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
The new append_messages_batch_to_live_compression_child query filters children by marker presence (IS NULL), so a delegate subagent's continuation — which the canonical parent-bound find_live_compression_child filter keeps adoptable — is excluded and the reroute fails closed.

Problems:

  • hermes_state.py:6883-6884 (new method): the inline child query disqualifies any child carrying _branched_from/_delegate_from in model_config, regardless of what the marker points at.
  • The canonical filter _NON_CONTINUATION_CHILD_FILTER_SQL (hermes_state.py:3734) binds the queried parent id for both markers instead; its comment (hermes_state.py:3726-3733) documents why: a delegate subagent's continuation inherits _delegate_from=<the delegate's own parent> (set at tools/delegate_tool.py:1679, passed verbatim by agent/conversation_compression.py:3296), so presence-matching misclassifies it as a delegate child and adoption fails closed.
  • tests/state/test_compression_lineage_guard.py:121-136 pins that a continuation with a foreign _delegate_from must remain adoptable; the PR's regression test exercises only a marker-free child (it patches find_live_compression_child with side_effect=AssertionError).

Solution:
Bind the queried parent id for both markers in the inline query, as _NON_CONTINUATION_CHILD_FILTER_SQL does (COALESCE(json_extract(COALESCE(model_config, '{}'), '$._delegate_from'), '') != ?), so a delegate subagent's continuation is adoptable.


Checked against fb30dae — the tip of fix/adopt-compression-continuation-on-flush when this was written — and a4af262, main at the same moment.

@ayushnangia

Copy link
Copy Markdown
Contributor

Heads-up from the forensics on #82001: @Al3xand3r1987 reproduced a flush failure that this PR's resolver can't recover — a two-level lineage (5358b2bc1ab2 → be2516 (end_reason='compression') → 3051a5 (live)). find_live_compression_child is depth-1 by design, so when the direct child is itself compressed it returns nothing and the turn still dies with session_persistence_failed. The store's get_compression_tip (hermes_state.py:6818) already does the transitive walk with sibling disambiguation (branch/delegate/tool exclusion, live-over-ws_orphan_reap preference, bounded); swapping the lookup to tip = get_compression_tip(old_id) + your existing single-live-continuation verification keeps the fail-closed contract and covers the reproduced case. That lineage makes a good additional red test — the fix direction here is right, it just needs the multi-hop resolver.

@miroslavb

Copy link
Copy Markdown

Thank you for establishing the closed-parent final-flush adoption direction here. I opened draft #86099 to cover the two blockers already identified in this thread: parent-bound handling of inherited foreign branch/delegate/tool markers, and the multi-generation A -> B -> C lineage reproduced in #82001. It also revalidates the selected tip inside the transcript write transaction and applies the same invariant to gateway/recovery writers.\n\n#86099 explicitly credits this PR and is not a request to merge both implementations; maintainers can choose the clean superseding path once its independent review/checks complete.

@Al3xand3r1987

Copy link
Copy Markdown
Contributor

Quick follow-up on the resolver choice — the multi-hop case from #82001 applies here too.

The selection query in append_messages_batch_to_live_compression_child is depth-1: it looks for direct live children of the parent (parent_session_id = ? AND ended_at IS NULL, excluding _branched_from/_delegate_from and source = 'tool', ORDER BY started_at ASC LIMIT 2). For a multi-hop lineage (p → mid → head, where mid is already compression-ended), the only direct child of p is mid, which has ended_at set — so the query returns 0 rows and the flush fails closed, even though a live head exists.

This isn't hypothetical: we now have four independent multi-hop reports in the wild — our own 2-hop repro, juliedarkheart's 2-hop WebUI-restart case, and 3-hop chains from gyu-bot and ayushnangia (all in #82001).

The atomic selection+append is a solid TOCTOU close. If multi-hop turns out to be common, promoting selection to the transitive tip (get_compression_tip, 100-hop bound, same branch/delegate/tool exclusions) would cover the chain cases too — we've been running that variant on both write paths since Aug 8 without recurrence. Happy to share the 3-hop regression test if useful.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @KCAYAAI — this PR had the right instincts from the start, and its fail-closed framing (never guess a target session; preserve the batch for a safe retry; report a compression-lineage failure instead of "full disk") is exactly the contract the landed fix keeps.

Heads-up that #86666 has now merged a full-chain version of this fix and closed #82001: after this PR was opened, the issue thread reproduced lineages with ≥2 compression hops (root(compressed) → mid(compressed) → tip(live)), which a depth-1 find_live_compression_child lookup cannot resolve — the direct child is itself already compression-ended. The merged change routes the agent flush, the gateway transcript flush, and the turn-start recovery preflight through the canonical transitive resolver SessionDB.get_compression_tip with a tip-liveness check and a one-adoption-per-flush budget, with co-author credit to the thread contributors who designed and verified it.

That supersedes the resolver here, so this PR is no longer needed as-is — but closing it is your call, so I'll leave that to you. Thanks again for kicking off the fix and for framing the fail-closed contract correctly.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(agent): adopt live compression child on flush

The transaction-level reroute (resolve child + insert under one BEGIN IMMEDIATE via _execute_write) is a solid fix for the compression race, and the fail-closed behavior for ambiguous continuations is well tested. Observations:

  1. hermes_state.py:append_messages_batch_to_live_compression_child — the inline child-selection query disqualifies children by presence of _branched_from/_delegate_from (json_extract(...) IS NULL), whereas the canonical _NON_CONTINUATION_CHILD_FILTER_SQL binds the queried parent id for both markers. The two filters therefore disagree: a delegate subagent's continuation inherits _delegate_from=<the delegate's own parent> (not this parent's id), so it is excluded by the inline query and the reroute fails closed, even though the canonical filter would keep it adoptable. Consider reusing the canonical filter (or mirroring its parent-binding semantics) so the reroute's adoptable-child set matches find_live_compression_child.

  2. Two user-facing strings for the same condition now exist and differ: finalize_turn emits "the previous session was compressed, but its live continuation could not be selected safely — send your message again" while _format_turn_completion_explanation emits "the turn was stopped because the previous session was compressed and its live compression continuation could not be selected safely...". If both surface in the same turn (error field + completion explanation), the user sees two differently-worded messages for one condition. Consider a single shared message constant.

  3. Scope asymmetry: only the batch flush path (append_messages_batch) reroutes to the live child. The single-message path (append_message) still raises CompressionSessionClosedError and stops the turn. If any live-turn write path can still hit a compression-closed parent through append_message, the race is only partially closed — worth confirming that path is unreachable for live turns or covering it too.

No blocking issues.

@ayushnangia

Copy link
Copy Markdown
Contributor

Thanks @teknium1 — appreciated. The fail-closed framing was the part we wanted reviewed hardest (never guess a target session, preserve the batch for retry), so this is good to hear. Keeping the depth-1 resolver note from @Al3xand3r1987 on the radar for the multi-hop case.

@alt-glitch alt-glitch added the duplicate This issue or pull request already exists label Aug 16, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #86409; the mechanism is the same.

@alt-glitch alt-glitch added area/sessions Session lifecycle, resume, persistence, history and removed duplicate This issue or pull request already exists labels Aug 16, 2026
@ayushnangia

Copy link
Copy Markdown
Contributor

Noting the triage comment: #86409 implements the transitive tip resolution (we verified its suite — 11/11), while this PR's distinct contribution is the fail-closed selection policy: never guess a target session, preserve the batch for a safe retry, report a compression error instead. Complementary boundaries rather than the same mechanism. If maintainers judge the mechanism absorbed, the one thing worth preserving from here is that fail-closed framing — happy to fold it into #86409 as a follow-up if that's the cleaner landing.

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 area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists 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.

8 participants