Repository navigation
Make the plan out-of-scope gate depth-agnostic for bare relative entries (#183) - #184
Conversation
…ies (#183) The plan out-of-scope escalation (`policy.enforce_plan_out_of_scope`, #169 slice 1) escalates a run when the agent's diff reaches into a path/area the approved plan explicitly promised not to touch (`DeliveryPlan.out_of_scope`). Its matcher, `files_matching_scope`, resolved each entry through the depth-anchored `_scope_entry_covers` — the helper shared with the plan-scope *drift* check (`files_outside_scope`). That sharing is the bug: the two gates have opposite polarity. Drift escalates when a file matches *nothing*, so it must stay depth-anchored (broadening would mark more files in-scope and weaken it). The out-of-scope gate escalates *on match*, so under-matching a nested bare entry silently fails to escalate — a plan's `out_of_scope: ["payments/**"]` protected only a repo-root `payments/` and let a nested `app/payments/charge.py` ride through. This is the exact governance-hole class #177/#179 closed for the other escalate-only path gates, missed here because this consumer reused the drift helper. Add `_scope_entry_covers_at_depth`, a depth-agnostic wrapper mirroring `escalating_path_match` (bare relative globs via `escalating_path_match`, bare directory prefixes via a contiguous-segment-run match; rooted `/…` and anchored `**/…` entries honoured as written). Route `files_matching_scope` through it and leave the anchored `_scope_entry_covers` the drift check depends on untouched. Escalate-only, orchestrator/engine-only, no Rego/PolicyInput/schema change (invariants unaffected). Tests: nested bare glob, nested multi-segment dir prefix, rooted/anchored honoured, negatives, an end-to-end orchestrator test that a nested bare out_of_scope glob escalates plan_out_of_scope, and a regression guard that the drift matcher stays anchored. AGENTS.md updated. Closes #183 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Syj9vwkCcQxoaMeLruSDcw
JeremySNR
left a comment
There was a problem hiding this comment.
Self-review — things a human reviewer should weigh:
1. Why a new helper instead of reusing escalating_path_match directly. escalating_path_match handles bare relative globs at depth, but not the bare directory-prefix form _scope_entry_covers supports (src/vendor covering app/src/vendor/lib.py) — escalating_path_match("app/src/vendor/lib.py", "src/vendor") is False because the **/-expansion requires the path to end at the pattern. So _scope_entry_covers_at_depth handles that case with an explicit contiguous-segment-run match. If a reviewer prefers to only close the (more common) glob case and defer the dir-prefix case, the segment-run branch can be dropped — but it's the same escalate-only safe direction and closes the gap fully, so I kept it.
2. The polarity split is the crux — please sanity-check it. _scope_entry_covers now has two consumers: files_outside_scope (drift, must stay anchored) and files_matching_scope (out-of-scope, now depth-agnostic via the wrapper). The regression test test_drift_matcher_stays_anchored locks this in, but it's the kind of shared-helper coupling that a future edit could re-break. If drift ever needs its own depth semantics, it must NOT reuse this wrapper.
3. Over-escalation is the only behaviour change, and it's bounded. More plan_out_of_scope escalations for nested bare entries — a human review, never a release. It only engages for plans that actually declare out_of_scope (LLM/code-aware planners); the template planner declares none, so single-tenant/offline deployments see zero change.
4. Test isolation note. The e2e test deliberately uses a non-sensitive path (app/legacy/…, not app/payments/…) because a payments-nested path would escalate first via the sensitive-area diff check (which now also matches at depth post-#179), masking whether the out-of-scope gate fired. Worth knowing if someone edits that fixture.
No security surface touched; no Rego/schema/audit-contract change. CI should be the deciding signal.
Generated by Claude Code
What & why
Closes #183.
The plan out-of-scope escalation (
policy.enforce_plan_out_of_scope, #169 slice 1) hands a run to a human when the agent's diff reaches into a path/area the approved plan explicitly promised not to touch (DeliveryPlan.out_of_scope). Its matcherfiles_matching_scoperesolved each entry through the depth-anchored_scope_entry_covers— the helper shared with the plan-scope drift check (files_outside_scope).That sharing is the bug: the two gates have opposite polarity.
glob_match).So a plan's
out_of_scope: ["payments/**"]protected only a repo-rootpayments/and let a nestedapp/payments/charge.pyride straight through — the exact governance-hole class #177/#179 closed for the forbidden BLOCK and the other escalate-only path globs, missed here because this consumer reused the drift helper.The change
_scope_entry_covers_at_depthinengines/risk.py: a depth-agnostic wrapper mirroringescalating_path_match— bare relative globs (payments/**) viaescalating_path_match, bare directory prefixes (src/vendor) via a contiguous-segment-run match; rooted (/…) / already-anchored (**/…) entries honoured exactly as written.files_matching_scoperoutes through it. The anchored_scope_entry_coversthat the drift check depends on is untouched.Scope of impact
PolicyInput/ schema change — this gate has no Rego mirror, so invariant Add repo catalog and catalog-backed context enrichment (Phase 1) #2 doesn't apply.out_of_scopeentries (the template planner declares none), so default deployments are byte-for-byte unchanged.Tests (full offline suite green)
Added to
tests/test_plan_out_of_scope.py:vendored≠vendor);out_of_scopeglob escalatesplan_out_of_scope(uses a non-sensitive path so the escalation is attributable to this gate alone);files_outside_scope(drift) stays anchored — a nested file whose only depth-expanded match would be the out-of-scope entry is still reported as outside scope, proving the drift gate is not weakened.Risks / follow-up
Low. The only behaviour change is more out-of-scope escalations for nested bare entries — the safe direction, and only for plans that declare
out_of_scope(LLM/code-aware planners). No follow-up identified;AGENTS.mdupdated per the maintenance rule (the #179 policy-row enumeration now notes the two-polarity split of_scope_entry_covers).🤖 Generated with Claude Code
https://claude.ai/code/session_01Syj9vwkCcQxoaMeLruSDcw
Generated by Claude Code
Note
Low Risk
Escalate-only engine change with no policy/Rego surface; default deployments unchanged until plans declare
out_of_scope.Overview
Fixes a governance hole in the plan out-of-scope gate (
policy.enforce_plan_out_of_scope): bare relative entries likepayments/**orsrc/vendornow match nested paths (e.g.app/payments/charge.py), not only repo-root directories.Why two matchers: plan-scope drift (
files_outside_scope) escalates when a file matches nothing—broadening would weaken it, so it keeps depth-anchored_scope_entry_covers. Out-of-scope escalates on match—under-matching nested paths let agents slip through; it now uses new_scope_entry_covers_at_depth(bare globs viaescalating_path_match, multi-segment prefixes via contiguous segment runs; rooted/…and**/…unchanged).Impact: orchestrator-only, escalate-only (more
REVIEW_REQUIRED, never fewer). No Rego/schema changes. Inert for template plans with emptyout_of_scope. Tests cover nested globs/prefixes, E2Eplan_out_of_scope, and a regression that drift matching stays anchored.Reviewed by Cursor Bugbot for commit da16ae2. Bugbot is set up for automated code reviews on this repo. Configure here.