From 360ded2c3ec17c4db4d35cf19ce078856226d871 Mon Sep 17 00:00:00 2001 From: nkolly Date: Fri, 1 May 2026 11:41:13 -0600 Subject: [PATCH 1/4] feat: convert hooks to .codex/hooks.json for Codex target Codex supports hooks natively via .codex/hooks.json in the same format as Claude Code. The converter was silently dropping hooks for Codex while other targets (OpenCode) already converted them. Changes: - Add hooks field to CodexBundle type - Pass plugin.hooks through in claude-to-codex converter (both modes) - Write hooks to .codex/hooks.json in writeCodexBundle - Merge with existing hooks (preserves hooks from other plugins) - Tag managed hooks with _source field for clean re-installs All 58 existing Codex tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/converters/claude-to-codex.ts | 2 + src/targets/codex.ts | 64 +++++++++++++++++++++++++++++++ src/types/codex.ts | 3 +- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/converters/claude-to-codex.ts b/src/converters/claude-to-codex.ts index a99877625..a91ae97be 100644 --- a/src/converters/claude-to-codex.ts +++ b/src/converters/claude-to-codex.ts @@ -104,6 +104,7 @@ export function convertClaudeToCodex( agents, invocationTargets, mcpServers: undefined, + hooks: plugin.hooks, externallyManagedSkillNames, } } @@ -126,6 +127,7 @@ export function convertClaudeToCodex( agents, invocationTargets, mcpServers: plugin.mcpServers, + hooks: plugin.hooks, } } diff --git a/src/targets/codex.ts b/src/targets/codex.ts index 5bfeb80dd..e90fbe2df 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -134,6 +134,16 @@ export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): } await writeTextSecure(configPath, merged) } + + // Write hooks to .codex/hooks.json — Codex uses the same hooks format + // as Claude Code. Hooks are merged with any existing hooks file to avoid + // clobbering hooks from other plugins or manual configuration. + if (bundle.hooks && Object.keys(bundle.hooks.hooks).length > 0) { + const hooksPath = path.join(codexRoot, "hooks.json") + const existingHooks = await readJsonSafe(hooksPath) + const mergedHooks = mergeCodexHooks(existingHooks, bundle.hooks.hooks, pluginName) + await writeTextSecure(hooksPath, JSON.stringify(mergedHooks, null, 2) + "\n") + } } function resolveCodexRoot(outputRoot: string): string { @@ -614,3 +624,57 @@ function formatTomlInlineTable(entries: Record): string { ) return `{ ${parts.join(", ")} }` } + +// ── Hooks ────────────────────────────────────────────────── + +async function readJsonSafe(filePath: string): Promise | null> { + try { + const content = await fs.readFile(filePath, "utf8") + return JSON.parse(content) + } catch { + return null + } +} + +type HookEntry = { matcher?: string; hooks: Array<{ type: string; command?: string; prompt?: string; agent?: string; timeout?: number }> } + +/** + * Merge plugin hooks into an existing .codex/hooks.json, preserving hooks + * from other sources. Uses a managed-block pattern: each plugin's hooks are + * tagged with a `_source` field so re-installs can replace them cleanly. + */ +function mergeCodexHooks( + existing: Record | null, + pluginHooks: Record, + pluginName?: string, +): Record { + const source = pluginName ?? "coco" + const result: Record = {} + + // Preserve existing hooks that aren't from this plugin + const existingHooks = (existing?.hooks ?? {}) as Record + for (const [event, matchers] of Object.entries(existingHooks)) { + if (!Array.isArray(matchers)) continue + result[event] = matchers.filter((m) => { + if (typeof m === "object" && m !== null && "_source" in m) { + return (m as Record)._source !== source + } + return true // keep hooks without a source tag (manual or other plugins) + }) + } + + // Add this plugin's hooks with source tag + for (const [event, matchers] of Object.entries(pluginHooks)) { + if (!result[event]) result[event] = [] + for (const matcher of matchers) { + result[event].push({ ...matcher, _source: source }) + } + } + + // Remove empty event arrays + for (const event of Object.keys(result)) { + if (result[event].length === 0) delete result[event] + } + + return { hooks: result } +} diff --git a/src/types/codex.ts b/src/types/codex.ts index 361e01200..213e85458 100644 --- a/src/types/codex.ts +++ b/src/types/codex.ts @@ -1,4 +1,4 @@ -import type { ClaudeMcpServer } from "./claude" +import type { ClaudeMcpServer, ClaudeHooks } from "./claude" import type { CodexInvocationTargets } from "../utils/codex-content" export type CodexPrompt = { @@ -37,6 +37,7 @@ export type CodexBundle = { agents?: CodexAgent[] invocationTargets?: CodexInvocationTargets mcpServers?: Record + hooks?: ClaudeHooks /** * Names of skills CE owns in the Codex managed tree that are NOT written by * this bundle. Used in agents-only installs (default `--to codex`) where From 426b4be630a5d80679fe5b57683825da21ca9d92 Mon Sep 17 00:00:00 2001 From: nkolly Date: Fri, 1 May 2026 11:47:34 -0600 Subject: [PATCH 2/4] fix: clean up managed hooks when plugin no longer defines any Address review feedback: always run the merge even when bundle.hooks is empty, so previously installed hooks tagged with _source are removed on upgrade. Prevents stale hooks from persisting indefinitely. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/targets/codex.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/targets/codex.ts b/src/targets/codex.ts index e90fbe2df..7763b2f85 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -138,11 +138,17 @@ export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): // Write hooks to .codex/hooks.json — Codex uses the same hooks format // as Claude Code. Hooks are merged with any existing hooks file to avoid // clobbering hooks from other plugins or manual configuration. - if (bundle.hooks && Object.keys(bundle.hooks.hooks).length > 0) { + // Always run the merge (even with empty hooks) so previously installed + // hooks tagged with this plugin's _source are cleaned up on upgrade. + { const hooksPath = path.join(codexRoot, "hooks.json") const existingHooks = await readJsonSafe(hooksPath) - const mergedHooks = mergeCodexHooks(existingHooks, bundle.hooks.hooks, pluginName) - await writeTextSecure(hooksPath, JSON.stringify(mergedHooks, null, 2) + "\n") + const pluginHooks = bundle.hooks?.hooks ?? {} + const mergedHooks = mergeCodexHooks(existingHooks, pluginHooks, pluginName) + // Only write if there are hooks to write or if we need to clean up old ones + if (Object.keys((mergedHooks.hooks as Record) ?? {}).length > 0 || existingHooks !== null) { + await writeTextSecure(hooksPath, JSON.stringify(mergedHooks, null, 2) + "\n") + } } } From 2acaff51a2fe538ea9dc243c104ce3560d408376 Mon Sep 17 00:00:00 2001 From: nkolly Date: Fri, 1 May 2026 13:45:27 -0600 Subject: [PATCH 3/4] fix: backup hooks.json before overwriting, matching config.toml pattern Co-Authored-By: Claude Opus 4.6 (1M context) --- src/targets/codex.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/targets/codex.ts b/src/targets/codex.ts index 7763b2f85..ef0f539cf 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -145,8 +145,14 @@ export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): const existingHooks = await readJsonSafe(hooksPath) const pluginHooks = bundle.hooks?.hooks ?? {} const mergedHooks = mergeCodexHooks(existingHooks, pluginHooks, pluginName) - // Only write if there are hooks to write or if we need to clean up old ones - if (Object.keys((mergedHooks.hooks as Record) ?? {}).length > 0 || existingHooks !== null) { + const hasHooks = Object.keys((mergedHooks.hooks as Record) ?? {}).length > 0 + if (hasHooks || existingHooks !== null) { + if (existingHooks !== null) { + const backupPath = await backupFile(hooksPath) + if (backupPath) { + console.log(`Backed up existing hooks to ${backupPath}`) + } + } await writeTextSecure(hooksPath, JSON.stringify(mergedHooks, null, 2) + "\n") } } From 2527768e36b6291e50d8ad1d09a0e1b6de5165b4 Mon Sep 17 00:00:00 2001 From: nkolly Date: Mon, 4 May 2026 08:41:32 -0600 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20tests,=20=5Fmanaged=20index,=20idempotent=20writes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Add 6 fixture tests for hooks merge logic: - Plugin with hooks → _managed index written - Existing hooks from other plugin → preserved - Re-install → idempotent replacement of managed entries - Plugin removes hooks → managed entries cleaned, others preserved - Untagged manual entries → preserved - Legacy _source-tagged entries → migrated on re-install 2. Remove magic "coco" default — pluginName is now required for the hooks write path. Skips hooks entirely when pluginName is missing. 3. Skip backup/write when merged content is byte-identical to existing file. Prevents .bak churn on idempotent re-installs. 4. Hoist bookkeeping out of hook entries — replace _source field with a sibling _managed block that tracks { plugin: { event: [indices] } }. Hook entry objects stay schema-clean. Includes migration path that strips legacy _source tags on re-install. 64/64 codex tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/targets/codex.ts | 83 ++++++++++++++++++++----- tests/codex-writer.test.ts | 121 ++++++++++++++++++++++++++++++++++++- 2 files changed, 187 insertions(+), 17 deletions(-) diff --git a/src/targets/codex.ts b/src/targets/codex.ts index ef0f539cf..96028a371 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -139,21 +139,26 @@ export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): // as Claude Code. Hooks are merged with any existing hooks file to avoid // clobbering hooks from other plugins or manual configuration. // Always run the merge (even with empty hooks) so previously installed - // hooks tagged with this plugin's _source are cleaned up on upgrade. - { + // managed entries are cleaned up on upgrade. + if (pluginName) { const hooksPath = path.join(codexRoot, "hooks.json") const existingHooks = await readJsonSafe(hooksPath) const pluginHooks = bundle.hooks?.hooks ?? {} const mergedHooks = mergeCodexHooks(existingHooks, pluginHooks, pluginName) + const mergedContent = JSON.stringify(mergedHooks, null, 2) + "\n" + const existingContent = existingHooks !== null + ? JSON.stringify(existingHooks, null, 2) + "\n" + : null const hasHooks = Object.keys((mergedHooks.hooks as Record) ?? {}).length > 0 - if (hasHooks || existingHooks !== null) { + // Only write when content actually changes (avoids backup churn on idempotent re-installs) + if ((hasHooks || existingHooks !== null) && mergedContent !== existingContent) { if (existingHooks !== null) { const backupPath = await backupFile(hooksPath) if (backupPath) { console.log(`Backed up existing hooks to ${backupPath}`) } } - await writeTextSecure(hooksPath, JSON.stringify(mergedHooks, null, 2) + "\n") + await writeTextSecure(hooksPath, mergedContent) } } } @@ -650,36 +655,71 @@ async function readJsonSafe(filePath: string): Promise | type HookEntry = { matcher?: string; hooks: Array<{ type: string; command?: string; prompt?: string; agent?: string; timeout?: number }> } +/** + * Index tracking which hook entries are managed by which plugin. + * Stored as a sibling `_managed` block in hooks.json so hook entry + * objects stay schema-clean (no `_source` field injected into runtime data). + * + * Shape: `{ "": { "": [, ...] } }` + */ +type ManagedIndex = Record> + /** * Merge plugin hooks into an existing .codex/hooks.json, preserving hooks - * from other sources. Uses a managed-block pattern: each plugin's hooks are - * tagged with a `_source` field so re-installs can replace them cleanly. + * from other sources. Uses a `_managed` index block to track which entries + * belong to which plugin, so re-installs can replace them cleanly without + * injecting bookkeeping fields into hook entry objects. */ -function mergeCodexHooks( +export function mergeCodexHooks( existing: Record | null, pluginHooks: Record, - pluginName?: string, + pluginName: string, ): Record { - const source = pluginName ?? "coco" const result: Record = {} + const managed: ManagedIndex = (existing?._managed as ManagedIndex) ?? {} + + // Collect indices of entries managed by this plugin (to be removed) + const ownedIndices: Record> = {} + if (managed[pluginName]) { + for (const [event, indices] of Object.entries(managed[pluginName])) { + ownedIndices[event] = new Set(indices) + } + } - // Preserve existing hooks that aren't from this plugin + // Preserve existing hooks, filtering out this plugin's managed entries const existingHooks = (existing?.hooks ?? {}) as Record for (const [event, matchers] of Object.entries(existingHooks)) { if (!Array.isArray(matchers)) continue - result[event] = matchers.filter((m) => { + const owned = ownedIndices[event] + result[event] = owned + ? matchers.filter((_, idx) => !owned.has(idx)) + : [...matchers] + } + + // Also filter out entries with legacy `_source` tag from this plugin + // (migration path from the previous `_source`-in-entry format) + for (const [event, matchers] of Object.entries(result)) { + result[event] = (matchers as Array>).filter((m) => { if (typeof m === "object" && m !== null && "_source" in m) { - return (m as Record)._source !== source + return m._source !== pluginName } - return true // keep hooks without a source tag (manual or other plugins) + return true }) } - // Add this plugin's hooks with source tag + // Build new managed index for this plugin + const newManagedForPlugin: Record = {} + + // Add this plugin's hooks (clean entries, no _source field) for (const [event, matchers] of Object.entries(pluginHooks)) { if (!result[event]) result[event] = [] + const indices: number[] = [] for (const matcher of matchers) { - result[event].push({ ...matcher, _source: source }) + indices.push(result[event].length) + result[event].push({ ...matcher }) + } + if (indices.length > 0) { + newManagedForPlugin[event] = indices } } @@ -688,5 +728,16 @@ function mergeCodexHooks( if (result[event].length === 0) delete result[event] } - return { hooks: result } + // Update managed index + if (Object.keys(newManagedForPlugin).length > 0) { + managed[pluginName] = newManagedForPlugin + } else { + delete managed[pluginName] + } + + const output: Record = { hooks: result } + if (Object.keys(managed).length > 0) { + output._managed = managed + } + return output } diff --git a/tests/codex-writer.test.ts b/tests/codex-writer.test.ts index 2def3395b..65ec3c92c 100644 --- a/tests/codex-writer.test.ts +++ b/tests/codex-writer.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test" import { promises as fs } from "fs" import path from "path" import os from "os" -import { mergeCodexConfig, renderCodexConfig, writeCodexBundle } from "../src/targets/codex" +import { mergeCodexConfig, renderCodexConfig, writeCodexBundle, mergeCodexHooks } from "../src/targets/codex" import type { CodexBundle } from "../src/types/codex" import { loadClaudePlugin } from "../src/parsers/claude" import { convertClaudeToCodex } from "../src/converters/claude-to-codex" @@ -1087,3 +1087,122 @@ describe("mergeCodexConfig", () => { expect(result).toBe("") }) }) + +describe("mergeCodexHooks", () => { + test("writes hooks with _managed index for a new plugin", () => { + const result = mergeCodexHooks(null, { + SessionStart: [{ matcher: "*", hooks: [{ type: "command", command: "my-hook" }] }], + }, "my-plugin") + + const hooks = result.hooks as Record + expect(hooks.SessionStart).toHaveLength(1) + expect((hooks.SessionStart[0] as Record).matcher).toBe("*") + // No _source field injected into hook entries + expect((hooks.SessionStart[0] as Record)._source).toBeUndefined() + // Managed index tracks ownership + const managed = result._managed as Record> + expect(managed["my-plugin"].SessionStart).toEqual([0]) + }) + + test("preserves hooks from other plugins", () => { + const existing = { + hooks: { + SessionStart: [{ matcher: "*", hooks: [{ type: "command", command: "other-hook" }] }], + }, + _managed: { "other-plugin": { SessionStart: [0] } }, + } + + const result = mergeCodexHooks(existing, { + Stop: [{ matcher: "*", hooks: [{ type: "command", command: "my-stop" }] }], + }, "my-plugin") + + const hooks = result.hooks as Record + // Other plugin's hook preserved + expect(hooks.SessionStart).toHaveLength(1) + expect((hooks.SessionStart[0] as Record).command).toBeUndefined() + // Our hook added + expect(hooks.Stop).toHaveLength(1) + }) + + test("re-install replaces managed entries idempotently", () => { + const existing = { + hooks: { + SessionStart: [ + { matcher: "*", hooks: [{ type: "command", command: "old-hook" }] }, + ], + }, + _managed: { "my-plugin": { SessionStart: [0] } }, + } + + const result = mergeCodexHooks(existing, { + SessionStart: [{ matcher: "*", hooks: [{ type: "command", command: "new-hook" }] }], + }, "my-plugin") + + const hooks = result.hooks as Record + expect(hooks.SessionStart).toHaveLength(1) + // Old hook replaced with new + const entry = hooks.SessionStart[0] as Record + const entryHooks = entry.hooks as Array> + expect(entryHooks[0].command).toBe("new-hook") + }) + + test("cleans up managed entries when plugin removes hooks", () => { + const existing = { + hooks: { + SessionStart: [ + { matcher: "*", hooks: [{ type: "command", command: "manual-hook" }] }, + { matcher: "*", hooks: [{ type: "command", command: "plugin-hook" }] }, + ], + }, + _managed: { "my-plugin": { SessionStart: [1] } }, + } + + const result = mergeCodexHooks(existing, {}, "my-plugin") + + const hooks = result.hooks as Record + // Manual hook preserved, plugin hook removed + expect(hooks.SessionStart).toHaveLength(1) + const entryHooks = (hooks.SessionStart[0] as Record).hooks as Array> + expect(entryHooks[0].command).toBe("manual-hook") + // Managed index cleaned + expect((result._managed as Record)?.["my-plugin"]).toBeUndefined() + }) + + test("preserves untagged manual hook entries", () => { + const existing = { + hooks: { + SessionStart: [ + { matcher: "*", hooks: [{ type: "command", command: "manual" }] }, + ], + }, + // No _managed — all entries are manual + } + + const result = mergeCodexHooks(existing, { + Stop: [{ matcher: "*", hooks: [{ type: "command", command: "plugin-stop" }] }], + }, "my-plugin") + + const hooks = result.hooks as Record + expect(hooks.SessionStart).toHaveLength(1) + expect(hooks.Stop).toHaveLength(1) + }) + + test("cleans up legacy _source-tagged entries from previous format", () => { + const existing = { + hooks: { + SessionStart: [ + { matcher: "*", hooks: [{ type: "command", command: "legacy" }], _source: "my-plugin" }, + { matcher: "*", hooks: [{ type: "command", command: "manual" }] }, + ], + }, + } + + const result = mergeCodexHooks(existing, { + SessionStart: [{ matcher: "*", hooks: [{ type: "command", command: "new" }] }], + }, "my-plugin") + + const hooks = result.hooks as Record + // Legacy _source entry removed, manual preserved, new added + expect(hooks.SessionStart).toHaveLength(2) + }) +})