Skip to content

fix(ci): prove Unity cleanup before lock release#270

Merged
Eli Pinkerton (wallstop) merged 8 commits into
masterfrom
dev/wallstop/resource-lifecycle
Jul 13, 2026
Merged

fix(ci): prove Unity cleanup before lock release#270
Eli Pinkerton (wallstop) merged 8 commits into
masterfrom
dev/wallstop/resource-lifecycle

Conversation

@wallstop

@wallstop Eli Pinkerton (wallstop) commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • emit fail-closed Unity cleanup proof without masking job results
  • bound all final cleanup steps and pass proof to lock release
  • enforce exact acquire -> licensed work -> cleanup -> release contracts across six windows

Verification

  • focused lifecycle tests: 12/12
  • Prettier and actionlint
  • adversarial implementation/re-review: zero remaining issues

Depends on Ambiguous-Interactive/ambiguous-organization-build-lock#25.


Note

Medium Risk
Changes how org Unity locks are released when license return is unproven (depends on ambiguous-organization-build-lock#25); mis-signaling could quarantine runners or leak seats, though return steps remain non-fatal to job outcome.

Overview
Adds fail-closed Unity license cleanup proof so organization lock release can tell a confirmed seat return from an uncertain one, without making the return step fail the job.

The return-unity-license composite now exposes a resource-safe output (defaults false; only set true on exit code 0 or the existing allowlisted nonzero log pattern). Unity workflows give the return step an id, a 5-minute timeout, continue-on-error: true, and pass resource-safe into release-build-lock@v1 on the following Release organization Unity lock step. The same pattern is applied across six licensed windows (unity-tests, benchmarks, perf, release editmode/unitypackage, GameCI experiment); GameCI’s comment now treats the backstop as real using the provisioned editor path instead of documenting a no-op.

ci-aggregate-workflow.test.js locks in acquire → licensed work → return → release ordering and the return action’s non-masking exit 0 contract.

Reviewed by Cursor Bugbot for commit 957e109. Bugbot is set up for automated code reviews on this repo. Configure here.

…ce-lifecycle

# Conflicts:
#	.github/workflows/perf-numbers.yml
#	.github/workflows/release.yml
#	.github/workflows/unity-benchmarks.yml
#	.github/workflows/unity-gameci-experiment.yml
#	.github/workflows/unity-tests.yml
#	scripts/__tests__/ci-aggregate-workflow.test.js
Copilot AI review requested due to automatic review settings July 12, 2026 20:13
@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Please review cleanup proof classification, non-masking timeout behavior, and all lock-window contracts.

@wallstop

Copy link
Copy Markdown
Collaborator Author

Cursor (@cursor) review

Please review cleanup proof classification, non-masking timeout behavior, and all lock-window contracts.

Comment thread .github/workflows/unity-gameci-experiment.yml
Comment thread .github/workflows/unity-tests.yml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens CI’s licensed Unity “lock window” lifecycle by emitting a fail-closed cleanup proof from the return-unity-license composite action and wiring that proof into lock release steps across all Unity-licensed workflows, with contract tests enforcing the acquire → work → cleanup → release ordering.

Changes:

  • Add a fail-closed resource-safe output to .github/actions/return-unity-license (defaults false, flips true only on proven successful return conditions, and still never fails the job).
  • Update all six Unity lock windows to: assign an explicit id to the return step, bound it with a 5-minute step timeout, and pass resource-safe into release-build-lock@v1.
  • Extend scripts/__tests__/ci-aggregate-workflow.test.js with workflow-contract tests for ordering and for return-action proof classifications.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/__tests__/ci-aggregate-workflow.test.js Adds contract tests enforcing six Unity lock windows’ lifecycle ordering and proof wiring, plus proof-classification assertions for the return action.
.github/workflows/unity-tests.yml Adds return_unity_license step id + timeout and forwards resource-safe into the lock release step.
.github/workflows/unity-gameci-experiment.yml Updates comment to reflect proof limitations, adds return step id + timeout, forwards resource-safe into lock release.
.github/workflows/unity-benchmarks.yml Adds return step id + timeout and forwards resource-safe into the lock release step.
.github/workflows/release.yml Adds return step id + timeout in both Unity jobs and forwards resource-safe into each lock release step.
.github/workflows/perf-numbers.yml Adds return step id + timeout and forwards resource-safe into the lock release step.
.github/actions/return-unity-license/action.yml Introduces resource-safe composite output and sets it fail-closed unless return is proven successful.

Copilot AI review requested due to automatic review settings July 12, 2026 20:19
@wallstop Eli Pinkerton (wallstop) removed the request for review from Copilot July 12, 2026 20:19

Copilot AI commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Please review cleanup proof classification, non-masking timeout behavior, and all lock-window contracts.

Addressed in 2f8866c: the six bounded Return Unity license steps now use continue-on-error: true so a GitHub-enforced timeout cannot mask the licensed job result, and the CI aggregate workflow contract test now pins that invariant alongside the existing cleanup proof and lock-release wiring.

@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Please review exact head: all cleanup timeouts are non-masking, and the GameCI cleanup documentation now matches its provisioned-editor proof behavior.

Copilot AI review requested due to automatic review settings July 12, 2026 20:28
@wallstop

Copy link
Copy Markdown
Collaborator Author

Cursor (@cursor) review

Please review exact head: all cleanup timeouts are non-masking, and the GameCI cleanup documentation now matches its provisioned-editor proof behavior.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b2e9793. Configure here.

Copilot AI commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Please review exact head: all cleanup timeouts are non-masking, and the GameCI cleanup documentation now matches its provisioned-editor proof behavior.

Reviewed exact head b2e9793: no issues found.

  • All Unity cleanup backstops remain timeout-bounded with continue-on-error: true, and scripts/__tests__/ci-aggregate-workflow.test.js still enforces that non-masking contract across every lock window.
  • .github/workflows/unity-gameci-experiment.yml now documents the GameCI backstop correctly: the proof comes from the provisioned editor return step, so resource-safe only flips when that local return proves success/already-absent state.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread scripts/__tests__/ci-aggregate-workflow.test.js
@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Please review exact head: the lifecycle contract now explicitly rejects any missing acquire/work/cleanup/release step before checking order.

Copilot AI review requested due to automatic review settings July 12, 2026 20:40
@wallstop

Copy link
Copy Markdown
Collaborator Author

Cursor (@cursor) review

Please review exact head: the lifecycle contract now explicitly rejects any missing acquire/work/cleanup/release step before checking order.

@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Dispatch throughput (Unity 6000.3.16f1)

Runner: 13th Gen Intel(R) Core(TM) i9-13900KF, 24C/32T @ 3000MHz; 64GB DDR5@4200; NVIDIA GeForce RTX 3060; Microsoft Windows 11 Pro N (10.0.26200)

Regenerated by CI from commit 957e109 (run logs). These numbers MOVED versus the committed doc; CI commits the refreshed doc directly to the default branch after this PR merges.

Latest CI benchmark run: Unity 6000.3.16f1, commit 957e10983930744638013578ac492c0125c0c512.

Runner: 13th Gen Intel(R) Core(TM) i9-13900KF, 24C/32T @ 3000MHz; 64GB DDR5@4200; NVIDIA GeForce RTX 3060; Microsoft Windows 11 Pro N (10.0.26200)

Dispatch throughput - Standalone (IL2CPP)

Platform: Standalone IL2CPP x64 Release (WindowsPlayer; Unity 6000.3.16f1).

Scenario Throughput / Wall clock
Empty Bus Dispatch 49.46 M emits/sec
Untargeted Flood (One Handler) 37.62 M emits/sec
Untargeted Flood (Two Handlers, One Priority) 36.80 M emits/sec
Untargeted Flood (Three Handlers, One Priority) 33.61 M emits/sec
Untargeted Flood (Four Handlers, One Priority) 30.62 M emits/sec
Untargeted Flood (Four Handlers, Four Priorities) 31.15 M emits/sec
Untargeted Flood (Sixteen Handlers, One Priority) 18.20 M emits/sec
Untargeted Flood (One Inactive Handler) 39.01 M emits/sec
Untargeted First Dispatch (Cold, Distinct Types) 0.197 ms
Targeted Flood (No Matching Target) 15.77 M emits/sec
Targeted Flood (One Listener) 12.02 M emits/sec
Targeted Flood (Sixteen Listeners) 8.62 M emits/sec
Targeted First Dispatch (Cold, Distinct Types) 0.196 ms
Broadcast Flood (One Handler) 21.50 M emits/sec
Broadcast First Dispatch (Cold, Distinct Types) 0.188 ms
Interceptor Heavy (Four Interceptors) 4.08 M emits/sec
Post-Processing Heavy (Four Post-Processors) 14.54 M emits/sec
Message Bus Construction (1000) 29.923 ms
Registration Token Construction (1000, Prebuilt Handler + Bus) 0.060 ms
Registration Flood (1000 Types, Cold Bus) 598.342 ms
Registration Flood (1000 Types, Warm JIT) 4.413 ms
Untargeted Registration (Marginal, 1000 Same-Type) 0.414 ms
Targeted Registration (Marginal, 1000 Same-Type) 0.461 ms
Broadcast Registration (Marginal, 1000 Same-Type) 0.460 ms
Deregistration Flood (1000 Types, Cold) 1.974 ms
Deregistration Flood (1000 Types, Warm JIT) 1.231 ms

Dispatch throughput - PlayMode (Mono)

Platform: Editor PlayMode Mono x64 Release (WindowsEditor; Unity 6000.3.16f1).

Scenario Throughput / Wall clock GC allocs GC bytes
Empty Bus Dispatch 29.18 M emits/sec 0 0
Untargeted Flood (One Handler) 23.96 M emits/sec 0 0
Untargeted Flood (Two Handlers, One Priority) 22.02 M emits/sec 0 0
Untargeted Flood (Three Handlers, One Priority) 21.13 M emits/sec 0 0
Untargeted Flood (Four Handlers, One Priority) 19.73 M emits/sec 0 0
Untargeted Flood (Four Handlers, Four Priorities) 20.50 M emits/sec 0 0
Untargeted Flood (Sixteen Handlers, One Priority) 9.85 M emits/sec 0 0
Untargeted Flood (One Inactive Handler) 24.97 M emits/sec 0 0
Untargeted First Dispatch (Cold, Distinct Types) 3.402 ms 41 11,318
Targeted Flood (No Matching Target) 25.22 M emits/sec 0 0
Targeted Flood (One Listener) 18.05 M emits/sec 0 0
Targeted Flood (Sixteen Listeners) 8.93 M emits/sec 0 0
Targeted First Dispatch (Cold, Distinct Types) 5.290 ms 42 19,876
Broadcast Flood (One Handler) 16.40 M emits/sec 0 0
Broadcast First Dispatch (Cold, Distinct Types) 3.704 ms 41 19,534
Interceptor Heavy (Four Interceptors) 3.13 M emits/sec 0 0
Post-Processing Heavy (Four Post-Processors) 11.89 M emits/sec 0 0
Message Bus Construction (1000) 31.573 ms 40,011 2,480,736
Registration Token Construction (1000, Prebuilt Handler + Bus) 0.111 ms 1,002 120,096
Registration Flood (1000 Types, Cold Bus) 3137.449 ms 52,145 3,813,528
Registration Flood (1000 Types, Warm JIT) 11.552 ms 30,057 2,489,792
Untargeted Registration (Marginal, 1000 Same-Type) 0.544 ms 2,030 640,484
Targeted Registration (Marginal, 1000 Same-Type) 0.594 ms 2,030 672,484
Broadcast Registration (Marginal, 1000 Same-Type) 0.610 ms 2,030 672,484
Deregistration Flood (1000 Types, Cold) 107.803 ms 42 116,856
Deregistration Flood (1000 Types, Warm JIT) 1.893 ms 42 116,856

Library comparison - throughput (Standalone (IL2CPP))

Technology Global -> 1 subscriber Global -> 16 subscribers Keyed/targeted -> 1 of many Priority-ordered dispatch Filtered/intercepted dispatch Post-processing dispatch Subscribe/unsubscribe churn Struct message (no boxing)
DxMessaging 35.80 M emits/sec 18.15 M emits/sec 11.19 M emits/sec 30.75 M emits/sec 9.02 M emits/sec 15.64 M emits/sec 0.91 M emits/sec 36.18 M emits/sec
MessagePipe 86.34 M emits/sec 17.34 M emits/sec 10.68 M emits/sec N/A 74.28 M emits/sec N/A 2.06 M emits/sec 94.37 M emits/sec
UniRx MessageBroker 5.11 M emits/sec 2.75 M emits/sec N/A N/A N/A N/A 0.75 M emits/sec 5.13 M emits/sec
Zenject SignalBus 2.53 M emits/sec 1.35 M emits/sec N/A N/A N/A N/A 1.64 M emits/sec 2.45 M emits/sec
Unity Atoms 124.52 M emits/sec 47.95 M emits/sec 134.37 M emits/sec N/A N/A N/A 13.28 M emits/sec N/A
ScriptableObject channel 136.02 M emits/sec 27.60 M emits/sec 192.81 M emits/sec N/A N/A N/A 35.29 M emits/sec 184.47 M emits/sec
UnityEvent 93.71 M emits/sec 11.31 M emits/sec 102.88 M emits/sec N/A N/A N/A 3.56 M emits/sec 104.22 M emits/sec
C# event 294.11 M emits/sec 53.62 M emits/sec 75.66 M emits/sec N/A N/A N/A 12.99 M emits/sec 288.13 M emits/sec
Unity SendMessage 8.28 M emits/sec 1.05 M emits/sec 8.19 M emits/sec N/A N/A N/A N/A N/A

Library comparison - GC allocations per 10k ops (PlayMode (Mono))

Technology Global -> 1 subscriber Global -> 16 subscribers Keyed/targeted -> 1 of many Priority-ordered dispatch Filtered/intercepted dispatch Post-processing dispatch Subscribe/unsubscribe churn Struct message (no boxing)
DxMessaging 0 0 0 0 0 0 80,000 0
MessagePipe 0 0 0 N/A 0 N/A 20,000 0
UniRx MessageBroker 0 0 N/A N/A N/A N/A 150,000 0
Zenject SignalBus 20,000 20,000 N/A N/A N/A N/A 70,000 20,000
Unity Atoms 0 0 0 N/A N/A N/A 0 N/A
ScriptableObject channel 0 0 0 N/A N/A N/A 0 0
UnityEvent 0 0 0 N/A N/A N/A 50,000 0
C# event 0 0 0 N/A N/A N/A 0 0
Unity SendMessage 10,000 10,000 10,000 N/A N/A N/A N/A N/A

Library comparison - GC allocated bytes per 10k ops (PlayMode (Mono))

Technology Global -> 1 subscriber Global -> 16 subscribers Keyed/targeted -> 1 of many Priority-ordered dispatch Filtered/intercepted dispatch Post-processing dispatch Subscribe/unsubscribe churn Struct message (no boxing)
DxMessaging 0 0 0 0 0 0 7,560,000 0
MessagePipe 0 0 0 N/A 0 N/A 560,000 0
UniRx MessageBroker 0 0 N/A N/A N/A N/A 6,650,000 0
Zenject SignalBus 600,000 600,000 N/A N/A N/A N/A 3,200,000 600,000
Unity Atoms 0 0 0 N/A N/A N/A 0 N/A
ScriptableObject channel 0 0 0 N/A N/A N/A 0 0
UnityEvent 0 0 0 N/A N/A N/A 2,960,000 0
C# event 0 0 0 N/A N/A N/A 0 0
Unity SendMessage 200,000 200,000 200,000 N/A N/A N/A N/A N/A

@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

DxMessaging perf deltas vs master baseline (Unity 6000.3.16f1, Standalone IL2CPP)

Regenerated by CI (run logs). Changed beyond tolerance: yes. Regression gate: passed.

Reading the Delta column: the sign is normalized so + ALWAYS means better and - ALWAYS means worse, for every metric -- a + percentage is higher throughput (emits/sec) OR lower latency (wall clock ms). This Standalone IL2CPP player strips the profiler, so it cannot measure GC allocations or bytes; those columns are omitted from this comment entirely (not shown as n/a). Measured allocation counts and their regression coverage come from the in-editor Mono leg and the allocation test suite. When a leg DOES measure them, allocation/byte changes render as N fewer (better) / N more (worse).

Scenario Baseline Current Delta
Empty Bus Dispatch 48.82 M emits/sec 49.46 M emits/sec +1.32%
Untargeted Flood (One Handler) 40.02 M emits/sec 37.62 M emits/sec -5.99%
Untargeted Flood (Two Handlers, One Priority) 38.09 M emits/sec 36.80 M emits/sec -3.39%
Untargeted Flood (Three Handlers, One Priority) 36.29 M emits/sec 33.61 M emits/sec -7.39%
Untargeted Flood (Four Handlers, One Priority) 34.21 M emits/sec 30.62 M emits/sec -10.50%
Untargeted Flood (Four Handlers, Four Priorities) 34.89 M emits/sec 31.15 M emits/sec -10.72%
Untargeted Flood (Sixteen Handlers, One Priority) 19.56 M emits/sec 18.20 M emits/sec -6.94%
Untargeted Flood (One Inactive Handler) 40.05 M emits/sec 39.01 M emits/sec -2.58%
Untargeted First Dispatch (Cold, Distinct Types) 0.198 ms 0.197 ms +0.51%
Targeted Flood (No Matching Target) 15.81 M emits/sec 15.77 M emits/sec -0.27%
Targeted Flood (One Listener) 12.03 M emits/sec 12.02 M emits/sec -0.08%
Targeted Flood (Sixteen Listeners) 9.08 M emits/sec 8.62 M emits/sec -4.98%
Targeted First Dispatch (Cold, Distinct Types) 0.200 ms 0.196 ms +2.00%
Broadcast Flood (One Handler) 22.72 M emits/sec 21.50 M emits/sec -5.34%
Broadcast First Dispatch (Cold, Distinct Types) 0.194 ms 0.188 ms +3.09%
Interceptor Heavy (Four Interceptors) 4.09 M emits/sec 4.08 M emits/sec -0.27%
Post-Processing Heavy (Four Post-Processors) 14.81 M emits/sec 14.54 M emits/sec -1.87%
Message Bus Construction (1000) 17.334 ms 29.923 ms -72.63%
Registration Token Construction (1000, Prebuilt Handler + Bus) 0.062 ms 0.060 ms +3.23%
Registration Flood (1000 Types, Cold Bus) 558.092 ms 598.342 ms -7.21%
Registration Flood (1000 Types, Warm JIT) 4.308 ms 4.413 ms -2.44%
Untargeted Registration (Marginal, 1000 Same-Type) 0.418 ms 0.414 ms +0.96%
Targeted Registration (Marginal, 1000 Same-Type) 0.454 ms 0.461 ms -1.54%
Broadcast Registration (Marginal, 1000 Same-Type) 0.462 ms 0.460 ms +0.43%
Deregistration Flood (1000 Types, Cold) 2.091 ms 1.974 ms +5.60%
Deregistration Flood (1000 Types, Warm JIT) 1.501 ms 1.231 ms +17.99%
Global -> 1 subscriber 35.70 M emits/sec 35.80 M emits/sec +0.28%
Global -> 16 subscribers 18.81 M emits/sec 18.15 M emits/sec -3.51%
Keyed/targeted -> 1 of many 11.87 M emits/sec 11.19 M emits/sec -5.80%
Priority-ordered dispatch 31.16 M emits/sec 30.75 M emits/sec -1.31%
Filtered/intercepted dispatch 9.32 M emits/sec 9.02 M emits/sec -3.24%
Post-processing dispatch 16.16 M emits/sec 15.64 M emits/sec -3.21%
Subscribe/unsubscribe churn 0.94 M emits/sec 0.91 M emits/sec -2.97%
Struct message (no boxing) 35.27 M emits/sec 36.18 M emits/sec +2.56%

Copilot AI commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Please review exact head: the lifecycle contract now explicitly rejects any missing acquire/work/cleanup/release step before checking order.

Confirmed in 12dc1b1: positions.every((position) => position >= 0) now gates the order assertion, so any missing lifecycle step name (acquire/work/cleanup/release) fails the test explicitly rather than silently passing with a -1 in the sorted positions array. All 246 tests pass.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 12dc1b1. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread scripts/__tests__/ci-aggregate-workflow.test.js Outdated
@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Please review exact head 957e109: cross-workflow missing-job diagnostics now include file and job id.

Copilot AI review requested due to automatic review settings July 12, 2026 20:50
@wallstop

Copy link
Copy Markdown
Collaborator Author

Cursor (@cursor) review

Please review exact head 957e109: cross-workflow missing-job diagnostics now include file and job id.

Copilot AI commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Please review exact head 957e109: cross-workflow missing-job diagnostics now include file and job id.

Confirmed in 957e109: getJobBlock now takes an optional sourceName (defaults to "ci.yml" for backward compat) and the assertion message is ${sourceName}:${jobId} job must exist, so any renamed workflow/job produces a scoped diagnostic like unity-tests.yml:run-tests job must exist rather than the generic ci.yml must define.... All 246 tests pass.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 957e109. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@wallstop Eli Pinkerton (wallstop) enabled auto-merge (squash) July 12, 2026 20:59
@wallstop Eli Pinkerton (wallstop) merged commit c5fe595 into master Jul 13, 2026
46 of 54 checks passed
@wallstop Eli Pinkerton (wallstop) deleted the dev/wallstop/resource-lifecycle branch July 13, 2026 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants