Skip to content

feat(sheets): add Sheets write tools#326

Open
tuannvm wants to merge 7 commits into
gemini-cli-extensions:mainfrom
tuannvm:feat/sheets-write-tools
Open

feat(sheets): add Sheets write tools#326
tuannvm wants to merge 7 commits into
gemini-cli-extensions:mainfrom
tuannvm:feat/sheets-write-tools

Conversation

@tuannvm

@tuannvm tuannvm commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #253 by adding six Google Sheets write tools behind the off-by-default sheets.write feature group.

Sheets Write Tools

Adds:

  • sheets.updateRange
  • sheets.appendRange
  • sheets.clearRange
  • sheets.createSpreadsheet
  • sheets.addSheet
  • sheets.deleteSheet

When sheets.write is enabled, the extension requests the spreadsheets OAuth scope and registers all six write tools.

Use Cases

Write data

  1. sheets.updateRange with spreadsheetId, range: "Sheet1!A1:B2", and values: [["Name", "Score"], ["Alice", 95]]
  2. sheets.appendRange to add rows after the last data row
  3. sheets.clearRange to clear a specific A1 range

Manage Sheets structure

  1. sheets.createSpreadsheet with title and optional sheetTitles
  2. sheets.addSheet and sheets.deleteSheet to manage tabs

Enabling Sheets Write Tools

Write tools are disabled by default, consistent with other non-published or experimental write scopes. To enable:

export WORKSPACE_FEATURE_OVERRIDES="sheets.write:on"

See Feature Configuration for details.

Changes

  • SheetsService.ts: Added write methods, shared write-error handling with isError: true, URL ID extraction, and non-overclaiming delete confirmation
  • index.ts: Registered six Sheets write tools with Zod input schemas
  • feature-config.ts: Populated the off-by-default sheets.write feature group
  • docs/feature-configuration.md and docs/index.md: Documented the new Sheets write tools
  • SheetsService.test.ts: Added success, option, error, URL extraction, and defensive response-shape coverage
  • CalendarService.ts: Removed out-of-scope public-surface/cosmetic churn from the PR diff

Testing

Local verification:

  • npm run lint
  • npm run format:check
  • npm run test:ci — 528 tests passed
  • npm run build

Open Maintainer Decision

This PR overlaps with #342 on the sheets.write group and append naming: sheets.appendRange here vs sheets.appendRows there. The final naming should be reconciled before merge.

tuannvm added 2 commits March 2, 2026 11:15
Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly expands the Google Workspace integration by adding comprehensive Google Sheets management capabilities and enhancing Google Calendar event handling. New Sheets tools include updating, appending, and clearing ranges, as well as creating spreadsheets and managing individual sheets. The Google Sheets API scope has been updated to allow write access. For Google Calendar, the service now supports adding Google Meet links and file attachments during event creation and updates. A critical improvement was identified regarding the use of the update method in the Calendar service, which performs a full resource replacement and may inadvertently delete existing event data; switching to a patch operation is recommended to ensure partial updates.

Comment thread workspace-server/src/services/CalendarService.ts Outdated
tuannvm added 2 commits April 7, 2026 08:29
Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>

# Conflicts:
#	workspace-server/src/__tests__/services/CalendarService.test.ts
#	workspace-server/src/index.ts
#	workspace-server/src/services/CalendarService.ts
#	workspace-server/src/services/SheetsService.ts
…y default

Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
@tuannvm tuannvm marked this pull request as ready for review April 7, 2026 15:52
@tuannvm tuannvm changed the title Feat/sheets write tools feat(sheets,calendar): add Sheets write tools and fix Calendar updateEvent to use patch Apr 7, 2026
@bdoyle0182

Copy link
Copy Markdown

@allenhutchison is this something we can consider now with custom scoping for consumers of the server being available?

Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
@bdoyle0182

Copy link
Copy Markdown

@allenhutchison i see that slides writes made it in awesome! what do we need to push this one through? getting slides and sheets writes through makes this server very robust and covers most of our users' use case requests

@allenhutchison allenhutchison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

The six Sheets write methods are clean and faithfully follow the existing SheetsService conventions (ID extraction, logging, result shape), the feature-gating is correct (off-by-default sheets.write), and the tests assert exact request shapes including valueInputOption and insertDataOption. Nice work. I've left inline notes on error-signalling and a couple of scope/coordination items; grouping the rest here.

Needs a maintainer decision first

This PR overlaps directly with open #342 on the sheets.write group and on append-tool naming (appendRange here vs appendRows there) — see the inline note on feature-config.ts. Worth resolving before merge so the two PRs don't fight.

Docs parity (please update before merge)

The repo enumerates every tool per feature group, but the 6 new write tools aren't listed:

  • docs/feature-configuration.md — has a ### sheets.read section (line 194) but no ### sheets.write; the scope table (line 25) lists only sheets | read. Add a sheets.write row + section listing the 6 tools.
  • docs/index.md (lines 39-42) — lists only the 3 read tools; add the 6 write tools.

(Same parity expectation that came up on #342.)

Test coverage

Solid — every method has a success test with full request-shape assertions plus an error test. Three low-effort gaps worth closing:

  • URL-vs-raw-ID extraction is never exercised (every test passes a raw id, so the extractDocId(...) branch of extractDocId(spreadsheetId) || spreadsheetId is untested). One test passing a full Sheets URL and asserting the bare id reaches the API covers the shared pattern.
  • appendRange with updates undefined — asserts the ?. path resolves without throwing.
  • addSheet with empty replies — asserts { sheetId: undefined } rather than a throw.

Optional hardening

The Zod schemas validate types but not emptiness — .min(1) on range/title/values and sheetIdz.number().int().nonnegative() would turn late, opaque Google API errors into immediate validation errors. Not a blocker.

Strengths

  • Methods mirror existing SheetsService conventions exactly.
  • valueInputOption and the cell-value union are expressed identically in the TS types and Zod schemas — no drift.
  • createSpreadsheet's optional-sheetTitles branches are both covered.

Review assisted by automated analysis agents; findings verified against the diff.

logToFile(
`[SheetsService] Error during sheets.updateRange: ${errorMessage}`,
);
return {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These six write methods return errors as a normal tool result whose only failure signal is an { error } body — none set isError: true. The MCP client treats the call as success unless it parses the JSON, so a failed write (permission denied, bad range, quota) can be reported to the user as if the mutation succeeded. DriveService (handleError, line 38) and DocsService already set isError: true; the new Sheets methods follow the older read-method convention instead. Recommend adding isError: true to all six catch blocks — ideally extracting a shared handleError like Drive's rather than repeating the block six times. (Same pattern at :295, :342, :397, :448, :499.)

{
type: 'text' as const,
text: JSON.stringify({
message: `Successfully deleted sheet ${sheetId}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteSheet discards the API response and always returns Successfully deleted sheet ${sheetId}. The Sheets batchUpdate API does throw on a genuinely invalid delete (unknown sheetId, deleting the last sheet), so the catch covers the common failures — but the success message is fabricated regardless of what the response actually contained. Combined with the missing isError (see the updateRange thread), any non-throwing edge case yields a confident false confirmation. At minimum, base the success claim on the call completing rather than asserting it unconditionally.

type: 'text' as const,
text: JSON.stringify({
updates: {
updatedRange: response.data.updates?.updatedRange,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

response.data.updates?.updatedRange (and the three sibling fields) will silently produce an all-undefined payload if updates is absent — a "success" that reports zero identifiable cells written. addSheet (replies?.[0]?.addSheet) and createSpreadsheet have the same shape. In practice the API populates these on a real 200, so this is defensive hardening rather than a live bug, but consider returning isError with a "write may not have taken effect" message when the key field is missing, instead of an empty success.

* Attachments are fully replaced (not appended) when provided.
*/
interface EventAttachment {
export interface EventAttachment {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things in this file look like out-of-scope churn for a Sheets-write PR:

  1. export interface EventAttachmentEventAttachment is only referenced inside CalendarService.ts (verified: no external importers), so the new export is dead public surface.
  2. The updateParamspatchParams rename (line 656/669) is purely cosmetic — main already calls calendar.events.patch(...), so this PR doesn't actually change updatepatch as the description claims.

Recommend dropping both so the diff stays scoped to the Sheets tools, and updating the PR description (the "fix Calendar updateEvent to use patch" headline is already true on main).

scopes: scopes('spreadsheets'),
tools: [],
tools: [
'sheets.updateRange',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads-up on a collision: open PR #342 also populates this same sheets.write tools array — with sheets.appendRows, whereas this PR adds sheets.appendRange. So (a) whichever merges first will force a conflict on the other here, and (b) the two introduce overlapping append functionality under different names. This needs a maintainer decision to reconcile (pick one append tool / one naming) before either lands.

Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
@tuannvm tuannvm changed the title feat(sheets,calendar): add Sheets write tools and fix Calendar updateEvent to use patch feat(sheets): add Sheets write tools Jun 6, 2026
@tuannvm tuannvm requested a review from allenhutchison June 6, 2026 05:10
@bdoyle0182

Copy link
Copy Markdown

@allenhutchison I think we should get this one in this week and then cut a release as we'll have both sheets and slides write tools added in

@matheusrmorgado matheusrmorgado left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tuannvm, nice work on this PR. The exact request-shape assertions in the tests and the off-by-default feature gating make this easy to trust, and the centralized handleError with isError: true is a clean response to the earlier review round.

We've been building and battle-testing similar Sheets write tooling for LLM agents internally, and we hit a few edge cases on exactly these API paths. Sharing them in case they're useful. None of these are blocking, and most are one-line description tweaks:

1. appendRange: consider exposing insertDataOption

It's currently hardcoded to INSERT_ROWS. The other mode, OVERWRITE, fills the rows below the detected table without inserting new rows, so it doesn't shift down unrelated content that lives below the table. Both modes are useful in practice: template-filling flows usually want OVERWRITE, while log-style appends want INSERT_ROWS. An optional enum defaulting to INSERT_ROWS would keep current behavior with no breaking change.

2. USER_ENTERED coercion is a real trap for agent-written data

Defaulting to USER_ENTERED is the right call, but agents corrupt data with it more often than you'd expect: "03/04" becomes a date, "0123" loses its leading zero, "TRUE" becomes a boolean. Adding one sentence to the valueInputOption description ("strings that look like dates, numbers, or booleans will be converted; use RAW to store text verbatim") measurably reduced these incidents for us, since models do read tool descriptions.

3. A1 quoting rules in the range descriptions

Sheet names containing spaces or special characters must be single-quoted, and literal apostrophes doubled: 'It''s data'!A1:B2. Models get this wrong surprisingly often. A short example in the range description helped a lot on our side.

4. appendRange table-detection semantics are worth a sentence

values.append appends after the last row of the contiguous data block it detects within range, which is not necessarily the last used row of the sheet. With disjoint data blocks the write can land somewhere unexpected. A brief note in the tool description helps agents anchor the range to the intended table.

5. deleteSheet.sheetId: consider z.number().int().nonnegative()

A float currently passes the schema and only fails at the API with a less actionable message.

Happy to elaborate on any of these. Thanks for pushing Sheets write support forward, we're looking forward to using it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth scope upgrade needed: spreadsheets.readonly → spreadsheets for write tools

6 participants