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 carrying args={"path": "/etc/passwd"} is admitted on the strength of its kind alone. Materializer drops 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_depth is the caller's word. The checker cannot see how deep the run actually is, so a caller that always passes 0 has 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 kind rather than instance name means 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.check and the five checks; :775 _find_cycle; :271 the tier-ordering note
grapharc/planner/proposal.py — ProposedNode, Subgraph, _NAME (note extra="forbid", so a proposal cannot carry code)
grapharc/planner/materialize.py — Materializer, forward_args, and the fingerprint match that binds materialisation to the authorisation
grapharc/planner/loop.py — where parent_depth is passed
grapharc/policy/engine.py — check_node, if argument rules belong in the document
What to change
Two separable pieces; either is a valid PR.
Argument authorisation. The design question is where the schema comes from.
- A
NodeSpec could 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.
- Or argument rules live in the policy TOML, next to the node and edge rules.
- Whichever you choose: a rejection must remain data.
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 the admission trace event's failed-check list.
- Nothing may run during a check:
NodeSpec.factory is never called and the budget meter is read, not written. Validation must not violate that.
- Decide what happens to
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 RunContext or 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 in args being refused, and a caller that lies about parent_depth gaining nothing.
Acceptance criteria
Skill 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 kind and never on name, 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.
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.