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"