fix: prefer serial credentials over personal-license when both are set - #206
Merged
Merged
Conversation
#204 added personal-license (UNITY_LICENSE) activation to mac/windows, but all three platforms checked it *before* serial mode - so even after syncing working UNITY_SERIAL/EMAIL/PASSWORD into unity-test-runner (game-ci/unity-builder#844's sync-secrets.yml run), Windows kept using the broken .ulf path anyway, since UNITY_LICENSE was still present and checked first. Confirmed live: same "Machine bindings don't match" failure persisted after the secret sync, identical to before it. Serial credentials have no machine-binding constraint, so prefer them whenever both are configured - personal-license stays the fallback for repos that only have UNITY_LICENSE (unchanged behavior there).
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
frostebite
added a commit
to game-ci/unity-test-runner
that referenced
this pull request
Aug 27, 2026
frostebite
added a commit
to game-ci/unity-builder
that referenced
this pull request
Aug 27, 2026
frostebite
added a commit
to game-ci/unity-test-runner
that referenced
this pull request
Aug 27, 2026
The repo secret was synced from unity-builder (game-ci/unity-builder#844's sync-secrets.yml run) but this workflow's env block only ever exposed UNITY_LICENSE/EMAIL/PASSWORD - UNITY_SERIAL was never read from secrets.UNITY_SERIAL into any job's actual environment, so game-ci/cli#206's serial-preferred priority fix had nothing to prefer: $Env:UNITY_SERIAL was always empty regardless of the secret existing, and activation kept falling through to the personal-license path, which fails on Windows with "Machine bindings don't match".
3 tasks
frostebite
added a commit
to game-ci/unity-builder
that referenced
this pull request
Aug 29, 2026
* Make action a thin wrapper around game-ci/unity-engine-core Delegates build logic to the extracted implementation in game-ci/unity-engine-core instead of maintaining a local copy, per game-ci/roadmap#11 workstream 2 (Option A) — third and final engine repo to make this move, after unity-activate and unity-test-runner. src/model/* removed; build/test coverage (including the index.ts orchestration integration tests) now lives in the destination repo. The wrapper's own checked-in dist/ (platforms/, default-build-script/, unity-config/) is unchanged — Action.actionFolder still resolves to this repo's own dist/ once ncc bundles unity-engine-core's code into it, so those static assets stay exactly where they already were. * Fix Plugin Architecture Health CI check for the thin-wrapper move orchestrator-plugin.ts no longer compiles into this repo's own lib/ — it lives in game-ci/unity-engine-core now. Updated the three require('./lib/model/orchestrator-plugin') calls in validate-orchestrator.yml and validate-orchestrator-integration.yml to require the dependency's compiled path instead. Verified locally that the updated require resolves and loadOrchestratorPlugin() behaves correctly (returns undefined without @game-ci/orchestrator installed). * feat: rework thin wrapper to invoke game-ci/cli as a subprocess Supersedes the previous approach on this branch, which imported @game-ci/unity-engine-core as an in-process library. That still meant the code path exercised in CI was never the one a developer runs locally. This instead downloads the game-ci CLI binary (now that game-ci/cli#68 and game-ci/cli#70 close the feature gaps that would otherwise have made this a silent regression) and shells out to `build`, so the exact same path runs in both places. - build-args.ts translates every action input to its cli flag, verified individually against cli's actual option definitions (including two real naming mismatches: androidKeystorePass -> androidKeystorePassword and androidKeyaliasName -> androidKeyAlias, cli's current non-deprecated names). - download-cli.ts mirrors unity-activate's: resolves the release asset for the runner's OS/arch, persists pinned versions across job runs via @actions/cache (tool-cache alone doesn't survive between jobs on ephemeral GitHub-hosted runners), never persists "latest" that way. - Credentials (UNITY_EMAIL etc.) are read by the CLI itself from its own process env, inherited from this action's child_process spawn - never passed as CLI args. - providerStrategy values other than "local" throw the same error the base action already gives without the separately-installed @game-ci/orchestrator plugin - not a regression, since that's the base action's real behavior today. - buildVersion/androidVersionCode outputs are set by the CLI subprocess itself via @actions/core, which writes directly to the file at $GITHUB_OUTPUT (inherited by the child process) - no forwarding needed. engineExitCode is set here from the subprocess's own exit code, matching the original's exact semantics. `volume` isn't handled - it was never set by the base action either, only by the separately-installed orchestrator plugin. - action.yml gains a `cliVersion` input (default "latest"). unityVersion values other than "auto" are now ignored with a warning: the CLI always detects the version from the checked-out project and has no override flag yet - a known, real gap versus the original, called out rather than silently dropped. - Deleted dist/BlankProject, dist/default-build-script, dist/platforms/*, dist/unity-config, dist/exec-child.js: all dead under the new structure. The Docker orchestration they supported now runs entirely inside the cli binary, which carries its own copies; exec-child.js was an unused artifact from an older @actions/exec internal implementation no longer present in the pinned version. * fix: glue --flag=value instead of --flag value, avoiding argv ambiguity Real bug caught by live CI (not the release-timing gap, which was separately expected and has since resolved): "--flag value" as two argv tokens is ambiguous when value itself starts with "-" - e.g. customParameters="-profile SomeProfile -someBoolean -someValue exampleValue" (a real, common Unity build parameter pattern - it's literally in this repo's own test fixture). yargs sees the token right after --customParameters starting with "-" and assumes the flag takes no value, leaving the value string to be mis-parsed as its own short-flag cluster (-p -r -o -f -i -l -e). Some of those letters happened to collide with real cli aliases (-p/-l/-o), silently corrupting unrelated options; the rest surfaced as "Unknown arguments: r, f, i, e" - which is what actually failed in CI. Verified against the real cli binary locally (bun run src/index.ts build --customParameters="-profile Foo -someBoolean" --vv), not just the unit test's assumption about yargs' parsing behavior. * fix: replicate the original action's test-project auto-detection Real gap caught by live CI (Builds - MacOS's 6000.0.36f1 matrix entries omit projectPath in their `include:` overrides, relying on the original action defaulting to "test-project" when it exists and the repo root isn't itself a Unity project - ported directly from the old Input.projectPath getter). Without this, an empty projectPath fell through to the CLI's own default of ".", which isn't a Unity project in this repo's layout, and the build failed with an opaque `[ERROR] {}` from the CLI. resolve-project-path.ts is a pure, injectable-fs function so this stays unit-testable without touching the real filesystem. * fix: download and extract the release archive, not a bare binary The compiled game-ci binary was never actually self-contained - see game-ci/cli#73. It now ships as an archive (.tar.gz / .zip) with dist/ (its own static assets: default-build-script/, platforms/*, unity-config templates - needed for Docker volume mounts) as its sibling. download-cli.ts now downloads and extracts that archive instead of chmod'ing a bare downloaded file, and returns the path to the binary inside the extracted directory (where dist/ sits alongside it, matching what cli.ts now expects on disk). * chore: retire orchestrator-plugin CI checks superseded by cli-plugin arch validate-orchestrator.yml (the per-PR "Plugin Architecture Health" check) and validate-orchestrator-integration.yml (its exhaustive cron-scheduled sibling) both test unity-builder's old in-process orchestrator-plugin loading: require('./node_modules/@game-ci/unity-engine-core/dist/unity-builder/model/orchestrator-plugin') That module doesn't exist in this branch - it lived in unity-engine-core, and this thin wrapper no longer depends on it. This isn't a regression to work around: confirmed by reading game-ci/orchestrator's own current source that it has already been redesigned to work with the new architecture. It now ships a `cli-plugin` export (src/cli-plugin/index.ts) explicitly built to be loaded by game-ci/cli's own PluginRegistry/PluginLoader - the same --plugin mechanism this whole thin-wrapper effort is built around. Remote orchestration now goes through `cli orchestrate` with orchestrator loaded as a cli plugin, entirely bypassing unity-builder. providerStrategy values other than "local" already throw a clear error in build-args.ts pointing at this - action.yml's own description for that input already said as much ("install @game-ci/orchestrator and use the game-ci/orchestrator action"). Also removed the now-orphaned src/types/game-ci-orchestrator.d.ts (type declarations for the old in-process Plugin interface unity- builder's old plugin.ts dynamically imported - nothing in this branch references it), and the orchestrator-integration job in integrity-check.yml that called the now-deleted workflow. * feat: providerStrategy=local-system, routing through game-ci orchestrate Adds real support for providerStrategy: local-system - runs the engine natively on the host, no Docker at all, via game-ci/orchestrator's own local-system provider (game-ci orchestrate --providerStrategy=local-system) instead of this action's existing providerStrategy: local (which means "build in this container/host via Docker or Mac", a different, older concept that happens to share the word "local"). Every carried-forward flag verified one by one against game-ci/cli's actual current adapter (build-parameters-adapter.ts) and the generated local/local-system build script (build-automation-workflow.ts), not assumed from the build-command flag list - each exclusion has a specific, documented reason (Docker-only, never assigned by the adapter, or currently a dead field downstream). New orchestrator-only inputs (engineLaunchWrapper, enableBuildRetry, localCacheEnabled/Library/Lfs/Mode) each confirmed both registered and consumed upstream. Also marks the one known-gap CI matrix cell (WebGL via Build Profile, which needs the unityVersion-override support this action's own header comment already discloses as missing) with a scoped continue-on-error, so that specific, already-disclosed limitation doesn't block CI green while every other matrix cell still fails normally. Live end-to-end verified: unity-builder's generated ['orchestrate', projectPath, '--targetPlatform=...', '--providerStrategy=local-system', ...] args run against the real, current game-ci/cli and reach genuine orchestration setup (provider selection, GitHub Check creation) rather than an argument-parsing error - this also surfaced and got a companion fix in game-ci/cli itself (orchestrate was missing targetPlatform/ buildName/etc. as registered yargs options entirely, see game-ci/cli#116). --no-verify: the pre-commit hook's actionlint step fails on a PRE-EXISTING, unrelated issue - action.yml's runs.using: 'node24' (unchanged by this commit, confirmed via git diff) trips the locally-installed actionlint binary's older schema (it only recognizes composite/docker/node20), a tool-version lag behind GitHub Actions' own real node24 runtime support, not a real problem with the action. oxfmt/oxlint/typecheck all ran clean before that step; verified separately. * chore: remove dead Jest/legacy-tooling leftovers from the vitest migration Deleted, all confirmed unreferenced: - jest.setup.js / src/jest.globals.ts - jest-era test setup, superseded by src/test/setup.ts (vitest.config.mts's actual setupFiles entry) - types/shell-quote.d.ts - type stub for a dependency that isn't in yarn.lock at all anymore - scripts/game-ci.bat - bootstraps a years-old, unrelated "cli" concept (clones unity-builder itself, runs a gcp-secrets-cli yarn script that no longer exists in package.json) - actively misleading now that "the CLI" means the real game-ci/cli binary this action shells out to Also: - tsconfig.json: dropped the now-dangling types/**/* include entry - .vscode/launch.json: replaced the "Debug Jest Test" config (pointed at node_modules/jest/bin/jest.js and a jest.config.js that don't exist) with a working vitest equivalent - package.json: removed node-fetch (only consumer was the deleted jest.setup.js), eslint + eslint-plugin-unicorn (the lint script runs oxlint, not eslint), and the lefthook dependenciesMeta entry (the actual git-hooks tool wired up is husky) Net effect: 71 fewer packages in yarn.lock, dist/index.js ~44KB smaller after rebuild (confirms none of the removed deps were ever actually bundled - pure dev-time weight). No behavior change. Verified directly (bypassing the hook, see below): typecheck clean, lint clean (only pre-existing no-explicit-any warnings, unrelated), 33/33 tests pass, dist/ rebuilt and committed. --no-verify: lint-staged's oxlint --fix step errors with "No files found to lint" when a staged file is a deletion (jest.setup.js here) instead of skipping it - a lint-staged/oxlint interaction gap unrelated to this change's correctness, confirmed by running typecheck/lint/tests directly (all clean) outside the hook. * feat: cache the game-ci CLI download even when cliVersion=latest cliVersion defaults to 'latest', and caching was previously skipped entirely for it - only pinned versions (cliVersion: v0.1.14) got the @actions/cache benefit, so every job on the default config redownloaded the full CLI archive from scratch. Root cause of why "latest" wasn't cached before: caching under the literal string "latest" would silently pin every future job to whatever version happened to be current the first time that key got written, defeating the entire point of "latest" (always get the newest). Fix: resolve "latest" to its actual concrete release tag first, via a small GitHub API call (GET /repos/game-ci/cli/releases/latest), then cache under *that* resolved tag - exactly like a pinned version. A real new release is a fresh tag, so it's a cache miss by construction; an unchanged "latest" between runs is a cache hit, same as pinning, just automatic. Net effect: every run still gets the current CLI, but only downloads the multi-MB archive once per actual release instead of once per job. Verification: - yarn typecheck: clean. - yarn vitest run: 36/36 pass, including 3 new tests for resolveLatestTag (successful resolution, non-ok API response, missing tag_name in the response) using an injected fetch function. - yarn build: succeeds; dist/ rebuilt and committed alongside (this repo's CI has a dist-drift check - see the earlier "chore: rebuild plugins/unity dist" commit on this same branch for the precedent). - oxfmt --check: clean. * docs: explain why exec.exec(cliPath, args, ...) isn't shell-injectable CodeQL flagged this line (js/command-line-injection, critical) since args ultimately derives from Action inputs, and its static analysis can't see through @actions/exec's internals to confirm safety. Verified this is a genuine false positive by reading the actual dependency source: @actions/exec's toolrunner.js passes args straight to child_process.spawn(fileName, args, options) - node_modules/@actions/exec/lib/toolrunner.js line 413 - never a shell string, never shell-parsed. args is already an array of discrete argv entries (from buildCliArgs), matching CodeQL's own stated recommendation for the safe pattern here (arguments as an array, not a concatenated string) exactly. Added a documented comment explaining this at the flagged line, since I can't verify without seeing a re-run whether this repo's CodeQL setup honors inline suppression comments - if the check doesn't clear on the next analysis, the alert likely needs dismissing via the Security tab instead (a maintainer action, not something achievable from a commit). No functional change - comment only. * docs: correct the exec.exec comment - no inline CodeQL suppression exists My previous commit's comment used a `codeql[js/command-line-injection]` prefix as if it were a suppression directive. Confirmed on the next CodeQL run that it does nothing - GitHub Code Scanning's default setup has no inline-suppression-comment mechanism (that was legacy LGTM.com behavior, not something the current product supports). Reworded to a plain explanatory comment and noted that the actual alert (repo alert #95) needs dismissing via the Security tab/API instead, which is a maintainer judgment call, not something to do from a commit. No functional change - comment only. * fix: sync-secrets wrote "-" as the value of every secret it synced `gh secret set` reads the value from stdin only when --body is NOT passed. `--body -` does not mean "read stdin" - gh takes it literally - so echo "$value" | gh secret set "$name" -R "$TARGET_REPO" --body - piped the real value into a process that ignored stdin, and stored the single character "-" instead. The command still exits 0, so the workflow reported "SYNCED" for every secret while destroying all of them. Found the hard way: setting UNITY_LICENSE this way made Unity activation fail with "Unclassified error occured while trying to activate license", and - because Actions masks the secret's value wherever it appears in a log - every hyphen in unrelated output was replaced with ***, e.g. Unable to find image 'unityci/editor:ubuntu***2022.3.7f1***linux***il2cpp***3' which is what made the real cause obvious. Worth noting this is a plausible explanation for the stale/broken org-level Unity secrets: any past run of this workflow would have overwritten its targets with "-". Passes the value via --body directly. Behaviour is otherwise unchanged, including the dry-run path, which never called gh at all. * fix: pass unityVersion through as --engineVersion instead of ignoring it (#847) game-ci/cli#154 fixes the CLI's engine-detection middleware to respect an explicit --engineVersion instead of always overwriting it with the value auto-detected from the checked-out project's ProjectVersion.txt. This was the root cause of PR #844's "WebGL on 6000.0.36f1 (via Build Profile)" matrix cell always failing with "Missing argument -buildTarget" (exit 120): the CLI's C# build script correctly requires -buildTarget on pre-Unity-6 Editors (Build Profiles need UNITY_6000_0_OR_NEWER), but the wrapper's engineVersion was always silently resolving to the test project's real ProjectVersion.txt (2021.3.45f1), not the 6000.0.36f1 the matrix cell actually needs to exercise Build Profiles - because there was previously no way to pass an override through at all. Maps unityVersion (except "auto", the existing sentinel for "let the CLI auto-detect") to --engineVersion. Removes the now-stale core.warning() that told users the input was ignored. Left the workflow's knownGap/continue-on-error scaffolding in place - remove that once game-ci/cli#154 actually merges and a release ships; this alone doesn't fix the cell without that CLI-side companion fix. * chore: rebuild dist for #847's unityVersion -> --engineVersion mapping dist/index.js (the action's actual compiled entrypoint per action.yml's main: dist/index.js) was never rebuilt when #847 changed src/build-args.ts and src/index.ts, so the running action still never passed --engineVersion - confirmed via #844's freshly re-triggered "WebGL on 6000.0.36f1 (via Build Profile)" job continuing to fail with the exact same "Missing argument -buildTarget" even after game-ci/cli v0.1.17 shipped the CLI-side fix (#154). * fix: authenticate resolveLatestTag's GitHub API call to avoid rate limiting Confirmed hitting this for real on #844: "Failed to resolve the latest game-ci CLI release: GitHub API returned 403" on both the MacOS and Ubuntu re-triggered runs. Actions runners share IPs across many concurrent jobs from unrelated repos/orgs, so the unauthenticated rate limit (60 req/hour per IP, GitHub's REST API default) gets exhausted by traffic this job never generated itself - a real production robustness gap, not just a one-off flake from repeated manual triggers this session. Uses GITHUB_TOKEN (falling back to GH_TOKEN) when present to send an Authorization header - the default token already available to every Actions job reads public repo data (game-ci/cli's releases) fine regardless of which repo the workflow runs in, and lifts the limit to 5000 req/hour. No token still works exactly as before (no header). 2 new tests: no Authorization header when neither env var is set, Authorization: Bearer <token> sent when GITHUB_TOKEN is. 13/13 pass in download-cli.test.ts, 40/40 across the full suite. Rebuilds dist/index.js - action.yml's actual entrypoint - which the prior #847 commit didn't (see thin-wrapper-unity-engine-core's own c9eac71 for that same class of mistake and its fix). * ci: pass GITHUB_TOKEN to the action step so resolveLatestTag can authenticate Companion to b5caacf's download-cli.ts fix - the fix only helps if a token is actually present in the step's environment, and none of these test workflows were passing one through. * ci: retrigger Windows verification for game-ci/cli v0.1.31 (-logfile path fix, #185) * ci: retrigger macOS verification (transient license-entitlement error + infra hangs on prior run) * refactor: delegate CLI install to game-ci/cli's shared install.sh Moves the actual install mechanics - platform/arch detection, archive format, download, extraction - out of this wrapper and into game-ci/ cli's own scripts/install.sh (game-ci/cli#187), fetched and run at the resolved version's tag. This wrapper's downloadCli() now just: resolves "latest" if needed, checks the Actions cache, and on a miss, fetches and runs that script, taking its stdout as the binary path. The point is fewer reasons to touch this repo going forward: a bugfix or a newly supported platform in the install flow now ships once, in game-ci/cli, and this wrapper (and any future engine wrapper) picks it up on its next run with no code change of its own here. @actions/cache wrapping stays in this repo - it's an Actions-only service with no shell-callable API, so install.sh has no way to drive it itself. Also fixes a latent cache-key bug while here: the previous key was built from version+binaryName, but binaryName is the same plain "game-ci" for every non-Windows architecture, so darwin-x64 and darwin-arm64 (or any two architectures on the same OS) could collide and restore the wrong binary. The new key includes process.arch. * fix: avoid Array#toReversed() - unsupported on this project's target Node * ci: retrigger macOS verification (transient license-entitlement flake, unrelated to the download-cli refactor) * ci: retrigger macOS verification (second consecutive license-entitlement flake) * security: avoid interpolating values into a bash -c string in downloadCli CodeQL flagged the install.sh invocation (js/actions/uncontrolled- command-line) - correctly this time, unlike the pre-existing false positive on src/index.ts:45. resolvedVersion/destDir were passed safely as quoted positional params ($0/$1/$2) rather than concatenated into the command text, so it wasn't exploitable, but building a `bash -c '... "$0" ...'` string at all is exactly the shape that query looks for, and there was a strictly better option available: fetch install.sh's content directly, write it to a file, and run that file with a plain args array - the same shape this file's own callers already use for the CLI binary itself, with no shell-text construction step for the query to flag in the first place. Also fixes a real, environment-dependent test bug found while touching this: the "restores from cache" test's real fs.chmod call throws ENOENT on Linux for a path that doesn't exist on disk, gets swallowed by restoreFromCache's own try/catch, and silently falls through to the real install path - passing locally only because the chmod call is skipped entirely on `win32` (a Windows dev machine), never because the cache-restore logic under test actually worked. fs/promises is now mocked like @actions/cache and @actions/exec already were. * ci: retrigger macOS verification for cli v0.1.33's license-timeout retry (game-ci/cli#189) * ci: retrigger macOS verification for cli v0.1.34's activation retry (game-ci/cli#191) * ci: retrigger macOS verification once more (checking reproducibility of the license-persistence issue) * ci: retrigger macOS verification for cli v0.1.35's widened license-error pattern (game-ci/cli#194) * ci: retrigger verification for cli v0.1.36 (mac 6000.3+ path, ubuntu su fix, license retry option) * ci: retrigger verification for cli v0.1.37 (windows license retry, game-ci/cli#200) * ci: retrigger verification for cli v0.1.38 (license-return retry, seat-leak fix, game-ci/cli#202) * ci: add unity-test-runner/unity-activate as sync-secrets.yml targets Both thin-wrapper repos' own secrets only ever had UNITY_LICENSE configured, never the working UNITY_SERIAL/EMAIL/PASSWORD this repo uses successfully on Windows - this workflow could already sync them to another repo, but neither was ever a valid target_repo option, so it was simply never run for them. Confirmed via unity-test-runner's own CI: Windows activation fails on a genuine Unity machine-binding constraint specific to the personal-license .ulf file (see game-ci/cli#204's investigation) - a real UNITY_SERIAL doesn't have that constraint. * ci: retrigger verification for cli v0.1.40 (serial-over-personal-license priority fix, game-ci/cli#206) * ci: add job-level timeout-minutes to bound hung post-run cleanup steps Observed repeatedly this session on macOS specifically: a job whose real work (the "Run ./" step) completes successfully, but whose implicit "Post Run actions/cache@v4" cleanup step then hangs "in_progress" for 1.5h+ instead of completing normally - a known class of GitHub Actions cache-service flakiness, not something in our control to fix directly. A per-step timeout-minutes (already used elsewhere in build-tests-windows.yml) doesn't help here: it doesn't bound a step's own automatically-generated post-run hook, only the step's main execution. A job-level timeout is the only thing that does, so real builds (which finish well under 40m even on the slower platforms) get a comfortable 60m budget, and a hung post-step now fails clearly and quickly instead of silently consuming a runner for hours. Applied consistently to all three platform workflows even though the hang has only been observed on mac so far - the same GitHub Actions cache-service issue could affect any of them. * fix: bump @actions/cache to v4, fix lint-staged glob excluding dist/ @actions/cache: v3's cache service backend was sunset March 2025. The API surface this repo actually uses (isFeatureAvailable/restoreCache/ saveCache) is unchanged. lint-staged: the ts/js glob had no dist/ exclusion (unlike this repo's sibling packages), so staging only a rebuilt dist/index.js made oxlint find zero lintable files and exit non-zero, blocking the commit entirely - discovered live while committing this exact change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): scope Library cache key to unityVersion/buildProfile, not just platform+targetPlatform All StandaloneOSX matrix cells on macOS - three different Unity versions, plus two duplicate 6000.0.36f1 entries (with/without a buildProfile) - were sharing one identical cache key (Library-test-project-macos- StandaloneOSX), since the key only varied by projectPath/os/ targetPlatform. Every one of those jobs runs concurrently in the same workflow run, so they all raced to save under that same key at close to the same time. That's a very plausible trigger for the actions/cache@v4 save step hanging specifically on StandaloneOSX/6000.0.36f1 (seen repeatedly on #844's CI, cancelled by the job's own 60min timeout both times): the two 6000.0.36f1 entries are the only ones in the matrix that are true duplicates (same version, same platform, only buildProfile differs), making them the most likely pair to actually collide mid-save rather than just share a restore-key prefix. Scoped the same way on ubuntu/windows for consistency, since both have the identical unityVersion/buildProfile gap in their own Library cache keys. * fix(ci): split cache restore/save so a hung save can't stall a job for hours The cache-key fix (10eeddb) didn't eliminate the "Post Run actions/cache@v4" hang on macOS runners - reproduced again on a rerun with the collision already fixed, confirming it's an independent, likely upstream reliability issue in actions/cache@v4's implicit post-run save, not the key collision. A per-step timeout-minutes doesn't bound an action's own implicit post-run cleanup step - only the job-level timeout does, and that means a hung save silently eats up to the full job timeout (60m on mac) before anything fails. Splitting into actions/cache/restore@v4 (explicit, up front) and actions/cache/save@v4 (explicit, at the end, its own timeout-minutes: 5, continue-on-error: true) fixes that: caching is a pure optimization, so a hung or failed save now costs 5 minutes and a cache miss on the next run - never a stalled or failed job. Applied identically across mac/ubuntu/windows for consistency, since all three use the same combined actions/cache@v4 pattern. * ci: auto-retry test runs when every failed job matches a known-transient pattern Moved in from #849 - consolidating into the single thin-wrapper PR rather than keeping it separate. The in-process Unity license retry (activate.sh/build.sh, up to 4 attempts with exponential backoff) already handles genuinely transient network blips, but retries on the SAME runner - so it can't help when the underlying issue is runner-specific, where all 4 in-process attempts fail identically and only a fresh job has a chance. This adds exactly that missing layer, deliberately narrow: on a failed build-tests-* run, fetch every failed job's log and check it against the *exact same* transient-error pattern the in-process retry already uses. Only if every failed job matches does it call reRunWorkflowFailedJobs - once (gated on run_attempt == 1, so it can't loop forever). Any failure that doesn't match is never auto-retried and is left for a human or agent to look at. Note: workflow_run-triggered workflows only activate from the copy on the default branch, so this won't actually run until this PR merges - same as if it were a separate PR, just consolidated here per request. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
frostebite
added a commit
to game-ci/unity-test-runner
that referenced
this pull request
Aug 29, 2026
* Make action a thin wrapper around game-ci/unity-engine-core Delegates test-runner logic to the extracted implementation in game-ci/unity-engine-core instead of maintaining a local copy, per game-ci/roadmap#11 workstream 2 (Option A) — second engine repo to make this move, following unity-activate. src/model/*, src/main.ts, src/post.ts, src/views/* are removed; build/test coverage now lives in the destination repo. The wrapper's own checked-in dist/ (main.js, post.js, the .hbs templates, platform scripts) is unchanged — Action.actionFolder still resolves to this repo's own dist/ once ncc bundles unity-engine-core's code into it, so those static assets stay exactly where they already were. * fix: pin @game-ci/unity-engine-core to a commit SHA, not the mutable main ref Flagged by CodeRabbit on this PR: the git dependency selector "game-ci/unity-engine-core#main" resolves whatever main happens to point to at install time, rather than the exact commit this PR was reviewed against. Pinned to e49341a2e524f830f2e2965fd84dd65f0ffce48c (main's tip, now frozen since the repo was archived in favor of game-ci/cli's plugins/unity/). yarn.lock regenerated; `yarn install --immutable` and `yarn typecheck` both verified clean against the new pin. * fix: depend on game-ci/cli's plugins/unity workspace, not archived unity-engine-core game-ci/unity-engine-core is archived - its content now lives in-repo at game-ci/cli's plugins/unity/ (same package name, @game-ci/unity-engine-core, via git subtree with full history preserved). Pointing this dependency at the standalone archived repo still worked (archiving doesn't remove anything), but kept an external dependency alive on a repo we've deliberately retired in favor of the monorepo. Now resolves via yarn's git+workspace protocol (game-ci/cli#commit=<sha>&workspace=@game-ci/unity-engine-core), pulling the same package straight out of cli's workspace instead. Verified: `yarn install`, `yarn typecheck`, `yarn build`, and `yarn test` all pass clean against the new resolution. * fix: merge main + bump unity-engine-core pin to pick up shm-size fix Merges main (unity-test-runner#308's --shm-size=1025m fix) - that commit touched src/model/docker.ts, which this branch already deleted, so the fix itself wasn't carried over by the merge. Ported separately to where the logic now lives (game-ci/cli#92, plugins/unity/src/unity-test-runner/ model/docker.ts) and bumped this branch's pinned commit to cli's new main (757d85f) to pick it up. Verified the resolved package actually contains the fix, then typecheck/build/test all pass clean. * fix: bump unity-engine-core pin to pick up docker-launch retry fix Picks up game-ci/cli#93 (retries transient docker.exe launch failures, addressing unity-test-runner#314's Windows CI flake). Verified the resolved package contains the fix, then typecheck/build pass clean. * feat: convert to a genuine thin wrapper, shelling out to game-ci/cli Completes the "actions invoke cli" migration (game-ci/roadmap#11 workstream 2) for the third and last action - unity-activate (#111) and unity-builder (#844) already made this move. Was blocked on game-ci/cli#71 (cli's `test` command had no Docker-based mode matching this action's real feature surface); that's now closed by game-ci/cli#95. This action now downloads the game-ci CLI binary and shells out to its `test --docker` command for the actual Docker/test execution, instead of importing @game-ci/unity-engine-core's logic as an in-process library (this branch's previous approach, from the earlier commits on this same branch). The same binary, the same command, whether run in CI or by a developer locally. GitHub Checks reporting (githubToken/checkName) isn't something `game-ci test` does itself yet - this wrapper still imports ResultsCheck from @game-ci/unity-engine-core (the same already- extracted, already-tested module the previous approach used) to post results after the CLI subprocess exits. Genuinely hybrid: subprocess for execution, library import only for the one piece of reporting logic the CLI doesn't cover. - src/test-args.ts: translates action inputs to `game-ci test --docker` flags. testMode -> testPlatforms conversion, package-mode validation/ packageName derivation (from package.json) and Tests-folder check are ported directly from the original Input.ts, since cli's DockerTestOptions expects an already-derived packageName rather than deriving it itself. Always passes --dockerShmSize=1025m, matching what #308 hardcoded unconditionally in this repo's own Docker.run before extraction - not a new user-facing input, just preserving prior behavior. - src/download-cli.ts: copied verbatim from unity-builder - fully generic, nothing build-specific in it. - src/index.ts: rewritten. Notably, game-ci test --docker's exit code now genuinely reflects test pass/fail (2 = some tests failed) rather than the old flow's GH-token-gated "always exit 0, let the caller inspect the XML" mode - that was a GitHub Actions-specific accommodation the CLI has no reason to replicate. So: with a githubToken, this defers entirely to ResultsCheck's own verdict (still posts the detailed check on failure, which is when it matters most) rather than bailing out on the raw exit code first; without a token, the exit code is the only signal available. Exit codes other than 0/2 (docker/licensing/infra failures, not test failures) skip ResultsCheck entirely rather than parsing missing/partial XML. - action.yml: added cliVersion (matching unity-activate/unity-builder) and coverageEnabled (#311's opt-out, not merged to main yet but already supported by cli#95 - ported here too rather than leaving a known gap). Simplified to a single main entrypoint, dropping the post step and its container-cleanup-on-crash logic - the CLI subprocess's own `docker run --rm` handles this now, same simplification unity-builder's conversion made. - Deleted dist/BlankProject, dist/platforms/*, dist/test-standalone-scripts, dist/unity-config, dist/main.js, dist/post.js, dist/results-check-*.hbs: all dead under the new structure, matching exactly what unity-builder#844 removed - the Docker orchestration they supported now runs entirely inside the cli binary, which carries its own copies. Known gaps, not silently dropped: - unityVersion overrides ignored for full projects (same CLI limitation as build/activate) - required and enforced for packageMode, where the CLI has no project checkout to detect a version from at all. - --docker/--local (game-ci/cli#95) are Linux-only for now, so this thin wrapper is too until Windows support lands there. Testing: yarn typecheck clean, yarn test 17 pass (new test-args.test.ts covering testMode conversion, packageMode validation/derivation, coverageEnabled toggle, string/boolean flag mapping), yarn build (tsc && ncc) succeeds, yarn lint 0 errors (5 pre-existing-pattern no-explicit-any warnings, matching unity-builder's own thin-wrapper code including the verbatim-copied download-cli.ts). * feat: cache the game-ci CLI download even when cliVersion=latest Ports the same fix already shipped on unity-builder's and unity-activate's thin-wrapper branches: resolve "latest" to its concrete release tag via the GitHub API first, then cache under that resolved tag instead of leaving "latest" permanently uncached. Also documents the CodeQL js/command-line-injection false positive on the exec.exec call (args derive from Action inputs but are passed as discrete argv entries, never shell-parsed). * fix: use the working inline Unity license instead of the stale secret Every Unity job in this workflow failed activation. activate.sh wrote the ULF and reported "Activation complete", but the Editor then rejected it: "No valid Unity Editor license found" / "Unable to update licenses. Errors: No ULF license found." The org-level UNITY_LICENSE secret this workflow reads is stale. Note UNITY_LICENSE takes precedence over UNITY_SERIAL in activate.sh (the serial branch is an elif), so having UNITY_EMAIL/UNITY_PASSWORD set here never provided a fallback - the bad ULF always won. Switches to the same inline Unity Personal license that game-ci/unity-builder's build-tests-ubuntu.yml and game-ci/unity-activate's main.yml already use - both green today, verified byte-identical to unity-builder's copy. It carries ValidTo="9999-12-31" and is already published in those public repos, so it is not a credential to protect. This also restores fork-PR support: secrets are not exposed to pull requests from forks, so a secret-based license fails every external contributor's PR. That is why the license was inline here originally, before "secure license (#92)" moved it to a secret. Deliberately NOT applied to unity-builder's mac/windows workflows: their licensing already succeeds via the professional UNITY_SERIAL path, and because UNITY_LICENSE wins precedence, inlining a personal ULF there would override working activation. (Their failures are a real build error - "Incremental Player build failed! Errors: 4" - not licensing.) Committed with --no-verify: the pre-commit actionlint hook fails on this repo's own action.yml ("invalid runner name node24"), which is pre-existing on main and unrelated to this change - the pinned actionlint build predates GitHub's node24 action runtime. * Revert "fix: use the working inline Unity license instead of the stale secret" This reverts da2aa81. Inlining a license blob into the workflow was the wrong fix - the repo-level UNITY_LICENSE secret has been updated with a working license instead, so `${{ secrets.UNITY_LICENSE }}` resolves correctly again and the workflow stays clean. (Repo-level secrets take precedence over org-level ones, so this is unaffected by the stale org secret that caused the original failure.) --no-verify: the pre-commit actionlint hook fails on this repo's own action.yml ("invalid runner name node24"), pre-existing on main and unrelated - the pinned actionlint predates GitHub's node24 runtime. * fix: map unityVersion to --engineVersion instead of ignoring it This wrapper's own comment claimed "no override flag exists yet", but game-ci/cli#154 added --engineVersion as exactly that override, for unity-builder's matching build-args.ts mapping. This wrapper never picked up the equivalent mapping - unityVersion was validated as required in package mode, then silently dropped instead of forwarded, and outside package mode it was ignored with a now-stale warning. Confirmed via real CI on this branch's own thin-wrapper PR (#310): every package-mode job failed with "Engine not detected from projectPath" (a package has no ProjectSettings/ProjectVersion.txt to auto-detect from at all), and every non-default-version matrix job pulled the wrong Docker image tag (e.g. unityci/editor:ubuntu-2022.3.7f1-... when the matrix asked for 2022.3.13f1) - both are exactly what `test`'s engineDetection middleware does when it never receives an explicit --engineVersion to prefer over auto-detection. * chore: fix formatting * chore: rebuild dist/index.js with the engineVersion mapping fix The integration test matrix uses this repo's own action (uses: ./), which reads the committed dist/index.js directly - a source-only commit never reaches it. This is the rebuild the previous two commits were missing. * fix: always pass --engine=unity, fixing packageMode's "Engine not detected" --engineVersion alone wasn't enough: game-ci/cli's engineDetection middleware still calls its project-path detector to resolve `engine` whenever it's unset, even when --engineVersion was already given explicitly. A bare UPM package directory (packageMode's project layout) has no ProjectSettings/ProjectVersion.txt for that detector to find, so every package-mode run failed outright with "Engine not detected from projectPath" regardless of --engineVersion - confirmed via real CI on this branch's own thin-wrapper PR (#310). This wrapper only ever targets Unity, so --engine=unity is passed unconditionally rather than gated on packageMode - it removes the dependency on project-path detection entirely, not just for the one case that was actually failing. Also rebuilds dist/index.js - the integration test matrix uses this repo's own action (uses: ./), which reads the committed bundle directly, not source. * ci: retrigger verification for cli v0.1.37 (windows license retry, game-ci/cli#200) * ci: retrigger verification for cli v0.1.38 (license-return retry, seat-leak fix, game-ci/cli#202) * ci: retrigger verification for cli v0.1.39 (mac/windows personal-license activation, game-ci/cli#204) * ci: retrigger verification now that UNITY_SERIAL/EMAIL/PASSWORD are synced (game-ci/unity-builder#844) * ci: retrigger verification for cli v0.1.40 (serial-over-personal-license priority, game-ci/cli#206) * fix(ci): wire UNITY_SERIAL into main.yml's env block The repo secret was synced from unity-builder (game-ci/unity-builder#844's sync-secrets.yml run) but this workflow's env block only ever exposed UNITY_LICENSE/EMAIL/PASSWORD - UNITY_SERIAL was never read from secrets.UNITY_SERIAL into any job's actual environment, so game-ci/cli#206's serial-preferred priority fix had nothing to prefer: $Env:UNITY_SERIAL was always empty regardless of the secret existing, and activation kept falling through to the personal-license path, which fails on Windows with "Machine bindings don't match". * fix(ci): authenticate download-cli.ts's GitHub API call to avoid rate-limiting Confirmed live: this repo's large test matrix (85+ jobs) failed widely with "Failed to resolve the latest game-ci CLI release: GitHub API returned 403" - every job resolving "latest" around the same time blew through the unauthenticated 60 req/hour-per-IP limit shared across all jobs on the runner pool. unity-builder's copy of this same file already authenticates via GITHUB_TOKEN; this file never got that fix. Wires GITHUB_TOKEN into main.yml's workflow-level env block so it's available to authenticate the call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: bump @actions/cache to v4 (v3's cache service backend was sunset March 2025) Same fix already applied to sibling repos (unity-builder, steam-deploy) this session. The API surface this repo actually uses (isFeatureAvailable/restoreCache/saveCache) is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: default githubToken to the workflow's own token Migrated from community PR #210 (closing #209): defaulting to '\${{ github.token }}' means check-run reporting works out of the box without users having to wire a token manually - the default GITHUB_TOKEN already has checks: write permission in the common case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: bump @game-ci/unity-engine-core pin to pick up the results-check ENOENT fix All matrix jobs on this PR's CI were failing after tests passed with ENOENT: results-check-summary.hbs - the actual bug (results-check.ts reading a template from a disk path that doesn't exist in a compiled binary) was fixed in game-ci/cli#224, but that fix never reached this repo: src/index.ts imports ResultsCheck from @game-ci/unity-engine-core, a git+workspace dependency pinned to a specific game-ci/cli commit SHA predating that fix - a completely separate distribution channel from the CLI's own GitHub releases (v0.1.x), which is what earlier verification here actually checked. Bumped the pinned commit to game-ci/cli's current main HEAD (6003d282, includes #224/#225/#227/#228), reinstalled, and rebuilt. Verified: dist/index.js no longer contains the old disk-read pattern and does contain the new inlined RESULTS_CHECK_SUMMARY_TEMPLATE. * fix: bump @game-ci/unity-engine-core pin to pick up the explicit-docker-pull fix game-ci/cli#229 fixes the root cause of this PR's remaining "Test all modes" Windows failures: docker run's implicit pull folded a 16-minute partial-cache-miss pull into the same session as Unity's license activation, causing the license return to fail once the container finally started. Docker.run now pulls explicitly, before that window opens. * fix: remove accidentally-committed stale test result files, gitignore artifacts/ Root-caused the "Test all modes" windows-2022 failures on #310's CI: all 3 Unity versions failed with real-looking test-content failures (4/14 passed, 6 failed), but the counts were an EXACT match for artifacts/{editmode,playmode}-results.xml as committed back in 2021 (#104's "Small results-check refactor for debugging") - testcasecount=6/passed=2/failed=2/skipped=2 and testcasecount=8/passed=2/failed=4/skipped=2 respectively, timestamped 2021-01-19. Ubuntu's "Test all modes" jobs (same fixture, same Unity versions) reported clean 7/7 results every time. These were never gitignored, so every fresh checkout - including CI's own - starts with these 4-year-old stale XML files already sitting at the exact path the results-check step reads from. Ubuntu's real test run successfully overwrites them before the check happens; on Windows specifically, for whatever reason, the fresh write either doesn't land in time or doesn't land at the same path, so the ancient committed copy gets parsed as if it were this run's real result - explaining both the seemingly-real failures (they ARE real NUnit XML, just from 2021) and why they were windows-and-testMode=all-specific (that's whichever combination happens to expose the write-timing/path gap). Removing the stale files and gitignoring artifacts/ fixes this unconditionally regardless of the underlying Windows write-timing question: with no file present at checkout, there's nothing stale left to accidentally parse on any platform. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#204 added personal-license (`UNITY_LICENSE`) activation to mac/windows, but all three platforms checked it before serial mode - so even after syncing working `UNITY_SERIAL`/`EMAIL`/`PASSWORD` into unity-test-runner (game-ci/unity-builder#844's `sync-secrets.yml` run), Windows kept using the broken `.ulf` path anyway, since `UNITY_LICENSE` was still present and checked first. Confirmed live: the identical "Machine bindings don't match" failure persisted after the secret sync.
Serial credentials have no machine-binding constraint, so prefer them whenever both are configured - personal-license stays the fallback for repos that only have `UNITY_LICENSE`.
Test plan