Skip to content

Stop restore retries from typing into an already-running agent - #12654

Closed
benstpierre wants to merge 4 commits into
manaflow-ai:mainfrom
benstpierre:fix/restore-retry-ownership
Closed

benstpierre wants to merge 4 commits into
manaflow-ai:mainfrom
benstpierre:fix/restore-retry-ownership

Conversation

@benstpierre

@benstpierre benstpierre commented Sep 14, 2026 •

Copy link
Copy Markdown

Fixes #12653

A restored agent can start while Bash 3.2 leaves cmux's cached shell state at promptIdle. The delayed retry then types a second restore selector into the running agent. Check existing matching-agent process evidence at the retry boundary, retire the retained input when the agent is alive, and mark the restore running. Both Workspace and Dock pass this evidence; an idle shell without live-agent proof keeps its one-shot retry for lost typeahead.

Validation: the regression is committed before the fix. Three of six test functions fail with 13 assertions before; all six functions / nine expanded cases pass after. Both full tagged app builds succeed. With an identical synthetic restore fixture, the native before app delivers the duplicate command and the native after app delivers none; the app confirms matching-process liveness in both. Xcode project normalization, test wiring, package lockfile policy, and whitespace checks pass.

The native fixture and screenshots are review evidence, separate from the production change. Native runtime verification covers Workspace delivery; Dock shares the gate and compiles. Missing process evidence and full persisted-session/Codex UI restoration remain outside the proven scope. No new user-facing strings or localization changes.

Related earlier symptom report: #9831. That report predates this retry; do not mark it fixed solely on this evidence.

Reproducible evidence

Before — the compiled app sends the duplicate selector into the synthetic agent:

Native build before fix

After — the compiled app retires the selector and the agent receives no duplicate input:

Native build after fix


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

Fixes #12653. Stops restore retries from typing a second restore selector into an already-running agent when Bash 3.2 leaves the cached shell state at promptIdle.

  • Matching live-agent process evidence now retires the retained input and marks the restore running.
  • Workspace and Dock both pass this evidence; an idle shell without live-agent proof still gets exactly one retry.
  • Adds RestoredStartupInputOwnershipTests covering live, transferred, abandoned, and retry-retention cases.

Written for commit a78f53e. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved restored session startup handling when a live agent is already running.
    • Prevented duplicate startup input resends and correctly retires pending retries once agent activity is detected.
    • Preserved independent startup retry behavior across panels and transferred sessions.
  • Tests

    • Added coverage for startup input ownership, slow shell responses, live-agent detection, abandoned restores, and panel-specific retry behavior.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5182748a-432f-4ae0-b725-538d9a54ba20

📥 Commits

Reviewing files that changed from the base of the PR and between 211b8bb and a78f53e.

📒 Files selected for processing (5)
  • Sources/DockSplitStore+RestoredAgentLifecycle.swift
  • Sources/RestoredAgentLifecycleCoordinator.swift
  • Sources/Workspace+AgentLifecycle.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/RestoredStartupInputOwnershipTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The restore retry now checks for a live agent before replaying retained startup input. Live-agent evidence clears the retry and marks the restore as running. New tests cover resend ownership and related panel and restore states.

Changes

Restore retry ownership

Layer / File(s) Summary
Live-agent retry gate
Sources/RestoredAgentLifecycleCoordinator.swift
takeStartupInputForResend clears pending input and sets .autoResumeCommandRunning when a live agent exists. Without live evidence, the existing prompt-idle requirement remains.
Live-agent evidence wiring
Sources/Workspace+AgentLifecycle.swift, Sources/DockSplitStore+RestoredAgentLifecycle.swift
Workspace and Dock derive live-agent state and pass it to the retry coordinator.
Retry ownership regression coverage
cmuxTests/RestoredStartupInputOwnershipTests.swift, cmux.xcodeproj/project.pbxproj
Adds tests for live ownership, one-shot resend, retained input, panel isolation, abandoned restores, and transferred retries. Registers the test file in the Xcode project.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

Merge Risk: ⚪ Minimal · up to a78f5

