Skip to content

perf(ci): count every pr-merge job's GITHUB_TOKEN calls - #10601

Merged
kyle-sexton merged 3 commits into
mainfrom
perf/pr-merge-api-accounting
Oct 28, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
perf/pr-merge-api-accounting

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: 52% of GITHUB_TOKEN spend unmeasured (GitHub Support, 00:49-01:42Z)

Summary

GitHub Support counted 4,069 GITHUB_TOKEN calls between 00:49 and 01:42Z on 2026-10-28, and only 1,941 of them came with an api-calls notice. 17 of the 22 pr-merge runs in that hour emitted none. Three pr-merge jobs call the API with the job token and log nothing: dispatch-full-run (the only job a push to main runs), unarmed-status and gate-check. This PR makes all three log their calls the way the other pr-merge jobs already do. It is instrumentation only.

job before after
post-merge-dispatch logs (#10551), except its dispatch POST also counts the dispatch POST
merge, enable-auto-merge log unchanged
unarmed-status none logs
dispatch-full-run none logs the cover read and each dispatch POST
gate-check none logs

Fix

  • unarmed-status, gate-check. Both already check out .base and call GitHub through createGitHub, which records a reading for each response under API_BUDGET_TOKEN. Each job now sets API_BUDGET_TOKEN: github-token and ends with the log-api-budget step (if: always(), continue-on-error). This is the same change ci: log GITHUB_TOKEN api-calls from ci-status, post-merge-dispatch and intake-reenter #10551 made to post-merge-dispatch.
  • dispatch-full-run. The job now does a sparse checkout of .github/actions/log-api-budget/ only, as pr-feedback-dispatch's rerun-ci-status job does. The checkout has continue-on-error, so if it fails, the job calls GitHub exactly as before and logs nothing. Its token gains contents: read, which the checkout needs. The cover read sources api-budget.bash and wraps gh, as post-merge-dispatch's copy of the same step already does.
  • The dispatch POST, in both jobs. Each attempt's gh api -i output is passed to api_budget_read. The POST is recorded, never guarded, so a stop recorded earlier in the job cannot refuse a dispatch that goes out today. The two step bodies stay identical.
  • The -i counting gap. In api-budget.bash, the -i|--include branch of the gh wrapper observed the headers but never kept the reading, so a wrapped -i call went uncounted. It now calls api_budget_read "$API_BUDGET_GH_TOKEN" "$out" >/dev/null before cat "$out". pr-require-checks.yml's verbatim copy in API_BUDGET_SH gets the same line. No wrapped caller also reads -i output itself, so nothing is counted twice.
  • enable-auto-merge. pythonVerify read only the totals from the reader's babysit-gh-calls line, so the per-endpoint JSON never reached the log. It now prints that line once per pull request, re-serialized from the parsed JSON so it is always one line. The shape matches the line the merge job tees, so one grep sums the calls per endpoint across both jobs.
  • Not done: the 304 poll count. The brief asked to count the 200 and 304 answers in dispatch-full-run's poll loop and to emit not-modified=M from an EXIT trap. That poll no longer exists: perf(pr-merge): batch main's whole-suite runs and stop sleeping after the dispatch #10491 removed it, and it merged at 00:36Z, before the window. No conditional GET is left in pr-merge.yml. The job-end log step runs if: always() and already emits not-modified=M, and the new test below shows a wrapped 304 is counted there.

Cost. The only new checkout is one sparse checkout per dispatch-full-run run. The same checkout in rerun-ci-status took 2-3 s on recent runs. unarmed-status and gate-check add no checkout, only the log step, which makes no API call.

Still not counted. The checkout's own git fetch and the artifact actions. Also github-token calls in merge and enable-auto-merge runs that never mint a lane token: their log steps run only when steps.lane-token.outputs.token != '', and a test pins that condition.

This PR touches pr-merge.yml and pr-require-checks.yml, which are on the merge-holds path list, so a person lifts the hold before it merges.

Verification

  • New test, every job that calls GitHub with the job token logs its api-calls (workflow.test.mjs). It finds every pr-merge job that uses ${{ github.token }}, without a hard-coded list. On main it failed with [ 'unarmed-status', 'dispatch-full-run', 'gate-check' ]. It passes on this branch.
  • New test, a -i call is counted in api-calls, and a 304 as not-modified (log-api-budget.test.mjs). Before the fix the output was ''. After the fix it prints github-token=1 not-modified=1.
  • New test: dispatch-full-run's cover read and dispatch POST run against the real library and leave two readings (github-token=2). The pythonVerify test now asserts the calls line is logged.
  • node --test .github/scripts/pr-merge/*.test.mjs .github/actions/log-api-budget/*.test.mjs .github/actions/check-trusted-trigger/*.test.mjs: 1054 pass, 0 fail. scripts/ci-rerun-ci-status.test.sh and scripts/ci-lanes-pr-hold.test.sh pass. test_merge_holds_sync.py: 10 passed. shellcheck finds nothing in api-budget.bash. actionlint reports only the existing concurrency.queue findings.
  • Left out: runner-policy.mjs, which could not run locally (its ajv dependency is not installed in the worktree); CI runs it.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 27, 2026 23:26
dispatch-full-run, unarmed-status and gate-check called GitHub with the
job token and wrote no api-calls notice. Each now ends with the
log-api-budget step; dispatch-full-run gets a sparse checkout of the
library (contents: read) and records its cover read and each dispatch
POST, record-only. The dispatch POST in post-merge-dispatch is recorded
the same way.

api-budget.bash's -i branch now keeps the reading of a call the caller
sends with -i, so it is counted (pr-require-checks.yml's verbatim copy
too). enable-auto-merge logs the verifier reader's babysit-gh-calls line
per pull request, as the merge job's tee does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0144XNKz9sKFKYo4gQtVW78R
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 28, 2026 04:16
@chatgpt-codex-connector

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 🔄 Running since 2026-10-28T04:17:01.032818Z a780943 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

Copy link
Copy Markdown
Contributor

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

Security review of #10601 at a780943: no security findings.

critical-findings: 0

I read the whole diff and the PR context. The change touches pr-merge.yml and pr-require-checks.yml, which are trusted paths. I checked the controls they gate and none is widened.

  • New checkout in dispatch-full-run (.github/workflows/pr-merge.yml, new step "Check out the api-budget library"). It checks out refs/heads/main only, with persist-credentials: false and a sparse path of /.github/actions/log-api-budget/. No pull-request or fork code reaches this job, so sourcing api-budget.bash from it runs only code already on main. The job token gains contents: read and keeps actions: write. The job token is the workflow's GITHUB_TOKEN, not the lane App token or the token broker, so no lane-token scope is widened. The credential is not persisted into .git.
  • Failed checkout. continue-on-error plus . ... 2>/dev/null || true and declare -F api_budget_read guards mean a missing library leaves the dispatch exactly as before. The dispatch POST is recorded only and never guarded, so no stop can refuse it. Nothing is added to the dispatch's authorization path.
  • gh api -i counting fix (api-budget.bash:142, mirrored in pr-require-checks.yml). api_budget_read only parses X-Ratelimit-* headers with strict regexes (^[0-9]+$, ^[a-z_]+$). It appends to a state file whose name must match ^[a-z][a-z0-9-]{0,39}$, and its output goes to /dev/null. The response body is untrusted, and it neither reaches a shell nor changes cat "$out".
  • enable-auto-merge.mjs. ghCallsLine prints the reader's babysit-gh-calls line, re-serialized from parsed JSON. Only object payloads are printed. The line is log output only. It is not used in any hold or merge decision, and ghCalls behaves as before. No merge hold is weakened.
  • unarmed-status and gate-check. Each adds API_BUDGET_TOKEN and a final if: always() log step, using the local ./.base/.github/actions/log-api-budget from the main checkout. Their permissions are unchanged.
  • Tests. The tests add no executable path to production.

No manifest files changed, so no manifest <field> lines apply. The PR context contains no embedded instructions to the review. The change spends no billed resource, so there is no cost hold.

@github-actions

Copy link
Copy Markdown
Contributor

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

Review of #10601: count every pr-merge job's GITHUB_TOKEN calls

No findings. I read the full diff and the surrounding code. I did not run any tests, so the PR's test results are author-claimed and unverified.

What I checked

  • -i branch of the gh wrapper (api-budget.bash:142, and the copy in pr-require-checks.yml). I searched the repo for every gh api -i call. The wrapper is only active in steps that call api_budget_wrap_gh, and none of them also reads -i output itself. The dispatch steps in pr-merge.yml and rerun-ci-status source the library without wrapping gh and call api_budget_read explicitly. gh_read in pr-require-checks.yml does not use -i. So nothing is counted twice, and the PR's claim holds.
  • dispatch-full-run.
    • The new contents: read is only for a sparse checkout of the library from refs/heads/main, with persist-credentials: false and continue-on-error. The job's other permission is actions: write.
    • Sourcing the library from the default branch is not an injection risk.
    • A failed checkout degrades to the old behaviour: || true on the source, declare -F guards on the reads, and the log step has continue-on-error.
    • Wrapping gh in the cover step matches what post-merge-dispatch already does. Rate-limit detection still reads cover-stderr, which still carries gh's own error text.
    • The dispatch POST is recorded but not guarded. api_budget_read only appends to the readings file, so it cannot refuse a dispatch.
  • set -e and here-strings. ! declare -F … || api_budget_read … <<<"$out" >/dev/null is safe on an empty $out (a connection failure). api_budget_read returns 0 there. A here-string that bash backs with a pipe can only lose the discarded body, not the reading, because awk reads it once. This is the same pattern as pr-feedback-dispatch.yml:423.
  • unarmed-status and gate-check. Each gets API_BUDGET_TOKEN: github-token and the same if: always(), continue-on-error log step as the other jobs. I read the updated step-name assertions in merge-job.test.mjs. The slice(2) loop still holds for the new step, because the step has continue-on-error: true.
  • enable-auto-merge.mjs. ghCallsLine re-serializes only a parsed JSON object. Malformed or non-object payloads return null and log nothing. Failure paths are tested.
  • New tests. The helpers and imports used in workflow.test.mjs all exist (coverBody, dispatchStep, tempDir, symlinkSync, fileURLToPath). The existing runCover tests run without .base, which the || true and declare -F guards cover.

Trusted paths. The change touches pr-merge.yml, pr-require-checks.yml and enable-auto-merge.mjs. I found no security issue in any of them: no new write permission, no new secret exposure, no widening of a guard. The change only adds instrumentation.

Spend. The change adds one sparse checkout per dispatch-full-run run and a few log steps. It uses Actions runner time and no paid setting or billed resource, so there is no hold.

Closing issues. The PR states "No related issue", so there are no acceptance criteria to check.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 7ccf1c8 Oct 28, 2026
83 checks passed
@kyle-sexton
kyle-sexton deleted the perf/pr-merge-api-accounting branch October 28, 2026 04:36
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