Skip to content

refactor(hooks): clean up Codex CLI hooks implementation (#1445) #1451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dyoshikawa wants to merge 2 commits into from
Closed

refactor(hooks): clean up Codex CLI hooks implementation (#1445) #1451

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7faaa0
refactor(hooks): consolidate codexcli/geminicli hooks onto shared con…
dyoshikawa Apr 8, 2026
e549860
refactor(hooks): wire logger and gate name/description passthrough
dyoshikawa Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/features/hooks/codexcli-hooks.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,31 @@ describe("CodexcliConfigToml", () => {
expect(content).toContain("myserver");
});

it("should merge with existing [features] section without overwriting other flags", async () => {
await ensureDir(join(testDir, ".codex"));
await writeFileContent(
join(testDir, ".codex", "config.toml"),
"[features]\nsome_other_flag = true\n",
);

const configToml = await CodexcliConfigToml.fromBaseDir({ baseDir: testDir });
const content = configToml.getFileContent();
expect(content).toContain("codex_hooks = true");
expect(content).toContain("some_other_flag = true");
});

it("should throw a clear error when existing config.toml is malformed", async () => {
await ensureDir(join(testDir, ".codex"));
await writeFileContent(
join(testDir, ".codex", "config.toml"),
"this is not = valid = toml [[\n",
);

await expect(CodexcliConfigToml.fromBaseDir({ baseDir: testDir })).rejects.toThrow(
/Failed to parse existing Codex CLI config\.toml/,
);
});

it("should set correct file paths", async () => {
const configToml = await CodexcliConfigToml.fromBaseDir({ baseDir: testDir });
expect(configToml.getRelativeDirPath()).toBe(".codex");
Expand Down
165 changes: 56 additions & 109 deletions src/features/hooks/codexcli-hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import { join } from "node:path";

import * as smolToml from "smol-toml";
import { z } from "zod/mini";

import type { AiFileParams } from "../../types/ai-file.js";
import type { ValidationResult } from "../../types/ai-file.js";
import type { HooksConfig } from "../../types/hooks.js";
import type { AiFileParams, ValidationResult } from "../../types/ai-file.js";
import {
CANONICAL_TO_CODEXCLI_EVENT_NAMES,
CODEXCLI_HOOK_EVENTS,
CODEXCLI_TO_CANONICAL_EVENT_NAMES,
CANONICAL_TO_CODEXCLI_EVENT_NAMES,
} from "../../types/hooks.js";
import { ToolFile } from "../../types/tool-file.js";
import { formatError } from "../../utils/error.js";
import { readFileContentOrNull } from "../../utils/file.js";
import type { Logger } from "../../utils/logger.js";
import type { RulesyncHooks } from "./rulesync-hooks.js";
import type { ToolHooksConverterConfig } from "./tool-hooks-converter.js";
import { canonicalToToolHooks, toolHooksToCanonical } from "./tool-hooks-converter.js";
import {
ToolHooks,
type ToolHooksForDeletionParams,
Expand All @@ -24,107 +24,26 @@ import {
} from "./tool-hooks.js";

/**
* Convert canonical hooks config to Codex CLI format.
* Filters shared hooks to CODEXCLI_HOOK_EVENTS, merges config.codexcli?.hooks,
* then converts to PascalCase and Codex CLI matcher/hooks structure.
* Unlike Claude Code or Gemini CLI, Codex CLI has no project directory variable,
* so commands are passed through as-is.
*/
function canonicalToCodexcliHooks(config: HooksConfig): Record<string, unknown[]> {
const codexSupported: Set<string> = new Set(CODEXCLI_HOOK_EVENTS);
const sharedHooks: HooksConfig["hooks"] = {};
for (const [event, defs] of Object.entries(config.hooks)) {
if (codexSupported.has(event)) {
sharedHooks[event] = defs;
}
}
const effectiveHooks: HooksConfig["hooks"] = {
...sharedHooks,
...config.codexcli?.hooks,
};
const codex: Record<string, unknown[]> = {};
for (const [eventName, definitions] of Object.entries(effectiveHooks)) {
const codexEventName = CANONICAL_TO_CODEXCLI_EVENT_NAMES[eventName] ?? eventName;
const byMatcher = new Map<string, HooksConfig["hooks"][string]>();
for (const def of definitions) {
const key = def.matcher ?? "";
const list = byMatcher.get(key);
if (list) list.push(def);
else byMatcher.set(key, [def]);
}
const entries: unknown[] = [];
for (const [matcherKey, defs] of byMatcher) {
const commandDefs = defs.filter((def) => !def.type || def.type === "command");
if (commandDefs.length === 0) continue;
const hooks = commandDefs.map((def) => ({
type: "command" as const,
...(def.command !== undefined && def.command !== null && { command: def.command }),
...(def.timeout !== undefined && def.timeout !== null && { timeout: def.timeout }),
}));
entries.push(matcherKey ? { matcher: matcherKey, hooks } : { hooks });
}
if (entries.length > 0) {
codex[codexEventName] = entries;
}
}
return codex;
}

/**
* Codex CLI hook entry as stored in each matcher group's `hooks` array.
* Uses `z.looseObject` so that unknown fields added by future Codex CLI
* versions are accepted and silently ignored during import.
*/
const CodexHookEntrySchema = z.looseObject({
type: z.optional(z.string()),
command: z.optional(z.string()),
timeout: z.optional(z.number()),
});

/**
* A matcher group entry in a Codex CLI event array.
* Each event maps to an array of these groups.
*/
const CodexMatcherEntrySchema = z.looseObject({
matcher: z.optional(z.string()),
hooks: z.optional(z.array(CodexHookEntrySchema)),
});

/**
* Extract hooks from Codex CLI hooks.json into canonical format.
* Converter config for Codex CLI hooks.
*
* Differences from Claude/Factory Droid:
* - `projectDirVar: ""` — Codex CLI has no project directory variable, so commands are
* passed through verbatim with no prefixing.
* - `supportedHookTypes: ["command"]` — Codex CLI only understands command-type hooks;
* prompt-type hooks are filtered out on export. (See `ToolHooksConverterConfig` for the
* intentional asymmetry with `toolHooksToCanonical`, which preserves prompt-type hooks
* on import to avoid silent round-trip data loss.)
* - `omitEmptyEvents: true` — events whose entries become empty after filtering are
* dropped entirely from the output, since Codex CLI's schema rejects empty event arrays.
*/
function codexcliHooksToCanonical(codexHooks: unknown): HooksConfig["hooks"] {
if (codexHooks === null || codexHooks === undefined || typeof codexHooks !== "object") {
return {};
}
const canonical: HooksConfig["hooks"] = {};
for (const [codexEventName, matcherEntries] of Object.entries(codexHooks)) {
const eventName = CODEXCLI_TO_CANONICAL_EVENT_NAMES[codexEventName] ?? codexEventName;
if (!Array.isArray(matcherEntries)) continue;
const defs: HooksConfig["hooks"][string] = [];
for (const rawEntry of matcherEntries) {
const parseResult = CodexMatcherEntrySchema.safeParse(rawEntry);
if (!parseResult.success) continue;
const entry = parseResult.data;
const hooks = entry.hooks ?? [];
for (const h of hooks) {
const hookType = h.type === "command" || h.type === "prompt" ? h.type : "command";
defs.push({
type: hookType,
...(h.command !== undefined && h.command !== null && { command: h.command }),
...(h.timeout !== undefined && h.timeout !== null && { timeout: h.timeout }),
...(entry.matcher !== undefined &&
entry.matcher !== null &&
entry.matcher !== "" && { matcher: entry.matcher }),
});
}
}
if (defs.length > 0) {
canonical[eventName] = defs;
}
}
return canonical;
}
const CODEXCLI_CONVERTER_CONFIG: ToolHooksConverterConfig = {
supportedEvents: CODEXCLI_HOOK_EVENTS,
canonicalToToolEventNames: CANONICAL_TO_CODEXCLI_EVENT_NAMES,
toolToCanonicalEventNames: CODEXCLI_TO_CANONICAL_EVENT_NAMES,
projectDirVar: "",
supportedHookTypes: new Set(["command"]),
omitEmptyEvents: true,
};

/**
* Build the content for `.codex/config.toml` with `[features] codex_hooks = true`.
Expand All @@ -134,7 +53,15 @@ function codexcliHooksToCanonical(codexHooks: unknown): HooksConfig["hooks"] {
async function buildCodexConfigTomlContent({ baseDir }: { baseDir: string }): Promise<string> {
const configPath = join(baseDir, ".codex", "config.toml");
const existingContent = (await readFileContentOrNull(configPath)) ?? smolToml.stringify({});
const configToml = smolToml.parse(existingContent);
let configToml: smolToml.TomlTable;
try {
configToml = smolToml.parse(existingContent);
} catch (error) {
throw new Error(
`Failed to parse existing Codex CLI config.toml at ${configPath}: ${formatError(error)}`,
{ cause: error },
);
}

if (typeof configToml.features !== "object" || configToml.features === null) {
// eslint-disable-next-line no-type-assertion/no-type-assertion
Expand Down Expand Up @@ -178,6 +105,14 @@ export class CodexcliHooks extends ToolHooks {
return { relativeDirPath: ".codex", relativeFilePath: "hooks.json" };
}

/**
* Auxiliary files generated alongside `.codex/hooks.json`.
* The processor calls this generically so it does not need codexcli-specific branching.
*/
static async getAuxiliaryFiles({ baseDir }: { baseDir: string }): Promise<ToolFile[]> {
return [await CodexcliConfigToml.fromBaseDir({ baseDir })];
}

static async fromFile({
baseDir = process.cwd(),
validate = true,
Expand All @@ -200,10 +135,19 @@ export class CodexcliHooks extends ToolHooks {
rulesyncHooks,
validate = true,
global = false,
}: ToolHooksFromRulesyncHooksParams & { global?: boolean }): Promise<CodexcliHooks> {
logger,
}: ToolHooksFromRulesyncHooksParams & {
global?: boolean;
logger?: Logger;
}): Promise<CodexcliHooks> {
const paths = CodexcliHooks.getSettablePaths({ global });
const config = rulesyncHooks.getJson();
const codexHooks = canonicalToCodexcliHooks(config);
const codexHooks = canonicalToToolHooks({
config,
toolOverrideHooks: config.codexcli?.hooks,
converterConfig: CODEXCLI_CONVERTER_CONFIG,
logger,
});
const fileContent = JSON.stringify({ hooks: codexHooks }, null, 2);

return new CodexcliHooks({
Expand All @@ -227,7 +171,10 @@ export class CodexcliHooks extends ToolHooks {
},
);
}
const hooks = codexcliHooksToCanonical(parsed.hooks);
const hooks = toolHooksToCanonical({
hooks: parsed.hooks,
converterConfig: CODEXCLI_CONVERTER_CONFIG,
});
return this.toRulesyncHooksDefault({
fileContent: JSON.stringify({ version: 1, hooks }, null, 2),
});
Expand Down
Loading
Loading