Skip to content

Commit 30dcd41

Browse files
committed
fix: compose client-agnostic tools eagerly so recovery restores them
The gating refactor made ALL helper-tool composition wait for the client (clientKnown), which broke the recovery contract: a node that restores a session via loadToolsByName without receiving an initialize (see tests/integration/internals.test.ts "load and restore tools from a tool list") silently lost every internal helper, since they sat in the pending queue and were never flushed. Split the two concerns that were conflated: - Mode-unresolved ('auto' before initialize): still defer the whole source — composing with the preliminary DEFAULT mode would pick wrong (non-widget) variants. This restores serverModeResolved as the compose-vs-defer gate. - Client-unknown: only withhold client-gated tools (report-problem) via isToolServable/isClientGatedTool; client-agnostic tools compose immediately, so explicit-mode recovery restores them without an initialize. The initialize flush re-composes and adds the gated tools once the client is known. Adds a unit test for the recovery case. Also renames the stale getServerInstructions `feedbackAvailable` param and fixes two stale comments left by the rename/gating commits. Claude-Session: https://claude.ai/code/session_01TV4x9VgMCvXGF7TftskLAn
1 parent 941e3e6 commit 30dcd41

4 files changed

Lines changed: 79 additions & 37 deletions

File tree

src/mcp/server.ts

Lines changed: 52 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ import {
9191
computeToolResponseBytes,
9292
getToolCallErrorUserText,
9393
} from '../utils/mcp.js';
94-
import { isToolBlockedForClient } from '../utils/mcp_clients.js';
94+
import { isClientGatedTool, isToolBlockedForClient } from '../utils/mcp_clients.js';
9595
import { buildPaymentRequiredResponse, isX402PaymentRequiredError } from '../utils/payment_errors.js';
9696
import { createProgressTracker } from '../utils/progress.js';
9797
import { getServerInstructions } from '../utils/server-instructions/index.js';
@@ -193,13 +193,17 @@ export class ActorsMcpServer {
193193
* values bypass auto-detect.
194194
*/
195195
private readonly serverModeOption: ServerModeOption;
196+
/** True once the server mode is final: at construction for explicit `default`/`apps`, or after
197+
* the initialize handler resolves `'auto'`. Composing before this in `'auto'` mode would use
198+
* the preliminary DEFAULT mode and produce the wrong (non-widget) tool variants, so composition
199+
* waits for it. Distinct from {@link clientKnown}, which only withholds client-gated tools. */
200+
private serverModeResolved: boolean;
196201
/**
197-
* Tool requests queued until the connecting client is known. Client-dependent composition
198-
* (helper tools, per-client blocklist) is deferred to the initialize handler because MCP
199-
* guarantees no `tools/list` before `initialize`. Actor tools are upserted immediately
200-
* (client-agnostic); we also capture the exact actor-tool slice fetched for each request so
201-
* the flush composes every entry against *its own* actor list rather than the accumulated
202-
* union across unrelated requests.
202+
* Tool sources queued until composition is possible. Enqueued when the mode is not yet resolved
203+
* (`'auto'` before initialize), and re-composed by the initialize flush — which is also when the
204+
* client becomes known, so any client-gated tools withheld by an eager compose are added then.
205+
* We capture the exact actor-tool slice fetched for each request so the flush composes every
206+
* entry against *its own* actor list rather than the accumulated union across unrelated requests.
203207
*/
204208
private pendingToolsUntilClientKnown: { input: Input; actorTools: ToolEntry[] }[] = [];
205209

@@ -237,6 +241,7 @@ export class ActorsMcpServer {
237241
// Preliminary resolution — re-resolved inside the initialize handler once
238242
// client capabilities are known (only for 'auto').
239243
this.serverMode = resolveServerMode(this.serverModeOption, false);
244+
this.serverModeResolved = this.serverModeOption !== 'auto';
240245

241246
const { setupSigintHandler = true } = options;
242247
this.server = new Server(getServerInfo(), {
@@ -329,6 +334,7 @@ export class ActorsMcpServer {
329334
if (resolved !== this.serverMode) {
330335
this.serverMode = resolved;
331336
}
337+
this.serverModeResolved = true;
332338
}
333339

334340
// Setting this makes `clientKnown` true, so the queued compose below (and any later
@@ -356,30 +362,38 @@ export class ActorsMcpServer {
356362
}
357363

358364
/** True once the connecting client is known (set in the initialize handler, or hydrated by a
359-
* recovery path). Client-dependent tool composition must wait for this so the per-client
360-
* blocklist can be applied at the single compose choke point. */
365+
* recovery path). Only client-gated tools wait for this so the per-client blocklist can be
366+
* applied; client-agnostic tools compose regardless. */
361367
private get clientKnown(): boolean {
362368
return this.options.initializeRequestData != null;
363369
}
364370

365371
/**
366-
* Compose the final tool list for the current client: resolve mode-specific tools, then drop
367-
* any that are not servable for this connection (see {@link isToolServable}). The single choke
368-
* point for client gating — used by both the initialize flush and post-init loads (e.g.
369-
* recovery), so a blocked tool can never slip in via a late load. Never filters actor tools
370-
* (they are always servable); safe to call before or after the client is known.
372+
* Compose the tool list for the current connection: resolve mode-specific tools, then drop any
373+
* not currently servable (see {@link isToolServable}). Client-agnostic tools compose eagerly, so
374+
* a recovery/rehydration load without an initialize still restores them. Client-gated tools are
375+
* withheld until the client is known and re-added by the initialize flush. Used by every
376+
* input-driven load path and the flush. (loadActorsAsTools upserts actor tools directly; actor
377+
* tools are never client-gated, so they need no filtering.)
371378
*/
372379
private composeToolsForClient(input: Input, actorTools: ToolEntry[]): ToolEntry[] {
373380
return getToolsForServerMode(input, actorTools, this.serverMode).filter((tool) =>
374381
this.isToolServable(tool.name),
375382
);
376383
}
377384

378-
/** Whether a tool may be served on this connection: not blocklisted for the client, and — for
379-
* report-problem, whose only function is forwarding submissions via telemetry — only when
380-
* telemetry is enabled (otherwise it would just fake an acknowledgement into the void). */
385+
/**
386+
* Whether a tool may be served on this connection right now:
387+
* - report-problem's only function is forwarding submissions via telemetry, so it is never
388+
* servable when telemetry is disabled (it would just fake an acknowledgement into the void).
389+
* - A client-gated tool (has a {@link TOOL_CLIENT_BLOCKLIST} rule) cannot be judged until the
390+
* client is known, so it is withheld until then — the initialize flush re-composes and adds it
391+
* if the client allows. Client-agnostic tools are always servable, so they compose eagerly and
392+
* survive a recovery load that never sees an initialize.
393+
*/
381394
private isToolServable(toolName: string): boolean {
382395
if (toolName === HELPER_TOOLS.PROBLEM_REPORT && !this.telemetryEnabled) return false;
396+
if (isClientGatedTool(toolName) && !this.clientKnown) return false;
383397
return !isToolBlockedForClient(toolName, this.options.initializeRequestData);
384398
}
385399

@@ -392,11 +406,10 @@ export class ActorsMcpServer {
392406

393407
this.pendingToolsUntilClientKnown = [];
394408

395-
// Notify after the flush so shared-state handlers (e.g. Redis recovery) see
396-
// the final tool set, including mode-specific helpers added here. Pre-init,
397-
// `loadToolsByName` may have fired `upsertTools(actorTools, true)` with actor
398-
// tools only (helpers still queued), and `loadToolsFromUrl` / `loadToolsFromInput`
399-
// don't notify at all — this call reconciles both paths to the complete set.
409+
// Notify after the flush so shared-state handlers (e.g. Redis recovery) see the final tool
410+
// set. Load paths already upserted the client-agnostic tools pre-init; re-upserting is
411+
// idempotent, and this pass adds the client-gated tools (e.g. report-problem) now that the
412+
// client is known, reconciling shared state to the complete set.
400413
if (tools.length > 0) this.upsertTools(tools, true);
401414
}
402415

@@ -491,16 +504,21 @@ export class ActorsMcpServer {
491504
paymentProvider: this.options.paymentProvider,
492505
});
493506

494-
if (!this.clientKnown) {
507+
// 'auto' before initialize: mode not final — composing now would pick wrong variants, so
508+
// defer the whole source to the initialize flush (actor tools are mode-agnostic, upsert now).
509+
if (!this.serverModeResolved) {
495510
this.pendingToolsUntilClientKnown.push({ input: restoreInput, actorTools });
496511
if (actorTools.length > 0) this.upsertTools(actorTools, true);
497512
return;
498513
}
499-
514+
// Mode is final: compose eagerly so client-agnostic tools restore even without an initialize
515+
// (recovery / rehydration). Client-gated tools are withheld until the client is known; queue
516+
// the source so the initialize flush re-composes and adds them.
500517
const toolsToLoad = this.composeToolsForClient(restoreInput, actorTools);
501518
if (toolsToLoad.length > 0) {
502519
this.upsertTools(toolsToLoad, actorTools.length > 0);
503520
}
521+
if (!this.clientKnown) this.pendingToolsUntilClientKnown.push({ input: restoreInput, actorTools });
504522
}
505523

506524
/**
@@ -528,10 +546,10 @@ export class ActorsMcpServer {
528546
paymentProvider: this.options.paymentProvider,
529547
});
530548

531-
if (!this.clientKnown) {
549+
if (!this.serverModeResolved) {
532550
this.pendingToolsUntilClientKnown.push({ input, actorTools });
533551
if (actorTools.length > 0) {
534-
log.debug('Loading actor tools from query parameters before the client is known');
552+
log.debug('Loading actor tools from query parameters before mode resolution');
535553
this.upsertTools(actorTools, false);
536554
}
537555
return;
@@ -542,12 +560,14 @@ export class ActorsMcpServer {
542560
log.debug('Loading tools from query parameters');
543561
this.upsertTools(tools, false);
544562
}
563+
if (!this.clientKnown) this.pendingToolsUntilClientKnown.push({ input, actorTools });
545564
}
546565

547566
/**
548567
* Two-phase: getActors (async, client-agnostic Apify fetch) then composeToolsForClient
549-
* (sync, client-dependent compose). When the client is not yet known, queue actorTools and
550-
* let the initialize handler compose them later.
568+
* (sync compose + servability filter). If the mode isn't resolved yet ('auto' before initialize)
569+
* the whole source is queued for the flush. Otherwise tools compose immediately; client-gated
570+
* tools are withheld until the client is known, and the source is queued so the flush adds them.
551571
*
552572
* Don't move the getActors await into the initialize handler — clients time out
553573
* waiting for InitializeResult. The queue buffers already-fetched data, not network
@@ -558,13 +578,14 @@ export class ActorsMcpServer {
558578
actorStore: this.actorStore,
559579
paymentProvider: this.options.paymentProvider,
560580
});
561-
if (!this.clientKnown) {
581+
if (!this.serverModeResolved) {
562582
this.pendingToolsUntilClientKnown.push({ input, actorTools });
563583
if (actorTools.length > 0) this.upsertTools(actorTools);
564584
return;
565585
}
566586
const tools = this.composeToolsForClient(input, actorTools);
567587
if (tools.length > 0) this.upsertTools(tools);
588+
if (!this.clientKnown) this.pendingToolsUntilClientKnown.push({ input, actorTools });
568589
}
569590

570591
/** Delete tools from the server and notify the handler.
@@ -874,8 +895,8 @@ export class ActorsMcpServer {
874895
let toolResult: unknown = null;
875896
const captureResult = <T>(r: T): T => {
876897
// On a failed result, nudge the agent to report the blocker via report-problem at the
877-
// moment it decides what to do next. Gated on the tool actually being served, so
878-
// Anthropic clients (hard-blocked) and feedback-off selections never see it.
898+
// moment it decides what to do next. Gated on the tool actually being served (see
899+
// isToolServable), so clients where it is blocklisted or telemetry is off never see it.
879900
const augmented = appendReportProblemNudge(r, {
880901
failingToolName: resolvedToolName,
881902
available: this.tools.has(HELPER_TOOLS.PROBLEM_REPORT),

src/utils/mcp_clients.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ export function isToolBlockedForClient(toolName: string, initializeRequestData?:
1717
return blockedClients.some((blocked) => clientName.includes(blocked));
1818
}
1919

20+
/**
21+
* True when `toolName` has any client-gating rule in {@link TOOL_CLIENT_BLOCKLIST}. Such a tool
22+
* cannot be judged until the connecting client is known, so composition withholds it until then;
23+
* tools with no rule are client-agnostic and compose immediately.
24+
*/
25+
export function isClientGatedTool(toolName: string): boolean {
26+
return TOOL_CLIENT_BLOCKLIST[toolName] !== undefined;
27+
}
28+
2029
/**
2130
* Determines if the MCP client supports dynamic tools based on the InitializeRequest data.
2231
*/

src/utils/server-instructions/index.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ import { SERVER_MODE } from '../../types.js';
1414
* Build server instructions for the given mode.
1515
*
1616
* Apps-only sections are omitted in default mode to prevent models from
17-
* attempting to call widget tools that are not registered. The feedback section is
18-
* emitted only when `feedbackAvailable` is true — i.e. `report-problem` is actually
19-
* served — so clients that never receive the tool (Anthropic surfaces, or a tool
20-
* selection that excludes the `report-problem` category) are not told to call it.
17+
* attempting to call widget tools that are not registered. The report-problem line is
18+
* emitted only when `reportProblemAvailable` is true — i.e. `report-problem` is actually
19+
* served — so clients that never receive the tool (Anthropic surfaces, telemetry off, or a
20+
* tool selection that excludes the `report-problem` category) are not told to call it.
2121
*/
22-
export function getServerInstructions(mode: SERVER_MODE = SERVER_MODE.DEFAULT, feedbackAvailable = false): string {
22+
export function getServerInstructions(mode: SERVER_MODE = SERVER_MODE.DEFAULT, reportProblemAvailable = false): string {
2323
const isApps = mode === SERVER_MODE.APPS;
2424

2525
return `
@@ -87,7 +87,7 @@ ${
8787
- **Dedicated Actor tools (e.g. ${RAG_WEB_BROWSER}) vs \`${HELPER_TOOLS.ACTOR_CALL}\`:**
8888
Prefer dedicated tools when available; use \`${HELPER_TOOLS.ACTOR_CALL}\` only when no specialized tool exists in the Apify store.
8989
${
90-
feedbackAvailable
90+
reportProblemAvailable
9191
? `
9292
If a tool or Actor fails and you cannot resolve it, you can report it with \`${HELPER_TOOLS.PROBLEM_REPORT}\`.
9393
`

tests/unit/mcp.server.report_problem_gating.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,16 @@ describe('report-problem client gating', () => {
110110

111111
expect(server.tools.has(HELPER_TOOLS.PROBLEM_REPORT)).toBe(false);
112112
});
113+
114+
// Only client-gated tools wait for the client; everything else must load eagerly, or a
115+
// recovery/rehydration node that restores tools via loadToolsByName without ever receiving an
116+
// initialize would silently lose all its helper tools.
117+
it('restores client-agnostic helper tools loaded before any initialize', async () => {
118+
const server = track(makeServer());
119+
120+
await server.loadToolsByName([HELPER_TOOLS.ACTOR_RUNS_GET, HELPER_TOOLS.DATASET_GET_ITEMS], {} as never);
121+
122+
expect(server.tools.has(HELPER_TOOLS.ACTOR_RUNS_GET)).toBe(true);
123+
expect(server.tools.has(HELPER_TOOLS.DATASET_GET_ITEMS)).toBe(true);
124+
});
113125
});

0 commit comments

Comments
 (0)