Skip to content

patch-release.js: non-interactive mode (--yes, --cm-trigger, --json) - #638

Merged
kriszyp merged 3 commits into
mainfrom
kris/patch-release-machine-flags
Jul 31, 2026
Merged

patch-release.js: non-interactive mode (--yes, --cm-trigger, --json)#638
kriszyp merged 3 commits into
mainfrom
kris/patch-release-machine-flags

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 31, 2026

Copy link
Copy Markdown
Member

Motivation

The dispatch release-automation app runs patch-release.js headlessly from a dev-agent.
Two problems blocked that:

  1. The script has four readline prompts that hang when stdin is a pipe or /dev/null.
  2. Prompt 2 (CM deploy trigger) defaults YES on EOF — silently deploying to prod if
    the script exits early or is piped without input. This is a known footgun.

Changes

Three new flags; existing interactive behavior is unchanged when flags are absent.

--yes

Non-interactive mode. All prompts auto-answered:

  • Prompt 1 (proceed with bump/tag/push): y
  • Prompt 2 (CM deploy): n — opt-in only, see below
  • Prompts 3/4 (return to original branches): y

The CM-deploy default is deliberately inverted from interactive mode. In interactive use,
pressing Enter (or EOF) triggers a deploy. In --yes mode the safe default is to skip the
deploy unless the caller explicitly opts in with --cm-trigger.

--cm-trigger

Triggers the CM release-to-environments workflow (step 6). With --yes: opt-in deploy.
Without --yes: skips prompt 2 and answers yes directly.

--json

Prints a final machine-readable line on stdout:

RESULT: {"ok":true,"target":"v5.1.27","coreVersion":null,"proVersion":"v5.1.27","coreBumped":false,"pushed":false,"cmTriggered":false,"dryRun":true}

Also emitted on fatal error paths before exit:

RESULT: {"ok":false,"error":"..."}

A die() helper consolidates the err()+JSON+process.exit() pattern that was scattered
across all error-exit sites.

Dry-run verification

node scripts/patch-release.js --dry-run --branch v5.1 --core-branch v5.1 --yes --json < /dev/null

Ran against the real repo: completed with no prompt hangs, no CM workflow triggered
(dry-run + no --cm-trigger), ended with a parseable RESULT line.

Output (abridged):

[dry-run] Skipping version bump, sync, and push.
  Skipped. Manually trigger release-to-environments with version_name=stable when ready.
✅ Done.
RESULT: {"ok":true,"target":"v5.1.27","coreVersion":null,"proVersion":"v5.1.27","coreBumped":false,"pushed":false,"cmTriggered":false,"dryRun":true}

🤖 Generated with Claude Code

… use

Unblocks automated release-cutting from the dispatch app, which runs the
script headlessly. In interactive mode prompt 2 (CM deploy trigger) defaults
YES on EOF — a known deploy footgun. --yes inverts this: CM deploy is skipped
unless --cm-trigger is also passed.

Flag semantics:
  --yes         Auto-confirm all prompts. Prompt 1 (proceed) → y. Prompts 3/4
                (branch return) → y. Prompt 2 (CM deploy) → n unless
                --cm-trigger is also set.
  --cm-trigger  Force CM deploy. With --yes: opt-in deploy. Without --yes:
                skips prompt 2 and answers y.
  --json        Print a final "RESULT: {...}" line with ok, target, coreVersion,
                proVersion, coreBumped, pushed, cmTriggered, dryRun. Fatal
                error paths emit RESULT: {"ok":false,"error":"..."} before exit.

Also introduces die() helper to avoid repeating the err+JSON+exit pattern
across all process.exit(1) sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds non-interactive execution support to the patch-release.js script via new --yes, --cm-trigger, and --json flags, allowing for automated release flows and machine-readable JSON output. The feedback highlights two important improvements: first, using fs.writeSync instead of process.stdout.write in the die helper to ensure JSON output is fully flushed before process.exit terminates the process; second, safely handling caught exceptions in the global catch block to prevent secondary crashes when the error is not an instance of Error.

Comment thread scripts/patch-release.js Outdated
Comment thread scripts/patch-release.js Outdated
@kriszyp
kriszyp marked this pull request as ready for review July 31, 2026 19:06
@kriszyp
kriszyp requested a review from a team as a code owner July 31, 2026 19:06
Comment thread scripts/patch-release.js
Comment thread scripts/patch-release.js Outdated
Comment thread scripts/patch-release.js
kriszyp and others added 2 commits July 31, 2026 13:39
- die() and the final RESULT block now write via fs.writeSync(1, ...)
  instead of process.stdout.write() — the latter races process.exit()
  when stdout is a pipe (the standard --json setup), which can drop the
  RESULT line once output crosses ~64 KiB.
- main().catch() no longer assumes the rejection is an Error (e?.message
  ?? String(e)).
- Declining the first confirmation prompt now emits an explicit
  RESULT: {"ok":false,"error":"aborted",...} line under --json instead
  of exiting silently.
- --cm-trigger no longer bypasses the deploy-confirmation prompt when
  --yes isn't also set — it only changes the prompt's wording. Only
  --cm-trigger + --yes auto-confirms.
