[AAASM-5529] ✅ (tests): Add enforcement-truth negative controls to the quick-start - #309
Merged
Chisanan232 merged 7 commits intoAug 6, 2026
Conversation
mypy 2.2.0 checks classmethod()'s argument against `def (type[Never], /, ...)`, so a named first parameter is rejected. The pre-commit mypy hook has been failing on remote/main since the bump, blocking every commit in this repo; the marker is runtime-inert (the argument was already positional). Unblocks AAASM-5529
Real, externally-observable side effects (a file on disk, a live loopback HTTP listener) plus an audit-recording interceptor that delegates every verdict to the real one. Existing deny tests assert over a closure-captured executed list, which proves the SDK did not call a function it holds — not that the effect the tool exists to produce was prevented. Refs AAASM-5529, Epic AAASM-5526
Runs the real init_assembly so the interceptor under test is the RuntimeQueryInterceptor the SDK actually builds, then drives the shared governed-tool chain so the SDK — not the test — decides whether the body runs. The side-effect assertion precedes the exception assertion so removing the deny fails the suite on the absence check. Refs AAASM-5529, Epic AAASM-5526
A real loopback listener records every request it receives, so the deny is asserted as zero deliveries rather than as a raised exception. The positive control on the same live fixture establishes reachability, which is what makes the empty request log evidence of prevention. Refs AAASM-5529, Epic AAASM-5526
Identity is read from the query the SDK presented to the authoritative runtime, not reconstructed by the test, and the allow control checks the same triple reaches the post-execution audit hook. An anonymous refusal is not usable evidence. Refs AAASM-5529, Epic AAASM-5526
…rough Under enforce with no agent_assembly._core installed the SDK has no authoritative verdict source. AAASM-5526 forbids that degraded path presenting as protected, so the control holds it to the same standard as every other one here: the file the tool would have written is absent. Refs AAASM-5529, Epic AAASM-5526
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TestDenyIsAttributable asserted isinstance(outcome, ToolExecutionBlockedError) ahead of the absence check, so the failed assert aborted the test before the side effect was ever inspected. Under the falsification mutation that control failed on "no error was raised" — the weak evidence this suite exists to replace — while its three siblings correctly failed on the side effect. Swapped to match the shape the other controls already use.
|
Chisanan232
deleted the
v0.0.1-rc.7/AAASM-5529/enforcement_negative_controls
branch
August 6, 2026 12:34
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
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.



Description
Adds enforcement-truth negative controls to the documented Python quick-start path (AAASM-5529, Epic AAASM-5526, Goal CBLPCRLM-13).
A negative control proves a denial prevented a side effect, not that a
DENIEDmessage was produced. Every existing deny test in this repo asserts either the returned verdict dict or an emptyexecutedlist captured by a closure. Both show the SDK did not call a function it holds a reference to; neither shows that the effect the tool exists to produce did not happen.Each control here:
init_assemblyso the interceptor under test is theRuntimeQueryInterceptorthe SDK actually builds (build_governance_interceptoris wrapped by a call-through spy, not replaced, so the real_register_adaptersruns);agent_assembly/adapters/_shared/tool_governance.py::run_governed_async_tool, the shared pre-execution gate behind the Google ADK and Pydantic AI quick-start tabs) so the SDK, not the test, decides whether the tool body runs;The side-effect assertion runs before the exception assertion on purpose. Asserting the exception first would short-circuit when enforcement is removed, leaving the absence check unexercised — the falsification run would then only prove "no error was raised".
Corrections to the first revision of this description
An independent review returned CHANGES-REQUIRED on three points. All three are recorded here rather than silently overwritten.
1. One control did not actually follow the ordering rule stated above.
TestDenyIsAttributable::test_the_runtime_saw_the_agent_and_tool_the_deny_was_decided_againstassertedisinstance(outcome, ToolExecutionBlockedError)beforeassert file_effect.occurred() is False. Under the falsification mutation that assert aborted the test first, so that control's absence check had never been shown to bite — it was passing for the weak reason this suite exists to eliminate. The other three deny controls were already correct. Fixed ina4027f2by swapping the two lines to match the shape the siblings already use. Before/after evidence below.The evidence of this was visible in the first revision of this description and was missed: the falsification block summarised 4 failed but quoted only 2 assertion messages, and the omitted pair contained the
isinstancefailure that contradicts the sentence immediately beneath it. A quoted failure list shorter than the failure count is not evidence for a claim about every control.2. The mypy numbers were wrong. The first revision claimed "exit 1, 9 errors, identical set to the baseline (9 errors / 186 files vs 9 errors / 188 files)". Re-measured with a cleared
.mypy_cacheon both sides, running the exact pre-commit gate, the real figures are branch: exit 0, 0 errors, 188 source files and pristinemain(a838c13): exit 1, 3 errors in 1 file, 186 source files. The substantive conclusion — zero new type errors introduced — holds and is in fact stronger than claimed, but "9 errors on the branch" was never reproducible.3. The bench exculpation was stated more strongly than the evidence supports. The first revision said both latency failures "reproduce running
test/bench/alone", implying determinism. They are flaky, not deterministic:test/benchalone reproduced them in 2 of 3 runs and was green in the third. The non-attribution conclusion still holds, and is now backed by the measurement that was missing — the same test fails on pristinemainwith none of this PR's files present. Numbers below.Falsification evidence
Produced by removing the deny in the SDK (not in the test) and re-running the whole control file:
Before
a4027f2— 4 failed, 5 passed, 4 rerun (exit 1). Three controls failed on the side effect; one did not:After
a4027f2— 4 failed, 5 passed, 4 rerun (exit 1). All four now fail on the absence assertion, and the third control's message has genuinely changed:Grepping the after-run output for
isinstance(outcomereturns 0 hits against a positive control of 4 hits foroccurred() is Falsein the same file, so the empty result is a real absence and not a broken probe.Source restored after each run in the same uninterruptible sequence; verified by
git status --porcelainbeing empty and a positive grep confirmingif status != "allow":is back at line 225, not merely that the mutation marker is gone.Type of Change
Breaking Changes
Related Issues
Sibling PRs (same ticket, one per SDK)
Recommended merge order: python-sdk → go-sdk → node-sdk. They are independent (no shared code), so the order only reflects reviewer load: this one and go-sdk are pure additions, while the node-sdk PR carries the findings that need a decision.
Acceptance-criteria mapping
TestFilesystemSideEffect,TestNetworkSideEffectoccurred()/content()/ listener request log; asserted before the exception in all four deny controls (see correction 1)TestDenyIsAttributable— identity read off the query the SDK presented to the authoritative runtime, plus the allow path'srecord_resultTestDegradedRuntimeCannotLookProtectedRuntimeQueryInterceptorinit_assemblybuilt one; the chain calls itscheck_tool_starttest/unit/core/_fake_core.py; no adapter code touchedNot covered here (reported to the ticket owner, not silently dropped): the CI job running the quick-start from a clean environment, and doc-snippet drift gating of the new controls.
Testing
How to read the pass counts below:
pytest.inisets--reruns 1, so every quoted count is pass-after-retry — a test that failed once and passed on the retry is reported as passed, with arerunin the summary line. Also,-p no:randomly(used in the first revision's commands) is inert in this repo:pytest-randomlyis not installed, so it disables nothing and ordering is plain collection order.All measured at
a4027f2on macOS 15 / Python 3.13.3 / mypy 2.2.0..venv/bin/python -m pytest test/unit/test_quickstart_negative_control.py --no-cov -q.venv/bin/python -m pytest test/ --no-cov -q.venv/bin/python -m pytest test/ --ignore=test/bench --no-cov -q.venv/bin/ruff check ..venv/bin/ruff format --checkon the 3 changed files.venv/bin/ruff format --check .scripts/check_contact_metadata.pywould be reformatted. Pre-existing; fails identically onmain, untouched here.uv run mypy --ignore-missing-imports --show-traceback(the exact pre-commit hook),.mypy_cacheclearedSuccess: no issues found in 188 source filesmaina838c13,.mypy_cacheclearedFound 3 errors in 1 file (checked 186 source files), all three intest_runner_spawn_patch.pyuv run pre-commit run --files <the 3 changed files>ruff check,ruff format,mypy, …)The branch's type-error set is empty, so it is trivially a strict subset of
main's three. The three baseline errors are exactly the ones the included fix below removes.Benchmark flakiness (not attributable to this PR)
test/bench/test_latency_contracts.pyasserts wall-clock latency budgets and is timing-flaky on a loaded workstation. Measured three consecutive runs on each side,pytest test/bench --no-cov -q:a4027f2maina838c13test_init_assembly_coldstart_latencyfails on pristinemain, with none of this PR's files collected — so the failure class predates this branch.test_detection_latency_under_50msalso tripped on the branch runs; both are wall-clock budget assertions on a machine under load, and neither timing is cited as evidence anywhere in this PR. The full-suite run above happened to be green on all of it.Included unrelated fix (called out, not hidden)
🚨 (test): Mark patched Runner.run stubs positional-only for mypy 2.2— mypy 2.2.0 checksclassmethod()'s argument againstdef (type[Never], /, ...), so a named first parameter is rejected. The pre-commitmypyhook fails on a cleanremote/mainwith exactly these 3 errors (measured above), which blocks every commit in this repo (--no-verifyis not an option). The/marker is runtime-inert: every call site passes the agent positionally, andinspect.signaturestill yields['agent', 'input', 'kwargs']. Happy to split this into its own PR if preferred.Checklist