Skip to content

fix(tools): repair SDK call mismatches that broke five tools#1186

Open
abhay-codes07 wants to merge 1 commit into
supermemoryai:mainfrom
abhay-codes07:fix/tools-sdk-call-mismatches
Open

fix(tools): repair SDK call mismatches that broke five tools#1186
abhay-codes07 wants to merge 1 commit into
supermemoryai:mainfrom
abhay-codes07:fix/tools-sdk-call-mismatches

Conversation

@abhay-codes07

@abhay-codes07 abhay-codes07 commented Jul 2, 2026

Copy link
Copy Markdown

What

Five @supermemory/tools operations were written against a different SDK surface than the supermemory@^3 the package pins, so they failed on every invocation. Because every tool wraps its body in try/catch and returns {success: false, error}, these never threw loudly — the model just gets a failure (or worse, a plausible-looking success with undefined data) and moves on.

Tool Bug Effect
documentDelete (ai-sdk) passed {docId} to documents.delete(id: string) no document could ever be deleted
memoryForget (ai-sdk + openai) called client.memories.forget, which doesn't exist in the v3 SDK TypeError on every call, surfaced as success: false
documentList (ai-sdk + openai) read response.documents; the list response keys items as memories always returned documents: undefined — models conclude the user has no documents
ClaudeMemoryTool str_replace / insert falsy checks rejected new_str: "" and insert_text: "" deleting text or inserting a blank line — standard memory-tool operations — were impossible
ClaudeMemoryTool delete / rename delete returned success without calling any API; rename never removed the old document "deleted" files kept appearing in listings and search — the model believes forgotten data is gone when it isn't

How

  • documents.delete(documentId) — plain string, matching the SDK signature (the OpenAI variant already did this correctly).
  • New src/shared/forget-memory.ts helper that calls DELETE /v4/memories directly with {containerTag, id?, content?, reason?} — the v3 SDK has no memories.forget, and the package already raw-fetches /v4/profile and /v4/conversations the same way, so this follows the established pattern rather than forcing a risky SDK major bump.
  • documentList returns response.memories (still exposed as documents in the tool result, so consumers are unaffected) and its schema now exposes the API's real page-based pagination. The old offset/status params were silently ignored by the API — passing offset: 10 returned the same first page — so they were removed rather than kept as dead knobs.
  • str_replace rejects only a missing new_str (empty string is the documented way to delete text); same for insert_text. old_str still must be non-empty since replacing "" would prepend.
  • delete and rename now call documents.delete on the backing document (rename skips the delete when both paths normalize to the same customId, since the add already replaced the content).

Testing

  • New offline unit suite src/tool-operations.test.ts (13 tests) mocks the SDK the same way the existing claude-memory.test.ts does, plus stubs global fetch for the forget endpoint, and asserts the exact call shapes: delete receives a string, list forwards page and returns the memories array, forget issues DELETE /v4/memories with the right body/auth header, empty-string new_str/insert_text work, 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).
  • Existing claude-memory.test.ts still passes; the stale getToolDefinitions().length assertion in tools.test.ts is updated from 2 to the actual 7.
  • tsc --noEmit error count drops from 155 to 148: the diff removes all eight SDK-mismatch errors (TS2345 on 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 every tool() call in the package — separate issue, not touched here.
  • biome check clean on all touched files (two pre-existing noExplicitAny warnings on lines I didn't change).

The live integration tests in tools.test.ts need SUPERMEMORY_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.mdx shows the response keyed "memories").

cc @MaheshtheDev


Session Details

  • Session: View Session
  • Requested by: Unknown
  • Address comments on this PR. Add (aside) to your comment to have me ignore it.

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).
Copilot AI review requested due to automatic review settings July 2, 2026 03:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment on lines +34 to +36
throw new Error(
`Supermemory forget memory failed: ${response.status} ${response.statusText}. ${errorText}`,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 },
})
Suggested change
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))

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@vorflux

vorflux Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Testing

Targeted validation passed for the internal packages/tools SDK wrapper fixes. The testing subagent classified this as an internal tooling/library bug fix with no UI or mobile testing required.

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.ts

Result:

Targeted tests: PASS — 16 tests passed.
Package build: PASS — `tsdown` built JS and `.d.ts` outputs, including `dist/shared/forget-memory.js`.
Changed-file Biome gate: PASS exit 0 with 2 warnings in `claude-memory.ts` about existing `any` usage.
Worktree after build: clean.

Verdict

Passed. Targeted tests, package build, and changed-file lint/format checks all completed successfully for the PR-relevant files.

@vorflux

vorflux Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Reviewed the SDK call-shape fixes across packages/tools, including the AI SDK, Claude memory, OpenAI tool wrappers, shared forgetMemoryRequest, and updated tool operation coverage. The review checked the changed diff against the installed supermemory SDK types and documented DELETE /v4/memories behavior.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants