[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
Los mantenedores suelen responder en 2 días
Ya se ha fusionado un pull request relacionado.
- #449 de @mrbobbytables — fusionado
Evaluación
- Dificultad
- 3/5
- Tiempo estimado
- Medio día
- Aptitud para principiantes
- 35/100
- Tipo de issue
- Refactorización
- Claridad
- Bastante claro
- Estado de actividad
- Activo
- Stack tecnológico
- github-actions, typescript
- Área
- authorization, testing
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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:e2eon 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 ofsrc/. All fourteen arms have 0 hits; together withauth.ts:44-45(payload.repository === undefineddebug) they are the whole e2e residual of the auth path. - Reachability:
grep -n "throw\|return false" src/utils/auth.ts— everythrowin the three probes is inside their owntry; the only way out isreturn.context.repois evaluated inside those sametryblocks.
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 inclose.ts,reopen.ts,lock.ts,milestone.ts,retitle.ts,unassign.ts,labels/remove.tsand thecould not get authorized userswrappers inassign.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
mockRejectedValueOncecases in__tests__/issueCommentTest/errorPaths.test.ts,__tests__/label/errorPaths.test.tsand thelogger downcases in__tests__/utils/auth.test.ts -
npm run build && npm run packso 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
- Lenguaje dominante
- TypeScript
- Estrellas
- 132
- Forks
- 24
- Merge medio
- 1 d 17 h
- PR fusionados (30 d)
- 218
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Tiene una plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de cncf/prow-github-actions
-
[quality] owners.ts: 13 behavioural mutants survive the suite, incl. a shared group cache key that gives every team the first team's membersPosiblemente ocupada @hivecommons-hive la tomó hoy. Abiertoagent/quality needs-kind stage/040-covered-by-pr testing
cncf/prow-github-actions#574 · 1 comentario ·
Los mantenedores suelen responder en 2 días
-
[quality] Unit msw servers reject unhandled requests but no test fails when the code under test swallows the rejection (158 tests run through fallbacks)Posiblemente ocupada @hivecommons-hive la tomó hace 2 días. Abiertoagent/quality needs-kind stage/040-covered-by-pr testing
Dificultad 4/5 3-5 días Aptitud para principiantes 35/100
cncf/prow-github-actions#570 · 3 comentarios ·
Los mantenedores suelen responder en 2 días
-
[quality] bundle fake answers unrouted requests with a plain 404; the harness should assert on unexpected traffic outside application fallbacksPosiblemente ocupada @hivecommons-hive la tomó hace 2 días. Abiertoagent/quality needs-kind stage/040-covered-by-pr testing
Dificultad 3/5 Medio día Aptitud para principiantes 48/100
cncf/prow-github-actions#568 · 1 comentario ·
Los mantenedores suelen responder en 2 días
-
[quality] sweep.ts delayed-reviewers OWNERS guard (line 80 false arm) is covered by neither the unit nor the bundle suitePosiblemente ocupada @hivecommons-hive la tomó hace 3 días. Abiertoagent/quality needs-kind stage/040-covered-by-pr testing
Dificultad 2/5 1-3 horas Aptitud para principiantes 25/100
cncf/prow-github-actions#560 · 2 comentarios ·
Los mantenedores suelen responder en 2 días
-
Follow-up: strengthen test contracts, CI enforcement, release delivery, and quality triagePosiblemente ocupada @hivecommons-hive la tomó hace 3 días. Abiertokind/cleanup quality stage/040-covered-by-pr tests
Dificultad 5/5 Más de una semana Aptitud para principiantes 18/100
cncf/prow-github-actions#554 · 1 comentario ·
Los mantenedores suelen responder en 2 días
Todos los issues de cncf/prow-github-actions
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
awesome-dsh-plugin/awesome-dsh-plugin#9184 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
farion1231/cc-switch#8829 ·
Los mantenedores suelen responder en 1 día
-
[core-client] Remove unused hand-maintained transport helpers after the definition-table cutoverAbiertodoc-audit enhancement
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100