From 92e7cd6902b906203af999a3ec4409ef776418c1 Mon Sep 17 00:00:00 2001 From: Trevor Hudson Date: Thu, 30 Jul 2026 16:45:56 -0700 Subject: [PATCH] Make it impossible for a test to spend inference tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the project's dividing line as a mechanism rather than a convention: anything that does not require inference is a TEST (automated, fast, every push); anything that costs tokens is an EVAL (deliberate, periodic, never in CI). The suite already complied — the live e2e suites use fake keys and point ROTE_BASE_URL_ at a local Messages stub. It complied by convention, which is not the same as being unable to do otherwise. Two DBOS e2e suites previously burned real subscription inference on every run, and that was caught by wall-clock timing rather than by any assertion, because the failure mode of this bug is silent spending. Two independent leaks, both closed in conftest: 1. **Credentials in the environment.** A developer running the suite with real keys exported would have any SDK constructed without an explicit key bill them. All vendor credential vars are now scrubbed per-test. 2. **A subscription CLI on PATH.** This is the one with nothing to scrub: `claude -p` authenticates from an OAuth session, so having no API key is not protection — not finding the binary is. Bare-name `shutil.which("claude" | "codex")` now returns None. Verified adversarially rather than by inspection. With the guard, a probe test cannot find the CLI and sees no credentials; without it, both succeed — there IS a claude binary on PATH on this machine, so this closes a live hole, not a theoretical one. Deliberately narrow in two directions, because an over-broad guard gets disabled: - Only BARE-NAME lookups are blocked. Several driver tests write a fake `claude` script under tmp_path and pass its absolute path as `executable=`; that is a stub, not a subscription, and it still resolves. The first attempt blocked those too and broke 9 tests. - Every other `which` delegates, so the e2e suites' node/npm/docker probes are untouched — blocking those would trade a spending bug for a silent-skip coverage bug, which is the trade this project keeps having to undo. `tests/test_no_inference_in_tests.py` verifies the guard itself, including that an opt-in test can still patch the lookup back: the guard is a default, not a cage. CLAUDE.md gains a "Tests vs. evals" section stating the rule, what enforces it, and the two consequences that are easy to get wrong — an e2e test is still a test (real npm, wrangler, Postgres, no inference → automated), and `@pytest.mark.slow` says what toolchain a suite needs, never whether it should run. 1175 fast + 27 slow pass; ruff, strict mypy, sanity-check clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SBuSDL7Svm7iDzgmhTpDSW --- CLAUDE.md | 47 ++++++++++++++- tests/conftest.py | 65 +++++++++++++++++++++ tests/test_no_inference_in_tests.py | 89 +++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 tests/test_no_inference_in_tests.py diff --git a/CLAUDE.md b/CLAUDE.md index 324cc4b..b240942 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -798,6 +798,51 @@ If a change of yours would violate any of them, stop and reconsider. --- +## Tests vs. evals: the dividing line is inference + +**Anything that does not require inference is a test.** Automated, run +on every push, and kept fast. + +**Anything that costs tokens is an eval.** Run deliberately and +periodically — `rote eval --run`, the compiler against a real skill, +a live gateway judge. Never wired into CI. + +This is not a style preference. A token-spending check in an automated +suite bills someone on every push, gets slower as it grows, and fails +for reasons that have nothing to do with the change under test. Both +halves of the split are enforced, not just documented: + +- `tests/conftest.py::_no_inference_in_tests` scrubs every vendor + credential from the environment and makes a bare-name `shutil.which` + lookup of `claude` / `codex` return None. The second half is the one + that matters: `claude -p` authenticates from an OAuth session, so + having no API key is *not* protection — not finding the binary is. + Two DBOS e2e suites burned real subscription inference on every run + until it was caught by wall-clock timing. +- `tests/test_no_inference_in_tests.py` verifies the guard itself, + because its failure mode is silent spending that no other assertion + would notice. +- Only *bare-name* lookups are blocked, so a test that writes a fake + `claude` stub and passes its absolute path still works — a guard that + forced tests to route around it would soon be disabled. + +Consequences worth keeping in mind: + +- **An e2e test is still a test.** The TypeScript suites run real + `npm install`, real `wrangler dev`, a real Inngest dev server and a + real Postgres — no inference, so they are automated (see the + `TypeScript e2e` / `DBOS-TS e2e` CI jobs). `@pytest.mark.slow` says + what *toolchain* a suite needs, never whether it should run. +- **Emitted judges and agent loops get stubbed, not called.** The live + suites point `ROTE_BASE_URL_` at a local Messages stub and + set a fake key. That is what makes it possible to prove the agent-loop + machinery end to end without paying per run. +- **Speed is a feature of the test half only.** The fast suite is ~18s; + keep it there. Evals are allowed to be slow and expensive because + nothing blocks on them. + +--- + ## Testing discipline: make assertions capable of failing Five real defects in this repo shipped past a green suite because a @@ -1164,7 +1209,7 @@ Don't waste time debugging stubs. These are intentional. pipeline with cross-process gate signaling via `DBOSClient`; real judge usage captured through the emitted `$ROTE_USAGE_LOG` hook; measurements appended to `~/.local/share/rote/eval-corpus.jsonl`) -- 1197 tests (1170 fast + 27 slow). Run with `pytest tests/` (fast +- 1202 tests (1175 fast + 27 slow). Run with `pytest tests/` (fast only — what runs by default). Slow tests cover the runtime e2e suites (Temporal, Cloudflare, DBOS, DBOS-TS, Inngest, MCP-over-stdio); the TS ones require a Node toolchain, DBOS-TS diff --git a/tests/conftest.py b/tests/conftest.py index 2bea300..c2608fa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,8 @@ from __future__ import annotations +import os +import shutil from pathlib import Path import pytest @@ -39,6 +41,69 @@ def _isolated_mcp_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None ) +#: Credentials that would let a test reach a real model. Scrubbed from +#: every test's environment — see :func:`_no_inference_in_tests`. +_INFERENCE_CREDENTIAL_VARS = ( + "ANTHROPIC_API_KEY", + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_BASE_URL", + "OPENAI_API_KEY", + "OPENAI_BASE_URL", + "ROTE_INFERENCE", + "ROTE_CLOUD_TOKEN", +) + +#: Subscription-auth CLIs. These are the dangerous ones: `claude -p` +#: needs no API key at all, so a test that merely *finds* the binary +#: spends the developer's subscription with nothing in the environment +#: to scrub. Exactly how two DBOS e2e suites quietly burned real +#: inference on every run until it was caught by timing. +_SUBSCRIPTION_CLIS = frozenset({"claude", "codex"}) + + +@pytest.fixture(autouse=True) +def _no_inference_in_tests(monkeypatch: pytest.MonkeyPatch) -> None: + """A test may never spend tokens. + + The project's split: anything that does not require inference is a + **test** — automated, fast, run on every push. Anything that costs + tokens is an **eval** — run deliberately and periodically, never + from CI. This fixture makes the first half impossible to violate by + accident instead of merely documented. + + Two independent leaks, both closed here: + + 1. **Credentials in the environment.** A developer running the suite + locally has real keys exported; a vendor SDK constructed without + an explicit key picks them up silently. + 2. **A subscription CLI on PATH.** `claude -p` authenticates from an + OAuth session, so no env var gates it — the only defense is not + finding the binary. `shutil.which` is wrapped rather than + replaced so that lookups the e2e suites depend on (node, npm, + docker, wrangler) still resolve normally. + + Only a *bare name* lookup is blocked, because only that consults + PATH and can reach the developer's real install. Several tests build + a fake `claude` script under tmp_path and pass its absolute path as + `executable=`; that is a stub, not a subscription, and it still + resolves. + + A test that legitimately needs one of these monkeypatches it back: + this fixture runs first, so a test-local patch wins. + """ + for var in _INFERENCE_CREDENTIAL_VARS: + monkeypatch.delenv(var, raising=False) + + real_which = shutil.which + + def _which_without_subscription_clis(cmd: str, *args: object, **kwargs: object): # noqa: ANN202 + if isinstance(cmd, str) and os.sep not in cmd and cmd in _SUBSCRIPTION_CLIS: + return None + return real_which(cmd, *args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(shutil, "which", _which_without_subscription_clis) + + @pytest.fixture(scope="session") def bdr_pipeline() -> Pipeline: """The canonical BDR pipeline — the IR that exercises all five node kinds. diff --git a/tests/test_no_inference_in_tests.py b/tests/test_no_inference_in_tests.py new file mode 100644 index 0000000..5028588 --- /dev/null +++ b/tests/test_no_inference_in_tests.py @@ -0,0 +1,89 @@ +"""The automated suite must not be able to spend inference tokens. + +The project's split: + +* **test** — needs no inference. Automated, fast, runs on every push. +* **eval** — costs tokens. Run deliberately and periodically + (``rote eval --run``), never from CI. + +`conftest._no_inference_in_tests` enforces the first half. These tests +verify the enforcement itself, because a guard nobody checks is a guard +that quietly stops working — and the failure mode is silent spending, +which no assertion elsewhere would ever notice. + +This is not hypothetical: two DBOS e2e suites burned real subscription +inference on every run until it was caught by wall-clock timing, and a +`claude` binary is on PATH on the maintainer's machine right now. +""" + +from __future__ import annotations + +import os +import shutil + +import pytest + + +def test_the_subscription_cli_is_unreachable_from_a_test() -> None: + """`shutil.which("claude")` is how the subscription lane finds its CLI. + + This is the leak with no environment variable to scrub: `claude -p` + authenticates from an OAuth session, so possessing no API key is not + protection. Not finding the binary is the only defense. + """ + assert shutil.which("claude") is None, ( + "a test can reach the real Claude CLI — an emitted judge or agent " + "loop reaching the subscription lane would spend real tokens with " + "nothing in the environment to stop it" + ) + assert shutil.which("codex") is None + + +def test_real_vendor_credentials_are_invisible_to_a_test() -> None: + """A vendor SDK built without an explicit key reads these from the env.""" + for var in ( + "ANTHROPIC_API_KEY", + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_BASE_URL", + "OPENAI_API_KEY", + "ROTE_CLOUD_TOKEN", + ): + assert os.environ.get(var) is None, ( + f"{var} is visible to tests — a developer running the suite with " + f"real credentials exported would bill them" + ) + + +def test_the_guard_does_not_break_toolchain_probes() -> None: + """The e2e suites gate on node/npm/docker; those must still resolve. + + A guard that blocked every `which` would turn the TypeScript e2e + suites into silent skips — trading a spending bug for a coverage + bug, which is exactly the trade this project keeps having to undo. + """ + assert shutil.which("python3") is not None + + +def test_a_test_supplied_stub_binary_still_resolves(tmp_path) -> None: # noqa: ANN001 + """Only bare-name PATH lookups are blocked. + + Several driver tests write a fake `claude` script and pass its + absolute path as ``executable=``. That is a stub, not a + subscription, and blocking it would force those tests to work around + the guard — the usual first step toward disabling it. + """ + stub = tmp_path / "claude" + stub.write_text("#!/bin/sh\necho stub\n", encoding="utf-8") + stub.chmod(0o755) + assert shutil.which(str(stub)) == str(stub) + + +def test_an_opt_in_test_can_restore_what_it_needs(monkeypatch: pytest.MonkeyPatch) -> None: + """The guard is a default, not a cage. + + A test that genuinely needs the lookup patches it back; the autouse + fixture runs first, so a test-local monkeypatch wins. Verifying this + keeps the guard from being seen as something to route around. + """ + monkeypatch.setattr(shutil, "which", lambda name, *a, **k: f"/fake/{name}") + assert shutil.which("claude") == "/fake/claude"