Skip to content

Bound overlapping agent hibernation evaluations - #9113

Merged
austinywang merged 4 commits into
mainfrom
issue-8740-main-thread-hibernation-poll
Jul 30, 2026
Merged

austinywang merged 4 commits into
mainfrom
issue-8740-main-thread-hibernation-poll

Conversation

@austinywang

@austinywang austinywang commented Jul 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • make the shared live-agent index the only owner of expensive session-registry refreshes
  • serialize periodic agent-hibernation evaluations with an explicit idle/running phase
  • coalesce timer delivery on the main actor and cancel evaluation work with controller lifecycle changes

Reproduction

From #8740:

  1. Open cmux with ~20 terminal surfaces across multiple workspaces
  2. Run multiple persistent terminal sessions inside cmux (e.g., zmx persistent sessions, pi coding agents, SSH connections, shell processes)
  3. Make sure agentHibernation is enabled in cmux.json with reasonable limits
  4. Wait 1–2 days

Verification

  • behavioral regression test holds one evaluation open, rejects later timer requests, and confirms a subsequent evaluation can run after returning to idle
  • tagged dev-build dogfood will cover 20+ terminal surfaces, over-cap hibernation, enable/disable lifecycle, and main-thread CPU sampling

The original history-dependent 1–2 day failure has no stack trace and cannot be honestly reproduced within this PR session. Verification therefore targets the concrete unbounded-overlap mechanism plus live over-cap behavior; this residual uncertainty will remain explicit.

Closes #8740


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Bound agent hibernation evaluations to run one at a time and moved the periodic timer to the main actor to stop overlapping work and CPU spikes, addressing #8740. Cancels in-flight work on lifecycle changes, re-checks the tracking gate after async index refresh, and ignores late completions; the shared live-agent index now owns and coalesces refreshes.

  • Bug Fixes
    • Serialize evaluations with idle/running phases and id-tracked tasks via startEvaluationIfIdle/finishEvaluation.
    • Gate timer and manual requests while active; main-actor timer calls scheduleEvaluation(now:); SharedLiveAgentIndex.indexRefreshingNow() returns a fresh index and coalesces callers; re-check the hibernation gate after suspension.
    • Clearing tracking cancels active work; tests cover gating, cancellation safety, and refresh coalescing.

Written for commit 7962a19. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added immediate “live agent index” refresh capability that coalesces concurrent refresh requests.
  • Bug Fixes
    • Improved evaluation scheduling to ensure evaluations run strictly one at a time.
    • Prevented canceled evaluations from interfering with subsequently started evaluations.
    • Cancel in-progress evaluation when hibernation tracking is reset.
  • Reliability
    • Evaluations now rely on the freshest available agent index when scheduling occurs.
  • Tests
    • Added automated coverage for sequential evaluation behavior, cancellation safety, and refresh coalescing.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Agent hibernation evaluation now uses a main-actor state machine that prevents overlapping runs, coordinates shared index refreshes, supports cancellation, and routes timer ticks through the new scheduling path. Tests cover serialized execution, stale completions, refresh coalescing, and project integration.

Changes

Agent hibernation scheduling

Layer / File(s) Summary
Evaluation state machine
Sources/App/AgentHibernationController+EvaluationScheduling.swift, Sources/App/AgentHibernationController.swift
Adds idle/running evaluation phases, guarded task creation, cancellation, and request-identity-based completion.
Timer and index refresh integration
Sources/App/AgentHibernationController.swift, Sources/SharedLiveAgentIndex.swift
Routes timer ticks through main-actor scheduling, coalesces shared index refreshes, and cancels active evaluations during tracking reset.
Scheduling validation and project wiring
cmuxTests/AgentHibernationEvaluationSchedulingTests.swift, cmux.xcodeproj/project.pbxproj
Registers the new files and tests serialized evaluation, stale completion handling, and concurrent refresh coalescing.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MainTimer
  participant AgentHibernationController
  participant SharedLiveAgentIndex
  MainTimer->>AgentHibernationController: scheduleEvaluation(now)
  AgentHibernationController->>SharedLiveAgentIndex: indexRefreshingNow()
  SharedLiveAgentIndex-->>AgentHibernationController: return refreshed index
  AgentHibernationController->>AgentHibernationController: evaluate(index, settings, now)
