forked from molecule-ai/molecule-core
fix(admin-memories): include each member's private namespace in export
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:<id> + 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:<id> 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) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang
parent
9a64aeaa2c
commit
ebc20794f3
@ -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:<id> 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:<childID> namespaces would be silently dropped from
|
||||
// the export. Inject each member's workspace:<id> 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:<id>)
|
||||
// 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:<id> → that specific workspace's name
|
||||
if owner := pickOwnerForNamespace(ns.Name, members); owner != "" {
|
||||
nsToOwner[ns.Name] = owner
|
||||
}
|
||||
}
|
||||
// Inject each member's private workspace:<id> namespace + its
|
||||
// owner. Children's private memories live in workspace:<childID>
|
||||
// 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 {
|
||||
|
||||
@ -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:<id> + team:root-1
|
||||
// + org:root-1. (Children's workspace:<id> 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:<that ID> +
|
||||
// team:<root> + org:<root>), 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:<id> 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) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user