Skip to content

Commit d29fdcf

Browse files
baozhoutaoclaude
andauthored
fix(devx): route missing GITHUB_REPOSITORY/GITHUB_TOKEN to EXIT_NOT_WIRED (#17362)
check:single-claim-paths only guarded PR_NUMBER, so a run with PR_NUMBER set but GITHUB_REPOSITORY unset assembled a request against an empty repo slug, threw an unhandled rejection, and exited 1 — this gate's FINDING code — instead of the exit-2 NOT WIRED path it already documents and reserves for exactly this. GITHUB_TOKEN is guarded the same way, since every real run needs it to read a PR's file list. readPrContext now returns a { wired: false, missing, number } shape for this half-wired case, and judge() routes it through the same NOT WIRED verdict text as the fully-unset case (still saying "judged nothing", never printing a clean-board mark). GITHUB_REPOSITORY/GITHUB_TOKEN are checked by truthiness rather than PR_NUMBER's Object.hasOwn presence check — the docblock says why the convention differs: both values are consumed directly to build the request, so an empty string reproduces the exact defect a presence check would wave through. Adds a --self-test battery for the missing-slug and missing-token inputs, including an assertion pinning the exit code as the literal number 2, and a reverse-control case proving all three variables present still resolves to a full, usable context. Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU Co-authored-by: Claude <noreply@anthropic.com>
1 parent b11bfb9 commit d29fdcf

1 file changed

Lines changed: 110 additions & 26 deletions

File tree

scripts/check-single-claim-paths.mjs

Lines changed: 110 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,10 @@
119119
*
120120
* 0 judged, clean.
121121
* 1 judged, an earlier open PR already claims a listed path.
122-
* 2 NOT WIRED — no PR context. A usage/wiring failure, never a verdict
123-
* about any PR, and never a statement that the board is clean.
122+
* 2 NOT WIRED — no PR context, or an INCOMPLETE one (`PR_NUMBER` present
123+
* but `GITHUB_REPOSITORY` or `GITHUB_TOKEN` missing, #16329). A
124+
* usage/wiring failure, never a verdict about any PR, and never a
125+
* statement that the board is clean.
124126
*
125127
* A gate that cannot read its input has verified nothing, and exiting 0 there
126128
* reads as "no violations" — the anti-pattern this repo keeps paying for. The
@@ -178,7 +180,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
178180
'First come, first served. Both arms, because failing the wrong one is': 11,
179181
'The failure has to carry the remedy, not just the verdict.': 3,
180182
'UNDETERMINED is its own answer. It must never read as clean, and it': 5,
181-
'Wiring absent: never clean, never an accusation.': 5,
183+
'Wiring absent: never clean, never an accusation.': 16,
182184
'The short-circuit. This is the property that makes the gate affordable,': 22,
183185
});
184186

@@ -298,20 +300,47 @@ export const SINGLE_CLAIM_PATHS = [
298300
export const declaredPaths = () => SINGLE_CLAIM_PATHS.map((entry) => entry.path);
299301

300302
/**
301-
* The PR context, or null when this process was handed none.
302-
*
303-
* Presence, not truthiness, for the same reason the sibling PR-scoped guard
304-
* uses it: the witness that the workflow really ran this step is the variable
305-
* existing, not it being non-empty.
303+
* The PR context: `null` (nothing wired at all), `{ wired: false, missing,
304+
* number }` (PR_NUMBER present but the run is still unusable), or the full
305+
* context. Three-way, not two-way, because "no PR context at all" and
306+
* "PR_NUMBER is set but GITHUB_REPOSITORY or GITHUB_TOKEN is not" are
307+
* different failures with different remedies, yet `judge` (below) must route
308+
* BOTH to the same EXIT_NOT_WIRED path — never the FINDING code, and never an
309+
* unhandled rejection from a request built with an empty slug (#16329).
310+
*
311+
* `PR_NUMBER`: `Object.hasOwn`, presence not truthiness, unchanged from
312+
* before — the reasoning still holds. Its VALUE is never read to build a
313+
* request; it only WITNESSES that the workflow ran this step, so an
314+
* accidentally-empty string would still be a real witness.
315+
*
316+
* `GITHUB_REPOSITORY` and `GITHUB_TOKEN`: truthiness (trimmed, non-empty),
317+
* deliberately a DIFFERENT convention from `PR_NUMBER`, spelled out here so
318+
* the split reads as a decision and not a silent drift from that rule. Both
319+
* values are consumed directly to build the request — the repo slug goes
320+
* straight into the URL path, the token straight into the Authorization
321+
* header (`githubApi`, below) — so for them an empty string is not a
322+
* DIFFERENT failure from an absent variable, it is the SAME failure: the
323+
* exact `/repos//pulls/…` URL from #16329 is built from a `GITHUB_REPOSITORY`
324+
* that a presence check would call "wired" the moment it is merely set to
325+
* `''` rather than left unset. Presence there would wave the defect straight
326+
* back through. And the token is "genuinely required" here unconditionally,
327+
* not just in some invocations: every real run of this script reads a PR's
328+
* file list over the network, and the wiring workflow always supplies a
329+
* token for that (pinned below: "the wiring passes a token, without which no
330+
* file list can be read").
306331
*/
307332
export function readPrContext(env) {
308333
const wired = Object.hasOwn(env, 'PR_NUMBER');
309334
if (!wired) return null;
310-
return {
311-
number: String(env.PR_NUMBER ?? '').trim(),
312-
repo: String(env.GITHUB_REPOSITORY ?? '').trim(),
313-
token: String(env.GITHUB_TOKEN ?? '').trim(),
314-
};
335+
336+
const number = String(env.PR_NUMBER ?? '').trim();
337+
const repo = String(env.GITHUB_REPOSITORY ?? '').trim();
338+
if (!repo) return { wired: false, missing: 'GITHUB_REPOSITORY', number };
339+
340+
const token = String(env.GITHUB_TOKEN ?? '').trim();
341+
if (!token) return { wired: false, missing: 'GITHUB_TOKEN', number };
342+
343+
return { number, repo, token };
315344
}
316345

317346
/**
@@ -322,19 +351,40 @@ export function readPrContext(env) {
322351
* `undetermined` carries the ones whose file list could not be walked to the
323352
* end, so the two can never be confused with each other or with a clean board.
324353
*/
354+
/**
355+
* The NOT WIRED verdict, shared by "nothing at all was handed to this run"
356+
* and "PR_NUMBER was handed but the run is still unusable" (#16329) — both
357+
* routes exit the same EXIT_NOT_WIRED code and say, in the reader's own
358+
* words, that NOTHING WAS MEASURED: neither green nor red, no accusation,
359+
* and never the FINDING exit code.
360+
*/
361+
function notWiredVerdict(reasonText) {
362+
return {
363+
exit: EXIT_NOT_WIRED,
364+
lines: [
365+
`check:single-claim-paths: NOT WIRED — ${reasonText}, so this run was handed no usable pull`,
366+
'request context and judged nothing. This is a wiring or usage failure, NOT a verdict: it says',
367+
'nothing about whether any PR claims a single-claim path, and no author caused it.',
368+
'',
369+
`Fix: run it from the workflow that supplies the context (${WIRING_WORKFLOW}), or locally with`,
370+
' PR_NUMBER=123 GITHUB_REPOSITORY=owner/repo GITHUB_TOKEN=... node scripts/check-single-claim-paths.mjs',
371+
],
372+
};
373+
}
374+
375+
const NOT_WIRED_REASON = Object.freeze({
376+
PR_NUMBER: 'PR_NUMBER is not set',
377+
GITHUB_REPOSITORY: 'GITHUB_REPOSITORY is not set (or set to an empty string)',
378+
GITHUB_TOKEN: 'GITHUB_TOKEN is not set (or set to an empty string)',
379+
});
380+
325381
export function judge(ctx) {
326382
if (ctx === null) {
327-
return {
328-
exit: EXIT_NOT_WIRED,
329-
lines: [
330-
'check:single-claim-paths: NOT WIRED — PR_NUMBER is not set, so this run was handed no pull',
331-
'request and judged nothing. This is a wiring or usage failure, NOT a verdict: it says nothing',
332-
'about whether any PR claims a single-claim path, and no author caused it.',
333-
'',
334-
`Fix: run it from the workflow that supplies the context (${WIRING_WORKFLOW}), or locally with`,
335-
' PR_NUMBER=123 GITHUB_REPOSITORY=owner/repo GITHUB_TOKEN=... node scripts/check-single-claim-paths.mjs',
336-
],
337-
};
383+
return notWiredVerdict(NOT_WIRED_REASON.PR_NUMBER);
384+
}
385+
386+
if (ctx.wired === false) {
387+
return notWiredVerdict(NOT_WIRED_REASON[ctx.missing]);
338388
}
339389

340390
const where = ctx.number ? `PR #${ctx.number}` : 'this PR';
@@ -589,9 +639,43 @@ function selfTest() {
589639
t('no PR context at all exits NOT WIRED', unwired.exit, EXIT_NOT_WIRED);
590640
t('NOT WIRED says it judged nothing', unwired.lines.join('\n').includes('judged nothing'), true);
591641
t('NOT WIRED does not read as a clean board', unwired.lines.join('\n').includes('✓'), false);
592-
t('a present PR number is wired', readPrContext({ PR_NUMBER: '42' })?.number, '42');
642+
t(
643+
'a fully wired environment (all three variables) is wired',
644+
readPrContext({ PR_NUMBER: '42', GITHUB_REPOSITORY: 'o/r', GITHUB_TOKEN: 't' })?.number,
645+
'42',
646+
);
593647
t('an unset environment is not wired', readPrContext({}), null);
594648

649+
// --- #16329: PR_NUMBER alone is not enough. GITHUB_REPOSITORY (and
650+
// GITHUB_TOKEN) missing must take the SAME NOT WIRED path — never the
651+
// FINDING exit code, and never a thrown request built from an empty slug.
652+
const missingRepo = readPrContext({ PR_NUMBER: '16326' });
653+
t('PR_NUMBER with no GITHUB_REPOSITORY is not fully wired', missingRepo?.wired, false);
654+
t('...and it is attributed to the right missing variable', missingRepo?.missing, 'GITHUB_REPOSITORY');
655+
const missingRepoVerdict = judge(missingRepo);
656+
t('a missing GITHUB_REPOSITORY exits NOT WIRED, never the FINDING code', missingRepoVerdict.exit, EXIT_NOT_WIRED);
657+
t('...and that exit code really is the number 2 (#16329, not just the named constant)', missingRepoVerdict.exit, 2);
658+
t('the missing-repo verdict says it judged nothing', missingRepoVerdict.lines.join('\n').includes('judged nothing'), true);
659+
t('the missing-repo verdict does not read as a clean board', missingRepoVerdict.lines.join('\n').includes('✓'), false);
660+
t('the missing-repo verdict names the missing variable', missingRepoVerdict.lines.join('\n').includes('GITHUB_REPOSITORY'), true);
661+
662+
// GITHUB_TOKEN is checked the same way and for the same reason: it is
663+
// consumed directly in the Authorization header, and the wiring always
664+
// supplies one (pinned further down: "the wiring passes a token").
665+
const missingToken = readPrContext({ PR_NUMBER: '16326', GITHUB_REPOSITORY: 'o/r' });
666+
t('PR_NUMBER + repo but no GITHUB_TOKEN is not fully wired', missingToken?.wired, false);
667+
t('...and it is attributed to GITHUB_TOKEN', missingToken?.missing, 'GITHUB_TOKEN');
668+
t('a missing GITHUB_TOKEN exits NOT WIRED too', judge(missingToken).exit, EXIT_NOT_WIRED);
669+
670+
// Reverse control: all three variables present is UNCHANGED by this fix —
671+
// it still resolves to a full, usable context, not the NOT WIRED shape.
672+
const fullyWired = readPrContext({ PR_NUMBER: '16326', GITHUB_REPOSITORY: 'o/r', GITHUB_TOKEN: 't' });
673+
t(
674+
'all three variables present is fully wired (reverse control)',
675+
fullyWired,
676+
{ number: '16326', repo: 'o/r', token: 't' },
677+
);
678+
595679
// --- The short-circuit. This is the property that makes the gate affordable,
596680
// and it is invisible in the verdict layer, so it is pinned here against a
597681
// recording fake API. Fixture paths name a tree that exists in no repo.
@@ -693,7 +777,7 @@ if (isMain) {
693777
}
694778
} else {
695779
const ctx = readPrContext(process.env);
696-
const resolved = ctx === null ? null : await collect(ctx, githubApi(ctx.token));
780+
const resolved = ctx === null || ctx.wired === false ? ctx : await collect(ctx, githubApi(ctx.token));
697781
const result = judge(resolved);
698782
const emit = result.exit === EXIT_CLEAN ? console.log : console.error;
699783
for (const line of result.lines) emit(line);

0 commit comments

Comments
 (0)