Repository navigation
fix(gemini-ledger): hand the tier report only records traceable to trusted runs - #2305
Conversation
…usted runs
The Gemini tier snapshot and history are built from token-usage artifacts
collected across the organisation. An artifact is untrusted input: a pull
request from a fork can upload one of any name and content, and a record
says nothing about its origin but a run id it wrote itself.
- token_report.sh decides trust from the Actions API, never from the
artifact: the uploading run's head repository is the repository itself,
and either the run was on the default branch (from the artifact listing,
no extra call) or the looked-up run confirms the head repository and was
not started by Dependabot. A fork run, a Dependabot run, a missing
workflow_run and a failed or mismatching look-up are not trusted.
- Every collected record is stamped provenance {run_id, trusted, basis},
replacing anything the artifact wrote. Only trusted records reach
GEMINI_RECORDS_OUT; the ignored ones are counted in the log, a warning
and the job summary. The cost report is not filtered.
- The collector reports what it could not read and how many handed-over
records are for a key index and model the caps file does not list (test
fixtures written to the real ledger by engine tests run inside an agent
session). gemini_tier_report.sh copies that into the snapshot as
collection.
- gq_tier_decision is the reader a dispatch decision must use: unknown
unless the snapshot is fresh, for the current quota day, trusted-only
and from a complete collection with no unlisted records. Nothing calls
it yet.
- The history cache is renamed so no record collected before the filter
is restored.
Refs #2147
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
dev-lead is withholding action on this item. It is labeled To re-enable automated pickup: remove the |
QA Lead — test-risk advisoryRisk tier: MEDIUM. Nothing calls What I'd shore up (highest leverage first):
Escalate? yes. Under the rate-limit condition this PR itself names, the "complete" flag fails open, so it can't be trusted to gate dispatch later. Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T19:39:15Z. |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to verify the provenance of token-usage records, ensuring only records from trusted runs are used to build the Gemini tier snapshot and make dispatch decisions. It updates token_report.sh to determine trust via the GitHub Actions API, filters out untrusted records, and writes a collection report. gemini_tier_report.sh is updated to include collection metadata in the snapshot, and gemini-quota.sh introduces gq_tier_decision to validate snapshots before allowing dispatch. Comprehensive tests are added in token_report_trust.bats. The review feedback recommends three improvements: leveraging the existing EXIT trap by creating the temporary trusted_dir inside jsonl_dir to prevent orphaned directories, using jq's optional chaining operator (?) in gq_tier_decision to safely handle potentially missing nested properties, and piping the output of gq_tier_snapshot directly into jq to avoid creating a temporary file.
CodeAnt Nitpicks1 code suggestion1. When a genuine same-repository branch run lookup fails, its records are dropped but no failure is counted. Dispatch may accept a complete snapshot missing that usage.Incomplete implementation · |
CodeAnt PR Risk: Low Risk
Assessed commit: |
… file as failed reads Review of the trusted-records change found reads that fail without making the collection incomplete, so real usage could be dropped while the snapshot still said complete. - A run look-up that FAILS (non-zero exit, timeout, rate limit, an empty or unparseable body, a body that is not a run object) is no longer the same as one that answers no. It is stamped run-lookup-failed and counted (run_lookups_failed); a look-up that answers and does not qualify stays run-not-verified and is only ignored. - A record file that is not valid JSONL is counted (record_files_malformed) instead of being dropped silently. - Either count above zero makes collection.complete false, so gq_tier_decision answers unknown. Both are in the snapshot schema. - A run is looked up once per collection, however many artifacts it uploaded (a re-run attempt uploads another under the same run id). - The trusted copy lives under the collection directory the EXIT trap already removes. The tier report builds the snapshot beside its target, moves it into place whole and removes its working files on every exit. - Tests: the snapshot the tier report writes is validated against the schema with and without a collection report (fails in CI when the validator is missing, skips with a notice elsewhere); a snapshot whose window, collection or tiers has the wrong type is unknown. Refs #2147 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review round 1, head
Suites: |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T22:27:32Z. |
…s a failed read An artifact with no usable workflow_run was treated like a fork's: untrusted and nothing missing. A fork is known to be a fork; this one may be real usage, so it now makes the collection incomplete. A repository whose listing file could not be created is counted as a failed listing too. Both from CodeAnt on the pull request. Refs #2147 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review round 2, head
Suites after merging |
|
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-12T02:46:20Z. |



Problem
The Gemini tier snapshot and its history are built from
token-usage-*artifacts collected across the organisation. Those artifacts are untrusted input: a pull request run from a fork of a public repository can upload one under any name, with any content. That is harmless while the snapshot is informational, and stops being harmless the moment anything gates dispatch on it. "Gemini trickle under a budget pause" (#2147) makes this its first acceptance criterion: records feeding a dispatch decision must be traceable to trusted runs.This is that criterion only. The trickle itself is not in this pull request and is not being built yet; see "What this does not do".
Design
What a record can prove. A record carries only what the job wrote, including its own
run_id; all of it, and the artifact's name, can be forged. What cannot be forged is what the Actions API says about the run that uploaded the artifact.The trust rule. An artifact is trusted when both hold:
workflow_run.head_repository_idequalsrepository_id(not a fork).That is the set of runs GitHub gives the repository's secrets to. Whoever controls such a run's code can already call Gemini with the real keys, so a forged ledger gains them nothing; fork and Dependabot runs get no secrets and cannot have made a real call. Anything that cannot be established (a failed or mismatching look-up, no
workflow_run, no actor) is untrusted."Default branch only" was rejected: on four days of real data it would drop about a fifth of genuine usage (269 of 1,353 artifacts), and missing usage makes Gemini look more available than it is.
Where it sits.
scripts/token_report.shclassifies each artifact at listing time, looks up non-default-branch runs, and stamps every record withprovenance {run_id, trusted, basis}, replacing anything the artifact wrote under that key. Only trusted records are handed to the tier report. Ignored and unreadable counts go to the log, a warning and the job summary."Cannot tell" is not "available". The snapshot gains a
collectionblock (complete or not, counts, reason). A new reader for future dispatch decisions,gq_tier_decision, answersunknownwhen the snapshot is unreadable, stale (default one hour), from the future, for a quota day that has ended, built from an incomplete collection, not marked trusted-runs-only, or holding records for a model or key with no caps row. Nothing calls it yet. The existing informationalgq_tier_can_runis unchanged.The history cache key gains
v2, so the store filled before the filter existed is not restored. The per-day history table loses up to seven earlier days, once.What this does not do
unknownwhile any are present; 709 of them look exactly like real calls and cannot be counted.Risk
collectionblock was hand-checked; the schema test needs a Python package that is not installed locally, so CI is the first real validation.workflow_runto still be collected for it.Test plan
tests/token_report_trust.bats(new, 27 tests), wired intolint.yml: the trust rule (fork, same-repository branch, default branch, Dependabot, an artifact declaring itself trusted, an unreadable or mismatching look-up), the hand-off, the snapshot's collection block, the decision reader's fail-closed cases, and the wiring.token_report44,test_gemini_quota44,test_gemini_tier_view48 (plus one that needs the missing Python package locally).token_report44,test_gemini_quota44, the fixed-path check 18;shellcheck --severity=warning -xandactionlintclean.Rollback
Revert the squash commit. The tier report goes back to reading every collected record, and the old history cache is used again.
Monitoring
download failed or timed outwarnings, for the added look-ups.Refs #2147
🤖 Generated with Claude Code