diff --git a/docs/plans/2026-03-23-001-feat-codex-hook-conversion-beta-plan.md b/docs/plans/2026-03-23-001-feat-codex-hook-conversion-beta-plan.md new file mode 100644 index 000000000..df065e599 --- /dev/null +++ b/docs/plans/2026-03-23-001-feat-codex-hook-conversion-beta-plan.md @@ -0,0 +1,297 @@ +--- +title: "feat: Add Codex hook conversion support" +type: feat +status: completed +date: 2026-03-23 +origin: docs/solutions/integration-issues/codex-hook-converter-gap.md +deepened: 2026-03-23 +--- + +# feat: Add Codex hook conversion support + +## Enhancement Summary + +**Deepened on:** 2026-03-23 +**Reviewers used:** kieran-typescript-reviewer, architecture-strategist, pattern-recognition-specialist, code-simplicity-reviewer, adding-converter-target-providers learnings, Codex hooks.json path verification + +### Key Improvements from Deepening +1. Use `CodexHookEventName` string literal union for compile-time safety against invalid events (prevents `deny_unknown_fields` rejections) +2. Merge two event sets into a single `CODEX_EVENTS` map with `toolScoped` flag (eliminates sync obligation) +3. Use options object for `renderCodexConfig()` instead of accumulating boolean params +4. Extract `isBashCompatibleMatcher()` as a named, testable function +5. Feature gate must be written outside the sync path's managed block to survive `syncToCodex` overwrites +6. Collapse 5 per-event tests to 2 (tool-scoped + non-tool-scoped) -- tests logic, not data +7. Confirmed `.codex/hooks.json` is correct via Codex source analysis (`codex-rs/hooks/src/engine/discovery.rs`) +8. Resolved `timeout` field naming: Codex uses `timeout` (number, seconds) -- not `timeoutSec` + +### New Considerations Discovered +- Hook command strings should NOT have `transformContentForCodex()` applied -- they are shell commands, not markdown with agent references +- Feature gate `[features]` section in config.toml must be placed outside sync managed block markers to avoid being clobbered +- Mixed matchers like `"Bash|Write"` need an explicit decision (skip + warn) +- Add one integration test using the full sample-plugin fixture for end-to-end coverage + +## Overview + +The Codex converter silently drops all hooks during Claude-to-Codex conversion -- the only converter that doesn't even warn. Codex has been incrementally shipping hook support (PreToolUse, PostToolUse, UserPromptSubmit, SessionStart, Stop), creating a growing conversion gap. This plan adds partial hook conversion for the 5 compatible events and specific warnings for everything else. + +## Problem Frame + +When a Claude plugin with hooks is converted to Codex, all hooks vanish silently. The `CodexBundle` type has no `hooks` field. Users get no indication that workflow-critical or security-critical hooks were dropped. Every other converter (Windsurf, Kiro, Copilot, Gemini) at minimum emits `console.warn`. + +See origin: `docs/solutions/integration-issues/codex-hook-converter-gap.md` for the full gap analysis including upstream PR links and compatibility matrix. + +## Requirements Trace + +- R1. Convert PreToolUse (Bash-matched, command type) hooks to Codex format +- R2. Convert PostToolUse (Bash-matched, command type) hooks to Codex format (depends on [openai/codex#15531](https://github.com/openai/codex/pull/15531)) +- R3. Convert UserPromptSubmit (command type) hooks to Codex format +- R4. Convert SessionStart (command type) hooks to Codex format +- R5. Convert Stop (command type) hooks to Codex format +- R6. Emit specific warnings for every unconvertible hook (non-Bash matchers, prompt/agent types, unsupported events) +- R7. Write `.codex/hooks.json` when convertible hooks exist +- R8. Enable the `codex_hooks` feature gate in `config.toml` when hooks are present +- R9. Preserve existing converter behavior when no hooks are present +- R10. Update Codex spec to document hook support + +## Scope Boundaries + +- Sync path (`src/sync/codex.ts`) is out of scope -- needs `ClaudeHomeConfig` to expose hooks first (see origin) +- Non-command hook types (`prompt`, `agent`) cannot be converted -- Codex only supports `command` +- Cross-converter completeness testing is a valuable follow-up but not in this PR +- No `transformContentForCodex()` on hook command strings -- they are shell commands, not markdown content with agent references or slash-command references + +## Context & Research + +### Relevant Code and Patterns + +- **Reference implementation**: `src/converters/claude-to-opencode.ts` -- the only converter that maps hooks today. Uses a `HOOK_EVENT_MAP` table (line 48) and a `convertHooks()` function (line 157) that iterates events, filters by support, and renders output +- **Codex converter**: `src/converters/claude-to-codex.ts` -- `convertClaudeToCodex()` processes agents, commands, skills but has zero hook handling +- **Codex types**: `src/types/codex.ts` -- `CodexBundle` has no `hooks` field +- **Claude hook types**: `src/types/claude.ts` lines 66-91 -- `ClaudeHooks`, `ClaudeHookMatcher`, `ClaudeHookEntry` (union of Command | Prompt | Agent) +- **Codex writer**: `src/targets/codex.ts` -- `writeCodexBundle()` writes prompts, skills, generated skills, config.toml. `renderCodexConfig()` only handles MCP servers. Exported and also called from `src/sync/codex.ts:23` +- **Warning pattern**: Windsurf/Kiro/Copilot converters use `if (plugin.hooks && Object.keys(plugin.hooks.hooks).length > 0) { console.warn(...) }` with "Warning: " prefix convention +- **Warning test pattern**: 4 of 5 converter test files use manual `console.warn` replacement (not `spyOn`): `const originalWarn = console.warn; console.warn = (msg) => warnings.push(msg)` with cleanup +- **Test fixtures**: `tests/fixtures/sample-plugin/hooks/hooks.json` -- all 13 Claude hook events with all three hook types +- **Codex converter tests**: `tests/codex-converter.test.ts` -- uses inline `ClaudePlugin` fixtures with `hooks: undefined` +- **Codex writer tests**: `tests/codex-writer.test.ts` -- uses `fs.mkdtemp` for temp dirs, `backupFile` pattern for config.toml +- **Sync path managed blocks**: `src/sync/codex.ts` uses `# BEGIN/END compound-plugin Claude Code MCP` markers to merge MCP config idempotently + +### Institutional Learnings + +- `docs/solutions/adding-converter-target-providers.md`: Phase 5 documents the "warn on unsupported features" pattern. The Codex converter never implemented it. Five converters (Codex, Droid, Pi, OpenClaw, Qwen) silently drop hooks. +- `docs/solutions/codex-skill-prompt-entrypoints.md`: Codex skill names come from directory basenames. Content rewriting must be selective -- do not rewrite arbitrary slash-shaped text. This reinforces the decision to NOT apply `transformContentForCodex` to hook command strings. + +### Upstream References + +- PreToolUse: [openai/codex#15211](https://github.com/openai/codex/pull/15211) (merged) -- shell-only, deny-only, tool name hardcoded to "Bash", matchers are regex +- PostToolUse: [openai/codex#15531](https://github.com/openai/codex/pull/15531) (draft, active) -- shell-only, block + additionalContext, `updatedMCPToolOutput` rejected +- UserPromptSubmit: [openai/codex#14626](https://github.com/openai/codex/pull/14626) (merged) -- no matcher support, blocked prompts never enter history, context injected as developer messages +- SessionStart: exists on Codex main -- `SessionStartSource` enum (Startup/Resume), `additional_contexts`, `should_stop`/`stop_reason` +- Stop: exists on Codex main + +### Verified: hooks.json Discovery Path + +Confirmed by reading `codex-rs/hooks/src/engine/discovery.rs`: Codex walks the `ConfigLayerStack` and joins `"hooks.json"` to each layer's `config_folder()`. For project layers, `config_folder()` returns the `.codex/` folder. So the correct output path is `.codex/hooks.json`. + +Codex also loads from `~/.codex/hooks.json` (user-level) and `/etc/codex/hooks.json` (system-level), but the converter should write project-level hooks only. + +## Key Technical Decisions + +- **hooks.json location**: `.codex/hooks.json` parallel to `config.toml` -- confirmed correct via `codex-rs/hooks/src/engine/discovery.rs` +- **Feature gate**: Auto-write `[features] codex_hooks = true` to `config.toml` when hooks are present -- without it, hooks are silently ignored at runtime. Must be written outside the sync path's managed block markers so `syncToCodex` does not clobber it. +- **PostToolUse**: Include conversion logic now, hold the PR until upstream #15531 merges -- avoids a second implementation pass. Add a TODO comment in the converter linking to the upstream PR. +- **Wildcard matchers on tool-scoped events**: Convert them -- on Codex, PreToolUse/PostToolUse only fire for Bash anyway, so `*` effectively means the same thing +- **Mixed matchers (e.g. `Bash|Write`)**: Skip with warning -- the matcher includes non-Bash tools that Codex cannot handle. Do not try to extract the Bash-compatible portion. +- **`${CLAUDE_PLUGIN_ROOT}` in commands**: Warn but do not rewrite -- Codex may have a different root convention +- **No content transformation on hook commands**: Do not apply `transformContentForCodex()` -- hook commands are shell commands, not markdown with agent references +- **Backup on re-conversion**: Yes for hooks.json, matching the existing config.toml pattern via `backupFile` +- **Timeout field**: Use `timeout` (not `timeoutSec`) -- confirmed as the primary field name in Codex's `HookHandlerConfig` struct. Value is in seconds. +- **Type-safe event names**: Use a `CodexHookEventName` string literal union to prevent invalid events at compile time + +## Open Questions + +### Resolved During Planning + +- **Where does hooks.json go?**: `.codex/hooks.json` -- confirmed via Codex source code in `codex-rs/hooks/src/engine/discovery.rs` +- **Should config.toml always be written when hooks exist?**: Yes -- even without MCP servers, `[features] codex_hooks = true` is needed +- **How to handle PostToolUse before upstream merges?**: Include the code, hold the PR. The schema is documented in the draft PR and unlikely to change significantly. +- **`timeout` vs `timeoutSec`?**: Use `timeout`. Codex's `HookHandlerConfig` uses `timeout` as the primary field, with `timeoutSec` as an alias. +- **Apply `transformContentForCodex` to hook commands?**: No. Hook commands are shell commands (e.g., `python3 /path/to/hook.py`), not markdown content with slash-command references. +- **How to handle `Bash|Write` mixed matchers?**: Skip with warning. The matcher includes non-Bash tools that Codex cannot fire hooks for. + +### Deferred to Implementation + +- **Exact Codex hooks.json field validation**: The `deny_unknown_fields` constraint requires precise field names. Implementation should validate against the generated JSON schemas from `codex-rs/hooks/schema/generated/`. If fields are rejected at runtime, adjust. +- **statusMessage field**: Codex hook configs support an optional `statusMessage`. Claude hooks don't have this field. Implementation may choose to omit it or add it as a future enhancement. + +## Implementation Units + +- [x] **Unit 1: Add Codex hook types to `CodexBundle`** + + **Goal:** Define the Codex-specific hook types and extend `CodexBundle` so the converter has a target to write to. + + **Requirements:** Foundation for R1-R5, R7 + + **Dependencies:** None + + **Files:** + - Modify: `src/types/codex.ts` + + **Approach:** + - Add a `CodexHookEventName` string literal union of the 5 supported events (`PreToolUse`, `PostToolUse`, `UserPromptSubmit`, `SessionStart`, `Stop`). This provides compile-time protection against writing an unsupported event to hooks.json -- critical given Codex's `deny_unknown_fields`. + - Add `CodexHookCommand` type with fields `type` (literal `"command"`), `command` (string), `timeout` (optional number) -- matching Codex's wire format + - Add `CodexHookMatcher` type with optional `matcher` string and `hooks` array of `CodexHookCommand` + - Add `CodexHooks` type wrapping `Partial>` + - Add optional `hooks` field to `CodexBundle` + - Keep types minimal -- no extra fields beyond what Codex's schema accepts + + **Patterns to follow:** + - `ClaudeHookCommand`, `ClaudeHookMatcher`, `ClaudeHooks` in `src/types/claude.ts` -- structurally similar but Codex types restrict to `command` type only and use a closed event name union instead of open `Record` + + **Verification:** + - TypeScript compiles. Existing tests still pass (no behavioral change yet). + +- [x] **Unit 2: Add hook conversion logic to the Codex converter** + + **Goal:** Convert compatible hooks from Claude format to Codex format, emitting warnings for everything unconvertible. + + **Requirements:** R1-R6, R9 + + **Dependencies:** Unit 1 + + **Files:** + - Modify: `src/converters/claude-to-codex.ts` + - Test: `tests/codex-converter.test.ts` + + **Approach:** + - Define a single `CODEX_EVENTS` map combining supported events with their `toolScoped` flag: + ``` + PreToolUse: toolScoped true, PostToolUse: toolScoped true, + UserPromptSubmit: toolScoped false, SessionStart: toolScoped false, Stop: toolScoped false + ``` + This replaces two separate sets and eliminates the sync obligation between them. + - Extract an `isBashCompatibleMatcher(matcher: string | undefined): boolean` function that returns true for `undefined`, `"*"`, `""`, `"Bash"`, `"^Bash$"` and false for everything else (including mixed matchers like `"Bash|Write"`). This is a named, testable function. + - Add a `convertHooksForCodex(hooks: ClaudeHooks): CodexHooks | undefined` function (receives `plugin.hooks` — the outer wrapper whose `.hooks` property is the `Record` map) that: + - Iterates `hooks.hooks` entries (the inner `.hooks` record keyed by event name) + - Skips events not in `CODEX_EVENTS` with a specific warning naming the event + - For tool-scoped events, checks matcher via `isBashCompatibleMatcher()` -- skips non-Bash matchers with warning + - Filters each matcher's hook entries to `command` type only -- warns about skipped `prompt`/`agent` entries + - Emits a one-time warning about PreToolUse deny-only semantics when PreToolUse hooks are converted + - Scans converted command strings for `${CLAUDE_PLUGIN_ROOT}` and warns if found + - Returns `CodexHooks` or `undefined` if no convertible hooks remain + - Keep `convertHooksForCodex` as a non-exported (private) function, consistent with all other helpers in the Codex converter + - Wire into `convertClaudeToCodex()`: `const hooks = plugin.hooks ? convertHooksForCodex(plugin.hooks) : undefined` + - Add `hooks` to the returned bundle + - Add a TODO comment on the PostToolUse entry in `CODEX_EVENTS` linking to upstream PR #15531 + + **Patterns to follow:** + - OpenCode's `HOOK_EVENT_MAP` table pattern at `src/converters/claude-to-opencode.ts` -- but simpler since Codex output is JSON not TypeScript, so we just filter and reshape rather than generating code + - Warning pattern from `src/converters/claude-to-windsurf.ts` -- use "Warning: " prefix convention + - Warning tests: use the dominant manual `console.warn` replacement pattern (not `spyOn`) + + **Test scenarios:** + - Converts a tool-scoped event (PreToolUse) with Bash matcher and command type -> present in bundle.hooks with correct structure + - Converts a non-tool-scoped event (UserPromptSubmit) with command type -> present in bundle.hooks + - Converts a full plugin with all 5 supported events -> all present in bundle.hooks (integration-level test) + - Skips non-Bash matcher on tool-scoped event (`Write|Edit`) -> warning emitted, not in bundle + - Skips mixed matcher (`Bash|Write`) -> warning emitted, not in bundle + - Skips `prompt` and `agent` type hooks -> warning emitted, only command hooks pass through + - Skips unsupported events (one test covering PostToolUseFailure, PermissionRequest, etc.) -> warnings emitted + - Plugin with no hooks -> hooks undefined, no warnings + - Plugin where all hooks are unconvertible -> hooks undefined, warnings emitted + - Preserves timeout field when present + - Wildcard matcher (`*`) on PreToolUse is converted (not skipped) + - `isBashCompatibleMatcher` unit tests: undefined, `*`, `""`, `Bash`, `^Bash$` return true; `Write|Edit`, `Bash|Write`, `Read` return false + + **Verification:** + - All new converter tests pass. All existing converter tests still pass. Run `bun test`. + +- [x] **Unit 3: Write hooks.json and feature gate in Codex writer** + + **Goal:** Output the converted hooks as `.codex/hooks.json` and enable the feature gate in `config.toml`. + + **Requirements:** R7, R8, R9 + + **Dependencies:** Unit 1 + + **Files:** + - Modify: `src/targets/codex.ts` + - Test: `tests/codex-writer.test.ts` + + **Approach:** + - In `writeCodexBundle()`, after writing skills/prompts, check if `bundle.hooks` has entries + - If so, write `.codex/hooks.json` with `JSON.stringify(bundle.hooks, null, 2)` -- use `backupFile` before overwriting + - Refactor `renderCodexConfig()` to accept an options object instead of positional params: + ``` + type CodexConfigOptions = { mcpServers?: Record; hasHooks?: boolean } + ``` + This prevents accumulating boolean params as more config sections are added. The function is exported and called from two places: `writeCodexBundle` (in `src/targets/codex.ts`) and `syncToCodex` (in `src/sync/codex.ts:23`, which passes only `mcpServers`). Add defaults for all options so the sync call site needs no changes. + - When `hasHooks` is true, emit `[features]\ncodex_hooks = true\n` at the top of the config output, before any MCP server sections. This ordering ensures the feature gate sits outside the sync path's managed block markers (`# BEGIN/END compound-plugin`), which wrap only MCP content. + - Ensure `config.toml` is written when hooks are present even if no MCP servers exist (currently `renderCodexConfig` returns `null` when no MCP servers) + + **Patterns to follow:** + - Existing `backupFile` + `writeText` pattern for `config.toml` in `src/targets/codex.ts` + - `resolveCodexRoot()` helper already handles `.codex` path resolution + + **Test scenarios:** + - Writes `.codex/hooks.json` when hooks present in bundle -- verify file exists and content is valid JSON matching the bundle + - Does not write hooks.json when hooks are undefined or empty + - Backs up existing hooks.json before overwriting + - Writes `[features] codex_hooks = true` in config.toml when hooks present + - Writes config.toml with feature flag even without MCP servers + - Config.toml with both MCP servers and feature flag includes both sections + + **Verification:** + - All new writer tests pass. Existing writer tests still pass (no behavioral change for hooks-free bundles). Run `bun test`. + +- [x] **Unit 4: Update Codex spec documentation and validate** + + **Goal:** Document the new hook support in the Codex target spec so the converter's behavior is discoverable. Run release validation. + + **Requirements:** R10 + + **Dependencies:** Units 1-3 (should reflect final implementation) + + **Files:** + - Modify: `docs/specs/codex.md` + + **Approach:** + - Add a `## Hooks` section documenting: + - Supported events and their capabilities/limitations + - `hooks.json` format and location (`.codex/hooks.json`) + - Feature gate requirement (`codex_hooks = true`) + - Limitations vs Claude Code (deny-only PreToolUse, no matchers for UserPromptSubmit, shell-only tool hooks, command type only) + - Links to upstream Codex PRs for reference + - Update the `Last verified` date + + **Verification:** + - Spec accurately reflects the implemented behavior. + - Run `bun run release:validate` to confirm no release-owned metadata was inadvertently changed. + +## System-Wide Impact + +- **Interaction graph:** The change touches the convert/install CLI path: `convertClaudeToCodex()` -> `writeCodexBundle()`. No callbacks, middleware, or observers. The sync path (`syncToCodex`) is unaffected and out of scope -- but the `renderCodexConfig` signature change must be backward-compatible (options object with defaults) since `syncToCodex` calls it. +- **Error propagation:** Hook conversion warnings go to `console.warn` (stderr). A malformed hook should not fail the entire conversion -- skip and warn. The writer should propagate I/O errors normally (same as config.toml writes). +- **State lifecycle risks:** The `[features]` section in config.toml must be written outside the sync path's managed block markers (`# BEGIN/END compound-plugin`). If written inside, a subsequent `syncToCodex` run would clobber it, silently disabling hooks. The writer owns this section independently of the managed MCP block. +- **API surface parity:** After this change, Codex joins OpenCode as a converter with hook support. All other converters remain warn-and-skip. The `release:validate` script should still pass since we are adding a field, not changing existing output. +- **Integration coverage:** The full convert-then-verify flow (load plugin with hooks -> convert to Codex -> assert hooks.json written with correct content) should be covered via one integration-level converter test using the full sample-plugin fixture, plus targeted writer tests. + +## Risks & Dependencies + +| Risk | Likelihood | Impact | Mitigation | +|---|---|---|---| +| PostToolUse PR #15531 changes schema before merge | Medium | Medium | Hold our PR; TODO comment links to upstream PR for easy verification | +| `deny_unknown_fields` rejects our hooks.json | Low | High | Use `CodexHookEventName` union to prevent invalid events at compile time; keep output minimal | +| hooks.json path not discovered by Codex at runtime | Low | High | Verified correct via `codex-rs/hooks/src/engine/discovery.rs` source analysis | +| Feature gate syntax wrong in config.toml | Low | Medium | Verify against `codex-rs/core/src/config.rs` for `[features]` parsing | +| Sync path clobbers feature gate | Low | High | Write `[features]` section outside managed block markers; document this constraint | +| `renderCodexConfig` signature change breaks sync caller | Low | Low | Use options object with defaults -- sync call site needs no changes | + +## Sources & References + +- **Origin document:** [docs/solutions/integration-issues/codex-hook-converter-gap.md](docs/solutions/integration-issues/codex-hook-converter-gap.md) -- full research with upstream PR links, compatibility matrix, and semantic differences +- OpenCode hook converter (reference): `src/converters/claude-to-opencode.ts` +- Converter target pattern: `docs/solutions/adding-converter-target-providers.md` +- Codex naming patterns: `docs/solutions/codex-skill-prompt-entrypoints.md` +- Codex hook discovery: `codex-rs/hooks/src/engine/discovery.rs` (confirms `.codex/hooks.json` path) +- Upstream PRs: [#15211](https://github.com/openai/codex/pull/15211) (PreToolUse), [#15531](https://github.com/openai/codex/pull/15531) (PostToolUse), [#14626](https://github.com/openai/codex/pull/14626) (UserPromptSubmit), [#11637](https://github.com/openai/codex/pull/11637) (SessionStart/PreCompact, closed) diff --git a/docs/solutions/integration-issues/codex-hook-converter-gap.md b/docs/solutions/integration-issues/codex-hook-converter-gap.md new file mode 100644 index 000000000..cfa1a09dc --- /dev/null +++ b/docs/solutions/integration-issues/codex-hook-converter-gap.md @@ -0,0 +1,160 @@ +--- +title: "Codex Hook Support: Cross-Platform Conversion Gap Analysis" +type: research +status: active +category: integration-issues +component: codex-converter +created: 2026-03-23 +updated: 2026-03-23 +affected_files: + - src/converters/claude-to-codex.ts + - src/types/codex.ts + - src/targets/codex.ts + - docs/specs/codex.md +tags: + - codex + - hooks + - converter + - cross-platform + - research +--- + +# Codex Hook Support: Cross-Platform Conversion Gap Analysis + +## Problem Statement + +The Codex converter (`src/converters/claude-to-codex.ts`) silently drops all hooks during Claude-to-Codex conversion with no warning emitted. Every other converter in the codebase (Windsurf, Kiro, Copilot, Gemini) at least emits `console.warn` for unsupported hooks. The `CodexBundle` type (`src/types/codex.ts`) has no `hooks` field, and the Codex spec (`docs/specs/codex.md`, last verified 2026-01-21) makes no mention of hooks. + +Meanwhile, Codex has been incrementally shipping hook support across multiple PRs, creating a growing conversion gap. + +## Investigation + +### Step 1: Current Converter State + +Checked `src/converters/claude-to-codex.ts` -- found zero references to hooks. The function `convertClaudeToCodex()` processes agents, commands, and skills but has no hook handling at all. No `console.warn`, no comment, nothing. + +For comparison, every other converter follows a documented pattern from `docs/solutions/adding-converter-target-providers.md` (Phase 5): + +```typescript +if (plugin.hooks && Object.keys(plugin.hooks.hooks).length > 0) { + console.warn("Warning: {Target} does not support hooks. Hooks were skipped.") +} +``` + +### Step 2: Codex Upstream Hook Support Research + +Researched four Codex PRs and the current main branch to map what Codex now supports: + +#### PreToolUse -- MERGED ([openai/codex#15211](https://github.com/openai/codex/pull/15211)) + +- Shell/Bash only -- tool name hardcoded to `"Bash"` regardless of underlying tool (`shell`, `local_shell`, `shell_command`, `container.exec`, `exec_command`) +- Deny-only -- `permissionDecision: "deny"` is the only supported decision. `allow`, `ask`, `updatedInput`, `additionalContext` all fail open with an error log +- Matchers are regex patterns tested against tool name (but tool name is always "Bash") +- Two blocking paths: JSON output with `permissionDecision: "deny"`, or exit code 2 + stderr +- Key files: `codex-rs/hooks/src/events/pre_tool_use.rs`, `codex-rs/core/src/hook_runtime.rs` + +#### PostToolUse -- DRAFT/WIP ([openai/codex#15531](https://github.com/openai/codex/pull/15531)) + +- Shell/Bash only (same as PreToolUse) +- Supports `block` decision + `additionalContext` injection +- `updatedMCPToolOutput` exists in the schema but is explicitly rejected at runtime +- Input includes `tool_response` (the command's output) in addition to `tool_input` +- `continue: false`, `stopReason`, `suppressOutput` all present in schema but unsupported +- Key files: `codex-rs/hooks/src/events/post_tool_use.rs`, `codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json` + +#### UserPromptSubmit -- MERGED ([openai/codex#14626](https://github.com/openai/codex/pull/14626)) + +- Fires before user prompt reaches the model +- Supports `block` decision + `additionalContext` injection +- No matcher support -- all registered hooks fire for every prompt (unlike Claude Code which supports matchers) +- Blocked prompts never enter conversation history (differs from Claude Code where they may still be recorded) +- Context injected as developer messages (not user messages) +- Feature-gated behind `codex_hooks = true` in `config.toml` `[features]` +- Key files: `codex-rs/hooks/src/events/user_prompt_submit.rs`, `codex-rs/hooks/schema/generated/user-prompt-submit.command.input.schema.json` + +#### SessionStart -- EXISTS ON MAIN + +- Supports `SessionStartSource` enum with `Startup` and `Resume` variants +- Supports `additional_contexts` (context injection) and `should_stop`/`stop_reason` +- A community PR ([openai/codex#11637](https://github.com/openai/codex/pull/11637)) proposed this but was closed due to OpenAI's contribution policy change -- OpenAI implemented it themselves +- Key files: `codex-rs/hooks/src/events/session_start.rs` + +#### Stop -- EXISTS ON MAIN + +- Exists alongside SessionStart in the `HookEventName` enum +- Fires at session end + +### Step 3: Hook Type Compatibility + +Only `command` type hooks are supported in Codex. Claude Code's `prompt` and `agent` hook types have no equivalent. + +## Hook Event Compatibility Matrix + +| Claude Event | Codex Status | Convertible? | Key Limitations | +|---|---|---|---| +| PreToolUse (Bash matcher) | Merged (PR #15211) | Yes | Deny-only; no allow/ask/updatedInput/additionalContext | +| PreToolUse (non-Bash) | N/A | No | Codex only fires for shell tools | +| PostToolUse (Bash matcher) | Draft (PR #15531) | Yes (pending merge) | Block + additionalContext only | +| PostToolUse (non-Bash) | N/A | No | Codex only fires for shell tools | +| UserPromptSubmit | Merged (PR #14626) | Yes | No matchers; blocked prompts don't enter history | +| SessionStart | Exists on main | Yes | Command-only | +| Stop | Exists on main | Yes | Command-only | +| PostToolUseFailure | N/A | No | No Codex equivalent | +| PermissionRequest | N/A | No | No Codex equivalent | +| Notification | N/A | No | No Codex equivalent | +| SessionEnd | N/A | No | No Codex equivalent | +| PreCompact | N/A | No | No Codex equivalent | +| Setup | N/A | No | No Codex equivalent | +| SubagentStart/Stop | N/A | No | No Codex equivalent | + +## Cross-Cutting Constraints + +- **Command-only**: Only `type: "command"` hooks. No `prompt` or `agent` types in Codex. +- **Feature-gated**: Requires `codex_hooks = true` in `config.toml` under `[features]`. +- **Strict schema**: `deny_unknown_fields` on all wire types -- extra fields cause parse failure. +- **Fail-open design**: Unsupported outputs, malformed JSON, non-zero/non-2 exit codes all resolve to "allow." +- **No Windows support**: Entire hook system disabled on Windows. +- **No async hooks**: `async: true` in config is recognized but skipped with a warning. + +## Semantic Differences from Claude Code + +1. **PreToolUse decisions**: Claude Code supports allow/deny/ask/updatedInput/additionalContext. Codex supports deny only. +2. **UserPromptSubmit matchers**: Claude Code supports matchers to selectively fire hooks. Codex has no matcher support. +3. **UserPromptSubmit blocking**: In Claude Code, blocked prompts may still enter conversation history. In Codex, they never do. +4. **Context injection**: Codex injects additionalContext as developer messages, which may have different model-visible behavior. + +## Recommended Solution + +1. Add `hooks` field to `CodexBundle` type in `src/types/codex.ts` +2. Convert the compatible subset: PreToolUse (Bash-matched, command type), PostToolUse (pending PR #15531 merge), UserPromptSubmit, SessionStart, Stop +3. Skip + warn for unconvertible hooks with specific reasons (non-Bash matchers, prompt/agent types, unsupported events) +4. Write `hooks.json` output in the Codex writer (`src/targets/codex.ts`) +5. Update `docs/specs/codex.md` with hook documentation +6. Add converter and writer tests + +## Prevention Strategies + +### Silent feature drops + +The core issue is that the Codex converter was never updated to even warn about hooks. Five converters (Codex, Droid, Pi, OpenClaw, Qwen) silently drop hooks. Consider: + +- Adding a cross-converter completeness test that asserts every converter either includes hooks in its bundle or emits a warning +- Framework-level validation in `src/targets/index.ts` that automatically checks for dropped features post-conversion + +### Tracking upstream changes + +The Codex spec (`docs/specs/codex.md`) was last verified 2026-01-21. Consider a capability matrix that maps each Claude feature to each target's support status with `last-verified` dates, and flagging entries older than 90 days as stale during `release:validate`. + +## Related Documentation + +- `docs/specs/codex.md` -- Codex target spec (needs hooks section added) +- `docs/specs/claude-code.md` -- Claude Code hook architecture reference +- `docs/solutions/adding-converter-target-providers.md` -- Documented converter pattern (Phase 5 warns on unsupported features; Codex never implemented this) +- `docs/solutions/codex-skill-prompt-entrypoints.md` -- Codex-specific conversion patterns +- `src/converters/claude-to-opencode.ts` -- Reference for full hook conversion (the only converter that maps hooks today) +- Commit `598222e` -- OpenCode PreToolUse try-catch fix (issue #85) + +## Refresh Candidates + +- `docs/solutions/adding-converter-target-providers.md` may need update once Codex hook conversion is implemented, since the pattern of "all new targets warn-and-skip hooks" will no longer be universal +- `docs/specs/codex.md` needs a hooks section diff --git a/docs/specs/codex.md b/docs/specs/codex.md index 13833d6da..0f7e3126a 100644 --- a/docs/specs/codex.md +++ b/docs/specs/codex.md @@ -1,6 +1,6 @@ # Codex Spec (Config, Prompts, Skills, MCP) -Last verified: 2026-01-21 +Last verified: 2026-03-23 ## Primary sources @@ -16,46 +16,104 @@ https://developers.openai.com/codex/mcp ## Config location and precedence -- Codex reads local settings from `~/.codex/config.toml`, shared by the CLI and IDE extension. citeturn2view0 -- Configuration precedence is: CLI flags → profile values → root-level values in `config.toml` → built-in defaults. citeturn2view0 -- Codex stores local state under `CODEX_HOME` (defaults to `~/.codex`) and includes `config.toml` there. citeturn4view0 +- Codex reads local settings from `~/.codex/config.toml`, shared by the CLI and IDE extension. citeturn2view0 +- Configuration precedence is: CLI flags -> profile values -> root-level values in `config.toml` -> built-in defaults. citeturn2view0 +- Codex stores local state under `CODEX_HOME` (defaults to `~/.codex`) and includes `config.toml` there. citeturn4view0 ## Profiles and providers -- Profiles are defined under `[profiles.]` and selected with `codex --profile `. citeturn4view0 -- A top-level `profile = ""` sets the default profile; CLI flags can override it. citeturn4view0 -- Profiles are experimental and not supported in the IDE extension. citeturn4view0 -- Custom model providers can be defined with base URL, wire API, and optional headers, then referenced via `model_provider`. citeturn4view0 +- Profiles are defined under `[profiles.]` and selected with `codex --profile `. citeturn4view0 +- A top-level `profile = ""` sets the default profile; CLI flags can override it. citeturn4view0 +- Profiles are experimental and not supported in the IDE extension. citeturn4view0 +- Custom model providers can be defined with base URL, wire API, and optional headers, then referenced via `model_provider`. citeturn4view0 ## Custom prompts (slash commands) -- Custom prompts are Markdown files stored under `~/.codex/prompts/`. citeturn3view0 -- Custom prompts require explicit invocation and aren’t shared through the repository; use skills to share or auto-invoke. citeturn3view0 -- Prompts are invoked as `/prompts:` in the slash command UI. citeturn3view0 -- Prompt front matter supports `description:` and `argument-hint:`. citeturn3view0turn2view3 -- Prompt arguments support `$1`–`$9`, `$ARGUMENTS`, and named placeholders like `$FILE` provided as `KEY=value`. citeturn2view3 -- Codex ignores non-Markdown files in the prompts directory. citeturn2view3 +- Custom prompts are Markdown files stored under `~/.codex/prompts/`. citeturn3view0 +- Custom prompts require explicit invocation and aren't shared through the repository; use skills to share or auto-invoke. citeturn3view0 +- Prompts are invoked as `/prompts:` in the slash command UI. citeturn3view0 +- Prompt front matter supports `description:` and `argument-hint:`. citeturn3view0turn2view3 +- Prompt arguments support `$1`-`$9`, `$ARGUMENTS`, and named placeholders like `$FILE` provided as `KEY=value`. citeturn2view3 +- Codex ignores non-Markdown files in the prompts directory. citeturn2view3 ## AGENTS.md instructions -- Codex reads `AGENTS.md` files before doing any work and builds a combined instruction chain. citeturn3view1 -- Discovery order: global (`~/.codex`, using `AGENTS.override.md` then `AGENTS.md`) then project directory traversal from repo root to CWD, with override > AGENTS > fallback names. citeturn3view1 -- Codex concatenates files from root down; files closer to the working directory appear later and override earlier guidance. citeturn3view1 +- Codex reads `AGENTS.md` files before doing any work and builds a combined instruction chain. citeturn3view1 +- Discovery order: global (`~/.codex`, using `AGENTS.override.md` then `AGENTS.md`) then project directory traversal from repo root to CWD, with override > AGENTS > fallback names. citeturn3view1 +- Codex concatenates files from root down; files closer to the working directory appear later and override earlier guidance. citeturn3view1 ## Skills (Agent Skills) -- A skill is a folder containing `SKILL.md` plus optional `scripts/`, `references/`, and `assets/`. citeturn3view3turn3view4 -- `SKILL.md` uses YAML front matter and requires `name` and `description`. citeturn3view3turn3view4 -- Required fields are single-line with length limits (name ≤ 100 chars, description ≤ 500 chars). citeturn3view4 -- At startup, Codex loads only each skill’s name/description; full content is injected when invoked. citeturn3view3turn3view4 -- Skills can be repo-scoped in `.agents/skills/` and are discovered from the current working directory up to the repository root. User-scoped skills live in `~/.agents/skills/`. citeturn1view1turn1view4 +- A skill is a folder containing `SKILL.md` plus optional `scripts/`, `references/`, and `assets/`. citeturn3view3turn3view4 +- `SKILL.md` uses YAML front matter and requires `name` and `description`. citeturn3view3turn3view4 +- Required fields are single-line with length limits (name <= 100 chars, description <= 500 chars). citeturn3view4 +- At startup, Codex loads only each skill's name/description; full content is injected when invoked. citeturn3view3turn3view4 +- Skills can be repo-scoped in `.agents/skills/` and are discovered from the current working directory up to the repository root. User-scoped skills live in `~/.agents/skills/`. citeturn1view1turn1view4 - Inference: some existing tooling and user setups still use `.codex/skills/` and `~/.codex/skills/` as legacy compatibility paths, but those locations are not documented in the current OpenAI Codex skills docs linked above. -- Codex also supports admin-scoped skills in `/etc/codex/skills` plus built-in system skills bundled with Codex. citeturn1view4 -- Skills can be invoked explicitly using `/skills` or `$skill-name`. citeturn3view3 +- Codex also supports admin-scoped skills in `/etc/codex/skills` plus built-in system skills bundled with Codex. citeturn1view4 +- Skills can be invoked explicitly using `/skills` or `$skill-name`. citeturn3view3 ## MCP (Model Context Protocol) -- MCP configuration lives in `~/.codex/config.toml` and is shared by the CLI and IDE extension. citeturn3view2turn3view5 -- Each server is configured under `[mcp_servers.]`. citeturn3view5 -- STDIO servers support `command` (required), `args`, `env`, `env_vars`, and `cwd`. citeturn3view5 -- Streamable HTTP servers support `url` (required), `bearer_token_env_var`, `http_headers`, and `env_http_headers`. citeturn3view5 +- MCP configuration lives in `~/.codex/config.toml` and is shared by the CLI and IDE extension. citeturn3view2turn3view5 +- Each server is configured under `[mcp_servers.]`. citeturn3view5 +- STDIO servers support `command` (required), `args`, `env`, `env_vars`, and `cwd`. citeturn3view5 +- Streamable HTTP servers support `url` (required), `bearer_token_env_var`, `http_headers`, and `env_http_headers`. citeturn3view5 + +## Hooks + +Codex supports lifecycle hooks via `hooks.json`, discovered at project level (`.codex/hooks.json`), user level (`~/.codex/hooks.json`), and system level (`/etc/codex/hooks.json`). Hooks must be enabled with `codex_hooks = true` under `[features]` in `config.toml`. + +### Supported events + +| Event | Scope | Capabilities | Upstream PR | +|---|---|---|---| +| PreToolUse | Shell/Bash only | Deny-only decisions; tool name hardcoded to "Bash" | [#15211](https://github.com/openai/codex/pull/15211) (merged) | +| PostToolUse | Shell/Bash only | Block + additionalContext; `updatedMCPToolOutput` rejected | [#15531](https://github.com/openai/codex/pull/15531) (draft) | +| UserPromptSubmit | All prompts | Block + additionalContext; no matchers; blocked prompts never enter history | [#14626](https://github.com/openai/codex/pull/14626) (merged) | +| SessionStart | Session lifecycle | additionalContext + should_stop/stop_reason | Exists on main | +| Stop | Session lifecycle | Fires at session end | Exists on main | + +### hooks.json format + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo before", "timeout": 30 } + ] + } + ], + "UserPromptSubmit": [ + { + "hooks": [ + { "type": "command", "command": "echo prompt" } + ] + } + ] + } +} +``` + +### Converter behavior + +- Only `command` type hooks are converted; `prompt` and `agent` types are skipped with warnings +- PreToolUse/PostToolUse matchers must be Bash-compatible (undefined, `*`, `""`, `Bash`, `^Bash$`); non-Bash matchers are skipped +- Mixed matchers (e.g. `Bash|Write`) are skipped entirely +- Wildcard matchers on tool-scoped events are normalized to `"Bash"` +- Non-tool-scoped events (UserPromptSubmit, SessionStart, Stop) omit the `matcher` field +- Unsupported events (PostToolUseFailure, PermissionRequest, Notification, SessionEnd, PreCompact, Setup, SubagentStart, SubagentStop) are skipped with warnings + +### Constraints + +- Only `type: "command"` hooks are supported (no `prompt` or `agent` types) +- Feature-gated behind `codex_hooks = true` in `config.toml` `[features]` +- `deny_unknown_fields` on all wire types -- extra fields cause parse failure +- Fail-open design: unsupported outputs, malformed JSON, non-zero/non-2 exit codes all resolve to "allow" +- No Windows support (entire hook system disabled) +- No async hooks (`async: true` is recognized but skipped with a warning) +- PreToolUse only supports deny decisions (allow/ask/updatedInput/additionalContext all fail open) +- Context injection uses developer messages (not user messages) diff --git a/src/converters/claude-to-codex.ts b/src/converters/claude-to-codex.ts index 238ca1921..2ed734f7f 100644 --- a/src/converters/claude-to-codex.ts +++ b/src/converters/claude-to-codex.ts @@ -1,6 +1,6 @@ import { formatFrontmatter } from "../utils/frontmatter" -import type { ClaudeAgent, ClaudeCommand, ClaudePlugin, ClaudeSkill } from "../types/claude" -import type { CodexBundle, CodexGeneratedSkill } from "../types/codex" +import type { ClaudeAgent, ClaudeCommand, ClaudeHooks, ClaudePlugin, ClaudeSkill } from "../types/claude" +import type { CodexBundle, CodexGeneratedSkill, CodexHookEventName, CodexHooks } from "../types/codex" import type { ClaudeToOpenCodeOptions } from "./claude-to-opencode" import { normalizeCodexName, @@ -91,12 +91,15 @@ export function convertClaudeToCodex( ) const generatedSkills = [...commandSkills, ...agentSkills] + const hooks = plugin.hooks ? convertHooksForCodex(plugin.hooks) : undefined + return { prompts: [...prompts, ...workflowPrompts], skillDirs, generatedSkills, invocationTargets, mcpServers: plugin.mcpServers, + ...(hooks ? { hooks } : {}), } } @@ -214,3 +217,91 @@ function uniqueName(base: string, used: Set): string { used.add(name) return name } + +const CODEX_EVENTS: Record = { + PreToolUse: { codexName: "PreToolUse", toolScoped: true }, + PostToolUse: { codexName: "PostToolUse", toolScoped: true }, // TODO: depends on openai/codex#15531 merging + UserPromptSubmit: { codexName: "UserPromptSubmit", toolScoped: false }, + SessionStart: { codexName: "SessionStart", toolScoped: false }, + Stop: { codexName: "Stop", toolScoped: false }, +} + +function isBashCompatibleMatcher(matcher: string | undefined): boolean { + return matcher === undefined || matcher === "*" || matcher === "" || matcher === "Bash" || matcher === "^Bash$" +} + +function convertHooksForCodex(hooks: ClaudeHooks): CodexHooks | undefined { + const result: CodexHooks = {} + let hasConvertibleHooks = false + let hasPreToolUse = false + let warnedPluginRoot = false + + for (const [eventName, matchers] of Object.entries(hooks.hooks)) { + const eventDef = CODEX_EVENTS[eventName] + if (!eventDef) { + console.warn(`Warning: Codex does not support ${eventName} hooks. Skipped.`) + continue + } + + const codexMatchers = [] + + for (const matcherGroup of matchers) { + if (eventDef.toolScoped && !isBashCompatibleMatcher(matcherGroup.matcher)) { + console.warn(`Warning: Codex only supports Bash hooks for ${eventName}. Matcher '${matcherGroup.matcher}' skipped.`) + continue + } + + const commandEntries = [] + for (const entry of matcherGroup.hooks) { + if (entry.type !== "command") { + console.warn(`Warning: Codex only supports command-type hooks. Skipped ${entry.type} hook in ${eventName}.`) + continue + } + commandEntries.push(entry) + } + + if (commandEntries.length === 0) continue + + const codexHookCommands = commandEntries.map((entry) => { + if (!warnedPluginRoot && entry.command.includes("$" + "{CLAUDE_PLUGIN_ROOT}")) { + console.warn("Warning: Codex hooks do not support $" + "{CLAUDE_PLUGIN_ROOT}. Command may not work as expected.") + warnedPluginRoot = true + } + const cmd: { type: "command"; command: string; timeout?: number } = { + type: "command", + command: entry.command, + } + if (entry.timeout !== undefined) { + cmd.timeout = entry.timeout + } + return cmd + }) + + const codexMatcher: { matcher?: string; hooks: typeof codexHookCommands } = { + hooks: codexHookCommands, + } + if (eventDef.toolScoped) { + codexMatcher.matcher = isBashCompatibleMatcher(matcherGroup.matcher) && (matcherGroup.matcher === undefined || matcherGroup.matcher === "*" || matcherGroup.matcher === "") + ? "Bash" + : matcherGroup.matcher! + } + + codexMatchers.push(codexMatcher) + } + + if (codexMatchers.length > 0) { + result[eventDef.codexName] = codexMatchers + hasConvertibleHooks = true + if (eventDef.codexName === "PreToolUse") { + hasPreToolUse = true + } + } + } + + if (hasPreToolUse) { + console.warn("Warning: Codex PreToolUse hooks only support deny decisions. Allow/ask/updatedInput are not available.") + } + + if (!hasConvertibleHooks) return undefined + return result +} diff --git a/src/sync/codex.ts b/src/sync/codex.ts index b7b894ef7..16b0c21e2 100644 --- a/src/sync/codex.ts +++ b/src/sync/codex.ts @@ -20,7 +20,7 @@ export async function syncToCodex( // Write MCP servers to config.toml (TOML format) if (Object.keys(config.mcpServers).length > 0) { const configPath = path.join(outputRoot, "config.toml") - const mcpToml = renderCodexConfig(config.mcpServers) + const mcpToml = renderCodexConfig({ mcpServers: config.mcpServers }) if (!mcpToml) { return } diff --git a/src/targets/codex.ts b/src/targets/codex.ts index f52902a7c..18d25160d 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -35,7 +35,18 @@ export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): } } - const config = renderCodexConfig(bundle.mcpServers) + const hasHooks = bundle.hooks != null && Object.keys(bundle.hooks).length > 0 + + if (hasHooks) { + const hooksPath = path.join(codexRoot, "hooks.json") + const backupPath = await backupFile(hooksPath) + if (backupPath) { + console.log(`Backed up existing hooks.json to ${backupPath}`) + } + await writeText(hooksPath, JSON.stringify({ hooks: bundle.hooks }, null, 2) + "\n") + } + + const config = renderCodexConfig({ mcpServers: bundle.mcpServers, hasHooks }) if (config) { const configPath = path.join(codexRoot, "config.toml") const backupPath = await backupFile(configPath) @@ -50,12 +61,28 @@ function resolveCodexRoot(outputRoot: string): string { return path.basename(outputRoot) === ".codex" ? outputRoot : path.join(outputRoot, ".codex") } -export function renderCodexConfig(mcpServers?: Record): string | null { - if (!mcpServers || Object.keys(mcpServers).length === 0) return null +export type CodexConfigOptions = { + mcpServers?: Record + hasHooks?: boolean +} + +export function renderCodexConfig(options: CodexConfigOptions = {}): string | null { + const { mcpServers, hasHooks } = options + const hasMcp = mcpServers != null && Object.keys(mcpServers).length > 0 + + if (!hasMcp && !hasHooks) return null const lines: string[] = ["# Generated by compound-plugin", ""] - for (const [name, server] of Object.entries(mcpServers)) { + if (hasHooks) { + lines.push("[features]") + lines.push("codex_hooks = true") + lines.push("") + } + + if (!hasMcp) return lines.join("\n") + + for (const [name, server] of Object.entries(mcpServers!)) { const key = formatTomlKey(name) lines.push(`[mcp_servers.${key}]`) diff --git a/src/types/codex.ts b/src/types/codex.ts index 8ed494cf2..cb61aa1a8 100644 --- a/src/types/codex.ts +++ b/src/types/codex.ts @@ -16,10 +16,31 @@ export type CodexGeneratedSkill = { content: string } +export type CodexHookEventName = + | "PreToolUse" + | "PostToolUse" // TODO: depends on openai/codex#15531 merging + | "UserPromptSubmit" + | "SessionStart" + | "Stop" + +export type CodexHookCommand = { + type: "command" + command: string + timeout?: number +} + +export type CodexHookMatcher = { + matcher?: string + hooks: CodexHookCommand[] +} + +export type CodexHooks = Partial> + export type CodexBundle = { prompts: CodexPrompt[] skillDirs: CodexSkillDir[] generatedSkills: CodexGeneratedSkill[] invocationTargets?: CodexInvocationTargets mcpServers?: Record + hooks?: CodexHooks } diff --git a/tests/codex-converter.test.ts b/tests/codex-converter.test.ts index a82c1875e..85ca6c0ce 100644 --- a/tests/codex-converter.test.ts +++ b/tests/codex-converter.test.ts @@ -502,4 +502,260 @@ Run \`/compound-engineering-setup\` to create a settings file.`, expect(description).not.toContain("\n") expect(description.endsWith("...")).toBe(true) }) + + // -- Hook conversion tests -- + + const defaultOpts = { agentMode: "subagent" as const, inferTemperature: false, permissions: "none" as const } + + function pluginWithHooks(hooks: Record }>>): ClaudePlugin { + return { + ...fixturePlugin, + commands: [], + agents: [], + skills: [], + hooks: { hooks: hooks as any }, + } + } + + test("converts PreToolUse Bash hooks to Codex format", () => { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo before", timeout: 30 }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.PreToolUse).toHaveLength(1) + expect(bundle.hooks!.PreToolUse![0].matcher).toBe("Bash") + expect(bundle.hooks!.PreToolUse![0].hooks).toHaveLength(1) + expect(bundle.hooks!.PreToolUse![0].hooks[0].command).toBe("echo before") + expect(bundle.hooks!.PreToolUse![0].hooks[0].timeout).toBe(30) + } finally { + console.warn = originalWarn + } + }) + + test("converts non-tool-scoped events without matcher", () => { + const plugin = pluginWithHooks({ + UserPromptSubmit: [{ matcher: "*", hooks: [{ type: "command", command: "echo prompt" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.UserPromptSubmit).toHaveLength(1) + expect(bundle.hooks!.UserPromptSubmit![0].matcher).toBeUndefined() + expect(bundle.hooks!.UserPromptSubmit![0].hooks).toHaveLength(1) + expect(bundle.hooks!.UserPromptSubmit![0].hooks[0].command).toBe("echo prompt") + } finally { + console.warn = originalWarn + } + }) + + test("converts all 5 supported events", () => { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo pre" }] }], + PostToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo post" }] }], + UserPromptSubmit: [{ hooks: [{ type: "command", command: "echo prompt" }] }], + SessionStart: [{ hooks: [{ type: "command", command: "echo start" }] }], + Stop: [{ hooks: [{ type: "command", command: "echo stop" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.PreToolUse).toBeDefined() + expect(bundle.hooks?.PostToolUse).toBeDefined() + expect(bundle.hooks?.UserPromptSubmit).toBeDefined() + expect(bundle.hooks?.SessionStart).toBeDefined() + expect(bundle.hooks?.Stop).toBeDefined() + } finally { + console.warn = originalWarn + } + }) + + test("skips non-Bash matcher on tool-scoped event with warning", () => { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher: "Write|Edit", hooks: [{ type: "command", command: "echo test" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks).toBeUndefined() + expect(warnings.some((w) => w.includes("Write|Edit"))).toBe(true) + } finally { + console.warn = originalWarn + } + }) + + test("skips prompt and agent type hooks with warnings", () => { + const plugin = pluginWithHooks({ + PostToolUse: [{ matcher: "Bash", hooks: [ + { type: "prompt", prompt: "review" }, + { type: "agent", agent: "reviewer" }, + { type: "command", command: "echo test" }, + ] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.PostToolUse).toHaveLength(1) + expect(bundle.hooks!.PostToolUse![0].hooks).toHaveLength(1) + expect(bundle.hooks!.PostToolUse![0].hooks[0].command).toBe("echo test") + expect(warnings.some((w) => w.includes("prompt"))).toBe(true) + expect(warnings.some((w) => w.includes("agent"))).toBe(true) + } finally { + console.warn = originalWarn + } + }) + + test("skips unsupported events with warnings", () => { + const plugin = pluginWithHooks({ + PostToolUseFailure: [{ hooks: [{ type: "command", command: "echo fail" }] }], + PermissionRequest: [{ hooks: [{ type: "command", command: "echo perm" }] }], + Notification: [{ hooks: [{ type: "command", command: "echo notify" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks).toBeUndefined() + const unsupportedWarnings = warnings.filter((w) => w.includes("does not support")) + expect(unsupportedWarnings).toHaveLength(3) + } finally { + console.warn = originalWarn + } + }) + + test("returns undefined hooks when plugin has no hooks", () => { + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(fixturePlugin, defaultOpts) + expect(bundle.hooks).toBeUndefined() + expect(warnings).toHaveLength(0) + } finally { + console.warn = originalWarn + } + }) + + test("returns undefined hooks when all hooks are unconvertible", () => { + const plugin = pluginWithHooks({ + Notification: [{ hooks: [{ type: "prompt", prompt: "notify" }] }], + SubagentStart: [{ hooks: [{ type: "agent", agent: "sub" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks).toBeUndefined() + expect(warnings.length).toBeGreaterThan(0) + } finally { + console.warn = originalWarn + } + }) + + test("preserves timeout field", () => { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo timed", timeout: 30 }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.PreToolUse?.[0]?.hooks[0]?.timeout).toBe(30) + } finally { + console.warn = originalWarn + } + }) + + test("converts wildcard matcher on tool-scoped event", () => { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher: "*", hooks: [{ type: "command", command: "echo test" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.PreToolUse).toBeDefined() + expect(bundle.hooks!.PreToolUse![0].matcher).toBe("Bash") + } finally { + console.warn = originalWarn + } + }) + + test("isBashCompatibleMatcher allows compatible matchers and rejects others", () => { + const compatibleMatchers = [undefined, "*", "", "Bash", "^Bash$"] + const incompatibleMatchers = ["Write|Edit", "Bash|Write", "Read"] + + for (const matcher of compatibleMatchers) { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher, hooks: [{ type: "command", command: "echo test" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks?.PreToolUse).toBeDefined() + } finally { + console.warn = originalWarn + } + } + + for (const matcher of incompatibleMatchers) { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher, hooks: [{ type: "command", command: "echo test" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + const bundle = convertClaudeToCodex(plugin, defaultOpts) + expect(bundle.hooks).toBeUndefined() + expect(warnings.some((w) => w.includes(matcher))).toBe(true) + } finally { + console.warn = originalWarn + } + } + }) + + test("emits PreToolUse deny-only warning", () => { + const plugin = pluginWithHooks({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo check" }] }], + }) + + const warnings: string[] = [] + const originalWarn = console.warn + console.warn = (msg: string) => warnings.push(msg) + try { + convertClaudeToCodex(plugin, defaultOpts) + expect(warnings.some((w) => w.includes("deny decisions"))).toBe(true) + } finally { + console.warn = originalWarn + } + }) }) diff --git a/tests/codex-writer.test.ts b/tests/codex-writer.test.ts index 4487171a7..06abf060e 100644 --- a/tests/codex-writer.test.ts +++ b/tests/codex-writer.test.ts @@ -3,7 +3,7 @@ import { promises as fs } from "fs" import path from "path" import os from "os" import { writeCodexBundle } from "../src/targets/codex" -import type { CodexBundle } from "../src/types/codex" +import type { CodexBundle, CodexHooks } from "../src/types/codex" async function exists(filePath: string): Promise { try { @@ -264,4 +264,142 @@ Workflow handoff: expect(installedSkill).not.toContain("/prompts:settings") expect(installedSkill).not.toContain("https://prompts:www.proofeditor.ai") }) + + test("writes hooks.json when hooks present", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-hooks-")) + const hooks: CodexHooks = { + PreToolUse: [{ hooks: [{ type: "command", command: "echo before" }] }], + UserPromptSubmit: [{ hooks: [{ type: "command", command: "echo prompt" }] }], + } + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + hooks, + } + + await writeCodexBundle(tempRoot, bundle) + + const hooksPath = path.join(tempRoot, ".codex", "hooks.json") + expect(await exists(hooksPath)).toBe(true) + + const parsed = JSON.parse(await fs.readFile(hooksPath, "utf8")) + expect(parsed.hooks.PreToolUse).toHaveLength(1) + expect(parsed.hooks.PreToolUse[0].hooks[0].command).toBe("echo before") + expect(parsed.hooks.UserPromptSubmit).toHaveLength(1) + expect(parsed.hooks.UserPromptSubmit[0].hooks[0].command).toBe("echo prompt") + }) + + test("does not write hooks.json when hooks undefined", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-no-hooks-")) + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + } + + await writeCodexBundle(tempRoot, bundle) + + const hooksPath = path.join(tempRoot, ".codex", "hooks.json") + expect(await exists(hooksPath)).toBe(false) + }) + + test("backs up existing hooks.json before overwriting", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-hooks-backup-")) + const codexRoot = path.join(tempRoot, ".codex") + const hooksPath = path.join(codexRoot, "hooks.json") + + // Create existing hooks.json + await fs.mkdir(codexRoot, { recursive: true }) + const originalContent = '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"echo old"}]}]}}\n' + await fs.writeFile(hooksPath, originalContent) + + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + hooks: { + PreToolUse: [{ hooks: [{ type: "command", command: "echo new" }] }], + }, + } + + await writeCodexBundle(codexRoot, bundle) + + // New hooks.json should be written + const newContent = await fs.readFile(hooksPath, "utf8") + expect(newContent).toContain("echo new") + + // Backup should exist with original content + const files = await fs.readdir(codexRoot) + const backupFileName = files.find((f) => f.startsWith("hooks.json.bak.")) + expect(backupFileName).toBeDefined() + + const backupContent = await fs.readFile(path.join(codexRoot, backupFileName!), "utf8") + expect(backupContent).toBe(originalContent) + }) + + test("writes feature gate in config.toml when hooks present", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-hooks-gate-")) + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + hooks: { + PreToolUse: [{ hooks: [{ type: "command", command: "echo test" }] }], + }, + } + + await writeCodexBundle(tempRoot, bundle) + + const configPath = path.join(tempRoot, ".codex", "config.toml") + expect(await exists(configPath)).toBe(true) + + const config = await fs.readFile(configPath, "utf8") + expect(config).toContain("[features]") + expect(config).toContain("codex_hooks = true") + }) + + test("writes config.toml with both feature gate and MCP servers", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-hooks-mcp-")) + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + hooks: { + PreToolUse: [{ hooks: [{ type: "command", command: "echo test" }] }], + }, + mcpServers: { + local: { command: "echo", args: ["hello"] }, + }, + } + + await writeCodexBundle(tempRoot, bundle) + + const configPath = path.join(tempRoot, ".codex", "config.toml") + expect(await exists(configPath)).toBe(true) + + const config = await fs.readFile(configPath, "utf8") + expect(config).toContain("[features]") + expect(config).toContain("codex_hooks = true") + expect(config).toContain("[mcp_servers.local]") + + // Feature gate should come before MCP servers + const featuresIndex = config.indexOf("[features]") + const mcpIndex = config.indexOf("[mcp_servers.local]") + expect(featuresIndex).toBeLessThan(mcpIndex) + }) + + test("does not write config.toml when no hooks and no MCP servers", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-no-config-")) + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + } + + await writeCodexBundle(tempRoot, bundle) + + const configPath = path.join(tempRoot, ".codex", "config.toml") + expect(await exists(configPath)).toBe(false) + }) })