From 17af2900c83f0b1644f547bd223c98362c0f672b Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Fri, 31 Jul 2026 18:49:02 -0700 Subject: [PATCH 1/3] fix(generate): fail the run when the ignore feature errors generateIgnoreCore was the last feature loop still swallowing per-target errors (warn + continue, exit 0 'All files are up to date'). Ignore files are what keep secrets out of AI tools' reach, so a silently-skipped ignore generation is the same fail-open bug the permissions feature had; it now logs at error level and rethrows, matching every other feature core, and the pinning unit test flips to fail-the-run. Closes #2551 Co-Authored-By: Claude Fable 5 --- src/lib/generate.test.ts | 10 ++++++---- src/lib/generate.ts | 7 +++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/lib/generate.test.ts b/src/lib/generate.test.ts index 138ca36ef..0b8fe254d 100644 --- a/src/lib/generate.test.ts +++ b/src/lib/generate.test.ts @@ -385,15 +385,17 @@ describe("generate", () => { ); }); - it("should handle errors gracefully and continue", async () => { + it("should fail the run on an ignore error instead of reporting success", async () => { + // Ignore files keep secrets out of AI tools' reach; a swallowed error + // used to exit 0 "up to date" while nothing was written (see #2486 for + // the permissions counterpart). mockConfig.getFeatures.mockReturnValue(["ignore"]); vi.mocked(IgnoreProcessor).mockImplementation(function () { throw new Error("Test error"); }); - const result = await generate({ logger, config: mockConfig as never }); - - expect(result.ignoreCount).toBe(0); + await expect(generate({ logger, config: mockConfig as never })).rejects.toThrow("Test error"); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining("Test error")); }); it("should skip writing when no rulesync files found", async () => { diff --git a/src/lib/generate.ts b/src/lib/generate.ts index 1b6307e1c..d34ac4d29 100644 --- a/src/lib/generate.ts +++ b/src/lib/generate.ts @@ -789,10 +789,13 @@ async function generateIgnoreCore(params: { allPaths.push(...result.paths); if (result.hasDiff) hasDiff = true; } catch (error) { - logger.warn( + // Ignore files are what keep secrets out of AI tools' reach — a + // silently-skipped ignore generation is the same fail-open bug the + // permissions feature had (#2486), so it fails the run the same way. + logger.error( `Failed to generate ${toolTarget} ignore files for ${outputRoot}: ${formatError(error)}`, ); - continue; + throw error; } } } From d7bc731805eca7d3b7d1b6b9f93b796d240182b4 Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Fri, 31 Jul 2026 18:51:06 -0700 Subject: [PATCH 2/3] test: pin fail-the-run behavior at the CLI level for ignore errors The generateCommand-level tests still pinned the swallowed-error behavior; they now assert the command rejects and no success banner is emitted. Co-Authored-By: Claude Fable 5 --- src/cli/commands/generate.test.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/cli/commands/generate.test.ts b/src/cli/commands/generate.test.ts index 83a23ec16..62f03115f 100644 --- a/src/cli/commands/generate.test.ts +++ b/src/cli/commands/generate.test.ts @@ -446,17 +446,17 @@ describe("generateCommand", () => { mockCwd.mockRestore(); }); - it("should handle errors in ignore processing gracefully", async () => { + it("should fail the command on an ignore error instead of reporting success", async () => { vi.mocked(IgnoreProcessor).mockImplementation(function () { throw new Error("Test error"); }); const options: GenerateOptions = {}; - // Should not throw, errors are caught and processing continues - await generateCommand(mockLogger, options); - - // Should still complete without error - expect(mockLogger.info).toHaveBeenCalledWith("✓ All files are up to date (ignore)"); + // A swallowed ignore error used to end with "All files are up to date" + // while nothing was written — the fail-open direction for the feature + // that keeps secrets away from AI tools (see #2551). + await expect(generateCommand(mockLogger, options)).rejects.toThrow("Test error"); + expect(mockLogger.info).not.toHaveBeenCalledWith("✓ All files are up to date (ignore)"); }); it("should skip ignore files when no rulesync files found", async () => { @@ -1225,17 +1225,18 @@ describe("generateCommand", () => { return mockRulesProcessor as any; }); - // Set up ignore processor to throw an error (errors are caught and ignored in lib) + // Set up ignore processor to throw an error — a feature error now + // fails the whole run instead of being silently ignored (#2551), so + // the rules that were written are reported through the thrown error + // path rather than a success banner. vi.mocked(IgnoreProcessor).mockImplementation(function () { throw new Error("Ignore error"); }); const options: GenerateOptions = {}; - await generateCommand(mockLogger, options); - - expect(mockLogger.success).toHaveBeenCalledWith("Written 2 rules"); - expect(mockLogger.success).toHaveBeenCalledWith( + await expect(generateCommand(mockLogger, options)).rejects.toThrow("Ignore error"); + expect(mockLogger.success).not.toHaveBeenCalledWith( "🎉 All done! Written 2 file(s) total (2 rules)", ); }); From 0ae33b8da164d36034ba3a34d126094c8445eae5 Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Fri, 31 Jul 2026 18:52:52 -0700 Subject: [PATCH 3/3] docs(test): correct the mixed-scenario comment - ignore runs first, rules never run Co-Authored-By: Claude Fable 5 --- src/cli/commands/generate.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/commands/generate.test.ts b/src/cli/commands/generate.test.ts index 62f03115f..0a00142c9 100644 --- a/src/cli/commands/generate.test.ts +++ b/src/cli/commands/generate.test.ts @@ -1226,9 +1226,9 @@ describe("generateCommand", () => { }); // Set up ignore processor to throw an error — a feature error now - // fails the whole run instead of being silently ignored (#2551), so - // the rules that were written are reported through the thrown error - // path rather than a success banner. + // fails the whole run instead of being silently ignored (#2551). + // Ignore is the first step in GENERATION_STEP_GRAPH, so the rules step + // never runs and no success banner is emitted. vi.mocked(IgnoreProcessor).mockImplementation(function () { throw new Error("Ignore error"); });