Skip to content

ci: release.yml hygiene — job timeouts + release concurrency (queue) - #52

Merged
IvanMurzak merged 1 commit into
mainfrom
pipeline/e2-release-hygiene
Aug 13, 2026
Merged

ci: release.yml hygiene — job timeouts + release concurrency (queue)#52
IvanMurzak merged 1 commit into
mainfrom
pipeline/e2-release-hygiene

Conversation

@IvanMurzak

Copy link
Copy Markdown
Owner

Adds two runaway guards to release.yml. Hygiene attributes only — no rename, no restructuring, no version files, gate topology untouched.

1. Job timeouts

Every job previously inherited the 6-hour GitHub default. Derived from the 5 most recent successful full release runs (30354826261, 30201966471, 29762726136, 29757793994, 29704719893) — a hung job could previously burn 6h of Actions time before anyone noticed.

Rule applied: ~2× observed p95, floor of 10 minutes, generous for the Unity-editor job. With n=5, p95 ≈ observed max.

Job Observed durations (5 runs) p95 ≈ max 2× p95 Chosen Headroom vs worst
check-version-tag 9s, 20s, 12s, 19s, 8s 20s 40s 10 min (floor) 30×
prepare-release-notes 9s, 8s, 10s, 12s, 9s 12s 24s 10 min (floor) 50×
build-signed-upm-package 13s, 10s, 9s, 8s, 10s 13s 26s 10 min (floor) 46×
build-unity-installer 4m29s, 4m42s, 4m48s, 4m57s, 7m24s 7m24s ~15 min 30 min (Unity-editor, generous) 4.1×
release-unity-plugin 15s, 18s, 13s, 11s, 5s 18s 36s 10 min (floor) 33×
cleanup-artifacts 8s, 4s, 5s, 6s, 6s 8s 16s 10 min (floor) 75×

build-unity-installer runs a Unity EditMode test and a package export inside the Unity image, so its variance is dominated by image pull and licence activation rather than by our code — hence 30 min rather than the arithmetic ~15.

The 9 test-unity-* jobs deliberately have NO timeout — it is not expressible here

These call a reusable workflow (uses: ./.github/workflows/test_unity_plugin.yml). timeout-minutes is not a permitted key on a reusable-workflow-calling job, and adding it is a hard validation error (The workflow is not valid ... Unexpected value 'timeout-minutes') that fails the entire workflow file — every job, including the PR run. Verified three independent ways:

  1. GitHub docs, closed allowlistReusing workflow configurations: "When you call a reusable workflow, you can only use the following keywords in the job containing the call"name, uses, with, with.<input_id>, secrets, secrets.<secret_id>, secrets.inherit, strategy, needs, if, concurrency, permissions. timeout-minutes is absent.
  2. GitHub's own parser schemaworkflow-v1.0.json: the workflow-job mapping declares no timeout-minutes, while the ordinary job-factory does.
  3. actionlint v1.7.12, run locally against a probe file:
    when a reusable workflow is called with "uses", "timeout-minutes" is not available.
    only following keys are allowed: "name", "uses", "with", "secrets", "needs", "if", and "permissions"
    

These are the longest jobs in the pipeline and remain unbounded — observed up to 17m45s (2023.2.22f1 editmode on windows-mono), with editmode legs routinely 11–17 min. Bounding them requires timeout-minutes on the jobs inside test_unity_plugin.yml, which is out of scope for this PR. Suggested follow-up, from the same 5 runs: editmode legs max 17m45s → ~45 min; playmode max 9m54s → ~30 min; standalone max 7m12s → ~30 min; version-consistency max 10s → 10 min.

2. Release concurrency

concurrency:
  group: release-${{ github.ref }}
  queue: max

cancel-in-progress is deliberately unset — an in-flight release must finish, since it may already have created the tag and GitHub Release.

queue: max is the load-bearing part. The documented default (queue: single) keeps at most one pending run and silently cancels and replaces it when another is queued — so back-to-back version bumps could see an earlier bump's release cancelled while never having run.

Queue syntax — doc citation

Verified against official sources before authoring:

Also documented and respected here: "The combination of queue: max and cancel-in-progress: true is not allowed and will result in a workflow validation error." Leaving cancel-in-progress unset is required, not stylistic. Queue order is FIFO; overflow past 100 pending is cancelled.

⚠️ actionlint flags queue as an unknown key — this is expected linter staleness, not a defect. The newest actionlint release is v1.7.12 (2026-03-30), which predates the queue feature (2026-05-07), so no actionlint version can validate it. The remaining actionlint output on this file is that single line and nothing else. GitHub's own parser is the authority here.

Validation performed

  • actionlint v1.7.12 on the modified file → only the known-stale queue line; clean baseline on the unmodified file beforehand.
  • Independent yaml.safe_load parse → 15 jobs; the 6 steps jobs carry timeouts, the 9 uses jobs carry none.
  • Diff is +19 / −0.

