Conversation
Refresh plugin architecture with Supermemory - Add project-level configuration system (.claude/.supermemory-claude/config.json) for per-repo API keys and container tags - Add /project-config command to manage project settings - Add /super-save skill for saving project knowledge (architectural decisions, bug fixes, design patterns) - Add repo container tag support for team-shared memories (separate from personal memories) - Rename prompt-hook to save-project-memory with dedicated repo-level memory saving - Remove observation hook (consolidate into other hooks) - Enhance context hook, search memory, and summary hook scripts - Update supermemory client with repo entity context support - Remove MCP config file
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| } | ||
|
|
||
| function getApiKey(settings) { | ||
| function getApiKey(settings, cwd) { |
There was a problem hiding this comment.
Bug: The getApiKey function is called without the required cwd parameter at all call sites, causing it to incorrectly use process.cwd() and ignore project-specific configurations.
Severity: HIGH
Suggested Fix
Update all five call sites of getApiKey to pass the available cwd variable. For example, in src/context-hook.js:24, change getApiKey(settings) to getApiKey(settings, cwd). Apply similar changes to the other four locations.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/lib/settings.js#L70
Potential issue: The `getApiKey` function signature was updated to accept a `cwd`
parameter to support loading project-specific configurations. However, all five call
sites (`context-hook.js:24`, `summary-hook.js:38`, `add-memory.js:17`,
`search-memory.js:38`, `save-project-memory.js:17`) were not updated to pass this new
parameter. As a result, `getApiKey` receives `undefined` for `cwd` and defaults to using
`process.cwd()`. This breaks the project-specific API key feature when a tool or hook is
run from a directory other than the project's root, as it will look for the
configuration file in the wrong location.
Did we get this right? 👍 / 👎 to inform future reviews.
| function shouldIncludeTool(toolName, includeList) { | ||
| if (includeList.length === 0) return false; | ||
| return includeList.includes(toolName.toLowerCase()); |
There was a problem hiding this comment.
Bug: The new shouldIncludeTool logic defaults to excluding all tools because the default includeTools setting is an empty array, silently breaking tool interaction capture.
Severity: CRITICAL
Suggested Fix
Change the default behavior of shouldIncludeTool to include a set of default tools if the includeTools list is empty, preserving the previous functionality. Alternatively, update the default configuration to populate includeTools with the previous default tools. The README should also be updated to reflect the new includeTools setting.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/lib/settings.js#L106-L108
Potential issue: The logic for including tools in memory transcripts was changed to use
a single `includeTools` setting. The new `shouldIncludeTool` function at
`src/lib/settings.js:106~108` returns `false` if the `includeTools` list is empty. Since
the default value for this setting is an empty array, no tool interactions will be
captured by default. This is a silent breaking change from the previous behavior where
several tools were captured by default, and it will cause incomplete memory storage
without any user configuration changes.
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
v2: Claude Code Supermemory Refreshed
Refresh plugin architecture with Supermemory