Skip to content

Keep agent lifecycle through display-level status clears - #9303

Closed
azooz2003-bit wants to merge 9 commits into
mainfrom
issue-9295-status-clear-lifecycle
Closed

azooz2003-bit wants to merge 9 commits into
mainfrom
issue-9295-status-clear-lifecycle

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 31, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #9295.

clear_status removed the visible entry, then called clearAgentPID with the bare status key. Workspace and Dock owners consequently removed lifecycle state even when that key did not own the structured agent PID. A live Dock-owned agent could lose its spinner, including when a stale Dock target followed the panel into a workspace.

The shared status-clear route now requires PID-key ownership before performing PID and lifecycle teardown. The status entry still clears, structured PID and lifecycle state survive, and explicit clear_agent_pid plus hibernation teardown keep their existing cleanup behavior. The implementation changes one operation boundary and leaves both owner implementations unchanged.

Regression coverage proves the Dock-owned lifecycle survives the first display clear, removes the old fake lifecycle re-report, then proves a stale-owner clear follows a Dock-to-workspace transfer without removing PID or lifecycle state.

Evidence:

Both 34-test hibernation runs pass the explicit PID teardown regressions and fail only the same unrelated testResumePreparationWithoutStartupInputStillLeavesHibernation expectation. The independent restoreCLIArgument compile repair is on main through #9334.

#9293 proposes the opposite product contract, where clearing a status also clears lifecycle state. This PR follows the existing regression expectation that display status and process lifecycle are separate.

The tests job has been red on main since #9091: "Live PID routing and
runtime mutations include a Dock-owned terminal" expects an agent's
lifecycle to survive a bare clear_status, but
controlSidebarScheduleStatusClear also runs clearAgentPID, and both
owners dropped agentLifecycleStates[statusKey] there even when the key
never recorded any PID state. A display-key clear therefore made a
live agent hibernatable. The failure was masked until #9279 because
workflow-guard-tests short-circuited the tests job for every PR.

Gate the lifecycle removal on actual PID ownership: the workspace side
clears lifecycle only for a key registered in agentPIDPanelIdsByKey,
and the Dock side only when the clear actually removed PID state.
Owned-key teardown (agent PID clears, hibernation entry, stale
structured-hook keys) keeps clearing lifecycle, and explicit clears
still go through clearAgentLifecycle.

Fixes #9295

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The control-sidebar status clear now enforces agent PID ownership. The Dock-owned terminal mutation test preserves the running lifecycle state and verifies that queued status clearing follows a moved panel without clearing its PID or lifecycle state.

Changes

Agent status cleanup

Layer / File(s) Summary
Status clearing and lifecycle transfer
Sources/TerminalController+ControlSidebarContext.swift, cmuxTests/AgentNotificationMutationBoundaryTests.swift
Status clearing now requires ownership of the agent PID key. The Dock-owned terminal test retains the .running lifecycle state and verifies that a queued clear removes status after panel movement while retaining the PID and lifecycle state.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • manaflow-ai/cmux-dev-artifacts#7407 — Covers Dock-owned agent status clearing with PID and lifecycle preservation during workspace movement.
  • manaflow-ai/cmux-dev-artifacts#7438 — Covers Dock-owned agent lifecycle and status mutations across ownership changes.
  • manaflow-ai/cmux-dev-artifacts#7381 — Covers the affected Dock-owned terminal mutation-boundary test.

Possibly related PRs

Suggested reviewers: austinywang, lawrencecchen

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy [#9295] by preserving lifecycle state for display-only clears while retaining cleanup for actual PID ownership and explicit teardown.
Out of Scope Changes check ✅ Passed The modified source and test files directly support the linked issue and PR objectives; no unrelated changes are identified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed The production diff only adds requireOwnedKey inside an existing @MainActor mutation closure; it adds no models, protocols, Sendable types, or background UI access.
Cmux Swift Blocking Runtime ✅ Passed The PR diff adds no blocking or timing primitive; production changes only pass requireOwnedKey, and test changes use queue draining and assertions.
Cmux Browser Automation Off-Main ✅ Passed The full branch diff changes only sidebar lifecycle cleanup and its tests; it contains no browser.* command, WebKit wait, worker-router, policy, or AppKit/WebKit threading change.
Cmux Expensive Synchronous Load ✅ Passed The diff only changes in-memory PID/lifecycle mutation and adds no synchronous history, file, JSON, directory, or syscall load to an interactive path.
Cmux Cache Substitution Correctness ✅ Passed The PR diff only adds requireOwnedKey to clearAgentPID and updates tests; it does not replace an authoritative read with a cache in a persistence, history, undo, or snapshot path.
Cmux No Hacky Sleeps ✅ Passed The PR changes only Swift production and test files; it adds no TypeScript, JavaScript, shell, or build/runtime-script delays covered by this rule.
Cmux Algorithmic Complexity ✅ Passed The PR production diff only adds requireOwnedKey to one clearAgentPID call; it introduces no loops, collection scans, sorting, joins, or batch rescans. Other changes are test-only.
Cmux Swift Concurrency ✅ Passed The PR adds only a requireOwnedKey argument in production and existing queued test assertions; no new Dispatch, Combine, completion-handler, or fire-and-forget Task patterns appear.
Cmux Swift @Concurrent ✅ Passed The patch changes only synchronous PID/lifecycle cleanup and an existing nonisolated scheduler; no async or @concurrent annotations changed, and mutations remain inside an explicit @MainActor enqueue.
Cmux Swift Package Boundaries ✅ Passed The diff only narrows existing Workspace/Dock/TerminalController lifecycle cleanup and depends on panel state and AgentHibernationController.shared; it adds no standalone reusable domain feature.
Cmux Swiftpm Lockfiles ✅ Passed The commit changes only three Swift source files and includes no Package.swift, Package.resolved, Xcode project, .gitignore, workflow, or dependency changes.
Cmux Swift Logging ✅ Passed The net PR diff changes status-clear ownership and tests only; added-line scans found no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data logging.
Cmux User-Facing Error Privacy ✅ Passed The net PR diff adds only PID ownership logic in production plus test changes; it adds no user-facing errors, alerts, command output, API bodies, or recovery copy.
Cmux Full Internationalization ✅ Passed The PR changes only PID/lifecycle control flow and test assertions; it adds no user-facing text, localization keys, catalogs, or web locale data.
Cmux Swiftui State Layout ✅ Passed The diff changes only Swift model/controller extensions; it adds no SwiftUI views, ObservableObject/@published state, geometry readers, lazy rows, or render-time state writes.
Cmux Architecture Rethink ✅ Passed The branch makes a small ownership-gated fix through the existing panel-owner path, adds no timing, lock, observer, side channel, or duplicate wiring, and extends regression coverage.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR diff only changes status/lifecycle mutation logic and test assertions; it adds or materially changes no NSWindow, NSPanel, NSWindowController, Window, or WindowGroup code.
Cmux Source Artifacts ✅ Passed The commit changes only three hand-written Swift source files under Sources; no logs, caches, build output, scratch directories, or other prohibited artifacts appear.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR changes only lifecycle-clearing logic in three production Swift files; added lines contain no test/debug guard, seam-named member, accessor, or visibility widening.
Cmux No Ambient Global State ✅ Passed The Swift diff changes existing method bodies and a call argument only; it adds no top-level API, mutable global, namespace type, or singleton state.
Title check ✅ Passed The title clearly describes the main change: preserving agent lifecycle state during display-level status clears.
Description check ✅ Passed The description clearly explains the fix, rationale, scope, regression coverage, and test evidence, despite omitting some template headings and checklist items.
✨ 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-9295-status-clear-lifecycle

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.

@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/DockSplitStore`+RestoredAgentLifecycle.swift:
- Around line 247-256: Preserve status-scoped lifecycle state until its final
owner is removed: in Sources/DockSplitStore+RestoredAgentLifecycle.swift lines
247-256, remove lifecycle state only when no remaining runtime.agentPIDKeys
entry maps to statusKey; in Sources/Workspace+PanelLifecycle.swift lines
324-330, after removeAgentPIDOwnership, apply the equivalent check against
agentPIDKeysByPanelId[ownedPanelId] and lifecycleStatusKey. Add behavior-level
coverage that clears one of two same-status owners and verifies lifecycle state
remains, then clears the final owner and verifies it is removed.
🪄 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: 6d3290d8-a694-4016-8091-40de7ecaa5e0

📥 Commits

Reviewing files that changed from the base of the PR and between 08164fd and 6a0c48b.

📒 Files selected for processing (2)
  • Sources/DockSplitStore+RestoredAgentLifecycle.swift
  • Sources/Workspace+PanelLifecycle.swift

Comment thread Sources/DockSplitStore+RestoredAgentLifecycle.swift Outdated
Keep clearAgentPID cleanup semantics unchanged for explicit teardown, including missing PID records. The display clear path now opts into requireOwnedKey so it cannot erase lifecycle owned by a different structured PID key.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 2 files

Re-trigger cubic

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

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

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

app-host unit tests red on main: 'Live PID routing and runtime mutations include a Dock-owned terminal' drops agent lifecycle on status clear

3 participants