diff --git a/CHANGELOG.md b/CHANGELOG.md index faab29554..1a0113461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Syncing a memory source a policy refuses says why + +When a connected app's policy refused the read behind a memory source's sync, `POST +/api/memory/sources/:id/sync` answered 503 "Memory is unavailable. Try again.", although the +refusal's own sentence was already saved on the source. It now answers 400 with that sentence, as +the plugin routes do for the same refusal. + ### `@Ops Lead` in a group addresses Ops Lead, not Ops as well In a group conversation, a reply naming `@Ops Lead` also addressed a Bot called Ops, because the diff --git a/server/src/memory/routes.ts b/server/src/memory/routes.ts index f3733d43a..192c48ad0 100644 --- a/server/src/memory/routes.ts +++ b/server/src/memory/routes.ts @@ -1,6 +1,7 @@ import { Hono, type MiddlewareHandler } from "hono"; import { bodyLimit } from "hono/body-limit"; import type { AppVariables } from "../auth/guards"; +import { PluginRefusedError } from "../plugins/store"; import type { MemoryIngestion } from "./ingestion"; import type { MemoryStore } from "./store"; import { MemoryNotFoundError, MemoryRefusedError } from "./types"; @@ -15,7 +16,12 @@ export function createMemoryRoutes( routes.onError((error, context) => { if (error instanceof MemoryNotFoundError) return context.json({ error: error.message }, 404); - if (error instanceof MemoryRefusedError) + // A connector's policy refusing the read is an answer to give the person, as it is on the + // plugin routes, and the same sentence `sync` saves on the source. + if ( + error instanceof MemoryRefusedError || + error instanceof PluginRefusedError + ) return context.json({ error: error.message }, 400); console.error( JSON.stringify({ diff --git a/server/tests/memory.test.ts b/server/tests/memory.test.ts index 05989af40..988b4a797 100644 --- a/server/tests/memory.test.ts +++ b/server/tests/memory.test.ts @@ -1,15 +1,20 @@ import { expect, test } from "bun:test"; import { z } from "zod"; +import type { AuthenticatedActor } from "../src/auth/guards"; +import type { MemoryIngestion } from "../src/memory/ingestion"; import { normalizeConnectorRecords, quoteMemoryContext, } from "../src/memory/ingestion"; +import { createMemoryRoutes } from "../src/memory/routes"; +import type { MemoryStore } from "../src/memory/store"; import { memoryTools } from "../src/memory/tools"; import { parseMemoryInput, parseMemoryPatch, parseMemorySourceInput, } from "../src/memory/types"; +import { PluginRefusedError } from "../src/plugins/store"; test("memory validates explicit facts and bounded source opt-in", () => { expect( @@ -137,3 +142,23 @@ test("every memory tool can be offered to a remote Bot as JSON Schema", async () }, ]); }); + +test("a source sync a connector's policy refuses answers 400 with the reason, not 503", async () => { + const routes = createMemoryRoutes( + {} as MemoryStore, + { + sync: async () => { + throw new PluginRefusedError("Tool refused by policy.", "deny-notion"); + }, + } as unknown as MemoryIngestion, + async (context, next) => { + context.set("actor", { id: "person" } as AuthenticatedActor); + await next(); + }, + ); + const response = await routes.request("/sources/source-1/sync", { + method: "POST", + }); + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ error: "Tool refused by policy." }); +});