Add chat-apply command to plugin#13
Conversation
|
Does this apply every single code block from all the messages in the chat? As opposed toapplying the code block under the cursor? |
mgmonteleone
left a comment
There was a problem hiding this comment.
Code Review Agent🛡️ with @mgmonteleone's authorization
Verdict: REQUEST CHANGES
The chat-apply idea is useful, but this implementation is not safe enough to merge yet. There are three blocking issues in autoload/augment/chat.vim.
Blocker: code block paths are trusted and written directly
current_block.path comes from chat/model output and is passed directly into mkdir(), readfile(), and writefile(). That allows a chat response to create or modify arbitrary paths reachable from Vim, including absolute paths, .. traversal, dotfiles, plugin files, or other files outside the user's intended workspace.
Please add explicit path validation before any filesystem writes. At minimum, reject absolute paths, home expansions, path traversal, and paths outside the current project/root. Ideally the command should also preview/confirm the target files before writing.
Blocker: existing files are always appended instead of applying or replacing the intended content
For readable files, s:ApplyCodeBlock() reads the existing file, adds a blank line, appends the block content, and writes the result back. That will corrupt the common case where the model emits a full replacement file or a snippet meant to replace an existing region. Running the command repeatedly would duplicate content each time.
Please implement semantics that match the command name and user expectation: either apply structured edits/diffs, replace whole-file code blocks intentionally, or ask the user to confirm append-vs-replace behavior per file. Blind append should not be the default for an apply command.
Blocker: the code-fence parsing regex is broken
The parser uses a Vim regex shaped like path=\(.+\) / mode=\(.+\). In Vim's default regex mode, + is literal unless escaped as \+ or the pattern uses very-magic mode. As written, this will fail to match normal path=... mode=... fence metadata reliably.
Please fix the parser and add coverage with representative Augment code snippets, including paths with nested directories and both matching and non-matching fences. The command should fail closed when parsing is ambiguous rather than silently writing the wrong thing.
mgmonteleone
left a comment
There was a problem hiding this comment.
Blocking issues: 1. Manual version bump conflicts with CI automation. 2. s:ApplyCodeBlock appends to files, which can corrupt them; should replace/patch instead. 3. Critical Security: Paths from AI output must be validated to prevent arbitrary file writes (e.g. path=/etc/passwd). 4. Regex needs to handle quoted attributes. Please restrict apply to the latest response only.
Command to apply changes from chat