Skip to content

Commit 9bdc384

Browse files
os-elon-muskclaude
andauthored
fix(scripts): route check-closing-target-claim through the proxy, and make a transport failure speak (#18998)
Fixes #18947 Clause-②: no `scripts/check-closing-target-claim.mjs` read GitHub through node's global `fetch` with no proxy route. In an agent container api.github.com is reachable only through the session proxy — which is also where the credential is injected — and `fetch` does not read the proxy variables, so every request arrived uncredentialled and GitHub answered 401. This gate folded that 401 into a per-target `UNDETERMINED` at **exit 0**: honest in prose, green in the one thing a derived-gate sweep records. Two commits, in the order the triage asks for: the route, then the negative control. ## Before / after / control — this container, transcripts Exit codes captured with redirect-then-`$?`; no token is printed by any leg. ⚠️ One substitution, declared: where a probe's `PR_BODY` bound **#18844**, the closing keyword is written `CLOSING` below. The real command used the ordinary spelling; it is masked here so that quoting a probe does not make this PR body bind a second card. The keyword next to #18947 is left literal — that is this PR's own target, already bound on line 1. **BEFORE** (`d8b12fca9`, the seat's own probe re-run at this base): ```text $ GITHUB_REPOSITORY=objectstack-ai/objectstack PR_NUMBER=18921 \ PR_HEAD_REF=claude/issue-18844-probe PR_BODY='CLOSING #18844' \ node scripts/check-closing-target-claim.mjs ::warning::UNDETERMINED - #18844 was not judged: its comment thread could not be read. ... ✓ check:closing-target-claim: PR #18921 closes no card this run could hold against a claim; ... 1 closing target(s) could not be judged - see the UNDETERMINED warning(s) above. exit=0 ``` **AFTER**, same invocation — the re-exec line, then a real per-target verdict: ```text ℹ️ re-exec with --use-env-proxy: HTTPS_PROXY is set (http://127.0.0.1:37101) and node's fetch does not read it. ✓ check:closing-target-claim: PR #18921 closes no card this run could hold against a claim; ... #18844 was not judged: the card is already closed, so the merge closes nothing. exit=0 ``` ⭐ The reading, reported and not steered: #18844 resolves to `TARGET_CLOSED`, not to a finding. The classifying read was really spent (it is the second read, taken only on the path about to go red), and it answered `state != open`. The declined-number policy this file already documents is what declines it — the merge closes nothing, so there is no second implementation to prevent. **AFTER, live acceptance pair** against the real board, which is the stronger evidence that the read reaches the thread at all — this card, this branch, and this card with a branch nobody claimed: ```text $ ... PR_HEAD_REF=claude/issue-18947-closing-target-claim-proxy-route PR_BODY='Closes #18947' ✓ check:closing-target-claim: PR #18921 closes #18947, and each carries a `Claim:` whose `Branch:` line names `claude/issue-18947-closing-target-claim-proxy-route`. exit=0 $ ... PR_HEAD_REF=claude/issue-18947-someone-else PR_BODY='Closes #18947' ::error::PR #18921 closes #18947 with no `Claim:` on the card naming `claude/issue-18947-someone-else` - `Closes #18947` - the governing `Claim:` on #18947 names `claude/issue-18947-closing-target-claim-proxy-route`, not `claude/issue-18947-someone-else`. exit=1 ``` **CONTROL A — the negative control the triage requires**: same command, `HTTPS_PROXY` / `https_proxy` unset, so the request really fails at the transport: ```text ❌ check:closing-target-claim: PREREQUISITE NOT MET - the board was not read. GitHub API 401 for /repos//issues/18844/comments?per_page=100&page=1 - ⛔ not a verdict. ⛔ NOTHING was judged. ... exit=3 ``` ⛔ No longer exit 0. 「修好之后,一次真实的传输失败必须能让门禁出声」 — it speaks. **CONTROL B — the network-error leg, live**: `HTTPS_PROXY=http://127.0.0.1:1` (a dead port). This one also proves the hand-off is invisible in the exit codes: the parent re-execs, the child refuses, and 3 comes back verbatim. ```text ℹ️ re-exec with --use-env-proxy: HTTPS_PROXY is set (http://127.0.0.1:1) ... ❌ check:closing-target-claim: PREREQUISITE NOT MET - the board was not read. GitHub API unreachable for /repos/.../issues/18844/comments?...: ECONNREFUSED - ⛔ not a verdict. exit=3 ``` ## The boundary between transport and truncation, and the line that draws it The card's own docblock said a transport failure is 「declared UNDETERMINED rather than an unhandled rejection」. That sentence is what this card turns around, for the whole-board case only. Where the catch sat: three sites inside `collect` — `apiOrNull` (the queue leg's PR read and the classifying issue read) and the `try` around the comment-thread walk. All three swallowed everything. The line that draws the new boundary is a single predicate, read at all three sites: ```js export function isBoardNotRead(error) { if (!(error instanceof GitHubApiError)) return false; if (error.networkError) return true; return error.status !== 404; } ``` - **Typed, never a message match.** `githubApi` now throws `GitHubApiError` carrying the HTTP status, or `networkError: true` when `fetch` itself rejected. A plain `Error` from anywhere else — a fake transport in the self-test, a bug — is **not** a transport failure and keeps its old per-target reading, so no untyped throw can promote itself to a whole-board refusal. Every pre-existing self-test case that threw `new Error('boom')` is green unchanged. - **404 stays per-target.** It is the one status carrying information about the resource the path *names*: that number does not resolve for this token. So does a thread walked past `MAX_COMMENT_PAGES`. Both remain `exit 0` + `::warning::UNDETERMINED` — 「never clean, never an accusation」 — and both are pinned. - **Everything else is about the run.** 401/403 (credential or permission), 429 (rate limit), 5xx (GitHub), or no connection at all. Those propagate **out of** `collect`, so nothing at all is judged, and the dispatch turns them into `boardNotReadRefusal(error)` — pure, `{ exit, lines }`, driven offline by the self-test. - **429 is a declared superset** of the set named on the card (401/403/5xx/network). It is the same class — the board was not read — and routing it to UNDETERMINED would be exactly today's silence under a different number. Called out here rather than folded in silently. The card's claim comment also named 404 in a parenthetical; the ruling section names 401/403/5xx/network *and* explicitly keeps 「a target whose number cannot be read」 as UNDETERMINED, so 404 is per-target. That tension is resolved in the ruling's direction, and stated rather than discovered. - **⛔ No `::error::` annotation on the refusal path**, matching NOT MEASURED: the exit code is what makes the job red, and an annotation would name an author who caused none of it. - **Only a transport failure is refused.** The dispatch re-checks `isBoardNotRead` and re-throws anything else, so a genuine bug in this gate stays an unhandled rejection where it is visible as one, instead of being filed under "GitHub was unreachable". - **Residual, stated in the header rather than discovered later**: a token that can see nothing in this repository gets 404 on every path, so every target reads UNDETERMINED at exit 0. That is the same shape as a board of numbers that do not resolve and no status distinguishes them. What this gate *can* tell apart is what it does tell apart. ## The route The shape `main` carries today, not PR #18980's unlanded `guard` parameter: the shared `proxyRearmPlan` is imported, `proxyPlanEnv()` folds this file's own guard name (`OS_CLOSING_TARGET_CLAIM_PROXY_REARMED`) onto the shared name, and `rearmThroughProxy()` is called **once**, only by a wired run about to read the API — never on `--self-test`, never on a NOT WIRED run. A sibling instrument's guard must **not** suppress this one's, which is pinned. On an Actions runner there is no proxy in the environment, the plan never re-arms, and nothing about this gate changes there. ## Exit register — before / after | code | before | after | |---|---|---| | 0 | judged, clean | unchanged | | 1 | judged, finding | unchanged | | 2 | NOT MEASURED — no usable context; "a usage, wiring **or transport** failure" | NOT MEASURED — no usable context; "a usage or wiring failure" | | 3 | — | **PREREQUISITE NOT MET** — a request failed at the transport, the board was not read, nothing judged | 3 is `EXIT_PREREQUISITE_NOT_MET`, imported from the half-state module exactly as `check-issue-citations.mjs` spells it, ⛔ not re-declared here. **The CI consumer needs no change — measured, not assumed.** `.github/workflows/closing-target-claim-guard.yml` invokes the script with a bare `run: node scripts/check-closing-target-claim.mjs`: no `continue-on-error`, no `|| true`, no `if:` on the step. A non-zero exit fails the step and reds the job, so exit 3 is a red job with no workflow edit. That exit is also unreachable on a runner for a routing reason: there is no proxy there, and the token is the job's own. ## Self-test | | before | after | |---|---|---| | cases | 80 | 105 | | batteries | 10 | 12 | | `SELF_TEST_BATTERY_FLOOR` | 10 | 12 | The floor rose; no existing battery's pin moved. Two batteries added — the routing decision (9 cases, offline, pinned exactly as the landed mapper pins it: set ⇒ route, absent ⇒ none, flag present ⇒ none, own guard set ⇒ none, the patrol's guard set ⇒ still routes, plus the two structural pins that the plan is imported and the hand-off is called once *before* the first network read) and the transport boundary (16 cases: each refused status, the 404 and untyped-throw exclusions, the fleet phrase, the quoted error, `⛔ not a verdict`, exit 3 being none of 0/1/2, no `::error::`, no `✓`, the route named when a proxy is configured, and both UNDETERMINED legs still at exit 0). ## Reader test > A derived-gate run in a container reads a real per-target verdict from this gate; with > no proxy it reads PREREQUISITE NOT MET and exit 3, never a ✓ with a warning. Held, by CONTROL A above and by the live acceptance pair. ## Changeset `skip-changeset`, measured rather than assumed. Every published package in this repo declares `files[]`, and every entry resolves inside that package's own directory (`dist`, `README.md`, `CHANGELOG.md`; `@objectstack/spec` adds generated dirs and `src/**/*.zod.ts`). `scripts/check-closing-target-claim.mjs` is at the repo root, under no `package.json` at all (there is none anywhere under `scripts/`), so it is inside no publishable package directory and no `files[]` entry can name it. The root manifest is `private: true`. Positive control: a file that *is* published — `packages/spec/src/data/object.zod.ts` — sits inside a published package directory and is matched by that package's `src/**/*.zod.ts` entry. Nothing published moves. ## Gates Derived from this worktree with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` — 29 commands, the derivation's own first line naming this repo and commit. Every one was run, exit code captured before any pipe, and the record reconciled back with `--ran` in the `COMMAND :: exit CODE` form the reconciler asks for: ```text ✓ dispatch-gates --ran: 29 derived famil(ies) accounted for — 29 run, 0 NOT-MEASURED (a DERIVED zero — all 29 recorded an exit code and none of them is 3). ``` **All 29 exit 0**, including this gate's own `pnpm check:closing-target-claim` (105 cases) and `pnpm check:pm-dispatch-gates` (1849 cases, 756.6s on this box, run detached behind an exit-capturing wrapper and waited on with `tail --pid`). `origin/main` moved during the run (`f347c793e`); the derivation was re-run after a fetch and the 29-command set is byte-identical. **Repo-wide lint, not a narrowing.** `eslint . --no-inline-config` was run in full at `c1e174eba`, after the final commit: **6858 files linted** (eslint's own count from `--format json`), **0 files with problems**, exit 0. So no narrowing needs declaring here. ## Local verification scope - (1) No package is touched, so the dependency closure is empty and there is no build leg. - (2) The changed file is a repo-root CI instrument in no package; its own test surface is its `--self-test`, run above. - (5) The diff edits a gate script, so that script's own suite is owed on top of the derived families — here the two are the same thing: `pnpm check:closing-target-claim` *is* `node scripts/check-closing-target-claim.mjs --self-test`, and it is in the derived list. - `pnpm lint` is a repo-scan CI owns; it fitted in the foreground here and was run whole (above), so there is nothing narrowed to declare. ## Files changed - `scripts/check-closing-target-claim.mjs` (only) --- _Generated by [Claude Code](https://claude.ai/code/session_01BTeBejoPUvRHN8WdAJC6oF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d4cb05c commit 9bdc384

1 file changed

Lines changed: 336 additions & 20 deletions

File tree

0 commit comments

Comments
 (0)