Skip to content

fix: preserve goals across compression session rotation - #47408

Closed
Umi4Life wants to merge 1 commit into
NousResearch:mainfrom
Umi4Life:fix/goal-survives-compression-rotation
Closed

Umi4Life wants to merge 1 commit into
NousResearch:mainfrom
Umi4Life:fix/goal-survives-compression-rotation

Conversation

@Umi4Life

@Umi4Life Umi4Life commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Context compression ends the active session and forks a child session (new session_id, linked via parent_session_id); /resume then redirects to that child. The goal/objective is keyed goal:<session_id> in state_meta and GoalManager does a flat lookup with no lineage fallback, so the objective read None on the child session after a long/compressed turn (and on resume).

Add migrate_goal(old, new) and call it at the rotation boundary in compress_context. Copy (not move), never overwrites an existing child goal, never raises — a migration failure can't abort the compression split. Flat GoalManager lookup and resume semantics are unchanged.

Tests: goal survives rotation + resume redirect, no-overwrite and no-source no-op invariants, and a focused test that compress_context actually calls migrate_goal during rotation.

Related to #15000: that issue covers compression-created child session chains for message history; this handles another session-scoped state affected by the same rotation.

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/goals.py — new module-level migrate_goal(old_session_id, new_session_id) helper beside save_goal/clear_goal. Copy semantics; returns True only when a goal is copied; skips if source absent, if old == new, or if the child already has a goal; swallows + logs errors (never raises).
  • agent/conversation_compression.py — call migrate_goal(old_session_id, agent.session_id) at the session-rotation boundary in compress_context (right after the title is propagated to the child), guarded so a migration failure can't abort the compaction split.
  • tests/hermes_cli/test_goals.py — TestGoalSurvivesCompressionRotation: goal survives rotation + resume redirect, no-overwrite invariant, no-source no-op invariant.
  • tests/run_agent/test_compression_persistence.py — test_compression_rotation_migrates_goal: drives the real compress_context rotation path (summarizer stubbed, no model call) and asserts migrate_goal(old, new) is invoked and the goal resolves under the rotated id.

How to Test

  1. Run the targeted suites: venv/bin/pytest tests/hermes_cli/test_goals.py tests/run_agent/test_compression_persistence.py → 60 passed.
  2. Regression proof (objective loss): on main, test_goal_survives_session_rotation_and_resume_redirect fails at GoalManager(child).has_goal() (objective lost across compression rotation); with this fix it passes.
  3. Guard proof (fix is load-bearing): temporarily neutralize the migrate_goal(...) call in compress_context → test_compression_rotation_migrates_goal fails (spy records no call); restore → passes.
  4. Manual: set a /goal, force a long turn that triggers compaction (or /compress), then /resume — the objective is still active under the rotated session instead of reading None.

Checklist

Code

Local platform: Debian GNU/Linux on x86_64, Python 3.11.15.

Documentation & Housekeeping

Context compression ends the active session and forks a child session
(new session_id, linked via parent_session_id); /resume then redirects
to that child. The goal/objective is keyed `goal:<session_id>` in
state_meta and GoalManager does a flat lookup with no lineage fallback,
so the objective read None on the child session after a long/compressed
turn (and on resume).

Add `migrate_goal(old, new)` and call it at the rotation boundary in
compress_context. Copy (not move), never overwrites an existing child
goal, never raises -- a migration failure can't abort the compression
split. Flat GoalManager lookup and resume semantics are unchanged.

Tests: goal survives rotation + resume redirect, no-overwrite and
no-source no-op invariants, and a focused test that compress_context
actually calls migrate_goal during rotation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 16, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for this — the diagnosis and fix are correct for the rotation path.

Context on why we're closing it: as of #52658 (#38763), compression.in_place now defaults to True. Compaction keeps ONE durable session id for the conversation's whole life — it no longer ends the session and forks a continuation id. Because the id no longer rotates, the active /goal stays attached to the same session automatically — there is no rotation boundary to migrate the goal across.

Rotation now only runs as an explicit opt-out (compression.in_place: false), and the direction is to treat that path as legacy. So this fix hardens a code path that no longer executes by default and that we're not investing further in. Closing as superseded by the in-place default — not a reflection on the code, which was a correct fix for the behavior at the time. Credit preserved in the issue/PR history. Appreciate the contribution.

@teknium1 teknium1 closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard 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