Skip to content

[quality] trigger.ts: /test matches workflow names and paths case-insensitively, /ok-to-test honours an existing label in any case, and the non-PR arm of /test and /ok-to-test leaves the run green — none of it is asserted #523

Closed

Description

@hivecommons-hive

Current scope and disposition (2026-10-21)

This issue tracks only its original finding below, not every unrelated finding folded into its comments. The table is the current routing record. Original reports and comments are preserved as history; repeated or superseded checklists in those comments are not additional closure requirements.

Finding Canonical owner / disposition
Original trigger name/path casing, existing OK-To-Test label, green non-PR commands This issue; #524
approveReview PAT submission and withdrawal-reason assertions #526 / #527; duplicate here
Dispatcher and label-sync non-Error bundle scenarios #528
Repeated remaining plugin non-Error bundle proposals #548; one canonical implementation, not separate new test files
requireMatchingLabel non-Error rule-loop arm #549; wrappers make the proposed direct bundle scenario unsuitable
run.ts, command.ts and config.ts unreachable coverage arms #544; repeated proposals here are superseded by that PR
mergeQueue classification and bot-login assertions #529
labelSync casing, description, counts and pagination assertions #531
lgtmBinding mutation assertions #540
hold cancel casing assertions #541
duration and labelCatalog mutation assertions #542
removeLabels continuation after 404 #543
issue-comment error cause assertions #547
labels/lgtm.ts coverage annotation #536 / #537; duplicate here
Reconsider e2e branch floor after combined changes #551; not implemented by #524 or the coverage PRs

Each open PR remains responsible for its own work even after this issue closes. A PR may close this issue only by resolving the original finding; other rows must remain tracked at their canonical destination.


Original report (preserved)

Finding

src/issueComment/trigger.ts is 100 % line- and branch-covered by npm run test:coverage and all but one branch (statusOf, line 323) by npm run test:coverage:e2e on main @ 88241f4. A single-operator mutation probe over the file (117 mutants: ===/!==, &&/||, ??/||, comparison bounds, .some/.every, .toLowerCase() removal, true/false, ++/--, continue/break, status codes) still left five non-equivalent survivors against npx vitest related src/issueComment/trigger.ts --run (43 files / 837 tests):

line mutant behaviour nothing asserts
272 (run.name ?? '').toLowerCase() === arg → (run.name ?? '') === arg /test e2e must match a workflow whose display name is E2E even when its file (end-to-end.yml) does not. The existing /test CI case passes through the lowercase fixture path ci.yml, so the name comparison is never the deciding arm.
275 run.path.toLowerCase() → run.path /test lint-checks must match .github/workflows/Lint-Checks.yml. Every fixture path is already lowercase.
121 label.toLowerCase() === okToTestLabel → label === okToTestLabel A pull request already carrying OK-To-Test must not have the label re-added, and with nothing pending gets the "No workflow runs waiting" comment rather than a rocket. Only the exact-case label is tested.
72 /test: cmd === undefined → cmd === null /test on a plain issue comments "only applies to pull requests" and leaves core.setFailed uncalled. The /retest sibling test asserts both; the /test and /ok-to-test ones assert only the comment, so a thrown TypeError after the comment goes unnoticed.
113 /ok-to-test: same same

Equivalent survivors, not actionable: count-- in rerunEach (only === 0 is observed), current === null in headRuns (GITHUB_WORKFLOW is always set in Actions), ??→|| on empty-string folds, and type-annotation undefined→null.

Recommendation

Add to __tests__/issueCommentTest/trigger.test.ts:

  • /test › an it.each with E2E/end-to-end.yml under /test e2e and Lint/Lint-Checks.yml under /test lint-checks, asserting the re-run and that no listing comment is posted.
  • /ok-to-test › the label-already-present case with { name: 'OK-To-Test' }.
  • /test and /ok-to-test › extend the "only applies to pull requests" tests with a setFailed spy asserted not called (and, for /test, the exact calls list; for /ok-to-test, that the runs endpoint is not read).

Test-only; no src/ or dist/ change. Disjoint from the open held PRs #510–#522 (auth, contributing, remove, command/labelMatch, sweep, tide, requireMatchingLabel, approve).

Evidence

  • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run)
  • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run; the two reports are produced by different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
  • probe: each mutant applied in turn to src/issueComment/trigger.ts, npx vitest related src/issueComment/trigger.ts --run → 837 passed for all five

Priority

  • Impact: medium — /test is the user-facing re-run command; case-insensitive matching is what makes /test ci work for a workflow named CI in a file not named ci.yml
  • Effort: low

Filed by quality agent (hold-gated mode)


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

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

Activity

  1. added
    testingCreated by Hive for agent-filed issue provenance
    agent/qualityCreated by Hive for agent-filed issue provenance
    on Oct 19, 2026
  2. github-actions commented on Oct 19, 2026

    @github-actions
    Contributor

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

  3. hivecommons-hive commented on Oct 19, 2026

    @hivecommons-hive
    ContributorAuthor

    Historical folded record. Use #523's current disposition table, not each repeated checklist below. Approve-review is #526/#527, LGTM coverage is #536/#537, shared annotations are #544, plugin bundle cases are #548/#549, and the branch-floor decision is #551. The other independent assertions retain their listed PRs.


    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] approveReview.ts: a PAT that is not the author never has its review submission asserted, and withdrawalReason's five-file bound, lgtm skip, user casing and withdrawer order are unpinned

    Finding

    src/plugins/approveReview.ts is 100 % line-covered by npm run test:coverage and by npm run test:coverage:e2e on main @ 88241f4 (the e2e run's only zero-hit arms are the non-Error folds at 226/230, the class already tracked on #507). A single-operator mutation probe over the file (39 mutants: ===/!==, &&/||, comparison bounds, .toLowerCase()/.sort() removal, continue/break, undefined checks) left five non-equivalent survivors against npx vitest related src/plugins/approveReview.ts --run (46 files / 920 tests):

    line mutant behaviour nothing asserts
    188 identity.login !== undefined && identity.login === owners.author → … || … A user token (PAT) whose login is not the author must submit the APPROVE review. The mutant warns "opened by the token's own identity" and never calls POST /pulls/1/reviews for every PAT. The only user-token "approved" case (user: 'Prow-Bot', author: 'prow-bot') is the self-approval one; every submit case uses an installation token (GET /user → 403).
    105 uncoveredFiles.length > reasonFiles → >= Exactly five uncovered files must read no approver covers a, b, c, d, e, not … and 0 more. Only 7 files (→ and 2 more) is tested.
    111 continue → break With lgtm_acts_as_approve: false, an lgtm* event skips only itself; the cancels after it still count. The existing fixture puts dave's cancel after carol's lgtm-cancel, but dave then re-approves, so dropping dave entirely yields the same string.
    113 latest.set(event.user.toLowerCase(), event) → event.user bob and Bob are one person: a later approve by Bob retracts bob's cancel, and a cancel by BOB is reported as bob. The existing fixture's Bob approve / bob cancel lands on bob either way.
    118 .sort() on the withdrawers The dismissal message lists withdrawers alphabetically, not in action order; the existing fixture inserts bob before carol, which is already sorted.

    Equivalent, not actionable: line 226 'status' in e || typeof e.status === 'number' (e.status cannot be a number when status is not in e).

    Recommendation

    Add to __tests__/plugins/approveReview.test.ts:

    • approve.github_review: approved › a user-token case with user: 'Prow-Bot', author: 'some-author' asserting calls = ['label', 'notifier', 'review'] and no warning.
    • withdrawalReason › exactly-five-files; bob cancel then Bob approve → no withdrawal, and BOB cancel → withdrawn by bob; carol lgtm-cancel before bob cancel → withdrawn by bob; dave, bob, carol acting in that order → listed bob, carol, dave.

    Test-only; no src/ or dist/ change. Disjoint from the open held PRs #510–#525 (none of them touches approveReview.test.ts).

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run, 163 files / 2206 tests); approveReview.ts 100 % lines, no zero-hit branch
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run, 83 files / 502 tests); approveReview.ts 100 % lines, BRDA 226/230 only (the two reports are produced by different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
    • probe: each mutant applied in turn to src/plugins/approveReview.ts, npx vitest related src/plugins/approveReview.ts --run → 920 passed for all five; with the recommended cases each mutant fails exactly one test

    Priority

    • Impact: medium — line 188 decides whether approve.github_review works at all with a PAT, the documented way around "Allow GitHub Actions to create and approve pull requests"
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] v8-ignore the two String(e) arms no dist/index.js input can reach: run.ts:40 and config.ts:657

    Finding

    npm run test:coverage:e2e on main @ 88241f4 reports two zero-hit BRDA records that no input to dist/index.js can reach:

    site arm why unreachable from the bundle
    src/run.ts:40 : String(error) in core.setFailed(error instanceof Error ? error.message : String(error)) every event handler reaches run() through runEventHandlers (events.ts:50) or handleIssueComment's normalizeError / sweep (handleIssueComment.ts:188,217), and handleCronJobs has its own .catch wrappers. All of them coerce a non-Error to Error before it propagates. The only exceptions that escape them are built-in TypeErrors (__tests__/bundle/runHandlerRejection.test.ts), which are Errors.
    src/utils/config.ts:657 : e in throw new Error(`${source}: ${e instanceof Error ? e.message : e}`) the try wraps only parseDuration(raw.lookback, 'sweep.lookback'), and parseDuration (src/utils/duration.ts:26) throws new Error(...) and nothing else.

    Both arms are driven by the unit suite (npm run test:coverage: run.ts and config.ts at 100 % branches on the same revision), so this is a reporting gap, not a test gap: the e2e branch floor (vitest.e2e-coverage.config.mjs, 97 %) carries two permanently-red records that each re-triage has to rediscover.

    The seven sibling String(e) arms that are reachable through the real Octokit client are being pinned in #528 (three of them) via a preload that throws a string from the Response.status getter; the two above are the ones that preload cannot reach, because no client call sits inside their try.

    Recommendation

    Mark both arms with the /* v8 ignore start -- … */ … /* v8 ignore stop */ pair the repository already uses (src/plugins/tide.ts:235, src/utils/config.ts:219, src/utils/labeling.ts:91). Verified locally on main @ 88241f4: /* v8 ignore next */ does NOT suppress a ternary arm (the BRDA:40,2,1,0 record survives); the start/stop pair around the : arm does (only BRDA:40,2,0,2 remains).

    src/run.ts lines 39–41, replace

      catch (error) {
        core.setFailed(error instanceof Error ? error.message : String(error))
      }

    with

      catch (error) {
        core.setFailed(error instanceof Error
          ? error.message
          /* v8 ignore start -- every handler reaches run() through runEventHandlers or normalizeError, which coerce a non-Error first; the unit suite drives this arm with a primitive rejection */
          : String(error))
        /* v8 ignore stop */
      }

    src/utils/config.ts lines 656–658, replace

        catch (e) {
          throw new Error(`${source}: ${e instanceof Error ? e.message : e}`)
        }

    with

        catch (e) {
          throw new Error(`${source}: ${e instanceof Error
            ? e.message
            /* v8 ignore start -- parseDuration throws only Error; the unit suite drives this arm directly */
            : e}`)
          /* v8 ignore stop */
        }

    Then npm run pack (the committed dist/index.js changes with src/), npm run test:coverage and npm run test:coverage:e2e to confirm both BRDA records are gone and the thresholds still pass.

    • src/run.ts:40 marked, dist/index.js repacked
    • src/utils/config.ts:657 marked, dist/index.js repacked

    Decision taken by hive: one issue for both sites, since it is the same mechanical edit and the same dist/index.js repack; the start/stop form over next because it is the one form verified to work on a ternary arm and the one the repository already uses. This is a src/ + dist/ change, which the quality lane does not PR; it needs a maintainer or a lane that may edit production files.

    Priority

    • Impact: low (reporting hygiene; no behaviour change)
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] mergeQueue.ts: classify's expected-head/head-oid/required/checks/forbidden alternatives and enqueuedByBot's ^ anchor each survive deletion unnoticed

    Finding

    src/utils/mergeQueue.ts is 100 % line- and branch-covered by npm run test:coverage and, apart from the non-Error arm of errorMessage (line 190, the String(e) shape #528's preload addresses), by npm run test:coverage:e2e on main @ 88241f4. A single-operator mutation probe over the file (23 mutants: boolean guards, ==/===, regex anchors and flags, each alternative of the classify patterns, >/>=, the position and enqueuer folds) still left six non-equivalent survivors against npx vitest related src/utils/mergeQueue.ts --run (60 files / 1166 tests):

    line mutant behaviour nothing asserts
    153 /^github-actions$|\[bot\]$/i → drop ^ a human whose login ends in github-actions (my-github-actions) is not an automation, so the gate must not dequeue an entry they made (tide.ts:362). github-actions-fan pins the $ anchor; nothing pins ^.
    168 drop expected head from the head_moved pattern The expected head OID does not match also contains head oid, so the alternative is never the deciding one
    168 drop head oid same fixture, the other way round; expectedHeadOid mismatch: head_oid changed only pins head_oid
    174 drop required from the not_ready pattern Required status checks have not passed also contains checks
    174 drop checks same fixture
    177 drop forbidden from the forbidden pattern the two fixtures say permission and Resource not accessible; a bare Forbidden falls to other

    classify's kind drives tide.ts:336's per-kind handling of a refused enqueue, so each alternative is a behaviour, not a wording.

    Equivalent survivor, not actionable: dropping $ after \[bot\] — GitHub logins cannot contain [, so [bot] can only ever be a suffix. All 16 other mutants were killed.

    Recommendation

    In __tests__/utils/mergeQueue.test.ts:

    • enqueue › the classifies "%s" as %s table: add one message per uncovered alternative — expected head did not match and the head OID of the pull request has changed → head_moved; Required reviews are missing and Checks are still running → not_ready; Forbidden → forbidden.
    • enqueuedByBot › add ['my-github-actions', false].

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

    • __tests__/utils/mergeQueue.test.ts: one classify row per alternative at lines 168, 174, 177 and the ^-anchor login — all six mutants above killed

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run; mergeQueue.ts 100 % lines, 100 % branches)
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run; mergeQueue.ts 96.66 % branches, the one zero-hit record BRDA:190 being the String(e) arm; the two reports come from different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
    • probe: each mutant applied in turn to src/utils/mergeQueue.ts, npx vitest related src/utils/mergeQueue.ts --run → 1166 passed for all six listed; with the recommended rows added and re-probed against npx vitest run __tests__/utils/mergeQueue.test.ts, each fails exactly one test

    Disjoint from the open held PRs #510–#528: none touches __tests__/utils/mergeQueue.test.ts or src/utils/mergeQueue.ts (#518 covers tide.ts via __tests__/plugins/tide*.test.ts; #528's mergeQueue.ts:190 is the e2e String(e) arm, not these).

    Priority

    • Impact: medium — enqueuedByBot decides whether the gate may undo a merge-queue entry; classify decides how a refused enqueue is reported and retried
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] labelSync.ts: the case-insensitive name and color folds, the null-description fold, the dry-run counts and per_page are unpinned — the 'only the case differs' test's fixture never re-cases anything

    Finding

    src/cronJobs/labelSync.ts is 100 % line- and branch-covered by npm run test:coverage and all but the String(e) arm at line 107 (claimed by #528) by npm run test:coverage:e2e on main @ 88241f4. A single-operator mutation probe over the file (18 mutants: ===/!==, !dryRun inversions, .toLowerCase() removal, ?? '' removal, continue removal, > 0 bound, array-push removal, template-literal counts, spread folds, per_page) against npx vitest related src/cronJobs/labelSync.ts --run (4 files / 77 tests) left six non-equivalent survivors:

    line mutant behaviour nothing asserts
    57 byName keyed on label.name.toLowerCase() → label.name A repository label Kind/Bug must count as the configured kind/bug (GitHub label names are unique case-insensitively, so the "create" the mutant performs is a 422 in production).
    63 lookup label.name.toLowerCase() → label.name A configured name with capitals (Needs-Triage) must find the repository's needs-triage. Every catalog name is lowercase, so only a mixed-case configured name can tell.
    130 desired.color.toLowerCase() !== current.color.toLowerCase() → exact compare D73A4A in the config vs d73a4a in the repository must be unchanged, not a PATCH on every run.
    133 (current.description ?? '') → current.description A configured description: "" against a repository description: null is in sync (GitHub returns null for an empty description, so the mutant PATCHes forever).
    86 dry-run would update ${plan.update.length} → would update 0 The dry-run line's counts are never asserted; only includes('would update') && includes('lgtm').
    52 per_page: 100 → 30 The paging test checks page but never per_page.

    Why 57/130 survive although a test is titled for them: writes nothing when only the case of the name or color differs … builds its fixture with withChanges({ name: 'Kind/Bug', … }, { name: 'LGTM', … }), but withChanges matches change.name === label.name exactly, so neither re-cased entry replaces anything and the fixture served is complete unchanged. The test passes with or without the case folds — it is vacuous for the property in its title.

    Equivalent, not actionable: lines 117–118 (...(label.color === undefined ? {} : { color }) → always spread; JSON.stringify drops undefined so the wire body is identical).

    Recommendation

    In __tests__/cronJobTest/labelSync.test.ts:

    • make withChanges match names case-insensitively and assert the served fixture actually contains Kind/Bug / LGTM
    • add a mixed-case configured name (Needs-Triage) against a lowercase repository label to the same test
    • add a case: configured description: "" vs repository description: null → no write
    • pin the dry-run core.info line exactly (counts and the (color, description) field list)
    • assert per_page=100 in the paging test

    Test-only; no src/ or dist/ change. Disjoint from the open held PRs #510–#530: none touches __tests__/cronJobTest/labelSync.test.ts (#528 adds only __tests__/bundle/ files for labelSync.ts:107).

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run, 163 files / 2206 tests); labelSync.ts 100 % branches
    • e2e: npm run test:coverage:e2e → 99.95 / 98.79 / 100 / 99.95 (same revision, local run, 83 files / 502 tests); labelSync.ts BRDA:107 only. The two reports come from different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents.
    • probe: each mutant applied in turn, npx vitest related src/cronJobs/labelSync.ts --run → 77 passed for all six; with the recommended cases each mutant fails exactly one test (78 total)

    Priority

    • Impact: medium — label-sync is the job that writes to every configured repository's labels; the case folds are what keep it from 422-ing on Bug vs bug or re-PATCHing uppercase hex colors on every schedule
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] e2e: drive the eight remaining non-Error rejection arms (sweep, require-matching-label, tide, mergeQueue, approveReview, trigger) through #528's NON_ERROR_REJECT preload

    Finding

    npm run test:coverage:e2e on main @ 88241f4 (local, 2026-10-20; same figures as CI run 39212349431 e2e-coverage) reports 98.79 % branches. After discounting the arms already claimed by open PRs — #510 (auth.ts:14-24), #511 (the four negative-count v8 artifacts), #528 (events.ts:50, handleIssueComment.ts:217, labelSync.ts:107) — and the arms that are unreachable from the bundle (run.ts:40, config.ts:657, both folded on #523 by #528; lgtm.ts:68, because bindLgtm always rethrows an Error; command.ts:23, because getLineArgs is only called after hasCommand returned true), eight zero-hit BRDA arms remain, all the same shape:

    File:line Arm at zero hits Catch's only thrower
    src/cronJobs/sweep.ts:132 String(e) of e instanceof Error ? e.message : String(e) any step() whose rejection is not caught inside the step
    src/plugins/requireMatchingLabel.ts:167 : e of e instanceof Error ? e.message : e apply() → POST /repos/{o}/{r}/issues/{n}/labels / .../comments
    src/plugins/tide.ts:137 : undefined of typeof e.status === 'number' ? e.status : undefined octokit.pulls.merge → PUT /repos/{o}/{r}/pulls/{n}/merge
    src/plugins/tide.ts:138 String(e) same
    src/utils/mergeQueue.ts:190 String(e) in the GraphQL error fold octokit.graphql → POST /graphql
    src/plugins/approveReview.ts:226 : undefined of errorStatus users.getAuthenticated / pulls.createReview
    src/plugins/approveReview.ts:230 String(e) of errorMessage same
    src/issueComment/trigger.ts:323 : undefined of statusOf actions.approveWorkflowRun (POST .../actions/runs/{id}/approve) or the /retest re-run (POST .../actions/runs/{id}/rerun)

    Every one of these catch blocks only ever sees a rejection from an Octokit call, and @octokit/request wraps every fetch failure in a RequestError with a numeric status, so no fake-GitHub response can reach the non-Error arm. Unit coverage (npm run test:coverage, same revision, CI job build-test) reaches all eight lines with explicit non-Error throws, so this is unit-covered / e2e-uncovered. The unit and e2e maps come from different transforms (vite vs. tsc+webpack, see vitest.e2e-coverage.config.mjs) and cannot be merged; both runs were analysed separately.

    #528 adds the one mechanism that can drive these arms through dist/index.js: __tests__/bundle/nonErrorRejectionPreload.cjs, a NODE_OPTIONS=--require preload that makes the first request matching NON_ERROR_REJECT="<METHOD> /path" reject the client with a plain string. #528 deliberately drives only three arms and names none of the eight above, so they are the follow-up to that PR, not a competing one.

    Recommendation

    One PR, after #528 merges (it reuses #528's preload; a second preload would be a duplicate implementation):

    • Add __tests__/bundle/nonErrorRejectionArms2.test.ts (or extend nonErrorRejectionArms.test.ts) with one dist/index.js run per row, each setting NON_ERROR_REJECT to the request in the third column and asserting a single ::error:: / ::warning:: that carries the string, no unhandled rejection, and the same request list as the RequestError-shaped failure.
    • For sweep.ts:132, fault a request that no inner catch wraps (for example the ok-to-test step's run listing), so the string reaches the sweep's own fold rather than requireMatchingLabel.ts:167.
    • Verify coverage-e2e/lcov.info no longer carries BRDA:132 (sweep), BRDA:167 (requireMatchingLabel), BRDA:137/BRDA:138 (tide), BRDA:190 (mergeQueue), BRDA:226/BRDA:230 (approveReview) or BRDA:323 (trigger) at zero hits, and that e2e branches rise from 98.97 % (post-test(bundle): cover non-Error rejection paths including auth #528) to about 99.4 %.

    Decision taken by hive: all eight arms are one deliverable because they share one mechanism, one test harness and one PR-sized diff; splitting them would put eight PRs on the same preload. Blocked on #528; do not implement against #528's branch.

    Priority


    Filed by quality agent (hold-gated mode)


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

    — 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] e2e: drive the six non-Error rejection arms #528 leaves zero-hit (sweep.ts:132, trigger.ts:323, approveReview.ts:226/230, requireMatchingLabel.ts:167, tide.ts:137-138, mergeQueue.ts:190)

    Finding

    npm run test:coverage:e2e on main @ 88241f4 (local run; coverage-e2e/lcov.info) reports 18 zero-hit branches once the two negative-count v8 artifacts #511 documents are discounted. #528 drives three of them (events.ts:50, handleIssueComment.ts:217, labelSync.ts:107) and parks the two unreachable ones (run.ts:40, config.ts:657) on #523. Six more are the same shape — the String(e) / non-object arm of a catch around an octokit call — and no open PR names them:

    File:line Arm
    src/cronJobs/sweep.ts:132 outcome.errors.push(\${name}: ${e instanceof Error ? e.message : String(e)}`)` in the per-PR step loop
    src/issueComment/trigger.ts:323 statusOf(error) — typeof error === 'object' false arm (via isConflict at :258)
    src/plugins/approveReview.ts:226 errorStatus(e) — non-object arm (callers :37, :212)
    src/plugins/approveReview.ts:230 errorMessage(e) — String(e) arm (caller :203)
    src/plugins/requireMatchingLabel.ts:167 errors.push(\${rule.missing_label}: ${e instanceof Error ? e.message : e}`)`
    src/plugins/tide.ts:137-138 merge catch: status non-object arm and message: String(e) arm
    src/utils/mergeQueue.ts:190 errorMessage fallthrough String(e) after the errors[] check

    All six are reachable from the bundle: each try awaits a real octokit call, so the one-shot preload #528 adds (__tests__/bundle/nonErrorRejectionPreload.cjs, NON_ERROR_REJECT=METHOD /path) can reject exactly that request with a plain string, the same way #528 does for GET /pulls/1, GET /milestones and POST /labels.

    Evidence. Unit (npm run test:coverage, same revision): 100 % lines, 1652/1656 branches — every one of these arms is unit-covered, the four misses are the #511 artifacts. E2E: zero hits at each line above. The two reports are analyzed separately — vitest.e2e-coverage.config.mjs states the vite vs. tsc+webpack branch maps do not combine.

    Recommendation

    One PR, cut from main after #528 merges, extending __tests__/bundle/nonErrorRejectionArms.test.ts with six runs through dist/index.js, one per row, each faulting the single request its try awaits and asserting the ::error:: / outcome text names the string:

    • sweep.ts:132 — workflow_dispatch sweep with e.g. POST /pulls/1/requested_reviewers faulted → sweep: #1 evaluated with 1 error(s)
    • trigger.ts:323 — /test with the status-creating request faulted (string is not a 409 → not treated as conflict)
    • approveReview.ts:226 + :230 — /approve with POST /pulls/1/reviews faulted
    • requireMatchingLabel.ts:167 — label event with the rule's POST /issues/1/labels faulted
    • tide.ts:137-138 — PUT /pulls/1/merge faulted → result: 'failed', status: undefined
    • mergeQueue.ts:190 — POST /graphql enqueue faulted

    Then confirm coverage-e2e/lcov.info no longer carries any of the seven BRDA lines at zero and the e2e branch figure rises above #528's 98.97 %.

    Why this is an issue and not a PR this kick. The preload lives only on #528's held branch. A branch cut from origin/main would have to re-implement it (a second copy of the same mechanism), and stacking on #528's head would move a held PR's history, which the hold guard treats as unreviewed drift. Decision taken by hive: wait for #528, then a single follow-on PR.

    Priority

    Refs #528


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

    — 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] lgtmBinding.ts: hasLgtmLabel's .some and case fold, unbindLgtm's pending body and 140-char bound, and isLgtmBound's per_page=100 survive mutation unnoticed

    Finding

    src/plugins/lgtmBinding.ts is 100 % line- and branch-covered by npm run test:coverage and by npm run test:coverage:e2e on main @ 88241f4 (it appears in neither run's uncovered-lines table). A single-operator mutation probe over the file (38 mutants: ===/!==, &&/||, ?? defaults, .some/.every, .find/.findLast, .toLowerCase() removal, slice bounds, status states and contexts, 403, per_page, 'labeled') still left four non-equivalent survivors against the ten suites that exercise the module (__tests__/plugins/lgtmBinding.test.ts, label/lgtm.test.ts, cronJobTest/lgtm.test.ts, cronJobTest/sweep.test.ts, plugins/tide.test.ts, plugins/tideMergeQueue.test.ts, plugins/pluginErrorPaths.test.ts, pullReqTest/handlePullReq.test.ts, label/errorPaths.test.ts, issueCommentTest/handleIssueComment.test.ts; 338 tests):

    line mutant behaviour nothing asserts
    38 hasLgtmLabel: labels.some(…) → labels.every(…) A pull request labelled lgtm and anything else (approved, kind/*) still counts as carrying lgtm. tide.ts:184 gates the stale-binding strip on hasLgtmLabel(first.labels); with .every, the realistic PR (lgtm + approved) never has its unbound lgtm stripped and merges. Every stale-lgtm fixture carries lgtm as its only label.
    38 hasLgtmLabel: drop .toLowerCase() A hand-applied LGTM is the same label; the same tide.ts:184 gate would skip it. The case fold is pinned on the labeled event (lgtmOnPullRequest(labeled('LGTM'))) but not on the label list hasLgtmLabel reads.
    94 unbindLgtm: description.slice(0, 140) → slice(0, 141) GitHub rejects a commit-status description over 140 characters; unbindLgtm's truncation is unasserted (only bindLgtm's is). The only unbindLgtm test is the 500-warns one, so the pending body it sends is never read either.
    114 isLgtmBound: per_page: 100 → per_page: 30 The combined status is read with per_page=100 so a head with more than 30 status contexts still finds prow/lgtm.

    Equivalent, not actionable: line 121 .find → .findLast (the combined status carries one entry per context), line 156 ?? '' → ?? 'x' (any non-lgtm default refuses the same payloads).

    Recommendation

    Add to __tests__/plugins/lgtmBinding.test.ts:

    • a hasLgtmLabel describe: ['approved', 'lgtm', 'kind/bug'] → true, ['LGTM'] → true, ['approved', 'kind/bug'] → false, [] → false
    • unbindLgtm happy path: observe POST /statuses/abc and assert the body is exactly { state: 'pending', context: 'prow/lgtm', description: the first 140 characters } from a 200-char input
    • isLgtmBound: observe GET /commits/abc/status and assert per_page=100 on the URL

    Test-only; no src/ or dist/ change. hasLgtmLabel is pinned at the unit level in lgtmBinding.test.ts rather than through a tide fixture because __tests__/plugins/tide.test.ts is occupied by #518. Disjoint from every open held PR (#510–#539): none touches __tests__/plugins/lgtmBinding.test.ts or src/plugins/lgtmBinding.ts (#537 edits src/labels/lgtm.ts, a different module).

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run, 2550 statements); lgtmBinding.ts 100 % lines, no zero-hit branch
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run, 83 files / 502 tests); lgtmBinding.ts 100 % lines, no zero-hit branch (the two reports are produced by different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
    • probe: each mutant applied in turn to src/plugins/lgtmBinding.ts, npx vitest run on the ten suites listed above → 338 passed for all four survivors; with the recommended cases, the .every mutant fails two tests and each of the other three fails exactly one

    Priority

    • Impact: medium — hasLgtmLabel decides whether tide checks the lgtm binding at all; the surviving .every mutant is exactly the shape that lets a stale hand-applied lgtm merge on any PR that also carries approved
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] hold.ts: /hold cancel's case fold on the hold labels is unpinned (Hold, DO-NOT-MERGE/HOLD survive mutation)

    Finding

    src/labels/hold.ts is 100 % line- and branch-covered by both npm run test:coverage and npm run test:coverage:e2e on main @ 88241f4 (it appears in neither run's uncovered table and has no zero-hit BRDA record in coverage-e2e/lcov.info). A single-operator mutation probe over the file (14 mutants: ===/!==, &&/||, command/keyword spellings, .toLowerCase() removal, list membership and order, .filter duplication, length === 0) against the six suites that exercise it (__tests__/label/hold.test.ts, label/errorPaths.test.ts, label/authorization.test.ts, issueCommentTest/handleIssueComment.test.ts, missingNumberMessage.test.ts, cronJobTest/lgtm.test.ts) left two non-equivalent survivors, both in cancelHold:

    line mutant behaviour nothing asserts
    71 wanted.has(label.toLowerCase()) → wanted.has(label) An issue whose label was created as Hold or DO-NOT-MERGE/HOLD is never released by /hold cancel, /unhold or /remove-hold — cancelHold logs could not find … to remove and returns. meetsMergeGate is pinned to block on Hold case-insensitively (__tests__/utils/mergeGate.test.ts), so the PR stays unmergeable with no way to clear it from a comment. Every cancel fixture carries the labels in lower case.
    70 [holdLabel, legacyHoldLabel].map(name => name.toLowerCase()) → .map(name => name) A configured hold.label with upper-case characters (ex: Do-Not-Merge/Hold) is compared verbatim, so the repository's actual do-not-merge/hold label is not released. Both hold.label fixtures are lower case.

    Equivalent, not actionable: line 70 swapping the two names (a Set).

    Recommendation

    Add to the cancel block of __tests__/label/hold.test.ts:

    • /hold cancel on an issue carrying Hold and DO-NOT-MERGE/HOLD → observe DELETE …/labels/Hold and DELETE …/labels/DO-NOT-MERGE%2FHOLD (the labels are removed under their own names), no setFailed
    • hold.label: Do-Not-Merge/Hold with the issue carrying do-not-merge/hold → observe DELETE …/labels/do-not-merge%2Fhold; no DELETE for the configured spelling

    Test-only; no src/ or dist/ change. Disjoint from every open held PR (#510–#540): none touches __tests__/label/hold.test.ts or src/labels/hold.ts (#512 label/remove.test.ts, #539 label/label.test.ts + label/lifecycle.test.ts).

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run, 163 files / 2206 tests); hold.ts 100 % lines, no zero-hit branch
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run, 83 files / 502 tests); hold.ts 100 % lines, no zero-hit branch (the two reports come from different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
    • probe: each mutant applied in turn to src/labels/hold.ts, npx vitest run on the six suites above → all pass for the two survivors; with the recommended cases each survivor fails exactly one test

    Priority

    • Impact: medium — /hold cancel is the only comment path that clears the merge block; the surviving mutants leave a mixed-case hold label stuck on the PR
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] duration.ts: parseDuration's whitespace trim and millisecond rounding, and labelCatalog.ts's case-insensitive needs- color, are not asserted

    Finding

    src/utils/duration.ts and src/utils/labelCatalog.ts are 100 % line- and branch-covered by both npm run test:coverage and npm run test:coverage:e2e on main @ 88241f4. A single-operator mutation probe (npx vitest related <file> --run; 1498 tests for duration.ts, 143 for labelCatalog.ts) still left three non-equivalent survivors:

    file:line mutant behaviour nothing asserts
    duration.ts:12 (text ?? '').trim() → (text ?? '') grace_period_duration: "5s " (trailing or leading whitespace, as yaml folding easily produces) parses to 5000 ms instead of throwing invalid grace_period_duration. Every existing fixture is already trimmed; '5 s' is rejected for its inner space, not for an outer one.
    duration.ts:29 Math.round(ms) → ms 1.5ms yields the integer 2, not 1.5. The doc comment promises milliseconds; the only fractional fixture, 1.5s, is already a whole 1500.
    labelCatalog.ts:86 label.name.toLowerCase().startsWith('needs-') → label.name.startsWith('needs-') A require_matching_label.missing_label of Needs-Kind still gets the ededed needs- color in desiredLabels. The only fixture is lowercase needs-kind.

    Equivalent survivor, not actionable: labeling.ts:148 e.name || '' → e.name ?? '' (name is string | undefined). The other 16 mutants over labeling.ts, labelCatalog.ts, comments.ts and duration.ts were all killed.

    Recommendation

    Test-only; no src/ or dist/ change. Files disjoint from every open held PR (#510–#541).

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run)
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run; the two reports are produced by different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
    • probe: each mutant applied in turn, npx vitest related → all passed for the three above

    Priority

    • Impact: low — both arms are defensive input folds; the trim one is the most likely to bite a real .prow.yaml
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] labeling.ts: removeLabels is never shown to keep deleting the later labels after an earlier one is already gone (404)

    Finding

    src/utils/labeling.ts is 100 % line- and branch-covered by npm run test:coverage and by npm run test:coverage:e2e on main @ 88241f4. A single-operator mutation probe over the file (16 mutants: ===/> bounds, .toLowerCase() removal, ||/??, the isNotFound guards, the memo set, per_page, addPrefix's separator and empty-prefix arm, the 404 arm of removeLabels) against npx vitest related src/utils/labeling.ts --run (67 files / 1369 tests) left one non-equivalent survivor:

    line mutant behaviour nothing asserts
    180-181 removeLabels: after core.debug(\label ${label} was already absent`), returninstead of continuing thefor` loop When one of several labels is already gone (DELETE … → 404), the labels after it must still be deleted. Every existing 404 case (__tests__/label/remove.test.ts "ignores a label that is already gone", the hold/prefixed/fixed callers) removes a single label, so a loop that silently stops at the first 404 passes the whole suite. removeLabels is the shared removal path for /remove, /remove-<prefix>, /hold cancel, /lgtm cancel, exclusive-prefix replacement (prefixed.ts:107), approve's label sync and require-matching-label's cleanup, so a regression there would leave stale labels behind on every one of them.

    Equivalent survivors, not actionable: e.name || '' → e.name ?? '' in getCurrentLabels (both fold ''/null/undefined to ''); dropping 'status' in error in isNotFound (error.status === 404 is already false for an object without status; the guard only narrows the TypeScript type).

    Recommendation

    Add a removeLabels describe to __tests__/utils/labeling.test.ts that calls removeLabels directly through a real newOctokit client against an msw DELETE /repos/Codertocat/Hello-World/issues/1/labels/:name handler recording the order of names:

    • ['gone', 'kind/bug', 'still-there'] with gone answered 404 → resolves, and all three names were requested in order.
    • ['first', 'broken', 'never'] with broken answered 500 → rejects with could not remove label broken, and never was not requested (pins the complementary "a real failure stops the loop" arm, which only the single-label cases assert today).

    Test-only; no src/ or dist/ change. Disjoint from every open held PR: #512 owns __tests__/label/remove.test.ts, #514 command.test.ts/labelMatch.test.ts, #542 duration.test.ts/labelCatalog.test.ts; none touches __tests__/utils/labeling.test.ts or src/utils/labeling.ts.

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run; the four reported branches are the negative-count v8 artifacts docs(contributing): record the four negative-count BRDA branches as a v8 await artifact, not a gap #511 documents)
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run; labeling.ts has no zero-hit branch; the two reports come from different transforms and are not merged, as vitest.e2e-coverage.config.mjs documents)
    • probe: each mutant applied in turn to src/utils/labeling.ts, npx vitest related src/utils/labeling.ts --run → 1369 passed for the survivor above; the proposed test fails under it (expected ['gone'] to deeply equal ['gone', 'kind/bug', 'still-there']) and passes on main

    Priority

    • Impact: medium — shared removal path for every label-removing command and plugin; the multi-label-with-a-gone-label case is a routine race (two commands in one comment, a concurrent sweep)
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] e2e coverage: v8-ignore the three bundle-unreachable fallback arms in run.ts:40, command.ts:23 and config.ts:657

    Finding

    npm run test:coverage:e2e on main @ 88241f4 reports 98.79 % branches (1636/1656). Three of the zero-hit records are fallback arms that no input to dist/index.js can reach, so they are re-triaged as gaps on every kick although they are not ones (the same situation #510 and #537 clear for auth.ts:24 and lgtm.ts:68):

    record arm why the bundle cannot reach it
    src/run.ts BRDA:40,2,1,0 core.setFailed(error instanceof Error ? error.message : String(error)) — the String(error) arm Every handler folds its rejections into an Error before run() sees them: runEventHandlers (events.ts:50), normalizeError (handleIssueComment.ts:217), the jobs .catch(e => core.setFailed(…)) in handlePullReq.ts:76 and handleCronJob.ts:69. The one path that does reach this catch from the bundle is a non-string comment body (TypeError: body.replace is not a function out of splitLines, found by instrumenting the coverage bundle locally) — a built-in, so an Error.
    src/utils/command.ts BRDA:23,0,1,0 getLineArgs's .at(-1) ?? '' fallback Its only callers, /milestone (milestone.ts:43) and /retitle (retitle.ts:31), run only after handleIssueComment matched the body with hasCommand(form, body) for the same commandPattern, and neither command has an alias in commandAliases; so findCommandArgs is never empty when getLineArgs runs.
    src/utils/config.ts BRDA:657,83,1,0 normalizeSweep's e instanceof Error ? e.message : e — the non-Error arm parseDuration (duration.ts:26) throws only new Error(...).

    All three arms are driven by the unit suite (npm run test:coverage → 100 % lines / 99.75 % branches: __tests__/run.test.ts, __tests__/utils/command.test.ts:91, __tests__/utils/configSweep.test.ts:13 with a mocked string throw), so they are covered by unit, not by e2e, and as defensive arms they are low priority. #523 folds them as "v8 ignore entries that still need a src/ change"; they are not covered by any open PR (#510 is auth.ts, #537 is lgtm.ts, #528 drives events.ts:50 / handleIssueComment.ts:217 / labelSync.ts:107 for real).

    Recommendation

    Annotate the three arms with /* v8 ignore next -- … */, the form src/utils/config.ts:219 already uses and #510 / #537 apply to the same class of arm, and repack dist/. No behaviour change; the unit tests that drive the arms stay.

    One mechanical note for whoever applies it: @vitest/coverage-v8's AST mapper honours a hint on the alternate of a ?: (so run.ts and config.ts keep their Error arm measured), but not on one operand of a ?? — for command.ts the hint has to sit on the return statement.

    This is one deliverable (the same one-line annotation at three sites), so it is one issue and one PR.

    Evidence

    • unit: npm run test:coverage → 100 / 99.75 / 100 / 100 (main @ 88241f4, local run, coverage/lcov.info)
    • e2e: npm run test:coverage:e2e → 99.96 / 98.79 / 100 / 99.95 (same revision, local run, coverage-e2e/lcov.info). The two reports come from different transforms (vite vs. tsc+ncc remap) and are not merged, as vitest.e2e-coverage.config.mjs documents; each is read separately above.

    Priority

    • Impact: low (reporting noise, no untested behaviour)
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] errorPaths: the eleven 'could not …: ${e}' wraps in src/issueComment are asserted by prefix only; dropping the cause goes unnoticed

    Finding

    Every could not … wrap in src/issueComment/* interpolates the rejection — throw new Error(\could not close issue: ${e}`) and ten siblings (assign.ts:42,73, unassign.ts:42,64, cc.ts:39,70, uncc.ts:38, reopen.ts:45, close.ts:60, retitle.ts:51, lock.ts:62) — so the Action failure names what GitHub refused (HttpError: Resource not accessible by integration`), which is the one line a maintainer has when a token lacks a permission.

    __tests__/issueCommentTest/errorPaths.test.ts asserts each of those eleven wraps by prefix only (rejects.toThrow(/could not close issue/)). A single-operator mutation probe on main @ 88241f4 that drops the : ${e} from close.ts:60 and reopen.ts:45 left the whole related suite green (npx vitest related src/issueComment/close.ts --run → 30 files / 576 passed, same for reopen.ts); the same holds for lock.ts:62 against errorPaths.test.ts directly. The other eight sites are the same shape and the same assertion form.

    Evidence

    • unit: npm run test:coverage on main @ 88241f4 (local) → 100 / 99.75 / 100 / 100; all eleven sites are line- and branch-covered, so this is a pinning gap, not a coverage gap. The four reported branch misses (close.ts:39, reopen.ts:36, approve.ts:504, auth.ts:220) are the negative-count v8 artifacts docs(contributing): record the four negative-count BRDA branches as a v8 await artifact, not a gap #511 documents.
    • e2e: coverage-e2e artifact of workflow run 39381633514 (main @ 88241f4, 2026-10-20) → 99.95 / 98.79 / 100 / 99.96; none of the eleven sites is a zero-hit record there either. The two reports come from different transforms and are not merged (vitest.e2e-coverage.config.mjs).
    • probe: close.ts:60 could not close issue: ${e} → could not close issue: 576/576 pass. reopen.ts:45 same: 576/576 pass.

    Recommendation

    In __tests__/issueCommentTest/errorPaths.test.ts, assert the full message for each of the eleven wraps: rejects.toThrow(`could not <step>: HttpError: Resource not accessible by integration`). Test-only; one file; no src/, dist/ or docs change. Disjoint from every open held PR (#510–#545): none touches errorPaths.test.ts.

    Priority

    • Impact: low — diagnostics only, but it is the only cause a failed /close, /lock, /assign … surfaces
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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] drive the eight remaining non-Error rejection arms (sweep:132, trigger:323, approveReview:226/230, requireMatchingLabel:167, tide:137-138, mergeQueue:190) through dist/index.js once #528 lands

    Finding

    npm run test:coverage:e2e on main @ 88241f4 (local; CI e2e-coverage job on run 39549983858 reports the same 99.95 % lines / 98.79 % branches) leaves 18 zero-hit BRDA records in coverage-e2e/lcov.info. Ten are already claimed by open held PRs (#510 auth.ts:14-17, #511 approve.ts:504, #528 events.ts:50 / handleIssueComment.ts:217 / labelSync.ts:107, #537 lgtm.ts:68, #544 run.ts:40 / command.ts:23 / config.ts:657). The remaining eight are not named by any open PR:

    record arm catch site octokit call that feeds it
    src/cronJobs/sweep.ts:132 String(e) sweep()'s per-step catch any step's first request, e.g. GET /repos/{o}/{r}/issues/{n}/comments in require-matching-label
    src/issueComment/trigger.ts:323 statusOf → undefined rerunAll's catch (isConflict then isForbidden) POST /repos/{o}/{r}/actions/runs/{id}/rerun
    src/plugins/approveReview.ts:226 errorStatus non-number → undefined tokenIdentity's rejection handler GET /user
    src/plugins/approveReview.ts:230 errorMessage String(e) submitApproval's catch POST /repos/{o}/{r}/pulls/{n}/reviews
    src/plugins/requireMatchingLabel.ts:167 : e enforceRequiredLabels's per-rule catch DELETE /repos/{o}/{r}/issues/comments/{id} or the listComments page
    src/plugins/tide.ts:137 status non-number → undefined mergeOnce's catch PUT /repos/{o}/{r}/pulls/{n}/merge
    src/plugins/tide.ts:138 String(e) same same
    src/utils/mergeQueue.ts:190 String(e) enqueue's catch → errorMessage POST /graphql (enqueuePullRequest)

    All eight are unit-covered (npm run test:coverage @ 88241f4: 100 % lines / 99.75 % branches, the four misses being the negative-count artifacts #511 documents), so this is an e2e-only gap. They share one root cause with the three arms #528 drives: Octokit wraps every fetch rejection in a RequestError, so no fake-GitHub status reaches a non-Error / status-less arm. The only way to reach them from dist/index.js is the NON_ERROR_REJECT preload #528 introduces (__tests__/bundle/nonErrorRejectionPreload.cjs, a NODE_OPTIONS=--require hook that makes the first matching METHOD /path response's status getter throw a plain string). The two suites' coverage maps come from different transforms (vite vs tsc+webpack) and do not combine (vitest.e2e-coverage.config.mjs), so unit and e2e are reported separately here on purpose.

    Recommendation

    One PR, after #528 merges (it supplies the preload; a second copy of that file would be the overlap the hold-gated preflight forbids):

    • Add __tests__/bundle/nonErrorRejectionArmsRemaining.test.ts with one dist/index.js run per row above, each setting NON_ERROR_REJECT="<METHOD> <path>" from the last column, asserting a single ::error::/::warning:: that names the string, no unhandled rejection, and the same request list as the Error-shaped failure the existing scenario for that path already asserts.
    • npm run test:coverage:e2e: coverage-e2e/lcov.info no longer carries any of the eight records at zero hits; branches rise from 98.97 % (post-test(bundle): cover non-Error rejection paths including auth #528/chore(coverage): v8-ignore the bundle-unreachable fallback arms in run.ts:40, command.ts:23 and config.ts:657 #544) toward 99.5 %.
    • Raise the branches floor in vitest.e2e-coverage.config.mjs from 97 to 98 so a scenario that stops exercising one of these arms fails the run (the file's own comment says the floor tracks just under what main reaches).

    Decision taken by hive: reuse #528's preload rather than add a second fault-injection mechanism; one test file rather than one per module, because every row is the same mechanical scenario.

    Priority

    Refs #528 (blocked on it merging).


    Filed by quality agent (hold-gated mode)


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

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

  4. hivecommons-hive commented on Oct 19, 2026

    @hivecommons-hive
    ContributorAuthor

    Superseded tracking record. #544 owns the command.ts annotation together with run.ts/config.ts. This is not a remaining requirement for #523/#524.


    Folded finding: src/utils/command.ts:23 is the one remaining e2e zero-hit branch that is neither a String(e) arm nor a v8 artifact

    npm run test:coverage:e2e on main @ 88241f4 (local run) lists 20 zero-hit BRDA records. #528 drives three; its "Not in this PR" note folds run.ts:40 / config.ts:657 here; #510/#507 own auth.ts:14-17,24; #511 documents the two negative-count artifacts. Of the rest, every one except command.ts:23 is the non-object / non-Error arm of an error fold (sweep.ts:132, trigger.ts:323, lgtm.ts:68, approveReview.ts:226,230, requireMatchingLabel.ts:167, tide.ts:137,138, mergeQueue.ts:190) and becomes drivable with the preload #528 adds, so they are not re-filed here.

    command.ts:23 is different in kind:

    export function getLineArgs(command: string, body: string): string {
      return findCommandArgs(command, body).at(-1) ?? ''
    }

    The ?? '' arm (BRDA:23,0,1,0) is reached only when the command matches no line of the body. Its two callers are milestone.ts:43 (/milestone) and retitle.ts:31 (/retitle); handleIssueComment.ts:128 dispatches either only after commandForms(command).some(form => hasCommand(form, commentBody)), commandForms returns just [command] for both (commandAliases has no entry for them, and neither is a dynamic label command), and hasCommand is findCommandArgs(command, body).length > 0 — the same scan getLineArgs repeats. So from dist/index.js the .at(-1) result is never undefined; the arm is a defensive default for direct callers and is exercised only by __tests__/utils/command.test.ts (unit: 100 % branches on the file).

    Verified replacement text (local, second checkout of main @ 88241f4)

    export function getLineArgs(command: string, body: string): string {
      /* v8 ignore start -- the dispatcher only calls /milestone and /retitle after hasCommand found a line, so .at(-1) is never undefined there; the '' arm is a defensive default kept for direct callers */
      return findCommandArgs(command, body).at(-1) ?? ''
      /* v8 ignore stop */
    }

    With only that change, npm run test:coverage:e2e → 83 files / 502 tests pass, command.ts leaves the uncovered table, coverage-e2e/lcov.info carries no BRDA:23 for the file, and e2e branches move 98.79 % → 98.85 %. Same v8 ignore start/stop form 7a1df7a (#483) and #510 use.

    This is a src/utils/command.ts + dist/index.js change, so — as with the run.ts:40 / config.ts:657 entry above — it needs a human or an ISSUES_PRS_MERGE agent to land; the quality lane does not PR production files. Applying it is mechanical: the block above replaces lines 22-24 verbatim, then npm run pack.

    • src/utils/command.ts:23 — v8 ignore the ?? '' arm of getLineArgs (text above), repack dist/

    Filed by quality agent (hold-gated mode)


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

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

  5. hivecommons-hive commented on Oct 19, 2026

    @hivecommons-hive
    ContributorAuthor

    Duplicate tracking record. #536 is the canonical LGTM coverage issue and #537 its implementation. This does not extend #523/#524.


    Folded finding: src/labels/lgtm.ts:68 is a third String(e) arm no dist/index.js input can reach

    npm run test:coverage:e2e on main @ 88241f4 (local run) still carries BRDA:68,8,1,0 for src/labels/lgtm.ts; npm run test:coverage on the same revision reports the file at 100 % branches, so — like run.ts:40 / config.ts:657 above — this is a reporting gap, not a test gap.

    The earlier folded note on this tracker listed lgtm.ts:68 among the sites that "become drivable with the preload #528 adds". That is not the case for this one. Correction, with the reason:

    // src/labels/lgtm.ts:64-69
    try {
      await bindLgtm(octokit, context, headSha, commenterId, context.payload.comment?.html_url)
    }
    catch (e) {
      await refuse(octokit, context, issueNumber, e instanceof Error ? e.message : String(e), e)
    }

    The try wraps only bindLgtm. bindLgtm (src/plugins/lgtmBinding.ts:58-75) catches every rejection of octokit.repos.createCommitStatus and rethrows new Error(...) on both of its paths (isForbidden → permission hint; otherwise could not bind lgtm to …: ${e}). A non-Error faulted into the client by the #528 preload therefore arrives at lgtm.ts:67 already wrapped as an Error, and String(e) is never evaluated. Nothing else inside the try can throw a non-Error (loadPullRequestOwners is outside it).

    Replacement text (mechanical)

    Replace src/labels/lgtm.ts:66-68 with:

        catch (e) {
          /* v8 ignore start -- bindLgtm rethrows every rejection as an Error, so the String(e) arm is unreachable from dist/index.js */
          await refuse(octokit, context, issueNumber, e instanceof Error ? e.message : String(e), e)
          /* v8 ignore stop */
        }

    then npm run pack so the committed dist/index.js follows src/.

    Verified locally on 88241f4 with that exact edit: npx eslint src/labels/lgtm.ts clean; after npm run pack:coverage and an e2e-config run of the bundle scenarios that enter handleLgtm, coverage-e2e/lcov.info carries no BRDA record for the refuse line while the sibling BRDA:62 records of the same function are still emitted — i.e. the ignore removes exactly that arm and nothing else. Same v8 ignore start/stop form as src/utils/labeling.ts:91, src/utils/mergeQueue.ts:66 and the run.ts:40 / config.ts:657 entry above.

    This is a src/labels/lgtm.ts + dist/index.js change, so it needs a human or an ISSUES_PRS_MERGE agent to land; the quality lane does not PR production files. Decision taken by hive: fold here rather than open a fourth issue, since it is the same mechanical edit and the same repack as the two sites already tracked.

    • src/labels/lgtm.ts:68 — v8 ignore the String(e) arm of the bindLgtm catch (text above), repack dist/

    Filed by quality agent (hold-gated mode)


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

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

  6. hivecommons-hive commented on Oct 20, 2026

    @hivecommons-hive
    ContributorAuthor

    Duplicate tracking record. #541 owns the hold-cancel casing assertions. This is independent of #523/#524.


    Folded finding: src/labels/hold.ts — /hold cancel's case fold on the hold labels is unpinned

    hive-open-issue consolidated this onto the tracker, so the record goes here. src/labels/hold.ts is 100 % covered by both npm run test:coverage and npm run test:coverage:e2e on main @ 88241f4 (local runs; no zero-hit BRDA). A 14-mutant single-operator probe against its six direct suites (label/hold, label/errorPaths, label/authorization, issueCommentTest/handleIssueComment, missingNumberMessage, cronJobTest/lgtm) left two non-equivalent survivors in cancelHold:

    line mutant behaviour nothing asserts
    71 wanted.has(label.toLowerCase()) → wanted.has(label) An issue whose label was created as Hold or DO-NOT-MERGE/HOLD is never released by /hold cancel, /unhold or /remove-hold; meetsMergeGate is pinned to block on Hold case-insensitively, so the PR stays unmergeable with no comment path to clear it. Every cancel fixture is lower case.
    70 [holdLabel, legacyHoldLabel].map(name => name.toLowerCase()) → .map(name => name) A mixed-case configured hold.label (ex: Do-Not-Merge/Hold) is compared verbatim, so the repository's do-not-merge/hold is not released.

    Equivalent: swapping the two names in the Set.

    Fix (test-only, __tests__/label/hold.test.ts, disjoint from every open held PR): /hold cancel on an issue carrying Hold + DO-NOT-MERGE/HOLD observes both DELETEs under their own names; hold.label: Do-Not-Merge/Hold with the issue carrying do-not-merge/hold observes DELETE …/do-not-merge%2Fhold only. Each survivor then fails exactly one test. PR follows.


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

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

  7. hivecommons-hive commented on Oct 20, 2026

    @hivecommons-hive
    ContributorAuthor

    Duplicate tracking record. #542 owns the duration and labelCatalog assertions. This is independent of #523/#524.


    Folded finding: src/utils/duration.ts trim/round and src/utils/labelCatalog.ts needs- case fold are unpinned

    hive-open-issue consolidated this onto the tracker, so the record goes here. Both files are 100 % covered by npm run test:coverage (100 / 99.75 / 100 / 100) and npm run test:coverage:e2e (99.96 / 98.79 / 100 / 99.95) on main @ 88241f4 (local runs; the two reports are not merged, per vitest.e2e-coverage.config.mjs). A single-operator mutation probe (npx vitest related <file> --run; 1498 tests for duration.ts, 143 for labelCatalog.ts, 1369 for labeling.ts, 1362 for comments.ts) left three non-equivalent survivors:

    file:line mutant behaviour nothing asserts
    duration.ts:12 (text ?? '').trim() → (text ?? '') grace_period_duration: "5s " parses to 5000 ms instead of throwing. Every fixture is pre-trimmed; '5 s' is rejected for its inner space, not an outer one.
    duration.ts:29 Math.round(ms) → ms 1.5ms → integer 2, not 1.5. The only fractional fixture, 1.5s, is already a whole 1500.
    labelCatalog.ts:86 label.name.toLowerCase().startsWith('needs-') → label.name.startsWith('needs-') missing_label: Needs-Kind still gets the ededed color in desiredLabels. The only fixture is lowercase needs-kind.

    Equivalent, not actionable: labeling.ts:148 e.name || '' → ?? (name is string | undefined). The other 16 mutants over the four files were killed.

    Fix: new __tests__/utils/duration.test.ts (the existing parseDuration table is in __tests__/plugins/requireMatchingLabel.test.ts, occupied by #520) and one case in __tests__/utils/labelCatalog.test.ts. Test-only. PR follows.

    Priority: low impact, low effort.


    Filed by quality agent (hold-gated mode)


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

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

  8. hivecommons-hive commented on Oct 21, 2026

    @hivecommons-hive
    ContributorAuthor

    Superseded tracking record. #549 owns the requireMatchingLabel annotation. The earlier proposals to reach this arm with a raw API rejection are superseded because intermediate helpers wrap the rejection. This is independent of #523/#524.


    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] requireMatchingLabel.ts:167's non-Error arm is bundle-unreachable; hint it like #544

    Finding

    npm run test:coverage:e2e on main @ 88241f4 reports one String(e)-shaped branch at zero hits that no open PR claims and that no bundle scenario can reach: src/plugins/requireMatchingLabel.ts:167, the : e arm of

    errors.push(`${rule.missing_label}: ${e instanceof Error ? e.message : e}`)

    Every call apply() makes rewraps its rejection in new Error(...) before it can reach this catch: labelIssue (could not add labels: …, src/utils/labeling.ts:66), removeLabels (labeling.ts:185), createComment (src/utils/comments.ts:34), botCommentsWithMarker (requireMatchingLabel.ts:245) and deleteMissingComments (requireMatchingLabel.ts:233). A bundle scenario that faults POST /issues/1/labels with a plain-string rejection (the preload mechanism of #528 / #548) was written and dropped from #548 because the arm still read could not add labels: <string> — the Error arm.

    Unit evidence: npm run test:coverage @ 88241f4 drives both arms (coverage/lcov.info: BRDA:167,23,0,4 / BRDA:167,23,1,1), via __tests__/plugins/requireMatchingLabel.test.ts:407 "reports a non-Error rejection from a rule verbatim". E2E evidence: coverage-e2e/lcov.info @ 88241f4: BRDA:167,23,1,0.

    This is the same class as #544 (run.ts:40, command.ts:23, config.ts:657): unit-covered, bundle-unreachable, so it will keep appearing in every e2e residual list until it carries a hint.

    Recommendation

    Apply the same marker #544 uses on config.ts:657. In src/plugins/requireMatchingLabel.ts replace

          errors.push(`${rule.missing_label}: ${e instanceof Error ? e.message : e}`)

    with

          // every call apply() makes (labelIssue, removeLabels, createComment, the comment list and delete) rewraps its rejection in an Error
          errors.push(`${rule.missing_label}: ${e instanceof Error ? e.message : /* v8 ignore next -- non-Error arm unreachable from dist/index.js; requireMatchingLabel.test.ts drives it */ e}`)

    then npm run pack to refresh dist/index.js, and confirm npm run test:coverage:e2e no longer lists requireMatchingLabel.ts under uncovered lines. Do not lower the e2e branch floor in vitest.e2e-coverage.config.mjs.

    • src/plugins/requireMatchingLabel.ts:167 carries the hint and dist/index.js is rebuilt

    This is a src/ + dist/ edit, which the quality lane does not PR (tests only); it needs a human or a lane that may change production files. Decision taken by hive: hint the arm rather than restructure apply()'s callers, matching #544.

    Priority

    • Impact: low
    • Effort: low

    Filed by quality agent (hold-gated mode)


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

    — 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