fix(tools): repair SDK call mismatches that broke five tools#1186
fix(tools): repair SDK call mismatches that broke five tools#1186abhay-codes07 wants to merge 1 commit into
Conversation
Several @supermemory/tools operations were written against a different supermemory SDK surface than the v3 SDK the package pins, so they failed on every invocation: - documentDelete (ai-sdk) passed an object to documents.delete, which takes a plain id string - no document could ever be deleted. - memoryForget (ai-sdk + openai) called client.memories.forget, which does not exist in the v3 SDK - every call threw and surfaced as success:false. It now hits DELETE /v4/memories directly via a shared helper, the same raw-fetch pattern the middleware already uses for /v4/profile and /v4/conversations. - documentList (ai-sdk + openai) read response.documents, but the list response keys its items as memories - the tool always returned documents: undefined. It also forwarded offset and status params the API does not have; the schema now exposes the API's real page-based pagination instead. - ClaudeMemoryTool.str_replace rejected new_str: "" (the standard way to delete text) because of a falsy check; insert likewise rejected inserting a blank line. Both now only reject missing values. - ClaudeMemoryTool.delete reported success without deleting anything, and rename left the old document in place, so "deleted" files kept showing up in listings and search. Both now call documents.delete. Testing: new offline unit suite (tool-operations.test.ts) mocks the SDK and global fetch and verifies each call shape; 9 of its 13 tests fail against the previous code. Updated the stale getToolDefinitions count (2 -> 7). tsc --noEmit error count drops from 155 to 148 - the change removes the eight SDK-mismatch errors and introduces none (the remaining errors are the pre-existing zod/ai schema-typing issue that affects every tool() call in the package).
| throw new Error( | ||
| `Supermemory forget memory failed: ${response.status} ${response.statusText}. ${errorText}`, | ||
| ) |
There was a problem hiding this comment.
The rule 'Include any supporting data as the cause argument instead of inlining into the string' is violated here. The error is constructed by inlining the status code, status text, and error body directly into the error message string. Instead, the supporting data (status, statusText, errorText) should be passed as the cause option to new Error(). For example:
throw new Error('Supermemory forget memory failed', {
cause: { status: response.status, statusText: response.statusText, body: errorText },
})| throw new Error( | |
| `Supermemory forget memory failed: ${response.status} ${response.statusText}. ${errorText}`, | |
| ) | |
| throw new Error('Supermemory forget memory failed', { | |
| cause: { status: response.status, statusText: response.statusText, body: errorText }, | |
| }) | |
Spotted by Graphite (based on custom rule: TypeScript style guide (Google))
Is this helpful? React 👍 or 👎 to let us know.
TestingTargeted validation passed for the internal Commands run: cd packages/tools && bun run test src/tool-operations.test.ts src/claude-memory.test.ts
cd packages/tools && bun run build
bunx biome ci packages/tools/src/ai-sdk.ts packages/tools/src/claude-memory.ts packages/tools/src/openai/tools.ts packages/tools/src/shared/forget-memory.ts packages/tools/src/tool-operations.test.ts packages/tools/src/tools-shared.ts packages/tools/src/tools.test.tsResult: Verdict✅ Passed. Targeted tests, package build, and changed-file lint/format checks all completed successfully for the PR-relevant files. |
SummaryReviewed the SDK call-shape fixes across Verdict✅ Reviewed — no issues found. The changed wrappers align with the SDK/API contracts and no bugs, breaking changes, security issues, or data-loss risks were identified in the reviewed diff. |
What
Five
@supermemory/toolsoperations were written against a different SDK surface than thesupermemory@^3the package pins, so they failed on every invocation. Because every tool wraps its body intry/catchand returns{success: false, error}, these never threw loudly — the model just gets a failure (or worse, a plausible-looking success withundefineddata) and moves on.documentDelete(ai-sdk){docId}todocuments.delete(id: string)memoryForget(ai-sdk + openai)client.memories.forget, which doesn't exist in the v3 SDKTypeErroron every call, surfaced assuccess: falsedocumentList(ai-sdk + openai)response.documents; the list response keys items asmemoriesdocuments: undefined— models conclude the user has no documentsClaudeMemoryToolstr_replace/insertnew_str: ""andinsert_text: ""ClaudeMemoryTooldelete/renamedeletereturned success without calling any API;renamenever removed the old documentHow
documents.delete(documentId)— plain string, matching the SDK signature (the OpenAI variant already did this correctly).src/shared/forget-memory.tshelper that callsDELETE /v4/memoriesdirectly with{containerTag, id?, content?, reason?}— the v3 SDK has nomemories.forget, and the package already raw-fetches/v4/profileand/v4/conversationsthe same way, so this follows the established pattern rather than forcing a risky SDK major bump.documentListreturnsresponse.memories(still exposed asdocumentsin the tool result, so consumers are unaffected) and its schema now exposes the API's realpage-based pagination. The oldoffset/statusparams were silently ignored by the API — passingoffset: 10returned the same first page — so they were removed rather than kept as dead knobs.str_replacerejects only a missingnew_str(empty string is the documented way to delete text); same forinsert_text.old_strstill must be non-empty since replacing""would prepend.deleteandrenamenow calldocuments.deleteon the backing document (rename skips the delete when both paths normalize to the same customId, since the add already replaced the content).Testing
src/tool-operations.test.ts(13 tests) mocks the SDK the same way the existingclaude-memory.test.tsdoes, plus stubs global fetch for the forget endpoint, and asserts the exact call shapes: delete receives a string, list forwardspageand returns thememoriesarray, forget issuesDELETE /v4/memorieswith the right body/auth header, empty-stringnew_str/insert_textwork, delete/rename actually delete. 9 of the 13 fail against the previous code (the 4 that pass are negative-path guards that were already correct).claude-memory.test.tsstill passes; the stalegetToolDefinitions().lengthassertion intools.test.tsis updated from 2 to the actual 7.tsc --noEmiterror count drops from 155 to 148: the diff removes all eight SDK-mismatch errors (TS2345on delete,TS2339 Property 'documents'…×3,TS2339 Property 'forget'…×2, plus two implicit-anys) and introduces none. The remaining 148 are the pre-existing zod/ai schema-typing incompatibility that hits everytool()call in the package — separate issue, not touched here.biome checkclean on all touched files (two pre-existingnoExplicitAnywarnings on lines I didn't change).The live integration tests in
tools.test.tsneedSUPERMEMORY_API_KEY/OPENAI_API_KEY, which I don't have — happy to iterate if a staging run surfaces anything, but the call shapes are verified against the SDK's type definitions and the repo's own API docs (list-memories/overview.mdxshows the response keyed"memories").cc @MaheshtheDev
Session Details
(aside)to your comment to have me ignore it.