diff --git a/workspace-server/cmd/server/main.go b/workspace-server/cmd/server/main.go index 6f4b82801..cb6a44e61 100644 --- a/workspace-server/cmd/server/main.go +++ b/workspace-server/cmd/server/main.go @@ -210,6 +210,12 @@ func main() { memBundle := memwiring.Build(db.DB) if memBundle != nil { wh.WithNamespaceCleanup(memBundle.NamespaceCleanupFn()) + // Issue #1755: route workspace-create `initial_memories` through + // the v2 plugin instead of the legacy `agent_memories` table. + // Same plugin client the MCP tools use, same namespace + // (`workspace:`); writes are visible to subsequent + // `recall_memory` calls on the same workspace. + wh.WithSeedMemoryPlugin(memBundle.Plugin) } // External-plugin env mutators — each plugin contributes 0+ mutators diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index 5bc6bd324..af0a1d666 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -804,7 +804,7 @@ func (h *OrgHandler) Import(c *gin.Context) { for i, gm := range tmpl.GlobalMemories { globalSeeds[i] = models.MemorySeed{Content: gm.Content, Scope: "GLOBAL"} } - seedInitialMemories(context.Background(), rootID, globalSeeds) + h.workspace.seedInitialMemories(context.Background(), rootID, globalSeeds) log.Printf("Org import: seeded %d global memories on root workspace %s", len(globalSeeds), rootID) } } diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index 0ab1e3b4b..14522c17d 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -258,7 +258,7 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX if len(wsMemories) == 0 { wsMemories = defaults.InitialMemories } - seedInitialMemories(ctx, id, wsMemories) + h.workspace.seedInitialMemories(ctx, id, wsMemories) // Handle external workspaces if ws.External { diff --git a/workspace-server/internal/handlers/workspace.go b/workspace-server/internal/handlers/workspace.go index f92c7e3a3..6b76a15f1 100644 --- a/workspace-server/internal/handlers/workspace.go +++ b/workspace-server/internal/handlers/workspace.go @@ -21,6 +21,7 @@ import ( "github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto" "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" "github.com/Molecule-AI/molecule-monorepo/platform/internal/events" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" "github.com/Molecule-AI/molecule-monorepo/platform/internal/models" "github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner" "github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth" @@ -74,12 +75,30 @@ type WorkspaceHandler struct { // memory plugin). main.go sets this to plugin.DeleteNamespace // when MEMORY_PLUGIN_URL is configured. namespaceCleanupFn func(ctx context.Context, workspaceID string) + // seedMemoryPlugin is the v2 memory plugin client used by + // seedInitialMemories (issue #1755) to write workspace-create + // `initial_memories` into the plugin instead of the legacy + // `agent_memories` table. nil-safe: when nil, seeding logs a loud + // warning and skips. After A1 (#1747) there is no SQL fallback — + // seeded memories with no plugin wired are simply not persisted. + // main.go attaches this alongside namespaceCleanupFn when + // MEMORY_PLUGIN_URL is set (memBundle.Plugin). + seedMemoryPlugin seedMemoryPluginAPI // asyncWG tracks goroutines launched by goAsync so tests can wait // for async DB users (restart, provision) before asserting results. // Matches the pattern from main commit 1c3b4ff3. asyncWG sync.WaitGroup } +// seedMemoryPluginAPI is the slice of the v2 memory plugin client that +// seedInitialMemories needs. Defining it as an interface here (parallel +// to memoryPluginAPI in mcp_tools_memory_v2.go) lets tests stub the +// plugin with a capture-only spy and keeps the handler decoupled from +// the concrete *client.Client. +type seedMemoryPluginAPI interface { + CommitMemory(ctx context.Context, namespace string, body contract.MemoryWrite) (*contract.MemoryWriteResponse, error) +} + // newHandlerHook, when non-nil, is invoked for every WorkspaceHandler // created via NewWorkspaceHandler. It is nil in production (zero cost); // the test harness sets it so setupTestDB can drain every handler's @@ -174,6 +193,19 @@ func (h *WorkspaceHandler) WithNamespaceCleanup(fn func(ctx context.Context, wor return h } +// WithSeedMemoryPlugin wires the v2 memory plugin so +// seedInitialMemories (issue #1755) routes workspace-create +// `initial_memories` through the plugin instead of the legacy +// `agent_memories` table. main.go passes memBundle.Plugin (a +// `*client.Client`); tests pass a stub matching the +// seedMemoryPluginAPI interface. Nil-safe: omitting this leaves the +// field nil and seedInitialMemories logs a warning + skips on each +// invocation. +func (h *WorkspaceHandler) WithSeedMemoryPlugin(p seedMemoryPluginAPI) *WorkspaceHandler { + h.seedMemoryPlugin = p + return h +} + // SetCPProvisioner wires the control plane provisioner for SaaS tenants. // Auto-activated when MOLECULE_ORG_ID is set (no manual config needed). // @@ -571,7 +603,7 @@ func (h *WorkspaceHandler) Create(c *gin.Context) { // Seed initial memories from the create payload (issue #1050). // Non-fatal: failures are logged but don't block workspace creation. - seedInitialMemories(ctx, id, payload.InitialMemories) + h.seedInitialMemories(ctx, id, payload.InitialMemories) // Broadcast provisioning event. Include `runtime` so the canvas can // populate the Runtime pill on the side panel immediately — without it diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index f63167732..2c6f4ff7a 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -12,6 +12,7 @@ import ( "github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto" "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" "github.com/Molecule-AI/molecule-monorepo/platform/internal/models" "github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner" "github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth" @@ -184,21 +185,45 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri // which transitions status to 'online' and broadcasts WORKSPACE_ONLINE } -// seedInitialMemories inserts a list of MemorySeed entries into agent_memories -// for the given workspace. Called during workspace creation and org import to -// pre-populate memories from config/template. Non-fatal: each insert is -// attempted independently and failures are logged. Issue #1050. -// maxMemoryContentLength is the maximum allowed size for a single memory content -// field. Content exceeding this limit is truncated to prevent storage exhaustion -// (CWE-400) and OOM on read paths. The limit is intentionally generous — it fits -// a ~64k context window worth of text — but small enough to prevent abuse. +// seedInitialMemories writes a list of MemorySeed entries through the v2 +// memory plugin for the given workspace. Called during workspace creation +// and org import to pre-populate memories from config/template (issue +// #1050). Non-fatal: each plugin call is attempted independently and +// failures are logged. +// +// Issue #1755: post-A1 (#1747) v2 is the only memory backend; writing +// into `agent_memories` would be invisible to `recall_memory` (which +// reads exclusively from the plugin). When the plugin isn't wired +// (WithSeedMemoryPlugin not called, i.e. `MEMORY_PLUGIN_URL` unset on +// platform-tenant), seedInitialMemories logs a loud warning and skips +// — seeded memories simply don't materialise on that operator's +// deployment. There is no SQL fallback. +// +// Scope handling: the v2 plugin's data model has no `scope` column. All +// seeded memories land in `workspace:` (the workspace's private +// namespace). Pre-A1 callers wrote TEAM/GLOBAL-scoped memories into +// `agent_memories` with `namespace=workspace:` anyway, so this is +// behaviour-preserving for LOCAL and a no-op-shrink for TEAM/GLOBAL +// (those scopes can be promoted later via an explicit +// `commit_memory_v2` call by the agent). +// +// maxMemoryContentLength is the maximum allowed size for a single +// memory content field. Content exceeding this limit is truncated to +// prevent storage exhaustion (CWE-400) and OOM on read paths. The +// limit is intentionally generous — it fits a ~64k context window +// worth of text — but small enough to prevent abuse. const maxMemoryContentLength = 100_000 // ~100 KiB of text -func seedInitialMemories(ctx context.Context, workspaceID string, memories []models.MemorySeed) { +func (h *WorkspaceHandler) seedInitialMemories(ctx context.Context, workspaceID string, memories []models.MemorySeed) { if len(memories) == 0 { return } + if h.seedMemoryPlugin == nil { + log.Printf("seedInitialMemories: ⚠️ skipping %d memories for workspace %s — v2 memory plugin not wired (set MEMORY_PLUGIN_URL on platform-tenant). Seeded memories from this template are not persisted.", len(memories), workspaceID) + return + } namespace := workspaceMemoryNamespace(workspaceID) + seeded := 0 for _, mem := range memories { scope := strings.ToUpper(mem.Scope) if scope == "" { @@ -211,9 +236,10 @@ func seedInitialMemories(ctx context.Context, workspaceID string, memories []mod if mem.Content == "" { continue } - // #1066: enforce content length limit to prevent storage exhaustion (CWE-400). - // Truncate oversized content rather than rejecting the whole insert so that - // template authors get a predictable fallback rather than a silent skip. + // #1066: enforce content length limit to prevent storage exhaustion + // (CWE-400). Truncate oversized content rather than rejecting the + // whole insert so that template authors get a predictable fallback + // rather than a silent skip. content := mem.Content if len(content) > maxMemoryContentLength { content = content[:maxMemoryContentLength] @@ -221,14 +247,25 @@ func seedInitialMemories(ctx context.Context, workspaceID string, memories []mod workspaceID, scope, len(mem.Content), maxMemoryContentLength) } redactedContent, _ := redactSecrets(workspaceID, content) - if _, err := db.DB.ExecContext(ctx, ` - INSERT INTO agent_memories (workspace_id, content, scope, namespace) - VALUES ($1, $2, $3, $4) - `, workspaceID, redactedContent, scope, namespace); err != nil { - log.Printf("seedInitialMemories: failed to insert memory for %s (scope=%s): %v", workspaceID, scope, err) + if _, err := h.seedMemoryPlugin.CommitMemory(ctx, namespace, contract.MemoryWrite{ + Content: redactedContent, + // Kind = fact: seeded memories are factual baseline knowledge, + // not session summaries or runtime checkpoints. + Kind: contract.MemoryKindFact, + // Source = runtime: the platform (not the agent) is writing + // these on the agent's behalf at provision time. Distinct from + // `agent` (commit_memory MCP call) and `user` (canvas write). + Source: contract.MemorySourceRuntime, + }); err != nil { + log.Printf("seedInitialMemories: plugin commit failed for %s (scope=%s): %v", workspaceID, scope, err) + continue } + seeded++ + } + if seeded > 0 { + log.Printf("seedInitialMemories: seeded %d/%d memories for workspace %s via v2 plugin (namespace=%s)", + seeded, len(memories), workspaceID, namespace) } - log.Printf("seedInitialMemories: seeded %d memories for workspace %s", len(memories), workspaceID) } // workspaceMemoryNamespace returns the canonical v2 memory namespace diff --git a/workspace-server/internal/handlers/workspace_provision_test.go b/workspace-server/internal/handlers/workspace_provision_test.go index a4275a347..0c515ac7d 100644 --- a/workspace-server/internal/handlers/workspace_provision_test.go +++ b/workspace-server/internal/handlers/workspace_provision_test.go @@ -1,9 +1,12 @@ package handlers import ( + "bytes" "context" "database/sql" + "errors" "fmt" + "log" "net/http" "os" "path/filepath" @@ -11,6 +14,7 @@ import ( "testing" "github.com/DATA-DOG/go-sqlmock" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" "github.com/Molecule-AI/molecule-monorepo/platform/internal/models" "github.com/Molecule-AI/molecule-monorepo/platform/internal/plugins" "github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner" @@ -577,49 +581,65 @@ func TestSanitizeRuntime_Allowlist(t *testing.T) { } } -// ==================== seedInitialMemories: coverage for #1167 / #1208 ==================== +// ==================== seedInitialMemories: coverage for #1167 / #1208 / #1755 ==================== +// +// Issue #1755 rewrote these tests. seedInitialMemories no longer +// INSERTs into agent_memories — it routes through the v2 memory +// plugin's CommitMemory contract. The tests below capture the +// MemoryWrite the stub plugin receives and assert on its shape +// (redaction, truncation, scope-skip, namespace) instead of sqlmock +// INSERT args. Same coverage, post-A1 backend. + +// seedPluginCall records what the stub plugin saw on each commit. +type seedPluginCall struct { + Namespace string + Body contract.MemoryWrite +} + +// stubSeedPlugin captures plugin commits for assertion-style tests. +// commitErr, when non-nil, is returned to the caller — used to +// exercise the "plugin error logged, loop continues" path. +type stubSeedPlugin struct { + calls []seedPluginCall + commitErr error +} + +func (s *stubSeedPlugin) CommitMemory(_ context.Context, ns string, body contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + s.calls = append(s.calls, seedPluginCall{Namespace: ns, Body: body}) + if s.commitErr != nil { + return nil, s.commitErr + } + return &contract.MemoryWriteResponse{ID: "mem-stub", Namespace: ns}, nil +} + +// newSeedTestHandler builds a minimal WorkspaceHandler with a stub +// plugin wired. Tests that want to exercise the "plugin not wired" +// path build their own handler with seedMemoryPlugin nil. +func newSeedTestHandler() (*WorkspaceHandler, *stubSeedPlugin) { + stub := &stubSeedPlugin{} + h := &WorkspaceHandler{seedMemoryPlugin: stub} + return h, stub +} // TestSeedInitialMemories_TruncatesOversizedContent covers the boundary cases for // the CWE-400 content-length limit introduced in PR #1167. Issue #1208 identified -// that the truncate-at-100k guard lacked unit test coverage. -// The test verifies that content at and over the 100,000-byte limit is handled -// correctly, and that content under the limit passes through unchanged. +// that the truncate-at-100k guard lacked unit test coverage. #1755 ported the +// assertion from sqlmock INSERT args to the plugin's captured MemoryWrite.Content. func TestSeedInitialMemories_TruncatesOversizedContent(t *testing.T) { - mock := setupTestDB(t) - tests := []struct { name string contentLen int - expectInsert bool expectTruncate bool }{ - { - name: "exactly at 100 kB limit — no truncation", - contentLen: 100_000, - expectInsert: true, - }, - { - name: "1 byte over limit — truncated", - contentLen: 100_001, - expectInsert: true, - expectTruncate: true, - }, - { - name: "far over limit — truncated", - contentLen: 500_000, - expectInsert: true, - expectTruncate: true, - }, - { - name: "well under limit — passes through unchanged", - contentLen: 50_000, - expectInsert: true, - }, + {name: "exactly at 100 kB limit — no truncation", contentLen: 100_000}, + {name: "1 byte over limit — truncated", contentLen: 100_001, expectTruncate: true}, + {name: "far over limit — truncated", contentLen: 500_000, expectTruncate: true}, + {name: "well under limit — passes through unchanged", contentLen: 50_000}, } // Content must avoid the redactSecrets base64-blob regex (33+ chars of // [A-Za-z0-9+/]). Spaces break the run. "hello world " = 12 bytes. - const unit = "hello world " // 12 bytes, contains space + const unit = "hello world " mkContent := func(n int) string { copies := (n / len(unit)) + 1 out := strings.Repeat(unit, copies) @@ -628,37 +648,37 @@ func TestSeedInitialMemories_TruncatesOversizedContent(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - workspaceID := "ws-trunc-" + tt.name + h, plugin := newSeedTestHandler() + workspaceID := "ws-trunc" content := mkContent(tt.contentLen) memories := []models.MemorySeed{{Content: content, Scope: "LOCAL"}} - if tt.expectInsert { - // The DB INSERT must receive content of exactly maxMemoryContentLength - // (not the full original length). This is the key assertion: the function - // truncates before calling ExecContext, so the mock expects 100_000 bytes. - expected := content - if len(content) > maxMemoryContentLength { - expected = content[:maxMemoryContentLength] - } - mock.ExpectExec(`INSERT INTO agent_memories`). - WithArgs(workspaceID, expected, "LOCAL", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(1, 1)) + expected := content + if len(content) > maxMemoryContentLength { + expected = content[:maxMemoryContentLength] } - seedInitialMemories(context.Background(), workspaceID, memories) + h.seedInitialMemories(context.Background(), workspaceID, memories) - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("unmet DB expectations: %v", err) + if len(plugin.calls) != 1 { + t.Fatalf("plugin should have been called once, got %d calls", len(plugin.calls)) + } + if plugin.calls[0].Body.Content != expected { + t.Errorf("plugin.Content length=%d want=%d (truncation contract)", + len(plugin.calls[0].Body.Content), len(expected)) + } + if plugin.calls[0].Namespace != "workspace:"+workspaceID { + t.Errorf("namespace = %q want workspace:%s", plugin.calls[0].Namespace, workspaceID) } }) } } -// TestSeedInitialMemories_RedactsSecrets verifies that redactSecrets is called -// before the INSERT so that credentials in template memories never land -// unredacted in agent_memories. Regression test for F1085 / #1132. +// TestSeedInitialMemories_RedactsSecrets verifies redactSecrets is called +// before the plugin commit so that credentials in template memories never +// reach the plugin. Regression test for F1085 / #1132, ported to v2 path. func TestSeedInitialMemories_RedactsSecrets(t *testing.T) { - mock := setupTestDB(t) + h, plugin := newSeedTestHandler() raw := "Remember to set OPENAI_API_KEY=sk-abcdef123456 in the config file" wantRedacted, changed := redactSecrets("ws-redact-test", raw) @@ -666,45 +686,46 @@ func TestSeedInitialMemories_RedactsSecrets(t *testing.T) { t.Fatalf("precondition: redactSecrets must change the test content") } - workspaceID := "ws-redact-test" memories := []models.MemorySeed{{Content: raw, Scope: "LOCAL"}} + h.seedInitialMemories(context.Background(), "ws-redact-test", memories) - // The INSERT must receive the REDACTED content, not the raw secret. - mock.ExpectExec(`INSERT INTO agent_memories`). - WithArgs(workspaceID, wantRedacted, "LOCAL", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(1, 1)) - - seedInitialMemories(context.Background(), workspaceID, memories) - - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("unmet DB expectations: %v", err) + if len(plugin.calls) != 1 { + t.Fatalf("plugin should have been called once, got %d", len(plugin.calls)) + } + if plugin.calls[0].Body.Content != wantRedacted { + t.Errorf("plugin received unredacted content; got %q want %q", + plugin.calls[0].Body.Content, wantRedacted) } } // TestSeedInitialMemories_InvalidScopeSkipped verifies that entries with an -// unrecognized scope value are silently skipped (not inserted). +// unrecognized scope value are silently skipped (not committed to plugin). func TestSeedInitialMemories_InvalidScopeSkipped(t *testing.T) { - mock := setupTestDB(t) - mock.ExpectationsWereMet() // no DB calls expected for invalid scope + h, plugin := newSeedTestHandler() memories := []models.MemorySeed{ {Content: "this should be skipped", Scope: "NOT_A_REAL_SCOPE"}, } - seedInitialMemories(context.Background(), "ws-bad-scope", memories) + h.seedInitialMemories(context.Background(), "ws-bad-scope", memories) - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("unexpected DB calls for invalid scope: %v", err) + if len(plugin.calls) != 0 { + t.Errorf("plugin should not have been called for invalid scope; got %d calls", len(plugin.calls)) } } // TestSeedInitialMemories_EmptyMemoriesNil verifies that a nil memories slice -// is handled without error (no DB calls). +// is handled without error (no plugin calls). func TestSeedInitialMemories_EmptyMemoriesNil(t *testing.T) { mock := setupTestDB(t) mock.ExpectationsWereMet() - seedInitialMemories(context.Background(), "ws-nil", nil) + h, plugin := newSeedTestHandler() + h.seedInitialMemories(context.Background(), "ws-nil", nil) + + if len(plugin.calls) != 0 { + t.Errorf("plugin should not have been called for nil memories; got %d", len(plugin.calls)) + } if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("unexpected DB calls for nil slice: %v", err) @@ -977,10 +998,11 @@ func containsStr(s, substr string) bool { // or broadcast payload contains ONLY the generic prod-safe message. // TestSeedInitialMemories_Truncation verifies that seedInitialMemories -// truncates content at maxMemoryContentLength before INSERT. Regression -// test for the error-sanitization / memory-seed contract. +// truncates content at maxMemoryContentLength before the plugin commit. +// Regression test for the CWE-400 boundary enforcement (#1167 + #1208). +// Ported from sqlmock to plugin-stub by #1755. func TestSeedInitialMemories_Truncation(t *testing.T) { - mock := setupTestDB(t) + h, plugin := newSeedTestHandler() // Content sized > maxMemoryContentLength so we can assert truncation // fires. Each "hello world " is 12 bytes; 8334 copies = 100008 bytes. @@ -993,41 +1015,37 @@ func TestSeedInitialMemories_Truncation(t *testing.T) { memories := []models.MemorySeed{ {Content: largeContent, Scope: "LOCAL"}, } + h.seedInitialMemories(context.Background(), "ws-1066-test", memories) - mock.ExpectExec(`INSERT INTO agent_memories`). - WithArgs(sqlmock.AnyArg(), expectTruncated, "LOCAL", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(0, 1)) - - seedInitialMemories(context.Background(), "ws-1066-test", memories) - - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("DB expectations not met: %v\n"+ - "INSERT should fire with truncated 100_000-byte content, not 100_001-byte", err) + if len(plugin.calls) != 1 { + t.Fatalf("expected 1 plugin call, got %d", len(plugin.calls)) + } + if plugin.calls[0].Body.Content != expectTruncated { + t.Errorf("plugin content length=%d want=%d (truncate-to-100k contract)", + len(plugin.calls[0].Body.Content), len(expectTruncated)) } } // TestSeedInitialMemories_ContentUnderLimit passes through unchanged. func TestSeedInitialMemories_ContentUnderLimit(t *testing.T) { - mock := setupTestDB(t) + h, plugin := newSeedTestHandler() memories := []models.MemorySeed{ {Content: "short content", Scope: "TEAM"}, } + h.seedInitialMemories(context.Background(), "ws-1066-under", memories) - mock.ExpectExec(`INSERT INTO agent_memories`). - WithArgs(sqlmock.AnyArg(), "short content", "TEAM", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(0, 1)) - - seedInitialMemories(context.Background(), "ws-1066-under", memories) - - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("DB expectations not met: %v", err) + if len(plugin.calls) != 1 { + t.Fatalf("expected 1 plugin call, got %d", len(plugin.calls)) + } + if plugin.calls[0].Body.Content != "short content" { + t.Errorf("plugin content = %q want %q", plugin.calls[0].Body.Content, "short content") } } // TestSeedInitialMemories_ExactlyAtLimit passes through unchanged (boundary case). func TestSeedInitialMemories_ExactlyAtLimit(t *testing.T) { - mock := setupTestDB(t) + h, plugin := newSeedTestHandler() // Exactly maxMemoryContentLength — should NOT be truncated. Content // must include spaces so redactSecrets doesn't collapse it into a @@ -1038,56 +1056,194 @@ func TestSeedInitialMemories_ExactlyAtLimit(t *testing.T) { memories := []models.MemorySeed{ {Content: atLimitContent, Scope: "LOCAL"}, } + h.seedInitialMemories(context.Background(), "ws-boundary", memories) - mock.ExpectExec(`INSERT INTO agent_memories`). - WithArgs(sqlmock.AnyArg(), atLimitContent, "LOCAL", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(0, 1)) - - seedInitialMemories(context.Background(), "ws-boundary", memories) - - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("DB expectations not met: %v", err) + if len(plugin.calls) != 1 { + t.Fatalf("expected 1 plugin call, got %d", len(plugin.calls)) + } + if plugin.calls[0].Body.Content != atLimitContent { + t.Errorf("at-limit content was modified; len got=%d want=%d", + len(plugin.calls[0].Body.Content), len(atLimitContent)) } } -// TestSeedInitialMemories_EmptyContent is skipped (no DB call). +// TestSeedInitialMemories_EmptyContent is skipped (no plugin call). func TestSeedInitialMemories_EmptyContent(t *testing.T) { - mock := setupTestDB(t) + h, plugin := newSeedTestHandler() memories := []models.MemorySeed{ {Content: "", Scope: "LOCAL"}, } + h.seedInitialMemories(context.Background(), "ws-empty", memories) - // seedInitialMemories skips empty content at line 234 — no DB call expected. - seedInitialMemories(context.Background(), "ws-empty", memories) - - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("DB expectations not met: %v", err) + if len(plugin.calls) != 0 { + t.Errorf("plugin should not have been called for empty content; got %d calls", len(plugin.calls)) } } -// TestSeedInitialMemories_OversizedWithSecrets truncates at 100k even when content -// contains credential patterns — the boundary enforcement runs before any other -// content inspection. +// TestSeedInitialMemories_OversizedWithSecrets verifies truncation fires +// BEFORE redaction even when content is secret-shaped — the boundary +// enforcement runs before any other content inspection. The redactor +// then collapses the truncated buffer into its placeholder form +// (e.g. "[REDACTED:API_KEY]"), so the final content is much shorter +// than 100k. The contract this test pins is: +// +// 1. Plugin IS called exactly once (oversized + secret-shaped content +// is not silently dropped). +// 2. The raw secret literal must NOT reach the plugin. +// 3. (Bonus) The content the plugin sees is the redactor's output, +// not the raw 200k. func TestSeedInitialMemories_OversizedWithSecrets(t *testing.T) { - mock := setupTestDB(t) + h, plugin := newSeedTestHandler() // 200k of content that looks like secrets — truncation must still fire at 100k. largeWithSecrets := "ANTHROPIC_API_KEY=sk-ant-xxxx" + strings.Repeat("X", 200_000) memories := []models.MemorySeed{ {Content: largeWithSecrets, Scope: "GLOBAL"}, } + h.seedInitialMemories(context.Background(), "ws-secrets", memories) - mock.ExpectExec(`INSERT INTO agent_memories`). - // Content must be truncated to exactly 100k before INSERT fires. - WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "GLOBAL", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(0, 1)) - - seedInitialMemories(context.Background(), "ws-secrets", memories) - - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("DB expectations not met: %v", err) + if len(plugin.calls) != 1 { + t.Fatalf("expected 1 plugin call, got %d", len(plugin.calls)) } + got := plugin.calls[0].Body.Content + if len(got) > maxMemoryContentLength { + t.Errorf("plugin content length = %d exceeds truncation cap %d", len(got), maxMemoryContentLength) + } + if strings.Contains(got, "sk-ant-xxxx") { + t.Errorf("plugin received raw secret literal — redaction did not fire: %q", got) + } +} + +// captureSeedLogs runs fn with the package-level log writer redirected +// into a buffer so tests can assert on operator-visible warnings. +// Restores the prior writer on exit so other tests aren't affected. +// Mirrors the pattern in internal/memory/wiring/wiring_test.go. +func captureSeedLogs(t *testing.T, fn func()) string { + t.Helper() + var buf bytes.Buffer + prev := log.Writer() + log.SetOutput(&buf) + t.Cleanup(func() { log.SetOutput(prev) }) + fn() + return buf.String() +} + +// TestSeedInitialMemories_PluginNotWired_LogsAndSkips covers #1755's +// fail-soft behavior: when the operator hasn't set MEMORY_PLUGIN_URL +// (so WithSeedMemoryPlugin was never called), seeding should NOT crash +// and NOT write anywhere — it logs an operator-actionable warning and +// returns. Log-capture pins the warning so a future refactor that +// drops the line silently regresses observability (#1759 review N1). +func TestSeedInitialMemories_PluginNotWired_LogsAndSkips(t *testing.T) { + h := &WorkspaceHandler{} // seedMemoryPlugin nil + + memories := []models.MemorySeed{ + {Content: "would never persist", Scope: "LOCAL"}, + {Content: "ditto", Scope: "TEAM"}, + } + + out := captureSeedLogs(t, func() { + // Must not panic; must not error. + h.seedInitialMemories(context.Background(), "ws-no-plugin", memories) + }) + + if !strings.Contains(out, "v2 memory plugin not wired") { + t.Errorf("operator-visibility regression: expected log to mention 'v2 memory plugin not wired', got:\n%s", out) + } + if !strings.Contains(out, "MEMORY_PLUGIN_URL") { + t.Errorf("operator-visibility regression: log should hint at MEMORY_PLUGIN_URL env var; got:\n%s", out) + } + if !strings.Contains(out, "ws-no-plugin") { + t.Errorf("operator-visibility regression: log should include workspace id for diagnosability; got:\n%s", out) + } +} + +// TestSeedInitialMemories_PluginCommitError_ContinuesLoop pins the +// "each plugin call is attempted independently" contract — if the +// plugin errors on commit, the loop must keep going for the next +// memory rather than aborting the whole seed batch. Log-capture +// pins that each failure is surfaced individually so operators can +// see WHICH seeds failed (#1759 review N1). +func TestSeedInitialMemories_PluginCommitError_ContinuesLoop(t *testing.T) { + stub := &stubSeedPlugin{commitErr: errors.New("plugin down")} + h := &WorkspaceHandler{seedMemoryPlugin: stub} + + memories := []models.MemorySeed{ + {Content: "one", Scope: "LOCAL"}, + {Content: "two", Scope: "LOCAL"}, + {Content: "three", Scope: "LOCAL"}, + } + + out := captureSeedLogs(t, func() { + h.seedInitialMemories(context.Background(), "ws-erroring-plugin", memories) + }) + + // All three should have been attempted even though each errored. + if len(stub.calls) != 3 { + t.Errorf("expected 3 plugin attempts despite errors, got %d", len(stub.calls)) + } + + // Each failure must produce a log line so an operator tailing + // stderr sees the failures one by one (not just a swallowed loop). + failures := strings.Count(out, "plugin commit failed") + if failures != 3 { + t.Errorf("expected 3 'plugin commit failed' log lines, got %d; output:\n%s", failures, out) + } + if !strings.Contains(out, "plugin down") { + t.Errorf("log should surface the underlying error message; got:\n%s", out) + } +} + +// TestSeedInitialMemories_PartialFailure_CounterIsAccurate covers the +// gap #1759 review flagged: a mixed batch where some seeds succeed and +// others fail. The "seeded %d/%d" summary log must reflect the actual +// success count, not the attempt count. +func TestSeedInitialMemories_PartialFailure_CounterIsAccurate(t *testing.T) { + callCount := 0 + stub := &stubSeedPlugin{} + // Wrap stub to fail every other call. + h := &WorkspaceHandler{seedMemoryPlugin: stubAlternatingFailures(stub, &callCount)} + + memories := []models.MemorySeed{ + {Content: "one", Scope: "LOCAL"}, + {Content: "two", Scope: "LOCAL"}, + {Content: "three", Scope: "LOCAL"}, + {Content: "four", Scope: "LOCAL"}, + } + + out := captureSeedLogs(t, func() { + h.seedInitialMemories(context.Background(), "ws-mixed", memories) + }) + + // All four attempted. + if callCount != 4 { + t.Errorf("expected 4 plugin attempts, got %d", callCount) + } + // Half failed → seeded counter should be 2/4 in the summary log. + if !strings.Contains(out, "seeded 2/4 memories") { + t.Errorf("expected 'seeded 2/4 memories' in summary log to reflect partial success; got:\n%s", out) + } +} + +// stubAlternatingFailures wraps a stub plugin so that every other +// CommitMemory call errors. callCount is incremented on each invocation +// (caller owns the pointer). +func stubAlternatingFailures(_ *stubSeedPlugin, callCount *int) seedMemoryPluginAPI { + return alternatingFailingPlugin{count: callCount} +} + +type alternatingFailingPlugin struct { + count *int +} + +func (a alternatingFailingPlugin) CommitMemory(_ context.Context, ns string, body contract.MemoryWrite) (*contract.MemoryWriteResponse, error) { + *a.count++ + if *a.count%2 == 1 { + // Odd-numbered calls (1st, 3rd) fail. + return nil, errors.New("transient plugin hiccup") + } + return &contract.MemoryWriteResponse{ID: fmt.Sprintf("mem-%d", *a.count), Namespace: ns}, nil } // ==================== error-sanitization regression tests ====================