feat(runtime): execute the transactional parent merge of child changesets - #322
Merged
Conversation
…sets Phase B / PR-3 of moving mutating parallel subagents toward `safe` (stacked on #320's changeset export). The merge plan is no longer just descriptive: after a run_subagents run whose plan requires a parent merge, the parent now APPLIES each child's exported changeset transactionally. - execute_subagent_changeset_merge: each mutating child is one transaction boundary. Its changeset is scope-revalidated (defense-in-depth against a tampered payload), baseline-verified (every entry's preimage must still match the live workspace via the existing drift comparator — an unverifiable baseline refuses the merge just like real drift), then applied as a unit. A mid-apply failure restores that child's captured baseline in reverse order; siblings are unaffected. Conflicted children are NEVER auto-applied — they surface as conflicted_unresolved for explicit resolution (deterministic AutoMerger routing lands in PR-4). - Each applied child is recorded as a parent mutation snapshot with transaction id subagent_merge:<child-id>, so the parent's existing rollback_transaction tool can undo exactly one child's merge. - run_subagents now reports merge_plan + merge_execution in its action data; the action only stays ok when every appliable changeset applied (applied/noop). Mutating delegation inside an open model-managed batch is refused up front (mutating_subagents_in_open_batch) before any child spawns. Tests: end-to-end parent run (real confined changeset child; file lands on the shared workspace via the parent; merge_execution recorded with the per-child transaction id), baseline drift refused with workspace intact, mid-apply failure rolled back (first entry restored), applied child undone via rollback_transaction, conflicted children kept unapplied, and delegation refused inside an open parent batch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 26 minutes and 26 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 (2)
📝 WalkthroughWalkthroughThis PR implements transactional parent-side merging of exported subagent changesets. The runtime now applies child staged changes to the parent workspace with baseline verification, write-scope enforcement, rollback-on-failure per child, and mutation snapshot recording. A pre-spawn guard rejects transactional_write subagents when a parent batch is open. Comprehensive test coverage validates the happy path, baseline drift handling, rollback behavior, conflict detection, and batch boundary enforcement. ChangesSubagent changeset merge feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 4044-4051: The loop that revalidates entries uses entry["path"]
but the actual write uses postimage["path"] and allows empty write_scope, which
lets a child changeset mutate a different path or skip validation; update the
validation in the entries loop (around build_generic_edit_file_preimage and
similar blocks) to first require the parent changeset's write_scope is
present/non-empty, then for each entry ensure entry.get("path"),
preimage.get("path") and postimage.get("path") are all present and equal (e.g.,
entry_path == preimage_path == postimage_path) before calling
build_generic_edit_file_preimage or proceeding, and if any check fails return
"baseline_unverifiable" with drift_checks unchanged; also ensure downstream code
uses the validated postimage_path for the actual write instead of trusting
entry["path"].
- Around line 2521-2553: The ToolActionResult returned by run_subagents does not
include the parent mutation snapshot ids/paths that
execute_subagent_changeset_merge appends, causing build_generic_edit_transaction
to miss prior mutations; update the ToolActionResult (in the return for
run_subagents) to include the merged snapshot metadata from merge_execution
(e.g., add keys like "merged_mutation_snapshot_ids" and/or
"merged_mutation_paths" populated from merge_execution's applied/parent
snapshots) so downstream build_generic_edit_transaction can inspect result.data
for prior mutations; locate the merge_execution variable and the
ToolActionResult construction and add those fields when merge_execution is not
None.
- Around line 4125-4149: The appended subagent-merge snapshot dict created
around transaction_id = subagent_merge_transaction_id(result_id) and added to
mutation_snapshots lacks the mandatory workspace_guard field, causing
validate_generic_edit_checkpoint_mutation_snapshot_integrity() to reject resumed
sessions; fix by including a workspace_guard entry in that snapshot (using the
same guard value used for other checkpoint/transaction snapshots or deriving it
from the current context) when building the dict for
SUBAGENT_MERGE_TRANSACTION_PREFIX so rollback_transaction and later validation
succeed.
In `@tests/test_agent_runtime.py`:
- Around line 4299-4360: Update the
test_subagent_merge_rolls_back_failed_application to exercise a two-child merge:
keep the existing "broken" changeset as one SimpleNamespace (e.g., id="edit-b")
and add a separate successful child changeset (e.g., id="edit-a") that creates
"src/ok.txt"; call execute_subagent_changeset_merge with results containing both
entries and merge_plan["mutating_result_ids"] listing both ids; then assert
overall merge_execution["status"] is "failed", assert the failing child's
outcome (identify by id "edit-b" in outcomes) has status
"apply_failed_rolled_back" and failed_path "src/broken.txt", and assert the
successful sibling's file (from "edit-a") still exists while the broken file
does not; keep snapshots == [] to verify no leakage.
- Around line 4251-4297: Add a new pytest that mirrors
test_subagent_merge_refuses_on_baseline_drift but triggers the
_verify_changeset_baseline branch that returns "baseline_unverifiable": create a
changeset whose entry has a preimage that cannot be revalidated (e.g.,
preimage.exists True with a preimage.content or content_encoding that does not
match or cannot be verified against the workspace), call
execute_subagent_changeset_merge with that changeset (similar to
results=[SimpleNamespace(id="edit-a", changeset=changeset)] and empty snapshots)
and assert merge_execution["status"] == "failed" and
merge_execution["outcomes"][0]["status"] == "baseline_unverifiable" and that no
files were modified and snapshots remain []. Ensure the test references
execute_subagent_changeset_merge and the internal verification behavior
(_verify_changeset_baseline) so the intended branch is covered.
🪄 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: 5c268a72-1b74-44b6-8d77-1fa7bd083cce
📒 Files selected for processing (2)
apps/backend/agents/runtime/adapters/generic_edit.pytests/test_agent_runtime.py
Addresses all 5 CodeRabbit findings and the Windows test failure: - Changesets are untrusted input at the parent boundary: the declared write_scope must be non-empty and normalizable (else scope_violation with reason missing_or_invalid_write_scope), and every entry's declared path must equal its preimage AND postimage paths (else invalid_changeset_path) — the postimage path is what actually gets written, so path mismatches could otherwise escape revalidation. - subagent_merge snapshots now carry workspace_guard: checkpoint integrity validation treats it as mandatory, so a recoverable stop after a subagent_merge rollback no longer fails resume validation. - run_subagents result data now surfaces mutation_snapshot_ids, mutated_paths, and affected_paths from applied outcomes (each applied outcome carries its mutation_snapshot_id), so transaction summaries record parent-side merge mutations and a later failing action in the same turn yields partial_failure with a recovery plan instead of plain failed. - Windows CI: the rollback test hand-crafted a preimage with byte counts that broke under CRLF newline translation; it now captures the real preimage via build_generic_edit_file_preimage. - New tests per review: baseline_unverifiable refusal; partial merge keeps the applied sibling while the failing child rolls back; malformed changesets (path mismatch, scope-less) rejected with nothing applied. 311 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>
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
Phase B / PR-4 of moving mutating parallel subagents toward `safe` (stacked on #322's parent-merge executor). Conflicts stop being a dead end: false positives auto-apply, real ones block finish until the parent resolves them explicitly. No AI merging anywhere on this path. - Real-conflict refinement: the merge plan flags children by DECLARED write-scope overlap; the executor now treats a child as conflicted only when its changeset's ACTUAL paths intersect another declared-conflicted child's actual paths. Overlapping scopes with disjoint edits — the common false positive — apply deterministically like any other child. - Real conflicts surface as conflicted_unresolved (now with conflict_paths) and are registered as pending on the parent session. Finish is vetoed (stop_reason=unresolved_subagent_conflicts, recoverable) until every pending conflict is resolved. - New local action resolve_subagent_conflict {result_id, resolution}: 'apply' merges that child's changeset onto the CURRENT workspace through the same baseline-verified transactional executor (recorded as subagent_merge:<id>, so it still rolls back as one unit; a sibling that already changed the file makes the baseline check fail — reconcile and retry, or discard); 'discard' drops the changeset. Validation errors list the pending ids. The action is mutating (blocked inside open batches) and schema'd; both prompt templates teach the rule. Tests: declared-overlap/disjoint-edit auto-apply; finishing with pending conflicts terminates recoverably with nothing applied; apply+discard resolutions land exactly the chosen child's content and unblock finish (transaction id asserted); resolve-action input validation. 392 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>
SonarCloud reliability gate flagged the unguarded snapshots[0] access; assert the expected lengths before indexing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
Phase B / PR-4 of moving mutating parallel subagents toward `safe` (stacked on #322's parent-merge executor). Conflicts stop being a dead end: false positives auto-apply, real ones block finish until the parent resolves them explicitly. No AI merging anywhere on this path. - Real-conflict refinement: the merge plan flags children by DECLARED write-scope overlap; the executor now treats a child as conflicted only when its changeset's ACTUAL paths intersect another declared-conflicted child's actual paths. Overlapping scopes with disjoint edits — the common false positive — apply deterministically like any other child. - Real conflicts surface as conflicted_unresolved (now with conflict_paths) and are registered as pending on the parent session. Finish is vetoed (stop_reason=unresolved_subagent_conflicts, recoverable) until every pending conflict is resolved. - New local action resolve_subagent_conflict {result_id, resolution}: 'apply' merges that child's changeset onto the CURRENT workspace through the same baseline-verified transactional executor (recorded as subagent_merge:<id>, so it still rolls back as one unit; a sibling that already changed the file makes the baseline check fail — reconcile and retry, or discard); 'discard' drops the changeset. Validation errors list the pending ids. The action is mutating (blocked inside open batches) and schema'd; both prompt templates teach the rule. Tests: declared-overlap/disjoint-edit auto-apply; finishing with pending conflicts terminates recoverably with nothing applied; apply+discard resolutions land exactly the chosen child's content and unblock finish (transaction id asserted); resolve-action input validation. 392 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>
|
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
Phase B / PR-4 of moving mutating parallel subagents toward `safe` (stacked on #322's parent-merge executor). Conflicts stop being a dead end: false positives auto-apply, real ones block finish until the parent resolves them explicitly. No AI merging anywhere on this path. - Real-conflict refinement: the merge plan flags children by DECLARED write-scope overlap; the executor now treats a child as conflicted only when its changeset's ACTUAL paths intersect another declared-conflicted child's actual paths. Overlapping scopes with disjoint edits — the common false positive — apply deterministically like any other child. - Real conflicts surface as conflicted_unresolved (now with conflict_paths) and are registered as pending on the parent session. Finish is vetoed (stop_reason=unresolved_subagent_conflicts, recoverable) until every pending conflict is resolved. - New local action resolve_subagent_conflict {result_id, resolution}: 'apply' merges that child's changeset onto the CURRENT workspace through the same baseline-verified transactional executor (recorded as subagent_merge:<id>, so it still rolls back as one unit; a sibling that already changed the file makes the baseline check fail — reconcile and retry, or discard); 'discard' drops the changeset. Validation errors list the pending ids. The action is mutating (blocked inside open batches) and schema'd; both prompt templates teach the rule. Tests: declared-overlap/disjoint-edit auto-apply; finishing with pending conflicts terminates recoverably with nothing applied; apply+discard resolutions land exactly the chosen child's content and unblock finish (transaction id asserted); resolve-action input validation. 392 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>
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
* feat(runtime): deterministic conflict path for subagent merges Phase B / PR-4 of moving mutating parallel subagents toward `safe` (stacked on #322's parent-merge executor). Conflicts stop being a dead end: false positives auto-apply, real ones block finish until the parent resolves them explicitly. No AI merging anywhere on this path. - Real-conflict refinement: the merge plan flags children by DECLARED write-scope overlap; the executor now treats a child as conflicted only when its changeset's ACTUAL paths intersect another declared-conflicted child's actual paths. Overlapping scopes with disjoint edits — the common false positive — apply deterministically like any other child. - Real conflicts surface as conflicted_unresolved (now with conflict_paths) and are registered as pending on the parent session. Finish is vetoed (stop_reason=unresolved_subagent_conflicts, recoverable) until every pending conflict is resolved. - New local action resolve_subagent_conflict {result_id, resolution}: 'apply' merges that child's changeset onto the CURRENT workspace through the same baseline-verified transactional executor (recorded as subagent_merge:<id>, so it still rolls back as one unit; a sibling that already changed the file makes the baseline check fail — reconcile and retry, or discard); 'discard' drops the changeset. Validation errors list the pending ids. The action is mutating (blocked inside open batches) and schema'd; both prompt templates teach the rule. Tests: declared-overlap/disjoint-edit auto-apply; finishing with pending conflicts terminates recoverably with nothing applied; apply+discard resolutions land exactly the chosen child's content and unblock finish (transaction id asserted); resolve-action input validation. 392 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> * fix(runtime): surface plural snapshot keys on resolve-apply result The resolve_subagent_conflict apply path now reports mutation_snapshot_ids / mutated_paths / affected_paths like an applied run_subagents merge, so transaction summaries record the parent-side mutation and later failures in the same turn become partial_failure. 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>
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
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>
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
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>
OBenner
added a commit
that referenced
this pull request
Jun 10, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-landing #321 onto develop
#321 was stacked on
feat/subagent-changeset-exportand got merged into that branch — but #320 had already been squash-merged to develop before that, so PR-3's content never reached develop (the stacked branch was orphaned). This PR cherry-picks the same commit cleanly onto current develop. Content is identical to reviewed #321.What it does (recap)
execute_subagent_changeset_merge— each mutating child is one transaction boundary:subagent_merge:<child-id>) —rollback_transactionundoes exactly one child's merge.conflicted_unresolved; deterministic AutoMerger routing is PR-4).run_subagentsaction data carriesmerge_plan+merge_execution; ok only when all appliable changesets applied. Mutating delegation inside an open model-managed batch refused up front.Tests
6 new (e2e parent-applies-child-changeset; baseline drift refused; mid-apply rollback; rollback_transaction undo; conflicted children kept unapplied; open-batch refusal) — 233 passed in the runtime file locally; ruff (CI-pinned 0.14.10) clean.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests