Repository navigation
Forward legacy cmux <agent>-hook commands so stale Cursor hooks return valid JSON - #4158
lawrencecchen wants to merge 2 commits into
Conversation
Stale `cmux <agent>-hook` entries in `~/.cursor/hooks.json` (and other agent hook configs) still invoke the legacy command names removed in commit 6beb3db. The unknown-command path prints CLI usage to stdout and exits non-zero, which combines with `|| echo '{}'` to produce "usage text + {}", which Cursor agent rejects as invalid JSON and uses to block every shell execution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Commit 6beb3db ("Namespace agent hook commands") renamed the per-agent hook commands from `cmux <agent>-hook <subcommand>` to `cmux hooks <agent> <subcommand>` and kept backwards-compat only for `codex-hook` and `feed-hook`. Every other agent (cursor, gemini, opencode, copilot, codebuddy, factory, qoder) lost its legacy alias. Stale entries in `~/.<agent>/hooks.json` still invoke the old names. The unknown-command path prints the full CLI usage to stdout and exits non-zero, which combines with the `|| echo '{}'` fallback in the installed hook command to produce "usage text + {}". Cursor agent rejects that as invalid JSON and blocks every shell execution. Detect any `<agent>-hook` form for a registered agent and forward it to `runGenericAgentHook`, mirroring the existing codex-hook compat shim across all agents. Generalize `isLegacyCmuxOwnedHookCommand` and the hook markers to recognize the legacy form per agent so reinstall purges stale entries instead of layering new ones beside them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR generalizes legacy ChangesLegacy agent-hook alias generalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (14 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 |
![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 f8a097a. Configure here.
| } | ||
| if command == "setup-hooks" || command == "uninstall-hooks" { try runSetupHooks(uninstall: command == "uninstall-hooks"); return } // Backwards compatibility for old hook setup docs/scripts. | ||
| if (command == "codex-hook" || command == "feed-hook"), processEnv["CMUX_SURFACE_ID"]?.isEmpty != false, processEnv["CMUX_WORKSPACE_ID"]?.isEmpty != false, | ||
| if Self.legacyAgentNameFromHookCommand(command) != nil, processEnv["CMUX_SURFACE_ID"]?.isEmpty != false, processEnv["CMUX_WORKSPACE_ID"]?.isEmpty != false, |
There was a problem hiding this comment.
feed-hook lost early-return {} outside cmux terminals
High Severity
The early-return guard that prints {} for hook commands running outside cmux terminals previously checked command == "codex-hook" || command == "feed-hook". The replacement uses Self.legacyAgentNameFromHookCommand(command), which explicitly returns nil for "feed". This means feed-hook invocations without CMUX_SURFACE_ID/CMUX_WORKSPACE_ID will no longer early-exit with {} — they'll fall through to socket connection, fail, and throw an error or block.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f8a097a. Configure here.
Greptile SummaryThis PR forwards legacy
Confidence Score: 3/5The core alias-forwarding fix is correct and well-tested, but feed-hook's graceful no-context exit was inadvertently removed and should be restored before merging. The agent-hook alias forwarding works correctly for all targeted agents and the new tests cover it well. However, the refactored no-context early-exit condition silently drops feed-hook coverage — users with bare cmux feed-hook entries lacking a shell-level CMUX_SURFACE_ID guard would now hit a socket connection attempt and throw instead of receiving {} immediately. CLI/cmux.swift line 2609 — the early no-context exit condition needs feed-hook added back explicitly alongside the new generic check. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["cmux <command> ..."] --> B{legacyAgentNameFromHookCommand\nreturns non-nil?}
B -- "Yes (e.g. cursor-hook)" --> C{CMUX_SURFACE_ID empty\nAND no --workspace/--surface?}
C -- Yes --> D["print('{}'); return"]
C -- No --> E[connect socket]
B -- "No" --> F{command == 'feed-hook'?}
F -- "Yes - no early exit" --> E
F -- No --> G{other early exits}
G --> E
E --> H{switch command}
H -- "feed-hook" --> I[runFeedHook]
H -- "default" --> J{legacyAgentNameFromHookCommand?}
J -- Yes --> K["runGenericAgentHook(def)"]
J -- No --> L["print usage + throw CLIError"]
Reviews (1): Last reviewed commit: "Forward legacy `cmux <agent>-hook` comma..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if Self.legacyAgentNameFromHookCommand(command) != nil, processEnv["CMUX_SURFACE_ID"]?.isEmpty != false, processEnv["CMUX_WORKSPACE_ID"]?.isEmpty != false, | ||
| !commandArgs.contains(where: { $0 == "--workspace" || $0 == "--surface" || $0.hasPrefix("--workspace=") || $0.hasPrefix("--surface=") }) { print("{}"); return } // Backwards compatibility for old installed hooks outside cmux terminals. |
There was a problem hiding this comment.
feed-hook loses its graceful no-context early exit
The original condition explicitly listed feed-hook here alongside codex-hook. The replacement uses legacyAgentNameFromHookCommand(command), but that function deliberately returns nil for "feed-hook" (candidate "feed" is excluded). So feed-hook no longer hits this path. Any legacy installation that invokes bare cmux feed-hook without a [ -n "$CMUX_SURFACE_ID" ] shell guard and runs outside a cmux terminal will now fall through to client.connect() (line 2687), which throws instead of printing {}.
| if Self.legacyAgentNameFromHookCommand(command) != nil, processEnv["CMUX_SURFACE_ID"]?.isEmpty != false, processEnv["CMUX_WORKSPACE_ID"]?.isEmpty != false, | |
| !commandArgs.contains(where: { $0 == "--workspace" || $0 == "--surface" || $0.hasPrefix("--workspace=") || $0.hasPrefix("--surface=") }) { print("{}"); return } // Backwards compatibility for old installed hooks outside cmux terminals. | |
| if (Self.legacyAgentNameFromHookCommand(command) != nil || command == "feed-hook"), processEnv["CMUX_SURFACE_ID"]?.isEmpty != false, processEnv["CMUX_WORKSPACE_ID"]?.isEmpty != false, | |
| !commandArgs.contains(where: { $0 == "--workspace" || $0 == "--surface" || $0.hasPrefix("--workspace=") || $0.hasPrefix("--surface=") }) { print("{}"); return } // Backwards compatibility for old installed hooks outside cmux terminals. |
![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/CMUXCLI`+AgentHookDefinitions.swift:
- Line 333: feedHookMarkers(for:) currently generates generic markers ["cmux
hooks feed --source", "cmux feed-hook --source"] which can match and remove
hooks from other agents; update feedHookMarkers(for:) to scope markers to the
specific agent by embedding the agent identifier (use def.name) into the marker
strings so they become e.g. "cmux hooks feed --source (def.name)" / "cmux
feed-hook --source (def.name)" or otherwise include def.name in both markers,
ensuring cleanup only affects hooks belonging to that agent.
🪄 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: fbb150dc-da4a-4718-8dc4-d5066719a2ed
📒 Files selected for processing (3)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftcmuxTests/CLILegacyHookAliasTests.swift
| markers.append("cmux feed-hook --source") | ||
| } | ||
| return markers | ||
| ["cmux hooks feed --source", "cmux feed-hook --source"] |
There was a problem hiding this comment.
Scope feed-hook removal markers to the current agent source.
feedHookMarkers(for:) is currently too broad. Matching only "cmux hooks feed --source" / "cmux feed-hook --source" can remove unrelated entries during reinstall/uninstall. Include \(def.name) in the marker so cleanup stays agent-scoped.
Suggested fix
static func feedHookMarkers(for def: AgentHookDef) -> [String] {
- ["cmux hooks feed --source", "cmux feed-hook --source"]
+ ["cmux hooks feed --source \(def.name)", "cmux feed-hook --source \(def.name)"]
}🤖 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/CMUXCLI`+AgentHookDefinitions.swift at line 333, feedHookMarkers(for:)
currently generates generic markers ["cmux hooks feed --source", "cmux feed-hook
--source"] which can match and remove hooks from other agents; update
feedHookMarkers(for:) to scope markers to the specific agent by embedding the
agent identifier (use def.name) into the marker strings so they become e.g.
"cmux hooks feed --source (def.name)" / "cmux feed-hook --source (def.name)" or
otherwise include def.name in both markers, ensuring cleanup only affects hooks
belonging to that agent.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Summary
~/.<agent>/hooks.jsonentries) was blocking every shell command withHook "[ ... ] && cmux cursor-hook shell-exec || echo '{}'" returned invalid JSON. The command was blocked for safety.cmux <agent>-hook <sub>→cmux hooks <agent> <sub>and kept compat shims only forcodex-hookandfeed-hook. cursor, gemini, opencode, copilot, codebuddy, factory, qoder all lost theirs.cmux cursor-hook shell-exec, the unknown-command path prints the full CLI usage to stdout and exits non-zero. Combined with the installed|| echo '{}'fallback, that produces "usage text +{}", which Cursor rejects as invalid JSON.Fix: detect any
<agent>-hooklegacy command for a registered agent and forward it torunGenericAgentHook, mirroring the existing codex-hook shim across all agents. GeneralizeisLegacyCmuxOwnedHookCommandso reinstall purges stale legacy entries instead of layering new ones beside them.Verification
Before/after via the exact shell wrapper Cursor invokes:
Reproduced via
cursor-agent --printon a minimal hooks.json containing only the legacy entry: before fix →Hook "...cursor-hook shell-exec..." returned invalid JSON. After fix → command runs.Test plan
testLegacyCursorHookAliasShellExecReturnsJSONWithoutHelpandtestLegacyGeminiHookAliasReturnsJSONWithoutHelpincmuxTests/CLILegacyHookAliasTests.swift, asserting exact{}\nstdout and noUsage:text. Two-commit structure: first commit adds the failing tests, second commit lands the fix.<agent>-hook session-startreturns{}exit 0.🤖 Generated with Claude Code
Note
Medium Risk
Touches CLI command dispatch and hook uninstall/upgrade detection; a mistake could misroute commands or fail to purge old hook entries, but changes are limited to legacy alias handling and covered by new regression tests.
Overview
Fixes stale hook configs that still invoke legacy
cmux <agent>-hook <subcommand>by detecting any registered legacy<agent>-hookcommand and forwarding it torunGenericAgentHookso it returns clean{}JSON instead of printing usage/unknown-command text.Generalizes legacy hook cleanup:
isLegacyCmuxOwnedHookCommand/marker lists now match old<agent>-hookandfeed-hook --source <agent>entries across all agents (not just Codex) so reinstall/uninstall can purge them. Adds regression tests ensuring legacycursor-hookandgemini-hookaliases exit 0 and emit exactly{}\nwith no help/usage output.Reviewed by Cursor Bugbot for commit f8a097a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Routes legacy
cmux <agent>-hookcommands to the namespaced dispatcher so stale hook entries return valid{}JSON instead of CLI usage text, preventing Cursor from blocking shell commands. Reinstall now detects and cleans these legacy entries across all agents.<agent>-hooktorunGenericAgentHook(cmux hooks <agent> <subcommand>).claude-hook/feed-hookbehavior and unknown-command handling unchanged.cursor-hook shell-execandgemini-hook session-startto assert exact{}output with no help text.Written for commit f8a097a. Summary will update on new commits.
Summary by CodeRabbit