Defer TTL-expired catalog refresh off the read path

.changeset/tool-sync-defer.md

@@ -0,0 +1,16 @@
+---
+"@executor-js/sdk": patch
+"@executor-js/api": patch
+"@executor-js/local": patch
+"@executor-js/host-selfhost": patch
+"@executor-js/host-cloudflare": patch
+"@executor-js/cloud": patch
+---
+
+**Fix: an old-but-working tool catalog no longer makes a tools read wait for an upstream handshake**
+
+A tools read re-lists any connection whose catalog is due, and until now it waited for every one of them before it answered. Most of that waiting was speculative. A catalog goes due for four reasons, and only three of them are somebody telling us it changed: the connection has never synced, a drift signal arrived mid-invocation, or the integration's configuration was revised. The fourth is only the clock — the freshness window elapsed on a connection whose catalog works and which nothing has reported as wrong. Re-verifying it is worth doing soon; it was never worth making a caller wait for, and on a workspace with several MCP servers it was the bulk of what a read paid for.
+
+Reads now split the two. The three invalidation triggers still re-list inline, because the answer the read is about to give is wrong until they run. An expired catalog is served as it stands and its listing is handed to the host to run once the read has answered, through a new optional `ExecutorConfig.deferToolSync`. A connection that both drifted and expired is an invalidation and stays inline; that falls out of the existing classification rather than needing a rule. The refresh claim is taken inside the deferred listing rather than when it is queued, so a batch that is enqueued and then never runs — an evicted isolate, a dropped background task — leaves no lease behind and the connection is simply offered again on the next read. Each read defers at most sixteen connections, because the background budget on the tightest host is a Cloudflare `waitUntil`; the rest stay due and are picked up by the following read, and both counts are reported on the read's span alongside the existing sync counters.
+
+A host with no way to outlive its own response leaves `deferToolSync` unset and the batch runs inline, which is the same work in the only order that host can do it in. The local daemon, self-host and the Cloudflare host run it detached; the cloud MCP session, whose database handle lives as long as the session, runs it under the session's `waitUntil`. Cloud's HTTP API plane deliberately does not: its postgres socket is released while the response is still a value, so there is no moment on that plane that is both after the response and before the connection closes.

.claude/skills/prod-telemetry/SKILL.md

@@ -42,9 +42,9 @@ join the same traces via traceparent).
   `executor.tools.result_count`, and the catalog-refresh counters
   `executor.tools.sync.candidates` (rows the stale scan returned) /
   `executor.tools.sync.synced` / `.incomplete` / `.failed` / `.lost_claim` /
-  `.skipped_claimed` / `.skipped_backoff` / `.skipped_parked`. A slow tools
-  read is almost always `candidates` > 0: subtract the child span durations to
-  confirm.
+  `.skipped_claimed` / `.skipped_backoff` / `.skipped_parked` / `.deferred` /
+  `.deferred_overflow`. A slow tools read is almost always `candidates` > 0:
+  subtract the child span durations to confirm.
 
   `candidates` is the one to alert on for scan cost: in steady state it is
   ZERO. Every trigger is cleared by the listing that answers it, drift marks
@@ -78,9 +78,44 @@ join the same traces via traceparent).
   request concurrency for one integration is cross-isolate deduplication
   working.
 
