fix(acp): gate patch mode=patch (V4A) behind ACP edit approval#28120
fix(acp): gate patch mode=patch (V4A) behind ACP edit approval#28120EloquentBrush0x wants to merge 1 commit into
Conversation
patch mode="replace" and write_file were correctly gated; mode="patch" (V4A multi-file format) returned None from build_edit_proposal, which maybe_require_edit_approval treated as "no approval needed", silently bypassing the guard regardless of the user's edit-approval policy. Fix: route mode="patch" through _proposal_for_patch_v4a so the approval requester is always invoked for V4A patches. build_acp_edit_tool_call renders per-file diff blocks (mirroring _build_patch_mode_content) so the Zed approval dialog shows a structured diff rather than raw patch text. should_auto_approve_edit now short-circuits on V4A proposals: workspace_session policy prompts (can't verify per-file containment); session policy auto-approves (symmetric with single-file behaviour). No behaviour change for CLI/gateway sessions where the ContextVar is unset. Fixes approval bypass introduced in NousResearch#27862.
|
BoardJames CI triage for the current head ( One branch-local issue I can reproduce on macOS before the full suite finishes: The failures are from the new ACP edit-approval tests using pytest Suggested owner fix: either keep these approval-path tests inside a repo/workspace temp directory that the file tool allows, or canonicalize the temp-root allowance with |
Problem
patchwithmode="patch"(V4A multi-file format) silently bypassed the ACP edit approval guard introduced in #27862.build_edit_proposalhandledwrite_fileandpatch mode=replace, but returnedNoneformode=patch.maybe_require_edit_approvaltreatsNoneas "no approval needed" and lets dispatch continue — so an ACP session under the default Ask policy would never prompt before a V4A patch executed.The intent to gate both tool types is visible in
model_tools.py: the exception-path deny covers{"write_file", "patch"}together, but the normal path missedmode=patch.Fix
build_edit_proposal— adds_proposal_for_patch_v4abranch so the approval requester is always called for V4A patches.build_acp_edit_tool_call— detects V4A proposals and renders per-filetool_diff_contentblocks (mirroring_build_patch_mode_content), so the Zed dialog shows a structured diff rather than raw patch text.should_auto_approve_edit— short-circuits on V4A proposals:workspace_sessionprompts (can't verify per-file containment without parsing every operation);sessionauto-approves (symmetric with single-file behaviour).No behaviour change for CLI/gateway sessions where the ContextVar is unset.
Tests
test_patch_v4a_rejection_does_not_mutatetest_patch_v4a_approval_request_captures_proposaltool_nameandmodetest_patch_v4a_tool_call_uses_per_file_diff_contentbuild_acp_edit_tool_callemits per-file blocks (skipped without ACP SDK)test_patch_v4a_auto_approval_*should_auto_approve_editpolicy matrix for V4AFixes approval bypass introduced in #27862.