diff --git a/workspace-server/cmd/memory-plugin-postgres/E2E.md b/workspace-server/cmd/memory-plugin-postgres/E2E.md new file mode 100644 index 00000000..23ad9d07 --- /dev/null +++ b/workspace-server/cmd/memory-plugin-postgres/E2E.md @@ -0,0 +1,68 @@ +# Real-subprocess E2E for memory-plugin-postgres + +The default `go test ./...` suite covers the plugin via in-process +sqlmock tests (PR-3). This directory ALSO ships build-tag-gated tests +that spawn the real binary against a live postgres — to catch +classes of bug in-process tests can't see: + +- Boot-path regressions (env var typos, panic-on-startup) +- Wire-format bugs sqlmock smooths over (the `pq.Array` issue we + hit during PR-3 development) +- HTTP/socket encoding edge cases +- C1 idempotency (real upsert against real postgres) + +## Running + +The tests skip silently unless an operator opts in with both: +- The `memory_plugin_e2e` build tag +- `MEMORY_PLUGIN_E2E_DB` env var pointing at a writable postgres + +### Quick local run (with docker) + +```bash +docker run --rm -d --name memory-plugin-e2e-pg \ + -e POSTGRES_PASSWORD=test -e POSTGRES_USER=test -e POSTGRES_DB=test \ + -p 5432:5432 \ + pgvector/pgvector:pg16 + +# Wait a few seconds for postgres to accept connections +until docker exec memory-plugin-e2e-pg pg_isready -U test >/dev/null 2>&1; do sleep 0.5; done + +MEMORY_PLUGIN_E2E_DB=postgres://test:test@localhost:5432/test?sslmode=disable \ + go test -tags memory_plugin_e2e -v -count=1 ./cmd/memory-plugin-postgres/ + +docker stop memory-plugin-e2e-pg +``` + +### CI integration + +These tests are NOT in the default required-checks set. Operators +gating cutover on the suite should add a separate workflow step: + +```yaml +- name: Memory plugin E2E + if: ${{ contains(github.event.pull_request.labels.*.name, 'memory-v2') }} + run: | + MEMORY_PLUGIN_E2E_DB=${{ secrets.MEMORY_PLUGIN_TEST_DSN }} \ + go test -tags memory_plugin_e2e -v -count=1 ./cmd/memory-plugin-postgres/ +``` + +## What each test pins + +| Test | Covers | +|---|---| +| `TestE2E_BootAndHealth` | Binary builds, starts, advertises all 5 capabilities | +| `TestE2E_FullCommitSearchForgetRoundTrip` | Real wire encoding (no sqlmock), full agent flow | +| `TestE2E_IdempotencyKey` | C1 fix end-to-end — upserts against real postgres | + +## What's still NOT covered + +- Migration drift (assumes the migrations dir is at the conventional + path; operator-customized layouts need their own test) +- Plugin-internal recovery (kill backing store mid-request, etc.) +- Concurrent commits with id collisions across processes +- TTL eviction (would need to extend test runtime past `expires_at`) + +These gaps apply equally to forks of this binary; they're listed in +[`testing-your-plugin.md`](../../../docs/memory-plugins/testing-your-plugin.md) +under "what the harness does NOT cover". diff --git a/workspace-server/cmd/memory-plugin-postgres/boot_e2e_test.go b/workspace-server/cmd/memory-plugin-postgres/boot_e2e_test.go new file mode 100644 index 00000000..b8b76543 --- /dev/null +++ b/workspace-server/cmd/memory-plugin-postgres/boot_e2e_test.go @@ -0,0 +1,289 @@ +//go:build memory_plugin_e2e + +// Package main's real-subprocess boot test (#293 fixup, RFC #2728). +// +// Build-tag gated so it only runs when an operator explicitly opts in: +// +// MEMORY_PLUGIN_E2E_DB=postgres://test:test@localhost:5432/test?sslmode=disable \ +// go test -tags memory_plugin_e2e -v ./cmd/memory-plugin-postgres/ +// +// Why a separate build tag: +// - The default `go test ./...` run shouldn't require docker or a +// live postgres +// - CI gates that DO want to run this can set the env var + tag +// - Operators verifying a custom plugin against the contract can +// copy this file as the template (replace the binary build step +// with their own) +// +// What this exercises that PR-11's swap test doesn't: +// - Real `go build` of cmd/memory-plugin-postgres/ +// - Real binary boot via os/exec — catches mixed-key panics, missing +// env vars, crash-on-startup issues that in-process tests skip +// - Real postgres connection — catches wire-format bugs (e.g. the +// pq.Array regression we hit during PR-3) +// - Real HTTP round-trip with a TCP socket — catches encoding edge +// cases sqlmock + httptest can't see +// +// What this does NOT cover: +// - Schema migration drift (assumes the migrations dir is at the +// conventional path; operator-customized layouts need their own +// test) +// - Plugin-internal recovery (kill backing store mid-request, etc.) + +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + "time" + + mclient "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/client" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" +) + +const ( + bootProbeTimeout = 30 * time.Second + bootProbeStep = 500 * time.Millisecond +) + +// requireE2EDB returns the test DSN. Skips the test (not fails) when +// the env var is unset — keeps `-tags memory_plugin_e2e` runs from +// crashing on dev machines without postgres. +func requireE2EDB(t *testing.T) string { + t.Helper() + dsn := os.Getenv("MEMORY_PLUGIN_E2E_DB") + if dsn == "" { + t.Skip("MEMORY_PLUGIN_E2E_DB not set — skipping real-subprocess boot test") + } + return dsn +} + +// buildBinary compiles cmd/memory-plugin-postgres/ to a temp dir. +// Returns the path of the built binary. Test cleanup deletes it. +func buildBinary(t *testing.T) string { + t.Helper() + dir := t.TempDir() + out := filepath.Join(dir, "memory-plugin-postgres") + if runtime.GOOS == "windows" { + out += ".exe" + } + // Find the cmd dir relative to this file. + _, thisFile, _, _ := runtime.Caller(0) + cmdDir := filepath.Dir(thisFile) + build := exec.Command("go", "build", "-o", out, ".") + build.Dir = cmdDir + build.Env = os.Environ() + if outErr, err := build.CombinedOutput(); err != nil { + t.Fatalf("go build failed: %v\n%s", err, outErr) + } + return out +} + +// startBinary launches the built binary with the supplied env. Returns +// the *exec.Cmd (test cleanup kills it) and the http URL it's listening +// on. Polls /v1/health until ready or times out. +func startBinary(t *testing.T, binary, dsn, listen string) (*exec.Cmd, string) { + t.Helper() + url := "http://" + listen + cmd := exec.Command(binary) + cmd.Env = append(os.Environ(), + "MEMORY_PLUGIN_DATABASE_URL="+dsn, + "MEMORY_PLUGIN_LISTEN_ADDR="+listen, + // Migrations dir lives next to the cmd source. The binary + // reads it relative to cwd by default; we set the env var + // override so the test doesn't depend on cwd. + "MEMORY_PLUGIN_MIGRATIONS_DIR="+migrationsDirForTest(t), + ) + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + cmd.Stdout = stdout + cmd.Stderr = stderr + if err := cmd.Start(); err != nil { + t.Fatalf("start binary: %v", err) + } + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + _ = cmd.Wait() + } + if t.Failed() { + t.Logf("binary stdout:\n%s", stdout.String()) + t.Logf("binary stderr:\n%s", stderr.String()) + } + }) + + deadline := time.Now().Add(bootProbeTimeout) + for time.Now().Before(deadline) { + resp, err := http.Get(url + "/v1/health") + if err == nil { + _ = resp.Body.Close() + if resp.StatusCode == 200 { + return cmd, url + } + } + // Bail early if the binary already exited. + if cmd.ProcessState != nil && cmd.ProcessState.Exited() { + t.Fatalf("binary exited during boot: stderr:\n%s", stderr.String()) + } + time.Sleep(bootProbeStep) + } + t.Fatalf("binary did not become ready within %v", bootProbeTimeout) + return nil, "" +} + +func migrationsDirForTest(t *testing.T) string { + t.Helper() + _, thisFile, _, _ := runtime.Caller(0) + return filepath.Join(filepath.Dir(thisFile), "migrations") +} + +// TestE2E_BootAndHealth: build + start the real binary, hit /v1/health, +// confirm capabilities match what the built-in plugin declares. Catches +// "binary doesn't start" / "wrong env var name" / "panics on first +// request" classes that in-process tests miss. +func TestE2E_BootAndHealth(t *testing.T) { + dsn := requireE2EDB(t) + binary := buildBinary(t) + _, url := startBinary(t, binary, dsn, "127.0.0.1:19100") + cl := mclient.New(mclient.Config{BaseURL: url}) + + hr, err := cl.Boot(context.Background()) + if err != nil { + t.Fatalf("Boot: %v", err) + } + if hr.Status != "ok" { + t.Errorf("status = %q", hr.Status) + } + wantCaps := map[string]bool{"fts": true, "embedding": true, "ttl": true, "pin": true, "propagation": true} + gotCaps := map[string]bool{} + for _, c := range hr.Capabilities { + gotCaps[c] = true + } + for c := range wantCaps { + if !gotCaps[c] { + t.Errorf("capability %q missing — built-in plugin should declare all 5", c) + } + } +} + +// TestE2E_FullCommitSearchForgetRoundTrip: the full agent flow against +// real postgres + real HTTP. Catches wire-format regressions (the +// pq.Array bug we hit during PR-3 development) and contract-level +// drift between Go bindings and the spec. +func TestE2E_FullCommitSearchForgetRoundTrip(t *testing.T) { + dsn := requireE2EDB(t) + binary := buildBinary(t) + _, url := startBinary(t, binary, dsn, "127.0.0.1:19101") + cl := mclient.New(mclient.Config{BaseURL: url}) + + ctx := context.Background() + ns := fmt.Sprintf("workspace:e2e-%d", time.Now().UnixNano()) + + // 1. Upsert namespace. + if _, err := cl.UpsertNamespace(ctx, ns, contract.NamespaceUpsert{Kind: contract.NamespaceKindWorkspace}); err != nil { + t.Fatalf("UpsertNamespace: %v", err) + } + t.Cleanup(func() { _ = cl.DeleteNamespace(context.Background(), ns) }) + + // 2. Commit a memory. + resp, err := cl.CommitMemory(ctx, ns, contract.MemoryWrite{ + Content: "user prefers tabs over spaces", + Kind: contract.MemoryKindFact, + Source: contract.MemorySourceAgent, + }) + if err != nil { + t.Fatalf("CommitMemory: %v", err) + } + if resp.ID == "" { + t.Fatal("plugin returned empty memory id") + } + + // 3. Search and find the memory we just wrote. + sresp, err := cl.Search(ctx, contract.SearchRequest{Namespaces: []string{ns}, Query: "tabs"}) + if err != nil { + t.Fatalf("Search: %v", err) + } + if len(sresp.Memories) == 0 { + t.Errorf("Search returned 0 memories, want at least 1") + } + found := false + for _, m := range sresp.Memories { + if m.ID == resp.ID && m.Content == "user prefers tabs over spaces" { + found = true + break + } + } + if !found { + got, _ := json.Marshal(sresp.Memories) + t.Errorf("committed memory not found in search results: %s", got) + } + + // 4. Forget the memory. + if err := cl.ForgetMemory(ctx, resp.ID, contract.ForgetRequest{RequestedByNamespace: ns}); err != nil { + t.Fatalf("ForgetMemory: %v", err) + } + + // 5. Search again — gone. + sresp, err = cl.Search(ctx, contract.SearchRequest{Namespaces: []string{ns}, Query: "tabs"}) + if err != nil { + t.Fatalf("Search after forget: %v", err) + } + for _, m := range sresp.Memories { + if m.ID == resp.ID { + t.Errorf("forgotten memory still in search results") + } + } +} + +// TestE2E_IdempotencyKey covers the C1 fix end-to-end: same id passed +// twice should upsert (one row, updated content), not duplicate. +func TestE2E_IdempotencyKey(t *testing.T) { + dsn := requireE2EDB(t) + binary := buildBinary(t) + _, url := startBinary(t, binary, dsn, "127.0.0.1:19102") + cl := mclient.New(mclient.Config{BaseURL: url}) + + ctx := context.Background() + ns := fmt.Sprintf("workspace:e2e-idem-%d", time.Now().UnixNano()) + if _, err := cl.UpsertNamespace(ctx, ns, contract.NamespaceUpsert{Kind: contract.NamespaceKindWorkspace}); err != nil { + t.Fatalf("UpsertNamespace: %v", err) + } + t.Cleanup(func() { _ = cl.DeleteNamespace(context.Background(), ns) }) + + fixedID := "11111111-2222-3333-4444-555555555555" + for i, content := range []string{"first version", "second version (updated)"} { + if _, err := cl.CommitMemory(ctx, ns, contract.MemoryWrite{ + ID: fixedID, + Content: content, + Kind: contract.MemoryKindFact, + Source: contract.MemorySourceAgent, + }); err != nil { + t.Fatalf("commit %d: %v", i, err) + } + } + + sresp, err := cl.Search(ctx, contract.SearchRequest{Namespaces: []string{ns}}) + if err != nil { + t.Fatalf("Search: %v", err) + } + matches := 0 + for _, m := range sresp.Memories { + if m.ID == fixedID { + matches++ + if m.Content != "second version (updated)" { + t.Errorf("upsert did not update content: got %q", m.Content) + } + } + } + if matches != 1 { + t.Errorf("upsert produced %d rows for id=%s, want 1", matches, fixedID) + } +}