Repository navigation
Conversation
AI assistance: OpenAI Codex; research support from Instinct and Jev.
khandrew1
requested changes
Sep 28, 2026
khandrew1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for this, the fix is correct. typeof [] === "object" lets a JSON array through as functionResponse.response, and Gemini requires a Struct there. The one-line Array.isArray guard is the right change. I checked it against multi-block text, isError, structuredContent and image results, and none of them regress.
Before merging, could you trim the tests?
- Keep the
it.eachtable intoGeminiContents. The three array rows (empty array, array of records, MCP text containing a JSON array) cover the fix. The scalar and object rows are cheap and pin existing behavior, so they're fine to keep. - Drop the
serializes array tool results as an object in provider requests (streaming=%s)test.chatandstreamChatboth build their request throughbuildBody→toGeminiContents, so this only re-tests the wiring the table already covers, and it adds a globalfetchstub, SSE fixtures and theafterEach(vi.unstubAllGlobals). Please also revert the extra imports (afterEach,vi,chat,streamChat) that only this test uses.
The changeset looks good as is.
Signed-off-by: Charan Rathore <[email protected]>
tonxxd
added a commit
that referenced
this pull request
Sep 30, 2026
* fix(inspector): release the upstream stream on Express client disconnect (#2394) * fix(inspector): release the upstream stream on Express client disconnect The Express adapter proxied responses with a read loop whose only exit was upstream EOF, and passed no signal into the outbound fetch. A client leaving during a long-lived SSE response left the loop writing to a dead socket while the upstream connection stayed open. Wire an AbortController to res close and thread its signal through the fetch and the read loop, matching packages/server/src/node-bridge.ts. * fix(inspector): carry the abort through the proxy's outbound fetch The Express adapter aborted on client disconnect but the signal stopped at the Hono proxy, so the outbound fetch and the upstream MCP connection kept running. Pass the inbound signal into that fetch, which covers every iteration of the manual redirect loop, and stop forwarding the resulting abort to Express as an application error. The existing test aborted only after the first chunk, so it proved reader cancellation and would have passed without this. Added a case that disconnects before the upstream sends headers, where cancelling the reader cannot help, and cleared the race timeout so it stops holding the worker. * fix(inspector): narrow abort handling to the expected AbortError * fix(inspector): match the abort by reason, not only by error shape fetch rejects with signal.reason verbatim, and the reason is not ours to choose. The Express middleware calls abort() with no argument, so there it is a DOMException named AbortError. @hono/node-server, which backs the standalone inspector and the vite dev API, aborts with a plain string instead, so an instanceof Error test misses every disconnect on that path and the request still ends in a logged stack and a 500 written to a socket that has already gone. Accept the reason by identity as well, which covers both without assuming who created the signal. * fix(inspector): preserve stream failure semantics and await buffered proxy reads Two follow-ups from review. res.end() moves out of the finally block onto the success path. Ending there also ended the downstream response when reader.read() threw on a genuine upstream failure, so the Express error handler received the error with res.writableEnded already true and could no longer change the response. Only listener cleanup stays in finally. proxyResponse(response) gains return await. It awaits response.arrayBuffer() for non-streaming responses, and returning the promise unawaited let a later rejection bypass the catch that turns a client disconnect into a quiet cancellation. With the signal now propagated, disconnecting after JSON headers arrive but before the body completes rejects that read. Regression coverage for both. The stream case destroys the upstream socket after its first chunk and asserts the client cannot read the response to completion, which fails if res.end() runs in finally. The buffered case disconnects after headers and asserts the server still serves the next request rather than taking it down with an escaped rejection. Both assert on observable behaviour rather than on metadata, since the existing test of this path asserts only that a handler was mounted and so cannot fail when the behaviour breaks. * fix(inspector): do not end an aborted response, and stop overclaiming a test Both Cubic findings on the previous commit are valid; verified each by running it rather than by reading. Cancelling the reader resolves a pending read with done, so the loop broke and res.end() ran on a response the client had already abandoned. res.end() is now guarded by !signal.aborted, so only a completed upstream stream ends the downstream response. The buffered-body test claimed to guard the return await change and did not. Measured: reverting the await leaves all tests green, so the claim was false. Driving the Fetch handler directly and asserting a 499, as suggested, does not distinguish it either, because every layer below also catches the escaped rejection. Rather than ship a test whose docstring asserts something it cannot detect, the test now says what it does prove, that an abort mid buffered-body settles as 499 rather than rejecting, and says plainly that it is a behaviour lock and not a regression guard for that change. It also synchronises on the upstream having sent headers instead of a fixed timeout, so the abort lands mid-body rather than in the pre-header path. Coverage, stated honestly: the stream fix is guarded, and reverting it fails the stream test. The abort guard and the return await are not covered. Both are correct by inspection and neither produced an observable difference from outside the proxy in anything I tried. If there is a shape that distinguishes them, I will add it. * chore: enter canary prerelease mode * chore(typescript): version packages (canary) * fix(client): decode MCP App HTML blobs as UTF-8 (#2632) * chore(typescript): version packages (canary) * fix(server): forward cancellation to proxied prompts (#2568) * fix(server): forward cancellation to proxied prompts Signed-off-by: Elioooon <[email protected]> * test(server): require proxied prompt invocation --------- Signed-off-by: Elioooon <[email protected]> * chore(typescript): version packages (canary) * fix(cli): route mcp-use servers env --help to env command help (#2690) * chore(typescript): version packages (canary) * fix(cli): reject an inline value on boolean flags instead of discarding it (#2521) * fix(cli): reject an inline value on boolean flags instead of discarding it parseArgs split `--flag=value` for every `--` token but the boolean cases never read the inline value, so `--tunnel=false` parsed as `--tunnel` and opened a public tunnel. takeValue now records that it consumed the inline value, and any flag reaching the end of the loop with an unconsumed inline value is rejected. Bare booleans, `--flag value`, and `--flag=value` on value-taking flags are unaffected. * chore(cli): bump the changeset to minor for the rejected inline value Rejecting `--tunnel=false`, which earlier versions accepted, changes behaviour for existing invocations, so a patch bump understates it. * test(cli): fold the boolean-flag check into the existing parser suite Per review: drop the separate test file and keep a single case in the existing parseArgs suite. The bare-form behaviour of every boolean flag is already covered there, so the remaining cases were redundant. --------- Co-authored-by: Aishwary Dongre <[email protected]> * chore(typescript): version packages (canary) * fix(agent): stop dispatching tools in runToolLoopNonStreaming when signal is aborted (#2667) * fix(agent): stop dispatching tools in runToolLoopNonStreaming when signal is aborted * test(agent): add abort signal coverage for OpenAIResponsesDriver.runToolLoopNonStreaming * test(agent): use afterEach restoreAllMocks for fetch spy cleanup * chore(typescript): version packages (canary) * chore(deps-dev): bump vitest from 4.1.10 to 4.1.11 in /libraries/typescript/packages/server/examples/sentry (#2627) * chore(deps-dev): bump vitest Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 4.1.10 to 4.1.11. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.11/packages/vitest) --- updated-dependencies: - dependency-name: vitest dependency-version: 4.1.11 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> * chore: add changeset for dependabot updates --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore(typescript): version packages (canary) * fix(agent): wrap Gemini array tool results in an object (#2675) * fix(agent): wrap Gemini array tool results in an object AI assistance: OpenAI Codex; research support from Instinct and Jev. * test(agent): trim Gemini tool-result cases to the reviewed table Signed-off-by: Charan Rathore <[email protected]> --------- Signed-off-by: Charan Rathore <[email protected]> * chore(typescript): version packages (canary) * feat(client): expose OAuth UserInfo in useMcp (#2688) * feat(client): expose OAuth UserInfo in useMcp * fix(client): address UserInfo review feedback * chore(typescript): version packages (canary) * chore(changelog): update changelogs for v20.3.14 and v2.7.2 releases - Added details for v20.3.14 including fixes for Express client disconnection and enhancements to the OAuth proxy. - Documented new features and fixes in v2.7.2, highlighting the exposure of OAuth UserInfo and stricter CLI flag handling. - Updated references to the Inspector changelog in the TypeScript changelog. * fix: stop a false vitest release and harden two canary tests (#2710) The Sentry example's vitest bump cannot apply under the workspace override and would publish a false mcp-use note. Keep the proxy stream test and pre-aborted OpenAI fetch from failing for the wrong reason, and document UserInfo proxying. * chore(typescript): version packages (canary) --------- Signed-off-by: Elioooon <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Charan Rathore <[email protected]> Co-authored-by: Mohammed Alkindi <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Steven Huang <[email protected]> Co-authored-by: Elioooon <[email protected]> Co-authored-by: Rohith Reddy Nama <[email protected]> Co-authored-by: aishwary-dongre <[email protected]> Co-authored-by: Aishwary Dongre <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Charan Rathore <[email protected]> Co-authored-by: Enrico Toniato <[email protected]>
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.
Pull Request Description
Where did you find this issue?
I found this while inspecting the TypeScript agent's tool-result conversion and reproduced it locally with the existing provider tests. This is not a report of a live Gemini outage.
Gemini requires
functionResponse.responseto be a JSON object (API reference). A tool returning a JSON array currently sends the array directly becausetypeof []is"object". This affects list/search results, including empty lists, in both streaming and non-streaming requests.Language / Project Scope
Changes
Wrap array results as
{ result: ... }, using the same wrapper already used for scalar results. Keep object, scalar, plain-text and image behavior unchanged.Implementation Details
Add
Array.isArray(parsed)to the existing normalization condition inbuildGeminiToolResponse. No new dependency or API. Extend the existing provider tests, including assertions on the serializedfetchrequest body for both request modes. Add a patch changeset for@mcp-use/agent.TypeScript Checklist
Packages Modified
docstests(agent package tests)clicreate-mcp-use-appmcp-use(server)mcp-use(client)inspector@mcp-use/agentPre-commit Checklist
pnpm lint:fixacross the monorepo. Scoped ESLint passes instead; no unrelated autofixes.pnpm build. Client and agent builds pass; unrelated server/UI builds were not run.Python Checklist
Not applicable. No Python changes.
Example Usage (Before)
For an MCP tool result
{ content: [{ type: "text", text: '[{"name":"sample"}]' }] }, the Gemini request contains:{"functionResponse":{"name":"list-records","response":[{"name":"sample"}]}}Example Usage (After)
{"functionResponse":{"name":"list-records","response":{"result":[{"name":"sample"}]}}}Documentation Updates
Patch changeset only. The provider's existing object-response contract is unchanged.
Testing
Apple Silicon, Node 24.21.0, repository-selected pnpm 11.13.1 and Vitest 4.1.10. Dependencies installed from the frozen lockfile for the client, agent and workspace tools.
From
libraries/typescript/packages/agent:./node_modules/.bin/vitest run src/llm/__tests__/providerToolResults.test.ts --maxWorkers=1: before the production fix, 5 failed and 11 passed. Failures show arrays where an object response is required. After the fix, 16 passed../node_modules/.bin/vitest run --exclude 'tests/integration/**' --exclude 'tests/docs/**' --maxWorkers=1: 19 files passed; 158 tests passed, 1 skipped.From
libraries/typescript:pnpm --filter @mcp-use/client buildandpnpm --filter @mcp-use/agent build: passed, including declaration generation../node_modules/.bin/eslint packages/agent/src/llm/__tests__/providerToolResults.test.ts packages/agent/src/llm/providers/google.ts: passed../node_modules/.bin/prettier --check packages/agent/src/llm/__tests__/providerToolResults.test.ts packages/agent/src/llm/providers/google.ts .changeset/gemini-array-tool-results.md: passed../node_modules/.bin/changeset status --since=6efd1046651e8357c94b39559385d58d9a9fd17d: passed after committing the changeset.git diff --check: passed.The first broad test attempt could not resolve the not-yet-built client package. Building the client resolved that setup failure; the complete agent unit rerun above passed. Fetch is mocked at the HTTP boundary. No paid/live Gemini calls, model-output evaluation, full monorepo lint/build, browser tests or cloud integration tests were run.
Backwards Compatibility
Only array-valued text tool results change wire shape, from an invalid top-level array to an object containing the same array. Existing object, scalar and multimodal results retain their behavior.
Related Issues
No existing issue to close. Checked open PRs and searches for Gemini arrays, Google tool responses and
functionResponse; no matching active fix found. The earlier image-result work in #1487 is already merged and concerns a different path.AI Assistance
AI assistance was used to investigate the source, implement the patch and tests, review the diff, and run the listed checks. Jev supplied advisory decisions and Instinct supplied research, which was checked against the source and GitHub. This disclosure does not claim independent human execution of the tests.
Summary by cubic
Fixes Gemini tool responses so array results are wrapped in an object, matching the provider's schema requirement that
functionResponse.responsebe a JSON object. Array-valued results (including empty arrays) are now sent as{ result: [...] }instead of a bare array; object, scalar, plain-text, and image results are unchanged.@mcp-use/agent.Written for commit a6d9010. Summary will update on new commits.