Uh oh!
There was an error while loading. Please reload this page.
fix(acp): emit diff content block directly from edit metadata (avoid applyPatch) - #37265
fix(acp): emit diff content block directly from edit metadata (avoid applyPatch)#37265skavans wants to merge 1 commit into
Conversation
Replaces the applyPatch-based reconstruction in permission.ts with a direct read of oldString/newString from the edit tool metadata. This fixes the case where the diff content block was silently dropped for edits inside indented blocks. Background: edit.ts trims common indentation from the unified diff (trimDiff) for human readability, but permission.ts then tried to re-derive the result via applyPatch(fileContent, trimmedDiff), which fails because the context lines no longer match the file (their indentation was stripped). applyPatch returns false, so no type:"diff" content block is sent to the ACP client. The fix passes the already-computed contentOld/contentNew through metadata and emits the diff block directly, avoiding the trim/apply round-trip. This mirrors the approach originally proposed in anomalyco#31783. Refs: anomalyco#34079 (introduced applyPatch reconstruction), anomalyco#31783 (earlier direct-read approach), anomalyco#31625 (trimDiff makes patches non-applicable), anomalyco#31781 (original report)
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes#37266
Type of change
What does this PR do?
The
toolCall.contentblock (type: "diff") is silently dropped from edit permission requests whenever the edited hunk sits entirely inside an indented block. ACP clients then have no diff to show next to the approval prompt. The actual edit still applies correctly via the fuzzy replacers inedit.ts— this is a preview-only regression.Why it happens:
edit.tstrims common indentation from the unified diff (trimDiff) for readability. Thenpermission.ts::diffContentForPatchtries to reconstruct the result by re-applying that indentation-stripped patch viaapplyPatch(fileContent, trimmedDiff).applyPatchreturnsfalsebecause the context lines no longer match the file (their indentation was removed). WithfuzzFactor: 0there is no tolerance, so the hunk can't be located, and no content block is emitted.This was introduced by #34079, which added the
applyPatchreconstruction. The earlier PR #31783 by @ReeSilva proposed a simpler approach — passingoldString/newStringdirectly from the edit tool metadata — but was closed in favor of #34079's broader scope. This PR revives that approach and extends it to the multi-file path (apply_patch.ts).The fix:
edit.ts,write.ts, andapply_patch.tsalready computecontentOld/contentNewbefore asking for permission. Pass them throughmetadata(oldString/newString), and inpermission.tsread them directly instead of re-deriving viaapplyPatch. This avoids the trim/apply round-trip entirely and has no failure mode.How did you verify your code works?
Standalone repro from the issue — previously
applyPatch(trimmed)→FAILED; after this change the content block is always emitted:Also tested end-to-end against an ACP client over
opencode acp: edits inside indented blocks now render a diff next to the approval prompt, matching edits near the top level.Screenshots / recordings
N/A — not a UI change in the TUI. The effect is visible only in external ACP clients.
Checklist