fix(microsoft-excel): clean up dead code found during integration audit#5454
Conversation
- Remove unreachable 'update' operation subblocks (Google Sheets copy-paste leftover — never a selectable dropdown option and not handled by the tool selector or any registered tool) - Fix microsoft_excel_table_add to trim spreadsheetId and OData-escape tableName, matching every other tool in this file - Drop phantom/dead type fields: Google-Sheets-only insertDataOption and responseValueRenderOption (never used), never-populated sheetId/sheetName/title/sheets metadata fields, unused rowIndex, and the unconfigurable majorDimension param (hardcoded to 'ROWS' now that it's inlined)
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Microsoft Graph request hardening: read, write, and table-add tools now wrap worksheet/sheet/table names with Types and API alignment: drops unused Google Sheets–style params and never-populated metadata fields; write tools always send Reviewed by Cursor Bugbot for commit dd765f7. Configure here. |
Greptile SummaryThis PR audits and cleans up the Microsoft Excel integration, removing dead code copied from the Google Sheets integration and fixing real bugs. The changes are well-scoped and the author confirmed correctness via type-check, API validation, and a full vitest run.
Confidence Score: 5/5Safe to merge — all changes are targeted dead-code removal and correctness fixes for a single integration with no shared dependencies. The PR removes code that was never reachable and patches two real but contained bugs (OData escaping and write response field mapping) applied consistently across every call site. Tests pass and API shapes were validated. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(microsoft-excel): fix write output f..." | Re-trigger Greptile |
… gaps
Found in an independent second-pass audit against the live Graph API
docs:
- microsoft_excel_write (v1) transformResponse read
updatedRange/updatedRows/updatedColumns/updatedCells from the Graph
response — none of these fields exist on the workbookRange resource
(the real fields are address/rowCount/columnCount), so these four
output fields were always undefined. Fixed to read the actual
fields, matching what the v2 write tool already does correctly.
- read.ts and write.ts (v1 and v2) built worksheets('name') OData
URLs with only encodeURIComponent, skipping escapeODataString — a
worksheet name containing an apostrophe would produce a malformed
request. Every other tool in this integration already escapes
correctly; read/write were the outliers.
- write.ts (v1) also wasn't trimming spreadsheetId before use,
inconsistent with every other tool here.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dd765f7. Configure here.
Summary
/validate-integrationaudit of Microsoft Excel (tools, block, types, OAuth scopes) against the live Microsoft Graph API docs — every endpoint, method, and request/response shape checked out correct, and scopes (Files.Read/Files.ReadWrite) already cover every operation, so no OAuth scope changes hereupdateoperation subblocks in the legacy block — leftover from a Google Sheets copy-paste,updatewas never a dropdown option and isn't handled by the tool selector or any registered toolmicrosoft_excel_table_addto trimspreadsheetIdand OData-escapetableName, matching every other tool in the file (a table name with an apostrophe would otherwise break the request URL)insertDataOption/responseValueRenderOption(never used for Excel), never-populatedsheetId/sheetName/title/sheetsmetadata fields, unusedrowIndex, and the unconfigurablemajorDimensionparam (now just hardcoded to'ROWS')Type of Change
Testing
bun run type-check— cleanbun run check:api-validation— passedbunx biome check— cleanbunx vitest run tools/microsoft_excel— 9/9 passingChecklist