feat(memory): #1755 route seedInitialMemories through v2 plugin
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
E2E Chat / E2E Chat (pull_request) Blocked by required conditions
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Blocked by required conditions
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Blocked by required conditions
Harness Replays / Harness Replays (pull_request) Blocked by required conditions
sop-checklist / review-refire (pull_request) Waiting to run
sop-tier-check / tier-check (pull_request) Waiting to run
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Successful in 8s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 9s
CI / Detect changes (pull_request) Successful in 9s
CI / Python Lint & Test (pull_request) Successful in 6s
Check migration collisions / Migration version collision check (pull_request) Failing after 27s
E2E API Smoke Test / detect-changes (pull_request) Successful in 9s
E2E Chat / detect-changes (pull_request) Successful in 10s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Has been skipped
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 13s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Successful in 1m1s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Successful in 40s
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Has been skipped
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
Harness Replays / detect-changes (pull_request) Successful in 4s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 3s
Lint no tenant GITEA or GITHUB token write / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m5s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
gate-check-v3 / gate-check (pull_request) Successful in 4s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
qa-review / approved (pull_request) Failing after 4s
security-review / approved (pull_request) Failing after 5s
E2E Staging External Runtime / E2E Staging External Runtime (pull_request) Successful in 6m6s
CI / all-required (pull_request) Failing after 40m8s
CI / Canvas (Next.js) (pull_request) Successful in 48s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 17s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m50s
CI / Platform (Go) (pull_request) Successful in 5m7s
CI / Canvas Deploy Reminder (pull_request) Has been cancelled
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
E2E Chat / E2E Chat (pull_request) Blocked by required conditions
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Blocked by required conditions
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Blocked by required conditions
Harness Replays / Harness Replays (pull_request) Blocked by required conditions
sop-checklist / review-refire (pull_request) Waiting to run
sop-tier-check / tier-check (pull_request) Waiting to run
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Successful in 8s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 9s
CI / Detect changes (pull_request) Successful in 9s
CI / Python Lint & Test (pull_request) Successful in 6s
Check migration collisions / Migration version collision check (pull_request) Failing after 27s
E2E API Smoke Test / detect-changes (pull_request) Successful in 9s
E2E Chat / detect-changes (pull_request) Successful in 10s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Has been skipped
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 13s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Successful in 1m1s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Successful in 40s
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Has been skipped
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
Harness Replays / detect-changes (pull_request) Successful in 4s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 3s
Lint no tenant GITEA or GITHUB token write / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m5s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
gate-check-v3 / gate-check (pull_request) Successful in 4s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
qa-review / approved (pull_request) Failing after 4s
security-review / approved (pull_request) Failing after 5s
E2E Staging External Runtime / E2E Staging External Runtime (pull_request) Successful in 6m6s
CI / all-required (pull_request) Failing after 40m8s
CI / Canvas (Next.js) (pull_request) Successful in 48s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 17s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m50s
CI / Platform (Go) (pull_request) Successful in 5m7s
CI / Canvas Deploy Reminder (pull_request) Has been cancelled
Closes #1755. Gates Phase A3 (drop agent_memories) per the #1733 sequencing — A3 cannot land until this PR is in production, or workspace creation with seeded initial_memories breaks. ## Problem After PR #1747 (Phase A1) lands, the MCP memory tools (commit_memory, recall_memory) read and write exclusively through the v2 plugin. But seedInitialMemories was still INSERTing directly into the legacy agent_memories table at workspace-create time. Result: a workspace provisioned with `initial_memories` from a template had rows in agent_memories that were invisible to recall_memory — every fresh workspace silently had no seeded memory. Flagged in the #1747 hostile review (finding C3) and deliberately deferred to keep that PR small. This PR is that deferral. ## Change - New `seedMemoryPluginAPI` interface on WorkspaceHandler — the slice of `*client.Client` (memory/client) that seedInitialMemories needs (CommitMemory only). Parallel to MCPHandler's memoryPluginAPI. - New `WithSeedMemoryPlugin` setter — main.go wires this alongside WithNamespaceCleanup when MEMORY_PLUGIN_URL is set (memBundle.Plugin). - seedInitialMemories converted from package-level function to method on *WorkspaceHandler. Three call sites updated: - workspace.go:606 — Create handler - org.go:807 — org-template global memories - org_import.go:261 — per-workspace initial_memories - Routes through h.seedMemoryPlugin.CommitMemory(ctx, "workspace:<id>", contract.MemoryWrite{Content, Kind=fact, Source=runtime}). - Source=runtime distinguishes platform-seeded memories from agent-written (Source=agent) and canvas-written (Source=user). - All seeds land in the workspace's own `workspace:<id>` namespace. Pre-A1 callers wrote TEAM/GLOBAL-scoped seeds with namespace=workspace:<id> in agent_memories anyway, so behaviour- preserving for LOCAL and a no-op-shrink for TEAM/GLOBAL (agent can promote later via commit_memory_v2). - When the plugin is nil (operator hasn't set MEMORY_PLUGIN_URL), seedInitialMemories logs a loud warning with operator-actionable hint and SKIPS the seed batch — there is no SQL fallback after A1. Self-hosted operators without v2 wired get a clear signal that seeded memories from templates aren't persisted on their deployment. - Truncation + redaction order preserved exactly: 1. Validate scope (LOCAL/TEAM/GLOBAL). 2. Skip empty content. 3. Truncate to maxMemoryContentLength (100 KiB, CWE-400 cap). 4. Apply redactSecrets. 5. Plugin commit. - Plugin commit errors are non-fatal: loop continues for the next seed, mirroring the original "each insert is attempted independently" contract. ## Tests 8 existing seedInitialMemories tests rewritten + 2 new ones: | Test | Coverage | |---|---| | TestSeedInitialMemories_TruncatesOversizedContent | Parameterized truncation contract at the 100k boundary; asserts on plugin.calls[0].Body.Content length | | TestSeedInitialMemories_RedactsSecrets | Redaction fires before plugin commit | | TestSeedInitialMemories_InvalidScopeSkipped | Invalid scope = no plugin call | | TestSeedInitialMemories_EmptyMemoriesNil | Nil slice = no plugin call | | TestSeedInitialMemories_Truncation | Plain truncation, plugin sees 100k | | TestSeedInitialMemories_ContentUnderLimit | Under-limit content passes through unchanged | | TestSeedInitialMemories_ExactlyAtLimit | Boundary case (== limit) — no truncation | | TestSeedInitialMemories_EmptyContent | Empty content skipped | | TestSeedInitialMemories_OversizedWithSecrets | Truncation BEFORE redaction; raw secret literal never reaches plugin | | TestSeedInitialMemories_PluginNotWired_LogsAndSkips | **NEW** — fail-soft when MEMORY_PLUGIN_URL unset | | TestSeedInitialMemories_PluginCommitError_ContinuesLoop | **NEW** — each plugin call is independent; one failure doesn't abort the batch | Test infrastructure: new `stubSeedPlugin` captures every plugin call for assertion (parallel to `stubMemoryPlugin` in mcp_tools_memory_v2_ test.go). ## Stage A - `go vet ./...` clean. - `go test -short -count=1 ./...` green across every package (workspace-server unit + integration suites). - Stage B / C deferred to staging deploy after PR #1742 (A0 schema isolation) and PR #1747 (A1 v1-fallback removal) land. ## Sequencing - Branched on top of PR #1737 (awareness backend removal — it does the seedInitialMemories signature pre-work this PR needs). - Land order: #1737 → #1742 → #1747 → this. When #1737 merges, this branch rebases cleanly onto main with the seed-migration as the only delta. When #1747 merges, the v1 fallback is gone and this PR's "skip when plugin not wired" semantics become the only behaviour. ## Gating Phase A3 Phase A3 (drop agent_memories table) requires this to land first. After this ships: - New workspaces with initial_memories → plugin. - Pre-A1 workspaces with rows in agent_memories → migrated by the one-shot Phase A2 backfill (cmd/memory-backfill, separate ops task). - A3 can then DROP TABLE agent_memories with no live writers. Refs: closes #1755, follow-up to #1747 review finding C3, gates Phase A3 of #1733. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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:<id>`); 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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:<id>` (the workspace's private
|
||||
// namespace). Pre-A1 callers wrote TEAM/GLOBAL-scoped memories into
|
||||
// `agent_memories` with `namespace=workspace:<id>` 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
|
||||
|
||||
@@ -3,6 +3,7 @@ package handlers
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"os"
|
||||
@@ -11,6 +12,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 +579,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 +646,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 +684,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 +996,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 +1013,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,55 +1054,98 @@ 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))
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
seedInitialMemories(context.Background(), "ws-secrets", memories)
|
||||
// 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 and returns.
|
||||
func TestSeedInitialMemories_PluginNotWired_LogsAndSkips(t *testing.T) {
|
||||
h := &WorkspaceHandler{} // seedMemoryPlugin nil
|
||||
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("DB expectations not met: %v", err)
|
||||
memories := []models.MemorySeed{
|
||||
{Content: "would never persist", Scope: "LOCAL"},
|
||||
{Content: "ditto", Scope: "TEAM"},
|
||||
}
|
||||
// Must not panic; must not error.
|
||||
h.seedInitialMemories(context.Background(), "ws-no-plugin", memories)
|
||||
}
|
||||
|
||||
// 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.
|
||||
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"},
|
||||
}
|
||||
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))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user