fix: treat a logged successful license return as success - #276
Conversation
Ivan reported still seeing "attempt 3/4" on v0.1.64, and answered the diagnostic question: the Machine Id is identical before and after, six times in one run. That ruled out the binding mismatch v0.1.64 fixed, and sent me looking for a second cause. Found it by measuring, on 2022.3.62f3, same container, matching machine id: [Licensing::Module] Error: Access token is unavailable; failed to update [Licensing::Module] Error: Failed to return entitlement license [Licensing::Client] Successfully returned ULF license with serial number: "..." exit=1 The return SUCCEEDS and Unity exits non-zero anyway. The seat really is gone - the cleanup step immediately after reports "Ulf license file not found". But the exit code says failure and the log carries "Access token is unavailable", which is in the transient list, so this retried four times against an already-returned licence and then warned that the return had failed. So it is a false failure, and the previous fix could not have helped: its guard keys on "Machine bindings don't match", which never appears here. Success is now read from the log rather than the exit code - the same thing activation already does, for the same reason, in the opposite direction. Applied to all four return scripts. Three tests, verified to fail with the guard disabled, including one asserting the editor is invoked exactly once so a silent regression to retrying is caught. Also fixed: five test cases carried "ci\example.com" instead of "ci@example.com", from the same placeholder substitution that did this once before. Harmless to the assertions, because the expectation was corrupted identically - which is why it survived a second time. Suites: 70 bash, 34 PowerShell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe licensing test script now passes unescaped email addresses and verifies that a successful ULF return is accepted even when the editor exits non-zero. ChangesLicensing test updates
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The regression test covers the prior retry behavior for a successful license return with a nonzero editor exit. No actionable merge risk remains. 🚥 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 |
With credentials, the editor really does hand a Personal seat back:
[Licensing::Module] Successfully returned the entitlement license
Measured on 2020.3.49f1, 2022.3.62f3 and 6000.6.0f1 in this repo's licensing
matrix. But that string was not in the success pattern, so the script did not
recognise its own success - and the editor then attempts a ULF return it cannot
do and says "Serial number unavailable for ULF return", which IS in the
transient list.
So 2020.3 returned the same already-returned licence four times, burnt ~2.5
minutes of backoff, and warned that the return had failed. Exactly the failure
mode #276 fixed for the serial path, on a string that path never emits.
scripts/test-licensing-steps.sh: 83 -> 86 tests. All three new assertions, plus
two existing ones, fail with the string removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: return the Personal seat with a command that can return it
`Unity.Licensing.Client --return-ulf` asks for a *file*. A Personal seat is an
entitlement, and no Unity_lic.ulf is written for it, so the return has never
worked:
Returning personal license seat
An error occured while trying to return the ULF license.
Ulf license file not found (/root/.local/share/unity3d/Unity/Unity_lic.ulf) (1404)
##[warning] Failed to return the Personal license seat after 4 attempts.
Measured on all six Unity versions of a user's run - 2020.3.49f1, 2021.3.45f2,
2022.3.62f3, 2023.2.22f1, 6000.3.23f1, 6000.6.0f1 - across licensing clients
1.12.1, 1.15.4, 1.16.2 and 1.18.3. Every one activated cleanly and then failed
to hand the seat back with the identical error, so this is not specific to the
editor fallback on old clients: --return-ulf is the wrong command for every
Personal activation this CLI performs.
The return now recognises a missing .ulf and hands the seat back through the
editor's -returnlicense instead, which is how the serial strategy already
returns an entitlement. It also picks its initial route the way activate.sh
picks its own - by probing the client's help text - so the two stay in step by
construction rather than by a flag one side has to remember to set.
Three defects surfaced by the same log:
- The capability helpers lived in activate.sh, which return_license.sh does
not source. Under runsteps.sh both run in one shell so it happened to be in
scope, but `game-ci return-license` on its own would have called an
undefined function. Moved beside unity_licensing_client_path.
- Both PowerShell ports of the v0.1.65 success-from-log guard broke out of the
retry loop without clearing the exit code, so a return that had succeeded
still warned that it failed.
- The failure warning hardcoded the retry maximum, so a return that broke out
on attempt 1 told the user it had tried four times. Two users read that
number off their logs while diagnosing this. It now reports what happened.
Nothing to hand back on either route now reports "nothing to return" rather
than warning about a leaked seat, which is its opposite.
scripts/test-licensing-steps.sh: 70 -> 80 tests. Each new assertion was
confirmed to fail with its fix reverted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test: assert the client route is tried before the editor fallback
The fallback case asserted only that -returnlicense was reached, so a run that
never tried the client at all would have passed it - which is the failure the
route selection itself could introduce. Confirmed by forcing the probe to
choose the editor: the new assertion fails, the old one did not.
Raised by CodeRabbit on #277.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: give the personal editor return the credentials it needs
Routing the return to the editor was necessary but not sufficient. Without
-username/-password the editor cannot refresh its access token, so the
entitlement return fails and it falls through to a ULF return it also cannot
do:
[Licensing::Module] Error: Access token is unavailable; failed to update
[Licensing::Module] Error: Failed to return entitlement license
[Licensing::Client] Successfully queried for the EntitlementGroupsDetails
[Licensing::Module] Error: Serial number unavailable for ULF return
Measured in this repo's own licensing matrix on 2020.3.49f1, 2022.3.62f3 and
6000.0.36f1, running this branch's scripts. The Machine Id is identical either
side of the return, which rules out the binding mismatch and leaves the missing
credentials as the cause.
The serial branch has passed them for exactly this reason since
game-ci/unity-test-runner#310, whose own comment records the same error string:
"without them Unity has no way to tell this was a serial-mode activation and
instead attempts a personal-license (ULF) return, which fails immediately with
'Serial number unavailable for ULF return'". The personal branch needed the
same treatment.
scripts/test-licensing-steps.sh: 81 -> 83 tests. Both new assertions were
confirmed to fail with the credentials removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: recognise the editor's wording for a returned entitlement
With credentials, the editor really does hand a Personal seat back:
[Licensing::Module] Successfully returned the entitlement license
Measured on 2020.3.49f1, 2022.3.62f3 and 6000.6.0f1 in this repo's licensing
matrix. But that string was not in the success pattern, so the script did not
recognise its own success - and the editor then attempts a ULF return it cannot
do and says "Serial number unavailable for ULF return", which IS in the
transient list.
So 2020.3 returned the same already-returned licence four times, burnt ~2.5
minutes of backoff, and warned that the return had failed. Exactly the failure
mode #276 fixed for the serial path, on a string that path never emits.
scripts/test-licensing-steps.sh: 83 -> 86 tests. All three new assertions, plus
two existing ones, fail with the string removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The return was succeeding all along
A user reported still seeing
attempt 3/4on v0.1.64, and answered the diagnostic question: the Machine Id is identical before and after, six times in one run. That ruled out the binding mismatch v0.1.64 fixed and sent me looking for a second cause.Measured on 2022.3.62f3, same container, matching machine id:
The return succeeds and Unity exits non-zero anyway. The seat really is gone — the cleanup step immediately after reports
Ulf license file not found. But the exit code says failure and the log carriesAccess token is unavailable, which is in the transient list, so it retried four times against an already-returned licence and then warned the return had failed.A false failure. The previous fix could never have helped: its guard keys on
Machine bindings don't match, which never appears here.Fix
Success is read from the log rather than the exit code — the same thing activation already does, for the same reason, in the opposite direction. All four return scripts.
Tests
Three, each verified to fail with the guard disabled, including one asserting the editor is invoked exactly once so a silent regression back to retrying is caught.
Suites: 70 bash, 34 PowerShell.
Also
Five test cases carried
ci\example.cominstead ofci@example.com, from the same placeholder substitution that did this once before. Harmless to the assertions — because the expectation was corrupted identically, which is exactly why it survived a second time.🤖 Generated with Claude Code
Summary by CodeRabbit