Skip to content

feat(report-api-budget): emit the api-calls notice - #942

Merged
kyle-sexton merged 1 commit into
mainfrom
feat/report-api-calls
Oct 31, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
feat/report-api-calls

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: instrumentation from a GITHUB_TOKEN rate-limit measurement, tracked in no issue.

Summary

The review lanes post api-budget but no api-calls, so the hub cannot tell a job's own requests from the rise in a counter other jobs share. Each pr-review-cli.yml job now posts github-token=N not-modified=M graphql=K. The reusable runs at @main, so this applies at merge.

Fix

  • The action's copy of api-budget.bash had no api_budget_emit_calls. It is added, identical to the claude-code-plugins function.
  • report-api-budget calls it in the token loop, as log-api-budget does, and writes the line to the step summary.
  • A job with no reading now writes its line at 0 instead of nothing, so a silent job and a job that made no request read differently.

Counted requests are those of watched Octokit clients and the responses files. A call outside both is not counted.

Verification

  • report-api-budget.test.cjs: 15 pass, including a 304 counted as not-modified and the zero line. The expected values are counted by hand.
  • pr-review-cli-api-calls.test.cjs: 9 pass. For each of six run shapes, each job's N equals the requests the fake answered.

Related

  • The claude-code-plugins change that reports each counter: feat(pr-pipeline): count job-token calls per counter and in five more jobs.
  • api_budget_emit here still prints one reading per resource. The header's identical-copies rule needs a follow-up once that change merges.

🤖 Generated with Claude Code

The review lanes posted api-budget but no api-calls, so the hub read their
requests as uncounted. The action's copy of api-budget.bash had no
api_budget_emit_calls: add it, identical to the claude-code-plugins copy,
and call it in the token loop as log-api-budget does. A job with no reading
now writes its line at 0, as log-api-budget's does.

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

chatgpt-codex-connector Bot commented Oct 31, 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-31T17:39:14.875412Z e84200f 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.

@github-actions

Copy link
Copy Markdown

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

No security issues found in this pull request. I read the full diff in .claude-lane/pr.diff. It adds an api-calls notice to .github/actions/report-api-budget/, updates the tests, and adds one README line.

  • Token names: the new api_budget_emit_calls checks each name against ^[a-z][a-z0-9-]{0,39}$ before using it, and action.yml applies the same check before the call. A token name can't carry a newline or :: workflow-command text into the notice, and awk's command-line assignment can't be fed escape sequences.
  • awk program: the program and its printf format string are constants. The only data in the output is the validated token name and integer counters taken from the job's own state file.
  • Paths: the state file path is built from RUNNER_TEMP and the validated name, so no path traversal is possible. An empty RUNNER_TEMP makes the function skip the token.
  • Step summary: the api-calls line written to $GITHUB_STEP_SUMMARY is built from that same awk output.
  • Requests and credentials: the change adds no network request, no token-scope change and no secret handling. It still calls no rate-limit endpoint.
  • Control paths: the diff does not touch any trust control, trusted path, merge hold, plugin manifest or instruction file, so none of those lenses apply.
  • Settled threads: the lane supplied no SETTLED THREADS: file, so nothing was dropped as settled.
  • Verifier subagents: no candidate findings survived, so I dispatched none.

critical-findings: 0

@github-actions

Copy link
Copy Markdown

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

Code review: PR #942, feat(report-api-budget): emit the api-calls notice

Findings: 0. No inline comments posted.

What I checked

  • api_budget_emit_calls in api-budget.bash:
    • The awk counts non-304 lines as N, 304 lines as M, and graphql-resource lines as K.
    • Unset counters print as 0 through %d.
    • A missing or empty state file falls back to /dev/null, which gives the zero line.
    • Token validation matches the action's own regex.
  • action.yml ordering: the api-budget lines still print first, then the api-calls line. A token with no reading now gets only its api-calls line at 0. The costs output and its step-summary line are unchanged, because [[ -n $lines ]] || continue still guards them.
  • The test updates match the new output order and the zero-reading case. The README and the action description agree with the code.
  • No closing issue, since the PR states No related issue. No .claude/rules file has a paths: glob that matches the changed files; the PR body contract rule covers the PR description, which follows the format it asks for. The settled-threads file was not supplied.

Not verified

  • I could not run report-api-budget.test.cjs or pr-review-cli-api-calls.test.cjs, because the sandbox blocked node --test. The "15 pass" and "9 pass" results in the PR body are author-claimed and unconfirmed by me.

Minor observations (not findings, below the bar)

  • api_budget_emit_calls counts every line in the state file, but api_budget_emit only counts lines with six fields (NF == 6). A blank or malformed line would add one to N. The state file is written only by api-budget.cjs and api_budget_read, so this is unlikely in practice.
  • The PR notes that the header's "identical copies" rule with claude-code-plugins needs a follow-up once that change merges. That is an acknowledged cross-repo item.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit b078cd0 Oct 31, 2026
29 checks passed
@kyle-sexton
kyle-sexton deleted the feat/report-api-calls branch October 31, 2026 18: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