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.