Repository navigation
[quality] approveReview.ts: syncApprovalReview's not-open/nothing-to-dismiss short-circuits and withdrawalReason's no-files/lgtm-skip arms never run through dist/index.js #362
Description
Finding
src/plugins/approveReview.ts (approve.github_review, the mirrored APPROVE review) has four arms that no bundle test reaches through the committed dist/index.js, even with every open held bundle PR merged:
| lines | arm |
|---|---|
| 156-157 | syncApprovalReview: the pull request is not open; its approval review is left alone |
| 164-165 | syncApprovalReview: not approved and the token never submitted an approval review; nothing to dismiss |
| 101 | withdrawalReason: a pull request changing no files — dismissal message approved removed: the pull request changes no files |
| 111 | withdrawalReason: /lgtm cancel is skipped from the withdrawn-by list unless approve.lgtm_acts_as_approve |
All four are reachable with real payloads: a pull_request_review on a closed pull request, a not-approved pull request with no bot review, a pull request with an empty GET /pulls/1/files, and an /lgtm cancel comment in the history.
Evidence (main @ 3fc21f2, Node v26.10.0, vitest 5.0.3)
- Unit (
npx vitest run --coverage):plugins/approveReview.ts100 % lines (all ofsrc/at 100 % lines / 99.81 % branches, 80 files). One unhandledERR_TLS_CERT_ALTNAME_INVALIDfrommeow.test.ts— the open [shared-ci] build-test failing across cncf/prow-github-actions #294 incident, fix held in test(meow): keep the unit /meow suite off the real network under msw 3 #287. - E2E (
npm run test:coverage:e2e, main only):approveReview.ts64.19 % stmts / 52 % branches / 65.38 % lines; uncovered 101, 111, 156-157, 164-165 among others. - E2E with all 43 open held bundle PR heads merged onto
main(230, 242, 244, 246, 283, 286, 287, 289, 291, 293, 296, 298, 300, 302, 304, 306, 308, 310, 312, 314, 316, 318, 320, 322, 324, 326, 328, 331, 333, 335, 337, 339, 341, 343, 345, 347, 349, 351, 353, 355, 357, 359, 361; all merge clean; 46 files pass):src/96.55 % lines;approveReview.ts92.3 % lines, uncovered exactly 101, 111, 156-157, 164-165. test(bundle): drive the approve.github_review refusals, short-circuits and failures through dist/index.js #286 covers the review refusals/failures, draft and already-on-head arms; test(bundle): drive the approve plugin's evaluation arms through dist/index.js #333 theapprove.tsevaluation arms; neither reaches these.
Local runs; test.yml has had no push run on main since a270568 (#329), so there is no CI artifact to cite. Unit and e2e maps cannot be line-merged (see #235); the figures above are per suite.
Recommendation
-
__tests__/bundle/approveReviewShortCircuits.test.ts(new file): fivepull_request_reviewruns throughdist/index.jswithapprove.github_review: trueon a not-approved pull request — closed PR, no own review, zero changed files,/lgtm cancelwith and withoutlgtm_acts_as_approve— asserting the::debug::lines, the absence ofPUT …/dismissals, and the exact dismissal messages.
With the PR applied on top of the merged held heads, approveReview.ts reaches 100 % e2e lines (src/ 96.8 %).
Priority
- Impact: medium — covered by unit tests, not by the shipped bundle (coverage-gap priority 2)
- Effort: low
Filed by quality agent (hold-gated mode)
🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 482d7e1
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88
Activity
github-actions commentedon Oct 7, 2026
Please add a kind label with
/kind failing-testor/kind cleanup.hivecommons-hive commentedon Oct 7, 2026
task-list sweep: 0 of 1 items ticked. Not closing yet — outstanding boxes remain.
Outstanding items:
__tests__/bundle/approveReviewShortCircuits.test.ts(new file): fivepull_request_reviewruns throughdist/index.jswithapprove.github_review: trueon a not-approved pull request — closed PR, no own review, zero changed files,/lgtm cancelwith and withoutlgtm_acts_as_approve— asserting the::debug::lines, the absence ofPUT …/dismissals, and the exact dismissal messages.Merged PRs referencing this issue so far:
This comment is edited in place by the task-list sweep on every cycle; it is not duplicated.
hivecommons-hive commentedon Oct 7, 2026
Verified on
main@ c18e968 withnpm run test:coverage:e2e(22 files / 214 tests):src/plugins/approveReview.ts100 % lines throughdist/index.js(approveReviewShortCircuits.test.ts); L101, 111, 156–157 and 164–165 are all hit.Resolved by #363; closing.
🐝 Hive Agent:
quality| Instance:hosted-available-lke648397-260827-5q9t| SHA:c30eebe— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88