Skip to content

Commit 78ded35

Browse files
committed
Seed post-create read state for #87
createSpreadsheet now records the same metadata-only state as readSpreadsheet after Drive has created the file, including when optional initial data fails. Bookkeeping failures preserve the successful payload and return safe reread guidance. copyFile now branches on the destination MIME type. Docs are re-fetched for their exact content and revision before receiving a read handle, Sheets receive the exact trackRead(id) state their read path creates, and arbitrary binary copies remain deliberately unread because no trustworthy content snapshot exists. Regression coverage proves create-then-write and batch-write, copy-then-write for Docs and Sheets, the intentional binary rejection, and safe success when a Docs seed fetch fails.
1 parent a95bf30 commit 78ded35

6 files changed

Lines changed: 352 additions & 5 deletions

File tree

dist/tools/drive/copyFile.js

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { publicError, isPublicError, wrapOperationError } from '../../errors.js';
22
import { z } from 'zod';
3-
import { getDriveClient } from '../../clients.js';
3+
import { getDriveClient, getDocsClient } from '../../clients.js';
4+
import { docsJsonToMarkdown } from '../../markdown-transformer/index.js';
5+
import { trackRead } from '../../readTracker.js';
6+
import { mintDocsReadHandle } from '../../docsHandles.js';
7+
8+
const GOOGLE_DOC_MIME_TYPE = 'application/vnd.google-apps.document';
9+
const GOOGLE_SHEET_MIME_TYPE = 'application/vnd.google-apps.spreadsheet';
410
export function register(server) {
511
server.addTool({
612
name: 'copyFile',
@@ -51,14 +57,62 @@ export function register(server) {
5157
const response = await drive.files.copy({
5258
fileId: args.fileId,
5359
requestBody: copyMetadata,
54-
fields: 'id,name,webViewLink',
60+
fields: 'id,name,webViewLink,mimeType,modifiedTime',
5561
supportsAllDrives: true,
5662
});
5763
const copiedFile = response.data;
64+
let readHandle;
65+
let readStateWarning;
66+
if (copiedFile.mimeType === GOOGLE_DOC_MIME_TYPE) {
67+
// A Docs mutation needs the exact content and revision it
68+
// is based on. Fetch the copied document rather than
69+
// treating the source copy operation as a content read.
70+
try {
71+
const docs = await getDocsClient();
72+
const seedRes = await docs.documents.get({ documentId: copiedFile.id, fields: '*' });
73+
const contentSource = seedRes.data;
74+
const markdownContent = docsJsonToMarkdown(contentSource);
75+
trackRead(copiedFile.id, copiedFile.modifiedTime, markdownContent, seedRes.data.revisionId);
76+
const minted = await mintDocsReadHandle({
77+
documentId: copiedFile.id,
78+
tabId: null,
79+
revisionId: seedRes.data.revisionId ?? null,
80+
contentSource,
81+
content: markdownContent,
82+
});
83+
readHandle = minted?.readHandle;
84+
}
85+
catch (seedError) {
86+
// The Drive copy already succeeded. Do not turn a
87+
// failed post-copy read into an orphaned file; leave
88+
// it unseeded so the next mutation fails closed.
89+
log.warn(`Copied Google Doc ${copiedFile.id} but read state could not be seeded: ${seedError.message}`);
90+
readStateWarning = 'The Google Doc copy was created, but its read state could not be seeded. Call readDocument before the next mutation.';
91+
}
92+
}
93+
else if (copiedFile.mimeType === GOOGLE_SHEET_MIME_TYPE) {
94+
// Sheets reads intentionally record no content or revision,
95+
// so a copied Sheet must use that same honest baseline.
96+
try {
97+
trackRead(copiedFile.id);
98+
}
99+
catch (seedError) {
100+
log.warn(`Copied Google Sheet ${copiedFile.id} but read state could not be seeded: ${seedError.message}`);
101+
readStateWarning = 'The Google Sheet copy was created, but its read state could not be seeded. Call readSpreadsheet before the next mutation.';
102+
}
103+
}
104+
// Arbitrary binary copies deliberately stay unseeded. Some
105+
// generic mutations (for example deleteFile) are guarded, but
106+
// copyFile has no content snapshot for a binary destination;
107+
// claiming a read here would silently weaken that guard.
58108
return JSON.stringify({
59109
id: copiedFile.id,
60110
name: copiedFile.name,
61111
url: copiedFile.webViewLink,
112+
...(readHandle && {
113+
readHandleNote: 'This document copy has been seeded as read. You can mutate it immediately without calling readDocument first.',
114+
}),
115+
...(readStateWarning && { warnings: [readStateWarning] }),
62116
}, null, 2);
63117
}
64118
catch (error) {

dist/tools/sheets/createSpreadsheet.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { UserError, wrapOperationError } from '../../errors.js';
22
import { z } from 'zod';
33
import { getDriveClient, getSheetsClient } from '../../clients.js';
44
import * as SheetsHelpers from '../../googleSheetsApiHelpers.js';
5+
import { trackRead } from '../../readTracker.js';
56
export function register(server) {
67
server.addTool({
78
name: 'createSpreadsheet',
@@ -51,11 +52,26 @@ export function register(server) {
5152
initialDataStatus = 'failed';
5253
}
5354
}
55+
// Match readSpreadsheet's deliberately metadata-only state.
56+
// The Drive file exists even if its optional initial-data write
57+
// failed, and the guard is about whether this tool just created
58+
// the target, not whether every follow-up request succeeded.
59+
// Tracking is best effort: a bookkeeping failure must never hide
60+
// a successfully-created spreadsheet from the caller.
61+
let readStateWarning;
62+
try {
63+
trackRead(spreadsheetId);
64+
}
65+
catch (seedError) {
66+
log.warn(`Spreadsheet ${spreadsheetId} created but read state could not be seeded: ${seedError.message}`);
67+
readStateWarning = 'Spreadsheet was created, but its read state could not be seeded. Call readSpreadsheet before the next mutation.';
68+
}
5469
return JSON.stringify({
5570
id: spreadsheetId,
5671
name: driveResponse.data.name,
5772
url: driveResponse.data.webViewLink,
5873
...(initialDataStatus ? { initialData: initialDataStatus } : {}),
74+
...(readStateWarning ? { warnings: [readStateWarning] } : {}),
5975
}, null, 2);
6076
}
6177
catch (error) {

tests/copyFile.test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import { describe, expect, it, jest } from '@jest/globals';
99
let fakeDrive;
1010
jest.unstable_mockModule('../dist/clients.js', () => ({
1111
getDriveClient: async () => fakeDrive,
12+
// copyFile only asks for a Docs client when Drive identifies the new
13+
// destination as a Google Doc. These #124 schema tests use generic
14+
// metadata, but the named export must still exist for ESM linking.
15+
getDocsClient: async () => { throw new Error('Docs client not used in this suite'); },
1216
}));
1317

1418
const { register } = await import('../dist/tools/drive/copyFile.js');
Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
// #87: a successful create or copy must seed only the read state that the
2+
// destination type can honestly support. Docs receive their fetched content
3+
// and revision, Sheets match readSpreadsheet's metadata-only tracker entry,
4+
// and arbitrary binaries stay unread because no mutation-safe content view is
5+
// available for them.
6+
import { describe, expect, it, jest } from '@jest/globals';
7+
8+
let fakeDocs;
9+
let fakeDrive;
10+
let fakeSheets;
11+
12+
jest.unstable_mockModule('../dist/clients.js', () => ({
13+
getDocsClient: async () => fakeDocs,
14+
getDriveClient: async () => fakeDrive,
15+
getSheetsClient: async () => fakeSheets,
16+
getAuthClient: async () => { throw new Error('client not used in this suite'); },
17+
getAuthClientIfReady: () => null,
18+
getCalendarClient: async () => { throw new Error('client not used in this suite'); },
19+
getFormsClient: async () => { throw new Error('client not used in this suite'); },
20+
getGmailClient: async () => { throw new Error('client not used in this suite'); },
21+
getScriptClient: async () => { throw new Error('client not used in this suite'); },
22+
getSlidesClient: async () => { throw new Error('client not used in this suite'); },
23+
getTasksClient: async () => { throw new Error('client not used in this suite'); },
24+
resetClients: () => {},
25+
withAuthRetry: (fn) => fn(),
26+
}));
27+
28+
const { register: registerCreateSpreadsheet } = await import('../dist/tools/sheets/createSpreadsheet.js');
29+
const { register: registerWriteSpreadsheet } = await import('../dist/tools/sheets/writeSpreadsheet.js');
30+
const { register: registerBatchWrite } = await import('../dist/tools/sheets/batchWrite.js');
31+
const { register: registerCopyFile } = await import('../dist/tools/drive/copyFile.js');
32+
const { register: registerAppendText } = await import('../dist/tools/docs/appendToGoogleDoc.js');
33+
const { register: registerDeleteFile } = await import('../dist/tools/drive/deleteFile.js');
34+
35+
const MODIFIED_TIME = '2026-08-31T12:00:00.000Z';
36+
const REVISION = 'copied-doc-revision';
37+
const log = { info() {}, warn() {}, error() {} };
38+
39+
let idSequence = 0;
40+
function freshId(label) {
41+
idSequence += 1;
42+
return `${label}-${idSequence}`;
43+
}
44+
45+
function getTool(register) {
46+
let tool;
47+
register({ addTool(definition) { tool = definition; } });
48+
return tool;
49+
}
50+
51+
function docPayload(id) {
52+
return {
53+
data: {
54+
id,
55+
revisionId: REVISION,
56+
body: {
57+
content: [{
58+
startIndex: 1,
59+
endIndex: 16,
60+
paragraph: {
61+
elements: [{
62+
startIndex: 1,
63+
endIndex: 16,
64+
textRun: { content: 'copied content\n' },
65+
}],
66+
},
67+
}],
68+
},
69+
},
70+
};
71+
}
72+
73+
function setGoogleMocks(id, {
74+
copiedMimeType = 'application/vnd.google-apps.spreadsheet',
75+
initialDataFails = false,
76+
docsSeedFails = false,
77+
} = {}) {
78+
let valuesUpdateCalls = 0;
79+
const valuesUpdate = jest.fn(async () => {
80+
valuesUpdateCalls += 1;
81+
if (initialDataFails && valuesUpdateCalls === 1) {
82+
throw new Error('simulated initial-data failure');
83+
}
84+
return { data: { updatedCells: 1, updatedRows: 1, updatedColumns: 1 } };
85+
});
86+
const documentsGet = jest.fn(async () => {
87+
if (docsSeedFails) throw new Error('simulated Docs seed failure');
88+
return docPayload(id);
89+
});
90+
const batchUpdate = jest.fn(async ({ requestBody }) => ({
91+
data: { writeControl: requestBody.writeControl },
92+
}));
93+
94+
fakeDocs = { documents: { get: documentsGet, batchUpdate } };
95+
fakeSheets = {
96+
spreadsheets: {
97+
values: {
98+
update: valuesUpdate,
99+
batchUpdate: jest.fn(async () => ({
100+
data: {
101+
totalUpdatedCells: 1,
102+
totalUpdatedRows: 1,
103+
totalUpdatedColumns: 1,
104+
totalUpdatedSheets: 1,
105+
},
106+
})),
107+
},
108+
},
109+
};
110+
const filesCreate = jest.fn(async () => ({
111+
data: { id, name: 'Created spreadsheet', webViewLink: `https://sheets/${id}` },
112+
}));
113+
const filesCopy = jest.fn(async () => ({
114+
data: {
115+
id,
116+
name: 'Copied file',
117+
webViewLink: `https://drive/${id}`,
118+
mimeType: copiedMimeType,
119+
modifiedTime: MODIFIED_TIME,
120+
},
121+
}));
122+
const filesGet = jest.fn(async ({ fileId }) => {
123+
if (fileId === 'source-file') {
124+
return { data: { name: 'Source file', parents: ['source-parent'] } };
125+
}
126+
return {
127+
data: {
128+
name: 'Copied file',
129+
mimeType: copiedMimeType,
130+
modifiedTime: MODIFIED_TIME,
131+
},
132+
};
133+
});
134+
fakeDrive = {
135+
files: {
136+
create: filesCreate,
137+
copy: filesCopy,
138+
get: filesGet,
139+
update: jest.fn(async () => ({ data: {} })),
140+
delete: jest.fn(async () => ({ data: {} })),
141+
},
142+
};
143+
return { documentsGet, batchUpdate, filesCreate, filesCopy, valuesUpdate };
144+
}
145+
146+
describe('create and copy read seeding (#87)', () => {
147+
it('createSpreadsheet then writeSpreadsheet succeeds without a redundant read', async () => {
148+
const id = freshId('created-sheet-write');
149+
const { filesCreate, valuesUpdate } = setGoogleMocks(id);
150+
151+
const created = await getTool(registerCreateSpreadsheet).execute({ title: 'Created sheet' }, { log });
152+
expect(JSON.parse(created).id).toBe(id);
153+
const written = await getTool(registerWriteSpreadsheet).execute({
154+
spreadsheetId: id, range: 'A1', values: [['created then written']],
155+
}, { log });
156+
157+
expect(written).toMatch(/Successfully wrote 1 cells/);
158+
expect(filesCreate).toHaveBeenCalledTimes(1);
159+
expect(valuesUpdate).toHaveBeenCalledTimes(1);
160+
});
161+
162+
it('createSpreadsheet still seeds when optional initial data fails, so batchWrite succeeds', async () => {
163+
const id = freshId('created-sheet-batch');
164+
setGoogleMocks(id, { initialDataFails: true });
165+
166+
const created = await getTool(registerCreateSpreadsheet).execute({
167+
title: 'Created sheet', initialData: [['this write fails']],
168+
}, { log });
169+
expect(JSON.parse(created)).toMatchObject({ id, initialData: 'failed' });
170+
const written = await getTool(registerBatchWrite).execute({
171+
spreadsheetId: id,
172+
data: [{ range: 'A1', values: [['batch write after create']] }],
173+
}, { log });
174+
175+
expect(written).toMatch(/Successfully batch-wrote 1 cells/);
176+
});
177+
178+
it('copyFile seeds a Google Doc from a fetched content and revision snapshot before appendText', async () => {
179+
const id = freshId('copied-doc');
180+
const { batchUpdate, documentsGet } = setGoogleMocks(id, {
181+
copiedMimeType: 'application/vnd.google-apps.document',
182+
});
183+
184+
const copied = await getTool(registerCopyFile).execute({ fileId: 'source-file' }, { log });
185+
expect(JSON.parse(copied).id).toBe(id);
186+
const appended = await getTool(registerAppendText).execute({
187+
documentId: id, text: 'after copy',
188+
}, { log });
189+
190+
expect(appended).toMatch(/Successfully appended text/);
191+
expect(documentsGet).toHaveBeenCalled();
192+
expect(batchUpdate.mock.calls[0][0].requestBody.writeControl).toEqual({ requiredRevisionId: REVISION });
193+
});
194+
195+
it('copyFile seeds a Google Sheet with the same metadata-only state as readSpreadsheet', async () => {
196+
const id = freshId('copied-sheet');
197+
setGoogleMocks(id, { copiedMimeType: 'application/vnd.google-apps.spreadsheet' });
198+
199+
const copied = await getTool(registerCopyFile).execute({ fileId: 'source-file' }, { log });
200+
expect(JSON.parse(copied).id).toBe(id);
201+
const written = await getTool(registerWriteSpreadsheet).execute({
202+
spreadsheetId: id, range: 'A1', values: [['after copy']],
203+
}, { log });
204+
205+
expect(written).toMatch(/Successfully wrote 1 cells/);
206+
});
207+
208+
it('keeps arbitrary binary copies unread, because deleteFile is guarded but copyFile did not read their content', async () => {
209+
const id = freshId('copied-binary');
210+
setGoogleMocks(id, { copiedMimeType: 'application/pdf' });
211+
212+
await getTool(registerCopyFile).execute({ fileId: 'source-file' }, { log });
213+
214+
// Intentional: deleteFile is the generic guarded mutation, but an
215+
// arbitrary binary copy has no trustworthy content snapshot here.
216+
await expect(getTool(registerDeleteFile).execute({ fileId: id }, { log }))
217+
.rejects.toThrow(/has not been read in this session/i);
218+
});
219+
220+
it('returns the successful copied-Doc payload when its best-effort seed fetch fails', async () => {
221+
const id = freshId('copied-doc-seed-fail');
222+
const { documentsGet, filesCopy } = setGoogleMocks(id, {
223+
copiedMimeType: 'application/vnd.google-apps.document', docsSeedFails: true,
224+
});
225+
226+
const copied = await getTool(registerCopyFile).execute({ fileId: 'source-file' }, { log });
227+
228+
expect(JSON.parse(copied)).toMatchObject({
229+
id,
230+
name: 'Copied file',
231+
url: `https://drive/${id}`,
232+
warnings: ['The Google Doc copy was created, but its read state could not be seeded. Call readDocument before the next mutation.'],
233+
});
234+
expect(filesCopy).toHaveBeenCalledTimes(1);
235+
expect(documentsGet).toHaveBeenCalledTimes(1);
236+
});
237+
});

0 commit comments

Comments
 (0)