Hibernate idle background terminal surfaces and cap live surfaces (LRU) - #5739
Closed
austinywang wants to merge 64 commits into
Closed
austinywang wants to merge 64 commits into
austinywang wants to merge 64 commits into
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 surface-hibernation planner and types, UserDefaults-backed SurfaceHibernationSettings, workspace/panel runtime support for entering/restoring suspended surfaces, CLI/command-palette/settings UI wiring, Xcode/CI/test integration, and comprehensive unit and integration tests. ChangesSurface Hibernation Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (6 errors, 1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 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 |
Contributor
Greptile SummaryThis PR introduces
Confidence Score: 4/5Safe to merge with one outstanding architectural concern from a prior round: foreground-process verification calls blocking kernel syscalls on the main actor; the new code bounds these to 32 per tick with caching, which substantially reduces worst-case impact but does not move them off-actor. The new policy, mechanism, and restore paths are well-designed and thoroughly tested. All previously flagged blocking I/O issues on interactive paths have been addressed in earlier commits. The outstanding concern is the bounded-but-still-main-actor foreground verification (proc_listchildpids + KERN_PROCARGS2), flagged in a prior review round and partially mitigated here by capping at 32 calls per tick with a 600s busy-result cache. New findings this round are style-level: a blanket UserDefaults.didChangeNotification subscription that spawns a task on every app-wide defaults write, and a minor double pasteboard read in paste(_:). Sources/App/AgentHibernationController.swift deserves a second look for the UserDefaults.didChangeNotification subscription and the remaining main-actor foreground verification path. Important Files Changed
Sequence DiagramsequenceDiagram
participant Timer as 30s Timer
participant AHC as AgentHibernationController
participant Planner as SurfaceHibernationPlanner
participant Workspace
participant Panel as TerminalPanel
participant Surface as TerminalSurface
participant IO as Utility Task
Timer->>AHC: timer fires
AHC->>AHC: agentHibernationRecords - census all panels
AHC->>AHC: applyBoundedForegroundVerification - max 32 syscalls
AHC->>Planner: selectedPanelKeys - agentCap union globalCap union unmountedIdle
Planner-->>AHC: keys to hibernate
AHC->>AHC: evaluateConfirmation - start 60s stability window
alt agent panel - confirmed
AHC->>Workspace: enterAgentHibernation
Workspace->>Panel: enterAgentHibernation
Panel->>Surface: suspendRuntimeSurfaceForHibernation
else plain shell shellRestart - confirmed
AHC->>Workspace: captureSurfaceHibernation
Workspace-->>AHC: scrollback and cwd
AHC->>IO: replayFilePath - atomic write off main actor
IO-->>AHC: replayFilePath
AHC->>AHC: stillQuiet check
AHC->>Workspace: commitSurfaceHibernation
Workspace->>Panel: enterSurfaceHibernation with replayFilePath
Panel->>Surface: suspendRuntimeSurfaceForHibernation
end
Note over Panel,Surface: Panel stays in layout, surface freed
Note over Panel: On user focus or input restore trigger
Panel->>Panel: prepareSurfaceHibernationRestore
Panel->>Surface: stageHibernationRestore - replayFilePath and cwd
Panel->>Surface: prepareHibernationResume
Surface->>Surface: createSurface - new shell in captured cwd
Surface-->>Panel: shell consumes CMUX_RESTORE_SCROLLBACK_FILE
Reviews (40): Last reviewed commit: "Write replay files off-actor; fix CRLF c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
austinywang
force-pushed
the
issue-5731-surface-hibernation
branch
from
June 10, 2026 01:38
82ed136 to
ee11525
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/SurfaceHibernationPolicy.swift`:
- Around line 75-102: selectedPanelKeys only implements the agent-cap rule; add
the global LRU cap and unmounted-idle rules: (1) compute a separate list of all
live surfaces (include inputs where mechanism == .agentResume OR .shellRestart
and where input.isLive) and, regardless of agentSettings.enabled, if count >
surfaceSettings.maxLiveSurfaces evict the leastRecentlyUsedFirst ones to meet
that cap; (2) also consider any input whose workspaceUnmountedAt != nil and now
- workspaceUnmountedAt! >= surfaceSettings.unmountedIdleSeconds as eligible for
hibernation (even if not agentResume); merge keys from the agent-cap eviction
(current liveRestorable path), the global-cap eviction, and the unmounted-idle
rule into the returned Set; preserve existing protections (!input.isProtected,
lifecycle.allowsHibernation, !hasUnconfirmedTerminalInput) and reuse
Self.leastRecentlyUsedFirst sorting and eligible.prefix(excess).map(\.key) logic
for each rule.
🪄 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
Run ID: 75097a9f-2bbb-47b9-af2b-bdb17cba994d
📒 Files selected for processing (4)
Sources/App/SurfaceHibernationPolicy.swiftSources/App/WorkspaceRuntimeSettings.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SurfaceHibernationPolicyTests.swift
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/App/SurfaceHibernationPolicy.swift (1)
75-102:⚠️ Potential issue | 🔴 CriticalImplementation incomplete: missing global LRU cap and unmounted-idle rules.
This implements only the agent-cap rule (rule 1). Per the doc comment at lines 64-73 and PR objectives, two additional rules are missing:
- Global LRU cap (rule 2): Should count every live surface (including
.shellRestart) and evict when exceedingsurfaceSettings.maxLiveSurfaces, regardless ofagentSettings.enabled.- Unmounted-idle rule (rule 3): Should hibernate surfaces whose workspace has been unmounted longer than
surfaceSettings.unmountedIdleSeconds.Evidence:
- Line 87: Returns
[]whenagentSettings.enabledis false, blocking global cap and unmounted-idle enforcement- Line 88: Filters only
.agentResume, excluding.shellRestartsurfaces from global cap censussurfaceSettingsparameter unused (should drivemaxLiveSurfacesandunmountedIdleSeconds)workspaceUnmountedAtfield never checked🤖 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/SurfaceHibernationPolicy.swift` around lines 75 - 102, selectedPanelKeys currently only applies the agent-cap rule; update it to also enforce the global LRU cap and the unmounted-idle rule: 1) compute liveAll = inputs.filter { $0.isLive } (include both .agentResume and .shellRestart) and, regardless of agentSettings.enabled, compute excessGlobal = liveAll.count - surfaceSettings.maxLiveSurfaces; if excessGlobal > 0 pick the least-recently-used surfaces from liveAll (using Self.leastRecentlyUsedFirst) that are eligible for hibernation (respecting isProtected, lifecycle.allowsHibernation, !hasUnconfirmedTerminalInput, and lastActivityAt) and include their keys; 2) compute unmountedIdleSet = inputs.filter { $0.workspaceUnmountedAt != nil && now - ($0.workspaceUnmountedAt!) >= surfaceSettings.unmountedIdleSeconds && $0.isLive && !$0.isProtected && $0.lifecycle.allowsHibernation && !$0.hasUnconfirmedTerminalInput }.map(\.key) and include these keys; 3) keep the existing agent-cap logic (when agentSettings.enabled) to produce agentEvict keys and then return the union of agentEvict, globalEvict, and unmountedIdleSet (as a Set) while ensuring selection order for capped evictions uses the LRU sort (Self.leastRecentlyUsedFirst) and you don’t double-evict the same key.
🤖 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.
Duplicate comments:
In `@Sources/App/SurfaceHibernationPolicy.swift`:
- Around line 75-102: selectedPanelKeys currently only applies the agent-cap
rule; update it to also enforce the global LRU cap and the unmounted-idle rule:
1) compute liveAll = inputs.filter { $0.isLive } (include both .agentResume and
.shellRestart) and, regardless of agentSettings.enabled, compute excessGlobal =
liveAll.count - surfaceSettings.maxLiveSurfaces; if excessGlobal > 0 pick the
least-recently-used surfaces from liveAll (using Self.leastRecentlyUsedFirst)
that are eligible for hibernation (respecting isProtected,
lifecycle.allowsHibernation, !hasUnconfirmedTerminalInput, and lastActivityAt)
and include their keys; 2) compute unmountedIdleSet = inputs.filter {
$0.workspaceUnmountedAt != nil && now - ($0.workspaceUnmountedAt!) >=
surfaceSettings.unmountedIdleSeconds && $0.isLive && !$0.isProtected &&
$0.lifecycle.allowsHibernation && !$0.hasUnconfirmedTerminalInput }.map(\.key)
and include these keys; 3) keep the existing agent-cap logic (when
agentSettings.enabled) to produce agentEvict keys and then return the union of
agentEvict, globalEvict, and unmountedIdleSet (as a Set) while ensuring
selection order for capped evictions uses the LRU sort
(Self.leastRecentlyUsedFirst) and you don’t double-evict the same key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 15e33d4f-a7b1-486f-aa68-e13630af7f98
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Sources/App/SurfaceHibernationPolicy.swiftSources/App/WorkspaceRuntimeSettings.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SurfaceHibernationPolicyTests.swift
SurfaceHibernationPlanner starts as a faithful reproduction of the shipped policy: only restorable-agent terminals are counted or evicted, and only under agent-cap pressure. The new tests encode the policy the fix must implement — a global LRU cap whose census covers every live surface (including ones materialized by background priming and queued socket input), hibernation of idle surfaces in long-unmounted workspaces, and busy/protected/deferred-startup exemptions — and fail against the shipped behavior. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unmounting a workspace previously only hid its terminal portal views:
the Ghostty runtime surface — Metal/IOSurface buffers, four surface
threads, and the PTY — stayed alive until panel close, so hidden
workspaces accumulated live surfaces without bound (112 live surfaces
against one visible workspace in the issue audit).
SurfaceHibernationPlanner now selects surfaces to reclaim with three
composed rules: the pre-existing agent cap (unchanged), a global LRU
cap over every live surface, and an unmounted-workspace idle rule.
Plain shells hibernate via a new shellRestart mechanism: scrollback and
working directory are captured, the runtime surface is freed, and the
next visit starts a fresh shell in that directory with the scrollback
replayed through the existing session-restore plumbing. Agent panels
keep their opt-in agent-resume mechanism; busy surfaces (not at a
prompt), visible panels, remote terminals, and panels with deferred
startup work or queued input are never reclaimed, and the 60s
output-stability confirmation window still applies before anything is
freed.
Restore triggers mirror agent hibernation auto-resume: workspace
visibility reconcile, panel focus, and every input path (which would
otherwise wedge waiting for a surface that cannot materialize while
suspended). Session snapshots persist the captured scrollback, and the
autosave fingerprint tracks transitions.
Settings (default on): terminal.surfaceHibernation.{enabled,
idleSeconds, unmountedIdleSeconds, maxLiveSurfaces} via cmux.json,
settings search, command palette, `cmux surface-hibernation`, and the
`surface_hibernation` socket command, with schema and docs entries and
en/ja localization for the settings row.
Part of #5731
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
austinywang
force-pushed
the
issue-5731-surface-hibernation
branch
from
June 10, 2026 02:31
ee11525 to
4b2ae3d
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/App/SurfaceHibernationPolicy.swift (1)
75-102:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPlanner logic still implements only agent-cap mode; global-cap and unmounted-idle rules are missing.
selectedPanelKeysstill exits when agent hibernation is disabled and only evaluates.agentResumeinputs, so it never enforces global live-surface cap or unmounted-idle hibernation. This diverges from the policy contract and breaks expected behavior for plain-shell surfaces.Suggested minimal fix
static func selectedPanelKeys( inputs: [SurfaceHibernationPlannerInput], agentSettings: AgentHibernationSettings.Values, surfaceSettings: SurfaceHibernationSettings.Values, now: TimeInterval ) -> Set<AgentHibernationPanelKey> { - // This mirrors the shipped AgentHibernationPlanner policy: only - // restorable-agent terminals are counted or evicted, and only under - // agent-cap pressure. Plain-shell surfaces are invisible to it, the - // global cap census does not exist, and workspaces unmounted for long - // periods keep every runtime surface alive - // (https://github.com/manaflow-ai/cmux/issues/5731). - guard agentSettings.enabled else { return [] } - let liveRestorable = inputs.filter { $0.mechanism == .agentResume && $0.isLive } - let excess = liveRestorable.count - agentSettings.maxLiveTerminals - guard excess > 0 else { return [] } - - let eligible = liveRestorable - .filter { input in - !input.isProtected && - input.lifecycle.allowsHibernation && - !input.hasUnconfirmedTerminalInput && - now - input.lastActivityAt >= agentSettings.idleSeconds - } - .sorted(by: Self.leastRecentlyUsedFirst) - - return Set(eligible.prefix(excess).map(\.key)) + func isEligible( + _ input: SurfaceHibernationPlannerInput, + idleSeconds: TimeInterval + ) -> Bool { + guard input.isLive else { return false } + guard input.mechanism != nil else { return false } + guard !input.isProtected, !input.isBusy, !input.hasUnconfirmedTerminalInput else { return false } + guard input.lifecycle.allowsHibernation else { return false } + return now - input.lastActivityAt >= idleSeconds + } + + var selected = Set<AgentHibernationPanelKey>() + + // Rule 1: agent-cap + if agentSettings.enabled { + let liveAgent = inputs.filter { $0.mechanism == .agentResume && $0.isLive } + let excess = liveAgent.count - agentSettings.maxLiveTerminals + if excess > 0 { + let eligible = liveAgent + .filter { isEligible($0, idleSeconds: agentSettings.idleSeconds) } + .sorted(by: Self.leastRecentlyUsedFirst) + selected.formUnion(eligible.prefix(excess).map(\.key)) + } + } + + // Rule 2: global live-surface cap + let liveCount = inputs.filter { $0.isLive }.count + let globalExcess = liveCount - surfaceSettings.maxLiveSurfaces + if globalExcess > 0 { + let eligible = inputs + .filter { isEligible($0, idleSeconds: surfaceSettings.idleSeconds) } + .sorted(by: Self.leastRecentlyUsedFirst) + selected.formUnion(eligible.prefix(globalExcess).map(\.key)) + } + + // Rule 3: unmounted-idle + let unmountedEligible = inputs.filter { input in + guard isEligible(input, idleSeconds: surfaceSettings.idleSeconds) else { return false } + guard let unmountedAt = input.workspaceUnmountedAt else { return false } + return now - unmountedAt >= surfaceSettings.unmountedIdleSeconds + } + selected.formUnion(unmountedEligible.map(\.key)) + + return selected }🤖 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/SurfaceHibernationPolicy.swift` around lines 75 - 102, selectedPanelKeys currently only runs when agentSettings.enabled and filters inputs to .agentResume, so it never enforces the global live-surface cap or unmounted-idle rules for plain-shell surfaces; update the function (selectedPanelKeys) to (1) consider both agent-cap and global-cap pressure by computing live counts across all inputs (not just .agentResume) and comparing to agentSettings.maxLiveTerminals and surfaceSettings.maxLiveSurfaces, (2) include plain-shell/unmounted surfaces when evaluating eligibility (remove the .agentResume-only filter), and (3) apply the unmounted-idle eviction rule using surfaceSettings.unmountedIdleSeconds (or the appropriate unmounted idle field) so surfaces with now - lastActivityAt >= unmountedIdleSeconds become eligible; preserve existing checks (isProtected, lifecycle.allowsHibernation, hasUnconfirmedTerminalInput) and use the same LRU sort (Self.leastRecentlyUsedFirst) to pick the needed number of keys for eviction under each cap.
🤖 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.
Duplicate comments:
In `@Sources/App/SurfaceHibernationPolicy.swift`:
- Around line 75-102: selectedPanelKeys currently only runs when
agentSettings.enabled and filters inputs to .agentResume, so it never enforces
the global live-surface cap or unmounted-idle rules for plain-shell surfaces;
update the function (selectedPanelKeys) to (1) consider both agent-cap and
global-cap pressure by computing live counts across all inputs (not just
.agentResume) and comparing to agentSettings.maxLiveTerminals and
surfaceSettings.maxLiveSurfaces, (2) include plain-shell/unmounted surfaces when
evaluating eligibility (remove the .agentResume-only filter), and (3) apply the
unmounted-idle eviction rule using surfaceSettings.unmountedIdleSeconds (or the
appropriate unmounted idle field) so surfaces with now - lastActivityAt >=
unmountedIdleSeconds become eligible; preserve existing checks (isProtected,
lifecycle.allowsHibernation, hasUnconfirmedTerminalInput) and use the same LRU
sort (Self.leastRecentlyUsedFirst) to pick the needed number of keys for
eviction under each cap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ff88383-18bb-4abf-a34c-d90160147719
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (5)
.github/workflows/ci.ymlSources/App/SurfaceHibernationPolicy.swiftSources/App/WorkspaceRuntimeSettings.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SurfaceHibernationPolicyTests.swift
…bernation # Conflicts: # .github/swift-file-length-budget.tsv
Greptile flagged the hardcoded isBusy: true as unexplained. Real agent panels are never at a shell prompt, and busy only exempts the shellRestart mechanism, so the helper mirrors production while the lifecycle gate stays the variable under test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/SurfaceHibernationPolicyTests.swift (1)
10-10:⚠️ Potential issue | 🔴 CriticalWire
cmuxTests/SurfaceHibernationPolicyTests.swiftinto thecmuxTeststarget sources build phase.
SurfaceHibernationPolicyTests.swiftis present incmux.xcodeproj/project.pbxproj, but it’s missing from thecmuxTeststargetPBXSourcesBuildPhase, so Xcode/CI will silently skip it.🤖 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 `@cmuxTests/SurfaceHibernationPolicyTests.swift` at line 10, The test file SurfaceHibernationPolicyTests.swift (contains final class SurfaceHibernationPolicyTests) is not included in the cmuxTests target's PBXSourcesBuildPhase; open the Xcode project file (cmux.xcodeproj/project.pbxproj) or use Xcode target settings and add SurfaceHibernationPolicyTests.swift to the cmuxTests target's Sources build phase so the test class is compiled and run as part of the cmuxTests suite.Source: Coding guidelines
🤖 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 `@web/data/cmux.schema.json`:
- Around line 439-471: The new terminal.surfaceHibernation schema (object
"surfaceHibernation" and its properties "enabled", "idleSeconds",
"unmountedIdleSeconds", "maxLiveSurfaces") includes English-only "description"
texts but lacks locale-backed descriptionKey entries; add a descriptionKey
string for each of those schema nodes (surfaceHibernation and each property) and
then add matching message keys in every locale file under web/messages/* as
defined by web/i18n/routing.ts so each locale has the corresponding translations
for these descriptionKey identifiers.
---
Outside diff comments:
In `@cmuxTests/SurfaceHibernationPolicyTests.swift`:
- Line 10: The test file SurfaceHibernationPolicyTests.swift (contains final
class SurfaceHibernationPolicyTests) is not included in the cmuxTests target's
PBXSourcesBuildPhase; open the Xcode project file
(cmux.xcodeproj/project.pbxproj) or use Xcode target settings and add
SurfaceHibernationPolicyTests.swift to the cmuxTests target's Sources build
phase so the test class is compiled and run as part of the cmuxTests suite.
🪄 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
Run ID: 19e00c72-537e-49f9-ac89-9f561077a78a
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (21)
CLI/cmux.swiftResources/Localizable.xcstringsSources/App/AgentHibernationController.swiftSources/App/SurfaceHibernationPolicy.swiftSources/App/WorkspaceRuntimeSettings.swiftSources/AppDelegate.swiftSources/CmuxSettingsJSONPathSupport.swiftSources/CommandPalette/CommandPaletteSettingsToggle.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/TerminalPanel.swiftSources/SettingsNavigation.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/Workspace.swiftcmuxTests/AgentHibernationTests.swiftcmuxTests/SurfaceHibernationPolicyTests.swiftdocs/configuration.mdweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.json
CodeRabbit flagged the new schema fields as missing descriptionKey coverage. Follow the leaf-property convention: descriptionKey entries resolve through docs.configuration.schemaDescriptions in both message catalogs (en, ja), with the plain description kept as fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main crossed the 500-line tracking threshold for this file without a budget entry, which fails the guard on every PR merge ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address autoreview findings on the default-on behavior: - Busy now also covers background jobs hanging off the prompt shell (ghostty_surface_foreground_pid + child-process check) and terminals with listening ports, so freeing the PTY cannot SIGHUP silent background work that produces no prompt or output signal. - Surface-only ticks skip RestorableAgentSessionIndex's expensive disk/process scan; the index only loads when the opt-in agent mechanism is enabled. Restored agent panels stay recognized through in-memory snapshots and running agents are protected by the busy gates. - Settings > Terminal now has real Surface Hibernation rows (toggle, idle seconds, hidden-workspace seconds, max live surfaces) backed by new SettingCatalog keys, matching the search entry and docs; the hibernation controller also reconciles its timer on UserDefaults changes so the Settings window toggles take effect immediately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two autoreview correctness findings: a scrollback-free autosave could overwrite the session snapshot with nil — the hibernation state holds the only copy of a freed surface's content — and the keystroke throttle could suppress the input timestamp that the agent unconfirmed-input guard compares against lifecycle changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bernation # Conflicts: # .github/swift-file-length-budget.tsv
Autoreview: the captured directory travels with the panel and must win over workspace metadata, which can be missing after a cross-workspace move while hibernated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The once-per-process stale replay purge ran a recursive removeItem in a static-let initializer whose first caller sits on the main actor (session restore, the hibernation timer); with many leftover files that delete is unbounded main-thread I/O. The calling thread now pays a single O(1) rename that moves the stale tree aside, and the recursive delete runs in a detached utility task, which also sweeps bundle-scoped discard directories left by earlier crashes. Renaming before the first write keeps the delete from racing freshly written replay files, which a startup-task purge could not guarantee. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hooks Two autoreview findings on state-loss edges: - recordTerminalInput cleared an existing prompt-survival count whenever later text-bearing input carried zero survivals. Queued payloads make that real: "cmd\npartial" followed by "x" before cmd's preexec lands dropped the count, so cmd's own prompt return cleared the pending guard while "partialx" sat editable — hibernation could then drop it. Survival counts now accumulate and only shell transitions consume them; an overcount after ^C merely delays eviction by a few prompts. - runtimeSupportsScrollbackReplay was keyed off the shell basename before applyManagedShellSpecificStartupEnvironment decided whether to install anything. The helper declines silently (unreadable bundled bootstrap; a custom startup command displacing the fish wrapper), leaving the flag overreported: restore would stage a replay file no hook consumes. The flag now requires installation evidence — the zsh ZDOTDIR redirection, the bash PROMPT_COMMAND bootstrap, or the applied fish wrapper — via a pure helper covered by unit tests alongside regression tests for both behaviors in the policy suite. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inal An empty capture left any older restoredTerminalScrollbackByPanelId entry (e.g. session-restore seeding whose replay never ran) in place, so a scrollback-free autosave landing in the restore window could persist scrollback that no longer reflects the terminal, resurrecting it on the next session restore. The hibernate-time seeding now mirrors the hibernated-save branch: non-empty captures seed the fallback, empty captures remove it. Regression test included. The companion autoreview finding on the busy-scan fan-out is rejected with analysis recorded: non-live panels short-circuit at the nil surface handle, children-busy panels early-return after one proc_listchildpids call, and the expensive childless-shell path only applies to panels the planner is actively draining (bounded per tick until under cap), so the fan-out is self-extinguishing rather than unbounded. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pending Two autoreview findings on panels getting stuck hibernation-exempt: - keyDown recorded pending command-line input before the consumption branches (sidebar mode shortcut, find escape, keyboard copy mode) decided the event never reaches the shell. A consumed printable key (e.g. copy-mode "y") armed a guard no shell transition can clear and reset the idle clock for input the shell never saw. The recording now sits past the consumption branches, where every remaining path delivers the event to the terminal. - Surfaces created while tracking was off are seeded pending, but only a command's prompt transition cleared that, so shells already idle at an empty prompt when the user re-enables hibernation stayed exempt forever. Seeded entries (tracked separately from input-backed ones) now clear on a prompt redraw with no command since the seed — empty Enter or ^C, both of which leave the line empty. Observed input replaces the seed marker, so typed-ahead text keeps the strict command-only clearing. Regression tests cover both reconciliation directions. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Once a restore begins, the replay path moves from the hibernation state into the surface's staged environment, where close() no longer saw it: a panel closed before the relaunched shell consumed the file leaked it for the rest of the process (the bundle-scoped purge only reclaims it at the next launch). TerminalSurface now owns discarding the staged copy, panel close calls it, and the createSurface one-shot consumption deletes the file when this launch installed no replay hook (integration toggled off between staging and creation), since nothing will ever consume it. The companion finding asking restore to replay even when shell integration is disabled is rejected with the reasoning recorded: app-relaunch session restore has always staged replay unconditionally with the same no-hook outcome, restoring a blank panel until integration returns would be strictly worse, and force-installing a hook against the user's explicit setting (or wrapping the launch command) adds disproportionate failure modes for a bounded transition corner — only panels already hibernated at the moment of the toggle are affected, and the record-time gate stops new hibernations immediately. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rge damage Two autoreview findings: - The workspace-level shell-activity dedupe swallowed repeat promptIdle reports before the hibernation controller saw them, which is the common state for an already-idle shell: the empty Enter that should requalify a seeded panel re-reports the same state and was dropped, leaving those panels exempt indefinitely. Every report (a real precmd/preexec execution) now reaches the controller before the dedupe, which still guards the state write and the restored-agent state machine. The new regression test drives the workspace entry point, which the earlier controller-level test bypassed. - An earlier merge resolution had stomped main's browser CLI feature: `cmux browser devtools/react-grab/focus-mode/zoom/history` verbs, the v2 browser socket methods and input-helper JS in TerminalController, TabManager's explicit-surface React Grab routing, the browser-automation docs page, and a skills file. All are restored from main, with this branch's additions (surface-hibernation CLI command, socket command, panel-state hashing, mobile hibernation plumbing) re-applied on top; the branch diff for those files is additive again. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The record builder verified the foreground process (child scan + argv read syscalls) for every hidden shell past the idle gate, so the high-cardinality scenario this feature targets could fan hundreds of main-actor syscalls per 30s tick during the drain-down. Verification now runs as a bounded pass after the census: candidates that pass the cheap gates (confirm-close, listening ports) are sorted oldest-first — the planner's eviction order — and only the first maxForegroundVerificationsPerEvaluation (8x the per-tick drain) pay the syscalls; the rest stay conservatively busy until a later tick. Worst case is now ~32 small syscalls per tick at any panel count. If the whole verified window is genuinely busy, reclamation degrades to a no-op rather than freeing an unverified PTY; that residual is documented at the constant. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntrols Two autoreview findings on hibernation availability: - The bounded foreground-verification window sorted oldest-first with a fixed budget, so a busy prefix (e.g. many long-lived attached tmux clients) was re-verified every tick while everything behind it stayed conservatively busy forever. Verified-busy candidates now enter a 10-minute cache and are marked busy without consuming budget, so the per-tick budget flows to unverified candidates and the window provably advances past any busy prefix. The pass moved into the controller, which owns the cross-tick cache; the record builder returns the census plus candidates. - The pending-line classifier only excluded a small allowlist of controls, so non-inserting editing/navigation keys (^L, ^A, ^E, ^K, ^W, ^R, …) armed a guard that only a full command cycle clears. All C0 controls and DEL are now non-arming except the three that can insert text: tab (completion), ^V (quoted-insert), and ^Y (yank). The new non-arming set is a strict superset of the old one, so no previously guarded input becomes unguarded. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…idates Two autoreview findings on the requalification and verification paths: - The promptIdle-based seeded-pending clear was unreachable in production: the shell integrations dedupe repeat state reports at the source (_CMUX_SHELL_ACTIVITY_LAST), so an empty Enter at an already-idle prompt never arrives as a transition. Seeds now clear on the bare-Enter keystroke itself, threaded from keyDown and the socket send paths: after Enter the line either submits (runs as a command), opens a PS2 continuation (needsConfirmClose keeps the panel busy), or was empty. ^C deliberately does not clear — it can resurface input typed ahead of a still-running pre-tracking command — and the hazardous promptIdle-without-command branch is removed for the same reason. Input-backed pending keeps the strict command-cycle rule. - Planner-ineligible candidates could exhaust the per-tick verification budget every evaluation: panels with pending command-line input verify as allowed (never cached busy), and hibernated panels fail closed into the cache and churn it on TTL refresh. Pending panels are now skipped in the bounded pass and non-live panels never enter the candidate list, so the budget flows to candidates the planner could actually select. Part of #5731 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… hardening
Three fixes:
- The hibernate-time replay write (createDirectory + atomic write of up
to a full truncated scrollback) ran synchronously on the main actor
inside the 30s timer, up to 4 panels per tick. The timer path now
splits hibernation into capture (main), file write (detached utility
task via the new SessionScrollbackReplayStore.replayFilePath), and a
revalidated commit: any activity or input observed during the write
hop aborts the transition and discards the file. The synchronous
Workspace/TerminalPanel entry points remain for tests and compose the
same capture/commit primitives.
- CI caught terminalInputClearsSeededPending("\r\n") returning false:
"\r\n" is a single CRLF grapheme Character that matches neither "\r"
nor "\n". All three input classifiers now operate on unicode scalars
(with CRLF collapsed before survival counting), so CRLF payloads
classify like their LF equivalents instead of arming an unclearable
guard.
- An earlier merge resolution had also stomped main's control-socket
FD_CLOEXEC hardening in CmuxControlSocket; restored from main (this
branch never intended to touch that package).
Part of #5731
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7 tasks done
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #5731 (P0: surface hibernation + LRU cap)
Problem
The issue's live audit found 112 live Ghostty surfaces while one workspace was visible — ~3.3 GB of IOSurface across 850 regions, ~1 GB of scrollback malloc, and four threads per surface. Unmounting a workspace only hides portal views (
hideAllTerminalPortalViews);ghostty_surface_freeruns solely on panel close. The only reclaim path that exists today — agent hibernation — covers restorable-agent panels only, is opt-in (default off), and its cap counts only those panels. Plain-shell surfaces are immortal, and surfaces force-created by bypass paths (background workspace priming, queued socket input on surface-less panels) never create eviction pressure.What this adds
One pure selection policy,
SurfaceHibernationPlanner, over all terminal panels. Three rules compose (union):AgentHibernationPlanneris replaced by this rule; its tests were migrated 1:1).maxLiveSurfaces, default 12) — the census counts every live surface, including exempt ones and bypass-path materializations, so they still create pressure; only idle, non-busy, non-visible, mechanism-capable surfaces are evicted, oldest first.unmountedIdleSeconds(default 30 min) hibernate even without cap pressure.A
shellRestarthibernation mechanism for plain shells: capture scrollback (session-persistence truncation policy) and the panel's working directory, free the runtime surface (ghostty_surface_free→ Metal/IOSurface buffers, the four surface threads, PTY), and on the next visit start a fresh shell in the captured directory with the scrollback replayed through the existingCMUX_RESTORE_SCROLLBACK_FILEplumbing.Restore triggers mirror agent-hibernation auto-resume: workspace selection / visibility reconcile, panel focus, focus-intent restore, and every input path (keyboard, socket, CLI — including the two observer-wait paths that would otherwise wedge on a panel whose surface can never materialize while suspended).
Safety exemptions (all planner-level and unit-tested): busy surfaces (
needs_confirm_quit, i.e. not safely at a prompt — conservatively true when shell integration is absent), visible panels of the visible workspace, unconfirmed terminal input, deferred startup work (initial command, tmux attach, initial input, queued input bytes), remote terminals, and agent panels (which hibernate only through their own opt-in agent mechanism). On top of eligibility, the existing 60-second output-stability confirmation window must pass before anything is reclaimed.Settings (default ON) —
terminal.surfaceHibernation.{enabled, idleSeconds, unmountedIdleSeconds, maxLiveSurfaces}in cmux.json, settings search row, command-palette toggle,cmux surface-hibernation <on|off>CLI,surface_hibernationsocket command, JSON schema, and docs. Defaulting on is the point of the P0: it only ever reclaims surfaces that are off-screen, at a prompt, quiet, and output-stable, and the visible state (scrollback + cwd) is restored on return.Persistence — the autosave fingerprint tracks hibernation transitions, and the session snapshot uses the captured scrollback for hibernated panels, so after an app relaunch they restore as normal (surface-less) panels with replay staged.
Red/green structure
SurfaceHibernationPolicyTestsagainst a planner scaffold that faithfully reproduces the shipped agent-only policy — the new policy tests fail (CI red), proving the gap.Commit 1 also adds a dedicated
Run surface hibernation policy regressionCI step (-only-testing:cmuxTests/SurfaceHibernationPolicyTests, mirroring the existing split-theme step). This was forced by a pre-existing hole in thetestsjob discovered while proving the red: the full unit pass intermittently crashes insideBrowserDeveloperToolsVisibilityPersistenceTests(Signal 11), every suite that sorts after the crash is skipped, and the job's expected-failure classifier then reports "All failures are expected, treating as pass" — so new suites late in the alphabet can silently never execute (two consecutive runs reproduced this). The dedicated step makes the policy tests a real gate regardless. The crash/soft-pass hole itself is worth a separate issue — it affects every suite fromCtoZon crashy runs, not just this one.Unit-tested vs. verified by inspection
ghostty_surface_free;suspendRuntimeSurfaceForHibernationreaches it on the next main-actor turn — the same already-shipping path agent hibernation uses. End-to-end memory reclamation was not measured in this PR.Localization audit
settings.terminal.surfaceHibernation— added toResources/Localizable.xcstringswith en + ja translations; the palette toggle and settings row reuse it, and enable/disable wrappers use existing localized formats.docs/configuration.md, and the web docs example follow theagentHibernationprecedent exactly (plaindescriptionstrings; that section has no entries inweb/messages/en.json/ja.jsonto mirror).agent-hibernationlikewise).Not in this PR
The third P0 item (stop mass agent auto-resume at launch) is a separate workstream item of #5731.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Settings
CLI & UI
Behavior
Docs & Tests