diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ab3a8b9..9e6988fa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### The Google Drive connector reaches files in shared drives + +Drive leaves shared drive items out of any `files.get` or `files.list` request that does not say it +supports shared drives, and none of the connector's requests said so. A document the person could +open in a shared drive was "File not found" to `get_file_metadata` and `read_file_content`, and never +appeared in `search_files` or `list_recent_files`. Those requests now say they support shared drives, +and the listings ask for shared drive items. Listings keep Drive's default `user` scope rather than +searching every shared drive. ### 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` diff --git a/server/src/plugins/google-drive-rest.ts b/server/src/plugins/google-drive-rest.ts index 52ce99b7b..069443bc8 100644 --- a/server/src/plugins/google-drive-rest.ts +++ b/server/src/plugins/google-drive-rest.ts @@ -45,6 +45,21 @@ const PAGE_SIZE = 25; const FILE_FIELDS = "id,name,mimeType,modifiedTime,webViewLink,size,owners(emailAddress)"; +/** + * Shared drives as well as My Drive. + * + * Drive leaves shared drive items out of every `files.get` and `files.list` that does not say it + * supports them. A company's documents often live in shared drives, so a file the person could open + * came back from `get_file_metadata` and `read_file_content` as a 404 "File not found", and never + * came back from a search at all. The listing also has to ask for those items. It keeps Drive's + * default `user` corpus, which Google recommends over `allDrives`. + */ +const SHARED_DRIVES = { supportsAllDrives: "true" } as const; +const SHARED_DRIVE_ITEMS = { + ...SHARED_DRIVES, + includeItemsFromAllDrives: "true", +} as const; + /** * Google's editor formats, and the plain-text export each one has. * @@ -343,6 +358,7 @@ export async function callTool( } const result = await request(connection, "/files", { + ...SHARED_DRIVE_ITEMS, pageSize: String(PAGE_SIZE), fields: `files(${FILE_FIELDS})`, // Drive's own ordering for "recent". Search leaves it to relevance. @@ -364,7 +380,7 @@ export async function callTool( const result = await request( connection, `/files/${encodeURIComponent(fileId)}`, - { fields: FILE_FIELDS }, + { ...SHARED_DRIVES, fields: FILE_FIELDS }, ); if (!result.ok) return failure(result.message); @@ -393,7 +409,7 @@ export async function callTool( const metadata = await request( connection, `/files/${encodeURIComponent(fileId)}`, - { fields: "id,name,mimeType" }, + { ...SHARED_DRIVES, fields: "id,name,mimeType" }, ); if (!metadata.ok) return failure(metadata.message); const file = (await metadata.response.json()) as DriveFile; @@ -426,6 +442,7 @@ export async function callTool( { mimeType: exportAs }, ) : await request(connection, `/files/${encodeURIComponent(fileId)}`, { + ...SHARED_DRIVES, alt: "media", }); if (!content.ok) return failure(content.message); diff --git a/server/tests/google-drive-rest.test.ts b/server/tests/google-drive-rest.test.ts index 303ed020a..4053f7231 100644 --- a/server/tests/google-drive-rest.test.ts +++ b/server/tests/google-drive-rest.test.ts @@ -399,3 +399,49 @@ describe("reading a file asks Drive what it is first", () => { expect(result.text.split("\n\n[truncated")[0]).toBe(`${heading}${filler}`); }); }); + +/* + * Drive leaves shared drive items out of any `files.get` or `files.list` that does not say it supports + * them. Without these parameters a document the person could open in a shared drive was a 404 by id + * and missing from every search, and shared drives are where many companies keep their documents. + */ +describe("a file in a shared drive is reached like one in My Drive", () => { + test("both listings ask Drive for shared drive items", async () => { + const calls = stubFetch({ files: [] }); + await callTool(connection, "search_files", { query: "roadmap" }); + await callTool(connection, "list_recent_files", {}); + + expect(calls).toHaveLength(2); + for (const call of calls) { + const params = new URL(call.url).searchParams; + expect(params.get("supportsAllDrives")).toBe("true"); + expect(params.get("includeItemsFromAllDrives")).toBe("true"); + } + }); + + test("looking a file up says the caller supports shared drives", async () => { + const calls = stubFetch({ id: "shared1", name: "Plan" }); + await callTool(connection, "get_file_metadata", { fileId: "shared1" }); + + expect(calls).toHaveLength(1); + expect(new URL(calls[0].url).searchParams.get("supportsAllDrives")).toBe( + "true", + ); + }); + + test("reading a file says so on the lookup and on the download", async () => { + const calls = stubFetch({ + id: "shared2", + name: "notes.txt", + mimeType: "text/plain", + }); + await callTool(connection, "read_file_content", { fileId: "shared2" }); + + expect(calls).toHaveLength(2); + for (const call of calls) { + expect(new URL(call.url).searchParams.get("supportsAllDrives")).toBe( + "true", + ); + } + }); +});