Repository navigation
Remove dead resume code left by #14560 - #14693
Merged
Merged
Conversation
#14560 deleted the persistent-SSH resume binding path. This removes the code that path left unreachable: - willRunStartupCommand: every call site passed false. Drop the parameter from TerminalStartupRestoreCoordinator, PendingTerminalStartupRestore and RestoredAgentLifecycleCoordinator.seedSessionRestore, the startupCommand breadcrumb field, and the autoResumeCommandRunning seed branch. - setStartupRestoreAdmissionFallbackCommand and its stored var: no app caller. Cancelling deferred admission now always clears the command. - WorkspaceRemoteRelayCommandRewriter.authenticatesRemoteResumeParameters: test-only. Tests now assert the resume MAC is stamped instead. - ControlSurfaceResumeSetInputs.remoteRelayParameters: never read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (19)
💤 Files with no reviewable changes (12)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughStartup restore admission and lifecycle state now depend on startup input rather than startup command state. Admission cancellation clears the command override. Remote resume inputs no longer retain relay parameters, and the resume-parameter authentication verifier is removed. ChangesStartup Restore Admission
Remote Resume Authentication
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to No merge-blocking issue is established; the change is ready for normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The affected restore and remote-resume paths remain guarded, and this review found no new security bypass. Some security coverage is incomplete, so the assessment is not a clean bill of health. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
Contributor
|
Merge receipt for |
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 25, 2026
8538fa9 Add a Focus Last action that toggles between the two most recent focus positions (manaflow-ai#14700) fba6c47 Don't leak the host's TERM_PROGRAM/COLORTERM into remote PTY sessions (manaflow-ai#9610) 10cdafe agent-chat: skip the launchd PATH prefix on Windows so agent CLIs resolve (manaflow-ai#12206) 9c8039a Add Reveal in Finder to the terminal context menu (manaflow-ai#14697) d57f584 Remove dead resume code left by manaflow-ai#14560 (manaflow-ai#14693) 67debd6 Report the closed surface's own ref from surface.close (manaflow-ai#14698) a75ab64 test(ios): fix stale CmuxMobileShell connection-recovery tests (manaflow-ai#14691) ec7b2f1 Predicted echo: seed alternate screen from ghostty, guard stale erases (manaflow-ai#14686) 9aa850d refactor: move the Cloud surface models into CmuxCloud (manaflow-ai#14390) da468e1 ci(e2e): order sibling waits by attempt start, adopt main's seed product (manaflow-ai#14684) ff02854 Request badge authorization so the Dock badge renders (manaflow-ai#14242) dc89b82 ci: iOS picker mints the routing token with the org runner permission (manaflow-ai#14690) 9650672 ci: label the org glaeda-minis runners with warm keys (manaflow-ai#14679) 992a2de Remove unreachable persistent-SSH resume binding code (manaflow-ai#14560) 6b4d976 ci(ios): read idle simulator minis live from the runners API (manaflow-ai#14542) 2f5d439 fix(ios): align hidden-marker and Iroh aggregation tests with build identity (manaflow-ai#14535) bcf0122 Start a never-shown terminal before surface.read_text and read_screen (manaflow-ai#14673) 60469a3 ci: reuse unit xctestrun for numeric locale tests (manaflow-ai#13414) 2d1bf1b ci: give the receipt contract's guard fixture every workflow (manaflow-ai#14685) ec52ce4 test: give the restore surface-context test its own socket (manaflow-ai#14680) # Conflicts: # .github/workflows/ci-cache-receipts.yml # .github/workflows/ci-owned-warm-labels.yml # .github/workflows/seed-derived-data.yml # .github/workflows/test-e2e.yml # .github/workflows/test-ios.yml
teamleaderleo
added a commit
that referenced
this pull request
Sep 25, 2026
Stops attaching the per-resume relay MAC (_cmux_remote_relay_authentication_code). Nothing verified it after #14693. Ingress still strips the field from old senders; the relay-wide request MAC is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Follow-up to #14560, which deleted the persistent-SSH resume binding path. Each item below was checked with
git grepon current main before removal.willRunStartupCommand: every call site (Workspace.swift, DockSplitStore.swift, DockSplitStore+SessionRestore.swift) passedfalse. Removed the parameter fromTerminalStartupRestoreCoordinator.runtimeSpawnPolicyandstage, thePendingTerminalStartupRestorefield, thestartupCommandbreadcrumb field, and theRestoredAgentLifecycleCoordinator.seedSessionRestorebranch that seeded.autoResumeCommandRunning.willRunStartupCommand || willRunStartupInputis now justwillRunStartupInput. Tests that passedwillRunStartupCommand: falsewere updated.setStartupRestoreAdmissionFallbackCommand(CmuxTerminal): it had no app caller. Removed it and thestartupRestoreAdmissionFallbackCommandstored var. Cancelling deferred admission now sets the command override tonil, which is what it already did with no fallback set. The package test that set a fallback now asserts that cancelling drops the resume command (overridenil).WorkspaceRemoteRelayCommandRewriter.authenticatesRemoteResumeParameters: only tests called it. Removed it and the assertions that exercised it inRemoteResumeBindingTests. The two affected tests now check that the rewriter still stamps the resume MAC onsurface.resume.set(including the escaped method name) and only on the exact method. All remaining private helpers are still used.ControlSurfaceResumeSetInputs.remoteRelayParameters: nothing read it. Removed the field, the init parameter, and the arguments atControlCommandCoordinator+Surface3.swiftandGhosttyNSView+ForkConversationContextMenu.swift.SurfaceResumeLaunchFlavor.persistentSSHencode/decode is unchanged. No files were deleted, so the pbxproj is untouched.Out of scope: the rewriter still stamps
_cmux_remote_relay_authentication_codeon relayedsurface.resume.set, and the app strips it on ingress, but nothing verifies it anymore. Removing that is a wire-format change, so it should be its own PR.Verification:
swift buildfor CmuxControlSocket passes. Agit grepsweep finds no remaining references to the removed symbols. The app and test targets are left to CI.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Follow-up to #14560 that removes code left unreachable after the persistent-SSH resume binding path was deleted.
willRunStartupCommandwas alwaysfalseat every call site; the parameter, the.autoResumeCommandRunningseed branch, and thestartupCommandbreadcrumb field are gone.setStartupRestoreAdmissionFallbackCommandhad no app callers; cancelling deferred admission now always clears the command override, matching prior runtime behavior.authenticatesRemoteResumeParameterswas test-only; the tests now assert the resume MAC is stamped onsurface.resume.setfor the exact method only.remoteRelayParameterswas never read; the field, init parameter, and call-site arguments are removed.The relayed resume MAC is still stamped and stripped on ingress, but nothing verifies it anymore; removing that is a wire-format change left for a separate PR.
Written for commit 04b590f. Summary will update on new commits.
Summary by CodeRabbit