Skip to content

chore(ci): re-pin the review callers to ci-workflows v0.65.8 - #9848

Merged
kyle-sexton merged 1 commit into
mainfrom
chore/repin-review-callers-key-credit
Oct 24, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
chore/repin-review-callers-key-credit

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

No related issue: owner decision Q57 (2026-10-24)

Summary

Re-pins both review callers to ci-workflows v0.65.8 (4f79cc9244498ae856504cf74aeeb05dcc9c6b43). That release adds the check-key-credit job (ci-workflows#882). While the reviews key's last credit stop is less than 6 hours old, the job skips the key job, and the subscription job reviews.

Fix

  • pr-review-hosted.yml and pr-review-security-hosted.yml: the pin moves from v0.65.7 to v0.65.8. Each pr-review* job also grants actions: read. The reusable's check-key-credit job requests that permission to read this workflow's own runs. A called workflow can only lower the caller's grant, so a caller that does not grant it fails at startup.
  • scripts/ci-review-lanes.test.sh: the pin check now expects v0.65.8. A new check fails when a caller job stops granting actions: read.

API budget (GITHUB_TOKEN)

The gate reads with the job token. That token's core budget ran out at 16:51-16:55Z today (api-budget remaining=0 used=5000).

  • Calls added per run, per lane:
    • One run-list call (GET .../actions/workflows/<file>/runs?status=completed&created=>=<now-6h>&per_page=100).
    • Then at most 3 job-list calls (GET .../runs/<id>/jobs?filter=latest). The newest run usually settles the question, so a run usually spends 2 calls.
  • At ~340 runs/day per lane:
    • That is about 680 calls/day per lane (about 28/hour), and about 57/hour for both lanes.
    • The worst case is 4 calls per run: about 113/hour for both lanes, about 2.3% of the 5,000/hour seen in the api-budget readings.
  • Calls saved while the key is out of credits: the gate skips the key job and with it the key job's own job-token reads. On run 40119772245 the key job's api-budget notice shows cost=5, about 6 core requests. Today a burned key spends those reads and the subscription job repeats them. With the gate, only the subscription job reads, so a burned-key run spends about 4 fewer calls net.
  • A refused read never blocks the review.
    • A 403 or 429 response throws, and decide catches it and returns "run the key" (.github/actions/check-key-credit/gate.cjs).
    • The step's log line is a plain check-key-credit: ... runs the key line, not ::error.
    • The step is continue-on-error, and the action wraps node ... || true, so the job stays green.
    • The refused response's rate-limit reading still reaches the job's api-budget notice.
  • Considered and not done: a per_page=1 read first, to skip the list when the previous run used the key successfully. It saves nothing. The common case already costs one list call plus one jobs call. A one-run list still needs the jobs call to see what the key did. It also cannot tell whether a carried credit stop is still in the window, which the 6-hour list provides.

Verification

  • bash scripts/ci-review-lanes.test.sh: PASS=83 FAIL=0 on current main. With the actions: read grant removed from one caller, it reports FAIL: pr-review-hosted.yml does not grant actions: read to job pr-review.
  • actionlint and zizmor --offline pass on both callers. shellcheck passes on the test script.
  • node .github/standards/runner-policy/runner-policy.mjs --root .: Runner policy passed, with the v0.65.8 contract from standards #877 synced in chore: sync standards components #9837.

Related

  • melodic-software/ci-workflows#882, released as v0.65.8.
  • melodic-software/standards#877, the v0.65.8 runner-policy contract.
  • Not yet verified live: the gate's carry step names the origin run through a steps.<id>.outputs value. The first credit stop on main after this merges is the probe. If the name does not render, the gate still fails open.

🤖 Generated with Claude Code

v0.65.8 adds the reusable's check-key-credit job, which reads this
workflow's own runs and skips the key job while the reviews key's last
credit stop is inside six hours. Each caller job grants actions: read for
it; without the grant the run fails at startup. The lane test pins
v0.65.8 and fails when a caller stops granting actions: read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

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-24T19:04:29.858155Z 81f0c56 PR opened
ℹ️ 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.

@kyle-sexton
kyle-sexton enabled auto-merge October 24, 2026 19:02
@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 81f0c564d273d7ff072570f728a4597be4377a7e: 0 finding(s), 0 as inline comments.

No findings. I read the full diff (3 files) and the PR context.

  • Pin bump: both callers move to pr-review-cli.yml@4f79cc92… (v0.65.8). The runner-policy file .github/standards/runner-policy/policy.json already has an allow entry for that SHA, so the pin should pass the policy check. The old v0.65.7 entry is still there, which is expected for a synced file.
  • actions: read grant: it is added at the job level of both pr-review and pr-review-security callers, next to the existing contents: read and pull-requests: write. This matches the PR's stated reason that a called workflow cannot raise the caller's permissions.
  • New test check in scripts/ci-review-lanes.test.sh: I traced the awk by reading it, not by running it. It starts at the exact job line, stops at the next 2-space-indented key, and matches actions: read as a whole token, so the trailing comment on the grant line is fine. It fails when the grant is missing from the job. The pin regex and messages are updated consistently.
  • Spend: the change adds a few GITHUB_TOKEN API reads per run and, by the PR's own estimate, saves some key-job reads when the key is out of credit. It adds no model spend, paid plan or billable resource, so it is not a cost hold. No trusted path is touched.
  • Author-claimed, not verified by me: the test counts (PASS=83), the actionlint/zizmor results, the API-budget figures and the live behavior of the gate's steps.<id>.outputs carry step. The PR itself says that last one is not yet verified live and that the gate fails open.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 81f0c564d273d7ff072570f728a4597be4377a7e: 0 finding(s), 0 as inline comments.

No security findings.

critical-findings: 0

Changes reviewed (3 files, none under a trusted path):

  • .github/workflows/pr-review-hosted.yml and .github/workflows/pr-review-security-hosted.yml:
    • The reusable workflow pin moves from v0.65.7 to v0.65.8, and stays a full 40-character commit SHA with a version comment, so it is not loosened or unpinned.
    • Each caller job gains actions: read. This is read-only, scoped to the job, and needed by the reusable's check-key-credit job to read this workflow's runs. The caller still grants only contents: read and pull-requests: write beyond that. Permission widening is zizmor's advisory lane, and I do not repeat it here. I found no exploit path.
  • scripts/ci-review-lanes.test.sh: only the expected pin and a new check that actions: read is granted. The awk and grep patterns are fixed strings and take no untrusted input.

Not verified: I have no network access here, so I could not confirm that SHA 4f79cc92… is the v0.65.8 tag of melodic-software/ci-workflows. The PR description says it is, and the repository's ci-review-lanes.test.sh and runner-policy checks pin it.

manifest <field>: no plugin manifest changed, so no manifest lines apply. The PR context contains no embedded instructions directed at the reviewer.

Spend: the added gate makes about 2 to 4 GITHUB_TOKEN API calls per run. This is not a billing change, so there is no cost hold.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 5e6ad84 Oct 24, 2026
74 of 76 checks passed
@kyle-sexton
kyle-sexton deleted the chore/repin-review-callers-key-credit branch October 24, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant