Skip to content

Add shell-free cmux restore verb - #9265

Merged
austinywang merged 31 commits into
mainfrom
issue-9258-cmux-restore-verb
Aug 1, 2026
Merged

austinywang merged 31 commits into
mainfrom
issue-9258-cmux-restore-verb

Conversation

@austinywang

@austinywang austinywang commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add a first-class cmux restore <kind> <checkpoint-id> / cmux restore --surface [id|ref] command alongside the existing restore-session and vm restore commands
  • persist launch argv, executable, cwd, environment, source, and permission mode additively, retrieve them over surface.resume.get, and keep them structured through direct execve
  • move Codex update-check suppression, Claude permission mode, managed wrapper routing, restore authorization, and Hermes preflights into typed restore planning
  • replace local restore startup payloads with one short readable CLI line, removing the local 900-byte split and local restore temp-script path

Architecture

The local restore path no longer reparses or rewrites a serialized command. The app types the short, readable cmux restore … command; the CLI reads a structured surface restore record, applies the guarded cwd policy, merges the persisted environment, resolves the managed wrapper/executable, runs typed provider preflights where needed, and replaces itself with the target process via execve. restore --surface accepts an optional id/ref; with no argument it resolves the current surface from CMUX_SURFACE_ID or, when shell profiles strip that variable, from the caller PTY/terminal index.

CMUX_AGENT_RESTORE_LAUNCH=<provider>:<session> is part of the child environment. Codex update suppression and Claude permission mode are argv construction, not shell-string splicing. There is no /bin/sh -c, login-shell hop, quoting round trip, byte-budget branch, or launcher file in the new local structured path.

The remaining shell-backed code has two explicit boundaries:

  1. Legacy persisted records: bindings written by older builds may have only command. The additive decoder preserves those records and cmux restore runs their existing canonicalized command through the user's configured shell (falling back to the account shell, then /bin/sh). New records never enter this fallback.
  2. Remote persistent SSH surfaces: the local CLI cannot execve a process on the remote host, so remote restore deliberately retains the existing serialized remote command transport in this PR. Structured fields are preserved across remote binding transformations, and existing remote/Hermes compatibility tests remain in place.

Fork-conversation and unrelated generic terminal startup launchers continue to use their existing one-shot transport; they are not session auto-restore and are outside this verb.

Compatibility and related issues

Tests

  • structured Codex and Claude resume argv/environment/cwd planning
  • update-check suppression, wrapper authorization, Claude permission mode, and Hermes typed preflights
  • direct binding argv/env with spaces, quotes, CJK, literal cwd-looking flags, and >900-byte payloads
  • additive structured binding Codable and socket set/get transport
  • command-only legacy fallback
  • bundled CLI subprocess coverage proving direct execve behavior and compatibility-shell behavior
  • local short startup input, unsafe identifier surface fallback, and explicit remote compatibility behavior
  • initial tagged Debug build (issue-9258-cmux-restore-verb) before PR iteration; GitHub CI validates the current review-fix HEAD
  • Swift warning budget, pbxproj normalization/check, test wiring lint, Swift parse, and git diff --check

Localization audit: changed restore help/error surfaces use the existing CLI localization convention. Every touched restore key has English and Japanese entries in Resources/Localizable.xcstrings, and the catalog parses successfully.

Closes #9258


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

Adds a first-class cmux restore <kind> <checkpoint-id> command to restore processes from structured session data without a shell. Local startup is now one short, readable CLI line via the bundled CLI, with typed cwd fallback, hardened validation, and a sanitized environment.

  • New Features

    • New restore CLI verb with help, suggestions, and --surface <id|ref>; documented in docs/cli-contract.md.
    • Persist and transport structured launch data via AgentLaunchCommand and socket ControlSurfaceRestoreRecord; add AgentRestoreRequestMode, persist permissionMode, and plan direct execve with AgentRestorePlanner and bounded provider preflights.
    • Route local restores through the bundled CLI; keep remote SSH restores on the existing serialized transport while preserving structured fields.
  • Bug Fixes

    • Stricter socket validation for launch_command and clearer localized CLI restore errors; add private diagnostics logging while returning product-safe messages.
    • Prefer live resume binding identity and apply typed cwd fallback consistently; keep startup input readable and ASCII-only.
    • Sanitize restore transport environment with selectedRestoreEnvironment (lowercase kind, retain PATH for pi/omp, ignore empty components); run preflights via posix_spawn.
    • Harden restore startup dispatch and bind the socket earlier so restored terminals can execute the short cmux restore command immediately.

Closes #9258

Written for commit 4d75d3a. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added the cmux restore command for resuming saved agent sessions.
    • Supports surface selection, structured launch details, working directories, environments, permissions, and provider setup.
    • Includes preflight checks and compatibility with legacy shell-based restore records.
  • Bug Fixes

    • Improved validation and error handling for invalid or incomplete restore data.
    • Reduced reliance on temporary launcher scripts during restoration.
  • Documentation

    • Documented cmux restore syntax and help behavior.
  • Tests

    • Added coverage for structured, legacy, local, remote, and provider-specific restores.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds the cmux restore command. It persists structured launch metadata, plans direct process execution, supports legacy shell records, exposes restore data through surface resume APIs, and removes launcher-script restore paths.

Changes

Structured restore flow

