Merge pull request #2277 from modelcontextprotocol/v2/fix/2220-empty-string-cursor
fix: send an empty-string pagination cursor verbatim on every list adapter
This commit is contained in:
commit
42babb7482
13 changed files with 482 additions and 44 deletions
|
|
@ -103,27 +103,18 @@ describe("replayableParams", () => {
|
|||
).toEqual({ params: { cursor: "abc" }, dropped: ["_meta"] });
|
||||
});
|
||||
|
||||
// `listTools` builds its params with `cursor !== undefined`, carrying `""`
|
||||
// deliberately — its own comment says dropping it asks for page one again.
|
||||
it("keeps an empty cursor on tools/list, which preserves it", () => {
|
||||
expect(replayableParams("tools/list", { cursor: "" })).toEqual({
|
||||
params: { cursor: "" },
|
||||
dropped: [],
|
||||
});
|
||||
});
|
||||
|
||||
// The other four adapters build theirs with a truthiness check, so `""` never
|
||||
// reaches the wire. Reporting it as kept would show `{"cursor":""}` in the
|
||||
// editor while `{}` was sent.
|
||||
// Every list adapter builds its params with `cursor !== undefined` (#2220),
|
||||
// so `""` reaches the wire on all five and the editor must show it.
|
||||
it.each([
|
||||
"tools/list",
|
||||
"prompts/list",
|
||||
"resources/list",
|
||||
"resources/templates/list",
|
||||
"tasks/list",
|
||||
])("drops an empty cursor on %s, which does not preserve it", (method) => {
|
||||
])("keeps an empty cursor on %s, which preserves it", (method) => {
|
||||
expect(replayableParams(method, { cursor: "" })).toEqual({
|
||||
params: undefined,
|
||||
dropped: ["cursor"],
|
||||
params: { cursor: "" },
|
||||
dropped: [],
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -118,18 +118,11 @@ export function replayableParams(
|
|||
// Only a *string* cursor survives: the dispatcher ignores any other type,
|
||||
// so keeping it would put a value in the editor that changes nothing.
|
||||
//
|
||||
// And an **empty** one survives on `tools/list` alone. `listTools` builds
|
||||
// its params with `cursor !== undefined`, carrying `""` deliberately —
|
||||
// its own comment explains that dropping it asks for page one again. The
|
||||
// other four adapters use a truthiness check and drop it. That asymmetry
|
||||
// looks like a latent bug in those four rather than an intention, but
|
||||
// this function's job is to describe what the dispatch *does*, so it
|
||||
// reports the empty cursor as dropped where it would be dropped.
|
||||
kept =
|
||||
typeof params.cursor === "string" &&
|
||||
(method === "tools/list" || params.cursor !== "")
|
||||
? ["cursor"]
|
||||
: [];
|
||||
// An **empty** string is a cursor like any other, and every one of these
|
||||
// five adapters now builds its params with `cursor !== undefined` (#2220
|
||||
// brought the other four into line with `listTools`), so `""` reaches the
|
||||
// wire on all of them and is reported as kept rather than dropped.
|
||||
kept = typeof params.cursor === "string" ? ["cursor"] : [];
|
||||
break;
|
||||
default:
|
||||
// `ping` takes nothing, and an unreplayable method never reaches here.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,157 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
|
||||
/**
|
||||
* Unit coverage for the pagination cursor every list adapter — `tools/list`,
|
||||
* `prompts/list`, `resources/list`, `resources/templates/list`, `tasks/list` —
|
||||
* puts on the wire (#2220).
|
||||
*
|
||||
* An MCP cursor is an **opaque string**: the spec places no constraint on its
|
||||
* content, so `""` is a `nextCursor` a server may legitimately hand back and
|
||||
* the client is required to send verbatim. Four adapters used to build their
|
||||
* params with a truthiness check, which cannot distinguish "no cursor" from
|
||||
* "the cursor is the empty string" — so they dropped `""` and silently
|
||||
* re-requested page one. Nothing surfaced an error, because the request was
|
||||
* well-formed; it just asked the wrong question.
|
||||
*
|
||||
* The outbound params are therefore the whole assertion here: what each case
|
||||
* pins is that `""` survives and that a genuinely absent cursor still sends no
|
||||
* `cursor` key at all. The SDK client is stubbed rather than connected — the
|
||||
* decision under test is made entirely in `InspectorClient`.
|
||||
*/
|
||||
describe("InspectorClient list cursor handling (#2220)", () => {
|
||||
/**
|
||||
* The one SDK call these tests care about. Named rather than inlined so the
|
||||
* `vi.fn` stub can be typed by it — that is what puts the request shape on
|
||||
* `request.mock.calls`, so each assertion reads `params` without a cast.
|
||||
*/
|
||||
type SdkRequest = (
|
||||
req: { method: string; params: Record<string, unknown> },
|
||||
schema: unknown,
|
||||
) => Promise<unknown>;
|
||||
|
||||
interface ClientInternals {
|
||||
client: { request: SdkRequest } | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* A structural view onto the private `client` field so a test can stub the
|
||||
* SDK client without connecting.
|
||||
*
|
||||
* The double cast is justified rather than incidental: `InspectorClient`
|
||||
* declares `client` `private`, so no single `as` relates it to a type that
|
||||
* exposes it, and there is no public setter — the public path is `connect()`,
|
||||
* which needs a transport, a live server and a handshake. It is safe because
|
||||
* the shape asserted is exactly the shape the class declares, so a rename or
|
||||
* a type change breaks these tests at the first use rather than silently
|
||||
* passing. The same seam is used by `inspectorClient-skills.test.ts`.
|
||||
*/
|
||||
function internals(client: InspectorClient): ClientInternals {
|
||||
return client as unknown as ClientInternals;
|
||||
}
|
||||
|
||||
function makeClient(): InspectorClient {
|
||||
return new InspectorClient(
|
||||
{ type: "stdio", command: "noop", args: [] },
|
||||
// `environment.transport` is only used on connect(); these tests never
|
||||
// connect, they stub the SDK client directly.
|
||||
{ environment: { transport: () => ({}) as never } },
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stub the SDK client so `request` resolves with a fixed result.
|
||||
*
|
||||
* Typed by {@link SdkRequest} rather than inferred, so `request.mock.calls`
|
||||
* carries the request shape.
|
||||
*/
|
||||
function stubRequest(client: InspectorClient, result: unknown) {
|
||||
const request = vi.fn<SdkRequest>(async () => result);
|
||||
internals(client).client = { request };
|
||||
return request;
|
||||
}
|
||||
|
||||
/**
|
||||
* The five adapters, each with the method it emits and an empty result of the
|
||||
* right shape. `listTools` was always correct and is included as the control:
|
||||
* if the guard it has always used ever regressed, these cases go red too.
|
||||
*/
|
||||
const ADAPTERS: {
|
||||
name: string;
|
||||
method: string;
|
||||
result: Record<string, unknown>;
|
||||
call: (client: InspectorClient, cursor?: string) => Promise<unknown>;
|
||||
}[] = [
|
||||
{
|
||||
name: "listTools",
|
||||
method: "tools/list",
|
||||
result: { tools: [] },
|
||||
call: (client, cursor) => client.listTools(cursor),
|
||||
},
|
||||
{
|
||||
name: "listPrompts",
|
||||
method: "prompts/list",
|
||||
result: { prompts: [] },
|
||||
call: (client, cursor) => client.listPrompts(cursor),
|
||||
},
|
||||
{
|
||||
name: "listResources",
|
||||
method: "resources/list",
|
||||
result: { resources: [] },
|
||||
call: (client, cursor) => client.listResources(cursor),
|
||||
},
|
||||
{
|
||||
name: "listResourceTemplates",
|
||||
method: "resources/templates/list",
|
||||
result: { resourceTemplates: [] },
|
||||
call: (client, cursor) => client.listResourceTemplates(cursor),
|
||||
},
|
||||
{
|
||||
name: "listRequestorTasks",
|
||||
method: "tasks/list",
|
||||
result: { tasks: [] },
|
||||
call: (client, cursor) => client.listRequestorTasks(cursor),
|
||||
},
|
||||
];
|
||||
|
||||
it.each(ADAPTERS)(
|
||||
"$name sends an empty-string cursor verbatim",
|
||||
async ({ method, result, call }) => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, result);
|
||||
|
||||
await call(client, "");
|
||||
|
||||
const sent = request.mock.calls[0][0];
|
||||
expect(sent.method).toBe(method);
|
||||
expect(sent.params.cursor).toBe("");
|
||||
},
|
||||
);
|
||||
|
||||
it.each(ADAPTERS)(
|
||||
"$name sends no cursor key when there is no cursor",
|
||||
async ({ method, result, call }) => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, result);
|
||||
|
||||
await call(client);
|
||||
|
||||
const sent = request.mock.calls[0][0];
|
||||
expect(sent.method).toBe(method);
|
||||
expect(sent.params).not.toHaveProperty("cursor");
|
||||
},
|
||||
);
|
||||
|
||||
it.each(ADAPTERS)(
|
||||
"$name forwards a non-empty cursor unchanged",
|
||||
async ({ result, call }) => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, result);
|
||||
|
||||
await call(client, "page-2");
|
||||
|
||||
const sent = request.mock.calls[0][0];
|
||||
expect(sent.params.cursor).toBe("page-2");
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
@ -101,6 +101,27 @@ describe("ManagedRequestorTasksState", () => {
|
|||
expect(client.listRequestorTasks).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("refresh walks past an empty-string cursor instead of stopping at page one", async () => {
|
||||
// A cursor is opaque and `""` is a legal `nextCursor` (#2220). Under the
|
||||
// truthiness guard this walk used to carry, page one's empty cursor ended
|
||||
// the loop — so the task list stopped at `t1` against a conforming server,
|
||||
// with nothing to show for it. The cursor each call receives is asserted
|
||||
// too, because a walk that stopped correctly but re-requested page one
|
||||
// would still produce three tasks with the wrong three requests.
|
||||
client.setStatus("connected");
|
||||
client.queueTaskPages(
|
||||
{ tasks: [task("t1")], nextCursor: "" },
|
||||
{ tasks: [task("t2")], nextCursor: "c2" },
|
||||
{ tasks: [task("t3")] },
|
||||
);
|
||||
|
||||
const result = await state.refresh();
|
||||
expect(result.map((t) => t.taskId)).toEqual(["t1", "t2", "t3"]);
|
||||
expect(client.listRequestorTasks.mock.calls.map((call) => call[0])).toEqual(
|
||||
[undefined, "", "c2"],
|
||||
);
|
||||
});
|
||||
|
||||
it("connect event triggers a refresh", async () => {
|
||||
client.setStatus("connected");
|
||||
client.queueTaskPages({ tasks: [task("t1")] });
|
||||
|
|
|
|||
168
clients/web/src/test/integration/mcp/empty-cursor.test.ts
Normal file
168
clients/web/src/test/integration/mcp/empty-cursor.test.ts
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
import { describe, it, expect, afterEach } from "vitest";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import { createTransportNode } from "@inspector/core/mcp/node/transport.js";
|
||||
import {
|
||||
createTestServerHttp,
|
||||
type TestServerHttp,
|
||||
createTestServerInfo,
|
||||
loadConfig,
|
||||
resolveConfig,
|
||||
} from "@modelcontextprotocol/inspector-test-server";
|
||||
|
||||
/**
|
||||
* Live coverage of `test-servers/configs/empty-cursor-http.json` — the
|
||||
* documented manual reproduction for #2220.
|
||||
*
|
||||
* The unit tests on the adapters assert the outbound params directly, which is
|
||||
* where the decision is made. What they cannot see is the round trip: that a
|
||||
* server may hand back `""` as a `nextCursor` at all, that the SDK carries it
|
||||
* through both directions without normalizing it away, and that a page walk
|
||||
* driven by it actually advances. Those are the premise of the fix, and a
|
||||
* change anywhere in that chain would leave the unit tests green while the
|
||||
* showcase server quietly stopped reproducing the bug.
|
||||
*
|
||||
* The server is built by **resolving the checked-in config** rather than by
|
||||
* hand, so a config that names a dead preset — or an `emptyStringCursor` flag
|
||||
* that stops being threaded through `resolveConfig` — fails here rather than
|
||||
* only when someone runs the repro by hand.
|
||||
*/
|
||||
describe("empty-string pagination cursor over the wire (#2220)", () => {
|
||||
let client: InspectorClient | null = null;
|
||||
let server: TestServerHttp | null = null;
|
||||
|
||||
const configPath = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"../../../../../../test-servers/configs/empty-cursor-http.json",
|
||||
);
|
||||
|
||||
afterEach(async () => {
|
||||
if (client) {
|
||||
try {
|
||||
await client.disconnect();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
client = null;
|
||||
}
|
||||
if (server) {
|
||||
try {
|
||||
await server.stop();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
server = null;
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Boot the showcase config. The harness picks the port rather than using the
|
||||
* config's fixed one, so this cannot collide with a showcase server someone
|
||||
* is running by hand.
|
||||
*/
|
||||
async function connectToShowcase(): Promise<InspectorClient> {
|
||||
const resolved = resolveConfig(loadConfig(configPath));
|
||||
const started = createTestServerHttp({
|
||||
...resolved,
|
||||
serverInfo: createTestServerInfo("empty-cursor-test", "1.0.0"),
|
||||
port: undefined,
|
||||
});
|
||||
await started.start();
|
||||
server = started;
|
||||
|
||||
const connected = new InspectorClient(
|
||||
{ type: "streamable-http", url: started.url },
|
||||
{ environment: { transport: createTransportNode } },
|
||||
);
|
||||
await connected.connect();
|
||||
client = connected;
|
||||
return connected;
|
||||
}
|
||||
|
||||
it("resolves the config with the empty-cursor flag intact", () => {
|
||||
const resolved = resolveConfig(loadConfig(configPath));
|
||||
expect(resolved.emptyStringCursor).toBe(true);
|
||||
expect(resolved.maxPageSize).toEqual({
|
||||
tools: 4,
|
||||
resources: 4,
|
||||
prompts: 4,
|
||||
});
|
||||
expect(resolved.tools).toHaveLength(12);
|
||||
});
|
||||
|
||||
/**
|
||||
* One walk per list, driven by the same three assertions: page one hands back
|
||||
* `""` rather than a numeric index, sending it back yields the *second* four
|
||||
* items rather than the first four again, and the walk still terminates.
|
||||
*
|
||||
* The names are the assertion for "advanced" — a length check alone would
|
||||
* pass on a server that re-served page one, which is exactly the failure.
|
||||
*/
|
||||
const WALKS: {
|
||||
label: string;
|
||||
walk: (
|
||||
client: InspectorClient,
|
||||
cursor?: string,
|
||||
) => Promise<{ names: string[]; nextCursor?: string }>;
|
||||
}[] = [
|
||||
{
|
||||
label: "tools/list",
|
||||
walk: async (connected, cursor) => {
|
||||
const page = await connected.listTools(cursor);
|
||||
return {
|
||||
names: page.tools.map((tool) => tool.name),
|
||||
nextCursor: page.nextCursor,
|
||||
};
|
||||
},
|
||||
},
|
||||
{
|
||||
label: "prompts/list",
|
||||
walk: async (connected, cursor) => {
|
||||
const page = await connected.listPrompts(cursor);
|
||||
return {
|
||||
names: page.prompts.map((prompt) => prompt.name),
|
||||
nextCursor: page.nextCursor,
|
||||
};
|
||||
},
|
||||
},
|
||||
{
|
||||
label: "resources/list",
|
||||
walk: async (connected, cursor) => {
|
||||
const page = await connected.listResources(cursor);
|
||||
return {
|
||||
names: page.resources.map((resource) => resource.uri),
|
||||
nextCursor: page.nextCursor,
|
||||
};
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
it.each(WALKS)(
|
||||
"$label advances past an empty-string cursor instead of re-serving page one",
|
||||
async ({ walk }) => {
|
||||
const connected = await connectToShowcase();
|
||||
|
||||
const first = await walk(connected);
|
||||
expect(first.names).toHaveLength(4);
|
||||
// The premise: the server really does hand back `""`, and nothing between
|
||||
// here and the wire turned it into `undefined`.
|
||||
expect(first.nextCursor).toBe("");
|
||||
|
||||
const second = await walk(connected, first.nextCursor);
|
||||
expect(second.names).toHaveLength(4);
|
||||
// On the pre-fix adapters this was `first.names` again.
|
||||
expect(second.names).not.toEqual(first.names);
|
||||
expect(second.nextCursor).toBe("8");
|
||||
|
||||
const third = await walk(connected, second.nextCursor);
|
||||
expect(third.names).toHaveLength(4);
|
||||
expect(third.nextCursor).toBeUndefined();
|
||||
|
||||
// All twelve, each seen exactly once — the walk covered the list rather
|
||||
// than looping over one page.
|
||||
const seen = [...first.names, ...second.names, ...third.names];
|
||||
expect(new Set(seen).size).toBe(12);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
@ -116,9 +116,12 @@ export class FakeInspectorClient
|
|||
listResourceTemplates = vi.fn(
|
||||
async () => this.resourceTemplatePages.shift() ?? { resourceTemplates: [] },
|
||||
);
|
||||
listRequestorTasks = vi.fn(
|
||||
async () => this.taskPages.shift() ?? { tasks: [] },
|
||||
);
|
||||
// Typed by its signature rather than inferred, so `mock.calls` carries the
|
||||
// cursor a test asserts on — the empty-string pagination case (#2220) turns
|
||||
// on *which* cursor each call received, not just how many there were.
|
||||
listRequestorTasks = vi.fn<
|
||||
(cursor?: string) => Promise<ListResult<"tasks", Task>>
|
||||
>(async () => this.taskPages.shift() ?? { tasks: [] });
|
||||
listSkills = vi.fn(async () => this.skillPages.shift() ?? { skills: [] });
|
||||
// `skills/get` echoes a minimal entry; tests that care override the mock.
|
||||
getSkill = vi.fn(async (uri: string) => ({
|
||||
|
|
|
|||
|
|
@ -3045,7 +3045,13 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
throw new Error("Client is not connected");
|
||||
}
|
||||
const result = await this.client.request(
|
||||
{ method: "tasks/list", params: cursor ? { cursor } : {} },
|
||||
{
|
||||
method: "tasks/list",
|
||||
// `!== undefined`, not truthiness: a cursor is opaque and `""` is a
|
||||
// legal value a server may hand back. Dropping it asks for page one
|
||||
// again, so a caller walking pages would loop on the first page.
|
||||
params: cursor !== undefined ? { cursor } : {},
|
||||
},
|
||||
ListTasksResultSchema,
|
||||
this.getRequestOptions(),
|
||||
);
|
||||
|
|
@ -5242,7 +5248,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
const effectiveMeta = this.mergeMeta(metadata);
|
||||
const params: ListResourcesRequest["params"] = {
|
||||
...(effectiveMeta ? { _meta: effectiveMeta } : {}),
|
||||
...(cursor ? { cursor } : {}),
|
||||
// `!== undefined`, not truthiness: a cursor is opaque and `""` is a
|
||||
// legal value a server may hand back. Dropping it asks for page one
|
||||
// again, so a caller walking pages would loop on the first page.
|
||||
...(cursor !== undefined ? { cursor } : {}),
|
||||
};
|
||||
const response = await this.invokeMcpClient(() =>
|
||||
this.client!.request(
|
||||
|
|
@ -5416,7 +5425,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
const effectiveMeta = this.mergeMeta(metadata);
|
||||
const params: ListResourceTemplatesRequest["params"] = {
|
||||
...(effectiveMeta ? { _meta: effectiveMeta } : {}),
|
||||
...(cursor ? { cursor } : {}),
|
||||
// `!== undefined`, not truthiness: a cursor is opaque and `""` is a
|
||||
// legal value a server may hand back. Dropping it asks for page one
|
||||
// again, so a caller walking pages would loop on the first page.
|
||||
...(cursor !== undefined ? { cursor } : {}),
|
||||
};
|
||||
const response = await this.invokeMcpClient(
|
||||
() =>
|
||||
|
|
@ -5480,7 +5492,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
const effectiveMeta = this.mergeMeta(metadata);
|
||||
const params: ListPromptsRequest["params"] = {
|
||||
...(effectiveMeta ? { _meta: effectiveMeta } : {}),
|
||||
...(cursor ? { cursor } : {}),
|
||||
// `!== undefined`, not truthiness: a cursor is opaque and `""` is a
|
||||
// legal value a server may hand back. Dropping it asks for page one
|
||||
// again, so a caller walking pages would loop on the first page.
|
||||
...(cursor !== undefined ? { cursor } : {}),
|
||||
};
|
||||
const response = await this.invokeMcpClient(() =>
|
||||
this.client!.request(
|
||||
|
|
|
|||
|
|
@ -181,7 +181,13 @@ export class ManagedRequestorTasksState extends TypedEventTarget<ManagedRequesto
|
|||
? { ...t, status: "cancelled" as const }
|
||||
: t,
|
||||
);
|
||||
this.tasks = cursor ? [...this.tasks, ...page] : page;
|
||||
// `=== undefined`, not truthiness, on both the append test and the loop
|
||||
// condition: a cursor is opaque and `""` is a legal value (#2220). Under
|
||||
// a truthiness check an empty `nextCursor` ended the walk after page one
|
||||
// AND made the next page replace the list rather than extend it — the
|
||||
// user-visible task list silently stopping at four rows against a
|
||||
// conforming server.
|
||||
this.tasks = cursor === undefined ? page : [...this.tasks, ...page];
|
||||
cursor = result.nextCursor;
|
||||
pageCount++;
|
||||
if (pageCount >= MAX_PAGES) {
|
||||
|
|
@ -189,7 +195,7 @@ export class ManagedRequestorTasksState extends TypedEventTarget<ManagedRequesto
|
|||
`Maximum pagination limit (${MAX_PAGES} pages) reached while listing requestor tasks`,
|
||||
);
|
||||
}
|
||||
} while (cursor);
|
||||
} while (cursor !== undefined);
|
||||
this.dispatchTypedEvent("tasksChange", this.tasks);
|
||||
return this.getTasks();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ as a missing capability rather than an error.
|
|||
| `modern-network-http.json` **(modern era)** | Network tab: `Mcp-*` headers + error taxonomy | [#1628](https://github.com/modelcontextprotocol/inspector/issues/1628) |
|
||||
| `xmcpheader-modern-http.json` **(modern era)** | Tools tab: `x-mcp-header` mirroring and exclusions | [#1632](https://github.com/modelcontextprotocol/inspector/issues/1632) |
|
||||
| `pagination-http.json` **(legacy era)** | Page-by-page list fetching | [#1721](https://github.com/modelcontextprotocol/inspector/issues/1721) |
|
||||
| `empty-cursor-http.json` **(legacy era)** | Pagination whose page-two cursor is `""` | [#2220](https://github.com/modelcontextprotocol/inspector/issues/2220) |
|
||||
| `structured-output-http.json` **(legacy era)** | Tools tab: a result's `structuredContent` section | [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908) |
|
||||
| `duplicate-tool-names-http.json` **(legacy era)** | A `tools/list` that repeats a tool name | [#1957](https://github.com/modelcontextprotocol/inspector/issues/1957) |
|
||||
| `nullable-fields-http.json` **(legacy era)** | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) |
|
||||
|
|
@ -247,6 +248,14 @@ Under SDK v2 a `tools/call` rejecting with `-32602` renders as a distinct error
|
|||
|
||||
Turn on **"Fetch Lists One Page at a Time"** (Server Settings — the `paginatedLists` setting, or the **Paginated** switch in a list sidebar) and the lists load page 1 only (4 items) with a **Load next page** control and an _N pages loaded_ status. Each click fetches the next 4 and appends them; Refresh resets to page 1. With the switch off (the default), the same lists auto-aggregate all three pages on connect.
|
||||
|
||||
### The empty-string cursor
|
||||
|
||||
`empty-cursor-http.json` is the same 12-item, 4-per-page server with one difference: it hands out the **empty string** as the cursor for page two (`emptyStringCursor`), and the usual numeric index for page three. An MCP cursor is opaque — the spec constrains neither its content nor its length — so `""` is a legal `nextCursor` and a client must send it back verbatim.
|
||||
|
||||
Every other fixture's cursor is a non-empty string, which is why this one exists: a client that builds its request params with a truthiness check (`cursor ? { cursor } : {}`) cannot tell `""` from "no cursor", so it drops it and re-requests page one. Nothing errors — the request is well-formed, it just asks the wrong question — and the symptom is a list that stops after four items, or a page walk that never advances ([#2220](https://github.com/modelcontextprotocol/inspector/issues/2220)).
|
||||
|
||||
Connect with the **default (legacy)** era, turn **Paginated** on, and click **Load next page** twice on any of the three lists: the count must go 4 → 8 → 12 and the control must disappear at the end. On a build carrying the old guard the second click returns items 1–4 again.
|
||||
|
||||
## Structured output
|
||||
|
||||
`structured-output-http.json` serves `list_items` (nested `structuredContent` — objects inside arrays inside an object, the shape from [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908)), `get_temp` (a flat three-key payload), and `echo` (no `outputSchema` at all). It is a plain streamable-HTTP server — connect with the **default (legacy)** protocol era.
|
||||
|
|
|
|||
19
test-servers/configs/empty-cursor-http.json
Normal file
19
test-servers/configs/empty-cursor-http.json
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
{
|
||||
"serverInfo": {
|
||||
"name": "empty-cursor-showcase",
|
||||
"version": "1.0.0"
|
||||
},
|
||||
"tools": [{ "preset": "numbered_tools", "params": { "count": 12 } }],
|
||||
"resources": [{ "preset": "numbered_resources", "params": { "count": 12 } }],
|
||||
"prompts": [{ "preset": "numbered_prompts", "params": { "count": 12 } }],
|
||||
"maxPageSize": {
|
||||
"tools": 4,
|
||||
"resources": 4,
|
||||
"prompts": 4
|
||||
},
|
||||
"emptyStringCursor": true,
|
||||
"transport": {
|
||||
"type": "streamable-http",
|
||||
"port": 6601
|
||||
}
|
||||
}
|
||||
|
|
@ -480,6 +480,24 @@ export interface ServerConfig {
|
|||
resourceTemplates?: number;
|
||||
prompts?: number;
|
||||
};
|
||||
/**
|
||||
* Hand out the **empty string** as the cursor for page two of every
|
||||
* paginated list, instead of the usual numeric index (#2220).
|
||||
*
|
||||
* An MCP cursor is opaque: the spec constrains neither its content nor its
|
||||
* length, so `""` is a legal `nextCursor` and a conforming client has to send
|
||||
* it back verbatim. A client that builds its request params with a
|
||||
* truthiness check cannot tell `""` from "no cursor", so it drops it and
|
||||
* silently re-requests page one — a list that stops after one page, or a
|
||||
* walk that loops on it forever, with no error anywhere because the request
|
||||
* is perfectly well-formed.
|
||||
*
|
||||
* Nothing else in this repo produces that shape: every other fixture's
|
||||
* cursor is a non-empty index string, which the buggy guard happens to carry
|
||||
* correctly. Off by default so the existing pagination fixtures keep their
|
||||
* numeric cursors.
|
||||
*/
|
||||
emptyStringCursor?: boolean; // default: false
|
||||
/**
|
||||
* Emit the named registered tools **twice** in `tools/list`, with the same
|
||||
* `name` on both entries and " (duplicate)" appended to the second's title.
|
||||
|
|
@ -1265,6 +1283,33 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
// Set up pagination handlers if maxPageSize is configured
|
||||
const maxPageSize = config.maxPageSize || {};
|
||||
|
||||
/**
|
||||
* The cursor codec every paginated list below shares.
|
||||
*
|
||||
* Ordinarily a cursor is the next page's start index rendered as a string.
|
||||
* Under {@link ServerConfig.emptyStringCursor} the *first* boundary — and
|
||||
* only that one — is handed out as `""` instead, which is what exercises a
|
||||
* client's ability to tell an empty cursor from an absent one (#2220). Later
|
||||
* boundaries stay numeric, so a fixture with more than two pages still walks
|
||||
* to the end.
|
||||
*
|
||||
* `decode` maps `""` back to that boundary only when the mode is on; with it
|
||||
* off an empty cursor means page one, exactly as the previous
|
||||
* `cursor ? parseInt(cursor, 10) : 0` did.
|
||||
*/
|
||||
const emptyStringCursor = config.emptyStringCursor === true;
|
||||
const cursorCodec = (pageSize: number) => ({
|
||||
encode: (index: number): string =>
|
||||
emptyStringCursor && index === pageSize ? "" : index.toString(),
|
||||
decode: (cursor: string | undefined): number => {
|
||||
if (cursor === undefined || cursor === "") {
|
||||
return emptyStringCursor && cursor === "" ? pageSize : 0;
|
||||
}
|
||||
const parsed = parseInt(cursor, 10);
|
||||
return Number.isNaN(parsed) ? 0 : parsed;
|
||||
},
|
||||
});
|
||||
|
||||
// Emit each named tool a second time, same `name`, title marked so the two
|
||||
// rows are told apart on screen. See ServerConfig.duplicateToolNames (#1957).
|
||||
//
|
||||
|
|
@ -1322,6 +1367,7 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
// No pagination configured: one page holding everything, so the duplicate
|
||||
// override can share this handler without inventing a page size.
|
||||
const pageSize = maxPageSize.tools ?? Number.MAX_SAFE_INTEGER;
|
||||
const codec = cursorCodec(pageSize);
|
||||
|
||||
// Convert registered tools to Tool format, mirroring the SDK's tools/list.
|
||||
// The input-schema JSON comes from the SDK's memoised converter; the
|
||||
|
|
@ -1351,11 +1397,11 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
// boundary exactly as a real server's would.
|
||||
const allTools = withDuplicates(withRawSchemas(registeredTools));
|
||||
|
||||
const startIndex = cursor ? parseInt(cursor, 10) : 0;
|
||||
const startIndex = codec.decode(cursor);
|
||||
const endIndex = startIndex + pageSize;
|
||||
const page = allTools.slice(startIndex, endIndex);
|
||||
const nextCursor =
|
||||
endIndex < allTools.length ? endIndex.toString() : undefined;
|
||||
endIndex < allTools.length ? codec.encode(endIndex) : undefined;
|
||||
|
||||
return {
|
||||
tools: page,
|
||||
|
|
@ -1371,6 +1417,7 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
async (request, ctx) => {
|
||||
const cursor = request.params?.cursor;
|
||||
const pageSize = maxPageSize.resources!;
|
||||
const codec = cursorCodec(pageSize);
|
||||
|
||||
// Collect all resources (static + from templates)
|
||||
const allResources: Resource[] = [];
|
||||
|
|
@ -1411,11 +1458,11 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
}
|
||||
}
|
||||
|
||||
const startIndex = cursor ? parseInt(cursor, 10) : 0;
|
||||
const startIndex = codec.decode(cursor);
|
||||
const endIndex = startIndex + pageSize;
|
||||
const page = allResources.slice(startIndex, endIndex);
|
||||
const nextCursor =
|
||||
endIndex < allResources.length ? endIndex.toString() : undefined;
|
||||
endIndex < allResources.length ? codec.encode(endIndex) : undefined;
|
||||
|
||||
return {
|
||||
resources: page,
|
||||
|
|
@ -1432,6 +1479,7 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
async (request) => {
|
||||
const cursor = request.params?.cursor;
|
||||
const pageSize = maxPageSize.resourceTemplates!;
|
||||
const codec = cursorCodec(pageSize);
|
||||
|
||||
// Convert registered resource templates to ResourceTemplate format
|
||||
const allTemplates: Array<{
|
||||
|
|
@ -1468,11 +1516,11 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
}
|
||||
}
|
||||
|
||||
const startIndex = cursor ? parseInt(cursor, 10) : 0;
|
||||
const startIndex = codec.decode(cursor);
|
||||
const endIndex = startIndex + pageSize;
|
||||
const page = allTemplates.slice(startIndex, endIndex);
|
||||
const nextCursor =
|
||||
endIndex < allTemplates.length ? endIndex.toString() : undefined;
|
||||
endIndex < allTemplates.length ? codec.encode(endIndex) : undefined;
|
||||
|
||||
return {
|
||||
resourceTemplates: page as ResourceTemplate[],
|
||||
|
|
@ -1487,6 +1535,7 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
mcpServer.server.setRequestHandler("prompts/list", async (request) => {
|
||||
const cursor = request.params?.cursor;
|
||||
const pageSize = maxPageSize.prompts!;
|
||||
const codec = cursorCodec(pageSize);
|
||||
|
||||
// Convert registered prompts to Prompt format. The argument descriptors
|
||||
// are derived from the config's raw arg shape (the SDK no longer exposes
|
||||
|
|
@ -1506,11 +1555,11 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
}
|
||||
}
|
||||
|
||||
const startIndex = cursor ? parseInt(cursor, 10) : 0;
|
||||
const startIndex = codec.decode(cursor);
|
||||
const endIndex = startIndex + pageSize;
|
||||
const page = allPrompts.slice(startIndex, endIndex);
|
||||
const nextCursor =
|
||||
endIndex < allPrompts.length ? endIndex.toString() : undefined;
|
||||
endIndex < allPrompts.length ? codec.encode(endIndex) : undefined;
|
||||
|
||||
return {
|
||||
prompts: page,
|
||||
|
|
|
|||
|
|
@ -80,6 +80,12 @@ export interface ConfigFile {
|
|||
resourceTemplates?: number;
|
||||
prompts?: number;
|
||||
};
|
||||
/**
|
||||
* Hand out `""` as the cursor for page two of every paginated list, instead
|
||||
* of the usual numeric index. See {@link ServerConfig.emptyStringCursor}
|
||||
* (#2220).
|
||||
*/
|
||||
emptyStringCursor?: boolean;
|
||||
/**
|
||||
* Names of registered tools to emit **twice** in `tools/list` (same `name`,
|
||||
* the second's title marked "(duplicate)") — the nonconforming-but-real shape
|
||||
|
|
|
|||
|
|
@ -93,6 +93,7 @@ export function resolveConfig(config: ConfigFile): ServerConfig {
|
|||
skills: config.skills,
|
||||
appElicitation: config.appElicitation,
|
||||
maxPageSize: config.maxPageSize,
|
||||
emptyStringCursor: config.emptyStringCursor,
|
||||
duplicateToolNames: config.duplicateToolNames,
|
||||
rawToolSchemas: config.rawToolSchemas,
|
||||
extensionGatedTools: config.extensionGatedTools,
|
||||
|
|
|
|||
Loading…
Reference in a new issue