Repository navigation
fix(mcp): send the extension name in app tool calls - #439
Conversation
The full TypeScript check hits Node's default heap limit on macOS\nCI. Set a 4 GiB budget only for frontend checks, retaining all\nchecks and regression assertions. Signed-off-by: Kyle De Freitas <kdefreitas@squareup.com>
The build runs the same full TypeScript check as just check, but its separate CI step still exceeded the default 2 GiB heap. Apply the same step-local 4 GiB budget without skipping any checks. Signed-off-by: Kyle De Freitas <kdefreitas@squareup.com>
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
APPROVE. Fresh static review of the exact three-dot comparison found no publishable changed-line defects. The request now sends the extension identity required by the pinned Goose contract while preserving the existing qualified tool name, and the schema, generated request types, validator, client serialization, and focused component expectation agree. The comparison does not change an eligible graphical Berd interface, so visual evidence is not required. Supplied GitHub evidence is structurally valid; nine checks passed and the non-required Transcript virtualization check failed its projection timing budget, so required checks still independently govern merge readiness. Final self-check covered the changed nested MCP app tool-call flow and CI behavior; accessibility; localization; navigation and consent guards; async, error, never-completes, lifecycle, and race behavior; test honesty; design-system and project rules; duplicate and overlap handling; and concrete evidence plus user effect for blocking severity.
Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
Completed checks currently reporting failure:
- Transcript virtualization: failure
Pending checks: 1 check(s) are not complete.
This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.
Summary
MCP apps omit
extensionNamewhen they call a server tool. The pinned Goose backend requires it, but Berd's vendored request schema also omits it. Send the extension name, correct the schema, and regenerate the SDK types and validator. Keep the existing prefixed tool name.The frontend CI check and build steps each get a step-local 4 GiB Node heap budget after TypeScript exhausted the default heap. All checks and regression tests stay enabled.
This is a request-contract fix only, not a complete MCP apps rendering fix. CSP, frame policy, security transport, and the backend pin are unchanged. The affected chat and agent laws remain unchanged; this does not alter message confirmation or dispatch behavior.
Known CI limitation
The non-required Transcript virtualization check failed its
mcp-dynamic-rowsprojection timing budget: 36.2 ms versus 12 ms. The prior run passed all 18 transcript scenarios with identical renderer, test, and dependency files; this update changes only the build step's heap budget. The timing failure is not fixed or waived by this PR. Performance thresholds remain unchanged.Related issue
Adjacent prerequisite for full native validation: #53. No duplicate request-contract PR found in the MCP-related PR search.
Testing
Compared the exact parent and PR with
NODE_OPTIONS=--max-old-space-size=2048 pnpm exec tsc --noEmit --extendedDiagnostics, then repeated with4096. The parent passed at 2 GiB; the PR exhausted its heap. Both passed at 4 GiB. The PR adds 2.5% more type instantiations.Compared the request definition directly with the pinned public Goose schema: exact equality. SDK generation is idempotent.
Public reproduction instructions use the official ext-apps v1.7.5 vanilla JS Get Time app at
92f46a574568a3ddac7600343b7d3c4c4ed7b588. Its Get Server Time button calls the real, key-free clock tool. Source checkout succeeded; dependency installation failed with archive integrity errors, so its build and stdio round trip are unverified.No native screenshots yet. No native app was launched or user profile changed. Native rendering and the guest-button round trip remain blocked pending separate security and frame-policy work, including #53. This request-only PR does not satisfy the native screenshot or full rendering acceptance work. The doc marks those steps as future acceptance work, not observed results.
The full-PR
sq agents reviewreturned no actionable findings after the CI fix.Generated with Goose