Skip to content

feat(sheets,calendar): add Sheets write tools and fix Calendar updateEvent to use patch#326

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

feat(sheets,calendar): add Sheets write tools and fix Calendar updateEvent to use patch#326
tuannvm wants to merge 6 commits into
gemini-cli-extensions:mainfrom
tuannvm:feat/sheets-write-tools

Conversation

@tuannvm
Copy link
Copy Markdown
Contributor

@tuannvm tuannvm commented Apr 7, 2026

Summary

Fixes #253 — Adds write tools to the Google Sheets service and fixes the Calendar updateEvent method to use events.patch (partial update) instead of events.update (full replacement).

Sheets Write Tools

Six new tools: sheets.updateRange, sheets.appendRange, sheets.clearRange, sheets.createSpreadsheet, sheets.addSheet, and sheets.deleteSheet. Upgrades the OAuth scope from spreadsheets.readonly to spreadsheets when enabled.

Calendar Fix

updateEvent now uses events.patch instead of events.update, preventing existing event fields (attendees, description, etc.) from being wiped when only updating specific fields.

Use Case

Sheets — 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

Sheets — manage structure:

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

Calendar — safe partial updates:

  1. calendar.updateEvent with eventId and only the fields to change — other fields are preserved

Enabling Sheets Write Tools

Write tools are disabled by default (consistent with slides.write, tasks.read, tasks.write). To enable:

export WORKSPACE_FEATURE_OVERRIDES="sheets.write:on"

This adds the spreadsheets scope to the OAuth flow and registers all 6 write tools. See Feature Configuration for details.

Changes

  • SheetsService.ts: Added updateRange, appendRange, clearRange, createSpreadsheet, addSheet, and deleteSheet methods
  • CalendarService.ts: Changed updateEvent to use events.patch instead of events.update for partial update semantics
  • index.ts: Registered 6 new Sheets write tools with Zod input schemas, using registerTool helper
  • feature-config.ts: Populated sheets.write feature group with tool names (defaultEnabled: false)
  • feature-config.test.ts: Verified sheets.write remains in the off-by-default group
  • SheetsService.test.ts: Added tests covering happy paths, options, and error handling for all write operations
  • CalendarService.test.ts: Updated tests to use events.patch mock

Testing

All 107 relevant tests pass (CalendarService, SheetsService, feature-config).

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>
Copy link
Copy Markdown
Contributor

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

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

Copy link
Copy Markdown
Contributor

@allenhutchison allenhutchison left a comment

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.

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

5 participants