fix(everything): prevent URL elicitation error-path infinite loop

src/everything/__tests__/tools.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi } from 'vitest';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
 import { registerEchoTool, EchoSchema } from '../tools/echo.js';
 import { registerGetSumTool } from '../tools/get-sum.js';
@@ -13,7 +13,10 @@ import { registerToggleSimulatedLoggingTool } from '../tools/toggle-simulated-lo
 import { registerToggleSubscriberUpdatesTool } from '../tools/toggle-subscriber-updates.js';
 import { registerTriggerSamplingRequestTool } from '../tools/trigger-sampling-request.js';
 import { registerTriggerElicitationRequestTool } from '../tools/trigger-elicitation-request.js';
-import { registerTriggerUrlElicitationTool } from '../tools/trigger-url-elicitation.js';
+import {
+  registerTriggerUrlElicitationTool,
+  __resetIssuedErrorPathElicitations,
+} from '../tools/trigger-url-elicitation.js';
 import { registerGetRootsListTool } from '../tools/get-roots-list.js';
 import { registerGZipFileAsResourceTool } from '../tools/gzip-file-as-resource.js';
 
@@ -708,6 +711,12 @@ describe('Tools', () => {
   });
 
   describe('trigger-url-elicitation', () => {
+    // The error-path marker is module-level state shared across cases; reset it
+    // so tests are independent of order and of each other's leftover keys.
+    beforeEach(() => {
+      __resetIssuedErrorPathElicitations();
+    });
+
     it('should not register when client does not support URL elicitation', () => {
       const handlers: Map<string, Function> = new Map();
       const mockServer = {
@@ -913,7 +922,7 @@ describe('Tools', () => {
       );
     });
 
-    it('should throw MCP error -32042 with required URL elicitation data when errorPath is true', async () => {
+    it('should throw MCP error -32042 with a prerequisite elicitation pointing at a different URL when errorPath is true', async () => {
       const handlers: Map<string, Function> = new Map();
       const mockServer = {
         registerTool: vi.fn((name: string, config: any, handler: Function) => {
@@ -928,7 +937,7 @@ describe('Tools', () => {
 
       const handler = handlers.get('trigger-url-elicitation')!;
 
-      expect.assertions(2);
+      expect.assertions(5);
 
       try {
         await handler(
@@ -942,13 +951,74 @@ describe('Tools', () => {
         );
       } catch (error: any) {
         expect(error.code).toBe(-32042);
-        expect(error.data.elicitations[0]).toEqual({
-          mode: 'url',
-          url: 'https://example.com/connect',
-          message: 'Authorization is required to continue.',
-          elicitationId: 'elicitation-xyz',
-        });
+        const prerequisite = error.data.elicitations[0];
+        expect(prerequisite.mode).toBe('url');
+        // The prerequisite must NOT reuse the failing URL, otherwise the client
+        // would complete it, retry, hit the same error, and loop forever.
+        expect(prerequisite.url).toBe('https://modelcontextprotocol.io');
+        expect(prerequisite.url).not.toBe('https://example.com/connect');
+        // It carries its own elicitation id for the prerequisite itself.
+        expect(typeof prerequisite.elicitationId).toBe('string');
+      }
+    });
+
+    it('should ignore errorPath and take the request path when the same call is retried after the prerequisite', async () => {
+      const handlers: Map<string, Function> = new Map();
+      const mockSendRequest = vi.fn().mockResolvedValue({ action: 'accept' });
+
+      const mockServer = {
+        registerTool: vi.fn((name: string, config: any, handler: Function) => {
+          handlers.set(name, handler);
+        }),
+        server: {
+          getClientCapabilities: vi.fn(() => ({ elicitation: { url: {} } })),
+        },
+      } as unknown as McpServer;
+
+      registerTriggerUrlElicitationTool(mockServer);
+
+      const handler = handlers.get('trigger-url-elicitation')!;
+      // A real client retries with the *same* arguments and does not echo the
+      // prerequisite's elicitationId. Note these args omit elicitationId, so the
+      // correlation must rely on stable inputs (session + url), not a per-call
+      // random id.
+      const args = {
+        url: 'https://example.com/connect',
+        message: 'Authorization is required to continue.',
+        errorPath: true,
+      };
+      const extra = { sessionId: 'session-1', sendRequest: mockSendRequest };
+
+      // First call: error path issues the prerequisite and throws -32042.
+      let prerequisiteUrl: string | undefined;
+      try {
+        await handler(args, extra);
+        throw new Error('expected first call to throw');
+      } catch (error: any) {
+        expect(error.code).toBe(-32042);
+        prerequisiteUrl = error.data.elicitations[0].url;
+        expect(prerequisiteUrl).toBe('https://modelcontextprotocol.io');
+        expect(mockSendRequest).not.toHaveBeenCalled();
       }
+
+      // Plain retry with identical arguments: errorPath is ignored and the call
+      // proceeds via the request path instead of throwing the prerequisite again.
+      const result = await handler({ ...args }, extra);
+
+      expect(mockSendRequest).toHaveBeenCalledWith(
+        expect.objectContaining({
+          method: 'elicitation/create',
+          params: expect.objectContaining({
+            mode: 'url',
+            url: 'https://example.com/connect',
+          }),
+        }),
+        expect.anything(),
+        expect.anything()
+      );
+      expect(result.content[0].text).toContain(
+        '✅ User completed the URL elicitation flow.'
+      );
     });
   });
 

src/everything/docs/features.md

@@ -23,7 +23,7 @@
 - `toggle-simulated-logging` (tools/toggle-simulated-logging.ts): Starts or stops simulated, random‑leveled logging for the invoking session. Respects the client’s selected minimum logging level.
 - `toggle-subscriber-updates` (tools/toggle-subscriber-updates.ts): Starts or stops simulated resource update notifications for URIs the invoking session has subscribed to.
 - `trigger-elicitation-request` (tools/trigger-elicitation-request.ts): Issues an `elicitation/create` request using form-mode fields (strings, numbers, booleans, enums, and format validation) and returns the resulting action/content.
-- `trigger-url-elicitation` (tools/trigger-url-elicitation.ts): Issues an `elicitation/create` request in URL mode (`mode: "url"`) with an `elicitationId`, or throws MCP error `-32042` (`UrlElicitationRequiredError`) when `errorPath=true`. Requires client capability `elicitation.url`.
+- `trigger-url-elicitation` (tools/trigger-url-elicitation.ts): Issues an `elicitation/create` request in URL mode (`mode: "url"`) with an `elicitationId`, or throws MCP error `-32042` (`UrlElicitationRequiredError`) when `errorPath=true`. On the error path the prerequisite elicitation it returns points at a different URL than the failing request (`https://modelcontextprotocol.io`); when the client satisfies it and retries the same call, the retry ignores `errorPath` and proceeds via the request path, so the client does not loop on the same error. The retry marker is one-shot per `(session, url, elicitationId)`: it is cleared on the recognized retry, so re-running the error path with identical arguments without an intervening prerequisite is treated as a retry and proceeds. Requires client capability `elicitation.url`.
 - `trigger-sampling-request` (tools/trigger-sampling-request.ts): Issues a `sampling/createMessage` request to the client/LLM using provided `prompt` and optional generation controls; returns the LLM's response payload.
 - `simulate-research-query` (tools/simulate-research-query.ts): Demonstrates MCP Tasks (SEP-1686) with a simulated multi-stage research operation. Accepts `topic` and `ambiguous` parameters. Returns a task that progresses through stages with status updates. If `ambiguous` is true and client supports elicitation, sends an elicitation request directly to gather clarification before completing.
 - `trigger-sampling-request-async` (tools/trigger-sampling-request-async.ts): Demonstrates bidirectional tasks where the server sends a sampling request that the client executes as a background task. Server polls for status and retrieves the LLM result when complete. Requires client to support `tasks.requests.sampling.createMessage`.

src/everything/docs/structure.md

@@ -156,7 +156,7 @@ src/everything
 - `trigger-elicitation-request.ts`
   - Registers a `trigger-elicitation-request` tool that sends an `elicitation/create` request to the client/LLM and returns the elicitation result.
 - `trigger-url-elicitation.ts`
-  - Registers a `trigger-url-elicitation` tool that either sends an out-of-band URL-mode `elicitation/create` request (`mode: "url"`) including an `elicitationId` (request path) or throws `UrlElicitationRequiredError` (`-32042`) for client-handled URL elicitation (error path).
+  - Registers a `trigger-url-elicitation` tool that either sends an out-of-band URL-mode `elicitation/create` request (`mode: "url"`) including an `elicitationId` (request path) or throws `UrlElicitationRequiredError` (`-32042`) for client-handled URL elicitation (error path). On the error path the carried prerequisite elicitation points at a different URL than the failing one (`https://modelcontextprotocol.io`), and when the client satisfies it and retries the same call, the retry ignores `errorPath` and proceeds via the request path — so the client does not loop on the same error.
 - `trigger-elicitation-request-async.ts`
   - Registers a `trigger-elicitation-request-async` tool that demonstrates bidirectional MCP tasks for elicitation. Sends an elicitation request with task metadata, then polls the client's `tasks/get` endpoint for completion status before fetching the final result.
 - `trigger-sampling-request.ts`

src/everything/tools/trigger-url-elicitation.ts

@@ -26,7 +26,9 @@ const TriggerUrlElicitationSchema = z.object({
       "Controls which elicitation mechanism is used. " +
         "When false (default), sends an elicitation/create request (request path). " +
         "When true, throws a UrlElicitationRequiredError (MCP error code -32042) so the client handles " +
-        "the URL elicitation via the error path rather than waiting for a response."
+        "the URL elicitation via the error path rather than waiting for a response. " +
+        "To clear the error, satisfy the prerequisite and retry this call with the same arguments; the " +
+        "retry ignores errorPath and proceeds, so the client does not loop on the same error."
     ),
 });
 
@@ -48,6 +50,32 @@ const config = {
   },
 };
 
+/**
+ * Tracks requests for which an error-path prerequisite has already been issued,
+ * keyed by the stable inputs a client resends when it retries the original tool
+ * call (session + URL + caller-supplied elicitationId).
+ *
+ * When the client satisfies the prerequisite and retries the same call, the
+ * matching entry lets us recognize the retry, ignore `errorPath`, and proceed
+ * via the request path instead of re-throwing `UrlElicitationRequiredError` —
+ * which would otherwise loop forever (throw -> client satisfies prerequisite ->
+ * retry -> throw -> ...).
+ *
+ * Demo simplification: entries are only removed on a recognized retry, so a
+ * client that triggers the error path and never retries leaves its key behind.
+ * That is acceptable for this reference server; a production implementation
+ * serving many long-lived sessions should evict entries (e.g. a
+ * `Map<string, timestamp>` with TTL-based cleanup).
+ */
+const issuedErrorPathElicitations = new Set<string>();
+
+/**
+ * Test-only helper to reset the module-level error-path state between cases.
+ * Not part of the tool's public behavior.
+ */
+export const __resetIssuedErrorPathElicitations = () =>
+  issuedErrorPathElicitations.clear();
+
 /**
  * Registers the 'trigger-url-elicitation' tool.
  *
@@ -56,8 +84,11 @@ const config = {
  *
  * Depending on the `errorPath` argument it either:
  *  - Sends an `elicitation/create` request and awaits the result (request path), or
- *  - Throws a `UrlElicitationRequiredError` (MCP error -32042) for the client to
- *    handle via the error path.
+ *  - Throws a `UrlElicitationRequiredError` (MCP error -32042) carrying a
+ *    prerequisite elicitation for the client to handle (error path). When the
+ *    client satisfies the prerequisite and retries the same call, the retry
+ *    ignores `errorPath` and proceeds via the request path, so the client does
+ *    not loop on the same error.
  *
  * @param {McpServer} server - The McpServer instance where the tool will be registered.
  */
@@ -85,6 +116,16 @@ export const registerTriggerUrlElicitationTool = (server: McpServer) => {
         } = args;
 
         const elicitationId = requestedElicitationId ?? randomUUID();
+        const sessionId = extra.sessionId ?? "default";
+
+        // Key the one-shot error-path marker on inputs the client resends
+        // verbatim when it retries the original tool call. A real client retries
+        // with the *same* arguments and does NOT echo the prerequisite's
+        // (server-generated) elicitationId, so we must key on stable inputs:
+        // the session, the requested URL, and the caller-supplied elicitationId
+        // (if any). Keying on the resolved/random elicitationId would change on
+        // every call and never match, re-throwing the prerequisite forever.
+        const errorPathKey = `${sessionId}\u0000${url}\u0000${requestedElicitationId ?? ""}`;
 
         const elicitationParams: ElicitRequestURLParams = {
           mode: "url",
@@ -93,12 +134,39 @@ export const registerTriggerUrlElicitationTool = (server: McpServer) => {
           elicitationId,
         };
 
-        // Error path: throw UrlElicitationRequiredError (-32042) for the client to handle
+        // Error path: signal the client via UrlElicitationRequiredError (-32042)
+        // so it handles a prerequisite URL elicitation before this request can
+        // proceed. Two things keep the client from looping forever:
+        //
+        //  1. The prerequisite points at a *different* URL than the one that
+        //     failed. Reusing the original `url` would make the client complete
+        //     the prerequisite, retry, and hit the same -32042 error endlessly.
+        //  2. We remember that we issued a prerequisite for this request. When
+        //     the client satisfies it and retries the same call, we recognize
+        //     the retry, *ignore* errorPath, and fall through to the request
+        //     path. Without this, the retry would re-enter the error path and
+        //     re-request the prerequisite URL — another loop.
         if (errorPath) {
-          throw new UrlElicitationRequiredError(
-            [elicitationParams],
-            "This request requires browser-based authorization."
-          );
+          if (issuedErrorPathElicitations.has(errorPathKey)) {
+            // Retry of a satisfied prerequisite: clear the one-shot marker and
+            // ignore errorPath, falling through to the request path below.
+            issuedErrorPathElicitations.delete(errorPathKey);
+          } else {
+            // Originating call: record that we issued a prerequisite for this
+            // request, then signal the client via -32042.
+            issuedErrorPathElicitations.add(errorPathKey);
+            const prerequisiteElicitation: ElicitRequestURLParams = {
+              mode: "url",
+              url: "https://modelcontextprotocol.io",
+              message:
+                "Open this link to satisfy the prerequisite, then retry the request.",
+              elicitationId: randomUUID(),
+            };
+            throw new UrlElicitationRequiredError(
+              [prerequisiteElicitation],
+              "This request requires browser-based authorization."
+            );
+          }
         }
 
         // Request path: send elicitation/create and await the user's response