Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

[quality] 14 commenter-auth catch arms (10 commands + auth.ts:181/210/217/224) are unreachable — only module mocks in unit tests keep them covered

Closed
  • Is something wrong?
#386 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

A pull request for this has already been merged.

  • #449 by @mrbobbytables — merged

Assessment

Difficulty
3/5
Estimated time
Half a day
Newbie friendliness
35/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
github-actions, typescript
Domain
authorization, testing

Research direction

Start with the three probes in src/utils/auth.ts (checkOrgMember, checkCollaborator, checkIssueComments) and the wrapper catches at auth.ts:179-182 and 205-225, then the command files listed in the issue (close.ts, reopen.ts, lock.ts, assign.ts and others). Remove the unreachable wrappers and the mockRejectedValueOnce cases in tests/issueCommentTest/errorPaths.test.ts, tests/label/errorPaths.test.ts and tests/utils/auth.test.ts. Done when npx vitest run passes and the dist/ drift gate passes after npm run build and npm run pack. The issue says the behaviour-change alternative needs a maintainer decision, so confirm the approach first.

Written by the indexing model from the issue text.

Description

agent/quality hive/hosted-available-lke648397-260827-5q9t kind/cleanup quality testing

Finding

Fourteen catch arms that wrap the commenter-authorization probes are unreachable from the shipped bundle. The three probes they guard — checkOrgMember (src/utils/auth.ts:37-67), checkCollaborator (:77-104) and checkIssueComments (:116-142) — each wrap their whole body in try { … } catch { core.warning(…); return false }, so they never reject. The wrappers around them can only fire if the probe module itself is replaced, which is exactly how the unit suite reaches them:

wrapper file:line unit test that reaches it how
could not check commentor auth src/issueComment/close.ts:40, reopen.ts:37 __tests__/issueCommentTest/errorPaths.test.ts vi.mock('../../src/utils/auth', { spy: true }) + checkCollaborator.mockRejectedValueOnce
could not check commenter auth lock.ts:47, milestone.ts:40, src/labels/remove.ts:37 same / __tests__/label/errorPaths.test.ts same
could not check Commentor auth retitle.ts:45 same same
couldn ot check commentor Auth unassign.ts:58 same same
could not get authorized users assign.ts:61, cc.ts:58, uncc.ts:57 same getOrgCollabCommentUsers.mockRejectedValueOnce
could not get authorized user src/utils/auth.ts:181 (getOrgCollabCommentUsers) __tests__/utils/auth.test.ts makes core.warning throw (logger down)
error in checking org member / could not check collaborator / could not check issue comments src/utils/auth.ts:210, :217, :224 (checkCommenterAuth) __tests__/utils/auth.test.ts:263-278 same core.warning throw

errorPaths.test.ts:19 says so itself: "could not check auth" branches are only reachable when the helper rejects.

Evidence (main @ aa6a0f8, Node v26.10.0, vitest 5.0.3):

  • Unit: npx vitest run --coverage → 100 % lines (132 files / 2057 tests). All fourteen arms are green only because of the module mocks above.
  • End-to-end: npm run test:coverage:e2e on a scratch branch = origin/main + the heads of all 33 open hold-gated bundle PRs (#283 … #385; 53 files / 420 tests pass) → 97.64 % lines of src/. All fourteen arms have 0 hits; together with auth.ts:44-45 (payload.repository === undefined debug) they are the whole e2e residual of the auth path.
  • Reachability: grep -n "throw\|return false" src/utils/auth.ts — every throw in the three probes is inside their own try; the only way out is return. context.repo is evaluated inside those same try blocks.

Why it matters for the test suite rather than tidiness: the unit report cannot distinguish "exercised by the bundle" from "exercised by a mock of the callee", so these arms keep a green line over error messages — including the misspelt couldn ot check commentor Auth — that no user can ever see, and the tests that cover them pin wording the action cannot emit.

Recommendation

One PR, one deliverable (the same mechanical edit at every site):

  • drop the try { … } catch (e) { throw new Error('could not check … auth: ' + e) } wrappers in close.ts, reopen.ts, lock.ts, milestone.ts, retitle.ts, unassign.ts, labels/remove.ts and the could not get authorized users wrappers in assign.ts, cc.ts, uncc.ts, calling the probe directly
  • drop the four wrappers inside src/utils/auth.ts (getOrgCollabCommentUsers:179-182, checkCommenterAuth:205-225)
  • delete the corresponding mockRejectedValueOnce cases in __tests__/issueCommentTest/errorPaths.test.ts, __tests__/label/errorPaths.test.ts and the logger down cases in __tests__/utils/auth.test.ts
  • npm run build && npm run pack so the dist/ drift gate passes

Alternative, if the wrappers are wanted as belt-and-braces: keep them but make the probes actually able to reject (rethrow non-404/302 statuses instead of return false), so the arms become reachable and the bundle suite can drive them. That is a behaviour change — today an API outage silently denies the command — and so also a maintainer call.

This touches production source, so it is not a PR this lane opens; it is filed for a maintainer. Companion to #366 (dead exports kept green by their own unit tests); kept separate because the fix sites do not overlap.

Priority

  • Impact: low
  • Effort: low

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 4948f6d

— hive: agent=quality backend=copilot model=claude-fable-5.1

Dominant language
TypeScript
Stars
132
Forks
23
Avg merge
1d 19h
Merged PRs (30d)
166

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cncf/prow-github-actions

All issues in cncf/prow-github-actions

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.