Skip to content

chore: remove dead CI config and fix misleading contracts#9313

Closed
mikhail-dcl wants to merge 1 commit into
fix/ci-workflow-bugsfrom
chore/ci-workflow-cleanup
Closed

chore: remove dead CI config and fix misleading contracts#9313
mikhail-dcl wants to merge 1 commit into
fix/ci-workflow-bugsfrom
chore/ci-workflow-cleanup

Conversation

@mikhail-dcl

Copy link
Copy Markdown
Collaborator

Pull Request Description

What does this PR change?

Second PR of the CI pipeline audit — stacked on #9312 (base is fix/ci-workflow-bugs; it retargets to dev automatically when #9312 merges). Pure dead-config removal and contract fixes; no pipeline behavior changes.

  • Dead triggers removed. workflow_dispatch in claude-pr-review, claude-pr-review-require, enforce-group-approvals, auto-assign, pr-comment-artifact-url and pr-comment-warnings: no job can run under it — every job's condition or payload access requires a PR/comment/workflow_run event (in claude-pr-review-require and auto-assign the github-script would throw on the missing payload). Same for the pull_request_review_comment trigger in claude-pr-review, which no job reads.
  • pr-closure.yml: dropped TARGET: t_${{ matrix.target }} — the job has no matrix (it evaluated to t_), and build.py --delete derives all platform targets from BRANCH_NAME (verified in scripts/cloudbuild/build.py::delete_current_target).
  • on-delete-release.yml: deleting the last remaining release wrote {"version": "null"} to latest.json (jq stringifies the missing tag_name of the 404 body). Now the update is skipped.
  • build-release-main-page.yml: workflow_run.workflows matches by workflow name only — the .github/workflows/build-release-main.yml path entry never matched anything. Also removed target_commitish: github.sha, which points at the dev head in workflow_run context (harmless only because the manually pushed tag takes precedence), and trimmed unused job outputs.
  • build-unitycloud.yml: removed defaults on required: true workflow_call inputs (a default on a required input can never be used — actionlint events finding); documented that the hardcoded check names in the skip step must stay in sync with test.yml (and vice versa in test.yml).
  • test.yml: deleted ~35 lines of long-dead commented-out lint steps (git history keeps them).
  • close-inactive-issues.yml: removed contents: write — actions/stale doesn't need it; the comment claiming it did was wrong.
  • version action: commit_sha is now declared required: false — composite actions never enforced it, three callers omit it, and the script already falls back to GITHUB_SHA. The declaration now matches reality.
  • build-profile-nightly.yml: the full_version job output actually carried short_version; renamed to version (its only consumer updated).

Deliberately not included: the audit's B5 finding (Dependency Security Review's MEDIUM-RISK pending status has no human resolution path) — any fix weakens a security gate, so the mechanism needs an explicit maintainer decision first.

Test Instructions

CI-only change; nothing to run in the client.

Steps (standard run): N/A

Prerequisites

Test Steps

  1. Confirm all still-triggered workflows parse: actionlint reports no new findings on touched files.
  2. After merge, the nightly profile build must still pass its version to Unity Cloud (get-info.outputs.version — renamed from full_version).
  3. Deleting a non-latest release must still refresh latest.json; deleting the only release must skip the S3 update instead of writing "null".

Additional Testing Notes

  • Removing dead workflow_dispatch triggers makes those workflows disappear from the manual "Run workflow" dropdown — that is intended; dispatching them never did anything.

Quality Checklist

  • Changes have been tested locally (actionlint on all touched files)
  • Documentation has been updated (if required) — N/A, inline comments added
  • Performance impact has been considered — none
  • For SDK features: Test scene is included — N/A

🤖 Generated with Claude Code

- drop workflow_dispatch triggers no job can handle (claude-pr-review,
  claude-pr-review-require, enforce-group-approvals, auto-assign,
  pr-comment-artifact-url, pr-comment-warnings) and the unhandled
  pull_request_review_comment trigger in claude-pr-review
- pr-closure.yml: remove TARGET env - there is no matrix in this job and
  build.py --delete derives targets from BRANCH_NAME
