Skip to content

feat(acp): require approval for Zed file edits#26626

Closed
HenkDz wants to merge 3 commits into
NousResearch:mainfrom
HenkDz:feat/acp-zed-edit-approval-diffs
Closed

feat(acp): require approval for Zed file edits#26626
HenkDz wants to merge 3 commits into
NousResearch:mainfrom
HenkDz:feat/acp-zed-edit-approval-diffs

Conversation

@HenkDz

@HenkDz HenkDz commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add ACP edit approval gating for file mutations before disk writes occur
  • build structured edit diffs for write_file and patch replace-mode and request Zed permission before executing
  • deny safely on rejection, requester errors, or diff-preparation failures so files remain unchanged
  • add regression coverage for approve/reject paths, new files, requester exceptions, and patch replace-mode

Test Plan

  • scripts/run_tests.sh tests/acp/test_edit_approval.py tests/acp/test_events.py tests/acp/test_tools.py -q

Notes

  • The guard is bound through an ACP-only ContextVar around run_conversation, so CLI/gateway sessions keep existing behavior.
  • Manual Zed reject-path verification is still recommended before merge because this is a safety/UX path.

@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/acp Agent Communication Protocol adapter tool/file File tools (read, write, patch, search) labels May 15, 2026
@HenkDz

HenkDz commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

CI note for reviewer/moderator:

The red test job appears unrelated to this PR's scoped ACP/Zed changes. Current main is already red in the same broad test job:

  • main run 25943019628, job 76264929597
  • failures include:
    • tests/hermes_cli/test_gateway_service.py::TestSystemUnitHermesHome::* → PermissionError: [Errno 13] Permission denied: '/root/.hermes/.../node/bin'
    • tests/tools/test_transcription_dotenv_fallback.py::*xai* → xAI provider selection assertions

This PR's focused local gate passed via scripts/run_tests.sh as listed in the PR body. Scope is limited to the changed ACP/Zed files in this branch; I did not broaden it to patch unrelated main/test-suite instability.

@teknium1 teknium1 mentioned this pull request May 18, 2026
Merged
@teknium1

teknium1 commented May 18, 2026

Copy link
Copy Markdown
Contributor

Auto-closed by the salvage merge — your commits from this PR (cd7f04849 + 316411cf2) were included in #27862's salvage of #27034 and merged as 0292398 with your authorship preserved. git log --author=HenkDz origin/main shows them on main. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/acp Agent Communication Protocol adapter P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) type/feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HenkDz @teknium1 @alt-glitch