[don-petry]

Problem

The org-wide Gemini usage ledger is almost entirely test fixture data: about 8 of 1,819 request-counting Gemini records over four days were real calls. The cause is not a test job. The dispatch job of dev-lead-reusable.yml sets TOKEN_LOG_FILE for the whole job; the agent session inherits it; when the agent runs this repository's tests as part of its work, suites that do not isolate the variable append their fixture calls, rejections and cooldowns to the run's real ledger; the run then uploads it.

Measured for this change, by running every bats file on its own with the variable exported: 16 of 247 files wrote 221 records (the largest, test_fix_reviews, 122). That is more than the seven suites the issue names, and includes three files outside tests/dev-lead/unit/ and two that mention the variable.

Design

Two independent halves; either alone stops the leak, and they fail in different ways.

1. Tests never see an inherited ledger. setup_suite() in tests/dev-lead/unit/setup_suite.bash unsets TOKEN_LOG_FILE, GEMINI_LEDGER_FILE (the Gemini library's override for the same file) and _ENGINE_USAGE_OUT (a per-call usage sidecar). A test that needs a ledger sets its own.

• bats runs setup_suite in the shell that starts every test file, so an unset there reaches the tests (verified with a probe suite on bats 1.13).
• bats loads one setup_suite.bash per run, the one beside the first path it is given, and never looks in subdirectories. So tests/ and tests/dev-lead/integration/ each gain a one-line setup_suite.bash that sources the unit one. Every directory holding bats files now has the same hooks.
• Side effect, intended: a run that starts in tests/ now also gets the git-identity guard the unit directory already had. All 157 top-level files give the same counts with it.

2. The model CLI is started without the ledger. New _engine_child in scripts/engine.sh runs its command under env -u for the same three variables, and all five CLI starts go through it (Copilot, Gemini's two paths, Claude's two paths). The parent shell keeps the variable and still records every call; the child never needed it. This stops whatever the session runs from writing to the ledger, tests or not.

No workflow file changes.

Risk

• The engine.sh half reaches production only when the dev-lead channel moves. This repository pins dev-lead/v139-ring0 for both the workflow and its scripts. The suite-hook half needs no tag: the agent runs tests from the branch it is working on, so any branch containing this commit stops leaking; branches cut earlier keep leaking until they pick it up.
• model-ab.yml also sets the variable and starts two claude --print probes directly, not through engine.sh. They grant no tools and are left as they are.
• The nested bats runs in the new guard add about 25 seconds to the unit job (measured on macOS). The guard file is not in lint.yml's list, so the time-limited bats job is unaffected. A change that touches only top-level tests/ does not trigger the workflow that runs the guard; it fires on the next change that does.
• Whether one full pass of the tests explains the volume seen in production was not confirmed: one pass yields 12 Gemini records, so the 1,811 fixture records over four days imply many passes or repeated runs of the Gemini suites by agent sessions.

Test plan

• Before and after, every bats file run alone in a clean environment with the three variables exported: 16 files and 221 records before; no file and no record after (249 files).
• tests/dev-lead/unit/test_token_ledger_isolation.bats (new, 3 tests): every directory with bats files has suite hooks, and a probe run through each sees no ledger variable; one real engine suite and three top-level suites, run nested with the variables exported, leave the ledger directory empty.
• tests/dev-lead/unit/test_engine_child_env.bats (new, 6 tests): for each launch site a stub CLI dumps its environment and finds none of the three variables, while the parent's ledger still gets the call's record; in one, the stub itself calls the token writers and the ledger still holds only the parent's record.
• Existing suites: per-file counts identical before and after across the engine, Gemini, fix-ci, fix-issue, fix-reviews and token suites. test_engine_writer test 15, which failed only when a ledger was inherited, now passes in that case too.
• Mutation-checked: 14 mutations, each turning named tests red. I re-ran two after rebasing: with the suite-level unset removed, all 3 isolation tests fail; with the child keeping the variables, all 6 child-environment tests fail.
• shellcheck --severity=warning -x clean; the fixed-path check exits 0; no negated assertion that cannot fail.
• I replaced the subagent's nested run of seven engine suites (139 seconds) with one suite: the isolation is suite-wide, so more suites added time and no coverage.
• Not run: CI; the live e2e scenarios.

Rollback

Revert the squash commit. Tests and agent sessions go back to inheriting the ledger path.

Monitoring

• After this merges: the next agent session that runs tests on a branch containing it should upload a ledger with no fixture model names (gemini-same, gemini-test, gemini-u1, …).
• After dev-lead/v139-ring0 moves: the "Gemini ledger provenance" section added by fix(gemini-ledger): hand the tier report only records traceable to trusted runs #2305 should report zero records for models or keys with no caps row. Until both land, the warning there is expected.

Closes #2304

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Warning

Review limit reached

You'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 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 203d39a2-7e65-4cef-a58a-5fc53f052fbf

---

📥 Commits

Reviewing files that changed from the base of the PR and between 5797da5 and 7a128c5.

---

📒 Files selected for processing (6)

• scripts/engine.sh
• tests/dev-lead/integration/setup_suite.bash
• tests/dev-lead/unit/setup_suite.bash
• tests/dev-lead/unit/test_engine_child_env.bats
• tests/dev-lead/unit/test_token_ledger_isolation.bats
• tests/setup_suite.bash

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[don-petry]

dev-lead is withholding action on this item.

It is labeled dev-lead:hands-off (deliberately kept off-limits to dev-lead (e.g. changes to the dev-lead automation itself)), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the dev-lead:hands-off label.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T21:54:03Z.

[gemini-code-assist]

Code Review

This pull request implements token ledger isolation by introducing an _engine_child wrapper in scripts/engine.sh that unsets ledger-related environment variables before invoking model CLIs, preventing child processes from polluting the real token ledger. It also unsets these variables in BATS suite-level hooks and adds comprehensive unit tests to verify the isolation. The review feedback suggests optimizing a find command in the tests to avoid spawning multiple dirname processes, and replacing a non-idiomatic return 1 assertion in BATS with false.

[don-petry]

QA Lead — test-risk advisory

Risk tier: LOW. The production change is one env -u wrapper on five CLI launch sites. Stub tests cover every site, and each half of the fix was mutation-checked. Rollback is a single revert, and the engine.sh half doesn't reach production until dev-lead/v139-ring0 moves.

What I'd shore up (highest leverage first):

• Guard against the next launch site. The isolation test enforces "every bats directory has hooks", but nothing makes a new timeout … claude|gemini|gh copilot call in engine.sh go through _engine_child. Add a cheap static check that fails on any direct launch. A sixth site would leak again with every test still green.
• Run CI before merge. The PR says CI was not run, and the 25 s timing was measured on macOS. _fresh_bats uses env -i and strips BATS_LIBEXEC from PATH, which depends on how bats installs its launcher. Confirm it is deterministic on the Linux runner's bats version, so it doesn't become a flaky gate.
• Make the post-merge monitoring a check. "No fixture model names in the uploaded ledger" is currently something someone has to remember to look at. A deny-list assertion (gemini-test, gemini-u1, gemini-fixture-*) in the fix(gemini-ledger): hand the tier report only records traceable to trusted runs #2305 provenance report would turn it into a check. Fixture cooldowns in the real ledger can throttle real Gemini keys, so this is worth guarding.
• Minor: the nested-writer case (test 6) only drives the Claude path. One Gemini variant would close the symmetry cheaply.

Escalate? no

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture
(bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

[codeant-ai]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; model CLI subprocesses no longer inherit the token-ledger paths, while the parent shell retains them for recording.
• Suite hooks and focused tests cover inherited-ledger isolation across test directories and the Claude, Gemini, and Copilot call paths.

Assessed commit: ff15b1b583bd

[don-petry]

Review round 1, head 7a128c5b. From the QA advisory:

• Guard against the next launch site: added. A static test fails on any model CLI started in engine.sh outside _engine_child, whether under timeout or bare. Unwrapping one existing site, or appending a bare claude --print, each turns it red and names the line.
• CI before merge: the first CI run on this pull request passed unit on the Linux runner, including the nested runs, so the env -i launcher works with that bats version. It reruns on this head.
• Make the monitoring a check: the detector already exists in fix(gemini-ledger): hand the tier report only records traceable to trusted runs #2305 rather than as a deny-list: it counts any Gemini record whose model or key index has no row in the caps file, warns, and makes the dispatch-decision reader answer unknown. That catches every fixture model name without listing them. It cannot catch fixture records that use real model names, which is why the fix is here at the source.
• Not done: a Gemini variant of the nested-writer case. The three ledger variables are removed by one helper for every CLI, and the per-site tests cover Gemini's two paths.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T22:30:33Z.