Restoring a remote workspace terminal hands it this machine's working directory - #9855
austinywang wants to merge 34 commits into
Conversation
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds persisted working-directory selection policies for agent restore. It updates launch-option sanitization, resume and fork command generation, remote session restoration, persistent SSH reattachment, auto-resume, hibernation, and regression tests. ChangesRemote restore working-directory flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Workspace
participant RestoreSnapshot
participant ResumeBuilder
participant RemoteBinding
Workspace->>RestoreSnapshot: retain trusted working-directory selection
RestoreSnapshot->>ResumeBuilder: resolve directory and sanitize launch options
ResumeBuilder->>RemoteBinding: provide constrained startup input
RemoteBinding-->>Workspace: restore or suppress remote resume
Merge Risk: 🔵 Low · up to The change is mergeable with owner follow-up for a bounded localization issue: the notification command still hardcodes one label and does not provide the new help text in all supported locales, which can cause inconsistent user-facing output. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (23 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 14.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 111 functions across 19 files. (3 skipped: 1 unsupported, 2 too large.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/AgentSessionAutoResumeSwiftTests.swift`:
- Around line 2259-2268: Update the remote command validation around
TerminalStartupWorkingDirectoryPrefix.shellWordRanges so each required step uses
test failure semantics rather than optional branching: require the --command-b64
token index, its following argument, successful base64 decoding, and UTF-8
conversion. Keep the existing assertions that the decoded remoteCommand excludes
unsafeStartupPayload and capturedDirectory, ensuring any malformed or missing
payload fails the test.
- Around line 2453-2536: The withRestoredRemoteSurface helper must stop mutating
UserDefaults.standard for
AgentSessionAutoResumeSettings.autoResumeAgentSessionsKey. Inject or expose a
test-scoped defaults store through Workspace and the restore path so
AgentSessionAutoResumeSettings.isEnabled(defaults:) reads the helper’s value
without affecting other tests; retain the existing AppDelegate.shared and active
tab-manager scoping.
In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentWorkingDirectoryOptionPolicy.swift`:
- Around line 26-37: Normalize the trimmed agent kind to lowercase before the
switch in the working-directory option policy. Update the policy logic around
AgentWorkingDirectoryOptionPolicy and its agentKind handling so case variants
such as Codex, Kimi, and Qoder select the same -C or -w options as their
lowercase identifiers, while preserving the generic fallback for other kinds.
🪄 Autofix
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: 3c6316e0-98e6-4498-89dc-4c333439d0c5
📒 Files selected for processing (23)
.github/workflows/ci.ymlCLI/CMUXCLI+SessionsListForkStartupInput.swiftCLI/cmux.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePlanner.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreWorkingDirectorySelection.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentWorkingDirectoryOptionPolicy.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreWorkingDirectorySelectionTests.swiftSources/AgentForkSupport.swiftSources/AgentRelaunchCommandBuilder.swiftSources/ControlSurfaceResumeTarget.swiftSources/RestorableAgentSession.swiftSources/RestorableAgentTypes.swiftSources/SessionIndexModels.swiftSources/SessionRestorableAgentSnapshot+Commands.swiftSources/TabManager.swiftSources/Workspace+RemoteSurfaceResumeBinding.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSettingsTests.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/PiVaultAgentPersistenceTests.swiftcmuxTests/SessionPersistenceTests.swift
|
Review closeout on 861d4e0: the cache-substitution finding is fixed in a53848c by persisting the authoritative cwd trust selection directly on SurfaceResumeBindingSnapshot. persistentSSHResumeCommand now consults that persisted binding policy rather than restoredAgentSnapshotsByPanelId: unavailable selections issue only a require-existing PTY reattach with no reconstructed agent startup payload, while exact selections embed the constrained, cwd-sanitized startup input. Focused behavior tests cover both paths and binding Codable persistence. The docstring item is intentionally limited to public policy APIs and non-obvious trust-boundary helpers, which are documented; adding boilerplate to private test helpers and straightforward app composition would not improve the API contract. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/TabManager.swift (1)
5933-5933: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHash
restoreWorkingDirectorySelectionon the resume binding too.Line 5933 adds the selection to the restorable-agent fingerprint.
hashSurfaceResumeBindingSnapshotdoes not hash the equivalent field, yetSurfaceResumeBindingSnapshotnow persistsrestoreWorkingDirectorySelection, andSessionPersistenceResumeBindingTests.restoreWorkingDirectorySelectionRoundTripsAndFailsClosedasserts it round-trips.A binding can change only its selection while
kind,checkpointId,command,cwd, andlaunchCommandstay equal.Workspace.setSurfaceResumeBindingproduces exactly that case:inheritingRestoreWorkingDirectorySelection(from:)copiescommand,cwd, andlaunchCommandfrom the previous binding and changes only the selection. The autosave fingerprint then does not move, and the autosave timer skips the write. The persisted binding keeps the stale trust policy, so the next restore can replay an untrusted local working directory.🐛 Proposed fix
hashOptionalString(snapshot.permissionMode, into: &hasher) hasher.combine(snapshot.allowsAutomaticResume) hasher.combine(snapshot.launchFlavor) + hasher.combine(snapshot.restoreWorkingDirectorySelection) if snapshot.isProcessDetected {Also applies to: 5979-6005
🤖 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/TabManager.swift` at line 5933, Update hashSurfaceResumeBindingSnapshot to include restoreWorkingDirectorySelection in the resume binding fingerprint, matching the existing restorable-agent hashing. Ensure selection-only changes produced by inheritingRestoreWorkingDirectorySelection and setSurfaceResumeBinding alter the autosave fingerprint so the updated binding is persisted.
🤖 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/SessionRestorableAgentSnapshot`+Commands.swift:
- Around line 238-276: Update
SurfaceResumeBindingSnapshot.applyingRestoreWorkingDirectorySelection to clear
constrained.command alongside constrained.cwd and constrained.launchCommand in
both early-return paths: when selection.permitsResume is false and when
constrainedAgent.resumeCommand(...) returns nil. Preserve the existing command
assignment on the successful path.
---
Outside diff comments:
In `@Sources/TabManager.swift`:
- Line 5933: Update hashSurfaceResumeBindingSnapshot to include
restoreWorkingDirectorySelection in the resume binding fingerprint, matching the
existing restorable-agent hashing. Ensure selection-only changes produced by
inheritingRestoreWorkingDirectorySelection and setSurfaceResumeBinding alter the
autosave fingerprint so the updated binding is persisted.
🪄 Autofix
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: df2bcb69-0dc4-4750-9faf-667309bed2ac
📒 Files selected for processing (15)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreWorkingDirectorySelection.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentWorkingDirectoryOptionPolicy.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreWorkingDirectorySelectionTests.swiftSources/ControlSurfaceResumeTarget.swiftSources/SessionPersistence.swiftSources/SessionRestorableAgentSnapshot+Commands.swiftSources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swiftSources/TabManager.swiftSources/Workspace+RemoteSurfaceResumeBinding.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/RemoteResumeBindingTests.swiftcmuxTests/SessionPersistenceResumeBindingTests.swiftcmuxTests/SessionPersistenceTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Greptile SummaryThis PR introduces a persisted trust policy for agent restore working directories so remote restores use only authoritative remote cwd provenance and otherwise fail closed.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the reviewed remote restore and persistent SSH paths. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Persisted remote terminal snapshot] --> B{Authoritative remote cwd available?}
B -->|Yes| C[Store exact cwd selection]
B -->|No, cwd-independent agent| D[Store exact nil selection]
B -->|No, directory-keyed agent| E[Store unavailable selection]
C --> F[Strip captured cwd arguments]
D --> F
E --> G[Suppress agent startup replay]
F --> H[Build constrained resume or fork command]
G --> I[Retain authenticated SSH transport reattach only]
H --> J[Restore remote agent]
Reviews (7): Last reviewed commit: "Keep binding authority through command r..." | Re-trigger Greptile |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
2 similar comments
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
…cal-cwd # Conflicts: # Sources/DockSplitStore+RestoredAgentLifecycle.swift # Sources/Workspace+AgentLifecycle.swift # cmux.xcodeproj/project.pbxproj
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
|
Mac fleet instructions for head JOB_JSON=$(~/.local/bin/cmux-ci submit --kind cmux --command 'CMUX_FLEET_BUILD_TAG=pr-9855-ac866557 /Users/Shared/cmux-build-fleet/recipes/cmux.sh https://github.com/manaflow-ai/cmux.git ac8665577f785eb0b5b44f2a369a3173849bce1a' --artifact artifacts/cmux.app.zip --workspace https://github.com/manaflow-ai/cmux/pull/9855 --source-digest ac8665577f785eb0b5b44f2a369a3173849bce1a --cache-key cmux:pr-9855 --min-free-bytes 268435456000 --label cmux --label ram48)
JOB_ID=$(python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])' <<<"$JOB_JSON")
~/.local/bin/cmux-ci wait "$JOB_ID" --receipt artifacts/fleet/$JOB_ID.json
~/.local/bin/cmux-ci publish-hq "$JOB_ID"Use an existing campaign job ID if one is already posted; do not submit a duplicate. A wait timeout leaves the remote job running. Published results will include an exact-head artifact link and timing/disk receipt. This recipe validates the macOS app only, not iOS or tests. Never use maclease or put credentials in a PR comment. |
This branch was successfully deployed
3 participants
Supersedes #8634 because that PR's head is fork-owned and cannot be advanced from the required origin branch. Its review history is preserved there.
Review comment closeout
Rechecked at
06985525eeb9859d1aa2d4ed4ad67351b4af7837: all 51 inline review threads have explicitaustinywangreplies; none is unresolved. Older resolved findings were rechecked against the implementation, and the fork, generic-retargeting, provider-aware shell-sanitization, and final remote-snapshot-policy findings required follow-up fixes and replies. Top-level CodeRabbit, Cubic, Cursor, and Greptile bodies were checked; no Codex review body exists in the complete queried history. The single audit comment records each ask and its disposition.Summary
Current-HEAD verification
c1c4d1b90fd5fc1dcdf469f55e8ab8932e808c18, compiled and executed seven tests and failed on 24 intended behavioral assertion issues.06985525eeb9859d1aa2d4ed4ad67351b4af7837and passed all seven intended tests, including exact-nil/unavailable fork argv, generic retargeting, Kimi/Qoder cwd flags, and Claude Teams worktree preservation.06985525eeb9859d1aa2d4ed4ad67351b4af7837and passed the unchangedGhosttyTerminalViewVisibilityPolicyTestssuite. This independently disproved the first shard's portal-visibility assertion as a deterministic PR regression.origin/mainatc006e64ae30c2a33fbe1abf337eb848f1f240743is included in HEAD. The branch and existing clone were preserved; no worktree was created.Trade-offs and remaining dogfood
103474014046) passed, followed by green aggregatetests/ci-status; no assertion was weakened and no source change was needed.invalid tag 'fix/remote-restore-local-cwd'; use letters, numbers, dot, underscore, hyphen. No tagged app launched. Interactive relaunch/SSH dogfood at this HEAD is not personally verified; older-head dogfood is not current-HEAD evidence. This is not a visual redesign, and no new screenshot/demo is claimed.This PR touches iOS/shared mobile code and is left open for Austin to dogfood and merge. Current-HEAD device/Simulator dogfood of the iOS provider-absence behavior was not performed in this continuation. No issue was closed; GitHub currently returns no closing-issue references for this PR.
Checklist
06985525ee(run 34656298316).Note
High Risk
Changes security-sensitive session restore and remote SSH resume paths where incorrect cwd replay could launch agents in the wrong directory; broad touch across binding persistence, sanitization, and deferred auto-resume.
Overview
Fixes remote and persistent-SSH agent restores replaying this machine’s captured cwd instead of an authenticated remote directory.
The PR introduces
AgentRestoreWorkingDirectorySelection(recorded fallback, exact, unavailable) and persists it on surface resume bindings and restorable agent snapshots. Resume, relaunch, fork, control-surface records, and startup input now fail closed when policy is unavailable or unscoped on remote agent hooks, and exactnilcwd no longer falls back to snapshot/launch captures. When policy is exact, captured argv cwd flags are stripped using a new per-built-in-agentAgentWorkingDirectoryOptionPolicy(Codex-C, Kimi/Qoder-w, Cursor vs Qoder--workspace, conservative behavior for custom agents).Dock/workspace binding updates inherit or refresh cwd policy across execution locations (local vs persistent-SSH PTY identity), block restore claims across mismatched
launchFlavor, and gate deferred remote auto-resume on exact remote selection. Persistent-SSH attach only embeds agent startup when cwd is exact; legacy hook bindings without policy invalidate the restore recipe. Smaller related fixes: terminal hibernation before a native Ghostty surface exists, mobile terminal lanes failing without a provider, and CI adding non-tolerant regressions for these restore boundaries.Reviewed by Cursor Bugbot for commit 0698552. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests