Skip to content

planner: derive admission depth from the active governed run #10

Open

Description

@Shashankss1205

Current status — remaining work is trustworthy depth

Argument validation for kinds declaring NodeSpec.args_schema landed in #98: admission validates arguments and materialization forwards the validated dump while retaining fingerprint binding. Kinds without a schema retain the documented forward_args opt-in contract.

This issue remains open because parent_depth is still supplied by the caller. The remaining change is to derive a conservative depth from the active governed execution scope so passing zero cannot bypass nested admission limits. The maintenance review and proposed failure tests are in the comments below.

The original two-part report is retained below for context; its argument-schema half is complete.


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

  • A proposal whose arguments violate a declared schema or rule is refused, with a reason code and remedy in feedback()
  • Nothing executes during a check — no factory call, no meter write
  • Every failed check is still reported, not just the first
  • The refusal appears on the admission trace event and does not inflate node-execution counts
  • Materialisation still binds to the authorisation by fingerprint
  • Depth cannot be understated by the caller (if you take that half)
  • The README paragraphs stating these two limits are updated
  • uv run pytest green, uv run ruff check . clean

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.

Activity

  1. added
    enhancementNew feature or request
    help wantedExtra attention is needed
    experience requiredDeep familiarity with the codebase or domain needed; not a starter task
    architectureChanges a subsystem boundary or a cross-cutting contract
    on Jul 29, 2026
  2. Shashankss1205 commented on Aug 8, 2026

    @Shashankss1205
    CollaboratorAuthor

    Status update rather than a close, because this issue has two halves and only one has landed.

    The args half is done, on main via #98. Admission grew a sixth check, ARGS, sitting between REGISTRY and POLICY: where a kind's NodeSpec declares an args_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. Materializer forwards 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 admission trace 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_depth is 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.

  3. Shashankss1205 commented on Oct 15, 2026

    @Shashankss1205
    CollaboratorAuthor

    Rechecked against current main: the argument-schema half is already implemented by #98, as the earlier status comment says. NodeSpec.args_schema, Check.ARGS, validated forwarding and fingerprint binding should be retained; this issue must not be closed as fully fixed while caller-supplied depth remains.

    The remaining bypass is still present: GovernedLoop passes its constructor’s parent_depth to AdmissionChecker, and neither the checker nor RunContext measures the active governed nesting. A robust depth fix needs a runtime-owned execution scope inherited across sync, async and worker-thread boundaries, incremented for materialized execution and reset on every exit. An explicit parent depth may conservatively increase that observed value but must never reduce it; understatement should become a structured DEPTH rejection, preserving the existing all-failures feedback and admission trace.

    Leaving this open for that focused change. Required adversarial checks: a nested loop/checker that supplies zero cannot gain authority, siblings do not inherit each other’s depth, exceptions restore the previous scope, and standalone checks retain a documented root-depth contract. A public mutable ctx.depth alone would merely move the caller-trust gap.

  4. changed the title [-]planner: the admission gate authorises a kind, not its arguments[/-] [+]planner: derive admission depth from the active governed run[/+] on Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    architectureChanges a subsystem boundary or a cross-cutting contractenhancementNew feature or requestexperience requiredDeep familiarity with the codebase or domain needed; not a starter taskhelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions