fix(tools): reject conflicting container config#1214
Conversation
SummaryReviewed the changes in Verdict✅ Reviewed — no issues found. The diff is narrowly scoped, the conflict check is placed before tag selection, and the added tests cover the relevant behavior. |
TestingTargeted tooling validation was performed for the changed Commands run: cd packages/tools && export PATH="$HOME/.bun/bin:$PATH"; bun run test src/tools-shared.test.ts
cd packages/tools && export PATH="$HOME/.bun/bin:$PATH"; bunx tsc --noEmit --module ESNext --moduleResolution bundler --target ES2022 --strict --skipLibCheck --types vitest src/tools-shared.test.ts
git diff --check origin/main..HEAD -- packages/tools/src/tools-shared.ts packages/tools/src/tools-shared.test.tsResult: Verdict✅ Passed. The changed helper behavior is covered by targeted tests and focused validation passed. |
| if (config?.projectId !== undefined && config.containerTags !== undefined) { | ||
| throw new Error( | ||
| "Supermemory tools config accepts either projectId or containerTags, not both.", | ||
| ) | ||
| } |
There was a problem hiding this comment.
The validation check is inconsistent with the actual usage logic below. The validation uses !== undefined while line 71 uses truthy checking (if (config?.projectId)). This means edge cases like projectId: "" or projectId: null will trigger the "both provided" error even though they are falsy and would be ignored by the actual logic.
For example:
getContainerTags({ projectId: "", containerTags: ["tag"] })Will throw an error claiming both are provided, even though the empty string would be treated as "not provided" by line 71's truthy check.
Fix by aligning the validation with the actual usage:
if (config?.projectId && config?.containerTags) {
throw new Error(
"Supermemory tools config accepts either projectId or containerTags, not both.",
)
}| if (config?.projectId !== undefined && config.containerTags !== undefined) { | |
| throw new Error( | |
| "Supermemory tools config accepts either projectId or containerTags, not both.", | |
| ) | |
| } | |
| if (config?.projectId && config?.containerTags) { | |
| throw new Error( | |
| "Supermemory tools config accepts either projectId or containerTags, not both.", | |
| ) | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Description
This PR adds runtime validation for @supermemory/tools container scoping configuration.
SupermemoryToolsConfig already documents projectId and containerTags as mutually exclusive,
but the implementation did not enforce that. When both were provided, getContainerTags()
silently preferred projectId and ignored containerTags.
Problem
Before this change:
supermemoryTools(apiKey, {
projectId: "abc",
containerTags: ["user_123"],
})
silently resolved to:
["sm_project_abc"]
The explicit containerTags value was ignored, which could cause memory/search operations to
run against an unexpected scope.
Changes
Why This Matters
This makes runtime behavior match the public config contract and prevents accidental use of
the wrong memory container.
Files Changed
Testing
Passed:
bun run --cwd packages/tools test src/tools-shared.test.ts
bunx biome check packages/tools/src/tools-shared.ts packages/tools/src/tools-shared.test.ts
Also ran:
bun run --cwd packages/tools check-types
That command currently fails on existing unrelated package-wide TypeScript errors outside this
change.
Session Details
(aside)to your comment to have me ignore it.