From 4ecdab3ff308eb73b880815f1ee46ff23778c9d7 Mon Sep 17 00:00:00 2001 From: Ravi Tharuma Date: Mon, 20 Jul 2026 02:07:59 +0200 Subject: [PATCH] fix(hooks): correct Factory Droid hooks file shape --- src/e2e/e2e-hooks.spec.ts | 11 +-- src/features/hooks/factorydroid-hooks.test.ts | 76 ++++++++++++++----- src/features/hooks/factorydroid-hooks.ts | 8 +- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/src/e2e/e2e-hooks.spec.ts b/src/e2e/e2e-hooks.spec.ts index 17e4e8e03..2ff0b62e3 100644 --- a/src/e2e/e2e-hooks.spec.ts +++ b/src/e2e/e2e-hooks.spec.ts @@ -19,9 +19,10 @@ import { * (e.g. claudecode uses PascalCase `Stop`), so checking command paths inside * the serialized hooks block is the most tool-agnostic assertion. */ -function assertHookCommandsPreserved(parsed: { hooks?: unknown }): void { - expect(parsed.hooks).toBeDefined(); - const serialized = JSON.stringify(parsed.hooks); +function assertHookCommandsPreserved(parsed: { hooks?: unknown }, rootIsHooks = false): void { + const hooks = rootIsHooks ? parsed : parsed.hooks; + expect(hooks).toBeDefined(); + const serialized = JSON.stringify(hooks); expect(serialized).toContain(".rulesync/hooks/session-start.sh"); expect(serialized).toContain(".rulesync/hooks/audit.sh"); } @@ -176,7 +177,7 @@ describe("E2E: hooks", () => { } else { // codexcli, factorydroid, goose: event-name casing/mapping // varies per tool, so verify the configured hook command paths are preserved. - assertHookCommandsPreserved(parsed); + assertHookCommandsPreserved(parsed, target === "factorydroid"); } } }); @@ -700,7 +701,7 @@ describe("E2E: hooks (global mode)", () => { expect(JSON.stringify(parsed.hooks)).toContain(".rulesync/hooks/session-start.sh"); expect(JSON.stringify(parsed.hooks)).toContain(".rulesync/hooks/audit.sh"); } else { - assertHookCommandsPreserved(JSON.parse(generatedContent)); + assertHookCommandsPreserved(JSON.parse(generatedContent), target === "factorydroid"); } }, ); diff --git a/src/features/hooks/factorydroid-hooks.test.ts b/src/features/hooks/factorydroid-hooks.test.ts index 4827ad219..c9762cf2f 100644 --- a/src/features/hooks/factorydroid-hooks.test.ts +++ b/src/features/hooks/factorydroid-hooks.test.ts @@ -35,6 +35,46 @@ describe("FactorydroidHooks", () => { }); describe("fromRulesyncHooks", () => { + it("should emit a dedicated hooks file that preserves an override command", async () => { + const config = { + version: 1, + hooks: {}, + factorydroid: { + hooks: { + sessionStart: [{ type: "command", command: ".rulesync/hooks/notify.sh" }], + }, + }, + }; + const rulesyncHooks = new RulesyncHooks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "hooks.json", + fileContent: JSON.stringify(config), + validate: false, + }); + + const factorydroidHooks = await FactorydroidHooks.fromRulesyncHooks({ + outputRoot: testDir, + rulesyncHooks, + validate: false, + global: true, + }); + + const parsed = JSON.parse(factorydroidHooks.getFileContent()); + expect(parsed).toEqual({ + SessionStart: [ + { + hooks: [ + { + type: "command", + command: '"$FACTORY_PROJECT_DIR"/.rulesync/hooks/notify.sh', + }, + ], + }, + ], + }); + }); + it("should filter shared hooks to Factory Droid-supported events and convert to PascalCase", async () => { await ensureDir(join(testDir, ".factory")); await writeFileContent(join(testDir, ".factory", "settings.json"), JSON.stringify({})); @@ -63,9 +103,9 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - expect(parsed.hooks.SessionStart).toBeDefined(); - expect(parsed.hooks.Stop).toBeDefined(); - expect(parsed.hooks.afterFileEdit).toBeUndefined(); + expect(parsed.SessionStart).toBeDefined(); + expect(parsed.Stop).toBeDefined(); + expect(parsed.afterFileEdit).toBeUndefined(); }); it("should prefix non-absolute commands with $FACTORY_PROJECT_DIR", async () => { @@ -94,7 +134,7 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - const sessionStartEntry = parsed.hooks.SessionStart[0]; + const sessionStartEntry = parsed.SessionStart[0]; expect(sessionStartEntry).toBeDefined(); expect(sessionStartEntry.matcher).toBeUndefined(); expect(sessionStartEntry.hooks[0].command).toContain("$FACTORY_PROJECT_DIR"); @@ -127,7 +167,7 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - expect(parsed.hooks.SessionStart[0].hooks[0].command).toBe( + expect(parsed.SessionStart[0].hooks[0].command).toBe( '"$FACTORY_PROJECT_DIR"/scripts/format.sh --fix --quiet', ); }); @@ -160,7 +200,7 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - expect(parsed.hooks.SessionStart[0].hooks[0].command).toBe( + expect(parsed.SessionStart[0].hooks[0].command).toBe( "$FACTORY_PROJECT_DIR/.factory/hooks/start.sh", ); }); @@ -202,9 +242,9 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - expect(parsed.hooks.SessionStart[0].hooks[0].command).toContain("factory-override.sh"); - expect(parsed.hooks.Notification).toBeDefined(); - expect(parsed.hooks.Notification[0].matcher).toBe("permission_prompt"); + expect(parsed.SessionStart[0].hooks[0].command).toContain("factory-override.sh"); + expect(parsed.Notification).toBeDefined(); + expect(parsed.Notification[0].matcher).toBe("permission_prompt"); }); it("should not leak Claude config hooks into Factory Droid output", async () => { @@ -245,9 +285,9 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); // Shared hooks should be present - expect(parsed.hooks.SessionStart[0].hooks[0].command).toContain("shared.sh"); + expect(parsed.SessionStart[0].hooks[0].command).toContain("shared.sh"); // Factory Droid-specific override should be present - expect(parsed.hooks.Stop).toBeDefined(); + expect(parsed.Stop).toBeDefined(); // Claude-specific hooks must NOT leak into Factory Droid output expect(JSON.stringify(parsed)).not.toContain("claude-only.sh"); expect(JSON.stringify(parsed)).not.toContain("claude-notify.sh"); @@ -303,8 +343,8 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); expect(parsed.otherKey).toBe("preserved"); - expect(parsed.hooks).toBeDefined(); - expect(parsed.hooks.SessionStart).toBeDefined(); + expect(parsed.hooks).toBeUndefined(); + expect(parsed.SessionStart).toBeDefined(); }); it("should handle hooks with matcher grouping", async () => { @@ -337,15 +377,15 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - expect(parsed.hooks.PreToolUse).toHaveLength(2); + expect(parsed.PreToolUse).toHaveLength(2); - const writeEntry = parsed.hooks.PreToolUse.find( + const writeEntry = parsed.PreToolUse.find( (e: Record) => e.matcher === "Write", ); expect(writeEntry).toBeDefined(); expect(writeEntry.hooks).toHaveLength(2); - const editEntry = parsed.hooks.PreToolUse.find( + const editEntry = parsed.PreToolUse.find( (e: Record) => e.matcher === "Edit", ); expect(editEntry).toBeDefined(); @@ -378,7 +418,7 @@ describe("FactorydroidHooks", () => { const content = factorydroidHooks.getFileContent(); const parsed = JSON.parse(content); - const hookDef = parsed.hooks.PreToolUse[0].hooks[0]; + const hookDef = parsed.PreToolUse[0].hooks[0]; expect(hookDef.type).toBe("prompt"); expect(hookDef.prompt).toBe("Check this tool call"); expect(hookDef.timeout).toBe(30000); @@ -409,7 +449,7 @@ describe("FactorydroidHooks", () => { }); const parsed = JSON.parse(factorydroidHooks.getFileContent()); - const hookDef = parsed.hooks.PreToolUse[0].hooks[0]; + const hookDef = parsed.PreToolUse[0].hooks[0]; expect(hookDef.prompt).toBe("Check this tool call"); expect(hookDef.model).toBeUndefined(); }); diff --git a/src/features/hooks/factorydroid-hooks.ts b/src/features/hooks/factorydroid-hooks.ts index 92db47d2c..a3ad90f92 100644 --- a/src/features/hooks/factorydroid-hooks.ts +++ b/src/features/hooks/factorydroid-hooks.ts @@ -115,7 +115,7 @@ export class FactorydroidHooks extends ToolHooks { converterConfig: FACTORYDROID_CONVERTER_CONFIG, logger, }); - const merged = { ...settings, hooks: factorydroidHooks }; + const merged = { ...settings, ...factorydroidHooks }; const fileContent = JSON.stringify(merged, null, 2); return new FactorydroidHooks({ outputRoot, @@ -127,9 +127,9 @@ export class FactorydroidHooks extends ToolHooks { } toRulesyncHooks(): RulesyncHooks { - let settings: { hooks?: unknown }; + let parsed: { hooks?: unknown }; try { - settings = JSON.parse(this.getFileContent()); + parsed = JSON.parse(this.getFileContent()); } catch (error) { throw new Error( `Failed to parse Factory Droid hooks content in ${join(this.getRelativeDirPath(), this.getRelativeFilePath())}: ${formatError(error)}`, @@ -139,7 +139,7 @@ export class FactorydroidHooks extends ToolHooks { ); } const hooks = toolHooksToCanonical({ - hooks: settings.hooks, + hooks: parsed.hooks ?? parsed, converterConfig: FACTORYDROID_CONVERTER_CONFIG, }); return this.toRulesyncHooksDefault({