Round-2 review fixes for the GITHUB_OUTPUT helper and the release scripts
that emit through it.
emitGithubOutputs (scripts/release/lib/github-output.ts):
- Replace the key newline/CR check with a full GitHub-Actions-safe charset
check: /^[A-Za-z_][A-Za-z0-9_-]*$/. A key containing "=" or whitespace
would silently corrupt the key=value line; rejecting up-front is
strictly safer. Value validation (single-line) is unchanged — "=" in
values is legal because GitHub splits on the first "=".
- Update the docblock accordingly.
prerelease.ts:
- Remove the dead `?? getCurrentVersion(scope)` fallback. The empty-list
guard above makes packages[0] guaranteed, and the fallback would have
masked a package.json missing its version field by emitting a version
divergent from what the loop publishes. Fail loudly with an explicit
exit instead.
- Drop the now-unused getCurrentVersion import.
- Add a comment above the dry-run emitGithubOutputs call explaining that
emitting in dry-run is safe — the publish workflow gates publish + the
verify guard on inputs.dry-run != true, so the dry-run emission only
serves local/e2e contract verification.
publish-release.ts:
- Hoist getPackagesForScope + empty-list guard above the prerelease-suffix
and registry checks. A misconfigured scope now fails with the clear
"no packages found" error instead of a misleading "not greater than
published" one. Loop is unchanged.
github-output.test.ts:
- Loosen the key-newline assertion from the JSON.stringify-coupled
/bad\\nkey/ to the stable /alphanumeric/ phrase from the new message.
- Add tests: "=" in key throws, space in key throws, empty key throws,
and "=" in value is accepted and written verbatim (note=a=b).
- Move vi.restoreAllMocks() to the top of afterEach so spies cannot leak
into env restore + rmSync cleanup.
Call sites audited:
- emitGithubOutputs: only ever called with {version, scope} (prerelease,
publish-release) — all valid under the new charset.
- publishVersion derivation: only used inside prerelease.ts main().
- getCurrentVersion: still imported by publish-release.ts, bump-prerelease.ts,
prepare-release.ts; only the prerelease.ts import was removed.
- getPackagesForScope hoist in publish-release.ts: `packages` was only
read inside the publish loop below; nothing earlier depended on it.
Hardens the new GITHUB_OUTPUT emission path so a malformed value can't smuggle
extra `key=value` lines into the workflow's step outputs, and so the workflow's
"Verify publish step emitted version" guard can't be fooled by a publish that
did nothing.
emitGithubOutputs now validates every key/value for `\n`/`\r` BEFORE the
GITHUB_OUTPUT early-return — a malformed value is a caller bug and should fail
loudly even when running locally. A multi-line value would need the heredoc
form, which this helper deliberately does not support.
prerelease.ts and publish-release.ts now fail loud when getPackagesForScope
returns an empty list. Without this, the new GITHUB_OUTPUT emission would make
the workflow's "Verify publish step emitted version" guard pass on a run that
published nothing — previously the missing output made such a run fail. The
guard runs BEFORE the dry-run branch in prerelease.ts. In publish-release.ts,
the inline iteration of getPackagesForScope(scope) is hoisted to a `packages`
const so the same guard fires before the publish loop.
The "no-op when GITHUB_OUTPUT is unset" test now spies on fs.appendFileSync
and asserts it wasn't called (the previous read of the unrelated temp file
was vacuously true). New tests cover newline/CR in value and newline in key.
The prerelease.ts usage string previously advertised `[--suffix <label>]`,
but the script never parses --suffix (suffix handling lives in
bump-prerelease.ts per the header comment). Removed.
Call sites enumerated:
- emitGithubOutputs: prerelease.ts (dry-run + post-publish), publish-release.ts
- getPackagesForScope: prerelease.ts, publish-release.ts (this commit);
bump-prerelease.ts, prepare-release.ts, versions.ts (not changed — out of
scope for this hardening)
Verification:
- npx vitest run --config scripts/release/vitest.config.mts → 91 passed
- Red-green for the newline validation: temporarily removed the validation,
the 3 new newline/CR tests failed (assertion: expected fn to throw); restored,
back to green.
- E2E: GITHUB_OUTPUT="$OUT" pnpm release:prerelease:dry succeeded and the
output file contained `version=1.59.5` and `scope=monorepo`.
Note: Fix 2's empty-list guard fires only on a misconfigured scope (no unit
test reachable — prerelease.ts is outside the vitest include glob and the
guard is boundary validation against a misconfigured scope, not a behavior
worth contriving a test harness for).
prerelease.ts published canaries successfully but never wrote the
version output the publish-release workflow's "Verify publish step
emitted version" guard reads, so every canary dispatch ended red after
a successful publish. Extract the GITHUB_OUTPUT append (previously
inline in publish-release.ts) into a shared lib/github-output.ts helper
and call it from both publish scripts.
Call-site enumeration:
- emitGithubOutputs: declared lib/github-output.ts; called from
prerelease.ts (dry-run path + after publish) and publish-release.ts
(replaces the inline appendFileSync block, same version=/scope= keys).
- No symbols removed; fs import in publish-release.ts still used (3
remaining call sites).
Maxim