diff --git a/src/features/hooks/codexcli-hooks.test.ts b/src/features/hooks/codexcli-hooks.test.ts index 31f4e188a..96d7dd06d 100644 --- a/src/features/hooks/codexcli-hooks.test.ts +++ b/src/features/hooks/codexcli-hooks.test.ts @@ -363,6 +363,25 @@ describe("CodexcliConfigToml", () => { expect(content).toContain("myserver"); }); + it("should preserve existing [features] values when enabling codex_hooks", async () => { + await ensureDir(join(testDir, ".codex")); + await writeFileContent(join(testDir, ".codex", "config.toml"), "[features]\nverbose = true\n"); + + const configToml = await CodexcliConfigToml.fromBaseDir({ baseDir: testDir }); + const content = configToml.getFileContent(); + expect(content).toContain("codex_hooks = true"); + expect(content).toContain("verbose = true"); + }); + + it("should throw a readable error when existing config.toml is invalid", async () => { + await ensureDir(join(testDir, ".codex")); + await writeFileContent(join(testDir, ".codex", "config.toml"), "[features"); + + await expect(CodexcliConfigToml.fromBaseDir({ baseDir: testDir })).rejects.toThrow( + "Failed to parse existing Codex CLI config", + ); + }); + it("should set correct file paths", async () => { const configToml = await CodexcliConfigToml.fromBaseDir({ baseDir: testDir }); expect(configToml.getRelativeDirPath()).toBe(".codex"); diff --git a/src/features/hooks/codexcli-hooks.ts b/src/features/hooks/codexcli-hooks.ts index 3db1f9b57..b87a35113 100644 --- a/src/features/hooks/codexcli-hooks.ts +++ b/src/features/hooks/codexcli-hooks.ts @@ -1,11 +1,8 @@ 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 { CODEXCLI_HOOK_EVENTS, CODEXCLI_TO_CANONICAL_EVENT_NAMES, @@ -15,6 +12,8 @@ import { ToolFile } from "../../types/tool-file.js"; import { formatError } from "../../utils/error.js"; import { readFileContentOrNull } from "../../utils/file.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, @@ -23,108 +22,14 @@ import { type ToolHooksSettablePaths, } 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 { - const codexSupported: Set = 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 = {}; - for (const [eventName, definitions] of Object.entries(effectiveHooks)) { - const codexEventName = CANONICAL_TO_CODEXCLI_EVENT_NAMES[eventName] ?? eventName; - const byMatcher = new Map(); - 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. - */ -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"]), + passthroughFields: ["name", "description"], +}; /** * Build the content for `.codex/config.toml` with `[features] codex_hooks = true`. @@ -134,7 +39,17 @@ function codexcliHooksToCanonical(codexHooks: unknown): HooksConfig["hooks"] { async function buildCodexConfigTomlContent({ baseDir }: { baseDir: string }): Promise { const configPath = join(baseDir, ".codex", "config.toml"); const existingContent = (await readFileContentOrNull(configPath)) ?? smolToml.stringify({}); - const configToml = smolToml.parse(existingContent); + let configToml: smolToml.TomlPrimitive; + try { + configToml = smolToml.parse(existingContent); + } catch (error) { + throw new Error( + `Failed to parse existing Codex CLI config at ${configPath}: ${formatError(error)}`, + { + cause: error, + }, + ); + } if (typeof configToml.features !== "object" || configToml.features === null) { // eslint-disable-next-line no-type-assertion/no-type-assertion @@ -203,7 +118,11 @@ export class CodexcliHooks extends ToolHooks { }: ToolHooksFromRulesyncHooksParams & { global?: boolean }): Promise { 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, + }); const fileContent = JSON.stringify({ hooks: codexHooks }, null, 2); return new CodexcliHooks({ @@ -227,7 +146,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), }); @@ -250,4 +172,13 @@ export class CodexcliHooks extends ToolHooks { validate: false, }); } + + static async getAuxiliaryFiles({ + baseDir = process.cwd(), + }: { + baseDir?: string; + global?: boolean; + } = {}): Promise { + return [await CodexcliConfigToml.fromBaseDir({ baseDir })]; + } } diff --git a/src/features/hooks/hooks-processor.test.ts b/src/features/hooks/hooks-processor.test.ts index b3d5003b0..008f1d7b8 100644 --- a/src/features/hooks/hooks-processor.test.ts +++ b/src/features/hooks/hooks-processor.test.ts @@ -8,6 +8,7 @@ import { createMockLogger } from "../../test-utils/mock-logger.js"; import { setupTestDirectory } from "../../test-utils/test-directories.js"; import { ensureDir, writeFileContent } from "../../utils/file.js"; import { ClaudecodeHooks } from "./claudecode-hooks.js"; +import { CodexcliConfigToml, CodexcliHooks } from "./codexcli-hooks.js"; import { CursorHooks } from "./cursor-hooks.js"; import { HooksProcessor } from "./hooks-processor.js"; import { RulesyncHooks } from "./rulesync-hooks.js"; @@ -238,6 +239,28 @@ describe("HooksProcessor", () => { expect(Array.isArray(parsed.hooks.SessionStart)).toBe(true); }); + it("should convert rulesync hooks to Codex CLI hooks and include auxiliary config.toml", async () => { + const config = { + version: 1, + hooks: { + sessionStart: [{ type: "command", command: "echo codex" }], + }, + }; + const rulesyncHooks = new RulesyncHooks({ + baseDir: testDir, + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "hooks.json", + fileContent: JSON.stringify(config), + validate: false, + }); + + const processor = new HooksProcessor({ logger, baseDir: testDir, toolTarget: "codexcli" }); + const toolFiles = await processor.convertRulesyncFilesToToolFiles([rulesyncHooks]); + expect(toolFiles).toHaveLength(2); + expect(toolFiles[0]).toBeInstanceOf(CodexcliHooks); + expect(toolFiles[1]).toBeInstanceOf(CodexcliConfigToml); + }); + it("should throw when no rulesync hooks file in list", async () => { const processor = new HooksProcessor({ logger, baseDir: testDir, toolTarget: "cursor" }); await expect(processor.convertRulesyncFilesToToolFiles([])).rejects.toThrow( diff --git a/src/features/hooks/hooks-processor.ts b/src/features/hooks/hooks-processor.ts index 214e59264..1b24d8b07 100644 --- a/src/features/hooks/hooks-processor.ts +++ b/src/features/hooks/hooks-processor.ts @@ -21,7 +21,7 @@ import type { ToolTarget } from "../../types/tool-targets.js"; import { formatError } from "../../utils/error.js"; import type { Logger } from "../../utils/logger.js"; import { ClaudecodeHooks } from "./claudecode-hooks.js"; -import { CodexcliConfigToml, CodexcliHooks } from "./codexcli-hooks.js"; +import { CodexcliHooks } from "./codexcli-hooks.js"; import { CopilotHooks } from "./copilot-hooks.js"; import { CursorHooks } from "./cursor-hooks.js"; import { DeepagentsHooks } from "./deepagents-hooks.js"; @@ -65,6 +65,10 @@ type ToolHooksFactory = { relativeFilePath: string; }; isDeletable?: (instance: ToolHooks) => boolean; + getAuxiliaryFiles?: (params: { + baseDir?: string; + global?: boolean; + }) => ToolFile[] | Promise; }; meta: { supportsProject: boolean; @@ -373,10 +377,12 @@ export class HooksProcessor extends FeatureProcessor { }); const result: ToolFile[] = [toolHooks]; - - // For codexcli, also generate .codex/config.toml with the feature flag - if (this.toolTarget === "codexcli") { - result.push(await CodexcliConfigToml.fromBaseDir({ baseDir: this.baseDir })); + const auxiliaryFiles = await factory.class.getAuxiliaryFiles?.({ + baseDir: this.baseDir, + global: this.global, + }); + if (auxiliaryFiles && auxiliaryFiles.length > 0) { + result.push(...auxiliaryFiles); } return result; diff --git a/src/features/hooks/tool-hooks-converter.ts b/src/features/hooks/tool-hooks-converter.ts index 3ac0c1d06..b5543c24c 100644 --- a/src/features/hooks/tool-hooks-converter.ts +++ b/src/features/hooks/tool-hooks-converter.ts @@ -24,6 +24,8 @@ export type ToolHooksConverterConfig = { canonicalToToolEventNames: Record; toolToCanonicalEventNames: Record; projectDirVar: string; + supportedHookTypes?: ReadonlySet<"command" | "prompt">; + passthroughFields?: ReadonlyArray<"name" | "description">; /** * When true, only dot-relative commands (e.g. ./script.sh, ../script.sh, .rulesync/hooks/x.sh) * are prefixed with projectDirVar. Bare executable commands like `npx prettier ...` are left intact. @@ -82,11 +84,20 @@ export function canonicalToToolHooks({ `matcher "${matcherKey}" on "${eventName}" hook will be ignored — this event does not support matchers`, ); } - const hooks = defs.map((def) => { + const hooks: Array> = []; + for (const def of defs) { + const hookType = def.type ?? "command"; + if ( + converterConfig.supportedHookTypes && + !converterConfig.supportedHookTypes.has(hookType) + ) { + continue; + } const commandText = def.command; const trimmedCommand = typeof commandText === "string" ? commandText.trimStart() : undefined; const shouldPrefix = + converterConfig.projectDirVar !== "" && typeof trimmedCommand === "string" && !trimmedCommand.startsWith("$") && (!converterConfig.prefixDotRelativeCommandsOnly || trimmedCommand.startsWith(".")); @@ -95,17 +106,28 @@ export function canonicalToToolHooks({ shouldPrefix && typeof trimmedCommand === "string" ? `${converterConfig.projectDirVar}/${trimmedCommand.replace(/^\.\//, "")}` : def.command; - return { - type: def.type ?? "command", + hooks.push({ + type: hookType, ...(command !== undefined && command !== null && { command }), ...(def.timeout !== undefined && def.timeout !== null && { timeout: def.timeout }), ...(def.prompt !== undefined && def.prompt !== null && { prompt: def.prompt }), - }; - }); + ...(converterConfig.passthroughFields?.includes("name") && + def.name !== undefined && + def.name !== null && { name: def.name }), + ...(converterConfig.passthroughFields?.includes("description") && + def.description !== undefined && + def.description !== null && { description: def.description }), + }); + } + if (hooks.length === 0) { + continue; + } const includeMatcher = matcherKey && !isNoMatcherEvent; entries.push(includeMatcher ? { matcher: matcherKey, hooks } : { hooks }); } - result[toolEventName] = entries; + if (entries.length > 0) { + result[toolEventName] = entries; + } } return result; } @@ -140,7 +162,9 @@ export function toolHooksToCanonical({ for (const h of hookDefs) { const cmd = typeof h.command === "string" ? h.command : undefined; const command = - typeof cmd === "string" && cmd.includes(`${converterConfig.projectDirVar}/`) + converterConfig.projectDirVar !== "" && + typeof cmd === "string" && + cmd.includes(`${converterConfig.projectDirVar}/`) ? cmd.replace( new RegExp( `^${converterConfig.projectDirVar.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\/?`, @@ -156,6 +180,10 @@ export function toolHooksToCanonical({ ...(command !== undefined && command !== null && { command }), ...(timeout !== undefined && timeout !== null && { timeout }), ...(prompt !== undefined && prompt !== null && { prompt }), + ...(converterConfig.passthroughFields?.includes("name") && + typeof h.name === "string" && { name: h.name }), + ...(converterConfig.passthroughFields?.includes("description") && + typeof h.description === "string" && { description: h.description }), ...(rawEntry.matcher !== undefined && rawEntry.matcher !== null && rawEntry.matcher !== "" && { matcher: rawEntry.matcher }), diff --git a/src/features/hooks/tool-hooks.ts b/src/features/hooks/tool-hooks.ts index 3f885c02c..71ffa2a38 100644 --- a/src/features/hooks/tool-hooks.ts +++ b/src/features/hooks/tool-hooks.ts @@ -68,4 +68,11 @@ export abstract class ToolHooks extends ToolFile { static forDeletion(_params: ToolHooksForDeletionParams): ToolHooks { throw new Error("Please implement this method in the subclass."); } + + static async getAuxiliaryFiles(_params: { + baseDir?: string; + global?: boolean; + }): Promise { + return []; + } }