Repository navigation
fix(server): compact MCP tool schemas (chase 261028 B09) - #1893
Merged
Merged
Conversation
Adapt model-facing schema compaction while retaining full runtime codecs and every GOAT tool. Apply it to image and screen registrations and include reporting guidance when a delegated child links a pull request, including an existing link. Upstream: ed44f251d89d4dae2f8dcf11c24fb3dec9dda4c7 (#25423)
Compact registered descriptors at GOAT's shared listing wrapper, after strict admission captures each full schema. Keep canonical and legacy names, access declarations, analytics, idempotency and screen-code registration intact. Cover tagged-union decoding and catalog-wide compaction through real registrations. Upstream: bc813670d9c428fa42707a0d78c01e03177016ba (#25790)
Expect the compact foreground boolean in each provider's advertised catalog. The HTTP contract test continues to verify that omitted and null inputs decode alike, booleans preserve their values, and invalid types fail before the handler runs. Upstream: bc813670d9c428fa42707a0d78c01e03177016ba (#25790)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GOAT advertises full MCP codec schemas, repeating safe-integer bounds, tagged-object shapes and JSON-string fallbacks in agent context. This compacts the advertised inputs while keeping the full runtime codecs and every existing GOAT tool.
GOAT adjustments
McpHttpServer.ts:1070, after admission captures the full schema. Canonical and legacy names, strict codecs, unknown-argument refusal, analytics, idempotency, access declarations and screen-code hooks stay on their existing paths.GOAT's own context measurement, using its served catalog (same tool counts before/after):
The existing core budget passes for every measured harness. The full listing remains a diagnostic measurement, outside the default core budget; no new upstream context snapshot or second CI ratchet was added.
Not taken
Upstream's consolidation of 18 tools into action tools and the accompanying handler, prompt, mobile and presentation rewrites were excluded. GOAT keeps its registry-backed canonical paths, compatibility aliases, per-action grants, delegated-task behavior and screen-code registration. Both assigned commits were adapted; neither was omitted wholesale.
Rename residue
git show <sha> | rg '^\+.*\b[Tt]3'was inspected for both commits. From ed44f251d8, all added branded names occur in excluded hunks:T3,T3McpToolSummaryAction,T3_MCP_TOOLS,T3_MCP_TOOL_NAMES,t3Action,t3McpToolPresentation,t3ToolSummaryCall,t3_pending_request,t3_pending_request_,t3_queue,t3_queue_,t3_thread_configure,t3_thread_fork,t3_thread_merge_back,t3_thread_send,t3_thread_transfer,t3_thread_transfers,t3code, andt3tools. This includes the old helper comment and T3 Connect wording, neither of which was imported.resolveT3McpSummaryActionandresolveT3McpToolDefinitionalso belong to excluded presentation hunks. bc813670d9 adds no matching names. No new rename rule or retained branded identifier was needed.Initial implementation verification (e8909bc)
nextwas absent; the catalog compaction case failed on the fullgoat_html_renderschema; the tagged registration case failed on the unmerged union.lockf -k /tmp/goat-heavy-check.lock vp test run apps/server/src/mcp/advertisedJsonSchema.test.ts apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts apps/server/src/mcp/toolkits/screen/tools.test.ts apps/server/src/mcp/toolkits/screen/handlers.test.ts— exit 0, 4 files / 26 tests passed.vp env exec node scripts/agent-surface/context-budget.ts --json --check— exit 0; all core budgets pass.--listing full --json— exit 0; measurements above.node scripts/goat-rename.ts --check— exit 0, zero residue and zero dropped paths.git diff --check— exit 0. Source-diff privacy scan found no local identity, session, workspace or credential strings.lockf -k /tmp/goat-heavy-check.lock vp test run apps/server/src/mcp/advertisedJsonSchema.test.ts apps/server/src/mcp/advertisedJsonSchema.registration.test.ts apps/server/src/mcp/McpHttpServer.test.ts apps/server/src/mcp/agentSurface.test.ts apps/server/src/mcp/toolListing.test.ts apps/server/src/mcp/ToolAdmission.test.ts apps/server/src/mcp/McpToolAccess.test.ts apps/server/src/mcp/McpToolAccess.capabilities.test.ts apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts apps/server/src/mcp/toolkits/screen/tools.test.ts apps/server/src/mcp/toolkits/screen/handlers.test.ts— exit 0, 11 files, 197 passed, zero failed, one existing TODO for goat_eval (Agent surface verification: conformance cases waiting on goat_call, goat_eval, goat_describe and handoff wait #778).lockf -k /tmp/goat-heavy-check.lock vp test run apps/server/src/mcp/harnessListing.test.ts apps/server/src/mcp/computerUseRequestSchema.test.ts— exit 0, 2 files / 8 tests passed. The initial changed-file gate exposed old nullable foreground advertising assertions; these initially asserted the compact boolean. Review round 1 below restores the nullable advertisement; runtime null/boolean/invalid-type checks remain intact.urlfails decoding; a valid target plus unknowntypois refused before the handler;{target:{kind:"number",number:1}}reaches the handler. HTTP computer-use calls accept null as omission and reject invalid foreground types through both direct calls and goat_call.lockf -k /tmp/goat-heavy-check.lock vp run check:changed— exit 0 on e8909bc: format, lint, both knip passes, GraphQL generation and both affected-package typechecks pass; 55 test files pass, 1,212 tests pass, zero fail, three existing skips and one existing TODO. Existing lint warnings remain. Complete output was inspected, with non-error compiler suggestions counted separately.Review round 1
Major finding 1 — fixed in 57ef6d3: preserve every nullable alternative, including
provider.update.displayNameandaccentColor, rather than inferring null-as-omission from optionality. Nullable tagged-object inputs still compact their object branches. The full 215-entry catalog audit found 686 lost nullable paths before the fix, including nested fields, and none afterward; raw image and screen registrations are included.Both new regression tests failed against the reviewed compactor. The real provider registration's HTTP
tools/listandgoat_describeschemas now retain clearing; direct calls andgoat_callpreserve omitted/custom/null values and refuse invalid types and unknown arguments without reaching the service.Focused verification:
vp test run apps/server/src/mcp/advertisedJsonSchema.test.ts apps/server/src/mcp/advertisedJsonSchema.registration.test.ts apps/server/src/mcp/computerUseRequestSchema.test.ts apps/server/src/mcp/agentSurface.test.ts apps/server/src/mcp/harnessListing.test.ts apps/server/src/mcp/toolkits/screen/tools.test.ts apps/server/src/mcp/toolkits/screen/handlers.test.ts apps/server/src/mcp/toolkits/provider/handlers.test.ts— exit 0, 8 files / 52 passed / zero failed / one existing TODO.node scripts/goat-rename.ts --checkandgit diff --check— exit 0.vp env exec node scripts/agent-surface/context-budget.ts --json --check— exit 0; core budgets pass. Fresh full-listing measurements above replace the pre-review numbers; preserving null intentionally gives up some compaction.lockf -k /tmp/goat-heavy-check.lock vp run check:changed— exit 0 on57ef6d3a6a3d303bd79cd97b2167ec80522ec2da: format, lint, both knip passes, GraphQL generation and both affected-package typechecks pass; 55 test files / 1,214 tests pass, zero fail, three existing skips and one existing TODO. Existing lint warnings and advisory compiler suggestions remain. The working tree is clean; the runner declined to cache because its inputs changed during the run.NOT RUN: repo-wide checks and UI/dev-server smoke (no visible change); CI completion (handed back without watching or merging).
Model: gpt-6.1-sol via GOAT delegated task (Codex harness)