ci(fork-sync): add : never return type as 4th calibration signal (#2435)#2444
Merged
alexey-pelykh merged 1 commit intomainfrom Apr 20, 2026
Merged
ci(fork-sync): add : never return type as 4th calibration signal (#2435)#2444alexey-pelykh merged 1 commit intomainfrom
: never return type as 4th calibration signal (#2435)#2444alexey-pelykh merged 1 commit intomainfrom
Conversation
…2435) Extends scripts/check-throwing-stub-callers.mjs (H7 of ADR 0005) to close the hypothetical gap where a throwing stub is reintroduced without any of the existing three calibration signals (variadic-unknown signature, fork-attributed throw message, Gutted marker comment). \`: never\` return type fires as a calibration signal only when the function has no typed non-variadic-unknown parameters — this preserves the implicit typed-helper exclusion. Legitimate typed error-throw helpers remain unflagged: exitHooksCliWithError(err: unknown): never -- typed param throwGatewayAuthResolutionError(reason: string): never throwPathEscapesBoundary(params: {...}): never throwUnresolvedGatewaySecretInput(path: string): never Uncalibrated stubs now fire — the hypothetical function foo(): never { throw new Error("bar"); } would be classified as a throwing stub if it had live callers. Changes: - isUnknownArrayType / hasNeverReturnType / hasTypedNonVariadicUnknownParams helpers (isUnknownArrayType also DRYs the prior inline logic in hasVariadicUnknownArgs) - classifyStubFunction: accepts returnType; computes neverReturnSignal gated by typed-param exclusion; added to guard + signals list - classifyFixture(sourceText) export for fixture-based testing - 10-case self-test suite via --self-test flag (zero-infrastructure; wired into ci.yml BEFORE the main gate step so classifier regressions surface early) - Header JSDoc documents the new signal Verification: - node scripts/check-throwing-stub-callers.mjs --self-test -> 10/10 PASS - node scripts/check-throwing-stub-callers.mjs -> 0 violations - pnpm check -> clean - Fresh-context adversarial validation (session 37d6f4b6) -> CLEAN - Fresh-context polish pass (session 825b76a6) -> CLEAN after 2 iterations Re-scope history: original #2435 proposed a separate new script with a 21-entry allowlist seed; 360 evaluation on 2026-04-20 found the naive grep count was noise-inflated and the extension approach avoids FP duplication with the existing gate. Issue title and AC re-scoped in parallel. HQ ADR 0005 H7 description updated accordingly. Orthogonal observation (not addressed here): .throwing-stub-callers-allowlist entry for resolveAgentRuntimeOrThrow is now stale (remediated in commit d4a01b9 as part of #2408 fix) — can be cleaned up in a follow-up.
This was referenced Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
scripts/check-throwing-stub-callers.mjs(ADR 0005 H7) with a fourth calibration signal:: neverreturn type. Closes the hypothetical gap where a throwing-stub regression could be reintroduced without any of the existing three calibration signals (variadic-unknown signature / fork-attributed throw message /// Gutted in RemoteClaw forkmarker comment).The new signal is gated by an explicit typed-param exclusion —
: neverfires as a signal ONLY when the function has no typed non-variadic-unknown parameters. This preserves the existing implicit typed-helper exclusion:: neverfunction foo(): never { throw ... }(uncalibrated stub)exitHooksCliWithError(err: unknown): neverthrowGatewayAuthResolutionError(reason: string): neverthrowPathEscapesBoundary(params: {...}): neverthrowUnresolvedGatewaySecretInput(path: string): neverlistProviderModels(..._args: unknown[]): never { throw ... }Re-scope history
Original #2435 proposed a separate new
scripts/check-throwing-stubs.mjswith a ~21-entry allowlist seed. A 360 evaluation on 2026-04-20 found:grep -rn ": never" src/ \| wc -lcount of 24 was noise-inflated (type-level conditional types, comments, etc.); real function: neverreturns are ~6-8, and most are legitimate typed error-throw helpers the existing gate correctly discriminatescheck-throwing-stub-callers.mjs(PR ci(repo): detect throwing-stub-with-live-callers anti-pattern (closes #2410) #2412 closing ci(repo): detect throwing-stub-with-live-callers anti-pattern #2410) already handles the fix(agents): restore resolveAgentRuntimeOrThrow body — regression stub crashed every agent dispatch #2408 patternRe-scoped to this extension approach. HQ ADR 0005 H7 description updated in parallel.
Changes
scripts/check-throwing-stub-callers.mjs:isUnknownArrayType,hasNeverReturnType,hasTypedNonVariadicUnknownParamshasVariadicUnknownArgsrefactored to delegate toisUnknownArrayType(DRY; single source of truth for unknown-array shape)classifyStubFunctionacceptsreturnType; computesneverReturnSignal = neverReturn && !hasTypedParamsclassifyFixture(sourceText)exported for fixture-based testing--self-testflag (zero-infrastructure; no new vitest config needed).github/workflows/ci.yml: newSelf-test classifier (fixture-based)step BEFORE the main gate step inthrowing-stub-callers-gatejob — classifier regressions surface earlyVerification
node scripts/check-throwing-stub-callers.mjs --self-test→ 10/10 PASSnode scripts/check-throwing-stub-callers.mjs→ 0 violations on main (shape unchanged)pnpm check→ clean (format + typecheck + lint + custom gates)Self-test fixture matrix
: neverstub (no params) → FLAGGED ✅ new: never(upstream-compat pattern) → FLAGGED (both signals): neverwithout throw body (loops forever) → NOT flagged: neverwith multi-statement body → NOT flagged (not a single-throw): never→ FLAGGED ✅ new: never→ NOT flagged: neverthrowing function → NOT flaggedOrthogonal observation (not fixed here)
.throwing-stub-callers-allowlistcontains a stale entry forresolveAgentRuntimeOrThrow— the underlying stub was remediated in commitd4a01b9190as part of #2408 fix. The gate correctly reports it as stale:Stale allowlist entries: 1. Worth cleaning up in a follow-up but orthogonal to this PR's scope.Cascade impact
H7 delivered. Unblocks Phase 2 (#2441 composite gate depends on H7/H8/H9 landing). H8 (#2436) and H9 (#2437) remain parallelizable.
Test plan
pnpm checkpassesCloses #2435.