fix: bring e2e tests into default CI, fix 3 stale against attribution hardening - #130
Merged
Merged
Conversation
… hardening (#127) Two example-bench e2e tests had gone stale against the intentional attribution-hardening commit 28337d1 (agent-declared intent/mode beat payload-supplied values; only user_id/user are trusted from an untrusted payload) without anyone noticing, because tests/examples/ was excluded from CI via addopts. Narrows addopts from "-m 'not e2e and not live'" to "-m 'not live'" so this class of regression can't go unnoticed again; none of the 10 non-live e2e tests need a network call or API key. - test_triad_e2e.py::test_triad_pipeline_completes_with_ledger: assert the agent's own instrument_app(intent=...) wins, not the payload value - test_triad_e2e.py::test_triad_cost_not_double_counted_without_parent_rollup: patch build_price_book on tokenops.control.run (where it's resolved), not on the example server module - test_bench_e2e.py: renamed test_run_dims_persisted_for_segmentation to test_run_dims_only_allowlisted_payload_keys_persist and rewrote it to assert the intended, hardened contract Full suite: 247 passed, 16 skipped, 1 deselected. ruff check/format clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
susheem-k
added a commit
that referenced
this pull request
Sep 12, 2026
…cope (#118) First increment of tokenops#118 Phase 2 — two small, independent, mechanical pieces from the locked decisions, landed ahead of the larger Ledger/LedgerView backend rewire: - Ledger.RunState -> LocalRunState (decision #9): makes the two-tier model explicit — this is the per-process Tier-1 cache, not the plane's authoritative run_state. Pure rename, no behavior change. - PolicyInstance.data_scope: local | global, default local (decision #5). Persisted in the local Store (additive migration, mirrors the pattern already used for dims/parent_span/governance_events) and round-tripped through governance_config_for. Not yet consumed by build_governor/Governor — grouping detectors by data_scope is part of the LedgerBackend rewire. Full suite: 237 passed, 16 skipped, 11 deselected (matches main's current baseline pre-#130). ruff check/format and mypy clean on touched files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tishachawla-jg
approved these changes
Sep 12, 2026
susheem-k
added this pull request to stack #135
September 12, 2026 12:24
susheem-k
added a commit
that referenced
this pull request
Sep 12, 2026
…cope (#118) First increment of tokenops#118 Phase 2 — two small, independent, mechanical pieces from the locked decisions, landed ahead of the larger Ledger/LedgerView backend rewire: - Ledger.RunState -> LocalRunState (decision #9): makes the two-tier model explicit — this is the per-process Tier-1 cache, not the plane's authoritative run_state. Pure rename, no behavior change. - PolicyInstance.data_scope: local | global, default local (decision #5). Persisted in the local Store (additive migration, mirrors the pattern already used for dims/parent_span/governance_events) and round-tripped through governance_config_for. Not yet consumed by build_governor/Governor — grouping detectors by data_scope is part of the LedgerBackend rewire. Full suite: 237 passed, 16 skipped, 11 deselected (matches main's current baseline pre-#130). ruff check/format and mypy clean on touched files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
susheem-k
added a commit
that referenced
this pull request
Sep 12, 2026
…cope (#131) * refactor: rename RunState to LocalRunState; add PolicyInstance.data_scope (#118) First increment of tokenops#118 Phase 2 — two small, independent, mechanical pieces from the locked decisions, landed ahead of the larger Ledger/LedgerView backend rewire: - Ledger.RunState -> LocalRunState (decision #9): makes the two-tier model explicit — this is the per-process Tier-1 cache, not the plane's authoritative run_state. Pure rename, no behavior change. - PolicyInstance.data_scope: local | global, default local (decision #5). Persisted in the local Store (additive migration, mirrors the pattern already used for dims/parent_span/governance_events) and round-tripped through governance_config_for. Not yet consumed by build_governor/Governor — grouping detectors by data_scope is part of the LedgerBackend rewire. Full suite: 237 passed, 16 skipped, 11 deselected (matches main's current baseline pre-#130). ruff check/format and mypy clean on touched files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat: wire Ledger onto LedgerBackend (precheck/apply_events) alongside Store (#118) Adds a backend= mode to Ledger, additive alongside the existing store=/in-memory modes (mutually exclusive with store=) — the core Ledger/LedgerView piece of the remote-only rewrite's Phase 2 checklist. build_governor/ControlPlaneClient don't construct it yet; that swap, and migrating the ~18 Store-dependent tests, are the next increments (staged deliberately, per the epic's own Phase 2/3 split). - record(): batches the step + spent_add (when cost > 0) into one apply_events call sharing an idempotency seq, so the ack's totals cover the run-total cum spend without a separate read_state round trip for the common case. - admit/complete/mark_halted/clear_halt: one-shot apply_events writes. Their idempotency keys are random (uuid4), not the contract's deterministic {run_id}:...:{seq} recipe — that recipe needs a real run_id to stay collision-free across processes for a non-run-scoped segment key (e.g. an agent-dimension concurrency cap shared across runs), which isn't always available at these call sites. A random key is correct for a one-shot, non-retried write either way; the deterministic recipe only earns its keep once a buffered backend needs a retried flush to regenerate the same key (the existing # TODO(buffering) seam in ledger_backend.py). - cost_micros/budget_left/is_halted/inflight: read via backend.read_state (precheck). No per-call batching across detectors yet (one read per method call) — Governor grouping detectors by data_scope into a single precheck per governed moment is a separate, later optimization, not required for correctness given there's no client-side write buffer yet either (decision #6). - velocity/recent/window/step_count are unchanged: always Tier-1 LocalRunState, never a backend round trip — those are inherently per-process reads. tests/test_ledger_backend_mode.py: 9 new tests against FakeLedgerBackend, including two Ledger instances sharing one backend (the actual cross-process scenario this whole rewire is for) for halt visibility and inflight counting. Full suite: 246 passed, 16 skipped, 11 deselected. ruff check/format and mypy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat: eliminate the local ledger — tokenops is now hard-dependent on the control plane (#118) The core swap for tokenops#118: ControlPlaneClient.from_env() now requires CONTROL_PLANE_URL/TOKENOPS_URL and raises if neither is set. There is no more TOKENOPS_EMBEDDED and no code path left that silently falls back to a local SQLite ledger. build_governor/tokenops_run construct Ledger(backend=...) (an HttpLedgerBackend, i.e. real precheck/events:batch traffic) for every live run. - client.py: from_env() requires a URL; new .backend property (lazy HttpLedgerBackend); should_mount_run_registration() always False (no embedded mode left to self-host POST /v1/runs under). The store= constructor kwarg remains as an explicit, visible test-only escape hatch — never reachable from from_env(), so no env misconfiguration can select it. - config.py/run.py: build_governor/build_governance_stack take backend=; run.py picks store vs backend off client_obj.embedded (not the local store= param, which was a real bug in an earlier draft of this change — a client= constructed with store= wasn't routing to its own Store). - http_store.py: HttpStore accepts an injectable httpx.Client (mirrors HttpLedgerBackend) so tests can point it at an in-process app. - dev_plane.py (new): launches a real agentplane-control-plane on a real localhost TCP port, in-process. Used by tokenops.demo (still zero-setup: it launches its own throwaway plane and configures it over HTTP) and by tests/conftest.py::live_plane_url for tests that must exercise ControlPlaneClient.from_env() itself (an in-process ASGI app isn't reachable that way since from_env() builds its own plain httpx.Client). - CI now installs agentplane-control-plane from the control-plane repo (git+.../control-plane@main) alongside .[dev,contract] — the [contract] tests and tests/examples/ e2e suite now actually run a real control plane in CI instead of importorskip-ing past it. They're load-bearing coverage now. - Migrated every TOKENOPS_EMBEDDED-dependent test (test_tokenops_run.py, test_control_plane_client.py, test_control_plane_app.py, test_governance_config_cache.py, test_demo.py, and the two tests/examples/ e2e files) onto either the explicit store= escape hatch or live_plane_url. - tests/examples/test_bench_e2e.py and test_triad_e2e.py now configure their policies/budgets on a real, in-process control plane over its own HTTP API (PUT /v1/budgets, PUT /v1/policies) instead of a local Store — this is the concrete demonstration that governance policies configured on the plane reach a live, multi-agent run and HALT/steer it (step_cap, cost_budget, output_runaway CANCEL+RETRY, tool_output_cap deep swap). - Removed the now-dead TOKENOPS_EMBEDDED convenience shims from the three example CLI clients (a2a, brief, triad) — nothing reads that var anymore. - README/onboarding/control-plane-deploy docs: removed the stale TOKENOPS_EMBEDDED documentation and precedence warnings. Full suite: 258 passed, 16 skipped, 1 deselected. ruff check/format clean. mypy clean on every touched file. Not in this change (separate, later Phase 2/3 checklist items): removing the bundled src/tokenops/server + src/tokenops/ui/streamlit, dropping span_id, Governor batching detectors by data_scope into one precheck per call, and migrating the ~18 policy-logic unit tests that construct Ledger()/Store() directly for white-box testing (not a production bypass — never reachable from ControlPlaneClient.from_env()). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: fix Quickstart to reflect the hard dependency on a running control plane Two active falsehoods left over from the previous pass on this file (docs/env-var table only, not the Quickstart copy itself): - Section 1's manual snippet called ControlPlaneClient.from_env() with no mention a control plane must already be running — it now raises immediately without one. - The Quickdeploy tip claimed "a single-process agent doesn't need it running at all", which was true under the old embedded-ledger fallback and is false now that tokenops has no ledger of its own (#118). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
31 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Establishes a 100%-passing regression baseline (including the example-bench e2e suite) before starting the remote-only "swap" work in #118.
tests/examples/was excluded from CI viaaddopts, so two tests silently drifted out of sync with the intentional attribution-hardening commit28337d1and nobody noticed.Closes #127.
test_triad_e2e.py::test_triad_pipeline_completes_with_ledger: asserted the old (pre-hardening) precedence where a payload-suppliedintentwins. The actual/intended contract is that the agent's owninstrument_app(intent=...)always wins over a payload value. Fixed the assertion.test_triad_e2e.py::test_triad_cost_not_double_counted_without_parent_rollup: monkeypatchedbuild_price_bookon the example server module, which never imports that name — pricing is resolved lazily intokenops.control.run. Fixed to patch the correct module.test_bench_e2e.py::test_run_dims_persisted_for_segmentation→ renamedtest_run_dims_only_allowlisted_payload_keys_persistand rewritten to assert the intended, hardened contract: only allow-listed payload keys (user_id/user) reachRunRecord.dims; an arbitrary tag (team) must not leak into segmentation (attribution._PAYLOAD_USER_DIM_ALLOWLIST).pyproject.toml: narrowedaddoptsfrom-m 'not e2e and not live'to-m 'not live'—e2e(real servers, real Governor/policy HALT/MUTATE, no network/API key needed) is now part of the default suite and CI, so this class of regression can't go unnoticed again.live(needs real keys/vendored frameworks) stays excluded.docs/testing.mdandCHANGELOG.mdto match.No production code changed — this is test-suite and CI-config only.
Test plan
python -m pytest -v -m "e2e or live"→ 10 passed, 1 skipped (only the key-gatedtest_live_baseline_smokeskips)python -m pytest -q(full default suite) → 247 passed, 16 skipped, 1 deselectedruff check src tests examplescleanruff format --check src tests examplesclean🤖 Generated with Claude Code