[quality] unit coverage gate in vitest.config.mjs (95/93/98/95) sits ~5 points under main's floor; docs/contributing.md:48 still documents 85/83/91/85

Finding

npm run test:coverage on main @ 1062ce7 (local run, coverage/coverage-summary.json read directly) reports:

Metric	%	Covered
lines	100	2424/2424
branches	99.7	1662/1667
functions	100	500/500
statements	100	2546/2546

vitest.config.mjs gates build-test at 95 / 93 / 98 / 95 (lines / branches / functions / statements). That is the floor #202 set when main stood at 96.16 / 94.55 / 99.14 / 96.35; the suite has since climbed to 100 % lines, so the gate now leaves roughly 120 lines, 110 branches, 10 functions and 125 statements of slack. A PR can drop a whole module's tests and build-test stays green — the same ~5-point gap #202 closed once already, and the gap #484 closed for the e2e run (vitest.e2e-coverage.config.mjs is set "just under what main reaches": 99 / 95 / 98 / 99, and main measures 99.95 / 97.96 / 100 / 99.96 at the same revision, so that floor is fine).

Separately, docs/contributing.md:48 still says CI enforces 85 / 83 / 91 / 85 — the pre-#202 values. #202 changed vitest.config.mjs only and left the doc line behind.

The 5 unit branches short of 100 % are approve.ts:415 (#497 drops it) and the four negative-count BRDA arms from ast-v8-to-istanbul#207 (close.ts:39, reopen.ts:36, approve.ts:503, auth.ts:220, already on #466), so a 99 % branch floor is reachable and stable.

Recommendation

Same rule as #202 / #484 — floor set just under what main reaches so a regression of about one point fails build-test:

vitest.config.mjs coverage.thresholds → lines: 99, branches: 99, functions: 99, statements: 99
docs/contributing.md:48 names the values actually in vitest.config.mjs

Files: vitest.config.mjs, docs/contributing.md. Disjoint from #497 (src/plugins/approve.ts, src/issueComment/meow.ts) and #498 (__tests__/bundle/tideOptionalFields.test.ts). Test configuration only; no src/ or dist/ change, so a hold-gated PR accompanies this issue.

Priority

Impact: medium (coverage-reporting: the gate exists but is too loose to catch a regression)
Effort: low

Filed by quality agent (hold-gated mode)

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 1062ce7

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

Please add a kind label with /kind feature, /kind failing-test or /kind cleanup.

Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 3 closed as completed in the last 14 days) already cite docs/contributing.md, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

[quality] prow.yaml and OWNERS examples outside docs/configuration.md are never parsed by a test, and the parsers accept typos silently

Finding

__tests__/configurationDocs.test.ts holds the prow.yaml example in docs/configuration.md to the parser and the code's defaults. No test reads the yaml examples in the other guides (grep -rn "examples.md\|automatic-merging.md\|labeling.md\|cron-jobs.md\|prow-for-your-org.md" __tests__ finds only prose references and one commands.md command-table check). On main @ 1062ce7 that leaves 19 prow-configuration blocks and 7 OWNERS blocks that are never fed to parseProwConfig / parseOwners:

file	prow.yaml blocks	OWNERS blocks
`docs/examples.md`	1 (`:22`)	2 (`:55`, `:71`)
`docs/automatic-merging.md`	5 (`:112`, `:147`, `:229`, `:315`, `:436`)	—
`docs/labeling.md`	5 (`:43`, `:74`, `:88`, `:117`, `:172`)	1 (`:187`)
`docs/cron-jobs.md`	1 (`:40`)	—
`docs/commands.md`	—	2 (`:188`, `:345`)
`docs/prow-for-your-org.md`	—	2 (`:168`, `:177`)
`docs/configuration.md` (beyond the main example)	6	—

This matters because the parsers are lenient where a reader would expect a hard error:

parseProwConfig only core.debugs an unknown top-level key (src/utils/config.ts:420-422).
Every section normalizer returns stripUndefined({ …known fields }), so an unknown field such as tide.merge_on_event is dropped without a word (src/utils/config.ts:561-567, same shape in normalizeApprove, normalizeBlunderbuss, …).
parseOwners ignores any key it does not read (src/utils/owners.ts:38-60).

So a renamed field, a renamed section or a typo in any of those 26 examples would ship in the docs and be "accepted" by the action while doing nothing. All 36 blocks across README.md and docs/ parse cleanly today; the gap is that nothing would notice when one stops doing so.

Recommendation

Add __tests__/docsYamlExamples.test.ts that extracts every fenced ```yaml block from README.md and docs/**/*.md (excluding docs/releases/), classifies each as workflow / OWNERS / prow.yaml, and asserts through dist-independent unit calls:

every block is a yaml mapping or a list of workflow steps
every OWNERS block parses with parseOwners, names at least one approver or reviewer, and uses only keys parseOwners reads (plus the Prow keys docs/commands.md says are accepted and ignored)
every prow.yaml block parses with parseProwConfig, has no top-level key the parser would debug-and-ignore, has no section field the normalizers would strip, and is accepted by resolveTide / resolveHoldLabel / resolveSweepLookback / resolveAuthorization / blunderbussSettings / approveSettings / lgtmSettings

Workflow blocks are already covered (workflows.test.ts, version.test.ts) and are only required to parse.

Verified locally that such a test fails on tide.merge_on_event, on aprovers: in an OWNERS example and on require_matching_labels: at the top level, and passes on main @ 1062ce7 (npm run test:coverage: 158 files / 2193 tests, totals unchanged at 100 / 99.7 / 100 / 100). Test-only; no src/, dist/ or workflow change. Disjoint from #497 (approve.ts/meow.ts), #498 (__tests__/bundle/tideOptionalFields.test.ts) and #500 (vitest.config.mjs, docs/contributing.md:48).

Priority

Impact: medium (documentation examples are what every adopter copies; the parsers accept typos silently)
Effort: low

Filed by quality agent (hold-gated mode)

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 1062ce7

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

Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 4 closed as completed in the last 14 days) already cite src/plugins/approve.ts, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

[quality] bundle e2e: the null issue.user arm of /close, /reopen and /approve's require_self_approval never runs through dist/index.js

Finding

GitHub's REST issue schema types user as nullable-simple-user (node_modules/@octokit/openapi-types/types.d.ts:16085): an issue whose author's account was deleted arrives with user: null. Three handlers read it defensively:

src/issueComment/close.ts:33 and src/issueComment/reopen.ts:30 — commenterId === context.payload.issue?.user?.login (the author shortcut before the collaborator check)
src/issueComment/approve.ts:94 — String(context.payload.issue?.user?.login ?? '') in the require_self_approval check

Every issue_comment scenario in __tests__/bundle/ builds its payload with comment() / prCommentEvent(), both of which set issue.user.login, so no bundle run ever sends an authorless issue.

Evidence (main@1062ce7, local):

Unit — npm run test:coverage: src/issueComment/approve.ts 100 / 100 / 100 / 100; close.ts and reopen.ts 100 lines, with the only "uncovered" branch (close.ts:39, reopen.ts:36) reported as a negative BRDA hit count (-3 / -1 in coverage/lcov.info), a v8-to-istanbul implicit-else artifact after the awaited || chain, not a reachable path.
E2E — npm run test:coverage:e2e: src/issueComment/approve.ts 96.15 % branches, line 94 uncovered; suite total 99.96 / 97.96 / 100 / 99.95 (491 tests).

So the arm is unit-covered but not driven through the packed bundle: priority 2 (medium) under the coverage evidence rules.

Recommendation

Add one bundle file, __tests__/bundle/nullIssueUser.test.ts, that sends payload.issue.user = null through dist/index.js:

/close by a collaborator (collaborator read decides, PATCH state closed) and by a non-collaborator (falls to authorization.close, nothing written)
/reopen by a collaborator
issue_comment /approve with require_self_approval: true: not refused as a self-approval, approves

Test-only; no src/, dist/ or workflow change. Disjoint from held PRs #497 (src/plugins/approve.ts, meow.ts), #498 (tideOptionalFields.test.ts), #500/#502 (thresholds), #501 (docsYamlExamples.test.ts), #504 (githubApiUrlDefault.test.ts).

Priority

Impact: medium (unit-covered, bundle-uncovered)
Effort: low

Filed by quality agent (hold-gated mode)

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

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

Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 4 closed as completed in the last 14 days) already cite src/plugins/approve.ts, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

[quality] bundle e2e: no scenario sends an issue with user: null, so src/issueComment/approve.ts:94 (require_self_approval on an authorless issue) never runs through dist/index.js

Finding

GitHub's REST issue schema types user as nullable-simple-user (@octokit/openapi-types types.d.ts:16085): an issue whose author's account was deleted arrives with user: null. src/issueComment/approve.ts:94 reads it defensively — String(context.payload.issue?.user?.login ?? '') — before the require_self_approval refusal, and /close and /reopen take the same issue?.user?.login author shortcut before their collaborator check.

Every issue_comment scenario in __tests__/bundle/ builds its payload with comment() (helpers.ts:12) or prCommentEvent() (ownersData.ts:12), both of which assign issue.user.login, so no bundle run has ever sent an authorless issue through dist/index.js.

Evidence (main@1062ce7, local runs):

E2E — npm run test:coverage:e2e (491 tests): src/issueComment/approve.ts 96.15 % branches, line 94 uncovered; suite total 99.96 / 97.96 / 100 / 99.95.
Unit — npm run test:coverage (2124 tests): src/issueComment/approve.ts 100 / 100 / 100 / 100 — the ?? '' arm is unit-covered.

Unit-covered, bundle-uncovered: priority 2 (medium) under the coverage evidence rules.

Recommendation

One new bundle file, __tests__/bundle/nullIssueUser.test.ts, sending payload.issue.user = null through dist/index.js:

/close by a collaborator (collaborator read decides, PATCH state closed) and by a non-collaborator (falls to authorization.close, nothing written)
/reopen by a collaborator (collaborator read decides, PATCH state open)
issue_comment /approve with require_self_approval: true by an OWNERS approver: not refused as a self-approval, approves

Test-only; no src/, dist/ or workflow change. Disjoint from held PRs #497 (src/plugins/approve.ts, meow.ts), #498 (tideOptionalFields.test.ts), #500/#502 (coverage thresholds), #501 (docsYamlExamples.test.ts), #504 (githubApiUrlDefault.test.ts).

Priority

Impact: medium
Effort: low

Filed by quality agent (hold-gated mode)

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

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

Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 4 closed as completed in the last 14 days) already cite src/plugins/approve.ts, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

[quality] coverage-e2e: approvalEvents' 'comment.body ?? ""' fold (plugins/approve.ts:101) never takes the absent arm through dist/index.js

Finding

npm run test:coverage:e2e on main @ 1062ce7 (local run, 78 files / 487 tests) reports src/plugins/approve.ts at 96.62 % branches. One of its zero-hit branches is approvalEvents (src/plugins/approve.ts:101):

for (const comment of comments) {
  const login = humanLogin(comment.user)
  const body = comment.body ?? ''        // ← coverage-e2e/lcov.info: BRDA:101,4,1,0
  if (login === undefined || body === '') {
    continue
  }

GitHub's REST issue-comment schema types body as optional, but every bundle scenario that lists GET /issues/1/comments for the approve plugin serves a body on each comment, so the absent arm — skip the comment, read the rest — only runs in the unit suite (npx vitest run --coverage, same revision: 100 % lines / 98.87 % branches for the file, the fold included).

Re-checked on the head of #508 (which serves bodyless comments to comments.ts, requireMatchingLabel.ts and trigger.ts): still BRDA:101,4,1,0. Not claimed by #497 (effectiveKind / upsertNotifier, lines 228/230/409/415), #498, #504 or #505.

Unit evidence: covered (__tests__/plugins/approve.test.ts, vitest v8, main @ 1062ce7).
E2E evidence: uncovered (npm run test:coverage:e2e, main @ 1062ce7, local run).

Recommendation

Add a bundle test that drives pull_request_review through dist/index.js with the comment listing holding a comment that has no body:

bodyless comment from an approver alone: not approved, approver suggested, no label write
bodyless comment followed by /approve from an approver: the bodyless one is skipped, approved is added and the notifier names only the approver
coverage-e2e/lcov.info no longer lists BRDA:101,…,0 under SF:src/plugins/approve.ts

Test-only; no src/, dist/ or workflow change.

Priority

Impact: low (unit-covered; e2e-only gap)
Effort: low

Filed by quality agent (hold-gated mode)

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 1062ce7

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

Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 12 closed as completed in the last 14 days) already cite vitest.e2e-coverage.config.mjs, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

[quality] once #497/#498/#504/#505/#508/#509 land, the e2e branch residual on main is 17 defensive arms at 14 lines that no dist/ input can reach — v8-ignore, drop, or accept?

Finding

npm run test:coverage:e2e on main @ 1062ce7 (local run, 78 files / 487 tests) reports 97.96 % branches (1633/1667). Of the 34 branch arms with zero hits, about half are already spoken for: #507 (auth.ts:24) and the held PRs #497 (approve.ts:228/230/409/415, meow.ts:88), #498 (tide.ts:258/426, sweep.ts:141), #504 (octokit.ts:8), #505 (issueComment/approve.ts:94), #508 (comments.ts:58, requireMatchingLabel.ts:248, trigger.ts:181) and #509 (plugins/approve.ts:101).

This issue inventories the other 17 arms — what the "Uncovered Line #s" column will still show once all of those land. Every one is a defensive fold that no bundle input can drive (coverage-e2e/lcov.info BRDA hit counts from the same run):

site	fold	zero-hit arm	why no `dist/` input reaches it
`src/run.ts:40`	`error instanceof Error ? error.message : String(error)`	`BRDA:40,2,1,0`	`handler` rejects with an `Error` or an Octokit `RequestError` (an `Error` subclass)
`src/cronJobs/labelSync.ts:107`	`e instanceof Error ? e.message : String(e)`	`BRDA:107,8,1,0`	same
`src/cronJobs/sweep.ts:132`	same	`BRDA:132,11,1,0`	same
`src/issueComment/handleIssueComment.ts:217`	`normalizeError`: `error instanceof Error ? error : new Error(String(error))`	`BRDA:217,25,1,0`	same
`src/issueComment/trigger.ts:323`	`statusOf`: `typeof error === 'object' && error !== null && …`	`BRDA:323,49,1,0`	`@octokit/request` wraps every failed `fetch` and every non-2xx response in `RequestError` (`fetch-wrapper.js:53/85/99`), always an object
`src/labels/lgtm.ts:68`	`e instanceof Error ? e.message : String(e)`	`BRDA:68,8,1,0`	Octokit rejection
`src/plugins/approve.ts:503`	`typeof base === 'string' ? branchHasOwners(…) : repoHasOwners(…)`	`BRDA:503,84,1,-26`	both callers (`approveOnPullRequest`, `approveOnReview`) run on `pull_request` / `pull_request_review` payloads, which always carry `pull_request.base.ref`
`src/plugins/requireMatchingLabel.ts:167`	`e instanceof Error ? e.message : e`	`BRDA:167,23,1,0`	Octokit rejection
`src/plugins/tide.ts:137`	`typeof e === 'object' && e !== null && …`	`BRDA:137,3,1,0`	`RequestError`
`src/plugins/tide.ts:138`	`e instanceof Error ? e.message : String(e)`	`BRDA:138,5,1,0`	same
`src/plugins/approveReview.ts:226`	`errorStatus`: `typeof e === 'object' && …`	`BRDA:226,22,1,0`	same
`src/plugins/approveReview.ts:230`	`errorMessage`: `e instanceof Error ? …`	`BRDA:230,24,1,0`	same
`src/utils/command.ts:23`	`getLineArgs`: `findCommandArgs(command, body).at(-1) ?? ''`	`BRDA:23,0,1,0`	only called from `milestone.ts:43` / `retitle.ts:31`, which `handleIssueComment.ts:128` dispatches only after `hasCommand` matched; `/milestone` and `/retitle` have no `commandAliases` entry, so `findCommandArgs` is never empty there
`src/utils/config.ts:657`	`e instanceof Error ? e.message : e`	`BRDA:657,83,1,0`	`parseDuration` throws `Error`
`src/utils/events.ts:50`	`e instanceof Error ? e : new Error(String(e))`	`BRDA:50,3,1,0`	handler rejections are `Error`s
`src/utils/mergeQueue.ts:190`	`e instanceof Error ? e.message : String(e)`	`BRDA:190,13,1,0`	Octokit / GraphQL rejections are `Error`s

Unit coverage (npx vitest run --coverage, same revision, 157 files / 2116 tests) is 100 % lines / 99.7 % branches; every arm above is reached only by unit cases that inject a primitive rejection with mockRejectedValueOnce('…') or call the helper directly — a spy, not an input the action can see. This is the shape #466 and #507 describe, at the remaining 14 lines.

The CI gate is not at risk: vitest.e2e-coverage.config.mjs holds branches at 95 (#502 proposes 97), and 1650/1667 ≈ 98.98 % once the held PRs land.

Recommendation

A maintainer's call, so this is parked for a decision rather than PR'd. The three options are mutually exclusive and all touch production code or the coverage configuration, which the quality lane does not PR:

Mark the fold, keep the behaviour — /* v8 ignore next */ on each of the 14 lines, as 7a1df7a did for the reset*() hooks. One PR, 14 one-line comments, dist/ repacked; the e2e report then names no src/ line, and the e2e branch floor can go to 99.
Drop the arms — follow refactor(tide): require fetchMergeability's initial read, drop the unreachable getPull fallback #492/refactor(approve, meow): drop the three dead arms left open on #466 #497: treat the caught value as Error (or the base as string) at each site and delete the unit cases that inject primitives. Smaller runtime, but 14 sites lose a guard against a non-Error throw from a future dependency.
Accept the residual — leave the code as is and treat the ≈99 % e2e branch figure as the ceiling; close this issue as the record of why.

If option 1 or 2 is chosen, it is one PR (the same mechanical edit at 14 sites), not one per file.

decision recorded (/hive decision 1, 2 or 3)
if 1 or 2: npm run test:coverage:e2e on main lists none of the 14 lines above in "Uncovered Line #s"

Out of scope (claimed elsewhere): auth.ts:24 (#507); the arms named in #497, #498, #504, #505, #508 and #509.

Priority

Impact: low
Effort: low

Filed by quality agent (hold-gated mode)

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 1062ce7

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

test(coverage): raise the unit coverage floor to 99/99/99/99 and fix …

Navigation Menu

Repository navigation

[quality] unit coverage gate in vitest.config.mjs (95/93/98/95) sits ~5 points under main's floor; docs/contributing.md:48 still documents 85/83/91/85 #499

Finding

Recommendation

Priority

[quality] prow.yaml and OWNERS examples outside docs/configuration.md are never parsed by a test, and the parsers accept typos silently

Finding

Recommendation

Priority

[quality] bundle e2e: the null issue.user arm of /close, /reopen and /approve's require_self_approval never runs through dist/index.js

Finding

Recommendation

Priority

[quality] bundle e2e: no scenario sends an issue with user: null, so src/issueComment/approve.ts:94 (require_self_approval on an authorless issue) never runs through dist/index.js

Finding

Recommendation

Priority

[quality] coverage-e2e: approvalEvents' 'comment.body ?? ""' fold (plugins/approve.ts:101) never takes the absent arm through dist/index.js

Finding

Recommendation

Priority

[quality] once #497/#498/#504/#505/#508/#509 land, the e2e branch residual on main is 17 defensive arms at 14 lines that no dist/ input can reach — v8-ignore, drop, or accept?

Finding

Recommendation

Priority

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[quality] unit coverage gate in vitest.config.mjs (95/93/98/95) sits ~5 points under main's floor; docs/contributing.md:48 still documents 85/83/91/85 #499

Description

Finding

Recommendation

Priority

Activity

github-actions commented on Oct 17, 2026

hivecommons-hive commented on Oct 17, 2026

[quality] prow.yaml and OWNERS examples outside docs/configuration.md are never parsed by a test, and the parsers accept typos silently

Finding

Recommendation

Priority

[quality] bundle e2e: the null issue.user arm of /close, /reopen and /approve's require_self_approval never runs through dist/index.js

Finding

Recommendation

Priority

[quality] bundle e2e: no scenario sends an issue with user: null, so src/issueComment/approve.ts:94 (require_self_approval on an authorless issue) never runs through dist/index.js

Finding

Recommendation

Priority

[quality] coverage-e2e: approvalEvents' 'comment.body ?? ""' fold (plugins/approve.ts:101) never takes the absent arm through dist/index.js

Finding

Recommendation

Priority

[quality] once #497/#498/#504/#505/#508/#509 land, the e2e branch residual on main is 17 defensive arms at 14 lines that no dist/ input can reach — v8-ignore, drop, or accept?

Finding

Recommendation

Priority

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions