Repository navigation
Fix fish Claude restore fallback - #6954
austinywang wants to merge 45 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughLaunch trust now uses instance-based helpers across classification, resume argv building, and trusted-launch checks. Workspace restore and snapshot logic now compare binding freshness against restorable agent captures before reusing bindings. ChangesAgent launch trust and restore precedence
Sequence Diagram(s)sequenceDiagram
participant Workspace.sessionPanelSnapshot
participant resumeBindingForSessionSnapshot
participant shouldPreferRestorableAgentSnapshot
participant SessionTerminalPanelSnapshot
Workspace.sessionPanelSnapshot->>resumeBindingForSessionSnapshot: derive snapshotResumeBinding
resumeBindingForSessionSnapshot->>shouldPreferRestorableAgentSnapshot: compare binding.updatedAt and launch capture time
shouldPreferRestorableAgentSnapshot-->>resumeBindingForSessionSnapshot: prefer or keep binding
resumeBindingForSessionSnapshot-->>Workspace.sessionPanelSnapshot: snapshotResumeBinding
Workspace.sessionPanelSnapshot->>SessionTerminalPanelSnapshot: initialize with snapshotResumeBinding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
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📝 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 |
|
@codex review |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 5 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
…ell-integration-ships-so-claude-byp
…ell-integration-ships-so-claude-byp
Greptile SummaryFixes the fish-shell (and similar) session-restore regression where a
Confidence Score: 5/5Safe to merge; the fix correctly sanitizes shell-dispatch captures and prefers fresher agent snapshots without discarding working directory or environment data. The shell-flag parser in The Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Hook as Agent Hook
participant Trust as AgentLaunchCaptureTrust
participant TLC as trustedLaunchCommand
participant SSPR as shouldPreferRestorableAgentSnapshot
participant Resume as AgentResumeCommandBuilder
Hook->>Trust: argvLooksLikeShellWrapper(["bash","--noprofile","--norc","-c","exec fish"])
Trust-->>Hook: true (shell dispatcher)
Hook->>TLC: "launchCommand {exec=bash, args=[...]}"
TLC->>Trust: launcherDescribesKind("claude", kind:"claude")
Trust-->>TLC: true
TLC->>Trust: argvLooksLikeShellWrapper(args)
Trust-->>TLC: true
TLC-->>Hook: "sanitized {exec=nil, args=[], cwd=launchCwd, env=CLAUDE_CONFIG_DIR}"
Note over Hook,Resume: At restore time
Hook->>SSPR: "binding(kind=claude, cmd=bash --resume SID, updatedAt=T1) vs snapshot(kind=claude, capturedAt=T2>T1)"
SSPR->>SSPR: commandLooksLikePoisonedShellResumeBinding?
SSPR-->>Hook: "true — prefer snapshot, binding=nil"
Hook->>Resume: "resumeShellCommand(kind=claude, launchCommand=sanitized)"
Resume->>Trust: argvLooksLikeShellWrapper([]) — false
Resume-->>Hook: claude --resume SID (fallback executable)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Hook as Agent Hook
participant Trust as AgentLaunchCaptureTrust
participant TLC as trustedLaunchCommand
participant SSPR as shouldPreferRestorableAgentSnapshot
participant Resume as AgentResumeCommandBuilder
Hook->>Trust: argvLooksLikeShellWrapper(["bash","--noprofile","--norc","-c","exec fish"])
Trust-->>Hook: true (shell dispatcher)
Hook->>TLC: "launchCommand {exec=bash, args=[...]}"
TLC->>Trust: launcherDescribesKind("claude", kind:"claude")
Trust-->>TLC: true
TLC->>Trust: argvLooksLikeShellWrapper(args)
Trust-->>TLC: true
TLC-->>Hook: "sanitized {exec=nil, args=[], cwd=launchCwd, env=CLAUDE_CONFIG_DIR}"
Note over Hook,Resume: At restore time
Hook->>SSPR: "binding(kind=claude, cmd=bash --resume SID, updatedAt=T1) vs snapshot(kind=claude, capturedAt=T2>T1)"
SSPR->>SSPR: commandLooksLikePoisonedShellResumeBinding?
SSPR-->>Hook: "true — prefer snapshot, binding=nil"
Hook->>Resume: "resumeShellCommand(kind=claude, launchCommand=sanitized)"
Resume->>Trust: argvLooksLikeShellWrapper([]) — false
Resume-->>Hook: claude --resume SID (fallback executable)
Reviews (28): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes fish-shell bootstrap poisoning of agent resume argv (issue #5796). When the cmux launch-capture PID fallback resolved to a
Confidence Score: 4/5The fix is targeted and well-tested; the two new regression tests cover the fish-bootstrap argv and the stale-poisoned-binding paths end-to-end. The only open question is whether a newer snapshot for a different agent kind should silently win over an older binding of a different kind (old code failed closed; new code resumes with the snapshot's kind). Both production paths ( Sources/Workspace.swift — specifically Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Session Restore: resumeBindingForSessionRestore()"] --> B{binding exists?}
B -- No --> Z["return nil (no restore)"]
B -- Yes --> C{isAgentHookBinding AND\nshouldPreferRestorableAgentSnapshot?}
C -- Yes --> D["return nil\n(discard stale binding)"]
C -- No --> E{isAgentHookBinding AND\nrestorableAgent exists?}
E -- No --> F["return binding as-is"]
E -- Yes --> G{checkpointId == sessionId?}
G -- No --> F
G -- Yes --> H{kind match?}
H -- No --> F
H -- Yes --> I["return binding with\nreconciled working directory"]
D --> J["restorableAgentForSessionRestore(snapshot, nil)"]
J --> K["return snapshot as-is"]
subgraph commandParts
L["executablePath or argv[0]"] --> M{executableLooksLikeShell?}
M -- Yes --> N["return (fallbackExecutable, [])"]
M -- No --> O["return (executable, tail)"]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["Session Restore: resumeBindingForSessionRestore()"] --> B{binding exists?}
B -- No --> Z["return nil (no restore)"]
B -- Yes --> C{isAgentHookBinding AND\nshouldPreferRestorableAgentSnapshot?}
C -- Yes --> D["return nil\n(discard stale binding)"]
C -- No --> E{isAgentHookBinding AND\nrestorableAgent exists?}
E -- No --> F["return binding as-is"]
E -- Yes --> G{checkpointId == sessionId?}
G -- No --> F
G -- Yes --> H{kind match?}
H -- No --> F
H -- Yes --> I["return binding with\nreconciled working directory"]
D --> J["restorableAgentForSessionRestore(snapshot, nil)"]
J --> K["return snapshot as-is"]
subgraph commandParts
L["executablePath or argv[0]"] --> M{executableLooksLikeShell?}
M -- Yes --> N["return (fallbackExecutable, [])"]
M -- No --> O["return (executable, tail)"]
end
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swift`:
- Around line 101-107: The shell-wrapper detection in AgentLaunchCaptureTrust
should not stop at the first non-flag argument, because value-taking shell
options like "-o" can make operands such as "pipefail" look like the command
payload. Update the argument scan in the trust-check logic to recognize known
shell options that consume a following value before deciding to return false, so
cases like bash "-o" "pipefail" "-c" "exec claude" are still detected as shell
bootstrap commands. Use the existing shellCommandStringFlag path as the anchor
and extend the loop to parse option/value pairs before treating a non-flag as
the payload.
In `@Sources/RestorableAgentSession.swift`:
- Around line 1263-1265: `trustedLaunchCommand` is creating a new
`AgentLaunchCaptureTrust` for every record, which repeats its lookup-table
initialization on the load path. Hoist or reuse a single
`AgentLaunchCaptureTrust` instance in the restore flow and pass it into
`trustedLaunchCommand` (or otherwise cache it in the caller) so the trust checks
use the shared instance instead of constructing one per session record.
🪄 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
Run ID: bed5fcee-31b5-413c-a931-687b3951dcae
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (9)
CLI/cmux.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeArgv.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchCaptureTrustTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeArgvTests.swiftSources/RestorableAgentSession.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftscripts/lint-namespace-types-baseline.txt
💤 Files with no reviewable changes (1)
- scripts/lint-namespace-types-baseline.txt
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swift (1)
99-116: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValue-taking shell options still defeat detection.
["bash", "-o", "pipefail", "-c", "exec claude"]returnsfalse:-ois not a command-string flag nor a startup-only flag, so the loop hitspipefail(a non-flag operand) and returnsfalse, letting a shell bootstrap survive the restore trust filter. Skip operands for known value-taking shell options (-o,-cvalue handling) before treating the first non-flag token as the command payload.🤖 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/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swift` around lines 99 - 116, The shell-wrapper detection in argvLooksLikeShell currently treats value-taking options like -o and -c as if the next token were a non-flag operand, which can incorrectly return false for real shell invocations. Update argvLooksLikeShell to recognize known value-taking shell options and skip their associated argument(s) before applying the non-flag/-- rejection logic, while keeping the existing shellCommandStringFlag and shellStartupOnlyFlag checks intact. Use the existing symbols argvLooksLikeShell, shellCommandStringFlag, shellStartupOnlyFlag, and executableLooksLikeShell to locate and adjust the parsing flow.
🤖 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
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swift`:
- Around line 110-113: The `isValueTakingOption` logic in
`AgentLaunchCaptureTrust` has a redundant `if` branch because both paths return
false, so the `--` versus non-flag distinction is currently ignored. Either
restore the intended special handling for `--`/non-flag arguments in that
method, or simplify the `argument == "--" || !argument.hasPrefix("-")` check to
a single unconditional false return if no differentiation is needed.
---
Duplicate comments:
In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swift`:
- Around line 99-116: The shell-wrapper detection in argvLooksLikeShell
currently treats value-taking options like -o and -c as if the next token were a
non-flag operand, which can incorrectly return false for real shell invocations.
Update argvLooksLikeShell to recognize known value-taking shell options and skip
their associated argument(s) before applying the non-flag/-- rejection logic,
while keeping the existing shellCommandStringFlag and shellStartupOnlyFlag
checks intact. Use the existing symbols argvLooksLikeShell,
shellCommandStringFlag, shellStartupOnlyFlag, and executableLooksLikeShell to
locate and adjust the parsing flow.
🪄 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
Run ID: 72ab95cf-efa7-4ae3-9b00-fad16ea566b2
📒 Files selected for processing (6)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchCaptureTrustTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeArgvTests.swiftSources/RestorableAgentSession.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RestorableAgentLaunchTrustTests.swift
…ell-integration-ships-so-claude-byp # Conflicts: # .github/swift-file-length-budget.tsv
…ell-integration-ships-so-claude-byp
…ell-integration-ships-so-claude-byp # Conflicts: # .github/swift-file-length-budget.tsv
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Fixes #5796
Summary
Validation
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fix auto-restore when a shell/login dispatcher is captured by falling back to the agent launcher so Claude and Codex resume correctly while keeping cwd and env. Prefer newer same‑kind launch snapshots over shell‑poisoned or legacy resume bindings; preserve shell‑named wrappers that aren’t dispatchers. Fixes #5796.
AgentLaunchCaptureTrustused across CLI, resume argv, restore, and sessions list; addsexecutableLooksLikeShelland improves argv scanning to handle combined/split startup flags and option operands (-l/-i/-m/-s,--noprofile/--norc/--no-rcs/--no-config,-o,--rcfile).sh -c …,zsh -lc …), resume falls back to the agent verb and clears args; shell‑named wrappers that aren’t dispatchers are preserved.updatedAt=0) or when the session id matches; detection guards only shell‑poisoned bindings and wrapper tokens (claude-teams/codex-teams), cross‑kind or kind‑less bindings remain.CFFIXED_USER_HOMEand forwardsCARGO_HOME/RUSTUP_HOME.Written for commit b69993b. Summary will update on new commits.
Summary by CodeRabbit
Testingframework.