Skip to content

feat: Official CLI Support — game-ci#813

Open
frostebite wants to merge 8 commits intomainfrom
feature/cli-support
Open

feat: Official CLI Support — game-ci#813
frostebite wants to merge 8 commits intomainfrom
feature/cli-support

Conversation

@frostebite
Copy link
Member

@frostebite frostebite commented Mar 5, 2026

Summary

Adds official CLI support to unity-builder, distributed as the game-ci command. Resolves #812.

Commands

  • game-ci build -- Build a Unity project locally or via Docker, mirroring all action inputs as CLI flags
  • game-ci activate -- Verify Unity license configuration (serial, license file, or floating license server)
  • game-ci orchestrate -- Run builds via orchestrator providers (AWS ECS, Kubernetes, local-docker)
  • game-ci cache list|restore|clear -- Inspect and manage build caches
  • game-ci status -- Show workspace info, Unity version detection, environment, and Docker availability
  • game-ci version -- Show version info
  • game-ci update -- Self-update to the latest release (standalone binary only)

Distribution Strategy

Primary: GitHub Releases with Standalone Binaries

Every release builds standalone binaries (no Node.js required) for:

Platform Architecture Asset Name
Linux x64 game-ci-linux-x64
Linux arm64 game-ci-linux-arm64
macOS x64 game-ci-macos-x64
macOS arm64 game-ci-macos-arm64
Windows x64 game-ci-windows-x64.exe

All binaries are uploaded as release assets with SHA256 checksums (checksums.txt).

Install Scripts

Linux / macOS:

curl -fsSL https://raw.githubusercontent.com/game-ci/unity-builder/main/install.sh | sh

Windows (PowerShell):

irm https://raw.githubusercontent.com/game-ci/unity-builder/main/install.ps1 | iex

Environment variables:

  • GAME_CI_VERSION -- Install a specific version (e.g., v2.0.0)
  • GAME_CI_INSTALL -- Custom install directory (default: ~/.game-ci/bin)

Self-Update

game-ci update              # Update to latest
game-ci update --version v2.1.0  # Update to specific version
game-ci update --force      # Force reinstall

npm (Optional, enabled on release)

npm install -g unity-builder
npx game-ci build --target-platform StandaloneLinux64

Future Providers

  • winget (Windows Package Manager)
  • Homebrew (macOS/Linux)
  • Chocolatey / Scoop (Windows)

Design

The CLI reuses the existing unity-builder modules -- Input, BuildParameters, Orchestrator, Docker, MacBuilder -- rather than duplicating logic. A thin input-mapper layer bridges yargs CLI flags into the existing Cli.options mechanism that Input.getInput() and OrchestratorOptions.getInput() already query.

Key files

File Purpose
src/cli.ts Entry point with shebang, yargs setup, command registration
src/cli/input-mapper.ts Maps CLI args to the existing Input/OrchestratorOptions system
src/cli/commands/build.ts Build command with all action input flags
src/cli/commands/activate.ts License activation verification
src/cli/commands/orchestrate.ts Orchestrator build delegation
src/cli/commands/cache.ts Cache management (list/restore/clear)
src/cli/commands/status.ts Workspace and environment status
src/cli/commands/version.ts Version info
src/cli/commands/update.ts Self-update from GitHub Releases
.github/workflows/release-cli.yml Build binaries, upload to release, optional npm publish
install.sh curl-pipe-sh installer for Linux/macOS
install.ps1 PowerShell installer for Windows
src/cli/__tests__/input-mapper.test.ts 16 tests for CLI argument mapping
src/cli/__tests__/commands.test.ts 31 tests for command module exports and flags
src/cli/__tests__/cli-integration.test.ts 9 integration tests spawning the CLI process

Release Workflow (release-cli.yml)

Triggers on GitHub Release published or manual workflow_dispatch.

  1. Build binaries -- Matrix build across 5 platform/arch combos using pkg
  2. Create checksums and upload -- SHA256 checksums, upload all assets to the release
  3. Publish to npm (optional) -- Enabled on release events, with npm provenance

Tracking

Test plan

  • npx tsc --noEmit passes (zero TypeScript errors)
  • ESLint passes with zero errors on all new files
  • Pre-commit hooks pass (prettier, eslint, jest, ncc build)
  • game-ci --help displays all commands (including update)
  • game-ci build --help shows all build flags with descriptions
  • game-ci version shows correct version info
  • game-ci status shows workspace detection and environment
  • game-ci cache list reports cache status
  • game-ci activate correctly reports missing license with exit code 2
  • game-ci update --help shows force and version flags
  • 56 unit/integration tests pass (16 input-mapper + 31 commands + 9 CLI integration)
  • Release workflow builds standalone binaries for 5 platforms
  • Install scripts auto-detect platform, verify checksums, configure PATH
  • End-to-end build test with a Unity project (requires Unity license)
  • Orchestrator integration test with AWS/K8s provider
  • Self-update end-to-end test (requires a published release)

Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Adds a full CLI (game-ci) with commands: build, activate, orchestrate, cache, status, version, and update.
    • Provides platform installers (install.sh, install.ps1) and a self-update workflow for the CLI.
  • Tests

    • Adds integration and unit tests covering CLI commands, help/version output, and argument-to-option mapping.
  • Chores

    • Adds CLI packaging, release workflow, a runnable CLI script, and updates Node engine requirement.

…commands

Introduces a yargs-based CLI entry point (src/cli.ts) distributed as the
`game-ci` command. The CLI reuses existing unity-builder modules — Input,
BuildParameters, Orchestrator, Docker, MacBuilder — so the same build
engine powers both the GitHub Action and the standalone CLI.

Commands: build, activate, orchestrate, cache (list/restore/clear),
status, version.

Closes #812

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@frostebite frostebite added enhancement New feature or request orchestrator Orchestrator module LTS 2.0 Orchestrator LTS v2.0 milestone labels Mar 5, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an official CLI ("game-ci") with yargs-based commands, input mapping, packaging/release workflows, cross-platform installers, and tests; introduces command implementations for build, activate, orchestrate, cache, status, version, and update, plus packaging and release automation.

Changes

