-
Notifications
You must be signed in to change notification settings - Fork 0
Add targeted patch tool for Evernote notes #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Introduces the 'evernote_patch_note' tool for applying targeted find-and-replace operations to note content without regenerating the entire note. Implements backend logic in EvernoteAPI, updates types for replacements and patch results, and adds comprehensive integration tests for various patching scenarios.
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR introduces a new patch note feature enabling targeted find-and-replace operations on Evernote notes. It adds type definitions, implements the patching logic in the API layer, integrates the tool with the MCP server, and includes comprehensive test coverage. Changes
Sequence DiagramsequenceDiagram
participant Client
participant MCPServer as MCP Server
participant EvernoteAPI
participant EvernoteService as Evernote Service
Client->>MCPServer: Call evernote_patch_note<br/>(guid, replacements)
MCPServer->>MCPServer: Validate input<br/>(replacements required)
MCPServer->>EvernoteAPI: patchNoteContent(guid, replacements)
EvernoteAPI->>EvernoteService: getNote(guid)
EvernoteService-->>EvernoteAPI: Note with ENML content
EvernoteAPI->>EvernoteAPI: Convert ENML to Markdown
EvernoteAPI->>EvernoteAPI: Apply replacements sequentially<br/>Track occurrences & replacements
EvernoteAPI->>EvernoteAPI: Validate changes<br/>(not empty, changes occurred)
EvernoteAPI->>EvernoteAPI: Convert Markdown back to ENML
EvernoteAPI->>EvernoteService: updateNote(note)
EvernoteService-->>EvernoteAPI: Update confirmation
EvernoteAPI-->>MCPServer: PatchNoteResult<br/>(success, changes[], warnings)
MCPServer->>MCPServer: Format human-readable summary
MCPServer-->>Client: Success/failure message<br/>with change details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/integration/mcp-tools.test.ts (1)
82-82: Test assertion is outdated and will fail with the actual tool count.The
expect(result.tools).toHaveLength(11)assertion doesn't match the actual tools registered. ThelistToolsHandlerinsrc/index.tsreturns the completetoolsarray containing all 26 tools (including the newevernote_patch_note), not just 11. Update the test to:
- Change the expected length from 11 to 26
- Add
evernote_patch_noteto the tools being checked
🤖 Fix all issues with AI agents
In `@src/evernote-api.ts`:
- Around line 134-170: patchNoteContent currently calls applyMarkdownToNote
after replacements which causes unreferenced resources to be dropped; update
patchNoteContent to preserve existing note.resources when applying the patched
markdown (or modify applyMarkdownToNote to merge resources) so attachments not
referenced in the new markdown are retained. Specifically, in patchNoteContent
(after getNote and convertENMLToMarkdown) ensure you pass the original
note.resources (or merge them inside applyMarkdownToNote) instead of letting
applyMarkdownToNote unconditionally replace resources; mirror the behavior used
by createNote which preserves and adds to existing resources. Make sure getNote,
convertENMLToMarkdown, and applyMarkdownToNote cooperate to avoid deleting
attachments that weren't explicitly removed by the replacement.
🧹 Nitpick comments (1)
src/index.ts (1)
1389-1422: Consider validating emptyfindstrings.The handler validates that
replacementsis a non-empty array, but doesn't check for emptyfindstrings. An emptyfindstring would match at every position in the content, leading to unexpected behavior:"abc".split("").join("X") // Returns "XaXbXcX"🔧 Suggested validation enhancement
case 'evernote_patch_note': { const { guid, replacements } = args as any; if (!replacements || !Array.isArray(replacements) || replacements.length === 0) { throw new Error('At least one replacement must be provided'); } + for (const r of replacements) { + if (!r.find || typeof r.find !== 'string' || r.find.length === 0) { + throw new Error('Each replacement must have a non-empty "find" string'); + } + } const result = await evernoteApi.patchNoteContent(guid, replacements);
| async patchNoteContent(guid: string, replacements: NoteReplacement[]): Promise<PatchNoteResult> { | ||
| // Fetch existing note with content and resources | ||
| const note = await this.getNote(guid, true, true); | ||
|
|
||
| // Convert ENML to markdown | ||
| let markdown = this.convertENMLToMarkdown(note.content, note.resources); | ||
|
|
||
| // Track changes | ||
| const changes: PatchNoteResult['changes'] = []; | ||
|
|
||
| // Apply replacements sequentially | ||
| for (const replacement of replacements) { | ||
| const { find, replace, replaceAll = true } = replacement; | ||
|
|
||
| // Count occurrences | ||
| const regex = new RegExp(find.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g'); | ||
| const matches = markdown.match(regex); | ||
| const occurrences = matches ? matches.length : 0; | ||
|
|
||
| // Perform replacement | ||
| let replaced = 0; | ||
| if (occurrences > 0) { | ||
| if (replaceAll) { | ||
| markdown = markdown.split(find).join(replace); | ||
| replaced = occurrences; | ||
| } else { | ||
| markdown = markdown.replace(find, replace); | ||
| replaced = 1; | ||
| } | ||
| } | ||
|
|
||
| changes.push({ | ||
| find, | ||
| occurrences, | ||
| replaced, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n src/evernote-api.ts | sed -n '134,170p'Repository: verygoodplugins/mcp-evernote
Length of output: 1488
🏁 Script executed:
cat -n src/evernote-api.ts | sed -n '470,483p'Repository: verygoodplugins/mcp-evernote
Length of output: 721
🏁 Script executed:
rg -n 'applyMarkdownToNote' src/evernote-api.ts -A 20Repository: verygoodplugins/mcp-evernote
Length of output: 1767
🏁 Script executed:
rg -n 'convertENMLToMarkdown|convertMarkdownToENML' src/evernote-api.ts -A 15 | head -80Repository: verygoodplugins/mcp-evernote
Length of output: 2584
🏁 Script executed:
rg -n 'buildResourcesFromAttachments' src/evernote-api.ts -B 2 -A 10Repository: verygoodplugins/mcp-evernote
Length of output: 1265
Data loss risk: Existing attachments are deleted during patch operations.
The applyMarkdownToNote method (lines 470-482) will unconditionally delete existing note resources if the markdown conversion doesn't produce new attachments. Since patchNoteContent calls applyMarkdownToNote after text replacements, any note with images or attachments that aren't referenced in the replacement text will lose those resources permanently.
This differs from createNote (lines 54-62), which preserves and adds to existing resources rather than replacing them. The patch operation should either preserve unreferenced attachments or require explicit attachment handling.
🤖 Prompt for AI Agents
In `@src/evernote-api.ts` around lines 134 - 170, patchNoteContent currently calls
applyMarkdownToNote after replacements which causes unreferenced resources to be
dropped; update patchNoteContent to preserve existing note.resources when
applying the patched markdown (or modify applyMarkdownToNote to merge resources)
so attachments not referenced in the new markdown are retained. Specifically, in
patchNoteContent (after getNote and convertENMLToMarkdown) ensure you pass the
original note.resources (or merge them inside applyMarkdownToNote) instead of
letting applyMarkdownToNote unconditionally replace resources; mirror the
behavior used by createNote which preserves and adds to existing resources. Make
sure getNote, convertENMLToMarkdown, and applyMarkdownToNote cooperate to avoid
deleting attachments that weren't explicitly removed by the replacement.
Introduces the 'evernote_patch_note' tool for applying targeted find-and-replace operations to note content without regenerating the entire note. Implements backend logic in EvernoteAPI, updates types for replacements and patch results, and adds comprehensive integration tests for various patching scenarios.