Skip to content

fix: write buildVersion/androidVersionCode to $GITHUB_OUTPUT - #301

Merged
frostebite merged 1 commit into
mainfrom
fix/cli-github-outputs
Oct 3, 2026
Merged

frostebite merged 1 commit into
mainfrom
fix/cli-github-outputs

Conversation

@frostebite

@frostebite frostebite commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Changes

  • core.setOutput writes a real record to $GITHUB_OUTPUT instead of only printing (mock) Output "<key>" is set to "<value>". It has printed-and-returned since the first CLI commit, which was harmless while the CLI was the action.
  • It stopped being harmless when unity-builder became a subprocess wrapper (#844, first shipped in unity-builder v6.0.0). The action runs the CLI as a child and relies on it to publish buildVersion/androidVersionCode, on the reasoning that $GITHUB_OUTPUT is inherited by the child. Nothing ever published them, so both outputs came out empty on every Unity build going through the CLI.
  • Fixing it here means unity-builder#854 - which works around the symptom by scraping that log line - is not needed, and the fix reaches users on a CLI release without waiting for a unity-builder one, since cliVersion defaults to latest.

The implementation mirrors the real @actions/core: append to $GITHUB_OUTPUT when it is set, and keep the printed line only as the standalone fallback when there is no runner. The delimiter form is load-bearing rather than cosmetic - key=value cannot carry a newline, and the value is a user-supplied version string.

Checklist

  • Read the contribution guide and accept the code of conduct
  • Readme (updated or not needed)
  • Tests (added, updated or not needed)

Notes for reviewers

  • src/module/actions/core.test.ts (new) covers the record shape, a value containing newlines, append-not-overwrite across several outputs, and the printed fallback with no $GITHUB_OUTPUT. src/model/output.test.ts gains an end-to-end case asserting Output.setBuildVersion / setAndroidVersionCode land in the file.
  • dist/index.js is deliberately untouched. It is already stale on main - last regenerated 2026-08-14 (Monorepo step 1: orchestrator + unity-engine-core in-repo, path-scoped review and CI #78), roughly 15 src/ commits ago - and nothing reads it: package.json points both main and bin at src/index.ts. bun run build cannot regenerate it on a fresh checkout for an unrelated reason: plugins/steam-workshop maps non-Bun conditions to ./dist/index.js, and plugin dist/ is not checked in. Happy to fold a regenerated bundle in here if you'd rather.
  • Needs a CLI release to reach users; unity-builder#854's workaround can be dropped once one ships.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Workflow outputs are now written to the configured output file, including values that span multiple lines.
    • Multiple outputs are appended without overwriting earlier values.
  • Bug Fixes
    • When no output file is configured, output continues to be displayed in the console.

… them

core.setOutput logged `(mock) Output "<key>" is set to "<value>"` and
returned; it never wrote the file a runner hands the step. The shim has
behaved that way since the first CLI commit, which was harmless while the
CLI *was* the action.

It stopped being harmless when unity-builder became a subprocess wrapper
(#844, first shipped in v6.0.0): the action runs the CLI as a child and
relies on it to publish buildVersion/androidVersionCode, since
$GITHUB_OUTPUT is inherited. Nothing published them, so both outputs
arrived empty on every Unity build.

Mirror the real @actions/core split - append to $GITHUB_OUTPUT when it is
set, and keep the printed line only as the standalone fallback. The
delimiter form is load-bearing: the value is a user-supplied version
string, and `key=value` cannot carry a newline.

Refs game-ci/unity-builder#854

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

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 35807d0d-edea-42a7-aea4-4d05e5470365
📥 Commits

Reviewing files that changed from the base of the PR and between 86908fe and 7889181.

📒 Files selected for processing (3)
  • src/model/output.test.ts
  • src/module/actions/core.test.ts
  • src/module/actions/core.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

core.setOutput now appends delimiter-form records to the file specified by GITHUB_OUTPUT. When the variable is absent, it continues to log mock output. Tests cover file writing, multiline values, multiple outputs, fallback logging, and version setter integration.

Changes

Action output writing

Layer / File(s) Summary
Write and verify action outputs
src/module/actions/core.ts, src/module/actions/core.test.ts, src/model/output.test.ts
core.setOutput writes delimited records to GITHUB_OUTPUT when configured and retains mock logging otherwise. Tests cover multiline values, multiple outputs, cleanup, and version setter output.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 78891

This change makes buildVersion and androidVersionCode outputs reach the runner output file. No actionable merge risk was found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 78891

Fixed output names and randomly delimited records limit the risks from user-supplied version strings. No introduced exploit was demonstrated, but downstream consumption, interrupted writes, and release compatibility remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is the CLI’s two version outputs and their external consumers. Separate control of GITHUB_OUTPUT could redirect append operations to another process-writable file, but no reachable lower-trust path to that control was established.

Trust Boundaries and Controls

  • inferred — User-supplied version data crosses into the runner output protocol. Fixed production keys and unpredictable delimiters constrain ordinary newline-based record injection. This does not establish how downstream consumers validate or use the resulting values.

Resilience and Maintainability Implications

  • inferred — Synchronous emission avoids deferred application-level writes, but does not prove crash atomicity or interprocess ordering. Handling partial records and repeated output names depends on runner semantics unavailable in the supplied evidence; no resulting security failure was demonstrated.

Hardening Proposals

  • proposed — Validate the external runner and wrapper contract across supported CLI upgrade and rollback combinations, including repeated outputs and failed or interrupted appends, before relying on unverified recovery behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: writing buildVersion and androidVersionCode to $GITHUB_OUTPUT.
Description check ✅ Passed The description includes the required Changes and Checklist sections and explains the problem, implementation, tests, and release context. The README checklist item remains unchecked, but this does no…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@frostebite
frostebite merged commit 7da0cea into main Oct 3, 2026
18 checks passed
@frostebite
frostebite deleted the fix/cli-github-outputs branch October 3, 2026 23:17
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.

1 participant