Skip to content

Commit f0fa6be

Browse files
os-zhuangclaude
andauthored
fix(pm): give ci-failure's transport probe the repo-scoped second stage (#10157)
* fix(pm): give ci-failure's transport probe the repo-scoped second stage Fixes #9966 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt * test(pm): make the fourth-class pins fail as assertions, not as a TypeError Found by the ablation itself: with stage 2 removed the 5xx case read `unwell.repo.status` off a null and crashed, so the harness never printed the class-4 failure it was there to show. A pin that crashes hides its siblings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent cc21aad commit f0fa6be

1 file changed

Lines changed: 191 additions & 11 deletions

File tree

‎scripts/pm/ci-failure.mjs‎

Lines changed: 191 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@
9494
* running, or zero check-runs on the sha. Zero is not a clean
9595
* repo, it is a broken scan.
9696
* 3 PREREQUISITE NOT MET — no usable transport (no token, exhausted quota,
97-
* unreachable api.github.com). Classified by
97+
* unreachable api.github.com, or an authenticated container
98+
* whose REPO-scoped reads are refused). Classified by
9899
* `check-half-states.mjs`'s probe, which is imported rather
99100
* than re-implemented: same numbers, same wording, one
100101
* instrument. Its header states the rule this file inherits —
@@ -128,6 +129,44 @@
128129
* instead of a crash loop. `--self-test` and `--help` never re-exec — they
129130
* open no socket.
130131
*
132+
* ## The transport, part two — an authenticated container that cannot read THIS
133+
* repo (#9966, the fourth container class)
134+
*
135+
* The probe above is `/rate_limit`, and on its own it CANNOT answer the question
136+
* this file asks. Measured here 2026-08-20, one container, seconds apart:
137+
*
138+
* GET /rate_limit -> 200, 14982 left, server: github.com
139+
* GET /user -> 200, the real login
140+
* GET /repos/objectstack-ai/objectstack -> 200 (this repo IS enabled here)
141+
* GET /repos/objectstack-ai/objectui -> 403, no server: github.com and no
142+
* x-ratelimit-* headers at all
143+
*
144+
* The refusal is per-REPOSITORY, not per-session, and the account-scoped reading
145+
* is genuinely healthy — byte-for-byte the healthy Routine runner's. So no care
146+
* applied to `/rate_limit` could ever classify this container, and a seat
147+
* pointing this file at a sibling repo (`PM_SWEEP_REPO=objectstack-ai/objectui`,
148+
* the cross-repo task CLAUDE.md describes) is a live specimen of the class.
149+
*
150+
* What that cost before the repo-scoped stage existed, measured on the same
151+
* container against the same sha — and it is WORSE than #9966 predicted. The
152+
* card expected the exit-3 reading to degrade into an UNDETERMINED or a raw HTTP
153+
* number. Measured, it degrades into an uncaught throw:
154+
*
155+
* PM_SWEEP_REPO=<a repo this session cannot read> ci-failure.mjs --sha <sha>
156+
* -> Error: GET /repos/.../check-runs -> HTTP 403 (stack trace, exit 1)
157+
*
158+
* Node exits 1 on an uncaught exception, and 1 in the table above is RED — "the
159+
* assertion text was retrieved for EVERY failing check, the output is the
160+
* answer". A caller branching on `$?`, which the table above tells it to do,
161+
* therefore read a transport refusal as a confident verdict about the TREE from
162+
* a container that had not read one byte of it. That is why the fix is a probe
163+
* STAGE and not a `catch` around the walk: exit 3 has to be reached before
164+
* anything is read, or the answer is only a politer wrong one.
165+
*
166+
* The remaining uncaught-throw path — a transport failure arriving MID-walk,
167+
* after the probe passed — is #10155, filed rather than fixed here: it needs an
168+
* exit-code decision (2 vs 3) that this card did not scope.
169+
*
131170
* ## History — this file reads none, so the shallow clone cannot mislead it
132171
*
133172
* Agent containers start from a 63-commit shallow clone (#9878), which answers
@@ -155,6 +194,7 @@ import {
155194
EXIT_PREREQUISITE_NOT_MET,
156195
classifyTransportProbe,
157196
describeProbe,
197+
needsRepoProbe,
158198
parseRemaining,
159199
} from './check-half-states.mjs';
160200
import { PROXY_FLAG, PROXY_REARM_GUARD, proxyRearmPlan } from './check-governed-merges.mjs';
@@ -590,14 +630,59 @@ async function probeRateLimit(token) {
590630
}
591631
}
592632

593-
async function probeTransport() {
594-
const authed = await probeRateLimit(TOKEN);
595-
const usable = !TOKEN || (authed.status === 200 && authed.rateLimitRemaining !== 0);
596-
const anon = usable ? (TOKEN ? null : authed) : await probeRateLimit('');
633+
/**
634+
* Stage 2 (#9966) — one repo-scoped GET, exercising the same authorization
635+
* decision that every read in the walk below needs.
636+
*
637+
* `GET /repos/{owner}/{repo}` and NOT `GET /user`: measured in this container,
638+
* `/user` answers 200 with the real login while a repo-scoped read of a repo
639+
* this session does not hold answers 403. An "is this a real endpoint" probe
640+
* green-lights the very class this stage exists to name — what has to be
641+
* exercised is the SCOPE, not the realness. One core request, and only on the
642+
* path that previously returned a green without having read anything
643+
* repo-scoped.
644+
*/
645+
async function probeRepoRead(token) {
646+
try {
647+
const res = await fetch(`${API}/repos/${OWNER_REPO}`, {
648+
headers: { accept: 'application/vnd.github+json', ...(token ? { authorization: `Bearer ${token}` } : {}) },
649+
});
650+
return { status: res.status, rateLimitRemaining: parseRemaining(res.headers.get('x-ratelimit-remaining')) };
651+
} catch (error) {
652+
return { networkError: error?.code ?? error?.message ?? 'unknown' };
653+
}
654+
}
655+
656+
/**
657+
* The two-stage probe. Stage 2 fires ONLY when stage 1 already said `reachable`
658+
* — exactly the path that used to green without repo-scoped evidence — so the
659+
* three failing classes short-circuit and cost precisely what they cost before.
660+
*
661+
* `needsRepoProbe` is IMPORTED for the same reason the classifier is: that
662+
* sequencing is one decision, and #9946 pinned it next to the verdicts it gates.
663+
* What stays local is the GATHERING POLICY, because it is genuinely NOT shared —
664+
* measured on both files as they stand, this one also spends an anonymous probe
665+
* when the quota reads 0, where `check-half-states.mjs` re-probes only on a
666+
* non-200. Hoisting the whole probe into one function would have to pick one of
667+
* those two and silently change the other file's request pattern.
668+
*
669+
* The two readers are injectable so `--self-test` can drive this against the
670+
* measured container classes while opening no socket, as the header promises.
671+
*/
672+
async function probeTransport({ token = TOKEN, rateLimit = probeRateLimit, repoRead = probeRepoRead } = {}) {
673+
const authed = await rateLimit(token);
674+
const usable = !token || (authed.status === 200 && authed.rateLimitRemaining !== 0);
675+
const anon = usable ? (token ? null : authed) : await rateLimit('');
597676
// The raw readings ride along: `classifyTransportProbe` returns null for a
598677
// shape it cannot name, and a caller that kept only the verdict would have
599678
// nothing to report about the container it just failed to classify.
600-
return { verdict: classifyTransportProbe({ token: TOKEN, authed, anon }), authed, anon };
679+
const account = classifyTransportProbe({ token, authed, anon });
680+
if (!needsRepoProbe(account)) return { verdict: account, authed, anon, repo: null };
681+
682+
// Re-classified with the repo reading ADDED, rather than patched on top of the
683+
// stage-1 verdict: one classifier, one place where a verdict is named.
684+
const repo = await repoRead(token);
685+
return { verdict: classifyTransportProbe({ token, authed, anon, repo }), authed, anon, repo };
601686
}
602687

603688
async function rest(path) {
@@ -846,7 +931,12 @@ function render(result, target) {
846931
// the real tree rather than against a fixture of it.
847932
// ---------------------------------------------------------------------------
848933

849-
function selfTest() {
934+
// Async because the fourth-class pin below drives the GATHERING, not only the
935+
// pure classifier — that is where this file's defect lived, and a pin that
936+
// exercised only `classifyTransportProbe` would restate #9946's self-test
937+
// instead of covering this file. It still opens no socket: the two readers are
938+
// injected.
939+
async function selfTest() {
850940
const failures = [];
851941
const t = (label, actual, expected = true) => {
852942
const ok = JSON.stringify(actual) === JSON.stringify(expected);
@@ -1177,6 +1267,90 @@ function selfTest() {
11771267
true,
11781268
);
11791269

1270+
// -- probeTransport: the two stages, and the FOURTH container class -------
1271+
// Measured 2026-08-20 in an agent container whose session holds THIS repo and
1272+
// no other: `/rate_limit` -> 200 with a real quota and `server: github.com`,
1273+
// `/user` -> 200 with the real login, `GET /repos/objectstack-ai/objectui` ->
1274+
// 403 with no `server: github.com` and no `x-ratelimit-*` headers at all.
1275+
const PLACEHOLDER = 'proxy00000abcd'; // the 14-char proxy placeholder, `unrecognized` shape
1276+
const healthyRate = { status: 200, rateLimitRemaining: 14982 };
1277+
const refusedRepo = { status: 403, rateLimitRemaining: null };
1278+
// Injected readers that record what was actually requested, so the SEQUENCING
1279+
// is pinned and not merely the verdict — no socket is opened.
1280+
const readers = (rate, repo) => {
1281+
const calls = [];
1282+
return {
1283+
calls,
1284+
rateLimit: async (token) => {
1285+
calls.push(token ? 'rate:token' : 'rate:anon');
1286+
return rate;
1287+
},
1288+
repoRead: async () => {
1289+
calls.push('repo');
1290+
return repo;
1291+
},
1292+
};
1293+
};
1294+
1295+
const class4 = readers(healthyRate, refusedRepo);
1296+
const class4Result = await probeTransport({ token: PLACEHOLDER, ...class4 });
1297+
t(
1298+
'#9966 class 4 (measured): a healthy /rate_limit plus a refused repo read is NOT reachable',
1299+
class4Result.verdict?.kind ?? null,
1300+
'repo-scope-refused',
1301+
);
1302+
t(
1303+
'...so the run exits 3 before the walk reads anything, instead of throwing on its first page',
1304+
class4Result.verdict?.kind !== 'reachable',
1305+
true,
1306+
);
1307+
t('...and the repo read really was the SECOND request, taken only after stage 1 said reachable',
1308+
class4.calls, ['rate:token', 'repo']);
1309+
// The regression pin proper: the same observations, classified the way this
1310+
// file did before the stage existed, still come back green. Remove the
1311+
// gathering above and this case is what the class-4 case decays into — which
1312+
// is what makes the fixture a pin rather than a restatement of the fix.
1313+
t(
1314+
'the defect itself: those SAME readings with no repo observation still classify as reachable',
1315+
classifyTransportProbe({ token: PLACEHOLDER, authed: healthyRate }).kind,
1316+
'reachable',
1317+
);
1318+
1319+
const healthy = readers(healthyRate, { status: 200, rateLimitRemaining: 14981 });
1320+
t(
1321+
'both stages passing is the healthy runner class, and it still greens',
1322+
(await probeTransport({ token: PLACEHOLDER, ...healthy })).verdict?.kind ?? null,
1323+
'reachable',
1324+
);
1325+
1326+
const badCred = readers({ status: 401, rateLimitRemaining: null }, healthyRate);
1327+
const badCredResult = await probeTransport({ token: PLACEHOLDER, ...badCred });
1328+
t('a failing stage 1 classifies exactly as it did before stage 2 existed', badCredResult.verdict?.kind ?? null, 'bad-credential');
1329+
t('...and short-circuits, so no failing class costs one request more than it used to',
1330+
badCred.calls, ['rate:token', 'rate:anon']);
1331+
1332+
const notVisible = readers(healthyRate, { status: 404, rateLimitRemaining: 14980 });
1333+
t(
1334+
'a repo-scoped 404 is repo-not-visible — a wrong PM_SWEEP_REPO, or a credential that cannot see it',
1335+
(await probeTransport({ token: PLACEHOLDER, ...notVisible })).verdict?.kind ?? null,
1336+
'repo-not-visible',
1337+
);
1338+
1339+
// A repo-scoped 5xx is left UNNAMED on purpose: the classifier's narrowness is
1340+
// the point, and the caller's loud generic failure beats a confident wrong
1341+
// diagnosis. What must never happen is that it falls back to `reachable`.
1342+
const unwell = await probeTransport({ token: PLACEHOLDER, ...readers(healthyRate, { status: 503, rateLimitRemaining: null }) });
1343+
t('a repo-scoped 5xx stays unclassified rather than being vouched for', unwell.verdict, null);
1344+
t('...and the stage-2 reading rides along, so the unclassified container can be reported', unwell.repo?.status ?? null, 503);
1345+
1346+
const anonOnly = readers(healthyRate, refusedRepo);
1347+
const anonResult = await probeTransport({ token: '', ...anonOnly });
1348+
t(
1349+
'with no token the anonymous reading is the primary one and stage 2 still fires',
1350+
[anonResult.verdict?.kind ?? null, anonOnly.calls],
1351+
['repo-scope-refused', ['rate:anon', 'repo']],
1352+
);
1353+
11801354
if (failures.length > 0) {
11811355
console.error(`✗ ci-failure --self-test (${failures.length} failure(s)):\n`);
11821356
for (const f of failures) console.error(` • ${f}`);
@@ -1193,7 +1367,10 @@ function selfTest() {
11931367
' HTTPS proxy re-arms itself with --use-env-proxy while the offline modes never do; --help\n' +
11941368
' still carries every documented invocation of this file; and the exit table holds — zero\n' +
11951369
' checks, still-running, and a red check whose assertion could not be retrieved all land on\n' +
1196-
' UNDETERMINED rather than on GREEN.',
1370+
' UNDETERMINED rather than on GREEN. The transport probe runs its two stages in order: a\n' +
1371+
' healthy /rate_limit is no longer enough to green a container whose repo-scoped reads are\n' +
1372+
' refused (the fourth class, measured), the repo read is spent ONLY after stage 1 says\n' +
1373+
' reachable, and a repo-scoped 5xx stays unclassified instead of falling back to reachable.',
11971374
);
11981375
}
11991376

@@ -1208,7 +1385,7 @@ if (!invokedDirectly) {
12081385
// as an import side effect would make this file impossible to reuse without
12091386
// also spending someone else's rate limit.
12101387
} else if (process.argv.includes('--self-test')) {
1211-
selfTest();
1388+
await selfTest();
12121389
} else if (process.argv.includes('--help') || process.argv.includes('-h')) {
12131390
console.log(usageText(readFileSync(fileURLToPath(import.meta.url), 'utf8')));
12141391
} else {
@@ -1235,12 +1412,15 @@ if (!invokedDirectly) {
12351412
console.error(' A 401/403 below is then a TRANSPORT reading, not a verdict about the credential.');
12361413
}
12371414

1238-
const { verdict: probe, authed, anon } = await probeTransport();
1415+
const { verdict: probe, authed, anon, repo } = await probeTransport();
12391416
if (probe === null) {
12401417
console.error('ci-failure: the transport probe returned a result its classifier does not recognise.');
12411418
console.error(` GET /rate_limit with the env token -> ${describeProbe(authed)}`);
12421419
console.error(` GET /rate_limit anonymously -> ${describeProbe(anon)}`);
1243-
console.error(' That is a gap in the classifier, not a verdict about the tree — report it with both readings.');
1420+
// Stage 2 has its own unclassified shape (a repo-scoped 5xx), and without
1421+
// this line the reader would see two healthy readings and no explanation.
1422+
if (repo) console.error(` GET /repos/${OWNER_REPO} (stage 2) -> ${describeProbe(repo)}`);
1423+
console.error(' That is a gap in the classifier, not a verdict about the tree — report it with every reading above.');
12441424
process.exit(EXIT_UNDETERMINED);
12451425
}
12461426
if (probe.kind !== 'reachable') {

0 commit comments

Comments
 (0)