Add remote agent hooks to the cmux ssh relay - #8440
Closed
austinywang wants to merge 23 commits into
Closed
austinywang wants to merge 23 commits into
austinywang wants to merge 23 commits into
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:
📝 WalkthroughWalkthroughAdds remote hook installation and invocation across the Go relay and macOS CLI, including filesystem snapshots and mutations, chunked payload transfers, bounded process execution, socket-worker routing, localization, and integration tests. ChangesRemote hook relay bridge
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant HookProcess
participant RemoteCLI
participant RelaySession
participant SocketWorker
participant InvocationBridge
HookProcess->>RemoteCLI: Invoke hook event
RemoteCLI->>RelaySession: Send hooks.invoke* request
RelaySession->>SocketWorker: Route hook method
SocketWorker->>InvocationBridge: Validate and execute request
InvocationBridge-->>SocketWorker: Return output or error
SocketWorker-->>RelaySession: Return v2 result
RelaySession-->>RemoteCLI: Return relayed response
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 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 |
austinywang
marked this pull request as ready for review
July 19, 2026 02:34
Contributor
Greptile SummaryThis PR adds the missing
Confidence Score: 4/5Safe to merge with one fix: the CLI bridge error formatter embeds raw internal codes in user-visible terminal output. The remote hook relay is well-structured: path traversal guards, pre-flight state capture with rollback, slot-based transfer staging with claimed-slot retention, kqueue-based bounded subprocess capture, and correct socket-worker routing for all five hook methods. One issue prevents a clean merge: the shared remoteHookBridgeError helper formats every failure as a raw snake_case code visible on the user terminal, while every other CLI error in the same file uses a distinct localized user-readable sentence. CLI/CMUXCLI+RemoteHookBridge.swift — remoteHookBridgeError and its ~20 call sites need distinct localized, actionable messages instead of the shared %@ format with raw internal codes. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as User (Linux)
participant GoRelay as cmuxd-remote (Go)
participant Bridge as RemoteHookInvocationBridge (Swift)
participant CLI as bundled cmux CLI (Swift)
User->>GoRelay: cmux hooks omp install
GoRelay->>Bridge: hooks.invoke [__remote-describe, omp]
Bridge->>CLI: cmux hooks __remote-describe omp
CLI-->>Bridge: JSON descriptor
Bridge-->>GoRelay: descriptor
GoRelay->>GoRelay: snapshot local filesystem
GoRelay->>Bridge: hooks.invoke.begin
Bridge-->>GoRelay: transfer_id
loop Each 6 KiB chunk
GoRelay->>Bridge: hooks.invoke.append
Bridge-->>GoRelay: appended
end
GoRelay->>Bridge: hooks.invoke.execute
Bridge->>CLI: cmux hooks __remote-configure
CLI->>CLI: restoreSnapshot to runInstaller to diffFiles
CLI-->>Bridge: mutation plan JSON
Bridge-->>GoRelay: plan
GoRelay->>GoRelay: validate and applyMutations
GoRelay-->>User: Done: 1 installed, 0 skipped
%%{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 User as User (Linux)
participant GoRelay as cmuxd-remote (Go)
participant Bridge as RemoteHookInvocationBridge (Swift)
participant CLI as bundled cmux CLI (Swift)
User->>GoRelay: cmux hooks omp install
GoRelay->>Bridge: hooks.invoke [__remote-describe, omp]
Bridge->>CLI: cmux hooks __remote-describe omp
CLI-->>Bridge: JSON descriptor
Bridge-->>GoRelay: descriptor
GoRelay->>GoRelay: snapshot local filesystem
GoRelay->>Bridge: hooks.invoke.begin
Bridge-->>GoRelay: transfer_id
loop Each 6 KiB chunk
GoRelay->>Bridge: hooks.invoke.append
Bridge-->>GoRelay: appended
end
GoRelay->>Bridge: hooks.invoke.execute
Bridge->>CLI: cmux hooks __remote-configure
CLI->>CLI: restoreSnapshot to runInstaller to diffFiles
CLI-->>Bridge: mutation plan JSON
Bridge-->>GoRelay: plan
GoRelay->>GoRelay: validate and applyMutations
GoRelay-->>User: Done: 1 installed, 0 skipped
Reviews (6): Last reviewed commit: "fix(remote): encode empty hook arguments..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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. |
added 10 commits
July 18, 2026 20:11
![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 `@cmux.xcodeproj/project.pbxproj`:
- Around line 7510-7515: Update every PBXSourcesBuildPhase.files entry in
cmux.xcodeproj/project.pbxproj to reference its corresponding PBXBuildFile UUID:
change sites 7510-7515 to trailing IDs 2, A, 8, 6, 6, 4; site 5802 from ...B to
...0C; site 7870 from ...3 to ...04; site 8138 from ...1 to ...02; and sites
8167-8171 to trailing IDs 2, 8, A, 4, 6. Verify each referenced UUID maps to an
isa = PBXBuildFile declaration.
In `@Resources/Localizable.xcstrings`:
- Around line 240265-240276: Update the English values for
cli.hooks.setup.tooManyTargets and cli.hooks.setup.conflictingTargets to use
“hook targets” and “hook target” respectively, while preserving the existing
Japanese translations and the rest of each message.
In `@Sources/RemoteHookInvocationBridge`+Transfers.swift:
- Around line 80-93: Update takeTransfer in the chunked invocation path to
enforce the same per-hook input limit as decodeInvocation after reassembling the
input, using maximumHookInputBytes for non-__remote-configure methods while
preserving the larger maximumInputBytes staging limit. Widen
maximumHookInputBytes from private to instance-internal within the type so
takeTransfer can reference it, and apply the cap based on the decoded invocation
arguments or method.
🪄 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: fc1c17e1-2709-464f-9421-18d95ce107c3
📒 Files selected for processing (27)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+CodexFireAndForgetHooks.swiftCLI/CMUXCLI+RemoteHookBridge.swiftCLI/RemoteHookDescriptor.swiftCLI/RemoteHookMutation.swiftCLI/RemoteHookPlan.swiftCLI/RemoteHookSnapshot.swiftCLI/RemoteHookSnapshotEntry.swiftCLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swiftPackages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swiftResources/Localizable.xcstringsSources/RemoteHookInvocation.swiftSources/RemoteHookInvocationBridge+Process.swiftSources/RemoteHookInvocationBridge+Transfers.swiftSources/RemoteHookInvocationBridge.swiftSources/RemoteHookInvocationBridgeError.swiftSources/RemoteHookTransferMetadata.swiftSources/TerminalController+RemoteHookInvocation.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteHookInvocationBridgeTests.swiftdaemon/remote/cmd/cmuxd-remote/cli.godaemon/remote/cmd/cmuxd-remote/hooks.godaemon/remote/cmd/cmuxd-remote/hooks_relay_test.godaemon/remote/cmd/cmuxd-remote/hooks_unit_test.go
added 6 commits
July 18, 2026 21:45
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/RemoteHookInvocationBridgeTests.swift`:
- Around line 9-83: Add dispatch-level tests using
RemoteHookInvocationBridge.handle for "hooks.invoke.cancel" and
"hooks.invoke.execute", alongside the existing direct transfer tests. Verify
cancellation returns the expected "cancelled" result and execution releases the
claimed transfer slot via its deferred cleanup, covering the dispatch wiring
rather than only cancelTransfer and takeTransfer.
🪄 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: fa90c239-40ef-4099-92e1-d5a5237353ef
📒 Files selected for processing (10)
CLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swiftResources/Localizable.xcstringsSources/RemoteHookInvocationBridge+Transfers.swiftSources/RemoteHookInvocationBridge.swiftSources/TerminalController.swiftcmuxTests/RemoteHookInvocationBridgeTests.swiftdaemon/remote/cmd/cmuxd-remote/hooks.godaemon/remote/cmd/cmuxd-remote/hooks_relay_test.go
Contributor
Author
|
Automated review follow-up for 89e6d93:
The final canonical branch review reports no actionable findings, and cmux-policy-check reports no Aziz-derived findings. |
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. |
lawrencecchen
added
the
stale-revisit
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
3 participants
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
Closes #8396
Testing
Local Xcode tests and XCUITests were intentionally not run per the issue instructions; CI exercises the wired cmuxTests target.
Demo Video
N/A: this changes remote CLI and relay behavior, not UI. The requested cloud reload command is intentionally deferred until after CI handoff.
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by CodeRabbit
New Features
hooks <agent> <action>for install/uninstall and chunked hook invocation staging (begin/append/execute with cancellation).__remote-*) with config snapshot planning and mutation application.Bug Fixes
hooks.invoke*on the socket-worker lane.Tests
Documentation/Chores