Skip to content

refactor(approve, meow): drop the three dead arms left open on #466 - #497

Open
hivecommons-hive[bot] wants to merge 1 commit into
mainfrom
quality/466-dead-arms
Open

hivecommons-hive[bot] wants to merge 1 commit into
mainfrom
quality/466-dead-arms

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Change

Files/functions claimed: src/plugins/approve.ts (effectiveKind, upsertNotifier), src/issueComment/meow.ts (fetchCatImage), __tests__/plugins/approve.test.ts, __tests__/issueCommentTest/meow.test.ts, __tests__/bundle/meow.test.ts, dist/index.js (repack). This is the remaining production-cleanup cluster from the reopen comment on #466; same shape as #462 and #492.

  • approve.ts:228,230 — effectiveKind's 'ignore' arms for review-approved / review-changes are unreachable: with ignore_review_state on, approve.ts:381 already passes [] as the review list to approvalEvents, so no review event ever carries those kinds. Both now return 'add' / 'remove' unconditionally. The unit case ignore_review_state: reviews neither add nor remove (which fed review events directly into computeApproval with the flag on, a state production never produces) is removed; the setting's real behaviour stays covered by the handler-level unit tests and the approveIgnoreReviewState bundle scenarios from test(bundle): drive approve.ignore_review_state through dist/index.js (approve.ts:380-381) #468.
  • approve.ts:415 — existing.body ?? '' is dead by construction: existing is only selected when comment.body contains the notifier marker. The find predicate is now a type guard (comment is IssueComment & { body: string }), so IssueComment['body'] stays typed optional (the point raised on refactor: drop defensive arms unreachable from dist/index.js #462) and no fallback or cast is needed.
  • meow.ts:80,88 — response.type !== 'opaqueredirect' and || 'a redirect' are browser-only: Node's fetch has no filtered responses, so redirect: 'manual' resolves with the real 3xx status. The unit case names a status-less response as a redirect (which stubbed fetch to resolve Response.error(), which the runtime never does) is removed; the bundle redirect assertion in __tests__/bundle/meow.test.ts is tightened from (302|a redirect) to 302.

The fourth item in the reopen comment (owners.ts:127-147 expandGroups team arms) already landed in #486 (__tests__/bundle/ownersGroups.test.ts), so after this merges nothing is left for #466 to track.

Verification (local, this branch)

  • npx tsc --noEmit clean; npx eslint src __tests__ 0 errors (one pre-existing cc.ts jsdoc warning, untouched)
  • npm run test:coverage: 157 files / 2114 tests pass; src/plugins/approve.ts and src/issueComment/meow.ts both 100% branch; coverage/lcov.info has no BRDA:…,0 entry
  • npm run build && npm run pack: dist/index.js repacked (6 lines changed)
  • npm run test:coverage:e2e: 78 files / 487 tests pass; coverage-e2e/lcov.info has no BRDA:228, BRDA:230 or BRDA:415 under SF:src/plugins/approve.ts, and meow.ts reads 100 / 100 / 100 / 100

Related Issue

Closes #466


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

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

…0,415; meow.ts:88)

effectiveKind's 'ignore' arms for review-approved / review-changes cannot
run: with ignore_review_state on, the caller already passes [] as the
review list, so no review event ever reaches them.

upsertNotifier's existing.body ?? '' is dead by construction: the comment
is only selected when its body contains the marker. Narrow the find
predicate with a type guard instead of a fallback.

fetchCatImage's opaqueredirect guard and the 'a redirect' fallback are
browser-only: Node's fetch has no filtered responses, so a manual
redirect resolves with the real 3xx status.

Unit cases that only reached the removed arms are deleted; the bundle
redirect assertion is tightened to 302; dist/ repacked.

Refs #466 (the three production-cleanup items from the reopen comment; the owners.ts item already landed in #486)

Signed-off-by: quality <quality@hive.kubestellar.io>
@hivecommons-hive
hivecommons-hive Bot requested a review from jpmcb as a code owner October 17, 2026 02:12
@hivecommons-hive hivecommons-hive Bot added the hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 17, 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.

@github-actions

Copy link
Copy Markdown
Contributor

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

@hivecommons-hive

Copy link
Copy Markdown
Contributor Author
a35e1ef

ℹ️ Sentinel notice — informational; author is trusted (hive agent), merge is not blocked

Hive flagged this PR (author @hivecommons-hive[bot], head a35e1efb1f6c) because it matches behaviors that can override security controls, escalate privileges or damage the codebase. This is a heuristic, not an accusation — a maintainer should confirm the change is intended before it merges.

  • test_removal — removes 25 test lines while adding 1 (Deletes test files or guts test coverage)
    • __tests__/bundle/meow.test.ts
    • __tests__/issueCommentTest/meow.test.ts
    • __tests__/plugins/approve.test.ts

Hive did not add the sentinel-alert label because this author is trusted. The finding remains recorded for audit and the dashboard Security tab; set sentinel.trusted_authors_block: true to require the blocking label for trusted authors too.

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. needs-kind

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[quality] coverage-e2e names tide.ts:99 uncovered; the zero is the never-reached options.initial ?? await getPull fallback on line 98

0 participants