Loading

Possibly related PRs

  • manaflow-ai/cmux#7352: Also changes SharedLiveAgentIndex refresh orchestration used by hibernation-related flows.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description has Summary and Verification, but it omits the required Demo Video, Review Trigger, and Checklist sections. Add the missing Demo Video, Review Trigger, and Checklist sections, and include a clear Testing section if Verification is kept.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #8740 by serializing evaluations, coalescing refreshes, and canceling in-flight work to avoid overlap.
Out of Scope Changes check ✅ Passed The added tests, project wiring, and index API are directly tied to the hibernation-evaluation fix and not out of scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed PASS: New scheduling stays on @MainActor, SharedLiveAgentIndex remains @MainActor, and no touched production type introduces unsafe Sendable/background access.
Cmux Swift Blocking Runtime ✅ Passed Production Swift adds actor-based evaluation phase/state; no new blocking waits, sleeps, locks, or main-queue sync. Blocking primitives appear only in tests.
Cmux Browser Automation Off-Main ✅ Passed Diff only touches hibernation scheduling/tests; no browser automation files or browser.* wait/routing changes are present.
Cmux Expensive Synchronous Load ✅ Passed The new path awaits SharedLiveAgentIndex.shared.indexRefreshingNow(), and the actual RestorableAgentSessionIndex.load() stays in a Task.detached loader off MainActor.
Cmux Cache Substitution Correctness ✅ Passed indexRefreshingNow() always reloads or awaits in-flight work, and the only new call site is evaluation scheduling—not a persistence/history/snapshot path.
Cmux No Hacky Sleeps ✅ Passed Only Swift files changed; no non-Swift runtime/build scripts were added, so the hacky-sleeps rule is not applicable.
Cmux Algorithmic Complexity ✅ Passed No new nested scans or per-target rescans were introduced; the PR adds linear, coalesced refresh/evaluation control and test-only coverage, which the rule allows.
Cmux Swift Concurrency ✅ Passed Production diff only removes a computed property and tightens cancelEvaluation; no new legacy async pattern was introduced. Test-only Task/DispatchSemaphore use is allowed.
Cmux Swift @Concurrent ✅ Passed PASS: the new async paths remain @MainActor/UI-bound, and SharedLiveAgentIndex.reload offloads loading via Task.detached; no new nonisolated async needs @concurrent.
Cmux Swift Package Boundaries ✅ Passed PASS: The patch is app-lifecycle glue plus an internal singleton cache helper; it doesn't move reusable domain logic across an existing SwiftPM boundary.
Cmux Swiftpm Lockfiles ✅ Passed Diff only touches Swift source/tests; no cmux-owned .gitignore, Package.swift, pbxproj, or Package.resolved changes, so the lockfile rule is not triggered.
Cmux Swift Logging ✅ Passed No added print/debugPrint/NSLog/Logger usage or ad hoc stdout/file logging in the touched runtime Swift files; the new code only schedules/cancels evaluation.
Cmux User-Facing Error Privacy ✅ Passed The diff only adds scheduling/state-machine code and tests; it adds no user-facing errors, alerts, or vendor/internal error details.
Cmux Full Internationalization ✅ Passed Only controller logic, tests, and project wiring changed; no user-facing strings, localization APIs, or locale/message catalog files were added.
Cmux Swiftui State Layout ✅ Passed No SwiftUI view/state/layout patterns were introduced; the diff only adds controller/index scheduling logic and tests, with no ObservableObject, GeometryReader, or lazy-row store refs.
Cmux Architecture Rethink ✅ Passed The diff adds explicit idle/running ownership and coalesced refreshes in one shared path; no new sleep/polling workaround or split lifecycle owner was introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR only changes hibernation scheduling/index-refresh logic and tests; no new or modified NSWindow/NSPanel/NSWindowController/WindowGroup code or cmux.* close-shortcut ownership appears.
Cmux Source Artifacts ✅ Passed All changed paths are hand-written Swift source, tests, or Xcode project config; no logs, caches, build output, or temp/artifact paths are present.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No new production test/debug seam was added; the new APIs are production-used and there are no debug/test-only guards or names in Sources.
Cmux No Ambient Global State ✅ Passed No new file-scope API or singleton was introduced; the new evaluation phase and scheduling logic live on AgentHibernationController, and existing shared singletons were only used.
Title check ✅ Passed The title is concise and accurately summarizes the main change: serializing overlapping agent hibernation evaluations.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch issue-8740-main-thread-hibernation-poll
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-8740-main-thread-hibernation-poll

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.

@austinywang
austinywang marked this pull request as ready for review July 29, 2026 01:07
@austinywang
austinywang force-pushed the issue-8740-main-thread-hibernation-poll branch from 44278fc to 7ac890f Compare July 29, 2026 04:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@Sources/App/AgentHibernationController`+EvaluationScheduling.swift:
- Around line 9-12: Remove the production-only evaluationTask accessor from the
AgentHibernationController evaluation scheduling implementation. Update
cancelEvaluation() to destructure evaluationPhase directly, and revise
AgentHibernationEvaluationSchedulingTests to await the operation-owned
completion signal instead of accessing the removed property.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 24a38471-fd9d-456f-969d-ae850a64a0d7

📥 Commits

Reviewing files that changed from the base of the PR and between 246f6ee and 7ac890f.

📒 Files selected for processing (5)
  • Sources/App/AgentHibernationController+EvaluationScheduling.swift
  • Sources/App/AgentHibernationController.swift
  • Sources/SharedLiveAgentIndex.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/AgentHibernationEvaluationSchedulingTests.swift

Comment thread Sources/App/AgentHibernationController+EvaluationScheduling.swift Outdated
@austinywang
austinywang force-pushed the issue-8740-main-thread-hibernation-poll branch from abdfe05 to a30df84 Compare July 30, 2026 02:32

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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+EvaluationScheduling.swift (1)

9-22: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Verify the duplicate AgentHibernationTrackingGate.isEnabled() check is intentional.

The gate is checked identically before and after await SharedLiveAgentIndex.shared.indexRefreshingNow() (Lines 12 and 15). If this is meant to re-validate state after the await (since tracking could be disabled while the refresh was in flight), a brief comment would help; if it's a leftover duplicate, the second check should be removed.

💡 Suggested clarification
     func scheduleEvaluation(now: Date) {
         startEvaluationIfIdle { [weak self] in
             guard let self,
                   AgentHibernationTrackingGate.isEnabled(),
                   let index = await SharedLiveAgentIndex.shared.indexRefreshingNow(),
                   !Task.isCancelled,
+                  // Re-check: tracking may have been disabled while awaiting the refresh.
                   AgentHibernationTrackingGate.isEnabled() else {
                 return
             }
🤖 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`+EvaluationScheduling.swift around
lines 9 - 22, Clarify the intent of the duplicate
AgentHibernationTrackingGate.isEnabled() checks in scheduleEvaluation: retain
the post-await check with a brief comment if it revalidates state after
indexRefreshingNow(), or remove it if redundant.
🤖 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`+EvaluationScheduling.swift:
- Around line 9-22: Clarify the intent of the duplicate
AgentHibernationTrackingGate.isEnabled() checks in scheduleEvaluation: retain
the post-await check with a brief comment if it revalidates state after
indexRefreshingNow(), or remove it if redundant.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b23f5aa1-7013-4114-a0a9-c1b07468a16f

📥 Commits

Reviewing files that changed from the base of the PR and between 7ac890f and abdfe05.

📒 Files selected for processing (2)
  • Sources/App/AgentHibernationController+EvaluationScheduling.swift
  • cmuxTests/AgentHibernationEvaluationSchedulingTests.swift

@austinywang

Copy link
Copy Markdown
Contributor Author

Addressed in 7962a19: the post-await AgentHibernationTrackingGate.isEnabled() check is intentional because stop() may disable tracking while the shared index refresh is suspended. I retained the safety revalidation and added an inline comment documenting that actor-reentrancy boundary.

@austinywang
austinywang merged commit af519dd into main Jul 30, 2026
7 checks passed
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.

Main thread pinned at 99% CPU with 20+ terminal surfaces — possible agent hibernation polling loop

1 participant