- on-delete-release.yml: don't write version "null" to latest.json when
  the last release is deleted
- build-release-main-page.yml: remove workflow_run entry that matches by
  file path (matching is by name only), drop target_commitish pointing
  at the dev head, trim unused job outputs
- build-unitycloud.yml: remove defaults on required workflow_call inputs
  (never used); document the check names shared with test.yml
- test.yml: delete long-dead commented-out lint steps; document check
  names referenced by build-unitycloud.yml skip statuses
- close-inactive-issues.yml: drop contents:write - actions/stale does
  not need it
- version action: commit_sha is optional (falls back to GITHUB_SHA);
  callers already rely on that
- build-profile-nightly.yml: rename misleading full_version output that
  actually carried short_version

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mikhail-dcl
mikhail-dcl requested review from a team as code owners July 7, 2026 08:55
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

badge

Build skipped — no changes detected under Explorer/.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

💰 Review cost: unknown · claude-sonnet-4-6

@claude

claude Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mikhail-dcl's task in 1m 34s —— View job


Security & correctness review — CI workflow cleanup

  • Fetch and read the full diff
  • Workflow security review (W.1–W.10) on the 14 touched workflows
  • Verify key correctness claims (build.py --delete targets, version SHA fallback, actions/stale perms)
  • Post verdict

This is a pure dead-config / contract-fix PR. Every change reduces attack surface or corrects a misleading declaration — nothing adds a new capability, dependency, action, or prompt-injection channel. No dependency files or binaries changed, so only the workflow security section (W.1–W.10) applies.

Security findings

None. Notable improvements:

  • close-inactive-issues.yml — drops contents: write. This is a genuine least-privilege win (W.10); actions/stale only needs issues/pull-requests: write, and the old comment's "deletion of stale state files" justification was fictional.
  • claude-pr-review.yml — removes the workflow_dispatch and pull_request_review_comment triggers. Fewer entry points into the secret-holding LLM workflow. The remaining issue_comment trigger and its actor gating are untouched and out of scope here.
  • build-release-main-page.yml — removes target_commitish: ${{ github.sha }}, which in a workflow_run context resolves to the default-branch (dev) head, not the release commit. Removing a latent footgun; the pushed tag correctly determines the release target.
  • build-unitycloud.yml — removes defaults on required: true workflow_call inputs (dead values; actionlint events).

Correctness claims — verified

  • pr-closure.yml dropping TARGET: t_${{ matrix.target }} ✅ — delete_current_target() (scripts/cloudbuild/build.py:499-527) derives target names purely from BRANCH_NAME (base_target_name at line 512), never reads TARGET, and sys.exit(0)s before any TARGET-consuming code. The removed env var was genuinely dead on the --delete path.
  • version/action.yml commit_sha: required: false ✅ — the parser step falls back to $GITHUB_SHA when the input is empty (.github/actions/version/action.yml:76-81), so the declaration now matches behavior.
  • on-delete-release.yml jq '.tag_name // empty' ✅ — correctly avoids writing the literal string "null" to latest.json when the /releases/latest API returns 404 with no tag_name.
  • build-release-main-page.yml workflow_run.workflows matches by name ✅ — the .github/workflows/build-release-main.yml path entry never matched and was dead.

Minor / non-blocking

  • pr-closure.yml still has no trailing newline at EOF (\ No newline at end of file) — pre-existing, not introduced here, but trivial to fix while touching the file.
  • The two new "keep in sync on rename" comments (build-unitycloud.yml skip step ↔ test.yml Test (<mode>) / Build (<target>) job names) accurately document a real coupling — good defensive documentation for a fragile hardcoded-check-name contract.

Verdict

All changes are dead-config removal, permission tightening, and contract corrections. No unresolved security concerns.

DEPENDENCY_REVIEW: PASS

@mikhail-dcl

Copy link
Copy Markdown
Collaborator Author

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

@mikhail-dcl mikhail-dcl closed this Jul 7, 2026
@mikhail-dcl
mikhail-dcl deleted the chore/ci-workflow-cleanup branch July 7, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant