diff --git a/CHANGELOG.md b/CHANGELOG.md index d6bc8424..c70dbe4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,22 +36,17 @@ already refused the same way. Sessions survive and nobody signs in again. +### Changed + +- **This deployment does not search documents itself.** A Bot answers from a live system by calling + that system's own search as the person asking, so the vendor decides what they may see and there is + no second copy of anybody's documents here to keep in step, to secure, or to leave behind when + somebody is removed. The local index that was being filled — `documents`, `chunks` and + `document_acls` — is read by nothing, and the connector that filled it is going away. Retrieval over + a copy of a customer's corpus is not a thing OpenBot does. + ### Added -- **A Bot can answer from a connected source, as the person asking.** The connectors have been - writing `documents`, `chunks` and `document_acls` and nothing ever read them back, so a deployment - that connected a source got rows in PostgreSQL and still no citation. A Bot now has a - `search_company_knowledge` tool, and it returns only the documents the person asking is allowed to - read — filtered in the database against that person's own principals rather than fetched and - filtered in the server, so a document they may not read is never handed over. A deny beats an - allow, and a document with no ACL rows is readable by nobody rather than by everybody. Each result - carries the document's title, the link that opens it, and the passage that matched. A search that - finds nothing says so, rather than returning an empty string a model would fill in from memory. - Every search is on the audit trail as `knowledge.searched`, naming the query and the documents - returned and never quoting their text. The tool is only offered when there is something to search. - Matching is PostgreSQL's own full-text search over the stored passages: nothing in the deployment - produces embeddings yet, so the vector column is left alone and ranking by meaning follows the - first connector that writes one. - **Releases are cut by a workflow, not by hand.** `Create release PR` bumps the version and promotes `## Unreleased` to a numbered section; merging the pull request it opens is what publishes. Merging builds and pushes one image to `ghcr.io/copilotkit/openbot`, signs a build provenance attestation diff --git a/server/src/index.ts b/server/src/index.ts index 462334c9..4cdac0bf 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -44,8 +44,6 @@ import { resolveModelApiKey, } from "./credentials"; import { createDatabase } from "./db/client"; -import { askerFor, createKnowledgeSearch } from "./knowledge/search"; -import { knowledgeSearchTool } from "./knowledge/tool"; import { createPeopleStore } from "./people/store"; import { createPluginStore } from "./plugins/store"; import { grantedTools } from "./plugins/tools"; @@ -266,15 +264,6 @@ const pluginStore = createPluginStore({ policy: () => policyStore.get(), }); -/** - * Reading back what the connectors wrote. - * - * `connectors/sync-persistence.ts` has been filling `documents`, `chunks` and `document_acls`, and - * nothing has ever read them. This is the read half, and it filters on the asker's own principals in - * SQL rather than here. See server/src/knowledge/search.ts. - */ -const knowledgeSearch = createKnowledgeSearch(database); - void recordAuditEvent(bootAuditStore, { eventType: "computer.policy_loaded", targetType: "policy", @@ -399,31 +388,8 @@ const app = createApp( stallGuard, // Tools run here, not in the browser. Each one still executes through the plugin store, so the // grant, the policy and the audit row are exactly where they were. - // - // The knowledge search is beside them rather than inside the plugin store, because it has no - // vendor to reach: it is a query against this deployment's own tables, and it writes its own - // `knowledge.searched` row. It is offered without a per-Bot grant because the ACL filter in the - // query is the access control — the search runs on the asker's principals, so no Bot can return a - // document the person asking could not open themselves. Offered only when there is something to - // search, so a deployment that has connected nothing does not describe a tool that can only - // answer "nothing found". - (actorId) => async (botId) => { - const granted = await grantedTools({ - store: pluginStore, - botId, - actorId, - }); - if (!(await knowledgeSearch.anyDocuments())) return granted; - return [ - ...granted, - knowledgeSearchTool({ - search: knowledgeSearch, - auditStore: bootAuditStore, - asker: await askerFor(database, actorId), - botId, - }), - ]; - }, + (actorId) => (botId) => + grantedTools({ store: pluginStore, botId, actorId }), /* * What the deployment tells a remote Bot about the run it is starting. * diff --git a/server/src/knowledge/search.ts b/server/src/knowledge/search.ts deleted file mode 100644 index c0496228..00000000 --- a/server/src/knowledge/search.ts +++ /dev/null @@ -1,232 +0,0 @@ -import { eq, isNull, sql } from "drizzle-orm"; -import type { Database } from "../db/client"; -import { documents, users } from "../db/schema"; - -/** - * Answering from the company's documents, with only the ones the asker may read. - * - * WHY THIS EXISTS. `connectors/sync-persistence.ts` writes `documents`, `chunks` and `document_acls` - * and nothing has ever read them back. The Knowledge coworker answers as though something were - * behind it, so a deployment that connects a source gets rows in Postgres and no citation. - * - * THE ACL IS THE POINT, and it is evaluated in SQL rather than in this process. A read path that - * fetches rows and filters them here has already fetched them: the wrong document is in memory, it is - * one refactor from being returned, and the query cost scales with the corpus rather than with the - * answer. The predicate below is the boundary, so the database never hands over a row the asker may - * not read. - * - * DENY WINS, AND SILENCE DENIES. A document is readable when some ACL row allows one of the asker's - * principals and no ACL row denies one. A document with no ACL rows at all is readable by nobody, - * which is the same shape the rest of this repo uses for a grant: absence is the refusal, so a - * document whose ACLs failed to sync is invisible rather than public. - * - * WHAT THIS DOES NOT DO is rank by meaning. `chunks.embedding` is `vector(1536)` and nothing in this - * repository has ever written one: `connectors/contract.ts` has the adapter supply embeddings and no - * adapter exists yet, and there is no embedding model in the tenant package or the environment. So - * matching here is PostgreSQL's own full-text search over `chunks.content`, which needs no - * configuration that a deployment does not already have. When an adapter starts writing embeddings, - * the ranking changes behind this signature and the ACL predicate does not move. - */ - -/** - * Who is asking, as principals rather than as a user row. - * - * `groups` is accepted and matched, and is empty in every deployment today: `users.groups` is written - * by nothing (see #82 and #92, which established that both halves of the group control are missing). - * Matching it anyway means a `group:` ACL starts working the day groups arrive from an identity - * provider instead of needing to be found and changed. Until then such a row matches nobody, which - * denies rather than permits. - */ -export type KnowledgeAsker = { - userId: string; - groups: readonly string[]; -}; - -export type KnowledgeCitation = { - documentId: string; - title: string; - /** Where a person opens the document. Stored by the connector, never built here. */ - url: string; - /** The matching passage, marked up by PostgreSQL around the terms that matched. */ - snippet: string; -}; - -export type KnowledgeSearch = { - search: (input: { - asker: KnowledgeAsker; - query: string; - limit?: number; - }) => Promise; - /** - * Whether there is anything to search at all. - * - * Asked so a deployment that has connected nothing does not offer its Bots a tool that can only - * answer "nothing found". A tool in the list is a sentence in the prompt and a call the model may - * spend a step on. - * - * Asked per run rather than at boot, for the reason plugins/tools.ts gives about grants: a source - * connected this morning should work this afternoon, not after a restart. - */ - anyDocuments: () => Promise; -}; - -/** Enough to answer from, few enough to fit a reply. */ -const DEFAULT_LIMIT = 5; -const MAX_LIMIT = 20; - -/** - * The text search configuration. - * - * Named rather than left to `default_text_search_config`, which is a server setting a deployment may - * have changed: the same query would then stem differently on two databases and neither would say so. - * - * Cast to `regconfig` at every use below. Passed as a bound parameter it arrives typed as text, and - * there is no `to_tsvector(text, text)` for PostgreSQL to resolve to — only the one-argument form and - * `to_tsvector(regconfig, text)` — so without the cast the query fails to plan rather than falling - * back to anything. - */ -const TEXT_CONFIG = "english"; - -/** - * How the asker's identity becomes the strings an ACL row is written against. - * - * One shape, in one place, because a mismatch between what is written and what is matched is a - * silent read failure rather than an error. - */ -export function principalsFor(asker: KnowledgeAsker): string[] { - return [ - `user:${asker.userId}`, - ...asker.groups.map((group) => `group:${group}`), - ]; -} - -function boundedLimit(requested: number | undefined): number { - if (requested === undefined || !Number.isFinite(requested)) { - return DEFAULT_LIMIT; - } - return Math.min(MAX_LIMIT, Math.max(1, Math.trunc(requested))); -} - -/** - * The asker, with the groups the deployment holds for them. - * - * A separate read rather than something carried on the request, because the actor a run is for is - * identified by id and nothing downstream of that has ever needed more. `users.groups` is `[]` for - * everybody today; see the note on {@link KnowledgeAsker}. - */ -export async function askerFor( - database: Database, - userId: string, -): Promise { - const [row] = await database - .select({ groups: users.groups }) - .from(users) - .where(eq(users.id, userId)) - .limit(1); - return { userId, groups: row?.groups ?? [] }; -} - -export function createKnowledgeSearch(database: Database): KnowledgeSearch { - return { - async anyDocuments() { - const [row] = await database - .select({ id: documents.id }) - .from(documents) - .where(isNull(documents.deletedAt)) - .limit(1); - return row !== undefined; - }, - - async search({ asker, query, limit }) { - const terms = query.trim(); - /* - * Answered here rather than by the database. `websearch_to_tsquery` turns an empty string into - * an empty query, which matches nothing, so the result would be the same — but a Bot calling - * this with no arguments should not become a query against every chunk in the deployment. - */ - if (terms === "") return []; - - /* - * One bound parameter per principal. - * - * Handing the driver a single JSON array and casting it to `jsonb` read better and was wrong: - * the value arrives already encoded, so `$1::jsonb` was a JSON *string* rather than an array and - * `jsonb_array_elements_text` refused it with "cannot extract elements from a scalar". A list of - * parameters has no encoding to get wrong, and the list is never empty because - * {@link principalsFor} always yields the asker's own `user:` principal. - */ - const principals = sql.join( - principalsFor(asker).map((principal) => sql`${principal}`), - sql`, `, - ); - - /* - * One row per document, not one per chunk. A long document matching in six places is one - * citation, and returning it six times would spend the answer's whole budget on it. - * `row_number` picks the best-ranked chunk per document, tie-broken by position so the same - * corpus and the same question give the same passage twice running. - */ - const rows = await database.execute<{ - document_id: string; - title: string; - canonical_url: string; - snippet: string; - }>( - sql` - with asked as ( - select websearch_to_tsquery(${TEXT_CONFIG}::regconfig, ${terms}) as tsq - ), - matched as ( - select d.id as document_id, - d.title as title, - d.canonical_url as canonical_url, - ts_rank(to_tsvector(${TEXT_CONFIG}::regconfig, c.content), asked.tsq) as rank, - ts_headline( - ${TEXT_CONFIG}::regconfig, - c.content, - asked.tsq, - 'MaxFragments=1, MaxWords=40, MinWords=12' - ) as snippet, - row_number() over ( - partition by d.id - order by ts_rank( - to_tsvector(${TEXT_CONFIG}::regconfig, c.content), - asked.tsq - ) desc, - c.position asc - ) as best - from chunks c - join documents d on d.id = c.document_id - cross join asked - where d.deleted_at is null - and to_tsvector(${TEXT_CONFIG}::regconfig, c.content) @@ asked.tsq - and exists ( - select 1 from document_acls a - where a.document_id = d.id - and a.effect = 'allow' - and a.principal in (${principals}) - ) - and not exists ( - select 1 from document_acls a - where a.document_id = d.id - and a.effect = 'deny' - and a.principal in (${principals}) - ) - ) - select document_id, title, canonical_url, snippet - from matched - where best = 1 - order by rank desc, title asc - limit ${boundedLimit(limit)} - `, - ); - - return [...rows].map((row) => ({ - documentId: row.document_id, - title: row.title, - url: row.canonical_url, - snippet: row.snippet, - })); - }, - }; -} diff --git a/server/src/knowledge/tool.ts b/server/src/knowledge/tool.ts deleted file mode 100644 index 474d8a1c..00000000 --- a/server/src/knowledge/tool.ts +++ /dev/null @@ -1,106 +0,0 @@ -import { z } from "zod"; -import { type AuditStore, recordAuditEvent } from "../audit"; -import type { GrantedTool } from "../plugins/tools"; -import type { KnowledgeAsker, KnowledgeSearch } from "./search"; - -/** - * The company's own documents, as something a Bot can call. - * - * WHY IT IS A TOOL rather than retrieval stapled to the prompt. A Bot that is handed passages before - * it speaks is answering from whatever the retriever guessed the question was. A Bot that calls this - * asks its own question, and the call is a thing the trail can record, the model can be refused, and - * a person can read back. It reaches the model the same way an MCP tool does — through - * `builtInAgentConfiguration`'s `tools`, executed on the server, never registered by a browser. - * - * WHAT MAKES IT SAFE IS THE QUERY, not this file. Every row is filtered against the asker's own - * principals in SQL before it leaves PostgreSQL (see search.ts). So this tool cannot return a - * document the person asking could not have opened themselves, whichever Bot is holding it. That is - * the reason it is offered without a per-Bot grant of its own: the grant would refine who may ask, - * and the ACL already decides what any answer may contain. - * - * SAYING NOTHING IS AN ANSWER. An empty result returns a sentence saying so rather than an empty - * string, because a model handed nothing fills the gap from training and cites a document that does - * not exist. The Knowledge coworker's own prompt already tells it to say when nothing is connected; - * this is the same instruction arriving as data. - */ - -/** Named for the model reading it. A Bot picks a tool by what the name and description promise. */ -export const KNOWLEDGE_TOOL_NAME = "search_company_knowledge"; - -const NOTHING_FOUND = - "No authorized document matched that. Say so plainly rather than answering from memory, and do not cite anything."; - -const parameters = z.object({ - query: z - .string() - .describe("What to look for, in the words a person would use."), -}); - -export function knowledgeSearchTool(options: { - search: KnowledgeSearch; - auditStore: AuditStore; - asker: KnowledgeAsker; - botId: string; -}): GrantedTool { - const { search, auditStore, asker, botId } = options; - - return { - name: KNOWLEDGE_TOOL_NAME, - description: - "Search the company's connected documents and return the matching ones with a link to each. " + - "Use this for any question about company policy, process, or history. Only documents the " + - "person asking is allowed to read are returned.", - parameters, - execute: async (args: unknown) => { - const parsed = parameters.safeParse(args); - /* - * A malformed call is answered, not thrown. The model is mid-run and an exception here ends the - * turn with nothing said; the same reasoning as the refusal path in plugins/tools.ts. - */ - if (!parsed.success) { - return "That search needs a query: a short phrase describing what to look for."; - } - - const citations = await search.search({ - asker, - query: parsed.data.query, - }); - - /* - * The row is written after the search rather than before it, and this is the one place in the - * deployment where that is the right way round. Nothing is being acted on: no document changes, - * nothing leaves the deployment, and the interesting fact is what came back rather than what was - * attempted. `knowledge.searched` was already declared in audit.ts for exactly this. - * - * The query text is recorded, the way a shell command's text is. What is not recorded is any - * passage: `content` is on the redaction list in audit.ts, and a snippet is the document's text - * under another name, so the row names the documents and never quotes them. - */ - await recordAuditEvent(auditStore, { - eventType: "knowledge.searched", - targetType: "knowledge", - targetId: botId, - actorUserId: asker.userId, - payload: { - bot: botId, - query: parsed.data.query, - matched: citations.length, - documents: citations.map((citation) => citation.documentId), - }, - }); - - if (citations.length === 0) return NOTHING_FOUND; - - /* - * Formatted for a model that has to cite it, so each document is one block with its link beside - * its passage. A JSON blob would be read as data to summarise; this reads as sources to quote. - */ - return citations - .map( - (citation) => - `${citation.title}\n${citation.url}\n${citation.snippet}`, - ) - .join("\n\n"); - }, - }; -} diff --git a/server/tests/knowledge-search.integration.test.ts b/server/tests/knowledge-search.integration.test.ts deleted file mode 100644 index 063eb630..00000000 --- a/server/tests/knowledge-search.integration.test.ts +++ /dev/null @@ -1,265 +0,0 @@ -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { randomUUID } from "node:crypto"; -import { eq, inArray } from "drizzle-orm"; -import { createDatabase } from "../src/db/client"; -import { - chunks, - connectorInstances, - documentAcls, - documents, -} from "../src/db/schema"; -import { createKnowledgeSearch } from "../src/knowledge/search"; -import { TEST_POOL } from "./support/database"; - -/** - * The read path, against a real database, because every property worth asserting here is a property - * of the query. - * - * The ACL is evaluated in SQL, so a test that stubbed the database would assert the shape of some - * TypeScript rather than whether the database hands over a document the asker may not read. That is - * the whole claim, so it is made against PostgreSQL: full-text search, `row_number`, and the two - * correlated subqueries are all things only PostgreSQL can be wrong about. - */ - -const database = createDatabase( - process.env.DATABASE_URL ?? - "postgres://openbot:openbot@localhost:5432/openbot", - TEST_POOL, -); - -const search = createKnowledgeSearch(database); - -const suite = randomUUID().slice(0, 8).replace(/-/g, ""); -const asker = { userId: `user_${suite}`, groups: [] as string[] }; - -/** Ids kept so teardown removes exactly what this file made and nothing else. */ -let connectorId = ""; -const documentIds: string[] = []; - -/** - * One document with one chunk and a set of ACL rows. - * - * `embedding` is required by the schema and is not read by this path, so it is a fixed vector rather - * than a pretend one: nothing here should read as though it carried meaning. - */ -async function addDocument(input: { - title: string; - body: string; - acls: { principal: string; effect: "allow" | "deny" }[]; - deleted?: boolean; -}): Promise { - const [document] = await database - .insert(documents) - .values({ - connectorInstanceId: connectorId, - sourceId: `${suite}-${input.title}`, - title: input.title, - canonicalUrl: `https://example.invalid/${encodeURIComponent(input.title)}`, - metadata: {}, - contentHash: randomUUID(), - ...(input.deleted ? { deletedAt: new Date() } : {}), - }) - .returning({ id: documents.id }); - - const id = document?.id as string; - documentIds.push(id); - - await database.insert(chunks).values({ - documentId: id, - position: 0, - content: input.body, - embedding: Array.from({ length: 1536 }, () => 0), - }); - - if (input.acls.length > 0) { - await database.insert(documentAcls).values( - input.acls.map((acl) => ({ - documentId: id, - principal: acl.principal, - effect: acl.effect, - })), - ); - } - - return id; -} - -beforeAll(async () => { - const [instance] = await database - .insert(connectorInstances) - .values({ - type: "google_drive", - status: "succeeded", - sourceMetadata: { suite }, - }) - .returning({ id: connectorInstances.id }); - connectorId = instance?.id as string; -}); - -afterAll(async () => { - // Chunks and ACLs cascade from the document, and documents cascade from the connector instance. - if (documentIds.length > 0) { - await database.delete(documents).where(inArray(documents.id, documentIds)); - } - if (connectorId) { - await database - .delete(connectorInstances) - .where(eq(connectorInstances.id, connectorId)); - } -}); - -describe("answering from what the asker may read", () => { - test("returns a document the asker is allowed, with a link and a passage", async () => { - await addDocument({ - title: "Expense policy", - body: "Meals under seventy five dollars need no receipt. Anything above five hundred needs your manager before you spend it.", - acls: [{ principal: `user:${asker.userId}`, effect: "allow" }], - }); - - const hits = await search.search({ asker, query: "receipt for meals" }); - - expect(hits).toHaveLength(1); - expect(hits[0]?.title).toBe("Expense policy"); - // The URL is the connector's, so a citation opens the document rather than a path built here. - expect(hits[0]?.url).toContain("Expense"); - // A passage rather than the whole chunk, with the matched term marked. This is what makes a - // citation a citation instead of a title. - expect(hits[0]?.snippet).toContain("receipt"); - }); - - test("does not return a document nothing allows", async () => { - // Absence is the refusal. A document whose ACLs failed to sync is invisible, not public, which is - // the same shape every other grant in this repo uses. - await addDocument({ - title: "Unowned runbook", - body: "The checkout certificate expired and nobody owned it.", - acls: [], - }); - - const hits = await search.search({ asker, query: "checkout certificate" }); - expect(hits.map((hit) => hit.title)).not.toContain("Unowned runbook"); - }); - - test("does not return a document allowed to somebody else", async () => { - await addDocument({ - title: "Board minutes", - body: "The board discussed the acquisition timeline in detail.", - acls: [{ principal: "user:someone_else", effect: "allow" }], - }); - - const hits = await search.search({ asker, query: "acquisition timeline" }); - expect(hits.map((hit) => hit.title)).not.toContain("Board minutes"); - }); - - test("a deny beats an allow on the same document", async () => { - /* - * Both rows match this asker. Deny has to win regardless of which row the planner reaches first, - * which is why it is a `not exists` over the whole ACL set rather than an ordering. - */ - await addDocument({ - title: "Salary bands", - body: "Engineering salary bands for the coming year, by level.", - acls: [ - { principal: `user:${asker.userId}`, effect: "allow" }, - { principal: `user:${asker.userId}`, effect: "deny" }, - ], - }); - - const hits = await search.search({ asker, query: "salary bands level" }); - expect(hits.map((hit) => hit.title)).not.toContain("Salary bands"); - }); - - test("a deny on a group beats an allow on the person", async () => { - // The asker is allowed by name and denied through a group they are in. Still denied. - const grouped = { userId: asker.userId, groups: ["contractors"] }; - await addDocument({ - title: "Employee handbook", - body: "Parental leave and sabbatical entitlements for permanent staff.", - acls: [ - { principal: `user:${asker.userId}`, effect: "allow" }, - { principal: "group:contractors", effect: "deny" }, - ], - }); - - const hits = await search.search({ - asker: grouped, - query: "parental leave sabbatical", - }); - expect(hits.map((hit) => hit.title)).not.toContain("Employee handbook"); - }); - - test("a group allow reaches somebody in that group and nobody else", async () => { - /* - * `users.groups` is written by nothing today, so in this deployment the second half of this is - * what actually happens for everybody. It is asserted both ways so that the day groups arrive - * from an identity provider, this test says whether they work rather than needing to be written - * then. - */ - await addDocument({ - title: "Finance calendar", - body: "Quarterly close dates and the reforecast window for each quarter.", - acls: [{ principal: "group:finance", effect: "allow" }], - }); - - const inFinance = await search.search({ - asker: { userId: asker.userId, groups: ["finance"] }, - query: "quarterly close reforecast", - }); - expect(inFinance.map((hit) => hit.title)).toContain("Finance calendar"); - - const notInFinance = await search.search({ - asker, - query: "quarterly close reforecast", - }); - expect(notInFinance.map((hit) => hit.title)).not.toContain( - "Finance calendar", - ); - }); - - test("does not return a document the connector has deleted", async () => { - // A soft delete is how a connector reports something removed at the source. Returning it would - // cite a document that is no longer there. - await addDocument({ - title: "Retired pricing sheet", - body: "Legacy pricing tiers withdrawn from sale last year.", - acls: [{ principal: `user:${asker.userId}`, effect: "allow" }], - deleted: true, - }); - - const hits = await search.search({ asker, query: "legacy pricing tiers" }); - expect(hits.map((hit) => hit.title)).not.toContain("Retired pricing sheet"); - }); - - test("one citation per document, however many passages match", async () => { - // A long document matching in several places is one citation. Returning it once per chunk would - // spend the whole answer on it. - const id = await addDocument({ - title: "Security review", - body: "Rotation of the signing key is scheduled quarterly.", - acls: [{ principal: `user:${asker.userId}`, effect: "allow" }], - }); - await database.insert(chunks).values({ - documentId: id, - position: 1, - content: "The signing key rotation runbook lives with the platform team.", - embedding: Array.from({ length: 1536 }, () => 0), - }); - - const hits = await search.search({ asker, query: "signing key rotation" }); - const mine = hits.filter((hit) => hit.title === "Security review"); - expect(mine).toHaveLength(1); - }); - - test("an empty question asks the database nothing", async () => { - expect(await search.search({ asker, query: " " })).toEqual([]); - }); - - test("the number of citations is bounded whatever is asked for", async () => { - const hits = await search.search({ - asker, - query: "receipt for meals", - limit: 9_999, - }); - expect(hits.length).toBeLessThanOrEqual(20); - }); -}); diff --git a/server/tests/knowledge-tool.test.ts b/server/tests/knowledge-tool.test.ts deleted file mode 100644 index 64c74ac7..00000000 --- a/server/tests/knowledge-tool.test.ts +++ /dev/null @@ -1,174 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import type { AuditEventInput, AuditStore } from "../src/audit"; -import type { - KnowledgeCitation, - KnowledgeSearch, -} from "../src/knowledge/search"; -import { knowledgeSearchTool } from "../src/knowledge/tool"; - -/** - * What the model is handed, and what the trail is left. - * - * The query itself is a property of PostgreSQL and is asserted against a real database in - * knowledge-search.integration.test.ts. Everything here is a property of this file: that an empty - * result becomes a sentence rather than an empty string, that a row is written either way, and that - * no passage reaches the row. - */ - -const asker = { userId: "u_1", groups: ["finance"] }; - -function recorder() { - const written: AuditEventInput[] = []; - const auditStore: AuditStore = { - insert: async (event) => { - written.push(event); - }, - }; - return { written, auditStore }; -} - -function searchReturning(citations: KnowledgeCitation[]): { - search: KnowledgeSearch; - asked: { query: string; userId: string }[]; -} { - const asked: { query: string; userId: string }[] = []; - return { - asked, - search: { - anyDocuments: async () => true, - search: async ({ query, asker: who }) => { - asked.push({ query, userId: who.userId }); - return citations; - }, - }, - }; -} - -const oneCitation: KnowledgeCitation[] = [ - { - documentId: "d_1", - title: "Expense policy", - url: "https://notion.example/expense-policy", - snippet: "Meals under $75 need no receipt.", - }, -]; - -describe("the knowledge tool", () => { - test("hands back each document with its link and its passage", async () => { - const { search } = searchReturning(oneCitation); - const tool = knowledgeSearchTool({ - search, - auditStore: recorder().auditStore, - asker, - botId: "knowledge", - }); - - const answer = await tool.execute({ query: "receipts" }); - - expect(answer).toContain("Expense policy"); - // The link, because a citation the person cannot open is not a citation. - expect(answer).toContain("https://notion.example/expense-policy"); - expect(answer).toContain("receipt"); - }); - - test("says nothing was found rather than returning an empty string", async () => { - // A model handed "" fills the gap from training and cites a document that does not exist. This is - // the same instruction the Knowledge coworker's prompt carries, arriving as data. - const { search } = searchReturning([]); - const tool = knowledgeSearchTool({ - search, - auditStore: recorder().auditStore, - asker, - botId: "knowledge", - }); - - const answer = await tool.execute({ query: "something nobody wrote down" }); - - expect(answer).not.toBe(""); - expect(answer.toLowerCase()).toContain("no authorized document"); - expect(answer.toLowerCase()).toContain("do not cite"); - }); - - test("asks on behalf of the person, not the Bot", async () => { - // The whole boundary. If this ever passed a service identity the ACL filter would be filtering - // against the wrong principals and would still look like it worked. - const { search, asked } = searchReturning(oneCitation); - const tool = knowledgeSearchTool({ - search, - auditStore: recorder().auditStore, - asker, - botId: "knowledge", - }); - - await tool.execute({ query: "receipts" }); - - expect(asked).toHaveLength(1); - expect(asked[0]?.userId).toBe("u_1"); - }); - - test("writes a row naming the documents and never quoting them", async () => { - const { search } = searchReturning(oneCitation); - const { written, auditStore } = recorder(); - const tool = knowledgeSearchTool({ - search, - auditStore, - asker, - botId: "knowledge", - }); - - await tool.execute({ query: "receipts" }); - - expect(written).toHaveLength(1); - const row = written[0]; - expect(row?.eventType).toBe("knowledge.searched"); - expect(row?.actorUserId).toBe("u_1"); - expect(row?.payload.query).toBe("receipts"); - expect(row?.payload.documents).toEqual(["d_1"]); - expect(row?.payload.matched).toBe(1); - - // The passage is the document's text under another name. A trail that quotes it has copied the - // document into a table with a different retention policy and a different audience. - const serialised = JSON.stringify(row?.payload); - expect(serialised).not.toContain("Meals under"); - expect(serialised).not.toContain("receipt."); - }); - - test("records a search that found nothing", async () => { - // "This Bot was asked six times last week and found nothing" is a fact about the corpus, and it - // only exists if the empty case writes a row too. - const { search } = searchReturning([]); - const { written, auditStore } = recorder(); - const tool = knowledgeSearchTool({ - search, - auditStore, - asker, - botId: "knowledge", - }); - - await tool.execute({ query: "nothing" }); - - expect(written).toHaveLength(1); - expect(written[0]?.payload.matched).toBe(0); - expect(written[0]?.payload.documents).toEqual([]); - }); - - test("answers a malformed call instead of ending the run", async () => { - // An exception here ends the turn with nothing said. Same reasoning as the refusal path in - // plugins/tools.ts. - const { search, asked } = searchReturning(oneCitation); - const { written, auditStore } = recorder(); - const tool = knowledgeSearchTool({ - search, - auditStore, - asker, - botId: "knowledge", - }); - - const answer = await tool.execute({ notAQuery: 12 }); - - expect(answer).toContain("needs a query"); - // And nothing was searched or recorded, because nothing was asked. - expect(asked).toHaveLength(0); - expect(written).toHaveLength(0); - }); -});