From 485d0f516a2040361f5604fd7b3c522ac8aaa25c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 01:37:48 -0700 Subject: [PATCH 1/3] test(review): document the last unreachable review-evasion coverage branch closePullRequest's only failure path is Octokit's request() call, which always rejects with a RequestError (an Error subclass) -- the `: new Error(...)` normalization branch is a type-safety fallback for the unknown-typed catch, not a reachable runtime path. Closes out patch coverage on the review-evasion protection PR: 100% line/branch across all 10 changed files. --- src/queue/processors.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 0d7b39b68e..ddb4400ec0 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10652,6 +10652,10 @@ async function closeReviewEvasionSelfCloseIfActive( // this enforcement. Propagate so the queue's own retry/backoff re-processes this job; on retry, the live // freshness check earlier in this function will see the PR as open (current, since we just reopened it) // and this handler will attempt the re-close again, converging once closePullRequest actually succeeds. + // The `: new Error(...)` fallback is unreachable in practice -- closePullRequest's only failure path is + // Octokit's `request()` call, which always rejects with a RequestError (an Error subclass), never a raw + // thrown value -- but `closeError` is typed `unknown`, so the branch stays as a type-safe normalization. + /* v8 ignore next */ throw closeError instanceof Error ? closeError : new Error(errorMessage(closeError)); } From a706afcebcba52251d2a63ceada3655c7d8f3a7d Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 02:26:40 -0700 Subject: [PATCH 2/3] test(review): document the last unreachable review-evasion coverage branch Adds a v8-ignore annotation for the defensive non-Error normalization arm in closeReviewEvasionSelfCloseIfActive's re-close-failure path. closePullRequest's only failure mode is Octokit's request() call, which always rejects with a RequestError (an Error subclass), never a raw thrown value -- the normalization exists only to satisfy the unknown-typed catch's type signature. Split the ternary into an if block with the ignore scoped to only the unreachable else/fallback, so the reachable Error rethrow path stays fully subject to coverage. No functional change. --- src/queue/processors.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ddb4400ec0..2f8e88864e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10652,11 +10652,16 @@ async function closeReviewEvasionSelfCloseIfActive( // this enforcement. Propagate so the queue's own retry/backoff re-processes this job; on retry, the live // freshness check earlier in this function will see the PR as open (current, since we just reopened it) // and this handler will attempt the re-close again, converging once closePullRequest actually succeeds. - // The `: new Error(...)` fallback is unreachable in practice -- closePullRequest's only failure path is - // Octokit's `request()` call, which always rejects with a RequestError (an Error subclass), never a raw - // thrown value -- but `closeError` is typed `unknown`, so the branch stays as a type-safe normalization. - /* v8 ignore next */ - throw closeError instanceof Error ? closeError : new Error(errorMessage(closeError)); + // The `else` (closeError NOT an Error, falling through to the normalization below) is unreachable in + // practice -- closePullRequest's only failure path is Octokit's `request()` call, which always rejects + // with a RequestError (an Error subclass), never a raw thrown value -- but `closeError` is typed + // `unknown`, so the fallback below stays as a type-safe normalization. The `if` body itself (the real + // rethrow) IS reachable and IS exercised by the existing re-close-failure tests -- only the else branch + // (this `if`'s implicit non-Error path) and the fallback statement below are ignored. + /* v8 ignore else */ + if (closeError instanceof Error) throw closeError; + /* v8 ignore next -- unreachable, see above. */ + throw new Error(errorMessage(closeError)); } // The close succeeded: post the public explanation, apply the configured label, record the strike -- in From bc0168e542804962fa1e96dcd80e374416285046 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 02:52:35 -0700 Subject: [PATCH 3/3] test(review): name the exact test covering the reachable re-close rethrow Names the specific existing regression test that exercises the `closeError instanceof Error` rethrow in closeReviewEvasionSelfCloseIfActive, so the branch-coverage annotation's claim is directly verifiable rather than asserted. No functional change. --- src/queue/processors.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 2f8e88864e..3a3050eff2 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10657,7 +10657,10 @@ async function closeReviewEvasionSelfCloseIfActive( // with a RequestError (an Error subclass), never a raw thrown value -- but `closeError` is typed // `unknown`, so the fallback below stays as a type-safe normalization. The `if` body itself (the real // rethrow) IS reachable and IS exercised by the existing re-close-failure tests -- only the else branch - // (this `if`'s implicit non-Error path) and the fallback statement below are ignored. + // (this `if`'s implicit non-Error path) and the fallback statement below are ignored. Concretely: a 500 + // from GitHub on the re-close PATCH makes closePullRequest reject with a RequestError, which is exactly + // what test/unit/queue.test.ts's "REGRESSION (gate-flagged): a retry after the re-close failure + // converges -- the PR ends up closed, and the strike is recorded exactly once" drives through this `if`. /* v8 ignore else */ if (closeError instanceof Error) throw closeError; /* v8 ignore next -- unreachable, see above. */