The restore retry change preserves matching live-agent ownership, including Dock binding-only restores, so no merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: preventing restore retries from sending input to an already-running agent.
Description check ✅ Passed The description provides a detailed summary, rationale, testing results, reproducible evidence, scope limitations, and regression coverage. It does not include the template's demo video, review trigge…
Linked Issues check ✅ Passed PR #12654 meets the coding requirements in issue #12653. Workspace and Dock pass matching-agent liveness evidence to takeStartupInputForResend. The coordinator clears retained startup input and sets…
Out of Scope Changes check ✅ Passed The reported changes stay within issue #12653. The source changes implement retry ownership and liveness handling. The added tests verify that behavior. The project-file changes register the tests. Th…
Cmux Swift Actor Isolation ✅ Passed The production diff does not introduce an actor-isolation mistake covered by the rule. RestoredAgentLifecycleCoordinator and DockSplitStore are already @MainActor. The new Workspace and Dock liv…
Cmux Swift Blocking Runtime ✅ Passed The production diff adds only live-agent evidence and state handling. It does not add semaphores, blocking waits, sleeps, polling, locks, synchronous main-queue calls, timers, or delayed dispatch. The…
Cmux Browser Automation Off-Main ✅ Passed PASS. The PR changes restored-agent startup retry ownership only. The authoritative diff contains no browser.* command, WebKit wait, cookie callback, screenshot callback, injected page hook, socket-wo…
Cmux Expensive Synchronous Load ✅ Passed PASS. The PR adds no synchronous agent-history loader, file read, JSON decode, directory scan, or transcript/session-store load. The new retry-boundary checks call the existing liveness helpers on the…
Cmux Cache Substitution Correctness ✅ Passed The diff does not replace a fresh authoritative read in a persistence, history, undo, or snapshot path. It adds an in-memory lifecycle snapshot lookup to supply agent identity and adds positive livene…
Cmux No Hacky Sleeps ✅ Passed PASS. The authoritative diff changes only Swift source/tests and the Xcode project file. It introduces no TypeScript, JavaScript, shell, or non-Swift build/runtime script changes. The runtime-no-hacky…
Cmux Algorithmic Complexity ✅ Passed PASS. The production diff adds only keyed panel lookups, one optional snapshot check, and bounded liveness evaluation. The liveness index resolves one panel and its candidate list is explicitly capped…
Cmux Swift Concurrency ✅ Passed PASS. The diff adds no prohibited legacy async pattern. The existing DispatchQueue.main.asyncAfter and Task { @mainactor ... } blocks remain structurally unchanged; the PR only adds liveness compu…
Cmux Swift @Concurrent ✅ Passed The diff introduces no async, nonisolated async, or @concurrent declarations. The changed retry work remains in existing UI-bound closures with an explicit Task { @mainactor ... } hop. `Restor…
Cmux Swift Package Boundaries ✅ Passed PASS: The diff does not introduce a new reusable cross-surface domain module. It adds a liveness gate to the existing @MainActor RestoredAgentLifecycleCoordinator, which is app-lifecycle state tie…
Cmux Swiftpm Lockfiles ✅ Passed The PR changes no Package.swift, .gitignore, workflow, dependency, or Package.resolved files. Its only Xcode project changes register RestoredStartupInputOwnershipTests.swift; no package-refer…
Cmux Swift Logging ✅ Passed The pull request adds no logging statements or Logger declarations. The production additions only pass and evaluate hasLiveAgent, update restore state, and clear retained input. The new test file co…
Cmux User-Facing Error Privacy ✅ Passed PASS: The reviewed production diff only changes restore-retry state handling and liveness checks. It adds no user-facing error, alert, command-output text, API error body, or recovery copy, and it int…
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes restore lifecycle state and adds test wiring. Production Swift adds no user-facing text; its additions are arguments, control flow, and developer comments. The onl…
Cmux Swiftui State Layout ✅ Passed PASS. The pull request changes restored-agent lifecycle logic and adds coordinator tests. The diff adds no SwiftUI view, ObservableObject/@Published/@StateObject/@EnvironmentObject state, GeometryRead…
Cmux Architecture Rethink ✅ Passed PASS: The diff adds no new sleep, polling, lock, observer, or delayed-dispatch mechanism. The existing asyncAfter retry remains unchanged. RestoredAgentLifecycleCoordinator remains the single owne…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The PR changes restored-agent lifecycle logic, project wiring, and a test-only coordinator suite. The authoritative diff adds no NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowG…
Cmux Source Artifacts ✅ Passed The diff changes only three hand-written Swift source files, one hand-written Swift test, and the Xcode project file. The new test is a deliberate regression test and the project changes register it a…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The production diff adds no test-build guard, debug/test-named member, visibility widening, or test-only accessor. takeStartupInputForResend remains an internal lifecycle operation with produc…
Cmux No Ambient Global State ✅ Passed PASS. The production diff adds only instance-method behavior and a method parameter. The changed methods are enclosed in DockSplitStore/Workspace extensions or RestoredAgentLifecycleCoordinator.…
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thank you for this, @benstpierre! You were on this first: a week later #13891 removed the whole startup-input resend path, so restore retries can't type into a running agent anymore and this patch has nothing left to guard. Really appreciate you chasing it :)

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.

Restore retry sends a second selector into an already-running agent on Bash 3.2

2 participants