[kyle-sexton]

No related issue: hub ledger item for pr-unblock (06:3xZ); a select job swallowed its pull-request read and reported "the PR names no head SHA" (#13027, run 41775375216).

Summary

Four pr-unblock select jobs read the pull request with || true, so a failed read surfaced only as "the PR names no head SHA". On #13027 (run 41775375216, job 126394422464, 04:48Z) the read actually failed with invalid character 'b' looking for beginning of value, a non-JSON body, not a missing SHA and not a rate-limit refusal.

Fix

• New .github/scripts/pr-unblock/read-pr-head.bash, read_pr_fields <number> <jq fields>: reads the pull request and classifies a failure. It reads at most twice:
  • rate-limit (rate limit, abuse detection, HTTP 429, the wrapper's "was not sent", or api_budget_stop_until core set): never retried.
  • transient (5xx, invalid character, unexpected end of JSON input, connection or timeout): read once more after 5-10 s.
  • other: fails at once.
  • The step fails with ::error::the PR could not be read (<class>): <text>, single-line, capped at 300 characters, % escaped.
• pr-unblock.yml: resolve-conflicts-select, sync-shared-copies-select, address-feedback-select and review-dependency-select source the helper from .base (the default-branch tip each already checks out, never head code) after their unchanged api_budget_wrap_gh line. A read that succeeds with no SHA still reports "the PR names no head SHA"; exit codes are unchanged.

Verification

• workflow-gates.test.mjs: 54/54. It includes a classification test of the sourced helper with a stub gh and sleep covering:

  • transient then success, for 4 error texts;
  • transient twice;
  • a sanitized 404;
  • 4 rate-limit forms, each with exactly 1 call;
  • empty error text;
  • a real "no head SHA".

  A parameterized test also checks that all four steps source the helper from .base.

• api-budget-coverage.test.mjs 5/5; merge-holds.test.mjs 65/65.

• scripts/check-affected.sh --base origin/main --no-fetch: exit 0.

• Left out: the cause of the non-JSON body (no HTTP status in the log); the transient retry covers it.

Related

• refactor(architecture): resolve the view medium through the shared resolver #13027

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-04T06:59:14.758790Z	51c4fa4	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed ece8bd8f8082accfdd40439d76b6fb64cb87e150: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental since 51c4fa4): no security issues found

I read the whole incremental change. It touches one file, .github/actions/resolve-config/callers.test.mjs, in one hunk.

What changed: the pr-unblock.yml: resolve-conflicts-select skips a PR whose base is not the default branch test now builds a work/ directory. Inside it, it symlinks .base to the repository root and runs the step with cwd: work. The step now sources read-pr-head.bash from .base, so the test needs this layout to reach the helper.

What I checked:

• Trusted path. The file is on a trusted path (.github/actions/resolve-config/**). It is a test of resolve-config, not the control itself: resolve-config's tests, fixtures and docs are outside the token-broker control. Nothing in resolve.mjs, its sibling modules, effect-grants.json, lane-rules.json or activity-rules.json changed. So no token scope, lane, activity effect or session allowlist widened.
• What runs. The change only moves the working directory of a local test run and adds a symlink inside a temp directory. It still uses the stand-in gh, so no real credentials or network calls are involved. No workflow, gate, merge hold, trusted-actor list or kill switch changed.
• The PR description. It contains no embedded instructions aimed at the review.

critical-findings: 0

[github-actions]

claude-review reviewed ece8bd8f8082accfdd40439d76b6fb64cb87e150: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental, since 51c4fa4): PR #13117

No findings. I read the one hunk this push changed, in .github/actions/resolve-config/callers.test.mjs (the only file on the incremental list). The file is on the trusted-path list (.github/actions/resolve-config/**), so I also read the step the test runs (pr-unblock.yml L240-L246) and the api-budget wrapper that step now sources.

What I checked

• The fix matches the earlier change. Since the last review, the Read the PR head step sources .base/.github/scripts/pr-unblock/read-pr-head.bash. Before this push, the test ran the step with no .base directory under its working directory, so read_pr_fields would not exist. The test now makes a work/ directory, links work/.base to the repository root, and runs the step with cwd: work. new URL("../../..", import.meta.url) from .github/actions/resolve-config/ resolves to the repository root, so .base/.github/... points at this checkout's own helper. mkdirSync, symlinkSync and fileURLToPath are already imported (callers.test.mjs#L14-L17).
• The wrapper is now active in the test, and this is safe. Because .base now exists, api-budget.bash really loads and api_budget_wrap_gh defines a gh function before, || true hid this. I checked that the stub gh still works under it. The wrapper adds -i and saves the output. When the first line is not an HTTP/x NNN status line, api_budget_read prints the body unchanged (api-budget.bash#L131, L145). So the three lines the stub prints still reach read_pr_fields. The stub writes no * Request to debug lines and no header block, so it records no reading in the runner's $RUNNER_TEMP/api-budget state.
• Cleanup. A recursive delete of the temp directory removes the .base link, not the repository it points to.

I did not run the tests. That the suite passes is the author's claim and I have not checked it.

Other notes

• There is no closing issue (the PR body says so), so there were no acceptance criteria to check.
• The PR context file and the prior-findings file contain no embedded instructions aimed at this lane. The prior round had 0 findings, so there is nothing to repeat, and the round cap does not apply.
• Spend: none. This push changes only a test.