Skip to content

feat(ai-sdk): re-export 7-tool surface - #1433

Open
Dhravya wants to merge 6 commits into
feat/tools-seven-tool-parityfrom
feat/ai-sdk-tool-reexports
Open

feat(ai-sdk): re-export 7-tool surface#1433
Dhravya wants to merge 6 commits into
feat/tools-seven-tool-parityfrom
feat/ai-sdk-tool-reexports

Conversation

@Dhravya

@Dhravya Dhravya commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

  • Re-export full tool set from @supermemory/tools/ai-sdk
  • Add unit tests for tool re-exports

Stacked on #1432

Test plan

  • bun run test:unit in packages/ai-sdk

Made with Cursor

…ools

Thin @supermemory/ai-sdk package now mirrors the canonical 7-tool surface
with updated tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

Claude finished @ved015's task in 1m 7s —— View job


Code Review: feat(ai-sdk): re-export 7-tool surface

  • Fetch PR diff and analyze changes
  • Review re-exported tools for bugs/security issues
  • Check for breaking changes in API surface
  • Review test changes
  • Post review summary

Overview: This PR replaces inline tool implementations in @supermemory/ai-sdk with re-exports from @supermemory/tools/ai-sdk, adding 5 new tools (getProfile, documentList, documentDelete, documentAdd, memoryForget) to the surface.

Issues found:

Cursor Bugbot correctly identified that includeFullDocs is accepted in the searchMemories schema but never passed to client.search() in packages/tools/src/ai-sdk.ts:54-62. I verified this — the parameter is defined in the schema (lines 29-42) and destructured, but the execute function on line 54 only uses informationToGet and limit, dropping includeFullDocs entirely.

However, this bug exists in the base branch (feat/tools-seven-tool-parity), not in this PR's changes. This PR is a pure re-export — it doesn't modify the tool implementations. The fix belongs in PR #1432 or the base branch.

The other review comments are style nits:

  • Graphite's as string assertion concern is technically correct but harmless here since the outer describe.skipIf() already guards the block
  • The Zod/AI SDK type compatibility concern was addressed in a prior commit (the latest ace3a76 merge)

No bugs introduced by this PR itself. The re-export is correct, tests are properly updated to skip without API keys, and the README documentation is comprehensive.

Score: 9/10

The code change is clean and well-structured. One point deducted because the stack carries forward the includeFullDocs bug from the base branch — ensure that's fixed in PR #1432 before merging this stack.


@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9c3f84b. Configure here.

export {
supermemoryTools,
searchMemoriesTool,
addMemoryTool,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Search ignores includeFullDocs

Medium Severity

Replacing the inline tools with re-exports from @supermemory/tools/ai-sdk changes searchMemories: the tool schema still accepts includeFullDocs, but the shared implementation never passes it to client.search, unlike the removed @supermemory/ai-sdk code that forwarded it to search. Callers and models cannot control full-document inclusion in results.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9c3f84b. Configure here.

Comment thread packages/ai-sdk/src/tools.test.ts Outdated
Comment on lines +12 to +13
const testApiKey = process.env.SUPERMEMORY_API_KEY as string
const testOpenAIKey = process.env.OPENAI_API_KEY as string

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 variables testApiKey and testOpenAIKey use type assertions (as string) to cast string | undefined to string. According to the style guide rule on Type assertions and casting, unnecessary type assertions should be avoided and type annotations should be used instead of assertions for object literals. While this is not an object literal, the spirit of the rule is to prefer type annotations over assertions. More importantly, using as string here is unsafe — it suppresses the undefined possibility without a runtime check. Consider using a non-null assertion only after a guard, or restructuring so the type is narrowed properly (e.g., via a runtime check before assignment).

Suggested change
const testApiKey = process.env.SUPERMEMORY_API_KEY as string
const testOpenAIKey = process.env.OPENAI_API_KEY as string
const testApiKey = process.env.SUPERMEMORY_API_KEY ?? (() => { throw new Error("SUPERMEMORY_API_KEY is not set") })()
const testOpenAIKey = process.env.OPENAI_API_KEY ?? (() => { throw new Error("OPENAI_API_KEY is not set") })()

Spotted by Graphite (based on custom rule: TypeScript style guide (Google))

Fix in Graphite


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

@Dhravya
Dhravya force-pushed the feat/ai-sdk-tool-reexports branch from acb8dc0 to 9c3f84b Compare August 8, 2026 02:47
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 8, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
supermemory-mcp ace3a76 Aug 23 2026, 02:34 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 8, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
supermemory-app ace3a76 Commit Preview URL

Branch Preview URL
Aug 23 2026, 02:36 PM

Dhravya commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

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

@claude claude Bot 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.

Overview: This PR replaces inline tool implementations in @supermemory/ai-sdk with re-exports from @supermemory/tools/ai-sdk, adding 5 new tools to the surface.

Issues found:

Cursor Bugbot correctly identified that includeFullDocs is accepted in the searchMemories schema but not passed to client.search() in packages/tools/src/ai-sdk.ts:59-65. I verified this — the parameter is destructured on line 55 but never used. This bug exists in the base branch (feat/tools-seven-tool-parity) and should be fixed in PR #1432, not here.

The re-export itself is correct — this PR simply surfaces whatever the shared package provides.

Score: 8/10

The code change is straightforward and the test coverage looks good. Fix the includeFullDocs bug in the base PR before merging this stack.

Comment thread packages/ai-sdk/src/tools.ts
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