ci: pr-triage sees every closing keyword form; codeql checkout keeps no credentials - #90
Conversation
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the gating lane (gating review).
Verdict: request changes. The regex change is correct: traced Fixes askalf/repo#12, a bare Closes #12, and a full issue URL through the new pattern and all three match, so the branch-derived Closes #N is no longer appended on top of an existing cross-repo or URL reference; the trailing \b only rejects forms like #12abc that GitHub would not honour either. The codeql persist-credentials: false addition matches every other checkout in the repo and CodeQL init/analyze do not need the git credential. The blocking finding is the new code comment, which is patch narration with an em dash rather than a description of what the code requires.
1. Blocking: .github/workflows/pr-triage.yml:106
// Every closing form GitHub honours, not just the bare one. A body // reading "Fixes askalf/repo#12" on a branch named issue-13-fix used // to look like it closed nothing, so `Closes #13` was appended and // MERGING CLOSED BOTH — one an issue the author never named. // Covers bare #N, cross-repo owner/repo#N, and the full issue URL.
This comment narrates the bug the patch fixes ("used to look like it closed nothing, so Closes #13 was appended and MERGING CLOSED BOTH — one an issue the author never named") instead of stating what the code needs to be true. That history belongs in the commit message and PR body, where it already is; in the file it reads as generated patch narration, including the em dash and the shouted clause, and goes stale the moment it merges.
Suggested fix:
Keep only the constraint the reader needs, e.g. `// Any reference form GitHub honours after a closing keyword: #N, owner/repo#N, or the full issue URL. Missing one here means a second Closes line gets appended below.`
rule:reads-as-generated
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the gating lane (gating review).
Verdict: approve. Approving. The new closing-keyword regex in pr-triage.yml is static (no untrusted interpolation), and I traced the three reference forms GitHub honours after a closing keyword through it: Closes #13, Fixes askalf/repo#12 and Resolves https://github.com/askalf/cordon/issues/12 all match, so the branch-derived Closes #N is no longer appended on top of an existing cross-repo or URL reference; the added trailing \b only rejects things like #12abc, which GitHub would not treat as a reference either. The rest of the block (branch parsing, issue lookup, body update) is unchanged. The codeql.yml checkout now sets persist-credentials: false, which is consistent with the other checkouts and safe since codeql-action init/analyze do not need git credentials; the workflow's permissions and pinned action SHAs are untouched. Comments and commit messages read as hand-written with no attribution or secrets. No tests exist for workflow scripts in this repo, so there is nothing for this diff to add.
From the 2026-09-25 audit of every action.
Closes #N. A body readingFixes askalf/repo#12on a branch namedissue-13-fixlooked like it closed nothing, soCloses #13was appended and merging closed both, one an issue the author never named. The regex now covers bare#N,owner/repo#Nand the full issue URL, the same one plumbline already runs (pr-triage.yml:107-113there).persist-credentials: falselike every other checkout in the repo.No behaviour change for a PR whose body already closes an issue in any form, or whose branch names none.