Repository navigation
fix(tools): community fixes for 3.1.2 - #1798
Conversation
…ntics Anthropic's memory_20250818 spec defines insert as: insert_text is inserted AFTER line insert_line, 0 inserts at the beginning of the file, and the valid range is [0, n_lines]. The implementation treated insert_line as a 1-based insert-BEFORE index with range [1, n_lines + 1]. Since the caller of this tool is Claude itself, which is trained on the spec semantics, every model-driven insert landed one line earlier than intended, insert_line: 0 (insert at top of file) was rejected as invalid, and insert_line: n_lines (append) inserted before the last line instead of after it. Fix the validation range to [0, n_lines], splice at insert_line directly (0-based insert-after), and update the error and success messages to match. One existing tool-operations test encoded the old insert-before behavior; its insert_line is adjusted so its expected output is unchanged under spec semantics. Adds four regression tests covering top-of-file, middle, append, and both out-of-range directions.
Three places where ClaudeMemoryTool diverges from the documented memory_20250818 wire format: - rename sends old_path/new_path, not path. handleCommand validated command.path, so every rename coming from a real model died with "Cannot read properties of undefined (reading 'startsWith')". path is still accepted as the source for existing callers. - insert_line means "insert after this line" (0 = top of file), but we spliced at insertLine - 1 and rejected 0, so every insert landed one line above where Claude asked and inserting at the top was impossible. - str_replace with new_str omitted is a deletion per the spec; we rejected it. The new tests mock the supermemory client so they run without an API key. Also fixed the rename example in the docs, which showed the same path shape the code expected.
…ect args
`getProfile`, `documentList` and `memoryForget` all declare `required: []`,
so a model may legitimately call them with no arguments at all. The OpenAI
API serialises that as `arguments: ""`, and `parseToolArguments` handed the
empty string straight to `JSON.parse`, so every no-argument call came back as
{"success":false,"error":"Invalid JSON arguments for getProfile"}
Those three tools were unreachable in their documented no-argument form.
The same gate also lets non-object JSON through. `"null"` parses cleanly and
then rejects in the destructuring parameter of every tool function --
`TypeError: Cannot destructure property 'containerTag' of 'object null'` --
which escapes `executeToolCall`, since it has no catch, and fails the whole
request. That is precisely the throw #1488 added this gate to contain. `"5"`
and `"\"text\""` are quieter but worse: they destructure to `undefined` and
call the API with no container tag at all.
Treat blank arguments as `{}`, and require the parsed value to be a non-null,
non-array object. Malformed JSON still returns the tool error #1488 added.
Adds eight regression tests. Six of them fail against the current
implementation -- two on the blank-argument path and four on the non-object
path, one carrying the raw TypeError. The two guard tests, malformed JSON and
an ordinary well-formed call, pass both before and after, so the behaviour
…l and addConversation
Ports the #1676 collision fix to v5 with a reversible path ID and a legacy-ID fallback, so files written by 3.1.1 still open and get overwritten in place (also while their content is still processing). Keeps new_str required for str_replace and moves the PR tests onto the v5 mocks.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
How to use the Graphite Merge QueueAdd the label Main to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
MaheshtheDev
changed the title
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-app | 92508cc | Commit Preview URL Branch Preview URL |
Oct 12 2026, 11:02 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-mcp | 92508cc | Oct 12 2026, 11:02 PM |
|
Tip Expected to resolve the linked issue. Linked issue: TOOLS-3 — community fixes shipped as @supermemory/tools 3.1.2 (rename via old_path, zero-argument OpenAI tool calls, insert_line semantics) Neither Also considered · 6 refuted
Analysed against 2 Cloudflare Workers and 1 repository
Polylane analysed Did this help? React 👍 or 👎 so the next review is sharper. |
- New claude-memory IDs use a cm_ prefix so they can never match a legacy ID, and a write refuses an ID held by another document. - rename overwrites an existing destination (including a legacy ID) instead of leaving a stale copy. - Non-404 lookup failures return an error instead of allowing a write; delete works while content is still processing. - OpenAI executor rejects blank or missing required arguments, and documentDelete requires a documentId. - MemoryCommand.path is required again for every command except rename.
…utor Names like constructor, toString and __proto__ resolved Object.prototype members as schemas and threw on parameters.required, failing the whole tool batch. The schema lookup now uses Object.hasOwn, so they return the usual unknown-function error.
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Independently verified exact commit 92508cc. The own-property schema guard restores structured errors for constructor, toString, proto, hasOwnProperty, valueOf and ordinary unknown names on fresh source and packed consumers, without fetches or whole-batch rejection; mixed registered/unknown calls retain per-call attribution. Frozen build/pack, 153 scoped tests, scoped lint and strict public types pass; focused packed contracts also pass on Node 20. Prior migration, required-input, facade and SDK/image evidence is reused only for verified unchanged code. No remaining independently confirmed blocker in this scoped review. Approval does not authorize merge or publication.
Bundles 8 community fixes for
@supermemory/tools, cherry-picked with their original authors, and adapts them to v5. Ships as 3.1.2...segments (fix(tools): reject parent-directory segments in Claude memory paths #1652),old_pathrename (fix(tools): handle the memory tool commands Claude actually sends #1597), path ID collisions (fix(tools): prevent customId collisions in claude memory tool (#1547) #1676).cm_-prefixed, so they never match a legacy ID. Files written by 3.1.1 are still found by their legacy ID and overwritten in place, including byrename. Failed lookups never trigger a write.new_strstays required forstr_replace, andMemoryCommand.pathstays required except forrename.Co-Authored-By: abhinav7x94 204053250+abhinav7x94@users.noreply.github.com
Co-Authored-By: rohitsux 71192000+rohitsux@users.noreply.github.com
Co-Authored-By: aniruddhaadak80 127435065+aniruddhaadak80@users.noreply.github.com
Co-Authored-By: Adityakk9031 143548997+Adityakk9031@users.noreply.github.com
Co-Authored-By: Rikinshah787 87516554+Rikinshah787@users.noreply.github.com
Co-Authored-By: Agnik47 140933190+Agnik47@users.noreply.github.com