From 065cd470a9bfe9ca4c803b98b756298bde357220 Mon Sep 17 00:00:00 2001 From: Emmanuel N Kyeyune Date: Wed, 13 May 2026 09:55:39 -0400 Subject: [PATCH] feat(cli): exit non-zero when error diagnostics are emitted Closes #98. The build CLI previously always exited with status 0 regardless of diagnostic severity, so pre-commit hooks that hide stdout could silently let broken workflows through. The CLI now sets process.exitCode = 1 when any diagnostic at error or fatal severity (after applying configured rules) is emitted. Users can downgrade specific codes via diagnostics.rules, suppress at call sites via Diagnostics.suppress, or disable the behaviour globally via the new diagnostics.failOnError config flag (defaults to true). --- docs/docs/guides/configuration.md | 16 +- docs/docs/guides/typed-actions.md | 16 ++ .../__mocks__/error-emitting.fixture.ts | 25 +++ .../__mocks__/warning-emitting.fixture.ts | 24 +++ .../src/commands/build.integration.spec.ts | 163 +++++++++++++++++- packages/cli/src/commands/build.ts | 11 +- packages/cli/src/commands/diagnostics.spec.ts | 114 ++++++++++++ packages/cli/src/commands/diagnostics.ts | 17 ++ packages/cli/src/commands/types/build.ts | 16 ++ 9 files changed, 398 insertions(+), 4 deletions(-) create mode 100644 packages/cli/src/commands/__mocks__/error-emitting.fixture.ts create mode 100644 packages/cli/src/commands/__mocks__/warning-emitting.fixture.ts diff --git a/docs/docs/guides/configuration.md b/docs/docs/guides/configuration.md index 230ac1c..5b4f607 100644 --- a/docs/docs/guides/configuration.md +++ b/docs/docs/guides/configuration.md @@ -229,7 +229,21 @@ Configure diagnostic warnings emitted during build when using `@github-actions-w - `"off"` - Suppress the diagnostic entirely - `"warn"` - Emit as a warning (default) -- `"error"` - Upgrade to an error (fails the build) +- `"error"` - Upgrade to an error. Causes the CLI to exit with a non-zero status code at the end of the build (see [`failOnError`](#failonerror) below). + +#### `failOnError` + +```json +{ + "diagnostics": { + "failOnError": true + } +} +``` + +When `true` (the default), the CLI exits with status code `1` after the build if any diagnostic at `error` or `fatal` severity (after applying `rules`) was emitted. When `false`, the CLI always exits `0` regardless of emitted diagnostics — matching pre-2.6.0 behaviour. + +The non-zero exit is useful in pre-commit hooks and CI steps where stdout may be hidden — it ensures broken workflows are caught instead of silently committed. To stop an individual diagnostic from failing the build without disabling the feature globally, downgrade it to `warn` (or `off`) via `rules`, or suppress it at the call site with `suppressWarnings` / `Diagnostics.suppress`. #### Exclude Patterns diff --git a/docs/docs/guides/typed-actions.md b/docs/docs/guides/typed-actions.md index 11a7338..00f4c22 100644 --- a/docs/docs/guides/typed-actions.md +++ b/docs/docs/guides/typed-actions.md @@ -117,6 +117,22 @@ In `wac.config.json`: } ``` +## Build Exit Code + +The CLI exits with status code `1` if any diagnostic at `error` or `fatal` severity was emitted during the build (after applying configured `rules`). Lower-severity diagnostics (`trace`, `debug`, `info`, `warning`) do not affect the exit code. + +This is especially useful inside a pre-commit hook or CI step where stdout may be hidden — a non-zero exit code ensures broken workflows are caught instead of silently committed. To stop a specific code from failing the build, downgrade it to `warn` (or `off`) via the `rules` configuration above, or suppress it at the call site with `suppressWarnings` / `Diagnostics.suppress`. + +To preserve the pre-2.6.0 behaviour where the CLI always exited `0` regardless of emitted diagnostics, set `failOnError` to `false`: + +```json +{ + "diagnostics": { + "failOnError": false + } +} +``` + ## Requesting New Actions If there's an action you'd like to see added, [open an issue](https://github.com/emmanuelnk/github-actions-workflow-ts/issues/new) or see [Adding Actions](/docs/contributing/adding-actions) to contribute it yourself. diff --git a/packages/cli/src/commands/__mocks__/error-emitting.fixture.ts b/packages/cli/src/commands/__mocks__/error-emitting.fixture.ts new file mode 100644 index 0000000..d87c4d3 --- /dev/null +++ b/packages/cli/src/commands/__mocks__/error-emitting.fixture.ts @@ -0,0 +1,25 @@ +import { + Workflow, + NormalJob, + Step, + Context, + Diagnostics, +} from '@github-actions-workflow-ts/lib' + +// Emit an error-severity diagnostic at import time to simulate a wac file +// that detects a problem during workflow construction. +const reporter = Context.getGlobalWacContext()?.diagnostics +reporter?.emit({ + severity: Diagnostics.DiagnosticSeverity.ERROR, + code: 'simulated-error', + message: 'simulated error from a wac file', +}) + +const job = new NormalJob('Test', { 'runs-on': 'ubuntu-latest' }).addSteps([ + new Step({ name: 'Noop', run: 'true' }), +]) + +export const test = new Workflow('error-emitting-mock', { + name: 'ErrorEmittingMock', + on: { workflow_dispatch: {} }, +}).addJob(job) diff --git a/packages/cli/src/commands/__mocks__/warning-emitting.fixture.ts b/packages/cli/src/commands/__mocks__/warning-emitting.fixture.ts new file mode 100644 index 0000000..c986d72 --- /dev/null +++ b/packages/cli/src/commands/__mocks__/warning-emitting.fixture.ts @@ -0,0 +1,24 @@ +import { + Workflow, + NormalJob, + Step, + Context, + Diagnostics, +} from '@github-actions-workflow-ts/lib' + +// Emit only a warning to confirm sub-error diagnostics do not change exit code. +const reporter = Context.getGlobalWacContext()?.diagnostics +reporter?.emit({ + severity: Diagnostics.DiagnosticSeverity.WARN, + code: 'simulated-warning', + message: 'simulated warning from a wac file', +}) + +const job = new NormalJob('Test', { 'runs-on': 'ubuntu-latest' }).addSteps([ + new Step({ name: 'Noop', run: 'true' }), +]) + +export const test = new Workflow('warning-emitting-mock', { + name: 'WarningEmittingMock', + on: { workflow_dispatch: {} }, +}).addJob(job) diff --git a/packages/cli/src/commands/build.integration.spec.ts b/packages/cli/src/commands/build.integration.spec.ts index dfb3275..1dc45c7 100644 --- a/packages/cli/src/commands/build.integration.spec.ts +++ b/packages/cli/src/commands/build.integration.spec.ts @@ -1,11 +1,24 @@ -import { describe, it, expect } from '@jest/globals' +import { + describe, + it, + expect, + afterEach, + beforeEach, + jest, +} from '@jest/globals' +import * as fs from 'fs' +import * as os from 'os' import * as path from 'path' import { fileURLToPath } from 'url' -import { importWorkflowFile } from './build.js' +import { Context } from '@github-actions-workflow-ts/lib' +import { generateWorkflowFiles, importWorkflowFile } from './build.js' +import { ConsoleDiagnosticsReporter } from './diagnostics.js' const __filename = fileURLToPath(import.meta.url) const __dirname = path.dirname(__filename) +const CLI_PACKAGE_ROOT = path.resolve(__dirname, '..', '..') + describe('build integration tests', () => { describe('importWorkflowFile', () => { it('should successfully import a .wac.ts file and return workflow exports', async () => { @@ -23,4 +36,150 @@ describe('build integration tests', () => { expect(result.test.workflow.name).toBe('ExampleMockTests') }) }) + + describe('error-severity diagnostics flip hasErrors on the reporter', () => { + let consoleErrorSpy: jest.SpiedFunction + + beforeEach(() => { + consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => {}) + }) + + afterEach(() => { + consoleErrorSpy.mockRestore() + Context.__internalSetGlobalContext(undefined as never) + }) + + it('should set reporter.hasErrors when a wac file emits an error diagnostic', async () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + Context.__internalSetGlobalContext({ diagnostics: reporter }) + + const mockWacPath = path.join( + __dirname, + '__mocks__', + 'error-emitting.fixture.ts', + ) + await importWorkflowFile(mockWacPath) + + expect(reporter.hasErrors).toBe(true) + }) + + it('should not set reporter.hasErrors when a wac file only emits a warning', async () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + Context.__internalSetGlobalContext({ diagnostics: reporter }) + + const mockWacPath = path.join( + __dirname, + '__mocks__', + 'warning-emitting.fixture.ts', + ) + await importWorkflowFile(mockWacPath) + + expect(reporter.hasErrors).toBe(false) + }) + }) + + describe('generateWorkflowFiles exit-code behaviour', () => { + let consoleSpies: { + log: jest.SpiedFunction + error: jest.SpiedFunction + } + let originalCwd: string + let originalExitCode: typeof process.exitCode + let tmpDir: string + + /** + * Build a temp project containing a single `*.wac.ts` file and (optionally) + * a `wac.config.json`. The wac file imports the library through a relative + * symlink so the dynamic import inside `importWorkflowFile` resolves it. + */ + const setupProject = (opts: { + wacFilename: string + configJson?: Record + }) => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wac-it-')) + + // Symlink the CLI package's node_modules into the temp project so the + // wac file can resolve '@github-actions-workflow-ts/lib' via dynamic + // import. (The workspace's `lib` lives under packages/cli/node_modules + // when pnpm sets up workspace links, not the repo-root node_modules.) + const cliNodeModules = path.join(CLI_PACKAGE_ROOT, 'node_modules') + fs.symlinkSync(cliNodeModules, path.join(tmpDir, 'node_modules'), 'dir') + + const wacSrc = fs.readFileSync( + path.join(__dirname, '__mocks__', opts.wacFilename), + 'utf-8', + ) + fs.writeFileSync(path.join(tmpDir, 'wf.wac.ts'), wacSrc) + + if (opts.configJson) { + fs.writeFileSync( + path.join(tmpDir, 'wac.config.json'), + JSON.stringify(opts.configJson), + ) + } + + // Output directory the build will write into. + fs.mkdirSync(path.join(tmpDir, '.github', 'workflows'), { + recursive: true, + }) + } + + beforeEach(() => { + consoleSpies = { + log: jest.spyOn(console, 'log').mockImplementation(() => {}), + error: jest.spyOn(console, 'error').mockImplementation(() => {}), + } + originalCwd = process.cwd() + originalExitCode = process.exitCode + process.exitCode = undefined + }) + + afterEach(() => { + process.chdir(originalCwd) + process.exitCode = originalExitCode + consoleSpies.log.mockRestore() + consoleSpies.error.mockRestore() + Context.__internalSetGlobalContext(undefined as never) + if (tmpDir) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + it('should set process.exitCode to 1 when a wac file emits an error diagnostic', async () => { + setupProject({ wacFilename: 'error-emitting.fixture.ts' }) + process.chdir(tmpDir) + + await generateWorkflowFiles({}) + + expect(process.exitCode).toBe(1) + expect(consoleSpies.error).toHaveBeenCalledWith( + expect.stringContaining( + 'Build completed with error diagnostics. Exiting with non-zero status code.', + ), + ) + }) + + it('should not set process.exitCode when only warnings are emitted', async () => { + setupProject({ wacFilename: 'warning-emitting.fixture.ts' }) + process.chdir(tmpDir) + + await generateWorkflowFiles({}) + + expect(process.exitCode).toBeUndefined() + }) + + it('should not set process.exitCode when failOnError is disabled in wac.config.json', async () => { + setupProject({ + wacFilename: 'error-emitting.fixture.ts', + configJson: { diagnostics: { failOnError: false } }, + }) + process.chdir(tmpDir) + + await generateWorkflowFiles({}) + + expect(process.exitCode).toBeUndefined() + }) + }) }) diff --git a/packages/cli/src/commands/build.ts b/packages/cli/src/commands/build.ts index 7c610b1..6c69130 100644 --- a/packages/cli/src/commands/build.ts +++ b/packages/cli/src/commands/build.ts @@ -385,8 +385,9 @@ export const generateWorkflowFiles = async ( // Track created directories to avoid duplicate creation attempts const createdDirectories = new Set() + const diagnosticsReporter = new ConsoleDiagnosticsReporter() Context.__internalSetGlobalContext({ - diagnostics: new ConsoleDiagnosticsReporter(), + diagnostics: diagnosticsReporter, diagnosticRules: config.diagnostics?.rules, }) @@ -407,4 +408,12 @@ export const generateWorkflowFiles = async ( console.log( `[github-actions-workflow-ts] Successfully generated ${workflowCount} workflow file(s)`, ) + + const failOnError = config.diagnostics?.failOnError ?? true + if (diagnosticsReporter.hasErrors && failOnError) { + console.error( + '[github-actions-workflow-ts] Build completed with error diagnostics. Exiting with non-zero status code.', + ) + process.exitCode = 1 + } } diff --git a/packages/cli/src/commands/diagnostics.spec.ts b/packages/cli/src/commands/diagnostics.spec.ts index 650e491..984e402 100644 --- a/packages/cli/src/commands/diagnostics.spec.ts +++ b/packages/cli/src/commands/diagnostics.spec.ts @@ -389,4 +389,118 @@ describe('ConsoleDiagnosticsReporter', () => { ) }) }) + + describe('hasErrors', () => { + afterEach(() => { + Context.__internalSetGlobalContext(undefined as never) + }) + + it('should default to false', () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + expect(reporter.hasErrors).toBe(false) + }) + + it.each([ + Diagnostics.DiagnosticSeverity.TRACE, + Diagnostics.DiagnosticSeverity.DEBUG, + Diagnostics.DiagnosticSeverity.INFO, + Diagnostics.DiagnosticSeverity.WARN, + ])('should stay false after emitting "%s" severity', (severity) => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + + reporter.emit({ + severity, + code: 'test-code', + message: 'sub-error diagnostic', + }) + + expect(reporter.hasErrors).toBe(false) + }) + + it.each([ + Diagnostics.DiagnosticSeverity.ERROR, + Diagnostics.DiagnosticSeverity.FATAL, + ])('should flip to true after emitting "%s" severity', (severity) => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + + reporter.emit({ + severity, + code: 'test-code', + message: 'error-level diagnostic', + }) + + expect(reporter.hasErrors).toBe(true) + }) + + it('should stay false when an error is downgraded to warn via rules', () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + Context.__internalSetGlobalContext({ + diagnostics: reporter, + diagnosticRules: { + 'test-code': 'warn', + }, + }) + + reporter.emit({ + severity: Diagnostics.DiagnosticSeverity.ERROR, + code: 'test-code', + message: 'downgraded to warn', + }) + + expect(reporter.hasErrors).toBe(false) + }) + + it('should flip to true when a warning is upgraded to error via rules', () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + Context.__internalSetGlobalContext({ + diagnostics: reporter, + diagnosticRules: { + 'test-code': 'error', + }, + }) + + reporter.emit({ + severity: Diagnostics.DiagnosticSeverity.WARN, + code: 'test-code', + message: 'upgraded to error', + }) + + expect(reporter.hasErrors).toBe(true) + }) + + it('should stay false when an error is suppressed via rules', () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + Context.__internalSetGlobalContext({ + diagnostics: reporter, + diagnosticRules: { + 'test-code': 'off', + }, + }) + + reporter.emit({ + severity: Diagnostics.DiagnosticSeverity.ERROR, + code: 'test-code', + message: 'suppressed', + }) + + expect(reporter.hasErrors).toBe(false) + }) + + it('should remain true once set, even after subsequent non-error diagnostics', () => { + const reporter = new ConsoleDiagnosticsReporter({ color: false }) + + reporter.emit({ + severity: Diagnostics.DiagnosticSeverity.ERROR, + code: 'first', + message: 'first error', + }) + reporter.emit({ + severity: Diagnostics.DiagnosticSeverity.WARN, + code: 'second', + message: 'later warning', + }) + + expect(reporter.hasErrors).toBe(true) + }) + }) }) diff --git a/packages/cli/src/commands/diagnostics.ts b/packages/cli/src/commands/diagnostics.ts index 5ecc75c..8bc77de 100644 --- a/packages/cli/src/commands/diagnostics.ts +++ b/packages/cli/src/commands/diagnostics.ts @@ -28,8 +28,18 @@ type ConsoleDiagnosticsReporterOptions = { export class ConsoleDiagnosticsReporter implements Diagnostics.DiagnosticsReporter { + private _hasErrors = false + constructor(private options: ConsoleDiagnosticsReporterOptions = {}) {} + /** + * Whether any diagnostic at `error` or `fatal` severity (after applying + * configured rules) has been emitted through this reporter. + */ + get hasErrors(): boolean { + return this._hasErrors + } + emit(d: Diagnostics.Diagnostic): void { // Get diagnostic rules from context const rules = Context.getGlobalWacContext()?.diagnosticRules @@ -42,6 +52,13 @@ export class ConsoleDiagnosticsReporter return } + if ( + effectiveSeverity === Diagnostics.DiagnosticSeverity.ERROR || + effectiveSeverity === Diagnostics.DiagnosticSeverity.FATAL + ) { + this._hasErrors = true + } + // Use the effective severity (may have been upgraded/downgraded) const severity = this.formatSeverity(effectiveSeverity) diff --git a/packages/cli/src/commands/types/build.ts b/packages/cli/src/commands/types/build.ts index af1fbbc..e37af46 100644 --- a/packages/cli/src/commands/types/build.ts +++ b/packages/cli/src/commands/types/build.ts @@ -28,6 +28,22 @@ export type DiagnosticsConfig = { * Keys are diagnostic codes like 'action-version-unverifiable' or 'action-version-semver-violation'. */ rules?: Record + /** + * Whether the CLI should exit with a non-zero status code when any diagnostic + * at `error` or `fatal` severity (after applying `rules`) is emitted during a build. + * + * This is useful when the CLI runs in a pre-commit hook or CI step where + * stdout may be hidden — a non-zero exit code ensures broken workflows are + * caught instead of silently committed. + * + * Set to `false` to preserve pre-2.6.0 behaviour where the CLI always exited + * with status 0 regardless of emitted diagnostics. Codes can also be + * downgraded individually via `rules`, or suppressed at call sites via + * `Diagnostics.suppress`. + * + * @default true + */ + failOnError?: boolean } /**