Repository navigation
planner: the admission gate authorises a kind, not its arguments #10
Description
Activity
- addedenhancementNew feature or requestNew feature or requesthelp wantedExtra attention is neededExtra attention is neededexperience requiredDeep familiarity with the codebase or domain needed; not a starter taskDeep familiarity with the codebase or domain needed; not a starter taskarchitectureChanges a subsystem boundary or a cross-cutting contractChanges a subsystem boundary or a cross-cutting contract
on Jul 29, 2026 Status update rather than a close, because this issue has two halves and only one has landed.
The args half is done, on
mainvia #98. Admission grew a sixth check,ARGS, sitting betweenREGISTRYandPOLICY: where a kind'sNodeSpecdeclares anargs_schema, a proposal's args must validate against that operator-supplied model or the proposal is rejected with the failing field named and the remedy naming the schema.Materializerforwards the validated dump and re-validates on build, so an edited proposal or a swapped registry refuses to build rather than executing against unchecked arguments. The fingerprint an approval binds to now includes the args, so "what ran is what was admitted" covers them.Against the acceptance criteria: a violating proposal is refused with a reason code and remedy; nothing executes during the check; every failed check is still reported rather than the first; the refusal rides the
admissiontrace event and does not inflate node-execution counts; materialisation still binds by fingerprint; the docs moved with the code (admission's six-check table, the governance cookbook, README and deep-dive Limits).The depth half is not done.
parent_depthis still supplied by the caller and the checker still has no way to verify how deep a run actually is, so a caller that always passes 0 has no recursion limit beyond the nesting visible inside a single proposal. That limit is stated in the module docstring, so it stays an honest documented gap rather than a silent one — which is the posture the issue itself asks for.Leaving this open for the depth half. If it would be clearer to track that separately, say so and I will split it and close this one.
Summary
Two related gaps in the admission gate, both documented and both deliberate:
Arguments are not authorised. No rule reaches
ProposedNode.args, so a proposal carryingargs={"path": "/etc/passwd"}is admitted on the strength of itskindalone.Materializerdrops args by default (forward_args=False); turning that on hands a model's unchecked dictionary to your factory, and gating it becomes the factory's job.parent_depthis the caller's word. The checker cannot see how deep the run actually is, so a caller that always passes0has no recursion limit beyond the nesting visible inside a single proposal.Why this matters
The admission gate is the part of this project with no prior art to copy, and it is the reason the rest exists: a planner proposes, a deterministic checker admits, and only then does anything execute. Its five checks are strong — kind in the registry, edge permitted between kinds, worst case within the remaining budget, depth within limit, acyclic — and a decision keyed on
kindrather than instancenamemeans renaming cannot launder a denied capability. Closing the argument gap is what would let graph engineering claim that an admitted proposal is safe to run rather than merely well-shaped, which is the difference between a gate and a shape check.Where in the code
grapharc/planner/admission.py—AdmissionChecker.checkand the five checks;:775_find_cycle;:271the tier-ordering notegrapharc/planner/proposal.py—ProposedNode,Subgraph,_NAME(noteextra="forbid", so a proposal cannot carry code)grapharc/planner/materialize.py—Materializer,forward_args, and the fingerprint match that binds materialisation to the authorisationgrapharc/planner/loop.py— whereparent_depthis passedgrapharc/policy/engine.py—check_node, if argument rules belong in the documentWhat to change
Two separable pieces; either is a valid PR.
Argument authorisation. The design question is where the schema comes from.
NodeSpeccould declare an args schema (a Pydantic model) that the checker validates a proposal's args against. This keeps the registry as the source of truth, which matches the existing rule that costs come from the registry, never the proposal.AdmissionResult.feedback()hands the per-check list with codes and remedies back to the planner as its next round's input, and the loop never retries an identical proposal. A new check must produce a reason code and a remedy in that same shape, and must appear in theadmissiontrace event's failed-check list.NodeSpec.factoryis never called and the budget meter is read, not written. Validation must not violate that.forward_args=False. If args are now authorised, is forwarding them still off by default?Real depth. Give the checker a trustworthy depth rather than a caller-supplied integer — most likely by threading it through
RunContextor the loop's own state, so a caller cannot understate it. Then decide whether an understated depth is a rejection or an error.How to verify
uv run pytest tests/test_admission.py tests/test_planner_loop.py -q uv run pytest -q uv run ruff check .Follow the adversarial style already in
tests/test_admission.py: renaming a denied kind does not evade the policy, nor does hiding the rename in a nested scope, and every failed check is reported rather than just the first. New tests should include a traversing path inargsbeing refused, and a caller that lies aboutparent_depthgaining nothing.Acceptance criteria
feedback()admissiontrace event and does not inflate node-execution countsuv run pytestgreen,uv run ruff check .cleanSkill level — experience required
This is the security core of the project, and the existing tests are adversarial on purpose — they assume someone is trying to get a denied capability past the gate. You need to understand why every decision keys on
kindand never onname, why a proposal cannot carry code, and why a rejection is data rather than a downgraded approval, before you change what is checked. Please propose your design in a comment first; a check that can be bypassed is worse than a documented gap, because the gap is at least honest.