From ebc20794f3547cd373a4cc5c927ea46d7369a78c Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 4 May 2026 09:44:06 -0700 Subject: [PATCH] fix(admin-memories): include each member's private namespace in export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReadableNamespaces(rootID) returns {workspace:rootID, team:rootID, org:rootID} — the workspace: namespace it surfaces is the root's only. The I3 batching change resolved namespaces once per root which silently dropped every child workspace's private memories from admin export (workspace:childID never reached the plugin search). Keep the per-root batching win for team:/org:/custom: namespaces; inject each member's workspace: + owner mapping explicitly so coverage matches the legacy per-workspace iteration. Cost stays at 1 SQL + N_roots resolver + 1 plugin search. Test changes: - New TestExport_IncludesEveryMembersPrivateNamespace uses a per-workspace resolver stub (mirrors real behaviour) and asserts every member's workspace: reaches the plugin search AND that children's private memories appear in the response with correct owner attribution. Verified to FAIL on the pre-fix code. - TestExport_BatchesPluginCallsByRoot updated to expect 5 namespaces (3 workspace + team + org) instead of 3 — it had pinned the buggy 3-namespace behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/handlers/admin_memories.go | 36 +++-- .../handlers/admin_memories_cutover_test.go | 129 +++++++++++++++++- 2 files changed, 152 insertions(+), 13 deletions(-) diff --git a/workspace-server/internal/handlers/admin_memories.go b/workspace-server/internal/handlers/admin_memories.go index 1cf3d8d3..15e66641 100644 --- a/workspace-server/internal/handlers/admin_memories.go +++ b/workspace-server/internal/handlers/admin_memories.go @@ -290,31 +290,47 @@ func (h *AdminMemoriesHandler) exportViaPlugin(c *gin.Context, ctx context.Conte rootToWorkspaces[w.RootID] = append(rootToWorkspaces[w.RootID], w) } - // 3. Resolve namespaces once per root + build namespace→workspace - // map for the eventual export-row mapping. - nsToOwner := make(map[string]string) // namespace → workspace_name (first matching wins) - allNamespaces := make(map[string]struct{}) // union for plugin search + // 3. Resolve team/org namespaces once per root, then add each + // member's private workspace: namespace explicitly. + // + // IMPORTANT: ReadableNamespaces(rootID) returns + // {workspace:rootID, team:rootID, org:rootID}. Calling it once + // per root is enough for team:/org:/custom: (those are shared by + // every member of the root group), but the workspace: namespace + // it returns is rootID's only — child members' private + // workspace: namespaces would be silently dropped from + // the export. Inject each member's workspace: below to keep + // coverage parity with the legacy per-workspace iteration. + nsToOwner := make(map[string]string) // namespace → workspace_name (first matching wins) + allNamespaces := make(map[string]struct{}) // union for plugin search for rootID, members := range rootToWorkspaces { - // Use the root workspace's id for resolution — any member's - // readable list is identical so we pick the canonical one. readable, err := h.resolver.ReadableNamespaces(ctx, rootID) if err != nil { log.Printf("admin/memories/export (cutover) root=%s: resolve: %v", rootID, err) continue } - // Pick the workspace whose primary namespace (workspace:) - // matches each entry. For team/org namespaces, attribute to - // the root (canonical owner). + // Collect non-workspace namespaces (team:/org:/custom:/...) from + // the root view; these are identical across every member. for _, ns := range readable { + if strings.HasPrefix(ns.Name, "workspace:") { + continue + } allNamespaces[ns.Name] = struct{}{} if _, alreadyMapped := nsToOwner[ns.Name]; alreadyMapped { continue } - // workspace: → that specific workspace's name if owner := pickOwnerForNamespace(ns.Name, members); owner != "" { nsToOwner[ns.Name] = owner } } + // Inject each member's private workspace: namespace + its + // owner. Children's private memories live in workspace: + // which the root-only resolve doesn't surface. + for _, m := range members { + ns := "workspace:" + m.ID + allNamespaces[ns] = struct{}{} + nsToOwner[ns] = m.Name + } } if len(allNamespaces) == 0 { diff --git a/workspace-server/internal/handlers/admin_memories_cutover_test.go b/workspace-server/internal/handlers/admin_memories_cutover_test.go index cedd75c6..36aa2409 100644 --- a/workspace-server/internal/handlers/admin_memories_cutover_test.go +++ b/workspace-server/internal/handlers/admin_memories_cutover_test.go @@ -541,7 +541,10 @@ func TestImport_SkipsWhenResolverErrors(t *testing.T) { // search per UNIQUE root. // // Setup: 3 workspaces under 1 root → 1 resolver call + 1 plugin call -// (was: 3 resolver + 3 plugin in the old code). +// (was: 3 resolver + 3 plugin in the old code). The plugin search +// receives 5 namespaces: each member's workspace: + team:root-1 +// + org:root-1. (Children's workspace: namespaces must be +// included or admin export silently drops their private memories.) func TestExport_BatchesPluginCallsByRoot(t *testing.T) { t.Setenv(envMemoryV2Cutover, "true") mock := installMockDB(t) @@ -556,8 +559,8 @@ func TestExport_BatchesPluginCallsByRoot(t *testing.T) { plugin := &stubAdminPlugin{ searchFn: func(_ context.Context, body contract.SearchRequest) (*contract.SearchResponse, error) { pluginSearchCount++ - if len(body.Namespaces) != 3 { - t.Errorf("plugin search call %d: namespaces len = %d, want 3 (workspace+team+org)", pluginSearchCount, len(body.Namespaces)) + if len(body.Namespaces) != 5 { + t.Errorf("plugin search call %d: namespaces len = %d, want 5 (3 workspace + team + org); got %v", pluginSearchCount, len(body.Namespaces), body.Namespaces) } return &contract.SearchResponse{}, nil }, @@ -578,6 +581,126 @@ func TestExport_BatchesPluginCallsByRoot(t *testing.T) { } } +// perWorkspaceResolver mimics the real resolver: ReadableNamespaces +// returns the SPECIFIC workspace's view (workspace: + +// team: + org:), not a constant set. The legacy +// stubAdminResolver hides the I3 silent-drop bug by ignoring its +// workspace-id argument. +type perWorkspaceResolver map[string][]namespace.Namespace + +func (r perWorkspaceResolver) ReadableNamespaces(_ context.Context, ws string) ([]namespace.Namespace, error) { + v, ok := r[ws] + if !ok { + return nil, errors.New("perWorkspaceResolver: unknown ws " + ws) + } + return v, nil +} +func (r perWorkspaceResolver) WritableNamespaces(_ context.Context, ws string) ([]namespace.Namespace, error) { + return r.ReadableNamespaces(nil, ws) +} + +// TestExport_IncludesEveryMembersPrivateNamespace pins the I3 follow-up +// fix: when a root group has multiple members, the export must surface +// each member's workspace: namespace, not just the root's. Before +// the fix, calling ReadableNamespaces(rootID) returned only +// workspace:rootID + team:rootID + org:rootID — every child workspace's +// private memories were silently dropped from admin export. +func TestExport_IncludesEveryMembersPrivateNamespace(t *testing.T) { + t.Setenv(envMemoryV2Cutover, "true") + mock := installMockDB(t) + + mock.ExpectQuery("WITH RECURSIVE chain"). + WillReturnRows(sqlmock.NewRows([]string{"id", "name", "root_id"}). + AddRow("root-1", "alpha", "root-1"). + AddRow("child-1", "alpha-child", "root-1"). + AddRow("child-2", "alpha-grandchild", "root-1")) + + resolver := perWorkspaceResolver{ + "root-1": { + {Name: "workspace:root-1", Kind: contract.NamespaceKindWorkspace, Writable: true}, + {Name: "team:root-1", Kind: contract.NamespaceKindTeam, Writable: true}, + {Name: "org:root-1", Kind: contract.NamespaceKindOrg, Writable: true}, + }, + "child-1": { + {Name: "workspace:child-1", Kind: contract.NamespaceKindWorkspace, Writable: true}, + {Name: "team:root-1", Kind: contract.NamespaceKindTeam, Writable: true}, + {Name: "org:root-1", Kind: contract.NamespaceKindOrg, Writable: true}, + }, + "child-2": { + {Name: "workspace:child-2", Kind: contract.NamespaceKindWorkspace, Writable: true}, + {Name: "team:root-1", Kind: contract.NamespaceKindTeam, Writable: true}, + {Name: "org:root-1", Kind: contract.NamespaceKindOrg, Writable: true}, + }, + } + + var passedNamespaces []string + plugin := &stubAdminPlugin{ + searchFn: func(_ context.Context, body contract.SearchRequest) (*contract.SearchResponse, error) { + passedNamespaces = append(passedNamespaces, body.Namespaces...) + return &contract.SearchResponse{Memories: []contract.Memory{ + {ID: "m-root", Namespace: "workspace:root-1", Content: "root private", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: time.Now().UTC()}, + {ID: "m-child1", Namespace: "workspace:child-1", Content: "child-1 private", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: time.Now().UTC()}, + {ID: "m-child2", Namespace: "workspace:child-2", Content: "child-2 private", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: time.Now().UTC()}, + {ID: "m-team", Namespace: "team:root-1", Content: "shared team", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: time.Now().UTC()}, + }}, nil + }, + } + h := NewAdminMemoriesHandler().withMemoryV2APIs(plugin, resolver) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/admin/memories/export", nil) + h.Export(c) + + if w.Code != http.StatusOK { + t.Fatalf("code = %d body=%s", w.Code, w.Body.String()) + } + + // Every member's private namespace must reach the plugin search. + want := []string{"workspace:root-1", "workspace:child-1", "workspace:child-2", "team:root-1", "org:root-1"} + got := make(map[string]bool, len(passedNamespaces)) + for _, ns := range passedNamespaces { + got[ns] = true + } + for _, w := range want { + if !got[w] { + t.Errorf("plugin search missing namespace %q (got %v)", w, passedNamespaces) + } + } + if len(passedNamespaces) != 5 { + t.Errorf("plugin search namespace count = %d, want 5 (3 workspace + team + org)", len(passedNamespaces)) + } + + // Children's private memories must appear in the export, attributed + // to the right workspace_name. + var entries []memoryExportEntry + if err := json.Unmarshal(w.Body.Bytes(), &entries); err != nil { + t.Fatalf("unmarshal: %v", err) + } + byID := map[string]memoryExportEntry{} + for _, e := range entries { + byID[e.ID] = e + } + for _, exp := range []struct{ id, ns, owner string }{ + {"m-root", "workspace:root-1", "alpha"}, + {"m-child1", "workspace:child-1", "alpha-child"}, + {"m-child2", "workspace:child-2", "alpha-grandchild"}, + } { + e, ok := byID[exp.id] + if !ok { + t.Errorf("export missing memory %s — children's private memories silently dropped", exp.id) + continue + } + if e.Namespace != exp.ns { + t.Errorf("memory %s namespace = %q, want %q", exp.id, e.Namespace, exp.ns) + } + if e.WorkspaceName != exp.owner { + t.Errorf("memory %s owner = %q, want %q", exp.id, e.WorkspaceName, exp.owner) + } + } +} + // TestPickOwnerForNamespace covers the namespace→workspace_name // attribution helper introduced in I3. func TestPickOwnerForNamespace(t *testing.T) {