diff --git a/packages/shared/src/validators/memory.ts b/packages/shared/src/validators/memory.ts index 679f6f2081f..c3805549033 100644 --- a/packages/shared/src/validators/memory.ts +++ b/packages/shared/src/validators/memory.ts @@ -130,6 +130,14 @@ export const memoryProviderConfigMetadataSchema = z }) .strict(); +const queryBooleanSchema = z.preprocess((value) => { + if (typeof value !== "string") return value; + const normalized = value.trim().toLowerCase(); + if (normalized === "true") return true; + if (normalized === "false") return false; + return value; +}, z.boolean()); + export const memoryHookPolicySchema = z .object({ enabled: z.boolean().optional(), @@ -295,10 +303,10 @@ export const memoryListRecordsQuerySchema = z runId: z.string().uuid().optional(), sourceKind: z.enum(MEMORY_SOURCE_KINDS).optional(), q: z.string().trim().min(1).max(500).optional(), - includeDeleted: z.coerce.boolean().optional().default(false), - includeRevoked: z.coerce.boolean().optional().default(false), - includeExpired: z.coerce.boolean().optional().default(false), - includeSuperseded: z.coerce.boolean().optional().default(false), + includeDeleted: queryBooleanSchema.optional().default(false), + includeRevoked: queryBooleanSchema.optional().default(false), + includeExpired: queryBooleanSchema.optional().default(false), + includeSuperseded: queryBooleanSchema.optional().default(false), limit: z.coerce.number().int().positive().max(200).optional().default(50), count: z.enum(["only"]).optional(), }) diff --git a/server/src/__tests__/memory-routes.test.ts b/server/src/__tests__/memory-routes.test.ts index bc24ee6b7c2..29769146e3d 100644 --- a/server/src/__tests__/memory-routes.test.ts +++ b/server/src/__tests__/memory-routes.test.ts @@ -258,7 +258,7 @@ describe("memory routes", () => { const res = await request(app) .get(`/api/companies/${companyA}/memory/records`) - .query({ count: "only", reviewState: "pending" }) + .query({ count: "only", reviewState: "pending", includeRevoked: "false", includeExpired: "false" }) .set("Origin", "http://localhost:3100"); expect(res.status).toBe(200); @@ -269,6 +269,7 @@ describe("memory routes", () => { count: "only", reviewState: "pending", includeRevoked: false, + includeExpired: false, }), expect.objectContaining({ actorType: "user", userId: "board-user" }), ); diff --git a/server/src/__tests__/memory-service.test.ts b/server/src/__tests__/memory-service.test.ts index 686a9666e0c..53abb72b6ba 100644 --- a/server/src/__tests__/memory-service.test.ts +++ b/server/src/__tests__/memory-service.test.ts @@ -1,5 +1,12 @@ -import { describe, expect, it, vi } from "vitest"; +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { companies, createDb } from "@paperclipai/db"; +import type { MemoryListRecordsQuery } from "@paperclipai/shared"; import { memoryService } from "../services/memory.js"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; function createQueuedSelectDb(selectResults: unknown[][]) { const calls: Array<{ kind: string }> = []; @@ -382,7 +389,7 @@ describe("memoryService hook policies", () => { updatedValues.push(value); return { where: vi.fn().mockReturnValue({ - returning: vi.fn().mockResolvedValue(updateResults.shift() ?? []), + returning: vi.fn().mockImplementation(() => Promise.resolve(updateResults.shift() ?? [])), }), }; }), @@ -420,6 +427,7 @@ describe("memoryService hook policies", () => { }); expect(insertedValues[1]).toMatchObject({ reviewState: "accepted", + createdByOperationId: null, metadata: { extraction: { mode: "paperclip_managed", @@ -428,7 +436,12 @@ describe("memoryService hook policies", () => { }, }, }); - expect(updatedValues[0]).toMatchObject({ + expect(updatedValues).toContainEqual( + expect.objectContaining({ + createdByOperationId: "77777777-7777-4777-8777-777777777777", + }), + ); + expect(updatedValues.find((value) => (value as { status?: string }).status === "succeeded")).toMatchObject({ status: "succeeded", operationId: "77777777-7777-4777-8777-777777777777", resultJson: { @@ -438,3 +451,92 @@ describe("memoryService hook policies", () => { }); }); }); + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +describeEmbeddedPostgres("memoryService local basic persistence", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-memory-service-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + it("captures records with a valid operation id and counts explicit revoked filters", async () => { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip Memory Test", + issuePrefix: `M${companyId.replace(/-/g, "").slice(0, 5).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + const service = memoryService(db); + const actor = { + actorType: "user" as const, + actorId: "board-user", + agentId: null, + userId: "board-user", + runId: null, + }; + const binding = await service.createBinding(companyId, { + key: "local-test", + name: "Local test", + providerKey: "local_basic", + config: {}, + enabled: true, + }); + await service.setCompanyDefault(companyId, binding.id); + + const captured = await service.capture( + companyId, + { + bindingKey: "local-test", + scope: { scopeType: "org", scopeId: companyId }, + scopeType: "org", + scopeId: companyId, + source: { kind: "manual_note", externalRef: "memory-service-test" }, + title: "Revoked filter regression", + content: "A memory record that will be revoked for count filter coverage.", + reviewState: "pending", + }, + actor, + ); + + expect(captured.records).toHaveLength(1); + expect(captured.records[0].createdByOperationId).toBe(captured.operation.id); + + await service.revoke( + companyId, + { + selector: { recordIds: [captured.records[0].id] }, + reason: "Verify revoked filters can see explicit revoked retention state", + }, + actor, + ); + + const revokedFilters: MemoryListRecordsQuery = { + retentionState: "revoked", + includeDeleted: false, + includeRevoked: true, + includeExpired: false, + includeSuperseded: false, + limit: 50, + }; + + await expect(service.countRecords(companyId, revokedFilters, actor)).resolves.toEqual({ count: 1 }); + + const revokedRecords = await service.listRecords(companyId, revokedFilters, actor); + expect(revokedRecords.map((record) => record.id)).toEqual([captured.records[0].id]); + }); +}); diff --git a/server/src/services/memory.ts b/server/src/services/memory.ts index 90877f9a930..aebfa34bc4c 100644 --- a/server/src/services/memory.ts +++ b/server/src/services/memory.ts @@ -924,7 +924,7 @@ export function memoryService( metadata?: Record; reviewState?: MemoryRecord["reviewState"]; }, - operationId: string, + operationId: string | null, ) { const scopeType = input.scopeType ?? normalizeScopeType(scope); const scopeId = input.scopeId ?? normalizeScopeId(binding.companyId, scopeType, scope); @@ -1122,6 +1122,27 @@ export function memoryService( }); } + async function attachCreatedByOperation( + companyId: string, + records: MemoryRecord[], + operationId: string, + ) { + if (records.length === 0) return records; + await db + .update(memoryLocalRecords) + .set({ + createdByOperationId: operationId, + updatedAt: new Date(), + }) + .where( + and( + eq(memoryLocalRecords.companyId, companyId), + inArray(memoryLocalRecords.id, records.map((record) => record.id)), + ), + ); + return records.map((record) => ({ ...record, createdByOperationId: operationId })); + } + function buildLocalBasicConditions(companyId: string, filters: MemoryListRecordsQuery, actor: ActorInfo) { const conditions = [eq(memoryLocalRecords.companyId, companyId)]; if (filters.bindingId) conditions.push(eq(memoryLocalRecords.bindingId, filters.bindingId)); @@ -1159,7 +1180,9 @@ export function memoryService( if (!filters.includeRevoked) conditions.push(isNull(memoryLocalRecords.revokedAt)); if (!filters.includeExpired) { conditions.push(or(isNull(memoryLocalRecords.expiresAt), sql`${memoryLocalRecords.expiresAt} > now()`)!); - conditions.push(or(isNull(memoryLocalRecords.retentionState), eq(memoryLocalRecords.retentionState, "active"))!); + if (!filters.retentionState) { + conditions.push(or(isNull(memoryLocalRecords.retentionState), eq(memoryLocalRecords.retentionState, "active"))!); + } } if (!filters.includeSuperseded) conditions.push(isNull(memoryLocalRecords.supersededByRecordId)); @@ -2257,7 +2280,7 @@ export function memoryService( metadata: data.metadata ?? {}, reviewState: data.reviewState, }, - operationId, + null, ); } else { if (!pluginMemoryProviders) { @@ -2284,7 +2307,7 @@ export function memoryService( records = providerResult.records; usage = providerResult.usage ?? []; providerResultJson = providerResult.resultJson ?? null; - await persistCatalogRecords(binding, records, operationId); + await persistCatalogRecords(binding, records, null); } const operation = await logOperation({ id: operationId, @@ -2318,6 +2341,7 @@ export function memoryService( recordCount: records.length, usage, }); + records = await attachCreatedByOperation(companyId, records, operation.id); return { operation, records } satisfies MemoryCaptureResult; }, @@ -2537,7 +2561,7 @@ export function memoryService( reviewNote: parsed.reason, citationJson: parsed.citation === undefined ? originalRow.citationJson : normalizeCitation(parsed.citation), supersedesRecordId: originalRow.id, - createdByOperationId: operationId, + createdByOperationId: null, createdAt: correctedAt, updatedAt: correctedAt, }) @@ -2568,11 +2592,16 @@ export function memoryService( }, recordCount: 1, }); + const [attachedCorrectedRow] = await db + .update(memoryLocalRecords) + .set({ createdByOperationId: operation.id, updatedAt: correctedAt }) + .where(and(eq(memoryLocalRecords.companyId, companyId), eq(memoryLocalRecords.id, correctedRecordId))) + .returning(); return { operation, originalRecord: mapRecord({ ...originalRow, supersededByRecordId: correctedRecordId, updatedAt: correctedAt }), - correctedRecord: mapRecord(correctedRow), + correctedRecord: mapRecord(attachedCorrectedRow ?? { ...correctedRow, createdByOperationId: operation.id }), } satisfies MemoryCorrectResult; },