Skip to content

test(owners): pin the per-team group cache key, parseGroup anchor, decode, defaultBranch and isNotFound guards of owners.ts - #575

Closed
hivecommons-hive[bot] wants to merge 1 commit into
mainfrom
quality/test-owners-guards
Closed

hivecommons-hive[bot] wants to merge 1 commit into
mainfrom
quality/test-owners-guards

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Test Improvement

Files / cluster claimed: __tests__/utils/ownersGuards.test.ts (new). No src/, dist/ or existing-test change. Disjoint from the held PRs on this module: #564 adds __tests__/utils/ownersGroupPagination.test.ts (group paginator fail-closed), #569 adds the bundle ownersParseArms/ownersProbeArms scenarios, #571 edits ownersAuth.test.ts/repoHasOwners.test.ts. Uses #572's harness only to measure; does not depend on it merging.

Ten unit tests pinning the thirteen behavioural contracts of src/utils/owners.ts that a Stryker run (--mutate src/utils/owners.ts, main @ e3119ad: 332 mutants, 15 survived, 95.48%) found unasserted, each applied by hand at its reported span to confirm it is killable:

  • expandGroups: two same-org teams resolve to their own members and each is read once (groupCache key ${owner}/${slug}, line 151 — with one key the second team silently gets the first team's members); alice@myorg/team stays a plain login and fetches no team (parseGroup leading anchor, line 99); the other-organization warning names the group and the supported org (line 139).
  • parseOwners: a bare options: key (YAML null) means no options (line 52).
  • loadOwnersTree: a blob with an encoding but no content is rejected with the "invalid OWNERS file returned from GitHub API for OWNERS" message (line 278; only encoding-less blobs were tested).
  • repoHasOwners: a payload with no repository at all falls back to repos.get (repository?.default_branch, line 468); a second repository is not answered from the first (hasOwnersCache key, line 398).
  • branchHasOwners: a string and a null rejection from git.getTree are reported as "error listing the tree of main: …" rather than tripping the in operator (isNotFound, line 482, six mutants); a tree entry without a path is skipped and the OWNERS file after it still found (line 447).

Re-running the harness with this file: 15 → 2 survivors (98.49%). The two left are owners.ts:161 (warning text #564 asserts exactly — not duplicated here) and owners.ts:317 (core.debug text, equivalent by the review standard #572 documents).

Validation

  • npx vitest run __tests__/utils/ownersGuards.test.ts → 10 passed
  • npm run test:coverage → 168 files / 2340 passed, 99.92 / 99.35 / 100 / 99.91 (unchanged: the module was already at 100%)
  • npx eslint __tests__/utils/ownersGuards.test.ts → clean
  • npx stryker run --mutate src/utils/owners.ts --force (on test(mutation): add a bounded Stryker harness for the merge-decision code #572's config, ps shimmed) before/after: 304→315 killed, 15→5 survived; the three pins added after the second run (options: null, string/null rejection) each verified by applying the mutant by hand and watching one test fail

Related Issue

Closes #574 — merging leaves nothing for the issue to track.

Refs #554 ("Strengthen assertions and the test harness": a consumer of the mutation harness, not one of its boxes).


Filed by quality agent (hold-gated mode). Human review required.

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

…code, defaultBranch and isNotFound guards

Running the bounded Stryker check against src/utils/owners.ts left 15
survivors at 100% line coverage. Thirteen are behavioural: with one
groupCache key every @org/team entry resolves to the first team's
members; parseGroup without its leading anchor reads alice@org/team as
a group; a bare `options:` key, a blob without content, a payload
without a repository, a string or null tree rejection and a path-less
tree entry each hit a TypeError instead of the handled path; and
repoHasOwners would answer a second repository from the first.

Each is pinned with a direct call in a new file so the harness reports
98.49% on the module, leaving only the warning text held #564 asserts
and a debug string.

Closes #574

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
@hivecommons-hive
hivecommons-hive Bot requested a review from jpmcb as a code owner October 24, 2026 12:28
@hivecommons-hive hivecommons-hive Bot added the hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 24, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

Important

Held for human review by the hive's ACMM level gate.

This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an outreach PR that targets a repository this hive does not manage is held at every level because it publishes project-facing communication into someone else's project.

Hive will keep the hold label until a human removes it. Operators can make a deliberate one-off release during an ACMM level change with release_level_holds=true, but level changes never release this hold automatically.

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read src/utils/owners.ts at the reviewed head and ran the new file (npx vitest run __tests__/utils/ownersGuards.test.ts → 10 passed). Each test pins a guard that is actually in the source:

  • per-team groupCache key ${owner}/${slug} and the catch-to-empty fallback — src/utils/owners.ts:151-164
  • parseGroup leading ^@ anchor so alice@myorg/team stays a plain login — src/utils/owners.ts:99-100
  • other-org warning text via warnOnce — src/utils/owners.ts:139
  • bare options: (YAML null) → noParentOwners: false — src/utils/owners.ts:51-54
  • decode rejecting a blob with encoding but no content — src/utils/owners.ts:278-280
  • repoHasOwners falling back to repos.get when the payload has no repository (defaultBranch, src/utils/owners.ts:468-477) and the per-repo hasOwnersCache key — src/utils/owners.ts:398
  • isNotFound's typeof error === 'object' && error !== null guard keeping a string/null rejection out of the in operator — src/utils/owners.ts:481-483; the path-less tree-entry skip — src/utils/owners.ts:447

The diff adds one new test file only — no src/ or existing-test changes, so the claimed disjointness from #564/#569/#571 holds by file set, and the fake-client paginate shape matches the real call site (octokit.paginate(octokit.teams.listMembersInOrg, …), line 153). The expand-groups test's assertion that the second team is read separately would indeed fail if the cache key dropped the slug.

Checks are green and the PR body's claims match the code. Looks correct to me; the hold label is the quality lane's gate, so a maintainer sign-off is what's left.

— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88

@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

/kind cleanup


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

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

@github-actions github-actions Bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind labels Oct 26, 2026
@mrbobbytables

Copy link
Copy Markdown
Member

Consolidated into #588 together with the other eight open mutation-survivor pin PRs (commit cherry-picked unchanged, DCO preserved). Closing in favour of that PR.

mrbobbytables added a commit that referenced this pull request Oct 26, 2026
, #578-#583, #585) (#588)

* test(cron): pin cronLgtm's merge count and its no-read short circuits

cronLgtm resolves to the number of pull requests it merged or enqueued,
and a pull request it skips before the shared merge path - gate failure
or locked - costs no further request. Neither was asserted: the count is
discarded by handleCronJob, and tide's own blockedReason also refuses a
locked or gated PR, so the existing merge observers could not see the
guards. A mutation run over src/cronJobs/lgtm.ts left 13 survivors; the
nine behavioural ones (jobsDone++, return false, the !gate.ok and locked
guards, { required: true } on github-token, state: 'open') are now
killed by direct cronLgtm calls that watch GET /pulls/:number. The four
left are log text.

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(owners): pin the per-team group cache key, parseGroup anchor, decode, defaultBranch and isNotFound guards

Running the bounded Stryker check against src/utils/owners.ts left 15
survivors at 100% line coverage. Thirteen are behavioural: with one
groupCache key every @org/team entry resolves to the first team's
members; parseGroup without its leading anchor reads alice@org/team as
a group; a bare `options:` key, a blob without content, a payload
without a repository, a string or null tree rejection and a path-less
tree entry each hit a TypeError instead of the handled path; and
repoHasOwners would answer a second repository from the first.

Each is pinned with a direct call in a new file so the harness reports
98.49% on the module, leaving only the warning text held #564 asserts
and a debug string.

Closes #574

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(tide): pin mergeOnce's status guard, the 403-only fork diagnosis, sorted workflow list, queue-path retryIf, entry-less dequeue and isUnknown's state arm

Each case fails when the guard it names is removed or inverted; line
coverage already reached them all (Stryker on src/plugins/tide.ts:
477/517 killed before, 488/517 after).

Refs #570
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(auth): pin the repository-less org check, the gone-user comment skip, the non-RequestError and half-formed root OWNERS reads, and the OWNERS path in role errors

A Stryker probe of src/utils/auth.ts (--mutate src/utils/auth.ts on the
harness of #572) left 22 of 273 mutants alive. Eleven of them flip a guard
the suite reaches but never distinguishes; this file pins each one:

- checkOrgMember on a payload without a repository returns false by the
  early return, not by a TypeError caught as an unexpected error, so no
  warning annotation is emitted
- checkIssueComments skips a comment whose user is null (e.user?.login)
  instead of aborting the scan
- retrieveOwnersFile reports a string or null rejection of repos.getContent
  as the OWNERS lookup error rather than tripping on 'status' in e
- a contents response with content but no encoding, or an encoding but no
  content, is rejected as an invalid OWNERS file rather than decoded
- a root OWNERS role that is not a list is reported as 'OWNERS at OWNERS:'
  from both assertRootOwner and rootOwnersIncludes

The seven remaining survivors are core.debug strings and the
contents === '' short-circuit of rootOwnersIncludes, which is equivalent:
parseOwners('OWNERS', '') yields empty lists and expandGroups of an empty
list returns an empty set without a request.

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(approve): pin the github-actions[bot] login-only skip, the uncoverable-file suggestion, the notifier whitespace and marker checks, the unlabeled other-label skip, the base-branch OWNERS probe and the required token

A Stryker probe of src/plugins/approve.ts (--mutate src/plugins/approve.ts on
the harness of #572, main@e3119ad) left 59 of 515 mutants alive. Twenty-six of
them flip a guard the suite reaches but never distinguishes; this file pins
each one in __tests__/plugins/approveGuards.test.ts:

- approvalEvents skips a github-actions[bot] comment by its login alone,
  without relying on the Bot user type
- suggestApprovers still names an approver when another changed file has no
  OWNERS entry at all, instead of dereferencing the missing per-file set
- computeApproval logs the approver who covers none of the changed files and
  says nothing for a pull request without an author
- renderNotifier's no-files body is exactly the verdict, the explanation and
  the marker
- approveOnPullRequest returns on `unlabeled` of another label without a
  single api call, probes the OWNERS of the event's base branch rather than
  the default branch, and fails on a missing github-token input before
  reading anything
- upsertNotifier recognises a stale notifier by a github-actions[bot] user
  without a Bot type, skips a comment whose user is gone, treats a notifier
  that differs only by surrounding whitespace as up to date, and never
  mistakes a bot comment without the literal marker for the notifier
- approveOnReview on a payload without a review or a pull request fails on
  the missing pull request, not on the missing review

Re-running the probe with this file kills all twenty-six; the 33 that remain
are equivalent (log strings, optional chains on values already checked,
switch defaults, an excluded.add after the covered files were removed).

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(mergeQueue): pin errorMessage's null-rejection guards, the once-per-run warning's initial state and the [bot] suffix anchor

Adds __tests__/utils/mergeQueueGuards.test.ts. Every line and branch of
src/utils/mergeQueue.ts is already covered, but five of its mutants survive
the Stryker harness: `npx stryker run --mutate src/utils/mergeQueue.ts`
(main@e3119ad) kills 106 of 111 (score 95.50%). The survivors, each
re-applied by hand at its reported span and confirmed to pass the existing
mergeQueue.test.ts:

- mergeQueue.ts:184 `e !== null` -> `true`, the whole
  `typeof e === 'object' && e !== null && 'errors' in e` conjunction -> `true`,
  and the final `&&` -> `||`: a null or undefined rejection of octokit.graphql
  would throw a TypeError out of errorMessage, so queueState would reject
  instead of warning and falling back to the REST merge, and enqueue would
  throw instead of returning `{ ok: false }`.
- mergeQueue.ts:64 `let warnedUnavailable = false` -> `true`: every existing
  test calls resetMergeQueueWarnings() (directly or via setupActionsEnv), so
  the first real run of a job never warning went unnoticed. Pinned on a fresh
  module instance (vi.resetModules) that nothing has reset.
- mergeQueue.ts:153 `/^github-actions$|\[bot\]$/i` -> `/^github-actions$|\[bot\]/i`:
  a login merely containing `[bot]` would count as an automation whose queue
  entry the gate may undo.

With this file all five are killed (manual re-application of each mutant
fails 1-3 of the new cases; 111/111 killed).

No production code changes.

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(lgtm): pin the refusal error log, the no-label cancel debug, the token guard, the missing-head error and isForbidden's non-object arms

A Stryker probe of src/labels/lgtm.ts and src/plugins/lgtmBinding.ts
(npx stryker run --mutate src/labels/lgtm.ts,src/plugins/lgtmBinding.ts
on the harness of #572, main @ e3119ad, Stryker 10.0.0 / Vitest 5.0.4,
8 workers, 6 m 36 s) left 24 of 202 mutants alive (88.12 %). Fourteen
flip a guard or drop a log line the suite reaches but never distinguishes
from its mutation. Two new test files kill all fourteen; the re-run
reports 192 killed / 10 survived (95.05 %).

__tests__/label/lgtmGuards.test.ts (src/labels/lgtm.ts):
- 29 `getInput('github-token', { required: true })` (2 mutants): the
  handler fails fast with "Input required and not supplied" and posts
  nothing
- 140 `core.error(msg)` in refuse: the refusal is logged as an error with
  the same text it replies with
- 96 the "could not find lgtm to remove" debug line of a cancel on an
  issue without the label, and that no DELETE follows

__tests__/plugins/lgtmBindingGuards.test.ts (src/plugins/lgtmBinding.ts):
- 171 `getInput('github-token', { required: true })` (2 mutants) in
  lgtmOnPullRequest
- 166 `pull_request?.head?.sha` -> `head.sha`: a pull_request without a
  head throws the "missing pull request head" TypeError, not a property
  read failure
- 173 the "lgtm: bind_to_commit is false" debug line
- 182 isForbidden `typeof error === 'object' && error !== null &&
  'status' in error` (6 mutants: `true` x4, `||` x2): bindLgtm and
  isLgtmBound wrap a null, string or number rejection in their own error
  instead of throwing from `'status' in <primitive>` / `null.status`

Reviewed survivors, not pinned (10): lgtm.ts 33, 34 (x2), 35, 65 and
lgtmBinding.ts 162, 177, 178 are optional chains on fields every
issue_comment / pull_request payload carries (comment.body, comment.user,
issue.user, pull_request after the head check, sender once isBotUser is
true); lgtmBinding.ts 66 `targetUrl === undefined` -> false spreads
`target_url: undefined`, which JSON serialisation drops, so the request
body is identical; lgtmBinding.ts 156 `label?.name ?? ''` -> any other
string still fails the `!== 'lgtm'` comparison and returns early.

Refs #554

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test(trigger): pin the guards of /retest, /test, /ok-to-test and okToTestOnPullRequest that a Stryker probe left unasserted

A Stryker probe of src/issueComment/trigger.ts (346 mutants) left 38 alive.
Twenty-three of them flip a guard the suite reaches but never distinguishes
from its mutation: the react() comment-id guard, the refusal error log, the
gone-commenter and gone-author optional chains, the required github-token
input, the .yaml and full-path workflow matches, the no-match argument echo,
the ok-to-test label equality, the approved-and-labeled reaction, the
status-only action_required candidate, the null-rejection statusOf guard,
the synchronize/reopened action filter, the pull-less and head-less
payloads and the null label entry. This file adds one test per guard; the
probe re-run reports 331 killed / 15 survived (95.66 %). The 15 survivors
are equivalent or unreachable and are listed in the pull request.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

* test: pin the milestone list separator, isBotUser's login arm, createComment's message and skipReadOnlyForkRun's payload guards

A Stryker probe over the issue-comment, label and utility modules left
nine behavioural survivors the unit suite reaches but never distinguishes
from their mutations. Three new files pin them:

- __tests__/issueCommentTest/milestoneAvailableList.test.ts: the
  not-found error joins the open milestones with ', ' (milestone.ts:66).
- __tests__/utils/commentsGuards.test.ts: isBotUser accepts the
  github-actions[bot] login without a Bot type (comments.ts:67),
  createComment rewraps a refused post as 'could not add comment: ...'
  (comments.ts:34), and createCommentOnce honours a login-only bot marker.
- __tests__/pullReqTest/readOnlyForkPayloads.test.ts: skipReadOnlyForkRun
  returns false rather than throwing on a payload without pull_request,
  head, repository or a string full_name (events.ts:23-25).

Closes #584

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>

---------

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Co-authored-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mrbobbytables
mrbobbytables deleted the quality/test-owners-guards branch October 26, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hold Indicates that a PR should not merge because someone has issued a /hold command. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[quality] owners.ts: 13 behavioural mutants survive the suite, incl. a shared group cache key that gives every team the first team's members

1 participant