mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
* feat(editFile): write-first with inline diff in chat
Replace the blocking accept/reject modal in editFile with a write-first
approach: changes are written to disk immediately, and a unified diff is
returned inline in the chat as a fenced \`\`\`diff block. Multiple
editFile calls in a single agent turn each produce their own diff
without requiring any user interaction.
Also cleans up associated rename/refactor of writeToFileTool →
writeFileTool across chain runner, streamer, and related tests.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(editFile): add Show Diff button and side-by-side diff view in ApplyView
- Pipe editFileDiffs through message types (ChatMessage, StoredMessage) and
MessageRepository so writeFile/editFile diffs are stored as a typed field
(same pattern as sources)
- Collect diffs in AutonomousAgentChainRunner.runReActLoop() by parsing the
JSON result of successful writeFile/editFile tool calls, then pass through
ReActLoopResult → handleResponse() → ChatMessage.editFileDiffs
- ChatSingleMessage reads message.editFileDiffs directly (instead of trying
to parse TOOL_CALL markers which don't exist in agent mode)
- ChatButtons shows a "Show diff" button when editFileDiffs is present;
clicking it opens ApplyView for each diff
- ApplyView redesigned: renders change blocks side-by-side (original left
with red highlights, modified right with green highlights) using the
SideBySideBlock component, unchanged blocks shown as plain text between
change blocks — restores the classic diff block presentation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Revert "feat(editFile): add Show Diff button and side-by-side diff view in ApplyView"
This reverts commit 80945493b48d50c26b9a01bc40de5eced2fcfd18.
* feat(editFile): show ApplyView with simple mode (no per-block buttons)
- editFile now shows the Apply View preview and blocks until user accepts/rejects
- Respects autoAcceptEdits setting (bypasses preview when enabled)
- Adds `simple` flag to ApplyViewState to hide per-block accept/reject/revert buttons
- Removes unused createPatch import
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(tools): update writeFile description to "Create or rewrite files in your vault"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address Codex review — GPT prompt, fuzzy match safety, tool ID migration
- modelAdapter: update GPT prompt to use editFile's oldText/newText params
instead of the old SEARCH/REPLACE block syntax
- ComposerTools: fix fuzzy match to replace only the matched span in the
original content; previously the entire file was rebuilt from the
fuzzy-normalized string, inadvertently applying trimEnd and quote/dash
normalization to text outside the matched span
- settings/model: migrate persisted autonomousAgentEnabledToolIds from
writeToFile→writeFile and replaceInFile→editFile on upgrade
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(editFile): add unit tests for fuzzy match via applyEditToContent
Extract pure replacement logic into applyEditToContent() for testability,
then wire editFileTool.func to call it (removes duplicated logic).
Tests cover:
- Exact match: basic replacement, deletion, NOT_FOUND, AMBIGUOUS
- Fuzzy via smart quotes (single, double), en-dash, NBSP
- Fuzzy via trailing whitespace (spaces, tabs)
- Key invariant: fuzzy match only replaces the matched span — smart quotes
and trailing spaces outside the span are left untouched
- Multiline fuzzy match with mixed artifacts
- CRLF and LF preservation
- BOM preservation
Also fixes mapFuzzyIndexToNormal to extend matchEnd to the full original
line length when the fuzzy match ends at end-of-line, preventing orphaned
trailing whitespace after replacement.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address Codex review — NFKC position mapping and legacy tag compat
- ComposerTools: fix mapFuzzyIndexToNormal to handle NFKC expansion
(e.g. Ⅳ → IV) by walking char-by-char within each line instead of
assuming a 1:1 column mapping; adds two unit tests covering this case
- ChatSingleMessage: normalise legacy <writeToFile> tags to <writeFile>
before rendering so saved chat history from before the tool rename
continues to display correctly
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(modelAdapter): update assertions for editFile oldText/newText prompt
Replace stale SEARCH/REPLACE references with the new parameter names.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(editFile): replace sentinel strings with ApplyEditResult discriminated union
- `applyEditToContent` now returns `{ ok: true; content: string } | { ok: false; reason: 'NOT_FOUND' } | { ok: false; reason: 'AMBIGUOUS'; occurrences: number }` instead of raw sentinel strings, eliminating false positives when file content matches 'NOT_FOUND' or starts with 'AMBIGUOUS:'
- Update `editFileTool` to pattern-match on the discriminated union
- Update `cleanMessageForCopy` regexes to also strip legacy `<writeToFile>` tags from saved chat history
- Update all `applyEditToContent` test assertions to use the new structured return type
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(editFile): sanitize file path before vault lookup
Apply `sanitizeFilePath` in `editFileTool` to match the same treatment
`writeFileTool` already does, preventing "File not found" failures when
the path has a basename that exceeds 255 bytes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(editFile): guard against degenerate fuzzy match inside NFKC expansion
When `oldText` fuzzy-matches a span whose boundaries both map to the same
position in the original (e.g. searching "I" inside fuzzy "IV" derived from
"Ⅳ"), `matchStart >= matchEnd` and the replacement would be zero-width or
corrupt. Return NOT_FOUND instead of silently producing a wrong edit.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(builtinTools): remove invalid daily:append/daily:prepend from prompt
The obsidianDailyNote tool schema only accepts 'daily', 'daily:read', and
'daily:path'. The prompt was incorrectly advertising non-existent
daily:append and daily:prepend commands, which would cause LLM-generated
tool calls to fail schema validation.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(editFile): add Stage 3 to tolerate trailing newline mismatch at EOF
LLMs frequently append \n to the last line of oldText even when the file
has no final newline. Neither exact nor fuzzy match would find this, causing
avoidable NOT_FOUND failures. Stage 3 retries with one trailing \n stripped
from oldText (and mirrors the trim on newText to preserve the file's
no-trailing-newline format).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(tests): update editFile integration test to oldText/newText shape
The argumentValidator was still asserting the legacy args.diff SEARCH/REPLACE
payload. Updated to check args.oldText and args.newText to match the new
editFile tool contract.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(editFile): count overlapping matches to prevent wrong-location edits
countOccurrences was advancing by searchText.length, so overlapping patterns
like "aba" in "ababa" were counted as 1 (non-overlapping) instead of 2. This
caused an ambiguous match to be silently applied at the first position rather
than returning AMBIGUOUS. Advance by 1 to catch all overlapping occurrences.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(editFile): round-trip check guards partial NFKC-expansion fuzzy matches
The existing matchStart >= matchEnd guard only caught degenerate zero-width
spans (e.g. "I" in "Ⅳ"). A partial match like "V" against "Ⅳ" produces a
valid non-zero span [0,1) but would replace the entire source character even
though "V" doesn't exist standalone in the file.
Fix: after mapping indices back to the original, verify that
normalizeForFuzzyMatch(original[matchStart:matchEnd]) === workingSearch.
If not, the match straddled an NFKC expansion boundary → return NOT_FOUND.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(editFile): return structured failed result on edit errors
Replace plain string returns with `{ result: "failed", message }` objects
for all failure paths in editFileTool (file not found, text not found,
ambiguous match, and exceptions), matching writeFileTool's pattern so
the LLM and result formatters can reliably detect failures via the
`result` field. No-op case returns `result: "accepted"` since the edit
technically succeeded.
Also fix a stale tool name reference in builtinTools prompt instructions
(writeToFile → writeFile).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
130 lines
4.3 KiB
TypeScript
130 lines
4.3 KiB
TypeScript
import { cleanMessageForCopy } from "./utils";
|
|
|
|
describe("cleanMessageForCopy", () => {
|
|
it("should remove Think blocks", () => {
|
|
const input = "Before text\n<think>This is my thought process</think>\nAfter text";
|
|
const expected = "Before text\n\nAfter text";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should remove writeFile blocks wrapped in XML codeblocks", () => {
|
|
const input = `Some text before
|
|
\`\`\`xml
|
|
<writeFile>
|
|
<path>test.md</path>
|
|
<content>File content here</content>
|
|
</writeFile>
|
|
\`\`\`
|
|
Some text after`;
|
|
const expected = "Some text before\n\nSome text after";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should remove standalone writeFile blocks", () => {
|
|
const input = `Text before
|
|
<writeFile>
|
|
<path>test.md</path>
|
|
<content>File content</content>
|
|
</writeFile>
|
|
Text after`;
|
|
const expected = "Text before\n\nText after";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should remove tool call markers", () => {
|
|
const input =
|
|
"Before\n<!--TOOL_CALL_START:123:localSearch:Local Search:🔍::true-->Searching...<!--TOOL_CALL_END:123:Found 5 results-->\nAfter";
|
|
const expected = "Before\n\nAfter";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should handle multiple blocks in one message", () => {
|
|
const input = `Start of message
|
|
<think>First thought</think>
|
|
Middle part
|
|
<writeFile><path>file.md</path><content>content</content></writeFile>
|
|
<!--TOOL_CALL_START:456:webSearch:Web Search:🌐::false-->Searching web<!--TOOL_CALL_END:456:Results-->
|
|
End of message`;
|
|
const expected = "Start of message\n\nMiddle part\n\nEnd of message";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should clean up multiple consecutive newlines", () => {
|
|
const input = `Text\n\n\n\n\nMore text`;
|
|
const expected = "Text\n\nMore text";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should preserve normal content", () => {
|
|
const input = `# Heading
|
|
This is a normal message with:
|
|
- Bullet points
|
|
- Code blocks: \`const x = 1;\`
|
|
- **Bold** and *italic* text
|
|
|
|
\`\`\`javascript
|
|
function test() {
|
|
return true;
|
|
}
|
|
\`\`\`
|
|
|
|
More content here.`;
|
|
expect(cleanMessageForCopy(input)).toBe(input);
|
|
});
|
|
|
|
it("should handle nested think blocks", () => {
|
|
const input =
|
|
"Before\n<think>Outer thought <think>Inner thought</think> back to outer</think>\nAfter";
|
|
// Since we're not handling nested blocks, the outer block will be removed but inner content remains
|
|
const expected = "Before\n back to outer</think>\nAfter";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should handle multiline content in blocks", () => {
|
|
const input = `Start
|
|
<think>
|
|
Line 1 of thought
|
|
Line 2 of thought
|
|
Line 3 of thought
|
|
</think>
|
|
End`;
|
|
const expected = "Start\n\nEnd";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should trim leading and trailing whitespace", () => {
|
|
const input = "\n\n Content with spaces \n\n";
|
|
const expected = "Content with spaces";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should handle empty message", () => {
|
|
expect(cleanMessageForCopy("")).toBe("");
|
|
});
|
|
|
|
it("should handle message with only blocks to remove", () => {
|
|
const input = "<think>Only a thought</think>";
|
|
const expected = "";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should remove agent reasoning blocks", () => {
|
|
const input = `<!--AGENT_REASONING:complete:12:["Searching notes","Read 3 notes","Analyzing content"]-->Here is my response based on the analysis.`;
|
|
const expected = "Here is my response based on the analysis.";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should remove agent reasoning blocks with surrounding content", () => {
|
|
const input = `Some intro text
|
|
<!--AGENT_REASONING:collapsed:5:["Searching notes"]-->
|
|
Here is the actual response.`;
|
|
const expected = "Some intro text\n\nHere is the actual response.";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
|
|
it("should handle agent reasoning blocks whose step summaries contain -->", () => {
|
|
const input = `<!--AGENT_REASONING:complete:8:["Step with --> inside"]-->Actual response.`;
|
|
const expected = "Actual response.";
|
|
expect(cleanMessageForCopy(input)).toBe(expected);
|
|
});
|
|
});
|