-- `executor.tools.sync` (child of the above, one per refreshed connection;
-  older spans carry the previous name `executor.tools.sync_stale` and no
-  trigger/outcome attrs) — `executor.integration`,
+  `deferred` counts candidates handed to the host to list AFTER the read
+  answered — `expired` ones only, never an invalidation. It is deliberately not
+  part of the terminal partition: those listings have no outcome yet at the time
+  this span closes, and each reports its own on its own
+  `executor.tools.sync` span later. So on a host that defers, a read with
+  `deferred` > 0 and `synced` == 0 is the healthy shape, and `candidates` will
+  still be > 0 on the next read until the background batch lands. On a host that
+  does NOT defer (cloud's `/api/*` plane, which cannot — its postgres socket
+  closes before the response is written) the same work runs inline and reports
+  as `synced`, with `deferred` flat at 0, so comparing the two planes' `synced`
+  rates directly is a mistake.
+
+  `deferred_overflow` is what did not fit in the per-read batch cap (16). A
+  steady non-zero value means a scope is shedding refresh work every read and
+  converging slowly — it is never dropped, but the freshness window it is
+  effectively running at is `ttl x ceil(due / 16)`. There is no log line for
+  this; the counter is the only signal.
+
+- `executor.tools.sync.deferred` — one per read that deferred anything, wrapping
+  the whole background batch, with `executor.tools.sync.deferred` (the batch
+  size) on it. Its parentage is the HOST's choice, so do not join on it. In
+  cloud (the MCP session DO) it is a ROOT span in its own trace: the DO
+  re-provides the tracer so the batch is not hung under a `executor.tools.list`
+  span that closed before it, which would re-inflate the very read latency the
+  deferral removes. On a long-lived host that forks a fiber instead, it stays a
+  child of that list span and simply outlives it.
+
+  When triaging "the catalog is stale and nothing is re-listing", the absence of
+  this span on a host that reports `deferred` > 0 is the signal — the host
+  dropped the batch (an evicted isolate, a `waitUntil` that never ran). That
+  costs one freshness window and nothing else; it is not data loss. Search by
+  `executor.integration` on the child `executor.tools.sync` spans rather than by
+  trace, since cloud's batch has no trace in common with the read.
+
+- `executor.tools.sync` (child of `executor.tools.list` for an inline refresh
+  and of `executor.tools.sync.deferred` for a background one, one per refreshed
+  connection; older spans carry the previous name `executor.tools.sync_stale`
+  and no trigger/outcome attrs) — `executor.integration`,
   `executor.connection`, `executor.tools.sync.trigger`
   (`cold`/`stale_marked`/`config_revised`/`expired`),
   `executor.tools.sync.claimed` (bool — false means another reader owned the

apps/cloud/src/api.request-scope.node.test.ts

@@ -154,6 +154,60 @@ describe("HttpRouter.toWebHandler request scoping", () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// WHEN the per-request resource dies, relative to the response bytes.
+//
+// `requestScopedMiddleware` wraps the ROUTE HANDLER in `Effect.scoped`, and the
+// response is written later still, by `HttpEffect.toHandled`'s callback. So the
+// postgres socket is already closed by the time the client has anything. That
+// is why cloud's HTTP plane leaves `ExecutorConfig.deferToolSync` unset (see
+// `engine/execution-stack.ts`): there is no window on this plane that is both
+// after the response and before the socket closes, so `waitUntil` would only
+// ever hand a dead pool to the deferred batch.
+//
+// Pinned rather than assumed. If the request lifecycle is ever restructured so
+// the scope outlives the written response, this test fails and the deferral
+// becomes available — which is a thing to be told, not to rediscover.
+// ---------------------------------------------------------------------------
+
+describe("per-request resource lifetime vs the written response", () => {
+  it("releases the request-scoped resource before the response resolves", async () => {
+    const order: string[] = [];
+    const Tracked = Layer.effectDiscard(
+      Effect.acquireRelease(
+        Effect.sync(() => {
+          order.push("acquire");
+        }),
+        () =>
+          Effect.sync(() => {
+            order.push("release");
+          }),
+      ),
+    );
+    const Route = HttpRouter.add(
+      "GET",
+      "/",
+      Effect.sync(() => {
+        order.push("handler");
+        return HttpServerResponse.jsonUnsafe({ ok: true });
+      }),
+    );
+    const handler = HttpRouter.toWebHandler(
+      Route.pipe(
+        Layer.provide(requestScopedMiddleware(Tracked).layer),
+        Layer.provideMerge(HttpServer.layerServices),
+      ),
+      { disableLogger: true },
+    ).handler;
+
+    const response = await handler(new Request("http://test.local/"));
+    order.push("response");
+
+    expect(response.status).toBe(200);
+    expect(order).toEqual(["acquire", "handler", "release", "response"]);
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Regression test against the prod handler factory. If anyone reverts
 // `makeApiLive` back to wiring `RequestScopedServicesLive` via

apps/cloud/src/engine/execution-stack-metered.ts

@@ -27,7 +27,7 @@ import {
 
 import { AutumnService } from "../extensions/billing/service";
 import type { DbService } from "../db/db";
-import { CloudExecutionSeamsLayer } from "../engine/execution-stack";
+import { CloudExecutionSeamsLayer, makeCloudExecutionSeamsLayer } from "../engine/execution-stack";
 import { makeExecutionLimitGate } from "./execution-gate";
 import { makeCloudExecutionRateLimiter } from "./execution-rate-limit";
 import { withExecutionUsageTracking } from "./execution-usage";
@@ -75,3 +75,17 @@ export const CloudMeteredExecutionStackLayer: Layer.Layer<
   never,
   AutumnService | DbService
 > = Layer.merge(CloudExecutionSeamsLayer, CloudMeteringEngineDecorator);
+
+/**
+ * The same stack over a caller-supplied `HostConfig`. The MCP session DO builds
+ * one so it can defer tool-catalog refreshes to its own `ctx.waitUntil` — its
+ * database handle outlives any single request, where the HTTP plane's does not.
+ * Everything else is identical, decorator included.
+ */
+export const makeCloudMeteredExecutionStackLayer = (
+  hostConfig: Layer.Layer<HostConfig>,
+): Layer.Layer<
+  DbProvider | PluginsProvider | HostConfig | CodeExecutorProvider | EngineDecorator,
+  never,
+  AutumnService | DbService
+> => Layer.merge(makeCloudExecutionSeamsLayer(hostConfig), CloudMeteringEngineDecorator);

apps/cloud/src/engine/execution-stack.ts

@@ -41,6 +41,7 @@ import {
   HostConfig,
   PluginsProvider,
   collectTables,
+  type HostConfigShape,
 } from "@executor-js/api/server";
 import { makeDynamicWorkerExecutor } from "@executor-js/runtime-dynamic-worker";
 import type { AnyPlugin } from "@executor-js/sdk";
@@ -88,18 +89,69 @@ export const CloudPluginsProvider: Layer.Layer<PluginsProvider> = Layer.succeed(
  */
 export const CLOUD_MOUNT_PREFIX = "/api" as const;
 
-export const CloudHostConfig: Layer.Layer<HostConfig> = Layer.sync(HostConfig, () => ({
-  // SSRF / private-network egress guard. Config-driven, NOT a test flag:
-  // production leaves `ALLOW_LOCAL_NETWORK` unset so the guard stays ON (`false`);
-  // the e2e dev-server env opts in with `"true"` so in-scenario fixture
-  // servers on localhost are reachable. See `hosted-http-client.ts`.
-  allowLocalNetwork: env.ALLOW_LOCAL_NETWORK === "true",
-  webBaseUrl: env.VITE_PUBLIC_SITE_URL ?? "https://executor.sh",
-  oauthCallbackPath: `${CLOUD_MOUNT_PREFIX}/oauth/callback`,
-  // WorkOS Vault is cloud's credential storage implementation detail, not a
-  // user-selectable provider surface.
-  exposeCredentialProviders: false,
-}));
+/**
+ * Cloud's host config, parameterized by the ONE seam its two execution planes
+ * genuinely disagree about: where a deferred tool-catalog refresh runs.
+ *
+ * The MCP session Durable Object holds its database handle for the life of the
+ * session, so it can hand the batch straight to `ctx.waitUntil` and forget it.
+ * The HTTP plane's handle dies with its request and gets no seam at all (see
+ * {@link CloudHostConfig}). Nothing else about the config differs, which is why
+ * this is a parameter rather than a second config module.
+ */
+export const makeCloudHostConfig = (
+  deferToolSync: HostConfigShape["deferToolSync"],
+): Layer.Layer<HostConfig> =>
+  Layer.sync(HostConfig, () => ({
+    // SSRF / private-network egress guard. Config-driven, NOT a test flag:
+    // production leaves `ALLOW_LOCAL_NETWORK` unset so the guard stays ON (`false`);
+    // the e2e dev-server env opts in with `"true"` so in-scenario fixture
+    // servers on localhost are reachable. See `hosted-http-client.ts`.
+    allowLocalNetwork: env.ALLOW_LOCAL_NETWORK === "true",
+    webBaseUrl: env.VITE_PUBLIC_SITE_URL ?? "https://executor.sh",
+    oauthCallbackPath: `${CLOUD_MOUNT_PREFIX}/oauth/callback`,
+    // WorkOS Vault is cloud's credential storage implementation detail, not a
+    // user-selectable provider surface.
+    exposeCredentialProviders: false,
+    deferToolSync,
+  }));
+
+/**
+ * The HTTP plane's config, and the one seam it CANNOT fill: `deferToolSync` is
+ * absent, so a tools read over `/api/*` still refreshes an expired catalog
+ * inline.
+ *
+ * Not an omission. The postgres pool a request's executor closes over is
+ * acquired and released inside `requestScopedMiddleware`'s `Effect.scoped`,
+ * which wraps the ROUTE HANDLER — so the pool is torn down while the handler's
+ * `HttpServerResponse` is still a value, strictly before `HttpEffect.toHandled`
+ * writes it. There is therefore no window on this plane that is both after the
+ * response and before the socket closes: work scheduled with `waitUntil` finds
+ * a dead pool, and work drained inside the scope is simply inline work with
+ * extra steps. `api.request-scope.node.test.ts` pins that ordering, so a future
+ * change to the request lifecycle will say so rather than making this comment
+ * quietly wrong.
+ *
+ * Holding the pool open past the response instead — deferring the scope close
+ * into `waitUntil` — is the one thing that would make the window exist, and it
+ * is the thing `closePostgres` was written to prevent. That finalizer awaits
+ * the teardown precisely so live-plus-closing sockets stay bounded by what is
+ * in flight; unbounded, the backlog is the sustained-load cascade recorded in
+ * `db/db.ts`. Trading a socket leak for a background refresh is not a trade
+ * this plane should make, and it would make it on EVERY request, not just the
+ * ones that deferred.
+ *
+ * The alternative — giving the batch a short-lived pool of its own via
+ * `makeDbLayer()` — does not fit either: the task the sdk hands over is already
+ * bound to the executor that built it, so re-targeting it means rebuilding a
+ * whole second executor in the background. That is a background scheduler, a
+ * different feature with a different failure surface, not a wiring detail.
+ *
+ * Cloud's high-volume tools read is `tools/list` over MCP, and that plane DOES
+ * defer: the session Durable Object holds its database handle for the life of
+ * the session (see `mcp/session-durable-object.ts`).
+ */
+export const CloudHostConfig: Layer.Layer<HostConfig> = makeCloudHostConfig(undefined);
 
 export const CloudCodeExecutorProvider: Layer.Layer<CodeExecutorProvider> = Layer.sync(
   CodeExecutorProvider,
@@ -114,13 +166,16 @@ export const CloudCodeExecutorProvider: Layer.Layer<CodeExecutorProvider> = Laye
  * exported so that overlay builds over the SAME four seams. There is no neutral
  * no-op-decorator variant anymore: every cloud execution meters.
  */
+export const makeCloudExecutionSeamsLayer = (
+  hostConfig: Layer.Layer<HostConfig>,
+): Layer.Layer<
+  DbProvider | PluginsProvider | HostConfig | CodeExecutorProvider,
+  never,
+  DbService
+> => Layer.mergeAll(CloudDbProvider, CloudPluginsProvider, hostConfig, CloudCodeExecutorProvider);
+
 export const CloudExecutionSeamsLayer: Layer.Layer<
   DbProvider | PluginsProvider | HostConfig | CodeExecutorProvider,
   never,
   DbService
-> = Layer.mergeAll(
-  CloudDbProvider,
-  CloudPluginsProvider,
-  CloudHostConfig,
-  CloudCodeExecutorProvider,
-);
+> = makeCloudExecutionSeamsLayer(CloudHostConfig);