Add workspace namespace isolation#982
Open
scottgl9 wants to merge 2 commits into
Open
Conversation
|
@scottgl9 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughAdds workspace namespaces as a separate scope from project, threads namespace through config, keys, APIs, and read/write flows, and updates docs and tests for isolated and wildcard namespace behavior. ChangesWorkspace namespace isolation
Sequence Diagram(s)sequenceDiagram
participant Client
participant ApiSessionStart as api::session::start
participant MemContext as mem::context
participant KVSessions as KV.sessions
Client->>ApiSessionStart: start session with namespace
ApiSessionStart->>KVSessions: store Session(namespace)
ApiSessionStart->>MemContext: sessionId, project, namespace
MemContext->>KVSessions: load sessions by project and namespace
MemContext-->>ApiSessionStart: context result
ApiSessionStart-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Introduce a first-class namespace/workspace boundary that is separate from the existing project field. This adds AGENTMEMORY_NAMESPACE and AGENTMEMORY_NAMESPACE_SCOPE, stamps namespace onto sessions, observations, memories, lessons, and project profiles, and enforces namespace-aware filtering across the core recall and API surfaces. Key behavior changes: - add namespace env/config loading with shared vs isolated modes - stamp namespace on write paths including session start, observe, remember, compress, and synthetic compression - enforce namespace filtering in mem::search, mem::smart-search, mem::context, and mem::enrich - filter REST list/read endpoints for sessions, observations, and memories in isolated mode - keep project as an intra-namespace identifier instead of the top-level workspace boundary - key project profiles by namespace+project so the same project slug can exist in multiple workspaces without collisions - extend lessons to carry namespace and avoid cross-namespace fingerprint collisions - document the feature in README and .env.example Validation: - focused namespace regression tests passed - build passed with npm run build - full unit suite was green except for one pre-existing unrelated fs-watcher test failure
scottgl9
force-pushed
the
feature/workspace-namespaces-from-main
branch
from
75408f3 to
e159a48
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/functions/smart-search.ts (1)
226-231: 🔒 Security & Privacy | 🟠 MajorLessons returned by smart-search bypass namespace isolation
The
mem::lesson-recallfunction supports anamespaceparameter, but the localrecallLessonshelper insrc/functions/smart-search.tsdoes not accept or forward it. Consequently, even whenfilterNamespaceis applied to the main search query, the lessons retrieved viarecallLessonsignore this scope, allowing data from other namespaces to leak into results.Update
recallLessonsto accept and pass thenamespace:Proposed fix
async function recallLessons( sdk: ISdk, query: string, limit: number, project?: string, + namespace?: string, ): Promise<CompactLessonResult[]> { try { const result = (await sdk.trigger({ function_id: "mem::lesson-recall", - payload: { query, limit, project }, + payload: { query, limit, project, namespace }, })) as { success?: boolean; lessons?: Array<Lesson & { score?: number }>; };Update the invocation in
src/functions/smart-search.ts:- ? recallLessons(sdk, data.query, lessonLimit, data.project) + ? recallLessons(sdk, data.query, lessonLimit, data.project, filterNamespace)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/functions/smart-search.ts` around lines 226 - 231, The `recallLessons` helper in `smart-search` is not forwarding namespace scope, so lesson recall can return results outside the filtered namespace. Update `recallLessons` to accept a `namespace` argument and pass it through to the `mem::lesson-recall` call, then update the `Promise.all` invocation in `smart-search` to supply the same namespace used by the main search path (from `filterNamespace`/query context) so lessons stay isolated.
🧹 Nitpick comments (1)
src/functions/remember.ts (1)
80-82: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNamespace supersession guard is intentionally stricter than the project guard.
Unlike the project check above (Line 77, which treats an unscoped legacy memory as a wildcard), this guard uses strict equality, so a namespaced write will never supersede a legacy memory that has no
namespace, and vice versa. This matches the "namespace is the stronger boundary" / fail-closed intent, but the divergence from the adjacent project semantics is easy to misread. A one-line clarifying note would help future readers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/functions/remember.ts` around lines 80 - 82, Add a brief clarifying comment in the remember flow near the strict namespace comparison in remember() to explain that namespace matching is intentionally fail-closed and stricter than the project guard above. Make it explicit that a namespaced write should not supersede an unscoped legacy memory, and that this strict equality is by design to keep namespace as the stronger boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 1355: The README wording is inaccurate because /context, /search,
/smart-search, and /enrich are read-only paths, not mutating endpoints. Update
the sentence in the namespace override section to describe these as endpoints
accepting a namespace override, while keeping /session/start, /observe, and
/remember referenced clearly if needed.
In `@src/functions/claude-bridge.ts`:
- Around line 126-134: The project profile lookup in ClaudeBridge is building
the wrong key because it calls makeProjectProfileKey with only
config.projectPath, so it never matches profiles stored by mem::profile under
the namespaced key. Update the profile fetch in claude-bridge.ts to include the
global namespace from getNamespace() when calling makeProjectProfileKey, keeping
the existing kv.get logic and projectSummary assignment intact. Use the
ClaudeBridgeConfig projectPath check and the mem profile key builder as the main
symbols to locate the fix.
In `@src/functions/context.ts`:
- Around line 40-50: Normalize the namespace before using it in context lookups
so the read key matches the profile write key. In the context-building function
in context.ts, compute a normalized namespace once with
normalizeNamespace(data.namespace) and pass that value to makeProjectProfileKey,
then reuse the same normalized namespace for the lesson/session filters and
header logic instead of raw data.namespace. Also add normalizeNamespace to the
existing import alongside makeProjectProfileKey.
In `@src/functions/enrich.ts`:
- Line 33: The namespace handling in enrich is missing the configured fallback,
so it can end up with an undefined namespace and filter out namespaced memories
in isolated mode. Update the namespace derivation in enrich to match observe by
applying the same getNamespace() fallback before calling normalizeNamespace, and
ensure the resulting namespace is then used consistently in the memory filter
and search path so reads default to the configured namespace.
In `@src/functions/lessons.ts`:
- Around line 121-123: Normalize data.namespace before applying the lessons
filter so it matches the stored namespace form. Update the namespace checks in
the lesson recall and lesson list paths in lessons.ts to compare against
normalizeNamespace(data.namespace) instead of the raw input, using the same
normalization behavior as mem::lesson-save. Make the change in the relevant
lesson filtering logic so callers with whitespace or overlong namespace values
still match the saved lessons.
In `@src/functions/search.ts`:
- Around line 364-375: Remove the dead fail-closed guard in search flow: the
isolated-namespace check in search.ts is unreachable because
isNamespaceScopeIsolated() already implies getNamespace() is set. Delete the
entire if/throw block in the search path so the namespace filter continues to
default to getNamespace() without the unreachable error, and keep the existing
isolation behavior intact.
In `@src/triggers/api.ts`:
- Around line 836-839: The namespace resolution in the filtering paths can fall
back to undefined in isolated mode, causing unfiltered reads instead of failing
closed. Update the logic around filterNamespace in the affected
session/observation/memory handlers to distinguish an explicit wildcardNamespace
from an isolated-mode fallback, and guard the isolated fallback by requiring
getNamespace() to return a resolved namespace before proceeding. If
isNamespaceScopeIsolated() is true and no namespace is configured, return a
closed/error response rather than continuing with undefined filtering.
- Line 295: The REST handlers in src/triggers/api.ts are normalizing malformed
explicit namespace inputs to undefined, which can accidentally fall back to the
default namespace. Update the affected endpoint paths that use
normalizeNamespace(body.namespace) and the related request builders so they
validate “provided but invalid” namespace values at the REST boundary and return
400 instead of dropping them. Keep the whitelisted payload behavior, but only
forward namespace when it is a valid string; otherwise reject the request before
calling the downstream trigger logic.
In `@src/triggers/events.ts`:
- Around line 12-32: Normalize the namespace before creating the Session and
triggering mem::context in src/triggers/events.ts, since the current handler
uses raw data.namespace and can persist unscoped or invalid values. Update the
async event handler to resolve a default via getNamespace when data.namespace is
missing, run it through normalizeNamespace, and use that normalized value both
in the Session object and the sdk.trigger payload. Also add the needed imports
for getNamespace and normalizeNamespace.
In `@test/api-namespace-isolation.test.ts`:
- Around line 163-175: The test for api::session::start only checks the response
payload, so it can miss a regression where the persisted session in KV.sessions
is not stamped with the default namespace. Update the api::session::start test
to also inspect the stored session record after triggering the action, using the
existing sdk and Session flow, and assert that the persisted session’s namespace
matches the configured default namespace (the same value currently expected on
result.body.session.namespace).
- Around line 194-201: The `api::memories` isolation test only verifies the
active namespace filter and misses the wildcard override path. Add a test
alongside the existing `sdk.trigger("api::memories", ...)` coverage that passes
`namespace: "*"` in the request and asserts the memories list is not restricted
to the active namespace, mirroring the session wildcard case. Use the
`api::memories` trigger-based test pattern and the `Memory[]` response shape to
keep the new assertion consistent with the existing suite.
---
Outside diff comments:
In `@src/functions/smart-search.ts`:
- Around line 226-231: The `recallLessons` helper in `smart-search` is not
forwarding namespace scope, so lesson recall can return results outside the
filtered namespace. Update `recallLessons` to accept a `namespace` argument and
pass it through to the `mem::lesson-recall` call, then update the `Promise.all`
invocation in `smart-search` to supply the same namespace used by the main
search path (from `filterNamespace`/query context) so lessons stay isolated.
---
Nitpick comments:
In `@src/functions/remember.ts`:
- Around line 80-82: Add a brief clarifying comment in the remember flow near
the strict namespace comparison in remember() to explain that namespace matching
is intentionally fail-closed and stricter than the project guard above. Make it
explicit that a namespaced write should not supersede an unscoped legacy memory,
and that this strict equality is by design to keep namespace as the stronger
boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46a55749-8631-4386-b747-44e19e918901
📒 Files selected for processing (24)
.env.exampleREADME.mdsrc/config.tssrc/functions/claude-bridge.tssrc/functions/compress-synthetic.tssrc/functions/compress.tssrc/functions/context.tssrc/functions/enrich.tssrc/functions/export-import.tssrc/functions/lessons.tssrc/functions/observe.tssrc/functions/profile.tssrc/functions/remember.tssrc/functions/search.tssrc/functions/smart-search.tssrc/state/memory-utils.tssrc/triggers/api.tssrc/triggers/events.tssrc/types.tssrc/utils/namespace.tstest/agent-isolation-search.test.tstest/api-namespace-isolation.test.tstest/cross-project-isolation.test.tstest/namespace-isolation.test.ts
Author
|
Pushed follow-up commit Addressed:
Validated with:
I left the generic docstring-coverage warning alone since it is repo-wide and not introduced by this PR. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/triggers/api.ts (1)
893-899: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve namespace before listing KV records.
These handlers still perform
kv.list(...)before rejecting invalid namespaces or isolated-mode misconfiguration. Move namespace resolution ahead of the full list read so malformed requests fail closed cheaply.♻️ Proposed adjustment
- const sessions = await kv.list<Session>(KV.sessions); const namespaceResult = resolveListNamespaceFilter( parseNamespaceInput(req.query_params?.["namespace"], { allowWildcard: true }), ); if ("status_code" in namespaceResult) { return namespaceResult; } const { filterNamespace } = namespaceResult; + const sessions = await kv.list<Session>(KV.sessions);Apply the same ordering to
api::observationsandapi::memories.Also applies to: 944-950, 1974-1980
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/triggers/api.ts` around lines 893 - 899, Resolve the namespace before calling kv.list in the affected handlers so invalid namespaces and isolated-mode mismatches fail fast without scanning records. Update the api::triggers flow here, and apply the same ordering in api::observations and api::memories: call parseNamespaceInput and resolveListNamespaceFilter first, return any status_code immediately, and only then proceed to the list read using filterNamespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/triggers/api.ts`:
- Around line 893-899: Resolve the namespace before calling kv.list in the
affected handlers so invalid namespaces and isolated-mode mismatches fail fast
without scanning records. Update the api::triggers flow here, and apply the same
ordering in api::observations and api::memories: call parseNamespaceInput and
resolveListNamespaceFilter first, return any status_code immediately, and only
then proceed to the list read using filterNamespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5762b947-1e8b-420f-93d7-952dcb7d5e24
📒 Files selected for processing (11)
README.mdsrc/functions/claude-bridge.tssrc/functions/context.tssrc/functions/enrich.tssrc/functions/lessons.tssrc/functions/remember.tssrc/functions/search.tssrc/functions/smart-search.tssrc/triggers/api.tssrc/triggers/events.tstest/api-namespace-isolation.test.ts
💤 Files with no reviewable changes (1)
- src/functions/search.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- src/functions/claude-bridge.ts
- src/functions/remember.ts
- src/functions/lessons.ts
- src/functions/context.ts
- src/functions/enrich.ts
- src/functions/smart-search.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a first-class workspace namespace boundary that is separate from
project.This introduces:
AGENTMEMORY_NAMESPACEAGENTMEMORY_NAMESPACE_SCOPE=shared|isolatedand wires namespace-aware behavior across the core read/write surfaces.
Closes #981.
Why
projectscoping is useful, but it is not the same thing as a top-level workspace/environment boundary.In shared-daemon setups, users may need multiple isolated environments such as:
workpersonalresearchThe same project slug can exist in more than one workspace, and project-only filtering is not a strong enough model for that.
This change makes:
namespace= workspace boundaryproject= project identifier inside that workspaceWhat changed
src/config.tsnamespaceto sessions, observations, memories, lessons, and profilesmem::searchmem::smart-searchmem::contextmem::enrich/agentmemory/sessions/agentmemory/observations/agentmemory/memoriesnamespace + projectto avoid collisionsREADME.mdand.env.exampleValidation
Validated with:
npm run buildpassingSummary by CodeRabbit
New Features
namespace="*"wildcard handling for list-style access.Bug Fixes
Documentation
.env.exampleand README with workspace namespace configuration guidance.Tests