From 48fde52e7c73c05fea5ac9196419302820d8126b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 21 Jul 2026 03:28:26 +0100 Subject: [PATCH 1/5] feat(cli): uniform JSON envelope with structured errors and changed reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap the --json / --output of the orchestration commands (release, gate, standing-pr) in one uniform envelope so humans, CI, and agents share a single output contract: { schemaVersion, status, changed, data, warnings, errors } - data carries each command's payload verbatim (VersionOutput etc.) — the envelope wraps, never replaces, so the manifest-compat invariant holds. - structured error codes replace prose-only failures in JSON mode, each with a category and a conservative retryable flag; the process exit code maps to the error code (config -> 2, llm -> 5, ...). - changed distinguishes real work from a no-op: dry runs, skip-if-published, and the read-only gate all report changed: false. - stream discipline: only the envelope is written to stdout, all diagnostics go to stderr, so parsing stdout as JSON is always safe. - the Action unwraps the envelope in parseReleaseOutput, so every existing Action output is derived unchanged. First PR for the #544 epic. Deliberately scoped to the orchestration commands; follow-ups extend the envelope to the version|notes|publish pipe commands (with input unwrapping to preserve the pipe) and expose dry-run as a structured plan. Claude-Session: https://claude.ai/code/session_01K6CmYRXTZQYs4MNo6AETxT Co-authored-by: Claude Opus 4.8 (1M context) --- docs/cli.md | 38 ++++++ packages/core/src/envelope.ts | 114 ++++++++++++++++++ packages/core/src/index.ts | 11 ++ packages/core/test/unit/envelope.spec.ts | 101 ++++++++++++++++ packages/release/src/commands/emitResult.ts | 29 ++++- packages/release/src/commands/gate-command.ts | 10 +- .../release/src/commands/release-command.ts | 12 +- .../src/commands/standing-pr-command.ts | 25 ++-- .../test/unit/commands/emitResult.spec.ts | 58 +++++++-- .../release/test/unit/release-command.spec.ts | 11 +- scripts/run-action.d.mts | 2 +- scripts/run-action.mjs | 10 +- test/integration/action-runner.spec.ts | 24 ++++ 13 files changed, 407 insertions(+), 38 deletions(-) create mode 100644 packages/core/src/envelope.ts create mode 100644 packages/core/test/unit/envelope.spec.ts diff --git a/docs/cli.md b/docs/cli.md index d1f81f13..4c3047b1 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -18,6 +18,44 @@ The `releasekit` dispatcher re-exports `version`, `notes`, and `publish`, so `re --- +## JSON output contract + +Every command that accepts `-j, --json` emits its result as a single **envelope** — one uniform shape shared by humans, CI, and agents. `--output ` writes the same envelope to a file instead of stdout (the reliable channel for the GitHub Action, since stdout can be polluted by subprocess or log noise, and a single stray byte breaks JSON parsing). + +```jsonc +{ + "schemaVersion": 1, // envelope contract version; stable across minor releases + "status": "success", // "success" | "error" + "changed": true, // did the command change state, or was everything already as desired? + "data": { /* … */ }, // command-specific payload (VersionOutput, gate result, …); null on error + "warnings": [{ "code": "…", "message": "…" }], + "errors": [{ "code": "…", "category": "…", "retryable": false, "message": "…" }] +} +``` + +- **`data`** carries the command's payload verbatim — the envelope wraps it, never replaces it. `releasekit release --json` still exposes its `VersionOutput` at `data.versionOutput`. +- **`changed`** separates real work from a no-op: a dry run is never `changed`; `standing-pr publish` reports `changed: false` when the release was already published (skip-if-published); `gate` is read-only and always `changed: false`. +- **`errors[]`** replaces prose-only failures in JSON mode. Each carries a stable machine `code`, a coarse `category`, a `retryable` flag (only `true` for known-transient failures — a timeout, 429, or 5xx from a provider — so an agent never retries an unknown failure), and a human `message`. +- **Stream discipline:** the envelope is the only thing on stdout; all diagnostics (progress, warnings, error text) go to stderr. Parsing stdout as JSON is always safe, and no command prompts interactively on a CI path. + +### Error codes and exit codes + +| `code` | `category` | Exit code | +|---|---|---| +| `GENERAL_ERROR` | `general` | 1 | +| `CONFIG_ERROR` | `config` | 2 | +| `INPUT_ERROR` / `INPUT_PARSE_ERROR` | `input` / `input-parse` | 3 | +| `TEMPLATE_ERROR` | `template` | 4 | +| `LLM_ERROR` | `llm` | 5 | +| `GITHUB_ERROR` | `github` | 6 | +| `GIT_ERROR` | `git` | 7 | +| `VERSION_ERROR` | `version` | 8 | +| `PUBLISH_ERROR` | `publish` | 9 | + +`schemaVersion` bumps only on a breaking change to the envelope shape and is stable across minor releases, so agents and CI can pin against it. + +--- + ## `releasekit preview` Post a release preview comment on the current pull request. This is the default command, so `releasekit` with no arguments runs `preview`. diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts new file mode 100644 index 00000000..2e1cb23c --- /dev/null +++ b/packages/core/src/envelope.ts @@ -0,0 +1,114 @@ +import { EXIT_CODES, ReleaseKitError } from './errors.js'; + +/** + * Version of the `--json` envelope contract. Bumped only on a breaking change to the envelope + * shape, and stable across minor releases, so agents and CI can pin against it. + */ +export const ENVELOPE_SCHEMA_VERSION = 1; + +export type EnvelopeStatus = 'success' | 'error'; + +export interface EnvelopeError { + /** Stable machine code mirroring `ReleaseKitError.code` (e.g. CONFIG_ERROR, LLM_ERROR). */ + code: string; + /** Coarse grouping derived from the code (e.g. config, llm, github). */ + category: string; + /** Whether the failure is transient and worth retrying. Only true when known-transient. */ + retryable: boolean; + message: string; +} + +export interface EnvelopeWarning { + code?: string; + message: string; +} + +/** + * The uniform result envelope every command emits in `--json` mode. `data` carries the + * command-specific payload (VersionOutput, gate result, standing-PR result) verbatim — the + * envelope wraps it, never replaces it, so the manifest-compat invariant holds. + */ +export interface Envelope { + schemaVersion: number; + status: EnvelopeStatus; + /** Whether the command changed state, vs. found everything already in the desired state. */ + changed: boolean; + data: T; + warnings: EnvelopeWarning[]; + errors: EnvelopeError[]; +} + +export function successEnvelope( + data: T, + opts: { changed?: boolean; warnings?: EnvelopeWarning[] } = {}, +): Envelope { + return { + schemaVersion: ENVELOPE_SCHEMA_VERSION, + status: 'success', + changed: opts.changed ?? false, + data, + warnings: opts.warnings ?? [], + errors: [], + }; +} + +export function errorEnvelope(errors: EnvelopeError[], opts: { warnings?: EnvelopeWarning[] } = {}): Envelope { + return { + schemaVersion: ENVELOPE_SCHEMA_VERSION, + status: 'error', + changed: false, + data: null, + warnings: opts.warnings ?? [], + errors, + }; +} + +const CODE_TO_EXIT: Record = { + CONFIG_ERROR: EXIT_CODES.CONFIG_ERROR, + INPUT_ERROR: EXIT_CODES.INPUT_ERROR, + INPUT_PARSE_ERROR: EXIT_CODES.INPUT_ERROR, + TEMPLATE_ERROR: EXIT_CODES.TEMPLATE_ERROR, + LLM_ERROR: EXIT_CODES.LLM_ERROR, + GITHUB_ERROR: EXIT_CODES.GITHUB_ERROR, + GIT_ERROR: EXIT_CODES.GIT_ERROR, + VERSION_ERROR: EXIT_CODES.VERSION_ERROR, + PUBLISH_ERROR: EXIT_CODES.PUBLISH_ERROR, +}; + +/** Map any thrown value to its process exit code, defaulting to GENERAL_ERROR. */ +export function exitCodeForError(error: unknown): number { + if (ReleaseKitError.isReleaseKitError(error)) { + return CODE_TO_EXIT[error.code] ?? EXIT_CODES.GENERAL_ERROR; + } + return EXIT_CODES.GENERAL_ERROR; +} + +/** Derive a coarse category from a code: CONFIG_ERROR -> config, INPUT_PARSE_ERROR -> input-parse. */ +function categoryForCode(code: string): string { + return ( + code + .replace(/_ERROR$/, '') + .replace(/_/g, '-') + .toLowerCase() || 'general' + ); +} + +/** Convert a thrown value into a structured envelope error. */ +export function toEnvelopeError(error: unknown): EnvelopeError { + if (ReleaseKitError.isReleaseKitError(error)) { + return { + code: error.code, + category: categoryForCode(error.code), + // Only ReleaseKitError subclasses that classify transience (LLMError) carry this; a missing or + // undefined flag stays conservatively non-retryable so an agent never retries an unknown failure. + retryable: (error as { retryable?: boolean }).retryable === true, + message: error.message, + }; + } + return { + code: 'GENERAL_ERROR', + category: 'general', + retryable: false, + message: error instanceof Error ? error.message : String(error), + }; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ca4d239f..1e972759 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -13,6 +13,17 @@ export { type GraphPackage, type WorkspaceDependencyGraph, } from './dependencyGraph.js'; +export { + ENVELOPE_SCHEMA_VERSION, + type Envelope, + type EnvelopeError, + type EnvelopeStatus, + type EnvelopeWarning, + errorEnvelope, + exitCodeForError, + successEnvelope, + toEnvelopeError, +} from './envelope.js'; export { EXIT_CODES, type ExitCode, ReleaseKitError } from './errors.js'; export { debug, diff --git a/packages/core/test/unit/envelope.spec.ts b/packages/core/test/unit/envelope.spec.ts new file mode 100644 index 00000000..bed046cc --- /dev/null +++ b/packages/core/test/unit/envelope.spec.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest'; +import { + ENVELOPE_SCHEMA_VERSION, + errorEnvelope, + exitCodeForError, + successEnvelope, + toEnvelopeError, +} from '../../src/envelope.js'; +import { EXIT_CODES, ReleaseKitError } from '../../src/errors.js'; + +class TestConfigError extends ReleaseKitError { + readonly code = 'CONFIG_ERROR'; + readonly suggestions: string[] = []; +} + +class TestLLMError extends ReleaseKitError { + readonly code = 'LLM_ERROR'; + readonly suggestions: string[] = []; + readonly retryable: boolean; + constructor(message: string, retryable: boolean) { + super(message); + this.retryable = retryable; + } +} + +describe('envelope', () => { + describe('successEnvelope', () => { + it('should wrap data with success status and defaults', () => { + expect(successEnvelope({ tags: ['v1.2.3'] })).toEqual({ + schemaVersion: ENVELOPE_SCHEMA_VERSION, + status: 'success', + changed: false, + data: { tags: ['v1.2.3'] }, + warnings: [], + errors: [], + }); + }); + + it('should carry changed and warnings when provided', () => { + const env = successEnvelope(null, { changed: true, warnings: [{ message: 'heads up' }] }); + expect(env.changed).toBe(true); + expect(env.warnings).toEqual([{ message: 'heads up' }]); + }); + }); + + describe('errorEnvelope', () => { + it('should carry error status, null data, and the given errors', () => { + const errors = [{ code: 'X_ERROR', category: 'x', retryable: false, message: 'boom' }]; + expect(errorEnvelope(errors)).toEqual({ + schemaVersion: ENVELOPE_SCHEMA_VERSION, + status: 'error', + changed: false, + data: null, + warnings: [], + errors, + }); + }); + }); + + describe('toEnvelopeError', () => { + it('should map a ReleaseKitError to its code and derived category', () => { + expect(toEnvelopeError(new TestConfigError('bad config'))).toEqual({ + code: 'CONFIG_ERROR', + category: 'config', + retryable: false, + message: 'bad config', + }); + }); + + it('should surface a retryable ReleaseKitError as retryable', () => { + expect(toEnvelopeError(new TestLLMError('timeout', true)).retryable).toBe(true); + }); + + it('should treat an unclassified error as non-retryable', () => { + expect(toEnvelopeError(new TestLLMError('auth', false)).retryable).toBe(false); + }); + + it('should map a plain Error to GENERAL_ERROR', () => { + expect(toEnvelopeError(new Error('oops'))).toEqual({ + code: 'GENERAL_ERROR', + category: 'general', + retryable: false, + message: 'oops', + }); + }); + + it('should stringify a non-Error thrown value', () => { + expect(toEnvelopeError('just a string').message).toBe('just a string'); + }); + }); + + describe('exitCodeForError', () => { + it('should map a ReleaseKitError code to its exit code', () => { + expect(exitCodeForError(new TestConfigError('x'))).toBe(EXIT_CODES.CONFIG_ERROR); + }); + + it('should default to GENERAL_ERROR for a plain Error', () => { + expect(exitCodeForError(new Error('x'))).toBe(EXIT_CODES.GENERAL_ERROR); + }); + }); +}); diff --git a/packages/release/src/commands/emitResult.ts b/packages/release/src/commands/emitResult.ts index e077686f..1e1678e8 100644 --- a/packages/release/src/commands/emitResult.ts +++ b/packages/release/src/commands/emitResult.ts @@ -1,16 +1,35 @@ import { writeFileSync } from 'node:fs'; +import { type Envelope, type EnvelopeWarning, errorEnvelope, successEnvelope, toEnvelopeError } from '@releasekit/core'; /** - * Emit a command's JSON result. When `--output` is given, write it to that file — the reliable + * Write an envelope to the JSON channel. When `--output` is given, write to that file — the reliable * channel the GitHub Action reads, since stdout can be polluted by subprocess or log noise and a - * single stray byte breaks JSON parsing. Otherwise, when `--json` is set, print to stdout as before. + * single stray byte breaks JSON parsing. Otherwise, when `--json` is set, print to stdout. Only the + * envelope goes to stdout; all diagnostics go to stderr via the logger (stream discipline). */ -export function emitResult(result: unknown, opts: { json?: boolean; output?: string }): void { - if (result === undefined || result === null) return; - const text = JSON.stringify(result, null, 2); +function writeEnvelope(envelope: Envelope, opts: { json?: boolean; output?: string }): void { + const text = JSON.stringify(envelope, null, 2); if (opts.output) { writeFileSync(opts.output, text); } else if (opts.json) { console.log(text); } } + +/** + * Emit a command's successful result inside the uniform CLI envelope. `data` becomes the envelope's + * `data` payload verbatim (VersionOutput etc. are preserved — the envelope wraps, never replaces). + */ +export function emitResult( + data: unknown, + opts: { json?: boolean; output?: string; changed?: boolean; warnings?: EnvelopeWarning[] }, +): void { + if (!opts.json && !opts.output) return; + writeEnvelope(successEnvelope(data ?? null, { changed: opts.changed, warnings: opts.warnings }), opts); +} + +/** Emit a structured error envelope for a thrown value on the JSON channel. */ +export function emitError(error: unknown, opts: { json?: boolean; output?: string }): void { + if (!opts.json && !opts.output) return; + writeEnvelope(errorEnvelope([toEnvelopeError(error)]), opts); +} diff --git a/packages/release/src/commands/gate-command.ts b/packages/release/src/commands/gate-command.ts index 4dab2bd0..28035a05 100644 --- a/packages/release/src/commands/gate-command.ts +++ b/packages/release/src/commands/gate-command.ts @@ -1,7 +1,7 @@ -import { setJsonMode, setLogLevel, setQuietMode } from '@releasekit/core'; +import { exitCodeForError, setJsonMode, setLogLevel, setQuietMode } from '@releasekit/core'; import { Command } from 'commander'; import { runGate } from '../gate/gate.js'; -import { emitResult } from './emitResult.js'; +import { emitError, emitResult } from './emitResult.js'; export function createGateCommand(): Command { return new Command('gate') @@ -28,12 +28,14 @@ export function createGateCommand(): Command { quiet: opts.quiet, }); - emitResult(result, { json: opts.json, output: opts.output }); + // gate is a read-only check — it never mutates state, so changed is always false. + emitResult(result, { json: opts.json, output: opts.output, changed: false }); process.exit(0); } catch (error) { + emitError(error, { json: opts.json, output: opts.output }); console.error(error instanceof Error ? error.message : String(error)); - process.exit(1); + process.exit(exitCodeForError(error)); } }); } diff --git a/packages/release/src/commands/release-command.ts b/packages/release/src/commands/release-command.ts index a805419c..efc119a8 100644 --- a/packages/release/src/commands/release-command.ts +++ b/packages/release/src/commands/release-command.ts @@ -1,9 +1,9 @@ -import { EXIT_CODES } from '@releasekit/core'; +import { EXIT_CODES, exitCodeForError } from '@releasekit/core'; import { Command, Option } from 'commander'; import { publishFromDraft, runReleaseDraft } from '../draft/draft.js'; import { runRelease } from '../release.js'; import type { ReleaseOptions } from '../types.js'; -import { emitResult } from './emitResult.js'; +import { emitError, emitResult } from './emitResult.js'; export function createReleaseCommand(): Command { return new Command('release') @@ -99,14 +99,18 @@ export function createReleaseCommand(): Command { ? await runReleaseDraft(options) : await runRelease(options); - emitResult(result, { json: options.json, output: opts.output }); + // A dry run plans but never mutates state, so it is never "changed"; a real run changed + // state iff it produced version updates. + const changed = !opts.dryRun && Boolean(result?.versionOutput?.updates?.length); + emitResult(result, { json: options.json, output: opts.output, changed }); if (!result) { process.exit(0); } } catch (error) { + emitError(error, { json: opts.json, output: opts.output }); console.error(error instanceof Error ? error.message : String(error)); - process.exit(EXIT_CODES.GENERAL_ERROR); + process.exit(exitCodeForError(error)); } }); } diff --git a/packages/release/src/commands/standing-pr-command.ts b/packages/release/src/commands/standing-pr-command.ts index 865b58e9..2046ffa5 100644 --- a/packages/release/src/commands/standing-pr-command.ts +++ b/packages/release/src/commands/standing-pr-command.ts @@ -1,8 +1,8 @@ -import { EXIT_CODES } from '@releasekit/core'; +import { EXIT_CODES, exitCodeForError } from '@releasekit/core'; import { Command } from 'commander'; import type { StandingPROptions } from '../standing-pr/standing-pr.js'; import { runStandingPRMerge, runStandingPRPublish, runStandingPRUpdate } from '../standing-pr/standing-pr.js'; -import { emitResult } from './emitResult.js'; +import { emitError, emitResult } from './emitResult.js'; export function createStandingPRCommand(): Command { const cmd = new Command('standing-pr').description( @@ -49,10 +49,11 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRUpdate(options); - emitResult(result, { json: opts.json, output: opts.output }); + emitResult(result, { json: opts.json, output: opts.output, changed: result.action !== 'noop' }); } catch (err) { + emitError(err, { json: opts.json, output: opts.output }); console.error(err instanceof Error ? err.message : String(err)); - process.exit(EXIT_CODES.GENERAL_ERROR); + process.exit(exitCodeForError(err)); } }); @@ -88,10 +89,16 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRPublish(options, prNumber); - emitResult(result, { json: opts.json, output: opts.output }); + // skip-if-published returns no updates — surface that as changed:false. + emitResult(result, { + json: opts.json, + output: opts.output, + changed: Boolean(result?.versionOutput?.updates?.length), + }); } catch (err) { + emitError(err, { json: opts.json, output: opts.output }); console.error(err instanceof Error ? err.message : String(err)); - process.exit(EXIT_CODES.GENERAL_ERROR); + process.exit(exitCodeForError(err)); } }); @@ -112,10 +119,12 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRMerge(options, { publish: opts.publish }); - emitResult(result, { json: opts.json, output: opts.output }); + // A non-null result means the PR was merged (nothing-to-merge returns null). + emitResult(result, { json: opts.json, output: opts.output, changed: result != null }); } catch (err) { + emitError(err, { json: opts.json, output: opts.output }); console.error(err instanceof Error ? err.message : String(err)); - process.exit(EXIT_CODES.GENERAL_ERROR); + process.exit(exitCodeForError(err)); } }); diff --git a/packages/release/test/unit/commands/emitResult.spec.ts b/packages/release/test/unit/commands/emitResult.spec.ts index 7816ca54..67a6c641 100644 --- a/packages/release/test/unit/commands/emitResult.spec.ts +++ b/packages/release/test/unit/commands/emitResult.spec.ts @@ -1,8 +1,9 @@ import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { ENVELOPE_SCHEMA_VERSION } from '@releasekit/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { emitResult } from '../../../src/commands/emitResult.js'; +import { emitError, emitResult } from '../../../src/commands/emitResult.js'; describe('emitResult', () => { const dirs: string[] = []; @@ -13,24 +14,44 @@ describe('emitResult', () => { vi.restoreAllMocks(); }); - it('should write the JSON result to the output file when --output is set', () => { + it('should write a success envelope wrapping the result to the output file', () => { const dir = mkdtempSync(join(tmpdir(), 'emit-result-')); dirs.push(dir); const file = join(dir, 'out.json'); const log = vi.spyOn(console, 'log').mockImplementation(() => {}); - emitResult({ versionOutput: { tags: ['v1.2.3'] } }, { json: true, output: file }); + emitResult({ versionOutput: { tags: ['v1.2.3'] } }, { json: true, output: file, changed: true }); - expect(JSON.parse(readFileSync(file, 'utf8'))).toEqual({ versionOutput: { tags: ['v1.2.3'] } }); + expect(JSON.parse(readFileSync(file, 'utf8'))).toEqual({ + schemaVersion: ENVELOPE_SCHEMA_VERSION, + status: 'success', + changed: true, + data: { versionOutput: { tags: ['v1.2.3'] } }, + warnings: [], + errors: [], + }); expect(log).not.toHaveBeenCalled(); }); - it('should print to stdout when --json is set and no --output', () => { + it('should print a success envelope to stdout when --json is set and no --output', () => { const log = vi.spyOn(console, 'log').mockImplementation(() => {}); emitResult({ a: 1 }, { json: true }); - expect(log).toHaveBeenCalledWith(JSON.stringify({ a: 1 }, null, 2)); + const printed = JSON.parse(log.mock.calls[0][0] as string); + expect(printed.status).toBe('success'); + expect(printed.data).toEqual({ a: 1 }); + expect(printed.changed).toBe(false); + }); + + it('should wrap a null result as an envelope with null data', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + emitResult(null, { json: true }); + + const printed = JSON.parse(log.mock.calls[0][0] as string); + expect(printed.status).toBe('success'); + expect(printed.data).toBeNull(); }); it('should do nothing when neither --json nor --output is set', () => { @@ -38,11 +59,30 @@ describe('emitResult', () => { emitResult({ a: 1 }, {}); expect(log).not.toHaveBeenCalled(); }); +}); + +describe('emitError', () => { + afterEach(() => vi.restoreAllMocks()); - it('should do nothing for a null or undefined result', () => { + it('should print an error envelope with a structured error to stdout', () => { const log = vi.spyOn(console, 'log').mockImplementation(() => {}); - emitResult(undefined, { json: true }); - emitResult(null, { json: true }); + + emitError(new Error('boom'), { json: true }); + + const printed = JSON.parse(log.mock.calls[0][0] as string); + expect(printed.status).toBe('error'); + expect(printed.data).toBeNull(); + expect(printed.errors[0]).toEqual({ + code: 'GENERAL_ERROR', + category: 'general', + retryable: false, + message: 'boom', + }); + }); + + it('should do nothing when neither --json nor --output is set', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + emitError(new Error('boom'), {}); expect(log).not.toHaveBeenCalled(); }); }); diff --git a/packages/release/test/unit/release-command.spec.ts b/packages/release/test/unit/release-command.spec.ts index 52a61d22..270ad8c9 100644 --- a/packages/release/test/unit/release-command.spec.ts +++ b/packages/release/test/unit/release-command.spec.ts @@ -1,9 +1,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -vi.mock('@releasekit/core'); vi.mock('../../src/release.js'); -import { EXIT_CODES } from '@releasekit/core'; +import { EXIT_CODES, successEnvelope } from '@releasekit/core'; import { createReleaseCommand } from '../../src/commands/release-command.js'; import { runRelease } from '../../src/release.js'; import type { ReleaseOptions, ReleaseOutput } from '../../src/types.js'; @@ -127,10 +126,10 @@ describe('createReleaseCommand', () => { }); describe('JSON output', () => { - it('should print result as JSON when --json is passed and runRelease returns output', async () => { + it('should print result wrapped in a success envelope when --json is passed', async () => { const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); await createReleaseCommand().parseAsync(['node', 'test', '--json']); - expect(consoleSpy).toHaveBeenCalledWith(JSON.stringify(mockOutput, null, 2)); + expect(consoleSpy).toHaveBeenCalledWith(JSON.stringify(successEnvelope(mockOutput, { changed: false }), null, 2)); consoleSpy.mockRestore(); }); @@ -141,11 +140,11 @@ describe('createReleaseCommand', () => { consoleSpy.mockRestore(); }); - it('should not print JSON when runRelease returns null', async () => { + it('should print an envelope with null data when runRelease returns null', async () => { vi.mocked(runRelease).mockResolvedValue(null); const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); await createReleaseCommand().parseAsync(['node', 'test', '--json']); - expect(consoleSpy).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(JSON.stringify(successEnvelope(null, { changed: false }), null, 2)); consoleSpy.mockRestore(); }); }); diff --git a/scripts/run-action.d.mts b/scripts/run-action.d.mts index db824522..096f1bb2 100644 --- a/scripts/run-action.d.mts +++ b/scripts/run-action.d.mts @@ -36,7 +36,7 @@ export function buildPreviewArgs(input: ActionInputs): string[]; export function buildRefreshAfterReleaseArgs(input: ActionInputs): string[]; export function parseReleaseOutput( stdout: string, -): { versionOutput?: { tags?: string[]; updates?: unknown[] } } | undefined; +): { versionOutput?: { tags?: string[]; updates?: unknown[] } } | null | undefined; export function resolveReleaseTags(args: { parsedTags?: string[]; gitTags?: string[]; allowRecovery?: boolean }): { tags: string[]; recovered: boolean; diff --git a/scripts/run-action.mjs b/scripts/run-action.mjs index 621daddf..227a295a 100644 --- a/scripts/run-action.mjs +++ b/scripts/run-action.mjs @@ -162,14 +162,22 @@ export function writeGateOutputs(stdout, verbose = false) { export function parseReleaseOutput(stdout, verbose = false) { const trimmed = stdout.trim(); if (!trimmed) return undefined; + let parsed; try { - return JSON.parse(trimmed); + parsed = JSON.parse(trimmed); } catch (err) { if (verbose) { console.error(`[run-action] Failed to parse JSON output: ${err.message}`); } return undefined; } + // The CLI wraps every result in the uniform envelope { schemaVersion, status, data, ... }; unwrap + // to the data payload so output derivation reads result fields (versionOutput, shouldRelease) + // directly. Legacy un-enveloped output is returned as-is for backward compatibility. + if (parsed && typeof parsed === 'object' && 'schemaVersion' in parsed && 'data' in parsed) { + return parsed.data; + } + return parsed; } // Sync-mode version tags on HEAD, straight from git. The release creates them at HEAD, so they diff --git a/test/integration/action-runner.spec.ts b/test/integration/action-runner.spec.ts index 19f4a459..308a4d92 100644 --- a/test/integration/action-runner.spec.ts +++ b/test/integration/action-runner.spec.ts @@ -256,6 +256,30 @@ describe('action runner', () => { expect(parsed?.versionOutput?.tags).toEqual(['v1.0.0']); }); + it('should unwrap the data payload from an enveloped success result', () => { + const enveloped = JSON.stringify({ + schemaVersion: 1, + status: 'success', + changed: true, + data: { versionOutput: { updates: [{ packageName: 'a' }], tags: ['v1.0.0'] } }, + warnings: [], + errors: [], + }); + expect(parseReleaseOutput(enveloped)?.versionOutput?.tags).toEqual(['v1.0.0']); + }); + + it('should unwrap null data from an enveloped error result', () => { + const enveloped = JSON.stringify({ + schemaVersion: 1, + status: 'error', + changed: false, + data: null, + warnings: [], + errors: [{ code: 'GENERAL_ERROR', category: 'general', retryable: false, message: 'boom' }], + }); + expect(parseReleaseOutput(enveloped)).toBeNull(); + }); + it('should return undefined for non-JSON release output', () => { expect(parseReleaseOutput('plain logs')).toBeUndefined(); }); From 9e503e92dcd6042a5b262d02bb41f8f033e55b84 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 21 Jul 2026 03:55:40 +0100 Subject: [PATCH 2/5] fix(test): unwrap the CLI envelope in e2e and package-test output consumers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The uniform --json envelope nests each result under `data`, so the e2e helper run_cli_json and the package-test release-output validator — both of which read `.versionOutput` at the top level — saw it move and failed. Unwrap the envelope to its data payload in both (tolerating non-enveloped/legacy output), so the existing `.versionOutput.*` reads keep working. Claude-Session: https://claude.ai/code/session_01K6CmYRXTZQYs4MNo6AETxT Co-authored-by: Claude Opus 4.8 (1M context) --- scripts/test-packages.ts | 7 +++++-- test/e2e/lib/helpers.sh | 10 ++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/scripts/test-packages.ts b/scripts/test-packages.ts index eb75e736..48c17974 100644 --- a/scripts/test-packages.ts +++ b/scripts/test-packages.ts @@ -215,9 +215,12 @@ function testReleaseDryRun(dir: string): void { 'Running releasekit release --dry-run --json', ); - // Validate JSON output + // Validate JSON output. Unwrap the uniform CLI envelope ({ schemaVersion, status, data, … }) to its + // data payload; tolerate legacy un-enveloped output. try { - const result = JSON.parse(output.trim()); + const parsed = JSON.parse(output.trim()); + const result = + parsed && typeof parsed === 'object' && 'schemaVersion' in parsed && 'data' in parsed ? parsed.data : parsed; if (!result.versionOutput) { throw new Error('Missing versionOutput in release output'); } diff --git a/test/e2e/lib/helpers.sh b/test/e2e/lib/helpers.sh index 78f7a1a0..159954f8 100755 --- a/test/e2e/lib/helpers.sh +++ b/test/e2e/lib/helpers.sh @@ -88,8 +88,14 @@ run_cli_json() { cat "$stderrfile" >&2 fi - # Output the JSON (stdout only) - cat "$tmpfile" + # Output the JSON (stdout only). Unwrap the uniform CLI envelope ({ schemaVersion, status, data, … }) + # to its data payload so callers read result fields (.versionOutput, …) directly. Output that isn't + # an envelope — non-JSON, or a command that isn't enveloped yet — passes through unchanged. + if jq -e 'type == "object" and has("schemaVersion") and has("data")' "$tmpfile" >/dev/null 2>&1; then + jq '.data' "$tmpfile" + else + cat "$tmpfile" + fi rm -f "$tmpfile" "$stderrfile" return $exit_code From b80ae2258af858511c739a6590af374e03c446ac Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 21 Jul 2026 03:58:11 +0100 Subject: [PATCH 3/5] fix(ci): unwrap the CLI envelope in the release workflow output parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _release.reusable.yml runs the release and standing-pr publish CLI directly and reads `.versionOutput` from its stdout — which the uniform envelope now nests under `data`. Without unwrapping, has_changes would read false and no tags would push. Unwrap RELEASE_OUTPUT to its data payload at both capture points (tolerating non-enveloped output), so the downstream reads and release-output.json are unchanged. Claude-Session: https://claude.ai/code/session_01K6CmYRXTZQYs4MNo6AETxT Co-authored-by: Claude Opus 4.8 (1M context) --- .github/workflows/_release.reusable.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/_release.reusable.yml b/.github/workflows/_release.reusable.yml index bd0e5cf6..3f1f1d10 100644 --- a/.github/workflows/_release.reusable.yml +++ b/.github/workflows/_release.reusable.yml @@ -165,6 +165,13 @@ jobs: exit $EXIT_CODE fi + # Unwrap the uniform CLI envelope ({ schemaVersion, status, data, … }) to its data payload so + # the .versionOutput / .releaseNotes reads below (and in release-output.json) work unchanged. + # Tolerate non-enveloped output. + if echo "$RELEASE_OUTPUT" | jq -e 'type == "object" and has("schemaVersion") and has("data")' > /dev/null 2>&1; then + RELEASE_OUTPUT=$(echo "$RELEASE_OUTPUT" | jq '.data') + fi + # Check if output is valid JSON (releasekit returns null for no changes) if echo "$RELEASE_OUTPUT" | jq -e '.versionOutput' > /dev/null 2>&1; then echo "has_changes=true" >> "$GITHUB_OUTPUT" @@ -203,6 +210,11 @@ jobs: exit $EXIT_CODE fi + # Unwrap the uniform CLI envelope (see the release path above) so the reads below work. + if echo "$RELEASE_OUTPUT" | jq -e 'type == "object" and has("schemaVersion") and has("data")' > /dev/null 2>&1; then + RELEASE_OUTPUT=$(echo "$RELEASE_OUTPUT" | jq '.data') + fi + if echo "$RELEASE_OUTPUT" | jq -e '.versionOutput' > /dev/null 2>&1; then echo "has_changes=true" >> "$GITHUB_OUTPUT" echo "$RELEASE_OUTPUT" > release-output.json From 2bc0a4b4136e0714e9cc096e1e5224454139595c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 21 Jul 2026 09:18:26 +0100 Subject: [PATCH 4/5] fix(cli): envelope pre-flight input-validation errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-flight CLI arg checks (--stable with --prerelease, --draft with --from-draft, invalid --from-draft, invalid --pr) exited via console.error + process.exit before the enveloped path, so in --json / --output mode they produced no envelope (empty stdout, no output file) — breaking the uniform error contract for ordinary input errors. The --pr check also exited with GENERAL_ERROR instead of INPUT_ERROR. Add a typed InputError (code INPUT_ERROR) in core and a failInput() helper that emits a structured INPUT_ERROR envelope, logs to stderr, and exits with the input-error code; route all pre-flight validations through it. Claude-Session: https://claude.ai/code/session_01K6CmYRXTZQYs4MNo6AETxT Co-authored-by: Claude Opus 4.8 (1M context) --- packages/core/src/errors.ts | 6 +++++ packages/core/src/index.ts | 2 +- packages/release/src/commands/emitResult.ts | 21 +++++++++++++++- .../release/src/commands/release-command.ts | 15 ++++++----- .../src/commands/standing-pr-command.ts | 7 +++--- .../release/test/unit/release-command.spec.ts | 20 +++++++++++++++ .../test/unit/standing-pr-command.spec.ts | 25 ++++++++++++++++++- 7 files changed, 81 insertions(+), 15 deletions(-) diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 4cbf52b0..b74a92cc 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -29,6 +29,12 @@ export abstract class ReleaseKitError extends Error { } } +/** A CLI input-validation failure — a bad flag combination or a malformed argument. */ +export class InputError extends ReleaseKitError { + readonly code = 'INPUT_ERROR'; + readonly suggestions: string[] = []; +} + export const EXIT_CODES = { SUCCESS: 0, GENERAL_ERROR: 1, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1e972759..a3b89e88 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -24,7 +24,7 @@ export { successEnvelope, toEnvelopeError, } from './envelope.js'; -export { EXIT_CODES, type ExitCode, ReleaseKitError } from './errors.js'; +export { EXIT_CODES, type ExitCode, InputError, ReleaseKitError } from './errors.js'; export { debug, error, diff --git a/packages/release/src/commands/emitResult.ts b/packages/release/src/commands/emitResult.ts index 1e1678e8..8a213bc0 100644 --- a/packages/release/src/commands/emitResult.ts +++ b/packages/release/src/commands/emitResult.ts @@ -1,5 +1,13 @@ import { writeFileSync } from 'node:fs'; -import { type Envelope, type EnvelopeWarning, errorEnvelope, successEnvelope, toEnvelopeError } from '@releasekit/core'; +import { + type Envelope, + type EnvelopeWarning, + EXIT_CODES, + errorEnvelope, + InputError, + successEnvelope, + toEnvelopeError, +} from '@releasekit/core'; /** * Write an envelope to the JSON channel. When `--output` is given, write to that file — the reliable @@ -33,3 +41,14 @@ export function emitError(error: unknown, opts: { json?: boolean; output?: strin if (!opts.json && !opts.output) return; writeEnvelope(errorEnvelope([toEnvelopeError(error)]), opts); } + +/** + * Report a CLI input-validation failure through the uniform contract: emit a structured INPUT_ERROR + * envelope on the JSON channel, log the message to stderr, and exit with the input-error code. Used + * for pre-flight arg checks so they honour the same envelope guarantee as runtime errors. + */ +export function failInput(message: string, opts: { json?: boolean; output?: string }): never { + emitError(new InputError(message), opts); + console.error(message); + process.exit(EXIT_CODES.INPUT_ERROR); +} diff --git a/packages/release/src/commands/release-command.ts b/packages/release/src/commands/release-command.ts index efc119a8..c8baa3e6 100644 --- a/packages/release/src/commands/release-command.ts +++ b/packages/release/src/commands/release-command.ts @@ -1,9 +1,9 @@ -import { EXIT_CODES, exitCodeForError } from '@releasekit/core'; +import { exitCodeForError } from '@releasekit/core'; import { Command, Option } from 'commander'; import { publishFromDraft, runReleaseDraft } from '../draft/draft.js'; import { runRelease } from '../release.js'; import type { ReleaseOptions } from '../types.js'; -import { emitError, emitResult } from './emitResult.js'; +import { emitError, emitResult, failInput } from './emitResult.js'; export function createReleaseCommand(): Command { return new Command('release') @@ -48,22 +48,21 @@ export function createReleaseCommand(): Command { .option('-q, --quiet', 'Suppress non-error output', false) .option('--project-dir ', 'Project directory', process.cwd()) .action(async (opts) => { + const io = { json: opts.json, output: opts.output }; + if (opts.stable && opts.prerelease) { - console.error('Error: Cannot use both --stable and --prerelease at the same time'); - process.exit(EXIT_CODES.INPUT_ERROR); + failInput('Cannot use both --stable and --prerelease at the same time', io); } if (opts.draft && opts.fromDraft !== undefined) { - console.error('Error: Cannot use both --draft and --from-draft at the same time'); - process.exit(EXIT_CODES.INPUT_ERROR); + failInput('Cannot use both --draft and --from-draft at the same time', io); } let fromDraftNumber: number | undefined; if (opts.fromDraft !== undefined) { fromDraftNumber = Number(opts.fromDraft); if (!Number.isInteger(fromDraftNumber) || fromDraftNumber <= 0) { - console.error(`Error: --from-draft expects a positive issue number, got "${opts.fromDraft}"`); - process.exit(EXIT_CODES.INPUT_ERROR); + failInput(`--from-draft expects a positive issue number, got "${opts.fromDraft}"`, io); } } diff --git a/packages/release/src/commands/standing-pr-command.ts b/packages/release/src/commands/standing-pr-command.ts index 2046ffa5..7d487608 100644 --- a/packages/release/src/commands/standing-pr-command.ts +++ b/packages/release/src/commands/standing-pr-command.ts @@ -1,8 +1,8 @@ -import { EXIT_CODES, exitCodeForError } from '@releasekit/core'; +import { exitCodeForError } from '@releasekit/core'; import { Command } from 'commander'; import type { StandingPROptions } from '../standing-pr/standing-pr.js'; import { runStandingPRMerge, runStandingPRPublish, runStandingPRUpdate } from '../standing-pr/standing-pr.js'; -import { emitError, emitResult } from './emitResult.js'; +import { emitError, emitResult, failInput } from './emitResult.js'; export function createStandingPRCommand(): Command { const cmd = new Command('standing-pr').description( @@ -81,8 +81,7 @@ export function createStandingPRCommand(): Command { // non-digit characters ('123abc' → 123), which would mask genuine input errors. const trimmed = String(opts.pr).trim(); if (!/^[1-9]\d*$/.test(trimmed)) { - console.error(`--pr must be a positive integer (got: ${opts.pr})`); - process.exit(EXIT_CODES.GENERAL_ERROR); + failInput(`--pr must be a positive integer (got: ${opts.pr})`, { json: opts.json, output: opts.output }); } prNumber = Number.parseInt(trimmed, 10); } diff --git a/packages/release/test/unit/release-command.spec.ts b/packages/release/test/unit/release-command.spec.ts index 270ad8c9..152fbd14 100644 --- a/packages/release/test/unit/release-command.spec.ts +++ b/packages/release/test/unit/release-command.spec.ts @@ -178,4 +178,24 @@ describe('createReleaseCommand', () => { consoleSpy.mockRestore(); }); }); + + describe('input validation', () => { + it('should emit an INPUT_ERROR envelope and exit 3 for --stable with --prerelease in json mode', async () => { + mockExit.mockImplementation(((code?: number) => { + throw new Error(`exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'error').mockImplementation(() => undefined); + + await expect( + createReleaseCommand().parseAsync(['node', 'test', '--json', '--stable', '--prerelease']), + ).rejects.toThrow(/exit\(3\)/); + + expect(runRelease).not.toHaveBeenCalled(); + const envelope = JSON.parse(logSpy.mock.calls[0]?.[0] as string); + expect(envelope.status).toBe('error'); + expect(envelope.errors[0]).toMatchObject({ code: 'INPUT_ERROR', category: 'input', retryable: false }); + logSpy.mockRestore(); + }); + }); }); diff --git a/packages/release/test/unit/standing-pr-command.spec.ts b/packages/release/test/unit/standing-pr-command.spec.ts index 11200096..7db98844 100644 --- a/packages/release/test/unit/standing-pr-command.spec.ts +++ b/packages/release/test/unit/standing-pr-command.spec.ts @@ -54,8 +54,9 @@ describe('createStandingPRCommand', () => { }) as never); const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); try { + // Exits with INPUT_ERROR (3), not the general error code. await expect(parseCommand(['publish', '--project-dir', '/test', '--pr', '123abc'])).rejects.toThrow( - /process\.exit/, + /process\.exit\(3\)/, ); expect(runStandingPRPublish).not.toHaveBeenCalled(); expect(errSpy).toHaveBeenCalledWith(expect.stringContaining('positive integer')); @@ -65,6 +66,28 @@ describe('createStandingPRCommand', () => { } }); + it('should emit an INPUT_ERROR envelope for an invalid --pr in json mode', async () => { + const { runStandingPRPublish } = await import('../../src/standing-pr/standing-pr.js'); + vi.mocked(runStandingPRPublish).mockClear(); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await expect(parseCommand(['publish', '--project-dir', '/test', '--json', '--pr', 'abc'])).rejects.toThrow( + /process\.exit\(3\)/, + ); + const envelope = JSON.parse(logSpy.mock.calls[0]?.[0] as string); + expect(envelope.status).toBe('error'); + expect(envelope.errors[0]).toMatchObject({ code: 'INPUT_ERROR', category: 'input', retryable: false }); + } finally { + exitSpy.mockRestore(); + logSpy.mockRestore(); + errSpy.mockRestore(); + } + }); + it('should pass --verbose, --quiet, --json flags through', async () => { const { runStandingPRUpdate } = await import('../../src/standing-pr/standing-pr.js'); await parseCommand(['update', '--verbose', '--quiet', '--json']); From 0add3ad3e679b6c01e2ce75b6798405a3f7c2d81 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 1 Aug 2026 19:16:39 +0100 Subject: [PATCH 5/5] fix(cli): derive standing-pr publish `changed` from publish effects, error on unresolved PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups to the envelope contract, from review of #608. `standing-pr publish` derived `changed` from `versionOutput.updates`, which comes from the merged PR's manifest and is populated on every run — the versions were already computed and landed on `main` by the merge. A re-run where every package was skipped as already-published still reported `changed: true`, the exact case the docs advertised it for. `publishDidChange` reads `publishOutput` instead. Release tags are created before the pipeline, `git.pushed` is set for any push attempt, and the GitHub-release stage reports success for an already-existing release, so registry results are the only honest signal — same `success && !skipped && !alreadyPublished` predicate the verify stage uses. `runStandingPRPublish` logged and returned null when it could not resolve a PR to publish (no GitHub context, no merged standing PR, unreadable or numberless event payload). Those now throw. Returning null made the envelope assert `status: "success"` on a misconfiguration — the one field an agent branches on. Null stays the answer for the genuine no-ops: pull_request:closed fires for every closed PR, so a non-release head ref or an unmerged PR must remain a quiet skip. `publishFromManifest` has no no-op outcome at all once a PR is named, so it now returns `ReleaseOutput` rather than `ReleaseOutput | null`. The docs claimed every `--json` command emits an envelope; `version`, `notes`, and `publish` do not, deliberately — they pipe JSON between processes and enveloping them means teaching the consuming side to unwrap. Scoped the claim to the orchestration commands and called out the exception. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27 --- docs/cli.md | 6 +- packages/release/src/commands/changed.ts | 20 ++++ .../release/src/commands/release-command.ts | 4 +- .../src/commands/standing-pr-command.ts | 9 +- .../release/src/standing-pr/standing-pr.ts | 43 ++++++--- .../test/unit/commands/changed.spec.ts | 96 +++++++++++++++++++ .../test/unit/standing-pr-command.spec.ts | 61 ++++++++++++ .../release/test/unit/standing-pr.spec.ts | 50 ++++------ 8 files changed, 237 insertions(+), 52 deletions(-) create mode 100644 packages/release/src/commands/changed.ts create mode 100644 packages/release/test/unit/commands/changed.spec.ts diff --git a/docs/cli.md b/docs/cli.md index 4c3047b1..50278aa4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -20,7 +20,9 @@ The `releasekit` dispatcher re-exports `version`, `notes`, and `publish`, so `re ## JSON output contract -Every command that accepts `-j, --json` emits its result as a single **envelope** — one uniform shape shared by humans, CI, and agents. `--output ` writes the same envelope to a file instead of stdout (the reliable channel for the GitHub Action, since stdout can be polluted by subprocess or log noise, and a single stray byte breaks JSON parsing). +The orchestration commands — `release`, `gate`, and `standing-pr` — emit their `-j, --json` result as a single **envelope**, one uniform shape shared by humans, CI, and agents. `--output ` writes the same envelope to a file instead of stdout (the reliable channel for the GitHub Action, since stdout can be polluted by subprocess or log noise, and a single stray byte breaks JSON parsing). + +> **Not yet enveloped:** the pipe commands `version`, `notes`, and `publish` still print their payload bare. They pipe JSON between processes, so wrapping their output means teaching the consuming side to unwrap on input — a separate change. Don't unwrap `.data` from those; check for `schemaVersion` if you need to handle both. ```jsonc { @@ -34,7 +36,7 @@ Every command that accepts `-j, --json` emits its result as a single **envelope* ``` - **`data`** carries the command's payload verbatim — the envelope wraps it, never replaces it. `releasekit release --json` still exposes its `VersionOutput` at `data.versionOutput`. -- **`changed`** separates real work from a no-op: a dry run is never `changed`; `standing-pr publish` reports `changed: false` when the release was already published (skip-if-published); `gate` is read-only and always `changed: false`. +- **`changed`** separates real work from a no-op: a dry run is never `changed`; `standing-pr publish` reads its registry results, so a re-run where every version was already published reports `changed: false`; `gate` is read-only and always `changed: false`. - **`errors[]`** replaces prose-only failures in JSON mode. Each carries a stable machine `code`, a coarse `category`, a `retryable` flag (only `true` for known-transient failures — a timeout, 429, or 5xx from a provider — so an agent never retries an unknown failure), and a human `message`. - **Stream discipline:** the envelope is the only thing on stdout; all diagnostics (progress, warnings, error text) go to stderr. Parsing stdout as JSON is always safe, and no command prompts interactively on a CI path. diff --git a/packages/release/src/commands/changed.ts b/packages/release/src/commands/changed.ts new file mode 100644 index 00000000..2f5518fe --- /dev/null +++ b/packages/release/src/commands/changed.ts @@ -0,0 +1,20 @@ +import type { ReleaseOutput } from '../types.js'; + +/** + * Did a `standing-pr publish` actually publish anything? + * + * `versionOutput.updates` cannot answer this. The manifest carries the versions the merge already + * landed on `main`, so it is populated on every run — including a re-run where every package was + * already published and nothing happened. Only `publishOutput` records what *this* invocation did. + * + * Within `publishOutput`, the registry results are the one honest signal. Release tags are created + * before the pipeline runs, `git.pushed` is set for any push attempt, and the GitHub-release stage + * reports `success: true` for an already-existing release — none of those distinguish a real publish + * from an idempotent no-op. A registry result does: `skipped` and `alreadyPublished` mark the + * packages that were passed over. Same predicate the verify stage uses to pick what to verify. + */ +export function publishDidChange(result: ReleaseOutput | null | undefined): boolean { + const output = result?.publishOutput; + if (!output || output.dryRun) return false; + return [...output.npm, ...output.cargo, ...output.pub].some((r) => r.success && !r.skipped && !r.alreadyPublished); +} diff --git a/packages/release/src/commands/release-command.ts b/packages/release/src/commands/release-command.ts index c8baa3e6..ecfc21e9 100644 --- a/packages/release/src/commands/release-command.ts +++ b/packages/release/src/commands/release-command.ts @@ -101,13 +101,13 @@ export function createReleaseCommand(): Command { // A dry run plans but never mutates state, so it is never "changed"; a real run changed // state iff it produced version updates. const changed = !opts.dryRun && Boolean(result?.versionOutput?.updates?.length); - emitResult(result, { json: options.json, output: opts.output, changed }); + emitResult(result, { ...io, changed }); if (!result) { process.exit(0); } } catch (error) { - emitError(error, { json: opts.json, output: opts.output }); + emitError(error, io); console.error(error instanceof Error ? error.message : String(error)); process.exit(exitCodeForError(error)); } diff --git a/packages/release/src/commands/standing-pr-command.ts b/packages/release/src/commands/standing-pr-command.ts index 7d487608..e6f33595 100644 --- a/packages/release/src/commands/standing-pr-command.ts +++ b/packages/release/src/commands/standing-pr-command.ts @@ -2,6 +2,7 @@ import { exitCodeForError } from '@releasekit/core'; import { Command } from 'commander'; import type { StandingPROptions } from '../standing-pr/standing-pr.js'; import { runStandingPRMerge, runStandingPRPublish, runStandingPRUpdate } from '../standing-pr/standing-pr.js'; +import { publishDidChange } from './changed.js'; import { emitError, emitResult, failInput } from './emitResult.js'; export function createStandingPRCommand(): Command { @@ -88,12 +89,8 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRPublish(options, prNumber); - // skip-if-published returns no updates — surface that as changed:false. - emitResult(result, { - json: opts.json, - output: opts.output, - changed: Boolean(result?.versionOutput?.updates?.length), - }); + // Read the publish effects, not the manifest's versions — see publishDidChange. + emitResult(result, { json: opts.json, output: opts.output, changed: publishDidChange(result) }); } catch (err) { emitError(err, { json: opts.json, output: opts.output }); console.error(err instanceof Error ? err.message : String(err)); diff --git a/packages/release/src/standing-pr/standing-pr.ts b/packages/release/src/standing-pr/standing-pr.ts index 3334fcc1..ef851697 100644 --- a/packages/release/src/standing-pr/standing-pr.ts +++ b/packages/release/src/standing-pr/standing-pr.ts @@ -1678,13 +1678,20 @@ async function assertManifestMatchesSource( // Core publish logic: finds manifest on the given PR, optionally extracts user-edited // notes from the PR body, then runs the publish step and cleans up the release branch. -export async function publishFromManifest(prNumber: number, options: StandingPROptions): Promise { +/** + * Publish the release described by a merged standing PR's manifest. Every failure throws — there is + * no "nothing to do" outcome once a specific PR has been named, so a caller that gets a result back + * knows the publish ran. + */ +export async function publishFromManifest(prNumber: number, options: StandingPROptions): Promise { const cwd = options.projectDir; const githubContext = getGitHubContext(); if (!githubContext?.token) { - error('No GitHub context (GITHUB_REPOSITORY or GITHUB_TOKEN) available'); - return null; + throw new Error( + `No GitHub context (GITHUB_REPOSITORY or GITHUB_TOKEN) available — cannot read the release manifest from ` + + `PR #${prNumber}.`, + ); } const forge = forgeFor(githubContext); @@ -1974,6 +1981,15 @@ export async function findLatestMergedStandingPR(forge: Forge, branch: string): return merged[0]?.number ?? null; } +/** + * Publish from a merged standing release PR. + * + * `null` means "nothing to do" — this run legitimately had no release to publish, and the caller + * reports success. Everything that went *wrong* throws: the JSON envelope reports `status: "error"` + * on those, so an agent or workflow can't read a misconfiguration as a successful no-op. The + * distinction matters most on the `pull_request: closed` trigger, which fires for every closed PR + * and must stay a quiet no-op for the ones that aren't the release branch. + */ export async function runStandingPRPublish( options: StandingPROptions, explicitPrNumber?: number, @@ -1998,8 +2014,10 @@ export async function runStandingPRPublish( try { event = JSON.parse(fs.readFileSync(eventPath, 'utf-8')); } catch (err) { - error(`Failed to read GitHub event: ${err instanceof Error ? err.message : String(err)}`); - return null; + throw new Error( + `Failed to read the GitHub event at ${eventPath}: ${err instanceof Error ? err.message : String(err)}. ` + + `The event payload decides which PR to publish — pass --pr explicitly to bypass it.`, + ); } } @@ -2019,8 +2037,10 @@ export async function runStandingPRPublish( } if (!prNumber) { - error('Could not determine PR number from GitHub event'); - return null; + throw new Error( + `Could not determine the PR number from the GitHub event: its pull_request payload matched release branch ` + + `'${releaseBranch}' and is merged, but carries no number. Pass --pr explicitly.`, + ); } return publishFromManifest(prNumber, options); @@ -2029,14 +2049,15 @@ export async function runStandingPRPublish( // No pull_request context — infer the merged standing PR from the API. const githubContext = getGitHubContext(); if (!githubContext?.token) { - error('No GitHub context (GITHUB_REPOSITORY or GITHUB_TOKEN) available — pass --pr explicitly'); - return null; + throw new Error( + 'No GitHub context (GITHUB_REPOSITORY or GITHUB_TOKEN) available, so the merged standing PR cannot be ' + + 'inferred — pass --pr explicitly.', + ); } const inferred = await findLatestMergedStandingPR(forgeFor(githubContext), releaseBranch); if (inferred === null) { - error(`No merged standing release PR found for branch '${releaseBranch}' — pass --pr explicitly`); - return null; + throw new Error(`No merged standing release PR found for branch '${releaseBranch}' — pass --pr explicitly.`); } info( diff --git a/packages/release/test/unit/commands/changed.spec.ts b/packages/release/test/unit/commands/changed.spec.ts new file mode 100644 index 00000000..efec06e1 --- /dev/null +++ b/packages/release/test/unit/commands/changed.spec.ts @@ -0,0 +1,96 @@ +import type { PublishOutput, PublishResult } from '@releasekit/publish'; +import { describe, expect, it } from 'vitest'; +import { publishDidChange } from '../../../src/commands/changed.js'; +import type { ReleaseOutput } from '../../../src/types.js'; + +function publishResult(overrides: Partial = {}): PublishResult { + return { + packageName: '@acme/widget', + version: '2.0.0', + registry: 'npm', + success: true, + skipped: false, + ...overrides, + }; +} + +function publishOutput(overrides: Partial = {}): PublishOutput { + return { + dryRun: false, + git: { committed: false, tags: ['v2.0.0'], pushed: true }, + npm: [], + cargo: [], + pub: [], + verification: [], + githubReleases: [], + publishSucceeded: true, + ...overrides, + }; +} + +function releaseOutput(publish?: PublishOutput): ReleaseOutput { + return { + // Always populated from the manifest, whether or not this run published anything — the reason + // `changed` can't be derived from it. + versionOutput: { + dryRun: false, + updates: [{ packageName: '@acme/widget', currentVersion: '1.4.2', newVersion: '2.0.0', bumpType: 'major' }], + changelogs: [], + tags: ['v2.0.0'], + } as ReleaseOutput['versionOutput'], + notesGenerated: false, + publishOutput: publish, + }; +} + +describe('publishDidChange', () => { + it('should be true when a package actually reached a registry', () => { + expect(publishDidChange(releaseOutput(publishOutput({ npm: [publishResult()] })))).toBe(true); + }); + + it('should be false when every package was already published', () => { + const output = publishOutput({ + npm: [publishResult({ skipped: true, alreadyPublished: true, reason: 'already published' })], + }); + expect(publishDidChange(releaseOutput(output))).toBe(false); + }); + + it('should be false when every package was skipped as private', () => { + const output = publishOutput({ npm: [publishResult({ skipped: true, reason: 'private' })] }); + expect(publishDidChange(releaseOutput(output))).toBe(false); + }); + + it('should be false for a push of pre-existing tags and releases with nothing published', () => { + const output = publishOutput({ + npm: [publishResult({ skipped: true, alreadyPublished: true })], + // Both report success for an already-existing tag/release, so neither is a change signal. + git: { committed: true, tags: ['v2.0.0'], pushed: true }, + githubReleases: [{ tag: 'v2.0.0', draft: false, prerelease: false, success: true }], + }); + expect(publishDidChange(releaseOutput(output))).toBe(false); + }); + + it('should be true when one package published among already-published ones', () => { + const output = publishOutput({ + npm: [publishResult({ packageName: '@acme/a', skipped: true, alreadyPublished: true }), publishResult()], + }); + expect(publishDidChange(releaseOutput(output))).toBe(true); + }); + + it('should read cargo and pub results alongside npm', () => { + expect(publishDidChange(releaseOutput(publishOutput({ cargo: [publishResult({ registry: 'cargo' })] })))).toBe( + true, + ); + expect(publishDidChange(releaseOutput(publishOutput({ pub: [publishResult({ registry: 'pub' })] })))).toBe(true); + }); + + it('should be false for a dry run that reports successful publishes', () => { + const output = publishOutput({ dryRun: true, npm: [publishResult()] }); + expect(publishDidChange(releaseOutput(output))).toBe(false); + }); + + it('should be false when there is no publish output at all', () => { + expect(publishDidChange(releaseOutput(undefined))).toBe(false); + expect(publishDidChange(null)).toBe(false); + }); +}); diff --git a/packages/release/test/unit/standing-pr-command.spec.ts b/packages/release/test/unit/standing-pr-command.spec.ts index 7db98844..ff260af7 100644 --- a/packages/release/test/unit/standing-pr-command.spec.ts +++ b/packages/release/test/unit/standing-pr-command.spec.ts @@ -88,6 +88,67 @@ describe('createStandingPRCommand', () => { } }); + // The manifest's versionOutput.updates is populated on every publish run, so it can't distinguish a + // real publish from an idempotent re-run — `changed` has to come from the publish effects. + describe('publish changed reporting', () => { + const manifestVersionOutput = { + dryRun: false, + updates: [{ packageName: '@acme/widget', currentVersion: '1.4.2', newVersion: '2.0.0', bumpType: 'major' }], + changelogs: [], + tags: ['v2.0.0'], + }; + const npmResult = { packageName: '@acme/widget', version: '2.0.0', registry: 'npm', success: true }; + + async function publishEnvelope(publishOutput: unknown) { + const { runStandingPRPublish } = await import('../../src/standing-pr/standing-pr.js'); + vi.mocked(runStandingPRPublish).mockResolvedValueOnce({ + versionOutput: manifestVersionOutput, + notesGenerated: false, + publishOutput, + } as never); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + try { + await parseCommand(['publish', '--project-dir', '/test', '--json', '--pr', '189']); + return JSON.parse(logSpy.mock.calls[0]?.[0] as string); + } finally { + logSpy.mockRestore(); + } + } + + it('should report changed:true when a package actually published', async () => { + const envelope = await publishEnvelope({ + dryRun: false, + git: { committed: false, tags: ['v2.0.0'], pushed: true }, + npm: [{ ...npmResult, skipped: false }], + cargo: [], + pub: [], + verification: [], + githubReleases: [], + publishSucceeded: true, + }); + expect(envelope.status).toBe('success'); + expect(envelope.changed).toBe(true); + }); + + it('should report changed:false when every version was already published', async () => { + const envelope = await publishEnvelope({ + dryRun: false, + // Tags pushed and the GitHub release "succeeded" (it already existed) — neither is a change. + git: { committed: false, tags: ['v2.0.0'], pushed: true }, + npm: [{ ...npmResult, skipped: true, alreadyPublished: true }], + cargo: [], + pub: [], + verification: [], + githubReleases: [{ tag: 'v2.0.0', draft: false, prerelease: false, success: true }], + publishSucceeded: true, + }); + expect(envelope.status).toBe('success'); + expect(envelope.changed).toBe(false); + // The manifest payload still rides along untouched — the envelope wraps, never replaces. + expect(envelope.data.versionOutput.updates).toHaveLength(1); + }); + }); + it('should pass --verbose, --quiet, --json flags through', async () => { const { runStandingPRUpdate } = await import('../../src/standing-pr/standing-pr.js'); await parseCommand(['update', '--verbose', '--quiet', '--json']); diff --git a/packages/release/test/unit/standing-pr.spec.ts b/packages/release/test/unit/standing-pr.spec.ts index 0a2768a0..7b827bbf 100644 --- a/packages/release/test/unit/standing-pr.spec.ts +++ b/packages/release/test/unit/standing-pr.spec.ts @@ -2450,33 +2450,26 @@ describe('runStandingPRPublish', () => { ).rejects.toThrow(/manifest not found/); }); - it('should return null when API inference finds no merged standing PR', async () => { + // The two cases below are failures to resolve a PR, not "nothing to release" — they throw so the + // JSON envelope reports status:"error" rather than a successful no-op. Contrast with the head-ref + // and not-merged skips below, which stay null: pull_request:closed fires for every closed PR. + it('should throw when API inference finds no merged standing PR', async () => { delete process.env.GITHUB_EVENT_PATH; await mockForge({ recentlyClosedPRs: [{ number: 80, mergedAt: null }] }); - const result = await runStandingPRPublish({ - projectDir: '/test', - verbose: false, - quiet: false, - json: false, - }); - - expect(result).toBeNull(); + await expect( + runStandingPRPublish({ projectDir: '/test', verbose: false, quiet: false, json: false }), + ).rejects.toThrow(/No merged standing release PR found/); }); - it('should return null when inference is needed but no GitHub token is available', async () => { + it('should throw when inference is needed but no GitHub token is available', async () => { delete process.env.GITHUB_EVENT_PATH; delete process.env.GITHUB_TOKEN; - const result = await runStandingPRPublish({ - projectDir: '/test', - verbose: false, - quiet: false, - json: false, - }); - - expect(result).toBeNull(); + await expect( + runStandingPRPublish({ projectDir: '/test', verbose: false, quiet: false, json: false }), + ).rejects.toThrow(/No GitHub context/); }); it('should return null when merged PR head ref does not match release branch', async () => { @@ -2864,7 +2857,7 @@ describe('runStandingPRPublish', () => { expect(branchDeletePushes()).toHaveLength(0); }); - it('should return null when no GitHub context is available for a merged release PR', async () => { + it('should throw when no GitHub context is available for a merged release PR', async () => { delete process.env.GITHUB_REPOSITORY; delete process.env.GITHUB_TOKEN; @@ -2875,9 +2868,9 @@ describe('runStandingPRPublish', () => { }), ); - const result = await runStandingPRPublish({ projectDir: '/test', verbose: false, quiet: false, json: false }); - - expect(result).toBeNull(); + await expect( + runStandingPRPublish({ projectDir: '/test', verbose: false, quiet: false, json: false }), + ).rejects.toThrow(/No GitHub context/); }); it('should throw with actionable message when manifest JSON is malformed', async () => { @@ -2925,18 +2918,13 @@ describe('publishFromManifest', () => { process.env = { ...originalEnv }; }); - it('should return null when no GitHub context is available', async () => { + it('should throw when no GitHub context is available', async () => { delete process.env.GITHUB_REPOSITORY; delete process.env.GITHUB_TOKEN; - const result = await publishFromManifest(42, { - projectDir: '/test', - verbose: false, - quiet: false, - json: false, - }); - - expect(result).toBeNull(); + await expect( + publishFromManifest(42, { projectDir: '/test', verbose: false, quiet: false, json: false }), + ).rejects.toThrow(/No GitHub context/); }); it('should throw when manifest comment is missing from PR', async () => {