diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e04c9183bca5..b5da8354ad66 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1387,6 +1387,7 @@ jobs: python3 tests/test_stress_cli_socket_api.py CMUX_CLI_BIN="$CLI_BIN" python3 tests/test_cli_omo_openagent_plugin_migration.py CMUX_CLI_BIN="$CLI_BIN" python3 tests/test_cli_socket_autodiscovery.py + CMUX_CLI_BIN="$CLI_BIN" python3 tests/test_codex_inject_args_layering.py python3 tests/test_codex_wrapper_resume_hooks.py python3 tests/test_claude_wrapper_hooks.py python3 tests/test_codex_wrapper_computer_use_mcp.py diff --git a/CLI/CMUXCLI+CodexFireAndForgetHooks.swift b/CLI/CMUXCLI+CodexFireAndForgetHooks.swift index de765293e55a..b3e0e3644aba 100644 --- a/CLI/CMUXCLI+CodexFireAndForgetHooks.swift +++ b/CLI/CMUXCLI+CodexFireAndForgetHooks.swift @@ -89,8 +89,10 @@ extension CMUXCLI { /// native child lifecycle hooks synchronously commit their ledger event and /// then return. All larger socket delivery remains non-blocking. /// Persistent hooks are inventoried read-only so the wrapper does not add a - /// duplicate cmux producer. Codex combines hook sources, so user-owned hooks - /// continue to run alongside these process-local entries. Only explicit + /// duplicate cmux producer. Codex appends this `-c` (session-flags) layer to + /// handlers already loaded from `hooks.json` and `config.toml`; never copy + /// user-owned hook groups into these values, because Codex would register and + /// run them twice (reference #12081). Only explicit /// `cmux hooks codex install` or `uninstall` commands mutate `CODEX_HOME`. /// No live socket is required. func emitCodexWrapperInjectArgs() throws { diff --git a/docs/agent-hooks.md b/docs/agent-hooks.md index ac52a36ed6f9..1a4164400ca1 100644 --- a/docs/agent-hooks.md +++ b/docs/agent-hooks.md @@ -135,6 +135,32 @@ You can also set the same preference in `~/.config/cmux/cmux.json`: When this is off, cmux still restores the saved window, workspace, pane, scrollback, and browser state. Restored agent terminals stay idle until you resume them manually. +## Codex hook precedence + +Inside cmux, the Codex wrapper adds `--enable hooks`, `--dangerously-bypass-hook-trust`, and one +`-c hooks.=[...]` pair for each cmux event that is not already covered by a persistent cmux +handler in `hooks.json`. These flags apply only to that process; cmux never rewrites `hooks.json` +or `config.toml` for this injection. + +Codex loads hooks in layers: managed hooks, the user layer (`$CODEX_HOME/hooks.json`, then +`[hooks]` in `config.toml`), project `.codex` layers, the command-line (`-c`) layer, and plugin +hooks. Handlers from each layer are appended. A `-c hooks.` assignment therefore adds +handlers for that event; it does not replace entries from `hooks.json`. This ordering is implemented +in [openai/codex's `codex-rs/hooks/src/engine/discovery.rs`](https://github.com/openai/codex/blob/main/codex-rs/hooks/src/engine/discovery.rs). + +User handlers are registered before cmux's handlers. Codex dispatches the handlers for one event together, so behavior must not depend on cmux's handler running first or last. + +cmux never copies user-owned hook groups into its `-c` values. Codex would register those groups +twice and run them twice. When `cmux hooks codex install` has already installed a persistent cmux +handler for an event, the wrapper skips that event so there is exactly one cmux producer per event. + +The trust flag applies to every `hooks.json`, `config.toml`, and plugin hook loaded by that launch. User and project hooks that Codex has not yet approved in `/hooks` therefore run inside cmux without an approval prompt. + +Set `CMUX_CODEX_HOOKS_DISABLED=1` to exec Codex with its configuration untouched. User hooks are +unaffected either way, but cmux loses session lifecycle and restore, Feed, notifications, session +rebinding, and Agent Hibernation for that process. Use `cmux hooks codex install` to keep one visible +cmux configuration in `hooks.json` instead of per-launch injection. + ## Environment overrides | Agent | Config directory override | Disable cmux hooks for one process | diff --git a/tests/test_codex_inject_args_layering.py b/tests/test_codex_inject_args_layering.py new file mode 100644 index 000000000000..f93a4170de7f --- /dev/null +++ b/tests/test_codex_inject_args_layering.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +""" +Regression coverage for Codex hook layering. + +Codex appends cmux's session-flags layer to hooks.json, so copying user-owned +hook groups into the injected values would register and run them twice (#12081). +This test guards that contract and the one-cmux-producer-per-event rule. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +import re +import subprocess +import tempfile + +from claude_teams_test_utils import resolve_cmux_cli + + +CODEX_EVENTS = ( + "PreToolUse", + "PermissionRequest", + "PostToolUse", + "PreCompact", + "PostCompact", + "SessionStart", + "SessionEnd", + "UserPromptSubmit", + "SubagentStart", + "SubagentStop", + "Stop", + "Interrupt", +) + +VALUE_PATTERN = re.compile(r"^hooks\.([A-Za-z]+)=(.*)$") +SHAPE_PATTERN = re.compile( + r"^\[\{hooks=\[\{type=\"command\",command='''(?P[^']+)''',timeout=\d+\}\]\}\]$" +) + + +def run_inject_args(cli: str, codex_home: str) -> list[str]: + env = os.environ.copy() + env["CODEX_HOME"] = codex_home + for key in list(env): + if key.startswith("CMUX_"): + del env[key] + result = subprocess.run( + [cli, "hooks", "codex", "inject-args"], + env=env, + check=True, + stdout=subprocess.PIPE, + ) + return [part.decode("utf-8") for part in result.stdout.split(b"\0") if part] + + +def user_hook_group(event: str) -> dict[str, object]: + return { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "user-{}-hook".format(event), + "timeout": 7, + } + ], + } + + +def write_hooks(home: Path, hooks: dict[str, list[dict[str, object]]]) -> bytes: + home.mkdir(parents=True, exist_ok=True) + path = home / "hooks.json" + path.write_text(json.dumps({"hooks": hooks}, indent=2) + "\n", encoding="utf-8") + return path.read_bytes() + + +def emitted_hooks( + args: list[str], require_session_start: bool = True +) -> tuple[dict[str, str], dict[str, str]]: + if args[:3] != ["--enable", "hooks", "--dangerously-bypass-hook-trust"]: + raise AssertionError("unexpected activation args: {!r}".format(args[:3])) + rest = args[3:] + if len(rest) % 2 != 0 or any(rest[index] != "-c" for index in range(0, len(rest), 2)): + raise AssertionError("expected only -c/value pairs: {!r}".format(rest)) + + values: dict[str, str] = {} + commands: dict[str, str] = {} + for index in range(1, len(rest), 2): + value = rest[index] + match = VALUE_PATTERN.fullmatch(value) + if match is None: + raise AssertionError("invalid hook value: {!r}".format(value)) + event, encoded = match.groups() + if event in values: + raise AssertionError("duplicate emitted event: {}".format(event)) + if "user-" in value: + raise AssertionError("user-owned hook copied into value: {!r}".format(value)) + shape = SHAPE_PATTERN.fullmatch(encoded) + if shape is None: + raise AssertionError("unexpected cmux hook shape: {!r}".format(value)) + command = shape.group("cmd") + command_path = Path(command) + if not ( + (command_path.name.startswith("cmux-codex-hook-") and command_path.name.endswith(".sh")) + or "cmux hooks codex" in command + ): + raise AssertionError("non-cmux command emitted: {!r}".format(command)) + values[event] = encoded + commands[event] = command + required = {"UserPromptSubmit", "Stop"} + if require_session_start: + required.add("SessionStart") + if not required.issubset(values): + raise AssertionError("missing required events: {!r}".format(sorted(required - set(values)))) + return values, commands + + +def main() -> int: + cli = resolve_cmux_cli() + with tempfile.TemporaryDirectory(prefix="cmux-codex-inject-args-", dir="/tmp") as root: + root_path = Path(root) + first_home = root_path / "case-one" + first_hooks = {event: [user_hook_group(event)] for event in CODEX_EVENTS} + first_bytes = write_hooks(first_home, first_hooks) + first_args = run_inject_args(cli, str(first_home)) + first_values, first_commands = emitted_hooks(first_args) + if (first_home / "hooks.json").read_bytes() != first_bytes: + raise AssertionError("case 1 changed hooks.json") + session_start_command = first_commands["SessionStart"] + + second_home = root_path / "case-two" + second_hooks = {event: [user_hook_group(event)] for event in CODEX_EVENTS} + second_hooks["SessionStart"].append( + {"hooks": [{"type": "command", "command": session_start_command}]} + ) + second_bytes = write_hooks(second_home, second_hooks) + second_args = run_inject_args(cli, str(second_home)) + second_values, _ = emitted_hooks(second_args, require_session_start=False) + if "SessionStart" in second_values: + raise AssertionError("persistent cmux SessionStart handler was not respected") + if not {"UserPromptSubmit", "Stop"}.issubset(second_values): + raise AssertionError("case 2 omitted required remaining events") + if (second_home / "hooks.json").read_bytes() != second_bytes: + raise AssertionError("case 2 changed hooks.json") + + print("PASS: Codex inject-args preserves user groups and one cmux producer per event") + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except Exception as exc: + print("FAIL: {}".format(exc)) + raise SystemExit(1)