diff --git a/.changeset/clear-app-executor-timers.md b/.changeset/clear-app-executor-timers.md new file mode 100644 index 000000000..5353a2c55 --- /dev/null +++ b/.changeset/clear-app-executor-timers.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Release completed app tool invocation timeout timers instead of retaining them until their deadlines. diff --git a/packages/plugins/apps/src/executor/app-tool-executor-process.fixture.ts b/packages/plugins/apps/src/executor/app-tool-executor-process.fixture.ts new file mode 100644 index 000000000..597a2f5da --- /dev/null +++ b/packages/plugins/apps/src/executor/app-tool-executor-process.fixture.ts @@ -0,0 +1,24 @@ +import { Effect } from "effect"; + +import { makeInProcessAppToolExecutor } from "./app-tool-executor"; + +const bundle = ` + export default { + "~executorAppTool": true, + description: "Fast", + input: undefined, + handler() { return { ok: true }; }, + }; +`; + +await Effect.runPromise( + makeInProcessAppToolExecutor().invoke( + bundle, + { toolName: "fast" }, + {}, + { call: async () => null }, + { timeoutMs: 30_000 }, + ), +); + +process.stdout.write("invocation complete\n"); diff --git a/packages/plugins/apps/src/executor/app-tool-executor.test.ts b/packages/plugins/apps/src/executor/app-tool-executor.test.ts index 4ebe10f1e..13ac6d505 100644 --- a/packages/plugins/apps/src/executor/app-tool-executor.test.ts +++ b/packages/plugins/apps/src/executor/app-tool-executor.test.ts @@ -1,4 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; +import { spawnSync } from "node:child_process"; +import { join } from "node:path"; +import { buildSync } from "esbuild"; import { Effect } from "effect"; import { bundleEntry } from "../pipeline/bundle"; @@ -13,6 +16,28 @@ const bundle = (source: string) => }); describe("in-process app tool executor", () => { + it("releases the invocation timeout after a successful call", () => { + const fixture = join(import.meta.dirname, "app-tool-executor-process.fixture.ts"); + const script = buildSync({ + entryPoints: [fixture], + bundle: true, + platform: "node", + format: "esm", + target: "node24", + write: false, + }) + .outputFiles.map((output) => output.text) + .join("\n"); + const result = spawnSync(process.execPath, ["--input-type=module"], { + encoding: "utf8", + input: script, + timeout: 5_000, + }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain("invocation complete"); + }); + it.effect("detects integration declarations and merges projected input fields", () => Effect.gen(function* () { const bundled = yield* bundle(` diff --git a/packages/plugins/apps/src/executor/app-tool-executor.ts b/packages/plugins/apps/src/executor/app-tool-executor.ts index 90be90bf4..8b6310e84 100644 --- a/packages/plugins/apps/src/executor/app-tool-executor.ts +++ b/packages/plugins/apps/src/executor/app-tool-executor.ts @@ -363,22 +363,30 @@ const selectTool = (exported: unknown, entry: string): unknown => { return exported[entry.slice(index + marker.length)]; }; -const timeout = (promise: Promise, timeoutMs: number): Promise => - Promise.race([ - promise, - new Promise((_, reject) => { - setTimeout( - () => - reject( - new AppExecutorError({ - kind: "timeout", - message: `app tool timed out after ${timeoutMs}ms`, - }), - ), - timeoutMs, - ); - }), - ]); +const timeout = async (promise: Promise, timeoutMs: number): Promise => { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout( + () => + reject( + new AppExecutorError({ + kind: "timeout", + message: `app tool timed out after ${timeoutMs}ms`, + }), + ), + timeoutMs, + ); + }), + ]); + } finally { + if (timer !== undefined) { + clearTimeout(timer); + } + } +}; const makeClient = (root: string, prefix: readonly string[], bridge: AppToolBridge): unknown => new Proxy(() => undefined, { diff --git a/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.ts b/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.ts index 50d377266..28d859e35 100644 --- a/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.ts +++ b/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.ts @@ -79,7 +79,7 @@ const mapCause = (cause: unknown, kind: AppExecutorError["kind"], message: strin // Bump this when appWorkerModule() changes so stable Worker Loader IDs do not // reuse an isolate running an older driver around byte-identical app bundles. -export const DRIVER_VERSION = "1"; +export const DRIVER_VERSION = "2"; const appWorkerModule = (): string => ` import { WorkerEntrypoint } from "cloudflare:workers"; @@ -247,11 +247,18 @@ export default class AppExecutor extends WorkerEntrypoint { const output = await tool.handler(decoded, split.integrations); return { output: await validateStandard(tool.output, output, "output") }; }; - const value = await Promise.race([ - invoke(), - new Promise((_, reject) => setTimeout(() => reject(fail("timeout", "app tool timed out after " + input.timeoutMs + "ms")), input.timeoutMs)), - ]); - return { ok: true, value }; + let timer; + try { + const value = await Promise.race([ + invoke(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(fail("timeout", "app tool timed out after " + input.timeoutMs + "ms")), input.timeoutMs); + }), + ]); + return { ok: true, value }; + } finally { + if (timer !== undefined) clearTimeout(timer); + } } catch (error) { return serializeFailure(error, "invoke"); } diff --git a/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.worker.test.ts b/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.worker.test.ts new file mode 100644 index 000000000..c52a99a25 --- /dev/null +++ b/packages/plugins/apps/src/executor/dynamic-worker-app-tool-executor.worker.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "@effect/vitest"; +import { env } from "cloudflare:workers"; +import { Effect } from "effect"; + +import { makeDynamicWorkerAppToolExecutor } from "./dynamic-worker-app-tool-executor"; + +type WorkerLoader = Parameters[0]["loader"]; + +describe("dynamic Worker app tool executor", () => { + it.effect("invokes a completed tool through the generated driver", () => + Effect.gen(function* () { + const loader = (env as { readonly LOADER: WorkerLoader }).LOADER; + const executor = makeDynamicWorkerAppToolExecutor({ loader }); + const bundle = ` + export default { + "~executorAppTool": true, + description: "Fast", + input: undefined, + handler() { return { ok: true }; }, + }; + `; + + const result = yield* executor.invoke( + bundle, + { toolName: "fast" }, + {}, + { call: async () => null }, + { timeoutMs: 30_000, isolateKey: "timer-cleanup" }, + ); + + expect(result.output).toEqual({ ok: true }); + }), + ); +}); diff --git a/packages/plugins/apps/src/executor/workerd-app-tool-executor.ts b/packages/plugins/apps/src/executor/workerd-app-tool-executor.ts index 6189aaa2e..15e3e3e4a 100644 --- a/packages/plugins/apps/src/executor/workerd-app-tool-executor.ts +++ b/packages/plugins/apps/src/executor/workerd-app-tool-executor.ts @@ -230,11 +230,18 @@ export default { const output = await tool.handler(decoded, split.integrations); return { output: await validateStandard(tool.output, output, "output") }; }; - const value = await Promise.race([ - invoke(), - new Promise((_, reject) => setTimeout(() => reject(fail("timeout", "app tool timed out after " + input.timeoutMs + "ms")), input.timeoutMs)), - ]); - return Response.json({ ok: true, value }); + let timer; + try { + const value = await Promise.race([ + invoke(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(fail("timeout", "app tool timed out after " + input.timeoutMs + "ms")), input.timeoutMs); + }), + ]); + return Response.json({ ok: true, value }); + } finally { + if (timer !== undefined) clearTimeout(timer); + } } return Response.json({ ok: false, kind: "invoke", message: "unknown operation" }); } catch (error) {