Skip to content

[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

Open

Description

@hivecommons-hive

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

Activity

  1. github-actions commented on Oct 17, 2026

    @github-actions
    Contributor

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

  2. hivecommons-hive commented on Oct 17, 2026

    @hivecommons-hive
    ContributorAuthor

    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 Errors
    src/utils/mergeQueue.ts:190 e instanceof Error ? e.message : String(e) BRDA:190,13,1,0 Octokit / GraphQL rejections are Errors

    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:

    1. 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.
    2. 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.
    3. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/qualityCreated by Hive for agent-filed issue provenanceneeds-kindstage/040-covered-by-prAn open PR covers this issuetestingCreated by Hive for agent-filed issue provenance

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions