Repository navigation
Prevent hibernation from reaping live agent processes - #6576
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a ChangesAgent Hibernation — Live Process Guard
Sequence Diagram(s)sequenceDiagram
participant Record
participant Controller
participant TailSamples
participant Confirmation
participant Planner
Record->>Controller: evaluate(record, isLiveByKey)
Note over Controller: Check record.hasLiveProcess
alt hasLiveProcess == true
Controller->>TailSamples: clear()
Controller->>Confirmation: clear()
Controller->>Planner: Input with hasLiveProcess=true
Planner->>Planner: Exclude from selection
else hasLiveProcess == false
Controller->>TailSamples: updateTailFingerprintSample()
Controller->>Planner: Input with hasLiveProcess=false
Planner->>Planner: Eligible for selection
Note over Planner: Check if excess & unprotected
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/App/AgentHibernationController.swift (1)
28-35: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRedundant
!input.hasLiveProcesscheck on line 35.Since
liveRestorable(line 28) already excludes entries wherehasLiveProcess == true, the second check at line 35 is redundant. The redundancy is harmless and provides defense-in-depth against future refactoring, so this is acceptable to keep.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/App/AgentHibernationController.swift` around lines 28 - 35, The filter on the eligible constant contains a redundant check for hasLiveProcess. Since liveRestorable is already filtered on line 28 to exclude any inputs where hasLiveProcess is true, the condition !input.hasLiveProcess on line 35 is unnecessary. Remove this redundant condition from the filter closure, keeping only the !input.isProtected check and any other remaining filter conditions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/App/AgentHibernationController.swift`:
- Around line 28-35: The filter on the eligible constant contains a redundant
check for hasLiveProcess. Since liveRestorable is already filtered on line 28 to
exclude any inputs where hasLiveProcess is true, the condition
!input.hasLiveProcess on line 35 is unnecessary. Remove this redundant condition
from the filter closure, keeping only the !input.isProtected check and any other
remaining filter conditions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88a17d52-d084-407b-8d48-98138f1df279
📒 Files selected for processing (1)
Sources/App/AgentHibernationController.swift
|
Thanks for the fast turnaround, @austinywang — gating both the LRU and idle paths on Flagging two items from the original "Expected behavior" for tracking (not blocking this PR):
Happy to help with a follow-up for either once this lands. |
…n-transcript-loss # Conflicts: # cmux.xcodeproj/project.pbxproj
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fdb36ec. Configure here.
Close the transcript-loss races around post-teardown restore monitors: - Registered monitors stay armed until a teardown commits; every quiesce is followed synchronously by a replacement monitor or a forfeit-armed monitor on the fresh snapshot, so a stubbed transcript is never left with zero monitors. - Forfeit-armed monitors use a retain-for-recovery disposal: an unrestored snapshot whose live path diverged moves into a bounded per-session recovery slot instead of being deleted or orphaned. - Bulk monitor cancellation chains a drain task that teardown batches await, so an unregistered cancelled monitor's final restore cannot race a batch. - Monitor registry keys resolve symlinks so aliased transcript paths cannot arm two monitors on one file. - Snapshot byte comparison loop-fills short reads and records a file-version triple revalidated with no suspension before SIGTERM. - Failed snapshot attempts replace a single retained recovery copy per session instead of accumulating full-transcript copies. Regression coverage: monitor replacement/handoff, forfeit-arm restore, bulk-cancel drain, symlink key aliasing, retained-slot dedupe, and post-copy/post-comparison snapshot races. Fixes #6565 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n-transcript-loss # Conflicts: # Sources/Workspace+PanelLifecycle.swift
Workspace+PanelLifecycle.swift reached the 500-line budget threshold on the speculative merge with main; move the cohesive sidebar status entry visibility helpers into their own extension file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The planner suite's shared-state reset cancelled every entry in the process-global restore-monitor registry, killing the serialized monitor suite's in-flight tasks when suites interleaved (flaked replacingOneTranscriptMonitorLeavesOtherTranscriptMonitorRunning). Clean up only the suite's own registry entry, keyed by request ID. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Summary
Task.sleepcalls with boundedContinuousClocksleepsTesting
git diff --checkpython3 scripts/normalize-pbxproj.py --check/Users/austinwang/manaflow/cmuxterm-hq/skills/review/autoreview/scripts/cmux-policy-check --mode local --base origin/mainpython3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv --base-ref "$(git merge-base origin/main HEAD)"xcodebuild -quiet -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -derivedDataPath /Users/austinwang/Library/Developer/Xcode/DerivedData/cmux-issue-6565-hibernation-transcript-loss -only-testing:cmuxTests/AgentHibernationPlannerSwiftTests -only-testing:cmuxTests/AgentHibernationTranscriptGuardTests -only-testing:cmuxTests/AgentHibernationTranscriptGuardScanTests test./scripts/reload.sh --tag issue-6565-hibernation-transcript-loss/Users/austinwang/manaflow/cmuxterm-hq/skills/review/autoreview/scripts/autoreview --mode branch --base origin/mainpython3 /Users/austinwang/manaflow/cmuxterm-hq/skills/autoreview/scripts/wait_pr_status.py 6576 --repo manaflow-ai/cmux->30 pass, 0 pending, 0 failDemo Video
Review Trigger
$autoreviewrequested. Latest Cursor Bugbot feedback addressed indd559f8099; canonical autoreview follow-ups addressed in28687f0a84,78429d600d,e91f95cf51,0cfb02648d,f88ef5d09b,3ac25b25f3,e369595d8f,0d07d8c1ec,9f9c8c0210,700f1c13da,a7ff6c834d,7a76137fe3,7694951ea9,d503d3b996,6b404a17e7,b89b39184a,8e842b30c5,fdb36ec4f3, and776ad6c3f1.Checklist
Closes #6565
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Prevents hibernation from tearing down panes with live scoped agent processes and adds a fail-closed Claude transcript guard that snapshots before teardown and safely restores clobbered
.jsonlfiles. Hardens restore monitors with a protection handoff and a bounded recovery slot to close remaining races. Closes #6565.Bug Fixes
transcriptPathfirst; scope hook records to the panel; search all config roots; prefer populated candidates; validate session IDs/paths; bound scans; skip workflow stubs; fail closed or mark temporary unable-to-protect.Refactors
AgentHibernationTranscriptGuard. Migrate tests to SwiftTestingwith coverage for resolver priority, scoped hook transcripts, ambiguous/unsafe transcripts, oversized-line scan, streaming restore, live-process reaping, PID-epoch invalidation, monitor handoff/recovery, bulk-cancel drain, symlink aliasing, and snapshot race handling.Workspace+SidebarStatusVisibility.swiftto keepWorkspace+PanelLifecycle.swiftwithin the file-length budget.Written for commit 2869915. Summary will update on new commits.
Summary by CodeRabbit
Note
High Risk
Changes agent hibernation teardown, SIGTERM timing, and on-disk Claude transcript files with complex async races; mistakes could drop conversations or leave orphaned processes.
Overview
Hibernation no longer tears down panes with live scoped agent processes—they still count toward the live cap but are excluded from planner selection, confirmation, and tail fingerprint sampling. PID changes bump a per-panel teardown epoch so in-flight teardowns abort when the runtime changes.
Confirmed teardown is now async and transcript-safe: Claude
.jsonlfiles are snapshotted off the main actor before SIGTERM/pty-close, with re-validation (fresh session index, fingerprints, protection, live-process checks) before commit. Ambiguous or unsafe transcript resolution fails closed with a temporary unable-to-protect backoff instead of risking conversation loss (#6565).A new
AgentHibernationTranscriptGuardresolves panel-scoped hook paths, copies and byte-validates snapshots, restores metadata-only clobbers via bounded post-teardown monitors (symlink-deduped, handoff on replacement, drain on bulk cancel), and retains unrestored copies in a per-session recovery slot. Controller logic is split across extension files; workspace PID hooks notify hibernation on process attach/detach.Reviewed by Cursor Bugbot for commit 2869915. Bugbot is set up for automated code reviews on this repo. Configure here.