fix: repair CI workflow bugs found in pipeline audit

[mikhail-dcl]

Pull Request Description

What does this PR change?

Fixes ten defects found in a full audit of the CI pipelines. No behavior is added — each change makes a workflow do what its code already claims to do.

1. test.yml watchdog never ran (verified on real runs, e.g. run 28808093323: Lint failure, watchdog skipped). Its if had no status-check function, so GitHub wrapped it in an implicit success() — false exactly when a needed job fails. Now always() && (…).
2. Any label add cancelled in-flight test runs. on: pull_request: types: [labeled] + unconditional cancel-in-progress: true meant bot labels added mid-run (new-dependency at the start of Dependency Security Review, claude-approved, no QA needed) killed a running 80-minute test job and replaced it with a run that skips everything. Cancellation and job conditions are now gated the same way build-unitycloud.yml already does it.
3. test.yml push/merge_group runs only worked by accident. The explicit clause checked refs/heads/main, which the dev-only push trigger can never produce; dev pushes (which seed the warning baseline) passed only because null == false coerces to true in Actions expressions. Event handling is now explicit.
4. build-unitycloud.yml dead merge_group trigger removed. Prebuild's if never matched the event, so merge-queue runs would skip every job and report green without building. If a merge queue is ever adopted, proper support must be added instead.
5. auto-sync-main-to-dev.yml failed on every push to main while a sync PR was already open (gh pr create errors on duplicates); also removed a dangling \ line continuation.
6. enforce-group-approvals.yml counted stale approvals. A reviewer who approved and later requested changes still satisfied the gate. Only each user's latest state-changing review (APPROVED / CHANGES_REQUESTED / DISMISSED) counts now; COMMENTED reviews don't supersede an approval.
7. pr-comment-artifact-url.yml / pr-comment-delete-artifact-url.yml matched the first github-actions[bot] comment of any kind and overwrote it — the warning-ratchet and ext-contribution comments share that author. Now scoped with body-includes: img.shields.io/badge/Build (present in all badge bodies, so existing PR comments keep working).
8. test-performance.yml Library cache never updated — constant key Library-Explorer-Ubuntu on an immutable cache pinned the first-ever entry forever. Now uses the same hashFiles(...) key + restore-keys as test.yml (and shares its cache).
9. githubToken: ${{ env.GITHUB_TOKEN }} in both test workflows referenced an env var that is never defined — removed the dead config (check reporting already goes through dorny/test-reporter).
10. build-game-ci-image.yml push trigger removed — on push all inputs.* are empty, producing image tag …:-- and empty build args.

Flagged, not changed (needs a maintainer decision): build-unitycloud.yml declares 12 workflow_dispatch inputs; GitHub documents a limit of 10. No dispatch runs exist in the last 100 runs to confirm whether manual dispatch still works. If it is broken, two inputs need to be merged or dropped (e.g. the two delta thresholds).

All changed files pass actionlint (only pre-existing findings remain repo-wide).

Test Instructions

CI-only change — there is no client behavior to run; verification is on the workflows themselves.

Steps (standard run): N/A (no Explorer/ changes; the build/test pipelines will skip or run unchanged on this PR).

Prerequisites

• None

Test Steps

1. On this PR, confirm Unity Test triggers normally and the watchdog job appears (it should be skipped when lint+tests pass).
2. Add any unrelated label (e.g. no QA needed) while a Unity Test run is in flight → the run must NOT be cancelled.
3. Add force-lint → lint re-runs; tests are not restarted.
4. After merge, push to dev → Unity Test still runs (baseline update path) — this exercises the explicit non-PR event condition.
5. On a PR with an approval that was later superseded by "request changes" from the same reviewer, QA and DEV Approvals must report the approval as missing.

Additional Testing Notes

• The find-comment scoping keeps matching pre-existing badge comments (they all contain img.shields.io/badge/Build), so no duplicate badge comments should appear on open PRs.
• Merge-queue is not currently enabled; removing the dead merge_group trigger changes nothing observable.

Quality Checklist

• Changes have been tested locally (actionlint on all touched workflows; watchdog bug verified against live run history)
• Documentation has been updated (if required) — N/A, comments added inline
• Performance impact has been considered (fewer cancelled/redundant CI runs; perf tests stop re-importing from a stale Library)
• For SDK features: Test scene is included — N/A

🤖 Generated with Claude Code

[github-actions]

Build skipped — no changes detected under Explorer/.

[claude]

Claude finished @mikhail-dcl's task in 4m 2s —— View job

---

PR Review: fix/ci-workflow-bugs

Tasks

• Load context (CLAUDE.md, docs/README.md)
• Get PR diff and changed files
• Review each workflow change for correctness
• Check for design/logic issues
• Assess complexity and QA requirements
• Post final verdict

---

Step 1 — Context

CI-only PR: 8 YAML workflow files, 84 additions, 25 deletions. No C# or runtime code changes. No ECS, Unity, or architecture docs relevant.

---

Step 2 — Root-cause check

All 10 fixes target root causes, not symptoms:

#	Claim	Verdict
1	watchdog never ran — if had implicit success()	✅ Fix is always() && (...)
2	Bot labels cancelled in-flight test runs	✅ Fix is conditional cancel-in-progress expression
3	push/merge_group condition dead-coded to refs/heads/main	✅ Fix replaces with github.event_name != 'pull_request'
4	Dead merge_group trigger in build-unitycloud.yml	✅ Removed, no behavior change (merge queue not enabled)
5	auto-sync failed every push after first PR opened	✅ Check-before-create pattern added
6	Stale approvals (approve then request-changes) still counted	✅ group_by + max_by(.submittedAt) jq fix
7	First bot comment of any kind was overwritten	✅ body-includes: img.shields.io/badge/Build scopes the match
8	Library cache key constant — pinned first-ever entry forever	✅ Now uses hashFiles(...) matching test.yml
9	githubToken: ${{ env.GITHUB_TOKEN }} referenced undefined env var	✅ Removed the dead config
10	build-game-ci-image.yml push trigger used empty inputs.*	✅ Push trigger removed

---

Step 3 — Design & integration

No new long-lived units, systems, plugins, or state introduced. All changes are corrections to existing workflow logic. Lifecycle ownership is not applicable to YAML workflows.

The cancel-in-progress logic in test.yml mirrors the pattern already used in build-unitycloud.yml (lines 174–196), which is a sound reference. The changes and test job conditions are now symmetric and consistent with each other (the only difference being force-lint not triggering tests, which is intentional and commented).

---

Step 4 — Member audit

No C# code — not applicable.

---

Step 5 — Line-level review

Non-blocking: Race condition in auto-sync-main-to-dev.yml

Two pushes to main arriving concurrently (both merges during a busy release) can both pass the if [ -z "$PR_NUMBER" ] guard before either creates the PR. The second gh pr create exits non-zero and the step fails — turning one run red. This is substantially better than the original (which always failed after the first run), but the gap remains.

Suggested fix — hoist the second gh pr list query out of the if block and suppress the create error:

          PR_NUMBER=$(gh pr list --head chore/sync --base dev --state open --json number --jq '.[0].number // empty')
          if [ -z "$PR_NUMBER" ]; then
            gh pr create \
              --base dev \
              --head chore/sync \
              --title "chore: sync main to dev" \
              --body ":crown: *An automated PR*" || true
          fi
          # Always re-query — covers both the create path and the race path
          PR_NUMBER=$(gh pr list --head chore/sync --base dev --state open --json number --jq '.[0].number')

          gh pr edit "$PR_NUMBER" --add-label auto-pr

Fix this →

Pre-existing (flagged, not introduced by this PR): 12 workflow_dispatch inputs in build-unitycloud.yml

GitHub documents a limit of 10 inputs for workflow_dispatch. build-unitycloud.yml currently has 12: profile, compression, clean_build, cache_strategy, sentry_enabled, script_debugging, delta_threshold_macos_mb, delta_threshold_windows_mb, enforce_delta, enforce_cap, force_sign, platforms. The PR correctly flags this for a maintainer decision and does not change it — noted here for completeness.

---

Step 6 — Complexity

SIMPLE — 8 CI YAML files only; no ECS, C#, or runtime code changes.

---

Step 7 — QA assessment

QA_REQUIRED: NO — All changes are in .github/workflows/. No user-facing behavior, no Unity runtime code.

---

Step 8 — Non-blocking warnings

None. Main.unity not touched.

---

Verdict

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: CI-only YAML workflow changes with no Unity runtime or C# code.
QA_REQUIRED: NO

[claude]

Claude finished @mikhail-dcl's task in 2m 21s —— View job

---

Security-focused review — CI workflow changes

• Read full diff of all 8 workflow files
• Dependency/supply-chain assessment (new-dependency label)
• Workflow security review (W.1–W.10)
• Correctness review of the 10 fixes
• Post verdict

---

Dependency / supply-chain review

No dependency changes exist in this PR. No edits to manifest.json, packages-lock.json, no new binaries under Plugins/ or Assets/**, and no .asmdef changes. The new-dependency label is a bot false-positive — fittingly, it's one of the very mid-run labels this PR is fixing the pipeline to stop reacting to. Nothing ships to the desktop client from this diff.

Workflow security review (STEP W)

No HIGH or MEDIUM security findings are introduced by this PR. Checked and cleared:

• W.4 / W.6 / W.8 (LLM & pull_request_target): none of the changed files invoke an LLM action or use pull_request_target. test.yml uses plain pull_request. ✅
• W.5 (tool scoping): the only new shell primitives are scoped gh calls (gh pr list/create/edit against the fixed chore/sync head, gh pr view --json reviews). Reviewer logins iterated in enforce-group-approvals are GitHub-username-constrained, so no injection surface. ✅
• Secrets: secrets.GITHUB_TOKEN in auto-sync is unchanged; test.yml/test-performance.yml only remove the dead githubToken: ${{ env.GITHUB_TOKEN }} — I confirmed GITHUB_TOKEN is never defined as an env var in either file, so it was an empty string and removal is a true no-op. ✅

Pre-existing (out of scope, not introduced here): the actions touched by this diff (peter-evans/create-or-update-comment@v3, actions/cache@v5) are tag-pinned, not SHA-pinned (W.7b). None of them consume secrets.*, so this is a low-priority hardening item and not something this PR should be blocked on.

Correctness of the 10 fixes — verified

The bug analysis is solid and I confirmed the mechanics of the trickier ones:

#	Fix	Verdict
1	watchdog always() && (…)	✅ Correct — without a status function GitHub wraps if in implicit success(), so it never fired on the failure it exists to catch.
2	Gated cancel-in-progress expression	✅ Non-trigger label adds now evaluate to false, leaving in-flight tests alive.
3	Explicit non-PR event gating in changes/test	✅ Draft-skip behavior preserved; push-to-dev now runs explicitly instead of via null == false coercion.
6	group_by(.author.login) | map(max_by(.submittedAt))	✅ submittedAt is a valid gh pr view --json reviews field; ISO-8601 sorts lexicographically, so latest-state-per-user is correct, and a superseding CHANGES_REQUESTED/DISMISSED now correctly drops the stale approval.
8	Perf cache key	✅ Byte-for-byte matches test.yml:525-526 (hashFiles('Explorer/Packages/packages-lock.json', 'Explorer/ProjectSettings/ProjectVersion.txt') + restore-keys); both hashed paths exist, so the two workflows genuinely share the cache.
7	body-includes: 'img.shields.io/badge/Build'	✅ Present in the badge comment on this very PR, so existing comments keep matching.

One minor robustness note (non-blocking) — auto-sync-main-to-dev.yml

The script has no set -e, and if gh pr create fails for a reason other than a duplicate (e.g. "no commits between dev and chore/sync"), PR_NUMBER stays empty and the final gh pr edit "$PR_NUMBER" --add-label auto-pr runs with an empty arg and errors — re-introducing a red run, which is the exact failure mode this fix targets. Consider guarding the edit:

if [ -n "$PR_NUMBER" ]; then
  gh pr edit "$PR_NUMBER" --add-label auto-pr
fi

(Cosmetic: the file still ends without a trailing newline.)

---

Verdict

Security posture is clean — no supply-chain surface, no new secret exposure, no injection vectors. The workflow logic fixes are correct as analyzed. The only item worth a maintainer glance is the empty-PR_NUMBER edge in auto-sync, which is a robustness nit rather than a security or blocking issue.

DEPENDENCY_REVIEW: PASS

---

💰 Review cost: $0.54 · claude-sonnet-4-6 · 16 turns

[github-actions]

🔒 Claude reviewed this PR — sensitive paths modified (.github/prompts, .github/workflows, or CODEOWNERS). Claude will not auto-approve these PRs — human DEV review is required. No QA needed (Claude reported QA_REQUIRED: NO).

[mikhail-dcl]

Superseded by #9314, which now targets dev and contains all three commits.