Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 28 additions & 15 deletions CLI/CMUXCLI+AgentHookDefinitions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,19 @@ extension CMUXCLI {
}

private static func isLegacyCmuxOwnedHookCommand(_ command: String, for def: AgentHookDef) -> Bool {
// Legacy cmux codex-hook and feed-hook commands only existed for Codex hooks.
guard def.name == "codex" else {
return false
}
// Legacy `cmux <agent>-hook` and `cmux feed-hook --source <agent>`
// commands were removed in commit 6beb3dbe1 ("Namespace agent hook
// commands") for every agent that had them, not only Codex. Detect
// them across all agents so reinstall purges stale entries from
// user hook config files.
let tokens = legacyCmuxCommandTokens(from: command, for: def)
guard !tokens.isEmpty,
URL(fileURLWithPath: String(tokens[0])).lastPathComponent == "cmux"
else {
return false
}

if tokens.count >= 2, tokens[1] == "codex-hook" {
if tokens.count >= 2, tokens[1] == "\(def.name)-hook" {
return true
}
if tokens.count >= 4, tokens[1] == "feed-hook", tokens[2] == "--source", tokens[3] == def.name {
Expand All @@ -286,6 +287,26 @@ extension CMUXCLI {
return false
}

/// If `command` looks like a legacy `<agent>-hook` CLI command name
/// (e.g. `cursor-hook`, `gemini-hook`), returns the canonical agent
/// name. Used to forward legacy invocations to the current
/// `cmux hooks <agent> <subcommand>` dispatcher so stale entries in
/// user hook config files keep returning valid JSON instead of
/// printing the CLI usage on unknown command.
static func legacyAgentNameFromHookCommand(_ command: String) -> String? {
let normalized = command.trimmingCharacters(in: .whitespacesAndNewlines).lowercased()
guard normalized.hasSuffix("-hook"), normalized.count > "-hook".count else {
return nil
}
let candidate = String(normalized.dropLast("-hook".count))
if candidate == "claude" || candidate == "feed" {
// claude-hook and feed-hook have their own dispatcher paths and
// are not aliases of `hooks <agent>`.
return nil
}
return agentDef(named: candidate)?.name
}

private static func legacyCmuxCommandTokens(from command: String, for def: AgentHookDef) -> [Substring] {
let guardedPrefix = "[ -n \"$CMUX_SURFACE_ID\" ] && [ \"$\(def.disableEnvVar)\" != \"1\" ] && command -v cmux >/dev/null 2>&1 && "
let fallbackSuffix = " || echo '{}'"
Expand All @@ -303,20 +324,12 @@ extension CMUXCLI {
}

static func hookMarkers(for def: AgentHookDef) -> [String] {
var markers = [def.hookMarker]
if def.name == "codex" {
markers.append("cmux codex-hook")
}
return markers
[def.hookMarker, "cmux \(def.name)-hook"]
}

/// Marker substrings used when removing / upgrading our own Feed bridge
/// entries on reinstall or uninstall.
static func feedHookMarkers(for def: AgentHookDef) -> [String] {
var markers = ["cmux hooks feed --source"]
if def.name == "codex" {
markers.append("cmux feed-hook --source")
}
return markers
["cmux hooks feed --source", "cmux feed-hook --source"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

}
}
28 changes: 23 additions & 5 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2606,7 +2606,7 @@ struct CMUXCLI {
}
}
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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f8a097a. Configure here.

!commandArgs.contains(where: { $0 == "--workspace" || $0 == "--surface" || $0.hasPrefix("--workspace=") || $0.hasPrefix("--surface=") }) { print("{}"); return } // Backwards compatibility for old installed hooks outside cmux terminals.
Comment on lines +2609 to 2610

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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 {}.

Suggested change
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.

if command == "hooks" {
if try runHooksNoSocketCommand(commandArgs: commandArgs) {
Expand Down Expand Up @@ -2712,7 +2712,7 @@ struct CMUXCLI {
}
}

let capturesSocketErrorsInsideCommand = ["claude-hook", "codex-hook", "feed-hook", "hooks"].contains(command) // Backwards compatibility aliases stay hidden from help.
let capturesSocketErrorsInsideCommand = ["claude-hook", "feed-hook", "hooks"].contains(command) || Self.legacyAgentNameFromHookCommand(command) != nil // Backwards compatibility aliases stay hidden from help.
do {
switch command {
case "ping":
Expand Down Expand Up @@ -3878,9 +3878,6 @@ struct CMUXCLI {
captureSocketTransportError(telemetry: cliTelemetry, stage: "claude_hook_dispatch", error: error, client: client)
throw error
}
case "codex-hook": // Backwards compatibility for older installed Codex hooks. Hidden from help.
guard let codexDef = Self.agentDef(named: "codex") else { print("{}"); return }
try runGenericAgentHook(def: codexDef, commandArgs: commandArgs, client: client, telemetry: cliTelemetry)
case "feed-hook": // Backwards compatibility for older installed Feed hooks. Hidden from help.
try runFeedHook(commandArgs: commandArgs, client: client, telemetry: cliTelemetry)
case "hooks":
Expand Down Expand Up @@ -3983,6 +3980,27 @@ struct CMUXCLI {
try runMarkdownCommand(commandArgs: commandArgs, client: client, jsonOutput: jsonOutput, idFormat: idFormat)

default:
// Backwards compatibility for legacy `cmux <agent>-hook ...` commands
// (e.g. `cmux cursor-hook shell-exec`). The legacy names were removed
// in commit 6beb3dbe1 ("Namespace agent hook commands"), but stale
// entries in user hook config files keep invoking them. Without this
// shim the unknown-command path prints CLI usage to stdout and exits
// non-zero, which combines with the `|| echo '{}'` fallback in the
// installed hook command to yield "usage text + {}". Cursor agent
// rejects that as invalid JSON and blocks every shell execution.
if let agentName = Self.legacyAgentNameFromHookCommand(command),
let def = Self.agentDef(named: agentName) {
cliTelemetry.breadcrumb("\(agentName)-hook.dispatch.legacy")
do {
try runGenericAgentHook(def: def, commandArgs: commandArgs, client: client, telemetry: cliTelemetry)
cliTelemetry.breadcrumb("\(agentName)-hook.completed")
} catch {
cliTelemetry.breadcrumb("\(agentName)-hook.failure")
captureSocketTransportError(telemetry: cliTelemetry, stage: "\(agentName)_hook_dispatch", error: error, client: client)
throw error
}
break
}
print(usage())
throw CLIError(message: "Unknown command: \(command)")
}
Expand Down
87 changes: 87 additions & 0 deletions cmuxTests/CLILegacyHookAliasTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,91 @@ extension CLINotifyProcessIntegrationRegressionTests {
XCTAssertFalse(result.stdout.contains("Usage:"), result.stdout)
XCTAssertFalse(result.stderr.contains("Usage:"), result.stderr)
}

// Regression: legacy `cmux <agent>-hook <subcommand>` aliases were removed
// in commit 6beb3dbe1 ("Namespace agent hook commands") but `~/.cursor/hooks.json`
// and other agent hook configs still point at them. 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
// garbage-followed-by-`{}`. Cursor agent rejects that as invalid JSON and
// blocks every shell command. The alias must succeed and emit exactly `{}\n`.
func testLegacyCursorHookAliasShellExecReturnsJSONWithoutHelp() throws {
try assertLegacyAgentHookAliasReturnsEmptyJSON(
agentName: "cursor",
subcommand: "shell-exec",
slug: "legacy-cursor",
stdin: #"{"command":"echo hello","cwd":"/tmp","hook_event_name":"beforeShellExecution"}"#
)
}

// Regression: same class of bug for gemini-hook. Confirms the alias dispatch
// generalizes across agents that lost their legacy `<agent>-hook` command.
func testLegacyGeminiHookAliasReturnsJSONWithoutHelp() throws {
try assertLegacyAgentHookAliasReturnsEmptyJSON(
agentName: "gemini",
subcommand: "session-start",
slug: "legacy-gemini",
stdin: #"{"hook_event_name":"SessionStart","session_id":"legacy-gemini"}"#
)
}

private func assertLegacyAgentHookAliasReturnsEmptyJSON(
agentName: String,
subcommand: String,
slug: String,
stdin: String
) throws {
let cliPath = try bundledCLIPath()
let socketPath = makeSocketPath(slug)
let listenerFD = try bindUnixSocket(at: socketPath)
let state = MockSocketServerState()
let root = FileManager.default.temporaryDirectory
.appendingPathComponent("cmux-\(slug)-\(UUID().uuidString)", isDirectory: true)
let workspaceId = "11111111-1111-1111-1111-111111111111"
let surfaceId = "22222222-2222-2222-2222-222222222222"

try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
defer {
Darwin.close(listenerFD)
unlink(socketPath)
try? FileManager.default.removeItem(at: root)
}

let serverHandled = startMockServer(listenerFD: listenerFD, state: state) { line in
guard let payload = self.jsonObject(line) else {
return line.trimmingCharacters(in: .whitespacesAndNewlines).hasPrefix("{")
? self.malformedRequestResponse(raw: line)
: "OK"
}
guard let id = payload["id"] as? String, let method = payload["method"] as? String else {
return self.malformedRequestResponse(id: payload["id"] as? String, raw: line)
}
if method == "surface.list" { return self.surfaceListResponse(id: id, surfaceId: surfaceId) }
return self.v2Response(id: id, ok: false, error: ["code": "unrecognized_method", "message": "unexpected method: \(method)"])
}

var environment = ProcessInfo.processInfo.environment
environment["CMUX_SOCKET_PATH"] = socketPath
environment["CMUX_WORKSPACE_ID"] = workspaceId
environment["CMUX_SURFACE_ID"] = surfaceId
environment["CMUX_AGENT_HOOK_STATE_DIR"] = root.path
environment["CMUX_CLI_SENTRY_DISABLED"] = "1"

let result = runProcess(
executablePath: cliPath,
arguments: ["\(agentName)-hook", subcommand],
environment: environment,
standardInput: stdin,
timeout: 5
)

wait(for: [serverHandled], timeout: 5)
XCTAssertFalse(result.timedOut, result.stderr)
XCTAssertEqual(result.status, 0, "stdout=\(result.stdout) stderr=\(result.stderr)")
XCTAssertEqual(result.stdout, "{}\n", "stderr=\(result.stderr)")
XCTAssertFalse(result.stdout.contains("Usage:"), result.stdout)
XCTAssertFalse(result.stderr.contains("Usage:"), result.stderr)
XCTAssertFalse(result.stdout.contains("Unknown command"), result.stdout)
XCTAssertFalse(result.stderr.contains("Unknown command"), result.stderr)
}
}
Loading