- A CM-trigger failure is now terminal (nonzero exit, RESULT ok:false)
  when --cm-trigger was explicitly requested, preserving pushed/version
  state — previously it was swallowed to stderr and the run reported
  ok:true with cmTriggered:false, so a dispatch caller couldn't tell
  "deploy skipped" from "deploy attempted and failed".

Extracted the decision logic (resolveDeployAnswer, buildAbortedResult,
buildCmFailureResult) into pure, exported functions with unit test
coverage in unitTests/scripts/patchRelease.test.mjs, since main() itself
has no seams for testing without live git/gh state.

Addresses review feedback on PR #638 from gemini-code-assist, cb1kenobi,
and kriszyp.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oc the new flags

Follow-up to the independent pre-push review of the prior commit:

- Deferred the CM-trigger fatal die() until after Step 7 (branch restore). Making a
  requested CM failure terminal meant it previously short-circuited before the repos were
  returned to their original branches, leaving a reused dispatch worktree stuck on the
  release branch.
- Extracted the synchronous RESULT-line write (die(), abort, and success paths) into a
  shared writeResult(result, fd) so the truncation-safe write mechanism itself has direct
  test coverage via a real fd, not just the objects it serializes.
- Documented --yes/--cm-trigger/--json in CONTRIBUTING.md's options table.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@kriszyp

kriszyp commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Pushed fixes for all 8 open threads (Gemini, cb1kenobi, and Kris's comments) in f782b1b:

  • die()/success/abort RESULT writes now go through fs.writeSync(1, ...) instead of process.stdout.write(), fixing the truncation race with process.exit() on a piped stdout (Gemini, Kris — line 87/89).
  • main().catch() no longer assumes the rejection is an Error (e?.message ?? String(e)) (Gemini).
  • Declining the first confirmation prompt under --json now emits RESULT: {"ok":false,"error":"aborted","aborted":true} (exit 0) instead of exiting silently (Kris, cb1kenobi).
  • A requested CM-trigger failure (--cm-trigger) is now terminal — nonzero exit, RESULT: {"ok":false,...} preserving pushed/version state — instead of being swallowed to stderr with ok:true (Kris, cb1kenobi).
  • --cm-trigger without --yes no longer bypasses the deploy confirmation for an interactive human — it only changes the prompt's wording now; only --cm-trigger + --yes auto-confirms (cb1kenobi).

Extracted the decision logic (resolveDeployAnswer, buildAbortedResult, buildCmFailureResult, writeResult) into pure, exported functions with unit coverage in unitTests/scripts/patchRelease.test.mjs, since main() itself has no seams for testing without live git/gh state.

I ran this change through an independent pre-push review (Codex + Grok) across two rounds. One real regression it caught in round 1 — making the CM failure terminal via die() skipped Step 7's branch-restore, which would leave a reused dispatch worktree stuck on the release branch — is fixed in f782b1b. Round 2 confirmed no new issues from that fix.

One finding I'm flagging rather than fixing here (both Codex and Grok raised it, consistently across rounds): --yes auto-answers the first confirmation prompt (version bump/sync/tag/push) without any programmatic check that every patch-labeled PR actually has a corresponding release-branch commit — that verification is currently visual-only, and --yes skips the human who'd normally do it. A headless run could version/tag/push an incomplete release if a cherry-pick silently failed or is still queued. Fixing it properly needs a real design decision (how to match a cherry-picked commit back to its source PR — patch-id? cherry-pick trailer parsing?), so I didn't want to guess at it inside a review-feedback pass. Flagging for a follow-up if it's worth doing before --yes sees real dispatch use.

🤖 Generated by Claude Sonnet 5

@kriszyp
kriszyp merged commit a716d5d into main Jul 31, 2026
27 checks passed
kriszyp added a commit that referenced this pull request Jul 31, 2026
- die() and the final RESULT block now write via fs.writeSync(1, ...)
  instead of process.stdout.write() — the latter races process.exit()
  when stdout is a pipe (the standard --json setup), which can drop the
  RESULT line once output crosses ~64 KiB.
- main().catch() no longer assumes the rejection is an Error (e?.message
  ?? String(e)).
- Declining the first confirmation prompt now emits an explicit
  RESULT: {"ok":false,"error":"aborted",...} line under --json instead
  of exiting silently.
- --cm-trigger no longer bypasses the deploy-confirmation prompt when
  --yes isn't also set — it only changes the prompt's wording. Only
  --cm-trigger + --yes auto-confirms.
- A CM-trigger failure is now terminal (nonzero exit, RESULT ok:false)
  when --cm-trigger was explicitly requested, preserving pushed/version
  state — previously it was swallowed to stderr and the run reported
  ok:true with cmTriggered:false, so a dispatch caller couldn't tell
  "deploy skipped" from "deploy attempted and failed".

Extracted the decision logic (resolveDeployAnswer, buildAbortedResult,
buildCmFailureResult) into pure, exported functions with unit test
coverage in unitTests/scripts/patchRelease.test.mjs, since main() itself
has no seams for testing without live git/gh state.

Addresses review feedback on PR #638 from gemini-code-assist, cb1kenobi,
and kriszyp.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@kriszyp
kriszyp deleted the kris/patch-release-machine-flags branch July 31, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants