diff --git a/README.md b/README.md index 4c48531939..23444fd233 100644 --- a/README.md +++ b/README.md @@ -421,7 +421,7 @@ RTK supports 17 AI coding tools. Each integration rewrites shell commands to `rt | **GitHub Copilot CLI** | `rtk init -g --copilot` | PreToolUse deny-with-suggestion (CLI limitation) | | **Cursor** | `rtk init -g --agent cursor` | preToolUse hook (hooks.json) | | **Gemini CLI** | `rtk init -g --gemini` | BeforeTool hook | -| **Codex** | `rtk init -g --codex` | AGENTS.md + RTK.md instructions | +| **Codex** | `rtk init -g --codex` | PreToolUse hook (`updatedInput`) + AGENTS.md | | **Windsurf** | `rtk init -g --agent windsurf` | .windsurfrules (project-scoped) | | **Cline / Roo Code** | `rtk init --agent cline` | .clinerules (project-scoped) | | **OpenCode** | `rtk init -g --opencode` | Plugin TS (tool.execute.before) | diff --git a/docs/contributing/TECHNICAL.md b/docs/contributing/TECHNICAL.md index 3a27b1f0a5..370d2a8502 100644 --- a/docs/contributing/TECHNICAL.md +++ b/docs/contributing/TECHNICAL.md @@ -320,7 +320,7 @@ Start here, then drill down into each README for file-level details. | [`cursor/`](../hooks/cursor/README.md) | Cursor IDE | Shell hook, empty JSON response requirement | | [`cline/`](../hooks/cline/README.md) | Cline / Roo Code | Rules file (prompt-level, no programmatic hook) | | [`windsurf/`](../hooks/windsurf/README.md) | Windsurf / Cascade | Rules file (workspace-scoped) | -| [`codex/`](../hooks/codex/README.md) | OpenAI Codex CLI | Awareness document, AGENTS.md integration | +| [`codex/`](../hooks/codex/README.md) | OpenAI Codex CLI | Native `PreToolUse` processor, hooks.json registration, AGENTS.md awareness | | [`opencode/`](../hooks/opencode/README.md) | OpenCode | TypeScript plugin, zx library, in-place mutation | --- @@ -338,7 +338,7 @@ RTK supports the following LLM agents through hook integrations: | Gemini CLI | Rust binary | `rtk hook gemini` reads JSON | Yes (`hookSpecificOutput`) | | Cline/Roo Code | Rules file | Prompt-level guidance | N/A (prompt) | | Windsurf | Rules file | Prompt-level guidance | N/A (prompt) | -| Codex CLI | Awareness doc | AGENTS.md integration | N/A (prompt) | +| Codex CLI | Rust binary | `rtk hook codex` reads JSON | Yes (`updatedInput`) | | OpenCode | TS plugin | `tool.execute.before` event | Yes (in-place mutation) | > **Details**: [`hooks/README.md`](../hooks/README.md) has the full JSON schemas for each agent. [`src/hooks/README.md`](../src/hooks/README.md) covers installation, integrity verification, and the rewrite command. diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 4622b29918..0020ef7826 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -41,7 +41,7 @@ Agent runs "cargo test" | Factory Droid | Shell hook (`PreToolUse`, matcher `Execute`) | Yes | | Cline / Roo Code | Rules file (prompt-level) | N/A | | Windsurf | Rules file (prompt-level) | N/A | -| Codex CLI | AGENTS.md instructions | N/A | +| Codex CLI | Rust binary (`PreToolUse`) | Yes | | Kilo Code | Rules file (prompt-level) | N/A | | Google Antigravity | Rules file (prompt-level) | N/A | | Mistral Vibe | Rust binary (`pre_tool`) | Yes | @@ -210,10 +210,14 @@ rtk init --global --agent windsurf # creates .windsurfrules in current projec ### Codex CLI ```bash -rtk init --codex # project-scoped (AGENTS.md) -rtk init --global --codex # user-global (~/.codex/AGENTS.md) +rtk init --codex # project-scoped (.codex/hooks.json + AGENTS.md) +rtk init --global --codex # user-global ($CODEX_HOME or ~/.codex/) +rtk init --codex --uninstall # remove project-scoped integration +rtk init --global --codex --uninstall # remove user-global integration ``` +Restart Codex after installation. Project-scoped hooks must be trusted when Codex prompts. The native `rtk hook codex` processor rewrites supported `Bash` commands through `PreToolUse.updatedInput`; Codex then applies its normal approval and sandbox checks to the rewritten command. + ### Kilo Code ```bash @@ -261,7 +265,7 @@ Strips only RTK's `[[hooks]]` block and the `~/.vibe/prompts/rtk.md` file. Any o | **Plugin** | TypeScript, JavaScript, or Python in agent's plugin system | Transparent, in-place mutation when the agent allows it | | **Rules file** | Prompt-level instructions | Guidance only — agent is told to prefer `rtk ` | -Rules file integrations (Cline, Windsurf, Codex, Kilo Code, Antigravity) rely on the model following instructions. Full hook integrations (Claude Code, Cursor, Gemini) are guaranteed — the command is rewritten before the agent sees it. Plugin integrations (OpenCode, Pi) use in-place mutation via the agent's TypeScript extension API. +Rules file integrations (Cline, Windsurf, Kilo Code, Antigravity) rely on the model following instructions. Full hook integrations (Claude Code, Cursor, Gemini, Codex) rewrite the command before execution. Plugin integrations (OpenCode, Pi) use in-place mutation via the agent's TypeScript extension API. ## Windows support diff --git a/hooks/README.md b/hooks/README.md index a878f5884f..54c9532e57 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -2,7 +2,7 @@ ## Scope -**Deployed hook artifacts** — the actual files installed on user machines by `rtk init`. These are shell scripts, TypeScript plugins, and rules files that run outside the Rust binary. They are **thin delegates**: parse agent-specific JSON, call `rtk rewrite` as a subprocess, format agent-specific response. Zero filtering logic lives here. +**Deployed hook artifacts** — the files and agent-specific configuration installed on user machines by `rtk init`. External scripts and plugins are thin delegates to `rtk rewrite`; native processors such as Codex call the same Rust rewrite registry directly. Zero filtering logic is duplicated in this directory. Owns: per-agent hook scripts and configuration files for 11 supported agents (Claude Code, Copilot, Cursor, Cline, Windsurf, Codex, OpenCode, Hermes, Pi, Oh My Pi, Mistral Vibe). @@ -27,14 +27,14 @@ LLM agent integrations that intercept CLI commands and route them through RTK fo Agent runs command (e.g., "cargo test --nocapture") -> Hook intercepts (PreToolUse / plugin event) -> Reads JSON input, extracts command string - -> Calls `rtk rewrite "cargo test --nocapture"` + -> Uses the shared RTK rewrite registry -> Registry matches pattern, returns "rtk cargo test --nocapture" -> Hook sends response in agent-specific JSON format -> Agent executes "rtk cargo test --nocapture" instead -> Filtered output reaches LLM (up to 90% fewer bash output bytes) ``` -All rewrite logic lives in the Rust binary (`src/discover/registry.rs`). Hook scripts are **thin delegates** that handle agent-specific JSON formats and call `rtk rewrite` for the actual decision. This ensures a single source of truth for all 70+ rewrite patterns. +All rewrite logic lives in the Rust binary (`src/discover/registry.rs`). External hook scripts call `rtk rewrite`; native Rust hook processors call the registry directly. This keeps one source of truth for all 70+ rewrite patterns. ## Directory Structure @@ -45,7 +45,7 @@ Each agent subdirectory has its own README with hook-specific details: - **[`cursor/`](cursor/README.md)** — Shell hook, Cursor JSON format, empty `{}` response requirement - **[`cline/`](cline/README.md)** — Rules file (prompt-level), `.clinerules` project-local installation - **[`windsurf/`](windsurf/README.md)** — Rules file (prompt-level), `.windsurfrules` workspace-scoped -- **[`codex/`](codex/README.md)** — Awareness document, `AGENTS.md` integration, `$CODEX_HOME` or `~/.codex/` location +- **[`codex/`](codex/README.md)** — Rust binary hook (`rtk hook codex`), `PreToolUse.updatedInput`, `.codex/hooks.json` / `$CODEX_HOME/hooks.json`, plus `AGENTS.md` awareness - **[`opencode/`](opencode/README.md)** — TypeScript plugin, `zx` library, `tool.execute.before` event, in-place mutation - **[`pi/`](pi/README.md)** — TypeScript extension, `tool_call` event, local `isBashToolCallEvent` guard, in-place mutation, `~/.pi/agent/extensions/`; **shared with Oh My Pi (OMP)** — OMP installs the same file at `.omp/extensions/` via its `legacy-pi-compat` layer - **[`hermes/`](hermes/README.md)** — Python plugin, `pre_tool_call` hook, in-place terminal command mutation @@ -62,7 +62,7 @@ Each agent subdirectory has its own README with hook-specific details: | Gemini CLI | Rust binary (`rtk hook gemini`) | Transparent rewrite | Yes (`hookSpecificOutput`) | | Cline / Roo Code | Custom instructions (rules file) | Prompt-level guidance | N/A | | Windsurf | Custom instructions (rules file) | Prompt-level guidance | N/A | -| Codex CLI | AGENTS.md / instructions | Prompt-level guidance | N/A | +| Codex CLI | Rust binary (`rtk hook codex`) | Transparent rewrite | Yes (`updatedInput`) | | OpenCode | TypeScript plugin (`tool.execute.before`) | In-place mutation | Yes | | Pi | TypeScript extension (`tool_call` event) | In-place mutation | Yes | | Oh My Pi (OMP) | TypeScript extension (`tool_call` event, shared with Pi) | In-place mutation | Yes | @@ -110,6 +110,34 @@ Each agent subdirectory has its own README with hook-specific details: Returns `{}` when no rewrite (Cursor requires JSON for all paths). +### Codex CLI (Rust Binary) + +**Input** (stdin): + +```json +{ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "permission_mode": "default", + "tool_input": { "command": "git status" } +} +``` + +**Output** (stdout, when rewritten): + +```json +{ + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "allow", + "permissionDecisionReason": "RTK auto-rewrite", + "updatedInput": { "command": "rtk git status" } + } +} +``` + +The `allow` value is required by Codex to accept `updatedInput`; Codex still runs its native approval and sandbox checks after applying the replacement. Those checks classify the rewritten command, and Codex does not currently unwrap the `rtk` binary for its safe/dangerous-command heuristics. Missing or unknown permission modes and other no-rewrite cases produce no stdout. + ### Copilot CLI (Rust Binary) **Input** (stdin, camelCase, `toolArgs` is JSON-stringified): @@ -277,9 +305,9 @@ New integrations must follow the [Exit Code Contract](#exit-code-contract) and [ | Tier | Mechanism | Maintenance | Examples | |------|-----------|-------------|----------| -| **Full hook** | Shell script or Rust binary, intercepts commands via agent's hook API | High — must track agent API changes | Claude Code, Cursor, Copilot, Gemini | +| **Full hook** | Shell script or Rust binary, intercepts commands via agent's hook API | High — must track agent API changes | Claude Code, Cursor, Copilot, Gemini, Codex | | **Plugin** | TypeScript/JS/Python plugin in agent's plugin system | Medium — agent manages loading | OpenCode, Hermes, Pi, Oh My Pi | -| **Rules file** | Prompt-level instructions the agent reads | Low — no code to break | Cline, Windsurf, Codex | +| **Rules file** | Prompt-level instructions the agent reads | Low — no code to break | Cline, Windsurf | ### Eligibility diff --git a/hooks/codex/README.md b/hooks/codex/README.md index 1c6d5c53a6..bdba375321 100644 --- a/hooks/codex/README.md +++ b/hooks/codex/README.md @@ -4,6 +4,37 @@ ## Specifics -- Prompt-level guidance via awareness document -- no programmatic hook -- `../rtk-awareness-full.md` is written to `RTK.md` and referenced from `AGENTS.md` via `@RTK.md`. No hook, so the `full` level is always used regardless of `awareness.level` -- Installed to `$CODEX_HOME` when set, otherwise `~/.codex/`, by `rtk init --codex` +- Native Rust `PreToolUse` processor: `rtk hook codex` +- Transparently rewrites `tool_input.command` with Codex's `updatedInput` response +- Registers a `Bash` matcher in `.codex/hooks.json` (project) or `$CODEX_HOME/hooks.json` (global) +- Writes the shared awareness document selected by `awareness.level` to `RTK.md`, referenced from `AGENTS.md` +- Installed by `rtk init --codex` (project) or `rtk init -g --codex` (global) +- Uninstalled by adding `--uninstall` to the corresponding project or global command + +Codex requires `permissionDecision: "allow"` in the hook response for `updatedInput` to take effect. Codex applies the replacement before its normal command approval and sandbox checks, so those native checks still run on the rewritten command. Codex's command safety classifier does not currently unwrap the `rtk` binary, so classification is based on the rewritten command. This can add prompts for known-safe commands or obscure signals for wrapped mutating commands such as `git push`. + +RTK rewrites the documented Codex permission modes. Missing or unknown permission modes, no match, malformed JSON, unsupported commands, heredocs, substitutions, and file redirections fail open: the hook exits successfully without stdout and Codex executes the original command. + +The Codex handler uses RTK's shared hook decision pipeline through `Host::Codex`. +RTK does not parse Codex execution rules or reuse another agent's permission +files: Codex's native execution layer remains responsible for those rules. +An internal `AskRewrite` still emits the required protocol-level `allow` with +`updatedInput`; it is not recorded internally as an explicit permission grant. +This preserves transparent rewriting but does not remove the classifier +limitation described above. + +## History in workspace-write sandboxes + +Command rewriting can work even when the sandbox prevents RTK from writing its +SQLite history database. If recording reports `SQLITE_CANTOPEN`, run +`rtk init --codex` outside the sandbox to see an optional Codex configuration +snippet using the actual database parent directory. The path follows +`RTK_DB_PATH`, then `tracking.database_path`, then the platform default. + +To opt in, merge the printed `sandbox_workspace_write.writable_roots` entry into +your Codex `config.toml`, preserving existing entries. SQLite needs directory +access for journal/WAL files as well as the database itself. RTK only prints the +snippet; it does not change sandbox settings. Review custom paths before granting +access, and prefer a dedicated RTK data directory. + +See the [Codex configuration reference](https://developers.openai.com/codex/config-reference/). diff --git a/src/hooks/README.md b/src/hooks/README.md index 0093fb78eb..e01250c391 100644 --- a/src/hooks/README.md +++ b/src/hooks/README.md @@ -29,7 +29,7 @@ LLM agent integration layer that installs, validates, and executes command-rewri | Claude-MD (legacy) | `rtk init --claude-md` | 134-line RTK block | CLAUDE.md | | Windsurf | `rtk init -g --agent windsurf` | `.windsurfrules` | -- | | Cline | `rtk init --agent cline` | `.clinerules` | -- | -| Codex | `rtk init --codex` | RTK.md in `$CODEX_HOME` or `~/.codex` | AGENTS.md | +| Codex | `rtk init --codex` | RTK.md + `.codex/hooks.json` (local) or `$CODEX_HOME/hooks.json` (global) | AGENTS.md + `PreToolUse` hook | | Cursor | `rtk init -g --agent cursor` | Cursor hook | hooks.json | | Pi | `rtk init --agent pi` | `.pi/extensions/rtk.ts` | -- | | Oh My Pi (OMP) | `rtk init --agent omp` | `.omp/extensions/rtk.ts` (shared Pi extension) | -- | @@ -91,14 +91,14 @@ Rules are loaded from all Claude Code `settings.json` files (project + global, i | Cursor (rtk hook cursor) | Ready | `permission: "ask",` — users will be prompted when Cursor enforces the permission; in the meantime, allow | | Gemini CLI (rtk hook gemini) | No (allow/deny only) | allow (limitation — no ask mode in Gemini) | | Copilot CLI (rtk hook copilot) | No updatedInput | deny-with-suggestion (unchanged) | -| Codex | ask parsed but no-op | allow (limitation — fails open) | +| Codex (`rtk hook codex`) | Native approval runs after rewrite | Emit required protocol `allow` with `updatedInput`; Codex then evaluates the rewritten command normally | | Mistral Vibe (rtk hook vibe) | No native ask surface | passthrough — Vibe's own approval prompt fires on the rewritten command | ### Implementation - `permissions.rs` — loads deny/ask/allow rules, evaluates precedence, returns `PermissionVerdict` - `rewrite_cmd.rs` — maps verdict to exit code (consumed by shell hook) -- `hook_cmd.rs` — maps verdict to JSON `permissionDecision` field (Copilot/Gemini) +- `hook_cmd.rs` — maps decisions to each agent's JSON protocol, including Codex `updatedInput` ## Exit Code Contract diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index 57d1e85e63..5a323ab126 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -10,6 +10,8 @@ pub const BEFORE_TOOL_KEY: &str = "BeforeTool"; /// Native Rust hook command for Claude Code (replaces rtk-rewrite.sh). pub const CLAUDE_HOOK_COMMAND: &str = "rtk hook claude"; +/// Native Rust hook command for Codex CLI. +pub const CODEX_HOOK_COMMAND: &str = "rtk hook codex"; /// Native Rust hook command for Cursor (replaces rtk-rewrite.sh). pub const CURSOR_HOOK_COMMAND: &str = "rtk hook cursor"; /// Native Rust hook command for Factory Droid. diff --git a/src/hooks/decision.rs b/src/hooks/decision.rs index f43cb7707a..e3d201828d 100644 --- a/src/hooks/decision.rs +++ b/src/hooks/decision.rs @@ -193,10 +193,9 @@ impl AgentPath { match agent { // `copilot` reads Claude Code's settings rather than a Copilot file // (see `hook_cmd`'s `vscode_response` and `copilot_cli_response`). - "antigravity" | "cline" | "codex" | "kilocode" | "kimi" | "windsurf" => { - Some(Self::RulesOnly) - } + "antigravity" | "cline" | "kilocode" | "kimi" | "windsurf" => Some(Self::RulesOnly), "claude" | "copilot" => Some(Self::InProcess(Host::Claude)), + "codex" => Some(Self::InProcess(Host::Codex)), "cursor" => Some(Self::InProcess(Host::Cursor)), "droid" => Some(Self::InProcess(Host::Droid)), "gemini" => Some(Self::InProcess(Host::Gemini)), @@ -408,6 +407,20 @@ mod tests { assert!(AgentPath::lookup("Claude").is_none()); } + #[test] + fn codex_uses_shared_decision_without_claiming_permission() { + assert!(matches!( + AgentPath::lookup("codex"), + Some(AgentPath::InProcess(Host::Codex)) + )); + assert_eq!( + check_command_for("git status", Host::Codex), + PermissionVerdict::Default + ); + let (deny, ask, allow) = super::super::permissions::load_rules_for(Host::Codex); + assert!(deny.is_empty() && ask.is_empty() && allow.is_empty()); + } + /// A rules-file agent has no hook and no permission rules, so its answer /// must not depend on any host's settings. #[test] diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index be6001e65d..c5a95af6c9 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -564,11 +564,34 @@ enum PayloadAction { }, Skip { decision: HookOutcome, + reason: &'static str, cmd: String, }, Ignore, } +fn pre_tool_use_rewrite_output( + v: &Value, + rewritten: &str, + permission_decision: Option<&str>, +) -> Value { + let mut updated_input = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); + if let Some(obj) = updated_input.as_object_mut() { + obj.insert("command".into(), Value::String(rewritten.to_string())); + } + + let mut hook_output = json!({ + "hookEventName": PRE_TOOL_USE_KEY, + "permissionDecisionReason": "RTK auto-rewrite", + "updatedInput": updated_input + }); + if let Some(decision) = permission_decision { + hook_output["permissionDecision"] = Value::String(decision.to_string()); + } + + json!({ "hookSpecificOutput": hook_output }) +} + fn process_claude_payload(v: &Value) -> PayloadAction { let cmd = match v .pointer("/tool_input/command") @@ -595,12 +618,14 @@ fn process_claude_payload_from_decision( HookDecision::Deny => { return PayloadAction::Skip { decision: HookOutcome::Deny, + reason: "skip:deny_rule", cmd: cmd.to_string(), } } HookDecision::Defer => { return PayloadAction::Skip { decision: HookOutcome::Defer, + reason: "skip:defer", cmd: cmd.to_string(), } } @@ -608,36 +633,15 @@ fn process_claude_payload_from_decision( HookDecision::AskRewrite(r) => (r, false), }; - let updated_input = { - let mut ti = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); - if let Some(obj) = ti.as_object_mut() { - obj.insert("command".into(), Value::String(rewritten.clone())); - } - ti - }; - - let mut hook_output = json!({ - "hookEventName": PRE_TOOL_USE_KEY, - "permissionDecisionReason": "RTK auto-rewrite", - "updatedInput": updated_input - }); - - if allow { - hook_output - .as_object_mut() - .unwrap() - .insert("permissionDecision".into(), json!("allow")); - } - PayloadAction::Rewrite { cmd: cmd.to_string(), + output: pre_tool_use_rewrite_output(v, &rewritten, allow.then_some("allow")), rewritten, decision: if allow { HookOutcome::Allow } else { HookOutcome::Ask }, - output: json!({ "hookSpecificOutput": hook_output }), } } @@ -721,22 +725,20 @@ pub fn run_claude() -> Result<()> { audit_log("rewrite", &cmd, &rewritten); log_hook_decision(&v, &cmd, decision, Some(&rewritten)); } - PayloadAction::Skip { decision, cmd } => { + PayloadAction::Skip { + decision, + cmd, + reason, + } => { // `rtk hook audit`'s skip-breakdown groups by a "skip:" prefix - // (see hook_audit_cmd.rs) — Skip is only ever reached via Deny/Defer, - // so map those to the reasons it expects rather than the bare - // HookOutcome::Display used for the Rewrite/tracking-DB paths. + // (see hook_audit_cmd.rs). Carry the reason separately from the + // decision so distinct defer causes remain diagnosable. // // Skip has no stdout response to write (Claude Code falls through to // its own native handling), but log_hook_decision is still deferred to // last for the same reason as the Rewrite arm above: it must never be // what a Bash tool call is waiting on. - let audit_action = match decision { - HookOutcome::Deny => "skip:deny_rule", - HookOutcome::Defer => "skip:defer", - HookOutcome::Allow | HookOutcome::Ask => "skip", - }; - audit_log(audit_action, &cmd, ""); + audit_log(reason, &cmd, ""); log_hook_decision(&v, &cmd, decision, None); } PayloadAction::Ignore => {} @@ -755,6 +757,146 @@ fn run_claude_inner(input: &str) -> Option { } } +// ── Codex CLI native hook ───────────────────────────────────── + +fn is_supported_codex_permission_mode(v: &Value) -> bool { + matches!( + v.get("permission_mode").and_then(Value::as_str), + Some("default" | "acceptEdits" | "plan" | "dontAsk" | "bypassPermissions") + ) +} + +fn process_codex_payload(v: &Value) -> PayloadAction { + if v.get("hook_event_name").and_then(Value::as_str) != Some(PRE_TOOL_USE_KEY) + || !matches!( + v.get("tool_name").and_then(Value::as_str), + Some("Bash" | "bash") + ) + { + return PayloadAction::Ignore; + } + + let cmd = match v + .pointer("/tool_input/command") + .and_then(Value::as_str) + .filter(|cmd| !cmd.is_empty()) + { + Some(cmd) => cmd, + None => return PayloadAction::Ignore, + }; + + // Codex deliberately includes this field in every hook event. Fail open + // for missing or future modes instead of assuming their approval semantics. + if !is_supported_codex_permission_mode(v) { + return PayloadAction::Skip { + decision: HookOutcome::Defer, + reason: "skip:unsupported_permission_mode", + cmd: cmd.to_string(), + }; + } + + if crate::discover::lexer::contains_unattestable_construct(cmd) { + return PayloadAction::Skip { + decision: HookOutcome::Defer, + reason: "skip:defer", + cmd: cmd.to_string(), + }; + } + + process_codex_payload_from_decision(v, cmd, decide_hook_action(cmd, permissions::Host::Codex)) +} + +fn process_codex_payload_from_decision( + v: &Value, + cmd: &str, + decision: HookDecision, +) -> PayloadAction { + let (rewritten, outcome) = match decision { + HookDecision::AllowRewrite(rewritten) => (rewritten, HookOutcome::Allow), + HookDecision::AskRewrite(rewritten) => (rewritten, HookOutcome::Ask), + HookDecision::Deny => { + return PayloadAction::Skip { + decision: HookOutcome::Deny, + reason: "skip:deny_rule", + cmd: cmd.to_string(), + } + } + HookDecision::Defer => { + return PayloadAction::Skip { + decision: HookOutcome::Defer, + reason: "skip:no_rewrite", + cmd: cmd.to_string(), + } + } + }; + + // Both AllowRewrite and AskRewrite require protocol-level allow: Codex + // cannot accept updatedInput with ask or an omitted permissionDecision. + // The internal outcome remains Ask for Default, not an RTK auto-approval. + // Its runtime applies the replacement before native approval and sandbox + // checks, so this is a protocol-level allow rather than RTK approving the + // command. Those checks inspect the rewritten argv, however, and Codex's + // safe/dangerous-command classifiers do not currently unwrap `rtk`. This + // can add prompts for known-safe commands and obscure classifier signals + // for wrapped mutating commands such as git push. Keep the passthrough gates + // above conservative and revisit this boundary whenever coverage expands. + PayloadAction::Rewrite { + cmd: cmd.to_string(), + output: pre_tool_use_rewrite_output(v, &rewritten, Some("allow")), + decision: outcome, + rewritten, + } +} + +/// Run the Codex CLI PreToolUse hook natively. +pub fn run_codex() -> Result<()> { + let input = match read_stdin_limited() { + Ok(input) => input, + Err(e) => { + let _ = writeln!(io::stderr(), "[rtk hook] Failed to read JSON input: {e}"); + return Ok(()); + } + }; + let input = strip_leading_bom(&input).trim(); + if input.is_empty() { + return Ok(()); + } + + let v: Value = match serde_json::from_str(input) { + Ok(v) => v, + Err(e) => { + let _ = writeln!(io::stderr(), "[rtk hook] Failed to parse JSON input: {e}"); + return Ok(()); + } + }; + + match process_codex_payload(&v) { + PayloadAction::Rewrite { + cmd, + rewritten, + output, + .. + } => { + audit_log("rewrite", &cmd, &rewritten); + let _ = writeln!(io::stdout(), "{output}"); + } + PayloadAction::Skip { cmd, reason, .. } => audit_log(reason, &cmd, ""), + PayloadAction::Ignore => {} + } + + Ok(()) +} + +#[cfg(test)] +fn run_codex_inner(input: &str) -> Option { + let input = strip_leading_bom(input).trim(); + let v: Value = serde_json::from_str(input).ok()?; + match process_codex_payload(&v) { + PayloadAction::Rewrite { output, .. } => Some(output.to_string()), + _ => None, + } +} + // ── Cursor native hook ───────────────────────────────────────── /// Run the Cursor Agent hook natively. @@ -906,21 +1048,7 @@ fn droid_response_from_decision(v: &Value, cmd: &str, decision: HookDecision) -> audit_log("rewrite", cmd, &rewritten); - let updated_input = { - let mut ti = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); - if let Some(obj) = ti.as_object_mut() { - obj.insert("command".into(), Value::String(rewritten)); - } - ti - }; - - Some(json!({ - "hookSpecificOutput": { - "hookEventName": PRE_TOOL_USE_KEY, - "permissionDecisionReason": "RTK auto-rewrite", - "updatedInput": updated_input - } - })) + Some(pre_tool_use_rewrite_output(v, &rewritten, None)) } /// Run the Factory Droid PreToolUse hook natively. @@ -1444,6 +1572,32 @@ mod tests { ); } + #[test] + fn test_pre_tool_use_rewrite_output_merges_input_and_optional_decision() { + let input = json!({ + "tool_input": { + "command": "git status", + "timeout": 30_000, + "description": "Inspect the working tree" + } + }); + + let ask = pre_tool_use_rewrite_output(&input, "rtk git status", None); + let ask_hook = &ask["hookSpecificOutput"]; + assert_eq!(ask_hook["hookEventName"], PRE_TOOL_USE_KEY); + assert_eq!(ask_hook["permissionDecisionReason"], "RTK auto-rewrite"); + assert!(ask_hook.get("permissionDecision").is_none()); + assert_eq!(ask_hook["updatedInput"]["command"], "rtk git status"); + assert_eq!(ask_hook["updatedInput"]["timeout"], 30_000); + assert_eq!( + ask_hook["updatedInput"]["description"], + "Inspect the working tree" + ); + + let allow = pre_tool_use_rewrite_output(&input, "rtk git status", Some("allow")); + assert_eq!(allow["hookSpecificOutput"]["permissionDecision"], "allow"); + } + // --- Claude handler --- fn claude_input(cmd: &str) -> String { @@ -1751,6 +1905,214 @@ mod tests { ); } + // --- Codex handler --- + + fn codex_input_with_permission_mode(cmd: &str, permission_mode: Option<&str>) -> String { + let mut input = json!({ + "session_id": "session-1", + "turn_id": "turn-1", + "hook_event_name": PRE_TOOL_USE_KEY, + "tool_name": "Bash", + "tool_use_id": "tool-1", + "tool_input": { "command": cmd } + }); + if let Some(permission_mode) = permission_mode { + input["permission_mode"] = json!(permission_mode); + } + input.to_string() + } + + // Normal Codex payloads include `default`; missing permission_mode is + // covered deliberately by the dedicated fail-open test below. + fn codex_input(cmd: &str) -> String { + codex_input_with_permission_mode(cmd, Some("default")) + } + + #[test] + fn test_codex_rewrite_uses_required_allow_shape() { + let result = run_codex_inner(&codex_input("git status")).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + let hook = &v["hookSpecificOutput"]; + + assert_eq!(hook["hookEventName"], PRE_TOOL_USE_KEY); + assert_eq!(hook["permissionDecision"], "allow"); + assert_eq!(hook["permissionDecisionReason"], "RTK auto-rewrite"); + assert_eq!(hook["updatedInput"]["command"], "rtk git status"); + } + + #[test] + fn test_codex_shared_decision_preserves_host_approval_and_deny() { + let input = json!({"tool_input": {"command": "git status", "timeout": 30000}}); + for (verdict, expected) in [ + (PermissionVerdict::Default, HookOutcome::Ask), + (PermissionVerdict::Ask, HookOutcome::Ask), + (PermissionVerdict::Allow, HookOutcome::Allow), + ] { + let decision = + super::super::decision::decide_with_params("git status", verdict, &[], &[]); + let PayloadAction::Rewrite { + output, decision, .. + } = process_codex_payload_from_decision(&input, "git status", decision) + else { + panic!("expected rewrite") + }; + assert_eq!(decision, expected); + assert_eq!(output["hookSpecificOutput"]["permissionDecision"], "allow"); + assert_eq!( + output["hookSpecificOutput"]["updatedInput"]["timeout"], + 30000 + ); + assert_eq!( + output["hookSpecificOutput"]["updatedInput"]["command"], + "rtk git status" + ); + } + let denied = super::super::decision::decide_with_params( + "git status", + PermissionVerdict::Deny, + &[], + &[], + ); + assert!(matches!( + process_codex_payload_from_decision(&input, "git status", denied), + PayloadAction::Skip { + decision: HookOutcome::Deny, + reason: "skip:deny_rule", + .. + } + )); + } + + #[test] + fn test_codex_skip_reasons_remain_distinct() { + for (command, mode, expected) in [ + ("git status", None, "skip:unsupported_permission_mode"), + ( + "git status", + Some("futureMode"), + "skip:unsupported_permission_mode", + ), + ("git status $(whoami)", Some("default"), "skip:defer"), + ("htop", Some("default"), "skip:no_rewrite"), + ] { + let input: Value = + serde_json::from_str(&codex_input_with_permission_mode(command, mode)).unwrap(); + let PayloadAction::Skip { reason, .. } = process_codex_payload(&input) else { + panic!("expected skip for {command}") + }; + assert_eq!(reason, expected); + } + } + + #[test] + fn test_codex_rewrites_all_documented_permission_modes() { + for permission_mode in [ + "default", + "acceptEdits", + "plan", + "dontAsk", + "bypassPermissions", + ] { + assert!( + run_codex_inner(&codex_input_with_permission_mode( + "git status", + Some(permission_mode) + )) + .is_some(), + "documented permission mode should rewrite: {permission_mode}" + ); + } + } + + #[test] + fn test_codex_unknown_or_missing_permission_mode_passes_through() { + assert!(run_codex_inner(&codex_input_with_permission_mode("git status", None)).is_none()); + assert!(run_codex_inner(&codex_input_with_permission_mode( + "git status", + Some("futureMode") + )) + .is_none()); + } + + #[test] + fn test_codex_rewrites_commands_that_still_need_native_approval() { + let result = run_codex_inner(&codex_input("cargo test")).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + + assert_eq!( + v.pointer("/hookSpecificOutput/updatedInput/command"), + Some(&json!("rtk cargo test")) + ); + } + + #[test] + fn test_codex_rewrite_preserves_tool_input_fields() { + let input = json!({ + "hook_event_name": PRE_TOOL_USE_KEY, + "tool_name": "Bash", + "permission_mode": "default", + "tool_input": { + "command": "git status --short", + "timeout": 30_000, + "description": "Inspect the working tree" + } + }) + .to_string(); + let result = run_codex_inner(&input).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + let updated = &v["hookSpecificOutput"]["updatedInput"]; + + assert_eq!(updated["command"], "rtk git status --short"); + assert_eq!(updated["timeout"], 30_000); + assert_eq!(updated["description"], "Inspect the working tree"); + } + + #[test] + fn test_codex_ignores_other_events_and_tools() { + let other_event = json!({ + "hook_event_name": "PostToolUse", + "tool_name": "Bash", + "tool_input": { "command": "git status" } + }) + .to_string(); + let other_tool = json!({ + "hook_event_name": PRE_TOOL_USE_KEY, + "tool_name": "apply_patch", + "tool_input": { "command": "git status" } + }) + .to_string(); + + assert!(run_codex_inner(&other_event).is_none()); + assert!(run_codex_inner(&other_tool).is_none()); + } + + #[test] + fn test_codex_passthrough_for_unsupported_or_unattestable_commands() { + assert!(run_codex_inner(&codex_input("rtk git status")).is_none()); + assert!(run_codex_inner(&codex_input("htop")).is_none()); + assert!(run_codex_inner(&codex_input("rm -rf /tmp/rtk-safety-test")).is_none()); + assert!(run_codex_inner(&codex_input("sudo rm -rf /tmp/rtk-safety-test")).is_none()); + assert!(run_codex_inner(&codex_input("git status > /tmp/status")).is_none()); + assert!(run_codex_inner(&codex_input("git status $(touch /tmp/x)")).is_none()); + } + + #[test] + fn test_codex_rewrites_compound_command_directly() { + let result = run_codex_inner(&codex_input("git add . && cargo test")).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + + assert_eq!( + v.pointer("/hookSpecificOutput/updatedInput/command"), + Some(&json!("rtk git add . && rtk cargo test")) + ); + } + + #[test] + fn test_codex_malformed_json_and_empty_command_pass_through() { + assert!(run_codex_inner("not valid json {{{").is_none()); + assert!(run_codex_inner(&codex_input("")).is_none()); + } + // --- Cursor handler --- fn cursor_input(cmd: &str) -> String { diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 272d74526d..e11588249d 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -14,17 +14,17 @@ use crate::hooks::constants::{ }; use super::constants::{ - BEFORE_TOOL_KEY, CLAUDE_DIR, CLAUDE_HOOK_COMMAND, CODEX_DIR, CURSOR_HOOK_COMMAND, DROID_DIR, - DROID_EXECUTE_MATCHER, DROID_HOME_ENV, DROID_HOOKS_FILE, DROID_HOOKS_SUBDIR, - DROID_HOOK_COMMAND, DROID_SETTINGS_FILE, GEMINI_HOOK_FILE, HERMES_DIR, HERMES_PLUGINS_SUBDIR, - HERMES_PLUGIN_INIT_FILE, HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, HOOKS_JSON, - HOOKS_SUBDIR, OMP_DIR, OMP_LOCAL_DIR, PI_AGENT_STATE_FILE, PI_CODING_AGENT_DIR_ENV, PI_DIR, - PI_EXTENSIONS_SUBDIR, PI_LOCAL_DIR, PI_PLUGIN_FILE, PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, - SETTINGS_JSON, VIBE_BASH_MATCH, VIBE_DIR, VIBE_HOOKS_FILE, VIBE_HOOK_COMMAND, VIBE_HOOK_NAME, - VIBE_PROMPTS_SUBDIR, VIBE_PROMPT_FILE, + BEFORE_TOOL_KEY, CLAUDE_DIR, CLAUDE_HOOK_COMMAND, CODEX_DIR, CODEX_HOOK_COMMAND, + CURSOR_HOOK_COMMAND, DROID_DIR, DROID_EXECUTE_MATCHER, DROID_HOME_ENV, DROID_HOOKS_FILE, + DROID_HOOKS_SUBDIR, DROID_HOOK_COMMAND, DROID_SETTINGS_FILE, GEMINI_HOOK_FILE, HERMES_DIR, + HERMES_PLUGINS_SUBDIR, HERMES_PLUGIN_INIT_FILE, HERMES_PLUGIN_MANIFEST_FILE, + HERMES_PLUGIN_NAME, HOOKS_JSON, HOOKS_SUBDIR, OMP_DIR, OMP_LOCAL_DIR, PI_AGENT_STATE_FILE, + PI_CODING_AGENT_DIR_ENV, PI_DIR, PI_EXTENSIONS_SUBDIR, PI_LOCAL_DIR, PI_PLUGIN_FILE, + PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, SETTINGS_JSON, VIBE_BASH_MATCH, VIBE_DIR, VIBE_HOOKS_FILE, + VIBE_HOOK_COMMAND, VIBE_HOOK_NAME, VIBE_PROMPTS_SUBDIR, VIBE_PROMPT_FILE, }; use super::integrity; -use super::is_claude_hook_command; +use super::{is_claude_hook_command, is_codex_hook_command}; use crate::core::config::AwarenessLevel; // Embedded OpenCode plugin (auto-rewrite) @@ -621,6 +621,46 @@ fn atomic_write(path: &Path, content: &str) -> Result<()> { Ok(()) } +/// Read a JSON file with path-aware errors. Missing files return `None` and +/// empty files are treated as an empty JSON object. +fn read_json_file(path: &Path) -> Result> { + if !path.exists() { + return Ok(None); + } + + let content = + fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))?; + let content = strip_leading_bom(&content); + if content.trim().is_empty() { + return Ok(Some(serde_json::json!({}))); + } + + from_json_str(content) + .map(Some) + .with_context(|| format!("Failed to parse {} as JSON", path.display())) +} + +/// Back up an existing JSON file before replacing it atomically. +fn backup_and_atomic_write(path: &Path, content: &str) -> Result> { + let backup_path = if path.exists() { + let backup_path = path.with_extension("json.bak"); + fs::copy(path, &backup_path).with_context(|| { + format!( + "Failed to backup {} to {}", + path.display(), + backup_path.display() + ) + })?; + Some(backup_path) + } else { + None + }; + + atomic_write(path, content) + .with_context(|| format!("Failed to update JSON file: {}", path.display()))?; + Ok(backup_path) +} + /// Prompt user for confirmation. /// Prints to stderr (stdout may be piped), reads from stdin, and defaults to /// No in non-interactive environments. @@ -1092,14 +1132,18 @@ pub fn uninstall_with_patch_mode( fn uninstall_codex(global: bool, ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; - if !global { - anyhow::bail!( - "Uninstall only works with --global flag. For local projects, manually remove RTK from AGENTS.md" - ); - } - - let codex_dir = resolve_codex_dir()?; - let removed = uninstall_codex_at(&codex_dir, ctx)?; + let removed = if global { + let codex_dir = resolve_codex_dir()?; + uninstall_codex_at(&codex_dir, ctx)? + } else { + uninstall_codex_with_paths( + Path::new(AGENTS_MD), + Path::new(RTK_MD), + &Path::new(CODEX_DIR).join(HOOKS_JSON), + &[RTK_MD_REF], + ctx, + )? + }; if removed.is_empty() { println!("RTK was not installed for Codex CLI (nothing to remove)"); @@ -1119,18 +1163,38 @@ fn uninstall_codex(global: bool, ctx: InitContext) -> Result<()> { } fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> { + let absolute_rtk_md_ref = codex_rtk_md_ref(codex_dir); + uninstall_codex_with_paths( + &codex_dir.join(AGENTS_MD), + &codex_dir.join(RTK_MD), + &codex_dir.join(HOOKS_JSON), + &[RTK_MD_REF, absolute_rtk_md_ref.as_str()], + ctx, + ) +} + +fn uninstall_codex_with_paths( + agents_md_path: &Path, + rtk_md_path: &Path, + hooks_json_path: &Path, + rtk_md_refs: &[&str], + ctx: InitContext, +) -> Result> { let InitContext { verbose, dry_run, .. } = ctx; let mut removed = Vec::new(); - let absolute_rtk_md_ref = codex_rtk_md_ref(codex_dir); - let rtk_md_path = codex_dir.join(RTK_MD); + if remove_codex_hook_from_file(hooks_json_path, ctx)? { + removed.push(format!("hooks.json: removed {} entry", CODEX_HOOK_COMMAND)); + } + if rtk_md_path.exists() { if dry_run { println!("[dry-run] would remove RTK.md: {}", rtk_md_path.display()); } else { - fs::remove_file(&rtk_md_path) + // nosemgrep: filesystem-deletion + fs::remove_file(rtk_md_path) .with_context(|| format!("Failed to remove RTK.md: {}", rtk_md_path.display()))?; if verbose > 0 { eprintln!("Removed RTK.md: {}", rtk_md_path.display()); @@ -1139,9 +1203,8 @@ fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> removed.push(format!("RTK.md: {}", rtk_md_path.display())); } - let agents_md_path = codex_dir.join(AGENTS_MD); if agents_md_path.exists() { - let content = fs::read_to_string(&agents_md_path) + let content = fs::read_to_string(agents_md_path) .with_context(|| format!("Failed to read AGENTS.md: {}", agents_md_path.display()))?; let mut working_content = content.clone(); @@ -1157,17 +1220,13 @@ fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> } if agents_changed { - atomic_write(&agents_md_path, &working_content).with_context(|| { + atomic_write(agents_md_path, &working_content).with_context(|| { format!("Failed to write AGENTS.md: {}", agents_md_path.display()) })?; } } - if remove_rtk_reference_from_agents( - &agents_md_path, - &[RTK_MD_REF, absolute_rtk_md_ref.as_str()], - ctx, - )? { + if remove_rtk_reference_from_agents(agents_md_path, rtk_md_refs, ctx)? { removed.push("AGENTS.md: removed @RTK.md reference".to_string()); } @@ -2634,19 +2693,28 @@ fn normalized_yaml_scalar(value: &str) -> Option { } fn run_codex_mode(global: bool, ctx: InitContext) -> Result<()> { - let (agents_md_path, rtk_md_path) = if global { + let (agents_md_path, rtk_md_path, hooks_json_path) = if global { let codex_dir = resolve_codex_dir()?; - (codex_dir.join(AGENTS_MD), codex_dir.join(RTK_MD)) + ( + codex_dir.join(AGENTS_MD), + codex_dir.join(RTK_MD), + codex_dir.join(HOOKS_JSON), + ) } else { - (PathBuf::from(AGENTS_MD), PathBuf::from(RTK_MD)) + ( + PathBuf::from(AGENTS_MD), + PathBuf::from(RTK_MD), + PathBuf::from(CODEX_DIR).join(HOOKS_JSON), + ) }; - run_codex_mode_with_paths(agents_md_path, rtk_md_path, global, ctx) + run_codex_mode_with_paths(agents_md_path, rtk_md_path, hooks_json_path, global, ctx) } fn run_codex_mode_with_paths( agents_md_path: PathBuf, rtk_md_path: PathBuf, + hooks_json_path: PathBuf, global: bool, ctx: InitContext, ) -> Result<()> { @@ -2675,13 +2743,22 @@ fn run_codex_mode_with_paths( RTK_MD_REF.to_string() }; - write_if_changed(&rtk_md_path, RTK_AWARENESS_FULL, RTK_MD, ctx)?; + write_if_changed(&rtk_md_path, awareness_content(ctx.awareness), RTK_MD, ctx)?; let added_ref = patch_agents_md(&agents_md_path, &rtk_md_ref, ctx)?; + let hook_added = patch_codex_hooks_json(&hooks_json_path, ctx)?; - print_rules_only_awareness_note("Codex CLI", ctx); if !dry_run { println!("\nRTK configured for Codex CLI.\n"); println!(" RTK.md: {}", rtk_md_path.display()); + println!( + " Hook: {} ({})", + hooks_json_path.display(), + if hook_added { + "registered" + } else { + "already present" + } + ); if added_ref { println!(" AGENTS.md: {} reference added", rtk_md_ref); } else { @@ -2698,11 +2775,161 @@ fn run_codex_mode_with_paths( agents_md_path.display() ); } + println!( + "\n Restart Codex. For a project hook, approve it when Codex asks you to trust it." + ); + match crate::core::tracking::get_db_path().and_then(|path| codex_tracking_config(&path)) { + Ok(config) => { + println!("\n Optional: if history recording fails with SQLITE_CANTOPEN in the"); + println!( + " workspace-write sandbox, grant write access to the tracking directory:" + ); + println!("\n{config}"); + println!( + " Merge this into Codex config.toml only if you want persistent history." + ); + println!(" Keep existing writable_roots; do not add a duplicate table."); + println!(" This grants access to the directory, including SQLite sidecar files."); + println!(" No Codex permission settings have been changed."); + } + Err(error) => eprintln!("rtk: warning: could not prepare tracking guidance: {error:#}"), + } } Ok(()) } +fn codex_tracking_config(db_path: &Path) -> Result { + let absolute = std::path::absolute(db_path).context("Failed to resolve RTK database path")?; + let parent = absolute + .parent() + .context("RTK database path has no parent")?; + let directory = fs::canonicalize(parent).unwrap_or_else(|_| parent.to_path_buf()); + let directory = directory + .to_str() + .context("RTK database directory is not valid UTF-8")?; + let quoted = toml::Value::String(directory.to_string()); + Ok(format!( + "sandbox_mode = \"workspace-write\"\n\n[sandbox_workspace_write]\nwritable_roots = [{quoted}]\n" + )) +} + +fn codex_hook_already_present(root: &serde_json::Value) -> bool { + root.pointer("/hooks/PreToolUse") + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .filter_map(|entry| entry.get("hooks")?.as_array()) + .flatten() + .filter_map(|hook| hook.get("command")?.as_str()) + .any(is_codex_hook_command) +} + +fn patch_codex_hooks_json(path: &Path, ctx: InitContext) -> Result { + let InitContext { + verbose, dry_run, .. + } = ctx; + let mut root = read_json_file(path)?.unwrap_or_else(|| serde_json::json!({})); + + if codex_hook_already_present(&root) { + return Ok(false); + } + + insert_hook_entry(&mut root, CODEX_HOOK_COMMAND)?; + let serialized = + serde_json::to_string_pretty(&root).context("Failed to serialize Codex hooks.json")?; + + if dry_run { + println!("[dry-run] would patch Codex hooks: {}", path.display()); + if verbose > 0 { + println!("[dry-run] content:\n{}", serialized); + } + return Ok(true); + } + + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!( + "Failed to create Codex config directory: {}", + parent.display() + ) + })?; + } + backup_and_atomic_write(path, &serialized)?; + if verbose > 0 { + eprintln!("Patched Codex hooks: {}", path.display()); + } + + Ok(true) +} + +fn remove_codex_hook_from_json(root: &mut serde_json::Value) -> bool { + let Some(pre_tool_use) = root + .pointer_mut("/hooks/PreToolUse") + .and_then(serde_json::Value::as_array_mut) + else { + return false; + }; + + let mut removed = false; + for entry in pre_tool_use.iter_mut() { + let Some(hooks) = entry + .get_mut("hooks") + .and_then(serde_json::Value::as_array_mut) + else { + continue; + }; + let before = hooks.len(); + hooks.retain(|hook| { + !hook + .get("command") + .and_then(serde_json::Value::as_str) + .is_some_and(is_codex_hook_command) + }); + removed |= hooks.len() != before; + } + pre_tool_use.retain(|entry| { + entry + .get("hooks") + .and_then(serde_json::Value::as_array) + .is_none_or(|hooks| !hooks.is_empty()) + }); + + removed +} + +fn remove_codex_hook_from_file(path: &Path, ctx: InitContext) -> Result { + let InitContext { + verbose, dry_run, .. + } = ctx; + let Some(mut root) = read_json_file(path)? else { + return Ok(false); + }; + if !remove_codex_hook_from_json(&mut root) { + return Ok(false); + } + + let serialized = + serde_json::to_string_pretty(&root).context("Failed to serialize Codex hooks.json")?; + if dry_run { + println!( + "[dry-run] would remove RTK hook entry from {}", + path.display() + ); + if verbose > 0 { + println!("[dry-run] content:\n{}", serialized); + } + return Ok(true); + } + + backup_and_atomic_write(path, &serialized)?; + if verbose > 0 { + eprintln!("Removed Codex RTK hook: {}", path.display()); + } + + Ok(true) +} + // --- upsert_rtk_block: idempotent RTK block management --- #[derive(Debug, Clone, Copy, PartialEq)] @@ -3220,23 +3447,6 @@ fn droid_hook_file_candidates(droid_dir: &Path) -> [DroidHookFile; 3] { ] } -/// Read a Droid config file as JSON. `Ok(None)` when the file doesn't exist; -/// an empty file parses as `{}`. -fn read_droid_json(path: &Path) -> Result> { - if !path.exists() { - return Ok(None); - } - let content = - fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))?; - let content = strip_leading_bom(&content); - if content.trim().is_empty() { - return Ok(Some(serde_json::json!({}))); - } - from_json_str(content) - .map(Some) - .with_context(|| format!("Failed to parse {} as JSON", path.display())) -} - /// The JSON object holding hook events for the given layout, if present. fn droid_events(root: &serde_json::Value, layout: DroidLayout) -> &serde_json::Value { match layout { @@ -3279,7 +3489,7 @@ fn resolve_droid_install_target(droid_dir: &Path) -> Result { }; if let Some(path) = &live_hooks_json { - if let Some(json) = read_droid_json(path)? { + if let Some(json) = read_json_file(path)? { if droid_has_pre_tool_use(&json, DroidLayout::Root) { return Ok(DroidHookFile { path: path.clone(), @@ -3289,7 +3499,7 @@ fn resolve_droid_install_target(droid_dir: &Path) -> Result { } } - if let Some(json) = read_droid_json(&settings)? { + if let Some(json) = read_json_file(&settings)? { if droid_has_pre_tool_use(&json, DroidLayout::Nested) { return Ok(DroidHookFile { path: settings, @@ -3375,7 +3585,7 @@ fn patch_droid_hook_file(file: &DroidHookFile, ctx: InitContext) -> Result verbose, dry_run, .. } = ctx; let path = &file.path; - let mut root = read_droid_json(path)?.unwrap_or_else(|| serde_json::json!({})); + let mut root = read_json_file(path)?.unwrap_or_else(|| serde_json::json!({})); if droid_hook_already_present(&root, file.layout) { if verbose > 0 { @@ -3397,16 +3607,11 @@ fn patch_droid_hook_file(file: &DroidHookFile, ctx: InitContext) -> Result return Ok(true); } - if path.exists() { - let backup_path = path.with_extension("json.bak"); - fs::copy(path, &backup_path) - .with_context(|| format!("Failed to backup to {}", backup_path.display()))?; + if let Some(backup_path) = backup_and_atomic_write(path, &serialized)? { if verbose > 0 { eprintln!("Backup: {}", backup_path.display()); } } - - atomic_write(path, &serialized)?; Ok(true) } @@ -3520,11 +3725,24 @@ pub fn uninstall_droid(global: bool, ctx: InitContext) -> Result<()> { fn uninstall_droid_at(droid_dir: &Path, ctx: InitContext) -> Result> { let mut removed = Vec::new(); + let mut errors = Vec::new(); for candidate in droid_hook_file_candidates(droid_dir) { - if remove_droid_hook_from_file(&candidate, ctx)? { - removed.push(format!("Droid hook file: {}", candidate.path.display())); + match remove_droid_hook_from_file(&candidate, ctx) { + Ok(true) => { + removed.push(format!("Droid hook file: {}", candidate.path.display())); + } + Ok(false) => {} + Err(error) => errors.push(format!("{}: {error:#}", candidate.path.display())), } } + + if !errors.is_empty() { + return Err(anyhow::anyhow!( + "Failed to uninstall RTK from one or more Droid hook files:\n - {}", + errors.join("\n - ") + )); + } + Ok(removed) } @@ -3536,7 +3754,7 @@ fn remove_droid_hook_from_file(file: &DroidHookFile, ctx: InitContext) -> Result } = ctx; let path = &file.path; - let mut root = match read_droid_json(path)? { + let mut root = match read_json_file(path)? { Some(v) => v, None => return Ok(false), }; @@ -3551,12 +3769,9 @@ fn remove_droid_hook_from_file(file: &DroidHookFile, ctx: InitContext) -> Result path.display() ); } else { - let backup_path = path.with_extension("json.bak"); - fs::copy(path, &backup_path).ok(); - let serialized = serde_json::to_string_pretty(&root).context("Failed to serialize Droid hook file")?; - atomic_write(path, &serialized)?; + backup_and_atomic_write(path, &serialized)?; if verbose > 0 { eprintln!("Removed RTK hook from {}", path.display()); @@ -4611,19 +4826,7 @@ fn patch_cursor_hooks_json(path: &Path, ctx: InitContext) -> Result { let InitContext { verbose, dry_run, .. } = ctx; - let mut root = if path.exists() { - let content = fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; - let content = strip_leading_bom(&content); - if content.trim().is_empty() { - serde_json::json!({ "version": 1 }) - } else { - from_json_str(content) - .with_context(|| format!("Failed to parse {} as JSON", path.display()))? - } - } else { - serde_json::json!({ "version": 1 }) - }; + let mut root = read_json_file(path)?.unwrap_or_else(|| serde_json::json!({ "version": 1 })); // Check idempotency if cursor_hook_already_present(&root) { @@ -4649,19 +4852,12 @@ fn patch_cursor_hooks_json(path: &Path, ctx: InitContext) -> Result { return Ok(true); } - // Backup if exists - if path.exists() { - let backup_path = path.with_extension("json.bak"); - fs::copy(path, &backup_path) - .with_context(|| format!("Failed to backup to {}", backup_path.display()))?; + if let Some(backup_path) = backup_and_atomic_write(path, &serialized)? { if verbose > 0 { eprintln!("Backup: {}", backup_path.display()); } } - // Atomic write - atomic_write(path, &serialized)?; - Ok(true) } @@ -4722,19 +4918,9 @@ fn remove_legacy_cursor_hooks_json_entries(path: &Path, ctx: InitContext) -> Res let InitContext { verbose, dry_run, .. } = ctx; - if !path.exists() { + let Some(mut root) = read_json_file(path)? else { return Ok(()); - } - - let content = - fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))?; - let content = strip_leading_bom(&content); - if content.trim().is_empty() { - return Ok(()); - } - - let mut root: serde_json::Value = - from_json_str(content).with_context(|| format!("Failed to parse {}", path.display()))?; + }; if !remove_legacy_cursor_hook_entries_from_json(&mut root) { return Ok(()); @@ -4750,7 +4936,7 @@ fn remove_legacy_cursor_hooks_json_entries(path: &Path, ctx: InitContext) -> Res let serialized = serde_json::to_string_pretty(&root).context("Failed to serialize hooks.json")?; - atomic_write(path, &serialized)?; + backup_and_atomic_write(path, &serialized)?; if verbose > 0 { eprintln!(" [ok] Removed legacy rtk-rewrite.sh entry from Cursor hooks.json"); @@ -4784,10 +4970,14 @@ fn remove_legacy_cursor_hook_entries_from_json(root: &mut serde_json::Value) -> /// Remove Cursor RTK artifacts: hook script + hooks.json entry fn remove_cursor_hooks(ctx: InitContext) -> Result> { + let cursor_dir = resolve_cursor_dir()?; + remove_cursor_hooks_at(&cursor_dir, ctx) +} + +fn remove_cursor_hooks_at(cursor_dir: &Path, ctx: InitContext) -> Result> { let InitContext { verbose, dry_run, .. } = ctx; - let cursor_dir = resolve_cursor_dir()?; let mut removed = Vec::new(); // 1. Remove hook script @@ -4809,34 +4999,33 @@ fn remove_cursor_hooks(ctx: InitContext) -> Result> { // 2. Remove RTK entry from hooks.json let hooks_json_path = cursor_dir.join(HOOKS_JSON); - if hooks_json_path.exists() { - let content = fs::read_to_string(&hooks_json_path) - .with_context(|| format!("Failed to read {}", hooks_json_path.display()))?; - let content = strip_leading_bom(&content); - - if !content.trim().is_empty() { - if let Ok(mut root) = from_json_str::(content) { - if remove_cursor_hook_from_json(&mut root) { - if dry_run { - println!( - "[dry-run] would remove RTK entry from Cursor hooks.json: {}", - hooks_json_path.display() - ); - } else { - let backup_path = hooks_json_path.with_extension("json.bak"); - fs::copy(&hooks_json_path, &backup_path).ok(); - - let serialized = serde_json::to_string_pretty(&root) - .context("Failed to serialize hooks.json")?; - atomic_write(&hooks_json_path, &serialized)?; + let root = match read_json_file(&hooks_json_path) { + Ok(root) => root, + Err(error) if error.downcast_ref::().is_some() => { + eprintln!( + "rtk: warning: leaving malformed Cursor hooks.json unchanged during uninstall: {error:#}" + ); + None + } + Err(error) => return Err(error), + }; + if let Some(mut root) = root { + if remove_cursor_hook_from_json(&mut root) { + if dry_run { + println!( + "[dry-run] would remove RTK entry from Cursor hooks.json: {}", + hooks_json_path.display() + ); + } else { + let serialized = serde_json::to_string_pretty(&root) + .context("Failed to serialize hooks.json")?; + backup_and_atomic_write(&hooks_json_path, &serialized)?; - if verbose > 0 { - eprintln!("Removed RTK hook from Cursor hooks.json"); - } - } - removed.push("Cursor hooks.json: removed RTK entry".to_string()); + if verbose > 0 { + eprintln!("Removed RTK hook from Cursor hooks.json"); } } + removed.push("Cursor hooks.json: removed RTK entry".to_string()); } } @@ -5160,8 +5349,8 @@ fn show_claude_config() -> Result<()> { println!(" rtk init -g --uninstall # Remove all RTK artifacts"); println!(" rtk init -g --claude-md # Legacy: full injection into ~/.claude/CLAUDE.md"); println!(" rtk init -g --hook-only # Hook only, no RTK.md"); - println!(" rtk init --codex # Configure local AGENTS.md + RTK.md"); - println!(" rtk init -g --codex # Configure $CODEX_HOME/AGENTS.md + $CODEX_HOME/RTK.md (or ~/.codex/)"); + println!(" rtk init --codex # Configure local AGENTS.md + RTK.md + hooks.json"); + println!(" rtk init -g --codex # Configure global AGENTS.md + RTK.md + hooks.json"); println!(" rtk init -g --opencode # OpenCode plugin only"); println!(" rtk init -g --agent cursor # Install Cursor Agent hooks"); @@ -5172,9 +5361,11 @@ fn show_codex_config() -> Result<()> { let codex_dir = resolve_codex_dir()?; let global_agents_md = codex_dir.join(AGENTS_MD); let global_rtk_md = codex_dir.join(RTK_MD); + let global_hooks_json = codex_dir.join(HOOKS_JSON); let global_rtk_md_ref = codex_rtk_md_ref(&codex_dir); let local_agents_md = PathBuf::from(AGENTS_MD); let local_rtk_md = PathBuf::from(RTK_MD); + let local_hooks_json = PathBuf::from(CODEX_DIR).join(HOOKS_JSON); println!("rtk Configuration (Codex CLI):\n"); @@ -5184,8 +5375,31 @@ fn show_codex_config() -> Result<()> { println!("[--] Global RTK.md: not found"); } + if global_hooks_json.exists() { + let content = fs::read_to_string(&global_hooks_json).with_context(|| { + format!( + "Failed to read global Codex hooks: {}", + global_hooks_json.display() + ) + })?; + match serde_json::from_str::(&content) { + Ok(root) if codex_hook_already_present(&root) => { + println!("[ok] Global hook: {}", global_hooks_json.display()); + } + Ok(_) => println!("[--] Global hooks.json exists but RTK hook is not configured"), + Err(_) => println!("[!!] Global hooks.json is invalid JSON"), + } + } else { + println!("[--] Global hook: not found"); + } + if global_agents_md.exists() { - let content = fs::read_to_string(&global_agents_md)?; + let content = fs::read_to_string(&global_agents_md).with_context(|| { + format!( + "Failed to read global Codex instructions: {}", + global_agents_md.display() + ) + })?; if has_rtk_reference(&content, &[RTK_MD_REF, global_rtk_md_ref.as_str()]) { println!("[ok] Global AGENTS.md: RTK.md reference"); } else if content.contains(RTK_BLOCK_START) { @@ -5203,8 +5417,31 @@ fn show_codex_config() -> Result<()> { println!("[--] Local RTK.md: not found"); } + if local_hooks_json.exists() { + let content = fs::read_to_string(&local_hooks_json).with_context(|| { + format!( + "Failed to read local Codex hooks: {}", + local_hooks_json.display() + ) + })?; + match serde_json::from_str::(&content) { + Ok(root) if codex_hook_already_present(&root) => { + println!("[ok] Local hook: {}", local_hooks_json.display()); + } + Ok(_) => println!("[--] Local hooks.json exists but RTK hook is not configured"), + Err(_) => println!("[!!] Local hooks.json is invalid JSON"), + } + } else { + println!("[--] Local hook: not found"); + } + if local_agents_md.exists() { - let content = fs::read_to_string(&local_agents_md)?; + let content = fs::read_to_string(&local_agents_md).with_context(|| { + format!( + "Failed to read local Codex instructions: {}", + local_agents_md.display() + ) + })?; if has_rtk_reference(&content, &[RTK_MD_REF]) { println!("[ok] Local AGENTS.md: @RTK.md reference"); } else if content.contains(RTK_BLOCK_START) { @@ -5217,8 +5454,9 @@ fn show_codex_config() -> Result<()> { } println!("\nUsage:"); - println!(" rtk init --codex # Configure local AGENTS.md + RTK.md"); - println!(" rtk init -g --codex # Configure $CODEX_HOME/AGENTS.md + $CODEX_HOME/RTK.md (or ~/.codex/)"); + println!(" rtk init --codex # Configure local AGENTS.md + RTK.md + hooks.json"); + println!(" rtk init -g --codex # Configure global AGENTS.md + RTK.md + hooks.json"); + println!(" rtk init --codex --uninstall # Remove local Codex RTK artifacts"); println!(" rtk init -g --codex --uninstall # Remove global Codex RTK artifacts"); Ok(()) @@ -6579,7 +6817,7 @@ mod tests { } #[test] - fn test_rules_only_agents_get_full_awareness_at_every_level() { + fn test_agents_get_appropriate_awareness_at_every_level() { for level in [ AwarenessLevel::Default, AwarenessLevel::High, @@ -6616,10 +6854,17 @@ mod tests { let codex_agents = temp.path().join("codex").join(AGENTS_MD); let codex_rtk = temp.path().join("codex").join(RTK_MD); fs::create_dir_all(temp.path().join("codex")).unwrap(); - run_codex_mode_with_paths(codex_agents, codex_rtk.clone(), false, ctx).unwrap(); + run_codex_mode_with_paths( + codex_agents, + codex_rtk.clone(), + temp.path().join("codex/hooks.json"), + false, + ctx, + ) + .unwrap(); assert_eq!( fs::read_to_string(&codex_rtk).unwrap(), - RTK_AWARENESS_FULL, + awareness_content(level), "codex with level {level}" ); } @@ -7219,21 +7464,96 @@ mod tests { let temp = TempDir::new().unwrap(); let agents_md = temp.path().join("AGENTS.md"); let rtk_md = temp.path().join("RTK.md"); + let hooks_json = temp.path().join(HOOKS_JSON); run_codex_mode_with_paths( agents_md.clone(), rtk_md.clone(), + hooks_json.clone(), true, InitContext::default(), ) .unwrap(); assert!(rtk_md.exists()); - assert_eq!(fs::read_to_string(&rtk_md).unwrap(), RTK_AWARENESS_FULL); + assert_eq!(fs::read_to_string(&rtk_md).unwrap(), RTK_AWARENESS_DEFAULT); assert_eq!( fs::read_to_string(&agents_md).unwrap(), format!("{}\n", codex_rtk_md_ref(temp.path())) ); + let hooks: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hooks_json).unwrap()).unwrap(); + assert!(codex_hook_already_present(&hooks)); + } + + #[test] + fn test_codex_tracking_config_limits_root_to_database_parent() { + let temp = TempDir::new().unwrap(); + let directory = temp.path().join("tracking data"); + fs::create_dir(&directory).unwrap(); + let config = codex_tracking_config(&directory.join("custom.db")).unwrap(); + let parsed: toml::Value = toml::from_str(&config).unwrap(); + assert_eq!(parsed["sandbox_mode"].as_str(), Some("workspace-write")); + let roots = parsed["sandbox_workspace_write"]["writable_roots"] + .as_array() + .unwrap(); + assert_eq!(roots.len(), 1); + assert_eq!( + roots[0].as_str().unwrap(), + fs::canonicalize(&directory).unwrap().to_str().unwrap() + ); + assert!(!directory.join("custom.db").exists()); + } + + #[test] + fn test_codex_tracking_config_escapes_paths_without_creating_files() { + let temp = TempDir::new().unwrap(); + let directory = temp.path().join("quoted \"directory\""); + let config = codex_tracking_config(&directory.join("history.db")).unwrap(); + let parsed: toml::Value = toml::from_str(&config).unwrap(); + assert_eq!( + parsed["sandbox_workspace_write"]["writable_roots"][0] + .as_str() + .unwrap(), + directory.to_str().unwrap() + ); + assert!(!directory.exists()); + } + + #[test] + fn test_patch_codex_hooks_is_idempotent_and_preserves_existing_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_json = temp.path().join(HOOKS_JSON); + fs::write( + &hooks_json, + serde_json::to_string_pretty(&serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [{ "type": "command", "command": "echo existing" }] + }], + "Stop": [{ + "hooks": [{ "type": "command", "command": "echo stop" }] + }] + } + })) + .unwrap(), + ) + .unwrap(); + + assert!(patch_codex_hooks_json(&hooks_json, InitContext::default()).unwrap()); + assert!(!patch_codex_hooks_json(&hooks_json, InitContext::default()).unwrap()); + + let root: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hooks_json).unwrap()).unwrap(); + assert!(codex_hook_already_present(&root)); + assert_eq!(root["hooks"]["PreToolUse"].as_array().unwrap().len(), 2); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo existing" + ); + assert_eq!(root["hooks"]["Stop"][0]["hooks"][0]["command"], "echo stop"); + assert!(hooks_json.with_extension("json.bak").exists()); } #[test] @@ -7467,6 +7787,62 @@ mod tests { ); } + #[test] + fn test_remove_droid_hook_propagates_backup_failure_and_preserves_file() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join(DROID_HOOKS_FILE); + let original = serde_json::to_string_pretty(&serde_json::json!({ + "PreToolUse": [{ + "matcher": DROID_EXECUTE_MATCHER, + "hooks": [{ "type": "command", "command": DROID_HOOK_COMMAND }] + }] + })) + .unwrap(); + fs::write(&path, &original).unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + + let err = remove_droid_hook_from_file( + &DroidHookFile { + path: path.clone(), + layout: DroidLayout::Root, + }, + InitContext::default(), + ) + .unwrap_err(); + + assert!(format!("{err:#}").contains("backup")); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } + + #[test] + fn test_uninstall_droid_continues_after_candidate_failure() { + let temp = TempDir::new().unwrap(); + let droid_dir = temp.path().join(DROID_DIR); + let root_path = droid_dir.join(DROID_HOOKS_FILE); + let legacy_path = droid_dir.join(DROID_HOOKS_SUBDIR).join(DROID_HOOKS_FILE); + fs::create_dir_all(legacy_path.parent().unwrap()).unwrap(); + + let hook_json = serde_json::to_string_pretty(&serde_json::json!({ + "PreToolUse": [{ + "matcher": DROID_EXECUTE_MATCHER, + "hooks": [{ "type": "command", "command": DROID_HOOK_COMMAND }] + }] + })) + .unwrap(); + fs::write(&root_path, &hook_json).unwrap(); + fs::write(&legacy_path, &hook_json).unwrap(); + fs::create_dir(root_path.with_extension("json.bak")).unwrap(); + + let err = uninstall_droid_at(&droid_dir, InitContext::default()).unwrap_err(); + + assert!(format!("{err:#}").contains(&root_path.display().to_string())); + assert_eq!(fs::read_to_string(&root_path).unwrap(), hook_json); + let legacy: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&legacy_path).unwrap()).unwrap(); + assert!(!droid_hook_already_present(&legacy, DroidLayout::Root)); + assert!(legacy_path.with_extension("json.bak").exists()); + } + #[test] fn test_droid_target_defaults_to_hooks_json() { // Fresh setup: the canonical hooks.json is created (Droid's own @@ -7633,6 +8009,32 @@ mod tests { assert!(content.contains("# Team rules")); } + #[test] + fn test_local_codex_install_can_be_uninstalled() { + let _cwd_guard = CWD_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let temp = TempDir::new().unwrap(); + let original_cwd = std::env::current_dir().unwrap(); + std::env::set_current_dir(temp.path()).unwrap(); + + fs::write(AGENTS_MD, "# Team rules\n").unwrap(); + run_codex_mode(false, InitContext::default()).unwrap(); + let uninstall_result = uninstall_codex(false, InitContext::default()); + + std::env::set_current_dir(original_cwd).unwrap(); + uninstall_result.unwrap(); + + assert!(!temp.path().join(RTK_MD).exists()); + assert!(!codex_hook_already_present( + &serde_json::from_str( + &fs::read_to_string(temp.path().join(CODEX_DIR).join(HOOKS_JSON)).unwrap() + ) + .unwrap() + )); + let agents = fs::read_to_string(temp.path().join(AGENTS_MD)).unwrap(); + assert_eq!(agents.trim_end(), "# Team rules"); + assert!(!agents.contains(RTK_MD_REF)); + } + #[test] fn test_uninstall_codex_at_removes_absolute_reference() { let temp = TempDir::new().unwrap(); @@ -7652,6 +8054,66 @@ mod tests { assert!(content.contains("# Team rules")); } + #[test] + fn test_remove_codex_hook_preserves_other_hooks_in_same_entry() { + let mut root = serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo user hook" }, + { "type": "command", "command": CODEX_HOOK_COMMAND } + ] + }], + "Stop": [{ + "hooks": [{ "type": "command", "command": "echo stop" }] + }] + } + }); + + assert!(remove_codex_hook_from_json(&mut root)); + assert!(!codex_hook_already_present(&root)); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo user hook" + ); + assert_eq!(root["hooks"]["Stop"][0]["hooks"][0]["command"], "echo stop"); + } + + #[test] + fn test_uninstall_codex_at_removes_hook_and_preserves_other_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_json = temp.path().join(HOOKS_JSON); + fs::write( + &hooks_json, + serde_json::to_string_pretty(&serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo user hook" }, + { "type": "command", "command": CODEX_HOOK_COMMAND } + ] + }] + } + })) + .unwrap(), + ) + .unwrap(); + + let removed = uninstall_codex_at(temp.path(), InitContext::default()).unwrap(); + + assert_eq!(removed.len(), 1); + let root: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hooks_json).unwrap()).unwrap(); + assert!(!codex_hook_already_present(&root)); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo user hook" + ); + assert!(hooks_json.with_extension("json.bak").exists()); + } + #[test] fn test_write_if_changed_dry_run_does_not_create_file() { let temp = TempDir::new().unwrap(); @@ -7709,10 +8171,12 @@ mod tests { let temp = TempDir::new().unwrap(); let agents_md = temp.path().join("AGENTS.md"); let rtk_md = temp.path().join("RTK.md"); + let hooks_json = temp.path().join(HOOKS_JSON); run_codex_mode_with_paths( agents_md.clone(), rtk_md.clone(), + hooks_json.clone(), true, InitContext { dry_run: true, @@ -7731,6 +8195,11 @@ mod tests { "dry-run must not create AGENTS.md: {}", agents_md.display() ); + assert!( + !hooks_json.exists(), + "dry-run must not create hooks.json: {}", + hooks_json.display() + ); } #[test] @@ -7955,6 +8424,56 @@ mod tests { assert_eq!(written, content); } + #[test] + fn test_read_json_file_handles_missing_and_empty_files() { + let temp = TempDir::new().unwrap(); + let missing = temp.path().join("missing.json"); + let empty = temp.path().join("empty.json"); + fs::write(&empty, " \n").unwrap(); + + assert!(read_json_file(&missing).unwrap().is_none()); + assert_eq!(read_json_file(&empty).unwrap(), Some(serde_json::json!({}))); + } + + #[test] + fn test_read_json_file_errors_include_the_path() { + let temp = TempDir::new().unwrap(); + let invalid = temp.path().join("invalid.json"); + fs::write(&invalid, "{").unwrap(); + + let parse_error = read_json_file(&invalid).unwrap_err(); + assert!(format!("{parse_error:#}").contains(&invalid.display().to_string())); + + let read_error = read_json_file(temp.path()).unwrap_err(); + assert!(format!("{read_error:#}").contains(&temp.path().display().to_string())); + } + + #[test] + fn test_backup_and_atomic_write_preserves_previous_content() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join("hooks.json"); + fs::write(&path, "old").unwrap(); + + let backup = backup_and_atomic_write(&path, "new").unwrap().unwrap(); + + assert_eq!(backup, path.with_extension("json.bak")); + assert_eq!(fs::read_to_string(path).unwrap(), "new"); + assert_eq!(fs::read_to_string(backup).unwrap(), "old"); + } + + #[test] + fn test_backup_and_atomic_write_failure_preserves_original() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join("hooks.json"); + fs::write(&path, "old").unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + + let err = backup_and_atomic_write(&path, "new").unwrap_err(); + + assert!(format!("{err:#}").contains("backup")); + assert_eq!(fs::read_to_string(path).unwrap(), "old"); + } + #[cfg(unix)] #[test] fn test_atomic_write_preserves_symlink() { @@ -8244,6 +8763,61 @@ mod tests { assert!(json_content["hooks"]["afterFileEdit"].is_array()); } + #[test] + fn test_patch_cursor_hook_propagates_backup_failure_and_preserves_file() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join(HOOKS_JSON); + let original = serde_json::to_string_pretty(&serde_json::json!({ + "version": 1, + "hooks": { + "preToolUse": [{ + "command": "./hooks/other.sh", + "matcher": "Shell" + }] + } + })) + .unwrap(); + fs::write(&path, &original).unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + + let err = patch_cursor_hooks_json(&path, InitContext::default()).unwrap_err(); + + assert!(format!("{err:#}").contains("backup")); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } + + #[test] + fn test_remove_cursor_hooks_keeps_malformed_json_best_effort() { + let temp = TempDir::new().unwrap(); + let cursor_dir = temp.path().join(CURSOR_DIR); + let hook_path = cursor_dir.join(HOOKS_SUBDIR).join(REWRITE_HOOK_FILE); + let hooks_json = cursor_dir.join(HOOKS_JSON); + fs::create_dir_all(hook_path.parent().unwrap()).unwrap(); + fs::write(&hook_path, "legacy hook").unwrap(); + fs::write(&hooks_json, "{").unwrap(); + + let removed = remove_cursor_hooks_at(&cursor_dir, InitContext::default()).unwrap(); + + assert!(!hook_path.exists()); + assert_eq!(fs::read_to_string(hooks_json).unwrap(), "{"); + assert_eq!( + removed, + vec![format!("Cursor hook: {}", hook_path.display())] + ); + } + + #[test] + fn test_remove_cursor_hooks_still_propagates_read_errors() { + let temp = TempDir::new().unwrap(); + let cursor_dir = temp.path().join(CURSOR_DIR); + let hooks_json = cursor_dir.join(HOOKS_JSON); + fs::create_dir_all(&hooks_json).unwrap(); + + let err = remove_cursor_hooks_at(&cursor_dir, InitContext::default()).unwrap_err(); + + assert!(format!("{err:#}").contains(&hooks_json.display().to_string())); + } + #[test] fn test_remove_cursor_hook_from_json() { let mut json_content = serde_json::json!({ diff --git a/src/hooks/mod.rs b/src/hooks/mod.rs index 5b439aef9d..33040142c2 100644 --- a/src/hooks/mod.rs +++ b/src/hooks/mod.rs @@ -17,15 +17,26 @@ pub mod rewrite_cmd; pub mod trust; pub mod verify_cmd; -pub fn is_claude_hook_command(command: &str) -> bool { +fn is_rtk_binary(binary: &str) -> bool { + let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary); + matches!(binary_name, "rtk" | "rtk.exe") +} + +fn is_rtk_hook_command(command: &str, agent: &str) -> bool { let parts = crate::discover::lexer::shell_split(command); - let [binary, hook, claude] = parts.as_slice() else { + let [parsed_binary, hook, target] = parts.as_slice() else { return false; }; - let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary); + is_rtk_binary(parsed_binary) && hook == "hook" && target == agent +} + +pub fn is_claude_hook_command(command: &str) -> bool { + is_rtk_hook_command(command, "claude") +} - binary_name == "rtk" && hook == "hook" && claude == "claude" +pub fn is_codex_hook_command(command: &str) -> bool { + is_rtk_hook_command(command, "codex") } #[cfg(test)] @@ -39,6 +50,9 @@ mod tests { assert!(is_claude_hook_command( "\"/opt/homebrew/bin/rtk\" hook claude" )); + assert!(is_claude_hook_command( + "/Users/jane/My\\ Apps/rtk hook claude" + )); } #[test] @@ -47,4 +61,20 @@ mod tests { assert!(!is_claude_hook_command("/opt/homebrew/bin/rtk hook cursor")); assert!(!is_claude_hook_command("echo rtk hook claude")); } + + #[test] + fn codex_hook_command_matches_bare_absolute_and_windows_rtk() { + assert!(is_codex_hook_command("rtk hook codex")); + assert!(is_codex_hook_command("/opt/homebrew/bin/rtk hook codex")); + assert!(is_codex_hook_command( + "\"C:\\Program Files\\rtk.exe\" hook codex" + )); + } + + #[test] + fn codex_hook_command_rejects_other_commands() { + assert!(!is_codex_hook_command("rtk hook claude")); + assert!(!is_codex_hook_command("echo rtk hook codex")); + assert!(!is_codex_hook_command("\"rtk\"evil hook codex")); + } } diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 3c44ad4a1a..abc90c90e8 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -34,6 +34,7 @@ pub fn check_command(cmd: &str) -> PermissionVerdict { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Host { Claude, + Codex, Cursor, Gemini, Droid, @@ -57,7 +58,11 @@ pub(crate) fn load_rules_for(host: Host) -> (Vec, Vec, Vec load_cursor_rules(), Host::Gemini => load_gemini_rules(), Host::Droid => load_droid_rules(), - Host::Vibe => (Vec::new(), Vec::new(), Vec::new()), + // Hosts with no RTK-side rule source. Codex enforces its native + // execution rules after updatedInput. Do not interpret either host's + // rules as Claude Bash patterns or borrow another host's settings. + // No RTK-side match means Default, not an explicit Allow. + Host::Codex | Host::Vibe => (Vec::new(), Vec::new(), Vec::new()), } } diff --git a/src/main.rs b/src/main.rs index b0849ae917..c0b06b0a46 100644 --- a/src/main.rs +++ b/src/main.rs @@ -405,7 +405,7 @@ enum Commands { #[arg(long)] uninstall: bool, - /// Target Codex CLI (uses AGENTS.md + RTK.md, no Claude hook patching) + /// Target Codex CLI (uses PreToolUse hook + AGENTS.md + RTK.md) #[arg(long)] codex: bool, @@ -953,6 +953,8 @@ enum Commands { enum HookCommands { /// Process Claude Code PreToolUse hook (reads JSON from stdin) Claude, + /// Process Codex CLI PreToolUse hook (reads JSON from stdin) + Codex, /// Process Cursor Agent hook (reads JSON from stdin) Cursor, /// Process Gemini CLI BeforeTool hook (reads JSON from stdin) @@ -2768,6 +2770,10 @@ fn run_cli() -> Result { hooks::hook_cmd::run_claude()?; 0 } + HookCommands::Codex => { + hooks::hook_cmd::run_codex()?; + 0 + } HookCommands::Cursor => { hooks::hook_cmd::run_cursor()?; 0 @@ -3716,6 +3722,17 @@ mod tests { )); } + #[test] + fn test_hook_codex_parses() { + let cli = Cli::try_parse_from(["rtk", "hook", "codex"]).unwrap(); + assert!(matches!( + cli.command, + Commands::Hook { + command: HookCommands::Codex + } + )); + } + #[test] fn test_hook_check_parses() { let cli = Cli::try_parse_from(["rtk", "hook", "check", "git", "status"]).unwrap();