Skip to content

Conversation

@codewarnab
Copy link

Problem

withSupermemory only read the API key from process.env.SUPERMEMORY_API_KEY, which does not work in browser environments.

Solution

  • Added optional apiKey field to WithSupermemoryOptions
  • Falls back to process.env.SUPERMEMORY_API_KEY if not provided
  • Updated middleware to accept and use the passed API key
  • Updated README documentation

Usage

Browser usage - pass apiKey directly:
const model = withSupermemory(openai('gpt-4'), 'user-123', { apiKey: 'your-key-here' });

Server usage - still works with env variable:
const model = withSupermemory(openai('gpt-4'), 'user-123');"

@graphite-app
Copy link

graphite-app bot commented Dec 1, 2025

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.

@claude
Copy link

claude bot commented Dec 2, 2025

Code Review for PR #599: feat(tools): allow passing apiKey via options for browser support

Summary

This PR adds browser support by allowing the API key to be passed via options instead of requiring process.env.SUPERMEMORY_API_KEY. Overall, this is a well-implemented feature that solves a real problem. Here's my detailed feedback:


✅ Strengths

  1. Clear problem statement - The PR description clearly explains the issue and solution
  2. Backward compatibility - Falls back to environment variable, ensuring no breaking changes
  3. Documentation updated - README.md properly updated with usage examples
  4. Consistent implementation - Changes are minimal and focused on the specific problem
  5. Good code quality - Follows existing patterns and conventions

🔍 Issues & Concerns

1. Missing Test Coverage ⚠️ HIGH PRIORITY

The most critical issue is that no tests were added for the new functionality. The existing test suite at /packages/tools/test/vercel.test.ts needs to be updated.

Required test additions:

// Test passing apiKey via options
it("should accept apiKey via options parameter", () => {
  const mockModel = createMockLanguageModel()
  const wrappedModel = withSupermemory(mockModel, TEST_CONFIG.containerTag, {
    apiKey: "custom-api-key"
  })
  
  expect(wrappedModel).toBeDefined()
})

// Test that options.apiKey takes precedence over env var
it("should prioritize options.apiKey over environment variable", () => {
  process.env.SUPERMEMORY_API_KEY = "env-key"
  const mockModel = createMockLanguageModel()
  
  const wrappedModel = withSupermemory(mockModel, TEST_CONFIG.containerTag, {
    apiKey: "options-key"
  })
  
  // Would need to spy on Supermemory constructor to verify "options-key" is used
  expect(wrappedModel).toBeDefined()
})

// Test error message when neither is provided
it("should throw descriptive error when neither apiKey option nor env var is set", () => {
  delete process.env.SUPERMEMORY_API_KEY
  const mockModel = createMockLanguageModel()
  
  expect(() => {
    withSupermemory(mockModel, TEST_CONFIG.containerTag, {})
  }).toThrow("SUPERMEMORY_API_KEY is not set — provide it via `options.apiKey` or set `process.env.SUPERMEMORY_API_KEY`")
})

2. Inconsistent Function Signature 📋 MEDIUM PRIORITY

In middleware.ts:69-75, the function signature is updated to accept apiKey:

export const createSupermemoryMiddleware = (
  containerTag: string,
  conversationId?: string,
  verbose = false,
  mode: "profile" | "query" | "full" = "profile",
  addMemory: "always" | "never" = "never",
  apiKey?: string,  // <-- New parameter added at the end
)

However, this is not exported in the public API. The function is only called internally by wrapVercelLanguageModel in /packages/tools/src/vercel/index.ts:67.

Recommendation: This is acceptable since it's an internal function, but verify that:

  • If createSupermemoryMiddleware is exported and used elsewhere, this could be a breaking change
  • Consider if the function should accept an options object instead of 6+ parameters (future refactor consideration)

3. Security Consideration 🔒 LOW-MEDIUM PRIORITY

The apiKey is now being passed around more openly. While this is necessary for browser support, consider:

Current code (middleware.ts:78-80):

const client = new Supermemory({
  apiKey: apiKey ?? process.env.SUPERMEMORY_API_KEY,
})

Suggestions:

  • Add a warning in the documentation about not hardcoding API keys in client-side code
  • Consider mentioning environment variables or secure key management for production browser apps
  • The README update mentions "recommended for browser usage" but doesn't warn about key exposure risks

Recommended README addition:

⚠️ **Security Note**: When using `apiKey` in browser environments, never hardcode your API key in client-side code. Use environment variables or fetch the key from your backend API to avoid exposing it in your JavaScript bundle.

4. Type Safety Enhancement 💡 MINOR IMPROVEMENT

The WithSupermemoryOptions interface is well-defined, but consider making the apiKey field more discoverable:

Current (index.ts:7-11):

interface WrapVercelLanguageModelOptions {
  conversationId?: string;
  verbose?: boolean;
  mode?: "profile" | "query" | "full";
  addMemory?: "always" | "never";
  /** Optional Supermemory API key — will fall back to `process.env.SUPERMEMORY_API_KEY` if not provided */
  apiKey?: string;
}

This looks good! The JSDoc comment is clear.

5. Error Message Consistency ✅ GOOD

The updated error message in index.ts:59 is excellent:

throw new Error("SUPERMEMORY_API_KEY is not set — provide it via `options.apiKey` or set `process.env.SUPERMEMORY_API_KEY`")

This is clear and actionable. ✅


📝 Minor Suggestions

  1. README Formatting: The updated README section could use a code example showing the security-conscious approach:
// ❌ Don't do this in browser code
const model = withSupermemory(openai('gpt-4'), 'user-123', { 
  apiKey: 'sm-abc123...' // Exposed in bundle!
})

// ✅ Do this instead
const apiKey = await fetch('/api/get-supermemory-key').then(r => r.text())
const model = withSupermemory(openai('gpt-4'), 'user-123', { apiKey })
  1. Type Export: Ensure WithSupermemoryOptions is exported if users need to reference it

🧪 Testing Checklist

Before merging, please verify:

  • Add tests for passing apiKey via options
  • Add tests for apiKey precedence (options > env var)
  • Add tests for updated error message
  • Manual testing in browser environment
  • Manual testing in Node.js environment (backward compatibility)
  • Run existing test suite: bun test packages/tools/test/vercel.test.ts

Performance & Architecture

  • ✅ No performance concerns - the change is minimal and only affects initialization
  • ✅ No architectural concerns - maintains existing patterns
  • ✅ Properly maintains backward compatibility

Verdict

Recommended Action: Request changes for test coverage, then approve.

The implementation is solid, but test coverage is mandatory before merging. Once tests are added addressing the security documentation point would be a valuable improvement.

Estimated effort to address: ~30 minutes for tests, ~10 minutes for README security note.

Great work solving the browser compatibility issue! 🎉

@claude
Copy link

claude bot commented Dec 2, 2025

Code Review: Browser Support for API Key

Summary

This PR adds the ability to pass an API key via options to withSupermemory, enabling browser-side usage where process.env is not available. The implementation is clean and maintains backward compatibility.


✅ Code Quality & Best Practices

Strengths:

  • Backward compatible: Falls back to process.env.SUPERMEMORY_API_KEY when not provided
  • Clear error messages: Updated error message clearly explains both options for providing the API key
  • Consistent implementation: API key is threaded through both index.ts and middleware.ts properly
  • Documentation updated: README correctly explains the new usage pattern

Minor suggestions:

  1. Type consistency: The apiKey? field is added to both WrapVercelLanguageModelOptions and the middleware function signature, but they could benefit from a shared type or JSDoc comment clarification about when to use each approach.

  2. Naming clarity: Consider renaming providedApiKey to apiKey in index.ts:57 since it's already clear from the nullish coalescing that it can come from two sources:

    const apiKey = options?.apiKey ?? process.env.SUPERMEMORY_API_KEY

🐛 Potential Issues

Missing parameter in middleware call (packages/tools/src/vercel/index.ts:68)

  • The createSupermemoryMiddleware function now expects 6 parameters (containerTag, conversationId, verbose, mode, addMemory, apiKey), but the call passes it correctly. ✅

🔒 Security Concerns

API Key Exposure Risk ⚠️
When used in browsers, the API key will be exposed in client-side code. Consider adding a warning in the documentation:

/**
 * @param options.apiKey - Optional Supermemory API key. 
 *   WARNING: Only use this in trusted browser environments. 
 *   For production browser apps, use a server-side proxy to protect your API key.
 *   Falls back to process.env.SUPERMEMORY_API_KEY for server environments.
 */

Recommendation: Add a security note in the README explaining best practices:

  • Browser usage should be limited to development/testing
  • Production apps should proxy requests through a backend to keep API keys secure

⚡ Performance Considerations

No performance concerns. The changes are minimal and don't introduce any overhead.


🧪 Test Coverage

Missing tests for the new feature

The existing test suite in packages/tools/test/vercel.test.ts only tests the environment variable path. Should add:

it('should use provided apiKey from options', () => {
  delete process.env.SUPERMEMORY_API_KEY
  
  const mockModel = createMockLanguageModel()
  const wrappedModel = withSupermemory(
    mockModel, 
    TEST_CONFIG.containerTag,
    { apiKey: 'explicit-api-key' }
  )
  
  expect(wrappedModel).toBeDefined()
})

it('should prefer options.apiKey over environment variable', () => {
  process.env.SUPERMEMORY_API_KEY = 'env-key'
  
  // Test that options.apiKey takes precedence
  // Would need to spy on Supermemory constructor to verify
})

📋 Recommendations

  1. Add tests for the new apiKey option parameter
  2. Add security warning in JSDoc and README about browser usage
  3. Consider example code showing both server and browser usage patterns
  4. (Optional) Add a dangerouslyExposeApiKeyInBrowser flag that must be explicitly set to true for browser usage, forcing developers to acknowledge the security implications

Overall Assessment

Status: ✅ Approved with suggestions

The implementation is solid and the feature is useful. The main concerns are:

  • Missing test coverage for the new parameter
  • Security documentation around browser API key exposure

These can be addressed in a follow-up or before merging. Great work on maintaining backward compatibility! 🎉

@claude
Copy link

claude bot commented Dec 2, 2025

Pull Request Review

Thank you for this contribution! This is a solid enhancement that enables browser usage of the withSupermemory middleware. Overall, the implementation is clean and well-thought-out. Here's my detailed feedback:

✅ Strengths

  1. Good API Design: The fallback pattern (options?.apiKey ?? process.env.SUPERMEMORY_API_KEY) is elegant and maintains backward compatibility
  2. Clear Documentation: Updated README with explicit guidance on when to use each approach
  3. Proper Error Messaging: Enhanced error message clearly explains both options for providing the API key
  4. Type Safety: Properly typed optional parameter with JSDoc comments
  5. Backward Compatible: No breaking changes for existing users

🔍 Code Quality Observations

Good:

  • Consistent parameter threading through the call stack
  • Minimal code changes focused on the specific feature
  • Clear separation of concerns

Minor Notes:

  • The implementation correctly passes the apiKey through to the middleware
  • The typing aligns well with the existing codebase patterns

🧪 Test Coverage

Issue: The PR does not include tests for the new apiKey option functionality.

The existing test file (test/vercel.test.ts) has comprehensive coverage but only tests the environment variable path. We should add tests for:

  1. Passing apiKey via options:
it('should accept apiKey via options parameter', () => {
  delete process.env.SUPERMEMORY_API_KEY
  const mockModel = createMockLanguageModel()
  
  const wrappedModel = withSupermemory(mockModel, 'test-tag', { 
    apiKey: 'explicit-api-key' 
  })
  
  expect(wrappedModel).toBeDefined()
})
  1. Priority order (options.apiKey takes precedence over env):
it('should prioritize options.apiKey over environment variable', () => {
  process.env.SUPERMEMORY_API_KEY = 'env-key'
  const mockModel = createMockLanguageModel()
  
  // Would need to verify the middleware receives 'explicit-key'
  const wrappedModel = withSupermemory(mockModel, 'test-tag', { 
    apiKey: 'explicit-key' 
  })
  
  expect(wrappedModel).toBeDefined()
})
  1. Error when neither is provided:
it('should throw error when neither options.apiKey nor env var is set', () => {
  delete process.env.SUPERMEMORY_API_KEY
  const mockModel = createMockLanguageModel()
  
  expect(() => {
    withSupermemory(mockModel, 'test-tag', {})
  }).toThrow('SUPERMEMORY_API_KEY is not set')
})

🔒 Security Considerations

Good:

  • No security vulnerabilities introduced
  • API key handling follows standard practices

Consideration:

  • For browser usage, developers should be aware that API keys will be exposed in client-side code
  • Consider adding a security note in the README warning about this (though this is implied by "browser usage")

⚡ Performance Considerations

  • No performance impact
  • The nullish coalescing operator (??) is efficient

📝 Suggestions

  1. Add test coverage for the new parameter (as detailed above)
  2. Optional - Add security note in README:
> **⚠️ Security Note**: When using `apiKey` in browser environments, be aware that the API key will be visible in client-side code. Consider using environment variables with proper access controls, or implementing a backend proxy for sensitive applications.

📋 Summary

This is a well-implemented feature that solves a real problem for browser-based usage. The code quality is good and the approach is sound.

Recommendation:Approve with minor improvements

The main gap is test coverage. I'd suggest adding the tests mentioned above before merging to maintain the repository's testing standards.


Review generated with assistance from Claude Code

@claude
Copy link

claude bot commented Dec 2, 2025

Pull Request Review: feat(tools): allow passing apiKey via options for browser support

Summary

This PR adds support for passing the API key via function options, enabling browser usage where process.env is not available. Overall, this is a well-implemented feature with clear use cases.


Strengths

  1. Clear Problem & Solution: The PR addresses a real limitation - browser environments don't have access to process.env.

  2. Backward Compatibility: The implementation maintains full backward compatibility by falling back to process.env.SUPERMEMORY_API_KEY when the option isn't provided.

  3. Good Documentation: The README and JSDoc comments have been properly updated to reflect the new option.

  4. Consistent Error Messages: The error message was updated to clearly explain both ways to provide the API key.


🔍 Code Quality Issues

1. Type Definition Inconsistency

The apiKey field was added to WrapVercelLanguageModelOptions (lines 5-10 in index.ts), but not to the exported type alias. The interface at lines 5-10 should match the documented interface in the README.

Current:

interface WrapVercelLanguageModelOptions {
	conversationId?: string;
	verbose?: boolean;
	mode?: "profile" | "query" | "full";
	addMemory?: "always" | "never";
	apiKey?: string; // ✅ Added here
}

export { wrapVercelLanguageModel as withSupermemory, type WrapVercelLanguageModelOptions as WithSupermemoryOptions }

This is actually correct - good job keeping them in sync!

2. Middleware Function Signature Complexity

The createSupermemoryMiddleware function now takes 6 parameters (packages/tools/src/vercel/middleware.ts:69-75). This is approaching the limit where parameter objects become more maintainable.

Current:

export const createSupermemoryMiddleware = (
	containerTag: string,
	conversationId?: string,
	verbose = false,
	mode: "profile" | "query" | "full" = "profile",
	addMemory: "always" | "never" = "never",
	apiKey?: string, // ✅ Added as 6th parameter
)

Suggestion: Consider refactoring to use an options object in a future PR:

export const createSupermemoryMiddleware = (
	containerTag: string,
	options?: {
		conversationId?: string;
		verbose?: boolean;
		mode?: "profile" | "query" | "full";
		addMemory?: "always" | "never";
		apiKey?: string;
	}
)

This would make future additions easier and improve readability. Note: This is a suggestion for future work, not a blocker for this PR.


🧪 Test Coverage

⚠️ Missing Test Coverage

The existing test suite (test/vercel.test.ts) does not include tests for the new apiKey option. The test on line 65-73 only tests the environment variable path:

it("should throw error if SUPERMEMORY_API_KEY is not set", () => {
	delete process.env.SUPERMEMORY_API_KEY
	const mockModel = createMockLanguageModel()
	expect(() => {
		withSupermemory(mockModel, TEST_CONFIG.containerTag)
	}).toThrow("SUPERMEMORY_API_KEY is not set")
})

Recommended Tests to Add:

  1. API key via options takes precedence over environment variable

    it("should use apiKey from options when both env and options are provided", () => {
      process.env.SUPERMEMORY_API_KEY = "env-key"
      const mockModel = createMockLanguageModel()
      const wrappedModel = withSupermemory(mockModel, "tag", { apiKey: "option-key" })
      // Verify option-key is used, not env-key
    })
  2. API key via options when env is not set

    it("should work with apiKey from options when env is not set", () => {
      delete process.env.SUPERMEMORY_API_KEY
      const mockModel = createMockLanguageModel()
      const wrappedModel = withSupermemory(mockModel, "tag", { apiKey: "option-key" })
      expect(wrappedModel).toBeDefined()
    })
  3. Error message when neither is provided

    it("should throw descriptive error when neither env nor option is set", () => {
      delete process.env.SUPERMEMORY_API_KEY
      const mockModel = createMockLanguageModel()
      expect(() => {
        withSupermemory(mockModel, "tag", {})
      }).toThrow("provide it via `options.apiKey` or set `process.env.SUPERMEMORY_API_KEY`")
    })

🔒 Security Considerations

⚠️ API Key Exposure Risk in Browser

When using this feature in browser environments, developers need to be aware of the security implications:

  1. API keys will be visible in browser DevTools - any key passed to the client-side code can be extracted
  2. Potential for abuse - exposed API keys can be used maliciously

Recommendation: Add a security warning to the README:

### Security Warning for Browser Usage

When using `apiKey` option in browser environments, be aware that:
- The API key will be visible in browser DevTools and network requests
- Never use privileged/admin API keys in browser code
- Consider implementing a backend proxy that handles API authentication
- Use user-specific, rate-limited API keys for client-side usage

Performance Considerations

No performance concerns identified. The change is a simple parameter pass-through with negligible overhead.


📋 Other Observations

  1. Consistent Code Style: The code follows the existing patterns and Biome formatting standards ✅

  2. TypeScript Types: All changes maintain proper type safety ✅

  3. Breaking Changes: None - this is purely additive ✅

  4. README Example: Good to include a browser usage example, though it could be more complete:

    // More realistic browser example:
    const apiKey = await fetchUserApiKey(); // From your auth system
    const model = withSupermemory(openai('gpt-4'), userId, { apiKey });

🎯 Recommendation

Approve with minor suggestions:

  • ✅ The core implementation is solid and solves the stated problem
  • ⚠️ Add test coverage for the new feature before merging
  • 💡 Consider adding security warnings to documentation
  • 💡 Future refactoring opportunity: convert to options object pattern

📝 Action Items for Merge

Must Have (Blocking):

  • Add test cases for apiKey option (3 tests suggested above)

Nice to Have (Non-blocking):

  • Add security warning section to README
  • Add more complete browser usage example
  • Consider future refactoring to options object (separate PR)

Overall Assessment: This is a valuable feature that's well-implemented. With the addition of proper test coverage, this PR is ready to merge. Great work on maintaining backward compatibility and updating documentation! 🎉

@claude
Copy link

claude bot commented Dec 3, 2025

Pull Request Review: feat(tools): allow passing apiKey via options for browser support

Overall Assessment

This is a well-implemented feature that adds browser support by allowing the API key to be passed as an option. The changes are minimal, focused, and maintain backward compatibility.

Code Quality & Best Practices

Strengths:

  • Backward compatible: Falls back to process.env.SUPERMEMORY_API_KEY when apiKey is not provided
  • Consistent implementation: The pattern is applied consistently across all three modified files
  • Clear documentation: JSDoc comments and README updated appropriately
  • Minimal footprint: Only 12 additions, 7 deletions - focused change
  • Proper TypeScript: Uses optional parameter syntax
  • Follows existing patterns: Matches the style of other options in the interface

Potential Bugs/Issues

Minor concern: The createSupermemoryMiddleware function signature change could theoretically be a breaking change if anyone is calling this function directly. However, this is likely an internal function and the parameter is optional.

Recommendation: Consider documenting in release notes that createSupermemoryMiddleware signature changed if it is part of public API.

Security Concerns

Good practices:

  • API key handling is appropriate
  • No hardcoded credentials
  • Maintains the same security posture as before

Browser exposure consideration:

  • When using apiKey option in browser environments, developers need to be careful not to expose their API keys in client-side code
  • Recommendation: Add a warning in the README that API keys should never be hardcoded in browser bundles

Performance Considerations

No performance impact - simple parameter passing with negligible overhead

Test Coverage

Current state:

  • Existing tests cover environment variable validation
  • Missing test coverage for the new apiKey option parameter

Recommended test additions:

  1. Should accept apiKey via options parameter
  2. Should prioritize options.apiKey over environment variable
  3. Should throw descriptive error when neither apiKey option nor env var is set

Additional Suggestions

The error message is excellent and very clear about both options. Consider adding more usage examples in the README showing both browser and server patterns.

Verdict

LGTM with minor suggestions - This PR is production-ready.

Before merging (optional but recommended):

  1. Add test coverage for the new apiKey option
  2. Add security warning to README about not exposing API keys in browser bundles
  3. Update release notes if createSupermemoryMiddleware is part of public API

Great work @codewarnab! This is a clean, focused contribution that adds real value.

Copilot AI review requested due to automatic review settings December 3, 2025 23:06
@graphite-app graphite-app bot requested a review from Dhravya December 3, 2025 23:06
Copy link
Member

@MaheshtheDev MaheshtheDev left a comment

Choose a reason for hiding this comment

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

tweaked few changes and updated the documentations. Now it all looks goo

@graphite-app
Copy link

graphite-app bot commented Dec 3, 2025

Movie gif. Sacha Baron Cohen as Borat sits in front of a map and raises his two thumbs, waggling his eyebrows and giving an approving nod and toothy grin. (Added via Giphy)

@MaheshtheDev MaheshtheDev enabled auto-merge (squash) December 3, 2025 23:07
Copilot finished reviewing on behalf of MaheshtheDev December 3, 2025 23:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to enable browser support for the withSupermemory middleware by allowing the Supermemory API key to be passed via an options.apiKey parameter instead of requiring process.env.SUPERMEMORY_API_KEY. The implementation adds an optional apiKey field to the options interface and updates the middleware to accept this parameter.

Key Changes:

  • Added optional apiKey field to WithSupermemoryOptions interface
  • Updated createSupermemoryMiddleware to accept apiKey as a parameter
  • Modified error message to guide users on both ways to provide the API key
  • Updated README documentation to mention the new option

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
packages/tools/src/vercel/index.ts Added apiKey to options interface, updated error handling to check both options.apiKey and process.env, and passes API key to middleware
packages/tools/src/vercel/middleware.ts Updated function signature to accept apiKey parameter and passes it to Supermemory client constructor
packages/tools/README.md Updated documentation to mention the new apiKey option and its usage patterns

Critical Issues Found:

  1. Breaking API Change: The createSupermemoryMiddleware function signature has been completely changed from accepting a Supermemory client instance as the first parameter to accepting containerTag and apiKey strings. This breaks all existing tests and potentially external users of this exported function.

  2. Incomplete Implementation: The addSystemPrompt function (called within the middleware) still uses process.env.SUPERMEMORY_API_KEY directly in its fetch call to the Supermemory API. This means the browser support is incomplete - the code will still fail in browser environments even when users provide an API key via options.apiKey.

  3. Test Failures: The error message was updated, which will cause the existing test expecting "SUPERMEMORY_API_KEY is not set" to fail with the new longer error message.

Comments suppressed due to low confidence (1)

