Unify local resume launcher scripts (#9200) - #9205
Merged
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughSession restore now propagates restoring working directories through resume APIs, uses a shared secure one-shot Zsh launcher, moves directory handling into generated scripts, updates workspace restore ownership, and expands launcher, history, legacy-resume, and startup-input tests. ChangesSession resume launcher flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workspace
participant DockSplitStore
participant ResumePolicy
participant LauncherStore
participant LoginShell
Workspace->>ResumePolicy: build resume launch with restoring cwd
DockSplitStore->>ResumePolicy: request terminal startup input
ResumePolicy->>LauncherStore: create invocation launcher
LauncherStore-->>DockSplitStore: return /bin/zsh script invocation
DockSplitStore->>LoginShell: inject launcher invocation
LoginShell->>LauncherStore: execute and delete launcher
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/AgentResumeReturnShellStartupTests.swift`:
- Line 153: Update all four Data-to-String conversions in
AgentResumeReturnShellStartupTests, including the sites around presentOutput and
the other referenced lines, to use the failable String initializer instead of
String(decoding:as:). Preserve the existing UTF-8 decoding behavior and handle
the optional result appropriately at each call site.
- Around line 134-141: Guard the inaccessible-directory test around the setup
using the existing file-manager and test symbols so it is skipped or returned
early when running with root privileges, where 0o000 remains accessible. Apply
the same guard to the related case noted around the second range, while
preserving the permission restoration and cleanup defer behavior.
In `@cmuxTests/SessionPersistenceTests.swift`:
- Around line 2566-2568: Clean up launcher scripts materialized by the
resume-startup tests around resumeStartupInput and
inlineResumeCommandResolvingLauncherScript. Configure these tests to use a
per-test temporary launcher directory and remove it during teardown, or
explicitly delete each generated script after inspection, including the
additional setup at the referenced second location.
In `@Sources/OneShotTerminalLauncherStore.swift`:
- Around line 18-31: Move the Logger initialization out of the
OneShotTerminalLauncherStore initializer into a file-scoped shared declaration
named logger, using nonisolated private let and the existing "com.cmuxterm.app"
subsystem and category. Remove the instance logger property and its initializer
assignment while leaving the remaining initialization unchanged.
In `@Sources/Workspace.swift`:
- Around line 1442-1444: Update the auto-resume path in Workspace around
restorableAgent?.resumeStartupInput to pass nil for restoringWorkingDirectory
when the saved agent cwd is missing or not enterable, matching
DockSplitStore+SessionRestore.swift; retain resumeSessionWorkingDirectory only
when it is valid for Ghostty to enter.
🪄 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: e74e3f85-bdc1-4208-9b2f-3a5de382ea40
📒 Files selected for processing (19)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Session/WorkspaceSessionRestorePolicyService.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Session/WorkspaceSurfaceResumeBinding.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Session/WorkspaceSessionRestorePolicyServiceTests.swiftSources/AgentRelaunchCommandBuilder.swiftSources/DockSplitStore+SessionRestore.swiftSources/OneShotTerminalLauncherStore.swiftSources/RestorableAgentSession.swiftSources/SessionPersistence.swiftSources/SessionRestorableAgentSnapshot+Commands.swiftSources/SessionRestoredTerminalCommandStore.swiftSources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swiftSources/Workspace.swiftSources/WorkspaceSurfaceResumeStartupLaunch+WorkingDirectory.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AgentResumeReturnShellStartupTests.swiftcmuxTests/ForkParentFallbackResidualTests.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swiftcmuxTests/WorkspaceUnitTests.swift
💤 Files with no reviewable changes (2)
- Sources/SessionRestoredTerminalCommandStore.swift
- Sources/WorkspaceSurfaceResumeStartupLaunch+WorkingDirectory.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/OneShotTerminalLauncherStore.swift (1)
57-60: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not treat an inaccessible ancestor as a missing directory.
At line 59,
cd -- /tmp/root/blocked/childfails becauseblockedis not searchable;[ ! -d /tmp/root/blocked/child ]also fails, so! -dis false and the shell continues from the host cwd instead of failing. Drop the fallback whencdfails for access/readability, and only fall back when a path component is positively missing. Add a component-wise accessibility check in a subshell and cover this nested-inaccessible-cwd case.🤖 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/OneShotTerminalLauncherStore.swift` around lines 57 - 60, Update the working-directory command generation in OneShotTerminalLauncherStore to stop treating an inaccessible ancestor as a missing directory. Replace the broad `[ ! -d ]` fallback with a subshell component-wise check that only permits continuation when a path component is positively absent, while propagating failures caused by inaccessible ancestors; add coverage for a nested inaccessible working directory.
🤖 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.
Outside diff comments:
In `@Sources/OneShotTerminalLauncherStore.swift`:
- Around line 57-60: Update the working-directory command generation in
OneShotTerminalLauncherStore to stop treating an inaccessible ancestor as a
missing directory. Replace the broad `[ ! -d ]` fallback with a subshell
component-wise check that only permits continuation when a path component is
positively absent, while propagating failures caused by inaccessible ancestors;
add coverage for a nested inaccessible working directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d3e99fe3-c29d-41d2-ad10-b48f1b6d6be5
📒 Files selected for processing (4)
Sources/OneShotTerminalLauncherStore.swiftSources/RestorableAgentSession.swiftSources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swiftcmuxTests/AgentResumeReturnShellStartupTests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
1353-1367: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
cwd: .ignorebefore applying a binding fallback.A registered agent with
cwd: .ignorecan still have an approved local binding. These fallbacks then passworkingDirectoryinto the launcher and host shell, despite.ignorerequiring no cwd guard or placement cwd.
Sources/Workspace.swift#L1353-L1367: passnilasrestoringWorkingDirectorywhenrestorableAgent.registration?.cwd == .ignore, rather than falling back to a saved/current directory.Sources/DockSplitStore+SessionRestore.swift#L181-L187: apply the same suppression and add coverage for local binding restore with.ignore.Based on learnings, registrations with
cwd: .ignoremust suppress both the resume cwd guard and terminal working-directory placement.🤖 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/Workspace.swift` around lines 1353 - 1367, Update the startup restore logic around candidateBindingWorkingDirectory and sessionRestorePolicy.surfaceResumeStartupLaunch in Sources/Workspace.swift:1353-1367 to pass nil whenever restorableAgent.registration?.cwd == .ignore, before applying saved or current-directory fallbacks. Apply the same suppression in Sources/DockSplitStore+SessionRestore.swift:181-187 and add coverage for local binding restoration with cwd: .ignore, ensuring both the resume cwd guard and terminal working-directory placement are skipped.Source: 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.
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 1353-1367: Update the startup restore logic around
candidateBindingWorkingDirectory and
sessionRestorePolicy.surfaceResumeStartupLaunch in
Sources/Workspace.swift:1353-1367 to pass nil whenever
restorableAgent.registration?.cwd == .ignore, before applying saved or
current-directory fallbacks. Apply the same suppression in
Sources/DockSplitStore+SessionRestore.swift:181-187 and add coverage for local
binding restoration with cwd: .ignore, ensuring both the resume cwd guard and
terminal working-directory placement are skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e24d92a3-c4c4-489e-ad04-8becd71f8e59
📒 Files selected for processing (6)
Sources/DockSplitStore+SessionRestore.swiftSources/OneShotTerminalLauncherStore.swiftSources/Workspace.swiftcmuxTests/AgentResumeReturnShellStartupTests.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
3624-3624: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRemove the time-based layout repair watchdog.
This introduces a two-second delayed teardown for unresolved rendering/focus work, so slow realization can be abandoned solely because the deadline elapsed. End the follow-up through explicit convergence or owner-lifecycle cancellation instead of a timer-driven repair path.
As per coding guidelines, “Do not introduce or materially expand timing or blocking repair paths such as …
Task.sleep, polling … to paper over lifecycle, focus, rendering … races.”Also applies to: 10364-10371
🤖 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/Workspace.swift` at line 3624, Remove the layoutFollowUpTimeoutScheduler and all timer-driven layout repair logic, including the related follow-up path around the layout convergence handling. End unresolved rendering/focus work only through explicit convergence or owner-lifecycle cancellation, without introducing delayed teardown, Task.sleep, polling, or other time-based fallbacks.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Workspace.swift`:
- Line 3624: Remove the layoutFollowUpTimeoutScheduler and all timer-driven
layout repair logic, including the related follow-up path around the layout
convergence handling. End unresolved rendering/focus work only through explicit
convergence or owner-lifecycle cancellation, without introducing delayed
teardown, Task.sleep, polling, or other time-based fallbacks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b5ef8e82-5377-4b4c-b1ed-17b437bd95c3
📒 Files selected for processing (4)
Sources/OneShotTerminalLauncherStore.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AgentResumeReturnShellStartupTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/AgentSessionAutoResumeSettingsTests.swift`:
- Around line 257-260: Extend the resume test around
resumeStartupInput(restoringWorkingDirectory:) to inspect the generated input
and assert that it contains remoteWorkingDirectory plus the corresponding cd
guard. Keep the existing restoredPanel.requestedWorkingDirectory assertion, but
verify the actual resume command input uses the remote cwd rather than only
checking surface metadata.
🪄 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: 185a8ab8-a5ba-43b0-af4d-a839df36c7d2
📒 Files selected for processing (2)
Sources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSettingsTests.swift
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/bin/zsh <private-script>invocation.ignorebehaviorValidation
git diff --checkCloses #9200
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Unifies all local resume flows behind a single history-hidden " /bin/zsh '<script>'" wrapper and moves cwd guarding into a new
OneShotTerminalLauncherStore. Avoids nested login shells and keeps working-directory delivery consistent across local and remote restore for #9200.OneShotTerminalLauncherStore(0700 dir, 0600 scripts, self-delete, TTL pruning) with direct and user-login-shell modes; local resumes use direct to avoid nested login; startup input begins with a leading space to hide in shell history.restoringWorkingDirectorythroughWorkspaceSessionRestorePolicyServiceandWorkspaceSurfaceResumeBinding.startupInputWithLauncherScript(..., restoringWorkingDirectory:); relaunch/resume builders gainedincludeWorkingDirectoryPrefixso launcher-aware commands omit cwd prefixes.Workspace.RestoredWorkingDirectoryGuardto preserve logical path spelling and handle symlinks/unmounted volumes.tmux/HUD starts now useOneShotTerminalLauncherStore.writeStartupCommand(...).SessionRestoredTerminalCommandStoreandWorkspaceSurfaceResumeStartupLaunch+WorkingDirectory.Written for commit cbe6c5f. Summary will update on new commits.
Summary by CodeRabbit
New Features
cdand self-delete after execution, with distinct “direct” and “login shell” modes.Bug Fixes
Tests