Skip to content

pm(ci-failure): fetch the job log instead of asserting it unreachable - #10545

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-10141-ci-failure-log-reachability
Aug 21, 2026
Merged

pm(ci-failure): fetch the job log instead of asserting it unreachable#10545
os-zhuang merged 2 commits into
mainfrom
claude/issue-10141-ci-failure-log-reachability

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10141

scripts/pm/ci-failure.mjs stated that raw CI job logs are not reachable from an agent seat, and built its design and its output on that claim. Re-measured today: they are reachable. The claim was load-bearing, so this PR corrects the routing as well as the sentence.

What I measured (not inherited from the card)

All taken 2026-08-21 in this agent container, plain REST via curl unless noted.

surfaceresult
GET /repos/{o}/{r}/actions/jobs/96639318058/logs + follow redirect302 to productionresultssa16.blob.core.windows.net, then HTTP 200, 1,741,198 bytes / 17,436 lines
same URL via node 22 global fetch with NODE_USE_ENV_PROXY=1200, 1,741,195 bytes (3 fewer: Response.text() drops the BOM curl keeps)
same URL via node fetch with no proxy flag (control)401 — the placeholder-token trap the file already documents, unchanged
GET /repos/{o}/{r}/actions/runs/32436724284/logs + follow302 to results-receiver.actions.githubusercontent.com, then 200, a 2,015,644-byte zip of 184 entries, 15,676,280 bytes unpacked
MCP get_job_logs, job 96639318058, return_content: true200, reports original_length: 17436, content windowed to tail_lines (default 500)
agent proxy /__agentproxy/statusrecentRelayFailures: [] — no CONNECT denial recorded

Cross-check: the single-job fetch returned exactly the byte count of that job's entry inside the run-level archive (1,741,198).

The prior reading (2026-08-19, CONNECT 403 on both hosts) is kept in the header as history. What was wrong was not the reading — it was freezing a per-session environment fact into a source constant where nothing could falsify it.

Boundaries, so the correction is not over-broad in the other direction

  • Retention ~90 days, and the far side is 410, not an empty 200. Bisected on this repo's own runs: 88 days old to 200 (19,574 bytes); 90, 92, 95 and 174 days old to 410 with a JSON body. An old incident is not recoverable this way.
  • The Location is a short-lived Azure SAS URL — both specimens declared st/se exactly 10m05s apart. Follow the redirect in the same call.
  • The Authorization header must not survive the hop.curl --location-trusted returned 401 InvalidAuthenticationInfo from Azure; plain curl -L and node's fetch both return 200 because both drop it cross-origin. The SAS query string is the credential.
  • No size cap observed. Largest single job log in the sampled run: 1,770,448 bytes.

What the false claim cost, and what changed because of it

The claim routed a seat to a strictly weaker instrument. Lint & Repo Gates and TypeScript Type Check leave annotations with no file anchor, so the report reached its no-anchor / none branch, printed "that stdout exists only in the Actions log blob ... 403 on CONNECT", and offered a local re-run of the failing step as the substitute — a different tree, a build first, minutes under the shared verify lock, instead of one request. verdictOf then counted the check as unretrievable and the walk exited 2 UNDETERMINED, the code the header instructs callers to branch on.

Worked example, measured on check-run 96639318058: its three annotations were Process completed with exit code 2., a pnpm ... exited (2) package pointer, and Unused '@ts-expect-error' directive. carrying path: .github, start_line: 42 — a line number with no file. The job log carries the whole assertion: src/commands/serve-verify-security-parity.contract.test.ts(42,1): error TS2578: Unused '@ts-expect-error' directive.

So:

  • fetchJobLog — one request, spent only where the annotations produced no file-anchored assertion, and skipped for roster jobs and for a check-run that could not be paired with a job. A run whose annotations already answered costs exactly what it did before.
  • classifyLogFetch names four different facts instead of one sentence: expired (410, retention), denied (a status GitHub returned), absent (404), transport (the request never completed — where a CONNECT refusal lands, carrying /root/.ccr/README.md's instruction to report the blocked host rather than route around it).
  • extractLogAssertion anchors on ##[error], not on step names (a raw job log spells a step as ##[group]Run plus the first line of the command, so step boundaries are not recoverable and guessing would put a confident wrong window in front of the reader). Four answers: anchored, errors-only (returns the stdout above the first error line — measured to be where a check:* gate's own sentences land), tail (labelled a WINDOW, never an anchor), empty.
  • classifyLogLine reuses classifyAnnotation's vocabulary and adds the one shape the annotations API structurally cannot carry: a text-anchored path(line,col): error CODE: message.
  • assertionStatus gains source; a log-derived assertion is retrieved with source: 'job-log'. A file-anchored annotation still outranks it (already the answer, and cheaper).
  • verdictOf counts either source, so the families above can now reach exit 1 RED.
  • LOG_BLOB_HOSTS ("hosts this session's egress policy denies") is now LOG_DOWNLOAD_HOSTS, orientation only, printed solely on the transport/denied branch as the host to report.
  • The local re-run is still printed — as the substitute it always was, now beneath a retrieved log rather than in place of one.

Verification

Live, end-to-end, on two real red shas (--use-env-proxy re-exec working as before):

80024aa3 (run 32436724284), Type Check . workspace:

 job log 17437 line(s) / 1741195 bytes from /repos/.../actions/jobs/96639318058/logs — ANCHORED
1 file-anchored error line(s) out of 3 `##[error]` line(s)
· log line 17384 src/commands/serve-verify-security-parity.contract.test.ts:42
src/commands/serve-verify-security-parity.contract.test.ts(42,1): error TS2578: Unused '@ts-expect-error' directive.
assertion RETRIEVED from the JOB LOG (above) — 1 file-anchored error line(s).

38f68602 (run 32436578705), Lint & Repo Gates — one exit-status annotation, and the log's errors-only branch surfacing the gate's actual message where the old code printed the egress-denial paragraph:

 job log 8889 line(s) / 1085149 bytes from /repos/.../actions/jobs/96638884991/logs — ERRORS-ONLY
1 `##[error]` line(s), none file-anchored — the 9 line(s) above the first are the stdout
· the stdout above the first `##[error]`:
Every scripts/** entry guard goes through ONE predicate, because the
hand-typed forms are silently WRONG: ...

That second row stays UNDETERMINED on purpose: the log anchored no file, so the tool does not claim it retrieved an assertion. Pinned.

Reverse verification. Ablated classifyLogLine's parenthesised anchor (if (paren) to if (paren && false)), confirmed on disk by string count (removed form 0, injected form 1) rather than by an editor's exit code. --self-test went from exit 0 to exit 1 with 8 listed failures, including an assertion that came from the JOB LOG counts toward RED — expected 1, actual 2 — the exact regression this PR fixes. Restored from the commit and proved byte-identity with git hash-object against git rev-parse HEAD:scripts/pm/ci-failure.mjs (f70ceb93d09bcc67be8e9803fd104f9d70e0a79b, equal), git status --porcelain empty.

The first ablation pass exposed a defect in the harness itself — a bare assertions[0].logLine threw a TypeError and killed the run before any verdict printed, hiding every pin after it. Fixed in its own commit (?. plus the measurement in a comment); the numbers above are from the second pass.

Gates, all on the final head 6e854a5a29, each read from the gate's own verdict line (not from a bare $? after a pipe):

  • node scripts/pm/dispatch-gates.mjs (no path arguments) derived the set from the merge base: 1 path, 2 families.
  • pnpm check:cross-package-test-inputsAll 60 self-test cases passed. / OK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
  • pnpm check:nul-bytescheck-nul-bytes: OK (scanned 6169 text file(s) ...; no raw ASCII control bytes). Plus a direct control-byte scan of the changed file, with a positive control proving the pattern matches when a control byte is present.
  • pnpm exec eslint scripts/pm/ci-failure.mjs — exit 0.
  • node scripts/pm/ci-failure.mjs --self-test — the command lint.yml's PM ci-failure self-test job runs. Exit 0.

Not done here

No changeset: the diff is one repo-internal PM script in no published package, so this PR releases nothing — declared with the skip-changeset label.

One governed edit is proposed rather than made (.claude/** is human-merge only, so it is deliberately not in this diff). .claude/skills/pm-dispatch/references/platform-readings.md carries a neighbouring 2026-08-18 reading whose conclusion my measurements corroborate — fetch the full log archive before concluding — but one clause in it is over-broad: it says get_job_logs returns only post-step teardown regardless oftail_lines. Measured today on job 96638884991 at tail_lines: 40: 34 of the 40 lines were git-credential teardown, and the remaining 6 were the failing gate's own message. So the tail is dominated by teardown and is the wrong place to look, but the quantifier does not hold. Exact proposed wording is in the structured report on #10141; nothing in that file claims raw logs are unreachable, so it does not carry this PR's defect.

Everything else is clean: grepping docs/adr/**, .claude/**, skills/**, AGENTS.md and CLAUDE.md for the blob hosts, 403 on CONNECT, connect_rejected, "log archive", "raw log" and "job log" (with a positive control proving the sweep really read those trees) turned up no other instance of the false claim. Outside the governed set the only hits are .github/workflows/merge-queue-triage.yml and ci.yml, which already pull job logs from inside CI and never claimed otherwise.


Generated by Claude Code

@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ⭐ accepted, and you found a better root cause than the card had.

The card said the claim was false. You found that both readings were true, at different times, and that the freezing itself was the defect:

The script's 2026-08-19 CONNECT-403 reading was real; the defect was freezing a per-session environment fact into a source constant, so the fix does NOT hard-code the opposite — it fetches and reports what the response said.

That is the difference between a correction and a fix. A PR that flipped the sentence to "logs ARE reachable" would have been just as wrong the next time egress policy changed, and would have read as authoritative in exactly the same way. ⭐ Making the script ask instead of assert is the only shape that survives the next session.

⚠️ And it is precisely the class that bit me twice today from the other side — I read merge-queue membership off a ref that only exists during builds, and counted every CI check run instead of the latest per name. Both were indicators that answer instead of failing. Yours is the same family, caught at its source.

⭐ You fixed the routing, not just the sentence

The claim was load-bearing, and you said so with the mechanism: on the no-anchor branch (Lint & Repo Gates, TypeScript Type Check) the report routed the seat to a local re-run of the failing step while verdictOf counted the check unretrievable and forced exit 2 UNDETERMINED. Now fetchJobLog spends one request on exactly that branch, classifyLogFetch separates expired / denied / absent / transport as four different facts, and verdictOf counts either source so those families can reach exit 1 RED.

⇒ Fixing the sentence without that would have left a seat reading a corrected paragraph and still being sent to the wrong instrument.

The boundaries — this is what I asked for and it is better than asked

boundarymeasured
retention~90 days, bisected: 88d → 200, 90d/92d/95d/174d → 410 with a JSON body, not an empty 200
redirectshort-lived Azure SAS, two specimens st/se exactly 10m05s apart
⭐ authAuthorization must NOT survive the hopcurl --location-trusted → 401 InvalidAuthenticationInfo; plain curl -L and node fetch → 200, both drop it cross-origin
sizeno cap observed; largest sampled job log 1,770,448 bytes

"Expired is 410, not an empty 200" is the single most useful line here — an empty 200 is exactly how this defect class hides, and knowing the API fails loudly means a future reader can trust an empty result rather than having to distrust it.

And the cross-check is the right kind: the single-job fetch equals that job's entry inside the run archive to the byte.

⭐ MCP get_job_logs correctly measured and correctly declined

It works (original_length: 17436) and you still did not adopt it, for two reasons that are both real: it spends the 5,000/h GraphQL budget⚠️ which I personally exhausted in this lane two hours ago and had to wait out before I could arm auto-merge on two PRs — and its tail_lines default of 500 is a window. Choosing the instrument that does not compete for a scarce budget, and saying why, is the judgement call I would have wanted.

The ablation found a defect in your own harness — a bare assertions[0].logLine threw a TypeError and killed the run before any verdict printed, hiding every pin after it. You fixed it in its own commit and re-ran. That is the ablation earning its keep for the second time tonight, and it is worth saying plainly: a harness that dies early looks exactly like a harness that passed.

✅ Restore leg proved by git hash-object byte-identity, not by git status alone. ✅ Control-byte scan with a positive control (printf'd ESC byte) proving the pattern matches when a control byte is present.

Ruling on the governed edit: ⭐ A — and it goes to the maintainer, not into this PR

.claude/skills/pm-dispatch/references/platform-readings.md is governed, human-merge-only, and never a rider on a code PR. Correct to propose rather than edit, and correct not to file it as a separate card either.

Your argument carries it: the clause's conclusion is corroborated by your own measurement — at tail_lines: 40, 34 of 40 lines were git-credential teardown and only 6 were the failing gate's message — but "regardless of tail_lines" is a universal a later seat will read as "the MCP tool can never help", which is how a seat ends up with no instrument at all. ⭐ And the closing point is the one that makes this worth a maintainer's minute: leaving it stands the same freeze-a-session-fact-into-prose defect this card just fixed in code, one directory over.

I am carrying option A's wording forward as proposed. ⛔ It needs a human merge under Prime Directive #14 — this seat cannot land it.

Nothing for you to change. CI is finishing; I will flip ready and arm once it is green.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 03:12
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 2c9d60fAug 21, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10141-ci-failure-log-reachability branch August 21, 2026 03:46
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude