refactor(memory): #1733 A1 — kill v1 SQL fallback, v2 plugin is the only backend
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Waiting to run
sop-checklist / review-refire (pull_request) Waiting to run
sop-tier-check / tier-check (pull_request) Waiting to run
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 4s
CI / Detect changes (pull_request) Successful in 5s
CI / Python Lint & Test (pull_request) Successful in 3s
E2E API Smoke Test / detect-changes (pull_request) Successful in 6s
E2E Chat / detect-changes (pull_request) Successful in 6s
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 12s
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Has been skipped
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Successful in 44s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 8s
Harness Replays / detect-changes (pull_request) Successful in 3s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Successful in 34s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 6s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 6s
Lint no tenant GITEA or GITHUB token write / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 3s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Failing after 1m3s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 1m14s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 1m27s
review-check-tests / review-check.sh regression tests (pull_request) Successful in 6s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m15s
gate-check-v3 / gate-check (pull_request) Successful in 4s
qa-review / approved (pull_request) Failing after 4s
security-review / approved (pull_request) Failing after 3s
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)
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m17s
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 1m10s
CI / Canvas (Next.js) (pull_request) Successful in 2s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m31s
CI / Platform (Go) (pull_request) Successful in 4m6s
E2E Chat / E2E Chat (pull_request) Successful in 2s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 2s
CI / all-required (pull_request) Successful in 23m38s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1m12s
Harness Replays / Harness Replays (pull_request) Successful in 2s
CI / Canvas Deploy Reminder (pull_request) Has been skipped