Cohort / File(s) Summary
Package & Packaging
package.json
Added bin.game-ci, pkg packaging config, game-ci script, bumped Node engine to >=20, and added yargs, @types/yargs, and pkg deps/devDeps.
CLI entry & input mapping
src/cli.ts, src/cli/input-mapper.ts
New yargs-based CLI entry and mapCliArgumentsToInput + CliArguments to translate kebab-case flags into internal Cli.options and disable GH input.
Commands
src/cli/commands/...
src/cli/commands/build.ts, src/cli/commands/activate.ts, src/cli/commands/orchestrate.ts, src/cli/commands/cache.ts, src/cli/commands/status.ts, src/cli/commands/version.ts, src/cli/commands/update.ts
Added seven command modules implementing build (local/docker/mac/orchestrator flows), activation/licensing logic, orchestrator invocation, cache management (list/restore/clear), workspace status reporting, version display, and a self-update workflow (download/replace executable).
Tests
src/cli/__tests__/*
src/cli/__tests__/cli-integration.test.ts, src/cli/__tests__/commands.test.ts, src/cli/__tests__/input-mapper.test.ts
Added CLI integration tests (spawned ts-node CLI), command metadata/flag validation tests (including update), and comprehensive input-mapper unit tests.
Release workflow & installers
.github/workflows/release-cli.yml, install.sh, install.ps1
New GitHub Actions workflow to build/package/publish CLI binaries (pkg), checksum/upload artifacts, optional npm publish; added POSIX and PowerShell installers with download, checksum verification, and PATH guidance.
CI adjustments
.github/workflows/integrity-check.yml
Updated CI Node version from 18 to 20 for integrity checks.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI as "CLI\n(src/cli.ts)"
    participant InputMapper as "Input Mapper\n(mapCliArgumentsToInput)"
    participant BuildParams as "BuildParameters"
    participant PlatformSetup as "PlatformSetup"
    participant LocalBuilder as "Local Builder\n(Docker/Mac)"
    participant Orchestrator as "Orchestrator"

    User->>CLI: game-ci build [flags]
    CLI->>InputMapper: mapCliArgumentsToInput(cliArguments)
    Note right of InputMapper: Cli.options set,\nmode='cli'
    InputMapper-->>CLI: mapped options
    CLI->>BuildParams: create BuildParameters / ImageTag

    alt providerStrategy == 'local'
        CLI->>PlatformSetup: setup()
        PlatformSetup-->>CLI: environment ready
        alt platform == macOS
            CLI->>LocalBuilder: MacBuilder.build(params)
        else
            CLI->>LocalBuilder: Docker.build(params, image)
        end
        LocalBuilder-->>CLI: build results
    else
        CLI->>Orchestrator: Orchestrator.run(buildParameters, image)
        Orchestrator-->>CLI: orchestration result
    end

    CLI->>User: output metadata (exit code, path, version)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • webbertakken
  • GabLeRoux
  • EricRibeiro

Poem

🐇
I hopped through code at break of dawn,
A tiny CLI was stitched and drawn.
Build, cache, update — hopping free,
game-ci leaps from shrub to tree.
Thump, nibble, ship — hooray for me!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'feat: Official CLI Support — game-ci' clearly and concisely summarizes the main change: adding official CLI support named game-ci to the unity-builder project.
Description check ✅ Passed The PR description is comprehensive, covers all required sections (changes, related issues, test plan), and thoroughly explains the CLI implementation, distribution strategy, design, and testing.
Linked Issues check ✅ Passed The PR fully implements the objectives from issue #812: delivers the game-ci CLI with all planned commands (build, activate, orchestrate, cache, status, version, update), mirrors Action inputs as CLI flags, reuses existing modules, supports standalone binaries and npm distribution, and defines proper exit codes.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing official CLI support: new CLI commands, input mapping, workflows for binary packaging, installer scripts, and comprehensive tests. The Node 20 update in workflows aligns with yargs dependency requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/cli-support

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 and usage tips.

Add .github/workflows/publish-cli.yml for publishing the CLI to npm on
release or via manual workflow_dispatch with dry-run support.

Add comprehensive test coverage for the CLI:
- input-mapper.test.ts: 16 tests covering argument mapping, boolean
  conversion, yargs internal property filtering, and Cli.options population
- commands.test.ts: 26 tests verifying command exports, builder flags,
  default values, and camelCase aliases for all six commands
- cli-integration.test.ts: 8 integration tests spawning the CLI process
  to verify help output, version info, and error handling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (2)
src/cli/commands/orchestrate.ts (1)

14-149: Consider extracting shared CLI option registration.

This option block largely duplicates src/cli/commands/build.ts. A shared helper would reduce drift and keep defaults/descriptions consistent across commands.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/orchestrate.ts` around lines 14 - 149, The CLI option block
duplicated in the builder function in orchestrate.ts should be extracted into a
shared helper (e.g., registerCommonBuildOptions or addOrchestrateOptions) that
encapsulates the repeated .option(...) calls and defaults; move that helper into
a new module (e.g., cli/options.ts) and replace the large option chains in
builder of orchestrate.ts and the equivalent builder in build.ts with a call to
this helper (pass the yargs instance and any command-specific overrides like
providerStrategy defaults), ensuring unique symbols referenced are the builder
function in orchestrate.ts, the corresponding builder in build.ts, and the new
registerCommonBuildOptions helper so both commands import and call it.
src/cli/commands/cache.ts (1)

57-61: Avoid double failure reporting in CLI error flow.

Calling core.setFailed(...) and rethrowing can produce duplicate error output/noisy stacks in yargs CLIs. Prefer normalizing the message and returning after setting failure state.

Proposed refactor
-    } catch (error: any) {
-      core.setFailed(`Cache operation failed: ${error.message}`);
-
-      throw error;
+    } catch (error: unknown) {
+      const message = error instanceof Error ? error.message : String(error);
+      core.setFailed(`Cache operation failed: ${message}`);
+      return;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/cache.ts` around lines 57 - 61, The catch block in
src/cli/commands/cache.ts currently calls core.setFailed(...) and then rethrows
the error, causing duplicate/noisy output; update the error handling in that
catch (the code using core.setFailed and throw error) to instead normalize the
error message, call core.setFailed(normalizedMessage) and then return (or set a
non-zero exit/return code) without rethrowing; remove the throw error to avoid
double reporting while preserving the failure state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Around line 62-70: The package.json currently pins "yargs" at "^18.0.0", which
is incompatible with the repository's Node engine (>=18.x); change the "yargs"
dependency entry in package.json to "17.7.2" (or "^17.7.2") to match Node >=12
compatibility, then regenerate lockfiles by running your package manager (npm
install or yarn install) so package-lock.json / yarn.lock reflects the
downgrade; ensure any related type package "@types/yargs" remains compatible
after the change and adjust it only if type errors appear.

In `@src/cli.ts`:
- Line 4: Remove the stale eslint-disable directive "eslint-disable-next-line
import/no-unresolved" from the top of the file; locate the exact comment and
delete that line so eslint-comments/no-unused-disable no longer flags it, and
only re-add a disable if an actual unresolved-import warning later reappears.
- Around line 29-36: In the main function's catch block (around cli.parse()),
detect YError cases and ensure they exit with code 2: when error.name ===
'YError' call core.error with the error message (or the error itself) and then
invoke process.exit(2); keep the existing behavior for non-YError errors (log
with core.error) but ensure they still exit (e.g., process.exit(1) or propagate)
as appropriate so parser/configuration errors from cli.parse() are mapped to
exit code 2.

In `@src/cli/commands/activate.ts`:
- Around line 42-45: Replace the direct environment/CLI reads for unitySerial,
unityLicense, and licensingServer with the unified Input accessors so activation
goes through the same input resolution pipeline as other commands: stop using
process.env.UNITY_SERIAL, process.env.UNITY_LICENSE and
cliArguments.unityLicensingServer and instead read those values via the
project's Input API (e.g., Input.get / Input.getOptional or the project's
equivalent) for the keys used by the rest of the CLI (UNITY_SERIAL,
UNITY_LICENSE, UNITY_LICENSING_SERVER); update the variables unitySerial,
unityLicense, and licensingServer in activate.ts to pull from Input and preserve
the same fallback/empty-string behavior.

In `@src/cli/commands/cache.ts`:
- Around line 33-35: The file always sets cacheDirectory to
(cliArguments.cacheDir as string) || path.join(projectPath, 'Library'), making
the restoreCache guard dead and causing restore/clear to silently use
<project>/Library; change this so --cache-dir is action-specific: do not default
cacheDirectory to 'Library' at the top-level — only resolve a default when
handling the specific actions that need it (e.g., inside the restore and clear
command handlers or inside restoreCache), or keep cacheDir undefined unless
provided and then in restoreCache (function restoreCache) compute
path.join(projectPath, 'Library') only when cacheDir is missing and the action
is restore/clear; update references to projectPath and cacheDirectory
accordingly so other actions ignore the default.
- Around line 122-137: The cache restore command currently only lists archives
and exits successfully; change the behavior to fail fast until restore is
implemented: after computing sorted (using cacheFiles, cacheDirectory, sorted),
replace the final core.info guidance messages with a non-success exit (e.g.,
call core.setFailed('cache restore not implemented - no restore performed') or
throw an Error or set process.exitCode = 1) so CI/automation surfaces the
missing restore step instead of succeeding silently.

---

Nitpick comments:
In `@src/cli/commands/cache.ts`:
- Around line 57-61: The catch block in src/cli/commands/cache.ts currently
calls core.setFailed(...) and then rethrows the error, causing duplicate/noisy
output; update the error handling in that catch (the code using core.setFailed
and throw error) to instead normalize the error message, call
core.setFailed(normalizedMessage) and then return (or set a non-zero exit/return
code) without rethrowing; remove the throw error to avoid double reporting while
preserving the failure state.

In `@src/cli/commands/orchestrate.ts`:
- Around line 14-149: The CLI option block duplicated in the builder function in
orchestrate.ts should be extracted into a shared helper (e.g.,
registerCommonBuildOptions or addOrchestrateOptions) that encapsulates the
repeated .option(...) calls and defaults; move that helper into a new module
(e.g., cli/options.ts) and replace the large option chains in builder of
orchestrate.ts and the equivalent builder in build.ts with a call to this helper
(pass the yargs instance and any command-specific overrides like
providerStrategy defaults), ensuring unique symbols referenced are the builder
function in orchestrate.ts, the corresponding builder in build.ts, and the new
registerCommonBuildOptions helper so both commands import and call it.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ae38484e-46f1-444b-b655-b92b497e7fd8

📥 Commits

Reviewing files that changed from the base of the PR and between 9d47543 and 5a42214.

⛔ Files ignored due to path filters (2)
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (9)
  • package.json
  • src/cli.ts
  • src/cli/commands/activate.ts
  • src/cli/commands/build.ts
  • src/cli/commands/cache.ts
  • src/cli/commands/orchestrate.ts
  • src/cli/commands/status.ts
  • src/cli/commands/version.ts
  • src/cli/input-mapper.ts

Comment on lines +62 to +70
"yargs": "^18.0.0"
},
"devDependencies": {
"@types/base-64": "^1.0.0",
"@types/jest": "^27.4.1",
"@types/node": "^17.0.23",
"@types/semver": "^7.3.9",
"@types/uuid": "^9.0.0",
"@types/yargs": "^17.0.35",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Project constraints ==="
if [ -f package.json ]; then
  echo "Node engine from package.json:"
  jq -r '.engines.node // "not specified"' package.json
  echo ""
  echo "yargs version from dependencies:"
  jq -r '.dependencies.yargs // "not found"' package.json
fi

echo ""
if [ -f tsconfig.json ]; then
  echo "TypeScript module setting:"
  jq -r '.compilerOptions.module // "not specified"' tsconfig.json
fi

echo ""
echo "=== yargs@18.0.0 metadata ==="
npm view yargs@18.0.0 engines type main --json 2>/dev/null || echo "Failed to fetch"

echo ""
echo "=== yargs@17.7.2 metadata ==="
npm view yargs@17.7.2 engines type main --json 2>/dev/null || echo "Failed to fetch"

Repository: game-ci/unity-builder

Length of output: 275


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Project constraints ==="
echo "Node engine from package.json:"
grep -A 1 '"engines"' package.json || echo "not specified"

echo ""
echo "yargs version from dependencies:"
grep '"yargs"' package.json

echo ""
echo "TypeScript module setting from tsconfig.json:"
cat tsconfig.json | head -30

echo ""
echo "=== yargs@18.0.0 full package info ==="
npm view yargs@18.0.0 2>/dev/null | head -50 || echo "Failed to fetch"

echo ""
echo "=== yargs@17.7.2 full package info ==="
npm view yargs@17.7.2 2>/dev/null | head -50 || echo "Failed to fetch"

Repository: game-ci/unity-builder

Length of output: 3159


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== yargs@18.0.0 detailed metadata ==="
npm view yargs@18.0.0 --json | jq '.type, .engines, .main, .exports | keys' 2>/dev/null || npm view yargs@18.0.0 engines.node type main 2>/dev/null || echo "Fetching direct metadata..."

echo ""
echo "=== yargs@17.7.2 detailed metadata ==="
npm view yargs@17.7.2 --json | jq '.type, .engines, .main, .exports | keys' 2>/dev/null || npm view yargs@17.7.2 engines.node type main 2>/dev/null || echo "Fetching direct metadata..."

echo ""
echo "=== Checking actual installed yargs versions in node_modules (if available) ==="
if [ -d "node_modules/yargs" ]; then
  echo "yargs package.json type and engines:"
  jq '.type, .engines' node_modules/yargs/package.json
else
  echo "yargs not installed in node_modules"
fi

Repository: game-ci/unity-builder

Length of output: 433


yargs@18 is incompatible with the current Node engine declaration.

yargs@18 requires Node ^20.19.0 || ^22.12.0 || >=23, but the project declares >=18.x, creating a version mismatch that can cause installation or runtime failures.

Downgrade to yargs@17.7.2, which supports Node >=12:

-    "yargs": "^18.0.0"
+    "yargs": "^17.7.2"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"yargs": "^18.0.0"
},
"devDependencies": {
"@types/base-64": "^1.0.0",
"@types/jest": "^27.4.1",
"@types/node": "^17.0.23",
"@types/semver": "^7.3.9",
"@types/uuid": "^9.0.0",
"@types/yargs": "^17.0.35",
"yargs": "^17.7.2"
},
"devDependencies": {
"@types/base-64": "^1.0.0",
"@types/jest": "^27.4.1",
"@types/node": "^17.0.23",
"@types/semver": "^7.3.9",
"@types/uuid": "^9.0.0",
"@types/yargs": "^17.0.35",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 62 - 70, The package.json currently pins "yargs"
at "^18.0.0", which is incompatible with the repository's Node engine (>=18.x);
change the "yargs" dependency entry in package.json to "17.7.2" (or "^17.7.2")
to match Node >=12 compatibility, then regenerate lockfiles by running your
package manager (npm install or yarn install) so package-lock.json / yarn.lock
reflects the downgrade; ensure any related type package "@types/yargs" remains
compatible after the change and adjust it only if type errors appear.

#!/usr/bin/env node

import yargs from 'yargs';
// eslint-disable-next-line import/no-unresolved
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove the stale eslint-disable directive.

Line 4 triggers eslint-comments/no-unused-disable and should be deleted unless a real unresolved-import warning is reintroduced.

🧰 Tools
🪛 ESLint

[error] 4-4: 'import/no-unresolved' rule is disabled but never reported.

(eslint-comments/no-unused-disable)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` at line 4, Remove the stale eslint-disable directive
"eslint-disable-next-line import/no-unresolved" from the top of the file; locate
the exact comment and delete that line so eslint-comments/no-unused-disable no
longer flags it, and only re-add a disable if an actual unresolved-import
warning later reappears.

Comment on lines +29 to +36
async function main() {
try {
await cli.parse();
} catch (error: any) {
if (error.name !== 'YError') {
core.error(`Error: ${error.message}`);
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Map parser/configuration errors to exit code 2.

The current YError path does not enforce the config-error exit code contract.

🔧 Proposed fix
 async function main() {
   try {
     await cli.parse();
   } catch (error: any) {
-    if (error.name !== 'YError') {
-      core.error(`Error: ${error.message}`);
-    }
+    if (error?.name === 'YError') {
+      process.exitCode = 2;
+      return;
+    }
+    process.exitCode = process.exitCode || 1;
+    core.error(`Error: ${error?.message ?? String(error)}`);
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 29 - 36, In the main function's catch block (around
cli.parse()), detect YError cases and ensure they exit with code 2: when
error.name === 'YError' call core.error with the error message (or the error
itself) and then invoke process.exit(2); keep the existing behavior for
non-YError errors (log with core.error) but ensure they still exit (e.g.,
process.exit(1) or propagate) as appropriate so parser/configuration errors from
cli.parse() are mapped to exit code 2.

Comment on lines +42 to +45
const unitySerial = process.env.UNITY_SERIAL;
const unityLicense = process.env.UNITY_LICENSE;
const licensingServer = cliArguments.unityLicensingServer || process.env.UNITY_LICENSING_SERVER || '';

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use unified Input accessors instead of direct env reads.

At Line 42-Line 45, activation bypasses the mapped CLI input pipeline and can behave differently from build/orchestrate input resolution.

🔧 Proposed fix
 import type { CommandModule } from 'yargs';
 import * as core from '@actions/core';
 import { mapCliArgumentsToInput, CliArguments } from '../input-mapper';
+import Input from '../../model/input';
@@
-      const unitySerial = process.env.UNITY_SERIAL;
-      const unityLicense = process.env.UNITY_LICENSE;
-      const licensingServer = cliArguments.unityLicensingServer || process.env.UNITY_LICENSING_SERVER || '';
+      const unitySerial = Input.unitySerial;
+      const unityLicense = Input.unityLicense;
+      const licensingServer = Input.unityLicensingServer || '';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const unitySerial = process.env.UNITY_SERIAL;
const unityLicense = process.env.UNITY_LICENSE;
const licensingServer = cliArguments.unityLicensingServer || process.env.UNITY_LICENSING_SERVER || '';
import type { CommandModule } from 'yargs';
import * as core from '@actions/core';
import { mapCliArgumentsToInput, CliArguments } from '../input-mapper';
import Input from '../../model/input';
const unitySerial = Input.unitySerial;
const unityLicense = Input.unityLicense;
const licensingServer = Input.unityLicensingServer || '';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/activate.ts` around lines 42 - 45, Replace the direct
environment/CLI reads for unitySerial, unityLicense, and licensingServer with
the unified Input accessors so activation goes through the same input resolution
pipeline as other commands: stop using process.env.UNITY_SERIAL,
process.env.UNITY_LICENSE and cliArguments.unityLicensingServer and instead read
those values via the project's Input API (e.g., Input.get / Input.getOptional or
the project's equivalent) for the keys used by the rest of the CLI
(UNITY_SERIAL, UNITY_LICENSE, UNITY_LICENSING_SERVER); update the variables
unitySerial, unityLicense, and licensingServer in activate.ts to pull from Input
and preserve the same fallback/empty-string behavior.

Comment on lines +33 to +35
const projectPath = (cliArguments.projectPath as string) || '.';
const cacheDirectory = (cliArguments.cacheDir as string) || path.join(projectPath, 'Library');

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make --cache-dir action-specific instead of globally defaulting to Library.

cacheDirectory is always populated (Line 34), so the guard in restoreCache (Line 111) is effectively dead. This causes restore/clear to silently target <project>/Library, which doesn’t match the command intent or examples.

Proposed fix
-    const projectPath = (cliArguments.projectPath as string) || '.';
-    const cacheDirectory = (cliArguments.cacheDir as string) || path.join(projectPath, 'Library');
+    const projectPath = (cliArguments.projectPath as string) || '.';
+    const cacheDirectory = (cliArguments.cacheDir as string) || '';

     try {
       switch (action) {
         case 'list': {
-          await listCache(cacheDirectory, projectPath);
+          await listCache(cacheDirectory || path.join(projectPath, 'Library'), projectPath);
           break;
         }

         case 'restore': {
+          if (!cacheDirectory) {
+            throw new Error('--cache-dir is required for restore');
+          }
           await restoreCache(cacheDirectory);
           break;
         }

         case 'clear': {
+          if (!cacheDirectory) {
+            throw new Error('--cache-dir is required for clear');
+          }
           await clearCache(cacheDirectory);
           break;
         }

Also applies to: 110-113

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/cache.ts` around lines 33 - 35, The file always sets
cacheDirectory to (cliArguments.cacheDir as string) || path.join(projectPath,
'Library'), making the restoreCache guard dead and causing restore/clear to
silently use <project>/Library; change this so --cache-dir is action-specific:
do not default cacheDirectory to 'Library' at the top-level — only resolve a
default when handling the specific actions that need it (e.g., inside the
restore and clear command handlers or inside restoreCache), or keep cacheDir
undefined unless provided and then in restoreCache (function restoreCache)
compute path.join(projectPath, 'Library') only when cacheDir is missing and the
action is restore/clear; update references to projectPath and cacheDirectory
accordingly so other actions ignore the default.

Comment on lines +122 to +137
const cacheFiles = fs.readdirSync(cacheDirectory).filter((f) => f.endsWith('.tar') || f.endsWith('.tar.lz4'));
if (cacheFiles.length === 0) {
core.info('No cache archives found to restore.');

return;
}

// Sort by modification time, newest first
const sorted = cacheFiles
.map((f) => ({ name: f, mtime: fs.statSync(path.join(cacheDirectory, f)).mtime }))
.sort((a, b) => b.mtime.getTime() - a.mtime.getTime());

core.info(`Found ${sorted.length} cache archive(s). Latest: ${sorted[0].name}`);
core.info('Use the orchestrator cache system for full restore functionality:');
core.info(' game-ci orchestrate --cache-key <key> ...');
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

cache restore currently succeeds without restoring anything.

The command only discovers archives and prints guidance, then exits successfully. This is misleading for automation and can mask a missing restore step.

Minimal safe behavior until restore is implemented
-  core.info(`Found ${sorted.length} cache archive(s). Latest: ${sorted[0].name}`);
-  core.info('Use the orchestrator cache system for full restore functionality:');
-  core.info('  game-ci orchestrate --cache-key <key> ...');
+  core.info(`Found ${sorted.length} cache archive(s). Latest: ${sorted[0].name}`);
+  throw new Error(
+    'cache restore is not implemented yet in this command. Use `game-ci orchestrate --cache-key <key> ...`.'
+  );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const cacheFiles = fs.readdirSync(cacheDirectory).filter((f) => f.endsWith('.tar') || f.endsWith('.tar.lz4'));
if (cacheFiles.length === 0) {
core.info('No cache archives found to restore.');
return;
}
// Sort by modification time, newest first
const sorted = cacheFiles
.map((f) => ({ name: f, mtime: fs.statSync(path.join(cacheDirectory, f)).mtime }))
.sort((a, b) => b.mtime.getTime() - a.mtime.getTime());
core.info(`Found ${sorted.length} cache archive(s). Latest: ${sorted[0].name}`);
core.info('Use the orchestrator cache system for full restore functionality:');
core.info(' game-ci orchestrate --cache-key <key> ...');
}
const cacheFiles = fs.readdirSync(cacheDirectory).filter((f) => f.endsWith('.tar') || f.endsWith('.tar.lz4'));
if (cacheFiles.length === 0) {
core.info('No cache archives found to restore.');
return;
}
// Sort by modification time, newest first
const sorted = cacheFiles
.map((f) => ({ name: f, mtime: fs.statSync(path.join(cacheDirectory, f)).mtime }))
.sort((a, b) => b.mtime.getTime() - a.mtime.getTime());
core.info(`Found ${sorted.length} cache archive(s). Latest: ${sorted[0].name}`);
throw new Error(
'cache restore is not implemented yet in this command. Use `game-ci orchestrate --cache-key <key> ...`.'
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/cache.ts` around lines 122 - 137, The cache restore command
currently only lists archives and exits successfully; change the behavior to
fail fast until restore is implemented: after computing sorted (using
cacheFiles, cacheDirectory, sorted), replace the final core.info guidance
messages with a non-success exit (e.g., call core.setFailed('cache restore not
implemented - no restore performed') or throw an Error or set process.exitCode =
1) so CI/automation surfaces the missing restore step instead of succeeding
silently.

…mand

Replace the npm-only publish-cli.yml with a comprehensive release-cli.yml
that builds standalone binaries via pkg for all platforms (Linux/macOS/Windows,
x64/arm64), uploads them as GitHub Release assets with SHA256 checksums,
and retains npm publish as an optional job.

Add curl-pipe-sh installer (install.sh) and PowerShell installer (install.ps1)
for one-liner installation from GitHub Releases. Both scripts auto-detect
platform/architecture, verify checksums, and guide PATH configuration.

Add `game-ci update` command for self-updating standalone binaries: checks
GitHub releases for newer versions, downloads the correct platform binary,
verifies it, and atomically replaces the running executable.

Distribution strategy: GitHub Releases (primary), npm (optional), with
winget/Homebrew/Chocolatey/Scoop as future providers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/publish-cli.yml (1)

48-55: Simplify boolean input comparison using the modern inputs context.

Lines 48 and 54 use string comparison (== 'true' / == 'false') with the inputs context. The inputs context (the recommended approach for workflow_dispatch inputs) handles boolean conversion automatically, so the string comparisons are unnecessary. Use the inputs directly: inputs['dry-run'] and !inputs['dry-run'].

Suggested refactor
       - name: Publish (dry run)
-        if: github.event_name == 'workflow_dispatch' && inputs.dry-run == 'true'
+        if: github.event_name == 'workflow_dispatch' && inputs['dry-run']
         run: npm publish --dry-run
         env:
           NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

       - name: Publish
-        if: github.event_name == 'release' || (github.event_name == 'workflow_dispatch' && inputs.dry-run == 'false')
+        if: github.event_name == 'release' || (github.event_name == 'workflow_dispatch' && !inputs['dry-run'])
         run: npm publish --provenance --access public
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-cli.yml around lines 48 - 55, Replace the string
boolean comparisons against the workflow_dispatch input with direct boolean
checks using the inputs context: update the dry-run step condition from
"github.event_name == 'workflow_dispatch' && inputs.dry-run == 'true'" to use
"github.event_name == 'workflow_dispatch' && inputs['dry-run']" and update the
Publish step condition from "... && inputs.dry-run == 'false'" to use "... &&
!inputs['dry-run']"; ensure you modify the conditions that guard the npm publish
--dry-run and npm publish --provenance steps (the steps currently using
inputs.dry-run string comparisons).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/publish-cli.yml:
- Around line 4-5: The workflow file path is wrong and the release trigger and
publish step need hardening: rename `.github/workflows/publish-cli.yml` to
`.github/workflows/release-cli.yml`, update the workflow so the `release`
trigger distinguishes prereleases (e.g., use `types: [published]` plus a
condition checking `github.event.release.prerelease`), and modify the npm
publish step (the step running `npm publish`) to publish prereleases with `--tag
next` while only publishing non-prereleases without an explicit tag (or with
`--tag latest`) so prereleases are not promoted to latest.

In `@src/cli/__tests__/input-mapper.test.ts`:
- Around line 5-8: The teardown in afterEach currently restores mocks and
Cli.options but doesn't reset the global flag GitHub.githubInputEnabled modified
by mapCliArgumentsToInput; update the afterEach block to also reset
GitHub.githubInputEnabled (e.g., set GitHub.githubInputEnabled = undefined or
its default) after jest.restoreAllMocks() and Cli.options = undefined so tests
don't leak state between runs.

---

Nitpick comments:
In @.github/workflows/publish-cli.yml:
- Around line 48-55: Replace the string boolean comparisons against the
workflow_dispatch input with direct boolean checks using the inputs context:
update the dry-run step condition from "github.event_name == 'workflow_dispatch'
&& inputs.dry-run == 'true'" to use "github.event_name == 'workflow_dispatch' &&
inputs['dry-run']" and update the Publish step condition from "... &&
inputs.dry-run == 'false'" to use "... && !inputs['dry-run']"; ensure you modify
the conditions that guard the npm publish --dry-run and npm publish --provenance
steps (the steps currently using inputs.dry-run string comparisons).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f9eb4f67-7105-46c1-976d-d30991a1c862

📥 Commits

Reviewing files that changed from the base of the PR and between 5a42214 and 5bdcf12.

📒 Files selected for processing (4)
  • .github/workflows/publish-cli.yml
  • src/cli/__tests__/cli-integration.test.ts
  • src/cli/__tests__/commands.test.ts
  • src/cli/__tests__/input-mapper.test.ts

Comment on lines +4 to +5
release:
types: [published]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/publish-cli.yml

Repository: game-ci/unity-builder

Length of output: 130


🌐 Web query:

npm publish default dist-tag prerelease versions recommended approach

💡 Result:

  • Default behavior: npm publish (with no --tag) publishes your version under the latest dist-tag. That means npm install your-pkg (with no version/tag) will install whatever is tagged latest. [1], [2]
  • Problem: If you publish a SemVer prerelease like 1.2.0-beta.1 without --tag, you’ll still move latest to that prerelease, and users doing a normal install may get it. [1], [3]
  • Recommended approach (common practice):
    1. Publish prereleases to a non-latest tag (often next, beta, alpha, rc):
      • npm publish --tag next [3]
    2. Tell testers to install explicitly:
      • npm install your-pkg@next (or @beta, etc.) [2], [3]
    3. When ready, publish the stable release without --tag so it becomes latest:
      • npm publish [1], [3]
  • If you need to adjust tags after publishing: use dist-tag commands (e.g., npm dist-tag add your-pkg@1.2.0 latest, npm dist-tag rm your-pkg next). [2]

Sources: npm dist-tags docs and guidance on publishing prereleases with explicit tags. [1], [2], [3], [4]


🏁 Script executed:

find .github/workflows -type f -name "*.yml" -o -name "*.yaml" | head -20

Repository: game-ci/unity-builder

Length of output: 443


🏁 Script executed:

ls -la .github/workflows/ 2>/dev/null || echo "Directory not found"

Repository: game-ci/unity-builder

Length of output: 878


🏁 Script executed:

cat -n .github/workflows/release-cli.yml

Repository: game-ci/unity-builder

Length of output: 6284


Correct file path and handle prerelease publication separately to prevent accidental promotion to latest.

The workflow file is .github/workflows/release-cli.yml (not publish-cli.yml). The trigger release: types: [published] fires for both stable releases and prereleases, and the npm publish command at line 168 does not specify a dist-tag, defaulting to latest. This causes prerelease versions to be tagged as latest, which users will receive on standard installations.

Separate prerelease publishing to use --tag next and add a condition to prevent prereleases from being tagged as latest:

Proposed hardening
+      - name: Publish prerelease
+        if: github.event_name == 'release' && github.event.release.prerelease
+        run: npm publish --provenance --access public --tag next
+        env:
+          NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
+
       - name: Publish to npm
-        if: >-
-          (github.event_name == 'release') || (github.event_name == 'workflow_dispatch' && inputs.publish-npm)
+        if: (github.event_name == 'release' && !github.event.release.prerelease) || (github.event_name == 'workflow_dispatch' && inputs.publish-npm)
         run: npm publish --provenance --access public
         env:
           NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-cli.yml around lines 4 - 5, The workflow file path
is wrong and the release trigger and publish step need hardening: rename
`.github/workflows/publish-cli.yml` to `.github/workflows/release-cli.yml`,
update the workflow so the `release` trigger distinguishes prereleases (e.g.,
use `types: [published]` plus a condition checking
`github.event.release.prerelease`), and modify the npm publish step (the step
running `npm publish`) to publish prereleases with `--tag next` while only
publishing non-prereleases without an explicit tag (or with `--tag latest`) so
prereleases are not promoted to latest.

Comment on lines +5 to +8
afterEach(() => {
jest.restoreAllMocks();
Cli.options = undefined;
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reset GitHub.githubInputEnabled in teardown to avoid cross-test state leakage.

mapCliArgumentsToInput disables GitHub input globally, but teardown currently restores only Cli.options. This can make other tests order-dependent.

✅ Suggested fix
 afterEach(() => {
   jest.restoreAllMocks();
   Cli.options = undefined;
+  GitHub.githubInputEnabled = true;
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
afterEach(() => {
jest.restoreAllMocks();
Cli.options = undefined;
});
afterEach(() => {
jest.restoreAllMocks();
Cli.options = undefined;
GitHub.githubInputEnabled = true;
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/__tests__/input-mapper.test.ts` around lines 5 - 8, The teardown in
afterEach currently restores mocks and Cli.options but doesn't reset the global
flag GitHub.githubInputEnabled modified by mapCliArgumentsToInput; update the
afterEach block to also reset GitHub.githubInputEnabled (e.g., set
GitHub.githubInputEnabled = undefined or its default) after
jest.restoreAllMocks() and Cli.options = undefined so tests don't leak state
between runs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/cli/commands/update.ts (2)

31-76: Inconsistent redirect limits between fetchJson (5) and downloadFile (10).

Both functions follow redirects, but fetchJson allows 5 while downloadFile allows 10. For consistency and since GitHub's asset downloads may involve multiple redirects, consider aligning both to 10.

♻️ Proposed fix
 function fetchJson(url: string): Promise<any> {
   return new Promise((resolve, reject) => {
     const get = (targetUrl: string, redirectCount: number) => {
-      if (redirectCount > 5) {
+      if (redirectCount > 10) {
         reject(new Error('Too many redirects'));

Also applies to: 81-116

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/update.ts` around lines 31 - 76, fetchJson currently stops
after 5 redirects while downloadFile allows 10, causing inconsistent behavior
and potential failures for GitHub asset downloads; update the redirect limit in
fetchJson (inside the inner get function) from 5 to 10 to match downloadFile and
ensure both functions follow the same redirect policy, keeping the same error
message logic but changing the numeric threshold and any related condition
checks that reference that limit; verify the change in the fetchJson function
definition and its get(targetUrl: string, redirectCount: number) implementation.

210-235: Semver comparison ignores pre-release suffixes.

parseVersionParts strips non-numeric suffixes, so v2.0.0-beta.1 and v2.0.0 compare as equal. If pre-release versions are used, this could cause unexpected behavior (e.g., not updating from beta to stable).

Consider using the existing semver package (already a dependency in package.json line 71) for robust comparison:

♻️ Proposed fix using semver package
+import semver from 'semver';

-function parseVersionParts(version: string): number[] {
-  return version
-    .replace(/^v/, '')
-    .split('.')
-    .map((part) => Number(part));
-}
-
-function compareSemver(a: string, b: string): number {
-  const partsA = parseVersionParts(a);
-  const partsB = parseVersionParts(b);
-
-  for (let index = 0; index < 3; index++) {
-    const x = partsA[index] || 0;
-    const y = partsB[index] || 0;
-    if (x < y) return -1;
-    if (x > y) return 1;
-  }
-
-  return 0;
-}

+function compareSemver(a: string, b: string): number {
+  const cleanA = a.replace(/^v/, '');
+  const cleanB = b.replace(/^v/, '');
+  return semver.compare(cleanA, cleanB);
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/update.ts` around lines 210 - 235, The current
parseVersionParts/compareSemver pair strips pre-release suffixes and treats
"v2.0.0-beta.1" as equal to "v2.0.0"; replace this custom logic with the semver
package to correctly handle pre-release and other semver edge cases:
import/require the existing semver dependency and update compareSemver to
delegate to semver.compare (or semver.rcompare as appropriate) returning -1/0/1,
and remove or stop using parseVersionParts so pre-release tags (e.g., "-beta.1")
are preserved in comparisons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@install.ps1`:
- Around line 85-87: The empty catch block after the checksum download silently
swallows errors; update that catch to log a clear warning or informational
message (including the caught error's message) so users know checksum
verification was skipped — in the catch following the checksum fetch/download
logic, call Write-Warning or Write-Host with a descriptive message and include
$_ or $_.Exception.Message to surface the failure details.
- Around line 112-119: The interactive Read-Host prompt can hang when the script
is piped; update the block around Read-Host/$AddToPath to first detect
non-interactive execution (e.g. check if $Host.Name -ne 'ConsoleHost' or wrap a
test of [Console]::KeyAvailable in a try/catch) and, when non-interactive or an
exception occurs, default $AddToPath to 'n' (do not modify PATH); otherwise
proceed with the current logic that calls
[Environment]::SetEnvironmentVariable('PATH', "$InstallDir;$UserPath", 'User'),
updates $env:PATH and calls Write-Info — ensure the detection and fallback are
applied before calling Read-Host so piped/redirected runs never hang.

In `@install.sh`:
- Around line 141-144: The PROFILE assignments in the shell case block (the
lines setting PROFILE in the zsh/bash/fish/* branches) use quoted tildes which
won't expand; update those assignments to use $HOME (e.g.,
PROFILE="$HOME/.zshrc", PROFILE="$HOME/.bashrc",
PROFILE="$HOME/.config/fish/config.fish", PROFILE="$HOME/.profile") so the path
expands correctly when used later in the script.
- Around line 192-196: The install flow currently calls install before
verify_checksum so a potentially tampered binary can be placed and made
executable; modify the flow so checksum verification occurs before making the
binary final: either (A) adjust the sequence to call verify_checksum immediately
after get_latest_version and before install makes the file executable, or (B)
change install to download to a temporary file, run verify_checksum against that
temp file, and only move/rename it into place and set permissions if
verification succeeds; if verification fails, ensure the temp file is removed
and the script exits non‑zero (also add cleanup in the install failure path).
Reference the functions detect_platform, get_latest_version, install and
verify_checksum when updating the flow.

---

Nitpick comments:
In `@src/cli/commands/update.ts`:
- Around line 31-76: fetchJson currently stops after 5 redirects while
downloadFile allows 10, causing inconsistent behavior and potential failures for
GitHub asset downloads; update the redirect limit in fetchJson (inside the inner
get function) from 5 to 10 to match downloadFile and ensure both functions
follow the same redirect policy, keeping the same error message logic but
changing the numeric threshold and any related condition checks that reference
that limit; verify the change in the fetchJson function definition and its
get(targetUrl: string, redirectCount: number) implementation.
- Around line 210-235: The current parseVersionParts/compareSemver pair strips
pre-release suffixes and treats "v2.0.0-beta.1" as equal to "v2.0.0"; replace
this custom logic with the semver package to correctly handle pre-release and
other semver edge cases: import/require the existing semver dependency and
update compareSemver to delegate to semver.compare (or semver.rcompare as
appropriate) returning -1/0/1, and remove or stop using parseVersionParts so
pre-release tags (e.g., "-beta.1") are preserved in comparisons.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d88889dc-4419-4e14-b1c9-82dea49dde25

📥 Commits

Reviewing files that changed from the base of the PR and between 5bdcf12 and 79d12aa.

⛔ Files ignored due to path filters (3)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (8)
  • .github/workflows/release-cli.yml
  • install.ps1
  • install.sh
  • package.json
  • src/cli.ts
  • src/cli/__tests__/cli-integration.test.ts
  • src/cli/__tests__/commands.test.ts
  • src/cli/commands/update.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/cli/tests/cli-integration.test.ts
  • src/cli/tests/commands.test.ts

Comment on lines +85 to +87
} catch {
# Checksums not available for this release; continue without verification
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Empty catch block silently swallows checksum fetch errors.

When the checksum file download fails, the script continues without any indication. Add a message for transparency.

🔧 Proposed fix
 } catch {
     # Checksums not available for this release; continue without verification
+    Write-Warn "Checksum file not available; skipping verification."
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch {
# Checksums not available for this release; continue without verification
}
} catch {
# Checksums not available for this release; continue without verification
Write-Warning "Checksum file not available; skipping verification."
}
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)

[warning] 85-87: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.

(PSAvoidUsingEmptyCatchBlock)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install.ps1` around lines 85 - 87, The empty catch block after the checksum
download silently swallows errors; update that catch to log a clear warning or
informational message (including the caught error's message) so users know
checksum verification was skipped — in the catch following the checksum
fetch/download logic, call Write-Warning or Write-Host with a descriptive
message and include $_ or $_.Exception.Message to surface the failure details.

Comment on lines +112 to +119
# Offer to add automatically
Write-Host ""
$AddToPath = Read-Host "Add to PATH now? (Y/n)"
if ($AddToPath -ne 'n' -and $AddToPath -ne 'N') {
[Environment]::SetEnvironmentVariable('PATH', "$InstallDir;$UserPath", 'User')
$env:PATH = "$InstallDir;$env:PATH"
Write-Info "Added to PATH. You can now run: game-ci --help"
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Interactive Read-Host may fail or hang when script is piped.

The documented usage (irm ... | iex) pipes the script, which can cause Read-Host to behave unexpectedly (hang or return empty). Consider detecting non-interactive mode or defaulting to not modifying PATH when piped.

🔧 Proposed fix
     # Offer to add automatically
     Write-Host ""
-    $AddToPath = Read-Host "Add to PATH now? (Y/n)"
-    if ($AddToPath -ne 'n' -and $AddToPath -ne 'N') {
+    # Check if running interactively
+    if ([Environment]::UserInteractive -and [Console]::KeyAvailable -ne $null) {
+        $AddToPath = Read-Host "Add to PATH now? (Y/n)"
+        if ($AddToPath -ne 'n' -and $AddToPath -ne 'N') {
+            [Environment]::SetEnvironmentVariable('PATH', "$InstallDir;$UserPath", 'User')
+            $env:PATH = "$InstallDir;$env:PATH"
+            Write-Info "Added to PATH. You can now run: game-ci --help"
+        }
+    } else {
+        Write-Info "Run the command above to add game-ci to your PATH."
+    }

Alternatively, wrap in a try/catch to handle the piped scenario gracefully.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Offer to add automatically
Write-Host ""
$AddToPath = Read-Host "Add to PATH now? (Y/n)"
if ($AddToPath -ne 'n' -and $AddToPath -ne 'N') {
[Environment]::SetEnvironmentVariable('PATH', "$InstallDir;$UserPath", 'User')
$env:PATH = "$InstallDir;$env:PATH"
Write-Info "Added to PATH. You can now run: game-ci --help"
}
# Offer to add automatically
Write-Host ""
# Check if running interactively
if ([Environment]::UserInteractive -and [Console]::KeyAvailable -ne $null) {
$AddToPath = Read-Host "Add to PATH now? (Y/n)"
if ($AddToPath -ne 'n' -and $AddToPath -ne 'N') {
[Environment]::SetEnvironmentVariable('PATH', "$InstallDir;$UserPath", 'User')
$env:PATH = "$InstallDir;$env:PATH"
Write-Info "Added to PATH. You can now run: game-ci --help"
}
} else {
Write-Info "Run the command above to add game-ci to your PATH."
}
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)

[warning] 113-113: File 'install.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.

(PSAvoidUsingWriteHost)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install.ps1` around lines 112 - 119, The interactive Read-Host prompt can
hang when the script is piped; update the block around Read-Host/$AddToPath to
first detect non-interactive execution (e.g. check if $Host.Name -ne
'ConsoleHost' or wrap a test of [Console]::KeyAvailable in a try/catch) and,
when non-interactive or an exception occurs, default $AddToPath to 'n' (do not
modify PATH); otherwise proceed with the current logic that calls
[Environment]::SetEnvironmentVariable('PATH', "$InstallDir;$UserPath", 'User'),
updates $env:PATH and calls Write-Info — ensure the detection and fallback are
applied before calling Read-Host so piped/redirected runs never hang.

Comment on lines +141 to +144
zsh) PROFILE="~/.zshrc" ;;
bash) PROFILE="~/.bashrc" ;;
fish) PROFILE="~/.config/fish/config.fish" ;;
*) PROFILE="~/.profile" ;;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tilde does not expand inside quotes; use $HOME instead.

The PROFILE assignments use quoted tildes (e.g., "~/.zshrc"), which won't expand to the user's home directory when printed or used.

🔧 Proposed fix
       case "$SHELL_NAME" in
-        zsh)  PROFILE="~/.zshrc" ;;
-        bash) PROFILE="~/.bashrc" ;;
-        fish) PROFILE="~/.config/fish/config.fish" ;;
-        *)    PROFILE="~/.profile" ;;
+        zsh)  PROFILE="$HOME/.zshrc" ;;
+        bash) PROFILE="$HOME/.bashrc" ;;
+        fish) PROFILE="$HOME/.config/fish/config.fish" ;;
+        *)    PROFILE="$HOME/.profile" ;;
       esac
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
zsh) PROFILE="~/.zshrc" ;;
bash) PROFILE="~/.bashrc" ;;
fish) PROFILE="~/.config/fish/config.fish" ;;
*) PROFILE="~/.profile" ;;
zsh) PROFILE="$HOME/.zshrc" ;;
bash) PROFILE="$HOME/.bashrc" ;;
fish) PROFILE="$HOME/.config/fish/config.fish" ;;
*) PROFILE="$HOME/.profile" ;;
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 141-141: Tilde does not expand in quotes. Use $HOME.

(SC2088)


[warning] 142-142: Tilde does not expand in quotes. Use $HOME.

(SC2088)


[warning] 143-143: Tilde does not expand in quotes. Use $HOME.

(SC2088)


[warning] 144-144: Tilde does not expand in quotes. Use $HOME.

(SC2088)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install.sh` around lines 141 - 144, The PROFILE assignments in the shell case
block (the lines setting PROFILE in the zsh/bash/fish/* branches) use quoted
tildes which won't expand; update those assignments to use $HOME (e.g.,
PROFILE="$HOME/.zshrc", PROFILE="$HOME/.bashrc",
PROFILE="$HOME/.config/fish/config.fish", PROFILE="$HOME/.profile") so the path
expands correctly when used later in the script.

Comment on lines +192 to +196
# Main
detect_platform
get_latest_version
install
verify_checksum
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Checksum verification occurs after binary is already installed.

The verify_checksum function is called after install, meaning a potentially corrupted or tampered binary is already written to disk and made executable before verification. If checksum verification fails, the binary remains installed.

Reorder to verify checksum before making the binary executable, or clean up on failure:

🔧 Proposed fix
 # Main
 detect_platform
 get_latest_version
 install
-verify_checksum
+verify_checksum || {
+  rm -f "${INSTALL_DIR}/${BINARY_NAME}"
+  error "Checksum verification failed. Binary removed."
+}

Alternatively, restructure to download to a temp file first, verify, then move to final location (similar to how install.ps1 handles this with Remove-Item on failure at line 81).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Main
detect_platform
get_latest_version
install
verify_checksum
# Main
detect_platform
get_latest_version
install
verify_checksum || {
rm -f "${INSTALL_DIR}/${BINARY_NAME}"
error "Checksum verification failed. Binary removed."
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install.sh` around lines 192 - 196, The install flow currently calls install
before verify_checksum so a potentially tampered binary can be placed and made
executable; modify the flow so checksum verification occurs before making the
binary final: either (A) adjust the sequence to call verify_checksum immediately
after get_latest_version and before install makes the file executable, or (B)
change install to download to a temporary file, run verify_checksum against that
temp file, and only move/rename it into place and set permissions if
verification succeeds; if verification fails, ensure the temp file is removed
and the script exits non‑zero (also add cleanup in the install failure path).
Reference the functions detect_platform, get_latest_version, install and
verify_checksum when updating the flow.

…safety

- Add process.exit(1) in cli.ts catch block so failures produce non-zero exit codes
- Add 6 missing build inputs: containerRegistryRepository, containerRegistryImageVersion,
  dockerIsolationMode, sshPublicKeysDirectoryPath, cacheUnityInstallationOnMac, unityHubVersionOnMac
- Add 6 missing orchestrate inputs: kubeStorageClass, readInputFromOverrideList,
  readInputOverrideCommand, postBuildSteps, preBuildSteps, customJob
- Fix activate command description to accurately reflect verification behavior
- Add null check before accessing result.BuildResults in orchestrate handler

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (3)
src/cli.ts (2)

34-38: ⚠️ Potential issue | 🟠 Major

Map YError to exit code 2 in the top-level catch.

Line 35 currently skips YError entirely, so parser/configuration failures can miss the documented config-error exit code.

Suggested fix
   } catch (error: any) {
-    if (error.name !== 'YError') {
-      core.error(`Error: ${error.message}`);
-      process.exit(1);
-    }
+    if (error?.name === 'YError') {
+      core.error(`Error: ${error?.message ?? String(error)}`);
+      process.exit(2);
+    }
+    core.error(`Error: ${error?.message ?? String(error)}`);
+    process.exit(1);
   }
#!/bin/bash
# Verify current top-level error mapping and related tests.
rg -n -C3 "YError|process\\.exit\\(|cli\\.parse\\(" src/cli.ts
rg -n -C3 "exit code|YError|parse" src/cli/__tests__
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 34 - 38, The top-level catch in main CLI (catch
block around cli.parse in src/cli.ts) currently ignores errors with error.name
=== 'YError', causing config/parse failures to not trigger the documented
config-error exit code; update that catch to detect YError and map it to
process.exit(2) (and emit a clear error via core.error or similar before
exiting), while preserving the existing behavior for non-YError cases (still
logging error.message and exiting with code 1).

4-4: ⚠️ Potential issue | 🟡 Minor

Remove the stale eslint-disable directive.

Line 4 still suppresses import/no-unresolved without an active violation and triggers eslint-comments/no-unused-disable.

Suggested fix
 import yargs from 'yargs';
-// eslint-disable-next-line import/no-unresolved
 import { hideBin } from 'yargs/helpers';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` at line 4, Remove the stale ESLint disable comment "//
eslint-disable-next-line import/no-unresolved" at the top of src/cli.ts (the
unused inline eslint-disable directive) so eslint-comments/no-unused-disable no
longer flags it; simply delete that line (or replace it with a contextual, valid
comment only if an actual import/no-unresolved violation exists), leaving the
rest of the file and imports unchanged.
src/cli/commands/activate.ts (1)

42-45: ⚠️ Potential issue | 🟠 Major

Use unified Input accessors for activation inputs.

Lines 42-45 bypass the mapped CLI input pipeline, so activation resolution can diverge from build/orchestrate behavior.

Suggested fix
 import type { CommandModule } from 'yargs';
 import * as core from '@actions/core';
 import { mapCliArgumentsToInput, CliArguments } from '../input-mapper';
+import Input from '../../model/input';
@@
-      const unitySerial = process.env.UNITY_SERIAL;
-      const unityLicense = process.env.UNITY_LICENSE;
-      const licensingServer = cliArguments.unityLicensingServer || process.env.UNITY_LICENSING_SERVER || '';
+      const unitySerial = Input.unitySerial;
+      const unityLicense = Input.unityLicense;
+      const licensingServer = Input.unityLicensingServer || '';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/activate.ts` around lines 42 - 45, The activation code is
reading environment/cli args directly (unitySerial, unityLicense,
licensingServer) instead of the project's unified Input accessors; replace those
direct reads with the mapped Input API (e.g., use Input.get or the project's
Input.resolve accessor for the keys used elsewhere such as "unitySerial",
"unityLicense", and "unityLicensingServer"), preserve the fallback/default
behavior (empty string) and ensure Input is imported where activate.ts declares
const unitySerial/const unityLicense/const licensingServer so activation uses
the same input pipeline as build/orchestrate.
🧹 Nitpick comments (1)
src/cli/commands/build.ts (1)

167-172: Convert run-as-host-user and skip-activation flags to boolean type for improved CLI ergonomics.

These options are currently defined as strings, which requires users to pass --skip-activation=true instead of just --skip-activation. Converting to boolean types aligns with their semantic meaning and the existing input-mapper already handles conversion to strings (line 96) for downstream compatibility.

Apply to:

  • src/cli/commands/build.ts: run-as-host-user (lines 167–172) and skip-activation (lines 197–202)
  • src/cli/commands/orchestrate.ts: skip-activation (lines 135–140)
  • src/cli/input-mapper.ts: Update CliArguments interface to declare runAsHostUser and skipActivation as boolean (lines 44, 62)
Suggested refactor for build.ts
       .option('run-as-host-user', {
         alias: 'runAsHostUser',
-        type: 'string',
+        type: 'boolean',
         description: 'Whether to run as a user that matches the host system',
-        default: 'false',
+        default: false,
       })
@@
       .option('skip-activation', {
         alias: 'skipActivation',
-        type: 'string',
+        type: 'boolean',
         description: 'Skip the activation/deactivation of Unity',
-        default: 'false',
+        default: false,
       })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/commands/build.ts` around lines 167 - 172, Change the CLI flags from
string to boolean: update the .option entries for 'run-as-host-user' and
'skip-activation' in the build command (the .option calls with alias
runAsHostUser and skipActivation) and the 'skip-activation' option in the
orchestrate command to use type: 'boolean' and default: false (leave aliases
unchanged). Also update the CliArguments interface in input-mapper (fields
runAsHostUser and skipActivation) to be boolean types so downstream code and the
existing string conversion remain compatible. Ensure no other logic expects
string values for these flags (adjust any parsing that assumes strings if
present).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/cli.ts`:
- Around line 34-38: The top-level catch in main CLI (catch block around
cli.parse in src/cli.ts) currently ignores errors with error.name === 'YError',
causing config/parse failures to not trigger the documented config-error exit
code; update that catch to detect YError and map it to process.exit(2) (and emit
a clear error via core.error or similar before exiting), while preserving the
existing behavior for non-YError cases (still logging error.message and exiting
with code 1).
- Line 4: Remove the stale ESLint disable comment "// eslint-disable-next-line
import/no-unresolved" at the top of src/cli.ts (the unused inline eslint-disable
directive) so eslint-comments/no-unused-disable no longer flags it; simply
delete that line (or replace it with a contextual, valid comment only if an
actual import/no-unresolved violation exists), leaving the rest of the file and
imports unchanged.

In `@src/cli/commands/activate.ts`:
- Around line 42-45: The activation code is reading environment/cli args
directly (unitySerial, unityLicense, licensingServer) instead of the project's
unified Input accessors; replace those direct reads with the mapped Input API
(e.g., use Input.get or the project's Input.resolve accessor for the keys used
elsewhere such as "unitySerial", "unityLicense", and "unityLicensingServer"),
preserve the fallback/default behavior (empty string) and ensure Input is
imported where activate.ts declares const unitySerial/const unityLicense/const
licensingServer so activation uses the same input pipeline as build/orchestrate.

---

Nitpick comments:
In `@src/cli/commands/build.ts`:
- Around line 167-172: Change the CLI flags from string to boolean: update the
.option entries for 'run-as-host-user' and 'skip-activation' in the build
command (the .option calls with alias runAsHostUser and skipActivation) and the
'skip-activation' option in the orchestrate command to use type: 'boolean' and
default: false (leave aliases unchanged). Also update the CliArguments interface
in input-mapper (fields runAsHostUser and skipActivation) to be boolean types so
downstream code and the existing string conversion remain compatible. Ensure no
other logic expects string values for these flags (adjust any parsing that
assumes strings if present).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8ea8550a-1f3b-4d29-bc10-148f4b7f603e

📥 Commits

Reviewing files that changed from the base of the PR and between 79d12aa and 280a10d.

📒 Files selected for processing (5)
  • src/cli.ts
  • src/cli/commands/activate.ts
  • src/cli/commands/build.ts
  • src/cli/commands/orchestrate.ts
  • src/cli/input-mapper.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/cli/commands/orchestrate.ts

Comment on lines +24 to +86
name: Build ${{ matrix.target }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- target: linux-x64
os: ubuntu-latest
pkg-target: node20-linux-x64
binary-name: game-ci-linux-x64
- target: linux-arm64
os: ubuntu-latest
pkg-target: node20-linux-arm64
binary-name: game-ci-linux-arm64
- target: macos-x64
os: macos-latest
pkg-target: node20-macos-x64
binary-name: game-ci-macos-x64
- target: macos-arm64
os: macos-latest
pkg-target: node20-macos-arm64
binary-name: game-ci-macos-arm64
- target: windows-x64
os: windows-latest
pkg-target: node20-win-x64
binary-name: game-ci-windows-x64.exe
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.release.tag_name || inputs.tag || github.ref }}

- uses: actions/setup-node@v4
with:
node-version: '20'

- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Build TypeScript
run: yarn build

- name: Verify CLI before packaging
run: node lib/cli.js version

- name: Build standalone binary
run: npx pkg lib/cli.js --target ${{ matrix.pkg-target }} --output ${{ matrix.binary-name }} --compress GZip

- name: Verify standalone binary (non-cross-compiled)
if: |
(matrix.target == 'linux-x64' && runner.os == 'Linux') ||
(matrix.target == 'macos-arm64' && runner.os == 'macOS' && runner.arch == 'ARM64') ||
(matrix.target == 'macos-x64' && runner.os == 'macOS' && runner.arch == 'X64') ||
(matrix.target == 'windows-x64' && runner.os == 'Windows')
run: ./${{ matrix.binary-name }} version
shell: bash

- uses: actions/upload-artifact@v4
with:
name: binary-${{ matrix.target }}
path: ${{ matrix.binary-name }}
retention-days: 5

create-checksums-and-upload:

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 2 days ago

Generally, the fix is to explicitly set least-privilege permissions for any job (or workflow) that uses the default GITHUB_TOKEN permissions. For this workflow, the build-binaries job only needs to read repository contents and interact with artifacts; it does not write to the repository, issues, or releases. Therefore, we should give it contents: read.

The best minimal fix without changing functionality is to add a permissions block under the build-binaries job, similar to the other jobs. Concretely, in .github/workflows/release-cli.yml, under jobs:, within the build-binaries: job definition and alongside runs-on and strategy, insert:

permissions:
  contents: read

No additional imports, methods, or definitions are needed; this is purely a YAML configuration change in the workflow file.

Suggested changeset 1
.github/workflows/release-cli.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/release-cli.yml b/.github/workflows/release-cli.yml
--- a/.github/workflows/release-cli.yml
+++ b/.github/workflows/release-cli.yml
@@ -23,6 +23,8 @@
   build-binaries:
     name: Build ${{ matrix.target }}
     runs-on: ${{ matrix.os }}
+    permissions:
+      contents: read
     strategy:
       fail-fast: false
       matrix:
EOF
@@ -23,6 +23,8 @@
build-binaries:
name: Build ${{ matrix.target }}
runs-on: ${{ matrix.os }}
permissions:
contents: read
strategy:
fail-fast: false
matrix:
Copilot is powered by AI and may make mistakes. Always verify output.
yargs@18.0.0 requires Node.js >=20.19.0 but the integrity-check
workflow was using Node 18, causing `yarn install` to fail with an
engine incompatibility error. Updated the workflow to Node 20 (matching
volta config and all other workflows) and aligned the engines field in
package.json accordingly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
package.json (1)

48-49: ⚠️ Potential issue | 🟠 Major

Tighten the Node floor to match yargs@18 requirements.

Line 48 (>=20.x) still allows unsupported Node 20 minors (20.0–20.18) for Line 77 (yargs@^18). This can break installs for valid-on-paper environments (including current Volta pin at Line 109).

Proposed fix
   "engines": {
-    "node": ">=20.x"
+    "node": ">=20.19.0"
   },
@@
   "volta": {
-    "node": "20.5.1",
+    "node": "20.19.0",
     "yarn": "1.22.19"
   }
#!/bin/bash
set -euo pipefail

echo "Project engine:"
jq -r '.engines.node' package.json

echo "Volta node pin:"
jq -r '.volta.node' package.json

echo "yargs dependency range:"
jq -r '.dependencies.yargs' package.json

echo "yargs@18.0.0 engine metadata:"
npm view yargs@18.0.0 engines --json

Expected verification result: yargs@18.0.0 requires Node ^20.19.0 || ^22.12.0 || >=23, so project constraints should not permit lower 20.x versions.

Also applies to: 77-77

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 48 - 49, The project Node engine floor
("engines.node") is too loose (">=20.x") for yargs@18 which requires Node
>=20.19.0; update package.json to tighten engines.node to at least ">=20.19.0"
(or mirror yargs' engines string "^20.19.0 || ^22.12.0 || >=23") and also update
the Volta pin ("volta.node") to a matching minimum (e.g., "20.19.0") so installs
cannot select unsupported 20.x minors; ensure you verify changes against
dependencies.yargs (which is ^18) to keep constraints consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@package.json`:
- Around line 48-49: The project Node engine floor ("engines.node") is too loose
(">=20.x") for yargs@18 which requires Node >=20.19.0; update package.json to
tighten engines.node to at least ">=20.19.0" (or mirror yargs' engines string
"^20.19.0 || ^22.12.0 || >=23") and also update the Volta pin ("volta.node") to
a matching minimum (e.g., "20.19.0") so installs cannot select unsupported 20.x
minors; ensure you verify changes against dependencies.yargs (which is ^18) to
keep constraints consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e7851364-63cd-4f42-88f5-99ca8a729d47

📥 Commits

Reviewing files that changed from the base of the PR and between 280a10d and c6e56f7.

📒 Files selected for processing (2)
  • .github/workflows/integrity-check.yml
  • package.json

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 20.81340% with 331 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.64%. Comparing base (9d47543) to head (77b9d90).

Files with missing lines Patch % Lines
src/cli/commands/update.ts 6.48% 173 Missing ⚠️
src/cli/commands/status.ts 11.94% 59 Missing ⚠️
src/cli/commands/activate.ts 13.79% 25 Missing ⚠️
src/cli/commands/build.ts 42.85% 20 Missing ⚠️
src/cli/commands/test.ts 39.39% 20 Missing ⚠️
src/cli/commands/orchestrate.ts 37.03% 17 Missing ⚠️
src/cli/commands/version.ts 22.72% 17 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #813      +/-   ##
==========================================
- Coverage   31.25%   29.64%   -1.61%     
==========================================
  Files          84      100      +16     
  Lines        4563     5188     +625     
  Branches     1103     1241     +138     
==========================================
+ Hits         1426     1538     +112     
- Misses       3137     3650     +513     
Files with missing lines Coverage Δ
src/cli/commands/shared-options.ts 100.00% <100.00%> (ø)
src/cli/input-mapper.ts 100.00% <100.00%> (ø)
src/model/cli/cli.ts 25.68% <100.00%> (ø)
src/cli/commands/orchestrate.ts 37.03% <37.03%> (ø)
src/cli/commands/version.ts 22.72% <22.72%> (ø)
src/cli/commands/build.ts 42.85% <42.85%> (ø)
src/cli/commands/test.ts 39.39% <39.39%> (ø)
src/cli/commands/activate.ts 13.79% <13.79%> (ø)
src/cli/commands/status.ts 11.94% <11.94%> (ø)
src/cli/commands/update.ts 6.48% <6.48%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…, extract shared options

- Add `test` / `t` command mirroring unity-test-runner (EditMode, PlayMode, All,
  code coverage, test filters)
- Add `o` short alias for `orchestrate` command
- Extract shared option builders (project, docker, android, orchestrator) to
  eliminate duplication across build/test/orchestrate
- Make `build` local-only — remove --provider-strategy (use `orchestrate` for
  remote builds)
- Absorb `cache` command into `status` (--cache-dir flag) — remove standalone
  cache command that was half-implemented
- Fix `list-worfklow` typo in internal CLI → `list-workflow`
- Add test-related fields to CliArguments input mapper
- Update all unit and integration tests (64 passing)

Command structure is now:
  game-ci build          Local build (Docker/macOS)
  game-ci test / t       Run Unity tests
  game-ci orchestrate / o  Remote build (AWS/K8s/etc)
  game-ci activate       License validation
  game-ci status         Project info + cache status
  game-ci version        Version info
  game-ci update         Self-update

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@frostebite frostebite marked this pull request as ready for review March 7, 2026 02:12
Use the Checks API to flip failed macOS build conclusions to neutral
(gray dash) so unstable builds don't show red X marks on PRs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request LTS 2.0 Orchestrator LTS v2.0 milestone orchestrator Orchestrator module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Official CLI Support — game-ci

1 participant