diff --git a/CHANGELOG.md b/CHANGELOG.md index 91b0bf82f..99ab3a8b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Google Drive search and recent files leave out what is in the trash + +Drive's `files.list` returns trashed files unless the query excludes them, and neither `search_files` +nor `list_recent_files` did. A document somebody had thrown away came back to the Bot as a match or +as a recently changed file, with nothing in its line to say it was in the trash, so the Bot could +answer from it as though it were current. Both now ask Drive to leave the trash out. Reading a file by its +id is unchanged. ### New conversations are still named once some older ones could not be Every pass of the job that names conversations offered at most twenty of those still without a name. diff --git a/server/src/plugins/google-drive-rest.ts b/server/src/plugins/google-drive-rest.ts index e020c0553..52ce99b7b 100644 --- a/server/src/plugins/google-drive-rest.ts +++ b/server/src/plugins/google-drive-rest.ts @@ -260,6 +260,16 @@ function fileLine(file: DriveFile): string { return `- ${parts.join(" · ")}`; } +/** + * Only files that are not in the trash. + * + * `files.list` returns trashed files unless the query says otherwise, so a document somebody had + * thrown away came back as a match, or as a recent file, with nothing in its line to say it was in + * the trash, and a model answers from what it is handed. A file read by its id is still read, trashed + * or not: that is a request for that file. + */ +const NOT_TRASHED = "trashed = false"; + /** * A Drive query string built from what somebody typed. * @@ -267,10 +277,12 @@ function fileLine(file: DriveFile): string { * apostrophe in a search term would otherwise end the clause and change the query's meaning — * searching for `don't` would become a syntax error at best, and at worst a different search than * the one asked for. Escaped, a term is only ever a term. + * + * Bracketed, so the {@link NOT_TRASHED} joined to it applies to both halves of the `or`. */ const driveQuery = (query: string) => { const escaped = query.replace(/\\/g, "\\\\").replace(/'/g, "\\'"); - return `name contains '${escaped}' or fullText contains '${escaped}'`; + return `(name contains '${escaped}' or fullText contains '${escaped}') and ${NOT_TRASHED}`; }; /** @@ -334,7 +346,9 @@ export async function callTool( pageSize: String(PAGE_SIZE), fields: `files(${FILE_FIELDS})`, // Drive's own ordering for "recent". Search leaves it to relevance. - ...(query ? { q: driveQuery(query) } : { orderBy: "modifiedTime desc" }), + ...(query + ? { q: driveQuery(query) } + : { q: NOT_TRASHED, orderBy: "modifiedTime desc" }), }); if (!result.ok) return failure(result.message); diff --git a/server/tests/google-drive-rest.test.ts b/server/tests/google-drive-rest.test.ts index 5ffd6a4c9..303ed020a 100644 --- a/server/tests/google-drive-rest.test.ts +++ b/server/tests/google-drive-rest.test.ts @@ -204,7 +204,7 @@ describe("a search becomes the right Drive request", () => { "https://www.googleapis.com/drive/v3/files", ); expect(url.searchParams.get("q")).toBe( - "name contains 'roadmap' or fullText contains 'roadmap'", + "(name contains 'roadmap' or fullText contains 'roadmap') and trashed = false", ); expect(calls[0].authorization).toBe("Bearer test-token"); }); @@ -220,17 +220,22 @@ describe("a search becomes the right Drive request", () => { const q = new URL(calls[0].url).searchParams.get("q"); expect(q).toBe( - "name contains 'don\\'t ship' or fullText contains 'don\\'t ship'", + "(name contains 'don\\'t ship' or fullText contains 'don\\'t ship') and trashed = false", ); }); - test("recent files are ordered by Drive rather than filtered", async () => { + /* + * `files.list` returns trashed files unless the query excludes them, so a document somebody had + * thrown away came back as a recent file, or as a match above, with nothing in its line to say it + * was in the trash. Both listings ask Drive to leave the trash out. + */ + test("recent files are ordered by Drive, and filtered only by the trash", async () => { const calls = stubFetch({ files: [] }); await callTool(connection, "list_recent_files", {}); const url = new URL(calls[0].url); expect(url.searchParams.get("orderBy")).toBe("modifiedTime desc"); - expect(url.searchParams.has("q")).toBe(false); + expect(url.searchParams.get("q")).toBe("trashed = false"); }); test("a search with nothing to search for is refused before the network", async () => {