Skip to content

Fix Hermes restore after turn end#5020

Open
lawrencecchen wants to merge 3 commits into
mainfrom
issue-5000-hermes-auto-restore
Open

Fix Hermes restore after turn end#5020
lawrencecchen wants to merge 3 commits into
mainfrom
issue-5000-hermes-auto-restore

Conversation

@lawrencecchen
Copy link
Copy Markdown
Contributor

@lawrencecchen lawrencecchen commented May 30, 2026

Fixes #5000

Summary:

  • Treat Hermes on_session_end as a turn boundary, matching how Hermes emits it after every response.
  • Keep the persisted Hermes restore route across on_session_end.
  • Keep on_session_finalize as the destructive session boundary that clears the restore binding and agent pid.

Red failure:

  • Red test commit: 7e8c94b08686a11af8ebd15077a4f58f256a4634
  • Red CI run: https://github.com/manaflow-ai/cmux/actions/runs/26681449424
  • Before proof: /Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-5000-hermes-auto-restore/auto-issue/20260530-032134/before-window/recording.mov | TLDR: red cmux consumes the Hermes route on per-turn on_session_end, prints stored_after_on_session_end=[], and sends surface.resume.clear plus clear_agent_pid.

Green verification:

  • Fix commits: a708ba41e37b8b819f2466af4e211d4e951d1ac6, 956a492e07d0fcf38ca991e7ca4f58a48f95be31
  • Fixed CI run: https://github.com/manaflow-ai/cmux/actions/runs/26681540883
  • Final local tagged reload: ./scripts/reload.sh --tag hrest passed after the Greptile cleanup.
  • Final cloud CLI harness: /Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-5000-hermes-auto-restore/auto-issue/20260530-032134/hermes-fixed2-output.txt confirms the final PR head keeps the route after on_session_end and clears on on_session_finalize.
  • After proof: /Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-5000-hermes-auto-restore/auto-issue/20260530-032134/after-window/recording.mov | TLDR: fixed cmux keeps hermes-session-restore after on_session_end, sends only feed.push, then clears on on_session_finalize.

Proof notes:

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment May 30, 2026 11:12am
cmux-staging Ready Ready Preview, Comment May 30, 2026 11:12am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

📝 Walkthrough

Walkthrough

Distinguishes per-turn agent hooks from true session boundaries by centralizing hook-name extraction and adding a predicate that treats hermes-agent on_session_end as a turn boundary; updates session-end handling and adds an integration test verifying restore-route preservation and finalization cleanup.

Changes

Hermes session restore via turn-boundary detection

Layer / File(s) Summary
Turn-boundary detection helpers
CLI/cmux.swift
Adds rawAgentHookEventName() to extract hook/event names from input.object/input.rawObject across multiple JSON key spellings, and sessionEndIsTurnBoundary() that treats grok and antigravity as boundaries and treats hermes-agent as a boundary only when the event name equals on_session_end (case-insensitive).
Session-end handler applies boundary detection
CLI/cmux.swift
Replaces the inline `def.name == "grok"
Hermes session lifecycle integration test
cmuxTests/CLIGenericHookPersistenceTests.swift
Adds testHermesAgentSessionEndPreservesRestoreRoute to verify that on_session_start persists the restore route, on_session_end (per-turn) preserves routing and emits only feed.push telemetry without clearing agent pid, and on_session_finalize clears routing and removes the stored route entry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I’m a rabbit in the code, nibbling at a bug so sly,
I chased the turn-boundary whisper till the restore could fly,
Hermes keeps its route through each little chat,
True-end cleans up proper — no more lost-format! 🐇✨

🚥 Pre-merge checks | ✅ 17 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix Hermes restore after turn end' directly summarizes the main change: addressing the issue where Hermes sessions fail to auto-restore by treating on_session_end as a turn boundary rather than destructive session end.
Linked Issues check ✅ Passed The code changes directly address #5000 requirements: treating Hermes on_session_end as a turn boundary (via sessionEndIsTurnBoundary()), preserving restore routes across on_session_end, and clearing only on on_session_finalize, with regression test coverage.
Out of Scope Changes check ✅ Passed All changes are tightly scoped to the Hermes session-end handling: refactoring hook event name extraction, centralizing turn boundary detection, and adding a focused integration test to verify the fix.
Cmux Swift Actor Isolation ✅ Passed Three local helper functions added to runGenericAgentHook() capture only value types and perform read-only operations. No MainActor violations, implicit models, or background access issues detected.
Cmux Swift Blocking Runtime ✅ Passed PR adds three synchronous, non-blocking Swift functions using only string operations. No blocking synchronization patterns introduced in production code. Test code uses standard XCTest utilities.
Cmux No Hacky Sleeps ✅ Passed PR modifies only Swift files. The rule scopes to TypeScript, JavaScript, shell, and build/runtime scripts; Swift is covered by a separate check.
Cmux Algorithmic Complexity ✅ Passed All code changes use tiny fixed-size collections (≤4 elements); no nested scans, batch rescans, or unbenchmarked algorithms on scalable user data. Test changes are exempt fixture code.
Cmux Swift Concurrency ✅ Passed PR introduces only synchronous helper functions and standard XCTest integration test; no legacy async patterns (DispatchQueue, Combine, Tasks, completion handlers) detected.
Cmux Swift @Concurrent ✅ Passed All new Swift functions in the PR are synchronous helpers with no async/concurrent issues. No @concurrent annotations are present or needed; no missing async boundaries detected.
Cmux Swift File And Package Boundaries ✅ Passed Focused bug fix with +10 net lines to existing oversized file, consolidating session-end logic; test additions allowed; no new mixed responsibilities.
Cmux Swift Logging ✅ Passed New helper functions contain no logging; the .sessionEnd case uses only debug-guarded #if DEBUG logging; test file has no logging violations.
Cmux User-Facing Error Privacy ✅ Passed No user-facing error/alert output added. Production changes are internal helpers only; test is developer-only. No prohibited information exposed.
Cmux Full Internationalization ✅ Passed PR contains only internal refactoring and tests. No user-facing text, string catalogs, or locale entries added. All strings are protocol tokens and identifiers exempt from i18n.
Cmux Swiftui State Layout ✅ Passed PR contains no SwiftUI code: CLI/cmux.swift is CLI handler code with no SwiftUI imports or state patterns; test file uses XCTest. Check not applicable.
Cmux Architecture Rethink ✅ Passed Small correctness fix with clear owner, invariant, and source of truth. No timing/blocking, new mutable state, or duplicate wiring patterns introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR changes only refactor Hermes hook event handling and add tests; no NSWindow, NSPanel, NSWindowController, or WindowGroup code added or modified.
Description check ✅ Passed The PR description comprehensively covers the issue being fixed, implementation details, testing approach, and includes extensive proof of verification through CI runs and local testing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-5000-hermes-auto-restore

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR fixes a bug where Hermes's per-turn on_session_end event was incorrectly treated as a full session termination, causing the persisted restore route to be cleared after every response. The fix introduces sessionEndIsTurnBoundary() and rawAgentHookEventName() helpers to discriminate event names inside the .sessionEnd handler.

  • rawAgentHookEventName() extracts the hook event name from the parsed input and is now shared between shouldSuppressGenericFeedTelemetry() (refactored) and the new sessionEndIsTurnBoundary().
  • sessionEndIsTurnBoundary() returns true for grok/antigravity (unchanged) and for hermes-agent only when the event name is on_session_end; on_session_finalize falls through to the existing destructive cleanup path.
  • The new integration test testHermesAgentSessionEndPreservesRestoreRoute verifies the full lifecycle: route survives on_session_end, surface.resume.clear and clear_agent_pid are only emitted on on_session_finalize.

Confidence Score: 5/5

Safe to merge. The change is narrowly scoped to the hermes-agent session-end branch and is guarded by a new integration test that exercises all three lifecycle events.

The new sessionEndIsTurnBoundary() helper correctly distinguishes on_session_end (turn boundary, keep restore route) from on_session_finalize (destructive cleanup) for hermes-agent, without touching the grok/antigravity path or any other agent. The nil-event-name edge case conservatively falls through to the destructive path, matching pre-fix behavior.

No files require special attention.

Important Files Changed

Filename Overview
CLI/cmux.swift Adds rawAgentHookEventName() helper and sessionEndIsTurnBoundary(), then gates the .sessionEnd turn-boundary branch on sessionEndIsTurnBoundary() instead of a hardcoded grok/antigravity check. Logic is correct: hermes-agent with on_session_end keeps the restore route; on_session_finalize falls through to the destructive cleanup path.
cmuxTests/CLIGenericHookPersistenceTests.swift Adds testHermesAgentSessionEndPreservesRestoreRoute, a full integration test that exercises session-start → on_session_end → on_session_finalize and asserts correct command emissions and stored-session state at each step.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[".sessionEnd case"] --> B{def.name == codex?}
    B -- yes --> C[retireCodexMonitorLeases]
    C --> D{sessionEndIsTurnBoundary?}
    B -- no --> D
    D -- yes --> E[sendAgentFeedTelemetry\nrecordPromptStop\nbreak — keep restore route]
    D -- no --> F{store.lookup found?}
    F -- yes --> G{suppressVisibleMutations?}
    G -- yes --> H[breadcrumb only]
    G -- no --> I[store.consume\nclearAgentSurfaceResumeBinding\nclear_agent_pid]
    F -- no --> J[no-op]
    subgraph sessionEndIsTurnBoundary
        K{grok or antigravity?} -- yes --> L[return true]
        K -- no --> M{hermes-agent?}
        M -- no --> N[return false]
        M -- yes --> O{event == on_session_end?}
        O -- yes --> P[return true]
        O -- no --> Q[return false]
    end
Loading

Reviews (2): Last reviewed commit: "fix: share hermes hook event lookup" | Re-trigger Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hermes (hermes-agent) sessions never auto-restore — per-turn on_session_end deletes the restore record

1 participant