Constraints honoured

  • ✅ File not renamed (NuGet Trusted Publishing matches by workflow file name).
  • ✅ No permissions: block added anywhere — the workflow had none and relies on default token permissions, so release-unity-plugin keeps its contents: write capability untouched. Per the task constraint, when in doubt permissions were not touched.
  • ✅ Gate topology untouched: check-version-tag → tests → atomic all-tests-gated publish. No needs: array and no if: condition was modified.
  • ✅ No version files touched — nothing releases from this PR.

The full 27-job PR matrix firing on this PR is expected and one-off.

Adds two runaway guards to release.yml without touching the gate topology.

1. timeout-minutes on all 6 directly-defined jobs (was: the 6h default).
   Values derived from 5 recent successful runs, ~2x observed p95 with a
   10-minute floor; build-unity-installer gets 30m as a Unity-editor job.

2. Workflow-level concurrency group `release-${{ github.ref }}` with
   `queue: max` and cancel-in-progress deliberately unset, so an in-flight
   release always finishes and a queued release is no longer silently
   cancelled by the next push.

The 9 test-unity-* jobs call a reusable workflow, where `timeout-minutes`
is not a permitted key — adding it is a hard "Invalid workflow file" error.
Bounding those requires a timeout inside test_unity_plugin.yml, which is
out of scope here.

No rename, no permissions block, no version files, gate topology unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017bgHTA223Fnjci6QbyC1bv
@IvanMurzak

Copy link
Copy Markdown
Owner Author

Code review — depth: medium (gating: advisory)

Verdict: APPROVE — no blocking findings. This is a clean, well-documented, pure-additive hygiene diff. I independently reproduced every load-bearing claim in the PR body rather than taking it on faith (details below).

DoD conformance

Item Status How verified
Only .github/workflows/release.yml touched gh pr view --json files → 1 file, MODIFIED, +19/−0
File not renamed changeType: MODIFIED, identical path both sides
No permissions: changes grep -n permissions on the head file → zero GHA-key matches (only an unrelated shell-comment string)
Gate topology untouched (no needs:/if: edits) Pulled base (main) and head content independently via gh api contents, ran a plain diffevery changed line is a pure addition; every needs:/if: line in the head file is byte-identical to base
No version files touched Single-file diff, confirmed above

Correctness

  1. Concurrency block level — correctly placed at workflow level (sibling of on:/env:/jobs:, zero indentation), not nested under a job. Confirmed by reading the raw head file.
  2. queue: max + no cancel-in-progress — verified against the live current GitHub docs (workflow-syntax page, fetched directly), not just the PR's citation:
    • single (default): "any existing pending job or workflow run in the same group will be cancelled and replaced" — matches the bug this PR fixes.
    • max: "Up to 100 jobs or workflow runs can be pending... When the queue is full, any additional jobs or workflow runs are canceled."
    • "The combination of queue: max and cancel-in-progress: true is not allowed and will result in a workflow validation error."
      All three claims check out verbatim against the current doc text I pulled myself. The PR's config (max + cancel-in-progress unset) is the valid, documented combination.
  3. Timeout values vs derivation table — spot-checked via gh api .../actions/runs/<id>/jobs against the actual 5 cited runs (30354826261, 30201966471, 29762726136, 29757793994, 29704719893), not just trusted the table:
    • build-unity-installer: all 5 durations reproduced exactly — 4m29s, 4m42s, 4m48s, 4m57s, 7m24s (max) — chosen 30 min bound gives ~4× headroom.
    • check-version-tag (9s), prepare-release-notes (9s), build-signed-upm-package (13s), release-unity-plugin (11s), cleanup-artifacts (6s) — each spot-checked against at least one real run and matched the table.
    • Also cross-checked the "17m45s (2023.2.22f1 editmode on windows-mono)" figure cited for the deferred test-unity-* follow-up — reproduced exactly (12:37:37→12:55:22 on run 30354826261) from real job data, not asserted.
    • Independent yaml.safe_load (PyYAML) parse of the head file: 15 jobs total, concurrency present with the exact expected values, timeout-minutes present on precisely the 6 non-uses jobs and absent on the 9 uses-calling test-unity-* jobs — matches the PR's own "15 jobs, 6 with, 9 without" claim.
  4. Reusable-workflow-job restriction — confirmed against the live reusing-workflow-configurations doc: the allowed keyword list for a uses:-calling job does not include timeout-minutes, so omitting it from the 9 test-unity-* jobs is correct, not an oversight.

