Skip to content

feat(core,client,server): ttl on list results (SEP-2549) #2070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
felixweinberger wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

feat(core,client,server): ttl on list results (SEP-2549) #2070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39e62a5
feat(core,client,server): ttl on list results (SEP-2549)
felixweinberger May 12, 2026
0ff1235
fix(types): revert hand-edit to auto-generated spec.types.ts; add @ts…
felixweinberger May 12, 2026
b8536fd
docs: resolve typedoc link warnings (treatWarningsAsErrors)
felixweinberger May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions packages/client/src/client/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ export class Client extends Protocol<ClientContext> {
private _cachedToolOutputValidators: Map<string, JsonSchemaValidator<unknown>> = new Map();
private _cachedKnownTaskTools: Set<string> = new Set();
private _cachedRequiredTaskTools: Set<string> = new Set();
/** Epoch ms after which the tool-metadata cache is stale (from SEP-2549 list `ttl`). */
private _toolCacheExpiresAt: number | undefined;
/** Set when the SEP-2549 ttl has elapsed. The cache is NOT cleared on expiry (validators stay enforced); this flag tells callers a refresh is recommended. */
private _toolCacheStale = false;
private _experimental?: { tasks: ExperimentalClientTasks };
private _listChangedDebounceTimers: Map<string, ReturnType<typeof setTimeout>> = new Map();
private _pendingListChangedConfig?: ListChangedHandlers;
Expand Down Expand Up @@ -924,11 +928,34 @@ export class Client extends Protocol<ClientContext> {
return this._cachedRequiredTaskTools.has(toolName);
}

/**
* Marks the tool-metadata cache stale when the SEP-2549 `ttl` has elapsed.
* Does NOT clear the caches: validators and required-task guards stay enforced
* until `listTools()` repopulates them. TTL is a freshness hint for re-fetching,
* not a signal to drop client-side enforcement (a server-supplied `ttl: 0` must
* not bypass output validation).
*/
private _maybeExpireToolCache(): void {
if (this._toolCacheExpiresAt !== undefined && Date.now() >= this._toolCacheExpiresAt) {
this._toolCacheStale = true;
this._toolCacheExpiresAt = undefined;
}
}

/** True when the tool list should be re-fetched (SEP-2549 `ttl` elapsed). Validators remain enforced while stale. */
get isToolCacheStale(): boolean {
this._maybeExpireToolCache();
return this._toolCacheStale;
}
Comment on lines +946 to +949
Copy link
Copy Markdown

@claude claude Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new public Client.isToolCacheStale getter (and the _maybeExpireToolCache / ≥1s-clamp / reset-on-listTools() logic behind it) has no test coverage — grepping for isToolCacheStale/toolCacheExpiresAt matches only client.ts. The PR description claims "New tests for ttl population + client staleness flag", but the added tests in mcp.test.ts only assert server-side ttl population; a fake-timer test exercising expiry, the clamp, and the stale-flag reset would close the gap.

Extended reasoning...

What's missing

Client gains a new public API surface in this PR:

  • get isToolCacheStale(): boolean — calls _maybeExpireToolCache() then returns _toolCacheStale.
  • _maybeExpireToolCache() — flips _toolCacheStale = true and clears _toolCacheExpiresAt once Date.now() >= _toolCacheExpiresAt.
  • cacheToolMetadata(tools, ttlSeconds?) — now sets _toolCacheExpiresAt = Date.now() + Math.max(1, ttlSeconds) * 1000 (the ≥1s clamp) and resets _toolCacheStale = false on every fresh listTools().

Grepping the repo for isToolCacheStale|toolCacheStale|toolCacheExpiresAt returns only packages/client/src/client/client.ts. No test file — neither packages/client/test/** nor test/integration/** — references the getter or the underlying state.

Why the PR description is misleading

"How Has This Been Tested?" says: "New tests for ttl population + client staleness flag". The diff adds exactly two tests to test/integration/test/server/mcp.test.ts:

  1. should include ttl on list results when listTtlSeconds is configured — asserts result.ttl === 60 on the four list endpoints, calling client.request({ method: '…/list' }) directly (which bypasses Client.listTools() and therefore never touches cacheToolMetadata).
  2. should omit ttl on list results when listTtlSeconds is not configured — asserts the property is absent.

Both are server-side wire-format tests. Neither calls client.listTools(), neither reads client.isToolCacheStale, and neither advances time. The "client staleness flag" half of the claim is not backed by the diff — this is the REVIEW.md recurring catch "prose that promises behavior the code no longer ships."

Step-by-step proof

  1. rg -l 'isToolCacheStale' → packages/client/src/client/client.ts (1 hit, the definition).
  2. rg -l 'toolCacheExpiresAt|_toolCacheStale' → same single file.
  3. Inspect the two new mcp.test.ts cases: they use client.request(…) (raw protocol), not client.listTools(), so even cacheToolMetadata(tools, result.ttl) is never reached in those tests.
  4. Therefore none of: (a) isToolCacheStale flipping to true after the ttl elapses, (b) Math.max(1, ttlSeconds) clamping a server-supplied ttl: 0, (c) _toolCacheStale resetting to false on a fresh listTools(), is exercised anywhere.

Why it matters

REVIEW.md checklist: "New behavior has vitest coverage including error paths". isToolCacheStale is a new public getter on the Client class with non-trivial, time-dependent semantics (and a security-adjacent clamp whose comment says "so a server cannot force immediate expiry of the validator cache"). It also currently has zero callsites in the SDK, so tests are the only thing that would catch a regression. Shipping it untested while the PR description asserts otherwise is worth flagging.

Suggested fix

Add a vitest case (e.g. in packages/client/test/client/client.test.ts or alongside the new integration tests) using vi.useFakeTimers():

test('isToolCacheStale tracks SEP-2549 ttl', async () => {
    vi.useFakeTimers();
    const mcpServer = new McpServer({ name: 's', version: '1' }, { listTtlSeconds: 0 }); // exercises the ≥1s clamp
    mcpServer.registerTool('t', {}, async () => ({ content: [] }));
    const client = new Client({ name: 'c', version: '1' });
    const [ct, st] = InMemoryTransport.createLinkedPair();
    await Promise.all([client.connect(ct), mcpServer.connect(st)]);

    await client.listTools();
    expect(client.isToolCacheStale).toBe(false);   // fresh after fetch
    vi.advanceTimersByTime(500);
    expect(client.isToolCacheStale).toBe(false);   // clamp: 0 → 1s, not yet stale
    vi.advanceTimersByTime(600);
    expect(client.isToolCacheStale).toBe(true);    // ttl elapsed

    await client.listTools();
    expect(client.isToolCacheStale).toBe(false);   // reset on re-fetch
    vi.useRealTimers();
});

This covers the getter, the clamp, and the reset in one place and makes the PR description accurate.


/**
* Cache validators for tool output schemas.
* Called after {@linkcode listTools | listTools()} to pre-compile validators for better performance.
*/
private cacheToolMetadata(tools: Tool[]): void {
private cacheToolMetadata(tools: Tool[], ttlSeconds?: number): void {
// Clamp to >= 1s so a server cannot force immediate expiry of the validator cache.
this._toolCacheExpiresAt = ttlSeconds === undefined ? undefined : Date.now() + Math.max(1, ttlSeconds) * 1000;
this._toolCacheStale = false;
this._cachedToolOutputValidators.clear();
this._cachedKnownTaskTools.clear();
this._cachedRequiredTaskTools.clear();
Expand Down Expand Up @@ -988,7 +1015,7 @@ export class Client extends Protocol<ClientContext> {
const result = await this._requestWithSchema({ method: 'tools/list', params }, ListToolsResultSchema, options);

// Cache the tools and their output schemas for future validation
this.cacheToolMetadata(result.tools);
this.cacheToolMetadata(result.tools, result.ttl);

return result;
}
Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/types/schemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,13 @@
* An opaque token representing the pagination position after the last returned result.
* If present, there may be more results available.
*/
nextCursor: CursorSchema.optional()
nextCursor: CursorSchema.optional(),
/**
* How long (in seconds) this result may be considered fresh before re-fetching (SEP-2549).
* Allows clients to cache list responses and poll on a predictable schedule, supplementing
* (not replacing) the `list_changed` notification mechanism.
*/
ttl: z.number().int().nonnegative().optional()

Check warning on line 622 in packages/core/src/types/schemas.ts

View check run for this annotation

Claude / Claude Code Review

Missing changeset for new public API across core/server/client 

This PR adds public API to three published packages — `@modelcontextprotocol/core` (`PaginatedResult.ttl`), `@modelcontextprotocol/server` (`ServerOptions.listTtlSeconds`), and `@modelcontextprotocol/client` (`Client.isToolCacheStale`) — but ships no `.changeset/*.md` entry (changeset-bot confirms "No Changeset found" at b8536fd). Without one, `changesets/action` will not bump versions or write CHANGELOG entries for any of these packages, so SEP-2549 ships silently; add a minor-bump changeset fo
Comment on lines +616 to +622
Copy link
Copy Markdown

@claude claude Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This PR adds public API to three published packages — @modelcontextprotocol/core (PaginatedResult.ttl), @modelcontextprotocol/server (ServerOptions.listTtlSeconds), and @modelcontextprotocol/client (Client.isToolCacheStale) — but ships no .changeset/*.md entry (changeset-bot confirms "No Changeset found" at b8536fd). Without one, changesets/action will not bump versions or write CHANGELOG entries for any of these packages, so SEP-2549 ships silently; add a minor-bump changeset for core/client/server (the bot comment has the one-click link).

Extended reasoning...

What's missing

This PR adds new public API surface to three separately-published packages but contains no changeset file:

  • @modelcontextprotocol/core — PaginatedResultSchema gains ttl: z.number().int().nonnegative().optional() (schemas.ts:616-622), which propagates ttl?: number onto the inferred PaginatedResult, ListToolsResult, ListResourcesResult, ListResourceTemplatesResult, ListPromptsResult, and ListTasksResult types.
  • @modelcontextprotocol/server — ServerOptions gains listTtlSeconds?: number (server.ts:90-96) and McpServer reads it to populate every list response.
  • @modelcontextprotocol/client — Client gains a public get isToolCacheStale(): boolean getter and cacheToolMetadata grows a ttlSeconds? parameter.

The changeset-bot comment on the PR explicitly reports "⚠️ No Changeset found — Latest commit: b8536fd", and the six changed files in the diff include nothing under .changeset/.

Why the repo expects one

.changeset/ already contains dozens of entries for directly comparable changes — e.g. add-resource-size-field.md (a single optional schema field, just like ttl) and respect-capability-negotiation.md — and .github/workflows/release.yml uses changesets/action to drive version bumps and CHANGELOG generation. Grepping .changeset/ for SEP-2549|listTtl|isToolCacheStale returns zero hits; the only ttl match is busy-rice-smoke.md, which is the unrelated "tasks - disallow requesting a null TTL" entry. So this PR is the first feature-level addition in the directory's history without a corresponding changeset.

Step-by-step proof

  1. PR diff touches packages/{core,server,client}/src/** and test/** — no .changeset/*.md is added.
  2. changeset-bot pinned comment at HEAD b8536fd: "Merging this PR will not cause a version bump for any packages."
  3. .github/workflows/release.yml runs changesets/action; with no pending changeset for these packages, the next release PR will not include a version bump or CHANGELOG entry for core/client/server.
  4. Consumers therefore receive the new ttl wire field, the new listTtlSeconds option, and the new isToolCacheStale getter under an unchanged version number with no release-notes mention — exactly the silent-API-addition the changesets workflow exists to prevent.

Why nothing else covers it

This is distinct from the four inline comments already on the PR (spec.types.ts hand-edit — now resolved; ServerOptions JSDoc placement; missing client-side test coverage; missing docs/ prose). None of those mention changesets or the release workflow. The bot comment surfaces the gap on the PR page, but it does not block merge, and three @claude review rounds have gone by without it being addressed.

Impact

Process / release-hygiene only — there is no runtime defect, hence nit. The practical consequence is that SEP-2549 ships without a semver bump or CHANGELOG line, which (a) makes the feature undiscoverable in release notes and (b) means downstream lockfiles won't pick it up as a new version. Easy to fix and routinely caught at merge time, but worth flagging since the PR claims "non-breaking change which adds functionality", which is precisely the changeset minor use-case.

Suggested fix

Add a single changeset (the bot's "Click here if you're a maintainer who wants to add a changeset" link pre-fills it), e.g.:

---
"@modelcontextprotocol/core": minor
"@modelcontextprotocol/server": minor
"@modelcontextprotocol/client": minor
---

feat: SEP-2549 list-result `ttl` — `PaginatedResult.ttl`, `ServerOptions.listTtlSeconds`, `Client.isToolCacheStale`

(@modelcontextprotocol/test-integration is private/test-only and does not need an entry; the bot's auto-suggested patch for it can be dropped.)

});

/**
Expand Down
6 changes: 6 additions & 0 deletions packages/core/test/spec.types.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,7 @@ type _K_ElicitRequestURLParams = Assert<AssertExactKeys<SDKTypes.ElicitRequestUR
type _K_PaginatedRequestParams = Assert<AssertExactKeys<SDKTypes.PaginatedRequestParams, SpecTypes.PaginatedRequestParams>>;
type _K_BaseMetadata = Assert<AssertExactKeys<SDKTypes.BaseMetadata, SpecTypes.BaseMetadata>>;
type _K_Implementation = Assert<AssertExactKeys<SDKTypes.Implementation, SpecTypes.Implementation>>;
// @ts-expect-error SEP-2549: SDK adds optional `ttl` ahead of upstream spec; remove once spec.types.ts has it
type _K_PaginatedResult = Assert<AssertExactKeys<SDKTypes.PaginatedResult, SpecTypes.PaginatedResult>>;
type _K_ListRootsResult = Assert<AssertExactKeys<SDKTypes.ListRootsResult, SpecTypes.ListRootsResult>>;
type _K_Root = Assert<AssertExactKeys<SDKTypes.Root, SpecTypes.Root>>;
Expand All @@ -847,9 +848,12 @@ type _K_ResourceTemplateReference = Assert<AssertExactKeys<SDKTypes.ResourceTemp
type _K_PromptReference = Assert<AssertExactKeys<SDKTypes.PromptReference, SpecTypes.PromptReference>>;
type _K_ToolAnnotations = Assert<AssertExactKeys<SDKTypes.ToolAnnotations, SpecTypes.ToolAnnotations>>;
type _K_Tool = Assert<AssertExactKeys<SDKTypes.Tool, SpecTypes.Tool>>;
// @ts-expect-error SEP-2549: SDK adds optional `ttl` ahead of upstream spec; remove once spec.types.ts has it
type _K_ListToolsResult = Assert<AssertExactKeys<SDKTypes.ListToolsResult, SpecTypes.ListToolsResult>>;
type _K_CallToolResult = Assert<AssertExactKeys<SDKTypes.CallToolResult, SpecTypes.CallToolResult>>;
// @ts-expect-error SEP-2549: SDK adds optional `ttl` ahead of upstream spec; remove once spec.types.ts has it
type _K_ListResourcesResult = Assert<AssertExactKeys<SDKTypes.ListResourcesResult, SpecTypes.ListResourcesResult>>;
// @ts-expect-error SEP-2549: SDK adds optional `ttl` ahead of upstream spec; remove once spec.types.ts has it
type _K_ListResourceTemplatesResult = Assert<AssertExactKeys<SDKTypes.ListResourceTemplatesResult, SpecTypes.ListResourceTemplatesResult>>;
type _K_ReadResourceResult = Assert<AssertExactKeys<SDKTypes.ReadResourceResult, SpecTypes.ReadResourceResult>>;
type _K_ResourceContents = Assert<AssertExactKeys<SDKTypes.ResourceContents, SpecTypes.ResourceContents>>;
Expand All @@ -859,6 +863,7 @@ type _K_Resource = Assert<AssertExactKeys<SDKTypes.Resource, SpecTypes.Resource>
// @ts-expect-error Genuine mismatch: SDK PromptArgument is missing 'title' from spec
type _K_PromptArgument = Assert<AssertExactKeys<SDKTypes.PromptArgument, SpecTypes.PromptArgument>>;
type _K_Prompt = Assert<AssertExactKeys<SDKTypes.Prompt, SpecTypes.Prompt>>;
// @ts-expect-error SEP-2549: SDK adds optional `ttl` ahead of upstream spec; remove once spec.types.ts has it
type _K_ListPromptsResult = Assert<AssertExactKeys<SDKTypes.ListPromptsResult, SpecTypes.ListPromptsResult>>;
type _K_GetPromptResult = Assert<AssertExactKeys<SDKTypes.GetPromptResult, SpecTypes.GetPromptResult>>;
type _K_TextContent = Assert<AssertExactKeys<SDKTypes.TextContent, SpecTypes.TextContent>>;
Expand Down Expand Up @@ -902,6 +907,7 @@ type _K_RelatedTaskMetadata = Assert<AssertExactKeys<SDKTypes.RelatedTaskMetadat
type _K_Task = Assert<AssertExactKeys<SDKTypes.Task, SpecTypes.Task>>;
type _K_CreateTaskResult = Assert<AssertExactKeys<SDKTypes.CreateTaskResult, SpecTypes.CreateTaskResult>>;
type _K_GetTaskResult = Assert<AssertExactKeys<SDKTypes.GetTaskResult, SpecTypes.GetTaskResult>>;
// @ts-expect-error SEP-2549: SDK adds optional `ttl` ahead of upstream spec; remove once spec.types.ts has it
type _K_ListTasksResult = Assert<AssertExactKeys<SDKTypes.ListTasksResult, SpecTypes.ListTasksResult>>;
type _K_CancelTaskResult = Assert<AssertExactKeys<SDKTypes.CancelTaskResult, SpecTypes.CancelTaskResult>>;
type _K_GetTaskPayloadResult = Assert<AssertExactKeys<SDKTypes.GetTaskPayloadResult, SpecTypes.GetTaskPayloadResult>>;
Expand Down
15 changes: 13 additions & 2 deletions packages/server/src/server/mcp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,18 @@ export class McpServer {
private _registeredTools: { [name: string]: RegisteredTool } = {};
private _registeredPrompts: { [name: string]: RegisteredPrompt } = {};
private _experimental?: { tasks: ExperimentalMcpServerTasks };
private readonly _listTtl: number | undefined;

constructor(serverInfo: Implementation, options?: ServerOptions) {
this.server = new Server(serverInfo, options);
// Wire schema is `.int().nonnegative()`; coerce so a fractional or negative
// option value does not make every client `listTools()` throw a Zod error.
this._listTtl = options?.listTtlSeconds === undefined ? undefined : Math.max(0, Math.round(options.listTtlSeconds));
}

/** Builds the SEP-2549 `ttl` slot for paginated list results when configured. */
private _ttl(): { ttl?: number } {
return this._listTtl === undefined ? {} : { ttl: this._listTtl };
}

/**
Expand Down Expand Up @@ -136,6 +145,7 @@ export class McpServer {
this.server.setRequestHandler(
'tools/list',
(): ListToolsResult => ({
...this._ttl(),
tools: Object.entries(this._registeredTools)
.filter(([, tool]) => tool.enabled)
.map(([name, tool]): Tool => {
Expand Down Expand Up @@ -467,7 +477,7 @@ export class McpServer {
}
}

return { resources: [...resources, ...templateResources] };
return { ...this._ttl(), resources: [...resources, ...templateResources] };
});

this.server.setRequestHandler('resources/templates/list', async () => {
Expand All @@ -477,7 +487,7 @@ export class McpServer {
...template.metadata
}));

return { resourceTemplates };
return { ...this._ttl(), resourceTemplates };
});

this.server.setRequestHandler('resources/read', async (request, ctx) => {
Expand Down Expand Up @@ -525,6 +535,7 @@ export class McpServer {
this.server.setRequestHandler(
'prompts/list',
(): ListPromptsResult => ({
...this._ttl(),
prompts: Object.entries(this._registeredPrompts)
.filter(([, prompt]) => prompt.enabled)
.map(([name, prompt]): Prompt => {
Expand Down
8 changes: 8 additions & 0 deletions packages/server/src/server/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ export type ServerOptions = ProtocolOptions & {
* @default {@linkcode DefaultJsonSchemaValidator} ({@linkcode index.AjvJsonSchemaValidator | AjvJsonSchemaValidator} on Node.js, `CfWorkerJsonSchemaValidator` on Cloudflare Workers)
*/
jsonSchemaValidator?: jsonSchemaValidator;

/**
* Optional `ttl` (in seconds) included on `tools/list`, `prompts/list`, `resources/list`,
* and `resources/templates/list` responses (SEP-2549). Tells clients how long the response
* may be considered fresh; clients may cache and re-poll on this schedule. Supplements, does
* not replace, the `list_changed` notification mechanism.
*/
listTtlSeconds?: number;
Comment on lines +90 to +96
Copy link
Copy Markdown

@claude claude Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 listTtlSeconds is declared on ServerOptions (the low-level Server option type) but is only consumed by McpServer — a user writing new Server(info, { listTtlSeconds: 60 }) will see this JSDoc promising "included on tools/list ... responses" yet the option is silently ignored, since Server has no built-in list handlers. Consider qualifying the JSDoc with "Only applied by McpServer; low-level Server users must add ttl to list responses manually", or introducing an McpServerOptions extends ServerOptions type for the field.

Extended reasoning...

What the issue is

The new listTtlSeconds option is declared on ServerOptions in packages/server/src/server/server.ts:90-96, which is the constructor-options type for the low-level Server class. However, the Server constructor never reads this field — it reads options.capabilities, options.instructions, and options.jsonSchemaValidator, but not options.listTtlSeconds. The only consumer is McpServer at packages/server/src/server/mcp.ts:82-83, which reads it to populate its built-in list handlers.

This makes listTtlSeconds the first field on ServerOptions that the low-level Server silently ignores, breaking the prior contract that every ServerOptions field is consumed by Server itself.

How it manifests

A user of the low-level API writes:

const server = new Server({ name: 's', version: '1' }, { listTtlSeconds: 60 });
server.setRequestHandler('tools/list', () => ({ tools: [...] }));

IntelliSense surfaces listTtlSeconds with the JSDoc "Optional ttl (in seconds) included on tools/list, prompts/list, resources/list, and resources/templates/list responses" — an unconditional promise. The user reasonably expects ttl: 60 to appear on their list responses. It does not: Server has no built-in list handlers and never injects anything into user-registered handler results, so the option is a silent no-op.

Why existing code doesn't prevent it

There is no separate McpServerOptions type — McpServer's constructor signature is (serverInfo, options?: ServerOptions), so the field had to land on ServerOptions to reach McpServer. That's a reasonable structural choice, but it means the JSDoc is now read by two audiences (low-level Server users and high-level McpServer users) while only being true for one of them. Nothing in the type system or runtime warns low-level users that the option is inert.

Step-by-step proof

  1. Construct new Server(info, { listTtlSeconds: 60 }).
  2. Server constructor (server.ts) assigns _capabilities, _instructions, _jsonSchemaValidator from options — listTtlSeconds is never referenced.
  3. User calls server.setRequestHandler('tools/list', () => ({ tools: [] })).
  4. Client sends tools/list; Protocol dispatches directly to the user handler. No code path reads listTtlSeconds or spreads a ttl into the result.
  5. Response is { tools: [] } with no ttl, contradicting the JSDoc.

Contrast with McpServer: its constructor stores Math.max(0, Math.round(options.listTtlSeconds)) in _listTtl and spreads ...this._ttl() into every built-in list handler result.

Impact

Low — Server is @deprecated ("Use McpServer instead"), and low-level users author their own list handlers anyway, so Server couldn't auto-inject ttl even if it wanted to. This is a documentation-accuracy / API-placement nit (matching the repo's recurring "prose that promises behavior the code doesn't ship" pattern), not a correctness bug.

Suggested fix

Either:

  • Append to the JSDoc: "Only applied by {@linkcode McpServer}; the low-level Server does not register list handlers, so direct Server users must include ttl on their list responses manually.", or
  • Introduce type McpServerOptions = ServerOptions & { listTtlSeconds?: number } and move the field there, narrowing the type to the class that actually honors it.

Comment on lines +90 to +96
Copy link
Copy Markdown

@claude claude Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This PR adds two new public API knobs (ServerOptions.listTtlSeconds and Client.isToolCacheStale) but does not add prose documentation for them under docs/ — the PR checklist's "I have added or updated documentation as needed" tick is currently inaccurate. A short section in docs/server.md (alongside the existing ## Server instructions section) and a mention of the staleness getter in docs/client.md would close the gap.

Extended reasoning...

What's missing

REVIEW.md's Tests & docs checklist says: "New feature: verify prose documentation is added (not just JSDoc), and assess whether examples/ needs a new or updated example." This PR introduces two user-facing API surfaces:

  • ServerOptions.listTtlSeconds (server.ts:90-96) — a new constructor option that causes McpServer to attach ttl to all four list responses.
  • Client.isToolCacheStale (client.ts) — a new public getter that tells callers when the SEP-2549 ttl has elapsed and the tool list should be re-fetched.

Both have JSDoc, but neither appears in the prose docs.

Step-by-step proof

  1. docs/ exists and contains server.md, client.md, server-quickstart.md, migration.md, etc.
  2. rg 'listTtlSeconds|isToolCacheStale|SEP-2549' docs/ → zero hits.
  3. rg 'listTtlSeconds|isToolCacheStale|SEP-2549' examples/ → zero hits.
  4. The full-repo grep matches only the six source/test files in this diff.
  5. Sibling ServerOptions fields do have prose docs, establishing the convention: docs/server.md has a dedicated ## Server instructions section with example code, and jsonSchemaValidator / enforceStrictCapabilities appear in docs/server.md, docs/client.md, and docs/migration.md. So listTtlSeconds is the first ServerOptions field with no prose coverage at all.
  6. The PR's own checklist ticks "[x] I have added or updated documentation as needed", while "[ ] Documentation update" under Types of changes is unticked — only JSDoc was added.

Why existing artifacts don't cover it

JSDoc surfaces in IntelliSense and the generated API reference, but the repo's review checklist explicitly distinguishes prose documentation from JSDoc, and the docs/*.md guides are where users learn that a feature exists in the first place. A user reading docs/server.md to discover "what options can I pass to McpServer?" will not see listTtlSeconds; a user reading docs/client.md to learn the caching behavior of listTools() will not learn that isToolCacheStale exists or that staleness is a re-fetch hint (validators stay enforced) rather than a cache-invalidation signal — a nuance worth spelling out in prose.

Impact

Documentation-completeness only — no runtime correctness impact, hence nit. The convention is also not perfectly uniform (not every ServerOptions field has its own ## heading), so this is a should-fix rather than a blocker. It is distinct from the three already-posted inline comments on this PR (spec.types.ts hand-edit, ServerOptions JSDoc placement, and missing client-side test coverage); none of those mention docs/.

Suggested fix

  • docs/server.md: add a short ## List result TTL (SEP-2549) section near ## Server instructions, showing new McpServer(info, { listTtlSeconds: 60 }) and noting that the value is rounded to a non-negative integer and supplements (does not replace) list_changed notifications.
  • docs/client.md: add a paragraph under the listTools() / caching discussion explaining client.isToolCacheStale, that it flips true once the server-supplied ttl elapses, and that validators remain enforced while stale (it's a re-fetch hint, not a drop-enforcement signal).
  • Optionally untick the docs checkbox in the PR description until the above lands.

};

/**
Expand Down
37 changes: 37 additions & 0 deletions test/integration/test/server/mcp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,43 @@ describe('Zod v4', () => {
]);
});

/***
* Test: SEP-2549 list TTL
*/
test('should include ttl on list results when listTtlSeconds is configured', async () => {
const mcpServer = new McpServer({ name: 'test server', version: '1.0' }, { listTtlSeconds: 60 });
const client = new Client({ name: 'test client', version: '1.0' });

mcpServer.registerTool('t', {}, async () => ({ content: [] }));
mcpServer.registerPrompt('p', {}, async () => ({ messages: [] }));
mcpServer.registerResource('r', 'file:///r', {}, async () => ({ contents: [] }));

const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
await Promise.all([client.connect(clientTransport), mcpServer.connect(serverTransport)]);

const tools = await client.request({ method: 'tools/list' });
expect(tools.ttl).toBe(60);
const prompts = await client.request({ method: 'prompts/list' });
expect(prompts.ttl).toBe(60);
const resources = await client.request({ method: 'resources/list' });
expect(resources.ttl).toBe(60);
const templates = await client.request({ method: 'resources/templates/list' });
expect(templates.ttl).toBe(60);
});

test('should omit ttl on list results when listTtlSeconds is not configured', async () => {
const mcpServer = new McpServer({ name: 'test server', version: '1.0' });
const client = new Client({ name: 'test client', version: '1.0' });

mcpServer.registerTool('t', {}, async () => ({ content: [] }));

const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
await Promise.all([client.connect(clientTransport), mcpServer.connect(serverTransport)]);

const tools = await client.request({ method: 'tools/list' });
expect(tools).not.toHaveProperty('ttl');
});

/***
* Test: Updating Existing Tool
*/
Expand Down
Loading