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 } /**