From 53372e02bb47a229af47b11342a6abe4912c1aa6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 21 Jul 2026 02:43:18 +0100 Subject: [PATCH] fix(action): read release outputs from a --output file, not scraped stdout Tier 2 of #594. The JSON-result CLI commands (release, gate, standing-pr update/publish) gain a `--output ` flag that writes the result JSON to a file via a shared emitResult() helper. run-action passes a temp --output file for those modes and reads the structured result from it (falling back to stdout only if the file is missing, then to Tier 1's git-derived tags). This removes the stdout-scraping fragility from ALL release outputs (has-changes / tags / version-output / release-output), not just `tags` (Tier 1, #597): a stray byte of subprocess or log noise on stdout can no longer corrupt them. Preview mode is unchanged (its markdown output stays on stdout). Addresses #594 (Tier 2). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K6CmYRXTZQYs4MNo6AETxT --- packages/release/src/commands/emitResult.ts | 16 +++++++ packages/release/src/commands/gate-command.ts | 6 +-- .../release/src/commands/release-command.ts | 6 +-- .../src/commands/standing-pr-command.ts | 14 ++---- .../test/unit/commands/emitResult.spec.ts | 48 +++++++++++++++++++ scripts/run-action.d.mts | 1 + scripts/run-action.mjs | 47 ++++++++++++++---- test/integration/action-runner.spec.ts | 35 ++++++++++++++ 8 files changed, 150 insertions(+), 23 deletions(-) create mode 100644 packages/release/src/commands/emitResult.ts create mode 100644 packages/release/test/unit/commands/emitResult.spec.ts diff --git a/packages/release/src/commands/emitResult.ts b/packages/release/src/commands/emitResult.ts new file mode 100644 index 00000000..e077686f --- /dev/null +++ b/packages/release/src/commands/emitResult.ts @@ -0,0 +1,16 @@ +import { writeFileSync } from 'node:fs'; + +/** + * Emit a command's JSON result. When `--output` is given, write it 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. + */ +export function emitResult(result: unknown, opts: { json?: boolean; output?: string }): void { + if (result === undefined || result === null) return; + const text = JSON.stringify(result, null, 2); + if (opts.output) { + writeFileSync(opts.output, text); + } else if (opts.json) { + console.log(text); + } +} diff --git a/packages/release/src/commands/gate-command.ts b/packages/release/src/commands/gate-command.ts index 97c5e4a2..4dab2bd0 100644 --- a/packages/release/src/commands/gate-command.ts +++ b/packages/release/src/commands/gate-command.ts @@ -1,6 +1,7 @@ import { setJsonMode, setLogLevel, setQuietMode } from '@releasekit/core'; import { Command } from 'commander'; import { runGate } from '../gate/gate.js'; +import { emitResult } from './emitResult.js'; export function createGateCommand(): Command { return new Command('gate') @@ -8,6 +9,7 @@ export function createGateCommand(): Command { .option('-c, --config ', 'Path to config file') .option('--scope ', 'Resolve scope name to target packages from ci.scopeLabels config') .option('-j, --json', 'Output results as JSON', false) + .option('--output ', 'Write the JSON result to a file instead of stdout') .option('-v, --verbose', 'Verbose logging', false) .option('-q, --quiet', 'Suppress non-error output', false) .option('--project-dir ', 'Project directory', process.cwd()) @@ -26,9 +28,7 @@ export function createGateCommand(): Command { quiet: opts.quiet, }); - if (opts.json) { - console.log(JSON.stringify(result, null, 2)); - } + emitResult(result, { json: opts.json, output: opts.output }); process.exit(0); } catch (error) { diff --git a/packages/release/src/commands/release-command.ts b/packages/release/src/commands/release-command.ts index b179c5f8..a805419c 100644 --- a/packages/release/src/commands/release-command.ts +++ b/packages/release/src/commands/release-command.ts @@ -3,6 +3,7 @@ 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'; export function createReleaseCommand(): Command { return new Command('release') @@ -42,6 +43,7 @@ export function createReleaseCommand(): Command { .option('--skip-github-release', 'Skip GitHub release creation', false) .option('--skip-verification', 'Skip post-publish verification', false) .option('-j, --json', 'Output results as JSON', false) + .option('--output ', 'Write the JSON result to a file instead of stdout') .option('-v, --verbose', 'Verbose logging', false) .option('-q, --quiet', 'Suppress non-error output', false) .option('--project-dir ', 'Project directory', process.cwd()) @@ -97,9 +99,7 @@ export function createReleaseCommand(): Command { ? await runReleaseDraft(options) : await runRelease(options); - if (options.json && result) { - console.log(JSON.stringify(result, null, 2)); - } + emitResult(result, { json: options.json, output: opts.output }); if (!result) { process.exit(0); diff --git a/packages/release/src/commands/standing-pr-command.ts b/packages/release/src/commands/standing-pr-command.ts index 37009c12..865b58e9 100644 --- a/packages/release/src/commands/standing-pr-command.ts +++ b/packages/release/src/commands/standing-pr-command.ts @@ -2,6 +2,7 @@ import { EXIT_CODES } 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'; export function createStandingPRCommand(): Command { const cmd = new Command('standing-pr').description( @@ -14,6 +15,7 @@ export function createStandingPRCommand(): Command { .option('--project-dir ', 'Project directory', process.cwd()) .option('--npm-auth ', 'NPM auth method (auto|oidc|token)', 'auto') .option('-j, --json', 'Output results as JSON', false) + .option('--output ', 'Write the JSON result to a file instead of stdout') .option('-v, --verbose', 'Verbose logging', false) .option('-q, --quiet', 'Suppress non-error output', false); @@ -47,9 +49,7 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRUpdate(options); - if (opts.json) { - console.log(JSON.stringify(result, null, 2)); - } + emitResult(result, { json: opts.json, output: opts.output }); } catch (err) { console.error(err instanceof Error ? err.message : String(err)); process.exit(EXIT_CODES.GENERAL_ERROR); @@ -88,9 +88,7 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRPublish(options, prNumber); - if (opts.json && result) { - console.log(JSON.stringify(result, null, 2)); - } + emitResult(result, { json: opts.json, output: opts.output }); } catch (err) { console.error(err instanceof Error ? err.message : String(err)); process.exit(EXIT_CODES.GENERAL_ERROR); @@ -114,9 +112,7 @@ export function createStandingPRCommand(): Command { try { const result = await runStandingPRMerge(options, { publish: opts.publish }); - if (opts.json && result) { - console.log(JSON.stringify(result, null, 2)); - } + emitResult(result, { json: opts.json, output: opts.output }); } catch (err) { console.error(err instanceof Error ? err.message : String(err)); process.exit(EXIT_CODES.GENERAL_ERROR); diff --git a/packages/release/test/unit/commands/emitResult.spec.ts b/packages/release/test/unit/commands/emitResult.spec.ts new file mode 100644 index 00000000..7816ca54 --- /dev/null +++ b/packages/release/test/unit/commands/emitResult.spec.ts @@ -0,0 +1,48 @@ +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { emitResult } from '../../../src/commands/emitResult.js'; + +describe('emitResult', () => { + const dirs: string[] = []; + + afterEach(() => { + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); + dirs.length = 0; + vi.restoreAllMocks(); + }); + + it('should write the JSON result to the output file when --output is set', () => { + 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 }); + + expect(JSON.parse(readFileSync(file, 'utf8'))).toEqual({ versionOutput: { tags: ['v1.2.3'] } }); + expect(log).not.toHaveBeenCalled(); + }); + + it('should print 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)); + }); + + it('should do nothing when neither --json nor --output is set', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + emitResult({ a: 1 }, {}); + expect(log).not.toHaveBeenCalled(); + }); + + it('should do nothing for a null or undefined result', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + emitResult(undefined, { json: true }); + emitResult(null, { json: true }); + expect(log).not.toHaveBeenCalled(); + }); +}); diff --git a/scripts/run-action.d.mts b/scripts/run-action.d.mts index 5bc6bb6a..db824522 100644 --- a/scripts/run-action.d.mts +++ b/scripts/run-action.d.mts @@ -52,4 +52,5 @@ export function runAction( signal: NodeJS.Signals | null; stdout: string; stderr: string; + outputJson: string; }>; diff --git a/scripts/run-action.mjs b/scripts/run-action.mjs index a4877649..621daddf 100644 --- a/scripts/run-action.mjs +++ b/scripts/run-action.mjs @@ -2,6 +2,7 @@ import { execFileSync, spawn } from 'node:child_process'; import { randomBytes } from 'node:crypto'; import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -309,6 +310,18 @@ export async function runAction(input, options = {}) { // `mode` is validated against validModes above, and every entry has a key here. const args = argBuilders[mode](input); + // Capture the JSON-result modes' structured output via a temp file (--output) instead of scraping + // stdout, which subprocess/log noise can corrupt — a single stray byte breaks JSON.parse. The CLI + // writes the result JSON to this file; we read it back after the run (see the `close` handler). + const OUTPUT_MODES = new Set(['release', 'gate', 'standing-pr-update', 'standing-pr-publish']); + let outputFile; + let outputDir; + if (OUTPUT_MODES.has(mode)) { + outputDir = fs.mkdtempSync(path.join(os.tmpdir(), 'releasekit-action-')); + outputFile = path.join(outputDir, 'result.json'); + args.push('--output', outputFile); + } + const projectDir = input.projectDir || '.'; const actionDir = fileURLToPath(import.meta.url).replace(/[/\\]scripts[/\\]run-action.mjs$/, ''); @@ -433,7 +446,21 @@ export async function runAction(input, options = {}) { }); child.on('close', (status, signal) => { - resolve({ mode, args, status, signal, stdout, stderr }); + // Prefer the --output file (reliable) over captured stdout for the structured result. + let outputJson = ''; + if (outputFile) { + try { + outputJson = fs.readFileSync(outputFile, 'utf-8'); + } catch { + outputJson = ''; + } + try { + fs.rmSync(outputDir, { recursive: true, force: true }); + } catch { + // best-effort temp cleanup + } + } + resolve({ mode, args, status, signal, stdout, stderr, outputJson }); }); }); } @@ -613,15 +640,19 @@ async function main() { const success = result.status === 0; const verbose = normalizeBoolean(input.verbose); + // The structured result comes from the --output file for the JSON-result modes (reliable), falling + // back to captured stdout for the rest (preview) and if the file is missing. + const structured = result.outputJson || (result.stdout ?? ''); + // parsed is used for release/preview output; gateParsed is parsed separately for gate output - const parsed = normalizeBoolean(input.json) ? parseReleaseOutput(result.stdout ?? '', verbose) : undefined; + const parsed = normalizeBoolean(input.json) ? parseReleaseOutput(structured, verbose) : undefined; // Write summary BEFORE setFailure (which calls process.exit) if (normalizeBoolean(input.summary, true)) { if (result.mode === 'release') { writeSummary(buildReleaseSummary(input, parsed, success)); } else if (result.mode === 'gate') { - const gateParsed = parseReleaseOutput(result.stdout ?? '', verbose); + const gateParsed = parseReleaseOutput(structured, verbose); writeSummary(buildGateSummary(input, gateParsed, success)); } } @@ -631,9 +662,9 @@ async function main() { if (!success) { writeCoreOutputs(result.mode, false); if (result.mode === 'gate') { - writeGateOutputs(result.stdout ?? '', verbose); + writeGateOutputs(structured, verbose); } else if (result.mode === 'standing-pr-update' || result.mode === 'standing-pr-publish') { - writeStandingPROutputs(result.stdout ?? '', verbose); + writeStandingPROutputs(structured, verbose); } setFailure(`ReleaseKit ${result.mode} failed with exit code ${result.status ?? 1}`); } @@ -641,11 +672,11 @@ async function main() { writeCoreOutputs(result.mode, true); if (result.mode === 'release') { - writeReleaseOutputs(input, result.stdout ?? ''); + writeReleaseOutputs(input, structured); } else if (result.mode === 'gate') { - writeGateOutputs(result.stdout ?? '', verbose); + writeGateOutputs(structured, verbose); } else if (result.mode === 'standing-pr-update' || result.mode === 'standing-pr-publish') { - writeStandingPROutputs(result.stdout ?? '', verbose); + writeStandingPROutputs(structured, verbose); } else if (result.mode === 'backfill' || result.mode === 'refresh-after-release') { // These stream their own progress and expose only the core success/mode outputs. } else { diff --git a/test/integration/action-runner.spec.ts b/test/integration/action-runner.spec.ts index 754d8758..19f4a459 100644 --- a/test/integration/action-runner.spec.ts +++ b/test/integration/action-runner.spec.ts @@ -317,6 +317,41 @@ describe('action runner', () => { expect(result.args).toContain('--dry-run'); }); + it('should pass --output and read the structured result from that file (not stdout)', async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'releasekit-action-runner-test-')); + tempDirs.push(tempDir); + const cliPath = path.join(tempDir, 'fake-cli.mjs'); + // The fake CLI writes JSON to its --output path and prints noise to stdout — outputJson must come + // from the file, immune to the stdout noise. + fs.writeFileSync( + cliPath, + [ + "import { writeFileSync } from 'node:fs';", + "const i = process.argv.indexOf('--output');", + "if (i !== -1) writeFileSync(process.argv[i + 1], JSON.stringify({ versionOutput: { tags: ['v1.2.3'] } }));", + "console.log('log noise that would corrupt JSON parsing');", + ].join('\n'), + 'utf-8', + ); + + const result = await runAction({ mode: 'release', projectDir: '.', json: 'true' }, { cliPath }); + + expect(result.args).toContain('--output'); + expect(JSON.parse(result.outputJson)).toEqual({ versionOutput: { tags: ['v1.2.3'] } }); + }); + + it('should not pass --output for modes without structured output (preview)', async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'releasekit-action-runner-test-')); + tempDirs.push(tempDir); + const cliPath = path.join(tempDir, 'fake-cli.mjs'); + fs.writeFileSync(cliPath, "console.log('ok')\n", 'utf-8'); + + const result = await runAction({ mode: 'preview', projectDir: '.' }, { cliPath }); + + expect(result.args).not.toContain('--output'); + expect(result.outputJson).toBe(''); + }); + it('should resolve with non-zero status for a failing cli', async () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'releasekit-action-runner-test-')); tempDirs.push(tempDir);