Test sweep: the offline scenarios must actually run — one was failing silently - #24
Merged
Merged
Conversation
…iling silently
A full test sweep (unit + regression + property-at-5000 + E2E) found the suite green at
3668 and the make targets green, then executed the `scenarios/` scripts themselves — a
layer nothing covered — and found that FIVE scenarios had no test coverage of any kind and
one of them had been failing offline.
`scenario_agent_authored_loop.py` exited 1 while still writing `evidence/*.json`, so a
committed artifact that claims to prove the happy promotion path actually recorded it
being REFUSED:
happy_promotion ok=False promoted=False refused=1
reason: "safety data missing: dimension(s) ['groundedness'] were never scored
— a judge's silence is not a pass (fail-closed)"
Not a code defect: INV-PROMOTE-3 is correct to refuse it. The scenario's `_passing_eval`
stub scored `correctness` and `safety` but never `groundedness`, and went stale when M18.1
added that requirement. Confirmed pre-existing by re-running on f5daa30 (round 20) —
identical failure, so it is not fallout from round 21's INV-OPS-7.
Nothing caught it because nothing ran the scenario. `make test` covers `tests/`; the
scenarios are only exercised by hand. That is the INV-AUDITMAP shape one level over: an
artifact asserting a claim, with no mechanism checking the assertion still holds.
tests/test_scenarios_execute.py closes it. Every scenario is now classified by RUNNING it
(an earlier version of the sweep grepped for the word "offline" and mis-classified three
live scenarios, because their prose mentions offline mode — recorded in the module):
13 offline-runnable -> executed in CI with AWS_*/SENTINEL_* stripped, exit 0 asserted
10 live-AWS -> exempted, each naming the precondition it needs
An unclassified scenario now fails the build, so a new script is either hermetic or a
decision someone recorded. The agent_authored_loop assertion reads the EVIDENCE FILE —
the artifact a reader trusts — and also asserts the three NEGATIVE paths still refuse, so
a stub change that made everything pass would not satisfy it. Positive-controlled:
reintroducing the stale stub fails both the run and the evidence check.
Also refreshes `evidence/feedback_loop_result.json`, which was stale in the same way but
with exit 0, so even less visible. Its old `sigma_filter_yaml` used
`dst_domain|endswith: 'assets.example.com'` — which matches `evilassets.example.com`,
having no dot boundary. The code was fixed to emit an apex + `.suffix` pair; the evidence
was never regenerated. That scenario is in the offline set now, so it stays current.
Verification: 3693 passed / 6 skipped (was 3668), green under random order; 42 property
assertions at max_examples=5000 (a confirmed negative, not an unsearched one); make smoke,
make demo, make seed-registry (dual-gate ok, 19 live), make create-harnesses (DRY_RUN) all
exit 0; ruff clean; both mypy gates clean; make ci green.
CI failed on all four Python versions with returncode 255: the new scenario tests hardcoded ["uv", "run", "python", ...] and CI has no `uv`. That is the FIFTH occurrence of this exact mistake, and the first one made in a test written AFTER round 20 built `tests/child_pytest.py` specifically to prevent it. The lesson that keeps not sticking is sharper than "resolve the launcher": a helper only prevents the recurrence for the call shape it EXPORTS. `child_pytest` exported "run pytest" and nothing else, so needing "run a python script" I copied the pattern instead of reusing the resolution. So the resolution is generalized rather than duplicated. `child_pytest` now also exports `resolve_python_launcher()` / `run_python_script()`, which probe candidates in order (parent interpreter, `uv run python`, `python3`) and — importantly — the probe requires the candidate to IMPORT sentinel_harness, not merely to start. A bare `python3` on PATH may exist with none of the dependencies, and a launcher that starts but cannot import turns every scenario into a false failure. As with the pytest side, a launcher that cannot start raises `ChildNeverRan` rather than returning an exit code, so "the child never ran" can never be read as "the script failed". Also fixes an env bug in my own first version: it rebuilt the child environment from a four-key allowlist (PATH/HOME plus two), which stripped UV_*/VIRTUAL_ENV and was part of why the child could not start. It now drops exactly the two families that could make a live-only scenario look hermetic — AWS_* and SENTINEL_* — and keeps the rest. Verified in a CI-shaped environment with uv removed from PATH entirely: the launcher resolves to .venv/bin/python, the child runs and produces output, and all 25 scenario tests pass. 3693 passed overall; ruff clean.
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.
What the sweep did
A full test sweep — unit + regression + property-at-5000-examples + E2E — found the suite green at 3668 and every
maketarget green, then went one layer further and executed thescenarios/scripts themselves. Nothing covered that layer.Five scenarios had no test coverage of any kind, and one of them had been failing.
The finding
scenario_agent_authored_loop.pyexited 1 while still writing itsevidence/*.json, so a committed artifact that claims to prove the happy promotion path actually recorded it being refused:Not a code defect. INV-PROMOTE-3 is right to refuse it. The scenario's
_passing_evalstub scoredcorrectnessandsafetybut nevergroundedness, and went stale when M18.1 added that requirement. Confirmed pre-existing by re-running onf5daa30(round 20) — identical failure, so it is not fallout from round 21.Nothing caught it because nothing ran the scenario.
make testcoverstests/; the scenarios are exercised only by hand. That is the INV-AUDITMAP shape one level over: an artifact asserting a claim, with no mechanism checking the assertion still holds.The fix
tests/test_scenarios_execute.py. Every scenario is classified by running it — an earlier pass of this sweep grepped for the word "offline" and mis-classified three live scenarios because their prose mentions offline mode, which is recorded in the module:AWS_*/SENTINEL_*stripped from the environment, exit 0 assertedAn unclassified scenario now fails the build, so a new script is either hermetic or a decision someone recorded.
The
agent_authored_loopassertion reads the evidence file — the artifact a reader trusts — and also asserts the three NEGATIVE paths still refuse, so a stub change that made everything pass would not satisfy it. Positive-controlled: reintroducing the stale stub fails both the run and the evidence check.A second stale artifact, caught by the same mechanism
evidence/feedback_loop_result.jsonwas stale in the same way but with exit 0, so even less visible. Its oldsigma_filter_yamlwas:No dot boundary — that filter would suppress an attacker-controlled lookalike as "known good". The code was fixed to emit an apex +
.suffixpair; the evidence was never regenerated, so the committed artifact still showed the vulnerable output. That scenario is in the offline set now, so it stays current.Verification
max_examples=5000— a confirmed negative, not an unsearched onemake smoke(14 passed, zero AWS),make demo,make seed-registry(dual-gate ok, 19 live,web_searchcorrectly pending),make create-harnesses(DRY_RUN) — all exit 0ruffclean · both mypy gates clean ·make cigreen (lint / coverage ≥88 / iac synth / secret-scan)