mirror of
https://github.com/zernie/vigiles.git
synced 2026-09-14 20:53:57 +08:00
Claude
615cf701ac
Codex on #226, and it landed one minute before I merged, so it is fixed here
rather than there.
THE FINDING IS REAL. The action's README requires `pull-requests: read` for the
`pull_request` path — with a token it calls `pulls.listFiles` (REST) instead of
diffing, which is exactly why that path needs no checkout. ci.yml declares no
top-level `permissions:`, so `changes` inherits the repository default, which
supplies it today. That is a default we do not control: a restrictive top-level
block added later, the standard hardening step, takes it away and every PR
blocks on a 403.
THE SUGGESTED FIX WOULD HAVE BROKEN IT. A job-level `permissions:` block
REPLACES the default rather than adding to it — everything unlisted becomes
`none` — so declaring only `pull-requests: read` strips `contents: read` and
kills the `actions/checkout@v4` the `push` path needs to diff with git. It is a
pair or it is a break, and each half fails a different way. Both are declared.
Pinned by src/ci-path-filter.test.ts, mutation-proven in both directions with
the patch shown to have landed each time:
drop contents:read (the suggestion as written) -> fails its own assertion
drop the block entirely (what main does today) -> fails its own assertion
restored -> 25 passed
ALSO RECORDED: THE OTHER FINDING, WHICH IS FALSE. It claimed an unset `base`
makes a push inspect only the latest commit, so an earlier commit's changes are
missed. Traced through the action's main.ts: with no `base` it defaults to the
repository's `default_branch` (main) while `head` comes from `github.ref` (also
main), so `isBaseSameAsHead` holds and `baseSha` becomes the push event's own
`before` — `git diff <before>..<head>`, the whole pushed range, exactly what the
hand-written classifier did. The "detected from last commit" branch needs
`before` to be absent, which a push payload never is.
Confirmed in the log of the first push run after the switch (34420710819):
Changes will be detected between 236615e97... and main
git diff --no-renames --name-status -z 236615e97...refs/remotes/origin/main
where 236615e97 is that push's `before`. Written into the workflow comment so
the next reader does not re-derive it — the reasoning, not just the verdict.
Gates: npm run check 18/18 · npm run coverage 3976 passed, 0 failed. The 99.94%
statement gate fails locally on src/mock-model.ts:147,339,374, untouched here
and green on CI (25 tests skip locally and never reach those lines).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>