revert(ci): stop triggering validation on PR edits

[Astro-Han]

Summary

Revert #4186 (83aa12a29c57e82bfe807e0913a35a90881cf6d0) in full. Its edited trigger created a newer skipped CI run after a title/body edit, hiding an earlier successful test from branch protection and leaving PR #5225 blocked with Required status check "test" is expected.

Restore the default pull_request activities: opened, synchronize, and reopened. Title, body, and base-branch edits no longer trigger CI. Remove the metadata concurrency routing, dynamic job name, conditional job skip, and their obsolete policy assertions. The required test context and validation implementation stay unchanged.

This intentionally restores the limitation described in #4181: retargeting a PR to main without changing its head does not start CI. Push a new commit or reopen the PR to validate the new merge base. This rollback keeps metadata edits out of CI instead of adding a separate retarget scheduler.

Refs #4181, #4186, #5225.

Verification

• 80 tests passed: node --test --test-concurrency=1 scripts/ci-workflow-policy.test.mjs scripts/ci-test-plan.test.mjs.
• actionlint .github/workflows/ci.yml, npm run format, npm run lint, and git diff --check passed.
• The original failure was independently reproduced in an isolated GitHub fixture with enforced required checks: a successful test produced CLEAN; a body-only edit produced a skipped run and BLOCKED despite the old success remaining in the API (fixture PR, skipped run). Fixture branches and protection were removed.
• Application build/typecheck and the full Maka suite were not run locally for this workflow-only revert.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex investigated the failure, ran isolated GitHub experiments, and authored the rollback and verification. Commits include Generated-by: Codex.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[jackwener]

Approve at 785f196fa07f16370eb86b8f2083960f80610a07. No findings at any severity. Below is what I checked and one piece of first-hand evidence that supports the revert.

The revert is complete. Across all three files there is no remaining reference to the edit trigger, the ignored-edit name, or the base-ref change expression — the only surviving matches for "edited" are unrelated prose. The trigger block is now a plain pull-request-on-main with no types: list, which is exactly the default set of opened, synchronised and reopened rather than a hand-written approximation of it. The concurrency group is back to its plain form and still cancels in progress only for pull requests.

The required context keeps its name, which is the risk this workflow's own comments care most about. Both before and after, a normal push reports a check named test: previously the conditional expression evaluated to that string, now the job name is simply static. So there is no window in which the protected context reports under a different name, and nothing needs to be re-pointed in branch protection.

The policy test is not weakened by the revert — it is strengthened. The test that asserted the reverted mechanism's exact expressions is removed, which is correct, since keeping it would fail against the code it described. Everything that guards the freeze risks is retained: no paths filter on the trigger, exactly one job and that job named test, and no needs: between jobs. Added to that list is a new assertion that the job carries no if: at all. That is the right guard to add here: the problem being reverted was a conditional that could skip the required context, so pinning its absence prevents the same shape from returning by a different route.

First-hand evidence that the reverted mechanism had a visible artifact. While reviewing unrelated pull requests earlier today I saw their check lists carrying an entry whose name was the raw workflow expression — the whole conditional, verbatim, reported as skipped — alongside the real test check. That is the conditional name surfacing un-evaluated on skipped runs. It never satisfied or cancelled the required context, so it was cosmetic rather than dangerous, but it did put an unreadable string in front of every reviewer of those pull requests. This revert removes it. I mention it because it is an observation from live pull requests rather than a reading of this diff.

The documented trade is consistent with the code. The branch-protection comment previously claimed the check runs for a pull request retargeted to main; that sentence is gone, matching the fact that a base-only retarget no longer triggers a run and needs a new commit or a reopen. I have no objection to the trade itself — it is a stated product decision — and I note only that the comment and the behaviour now agree, which is what would otherwise rot.

Verification limits. I read the diff and the resulting files and compared them against the behaviour described; I ran no workflow, no test and no lint. The eighty tests, actionlint, format and lint results reported for this change are the author's and are not my evidence. At the time of posting the required check is still running; this is a code-review conclusion and does not assert that checks pass. I did not perform the merge.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.

[jackwener]

Correction to my approval above. I called the reverted mechanism "cosmetic rather than dangerous". That was wrong, and it understated why this revert matters.

The author tested it directly and told me the claim was false. I verified the mechanism myself before writing this rather than taking the correction on trust, and my own queries confirm it.

On a single commit in a test repository under the same required-context protection, three check runs exist across three separate check suites:

suite	name	conclusion
94017268363	test	success
94017410468	github.event.action == 'edited' && … || 'test'	skipped
94017548669	test	success

The two runs the author cited share one head: 34709655373 is a successful pull-request run, and 34709709399 is a skipped pull-request run on the same commit. So a title or body edit does not merely add an oddly-named entry beside the real check — it creates its own check suite that contains no test context at all.

That is the part I got wrong. Branch protection requires the context test; when the newest suite for a head does not contain it, the requirement is unsatisfied regardless of an earlier suite having passed. The author observed the consequence end to end: the pull request went from clean to blocked after an edit on an already-passing head, and a real merge attempt returned 405. I did not reproduce the blocked state or the merge rejection myself — those are his measurements — but the suite layout I verified is precisely the mechanism that produces them, so I have no reason to treat them as anything other than established.

What this changes. My approval of the revert stands and needs no revision: the code is a complete, clean revert and the added guard against a conditional on the required job is the right safeguard. What changes is the severity of what is being reverted. This was not a display wart that happened to be tidied up along the way; it could leave a pull request unmergeable after an edit that touched no code, with the required check showing green on an earlier suite while the gate looked at a newer one that had none.

Why I got it wrong, stated plainly. I had seen the raw-expression name on live pull requests and reasoned that because a skipped run cannot satisfy or cancel a required context, it was harmless. That reasoning skipped a step: I checked what the skipped run could not do and never checked what its suite did to the requirement. A check suite without the required context is not neutral — it is the absence of a requirement that must be present. I should have queried the suites before characterising the impact, and the cost of not doing so was an understatement published in a review.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.