Fix Claude failed resume clobbering surface restore#6014
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR gates the publishing of Claude surface resume bindings ( ChangesResume Binding Eligibility Gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 (19 passed)
✨ Finishing Touches🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 84d4644. Configure here.
Greptile SummaryPrevents a failed
Confidence Score: 4/5Safe to merge for the common production path; the guard correctly blocks the clobbering bug for any hook invocation that carries CMUX_AGENT_LAUNCH_ARGV_B64 or a readable PID. The new helper correctly suppresses the resume binding on failed-resume hooks in the normal code path. The one gap — failing open when launch argv is entirely unresolvable — is an unusual fallback scenario and doesn't affect the well-tested primary path. No actor isolation, logging, or internationalization concerns were introduced. CLI/cmux.swift — specifically the fail-open return path in shouldPublishClaudeResumeBinding when neither CMUX_AGENT_LAUNCH_ARGV_B64 nor a valid PID is available. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Hook fires: SessionStart / UserPromptSubmit / Stop"] --> B{"parsedInput.sessionId present?"}
B -- No --> Z["Skip binding logic"]
B -- Yes --> C["shouldPublishClaudeResumeBinding()"]
C --> D{"normalizedSessionId valid?"}
D -- No --> E["return false"]
D -- Yes --> F{"transcript path present?\n(parsedInput or mappedSession)"}
F -- Yes --> G["return true"]
F -- No --> H{"claudeResumeLaunchSessionId() == sessionId?"}
H -- "Yes (failed resume)" --> I["return false\n→ skip isRestorable=true\n→ skip surface.resume.set"]
H -- "No / nil (fresh session)" --> G
G --> J["upsert isRestorable=true\npublishAgentSurfaceResumeBinding"]
I --> K["upsert isRestorable=false\n(no binding publish)"]
subgraph "claudeResumeLaunchSessionId"
L["Read CMUX_AGENT_LAUNCH_ARGV_B64\nor processArguments(pid)"] --> M{"--resume / -r found?"}
M -- Yes --> N["return normalizedHookValue(id)"]
M -- No / unavailable --> O["return nil → guard fails open"]
end
Reviews (1): Last reviewed commit: "Refresh Swift file length budget" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if let resumedSessionId = claudeResumeLaunchSessionId(env: env, fallbackPID: fallbackPID), | ||
| resumedSessionId == normalizedSessionId { | ||
| return false | ||
| } | ||
| return true |
There was a problem hiding this comment.
claudeResumeLaunchSessionId returns nil if CMUX_AGENT_LAUNCH_ARGV_B64 is absent and processArguments(for:) cannot read the PID (e.g., claudePid is nil). When both fallbacks fail, the code skips the resume-id comparison and falls through to return true, publishing the binding for a failed resume — the exact outcome the PR is trying to prevent. The bug is only fully suppressed when at least one argv source is reliably available on the hook invocation path.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@CLI/cmux.swift`:
- Around line 22974-23006: The resume parsing should return on the first match
and avoid recreating the prefix list each iteration: in
claudeResumeLaunchSessionId use a static constant for the prefixes (e.g., let
resumePrefixes = ["--resume=", "-r="]) and iterate using that constant, ensuring
you return immediately for the first "--resume"/"-r" positional match or the
first prefix match; also update the other resume parsing block (the second parse
near the later code) to include "-r=" in its hasPrefix checks so short-form
equals-style flags are handled consistently; keep existing calls to
claudeRawLaunchArguments, normalizedHookValue and the early-return behavior in
shouldPublishClaudeResumeBinding unchanged.
🪄 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: 804c3bfa-611b-43c2-b223-939f27595caf
📒 Files selected for processing (2)
CLI/cmux.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
| private func shouldPublishClaudeResumeBinding( | ||
| sessionId: String, | ||
| parsedInput: ClaudeHookParsedInput, | ||
| mappedSession: ClaudeHookSessionRecord?, | ||
| env: [String: String], | ||
| fallbackPID: Int? | ||
| ) -> Bool { | ||
| guard let normalizedSessionId = normalizedHookValue(sessionId) else { return false } | ||
| if normalizedHookValue(parsedInput.transcriptPath ?? mappedSession?.transcriptPath) != nil { | ||
| return true | ||
| } | ||
| if let resumedSessionId = claudeResumeLaunchSessionId(env: env, fallbackPID: fallbackPID), | ||
| resumedSessionId == normalizedSessionId { | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| private func claudeResumeLaunchSessionId(env: [String: String], fallbackPID: Int?) -> String? { | ||
| guard let arguments = claudeRawLaunchArguments(env: env, fallbackPID: fallbackPID) else { | ||
| return nil | ||
| } | ||
| for (index, argument) in arguments.enumerated() { | ||
| if argument == "--resume" || argument == "-r" { | ||
| guard index + 1 < arguments.count else { return nil } | ||
| return normalizedHookValue(arguments[index + 1]) | ||
| } | ||
| for prefix in ["--resume=", "-r="] where argument.hasPrefix(prefix) { | ||
| return normalizedHookValue(String(argument.dropFirst(prefix.count))) | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
Does Claude CLI support multiple --resume flags, and if so which one takes precedence?
💡 Result:
The Claude CLI does not support passing multiple --resume flags in a single command. The --resume flag is designed to accept a single argument—either a specific session ID, a session name, or no argument at all to trigger an interactive session picker [1][2]. If you attempt to provide multiple --resume flags, it is not a supported use case, and the behavior is not explicitly defined in the official documentation [1][2]. To resume a session, you should use the flag once, followed by the desired session identifier [1][2]: claude --resume If you need to switch between sessions, you can use the interactive /resume picker within an active session or run the command without arguments to open the picker [2]. Additionally, you can use --continue to resume the most recent session in the current directory [2].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file
ls -la
# Search for CLI/cmux.swift existence
if [ -f "CLI/cmux.swift" ]; then
echo "FOUND: CLI/cmux.swift"
else
echo "NOT FOUND: CLI/cmux.swift; searching..."
fd -a "cmux.swift" -t f
fi
# Print relevant regions for resume parsing logic
rg -n --no-heading "claudeResumeLaunchSessionId|--resume|\\-r\\b" CLI/cmux.swift || true
# Show surrounding code for the function
if [ -f "CLI/cmux.swift" ]; then
line=$(rg -n "claudeResumeLaunchSessionId" CLI/cmux.swift | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-80))
end=$((line+220))
echo "---- Function region (approx) lines ${start}-${end} ----"
nl -ba CLI/cmux.swift | sed -n "${start},${end}p"
fi
echo "---- All nearby resume parsing blocks (context) ----"
rg -n --context 6 --no-heading "if argument == \"--resume\" \\|\\| argument == \"-r\"|hasPrefix\\(\"--resume=\"\\)|hasPrefix\\(\"-r=\"\\)|\\[\"--resume=\", \"-r=\"\\]" CLI/cmux.swift || true
fiRepository: manaflow-ai/cmux
Length of output: 6963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ -f "CLI/cmux.swift" ]; then
echo "FOUND: CLI/cmux.swift"
else
echo "NOT FOUND: CLI/cmux.swift"
fd -a -t f -e swift "cmux.swift"
fi
if [ -f "CLI/cmux.swift" ]; then
rg -n --no-heading "claudeResumeLaunchSessionId|--resume|-r\\b" CLI/cmux.swift || true
line=$(rg -n "claudeResumeLaunchSessionId" CLI/cmux.swift | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-80))
end=$((line+260))
echo "---- Function region (approx) lines ${start}-${end} ----"
nl -ba CLI/cmux.swift | sed -n "${start},${end}p"
fi
echo "---- Candidate resume parsing blocks with context ----"
rg -n --context 8 --no-heading "resume|--resume|\\-r\\b|normalizedHookValue" CLI/cmux.swift | head -n 200
fiRepository: manaflow-ai/cmux
Length of output: 1651
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify file and inspect relevant resume parsing blocks in CLI/cmux.swift
if [ -f "CLI/cmux.swift" ]; then
echo "FOUND: CLI/cmux.swift"
else
echo "NOT FOUND: CLI/cmux.swift"
fd -a -t f "cmux.swift"
exit 0
fi
echo "---- resume-related matches ----"
rg -n --no-heading "--resume|\\-r\\b|claudeResumeLaunchSessionId|normalizedHookValue" CLI/cmux.swift
echo "---- showing function claudeResumeLaunchSessionId (approx) ----"
fn_line=$(rg -n --no-heading "claudeResumeLaunchSessionId" CLI/cmux.swift | head -n1 | cut -d: -f1)
start=$((fn_line-60))
end=$((fn_line+320))
nl -ba CLI/cmux.swift | sed -n "${start},${end}p" | sed -n '1,260p'
echo "---- showing any other occurrences of '--resume'/'-r' parsing (context) ----"
rg -n --context 12 --no-heading "if argument == \"--resume\" \\|\\| argument == \"-r\"|hasPrefix\\(\"--resume=\"\\)|hasPrefix\\(\"-r=\"\\)" CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 598
Use first --resume occurrence; make prefix list a constant; check -r= consistency
- Keep the current precedence in
claudeResumeLaunchSessionId: return immediately on the first--resume/-rmatch (and first--resume=/-r=prefix match). - In
CLI/cmux.swiftthere’s a second resume parsing block around line ~25920: it checkshasPrefix("--resume=")but no correspondinghasPrefix("-r=")match is present—align support there if-r=should behave the same. - Refactor the
["--resume=", "-r="]prefix list inclaudeResumeLaunchSessionIdinto astaticconstant so it isn’t recreated during the scan.
🤖 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/cmux.swift` around lines 22974 - 23006, The resume parsing should return
on the first match and avoid recreating the prefix list each iteration: in
claudeResumeLaunchSessionId use a static constant for the prefixes (e.g., let
resumePrefixes = ["--resume=", "-r="]) and iterate using that constant, ensuring
you return immediately for the first "--resume"/"-r" positional match or the
first prefix match; also update the other resume parsing block (the second parse
near the later code) to include "-r=" in its hasPrefix checks so short-form
equals-style flags are handled consistently; keep existing calls to
claudeRawLaunchArguments, normalizedHookValue and the early-return behavior in
shouldPublishClaudeResumeBinding unchanged.
1 participant
Root cause
Claude can emit SessionStart and UserPromptSubmit hooks for
claude --resume <id>before it fails withNo conversation found with session ID. cmux treated those no-transcript hook events as restorable and publishedsurface.resume.set, so a failed resume attempt could overwrite the pane's durable resume binding with the missing id.This was not the old subrouter state-path bug. Recent subrouter work correctly shares Claude state under
_shared; the bad id had hook/session-env state but no transcript. The cmux hook layer was trusting the failed launch hooks too early.Fix
Only publish Claude resume bindings from SessionStart, UserPromptSubmit, or Stop when the hook has a transcript-backed session, or when the launch argv is not
claude --resume <same session id>. This preserves valid transcript-backed resumes and fresh sessions while leaving the prior durable binding intact for failed first-sighting resumes.Verification
xcodebuild -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -derivedDataPath /tmp/cmux-subres test -only-testing:cmuxTests/CLINotifyProcessIntegrationRegressionTests/testClaudeFailedResumePromptDoesNotPublishSurfaceResumeBinding -only-testing:cmuxTests/CLINotifyProcessIntegrationRegressionTests/testClaudePromptSubmitFromNewSessionCanReplaceStoppedSession./scripts/reload-cloud.sh --tag subresTagged build: http://127.0.0.1:17320/subres
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes when Claude sessions become restorable and when pane resume bindings are published—session restore behavior for edge cases around failed resumes.
Overview
Prevents failed
claude --resumeattempts from overwriting a pane’s durable resume binding when hooks fire before Claude errors with a missing session.Claude hook handling now uses
shouldPublishClaudeResumeBindingon SessionStart, UserPromptSubmit, and Stop.surface.resume.setandisRestorablerun only when the hook has a transcript path or the process was not launched with--resume/-rtargeting the same session id (parsed from launch argv/env). Session promotion on SessionStart is gated the same way.Adds an integration test for a failed first-sighting resume (no
surface.resume.set,isRestorable: false) and adjusts an existing test to userunClaudeHookWithoutServerwhere appropriate; the mock server stub now acceptssurface.resume.set.Reviewed by Cursor Bugbot for commit aff11e2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests