fix(everything): use relatedTask option to enable elicitation over HTTP (#3228)

[galagaevdc]

Summary

Fixes #3228 — elicitation in simulate-research-query now works on all transports (STDIO, SSE, Streamable HTTP).

The original issue assumed elicitInputStream (a new SDK API) was required for HTTP support. It turns out the existing SDK already has the mechanism: passing { relatedTask: { taskId } } to sendRequest.

Root Cause

When sendRequest is called from the background runResearchProcess without relatedTask, the SDK sends the message directly through the transport using the relatedRequestId of the original tools/call request. On HTTP, that response stream is already closed (the CreateTaskResult was already returned), so the elicitation silently fails.

Fix

Pass { relatedTask: { taskId } } as the third argument to sendRequest when sending elicitation/create. When relatedTask is set, the SDK:

1. Enqueues the request in _taskMessageQueue instead of sending via transport directly
2. When the client calls tasks/result (as the spec says it SHOULD upon seeing input_required), the SDK dequeues and delivers the message through the tasks/result SSE stream
3. The client responds via POST → routed to the response handler → sendRequest resolves

This matches the spec's sequence diagram for the input_required flow: https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/tasks#input-required-status

Changes

• simulate-research-query.ts: add { relatedTask: { taskId } } to the sendRequest call; update comments and report text to remove the outdated "fails on HTTP" messaging
• tools.test.ts: add two tests verifying that sendRequest receives relatedTask for ambiguous queries, and is not called for non-ambiguous queries

Transport support after fix

Transport	Elicitation	Task Completion
STDIO	✅ Works	✅ Works
SSE	✅ Works	✅ Works
Streamable HTTP	✅ Works	✅ Works

Note

There is a bug in MCP inspector, so it will be impossible to test using it. I've already prepared fix for MCP inspector, but it's not merged yet modelcontextprotocol/inspector#1174

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 33s —— View job

---

Code Review

• Read changed files
• Analyze implementation correctness
• Review test quality
• Post review feedback

---

Overall: The fix is correct, minimal, and well-motivated. The one-line change to sendRequest (adding { relatedTask: { taskId } }) is exactly what's needed, and the PR description accurately explains the SDK-level mechanism. A few minor observations below.

---

simulate-research-query.ts

The core fix (src/everything/tools/simulate-research-query.ts:118) is clean and correct:

-          ElicitResultSchema
+          ElicitResultSchema,
+          { relatedTask: { taskId } }

Observations:

1. sendRequest: any typing (line 71): The any type with eslint-disable was pre-existing, but it's worth noting there's no type-safety around relatedTask's shape. If the SDK ever exports a proper type for the third argument of sendRequest, it would be worth adopting it here. Not a blocker.

2. Error message update is accurate: Changing "technical (default - elicitation unavailable on HTTP)" → "technical (default - elicitation unavailable)" correctly reflects that the catch block now handles genuine failures, not transport-specific ones. The matching string change on the report output (state.clarification.includes("unavailable on HTTP") → .includes("unavailable")) is also correct.

3. clientSupportsElicitation captured at registration time (line 239): Pre-existing behaviour — capabilities are negotiated once per connection, so this is fine for a demo server.

4. getTaskResult cleanup vs. background process (line 309): Pre-existing concern: researchStates.delete(extra.taskId) runs when the client fetches results, but runResearchProcess also holds a reference to state via closure (not via the Map), so the deletion is safe. Worth a quick read to confirm this is intentional.

---

tools.test.ts

The two new tests cover the key behaviour (relatedTask passed / not passed) and use the right vitest patterns.

Minor issues:

1. vi.useRealTimers() not in finally (lines 793, 818): If vi.runAllTimersAsync() throws, timers remain fake and will leak into subsequent tests. Should be:

   try {
     await vi.runAllTimersAsync();
   } finally {
     vi.useRealTimers();
   }

   Fix this →

2. Module-level researchStates Map not cleaned up between tests: Tests use different task IDs (task-abc, task-def) so there's no conflict today, but stale entries accumulate. An afterEach(() => researchStates.clear()) — or just calling taskHandlers.getTaskResult at the end of each test — would be more correct. Not a blocker given the current test structure.

3. Test 1 doesn't verify task completion: It only asserts sendRequest was called with the right args. Adding expect(mockTaskStore.storeTaskResult).toHaveBeenCalledWith('task-abc', 'completed', expect.anything()) would make it a more complete integration assertion. Optional.

---

Summary: The fix is correct and the tests cover the scenario they need to. The vi.useRealTimers() placement is the only thing worth fixing before merge; the rest are minor polish items.

[galagaevdc]

Hi @cliffhall, I’ve reviewed Claude’s comments. The only issue he suggested fixing is:

vi.useRealTimers() not in finally (lines 793, 818): If vi.runAllTimersAsync() throws, timers remain fake and will leak into subsequent tests. Should be:

try {
  await vi.runAllTimersAsync();
} finally {
  vi.useRealTimers();
}

However, this pattern is commonly used in other tests, so I don’t think it needs to be changed:

await vi.runAllTimersAsync();
vi.useRealTimers();

Please let me know if you have a different opinion.

[cliffhall]

Review — relatedTask for elicitation over HTTP

I pulled this PR down, rebased it onto current main, and verified it end-to-end. The approach is correct and the fix is the right one. Details and a few non-blocking observations below.

Verification

• Rebased onto current main (the branch was ~25 commits behind, including the zod v4 / SDK ^1.29.0 upgrade). It rebased cleanly, no conflicts.
• npm run build clean; vitest 61 passing, including the two new tests here.
• End-to-end over Streamable HTTP (SDK 1.29 client): task goes working → input_required, the elicitation/create is delivered, the client's accept resolves, and the final report shows the real "After receiving clarification..." path — not the "unavailable" fallback.
• End-to-end over stdio: same result, so adding relatedTask does not regress the transport that already worked.
• Confirmed in Inspector V2, which handles input_required correctly (as does Inspector V1 via the author's #1174).

The mechanism is right

The PR description nails it: the issue (#3228) assumed the new streaming elicitInputStream API (SDK #1210) was required, but { relatedTask: { taskId } } on the existing sendRequest is sufficient and simpler. For reference, both are now available in SDK 1.29 (#1210 merged 2026-02-11), but relatedTask is the lighter-weight, spec-aligned choice here — it enqueues the request and delivers it through the tasks/result stream on the client's poll, matching the spec's input_required sequence. No reason to reach for elicitInputStream for this.

The input_required → (elicit) → working status transitions are spec-compliant, and the comment/report-text updates accurately describe the new behavior.

Non-blocking: capability check is now too loose for form-mode

This is pre-existing (not introduced by this PR), but it becomes more relevant now that the elicitation path actually runs on HTTP:

const clientSupportsElicitation = clientCapabilities.elicitation !== undefined;
...
ambiguous: validatedArgs.ambiguous && clientSupportsElicitation,

Under SDK ≥1.29's getSupportedElicitationModes, a client that declares elicitation: { url: {} } (URL-mode only) but not form is treated as not supporting form mode — so this tool's form-mode elicitation/create is rejected with -32602 Client does not support form-mode elicitation requests, and the try/catch degrades to the default interpretation. It won't crash, but such a client silently won't get the disambiguation prompt. Consider gating on form-mode support specifically (the SDK exports getSupportedElicitationModes(capabilities.elicitation)) rather than "any elicitation capability." Reasonable as a follow-up rather than a blocker.

Nits (all optional)

• Tests assert the contract, not the delivery. The two new tests mock sendRequest and assert relatedTask is passed (good — that's the actual code change) and that non-ambiguous calls don't elicit. They don't exercise real cross-transport delivery, which is the point of the fix — that's inherently the SDK's job, and I covered it with the e2e runs above. Might be worth a line in the PR noting manual HTTP verification.
• Stronger assertion available. The first test could also assert the accepted clarification propagates into the stored result (e.g. storeTaskResult receives a report containing the chosen interpretation), locking in the accept-handling path, not just the request shape.
• Fallback string. The catch hardcodes "technical (default - elicitation unavailable)"; minor, and consistent with the existing decline/cancel strings.

Verdict

LGTM on substance — correct mechanism, verified on stdio + Streamable HTTP, spec-compliant flow. The only thing it strictly needs is a rebase onto current main (clean, as noted). The form-mode capability gating is worth a follow-up but isn't a blocker for the demo's happy path.

[cliffhall]

LGTM! 👍

Tested locally with a V1 inspector running PR #1174 and a V2 Inspector which handles input_required properly.

[cliffhall]

Thanks for this, @galagaevdc