Skip to content

Cloudflare host: partial tool catalogs, refresh-token reuse, and annotations dropped from describe.tool #1979

Description

@daviesayo

Summary

Running apps/host-cloudflare in production (v1.6.8, Workers + D1 + R2), I hit three defects that share one shape: state that is only correct on a single-process host silently degrades on Workers. Two of them are visible to callers as "the integration lost its tools" and "the connection is degraded again"; the third is a smaller gap in what describe.tool reports.

All three are read from main at f1d95f2 unless noted. I have a reproduction for the first one's arithmetic and live evidence for all three.


1. A concurrent read during a catalog rebuild sees a partial tool catalog (Cloudflare host)

What I observed

An MCP integration with 16 tools silently became an integration with 7 tools. Calling a missing one returned tool_not_found with the seven survivors offered as suggestions, so from the caller's side the server simply "did not have" get_digest any more. A connections.refresh restored all 16 immediately, twice.

The seven survivors were exactly the first seven tools in the server's tools/list order. Not a random subset, not an alphabetical one: a prefix.

Why 7

persistCatalog replaces a connection's catalog as delete-then-insert (packages/core/sdk/src/executor.ts, produceConnectionToolsUnshared):

yield* persistCatalog(
  Effect.gen(function* () {
    yield* core.deleteMany("tool", { where });
    yield* core.deleteMany("definition", { where });
    yield* core.createMany("tool", toolRows);
    yield* core.createMany("definition", definitionRows);
    yield* stampSynced(existingRow);
  }),
);

persistCatalog wraps that in transaction(...). On the Cloudflare host the wrapper is not a transaction:

// apps/host-cloudflare/src/db/d1.ts
// `interactiveTransactions: false` — D1 rejects interactive transactions, so
// the fuma adapter runs transaction callbacks directly (auto-commit per
// statement).
const { db: fumaDb, fuma } = createExecutorFumaDb(drizzleDb, {
  ...options,
  interactiveTransactions: false,
  maxBoundParameters: 100,
});

And createMany splits into batches sized by the bound-parameter budget (packages/core/fumadb/src/adapters/drizzle/query.ts):

const rowsPerStatement = Math.floor((budget - reservedParameters) / columnsPerRow);

The tool insert binds 13 columns per row (tenant, owner, subject, integration, connection, plugin_id, name, description, input_schema, output_schema, annotations, created_at, updated_at), so on D1:

floor(100 / 13) = 7 rows per statement

Each statement auto-commits. So during any rebuild of a 16-tool connection, a concurrent reader observes a committed catalog of 0, then 7, then 14, then 16 tools. My 7 was one insert batch. The window scales with catalog size: a 386-tool integration rebuilds through 55 committed intermediate states.

The rebuild that bit me was a background stale-catalog sync, triggered while I was calling a tool on the same connection.

Why it stays invisible

Every guard in this path is about the plugin's answer, not about readers:

  • result.incomplete === true preserves the old catalog (correct, and it works).
  • background + remoteToolCatalog + zero tools preserves a non-empty catalog (correct).
  • tool_sync_failed health is stamped on failure (correct).

None of them apply here, because nothing failed. The plugin returned all 16 tools; the catalog was simply readable mid-swap. The connection's health stays healthy the whole time, so there is no signal at all — the caller just sees a smaller catalog and a tool_not_found that reads like the server's own answer.

Suggested fix

D1 has no interactive transactions, but D1Database.batch() runs its statements in a single implicit transaction, which is exactly what this needs. Options, roughly in order of how much they change:

  1. Route persistCatalog through one batch on adapters that advertise no interactive transactions: collect the delete + insert statements and submit them as one batch(). Keeps the current shape, restores atomicity on D1.
  2. Generation pointer. Insert the new catalog under a new generation value, then flip the connection row's active_generation in one statement, then delete the old rows. Readers filter on the pointer, so they never see a half-built set. Works on any engine with no transaction support at all.
  3. Serve the old catalog while a rebuild is in flight (readers fall back to the previous generation).

Option 1 is the smallest; option 2 is the one that also survives an insert that fails halfway.

I am happy to open a PR for whichever you prefer.


2. The in-flight refresh gate does not dedup on Workers, so rotating refresh tokens get reused

What I observed

Connections show degraded in the dashboard. Clicking "Check now" reports an OAuth problem; clicking "Reconnect" opens a popup that shows nothing; dismissing it and re-checking reports healthy, with no other action taken. This has happened repeatedly across cloudflare_mcp and railway_mcp.

From the API side, in one session and a few minutes apart:

  • cloudflare_mcpoauth_refresh_failed: OAuth token refresh was rejected (invalid_client): Client not found — twice, with successful calls before and after each failure.
  • railway_mcpoauth_refresh_failed: refresh token refused (invalid_scope): refresh token missing requested scope.

The mechanism, as the code already documents it

packages/core/sdk/src/executor.ts is explicit about the gate's reach:

// SCOPE OF THE GUARANTEE: dedup reaches exactly as far as one root DB handle
// in one process. ... Multi-instance deployments are outside it for the same
// reason: a process-local map cannot [dedup across instances].

The Cloudflare host is a multi-instance deployment: every request can land in a different isolate, and there is no shared process. So refreshInFlight deduplicates nothing there. Two concurrent resolves of the same connection each redeem the same refresh token, and against a provider that rotates on every exchange (Cloudflare Access does this, and so do several DCR servers) the loser's token is already dead. That produces exactly the observed invalid_grant / invalid_client, marks the connection degraded, and then "heals" as soon as one refresh succeeds alone — which is what makes the dashboard flap.

The comment names the constraint honestly; the Cloudflare host then ships into precisely the deployment shape it excludes.

Suggested fix

The host already runs Durable Objects (McpSessionDO, McpExecutionOwnerDirectoryDO). A refresh lease in a DO keyed by owner:integration:connection would give real cross-isolate single-flight. A database-backed CAS lease on the connection row (claim, refresh, release) would work on any host and needs no new binding.

A second, smaller thing in the same area

oauth_scope is only updated when the token response echoes scope:

if (token.scope !== undefined) set.oauth_scope = token.scope;

When a provider never echoes scope (plenty do not), the column keeps the scopes requested at connect time, and the refresh then re-requests that set. For a provider that granted a subset, every refresh fails — which is what railway_mcp's invalid_scope: refresh token missing requested scope looks like. The comment at the refresh site says the intent is to send the granted set, so this is a gap between intent and what is actually stored, not a disagreement about the rule.


3. describe.tool drops tool annotations, so the model cannot see readOnlyHint

The MCP plugin deliberately persists upstream annotations (packages/plugins/mcp/src/sdk/plugin.ts):

//  real MCP tool name, the upstream annotations, and the tool's `_meta` map
//  into the persisted annotations so they survive to invokeTool with no

and core stores them on the tool row (annotations: tool.annotations ?? null). But DescribedTool never returns them (packages/core/execution/src/tool-invoker.ts):

const described: DescribedTool = {
  path,
  name: schema.name ?? path,
  description: schema.description,
  inputTypeScript: schema.inputTypeScript,
  outputTypeScript: ...,
  typeScriptDefinitions: ...,
};

I verified this against two different MCP integrations: describe.tool returns the same six keys for both and no annotations field, even though both servers advertise readOnlyHint and title and executor stored them.

The consequence is that code written inside execute cannot tell a read from a destructive write except by reading the prose description. Executor itself uses destructiveHint for its approval copy, so the data is right there. Adding a compact annotations (or just readOnlyHint / destructiveHint / title) to DescribedTool, and optionally to tools.search results, would close it.


Two latent variants of defect 1, worth the same treatment

Both are "a partial answer becomes the authoritative catalog", and neither sets incomplete:

  1. listAllTools page cap. MAX_LIST_TOOLS_PAGES = 100 exits the loop by falling out of the for, and the accumulated partial list is returned as a complete manifest. A server with a cycling cursor silently truncates the catalog instead of reporting a problem.
  2. All-or-nothing entry decode. extractManifestFromListToolsResult decodes the merged list with Schema.Array(ListedTool) and Option.getOrElse(() => []), so one malformed entry (for instance an annotations object that does not match McpToolAnnotations) drops every tool on the server, and the result still reports success. On an explicit refresh that replaces a working catalog with zero tools. The adjacent comment shows the all-or-nothing hazard was already understood for _meta; the same reasoning applies to the whole entry.

Environment

  • executor apps/host-cloudflare, deployed at tag v1.6.8, Workers + D1 + R2 + two Durable Objects, Cloudflare Access in front.
  • 18 integrations, 17 connections, 34 tool policies.
  • Source references above are from main at f1d95f2 (2026-09-11); the v1.6.8 code paths match for defects 1 and 3.
  • The affected integration is a first-party MCP server of ours (16 tools, one tools/list page, 84 KB, no nextCursor — confirmed against the server directly), so the partial catalog cannot have come from the server's own response.

Happy to open PRs for any of these, starting with 1 and 3, which are both small and self-contained.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions