Changeset 0.28.0.8 (#180)

2026-03-12 22:49:35 +00:00
parent f3f5afd14c
commit aa870f1040
14 changed files with 679 additions and 116 deletions
--- a/docs/ICD/memory.md
+++ b/docs/ICD/memory.md
@@ -4,45 +4,92 @@
 identifies facts, preferences, and context about the user and stores
 them for injection into future conversations.

+**Auth:** All user endpoints require authenticated session.
+Admin endpoints require platform admin role.
+
+---
+
 ### User Memory

 ```
 GET    /memories                    → { "data": [memory objects] }
-GET    /memories/count              → { "count": 42 }
-PUT    /memories/:id                ← { "content": "updated text" }
+GET    /memories/count              → { "active": 5, "pending": 2 }
+PUT    /memories/:id                ← { "key": "...", "value": "...", "confidence": 0.9 }
 DELETE /memories/:id
-POST   /memories/:id/approve        → approve a pending memory
-POST   /memories/:id/reject         → reject a pending memory
+POST   /memories/:id/approve        → approve a pending_review memory
+POST   /memories/:id/reject         → reject (archive) a pending_review memory
 ```

+Query params for `GET /memories`:
+
+| Param | Default | Description |
+|-------|---------|-------------|
+| `status` | `active` | Filter by status: `active`, `pending_review`, `archived` |
+| `query` | — | Keyword filter on key+value (PG: full-text, SQLite: LIKE) |
+
 Memory object:

 ```json
 {
  "id": "uuid",
-  "user_id": "uuid",
-  "persona_id": "uuid|null",
-  "scope": "user|persona",
-  "content": "User prefers Go for backend work",
-  "source_channel_id": "uuid",
-  "status": "pending|approved|rejected",
+  "scope": "user|persona|persona_user",
+  "owner_id": "uuid",
+  "user_id": "uuid|null",
+  "key": "preferred language",
+  "value": "Go with Gin framework for backend services",
+  "source_channel_id": "uuid|null",
  "confidence": 0.85,
+  "status": "active|pending_review|archived",
  "created_at": "...",
  "updated_at": "..."
 }
 ```

-`scope`: `user` memories apply across all conversations. `persona`
-memories are specific to interactions with that Persona.
+**Scopes:**
+
+| Scope | `owner_id` | `user_id` | Description |
+|-------|-----------|-----------|-------------|
+| `user` | user's ID | null | Personal facts, persist across all conversations |
+| `persona` | persona's ID | null | Shared across all users of a Persona |
+| `persona_user` | persona's ID | user's ID | Per-user facts within a Persona context |
+
+**Ownership:** `PUT` and `DELETE` enforce ownership — a user can only
+modify their own user-scope memories (`owner_id == user_id`).

 ### Admin Memory Review

 ```
-GET  /admin/memories/pending        → { "data": [memory objects with user info] }
+GET  /admin/memories/pending        → { "data": [memory objects] }
 POST /admin/memories/bulk-approve   ← { "ids": ["uuid1", "uuid2"] }
+                                    → { "approved": 2 }
 ```

 Platform admin can review and bulk-approve pending memories across
 all users.

+### AI Tools (non-REST)
+
+Two tools are registered for AI use during completions:
+
+| Tool | Description |
+|------|-------------|
+| `memory_save` | Save a fact with `key`, `value`, `confidence`. Saves as `active` (bypasses review). |
+| `memory_recall` | Search memories by query. Uses hybrid semantic+keyword recall when embedder is configured. |
+
+### Background Extraction
+
+A background scanner periodically finds conversations with enough
+new activity and calls the utility model to extract memorable facts.
+Extracted memories are saved as `pending_review`.
+
+Controlled by `memory_extraction_enabled` global config key and
+per-persona `memory_enabled` flag.
+
+### Memory Injection
+
+Active memories are injected into the system prompt at completion
+time via `BuildMemoryHint`. Uses hybrid semantic recall when an
+embedder is available, falling back to keyword/confidence ranking.
+Budget: ~6000 characters.
+
 ---
--- a/server/database/migrations/sqlite/008_memory.sql
+++ b/server/database/migrations/sqlite/008_memory.sql
@@ -12,6 +12,7 @@ CREATE TABLE IF NOT EXISTS memories (
    confidence        REAL NOT NULL DEFAULT 1.0,
    status            TEXT NOT NULL DEFAULT 'active'
                      CHECK (status IN ('active', 'pending_review', 'archived')),
+    embedding         TEXT,
    last_compacted_at TEXT,
    decay_rate        REAL NOT NULL DEFAULT 0.0,
    created_at        TEXT DEFAULT (datetime('now')),
--- a/server/handlers/integration_test.go
+++ b/server/handlers/integration_test.go
@@ -330,6 +330,16 @@ func setupHarness(t *testing.T) *testHarness {
 	// Tasks (v0.28.0)
 	taskH := NewTaskHandler(stores)
 	protected.GET("/tasks", taskH.ListMine)
+
+	// Memory (v0.18.0, audit v0.28.0)
+	memH := NewMemoryHandler(stores)
+	protected.GET("/memories", memH.ListMyMemories)
+	protected.PUT("/memories/:id", memH.UpdateMemory)
+	protected.DELETE("/memories/:id", memH.DeleteMemory)
+	protected.POST("/memories/:id/approve", memH.ApproveMemory)
+	protected.POST("/memories/:id/reject", memH.RejectMemory)
+	protected.GET("/memories/count", memH.MemoryCount)
+
 	protected.POST("/tasks", middleware.RequirePermission(authpkg.PermTaskCreate, stores), taskH.Create)
 	protected.GET("/tasks/:id", taskH.Get)
 	protected.PUT("/tasks/:id", middleware.RequirePermission(authpkg.PermTaskCreate, stores), taskH.Update)
@@ -423,6 +433,11 @@ func setupHarness(t *testing.T) *testHarness {
 	admin.POST("/tasks/:id/kill", taskAdm.KillRun)
 	admin.DELETE("/tasks/:id", taskAdm.Delete)

+	// Admin memory review
+	adminMemH := NewMemoryHandler(stores)
+	admin.GET("/memories/pending", adminMemH.ListPendingReview)
+	admin.POST("/memories/bulk-approve", adminMemH.BulkApprove)
+
 	return &testHarness{router: r, t: t}
 }

--- a/server/handlers/memory.go
+++ b/server/handlers/memory.go
@@ -43,7 +43,10 @@ func (h *MemoryHandler) ListMyMemories(c *gin.Context) {
 		return
 	}

-	c.JSON(http.StatusOK, gin.H{"memories": memories})
+	if memories == nil {
+		memories = []models.Memory{}
+	}
+	c.JSON(http.StatusOK, gin.H{"data": memories})
 }

 // UpdateMemory updates a memory's key/value/confidence.
@@ -84,7 +87,7 @@ func (h *MemoryHandler) UpdateMemory(c *gin.Context) {
 		existing.Confidence = *body.Confidence
 	}

-	if err := h.stores.Memories.Upsert(c.Request.Context(), existing); err != nil {
+	if err := h.stores.Memories.Update(c.Request.Context(), existing); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}
@@ -128,6 +131,11 @@ func (h *MemoryHandler) ApproveMemory(c *gin.Context) {
 		return
 	}

+	if existing.Status != models.MemoryStatusPendingReview {
+		c.JSON(http.StatusConflict, gin.H{"error": "only pending_review memories can be approved"})
+		return
+	}
+
 	existing.Status = models.MemoryStatusActive
 	if err := h.stores.Memories.Upsert(c.Request.Context(), existing); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
@@ -140,8 +148,26 @@ func (h *MemoryHandler) ApproveMemory(c *gin.Context) {
 // RejectMemory archives a pending_review memory.
 // POST /api/v1/memories/:id/reject
 func (h *MemoryHandler) RejectMemory(c *gin.Context) {
+	userID := c.GetString("user_id")
 	memoryID := c.Param("id")

+	existing, err := h.stores.Memories.GetByID(c.Request.Context(), memoryID)
+	if err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "memory not found"})
+		return
+	}
+
+	// Ownership check: user can only reject their own user-scope memories
+	if existing.Scope == models.MemoryScopeUser && existing.OwnerID != userID {
+		c.JSON(http.StatusForbidden, gin.H{"error": "not your memory"})
+		return
+	}
+
+	if existing.Status != models.MemoryStatusPendingReview {
+		c.JSON(http.StatusConflict, gin.H{"error": "only pending_review memories can be rejected"})
+		return
+	}
+
 	if err := h.stores.Memories.Archive(c.Request.Context(), memoryID); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
@@ -156,15 +182,23 @@ func (h *MemoryHandler) MemoryCount(c *gin.Context) {
 	userID := c.GetString("user_id")
 	ctx := c.Request.Context()

-	active, _ := h.stores.Memories.CountByOwner(ctx, models.MemoryScopeUser, userID)
+	active, err := h.stores.Memories.CountByOwner(ctx, models.MemoryScopeUser, userID)
+	if err != nil {
+		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+		return
+	}

 	// Count pending by listing
-	pending, _ := h.stores.Memories.List(ctx, models.MemoryFilter{
+	pending, err := h.stores.Memories.List(ctx, models.MemoryFilter{
 		Scope:   models.MemoryScopeUser,
 		OwnerID: userID,
 		Status:  models.MemoryStatusPendingReview,
 		Limit:   1000,
 	})
+	if err != nil {
+		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+		return
+	}

 	c.JSON(http.StatusOK, gin.H{
 		"active":  active,
@@ -177,22 +211,18 @@ func (h *MemoryHandler) MemoryCount(c *gin.Context) {
 // ListPendingReview returns all pending_review memories (admin only).
 // GET /api/v1/admin/memories/pending
 func (h *MemoryHandler) ListPendingReview(c *gin.Context) {
-	// This should be called from the admin route group which already checks admin role.
-	// We query across all users by listing each scope.
 	ctx := c.Request.Context()

-	// Query pending memories directly via raw SQL for efficiency
-	var memories []models.Memory
+	memories, err := h.stores.Memories.ListByStatus(ctx, models.MemoryStatusPendingReview, 200)
+	if err != nil {
+		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+		return
+	}

-	userMemories, _ := h.stores.Memories.List(ctx, models.MemoryFilter{
-		Scope:   models.MemoryScopeUser,
-		OwnerID: "%", // This won't work with List — we need a custom query
-		Status:  models.MemoryStatusPendingReview,
-		Limit:   100,
-	})
-	memories = append(memories, userMemories...)
-
-	c.JSON(http.StatusOK, gin.H{"memories": memories})
+	if memories == nil {
+		memories = []models.Memory{}
+	}
+	c.JSON(http.StatusOK, gin.H{"data": memories})
 }

 // BulkApprove approves multiple memories at once.
@@ -213,6 +243,9 @@ func (h *MemoryHandler) BulkApprove(c *gin.Context) {
 		if err != nil {
 			continue
 		}
+		if existing.Status != models.MemoryStatusPendingReview {
+			continue
+		}
 		existing.Status = models.MemoryStatusActive
 		if err := h.stores.Memories.Upsert(ctx, existing); err == nil {
 			approved++
--- a/server/handlers/memory_test.go
+++ b/server/handlers/memory_test.go
@@ -0,0 +1,435 @@
+package handlers
+
+import (
+	"net/http"
+	"testing"
+
+	"git.gobha.me/xcaliber/chat-switchboard/database"
+	"git.gobha.me/xcaliber/chat-switchboard/models"
+)
+
+// seedMemory inserts a memory directly into the DB for testing.
+func seedMemory(t *testing.T, ownerID, key, value, status string) string {
+	t.Helper()
+	id := seedID()
+	q := dialectSQL(`
+		INSERT INTO memories (id, scope, owner_id, key, value, confidence, status)
+		VALUES ($1, $2, $3, $4, $5, $6, $7)
+	`)
+	_, err := database.TestDB.Exec(q, id, models.MemoryScopeUser, ownerID, key, value, 0.9, status)
+	if err != nil {
+		t.Fatalf("seedMemory: %v", err)
+	}
+	return id
+}
+
+// ═══════════════════════════════════════════════
+// List
+// ═══════════════════════════════════════════════
+
+func TestMemory_ListEmpty(t *testing.T) {
+	h := setupHarness(t)
+	_, token := h.createAdminUser("memuser1", "memuser1@test.com")
+
+	w := h.request("GET", "/api/v1/memories", token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("list: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+
+	// Must use "data" key (envelope convention)
+	data, ok := resp["data"].([]interface{})
+	if !ok {
+		t.Fatalf("response must have 'data' array key, got: %s", w.Body.String())
+	}
+	if len(data) != 0 {
+		t.Fatalf("expected 0 memories, got %d", len(data))
+	}
+}
+
+func TestMemory_ListWithData(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memuser2", "memuser2@test.com")
+
+	seedMemory(t, userID, "lang", "Go", models.MemoryStatusActive)
+	seedMemory(t, userID, "editor", "Vim", models.MemoryStatusActive)
+	seedMemory(t, userID, "pending-fact", "something", models.MemoryStatusPendingReview)
+
+	// Default status=active
+	w := h.request("GET", "/api/v1/memories", token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("list: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+	data := resp["data"].([]interface{})
+	if len(data) != 2 {
+		t.Fatalf("expected 2 active memories, got %d", len(data))
+	}
+
+	// Explicit status=pending_review
+	w = h.request("GET", "/api/v1/memories?status=pending_review", token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("list pending: want 200, got %d", w.Code)
+	}
+	decode(w, &resp)
+	data = resp["data"].([]interface{})
+	if len(data) != 1 {
+		t.Fatalf("expected 1 pending memory, got %d", len(data))
+	}
+}
+
+func TestMemory_ListIsolation(t *testing.T) {
+	h := setupHarness(t)
+	userA, tokenA := h.createAdminUser("memA", "memA@test.com")
+	userB, _ := h.createAdminUser("memB", "memB@test.com")
+
+	seedMemory(t, userA, "a-fact", "user A", models.MemoryStatusActive)
+	seedMemory(t, userB, "b-fact", "user B", models.MemoryStatusActive)
+
+	w := h.request("GET", "/api/v1/memories", tokenA, nil)
+	var resp map[string]interface{}
+	decode(w, &resp)
+	data := resp["data"].([]interface{})
+	if len(data) != 1 {
+		t.Fatalf("user A should only see 1 memory, got %d", len(data))
+	}
+	mem := data[0].(map[string]interface{})
+	if mem["key"].(string) != "a-fact" {
+		t.Fatalf("wrong memory: got key %q", mem["key"])
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Update
+// ═══════════════════════════════════════════════
+
+func TestMemory_Update(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memupdater", "memupdater@test.com")
+
+	memID := seedMemory(t, userID, "old-key", "old-value", models.MemoryStatusActive)
+
+	w := h.request("PUT", "/api/v1/memories/"+memID, token, map[string]interface{}{
+		"key":   "new-key",
+		"value": "new-value",
+	})
+	if w.Code != http.StatusOK {
+		t.Fatalf("update: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+	mem := resp["memory"].(map[string]interface{})
+	if mem["key"].(string) != "new-key" {
+		t.Fatalf("key not updated: got %q", mem["key"])
+	}
+	if mem["value"].(string) != "new-value" {
+		t.Fatalf("value not updated: got %q", mem["value"])
+	}
+}
+
+func TestMemory_UpdateOwnershipDenied(t *testing.T) {
+	h := setupHarness(t)
+	ownerID, _ := h.createAdminUser("memowner", "memowner@test.com")
+	_, otherToken := h.createAdminUser("memother", "memother@test.com")
+
+	memID := seedMemory(t, ownerID, "secret", "mine", models.MemoryStatusActive)
+
+	w := h.request("PUT", "/api/v1/memories/"+memID, otherToken, map[string]interface{}{
+		"value": "hacked",
+	})
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("update other's memory: want 403, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+func TestMemory_UpdateNotFound(t *testing.T) {
+	h := setupHarness(t)
+	_, token := h.createAdminUser("memghost", "memghost@test.com")
+
+	w := h.request("PUT", "/api/v1/memories/"+seedID(), token, map[string]interface{}{
+		"value": "x",
+	})
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("update missing: want 404, got %d", w.Code)
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Delete
+// ═══════════════════════════════════════════════
+
+func TestMemory_Delete(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memdeleter", "memdeleter@test.com")
+
+	memID := seedMemory(t, userID, "to-delete", "gone", models.MemoryStatusActive)
+
+	w := h.request("DELETE", "/api/v1/memories/"+memID, token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("delete: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+
+	// Verify gone — list should be empty
+	w = h.request("GET", "/api/v1/memories", token, nil)
+	var resp map[string]interface{}
+	decode(w, &resp)
+	data := resp["data"].([]interface{})
+	if len(data) != 0 {
+		t.Fatalf("memory should be deleted, got %d", len(data))
+	}
+}
+
+func TestMemory_DeleteOwnershipDenied(t *testing.T) {
+	h := setupHarness(t)
+	ownerID, _ := h.createAdminUser("delowner", "delowner@test.com")
+	_, otherToken := h.createAdminUser("delother", "delother@test.com")
+
+	memID := seedMemory(t, ownerID, "no-touch", "x", models.MemoryStatusActive)
+
+	w := h.request("DELETE", "/api/v1/memories/"+memID, otherToken, nil)
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("delete other's memory: want 403, got %d", w.Code)
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Approve
+// ═══════════════════════════════════════════════
+
+func TestMemory_Approve(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memapprover", "memapprover@test.com")
+
+	memID := seedMemory(t, userID, "pending", "val", models.MemoryStatusPendingReview)
+
+	w := h.request("POST", "/api/v1/memories/"+memID+"/approve", token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("approve: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+	mem := resp["memory"].(map[string]interface{})
+	if mem["status"].(string) != models.MemoryStatusActive {
+		t.Fatalf("status should be active, got %q", mem["status"])
+	}
+}
+
+func TestMemory_ApproveStatusGuard(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memguard1", "memguard1@test.com")
+
+	// Try to approve an already-active memory
+	memID := seedMemory(t, userID, "already-active", "val", models.MemoryStatusActive)
+
+	w := h.request("POST", "/api/v1/memories/"+memID+"/approve", token, nil)
+	if w.Code != http.StatusConflict {
+		t.Fatalf("approve active memory: want 409, got %d: %s", w.Code, w.Body.String())
+	}
+
+	// Try to approve an archived memory
+	archivedID := seedMemory(t, userID, "archived", "val", models.MemoryStatusArchived)
+	w = h.request("POST", "/api/v1/memories/"+archivedID+"/approve", token, nil)
+	if w.Code != http.StatusConflict {
+		t.Fatalf("approve archived memory: want 409, got %d", w.Code)
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Reject
+// ═══════════════════════════════════════════════
+
+func TestMemory_Reject(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memrejecter", "memrejecter@test.com")
+
+	memID := seedMemory(t, userID, "to-reject", "val", models.MemoryStatusPendingReview)
+
+	w := h.request("POST", "/api/v1/memories/"+memID+"/reject", token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("reject: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+	if resp["rejected"] != true {
+		t.Fatal("response should have rejected: true")
+	}
+}
+
+func TestMemory_RejectStatusGuard(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memguard2", "memguard2@test.com")
+
+	// Can't reject an already-active memory
+	memID := seedMemory(t, userID, "active-reject", "val", models.MemoryStatusActive)
+
+	w := h.request("POST", "/api/v1/memories/"+memID+"/reject", token, nil)
+	if w.Code != http.StatusConflict {
+		t.Fatalf("reject active memory: want 409, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+func TestMemory_RejectOwnershipDenied(t *testing.T) {
+	h := setupHarness(t)
+	ownerID, _ := h.createAdminUser("rejowner", "rejowner@test.com")
+	_, otherToken := h.createAdminUser("rejother", "rejother@test.com")
+
+	memID := seedMemory(t, ownerID, "not-yours", "val", models.MemoryStatusPendingReview)
+
+	w := h.request("POST", "/api/v1/memories/"+memID+"/reject", otherToken, nil)
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("reject other's memory: want 403, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Count
+// ═══════════════════════════════════════════════
+
+func TestMemory_Count(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memcounter", "memcounter@test.com")
+
+	seedMemory(t, userID, "fact1", "a", models.MemoryStatusActive)
+	seedMemory(t, userID, "fact2", "b", models.MemoryStatusActive)
+	seedMemory(t, userID, "pend1", "c", models.MemoryStatusPendingReview)
+
+	w := h.request("GET", "/api/v1/memories/count", token, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("count: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+
+	// Response shape: {"active": N, "pending": N}
+	active := int(resp["active"].(float64))
+	pending := int(resp["pending"].(float64))
+	if active != 2 {
+		t.Fatalf("expected 2 active, got %d", active)
+	}
+	if pending != 1 {
+		t.Fatalf("expected 1 pending, got %d", pending)
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Admin — List Pending
+// ═══════════════════════════════════════════════
+
+func TestMemory_AdminListPending(t *testing.T) {
+	h := setupHarness(t)
+	userA, _ := h.createAdminUser("adminmemA", "adminmemA@test.com")
+	userB, _ := h.createAdminUser("adminmemB", "adminmemB@test.com")
+	_, adminToken := h.createAdminUser("memadmin", "memadmin@test.com")
+
+	// Seed pending memories for both users
+	seedMemory(t, userA, "factA", "from A", models.MemoryStatusPendingReview)
+	seedMemory(t, userB, "factB", "from B", models.MemoryStatusPendingReview)
+	seedMemory(t, userA, "active-A", "active", models.MemoryStatusActive) // should NOT appear
+
+	w := h.request("GET", "/api/v1/admin/memories/pending", adminToken, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("admin pending: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+	data := resp["data"].([]interface{})
+	if len(data) != 2 {
+		t.Fatalf("expected 2 pending memories across users, got %d", len(data))
+	}
+}
+
+func TestMemory_AdminListPendingRequiresAdmin(t *testing.T) {
+	h := setupHarness(t)
+	database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('allow_registration', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'")
+	database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('default_user_active', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'")
+	_, userToken := h.registerUser("normalmem", "normalmem@test.com", "password123")
+
+	w := h.request("GET", "/api/v1/admin/memories/pending", userToken, nil)
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("non-admin should be forbidden: want 403, got %d", w.Code)
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Admin — Bulk Approve
+// ═══════════════════════════════════════════════
+
+func TestMemory_AdminBulkApprove(t *testing.T) {
+	h := setupHarness(t)
+	userID, _ := h.createAdminUser("bulkowner", "bulkowner@test.com")
+	_, adminToken := h.createAdminUser("bulkadmin", "bulkadmin@test.com")
+
+	id1 := seedMemory(t, userID, "bulk1", "a", models.MemoryStatusPendingReview)
+	id2 := seedMemory(t, userID, "bulk2", "b", models.MemoryStatusPendingReview)
+	id3 := seedMemory(t, userID, "already-active", "c", models.MemoryStatusActive)
+
+	w := h.request("POST", "/api/v1/admin/memories/bulk-approve", adminToken, map[string]interface{}{
+		"ids": []string{id1, id2, id3},
+	})
+	if w.Code != http.StatusOK {
+		t.Fatalf("bulk approve: want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	decode(w, &resp)
+	approved := int(resp["approved"].(float64))
+	if approved != 2 {
+		t.Fatalf("expected 2 approved (skipping already-active), got %d", approved)
+	}
+}
+
+func TestMemory_AdminBulkApproveInvalidBody(t *testing.T) {
+	h := setupHarness(t)
+	_, adminToken := h.createAdminUser("bulkbad", "bulkbad@test.com")
+
+	w := h.request("POST", "/api/v1/admin/memories/bulk-approve", adminToken, "not-json")
+	if w.Code != http.StatusBadRequest {
+		t.Fatalf("bad body: want 400, got %d", w.Code)
+	}
+}
+
+// ═══════════════════════════════════════════════
+// Field Shape Verification
+// ═══════════════════════════════════════════════
+
+func TestMemory_FieldShape(t *testing.T) {
+	h := setupHarness(t)
+	userID, token := h.createAdminUser("memshape", "memshape@test.com")
+
+	seedMemory(t, userID, "test-key", "test-value", models.MemoryStatusActive)
+
+	w := h.request("GET", "/api/v1/memories", token, nil)
+	var resp map[string]interface{}
+	decode(w, &resp)
+	data := resp["data"].([]interface{})
+	mem := data[0].(map[string]interface{})
+
+	// Verify all expected fields exist
+	requiredFields := []string{"id", "scope", "owner_id", "key", "value", "confidence", "status", "created_at", "updated_at"}
+	for _, f := range requiredFields {
+		if _, ok := mem[f]; !ok {
+			t.Errorf("missing field %q in memory object", f)
+		}
+	}
+
+	// Verify no legacy "content" field
+	if _, ok := mem["content"]; ok {
+		t.Error("memory should not have 'content' field (legacy ICD)")
+	}
+	// Verify no legacy "persona_id" field
+	if _, ok := mem["persona_id"]; ok {
+		t.Error("memory should not have 'persona_id' field (legacy ICD)")
+	}
+
+	// Verify scope is correct
+	if mem["scope"].(string) != models.MemoryScopeUser {
+		t.Errorf("scope should be %q, got %q", models.MemoryScopeUser, mem["scope"])
+	}
+
+	// Confidence should be a number
+	confidence, ok := mem["confidence"].(float64)
+	if !ok || confidence < 0 || confidence > 1 {
+		t.Errorf("confidence should be 0-1 float, got %v", mem["confidence"])
+	}
+}
--- a/server/memory/extractor.go
+++ b/server/memory/extractor.go
@@ -117,8 +117,14 @@ func (e *Extractor) Extract(ctx context.Context, channelID, userID, teamID, pers
 	prompt := defaultExtractionPrompt
 	if personaID != "" && e.stores.Personas != nil {
 		persona, err := e.stores.Personas.GetByID(ctx, personaID)
-		if err == nil && persona.MemoryExtractionPrompt != nil && *persona.MemoryExtractionPrompt != "" {
-			prompt = *persona.MemoryExtractionPrompt
+		if err == nil {
+			// Respect persona memory_enabled flag (H6 — audit v0.28.0)
+			if !persona.MemoryEnabled {
+				return nil
+			}
+			if persona.MemoryExtractionPrompt != nil && *persona.MemoryExtractionPrompt != "" {
+				prompt = *persona.MemoryExtractionPrompt
+			}
 		}
 	}

--- a/server/memory/scanner.go
+++ b/server/memory/scanner.go
@@ -76,21 +76,23 @@ type candidate struct {
 	ChannelID string
 	UserID    string
 	TeamID    string
+	PersonaID string // non-empty if a persona is active on the channel
 }

 func (s *Scanner) scan() {
 	ctx := context.Background()

-	// Check global kill switch
+	// Check global kill switch.
+	// Default: ENABLED. Only disable if the key exists and is explicitly false.
+	// This inverts the previous behavior where a missing key blocked all extraction.
 	if s.stores.GlobalConfig != nil {
 		val, err := s.stores.GlobalConfig.Get(ctx, "memory_extraction_enabled")
-		if err != nil || val == nil {
-			return // disabled or not configured
-		}
-		// val is a map; check the "value" key
-		if enabled, ok := val["value"].(bool); !ok || !enabled {
-			return
+		if err == nil && val != nil {
+			if enabled, ok := val["value"].(bool); ok && !enabled {
+				return // explicitly disabled
+			}
 		}
+		// If key is missing (err != nil or val == nil), extraction is enabled.
 	}

 	candidates, err := s.findCandidates(ctx)
@@ -124,7 +126,7 @@ func (s *Scanner) scan() {
 			extractCtx, cancel := context.WithTimeout(ctx, 2*time.Minute)
 			defer cancel()

-			if err := s.extractor.Extract(extractCtx, cand.ChannelID, cand.UserID, cand.TeamID, ""); err != nil {
+			if err := s.extractor.Extract(extractCtx, cand.ChannelID, cand.UserID, cand.TeamID, cand.PersonaID); err != nil {
 				log.Printf("⚠ memory extraction failed: channel=%s user=%s: %v",
 					cand.ChannelID, cand.UserID, err)
 			}
@@ -135,6 +137,12 @@ func (s *Scanner) scan() {
 }

 // findCandidates queries for channels with enough new activity since last extraction.
+//
+// Fixes from v0.28.0 audit:
+// - Uses channels.user_id (was: non-existent created_by)
+// - Uses channels.team_id directly (was: JOIN to non-existent channel_users table)
+// - JOINs channel_participants + personas to check memory_enabled (H6)
+// - Filters out channels whose active persona has memory_enabled=false
 func (s *Scanner) findCandidates(ctx context.Context) ([]candidate, error) {
 	db := database.DB
 	if db == nil {
@@ -144,11 +152,17 @@ func (s *Scanner) findCandidates(ctx context.Context) ([]candidate, error) {
 	var query string
 	if database.IsSQLite() {
 		query = `
-			SELECT c.id, c.created_by, COALESCE(cu.team_id, '')
+			SELECT c.id, c.user_id, COALESCE(c.team_id, ''),
+			       COALESCE(cp.participant_id, '')
 			FROM channels c
-			LEFT JOIN channel_users cu ON cu.channel_id = c.id AND cu.user_id = c.created_by
-			LEFT JOIN memory_extraction_log mel ON mel.channel_id = c.id AND mel.user_id = c.created_by
-			WHERE c.created_by IS NOT NULL
+			LEFT JOIN channel_participants cp
+			       ON cp.channel_id = c.id AND cp.participant_type = 'persona'
+			LEFT JOIN personas p
+			       ON p.id = cp.participant_id
+			LEFT JOIN memory_extraction_log mel
+			       ON mel.channel_id = c.id AND mel.user_id = c.user_id
+			WHERE c.user_id IS NOT NULL
+			  AND (cp.id IS NULL OR p.memory_enabled = 1)
 			  AND (
 			    SELECT COUNT(*) FROM messages m
 			    WHERE m.channel_id = c.id
@@ -161,11 +175,17 @@ func (s *Scanner) findCandidates(ctx context.Context) ([]candidate, error) {
 		`
 	} else {
 		query = `
-			SELECT c.id, c.created_by, COALESCE(cu.team_id, '')
+			SELECT c.id, c.user_id, COALESCE(c.team_id::text, ''),
+			       COALESCE(cp.participant_id::text, '')
 			FROM channels c
-			LEFT JOIN channel_users cu ON cu.channel_id = c.id AND cu.user_id = c.created_by
-			LEFT JOIN memory_extraction_log mel ON mel.channel_id = c.id AND mel.user_id = c.created_by
-			WHERE c.created_by IS NOT NULL
+			LEFT JOIN channel_participants cp
+			       ON cp.channel_id = c.id AND cp.participant_type = 'persona'
+			LEFT JOIN personas p
+			       ON p.id = cp.participant_id
+			LEFT JOIN memory_extraction_log mel
+			       ON mel.channel_id = c.id AND mel.user_id = c.user_id
+			WHERE c.user_id IS NOT NULL
+			  AND (cp.id IS NULL OR p.memory_enabled = true)
 			  AND (
 			    SELECT COUNT(*) FROM messages m
 			    WHERE m.channel_id = c.id
@@ -187,7 +207,7 @@ func (s *Scanner) findCandidates(ctx context.Context) ([]candidate, error) {
 	var candidates []candidate
 	for rows.Next() {
 		var c candidate
-		if err := rows.Scan(&c.ChannelID, &c.UserID, &c.TeamID); err != nil {
+		if err := rows.Scan(&c.ChannelID, &c.UserID, &c.TeamID, &c.PersonaID); err != nil {
 			continue
 		}
 		candidates = append(candidates, c)
--- a/server/models/models_memory_persona.go
+++ b/server/models/models_memory_persona.go
@@ -1,18 +0,0 @@
-// ── models_memory_persona.go ────────────────
-// Phase 2 additions to the Persona struct.
-//
-// Add these fields to the Persona struct in models/models.go
-// (after the KBIDs field):
-//
-//     MemoryEnabled          bool    `json:"memory_enabled" db:"memory_enabled"`
-//     MemoryExtractionPrompt *string `json:"memory_extraction_prompt,omitempty" db:"memory_extraction_prompt"`
-//
-// Add to PersonaPatch:
-//
-//     MemoryEnabled          *bool   `json:"memory_enabled,omitempty"`
-//     MemoryExtractionPrompt *string `json:"memory_extraction_prompt,omitempty"`
-//
-// This file exists purely as documentation — the actual fields
-// must be added to the existing structs in models.go.
-
-package models
--- a/server/store/postgres/memory.go
+++ b/server/store/postgres/memory.go
@@ -5,7 +5,6 @@ import (
 	"database/sql"
 	"fmt"
 	"strings"
-	"time"

 	"git.gobha.me/xcaliber/chat-switchboard/models"
 	"git.gobha.me/xcaliber/chat-switchboard/store"
@@ -42,6 +41,16 @@ func (s *MemoryStore) Upsert(ctx context.Context, m *models.Memory) error {
 	return err
 }

+func (s *MemoryStore) Update(ctx context.Context, m *models.Memory) error {
+	_, err := DB.ExecContext(ctx, `
+		UPDATE memories
+		SET key = $1, value = $2, confidence = $3, status = $4,
+		    source_channel_id = $5, updated_at = now()
+		WHERE id = $6
+	`, m.Key, m.Value, m.Confidence, m.Status, m.SourceChannelID, m.ID)
+	return err
+}
+
 func (s *MemoryStore) GetByID(ctx context.Context, id string) (*models.Memory, error) {
 	m := &models.Memory{}
 	err := DB.QueryRowContext(ctx, `
@@ -220,6 +229,25 @@ func (s *MemoryStore) CountByOwner(ctx context.Context, scope, ownerID string) (
 	return count, err
 }

+func (s *MemoryStore) ListByStatus(ctx context.Context, status string, limit int) ([]models.Memory, error) {
+	if limit <= 0 || limit > 500 {
+		limit = 100
+	}
+	rows, err := DB.QueryContext(ctx, `
+		SELECT id, scope, owner_id, user_id, key, value,
+		       source_channel_id, confidence, status, created_at, updated_at
+		FROM memories
+		WHERE status = $1
+		ORDER BY created_at DESC
+		LIMIT $2
+	`, status, limit)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	return scanMemories(rows)
+}
+
 // ── Helpers ─────────────────────────────────

 func scanMemories(rows *sql.Rows) ([]models.Memory, error) {
@@ -240,30 +268,5 @@ func scanMemories(rows *sql.Rows) ([]models.Memory, error) {
 	return memories, rows.Err()
 }

-// scanMemory scans a single memory row — unused currently but available
-// for future single-row queries.
-func scanMemory(row *sql.Row) (*models.Memory, error) {
-	m := &models.Memory{}
-	err := row.Scan(
-		&m.ID, &m.Scope, &m.OwnerID, &m.UserID, &m.Key, &m.Value,
-		&m.SourceChannelID, &m.Confidence, &m.Status, &m.CreatedAt, &m.UpdatedAt,
-	)
-	if err == sql.ErrNoRows {
-		return nil, nil
-	}
-	return m, err
-}
-
 // ensure compile-time interface satisfaction
-var _ interface {
-	Upsert(ctx context.Context, m *models.Memory) error
-	GetByID(ctx context.Context, id string) (*models.Memory, error)
-	List(ctx context.Context, f models.MemoryFilter) ([]models.Memory, error)
-	Delete(ctx context.Context, id string) error
-	Archive(ctx context.Context, id string) error
-	Recall(ctx context.Context, userID string, personaID *string, query string, limit int) ([]models.Memory, error)
-	CountByOwner(ctx context.Context, scope, ownerID string) (int, error)
-} = (*MemoryStore)(nil)
-
-// ensure unused imports don't cause errors
-var _ = time.Now
+var _ store.MemoryStore = (*MemoryStore)(nil)
--- a/server/store/postgres/memory_hybrid.go
+++ b/server/store/postgres/memory_hybrid.go
@@ -2,7 +2,6 @@ package postgres

 import (
 	"context"
-	"database/sql"
 	"fmt"
 	"strings"

@@ -130,6 +129,3 @@ func mergeResults(semantic, keyword []models.Memory, limit int) []models.Memory

 	return result
 }
-
-// ensure unused
-var _ = sql.ErrNoRows
--- a/server/store/sqlite/memory.go
+++ b/server/store/sqlite/memory.go
@@ -41,6 +41,16 @@ func (s *MemoryStore) Upsert(ctx context.Context, m *models.Memory) error {
 	return err
 }

+func (s *MemoryStore) Update(ctx context.Context, m *models.Memory) error {
+	_, err := DB.ExecContext(ctx, `
+		UPDATE memories
+		SET key = ?, value = ?, confidence = ?, status = ?,
+		    source_channel_id = ?, updated_at = datetime('now')
+		WHERE id = ?
+	`, m.Key, m.Value, m.Confidence, m.Status, m.SourceChannelID, m.ID)
+	return err
+}
+
 func (s *MemoryStore) GetByID(ctx context.Context, id string) (*models.Memory, error) {
 	m := &models.Memory{}
 	err := DB.QueryRowContext(ctx, `
@@ -202,6 +212,25 @@ func (s *MemoryStore) CountByOwner(ctx context.Context, scope, ownerID string) (
 	return count, err
 }

+func (s *MemoryStore) ListByStatus(ctx context.Context, status string, limit int) ([]models.Memory, error) {
+	if limit <= 0 || limit > 500 {
+		limit = 100
+	}
+	rows, err := DB.QueryContext(ctx, `
+		SELECT id, scope, owner_id, user_id, key, value,
+		       source_channel_id, confidence, status, created_at, updated_at
+		FROM memories
+		WHERE status = ?
+		ORDER BY created_at DESC
+		LIMIT ?
+	`, status, limit)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	return s.scanMemories(rows)
+}
+
 // ── Helpers ─────────────────────────────────

 func (s *MemoryStore) scanMemories(rows *sql.Rows) ([]models.Memory, error) {
@@ -222,3 +251,6 @@ func (s *MemoryStore) scanMemories(rows *sql.Rows) ([]models.Memory, error) {
 	}
 	return memories, rows.Err()
 }
+
+// ensure compile-time interface satisfaction
+var _ store.MemoryStore = (*MemoryStore)(nil)
--- a/server/store/store_memory.go
+++ b/server/store/store_memory.go
@@ -21,6 +21,10 @@ type MemoryStore interface {
 	// If a memory with the same key exists, value/confidence/status are updated.
 	Upsert(ctx context.Context, m *models.Memory) error

+	// Update modifies an existing memory by ID. Use for handler edits where
+	// the key may change (Upsert conflicts on composite key, not on ID).
+	Update(ctx context.Context, m *models.Memory) error
+
 	// GetByID returns a single memory by ID.
 	GetByID(ctx context.Context, id string) (*models.Memory, error)

@@ -41,6 +45,10 @@ type MemoryStore interface {
 	// CountByOwner returns the number of active memories for a scope+owner.
 	CountByOwner(ctx context.Context, scope, ownerID string) (int, error)

+	// ListByStatus returns all memories with the given status across all owners.
+	// Used by admin review endpoints. Results ordered by created_at DESC.
+	ListByStatus(ctx context.Context, status string, limit int) ([]models.Memory, error)
+
 	// RecallHybrid returns active memories using vector similarity + keyword search.
 	// If queryVec is nil/empty, falls back to keyword-only (same as Recall).
 	RecallHybrid(ctx context.Context, userID string, personaID *string, query string, queryVec []float64, limit int) ([]models.Memory, error)
--- a/server/store/store_memory_hybrid.go
+++ b/server/store/store_memory_hybrid.go
@@ -1,15 +0,0 @@
-// ── store_memory_hybrid.go ──────────────────
-// Phase 2 addition to the MemoryStore interface.
-//
-// Add this method to the MemoryStore interface in store/store_memory.go:
-//
-//     // RecallHybrid returns active memories using vector similarity + keyword search.
-//     // If queryVec is nil/empty, falls back to keyword-only (same as Recall).
-//     // Merges results from applicable scopes with deduplication.
-//     RecallHybrid(ctx context.Context, userID string, personaID *string, query string, queryVec []float64, limit int) ([]models.Memory, error)
-//
-// The implementations are in:
-//   - store/postgres/memory_hybrid.go (pgvector cosine distance)
-//   - store/sqlite/memory_hybrid.go   (app-level cosine similarity)
-
-package store
--- a/src/js/memory-ui.js
+++ b/src/js/memory-ui.js
@@ -84,7 +84,7 @@ const MemoryUI = {

        try {
            const data = await API.listMemories(this._filter.status, this._filter.query);
-            this._memories = data.memories || [];
+            this._memories = data.data || [];

            if (!this._memories.length) {
                el.innerHTML = `<div class="empty-hint">No ${this._filter.status.replace('_', ' ')} memories${this._filter.query ? ' matching "' + esc(this._filter.query) + '"' : ''}</div>`;
@@ -211,7 +211,7 @@ const MemoryUI = {
    async approveAllPending() {
        try {
            const data = await API.listMemories('pending_review', '');
-            const ids = (data.memories || []).map(m => m.id);
+            const ids = (data.data || []).map(m => m.id);
            if (!ids.length) return UI.toast('No pending memories', 'info');
            if (!await showConfirm(`Approve all ${ids.length} pending memories?`)) return;
            await API.bulkApproveMemories(ids);
@@ -231,7 +231,7 @@ const MemoryUI = {

        try {
            const data = await API.adminListPendingMemories();
-            const pending = data.memories || [];
+            const pending = data.data || [];

            el.innerHTML = `
                <div class="admin-toolbar">