fix: prefer personal activation over a stale .ulf, not extraction - #255
Merged
Conversation
Supersedes the approach in #254 (v0.1.54): that release tried to extract the serial embedded in a personal .ulf and reuse it for a portable, account-bound serial activation. Verified correct end-to-end against a synthetic license file (mutation, docker env var construction, shell quoting - all directly tested), but still failed in production against at least one real user's actual .ulf (MirrorNetworking/Mirror#4127, "Machine bindings don't match" even after upgrading to v0.1.54) - something about a real, current personal .ulf's shape doesn't match closely enough for extraction to produce something Unity's licensing client accepts, and there's no way to get a real sample to debug further since it's a secret. Two independent users separately arrived at the same working fix by hand: drop the .ulf entirely and provide just the account credentials. This replaces the extraction attempt with exactly that - whenever a license file and full account credentials are both present and no genuine serial was given, force --unityLicensingMethod=personal through the existing explicit-override mechanism, the same escape hatch a user would reach for manually. No parsing, no new per-platform script logic, same reasoning both users already validated by hand. Co-Authored-By: Claude Sonnet 5 <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughUnity licensing now prefers personal activation when account credentials and a license file are present. Serial extraction from license files was removed. Tests cover activation selection and no-op conditions. ChangesUnity licensing selection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Unity activation now selects personal licensing when credentials and a license file are supplied, while preserving serial and explicit-method overrides. The covered selection paths leave no current merge-blocking risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
7 tasks
frostebite
added a commit
that referenced
this pull request
Sep 9, 2026
…256) Two related changes to how UNITY_LICENSING_METHOD auto-resolution works, both aimed at the same root problem: two real regressions (#254, #255) took multiple rounds to fully diagnose because nothing said out loud which of several plausible activation strategies actually got used, and the four platform scripts didn't even agree with each other about which one that should be. 1. Unify precedence across all four platform scripts. ubuntu, mac and windows/steps already agreed: file -> serial -> floating -> personal. The windows *container* script set diverged in two ways that predated this session entirely: it checked floating before serial, and its catch-all attempted a doomed serial activation with whatever credentials happened to be set (including none at all) instead of the same clear "could not be determined" guidance the other three already gave. Aligned the minority to the majority rather than the reverse, so this only changes behaviour for the narrow edge cases where the old windows-container-only order actually mattered: a build that set both a complete serial triple and a licensing server (took floating, now takes serial, matching every other platform), or one with no usable credentials at all (previously ran Unity with empty -serial/-username/-password and failed with Unity's generic error, now gets the same guidance message every other platform already gives). 2. Warn out loud whenever a credential naming a *specific* strategy (a .ulf, a real serial, a licensing server) is silently overridden by a different auto-resolved strategy. This is purely additive - it changes no resolution outcome on any platform, just makes an already-surprising outcome visible in the log instead of only discoverable by reading source. An explicit --unityLicensingMethod is never second-guessed and never triggers it. New test coverage: scripts/test-licensing-steps.ps1 is a new sibling of the existing bash suite, covering both Windows script sets end-to-end (no PowerShell test harness existed for them before this - a syntax check alone would not have caught a mistake in the precedence change). Wired into tests.yml; pwsh is preinstalled on ubuntu-latest, so no Windows runner is needed. 5 new bash cases and 30 new PowerShell cases cover the warning and the unified precedence directly. Co-authored-by: Claude Sonnet 5 <[email protected]>
7 tasks
frostebite
added a commit
that referenced
this pull request
Sep 9, 2026
…ch (#258) * fix: fall back from file to personal only on a machine-binding mismatch Supersedes #255 (v0.1.55): that release unconditionally preferred personal activation over a license file whenever both a .ulf and full account credentials were present, forcing --unityLicensingMethod statically before any script ran. It fixed "Machine bindings don't match" for the Unity versions that hit it, but broke activation outright on older ones - confirmed live against MirrorNetworking/ Mirror: 2020.3.49f1's file-mode activation had been succeeding the whole time (it's in the very first pre-fix CI run, before any of this session's changes), and broke only after v0.1.55 forced it onto personal instead, whose Unity.Licensing.Client 1.12.1 (bundled with that Unity version) doesn't recognise --activate-all/--include-personal at all - those flags exist only in a newer client (1.18.3, bundled with 6000.6.0f1, which is exactly where machine-binding activation fails). A static, version-blind preference cannot get both cases right at once - whichever way it defaults, some Unity version's already-working setup breaks. The real fix has to be reactive, not a static up-front choice: try `file` first, unconditionally - so every setup that already worked, on any Unity version, keeps working exactly as before - and fall back to `personal` only when that fails with the one specific, recognisable "Machine bindings don't match" signature, which is the only case that was ever actually broken. Implemented in each platform's own activate.{sh,ps1} rather than centrally, reusing each platform's existing personal-activation retry loop inline. A successful fallback sets GAME_CI_ACTIVATED_VIA=personal so return_license.{sh,ps1} returns the seat it actually consumed instead of leaking it - the static env vars alone would still resolve to `file` (nothing to return) and silently leak a real personal seat. Verified this persists correctly across each platform's actual process/scope boundary: bash sources activate.sh and return_license.sh in the same shell session (runsteps.sh), and while entrypoint.ps1 invokes each Windows container step with `&` rather than dot-sourcing, $Env: variables are process-wide regardless, so they still carry over. licensing-method.ts's TypeScript-side preferPersonalOverFile is removed entirely, along with its cli.ts call site - this needed to be a runtime, activation-log-reactive decision, which only the platform scripts can make. Test plan: - 10 new bash cases (scripts/test-licensing-steps.sh, run against a stub Unity.Licensing.Client/unity-editor): the fallback engages and succeeds on the exact failure signature with credentials available; does not engage without credentials (still reports the original failure); does not engage for a different, unrelated failure; and a successful fallback resolves the return strategy to personal. - 2 new PowerShell cases per script set (container + steps, both now covered by scripts/test-licensing-steps.ps1) verifying the return strategy honours GAME_CI_ACTIVATED_VIA. - bash scripts/test-licensing-steps.sh: 47/47 pass - pwsh scripts/test-licensing-steps.ps1: 34/34 pass - bash scripts/validate-platform-scripts.sh: clean - bun test ./src: same 2 pre-existing, unrelated failures as clean main (confirmed via stash-and-compare), no regressions - node scripts/generate-embedded-assets.mjs --check: clean Co-Authored-By: Claude Sonnet 5 <[email protected]> * test: assert file activation runs before the personal fallback Addresses CodeRabbit feedback on #258: the existing assertions prove personal activation ran and succeeded, but not that unity-editor (the file attempt) ran first - a regression that skipped straight to personal could satisfy them by emitting the same fallback output. Checking the first ARGV_LOG line is EDITOR makes the file-first contract explicit instead of relying on it indirectly. Co-Authored-By: Claude Sonnet 5 <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]>
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
Supersedes the approach in #254 (shipped as v0.1.54).
That release tried to extract the serial embedded in a personal
.ulfand reuse it for a portable, account-bound serial activation - avoiding the "Machine bindings don't match" error a raw.ulfhits on any machine other than the one it was originally activated for (which is every ephemeral CI container, every run).I verified that approach correct end-to-end against a synthetic license file: mutation of
options.unitySerial, the resulting Docker env var construction, and the shell-quoting of the finaldocker runcommand were all directly tested and confirmed working.It still failed in production against at least one real user's actual
.ulf(MirrorNetworking/Mirror#4127 - same "Machine bindings don't match" error, even after upgrading to v0.1.54, confirmed via the run log). Something about a real, current personal.ulf's shape doesn't match closely enough for extraction to produce something Unity's licensing client actually accepts - and there's no way to get a real sample to debug further, since it's correctly treated as a secret.Two independent users (Mirror's maintainer and a separate Discord reporter, "freakadelle") separately arrived at the exact same working fix by hand: drop the
.ulfentirely and provide just the account credentials (UNITY_EMAIL/UNITY_PASSWORD).This PR replaces the extraction attempt with exactly that, more simply and more robustly: whenever a license file (
unityLicenseorunityLicenseFile) and full account credentials are both present, and no genuine serial was given, force--unityLicensingMethod=personalthrough the already-existing explicit-override mechanism (resolveLicensingMethod) - the same escape hatch a user would reach for manually. No XML/base64 parsing, no new per-platform script logic - it needs none, sinceresolveLicensingMethod's explicit override already wins on every platform unconditionally.Deliberately narrow, same as the previous version:
unitySerialis already set - a real Pro/Plus/Enterprise serial is never second-guessed.auto--unityLicensingMethodis already set - includingfile, for someone who deliberately wants the raw-file behaviour (e.g. a self-hosted runner with a persistent, already-matching.ulfand no account credentials at all).unityEmailandunityPassword- nothing to activate personally with.Test plan
bun test ./src/logic/unity/license/licensing-method.test.ts- 15/15 passbun test ./src- same 2 pre-existing, unrelated failures as cleanmain(confirmed via stash-and-compare), no regressionsplugins/unity/distchanges made it into the commit (CRLF/mtime noise from this checkout, verified zero real content diff)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes