fix(vscode): prevent edit_existing_file from silently failing without error feedback

RO
4mo ago
+398-53

Summary

Fixes #12317

When using the edit_existing_file tool in VS Code, edits can silently fail: the chat reports "File edited successfully" but the file remains unchanged. This happens because the apply-state lifecycle can stall without reaching the "closed" status, and because non-ASCII file paths can fail URI resolution.

Root cause

Two interacting failure modes:

  1. Apply state lifecycle stall: The edit_existing_file tool dispatches an apply operation and waits for VS Code's VerticalDiffManager to complete the diff streaming and emit a "closed" status. If deterministicApplyLazyEdit fails (common for insertions where AST matching can't find corresponding nodes), the fallback streamLazyApply runs an LLM-based diff. If that stream produces no output, errors, or is aborted, streamDiffLines() can return without emitting "closed". The tool call is stuck in "calling" status indefinitely, and the LLM's pre-generated "File edited successfully" text appears as a false success signal.

  2. Non-ASCII path resolution failure: editToolImpl calls resolveRelativePathInDir() which uses URI-encoded paths with fileExists(). Non-ASCII characters in file paths cause the URI encoding to mismatch the filesystem, making fileExists return false. The tool throws a "does not exist" error, but this error can be swallowed in certain UI states.

Changes

  • Wrap streamDiffLines() and instantApplyDiff() in VerticalDiffManager with try/finally blocks that guarantee a "closed" status update is always emitted, even if the diff stream yields nothing or throws
  • Send an error-state update when handler creation fails (early return), preventing silent stalls
  • Add a timeout safety net in applyForEditTool that detects apply states stuck without reaching "closed" or "done" and dispatches an error after 60 seconds (excludes "done" state so user review time doesn't trigger false timeout)
  • Track done-phase lifecycle using a module-scoped doneStreams set to distinguish successful edits (which transition through "done" before "closed") from bail-out failures (which go directly to "closed"), preventing false "Edit Success" reports for edits that never generated diffs
  • Clear the apply timeout unconditionally after the IDE messenger responds, preventing timer leaks during rapid multi-file edits
  • Add a raw-path fallback in editToolImpl for non-ASCII filenames, attempting direct fileExists() with the original path before falling back to the open-files check

What this doesn't change

  • The diff generation logic in deterministicApplyLazyEdit, streamLazyApply, or applyCodeBlock is untouched; this fix addresses the error reporting and lifecycle management, not the diff algorithm itself
  • The single_find_and_replace and multi_edit tools share the same applyForEditTool dispatch and benefit from the timeout fix without additional changes
  • Chat-mode tool call rendering (the <tool_call> raw output reported by AndyP2) is a separate issue related to tool call parsing in chat mode, not addressed here

Checklist

  • Code follows project conventions
  • Tests pass locally
  • Prettier formatting verified
  • No unrelated changes included

Tests

  • gui/src/redux/thunks/handleApplyStateUpdate.test.ts - updated/new tests for timeout safety net behavior
  • Manual test: open a file in VS Code, use edit_existing_file with insertion-only changes, verify either the edit applies or a clear error message appears in the chat
  • Manual test: open a file with non-ASCII characters in its path, use edit_existing_file, verify it either edits correctly or reports a clear path resolution error

Summary by cubic

Stops edit_existing_file in VS Code from reporting success when no changes were applied by fixing the apply lifecycle and surfacing errors. Also adds a timeout for stalled applies and fixes non-ASCII path resolution. Fixes #12317.

  • Bug Fixes
    • Always emit a "closed" status from VerticalDiffManager using try/finally; send an error-state update when handler creation fails.
    • Add a 60s timeout in applyForEditTool to flag stalled applies; clear the timer reliably and ignore normal "done" waits.
    • Track "done" streams to distinguish real edits from bail-outs; avoid false "Edit Success" and report "Edit Failed" when "closed" fires without "done".
    • Include auto-formatting diffs in tool output when present.
    • Fix non-ASCII paths by falling back to the raw filepath in editToolImpl.
    • Extends to single_find_and_replace and multi_edit since they share the same apply flow.