Skip to content

feat: add improved diff preview for ACP agent file edits#43

Open
Henry-916 wants to merge 7 commits into
formulahendry:mainfrom
Henry-916:feat/improved-diff-preview
Open

feat: add improved diff preview for ACP agent file edits#43
Henry-916 wants to merge 7 commits into
formulahendry:mainfrom
Henry-916:feat/improved-diff-preview

Conversation

@Henry-916

Copy link
Copy Markdown

Summary

Adds real-time diff preview when ACP agents edit files, using VS Code's native diff editor with a custom URI scheme.

Inspired by Cline's DiffViewProvider architecture. Addresses the issues identified in PR #37 (race conditions, temp file leaks, resource cleanup).

Changes

New files

  • src/handlers/DiffContentProvider.ts (20 lines) — TextDocumentContentProvider for acp-diff: custom URI scheme. Old file content is base64-encoded in the URI query parameter, eliminating temp files entirely.
  • src/handlers/DiffPreviewHandler.ts (395 lines) — Core diff preview logic:
    • Listens to session/update (tool_call / tool_call_update) notifications
    • Synchronous snapshot on pending/in_progress status — uses fs.readFileSync (no async race condition)
    • Opens vscode.diff on completed status
    • 4-level file path extraction: locationscontent[diff]rawInput key scan → title regex
    • Decoration highlighting for active edit locations
    • Proper dispose() with removeListener + decoration cleanup

Modified files

  • src/extension.ts (+12 lines) — Registers AcpDiffContentProvider and DiffPreviewHandler with proper lifecycle management via context.subscriptions.

Design decisions

Decision Rationale
Custom URI scheme (acp-diff:) No temp files, no cleanup needed, no file path collisions
Synchronous fs.readFileSync Eliminates the race condition in PR #37 where async cache could read post-edit content
MD5 hash prefix in URI path Prevents collisions between same-named files in different directories
isEditKind filter (edit/delete/move only) Avoids false positives from read/search/think/execute tool calls
tryFinishWithDiffContent fallback Handles agents that don't send pending status but include diff content in completed

Comparison with PR #37

Issue PR #37 This PR
Race condition 🔴 Async cacheOldContent can read post-edit content ✅ Synchronous fs.readFileSync
Temp files 🔴 Writes to os.tmpdir(), never cleaned ✅ Zero temp files (URI-encoded)
File name collision 🔴 path.basename only ✅ MD5 hash prefix
Resource leak 🔴 dispose() doesn't removeListener ✅ Full cleanup
Type safety 🔴 (update as any) ✅ ACP SDK types

Known limitations (follow-up)

  • Non-UTF-8 encoding: currently assumes UTF-8 (could add iconv-lite detection)
  • Large files (>200KB): base64 URI may exceed limits (could fall back to temp file)
  • No streaming diff yet (shows only on completed, not during edit)

Testing

  1. npm install && npm run compile — compiles cleanly
  2. Press F5 in VS Code to launch Extension Development Host
  3. Connect to any ACP agent (e.g., hermes acp)
  4. Ask the agent to edit a file — diff view should open automatically

Closes #37

🤖 Generated with Claude Code and Reasonix

Henry-916 and others added 6 commits June 12, 2026 12:08
Implements real-time diff preview when ACP agents edit files, using
VS Code's native diff editor with a custom URI scheme.

Key changes:
- DiffContentProvider: custom acp-diff: URI scheme (base64-encoded
  old content, no temp files)
- DiffPreviewHandler: listens to session/update tool_call notifications,
  synchronously snapshots old content before edits, opens diff on completion
- extension.ts: registers both providers with proper lifecycle management

Architecture inspired by Cline's DiffViewProvider. Fixes race conditions
and resource leaks from PR formulahendry#37 by using synchronous file reads and
URI-based content storage.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Reasonix <noreply@deepseek.com>
Two issues found during testing with Hermes ACP adapter:
1. No 'kind' field in tool_call notification — added title-based
   heuristic fallback (patch/edit/write/replace/modify/create etc.)
2. No tool_call_update with 'completed' after edit — added
   FileSystemWatcher as fallback to detect file changes on disk

Also cleans up file watchers properly in cleanupEdit and cleanupAll.
Per Claude Code + Reasonix review:
- Add 800ms debounce to FileSystemWatcher to avoid partial-write diffs
- Narrow isEditKind title regex: require both edit verb AND file reference
- Add onDidCreate handler for new file creation
- Clean up debounce timers in cleanupEdit and cleanupAll
- Store timer in debounceTimers map for proper cleanup
Both Claude Code and Reasonix flagged that the debounce timer
was never stored in debounceTimers Map, so cleanupEdit couldn't
cancel it. Now stored immediately after setTimeout in onChangeHandler.
Root cause: Hermes ACP sends minimal tool_call notifications without
locations/content/rawInput. File path only appears in title as relative
path (e.g. 'patch (replace): index.html').

Changes:
- Add relative path regex to title fallback (matches 'index.html', 'src/file.ts')
- Add normalizePath() to convert relative paths to absolute using workspace root
- Call normalizePath in startEdit before snapshotting and watching
Hermes ACP sends tool_call notifications without a 'status' field.
The handler was checking  but status was
undefined, so startEdit was never called. Now defaults to 'pending'
when status is missing.

Also: removed debug logging added during investigation.
@Henry-916
Henry-916 force-pushed the feat/improved-diff-preview branch from 0dea41d to d6ccbd7 Compare June 12, 2026 16:08
- Use createQuickPick instead of showQuickPick for activeItems control
- Most recent session is pre-selected (press Enter to resume immediately)
- No sessions = skip QuickPick entirely, create new session
- Updated placeholder text to guide user
alanjds pushed a commit to alanjds/vscode-acp that referenced this pull request Jun 30, 2026
Cherry-picked and adapted from formulahendry/vscode-acp:

- fix(formulahendry#46): use correct Copilot package (@github/copilot@latest)
- fix(#2): serialize concurrent permission requests to prevent silent
  cancellations when multiple QuickPick dialogs overlap
- fix(formulahendry#6): respect acp.defaultWorkingDirectory in CWD resolution so
  workspace-scoped agents (kiro-cli etc.) find their config files
- fix(formulahendry#13): add 30s AbortController timeout to registry fetch;
  make connect/newConversation/restart progress notifications cancellable
- fix(formulahendry#47): redirect ACP pipe to fd 3 on login shells to prevent
  profile scripts (e.g. distrobox_profile.sh) from consuming fd 0
- feat(formulahendry#18): security hardening — workspace path validation in
  FileSystemHandler, env var filtering and shell:false in TerminalHandler
- feat(formulahendry#42): editor context link — toggle to prepend active file/cursor
  info to chat prompts (acp.toggleEditorContext command)
- feat(formulahendry#40): add Grok Build as pre-configured agent
- feat(formulahendry#35): add openHarness as pre-configured agent
- feat(formulahendry#44): add siGit Code as pre-configured agent
- feat(formulahendry#37/agent): add Reasonix as pre-configured agent
  (diff preview implementation deferred to separate PR using formulahendry#43 approach)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant