[kyle-sexton]

No related issue: efficiency register E9-E14 (owner directive 2026-10-23)

Summary

ci-status in pr-require-checks.yml spent GITHUB_TOKEN requests reading the same data more than once per run. It read the base commit's .work-item-tracker.json and trusted-actor list through the contents API in up to four places, and on exempt-author runs it read the PR file list twice. What it accepts and rejects is unchanged.

Fix

• Item 6 (base config). A new step, Check out the base commit's hold config, does a sparse checkout of /.work-item-tracker.json and /.github/standards/trusted-actors/trusted-actors.json at pull_request.base.sha || merge_group.base_sha, with persist-credentials: false and continue-on-error: true. The cost-hold, human-gated, hold-lift and merge-group steps read the files from BASE_CONFIG.
  • If the checkout failed, BASE_CONFIG is empty and each step treats the config as unreadable, as it did on a failed API read: fail closed, or the cost hold stays.
  • If the checkout succeeded but a file is missing, that is the old 404: the tracker falls back to needs-human.
  • The step is listed in scripts/lane-coverage-step-opt-outs.txt with that reason.
  • The checkout makes no REST call (as of actions/checkout v7.0.1, the pinned 3d3c42e5). Its API call sites all return early for a 40-hex ref on a runner with git:
    • default branch: git-source-provider.ts:154
    • object format: github-api-helper.ts:137-139
    • stale-merge tracepoint: ref-helper.ts:226-235, which needs a refs/pull/ ref
    • archive download: only without git
• PR file list. The broker-pin step keeps the raw pulls/N/files pages in $RUNNER_TEMP/pr-files.json, and each consumer applies its own jq filter. The review-lane step's exempt-author branch reuses that file and falls back to its own read when the file is absent.
• Item 7 (hold-lift short-circuit): not done, because it would remove the gate.
  • The step runs only when steps.human_gated.outputs.held == 'false', so the PR never carries the label at that point. "Exit when no hold label" would therefore exit on every run.
  • The actor list was already read only after a hold was found.
  • Both remaining reads decide the no-hold verdict: a hold is the newer of a lane hold comment and a bot labeled event.
  • A new test pins that two-read floor.
  • Reusing cost_hold's events read was rejected: it happens before human_gated reads the labels, so a label flip in between could pass a run that should fail.
• pulls.get. No change in this PR's steps. A typical run reads it zero times there; it is read only on the cost-hold decision path or for exempt authors.

Calls per ci-status run, counted from the code

Scope: REST requests issued by ci-status's own steps, one page per --paginate read. The pinned check-contract and aggregate-results composites are excluded and unchanged. The savings apply to contract-only runs (label flips, body edits) too, since these steps run there.

run shape	before	after
typical non-draft PR (files 1, cost-hold 3→1, human-gated 2→1, hold-lift 2, review lanes 4)	12	9
draft PR (no review-lane step)	8	5
label on (human-gated fails, reads hold comments)	11	8
hold lifted (hold-lift reads the actor list)	13	9
exempt author (pulls.get 1, files 2→1)	10	6
merge_group (tracker 1→0, labels 1)	2	1

Main's #9399 (merged into this branch) added Fail a lanes App pull request its body or a member holds. That step does one pulls.get on every pull_request and merge_group run, plus three more reads on lanes App pull requests. It is identical before and after this PR, so the table leaves it out: for totals on current main, add 1 to every row. Sharing its pulls.get and file read with the steps here is a follow-up, not done here.

By item:

• Item 6: contents reads go from 3 to 0 on a typical PR run, 4 to 0 on a held one, and 1 to 0 on merge_group.
• Item 7: hold-lift stays at 2 reads.
• File list: 2 to 1 on exempt-author runs only.

Verification

• Every scripts/ci-*.test.sh passes: broker-pin 25, cost-decision 72, human-gated 38, lifted-by 33, merge-group 22, review-lanes 55, lanes-pr-hold 62, plus ordered-skips, queued-pr, rerun-ci-status and squash-header-lines. Every case for a retried or unreadable contents read became a fixture case: checkout failed, file missing, file invalid. Each run also asserts that no /contents/ read happens.
• New cases:
  • the checkout's ref, credentials, sparse paths and step order
  • a failed checkout fails closed in human-gated, merge-group and held hold-lift, and clears nothing in cost-hold
  • with no hold, the actor list is not needed
  • the no-hold path makes exactly the comments and events reads
  • the broker step leaves its pages, including two pages, for the review-lane step, which reuses them across pages, including a renamed trust-gate path and a short list
• Clean: actionlint (with shellcheck), shellcheck on the changed tests, zizmor offline, runner policy, check-jsonschema, check-docs-only-gate, check-lane-coverage, check-shell-portability, check-cant-fail-scan, check-test-tmp-cleanup. scripts/check-affected.sh --tests passed on the merged head.
• ci-test-node-stop-commands.test.sh needs PyYAML. With the pinned CI requirements installed it passes, inside check-affected --tests.
• The merge_group path does not run before merge. ci-hold-merge-group.test.sh is its only proof.
• Fresh-context code and security reviews of the diff found nothing to fix. Both advisory suggestions (a multi-page broker fixture, and the timeout-budget comment) are applied.

Related

• Rate-limit retry: a separate change makes ci-status wait for x-ratelimit-reset. This PR leaves the read_api helpers and the surviving retry loops untouched.

🤖 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-10-23T15:29:43.607769Z	c3a40c7	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 c0ad9bae8d709c8f2ccb0f644150a6da6adcb334: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

critical-findings: 0

