fix(office-excel): support Office.js add-in embed and surface Graph errors#4479
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Extends the chat embed CSP ( Standardizes Microsoft Graph error surfacing across Excel APIs/tools by adding shared Graph error parsers/extractors ( Reviewed by Cursor Bugbot for commit 97e82d8. Configure here. |
Greptile SummaryThis PR adds Office.js add-in support to the
Confidence Score: 5/5Safe to merge; changes are well-scoped with good test coverage and no regressions to existing error paths. The Graph error extraction logic is thoroughly unit-tested (9 cases covering chain depth, deduplication, fallback to code, non-JSON bodies). The framework's tool executor handles non-OK responses before calling transformResponse, so the removal of the manual !response.ok guard in worksheet_add.ts is correct. The Office.js history patch follows the documented community fix. The one inconsistency in csp.ts has no current production impact. apps/sim/lib/core/security/csp.ts — script-src inconsistency is a latent risk if dynamic script sources are ever added. Important Files Changed
Sequence DiagramsequenceDiagram
participant E as Excel Add-in
participant P as ChatPage (Server)
participant OI as OfficeEmbedInit (Client)
participant OJS as Office.js CDN
participant T as Excel Tool (executor)
participant G as Microsoft Graph API
E->>P: GET /chat/[id]?embed=office
P->>P: isOfficeEmbed = true
P->>OI: render OfficeEmbedInit
OI->>OI: cache history.replaceState & pushState
OI->>OJS: load office.js (afterInteractive)
OJS->>OJS: monkey-patches history methods
OJS-->>OI: onReady fires
OI->>OI: restore cached history methods
OI->>OJS: Office.onReady()
Note over T,G: Tool execution flow
T->>G: PATCH /workbook/range (with retry)
alt Success
G-->>T: 200 OK
T->>T: transformResponse
else Graph Error
G-->>T: 4xx/5xx + error body
T->>T: parseGraphErrorFromData
T-->>T: throw human-readable error
end
Reviews (2): Last reviewed commit: "fix(office-excel): delegate to parseGrap..." | Re-trigger Greptile |
|
@greptile |
|
@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 97e82d8. Configure here.
…rrors (#4479) * fix(office-excel): support Office.js add-in embed and surface Graph errors * fix(office-excel): delegate to parseGraphErrorFromData and handle array embed param
Summary
Office.onReady()on/chat/[id]?embed=officeso Excel/Word/PowerPoint/Outlook recognize the page as a valid add-inhistory.replaceState/pushStatebefore Office.js loads and restoring after (documented community fix)https://appsforoffice.microsoft.commicrosoft-graph-errorsextractor and wire it through Excel tools (read, write, table_add, worksheet_add) so users see real Graph error messages instead of "We're sorry. We ran into a problem"/api/tools/microsoft_excel/{drives,sheets}routes with the sharedextractGraphErrorhelperType of Change
Testing
utils.test.ts9,csp.test.ts28)bun run check:api-validationpassesChecklist