CTO decision 2026-05-23: same Postgres, separate schema, v2 plugin is
SSOT for tenant memory. A0 (PR #1742) isolates plugin tables under the
`memory_plugin` schema. A1 (this PR) removes the silent fallback paths
that route to `agent_memories` when MEMORY_PLUGIN_URL is unset, so a
half-configured deployment fails loudly on the first memory call rather
than quietly writing to a frozen table no one reads.

## Surfaces removed

1. **`toolCommitMemory` legacy SQL branch** (`internal/handlers/mcp_tools.go`).
   Was: route through v2 shim when `memoryV2Available() == nil`, else
   INSERT directly into `agent_memories`. Now: error
   `memory plugin is not configured (set MEMORY_PLUGIN_URL)` when v2 is
   not wired; route through the shim otherwise. ~30 LOC of inline SQL +
   scope validation gone — the shim already enforces the same scope
   rules (it just talks to the plugin).

2. **`toolRecallMemory` legacy SQL branch** (same file). Was: three
   scope-specific SELECTs on `agent_memories` when v2 unwired. Now:
   same error semantics as commit. ~60 LOC gone.

3. **`AdminMemoriesHandler.Export` legacy SQL branch**
   (`internal/handlers/admin_memories.go`). Was: when
   `cutoverActive() == false` (which checked both `MEMORY_V2_CUTOVER`
   *and* v2 wiring), run a direct `SELECT FROM agent_memories JOIN
   workspaces`. Now: respond `503 Service Unavailable` with hint
   `memory plugin is not configured (set MEMORY_PLUGIN_URL)`. Plugin
   path unchanged.

4. **`AdminMemoriesHandler.Import` legacy SQL branch** (same file).
   Was: per-entry workspace lookup + dedup check + INSERT into
   `agent_memories`. Now: same 503 when plugin not wired.

5. **`cutoverActive()` + `envMemoryV2Cutover` constant** removed —
   replaced with `memoryV2Wired()` which only checks `h.plugin != nil
   && h.resolver != nil`. The `MEMORY_V2_CUTOVER` env var is no longer
   read inside the platform binary (CP user-data still inspects it
   before spawning the sidecar, but workspace-server does not branch
   on it). Tenant operators upgrading to this build cannot accidentally
   stay in v1 by unsetting `MEMORY_V2_CUTOVER`.

6. **Boot-time guard simplification** in
   `internal/memory/wiring/wiring.go`. The `cutover` boolean and its
   two loud-WARN log lines are gone (the half-configured-cutover
   failure mode they guarded against is structurally impossible now).
   The one remaining boot log when `MEMORY_PLUGIN_URL` is unset
   explicitly tells the operator that v2 MCP tools will return errors
   until the env is set.

## Surfaces deliberately NOT removed in this PR

- **`MemoriesHandler` REST endpoints** (`POST/GET/DELETE/PATCH
  /workspaces/:id/memories` in `memories.go`). These are the
  canvas-side surface and still write to `agent_memories`. Their
  cutover is the #1734 work (canvas Memory tab fix); coupling that
  here would make this PR much bigger and would conflict with the tab
  rewrite already drafted in #1734.

- **`seedInitialMemories`** (`workspace_provision.go`) still writes to
  `agent_memories` during workspace creation. Its v2 migration is
  deferred to Phase A3 (when we drop the table entirely) — early
  migration would force every test that exercises workspace creation
  to wire a v2 stub.

- **REST `/admin/memories/export` and `/admin/memories/import`
  request-decode path** — the `400 Bad Request` reject on malformed
  JSON runs before any plugin check and is preserved by the surviving
  `TestAdminMemories_Import_InvalidJSON`.

## Tests rewritten

- `TestCutoverActive` → `TestMemoryV2Wired` (the env-flag dimension is
  gone; only wiring matters now).
- `TestExport_LegacyPathWhenCutoverInactive` → `TestExport_503When
  PluginNotWired` + new sibling `TestImport_503WhenPluginNotWired`.
- `TestToolCommitMemory_FallsThroughToLegacyWhenV2Unwired` →
  `TestToolCommitMemory_ErrorsWhenV2Unwired` (asserts on the
  error-message hint at `MEMORY_PLUGIN_URL`).
- `TestToolRecallMemory_FallsThroughToLegacyWhenV2Unwired` →
  `TestToolRecallMemory_ErrorsWhenV2Unwired` (same).
- `TestBuild_WarnsWhenCutoverWithoutPluginURL`,
  `TestBuild_LoudWarnWhenCutoverAndProbeFails`,
  `TestBuild_NoWarnWhenNeitherSet`, `TestBuild_QuietProbeFailWhenCutoverOff`
  → consolidated to `TestBuild_LogsWhenURLUnset` +
  `TestBuild_LogsWhenProbeFails`.

## Tests deleted (legacy SQL coverage now redundant)

- `TestAdminMemories_Export_{Success,Empty,QueryError,RedactsSecrets}`,
  `TestAdminMemories_Import_{Success,WorkspaceNotFound_SkipsEntry,
  DuplicateSkipped,RedactsSecretsBeforeDedup,PreservesCreatedAt}` —
  every assertion was about the deleted SQL path. The plugin path is
  fully covered by 16 tests in `admin_memories_cutover_test.go`
  (listed in the file header for traceability).
- `TestMCPHandler_CommitMemory_LocalScope_Success`,
  `TestMCPHandler_CommitMemory_SecretInContent_IsRedactedBeforeInsert`,
  `TestMCPHandler_CommitMemory_CleanContent_PassesThrough`,
  `TestMCPHandler_RecallMemory_LocalScope_Empty` — same reason; the v2
  shim path is covered by `TestToolCommitMemory_RoutesThroughV2WhenWired`,
  `TestToolRecallMemory_RoutesThroughV2WhenWired`, and every test in
  `mcp_tools_memory_v2_test.go`.
- `TestMCPHandler_CommitMemory_GlobalScope_Blocked_ScrubsInternalError`
  and its `RecallMemory_` sibling are KEPT (renamed to drop the
  `GlobalScope_Blocked_` qualifier) because they validate the OFFSEC-001
  JSON-RPC scrub layer in `mcp.go dispatchRPC`, which is orthogonal to
  the memory backend.

Net diff: −752 LOC across 9 files.

## Stage gates

**Stage A — local build + test sweep**:
- `go vet ./...` clean.
- `go test -count=1 ./internal/handlers/... ./internal/memory/...
  ./cmd/memory-plugin-postgres/...` all green (~25 s).
- Manual: confirmed `memoryV2Available()` still returns its hint
  message ("set MEMORY_PLUGIN_URL"), and that the JSON-RPC scrub
  replaces it with the OFFSEC-001 constant.

**Stage B / C — deferred to staging deploy after gating PR lands**.

## Gating

**DO NOT merge until PR #1742 (A0 schema isolation) has been merged
and rolled out to every tenant's image cache.** A tenant booting this
build without a reachable memory plugin sidecar will return errors
on every `commit_memory` / `recall_memory` / admin export+import call.
That's the intended behavior — but it should only happen after the
sidecar is reliably running on every tenant EC2.

The existing tenant entrypoint (`entrypoint-tenant.sh:35-66`,
since 2026-05-14) already aborts boot when `MEMORY_V2_CUTOVER=true`
and the plugin doesn't come up in 30s, so SaaS tenants are protected
by the CP boot gate — but only IF they're running the post-2026-05-14
image. Audit `org_instances.fly_machine_id` boot logs before
flipping.

## Risks

- Self-hosted operators who haven't set `MEMORY_PLUGIN_URL` will get
  errors instead of working `commit_memory`. The error message tells
  them exactly what to set. Acceptable trade-off for SSOT.
- Tenants currently running on a pre-2026-05-14 tenant image
  (entrypoint doesn't spawn sidecar) will start failing memory calls
  on first restart after the new core image rolls out. Confirm CP
  has recycled every tenant onto the current image before merging.
- `seedInitialMemories` still writes to `agent_memories`; rows there
  remain readable to any operator query bypassing the API. They'll
  go away with the table in Phase A3.

Refs: #1733 (memory SSOT consolidation), #1742 (Phase A0 — schema
isolation, gates this PR), #1734 (Memory tab UI fix, unblocked once
this lands).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-23 15:06:52 -07:00
parent 656176d511
commit 2aa6e8adf3
9 changed files with 205 additions and 957 deletions
@@ -5,7 +5,6 @@ import (
"database/sql"
"log"
"net/http"
"os"
"strings"
"time"
@@ -16,19 +15,12 @@ import (
"github.com/gin-gonic/gin"
)
// envMemoryV2Cutover gates whether admin export/import routes through
// the v2 plugin (PR-8 / RFC #2728). When unset, the legacy direct-DB
// path runs unchanged so operators who haven't enabled the plugin
// keep working.
const envMemoryV2Cutover = "MEMORY_V2_CUTOVER"
// AdminMemoriesHandler provides bulk export/import of agent memories for
// backup and restore across Docker rebuilds (issue #1051).
//
// PR-8 (RFC #2728): when wired with the v2 plugin via WithMemoryV2 AND
// MEMORY_V2_CUTOVER is true, export reads from the plugin's namespaces
// and import writes through the plugin. Both paths preserve the
// SAFE-T1201 redaction shipped in F1084 + F1085.
// Issue #1733: the v2 plugin is the only supported backend. Export
// reads from the plugin's namespaces; import writes through the plugin.
// Both paths preserve the SAFE-T1201 redaction shipped in F1084 + F1085.
type AdminMemoriesHandler struct {
plugin adminMemoriesPlugin
resolver adminMemoriesResolver
@@ -69,12 +61,12 @@ func (h *AdminMemoriesHandler) withMemoryV2APIs(plugin adminMemoriesPlugin, reso
return h
}
// cutoverActive reports whether the export/import path should route
// through the v2 plugin.
func (h *AdminMemoriesHandler) cutoverActive() bool {
if os.Getenv(envMemoryV2Cutover) != "true" {
return false
}
// memoryV2Wired reports whether the v2 plugin + resolver are attached.
// Issue #1733: v2 is now the only path; this replaces the prior
// cutoverActive() gate (which also checked MEMORY_V2_CUTOVER=true) —
// the env-flag double-check is gone since there's no legacy fallback
// to choose against.
func (h *AdminMemoriesHandler) memoryV2Wired() bool {
return h.plugin != nil && h.resolver != nil
}
@@ -97,48 +89,19 @@ type memoryExportEntry struct {
// before returning so that any credentials stored before SAFE-T1201 (#838)
// was applied do not leak out via the admin export endpoint.
//
// CUTOVER (PR-8 / RFC #2728): when MEMORY_V2_CUTOVER=true and the v2
// plugin is wired, reads from the plugin instead of agent_memories.
// Issue #1733: reads exclusively from the v2 plugin. The legacy direct
// agent_memories scan is gone — operators without a configured plugin
// get a 503 explaining the required setup.
func (h *AdminMemoriesHandler) Export(c *gin.Context) {
ctx := c.Request.Context()
if h.cutoverActive() {
h.exportViaPlugin(c, ctx)
if !h.memoryV2Wired() {
c.JSON(http.StatusServiceUnavailable, gin.H{
"error": "memory plugin is not configured (set MEMORY_PLUGIN_URL)",
})
return
}
rows, err := db.DB.QueryContext(ctx, `
SELECT am.id, am.content, am.scope, am.namespace, am.created_at,
w.name AS workspace_name
FROM agent_memories am
JOIN workspaces w ON am.workspace_id = w.id
ORDER BY am.created_at
`)
if err != nil {
log.Printf("admin/memories/export: query error: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "export query failed"})
return
}
defer rows.Close()
memories := make([]memoryExportEntry, 0)
for rows.Next() {
var m memoryExportEntry
if err := rows.Scan(&m.ID, &m.Content, &m.Scope, &m.Namespace, &m.CreatedAt, &m.WorkspaceName); err != nil {
log.Printf("admin/memories/export: scan error: %v", err)
continue
}
// F1084 / #1131: redact secrets before returning so pre-SAFE-T1201
// memories (stored before redactSecrets was mandatory) don't leak.
redacted, _ := redactSecrets(m.WorkspaceName, m.Content)
m.Content = redacted
memories = append(memories, m)
}
if err := rows.Err(); err != nil {
log.Printf("admin/memories/export: rows error: %v", err)
}
c.JSON(http.StatusOK, memories)
h.exportViaPlugin(c, ctx)
}
// memoryImportEntry is the JSON shape accepted on import. Matches export format.
@@ -160,8 +123,9 @@ type memoryImportEntry struct {
// with embedded credentials cannot land unredacted in agent_memories (SAFE-T1201
// parity with the commit_memory MCP bridge path).
//
// CUTOVER (PR-8 / RFC #2728): when MEMORY_V2_CUTOVER=true and the v2
// plugin is wired, writes through the plugin instead of agent_memories.
// Issue #1733: writes exclusively through the v2 plugin. The legacy
// direct agent_memories insert path is gone — operators without a
// configured plugin get a 503 explaining the required setup.
func (h *AdminMemoriesHandler) Import(c *gin.Context) {
ctx := c.Request.Context()
@@ -171,85 +135,13 @@ func (h *AdminMemoriesHandler) Import(c *gin.Context) {
return
}
if h.cutoverActive() {
h.importViaPlugin(c, ctx, entries)
if !h.memoryV2Wired() {
c.JSON(http.StatusServiceUnavailable, gin.H{
"error": "memory plugin is not configured (set MEMORY_PLUGIN_URL)",
})
return
}
imported := 0
skipped := 0
errors := 0
for _, entry := range entries {
// 1. Resolve workspace by name
var workspaceID string
err := db.DB.QueryRowContext(ctx,
`SELECT id FROM workspaces WHERE name = $1 LIMIT 1`,
entry.WorkspaceName,
).Scan(&workspaceID)
if err != nil {
log.Printf("admin/memories/import: workspace %q not found, skipping", entry.WorkspaceName)
skipped++
continue
}
// F1085 / #1132: scrub credential patterns before persistence so that
// imported memories with secrets don't bypass SAFE-T1201 (#838).
// Must run BEFORE the dedup check so the redacted content is what
// gets stored — otherwise re-importing the same backup would produce
// a duplicate with different (original, unredacted) content.
content, _ := redactSecrets(workspaceID, entry.Content)
// 2. Check for duplicate (same workspace + content + scope) using
// the redacted content so that two backups with the same original
// secret (same placeholder output) are treated as duplicates.
var exists bool
err = db.DB.QueryRowContext(ctx,
`SELECT EXISTS(SELECT 1 FROM agent_memories WHERE workspace_id = $1 AND content = $2 AND scope = $3)`,
workspaceID, content, entry.Scope,
).Scan(&exists)
if err != nil {
log.Printf("admin/memories/import: duplicate check error for workspace %q: %v", entry.WorkspaceName, err)
errors++
continue
}
if exists {
skipped++
continue
}
// 3. Insert the memory, preserving original created_at if provided
namespace := entry.Namespace
if namespace == "" {
namespace = "general"
}
if entry.CreatedAt != "" {
_, err = db.DB.ExecContext(ctx,
`INSERT INTO agent_memories (workspace_id, content, scope, namespace, created_at) VALUES ($1, $2, $3, $4, $5)`,
workspaceID, content, entry.Scope, namespace, entry.CreatedAt,
)
} else {
_, err = db.DB.ExecContext(ctx,
`INSERT INTO agent_memories (workspace_id, content, scope, namespace) VALUES ($1, $2, $3, $4)`,
workspaceID, content, entry.Scope, namespace,
)
}
if err != nil {
log.Printf("admin/memories/import: insert error for workspace %q: %v", entry.WorkspaceName, err)
errors++
continue
}
imported++
}
c.JSON(http.StatusOK, gin.H{
"imported": imported,
"skipped": skipped,
"errors": errors,
"total": len(entries),
})
h.importViaPlugin(c, ctx, entries)
}
// exportViaPlugin reads memories from the v2 plugin and emits them in
@@ -101,26 +101,24 @@ func installMockDB(t *testing.T) sqlmock.Sqlmock {
return mock
}
// --- cutoverActive ---
// --- memoryV2Wired ---
func TestCutoverActive(t *testing.T) {
func TestMemoryV2Wired(t *testing.T) {
cases := []struct {
name string
envVal string
plugin adminMemoriesPlugin
resolver adminMemoriesResolver
want bool
}{
{"env unset", "", &stubAdminPlugin{}, adminRootResolver(), false},
{"env true but unwired", "true", nil, nil, false},
{"env false", "false", &stubAdminPlugin{}, adminRootResolver(), false},
{"env true wired", "true", &stubAdminPlugin{}, adminRootResolver(), true},
{"both nil", nil, nil, false},
{"plugin only", &stubAdminPlugin{}, nil, false},
{"resolver only", nil, adminRootResolver(), false},
{"both wired", &stubAdminPlugin{}, adminRootResolver(), true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Setenv(envMemoryV2Cutover, tc.envVal)
h := &AdminMemoriesHandler{plugin: tc.plugin, resolver: tc.resolver}
if got := h.cutoverActive(); got != tc.want {
if got := h.memoryV2Wired(); got != tc.want {
t.Errorf("got %v, want %v", got, tc.want)
}
})
@@ -147,7 +145,6 @@ func TestWithMemoryV2APIs_AttachesDeps(t *testing.T) {
// --- Export via plugin ---
func TestExport_RoutesThroughPluginWhenCutoverActive(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
@@ -191,7 +188,6 @@ func TestExport_RoutesThroughPluginWhenCutoverActive(t *testing.T) {
}
func TestExport_DeduplicatesByMemoryID(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
// Two workspaces, both will see the same team-shared memory.
@@ -222,7 +218,6 @@ func TestExport_DeduplicatesByMemoryID(t *testing.T) {
}
func TestExport_SkipsWorkspaceWhenResolverFails(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
WillReturnRows(sqlmock.NewRows([]string{"id", "name", "root_id"}).
@@ -244,7 +239,6 @@ func TestExport_SkipsWorkspaceWhenResolverFails(t *testing.T) {
}
func TestExport_SkipsWorkspaceWhenPluginSearchFails(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
WillReturnRows(sqlmock.NewRows([]string{"id", "name", "root_id"}).
@@ -268,7 +262,6 @@ func TestExport_SkipsWorkspaceWhenPluginSearchFails(t *testing.T) {
}
func TestExport_WorkspacesQueryFails(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
WillReturnError(errors.New("db dead"))
@@ -287,7 +280,6 @@ func TestExport_WorkspacesQueryFails(t *testing.T) {
}
func TestExport_EmptyReadable(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
WillReturnRows(sqlmock.NewRows([]string{"id", "name", "root_id"}).
@@ -309,7 +301,6 @@ func TestExport_EmptyReadable(t *testing.T) {
}
func TestExport_RedactsSecretsInPluginPath(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
WillReturnRows(sqlmock.NewRows([]string{"id", "name", "root_id"}).
@@ -337,7 +328,6 @@ func TestExport_RedactsSecretsInPluginPath(t *testing.T) {
// --- Import via plugin ---
func TestImport_RoutesThroughPluginWhenCutoverActive(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WithArgs("alpha").
@@ -368,7 +358,6 @@ func TestImport_RoutesThroughPluginWhenCutoverActive(t *testing.T) {
}
func TestImport_SkipsUnknownWorkspace(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WithArgs("ghost").
@@ -395,7 +384,6 @@ func TestImport_SkipsUnknownWorkspace(t *testing.T) {
}
func TestImport_PluginUpsertNamespaceError(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("root-1"))
@@ -425,7 +413,6 @@ func TestImport_PluginUpsertNamespaceError(t *testing.T) {
}
func TestImport_PluginCommitError(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("root-1"))
@@ -455,7 +442,6 @@ func TestImport_PluginCommitError(t *testing.T) {
}
func TestImport_RedactsBeforePluginSeesContent(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("root-1"))
@@ -482,7 +468,6 @@ func TestImport_RedactsBeforePluginSeesContent(t *testing.T) {
}
func TestImport_SkipsUnknownScope(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("root-1"))
@@ -508,7 +493,6 @@ func TestImport_SkipsUnknownScope(t *testing.T) {
}
func TestImport_SkipsWhenResolverErrors(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("SELECT id::text FROM workspaces").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("root-1"))
@@ -545,7 +529,6 @@ func TestImport_SkipsWhenResolverErrors(t *testing.T) {
// + org:root-1. (Children's workspace:<id> namespaces must be
// included or admin export silently drops their private memories.)
func TestExport_BatchesPluginCallsByRoot(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
@@ -605,7 +588,6 @@ func (r perWorkspaceResolver) WritableNamespaces(_ context.Context, ws string) (
// workspace:rootID + team:rootID + org:rootID — every child workspace's
// private memories were silently dropped from admin export.
func TestExport_IncludesEveryMembersPrivateNamespace(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "true")
mock := installMockDB(t)
mock.ExpectQuery("WITH RECURSIVE chain").
@@ -775,25 +757,43 @@ func TestSkipImport_ErrorMessage(t *testing.T) {
}
}
// --- Confirm legacy paths still work when env is unset ---
// --- 503 when plugin is not wired (issue #1733) ---
//
// The legacy SQL-backed Export/Import path was removed; both endpoints
// now respond 503 with a clear hint when v2 isn't configured.
func TestExport_LegacyPathWhenCutoverInactive(t *testing.T) {
t.Setenv(envMemoryV2Cutover, "")
mock := installMockDB(t)
mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace").
WillReturnRows(sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"}))
h := NewAdminMemoriesHandler().withMemoryV2APIs(&stubAdminPlugin{}, adminRootResolver())
func TestExport_503WhenPluginNotWired(t *testing.T) {
installMockDB(t)
h := NewAdminMemoriesHandler() // no WithMemoryV2 → plugin nil
gin.SetMode(gin.TestMode)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest("GET", "/admin/memories/export", nil)
h.Export(c)
if w.Code != http.StatusOK {
t.Errorf("code = %d body=%s", w.Code, w.Body.String())
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("code = %d body=%s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("legacy SQL path not exercised: %v", err)
if !strings.Contains(w.Body.String(), "MEMORY_PLUGIN_URL") {
t.Errorf("body must hint at MEMORY_PLUGIN_URL: %s", w.Body.String())
}
}
func TestImport_503WhenPluginNotWired(t *testing.T) {
installMockDB(t)
h := NewAdminMemoriesHandler()
gin.SetMode(gin.TestMode)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest("POST", "/admin/memories/import",
bytes.NewBufferString(`[]`))
c.Request.Header.Set("Content-Type", "application/json")
h.Import(c)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("code = %d body=%s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "MEMORY_PLUGIN_URL") {
t.Errorf("body must hint at MEMORY_PLUGIN_URL: %s", w.Body.String())
}
}
@@ -2,220 +2,46 @@ package handlers
import (
"bytes"
"database/sql"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/DATA-DOG/go-sqlmock"
"github.com/gin-gonic/gin"
)
// newAdminMemoriesHandler is a test helper that returns an AdminMemoriesHandler.
func newAdminMemoriesHandler() *AdminMemoriesHandler {
return NewAdminMemoriesHandler()
}
// adminPost builds a POST /admin/memories/import request.
func adminPost(t *testing.T, h *AdminMemoriesHandler, body interface{}) *httptest.ResponseRecorder {
t.Helper()
b, _ := json.Marshal(body)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest("POST", "/admin/memories/import", bytes.NewReader(b))
c.Request.Header.Set("Content-Type", "application/json")
h.Import(c)
return w
}
// adminGet builds a GET /admin/memories/export request.
func adminGet(t *testing.T, h *AdminMemoriesHandler) *httptest.ResponseRecorder {
t.Helper()
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest("GET", "/admin/memories/export", nil)
h.Export(c)
return w
}
// ─────────────────────────────────────────────────────────────────────────────
// Export tests
// ─────────────────────────────────────────────────────────────────────────────
func TestAdminMemories_Export_Success(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
now := time.Now().UTC().Truncate(time.Second)
rows := sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"}).
AddRow("mem-1", "hello world", "LOCAL", "ws-1", now, "my-workspace").
AddRow("mem-2", "another fact", "TEAM", "ws-1", now, "my-workspace")
mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,").
WillReturnRows(rows)
w := adminGet(t, h)
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var memories []map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &memories); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if len(memories) != 2 {
t.Errorf("expected 2 memories, got %d", len(memories))
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestAdminMemories_Export_Empty(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
rows := sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"})
mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,").
WillReturnRows(rows)
w := adminGet(t, h)
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var memories []interface{}
if err := json.Unmarshal(w.Body.Bytes(), &memories); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if len(memories) != 0 {
t.Errorf("expected 0 memories, got %d", len(memories))
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestAdminMemories_Export_QueryError(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,").
WillReturnError(sql.ErrConnDone)
w := adminGet(t, h)
if w.Code != http.StatusInternalServerError {
t.Errorf("expected 500, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestAdminMemories_Export_RedactsSecrets(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
// Content with a secret pattern. Export must call redactSecrets and return
// the redacted form, not the raw credential.
secretContent := "Remember to use OPENAI_API_KEY=sk-1234567890abcdefgh for the model"
redacted, _ := redactSecrets("my-workspace", secretContent)
now := time.Now().UTC().Truncate(time.Second)
rows := sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"}).
AddRow("mem-secret", secretContent, "LOCAL", "my-workspace", now, "my-workspace")
mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,").
WillReturnRows(rows)
w := adminGet(t, h)
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var memories []map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &memories); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if len(memories) != 1 {
t.Fatalf("expected 1 memory, got %d", len(memories))
}
// The exported content must be the REDACTED version, not the raw secret.
if content, ok := memories[0]["content"].(string); ok {
if content == secretContent {
t.Errorf("Export returned raw secret %q — F1084 regression: redactSecrets not called", secretContent)
}
if content != redacted {
t.Errorf("Export content = %q, want redacted %q", content, redacted)
}
// Confirm the redacted version doesn't contain the raw key fragment.
if len(content) > 10 && content == "OPENAI_API_KEY=[REDACTED:" {
t.Errorf("redaction appears incomplete: %q", content)
}
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// ─────────────────────────────────────────────────────────────────────────────
// Import tests
// ─────────────────────────────────────────────────────────────────────────────
func TestAdminMemories_Import_Success(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
// Workspace lookup returns one row.
mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1").
WithArgs("my-workspace").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1"))
// Duplicate check returns false.
mock.ExpectQuery("SELECT EXISTS").
WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
// Insert succeeds. Handler uses 4-arg INSERT when created_at is absent.
mock.ExpectExec("INSERT INTO agent_memories").
WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL", "general").
WillReturnResult(sqlmock.NewResult(1, 1))
w := adminPost(t, h, []map[string]interface{}{
{
"content": "important fact",
"scope": "LOCAL",
"workspace_name": "my-workspace",
},
})
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["imported"].(float64) != 1 {
t.Errorf("imported = %v, want 1", resp["imported"])
}
if resp["skipped"].(float64) != 0 {
t.Errorf("skipped = %v, want 0", resp["skipped"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// Issue #1733: every legacy SQL-path test in this file was removed when
// the v1 fallback was deleted from AdminMemoriesHandler. The v2-plugin
// coverage (the only path now) lives in admin_memories_cutover_test.go:
//
// - TestExport_RoutesThroughPluginWhenCutoverActive
// - TestExport_DeduplicatesByMemoryID
// - TestExport_SkipsWorkspaceWhenResolverFails
// - TestExport_SkipsWorkspaceWhenPluginSearchFails
// - TestExport_WorkspacesQueryFails
// - TestExport_EmptyReadable
// - TestExport_RedactsSecretsInPluginPath
// - TestExport_BatchesPluginCallsByRoot
// - TestExport_IncludesEveryMembersPrivateNamespace
// - TestImport_RoutesThroughPluginWhenCutoverActive
// - TestImport_SkipsUnknownWorkspace
// - TestImport_PluginUpsertNamespaceError
// - TestImport_PluginCommitError
// - TestImport_RedactsBeforePluginSeesContent
// - TestImport_SkipsUnknownScope
// - TestImport_SkipsWhenResolverErrors
// - TestExport_503WhenPluginNotWired (new in A1)
// - TestImport_503WhenPluginNotWired (new in A1)
//
// Only the JSON-envelope rejection test stays here because it runs
// before the plugin gate.
// TestAdminMemories_Import_InvalidJSON verifies that a malformed
// payload is rejected with HTTP 400 before any plugin or DB call is
// attempted. This guards the request-decode path independent of the
// memory backend choice.
func TestAdminMemories_Import_InvalidJSON(t *testing.T) {
_ = setupTestDB(t)
h := newAdminMemoriesHandler()
h := NewAdminMemoriesHandler()
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
@@ -227,175 +53,3 @@ func TestAdminMemories_Import_InvalidJSON(t *testing.T) {
t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
}
}
func TestAdminMemories_Import_WorkspaceNotFound_SkipsEntry(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
// Workspace lookup returns no rows.
mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1").
WithArgs("ghost-workspace").
WillReturnError(sql.ErrNoRows)
w := adminPost(t, h, []map[string]interface{}{
{
"content": "some fact",
"scope": "LOCAL",
"workspace_name": "ghost-workspace",
},
})
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["skipped"].(float64) != 1 {
t.Errorf("skipped = %v, want 1 (workspace not found)", resp["skipped"])
}
if resp["imported"].(float64) != 0 {
t.Errorf("imported = %v, want 0", resp["imported"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestAdminMemories_Import_DuplicateSkipped(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
// Workspace lookup succeeds.
mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1").
WithArgs("my-workspace").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1"))
// Duplicate check returns true → entry is skipped.
mock.ExpectQuery("SELECT EXISTS").
WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
w := adminPost(t, h, []map[string]interface{}{
{
"content": "already stored fact",
"scope": "LOCAL",
"workspace_name": "my-workspace",
},
})
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["skipped"].(float64) != 1 {
t.Errorf("skipped = %v, want 1 (duplicate)", resp["skipped"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestAdminMemories_Import_RedactsSecretsBeforeDedup verifies F1085 (#1132):
// redactSecrets is called BEFORE the deduplication check so that two backups
// with the same original secret each get the same placeholder and dedup works.
// The DB dedup query must receive the REDACTED content, not the raw credential.
func TestAdminMemories_Import_RedactsSecretsBeforeDedup(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
rawContent := "the key is OPENAI_API_KEY=sk-1234567890abcdefgh"
redacted, changed := redactSecrets("my-workspace", rawContent)
if !changed {
t.Fatalf("precondition: redactSecrets must change the test content")
}
// Workspace lookup.
mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1").
WithArgs("my-workspace").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1"))
// Dedup check — the sqlmock must be set up for the REDACTED content,
// because Import calls redactSecrets before running the dedup query.
// If redactSecrets is not called, the mock would match on rawContent instead.
mock.ExpectQuery("SELECT EXISTS").
WithArgs("ws-uuid-1", redacted, "LOCAL").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
// Insert — receives the redacted content (not raw). Handler uses the
// 4-arg INSERT when created_at is absent from the payload.
mock.ExpectExec("INSERT INTO agent_memories").
WithArgs("ws-uuid-1", redacted, "LOCAL", "general").
WillReturnResult(sqlmock.NewResult(1, 1))
w := adminPost(t, h, []map[string]interface{}{
{
"content": rawContent,
"scope": "LOCAL",
"workspace_name": "my-workspace",
},
})
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["imported"].(float64) != 1 {
t.Errorf("imported = %v, want 1", resp["imported"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v (F1085 regression: redactSecrets not called before dedup)", err)
}
}
func TestAdminMemories_Import_PreservesCreatedAt(t *testing.T) {
mock := setupTestDB(t)
h := newAdminMemoriesHandler()
origTime := "2026-01-15T10:30:00Z"
// Workspace lookup.
mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1").
WithArgs("my-workspace").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1"))
// Dedup check.
mock.ExpectQuery("SELECT EXISTS").
WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
// Insert with created_at — must use the 5-arg INSERT.
mock.ExpectExec("INSERT INTO agent_memories").
WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL", "general", origTime).
WillReturnResult(sqlmock.NewResult(1, 1))
w := adminPost(t, h, []map[string]interface{}{
{
"content": "a fact",
"scope": "LOCAL",
"workspace_name": "my-workspace",
"created_at": origTime,
},
})
if w.Code != http.StatusOK {
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["imported"].(float64) != 1 {
t.Errorf("imported = %v, want 1", resp["imported"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
+39 -193
View File
@@ -485,63 +485,30 @@ func TestMCPHandler_ListPeers_ReturnsSiblings(t *testing.T) {
// tools/call — commit_memory
// ─────────────────────────────────────────────────────────────────────────────
func TestMCPHandler_CommitMemory_LocalScope_Success(t *testing.T) {
h, mock := newMCPHandler(t)
mock.ExpectExec("INSERT INTO agent_memories").
WithArgs(sqlmock.AnyArg(), "ws-1", "important fact", "LOCAL", "ws-1").
WillReturnResult(sqlmock.NewResult(1, 1))
w := mcpPost(t, h, "ws-1", map[string]interface{}{
"jsonrpc": "2.0",
"id": 9,
"method": "tools/call",
"params": map[string]interface{}{
"name": "commit_memory",
"arguments": map[string]interface{}{
"content": "important fact",
"scope": "LOCAL",
},
},
})
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp mcpResponse
json.Unmarshal(w.Body.Bytes(), &resp)
if resp.Error != nil {
t.Fatalf("unexpected error: %+v", resp.Error)
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestMCPHandler_CommitMemory_GlobalScope_Blocked_ScrubsInternalError verifies
// two contracts at once on the GLOBAL-scope-blocked path:
// Issue #1733: the legacy SQL success-path tests for commit_memory and
// recall_memory have been removed — the v2 plugin is the only backend
// and its success paths are covered by:
// - TestToolCommitMemory_RoutesThroughV2WhenWired (legacy-shim test)
// - TestToolRecallMemory_RoutesThroughV2WhenWired (legacy-shim test)
// - Every test in mcp_tools_memory_v2_test.go
// The unwired-path tests live in mcp_tools_memory_legacy_shim_test.go
// (TestToolCommitMemory_ErrorsWhenV2Unwired and its recall sibling).
//
// 1. C3 invariant (commit_memory with scope=GLOBAL aborts on the MCP bridge
// before touching the DB), AND
// 2. OFFSEC-001 / #259 scrub contract (commit 7d1a189f): the JSON-RPC error
// returned to the client is a CONSTANT — code=-32000, message="tool call
// failed" — with the production-internal err.Error() text logged
// server-side, never reflected back to the caller.
//
// Prior to this rename the test asserted that the client-visible message
// CONTAINED the substring "GLOBAL", which was the human-readable internal
// error from toolCommitMemory. mc#664 Class 2 flipped that assertion the
// right way around: now the test FAILS if the scrub regresses (i.e. if the
// internal string is ever reflected back to the wire), and PASSES iff the
// scrubbed constant reaches the client.
//
// Coupling note: the constant string "tool call failed" and the code -32000
// are the same values asserted by
// TestMCPHandler_dispatchRPC_UnknownTool_ReturnsConstantMessage — both are
// the OFFSEC-001 contract for the dispatch-failure branch in mcp.go (the
// third err.Error() leak that 7d1a189f scrubbed). If those constants ever
// change, both tests must move together.
func TestMCPHandler_CommitMemory_GlobalScope_Blocked_ScrubsInternalError(t *testing.T) {
// The two scope-blocked tests below remain because they validate the
// OFFSEC-001 JSON-RPC scrub layer (mcp.go dispatchRPC), which is
// orthogonal to the memory backend. After A1 the underlying error
// shifts from "GLOBAL scope is not permitted" to "memory plugin is
// not configured" — but the client-visible message stays "tool call
// failed", which is what the scrub assertion actually proves.
// TestMCPHandler_CommitMemory_RequestErrors_ScrubsInternalError verifies the
// OFFSEC-001 / #259 scrub contract on the commit_memory tool: any internal
// error gets replaced with the constant "tool call failed" + code -32000.
// Before A1 (issue #1733) this asserted on the GLOBAL-scope block; after
// A1 the underlying error is "memory plugin is not configured", which is
// equally not-leaked — proving the scrub layer is what's enforcing the
// contract, not the specific error string upstream.
func TestMCPHandler_CommitMemory_RequestErrors_ScrubsInternalError(t *testing.T) {
h, mock := newMCPHandler(t)
// No DB expectations — handler must abort before touching the DB (C3).
@@ -610,112 +577,22 @@ func TestMCPHandler_CommitMemory_GlobalScope_Blocked_ScrubsInternalError(t *test
}
}
// TestMCPHandler_CommitMemory_SecretInContent_IsRedactedBeforeInsert verifies
// the SAFE-T1201 (#838) fix on the MCP bridge path. PR #881 closed the HTTP
// handler but missed this one — an agent tool-call carrying plain-text
// credentials must have them scrubbed before the INSERT reaches the DB.
//
// The test asserts via the sqlmock `WithArgs` matcher that the content column
// binds the REDACTED form, not the raw input. sqlmock verifies the exact arg
// values, so a regression (removing the redactSecrets call) would fail with
// "argument mismatch" rather than silently persisting the secret.
func TestMCPHandler_CommitMemory_SecretInContent_IsRedactedBeforeInsert(t *testing.T) {
h, mock := newMCPHandler(t)
// Content with three distinct secret patterns covered by redactSecrets:
// - env-var assignment (ANTHROPIC_API_KEY=)
// - Bearer token
// - sk-… prefixed key
rawContent := "key=ANTHROPIC_API_KEY=sk-ant-xxxxxxxxxxxxxxxx auth=Bearer ghp_yyyyyyyyyyyyy note=sk-proj-zzzzzzzzzzzzzzzzzzzz"
// Derive what redactSecrets will produce so the sqlmock arg match is
// exact. This keeps the test brittle-on-purpose: if redactSecrets's
// output shape changes, this test must be re-derived, which surfaces
// the change during review.
expected, changed := redactSecrets("ws-1", rawContent)
if !changed {
t.Fatalf("precondition failed — redactSecrets must change the test content; got unchanged %q", expected)
}
if bytes.Contains([]byte(expected), []byte("sk-ant-xxxxxxxxxxxxxxxx")) {
t.Fatalf("precondition failed — redacted content still contains raw secret: %s", expected)
}
mock.ExpectExec("INSERT INTO agent_memories").
WithArgs(sqlmock.AnyArg(), "ws-1", expected, "LOCAL", "ws-1").
WillReturnResult(sqlmock.NewResult(1, 1))
w := mcpPost(t, h, "ws-1", map[string]interface{}{
"jsonrpc": "2.0",
"id": 99,
"method": "tools/call",
"params": map[string]interface{}{
"name": "commit_memory",
"arguments": map[string]interface{}{
"content": rawContent,
"scope": "LOCAL",
},
},
})
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp mcpResponse
json.Unmarshal(w.Body.Bytes(), &resp)
if resp.Error != nil {
t.Fatalf("unexpected JSON-RPC error: %+v", resp.Error)
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("sqlmock mismatch — content was NOT redacted before insert: %v", err)
}
}
// TestMCPHandler_CommitMemory_CleanContent_PassesThrough confirms that the
// redactor is a no-op on content with no credentials — a regression where
// redactSecrets corrupted benign content would be a user-visible bug.
func TestMCPHandler_CommitMemory_CleanContent_PassesThrough(t *testing.T) {
h, mock := newMCPHandler(t)
cleanContent := "the quick brown fox jumps over the lazy dog — no secrets here"
// Bind the exact string — no wildcards — so that any transformation
// (whitespace, case, truncation) would fail the arg match.
mock.ExpectExec("INSERT INTO agent_memories").
WithArgs(sqlmock.AnyArg(), "ws-1", cleanContent, "TEAM", "ws-1").
WillReturnResult(sqlmock.NewResult(1, 1))
w := mcpPost(t, h, "ws-1", map[string]interface{}{
"jsonrpc": "2.0",
"id": 100,
"method": "tools/call",
"params": map[string]interface{}{
"name": "commit_memory",
"arguments": map[string]interface{}{
"content": cleanContent,
"scope": "TEAM",
},
},
})
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("clean content should pass through unchanged: %v", err)
}
}
// Issue #1733: the legacy SQL-path redaction tests for commit_memory
// (SecretInContent_IsRedactedBeforeInsert, CleanContent_PassesThrough)
// have been removed. The v2 plugin path performs the same redaction
// (mcp_tools_memory_v2.go:122 + :242); its coverage lives in
// mcp_tools_memory_v2_test.go.
// ─────────────────────────────────────────────────────────────────────────────
// tools/call — recall_memory
// ─────────────────────────────────────────────────────────────────────────────
// TestMCPHandler_RecallMemory_GlobalScope_Blocked_ScrubsInternalError verifies
// C3 (GLOBAL scope blocked on MCP bridge) is enforced and that the OFFSEC-001
// scrub contract applies: the client-visible error.message is the constant
// "tool call failed", NOT the descriptive internal reason. The internal reason
// ("GLOBAL scope is not permitted via the MCP bridge") is logged server-side
// but must never reach the wire.
func TestMCPHandler_RecallMemory_GlobalScope_Blocked_ScrubsInternalError(t *testing.T) {
// TestMCPHandler_RecallMemory_RequestErrors_ScrubsInternalError mirrors the
// commit_memory scrub test on the recall_memory path. After A1 (issue
// #1733) the upstream error is "memory plugin is not configured" rather
// than the GLOBAL-scope block, but the scrub layer (mcp.go dispatchRPC)
// still must replace it with the constant "tool call failed".
func TestMCPHandler_RecallMemory_RequestErrors_ScrubsInternalError(t *testing.T) {
h, mock := newMCPHandler(t)
// No DB expectations — handler must abort before touching the DB.
@@ -770,42 +647,11 @@ func TestMCPHandler_RecallMemory_GlobalScope_Blocked_ScrubsInternalError(t *test
}
}
func TestMCPHandler_RecallMemory_LocalScope_Empty(t *testing.T) {
h, mock := newMCPHandler(t)
mock.ExpectQuery("SELECT id, content, scope, created_at").
WithArgs("ws-1", "").
WillReturnRows(sqlmock.NewRows([]string{"id", "content", "scope", "created_at"}))
w := mcpPost(t, h, "ws-1", map[string]interface{}{
"jsonrpc": "2.0",
"id": 12,
"method": "tools/call",
"params": map[string]interface{}{
"name": "recall_memory",
"arguments": map[string]interface{}{
"query": "",
"scope": "LOCAL",
},
},
})
var resp mcpResponse
json.Unmarshal(w.Body.Bytes(), &resp)
if resp.Error != nil {
t.Fatalf("unexpected error: %+v", resp.Error)
}
result, _ := resp.Result.(map[string]interface{})
content, _ := result["content"].([]interface{})
item, _ := content[0].(map[string]interface{})
text, _ := item["text"].(string)
if text != "No memories found." {
t.Errorf("expected 'No memories found.', got %q", text)
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// Issue #1733: TestMCPHandler_RecallMemory_LocalScope_Empty removed —
// it asserted on the legacy SQL SELECT path. The v2 empty-result
// rendering is covered by TestToolRecallMemory_RoutesThroughV2WhenWired
// (mcp_tools_memory_legacy_shim_test.go) which uses a stub plugin that
// returns an empty SearchResponse.
// ─────────────────────────────────────────────────────────────────────────────
// tools/call — send_message_to_user
+13 -116
View File
@@ -363,127 +363,24 @@ 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)
// Issue #1733 — v2 memory plugin is now the only path. The legacy
// SQL fallback on `agent_memories` is gone; an unconfigured plugin
// returns a clear error to the agent rather than silently writing
// into a stale table no one reads.
if err := h.memoryV2Available(); err != nil {
return "", err
}
content, _ := args["content"].(string)
scope, _ := args["scope"].(string)
if content == "" {
return "", fmt.Errorf("content is required")
}
if scope == "" {
scope = "LOCAL"
}
// C3: GLOBAL scope is blocked on the MCP bridge.
if scope == "GLOBAL" {
return "", fmt.Errorf("GLOBAL scope is not permitted via the MCP bridge — use LOCAL or TEAM")
}
if scope != "LOCAL" && scope != "TEAM" {
return "", fmt.Errorf("scope must be LOCAL or TEAM")
}
memoryID := uuid.New().String()
// SAFE-T1201 (#838): scrub known credential patterns before persistence so
// plain-text API keys pulled in via tool responses can't land in the
// memories table (and leak into shared TEAM scope). Reuses redactSecrets
// already shipped for the HTTP path in PR #881 — this was the MCP-bridge
// sibling the original fix missed. Runs on every write regardless of scope.
content, _ = redactSecrets(workspaceID, content)
_, err := h.database.ExecContext(ctx, `
INSERT INTO agent_memories (id, workspace_id, content, scope, namespace)
VALUES ($1, $2, $3, $4, $5)
`, memoryID, workspaceID, content, scope, workspaceID)
if err != nil {
log.Printf("MCPHandler.commit_memory workspace=%s: %v", workspaceID, err)
return "", fmt.Errorf("failed to save memory")
}
return fmt.Sprintf(`{"id":%q,"scope":%q}`, memoryID, scope), nil
return h.commitMemoryLegacyShim(ctx, workspaceID, args)
}
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)
// Issue #1733 — v2 memory plugin is now the only path. Same shape
// as toolCommitMemory: an unconfigured plugin is an error, not a
// quiet read from a frozen v1 table.
if err := h.memoryV2Available(); err != nil {
return "", err
}
query, _ := args["query"].(string)
scope, _ := args["scope"].(string)
// C3: GLOBAL scope is blocked on the MCP bridge.
if scope == "GLOBAL" {
return "", fmt.Errorf("GLOBAL scope is not permitted via the MCP bridge — use LOCAL, TEAM, or empty")
}
var rows *sql.Rows
var err error
switch scope {
case "LOCAL":
rows, err = h.database.QueryContext(ctx, `
SELECT id, content, scope, created_at
FROM agent_memories
WHERE workspace_id = $1 AND scope = 'LOCAL'
AND ($2 = '' OR content ILIKE '%' || $2 || '%')
ORDER BY created_at DESC LIMIT 50
`, workspaceID, query)
case "TEAM":
// Team scope: parent + all siblings.
rows, err = h.database.QueryContext(ctx, `
SELECT m.id, m.content, m.scope, m.created_at
FROM agent_memories m
JOIN workspaces w ON w.id = m.workspace_id
WHERE m.scope = 'TEAM'
AND w.status != 'removed'
AND (w.id = $1 OR w.parent_id = (SELECT parent_id FROM workspaces WHERE id = $1 AND parent_id IS NOT NULL))
AND ($2 = '' OR m.content ILIKE '%' || $2 || '%')
ORDER BY m.created_at DESC LIMIT 50
`, workspaceID, query)
default:
// Empty scope → LOCAL only for the MCP bridge (GLOBAL excluded per C3).
rows, err = h.database.QueryContext(ctx, `
SELECT id, content, scope, created_at
FROM agent_memories
WHERE workspace_id = $1 AND scope IN ('LOCAL', 'TEAM')
AND ($2 = '' OR content ILIKE '%' || $2 || '%')
ORDER BY created_at DESC LIMIT 50
`, workspaceID, query)
}
if err != nil {
return "", fmt.Errorf("memory search failed: %w", err)
}
defer rows.Close()
type memEntry struct {
ID string `json:"id"`
Content string `json:"content"`
Scope string `json:"scope"`
CreatedAt string `json:"created_at"`
}
var results []memEntry
for rows.Next() {
var e memEntry
if err := rows.Scan(&e.ID, &e.Content, &e.Scope, &e.CreatedAt); err != nil {
continue
}
results = append(results, e)
}
if err := rows.Err(); err != nil {
return "", fmt.Errorf("memory scan error: %w", err)
}
if len(results) == 0 {
return "No memories found.", nil
}
b, _ := json.MarshalIndent(results, "", " ")
return string(b), nil
return h.recallMemoryLegacyShim(ctx, workspaceID, args)
}
// ─────────────────────────────────────────────────────────────────────────────
@@ -2,14 +2,13 @@ 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.
// (namespace-based).
//
// 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).
// Issue #1733: v2 is now the only memory backend. Callers in
// mcp_tools.go MUST verify h.memv2 is wired before invoking these
// helpers (toolCommitMemory / toolRecallMemory both check
// memoryV2Available and short-circuit with an error when not wired).
// The previous "fall through to direct SQL" branch is gone.
//
// Translation:
// commit: LOCAL → workspace:<self>
@@ -512,41 +512,38 @@ func TestToolRecallMemory_RoutesThroughV2WhenWired(t *testing.T) {
}
}
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()
// Issue #1733: v2 is the only path; commit/recall return a clear error
// (not a silent SQL fallback) when MEMORY_PLUGIN_URL is unset.
func TestToolCommitMemory_ErrorsWhenV2Unwired(t *testing.T) {
db, _, _ := sqlmock.New()
defer db.Close()
mock.ExpectExec("INSERT INTO agent_memories").
WillReturnResult(sqlmock.NewResult(0, 1))
h := &MCPHandler{database: db}
h := &MCPHandler{database: db} // no withMemoryV2APIs → memv2 nil
_, err := h.toolCommitMemory(context.Background(), "root-1", map[string]interface{}{
"content": "x",
"scope": "LOCAL",
})
if err != nil {
t.Fatalf("err: %v", err)
if err == nil {
t.Fatal("expected error when v2 unwired, got nil")
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("legacy SQL path not exercised: %v", err)
if !strings.Contains(err.Error(), "MEMORY_PLUGIN_URL") {
t.Errorf("error must hint at MEMORY_PLUGIN_URL: %v", err)
}
}
func TestToolRecallMemory_FallsThroughToLegacyWhenV2Unwired(t *testing.T) {
db, mock, _ := sqlmock.New()
func TestToolRecallMemory_ErrorsWhenV2Unwired(t *testing.T) {
db, _, _ := 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 == nil {
t.Fatal("expected error when v2 unwired, got nil")
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("legacy SQL path not exercised: %v", err)
if !strings.Contains(err.Error(), "MEMORY_PLUGIN_URL") {
t.Errorf("error must hint at MEMORY_PLUGIN_URL: %v", err)
}
}
@@ -39,41 +39,30 @@ type Bundle struct {
// Build returns a wired Bundle if MEMORY_PLUGIN_URL is set, else nil.
//
// It probes /v1/health at boot — when the plugin is unreachable, we
// log a warning but STILL return the bundle. The MCP layer's
// circuit breaker handles ongoing unavailability; we don't want to
// block workspace-server boot just because the memory plugin is
// briefly down.
// log a warning but STILL return the bundle. The MCP layer's circuit
// breaker handles ongoing unavailability.
//
// Silent-misconfig guard: if MEMORY_V2_CUTOVER=true is set without
// MEMORY_PLUGIN_URL, the cutoverActive() check in handlers silently
// returns false and the legacy SQL path serves every request. The
// operator sees no errors, no warnings, and assumes the cutover is
// live. Log a LOUD WARN at boot when the env is half-configured so
// the misconfig is visible in the boot log, not detectable only by
// observing that the legacy table is still being written to.
// Issue #1733: when MEMORY_PLUGIN_URL is unset the bundle is nil and
// every memory MCP tool returns a clear "plugin not configured" error
// (mcp_tools.go). There is no longer a silent SQL fallback to
// agent_memories, so the previous half-configured-cutover guard is
// gone — a missing URL fails loudly on first memory call instead of
// quietly serving stale legacy data.
//
// MEMORY_V2_CUTOVER is left intact as a deployment marker (CP user-data
// reads it before spawning the sidecar in entrypoint-tenant.sh); we no
// longer branch on it inside the platform binary.
func Build(db *sql.DB) *Bundle {
cutover := os.Getenv("MEMORY_V2_CUTOVER") == "true"
pluginURL := os.Getenv("MEMORY_PLUGIN_URL")
if pluginURL == "" {
if cutover {
log.Printf("memory-plugin: ⚠️ MEMORY_V2_CUTOVER=true but MEMORY_PLUGIN_URL is unset — cutover is INACTIVE, legacy SQL path is serving every request. Either unset MEMORY_V2_CUTOVER or point MEMORY_PLUGIN_URL at a reachable plugin server.")
}
log.Printf("memory-plugin: MEMORY_PLUGIN_URL is unset — v2 memory MCP tools (commit_memory, recall_memory, admin export/import) will return 'plugin not configured' to callers. Set MEMORY_PLUGIN_URL to activate.")
return nil
}
plugin := mclient.New(mclient.Config{})
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if hr, err := plugin.Boot(ctx); err != nil {
// Log even louder when cutover is on — an unreachable plugin
// during cutover means writes that the operator THINKS are
// going to v2 will silently fall back to legacy via the
// circuit breaker on each request. Make it impossible to miss.
if cutover {
log.Printf("memory-plugin: ⚠️ MEMORY_V2_CUTOVER=true and MEMORY_PLUGIN_URL=%s but /v1/health probe failed (%v). Cutover writes will fall back to legacy via circuit breaker. Verify the plugin server is reachable.", pluginURL, err)
} else {
log.Printf("memory-plugin: /v1/health probe failed (will retry per-request): %v", err)
}
log.Printf("memory-plugin: ⚠️ /v1/health probe failed at boot (%v). MCP memory calls will error until the plugin becomes reachable. Verify MEMORY_PLUGIN_URL=%s.", err, pluginURL)
} else {
log.Printf("memory-plugin: ok, capabilities=%v", hr.Capabilities)
}
@@ -166,69 +166,43 @@ func captureLogs(t *testing.T, fn func()) string {
return buf.String()
}
// TestBuild_WarnsWhenCutoverWithoutPluginURL pins the silent-misconfig
// guard: an operator who flips MEMORY_V2_CUTOVER=true without also
// pointing MEMORY_PLUGIN_URL at a plugin server has just disabled the
// cutover with no error visible. Without this WARN, the only signal
// is "the legacy table is still being written to" — invisible to
// every operator who doesn't explicitly check.
func TestBuild_WarnsWhenCutoverWithoutPluginURL(t *testing.T) {
t.Setenv("MEMORY_V2_CUTOVER", "true")
// Issue #1733: the old "cutover-without-URL" and "cutover-with-failing-
// probe" loud-warning tests are gone — workspace-server no longer
// branches on MEMORY_V2_CUTOVER (v2 is unconditional now), so the
// half-configured-cutover failure mode they guarded against can no
// longer occur. The two surviving tests below pin the new shape:
// one log line when URL is unset, one when the boot-time probe fails.
// TestBuild_LogsWhenURLUnset confirms the operator-visible boot log
// line that fires when MEMORY_PLUGIN_URL is unset — every memory MCP
// tool will then return "plugin not configured" to callers.
func TestBuild_LogsWhenURLUnset(t *testing.T) {
t.Setenv("MEMORY_PLUGIN_URL", "")
out := captureLogs(t, func() {
if got := Build(nil); got != nil {
t.Errorf("expected nil bundle, got %+v", got)
}
})
if !strings.Contains(out, "MEMORY_V2_CUTOVER=true") || !strings.Contains(out, "MEMORY_PLUGIN_URL is unset") {
t.Errorf("expected loud WARN about half-configured cutover; got log:\n%s", out)
if !strings.Contains(out, "MEMORY_PLUGIN_URL is unset") {
t.Errorf("expected boot log to mention MEMORY_PLUGIN_URL is unset; got:\n%s", out)
}
}
// TestBuild_NoWarnWhenNeitherSet pins the happy default: an operator
// running without the v2 plugin should not see scary warnings.
func TestBuild_NoWarnWhenNeitherSet(t *testing.T) {
t.Setenv("MEMORY_V2_CUTOVER", "")
t.Setenv("MEMORY_PLUGIN_URL", "")
out := captureLogs(t, func() { _ = Build(nil) })
if strings.Contains(out, "MEMORY_V2_CUTOVER") {
t.Errorf("expected no MEMORY_V2_CUTOVER warning when env is unset; got log:\n%s", out)
}
}
// TestBuild_LoudWarnWhenCutoverAndProbeFails pins the second
// half-config case: cutover is on AND plugin URL is set, but the
// /v1/health probe fails (server down or wrong URL). Without this
// loud WARN, the operator sees only the generic "probe failed" line
// that gets emitted even when cutover is OFF — hiding the fact that
// real cutover writes will quietly fall back via circuit breaker.
func TestBuild_LoudWarnWhenCutoverAndProbeFails(t *testing.T) {
t.Setenv("MEMORY_V2_CUTOVER", "true")
// TestBuild_LogsWhenProbeFails confirms the boot log line that fires
// when MEMORY_PLUGIN_URL is set but the plugin is unreachable. The
// bundle is still returned (per the comment on Build) so the platform
// keeps booting — MCP calls error per-request until the plugin recovers.
func TestBuild_LogsWhenProbeFails(t *testing.T) {
t.Setenv("MEMORY_PLUGIN_URL", "http://127.0.0.1:1") // bogus port
db, _, _ := sqlmock.New()
defer db.Close()
out := captureLogs(t, func() { _ = Build(db) })
if !strings.Contains(out, "MEMORY_V2_CUTOVER=true") || !strings.Contains(out, "probe failed") {
t.Errorf("expected loud WARN about cutover-with-failing-probe; got log:\n%s", out)
}
}
// TestBuild_QuietProbeFailWhenCutoverOff: the operator is in PRE-cutover
// mode (plugin URL set, cutover off — they're warming up the plugin).
// A failing probe in this state is not a misconfig — it should log the
// generic message, NOT the loud cutover-specific one (so log noise
// doesn't drown out real cutover misconfigs in dashboards).
func TestBuild_QuietProbeFailWhenCutoverOff(t *testing.T) {
t.Setenv("MEMORY_V2_CUTOVER", "")
t.Setenv("MEMORY_PLUGIN_URL", "http://127.0.0.1:1")
db, _, _ := sqlmock.New()
defer db.Close()
out := captureLogs(t, func() { _ = Build(db) })
if strings.Contains(out, "MEMORY_V2_CUTOVER=true") {
t.Errorf("expected no cutover-specific warning when cutover is off; got log:\n%s", out)
}
if !strings.Contains(out, "probe failed") {
t.Errorf("expected generic probe-failed log; got log:\n%s", out)
out := captureLogs(t, func() {
if got := Build(db); got == nil {
t.Error("expected non-nil bundle when URL is set, got nil")
}
})
if !strings.Contains(out, "/v1/health probe failed") {
t.Errorf("expected boot log to mention probe failure; got:\n%s", out)
}
}