feat(runtime): enforce subagent write scopes behind the mutating policy#319
Conversation
Phase A / PR-1 of moving mutating parallel subagents toward `safe` (docs/roadmap/non-claude-provider-autonomy.md Phase 1.2). Until now the subagent merge contract was descriptive only: merge_policy/write_scope were metadata, children ran text-only, and parse_runtime_subagent_action_task hard-rejected anything but read_only. Make the contract enforced by construction: - RuntimeRequirements.mutating_subagent(): workspace-edit surface WITHOUT shell (opaque commands cannot be scope-confined) plus the `subagents` capability, which only RuntimePolicy.mutating_subagents_enabled grants — so transactional_write children stay behind the autonomy policy (bold today; safe after the merge-execution and evidence phases land). - run_subagents accepts merge_policy=transactional_write with a required, bounded, workspace-relative write_scope (normalized, ".."/absolute/drive paths rejected); read_only tasks may not declare a scope. Support resolution is policy-aware and computed from the strictest task, so a mutating task on an ungated session is refused up front with the missing `subagents` capability named. - GenericEditRuntimeSession gains write_scope_guard: path-bearing mutations outside the scope and all run_command actions fail with write_scope_violation before execution; recovery tools (rollback/repair/ abort) keep working on previously validated targets. create_runtime_session threads the guard so factories can build confined children. - The orchestrator refuses to run a mutating child on a session whose write_scope_guard is missing or broader than the task's declared scope. - Child prompt + generic_edit prompt template document the transactional_write contract. Tests: 7 new (path normalization/scope matching, parser contract, policy gating, unconfined/overbroad child refusal, policy-refused action with the factory never invoked, bold end-to-end mutating child with transactional_parent_merge plan, in-loop guard blocking out-of-scope write and run_command while in-scope writes land). Full affected surface green: 447 passed (agent_runtime, runtime_capabilities, runtime_modes, QA integration, provider smoke, autonomy level/policy). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
|
Warning Review limit reached
More reviews will be available in 32 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a write-scope guard mechanism for confining mutating subagent writes to declared directory scopes. It adds a transactional-write merge policy, path normalization utilities, session-level enforcement, policy-aware orchestrator gating, and comprehensive unit/integration tests covering the entire feature. ChangesTransactional Write Subagent Confinement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/agents/runtime/adapters/generic_edit.py (1)
1802-1808:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce write-scope violations before batch-isolation errors.
With current ordering,
run_commandinside an open batch returns a batch-boundary error instead ofwrite_scope_violation. The guarded-session contract says guardedrun_commandshould always fail as write-scope violation.Suggested ordering fix
- isolation_error = self._opaque_batch_mutation_result(action) - if isolation_error is not None: - return isolation_error - - scope_error = self._write_scope_violation_result(action) + scope_error = self._write_scope_violation_result(action) if scope_error is not None: return scope_error + + isolation_error = self._opaque_batch_mutation_result(action) + if isolation_error is not None: + return isolation_error🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/agents/runtime/adapters/generic_edit.py` around lines 1802 - 1808, In run_command (in apps/backend/agents/runtime/adapters/generic_edit.py) the call order should check write-scope violations before batch/isolation errors: call self._write_scope_violation_result(action) first and return its result if non-None, then call self._opaque_batch_mutation_result(action); i.e., swap the two checks so guarded run_command inside an open batch returns write_scope_violation via _write_scope_violation_result instead of a batch-boundary/_opaque_batch_mutation_result error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/agents/runtime/adapters/generic_edit.py`:
- Around line 74-77: The prompt for run_subagents contains contradictory
guidance: earlier text describes supporting transactional_write children with
enforced write_scope, but the later instruction still mandates read-only-only
usage and thus blocks the mutating path; update the run_subagents prompt text to
consistently allow transactional_write when policy permits and to explicitly
state that write operations must be restricted to the declared write_scope
enforced by the runtime (keep the read_only behavior for children without
transactional_write), ensure the prompt no longer contains any line that says
"read-only-only" or otherwise forbids writes, and keep references to
run_subagents, transactional_write, read_only, and write_scope so the intended
behavior is unambiguous.
- Around line 2908-2915: The current branch handling merge_policy ==
DEFAULT_SUBAGENT_MERGE_POLICY incorrectly treats an explicitly declared empty
list write_scope (raw_scope == []) as equivalent to no scope; update the logic
in the run_subagents handling to only accept raw_scope if it is truly
absent/unspecified (e.g., raw_scope is None or raw_scope == ()), and treat any
other explicit declaration (including []) as a violation: raise
GenericEditRuntimeError with the existing message using index, merge_policy, and
raw_scope context; keep the early return only for the truly unspecified case.
In `@apps/backend/agents/runtime/subagents.py`:
- Line 36: The type annotation on the function is_mutating_subagent_task uses a
quoted forward reference; remove the quotes so the parameter reads task:
RuntimeSubagentTask instead of task: "RuntimeSubagentTask". Update the function
signature in subagents.py accordingly (ensure RuntimeSubagentTask is imported or
available in scope).
---
Outside diff comments:
In `@apps/backend/agents/runtime/adapters/generic_edit.py`:
- Around line 1802-1808: In run_command (in
apps/backend/agents/runtime/adapters/generic_edit.py) the call order should
check write-scope violations before batch/isolation errors: call
self._write_scope_violation_result(action) first and return its result if
non-None, then call self._opaque_batch_mutation_result(action); i.e., swap the
two checks so guarded run_command inside an open batch returns
write_scope_violation via _write_scope_violation_result instead of a
batch-boundary/_opaque_batch_mutation_result error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70b3b402-1c96-4301-af91-227e30368ad6
📒 Files selected for processing (7)
apps/backend/agents/runtime/__init__.pyapps/backend/agents/runtime/adapters/__init__.pyapps/backend/agents/runtime/adapters/generic_edit.pyapps/backend/agents/runtime/capabilities.pyapps/backend/agents/runtime/local_actions.pyapps/backend/agents/runtime/subagents.pytests/test_agent_runtime.py
- ruff UP037 (the CI lint failure): drop the quoted annotation in is_mutating_subagent_task — the module already uses `from __future__ import annotations`. - Prompt contradiction: both prompt templates (JSON loop AND native tool loop) still carried the stale "run_subagents only for read-only" rule; align rule lines and the native template intro with the transactional_write contract. - Parser: a read_only task now rejects ANY declared write_scope, including an empty list — "read_only does not declare a scope" is the contract, not "declares an empty one". - Guard ordering: write-scope confinement outranks batch isolation, so a guarded run_command inside an open batch reports write_scope_violation instead of a batch-boundary error. - ruff format on the new list comprehension. Tests: empty-scope rejection case + an in-batch run_command iteration asserting write_scope_violation (and no batch_boundary_error). 298 passed across agent_runtime / runtime_capabilities / runtime_modes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
|
…320) Phase A / PR-2 of moving mutating parallel subagents toward `safe` (follows #319's write-scope enforcement). A transactional_write child no longer needs to touch the shared workspace at all: its mutations are auto-staged and exported as a changeset the parent will apply transactionally (Phase B). - GenericEditRuntimeSession(changeset_export=True): mutations auto-stage into the runtime-owned __changeset__ batch (existing staged-batch mechanics: materialize-per-action + restore, so the real workspace stays at baseline between actions and after finish). Model-issued begin/commit/abort batch is rejected — staging is runtime-owned. At a clean finish the staged pre/postimages are finalized into a changeset payload (first preimage + last postimage per path, exportability via the existing materializability predicate) returned on AgentRunResult.changeset; the open-batch finish veto now applies only to model-managed batches. Error/recovery finishes export nothing. - Orchestrator: mutating children must be changeset-exporting (the confinement check refuses directly-writing sessions); children run against execution requirements (generic_edit for mutating tasks) since task.requirements is the parent-side policy gate, not what the child session physically does; every child gets a per-child artifact namespace (artifacts/subagents/<task-id>/) so a runtime child never clobbers the parent's — or a sibling's — generic_edit trace/checkpoint artifacts. RuntimeSubagentResult.changeset is persisted in the per-child artifact, and the merge plan now records changeset_result_ids / missing_changeset_result_ids for the Phase B executor. - Production wiring (agents/coder.py): the subagent session factory builds a confined changeset-exporting generic_edit child for transactional_write tasks (write_scope_guard from the task), keeping analysis_only for read-only children. Tests: changeset export leaves the workspace untouched (edit + create paths, pre/postimages asserted); model batch control rejected without poisoning staging; an end-to-end orchestrator run with a REAL confined generic_edit child (changeset in result + per-child artifact, child runtime artifacts namespaced, merge plan updated); confined-but- directly-writing child refused. 382 passed across the affected surface. Signed-off-by: Oleg Miagkov <mrobenner@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase D / PR-7 — the final step of "mutating subagents -> safe" (stacked on the PR-6 gate requirement). The protocol is no longer scaffolded: - children are write-scope confined with runtime enforcement (#319), - they stage mutations and export changesets without touching the shared workspace (#320), - the parent applies each child transactionally with per-child rollback and baseline verification (#322), - real conflicts are never auto-applied — they block finish until the parent resolves explicitly (#323), - the whole mechanism is covered by the deterministic subagent_merge_probe (PR-5) which the promotion gate now requires (PR-6). With that evidence chain in place, AutonomyLevel.SAFE now defaults mutating_subagents_enabled=True. bold's remaining distinction is skipping the evidence gate. The explicit AUTO_CODE_MUTATING_SUBAGENTS=false override still opts out (covered by a new test), per ADR-006 precedence. ADR-006's level table updated. The .env.example and UI level descriptions live on the still-open #309/#318 branches and are updated there. Tests: safe-level expectations flipped (autonomy level + direct-API runtime policy grant); explicit env opt-out covered. 694 passed across the affected surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
Phase D / PR-7 — the final step of "mutating subagents -> safe" (stacked on the PR-6 gate requirement). The protocol is no longer scaffolded: - children are write-scope confined with runtime enforcement (#319), - they stage mutations and export changesets without touching the shared workspace (#320), - the parent applies each child transactionally with per-child rollback and baseline verification (#322), - real conflicts are never auto-applied — they block finish until the parent resolves explicitly (#323), - the whole mechanism is covered by the deterministic subagent_merge_probe (PR-5) which the promotion gate now requires (PR-6). With that evidence chain in place, AutonomyLevel.SAFE now defaults mutating_subagents_enabled=True. bold's remaining distinction is skipping the evidence gate. The explicit AUTO_CODE_MUTATING_SUBAGENTS=false override still opts out (covered by a new test), per ADR-006 precedence. ADR-006's level table updated. The .env.example and UI level descriptions live on the still-open #309/#318 branches and are updated there. Tests: safe-level expectations flipped (autonomy level + direct-API runtime policy grant); explicit env opt-out covered. 694 passed across the affected surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
…326) * chore: retrigger CI after base retarget to develop Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com> * feat(autonomy): enable mutating parallel subagents at the safe level Phase D / PR-7 — the final step of "mutating subagents -> safe" (stacked on the PR-6 gate requirement). The protocol is no longer scaffolded: - children are write-scope confined with runtime enforcement (#319), - they stage mutations and export changesets without touching the shared workspace (#320), - the parent applies each child transactionally with per-child rollback and baseline verification (#322), - real conflicts are never auto-applied — they block finish until the parent resolves explicitly (#323), - the whole mechanism is covered by the deterministic subagent_merge_probe (PR-5) which the promotion gate now requires (PR-6). With that evidence chain in place, AutonomyLevel.SAFE now defaults mutating_subagents_enabled=True. bold's remaining distinction is skipping the evidence gate. The explicit AUTO_CODE_MUTATING_SUBAGENTS=false override still opts out (covered by a new test), per ADR-006 precedence. ADR-006's level table updated. The .env.example and UI level descriptions live on the still-open #309/#318 branches and are updated there. Tests: safe-level expectations flipped (autonomy level + direct-API runtime policy grant); explicit env opt-out covered. 694 passed across the affected surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com> * chore: retrigger CI after base retarget to develop Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com> * chore: retrigger CI on develop base Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com> --------- Signed-off-by: Oleg Miagkov <mrobenner@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>



Context — Phase A / PR-1 of "mutating subagents → safe"
Per the agreed plan (4 phases, 7 PRs), the subagent merge contract today is descriptive only:
merge_policy/write_scopeare metadata, children runtext_only, the parser hard-rejects anything butread_only, andbuild_subagent_merge_plandescribes atransactional_parent_mergenobody executes. This PR makes the contract enforced by construction — the prerequisite for ever letting children mutate.What changes
1. New requirements tier —
RuntimeRequirements.mutating_subagent(): workspace-edit surface withoutshell(opaque commands can't be scope-confined) plus thesubagentscapability, which onlyRuntimePolicy.mutating_subagents_enabledgrants. Sotransactional_writechildren stay behind the autonomy policy: bold today, safe after the merge-execution (Phase B) and evidence (Phase C) PRs land.2. Parser accepts the mutating contract —
run_subagentstasks may declaremerge_policy: "transactional_write"with a required, bounded, workspace-relativewrite_scope(≤16 paths, normalized;../absolute/Windows-drive rejected).read_onlytasks may NOT declare a scope. Unknown policies are rejected naming the supported set.3. Policy-aware support resolution — computed from the strictest task: a mutating task on an ungated session is refused up front, naming the missing
subagentscapability; the child session factory is never invoked.4. Runtime-enforced write scope —
GenericEditRuntimeSession(write_scope_guard=...): path-bearing mutations outside the scope and allrun_commandactions fail withwrite_scope_violationbefore execution (same pre-execution guard pattern as the opaque-batch check). Recovery tools (rollback/repair/abort) keep working on previously validated targets.create_runtime_sessionthreads the guard so factories can build confined children.5. Orchestrator confinement check — a mutating child only runs on a session whose
write_scope_guardis present and no broader than the task's declared scope; otherwise the child errors without running.What this PR intentionally does NOT do
safeflip (Phase D).safebehavior is unchanged;read_onlychildren are byte-for-byte unchanged.Tests
7 new tests: path normalization + scope matching (prefix-trick
srcxvssrccovered), parser contract, policy gating of support, unconfined/overbroad child refusal, policy-refused action (factory never invoked), bold end-to-end mutating child producing atransactional_parent_mergeplan, and an in-loop guard test (in-scope write lands, out-of-scope write +run_commandblocked withwrite_scope_violation).Full affected surface green: 447 passed (agent_runtime, runtime_capabilities, runtime_modes, QA integration, provider smoke, autonomy level/policy).
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests