fix(engine): keep the token ledger out of test suites and out of the model CLI's environment

Merged
DO
10d ago
+287-5

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 #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