packages/tools/src/vercel/middleware.ts:76

  • The function signature for createSupermemoryMiddleware has been changed in a breaking way. The old signature accepted a Supermemory client instance as the first parameter, but now expects containerTag (string) and apiKey (string) as the first two parameters.

This breaks all existing tests in test/vercel.test.ts which call this function with the old signature like createSupermemoryMiddleware(mockSupermemory, TEST_CONFIG.containerTag). The tests will fail because they're passing a Supermemory client instance where a string is now expected.

Additionally, this is exported as a public API function, so this breaking change could affect external users of the library. Consider either:

  1. Updating all tests to match the new signature, or
  2. Maintaining backward compatibility by accepting both signatures (e.g., using function overloads or checking the type of the first parameter)
export const createSupermemoryMiddleware = (
	containerTag: string,
	apiKey: string,
	conversationId?: string,
	verbose = false,
	mode: "profile" | "query" | "full" = "profile",
	addMemory: "always" | "never" = "never",
): LanguageModelV2Middleware => {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +59 to +60
if (!providedApiKey) {
throw new Error("SUPERMEMORY_API_KEY is not set — provide it via `options.apiKey` or set `process.env.SUPERMEMORY_API_KEY`")
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The error message has been updated to be more descriptive, but this will break the existing test in test/vercel.test.ts at line 72 which expects the error message to be exactly "SUPERMEMORY_API_KEY is not set".

The test uses .toThrow("SUPERMEMORY_API_KEY is not set") which will fail because the new error message is "SUPERMEMORY_API_KEY is not set — provide it via \options.apiKey` or set `process.env.SUPERMEMORY_API_KEY`"`.

The test should be updated to either:

  1. Match the new error message, or
  2. Use a partial match like .toThrow(/SUPERMEMORY_API_KEY is not set/)

Copilot uses AI. Check for mistakes.
Comment on lines 79 to 81
const client = new Supermemory({
apiKey: process.env.SUPERMEMORY_API_KEY,
apiKey,
})
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The middleware now accepts apiKey as a parameter and passes it to the Supermemory client constructor (line 80), but this is insufficient for browser support. The addSystemPrompt function (called later in this middleware) still uses process.env.SUPERMEMORY_API_KEY directly in its fetch call to the Supermemory API (in memory-prompt.ts line 23).

This means that even when users provide an API key via options.apiKey, the code will still fail in browser environments when addSystemPrompt tries to access process.env.

The apiKey parameter needs to be passed through to addSystemPrompt and supermemoryProfileSearch functions as well to fully support browser environments.

Copilot uses AI. Check for mistakes.
apiKey?: string
}
```

Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The apiKey option is documented in the JSDoc comment (line 398-399), but it would be helpful to also add a descriptive bullet point below this code block (similar to conversationId, verbose, mode, and addMemory) to document the fallback behavior and usage guidance.

Suggested addition after line 401:

  • apiKey: Optional Supermemory API key. Use this in browser environments where environment variables are not available (falls back to process.env.SUPERMEMORY_API_KEY if not provided)
Suggested change
- **apiKey**: Optional Supermemory API key. Use this in browser environments where environment variables are not available (falls back to `process.env.SUPERMEMORY_API_KEY` if not provided)

Copilot uses AI. Check for mistakes.
Comment on lines 59 to +60
- `withSupermemory` will take advantage supermemory profile v4 endpoint personalized based on container tag
- Make sure you have `SUPERMEMORY_API_KEY` in env
- You can provide the Supermemory API key via the `apiKey` option to `withSupermemory` (recommended for browser usage), or fall back to `SUPERMEMORY_API_KEY` in the environment for server usage.
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

[nitpick] The documentation mentions that the apiKey option is "recommended for browser usage" (line 60), but there's no code example demonstrating how to use it. Consider adding an example after the basic usage example to show browser usage:

// Browser usage - pass apiKey directly
const modelWithMemory = withSupermemory(openai("gpt-5"), "user_id_life", {
  apiKey: "your-supermemory-api-key"
})

This would make it clearer how users should implement the feature that this PR is introducing.

Copilot uses AI. Check for mistakes.
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