Skip to content

refactor mastra to object-based API (v2.0.0)#829

Open
sreedharsreeram wants to merge 2 commits intomainfrom
04-03-refactor_mastra_to_object-based_api_v2.0.0_
Open

refactor mastra to object-based API (v2.0.0)#829
sreedharsreeram wants to merge 2 commits intomainfrom
04-03-refactor_mastra_to_object-based_api_v2.0.0_

Conversation

@sreedharsreeram
Copy link
Copy Markdown
Contributor

No description provided.

@mintlify
Copy link
Copy Markdown
Contributor

mintlify bot commented Apr 4, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
supermemory 🟢 Ready View Preview Apr 4, 2026, 4:40 AM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Apr 4, 2026

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 83baa43 Commit Preview URL Apr 04 2026, 06:44 PM

Copy link
Copy Markdown
Contributor Author


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
Copy link
Copy Markdown

claude bot commented Apr 4, 2026

Claude finished @sreedharsreeram's task in 1m 45s —— View job


Review Complete

  • Analyzing PR context and existing reviews
  • Getting full diff and reviewing changes
  • Checking for bugs, security issues, and breaking changes
  • Posting review summary

Overview: Refactors Mastra integration from positional arguments (containerTag, threadId, options) to a single object-based API with containerTag and customId as required properties.

Issues found: None — this looks good to ship.

Details:

  • Clean API migration with proper v2.0.0 version bump (packages/tools/package.json:4)
  • Migration guide included in README with old vs new API examples
  • Input validation is thorough: checks for empty strings and whitespace (processor.ts:64-82)
  • getEffectiveCustomId correctly prioritizes RequestContext for dynamic overrides (processor.ts:112-117), addressing the earlier Sentry comment
  • Breaking change is appropriately handled — TypeScript will catch old callers at compile time since containerTag and customId are now required fields in SupermemoryMastraOptions
  • Documentation updated in both README and apps/docs/integrations/mastra.mdx

Regarding previous review comments:

  • Sentry's dead code concern was fixed — the current implementation checks requestContext FIRST before falling back to the default
  • Entelligence's breaking change concern is valid but properly addressed: major version bump, migration docs, and TypeScript enforcement

Score: 10/10

This is a well-executed breaking change with comprehensive documentation, proper versioning, and good validation/error messages. The object-based API is cleaner and more explicit about required parameters.

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Overview: Refactors Mastra integration from positional arguments (containerTag, options) to a single object-based API with containerTag and conversationId as required properties.

Issues found: None — this looks good to ship.

Details:

  • Clean API migration: threadIdconversationId, both now required with proper validation
  • Input validation is thorough (checks empty strings, whitespace)
  • getEffectiveConversationId still supports fallback to Mastra's RequestContext for flexibility
  • Tests comprehensively updated to match new API
  • No security concerns, no race conditions, no breaking logic changes

Score: 10/10

This is a well-executed breaking change with good validation and error messages. The object-based API is cleaner and more explicit about required parameters.

@entelligence-ai-pr-reviews
Copy link
Copy Markdown


Confidence Score: 3/5 - Review Recommended

Not safe to merge without review — while this PR successfully unifies the Mastra integration API and adds meaningful eager validation in createProcessorContext, the signature change in withSupermemory from (config, containerTag, options?) to (config, options) is a silent breaking change that will cause runtime misbehavior for any existing caller passing a string as the second argument. Because containerTag was previously a positional string argument, old callers won't get a compile-time error — the string will simply be interpreted as the options object, silently discarding both the container tag and any real options object passed as the third argument. This is a major v2.0.0 release, so migration documentation and ideally a runtime guard or overload check in wrapper.ts should be in place before merging.

Key Findings:

  • In packages/tools/src/mastra/wrapper.ts, the withSupermemory function signature dropped the positional containerTag string parameter without any runtime detection of old-style callers; a caller using withSupermemory(config, 'tag', { apiKey }) will silently pass a string where SupermemoryMastraOptions is expected, causing containerTag and apiKey to both be lost with no error thrown.
  • The promotion of conversationId (formerly threadId) to a required field combined with the eager validation in createProcessorContext is a good defensive improvement, but it means any consumer that previously omitted threadId will now throw at runtime rather than silently proceeding — this needs to be clearly documented in the v2.0.0 migration guide.
  • The heuristic analysis flagged 1 significant issue (score 7.5) in wrapper.ts with no corresponding fix or deprecation shim present in the diff, which is the primary risk vector for this PR.
Files requiring special attention
  • packages/tools/src/mastra/wrapper.ts
  • packages/tools/src/mastra/index.ts

@entelligence-ai-pr-reviews
Copy link
Copy Markdown


Confidence Score: 4/5 - Mostly Safe

Safe to merge — the refactor cleanly redesigns @supermemory/tools Mastra integration to a unified options-object API with mandatory containerTag and conversationId fields, removing the ambiguous threadId pattern. No review comments were generated and heuristic analysis found zero critical, significant, or medium issues across the 8/10 reviewed changed files. The stricter error handling and consolidated factory function signatures (withSupermemory, createSupermemoryProcessor, createSupermemoryOutputProcessor, createSupermemoryProcessors) represent a well-scoped breaking change appropriate for a major version bump.

Key Findings:

  • All factory functions now enforce mandatory containerTag and conversationId fields at the type level, eliminating the runtime ambiguity that existed with the optional threadId field in the previous API design.
  • No logic bugs, null-safety issues, or security concerns were identified by automated heuristics or reviewer comments across the changed files, suggesting the implementation is internally consistent.
  • The breaking API change is correctly versioned as v2.0.0, and the removal of threadId is complete — no leftover references were flagged, indicating a clean migration rather than a partial refactor.
  • Two files from the changeset were not reviewed (2/10 uncovered), which is a minor gap — if those files contain integration glue code or exports, a manual spot-check would provide additional confidence before merging.
  • 1 previous unresolved comment(s) likely resolved in latest diff (score-only signal; thread status unchanged)
Files requiring special attention
  • src/mastra/index.ts
  • src/mastra/types.ts

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@socket-security
Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​better-fetch/​fetch@​1.1.21931007582100
Updated@​ai-sdk/​anthropic@​2.0.74 ⏵ 1.2.1277 -210084 -198100
Added@​ai-sdk/​groq@​1.2.91001008398100
Added@​ai-sdk/​google@​1.2.22971008398100

View full report

@entelligence-ai-pr-reviews
Copy link
Copy Markdown


Confidence Score: 4/5 - Mostly Safe

Safe to merge — this v2.0.0 breaking-change refactor cleanly unifies the API surface of @supermemory/tools Mastra integration by moving containerTag into the options object and renaming threadId to customId with proper validation. The PR is well-scoped and the heuristic analysis found no logic bugs, security concerns, or missing validations across the 8 reviewed files. The customId required-field guard (throwing '[supermemory] customId is required' on empty/whitespace) is a meaningful improvement over the previous implicit behavior, and the object-based API pattern is idiomatic and forward-compatible.

Key Findings:

  • The customId required-field validation in SupermemoryMastraOptions correctly rejects empty or whitespace values with a descriptive error, which is a positive improvement over a silently undefined threadId.
  • Moving containerTag from a positional parameter into the unified options object across withSupermemory, createSupermemoryProcessors, createSupermemoryProcessor, and createSupermemoryOutputProcessor reduces call-site ambiguity and is a clean, consistent API change.
  • No automated review comments were generated and heuristic analysis found zero critical, significant, or medium-severity issues, indicating the implementation is straightforward and the refactor is internally consistent.
  • As a breaking v2.0.0 release, callers of the previous positional containerTag API will silently receive incorrect behavior if not updated — the PR itself is correct, but downstream migration burden is non-trivial and documentation/changelog clarity is important.
Files requiring special attention
  • src/mastra/index.ts
  • src/mastra/types.ts
  • README.md

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.

1 participant