Close the model-approval gate's boot hole, and test the chain at its joins - #777
Merged
Conversation
The lookup family has a two-layer posture that deps.py states in prose: the make_*_kernel primitives default to disarmed stubs so an integration test never seeds what it does not exercise, and the fail-loud requirement lives one layer up, in build_kernel's Postgres branch, which raises when a factory a real deployment must bind is missing. language_model_lookup had its first half and never got its second. The obligation is recent: until seed_agent started checking a seeded Agent's model, the seeds appended directly and no seeded agent was gated at all, so a missing lookup was genuinely harmless. Arming that gate is what created the Layer B obligation, and nothing discharged it. The stub does not weaken this gate, it removes it. AlwaysApproved answers every identity with an Approved entry, so define_agent and seed_agent both keep passing and nothing anywhere records that no catalog was consulted. A disarmed gate and an approving one are indistinguishable from the outside, and this gate is the only place the facility's model-provenance constraint is enforced. So the absence stops startup instead of reading as agreement. Operationally this costs nothing: main.py binds the factory in source, not configuration, so no operator can leave it unset. The guard fires only if that binding is deleted, which is what it is for. Three docstrings claimed the settled posture was permissive. The kernel field's said it "mirrors the spend_lookup opt-in posture", citing as authority for permissiveness the one field in the family with no default at all and a named place in this same guard. Reading half that precedent is the plausible route to the gap, so all three now state both halves. Verified by mutation: disabling the guard fails exactly the new test and nothing else. 52,318 unit + architecture passing. Not covered, and deliberately left for the chain test: no CI check asserts that main.py binds this factory, or the financial pair either. Deleting the binding is caught at boot, not in the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seeding a LanguageModel-brained Agent reaches its verdict through five links: seed_language_models appends the catalog streams, the projection turns those events into proj_agent_language_model_summary rows, drain_projections advances the bookmark so the rows are visible, PostgresLanguageModelLookup reads them, and seed_agent's gate consumes the answer. Every link had a test. Nothing crossed a join. The adapter's own integration test is the clearest case: it fills the read model by hand-written INSERT, so it proves the SQL reads a correct table and cannot see whether anything fills it. The drain covering THIS projection was asserted nowhere, because both architecture guards read main.py and assert about main.py, which can show a drain call sits between the two seeds but not that it reaches the table the gate will read. So the whole sequence could break with a green suite. Dropping the projection from register_agent_projections is the one-line version, and until now it failed nothing while making a fresh boot refuse both shipped agents that declare real Anthropic models. Three tests, reproducing the composition root's real sequence with the REAL adapter bound, which build_postgres_deps otherwise leaves at the always-approved stub: the chain admits CautionDrafter; the same kernel REFUSES when the drain is removed, which is what makes the drain load-bearing rather than incidental; and the drain fills the specific identity the gate asks for, so a failure names the broken link. Verified by mutation, both directions. Dropping the projection from the registry fails two of the three while the architecture guard and the adapter integration test both stay GREEN, which is the gap stated as an experiment. Swapping the real adapter back to the stub fails exactly the refusal test. That second mutation also shows the end-to-end test passes under the stub on its own, so its docstring now says it is only meaningful paired with the refusal test rather than letting a later reader take it for an independent check. Adds a language_model_lookup passthrough to build_postgres_deps; the binding had no way in before. 1,377 integration + 52,350 unit and architecture passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
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
1 participant
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.
Item 3 of the brain-arc follow-ups. Two commits, one knot.
The defect
deps.pydocuments a deliberate two-layer posture: themake_*_kernelprimitives default to disarmed stubs so an integration test never seeds what it does not exercise, and the fail-loud requirement lives one layer up, inbuild_kernel's Postgres branch, which raises when a factory a real deployment must bind is missing.language_model_lookuphad its first half and never got its second. The obligation is recent: untilseed_agentstarted checking a seeded Agent's model, the seeds appended directly and no seeded agent was gated at all, so a missing lookup was genuinely harmless. Arming that gate created the obligation, and nothing discharged it.The stub does not weaken this gate, it removes it.
AlwaysApprovedanswers every identity with an Approved entry, sodefine_agentandseed_agentboth keep passing and nothing records that no catalog was consulted.Three docstrings said the settled posture was permissive. The kernel field's cited "the
spend_lookupopt-in posture" as authority, andspend_lookupis the one field in the family with no default at all plus a named place in this same guard. Reading half that precedent is the plausible route to the gap.The second half
Seeding an LLM-brained Agent reaches its verdict through five links: seed the catalog streams, project them into
proj_agent_language_model_summary, drain so the rows are visible, read them, gate on the answer. Every link had a test. Nothing crossed a join.The adapter's integration test fills the read model by hand-written INSERT, so it proves the SQL reads a correct table and cannot see whether anything fills it. The drain covering this projection was asserted nowhere: both architecture guards read
main.pyand assert aboutmain.py.That matters more after commit 1, which is the honest argument for doing both together. Before, a broken chain was quiet, because the stub approved everything. Now the gate is guaranteed armed, so a broken chain refuses boot, and the two agents that declare real Anthropic models are exactly the ones refused.
Verification
Mutation, both directions:
register_agent_projectionsfails two of the three new tests while the architecture guard and the adapter integration test both stay green. That is the gap stated as an experiment.That second mutation also showed the end-to-end test passes under the stub on its own, so its docstring now says it is only meaningful paired with the refusal test, rather than letting a later reader take it for an independent check.
1,377 integration + 52,350 unit and architecture passing.
Known gap, not closed here
No CI check asserts that
main.pybinds this factory, or the financial pair either. Deleting the binding is caught at boot, not in the suite.🤖 Generated with Claude Code