What I reviewed. I read all of .claude-lane/pr.diff (8 files). It changes .github/workflows/pr-require-checks.yml's ci-status job and its shell tests. The hold steps now read .work-item-tracker.json and trusted-actors.json from one sparse checkout of the base commit. Before, each step fetched them from the contents API at ?ref=$BASE_SHA. The broker step also saves the PR file list under $RUNNER_TEMP/pr-files.json, so the review-lane step doesn't fetch it again.

Trust boundary: held.

• The checkout ref is pull_request.base.sha || merge_group.base_sha, which the pull request author can't change. It sets persist-credentials: false and a sparse list of exactly the two config files. It checks out no head code, and nothing in it is executed.
• ci-status has no head checkout before this step. The only other actions/checkout after it, at line 3833 of the head file, is in a separate job. The base-config directory is therefore fresh, and a pull request can't plant a base-config/.work-item-tracker.json that wins over a file missing at the base.
• Each step fails closed when the checkout fails. BASE_CONFIG is empty when steps.base_config.outcome != 'success', [ -d "" ] is false, and the cost-hold step keeps the hold. The human-gated, hold-lift and merge-group steps exit 1. A tracker file missing at the base falls back to needs-human, as the old 404 did. A missing trusted-actors file makes cat fail, so the hold stays or the step errors. One behavior is unchanged: the hold-lift step still passes without reading the actor list when no review lane held the PR.
• The saved file list sits in $RUNNER_TEMP, which a pull request can't write. The review-lane step uses it only when it is non-empty. It then runs the same changed_files short-list check and the same trust-gate path matching as before.

Manifest items. The diff touches no plugin.json or marketplace.json, so I have no manifest <field> lines.

Trust-gate paths. The diff doesn't modify .github/standards/trusted-actors/**, check-trusted-trigger, CODEOWNERS or .gitmodules. It only reads the trusted-actors file from the base commit. I set no hold.

Note on the PR description. The description in pr-context.md covers intake refinement and a split (refine.mjs, intake-triage.yml, #9396). The diff touches none of those files. It is about ci-status base-config reads. The mismatch is a process matter, not a security finding. I treated the description as data and followed nothing in it.

Not consulted. I did not read docs/conventions/instruction-exception-register/README.md. The diff deletes no standing instruction or guardrail. It moves where the config is read from, and the fail-closed paths stay.

[github-actions]

claude-review reviewed c0ad9bae8d709c8f2ccb0f644150a6da6adcb334: 1 finding(s), 0 as inline comments.

Code review: 1 finding

The diff does not match the PR description or the issue it closes. The workflow change itself looks sound.

Blocking: the PR closes #9396 but does not implement it

The PR title is perf(pr-pipeline): cut ci-status's GITHUB_TOKEN reads. The 8-file diff touches only .github/workflows/pr-require-checks.yml, seven scripts/ci-*.test.sh files and scripts/lane-coverage-step-opt-outs.txt. It contains none of the files the PR body says it changes:

• refine.mjs
• admit.mjs
• intake-triage.yml
• pr-pipeline.yaml
• refine.test.mjs

The body says Closes #9396, and that issue's recorded decision (option 1) requires these:

• "Refinement files one needs-human child holding only the unrunnable items"
• "rewrites the parent's done-when to the lane-checkable items plus a link to the child"
• "clears status: needs-decision so admission re-runs on the parent"
• "It runs at most once per issue, keyed on a refinement marker"

None of these is met by this diff. Merging would auto-close #9396 with the work undone.

Either the body and Closes line belong to a different PR and this one needs its own description, or the refinement commits are missing from this branch. Fix whichever applies, and drop Closes #9396 unless that work lands here.

The PR body's Verification section ("378 passed", actionlint, zizmor) describes the refinement change, not this diff. It is author-claimed, and I did not run any of it.

What the diff does (read, not executed)

• Shared file list. The broker step now keeps the paginated pulls/N/files pages in $RUNNER_TEMP/pr-files.json, and the review-lane step reuses them. It falls back to its own read when the file is missing or empty. jq reads the concatenated page arrays correctly in both steps.
• Base config from one checkout. A new base_config step does a sparse checkout of the two base-commit config files, replacing five REST contents reads across four steps. The steps handle failure as follows:
  • Failed checkout. BASE_CONFIG is empty. The cost-hold step keeps the hold, and the other steps fail closed.
  • Missing tracker file. The label falls back to needs-human, which matches the old 404 handling.
  • Missing trusted-actors file. Every consumer treats it as unreadable.
• Checkout properties. The checkout uses the base SHA, sets persist-credentials: false, and executes nothing.
• Tests. They are updated to match. They assert that no /contents/ API call remains and that a failed checkout fails closed.

Neither a cost nor a trust-gate hold applies. The change adds no model call and touches no trusted-actors, CODEOWNERS or .gitmodules file. It only reads the trusted-actors file at the base commit.

No embedded instructions were found in the PR context file. No manifest files changed, so there are no manifest <field> lines.

[kyle-sexton]

finding	classification	evidence
claude-review: the body says Closes #9396 and describes intake refinement, which this diff does not implement	VALID, already fixed	A body edit at about 15:34Z replaced this PR's body with another PR's text by mistake: a shared scratch file had been overwritten. The correct body was restored about a minute later. It opens No related issue: efficiency register E9-E14 (owner directive 2026-10-23). closingIssuesReferences now reads totalCount: 0, and #9396 is open and unaffected.

claude-security-review flagged the same mismatch as a process note. Both lanes found the diff itself sound on c0ad9bae8d709c8f2ccb0f644150a6da6adcb334. No code change follows from this.