Layer / File(s) Summary
Restore contracts and models
Packages/macOS/CMUXAgentLaunch/..., Packages/macOS/CmuxControlSocket/.../Surface/Control*.swift
Adds typed launch, restore request, invocation, and surface restore-record models.
Restore planning and authorization
Packages/macOS/CMUXAgentLaunch/.../AgentRestore*.swift
Builds direct invocations, restores environments, routes managed agents, and creates provider preflights.
Persistence and workspace integration
Sources/SessionPersistence.swift, Sources/ControlSurfaceResumeTarget.swift, Sources/Workspace.swift, Sources/RestorableAgentSession.swift, Packages/macOS/CmuxWorkspaces/...
Persists launch metadata and permission modes. Generates compact restore commands. Removes launcher-script and temporary-directory restore parameters.
CLI restore execution
CLI/CMUXCLI+Restore.swift, CLI/CMUXCLI+RestorePreflight.swift, CLI/cmux.swift
Adds restore selection, record validation, working-directory handling, preflight execution, direct execve, legacy shell fallback, and deferred socket startup.
Validation and compatibility coverage
cmuxTests/*, Packages/macOS/*/Tests/*, docs/cli-contract.md, scripts/stress-cli-socket-api.py
Tests structured transport, provider handling, legacy records, path retargeting, socket readiness, CLI errors, and compact startup input.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CMUXCLI
  participant ControlSocket
  participant AgentRestorePlanner
  participant RestoredProcess
  User->>CMUXCLI: cmux restore kind checkpoint
  CMUXCLI->>ControlSocket: fetch persisted restore record
  ControlSocket-->>CMUXCLI: return structured launch data
  CMUXCLI->>AgentRestorePlanner: create restore invocation
  AgentRestorePlanner-->>CMUXCLI: return argv, cwd, environment, and preflights
  CMUXCLI->>RestoredProcess: run preflights and execve target
Loading

Possibly related PRs

Suggested reviewers: azooz2003-bit, lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (6 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Swift Blocking Runtime ❌ Error CLI/CMUXCLI+RestorePreflight.swift adds production kevent waits with 10-second and grace-period deadlines, plus blocking waitpid reaping. Replace the synchronous timed wait/retry path with a non-blocking process-exit completion signal and cancellation-aware timeout handling.
Cmux Cache Substitution Correctness ❌ Error New restore_record uses restoredResumeSessionWorkingDirectoriesByPanelId before the current binding cwd, but setSurfaceResumeBinding does not invalidate that cache; stale cwd data can reach CLI res... Clear or identity-key restoredResumeSessionWorkingDirectoriesByPanelId when bindings change, or validate its session/generation before using it and fall back to the current binding cwd.
Cmux Swiftpm Lockfiles ❌ Error cmux.xcodeproj adds the CmuxSentryTelemetry SwiftPM reference and products, but its root Xcode Package.resolved has no diff in HEAD^..HEAD. Update cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved with the resolved Xcode dependency changes and include it in the PR.
Cmux User-Facing Error Privacy ❌ Error Restore CLI errors expose persisted kind/mode, executable argv, and cwd paths; the socket error exposes launch_command.arguments. CLIError prints these directly to stderr. Use generic restore errors without persisted provider data, argv, paths, or payload field names. Keep detailed values in sanitized internal logs only.
Cmux Full Internationalization ❌ Error Resources/Localizable.xcstrings adds 25 restore/socket user-facing keys with only en and ja, but the catalog already supports 20 locales; 18 locale entries are missing for every key. Add translated entries for ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant to all 25 new keys in Resources/Localizable.xcstrings.
Cmux Architecture Rethink ❌ Error Implicit restore adds a socket-startup race path: it calls a 10-second filesystem observer with queue.async and semaphore.wait in waitForConnectableSocket. Make app bootstrap own socket readiness and expose one lifecycle-backed restore handshake; remove the CLI watcher/semaphore path and test that readiness transition.
Docstring Coverage ⚠️ Warning Docstring coverage is 9.77% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (18 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation satisfies #9258 by adding structured restore, direct local execution, legacy fallback, provider handling, and remote compatibility.
Out of Scope Changes check ✅ Passed The code, tests, documentation, localization, and remote compatibility updates support the linked restore objectives without unrelated changes.
Cmux Swift Actor Isolation ✅ Passed Restore changes add Sendable value models and pure planners; UI access remains in @MainActor ControlSurfaceResumeTarget/TerminalController, with no new shared mutable Sendable classes or async serv...
Cmux Browser Automation Off-Main ✅ Passed The PR diff has no browser/WebKit automation hunks and leaves TerminalController, the worker policy, router, and policy tests unchanged; it does not worsen existing browser debt.
Cmux Expensive Synchronous Load ✅ Passed The PR adds no production calls to RestorableAgentSessionIndex.load or large history/transcript/JSONL scans; restore socket work uses focused in-memory structured records, while existing cached fal...
Cmux No Hacky Sleeps ✅ Passed The only non-Swift code change is the stress-test harness; it adds restore and restore --help cases, with no new sleep, timer, polling, or delay code.
Cmux Algorithmic Complexity ✅ Passed Restore code uses linear scans for one argv/environment/PATH and fixed-size lists; review found no nested scalable-collection scans, per-target rescans, or batch joins over workspaces or sessions.
Cmux Swift Concurrency ✅ Passed Added production Swift has no new background Dispatch, Combine state, completion-handler, or fire-and-forget Task pattern; restore preflight uses synchronous POSIX/kqueue/waitpid APIs.
Cmux Swift @Concurrent ✅ Passed The PR diff adds no async or @concurrent functions. Restore work is synchronous CLI/planner code, and added nonisolated helpers perform synchronous value mapping only.
Cmux Swift Package Boundaries ✅ Passed Restore models/planning are isolated in CMUXAgentLaunch with package tests, and socket records are in CmuxControlSocket; app and CLI changes remain composition plus CLI/POSIX process glue.
Cmux Swift Logging ✅ Passed Restore production Swift adds no print, debugPrint, dump, NSLog, Logger, or ad hoc diagnostic writes; CLIError output is user-facing, and the existing #if DEBUG file logger is unchanged.
Cmux Swiftui State Layout ✅ Passed The main..HEAD diff adds no SwiftUI views, state wrappers, GeometryReader, lazy/list rows, or render-time state writes; TabManager and Workspace only receive incidental restore/hash/API edits.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The restore PR adds no standalone window code or identifier assignments; changed NSWindow uses are existing/test-only, and scripts/lint_auxiliary_window_close_shortcuts.py passes.
Cmux Source Artifacts ✅ Passed The 51 PR paths are source, tests, localization, Xcode config, docs, or a script; no artifact directories, binary files, logs, caches, temp folders, or build output were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR adds no DEBUG/TESTING guards or test/debug-named members in changed production Sources; restore APIs have product callers in the CLI and application.
Cmux No Ambient Global State ✅ Passed Restore functions are methods on CMUXCLI extensions; new production types are constructable value types, with only allowed static constants and no new globals or singletons.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a shell-free cmux restore command.
Description check ✅ Passed The description explains the changes, rationale, architecture, compatibility, testing, and linked issue, but omits several template sections.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9258-cmux-restore-verb

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.

@cursor

cursor Bot commented Jul 31, 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.

@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: 17

🤖 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 `@CLI/CMUXCLI`+Restore.swift:
- Around line 340-360: Update restoreCompatibilityShell at
CLI/CMUXCLI+Restore.swift#L324-L338 and resolveRestoreExecutable at
CLI/CMUXCLI+Restore.swift#L340-L360 so every candidate is accepted only when it
is not a directory and is executable; apply this to SHELL, pw_shell, explicit
executable paths, and PATH candidates, continuing the PATH scan for directory
candidates. Use a shared helper if appropriate to keep both resolver flows
consistent.
- Around line 79-110: Restructure restore flow around
AgentRestorePlanner().invocation so structured planning is attempted first and,
when it returns nil, a usable record.legacyCommand is executed through
execLegacyRestoreCommand with the existing working-directory and environment
handling. Do not gate the legacy fallback on launchCommand or preparedArguments
being nil, so empty arrays and sanitization failures also fall back correctly;
otherwise preserve the existing unsupported-mode and incomplete-data errors when
no legacy command is available.

In `@cmuxTests/AgentResumeReturnShellStartupTests.swift`:
- Around line 63-68: Remove the vacuous directory-emptiness assertion from the
test, along with the now-unused root temporary-directory setup and defer. Update
the test signature to remove throws if no remaining operation can throw; do not
add injection unless the test is changed to exercise the component that writes
launcher files.

In `@cmuxTests/CMUXCLIErrorOutputRegressionTests.swift`:
- Around line 133-136: Strip every inherited environment key whose name begins
with CMUX_ before setting test-specific variables in both restore-test setup
sites at cmuxTests/CMUXCLIErrorOutputRegressionTests.swift lines 133-136 and
176-180; retain the existing assignments for CMUX_CLI_SENTRY_DISABLED,
CMUX_SOCKET_PATH, CMUX_SURFACE_ID, and SHELL.
- Line 152: Update the fake executable used by the CMUXCLIErrorOutput regression
test to print its argv[0], then replace the ineffective “no shell hop” stdout
check with an assertion that the printed argv[0] equals the expected executable
path. Keep the existing command-output assertions unchanged.

In `@docs/cli-contract.md`:
- Line 557: Update the cmux restore command entry in docs/cli-contract.md to
document the accepted --surface <id|ref> selector form alongside the existing
<kind> <checkpoint-id> usage, without changing the documented restore behavior.

In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreInvocation.swift`:
- Around line 2-13: Update AgentRestorePreflightInvocation so an empty arguments
array cannot be constructed, preferably by making its initializer failable and
rejecting empty input; preserve the existing arguments and environment storage
for valid invocations. Ensure runRestorePreflight can rely on arguments[0] being
present without introducing a separate unchecked validation path.

In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePlanner.swift`:
- Around line 79-84: Separate the argv rewrite from preflight construction in
hermesPreflights: introduce distinct operations so provider argument rewriting
occurs explicitly before requesting preflights, while preserving the current
ordering and rewritten-argv behavior even when no preflights are returned.
Update the caller around the preflights assignment and retain the existing test
contract.
- Line 152: Normalize agent kind values before all routing comparisons in
AgentRestorePlanner, including the "pi"/"omp", "claude", and "hermes-agent"
branches. Reuse the normalization already applied by AgentRestoreLaunch(kind:)
or introduce a shared helper/typed representation so casing and surrounding
whitespace are handled consistently across routeManagedWrapper, permission-mode,
and auth-selection logic.
- Around line 140-147: Update restoredEnvironment(for:) to create and merge the
captured environment once before branching on request.mode, then reuse that
merged value in the .direct and other mode paths. Remove the duplicated
initialization and merge logic while preserving request.environment’s
binding-value precedence.
- Around line 96-136: Normalize preparedArguments once in the planner using the
existing nonEmpty helper, then use that normalized value in the .direct,
.relaunchAgent, and .resumeAgent fallback paths. Replace the direct optional
mappings and fallback references with the normalized non-empty value while
preserving .relaunchAgent’s built-in relaunch behavior when arguments are absent
or empty.
- Around line 209-234: Add test coverage for the AgentRestorePlanner path that
builds Hermes Codex config preflights, using an environment without
CUSTOM_BASE_URL while ambientEnvironment supplies the default through
applyingDefaultCodexBaseURL. Assert the generated invocations use the resolved
default base URL and resolved environment, while preserving the existing
captured-environment case.

In
`@Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreLaunchTests.swift`:
- Around line 129-168: The test name
directBindingPreservesLongAndShortStructuredArgumentsIdentically does not match
its single-case assertions. Rename it to describe preservation of one structured
argv, keeping the existing long-argument regression coverage and assertions
unchanged.

In
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator`+Surface3.swift:
- Around line 69-70: Update the surface control-command parsing around
controlAgentLaunchCommand so omitted or null launch_command remains absent,
while any non-null malformed value produces invalid_params instead of being
forwarded as nil. Preserve structured invocation data for valid commands and
prevent malformed input from selecting the legacy restore path. Add the
socket-domain error through ControlSurfaceResumeStrings, resolving its localized
text via the app conformance rather than package-local String(localized:).
- Around line 216-273: The launch-command environment currently bypasses the
established sanitization boundary before resume data is stored or returned.
Update controlAgentLaunchCommand(_:) and the related payload flow, including
surfaceRestoreRecordPayload(_:), to validate or sanitize environment fields
through the existing AgentLaunchEnvironmentPolicy/persisted-launch-command path,
rejecting unsafe secrets or emitting only sanitized values while preserving
valid environment entries.

In `@Sources/ControlSurfaceResumeTarget.swift`:
- Around line 319-331: The launch-command mapping is duplicated and can silently
omit newly added fields. In Sources/ControlSurfaceResumeTarget.swift:319-331,
make controlAgentLaunchCommand accessible to the sibling extension while
retaining it as the single mapping implementation; in
Sources/TerminalController+ControlSurfaceContext.swift:98-108, replace the
inline ControlAgentLaunchCommand construction with
effective.launchCommand.map(controlAgentLaunchCommand).

In `@Sources/SurfaceResumeCommandCanonicalizer`+PortableAgentExecutable.swift:
- Around line 64-81: Update restoreCLIArgument to reject values whose first
character is "-", while preserving the existing trimming, non-empty, and
allowlist validation. Keep localRestoreCLIInput unchanged so valid positional
kind and checkpointId arguments continue to be emitted normally.
🪄 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: 66087b1b-b253-4403-b2b5-72367317deb6

📥 Commits

Reviewing files that changed from the base of the PR and between b5294c4 and d5a8b52.

📒 Files selected for processing (44)
  • CLI/CMUXCLI+CommandSuggestions.swift
  • CLI/CMUXCLI+Restore.swift
  • CLI/cmux.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCommand.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreInvocation.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreLaunch.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePlanner.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreRequest.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreLaunchTests.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface3.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceRestoreRecord.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceResumeBinding.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceResumeSetInputs.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceResumeSnapshot.swift
  • Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorSurfaceTests.swift
  • Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeSurfaceControlCommandContext.swift
  • Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Session/WorkspaceSessionRestorePolicyService.swift
  • Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Session/WorkspaceSurfaceResumeBinding.swift
  • Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Session/WorkspaceSessionRestorePolicyServiceTests.swift
  • Sources/ControlSurfaceResumeTarget.swift
  • Sources/DockSplitStore+SessionRestore.swift
  • Sources/RestorableAgentSession.swift
  • Sources/RestorableAgentTypes.swift
  • Sources/SessionPersistence.swift
  • Sources/SurfaceResumeBindingSnapshot+Remote.swift
  • Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift
  • Sources/TabManager.swift
  • Sources/TerminalController+ControlSurfaceContext.swift
  • Sources/TmuxResumeParser.swift
  • Sources/Workspace+RemoteSurfaceResumeBinding.swift
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/AgentResumeReturnShellStartupTests.swift
  • cmuxTests/AgentSessionAutoResumeSettingsTests.swift
  • cmuxTests/AppDelegateIssue2907RoutingTests.swift
  • cmuxTests/CMUXCLIErrorOutputRegressionTests.swift
  • cmuxTests/ForkParentFallbackResidualTests.swift
  • cmuxTests/ResumeLauncherCwdConsistencyTests.swift
  • cmuxTests/SessionPersistenceResumeBindingTests.swift
  • cmuxTests/SessionPersistenceTests.swift
  • cmuxTests/SurfaceResumeBindingCodexUpdateCheckTests.swift
  • cmuxTests/WorkspaceUnitTests.swift
  • docs/cli-contract.md
  • scripts/stress-cli-socket-api.py

Comment thread CLI/CMUXCLI+Restore.swift
Comment thread CLI/CMUXCLI+Restore.swift Outdated
Comment thread cmuxTests/AgentResumeReturnShellStartupTests.swift Outdated
Comment thread cmuxTests/CMUXCLIErrorOutputRegressionTests.swift Outdated
Comment thread cmuxTests/CMUXCLIErrorOutputRegressionTests.swift Outdated
Comment thread Sources/ControlSurfaceResumeTarget.swift Outdated
Comment thread Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift Outdated

@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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
cmuxTests/SessionPersistenceResumeBindingTests.swift (1)

73-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

This temporary-directory assertion cannot fail.

temporaryDirectory is created at Line 76 but is never passed to binding.restoreStartupInput(). For an agent-hook binding, that call resolves to localRestoreCLIInput, which builds only a string and performs no file I/O. The contentsOfDirectory(...).isEmpty check at Lines 97-102 is therefore true unconditionally and proves nothing about restore behavior.

Since the local-restore-verb path never touches the file system by design, remove the temp-directory setup and this assertion rather than trying to make it meaningful.

💚 Proposed change to drop the vacuous check
     func localRestoreUsesOneShortCLICommandRegardlessOfBindingSize() throws {
         let sessionId = "a22293b7-bcef-4707-8439-2f538c8517a4"
-        let temporaryDirectory = FileManager.default.temporaryDirectory
-            .appending(path: "cmux-restore-verb-\(UUID().uuidString)", directoryHint: .isDirectory)
-        try FileManager.default.createDirectory(
-            at: temporaryDirectory,
-            withIntermediateDirectories: true
-        )
-        defer { try? FileManager.default.removeItem(at: temporaryDirectory) }
-
         let binding = SurfaceResumeBindingSnapshot(
             kind: "codex",
             command: "codex resume \(sessionId) " + String(repeating: "--config model_provider=subrouter ", count: 80),
             checkpointId: sessionId,
             source: "agent-hook",
             autoResume: true
         )

         let startupInput = try `#require`(binding.restoreStartupInput())

         `#expect`(
             startupInput
                 == " \(AgentRestoreLaunch.bundledCLIStartupExecutableToken()) restore codex \(sessionId)\n"
         )
-        `#expect`(
-            try FileManager.default.contentsOfDirectory(
-                at: temporaryDirectory,
-                includingPropertiesForKeys: nil
-            ).isEmpty
-        )
     }
🤖 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/SessionPersistenceResumeBindingTests.swift` around lines 73 - 103,
Remove the unused temporaryDirectory setup, directory creation/cleanup, and
contentsOfDirectory assertion from
localRestoreUsesOneShortCLICommandRegardlessOfBindingSize. Keep the test focused
on validating binding.restoreStartupInput() and its expected short CLI command.
Sources/RestorableAgentSession.swift (1)

775-817: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Shared root cause: identical safe-restore-argument regex duplicated across two files. Both RestorableAgentSession.swift's isSafeRestoreCLIArgument and SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift's restoreCLIArgument use the pattern ^[A-Za-z0-9._:+][A-Za-z0-9._:+-]*$ to decide whether kind/checkpointId is safe to interpolate into the cmux restore <kind> <checkpoint> line. A past review already had to fix a leading-- gap in one copy; nothing enforces the two copies stay in sync going forward.

  • Sources/RestorableAgentSession.swift#L775-L817: extract isSafeRestoreCLIArgument's regex logic into one shared predicate (for example next to AgentRestoreLaunch.bundledCLIStartupExecutableToken()).
  • Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift#L73-L85: replace restoreCLIArgument's inline regex check with a call to the same shared predicate.
🤖 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/RestorableAgentSession.swift` around lines 775 - 817, Centralize the
safe restore-CLI argument validation currently implemented by
RestorableAgentSession.isSafeRestoreCLIArgument into one shared predicate near
AgentRestoreLaunch.bundledCLIStartupExecutableToken(). Update
Sources/RestorableAgentSession.swift lines 775-817 to use that predicate, and
replace the inline regex check in
Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift lines
73-85 within restoreCLIArgument with the same shared predicate; preserve the
existing pattern and leading-hyphen protection.
♻️ Duplicate comments (1)
Sources/ControlSurfaceResumeTarget.swift (1)

336-348: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

controlAgentLaunchCommand is still private; the duplicate mapping likely persists.

A previous review asked to drop private from controlAgentLaunchCommand so Sources/TerminalController+ControlSurfaceContext.swift could call it as the single AgentLaunchCommandSnapshot -> ControlAgentLaunchCommand mapping, instead of constructing ControlAgentLaunchCommand(...) inline a second time. The function is still private here, and Swift private restricts access to the declaring file, so the sibling file cannot call it as-is. A field added to AgentLaunchCommandSnapshot can still silently drop out of resume_binding or restore_record if only one of the two mapping sites is updated.

Sources/TerminalController+ControlSurfaceContext.swift is not in this review batch, so I cannot confirm whether its inline construction was removed independently. If it was, this comment does not apply; if not, drop private here and route the sibling call site through this function.

#!/bin/bash
# Description: Check whether TerminalController+ControlSurfaceContext.swift still
# constructs ControlAgentLaunchCommand inline instead of calling controlAgentLaunchCommand.
rg -n -B3 -A10 'ControlAgentLaunchCommand\(' Sources/TerminalController+ControlSurfaceContext.swift
🤖 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/ControlSurfaceResumeTarget.swift` around lines 336 - 348, Remove the
private access modifier from controlAgentLaunchCommand so
TerminalController+ControlSurfaceContext.swift can call this shared
AgentLaunchCommandSnapshot-to-ControlAgentLaunchCommand mapping. Update the
sibling call site to use controlAgentLaunchCommand instead of constructing
ControlAgentLaunchCommand inline, preserving a single mapping implementation.
🤖 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 `@cmuxTests/CMUXCLIErrorOutputRegressionTests.swift`:
- Around line 194-238: Remove inherited CMUX_* environment entries from the
environment created in
testRestoreFallsBackWhenStructuredPlannerCannotBuildInvocation before assigning
the test-specific CMUX_CLI_SENTRY_DISABLED, CMUX_SOCKET_PATH, and
CMUX_SURFACE_ID values. Follow the existing cleanup pattern used by
testRestorePositionalFormRequiresSurfaceContext, and apply the same protection
to the sibling restore-test environment setups identified in this file.

In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreLaunch.swift`:
- Around line 17-30: Update AgentRestoreLaunch.bundledCLIStartupExecutableToken
to quote bundled paths containing ! safely for csh/tcsh as well as POSIX and
fish, avoiding history substitution while preserving existing apostrophe
escaping and fallback behavior. Add coverage for paths containing ! across
POSIX, fish, and csh/tcsh quoting cases.

---

Outside diff comments:
In `@cmuxTests/SessionPersistenceResumeBindingTests.swift`:
- Around line 73-103: Remove the unused temporaryDirectory setup, directory
creation/cleanup, and contentsOfDirectory assertion from
localRestoreUsesOneShortCLICommandRegardlessOfBindingSize. Keep the test focused
on validating binding.restoreStartupInput() and its expected short CLI command.

In `@Sources/RestorableAgentSession.swift`:
- Around line 775-817: Centralize the safe restore-CLI argument validation
currently implemented by RestorableAgentSession.isSafeRestoreCLIArgument into
one shared predicate near AgentRestoreLaunch.bundledCLIStartupExecutableToken().
Update Sources/RestorableAgentSession.swift lines 775-817 to use that predicate,
and replace the inline regex check in
Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift lines
73-85 within restoreCLIArgument with the same shared predicate; preserve the
existing pattern and leading-hyphen protection.

---

Duplicate comments:
In `@Sources/ControlSurfaceResumeTarget.swift`:
- Around line 336-348: Remove the private access modifier from
controlAgentLaunchCommand so TerminalController+ControlSurfaceContext.swift can
call this shared AgentLaunchCommandSnapshot-to-ControlAgentLaunchCommand
mapping. Update the sibling call site to use controlAgentLaunchCommand instead
of constructing ControlAgentLaunchCommand inline, preserving a single mapping
implementation.
🪄 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: afd8cb57-735a-45f1-990c-b8676be6e2a6

📥 Commits

Reviewing files that changed from the base of the PR and between d5a8b52 and 8c8169e.

📒 Files selected for processing (14)
  • CLI/CMUXCLI+Restore.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreLaunch.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePlanner.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreLaunchTests.swift
  • Sources/ControlSurfaceResumeTarget.swift
  • Sources/RestorableAgentSession.swift
  • Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift
  • cmuxTests/AgentResumeReturnShellStartupTests.swift
  • cmuxTests/AppDelegateIssue2907RoutingTests.swift
  • cmuxTests/CMUXCLIErrorOutputRegressionTests.swift
  • cmuxTests/ForkParentFallbackResidualTests.swift
  • cmuxTests/ResumeLauncherCwdConsistencyTests.swift
  • cmuxTests/SessionPersistenceResumeBindingTests.swift
  • cmuxTests/SessionPersistenceTests.swift

Comment thread cmuxTests/CMUXCLIErrorOutputRegressionTests.swift
Comment thread Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreLaunch.swift Outdated
@cursor

cursor Bot commented Jul 31, 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.

@cursor

cursor Bot commented Jul 31, 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.

@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: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface3.swift (1)

236-275: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject unparseable launch-command inputs.

controlAgentLaunchCommandPayload serializes launcher, executable_path, working_directory, environment, and source without validation; Surface3.controlAgentLaunchCommand only parses arguments and environment.values but still accepts invalid types for the other execution fields (nil, .null, .string). Reject malformed launch-command JSON before building AgentRestoreInvocation so restore cannot pass executable paths, cwd, environment objects, or other fields with the wrong shape to execve.

🤖 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
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator`+Surface3.swift
around lines 236 - 275, Update controlAgentLaunchCommandPayload and
Surface3.controlAgentLaunchCommand to validate all launch-command fields before
constructing ControlAgentLaunchCommand: require executable_path,
working_directory, and source to have their expected serialized types, and
validate environment as a string-valued object when present. Reject missing,
null, or malformed execution fields rather than allowing rawString/stringMap
fallbacks, while preserving valid optional-field behavior and the existing
arguments validation.
CLI/CMUXCLI+Restore.swift (1)

95-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate the two legacy fallback paths into one.

The file now has two legacy execution paths. Lines 98-108 run when both structured fields are absent. Lines 145-155 run when planning returns nil. Both compute a working directory, set PWD, close the client, and exec the compatibility shell, but they resolve the working directory differently: line 101 uses only record.workingDirectory, while the structured path also considers record.launchCommand?.workingDirectory. One shared helper keeps the two exits consistent and removes the divergence risk.

The first block is also load-bearing for records with an unrecognized mode, so keep that ordering when you extract the helper.

♻️ Suggested extraction
+    private func execLegacyRestore(
+        command: String,
+        workingDirectory: String?,
+        environment: [String: String],
+        client: SocketClient
+    ) throws -> Never {
+        var legacyEnvironment = environment
+        if let workingDirectory {
+            legacyEnvironment["PWD"] = workingDirectory
+        }
+        client.close()
+        try execLegacyRestoreCommand(command, environment: legacyEnvironment)
+        throw CLIError(message: "restore: compatibility shell did not replace the process")
+    }
🤖 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 `@CLI/CMUXCLI`+Restore.swift around lines 95 - 157, Extract the duplicated
legacy restore execution into one local helper that accepts the legacy command,
resolves the working directory using the same
record.workingDirectory/launchCommand?.workingDirectory fallback as the
structured path, sets PWD when applicable, closes client, and calls
execLegacyRestoreCommand. Use this helper for both the initial
missing-structured-fields branch and the invocation == nil fallback, while
preserving the initial branch before AgentRestoreRequestMode validation.
Sources/TerminalController+ControlSurfaceContext.swift (1)

90-111: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Filter launchCommand.environment when building resume_binding.

controlResumeBinding serializes ControlAgentLaunchCommand.environment directly, while surfaceResumeBindingPayload echoes it over the socket. controlAgentLaunchCommand($0) only filters when replaySafeEnvironmentFor is provided, so captured secrets still appear under resume_binding.launch_command.environment. Match the restore-record path and pass the binding’s effective kind so AgentLaunchEnvironmentPolicy().selectedRestoreEnvironment(from:kind:) redacts provider-sensitive values.

🔒 Proposed fix
             launchCommand: effective.launchCommand.map {
-                controlAgentLaunchCommand($0)
+                controlAgentLaunchCommand($0, replaySafeEnvironmentFor: effective.kind)
             },
🤖 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/TerminalController`+ControlSurfaceContext.swift around lines 90 -
111, Update controlResumeBinding’s launchCommand mapping to call
controlAgentLaunchCommand with the binding’s effective kind via
replaySafeEnvironmentFor. Ensure the resulting
resume_binding.launch_command.environment is filtered by
AgentLaunchEnvironmentPolicy().selectedRestoreEnvironment(from:kind:), matching
the restore-record path and preventing provider-sensitive values from being
serialized.
Resources/Localizable.xcstrings (1)

132186-132218: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Complete localization coverage for all new keys.

These keys provide only en and ja, but the catalog supports 20 locales. Add the remaining 18 locale entries at both sites.

  • Resources/Localizable.xcstrings#L132186-L132218: add translations for mobile.pairing.codeMode.legacyDetail and mobile.pairing.codeMode.useLegacy.
  • Resources/Localizable.xcstrings#L227420-L227436: add translations for socket.surface.resume.launchCommandMustBeValid.
🤖 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 `@Resources/Localizable.xcstrings` around lines 132186 - 132218, Complete
localization coverage in Resources/Localizable.xcstrings at 132186-132218 by
adding the remaining 18 supported locale entries for
mobile.pairing.codeMode.legacyDetail and mobile.pairing.codeMode.useLegacy,
preserving the existing English and Japanese translations. Also update
Resources/Localizable.xcstrings at 227420-227436 to add all remaining locale
entries for socket.surface.resume.launchCommandMustBeValid, using accurate
translations and the catalog’s existing locale structure.

Sources: Coding guidelines, Path instructions, Learnings

🤖 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 `@CLI/cmux.swift`:
- Around line 3789-3805: Update the restore failure handling in the socket
connection catch block to use the friendly “cmux is not ready” retry message
only when command is "restore" and explicitSocketPath is nil. For an explicit
socket path, surface the existing plain connection error instead, while
preserving the current telemetry and implicit restore behavior.

In `@CLI/CMUXCLI`+RestorePreflight.swift:
- Around line 33-52: Update the preflight file-action setup in
CMUXCLI+RestorePreflight so the spawned child’s STDERR_FILENO is redirected to
/dev/null alongside STDIN_FILENO and STDOUT_FILENO. Preserve the existing
redirectStatus failure flow and ensure the stderr action is only added after the
prior actions succeed.
- Around line 90-113: Move the deadline check in the wait loop surrounding
waitpid to the beginning of each iteration, before calling waitpid or handling
EINTR. Preserve the existing timeout termination and error behavior, ensuring
repeated EINTR results cannot bypass the 10-second deadline.

In `@cmuxTests/CMUXCLIErrorOutputRegressionTests.swift`:
- Around line 2191-2223: Remove the unreachable guard !request.isEmpty in
handle(clientFD:), since request always contains at least the newline before the
inner loop exits and read failures return earlier. Leave the existing request
parsing and response flow unchanged.

In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreRequest.swift`:
- Around line 29-30: Wire preparedArgumentsWorkingDirectory through the
persisted/transport restore schema used by CMUXCLI+Restore.swift, including
decoding the prepared_arguments_working_directory field alongside
PreparedArguments. Populate this value when constructing the production
ControlSurfaceRestoreRecord so
AgentRestorePlanner.retargPreparedWorkingDirectory receives the captured cwd
instead of falling back to launchCommand.workingDirectory.

In `@Resources/Localizable.xcstrings`:
- Around line 227420-227436: Update the English value for
socket.surface.resume.launchCommandMustBeValid to explicitly state that
launch_command.arguments must be a non-empty array of strings, then revise the
Japanese localization to convey the same structure and validation requirement.

In `@Sources/ControlSurfaceResumeTarget.swift`:
- Around line 345-350: Update the launch-command environment filtering flow in
controlAgentLaunchCommand so the kind passed to selectedRestoreEnvironment is
lowercased, while preserving the original normalizedKind value for other uses
and dispatch behavior.

---

Outside diff comments:
In `@CLI/CMUXCLI`+Restore.swift:
- Around line 95-157: Extract the duplicated legacy restore execution into one
local helper that accepts the legacy command, resolves the working directory
using the same record.workingDirectory/launchCommand?.workingDirectory fallback
as the structured path, sets PWD when applicable, closes client, and calls
execLegacyRestoreCommand. Use this helper for both the initial
missing-structured-fields branch and the invocation == nil fallback, while
preserving the initial branch before AgentRestoreRequestMode validation.

In
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator`+Surface3.swift:
- Around line 236-275: Update controlAgentLaunchCommandPayload and
Surface3.controlAgentLaunchCommand to validate all launch-command fields before
constructing ControlAgentLaunchCommand: require executable_path,
working_directory, and source to have their expected serialized types, and
validate environment as a string-valued object when present. Reject missing,
null, or malformed execution fields rather than allowing rawString/stringMap
fallbacks, while preserving valid optional-field behavior and the existing
arguments validation.

In `@Resources/Localizable.xcstrings`:
- Around line 132186-132218: Complete localization coverage in
Resources/Localizable.xcstrings at 132186-132218 by adding the remaining 18
supported locale entries for mobile.pairing.codeMode.legacyDetail and
mobile.pairing.codeMode.useLegacy, preserving the existing English and Japanese
translations. Also update Resources/Localizable.xcstrings at 227420-227436 to
add all remaining locale entries for
socket.surface.resume.launchCommandMustBeValid, using accurate translations and
the catalog’s existing locale structure.

In `@Sources/TerminalController`+ControlSurfaceContext.swift:
- Around line 90-111: Update controlResumeBinding’s launchCommand mapping to
call controlAgentLaunchCommand with the binding’s effective kind via
replaySafeEnvironmentFor. Ensure the resulting
resume_binding.launch_command.environment is filtered by
AgentLaunchEnvironmentPolicy().selectedRestoreEnvironment(from:kind:), matching
the restore-record path and preventing provider-sensitive values from being
serialized.
🪄 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: 29aa9324-7edb-495e-a25b-a8e5dbded277

📥 Commits

Reviewing files that changed from the base of the PR and between 8c8169e and a5a582d.

📒 Files selected for processing (34)
  • CLI/CMUXCLI+Restore.swift
  • CLI/CMUXCLI+RestorePreflight.swift
  • CLI/cmux.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreInvocation.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreLaunch.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePlanner.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreRequest.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchEnvironmentPolicyTests.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreLaunchTests.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface3.swift
  • Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceResumeStrings.swift
  • Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift
  • Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorSurfaceTests.swift
  • Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeSurfaceControlCommandContext.swift
  • Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Session/WorkspaceSessionRestorePolicyService.swift
  • Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Session/WorkspaceSessionRestorePolicyServiceTests.swift
  • Resources/Localizable.xcstrings
  • Sources/ControlSurfaceResumeTarget.swift
  • Sources/RestorableAgentSession.swift
  • Sources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swift
  • Sources/TabManager.swift
  • Sources/TerminalController+ControlSurfaceContext.swift
  • Sources/TerminalController+ControlSurfaceContext3.swift
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/AgentResumeReturnShellStartupTests.swift
  • cmuxTests/AppDelegateIssue2907RoutingTests.swift
  • cmuxTests/CMUXCLIErrorOutputRegressionTests.swift
  • cmuxTests/ForkParentFallbackResidualTests.swift
  • cmuxTests/ResumeLauncherCwdConsistencyTests.swift
  • cmuxTests/SessionPersistenceResumeBindingTests.swift
  • cmuxTests/SessionPersistenceTests.swift
  • docs/cli-contract.md
💤 Files with no reviewable changes (1)
  • Sources/Workspace.swift

Comment thread CLI/cmux.swift Outdated
Comment thread CLI/CMUXCLI+RestorePreflight.swift
Comment thread CLI/CMUXCLI+RestorePreflight.swift Outdated
Comment thread cmuxTests/CMUXCLIErrorOutputRegressionTests.swift
Comment thread Resources/Localizable.xcstrings
Comment thread Sources/ControlSurfaceResumeTarget.swift
azooz2003-bit added a commit that referenced this pull request Aug 1, 2026
5bf9595 (#9265) left the final expression of a multi-statement String?
method without an explicit return; every target compiling this file fails,
which currently blocks all merge-gate runs.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Aug 1, 2026
5bf9595 (#9265) left the final expression of a multi-statement String?
method without an explicit return; every target compiling this file fails,
which currently blocks all merge-gate runs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Aug 3, 2026
* test(ios): cover workspace group row actions

* fix(ios): restore workspace group row actions

* test(ios): cover group destructive confirmations

* test(ios): cover group read-state action refresh

* fix(ios): refresh group native action state

* test(ios): cover group native action inputs

* fix(ios): refresh group native action inputs

* test(ios): cover group swipe completion and rename alert

* fix(ios): restore workspace preview compilation

* test(ios): target visible group rename fixture

* fix(ios): preserve group swipe completion and compact rename

* test(ios): exercise group action presentation lifecycles

* test(ios): preserve workspace actions on group menus

* fix(ios): preserve workspace actions on group menus

* test(ios): exercise full group read swipe

* test(ios): isolate native group menu assertions

* test(ios): cover preserved group actions

* test(ios): keep preview fixture state owned

* test(ios): cover preserved group create actions

* fix(ios): preserve group creation entrypoints

* fix(ios): make destructive group requests atomic

* test(ios): cover configured group icons

* fix(ios): sync effective group icons

* test(ios): target live group row swipe

* test(ios): disambiguate group workspace rename

* fix(ios): disambiguate group workspace rename

* test: cover disconnected iOS dogfood launch

* fix: require connected iOS dogfood launches

* fix: make mobile readiness event driven

* Add failing iroh wake reconnect regressions

* Guarantee bounded foreground reconnect

* fix: harden mobile readiness lifecycle

* perf: buffer deadline event reads

* Add failing test: session snapshot mid-revalidation must classify transient

Every launch/foreground kicks a /users/me revalidation and
sessionTokenTransitionIsActive is true for its whole round trip.
authenticatedSessionSnapshot() throws .unauthorized for that window, which
the iroh broker token source treats as signed out, so endpoint activation
fails closed (endpointFailed authorizationFailed) on every app launch until
the revalidation completes. The same state is already classified
.networkError by accessToken(); the snapshot must match.

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

* Classify transient token misses as connectivity, not authorization failure

Three-layer fix for the launch-time wedge where every iroh endpoint
activation failed closed (endpointFailed authorizationFailed) while a
foreground session revalidation owned the token store:

1. AuthCoordinator.authenticatedSessionSnapshot() now throws .networkError
   while sessionTokenTransitionIsActive, matching accessToken()'s
   classification. Every launch/foreground kicks a network /users/me
   revalidation, and that window previously read as "signed out".

2. CmxIrohBrokerTokenSource.credentialPair is now throwing. A throw means
   "cannot read a coherent pair right now" and the broker classifies it
   .connectivity, so retry policies, verified-policy preservation, and the
   cached offline-policy bootstrap all apply. nil still means definitively
   signed out and fails closed with .missingAuthentication.

3. The iOS activation token source maps AuthError.unauthorized to nil
   (fail closed) and rethrows every transient failure instead of collapsing
   both into nil with try?.

Diagnosed from cmuxdiag exports on build 1.0.4 (20260731034828): three
consecutive relayPolicyRefreshFailed/endpointFailed(authorizationFailed)
within 10ms each (no network round trip) at launch, recovering only ~15s
later when the revalidation settled and the backoff retried.

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

* Apply the same transient-token classification to the Mac host runtime

The Mac host's activation token source had the identical try? collapse:
a session revalidation window read as signed-out and tore the host
runtime down as unauthorized. Same mapping as iOS: unauthorized fails
closed with nil, transient failures rethrow and classify connectivity.

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

* Add failing wake-auth transport regressions

A broker 401 at app wake (token pair rotated by another lane between
capture and server validation) must not tear down the verified iroh
runtime, and the Mac being redialed must not be dialed a second time as
a background-control aggregation candidate.

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

* Survive wake-time broker auth rejections without endpoint teardown

At app wake the relay-policy refresh races the RPC lane's force token
refresh: the pair captured coherently a moment earlier reaches the
broker after rotation and gets a 401. That single 401 used to fail the
endpoint, clear routes and the offline cache (or tear down the whole
runtime on warm wakes), and nap 30-36s of flat backoff, turning a
seconds-long token race into the 30s-2.5min reconnect outages visible
in every wake ring.

Four changes:
- CmxIrohTrustBrokerClient recovers exactly once from a 401: the token
  source re-captures (force-minting only when the rejected access token
  is unchanged) and the request retries with the recovered pair. Frozen
  pinned sources (sign-out revocation) opt out by default.
- 401/403 now preserve verified policy during refresh, and 401 retries
  initial activation; resolvePolicy falls back to the verified offline
  bootstrap on auth rejections like it already did for connectivity, so
  LAN and cached-relay dials keep working while auth settles.
- The relay-policy refresh loop retries authorization failures on a
  2s..120s ladder instead of the flat 30s+jitter schedule.
- The Mac being redialed is excluded from secondary aggregation while a
  stored-Mac reconnect is in flight, removing the duplicate
  background-control dial (and its drain wait) from every recovery.

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

* Handle route-gated diagnostics in iOS settings

* test: remove source-shape admission assertion

* test(iroh): cover cached registration recovery

* fix(iroh): recover cached host registration

* test connection readiness failures

* test: cover cached host binding publication

* fix: publish cached mobile host binding

* iOS: replace disconnect chrome with Mail-style status line under the computers picker

While a reconnect attempt has not been rejected, the last visible workspace
list and terminals stay accessible. The workspace list shows a caption status
line (spinner + Reconnecting… / Not Connected) under the computers picker,
like Mail's Checking for Mail…; the terminal keeps only the compact status
pill. The full-screen TerminalDisconnectedOverlay, the list's
Disconnected/Reconnecting status row for non-startup states, and the
connection status toasts are removed. The reauth banner (rejected
connection, Sign Out is the only fix) and the initial-restore status row
(Retry / Add Computer, possibly no cached content) remain. Input gating and
the pill's recovery folding, previously behind the Toasts beta flag, are now
unconditional; a Reconnect item appears in the picker menu while Not
Connected.

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

* Harden mobile connection readiness

* Keep subscription readiness separate from recovery

* Model delayed subscription acknowledgements

* test: cover usable mobile session readiness

* fix: require usable mobile connection readiness

* test: fail closed across broker auth cancellation

* test: cover complete iOS dogfood readiness

* test: fail closed when Mac pairing setup is unavailable

* fix: make iOS dev reload dogfood ready

* test: require ensure-mac to self-heal exact tag

* fix: let ensure-mac relaunch its exact tag

* fix(ios): keep list probe state coordinator-owned

* test(ios): pass active listener to recovery validation

* test: cover unsigned simulator identity evidence

* fix: trust seeded identity in unsigned simulator

* test: disambiguate group rename alert save

* test: expose expired-ticket group rename failure

* Authorize mac-scoped workspace mutations by Stack account, not ticket lifetime

The mobile data plane's design authority is the signed-in Stack account;
attach tickets are route discovery plus scope narrowing. Four verbs
(workspace.move, workspace.group.action, workspace.group.create, and
workspace.create with group_id) still hard-required a current attach
ticket, and minted tickets default to a 600s TTL, so iOS drag-and-drop
and the + button's New Workspace Group item silently disappeared ten
minutes after pairing (and never appeared for tokenless zero-touch
pairings).

Host: ticketAuthorizationResultIfNeeded no longer fails these verbs when
the attach token is missing, unknown, or expired; a token that maps to a
current stored ticket still narrows scope, so workspace-pinned tickets
remain rejected for Mac-wide mutations. Advertised as
workspace.mutations.account_auth.v1.

iOS: MobileShellWorkspaceMutationTicketPolicy mirrors the host: against
hosts advertising the capability, mutations stay allowed unless a
current workspace-scoped ticket narrows the connection; legacy hosts
keep the fail-closed behavior. Applied to the foreground gate, the
per-target mutation gate, and secondary-Mac handle capabilities.

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

* test(ios): cover recovery transport drain

* fix(ios): drain stale route before recovery

* test: expose process-local readiness clock

* fix: use system monotonic readiness clock

* test(ios): expose scoped-ticket group rename gap

* fix(ios): preserve account-authorized group actions

* test(ios): keep group menus group scoped

* fix(ios): keep workspace group menus group scoped

* fix(ios): pass readiness clock after main merge

* test(ios): close group action review gaps

* Fix missing return in restoreCLIArgument (main compile break)

5bf9595 (#9265) left the final expression of a multi-statement String?
method without an explicit return; every target compiling this file fails,
which currently blocks all merge-gate runs.

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

* fix(ios): redact workspace mutation failure diagnostics

An rpcError message is an arbitrary host string; exported diagnostics now
carry only the bounded DiagnosticFailureKind plus the short RPC code, and
the os.log line marks the raw error private.

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

* fix(ios): stop presenting gated connect attempts as timeouts

connectAttemptGated means another attempt owns the route, not that the
Mac failed to respond. New pairing category with wait-for-active-attempt
copy and guidance (en+ja) instead of 'No response from …' timeout text.

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

* fix(ios): add missing statusLine keys to MobileShellUI catalog

mobile.workspaces.statusLine.reconnecting/notConnected were referenced by
WorkspaceConnectionStatusLineView but absent from the package catalog, so
Japanese fell back to English defaults.

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

* fix(cli): monotonic events timeout budget, deterministic reconnect wait

The --timeout budget now runs on ContinuousClock so wall-clock changes
cannot expire or extend it; each socket call derives a fresh short-lived
Date from the monotonic remainder and authentication re-checks the budget
first. The reconnect pause replaces the Timer+RunLoop pump (which can spin
or park on the CLI's unpumped command thread) with a bounded thread sleep
clamped to the remaining budget.

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

* fix(ios): drop stale swiped-row identity on structural refresh

A structural update invalidates the row identity captured at swipe start;
keeping editedItemID could defer a reload against a row that no longer
exists.

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

* test(ios): align drop fixture with connection chrome

* fix: return validated restore argument

* test(ios): port drop tests to the status-line WorkspaceListTable API

Main's drop tests (from #8602) still passed connectionRecoveryFailed,
isRecoveringConnection, and retryConnectionRecovery, which this branch's
status-line rework removed from WorkspaceListTable; the package no longer
compiled on the merged tree.

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

* test(ios): drop superseded relayPolicyRetrySchedule test

The cause-aware relayPolicyRetrySchedule(for:) API this test pinned was
replaced by the shared foreground reconnect-backoff ladder during the
connection-supervisor cross-merge (see the scheduleRelayPolicyRefresh
comment); the symbol exists nowhere, so cmuxFeatureTests did not compile.
The fast-auth-retry concern lives in the ladder's own coverage.

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

* test(ios): drop superseded relay schedule assertion

* test(ios): identify inherited group menu actions

* test(ios): lock group menu action order

* test(iroh): expose truncated registration discovery

* fix(iroh): distrust truncated registration discovery

* test(connectivity): expose truncated sync snapshots

* fix(connectivity): prove complete sync snapshots

* test(connectivity): expose discovery revision races

* fix(connectivity): snapshot routes atomically

* test(ios): expose discovery blocking saved reconnect

* fix(ios): prioritize saved routes during recovery

* test(connectivity): expose endpoint recovery race

* fix(connectivity): await endpoint recovery before dialing

* fix(connectivity): fail closed on offline auth fallback

* Harden mobile group actions and reconnect readiness

* Include mobile debug registry source

* Fix group rename alert target lifetime

* Address workspace merge policy findings

* Scope reconnect policy to owning view

* Fix SSH retry test diagnostic compilation

* Align host refresh tests with auth recovery

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
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.

Session restore composes giant shell one-liners and temp zsh scripts; replace with a first-class cmux restore <agent> <session-id> verb

1 participant