Repository navigation
[quality] approveReview.ts: a PAT that is not the author never has its review submission asserted, and withdrawalReason's five-file bound, lgtm skip, user casing and withdrawer order are unpinned #526
Description
Finding
src/plugins/approveReview.ts is 100 % line-covered by npm run test:coverage and by npm run test:coverage:e2e on main @ 88241f4 (the e2e run's only zero-hit arms are the non-Error folds at 226/230, the known non-Error-fold class). A single-operator mutation probe over the file (39 mutants: ===/!==, &&/||, comparison bounds, .toLowerCase()/.sort() removal, continue/break, undefined checks) left five non-equivalent survivors against npx vitest related src/plugins/approveReview.ts --run (46 files / 920 tests):
| line | mutant | behaviour nothing asserts |
|---|---|---|
| 188 | identity.login !== undefined && identity.login === owners.author → … || … |
A user token (PAT) whose login is not the author must submit the APPROVE review. The mutant warns "opened by the token's own identity" and never calls POST /pulls/1/reviews for every PAT. The only user-token "approved" case (user: 'Prow-Bot', author: 'prow-bot') is the self-approval one; every submit case uses an installation token (GET /user → 403). |
| 105 | uncoveredFiles.length > reasonFiles → >= |
Exactly five uncovered files must read no approver covers a, b, c, d, e, not … and 0 more. Only 7 files (→ and 2 more) is tested. |
| 111 | continue → break |
With lgtm_acts_as_approve: false, an lgtm* event skips only itself; the cancels after it still count. The existing fixture puts dave's cancel after carol's lgtm-cancel, but dave then re-approves, so dropping dave entirely yields the same string. |
| 113 | latest.set(event.user.toLowerCase(), event) → event.user |
bob and Bob are one person: a later approve by Bob retracts bob's cancel, and a cancel by BOB is reported as bob. The existing fixture's Bob approve / bob cancel lands on bob either way. |
| 118 | .sort() on the withdrawers |
The dismissal message lists withdrawers alphabetically, not in action order; the existing fixture inserts bob before carol, which is already sorted. |
Equivalent, not actionable: line 226 'status' in e || typeof e.status === 'number' (e.status cannot be a number when status is not in e).
Recommendation
Add to __tests__/plugins/approveReview.test.ts:
approve.github_review: approved› a user-token case withuser: 'Prow-Bot', author: 'some-author'assertingcalls=['label', 'notifier', 'review']and no warning.withdrawalReason› exactly-five-files;bob cancelthenBob approve→ no withdrawal, andBOB cancel→withdrawn by bob;carol lgtm-cancelbeforebob cancel→withdrawn by bob;dave, bob, carolacting in that order → listedbob, carol, dave.
Test-only; no src/ or dist/ change. Disjoint from the open held PRs #510–#525 (none of them touches approveReview.test.ts).
Evidence
- unit:
npm run test:coverage→ 100 / 99.75 / 100 / 100 (main @ 88241f4, local run, 163 files / 2206 tests);approveReview.ts100 % lines, no zero-hit branch - e2e:
npm run test:coverage:e2e→ 99.96 / 98.79 / 100 / 99.95 (same revision, local run, 83 files / 502 tests);approveReview.ts100 % lines, BRDA 226/230 only (the unit and e2e reports come from different transforms and are read separately, per the e2e coverage config) - probe: each mutant applied in turn to
src/plugins/approveReview.ts,npx vitest related src/plugins/approveReview.ts --run→ 920 passed for all five; with the recommended cases each mutant fails exactly one test
Priority
- Impact: medium — line 188 decides whether
approve.github_reviewworks at all with a PAT, the documented way around "Allow GitHub Actions to create and approve pull requests" - Effort: low
Filed by quality agent (hold-gated mode)
🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 28def9c
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88
Activity
github-actions commentedon Oct 19, 2026
Please add a kind label with
/kind feature,/kind failing-testor/kind cleanup.