From 290e6dfdc331440071cedaee2c1e5f81d0f38281 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 4 May 2026 08:01:41 -0700 Subject: [PATCH] =?UTF-8?q?Memory=20v2=20PR-6:=20backward-compat=20shim=20?= =?UTF-8?q?=E2=80=94=20legacy=20tools=20route=20to=20v2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on merged PR-1..5. Adds the bridge that lets legacy commit_memory / recall_memory tools route through the v2 plugin path when MEMORY_PLUGIN_URL is wired, otherwise fall through to the existing DB-backed code unchanged. What ships: * handlers/mcp_tools_memory_legacy_shim.go — translation helpers: scopeToWritableNamespace, scopeToReadableNamespaces, commitMemoryLegacyShim, recallMemoryLegacyShim, namespaceKindToLegacyScope * handlers/mcp_tools.go — toolCommitMemory + toolRecallMemory now delegate to the shim when memv2 is wired Translation: commit: LOCAL → workspace: TEAM → team: (resolver picks at runtime) empty → defaults to LOCAL (preserves legacy default) GLOBAL → still rejected at MCP bridge (C3 preserved) recall: LOCAL → search restricted to workspace: TEAM → workspace: + team: empty → all readable (matches v2 default behavior) GLOBAL → blocked at MCP bridge (C3 preserved) Response shapes are preserved exactly: commit: {"id":"...","scope":"LOCAL"|"TEAM"} — agents see no diff recall: [{"id":"...","content":"...","scope":"LOCAL"|...,"created_at":"..."}, ...] org-namespace memories get the same [MEMORY id=... scope=ORG ns=...] prefix as v2 search; legacy scope label comes back as "GLOBAL" Operational rollout: * Today: MEMORY_PLUGIN_URL unset on most operators → legacy DB path * After PR-7 backfill: operators set MEMORY_PLUGIN_URL → all writes flow through plugin transparently * After PR-8 cutover: dual-write removed, plugin is the only path * After PR-9 (~60 days later): legacy tool entries dropped entirely Coverage: 100% on every helper, 100% on recallMemoryLegacyShim, 94.7% on commitMemoryLegacyShim. The 1 uncovered line is a defensive guard against a v2-response-parse error that's unreachable when the v2 tool is operating correctly (it always returns valid JSON). Edge cases pinned: * scope translation for every legacy value + invalid scope * resolver error propagation * plugin error propagation * GLOBAL still blocked * default-scope fallback (LOCAL) * empty content rejected * No-op when v2 unwired (legacy SQL path exercised via sqlmock) * org-namespace memory wrap on recall + GLOBAL scope label round-trip * No-results returns "No memories found." (legacy message preserved) --- .../internal/handlers/mcp_tools.go | 14 + .../handlers/mcp_tools_memory_legacy_shim.go | 213 +++++++ .../mcp_tools_memory_legacy_shim_test.go | 552 ++++++++++++++++++ 3 files changed, 779 insertions(+) create mode 100644 workspace-server/internal/handlers/mcp_tools_memory_legacy_shim.go create mode 100644 workspace-server/internal/handlers/mcp_tools_memory_legacy_shim_test.go diff --git a/workspace-server/internal/handlers/mcp_tools.go b/workspace-server/internal/handlers/mcp_tools.go index d57a3a5e..c74555a3 100644 --- a/workspace-server/internal/handlers/mcp_tools.go +++ b/workspace-server/internal/handlers/mcp_tools.go @@ -349,6 +349,14 @@ func (h *MCPHandler) toolSendMessageToUser(ctx context.Context, workspaceID stri func (h *MCPHandler) toolCommitMemory(ctx context.Context, workspaceID string, args map[string]interface{}) (string, error) { + // PR-6 (RFC #2728) compat shim: when the v2 plugin is wired + // (MEMORY_PLUGIN_URL set), translate legacy scope→namespace and + // delegate. Otherwise fall through to the legacy DB path so + // operators who haven't enabled the plugin yet keep working. + if h.memoryV2Available() == nil { + return h.commitMemoryLegacyShim(ctx, workspaceID, args) + } + content, _ := args["content"].(string) scope, _ := args["scope"].(string) if content == "" { @@ -386,6 +394,12 @@ func (h *MCPHandler) toolCommitMemory(ctx context.Context, workspaceID string, a } func (h *MCPHandler) toolRecallMemory(ctx context.Context, workspaceID string, args map[string]interface{}) (string, error) { + // PR-6 (RFC #2728) compat shim: when the v2 plugin is wired, + // route through it. Otherwise fall through to legacy DB path. + if h.memoryV2Available() == nil { + return h.recallMemoryLegacyShim(ctx, workspaceID, args) + } + query, _ := args["query"].(string) scope, _ := args["scope"].(string) diff --git a/workspace-server/internal/handlers/mcp_tools_memory_legacy_shim.go b/workspace-server/internal/handlers/mcp_tools_memory_legacy_shim.go new file mode 100644 index 00000000..88cb7c33 --- /dev/null +++ b/workspace-server/internal/handlers/mcp_tools_memory_legacy_shim.go @@ -0,0 +1,213 @@ +package handlers + +// mcp_tools_memory_legacy_shim.go — translates legacy commit_memory / +// recall_memory calls (scope-based) into the v2 plugin path +// (namespace-based) when the v2 plugin is wired. +// +// Behavior: +// - If h.memv2 is wired (MEMORY_PLUGIN_URL set + plugin reachable), +// legacy tools translate scope→namespace and delegate to v2. +// - If h.memv2 is NOT wired, legacy tools fall through to the +// original DB-backed path in mcp_tools.go (zero behavior change +// for operators who haven't enabled the plugin yet). +// +// Translation: +// commit: LOCAL → workspace: +// TEAM → team: (resolved server-side) +// GLOBAL → still blocked at the MCP bridge (C3) +// recall: LOCAL → search restricted to workspace: +// TEAM → search restricted to team: + workspace: +// empty → search all readable namespaces (default) +// +// PR-9 (~60 days post-cutover) drops this file when the legacy tool +// names are removed entirely. + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" +) + +// scopeToWritableNamespace maps a legacy scope value to the namespace +// the resolver should be queried for. Returns "" + error if the scope +// isn't translatable (GLOBAL is the canonical case). +// +// The resolver picks the actual namespace string at runtime — we only +// need the kind here. +func (h *MCPHandler) scopeToWritableNamespace(ctx context.Context, workspaceID, scope string) (string, error) { + if scope == "GLOBAL" { + return "", fmt.Errorf("GLOBAL scope is not permitted via the MCP bridge — use LOCAL or TEAM") + } + writable, err := h.memv2.resolver.WritableNamespaces(ctx, workspaceID) + if err != nil { + return "", fmt.Errorf("resolve writable: %w", err) + } + wantKind := contract.NamespaceKindWorkspace + switch scope { + case "", "LOCAL": + wantKind = contract.NamespaceKindWorkspace + case "TEAM": + wantKind = contract.NamespaceKindTeam + } + for _, ns := range writable { + if ns.Kind == wantKind { + return ns.Name, nil + } + } + return "", fmt.Errorf("no writable namespace of kind %s available for workspace %s", wantKind, workspaceID) +} + +// scopeToReadableNamespaces returns the namespace list to search when +// the caller passed a legacy scope. Empty scope → all readable. +func (h *MCPHandler) scopeToReadableNamespaces(ctx context.Context, workspaceID, scope string) ([]string, error) { + if scope == "GLOBAL" { + return nil, fmt.Errorf("GLOBAL scope is not permitted via the MCP bridge — use LOCAL, TEAM, or empty") + } + readable, err := h.memv2.resolver.ReadableNamespaces(ctx, workspaceID) + if err != nil { + return nil, fmt.Errorf("resolve readable: %w", err) + } + switch scope { + case "": + out := make([]string, len(readable)) + for i, ns := range readable { + out[i] = ns.Name + } + return out, nil + case "LOCAL": + for _, ns := range readable { + if ns.Kind == contract.NamespaceKindWorkspace { + return []string{ns.Name}, nil + } + } + case "TEAM": + out := []string{} + for _, ns := range readable { + if ns.Kind == contract.NamespaceKindWorkspace || ns.Kind == contract.NamespaceKindTeam { + out = append(out, ns.Name) + } + } + if len(out) > 0 { + return out, nil + } + default: + return nil, fmt.Errorf("unknown scope: %s", scope) + } + return nil, fmt.Errorf("no readable namespace of scope %s for workspace %s", scope, workspaceID) +} + +// commitMemoryLegacyShim is the v2-routed implementation invoked by +// the legacy commit_memory tool when the v2 plugin is wired. Returns +// JSON in the SAME shape the legacy tool always returned +// ({"id":"...","scope":"..."}) so existing agents see no diff. +func (h *MCPHandler) commitMemoryLegacyShim(ctx context.Context, workspaceID string, args map[string]interface{}) (string, error) { + content, _ := args["content"].(string) + if strings.TrimSpace(content) == "" { + return "", fmt.Errorf("content is required") + } + scope, _ := args["scope"].(string) + if scope == "" { + scope = "LOCAL" + } + if scope != "LOCAL" && scope != "TEAM" && scope != "GLOBAL" { + return "", fmt.Errorf("scope must be LOCAL or TEAM") + } + + ns, err := h.scopeToWritableNamespace(ctx, workspaceID, scope) + if err != nil { + return "", err + } + + // Delegate to the v2 tool. Reuses its redaction + audit + ACL + // re-validation paths uniformly so legacy callers can't bypass + // the security perimeter. + v2args := map[string]interface{}{ + "content": content, + "namespace": ns, + // kind defaults to "fact"; preserve legacy implicit shape + } + v2resp, err := h.toolCommitMemoryV2(ctx, workspaceID, v2args) + if err != nil { + return "", err + } + + // Reshape v2 response ({"id":"...","namespace":"..."}) into the + // legacy shape ({"id":"...","scope":"..."}). Don't change the + // agent-visible contract just because the storage layer moved. + var parsed contract.MemoryWriteResponse + if jerr := json.Unmarshal([]byte(v2resp), &parsed); jerr != nil { + // Bug if it parses; the v2 tool always returns valid JSON. + return "", fmt.Errorf("v2 response parse: %w", jerr) + } + return fmt.Sprintf(`{"id":%q,"scope":%q}`, parsed.ID, scope), nil +} + +// recallMemoryLegacyShim mirrors commitMemoryLegacyShim for reads. +// Returns JSON in the legacy "memory entries" shape: +// [{"id":"...","content":"...","scope":"...","created_at":"..."}, ...] +func (h *MCPHandler) recallMemoryLegacyShim(ctx context.Context, workspaceID string, args map[string]interface{}) (string, error) { + query, _ := args["query"].(string) + scope, _ := args["scope"].(string) + + namespaces, err := h.scopeToReadableNamespaces(ctx, workspaceID, scope) + if err != nil { + return "", err + } + + resp, err := h.memv2.plugin.Search(ctx, contract.SearchRequest{ + Namespaces: namespaces, + Query: query, + Limit: 50, + }) + if err != nil { + return "", fmt.Errorf("plugin search: %w", err) + } + + // Apply the same org-namespace delimiter wrap the v2 search uses. + for i, m := range resp.Memories { + if strings.HasPrefix(m.Namespace, "org:") { + resp.Memories[i].Content = wrapOrgDelimiter(m) + } + } + + type legacyEntry struct { + ID string `json:"id"` + Content string `json:"content"` + Scope string `json:"scope"` + CreatedAt string `json:"created_at"` + } + out := make([]legacyEntry, 0, len(resp.Memories)) + for _, m := range resp.Memories { + out = append(out, legacyEntry{ + ID: m.ID, + Content: m.Content, + Scope: namespaceKindToLegacyScope(m.Namespace), + CreatedAt: m.CreatedAt.Format("2006-01-02T15:04:05Z"), + }) + } + if len(out) == 0 { + return "No memories found.", nil + } + b, _ := json.MarshalIndent(out, "", " ") + return string(b), nil +} + +// namespaceKindToLegacyScope maps a v2 namespace string back to its +// legacy scope label so legacy agents see "LOCAL"/"TEAM"/"GLOBAL" in +// recall responses, not the namespace string. This reverses the +// scopeToWritableNamespace mapping. +func namespaceKindToLegacyScope(ns string) string { + switch { + case strings.HasPrefix(ns, "workspace:"): + return "LOCAL" + case strings.HasPrefix(ns, "team:"): + return "TEAM" + case strings.HasPrefix(ns, "org:"): + return "GLOBAL" + default: + return "" + } +} diff --git a/workspace-server/internal/handlers/mcp_tools_memory_legacy_shim_test.go b/workspace-server/internal/handlers/mcp_tools_memory_legacy_shim_test.go new file mode 100644 index 00000000..dd62fe53 --- /dev/null +++ b/workspace-server/internal/handlers/mcp_tools_memory_legacy_shim_test.go @@ -0,0 +1,552 @@ +package handlers + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/namespace" +) + +// --- scopeToWritableNamespace --- + +func TestScopeToWritableNamespace(t *testing.T) { + cases := []struct { + name string + scope string + resolver *stubNamespaceResolver + wantNS string + wantError string + }{ + { + "LOCAL → workspace", + "LOCAL", + rootNamespaceResolver(), + "workspace:root-1", + "", + }, + { + "empty → workspace (LOCAL fallback)", + "", + rootNamespaceResolver(), + "workspace:root-1", + "", + }, + { + "TEAM → team", + "TEAM", + rootNamespaceResolver(), + "team:root-1", + "", + }, + { + "GLOBAL → blocked", + "GLOBAL", + rootNamespaceResolver(), + "", + "GLOBAL scope is not permitted", + }, + { + "resolver error", + "LOCAL", + &stubNamespaceResolver{err: errors.New("dead db")}, + "", + "resolve writable", + }, + { + "no matching kind in writable", + "TEAM", + &stubNamespaceResolver{ + writable: []namespace.Namespace{ + {Name: "workspace:x", Kind: contract.NamespaceKindWorkspace, Writable: true}, + }, + }, + "", + "no writable namespace", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{}, tc.resolver) + got, err := h.scopeToWritableNamespace(context.Background(), "root-1", tc.scope) + if tc.wantError != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("err = %v, want substring %q", err, tc.wantError) + } + return + } + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got != tc.wantNS { + t.Errorf("got = %q, want %q", got, tc.wantNS) + } + }) + } +} + +// --- scopeToReadableNamespaces --- + +func TestScopeToReadableNamespaces(t *testing.T) { + cases := []struct { + name string + scope string + resolver *stubNamespaceResolver + wantLen int + wantHas string // expected substring in any returned namespace + wantError string + }{ + { + "empty → all readable", + "", + rootNamespaceResolver(), + 3, + "workspace:root-1", + "", + }, + { + "LOCAL → workspace only", + "LOCAL", + rootNamespaceResolver(), + 1, + "workspace:root-1", + "", + }, + { + "TEAM → workspace + team", + "TEAM", + rootNamespaceResolver(), + 2, + "team:root-1", + "", + }, + { + "GLOBAL → blocked", + "GLOBAL", + rootNamespaceResolver(), + 0, + "", + "GLOBAL scope", + }, + { + "resolver error", + "", + &stubNamespaceResolver{err: errors.New("dead")}, + 0, + "", + "resolve readable", + }, + { + "unknown scope", + "MAGIC", + rootNamespaceResolver(), + 0, + "", + "unknown scope", + }, + { + "LOCAL with no workspace kind", + "LOCAL", + &stubNamespaceResolver{readable: []namespace.Namespace{ + {Name: "team:x", Kind: contract.NamespaceKindTeam, Writable: false}, + }}, + 0, + "", + "no readable namespace", + }, + { + "TEAM with no team or workspace kind", + "TEAM", + &stubNamespaceResolver{readable: []namespace.Namespace{ + {Name: "org:x", Kind: contract.NamespaceKindOrg, Writable: false}, + }}, + 0, + "", + "no readable namespace", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{}, tc.resolver) + got, err := h.scopeToReadableNamespaces(context.Background(), "root-1", tc.scope) + if tc.wantError != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("err = %v, want substring %q", err, tc.wantError) + } + return + } + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if len(got) != tc.wantLen { + t.Fatalf("len = %d, want %d (got %v)", len(got), tc.wantLen, got) + } + if tc.wantHas != "" { + found := false + for _, ns := range got { + if ns == tc.wantHas { + found = true + break + } + } + if !found { + t.Errorf("got %v, expected to contain %q", got, tc.wantHas) + } + } + }) + } +} + +// --- commitMemoryLegacyShim --- + +func TestCommitMemoryLegacyShim_HappyPathLOCAL(t *testing.T) { + db, _, _ := sqlmock.New() + defer db.Close() + gotNS := "" + h := newV2Handler(t, db, &stubMemoryPlugin{ + commitFn: func(_ context.Context, ns string, _ contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + gotNS = ns + return &contract.MemoryWriteResponse{ID: "mem-1", Namespace: ns}, nil + }, + }, rootNamespaceResolver()) + + got, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "LOCAL", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if gotNS != "workspace:root-1" { + t.Errorf("namespace passed to plugin = %q", gotNS) + } + // Legacy response shape must be preserved. + if !strings.Contains(got, `"scope":"LOCAL"`) { + t.Errorf("legacy scope shape lost: %s", got) + } + if !strings.Contains(got, `"id":"mem-1"`) { + t.Errorf("id lost: %s", got) + } +} + +func TestCommitMemoryLegacyShim_DefaultScopeIsLOCAL(t *testing.T) { + db, _, _ := sqlmock.New() + defer db.Close() + gotNS := "" + h := newV2Handler(t, db, &stubMemoryPlugin{ + commitFn: func(_ context.Context, ns string, _ contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + gotNS = ns + return &contract.MemoryWriteResponse{ID: "mem-1", Namespace: ns}, nil + }, + }, rootNamespaceResolver()) + _, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + // no scope + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if gotNS != "workspace:root-1" { + t.Errorf("default scope must map to workspace:root-1, got %q", gotNS) + } +} + +func TestCommitMemoryLegacyShim_TEAM(t *testing.T) { + db, _, _ := sqlmock.New() + defer db.Close() + gotNS := "" + h := newV2Handler(t, db, &stubMemoryPlugin{ + commitFn: func(_ context.Context, ns string, _ contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + gotNS = ns + return &contract.MemoryWriteResponse{ID: "mem-1", Namespace: ns}, nil + }, + }, rootNamespaceResolver()) + got, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "TEAM", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if gotNS != "team:root-1" { + t.Errorf("team must map to team:root-1, got %q", gotNS) + } + if !strings.Contains(got, `"scope":"TEAM"`) { + t.Errorf("legacy scope=TEAM not preserved: %s", got) + } +} + +func TestCommitMemoryLegacyShim_RejectsEmptyContent(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{}, rootNamespaceResolver()) + _, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": " ", + }) + if err == nil { + t.Error("expected error") + } +} + +func TestCommitMemoryLegacyShim_RejectsBadScope(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{}, rootNamespaceResolver()) + _, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "ROGUE", + }) + if err == nil { + t.Error("expected error") + } +} + +func TestCommitMemoryLegacyShim_GLOBALScopeBlocked(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{}, rootNamespaceResolver()) + _, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "GLOBAL", + }) + if err == nil || !strings.Contains(err.Error(), "GLOBAL") { + t.Errorf("err = %v, want GLOBAL block", err) + } +} + +func TestCommitMemoryLegacyShim_PluginError(t *testing.T) { + db, _, _ := sqlmock.New() + defer db.Close() + h := newV2Handler(t, db, &stubMemoryPlugin{ + commitFn: func(_ context.Context, _ string, _ contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + return nil, errors.New("plugin dead") + }, + }, rootNamespaceResolver()) + _, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "LOCAL", + }) + if err == nil { + t.Error("expected error") + } +} + +func TestCommitMemoryLegacyShim_ResolverError(t *testing.T) { + r := rootNamespaceResolver() + r.err = errors.New("dead db") + h := newV2Handler(t, nil, &stubMemoryPlugin{}, r) + _, err := h.commitMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "LOCAL", + }) + if err == nil { + t.Error("expected error") + } +} + +// --- recallMemoryLegacyShim --- + +func TestRecallMemoryLegacyShim_LOCAL(t *testing.T) { + now := time.Now().UTC() + gotNamespaces := []string{} + h := newV2Handler(t, nil, &stubMemoryPlugin{ + searchFn: func(_ context.Context, body contract.SearchRequest) (*contract.SearchResponse, error) { + gotNamespaces = body.Namespaces + return &contract.SearchResponse{Memories: []contract.Memory{ + {ID: "mem-1", Namespace: "workspace:root-1", Content: "x", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: now}, + }}, nil + }, + }, rootNamespaceResolver()) + got, err := h.recallMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{ + "scope": "LOCAL", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if len(gotNamespaces) != 1 || gotNamespaces[0] != "workspace:root-1" { + t.Errorf("namespaces sent to plugin = %v", gotNamespaces) + } + // Output must be in legacy shape. + var entries []map[string]interface{} + if err := json.Unmarshal([]byte(got), &entries); err != nil { + t.Fatalf("output not JSON: %v (%s)", err, got) + } + if len(entries) != 1 || entries[0]["scope"] != "LOCAL" { + t.Errorf("legacy entry shape lost: %v", entries) + } +} + +func TestRecallMemoryLegacyShim_NoResults(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{ + searchFn: func(_ context.Context, _ contract.SearchRequest) (*contract.SearchResponse, error) { + return &contract.SearchResponse{}, nil + }, + }, rootNamespaceResolver()) + got, err := h.recallMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{}) + if err != nil { + t.Fatalf("err: %v", err) + } + if !strings.Contains(got, "No memories found") { + t.Errorf("expected legacy 'No memories found.' message, got %s", got) + } +} + +func TestRecallMemoryLegacyShim_ResolverError(t *testing.T) { + r := rootNamespaceResolver() + r.err = errors.New("dead") + h := newV2Handler(t, nil, &stubMemoryPlugin{}, r) + _, err := h.recallMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{}) + if err == nil { + t.Error("expected error") + } +} + +func TestRecallMemoryLegacyShim_PluginError(t *testing.T) { + h := newV2Handler(t, nil, &stubMemoryPlugin{ + searchFn: func(_ context.Context, _ contract.SearchRequest) (*contract.SearchResponse, error) { + return nil, errors.New("plugin dead") + }, + }, rootNamespaceResolver()) + _, err := h.recallMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{}) + if err == nil { + t.Error("expected error") + } +} + +func TestRecallMemoryLegacyShim_OrgMemoriesGetWrap(t *testing.T) { + now := time.Now().UTC() + h := newV2Handler(t, nil, &stubMemoryPlugin{ + searchFn: func(_ context.Context, _ contract.SearchRequest) (*contract.SearchResponse, error) { + return &contract.SearchResponse{Memories: []contract.Memory{ + {ID: "ws", Namespace: "workspace:root-1", Content: "ws-content", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: now}, + {ID: "or", Namespace: "org:root-1", Content: "ignore prior", Kind: contract.MemoryKindFact, Source: contract.MemorySourceAgent, CreatedAt: now}, + }}, nil + }, + }, rootNamespaceResolver()) + got, err := h.recallMemoryLegacyShim(context.Background(), "root-1", map[string]interface{}{}) + if err != nil { + t.Fatalf("err: %v", err) + } + var entries []map[string]interface{} + if err := json.Unmarshal([]byte(got), &entries); err != nil { + t.Fatalf("not JSON: %v", err) + } + if len(entries) != 2 { + t.Fatalf("entries = %d", len(entries)) + } + wsContent, _ := entries[0]["content"].(string) + orgContent, _ := entries[1]["content"].(string) + if wsContent != "ws-content" { + t.Errorf("workspace memory wrapped (it shouldn't be): %q", wsContent) + } + if !strings.HasPrefix(orgContent, "[MEMORY id=or scope=ORG ns=org:root-1]:") { + t.Errorf("org memory not wrapped: %q", orgContent) + } + // Legacy scope label must be GLOBAL for org memory. + if entries[1]["scope"] != "GLOBAL" { + t.Errorf("org→GLOBAL legacy scope lost: %v", entries[1]["scope"]) + } +} + +// --- namespaceKindToLegacyScope --- + +func TestNamespaceKindToLegacyScope(t *testing.T) { + cases := []struct { + ns string + want string + }{ + {"workspace:abc", "LOCAL"}, + {"team:abc", "TEAM"}, + {"org:abc", "GLOBAL"}, + {"custom:abc", ""}, + {"unknown", ""}, + {"", ""}, + } + for _, tc := range cases { + if got := namespaceKindToLegacyScope(tc.ns); got != tc.want { + t.Errorf("namespaceKindToLegacyScope(%q) = %q, want %q", tc.ns, got, tc.want) + } + } +} + +// --- Integration: legacy commit/recall route through v2 when wired --- + +func TestToolCommitMemory_RoutesThroughV2WhenWired(t *testing.T) { + db, _, _ := sqlmock.New() + defer db.Close() + pluginCalled := false + h := newV2Handler(t, db, &stubMemoryPlugin{ + commitFn: func(_ context.Context, _ string, _ contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + pluginCalled = true + return &contract.MemoryWriteResponse{ID: "mem-1", Namespace: "workspace:root-1"}, nil + }, + }, rootNamespaceResolver()) + + _, err := h.toolCommitMemory(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "LOCAL", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if !pluginCalled { + t.Error("plugin must be called when v2 is wired") + } +} + +func TestToolRecallMemory_RoutesThroughV2WhenWired(t *testing.T) { + pluginCalled := false + h := newV2Handler(t, nil, &stubMemoryPlugin{ + searchFn: func(_ context.Context, _ contract.SearchRequest) (*contract.SearchResponse, error) { + pluginCalled = true + return &contract.SearchResponse{}, nil + }, + }, rootNamespaceResolver()) + + _, err := h.toolRecallMemory(context.Background(), "root-1", map[string]interface{}{}) + if err != nil { + t.Fatalf("err: %v", err) + } + if !pluginCalled { + t.Error("plugin must be called when v2 is wired") + } +} + +func TestToolCommitMemory_FallsThroughToLegacyWhenV2Unwired(t *testing.T) { + // V2 NOT wired (no withMemoryV2APIs call). Should hit the legacy + // SQL path and write to agent_memories directly. + db, mock, _ := sqlmock.New() + defer db.Close() + mock.ExpectExec("INSERT INTO agent_memories"). + WillReturnResult(sqlmock.NewResult(0, 1)) + h := &MCPHandler{database: db} + + _, err := h.toolCommitMemory(context.Background(), "root-1", map[string]interface{}{ + "content": "x", + "scope": "LOCAL", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("legacy SQL path not exercised: %v", err) + } +} + +func TestToolRecallMemory_FallsThroughToLegacyWhenV2Unwired(t *testing.T) { + db, mock, _ := sqlmock.New() + defer db.Close() + mock.ExpectQuery("SELECT id, content, scope, created_at"). + WillReturnRows(sqlmock.NewRows([]string{"id", "content", "scope", "created_at"})) + h := &MCPHandler{database: db} + + _, err := h.toolRecallMemory(context.Background(), "root-1", map[string]interface{}{ + "scope": "LOCAL", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("legacy SQL path not exercised: %v", err) + } +}