Risk

  • queue: max cannot make the release gate topology (check-version-tag → tests → atomic publish) misbehave — concurrency governs which workflow runs are pending/cancelled, never job-level needs/if semantics within a run. Confirmed the diff touches neither.
  • Does the new concurrency block make an unbounded hang in a test-unity-* job worse than today? No. An in-progress run was never cancellable by a newly queued run either before or after this change (that only happens with cancel-in-progress: true, which is deliberately absent). The only behavior this diff changes is what happens to a second queued release while the first is running: before, the pending slot silently replaced (release lost); after, it queues (up to 100) and waits its turn. Net effect is strictly safer, not riskier, for the hang scenario.
  • 30 min bound on build-unity-installer vs 7m24s observed max — 4× headroom, reasonable; this job also does a Unity image pull + licence activation, which the PR body's comment on the job itself correctly attributes as the source of variance.
  • 10-min floors on sub-20s jobs — 30–75× headroom per the reproduced data; safe.
  • Residual gap (already flagged by the author, not new): the 9 test-unity-* jobs remain unbounded at GitHub's platform 6h default, since timeout-minutes is a hard validation error on a uses:-calling job. Correctly out of scope for this PR.

Adjacent

  • Workflow parses — confirmed independently via yaml.safe_load, not just by trusting the PR body's actionlint claim.
  • Comment accuracy — both new comment blocks (the concurrency: rationale and the build-unity-installer timeout rationale) check out against real API data and current GitHub docs, verified above.

Findings

  • P1 (blocking): none.
  • P2 (advisory, actionable): none — I looked for one (queue-overflow at 100 pending runs, workflow_dispatch on a non-main ref forming a separate concurrency group) and couldn't construct a scenario that survives being argued against for this repo's actual push volume/trigger shape, so I'm not padding the list with it.
  • P3 (advisory, FYI): the PR body already hands over a fully-derived follow-up timeout table for the 9 test-unity-* jobs (editmode ~45 min, playmode/standalone ~30 min, version-consistency 10 min, sourced from the same 5 runs) — worth turning into an actual tracked follow-up task so that derivation doesn't only live in this PR's description.

What I verified vs. took on faith

Verified directly (not trusted from the PR body): live GitHub docs text for queue/cancel-in-progress semantics and the reusable-workflow allowed-keyword list; base vs. head diff of the raw file content (confirms pure-additive, no needs/if/permissions edits); independent YAML parse of the head file; actual job timing data for all 6 timeout-bearing jobs across the 5 cited runs, plus the 17m45s editmode figure for the deferred jobs. Took on faith: the PR's own actionlint run output (didn't have actionlint available locally to rerun) and that the "5 most recent successful full release runs" were the right sample (didn't independently re-derive which 5 runs "most recent successful" should be, just verified the 5 named ones exist and match their claimed durations).

@IvanMurzak
IvanMurzak merged commit 1dd9198 into main Aug 13, 2026
63 checks passed
@IvanMurzak
IvanMurzak deleted the pipeline/e2-release-hygiene branch August 13, 2026 19:52
IvanMurzak added a commit to IvanMurzak/Godot-AI-Particles that referenced this pull request Aug 13, 2026
* ci: bump_version direct mode (push+dispatch) + release.yml hygiene

bump_version.yml gains a `mode` input (choice: pr | direct, default pr).

- mode=pr keeps today's behaviour exactly: branch + PR, all six steps
  untouched. The only edit to that job is the `if:` guard that selects it.
- mode=direct commits the same bump (same commands/bump-version.ps1, same
  argument) straight to the default branch with the job's GITHUB_TOKEN, then
  dispatches release.yml explicitly. A GITHUB_TOKEN push creates no workflow
  runs, so the dispatch is the only thing that starts the release, which is
  why the job carries `actions: write` alongside `contents: write`.

The job conditions are `!= 'direct'` / `== 'direct'` so they are exhaustive:
a dispatch that omits `mode` falls back to the legacy pr path rather than
matching neither job and silently doing nothing.

release.yml receives D12 hygiene only, mirroring the merged Unity pilot
(IvanMurzak/Unity-AI-Animation#52):

- workflow-level concurrency `release-${{ github.ref }}` with `queue: max`
  and cancel-in-progress deliberately unset.
- timeout-minutes 10 on check-version-tag (observed 4-6s) and 15 on publish
  (observed 26s on the real NUGET_USER path).
- the `test` job calls a reusable workflow, where timeout-minutes is a hard
  validation error, so it is documented rather than bounded.

File not renamed; publish OIDC/NuGet gating and gate topology untouched.
No version files touched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017bgHTA223Fnjci6QbyC1bv

* ci: pass github.ref_name through env in the direct job's push/dispatch

Review follow-up on #5. The Push and "Dispatch release.yml" steps inlined
`${{ github.ref_name }}` directly into their `run:` bodies, which contradicts
the data-not-script-text principle the rest of the direct job follows (and
which the PR body claims). Both now take TARGET_REF via `env:` and reference
the shell variable instead.

No behavioural change: TARGET_REF resolves to the same ref the inline
expansion produced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017bgHTA223Fnjci6QbyC1bv

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant