Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix looks right: authenticated TinyFish searches now run an MCP initialize, forward the minted Mcp-Session-Id on tools/call, and fail with a clear message when no id comes back. The keyless path is unchanged. I couldn't test against the live endpoint without a TinyFish key. The updated websearch tests pass here (12/12) and fail on the base (2 TinyFish failures), and typecheck is clean.
Public API is unchanged (WebSearchMcp.initialize/PROTOCOL_VERSION are internal to core).
Two things worth a look, both inline: the hand-rolled case-insensitive header lookup duplicates what Effect's Headers already guarantees, and the handshake skips the notifications/initialized message the MCP spec requires before further requests.
Related: #38891
| } | ||
|
|
||
| /** Read a response header by name without assuming the client normalized its casing. */ | ||
| const header = (headers: Readonly<Record<string, string | undefined>>, name: string) => { |
There was a problem hiding this comment.
Effect's Headers always stores response header names in lowercase (Headers.fromInput normalizes them), so this helper isn't needed. Headers.get(response.headers, "mcp-session-id") (or just response.headers["mcp-session-id"]) does the same job. The style guide asks not to re-check values the library already guarantees.
| ) | ||
| return yield* Effect.gen(function* () { | ||
| const response = yield* HttpClient.withScope(HttpClient.filterStatusOk(http)).execute(request) | ||
| const session = header(response.headers, "mcp-session-id") |
There was a problem hiding this comment.
Per the MCP lifecycle spec, the client must send a notifications/initialized notification (with the new Mcp-Session-Id) after initialize and before other requests. TinyFish seems to accept calls without it today, but a stricter server or a future TinyFish change could reject the tools/call. Since this helper is generic, it would be safer to send the notification here. Also note that each search opens a session that is never closed (no DELETE). That's probably fine for TinyFish, but worth knowing.
The tinyfish provider posts a bare tools/call to its MCP endpoint. In authenticated mode TinyFish mints an Mcp-Session-Id on initialize and rejects every post-initialize request without it, so with TINYFISH_API_KEY set every search failed with HTTP 400 (surfaced as 503). Keyless mode is stateless, which is why it kept working. Adds an initialize handshake to WebSearchMcp and uses it on the authenticated path, forwarding the returned session id on the search call. A missing session id now fails with a clear message.
…ialized notification
a8c8cd0 to
4277952
Compare
|
I don't think the mcp init needs to be a shared helper, none of the other websearches use it. Also added Thank you for your help! |
Tarquinen
changed the title
Issue for this PR
Closes #51263
Type of change
What does this PR do?
tinyfishposts a baretools/calltohttps://agent.tinyfish.ai/mcp. In authenticated mode TinyFish mints anMcp-Session-Idoninitializeand rejects every post-initialize request without it, so whenever a key is configured (env var or stored credential) every search fails with HTTP 400, surfaced as a 503. Keyless mode is stateless, which is why it kept working.This adds an
initializehandshake toWebSearchMcpand uses it on the authenticated path, forwarding the minted session id on the search call. The keyless path is unchanged, and a handshake that returns no session id now fails with a clear message instead of an opaque MCP error.This is the V2 port of the fix in #52709.
How did you verify your code works?
packages/core:bun test test/plugin/websearch.test.ts-> 12 pass, 0 fail;bun run typecheckclean.tools/callwithX-API-Keyalone ->400 Missing required Mcp-Session-Id header;initialize->tools/callcarrying the returned session id ->200with results.Note: the websearch tests assert keyless headers, so they must run without
TAVILY_API_KEY/TINYFISH_API_KEY/EXA_API_KEY/FIRECRAWL_API_KEY/PARALLEL_API_KEYin the environment, otherwise unrelated providers resolve a key. I ran them with those unset.Checklist
Added by the opencode bot (AI-generated).
Related: #38891