History

run independent tool-call reads concurrently

Rhys Sullivan committed 2 months ago

Commit 97bb18c1 parent 4d4ad7c3 files · +291 −29Browse files

Unified · Split

3 files
  1. .changeset/toolcall-parallel-reads.md +7 −0
  2. packages/core/sdk/src/executor.test.ts +231 −1
  3. packages/core/sdk/src/executor.ts +53 −28
.changeset/toolcall-parallel-reads.md+7−0
@@ -0,0 +1,7 @@
1+---
2+"@executor-js/sdk": patch
3+---
4+
5+**Faster dynamic tool calls: independent storage reads run concurrently**
6+
7+Every dynamic tool call paid for its bookkeeping reads one at a time: first the tool row, then the active policy rules, then the connection row, and later the credential resolution followed by the integration row. Each read is a separate storage round-trip, so the serial chain added tens of milliseconds per call locally and more against a remote database. The reads are mutually independent, so they now run concurrently: the tool, policy, and connection reads overlap before approval, and credential resolution overlaps the integration read after approval. Approval enforcement still completes before credential resolution starts — a declined call never triggers a token refresh — and each read's failure still surfaces at the same point with the same error as before.
packages/core/sdk/src/executor.test.ts+231−1
@@ -1,7 +1,10 @@
11 import { describe, expect, it } from "@effect/vitest";
22 import { Data, Effect, Predicate, Result } from "effect";
33
4+import { ElicitationResponse, type ElicitationHandler } from "./elicitation";
45 import { ToolNotFoundError } from "./errors";
6+import { createExecutor } from "./executor";
7+import type { FumaDb } from "./fuma-runtime";
58 import {
69 AuthTemplateSlug,
710 ConnectionName,
@@ -15,7 +18,7 @@ import { · 7 unmodified lines
1518 import { definePlugin } from "./plugin";
1619 import type { CredentialProvider } from "./provider";
1720 import { IntegrationDetectionResult } from "./types";
18−import { makeTestExecutor, memoryCredentialsPlugin } from "./testing";
21+import { makeTestConfig, makeTestExecutor, memoryCredentialsPlugin } from "./testing";
1922 import { serveOAuthTestServer } from "./testing/oauth-test-server";
2023
2124 // removed: v1 secret browser-handoff, source.configure, case-insensitive tool-id
@@ -783,3 +786,230 @@ describe("muscle memory (observed output shapes)", () => { · 761 unmodified lines
783786 }),
784787 );
785788 });
789+
790+// ---------------------------------------------------------------------------
791+// Dynamic tool-call read concurrency. The invoke path runs its independent
792+// storage reads concurrently: tool row + policy rules + connection row before
793+// approval, then credential resolution + integration row after approval.
794+// These tests pin the overlap itself AND the invariants the overlap must not
795+// disturb — most importantly that a declined approval never starts credential
796+// resolution (which can trigger an upstream token refresh).
797+// ---------------------------------------------------------------------------
798+
799+/** Records which armed reads are simultaneously in flight. Each armed read
800+ * registers on entry, then HOLDS its result until every other armed read has
801+ * also started — so genuinely concurrent reads all overlap and the probe
802+ * observes it. A sequential ordering would deadlock on that barrier, so a
803+ * fallback timer releases each held read; the run then completes with
804+ * `sawAllInFlight() === false` and the assertion (not a hang) reports the
805+ * regression. */
806+const makeReadOverlapProbe = () => {
807+ let targets: ReadonlySet<string> = new Set();
808+ let sawAll = false;
809+ const started = new Set<string>();
810+ const inFlight = new Set<string>();
811+ let release: (() => void) | undefined;
812+ let barrier: Promise<void> | undefined;
813+ const observe = async (key: string): Promise<void> => {
814+ if (barrier === undefined || !targets.has(key) || started.has(key)) return;
815+ started.add(key);
816+ inFlight.add(key);
817+ if (started.size === targets.size) {
818+ if (inFlight.size === targets.size) sawAll = true;
819+ release?.();
820+ }
821+ await Promise.race([
822+ barrier,
823+ new Promise<void>((resolve) => {
824+ setTimeout(resolve, 250);
825+ }),
826+ ]);
827+ inFlight.delete(key);
828+ };
829+ return {
830+ arm: (keys: readonly string[]) => {
831+ targets = new Set(keys);
832+ barrier = new Promise<void>((resolve) => {
833+ release = resolve;
834+ });
835+ },
836+ observe,
837+ sawAllInFlight: () => sawAll,
838+ };
839+};
840+
841+/** Wrap a test `FumaDb` so every read reports to the probe as
842+ * `<table>.<method>` before it executes. `withContext` re-wraps so the
843+ * executor's context-bound handles stay observed. */
844+const withObservedReads = (db: FumaDb, observe: (key: string) => Promise<void>): FumaDb => {
845+ const wrap = (inner: FumaDb): FumaDb =>
846+ new Proxy(inner, {
847+ get(target, prop) {
848+ if (prop === "withContext") {
849+ return (context: unknown) =>
850+ wrap((target.withContext as (c: unknown) => FumaDb)(context));
851+ }
852+ if (prop === "findFirst" || prop === "findMany") {
853+ return async (table: unknown, query: unknown) => {
854+ await observe(`${String(table)}.${prop}`);
855+ return (Reflect.get(target, prop) as (t: unknown, q: unknown) => Promise<unknown>).call(
856+ target,
857+ table,
858+ query,
859+ );
860+ };
861+ }
862+ return Reflect.get(target, prop);
863+ },
864+ });
865+ return wrap(db);
866+};
867+
868+const countingProvider = (calls: { count: number }, onGet?: () => Promise<void>) => {
869+ const store = new Map<string, string>();
870+ const provider: CredentialProvider = {
871+ key: ProviderKey.make("memory"),
872+ writable: true,
873+ get: (id) =>
874+ Effect.promise(async () => {
875+ calls.count++;
876+ await onGet?.();
877+ return store.get(String(id)) ?? null;
878+ }),
879+ set: (id, value) => Effect.sync(() => void store.set(String(id), value)),
880+ };
881+ return provider;
882+};
883+
884+const invokeConcurrencyPlugin = (provider: CredentialProvider) =>
885+ definePlugin(() => ({
886+ id: "demo" as const,
887+ credentialProviders: [provider],
888+ storage: () => ({}),
889+ resolveTools: () =>
890+ Effect.succeed({ tools: [{ name: ToolName.make("run"), description: "run" }] }),
891+ invokeTool: ({ toolRow }) => Effect.succeed({ ran: toolRow.name }),
892+ extension: (ctx) => ({
893+ seed: () => ctx.core.integrations.register({ slug: INTEG, description: "Demo", config: {} }),
894+ }),
895+ }))();
896+
897+const seedRunConnection = <
898+ E extends {
899+ readonly demo: { readonly seed: () => Effect.Effect<unknown, unknown> };
900+ readonly connections: {
901+ readonly create: (input: {
902+ owner: "org";
903+ name: ConnectionName;
904+ integration: IntegrationSlug;
905+ template: AuthTemplateSlug;
906+ from: { provider: ProviderKey; id: ProviderItemId };
907+ }) => Effect.Effect<unknown, unknown>;
908+ };
909+ },
910+>(
911+ executor: E,
912+) =>
913+ Effect.gen(function* () {
914+ yield* executor.demo.seed();
915+ yield* executor.connections.create({
916+ owner: "org",
917+ name: CONN,
918+ integration: INTEG,
919+ template: TEMPLATE,
920+ from: {
921+ provider: ProviderKey.make("memory"),
922+ id: ProviderItemId.make("v"),
923+ },
924+ });
925+ });
926+
927+describe("execute read concurrency", () => {
928+ it.effect("overlaps the tool, policy, and connection reads on the invoke path", () =>
929+ Effect.gen(function* () {
930+ const probe = makeReadOverlapProbe();
931+ const config = makeTestConfig({ plugins: [demoPlugin] as const });
932+ const executor = yield* createExecutor({
933+ ...config,
934+ db: withObservedReads(config.db, probe.observe),
935+ });
936+ yield* seedRunConnection(executor);
937+
938+ probe.arm(["tool.findFirst", "tool_policy.findMany", "connection.findFirst"]);
939+ const out = yield* executor.execute(addr("run"), {});
940+ expect(out).toEqual({ ran: "run" });
941+ // All three pre-approval reads were in flight at the same instant.
942+ expect(probe.sawAllInFlight()).toBe(true);
943+ }),
944+ );
945+
946+ it.effect("overlaps credential resolution with the integration-row read after approval", () =>
947+ Effect.gen(function* () {
948+ const probe = makeReadOverlapProbe();
949+ const calls = { count: 0 };
950+ const provider = countingProvider(calls, () => probe.observe("credential.get"));
951+ const config = makeTestConfig({
952+ plugins: [invokeConcurrencyPlugin(provider)] as const,
953+ });
954+ const executor = yield* createExecutor({
955+ ...config,
956+ db: withObservedReads(config.db, probe.observe),
957+ });
958+ yield* seedRunConnection(executor);
959+
960+ probe.arm(["credential.get", "integration.findFirst"]);
961+ const out = yield* executor.execute(addr("run"), {});
962+ expect(out).toEqual({ ran: "run" });
963+ expect(probe.sawAllInFlight()).toBe(true);
964+ }),
965+ );
966+
967+ it.effect("a declined approval never resolves credentials", () =>
968+ Effect.gen(function* () {
969+ const calls = { count: 0 };
970+ const executor = yield* makeTestExecutor({
971+ plugins: [invokeConcurrencyPlugin(countingProvider(calls))] as const,
972+ });
973+ yield* seedRunConnection(executor);
974+ yield* executor.policies.create({
975+ owner: "org",
976+ pattern: "demo.*",
977+ action: "require_approval",
978+ });
979+
980+ // Setup (connection create / tool sync) may read the credential; only
981+ // reads issued by the declined call itself are the regression signal.
982+ calls.count = 0;
983+ const decliningHandler: ElicitationHandler = () =>
984+ Effect.succeed(ElicitationResponse.make({ action: "decline" }));
985+ const result = yield* Effect.result(
986+ executor.execute(addr("run"), {}, { onElicitation: decliningHandler }),
987+ );
988+ expect(Result.isFailure(result)).toBe(true);
989+ if (!Result.isFailure(result)) return;
990+ expect(Predicate.isTagged("ElicitationDeclinedError")(result.failure)).toBe(true);
991+ // The decline happened BEFORE credential resolution started: a token
992+ // refresh (a network side effect) must never fire for a declined call.
993+ expect(calls.count).toBe(0);
994+ }),
995+ );
996+
997+ it.effect("fails with ConnectionNotFoundError when the tool row outlives its connection", () =>
998+ Effect.gen(function* () {
999+ const config = makeTestConfig({ plugins: [demoPlugin] as const });
1000+ const executor = yield* createExecutor(config);
1001+ yield* seedRunConnection(executor);
1002+
1003+ // Remove ONLY the connection row, leaving the tool rows behind — the
1004+ // inconsistent state the ConnectionNotFoundError branch reports. The
1005+ // concurrent connection read must still surface this error, not a
1006+ // policy or tool-row failure.
1007+ yield* Effect.promise(() => config.db.deleteMany("connection", {}));
1008+
1009+ const result = yield* Effect.result(executor.execute(addr("run"), {}));
1010+ expect(Result.isFailure(result)).toBe(true);
1011+ if (!Result.isFailure(result)) return;
1012+ expect(Predicate.isTagged("ConnectionNotFoundError")(result.failure)).toBe(true);
1013+ }),
1014+ );
1015+});
packages/core/sdk/src/executor.ts+53−28
@@ -5794,19 +5794,40 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 5793 unmodified lines
57945794 return yield* new ToolNotFoundError({ address });
57955795 }
57965796
5797− // Find the tool row — projected: invoke needs routing/policy fields
5798− // only, never the multi-KB input/output schema JSON (`tools.schema`
5799− // is the schema-bearing surface).
5800− const row = yield* core.findFirst("tool", {
5801− where: (b: AnyCb) =>
5802− b.and(
5803− byOwner(parsed.owner)(b),
5804− b("integration", "=", String(parsed.integration)),
5805− b("connection", "=", String(parsed.connection)),
5806− b("name", "=", String(parsed.tool)),
5797+ // The three storage reads this call needs — the tool row (projected:
5798+ // invoke needs routing/policy fields only, never the multi-KB
5799+ // input/output schema JSON; `tools.schema` is the schema-bearing
5800+ // surface), the active policy rule set, and the connection row — are
5801+ // mutually independent, so they run concurrently instead of paying
5802+ // three serial round-trips. The policy and connection reads are
5803+ // captured as Exits and unwrapped at exactly the points the
5804+ // sequential code performed them, so the caller-visible error for a
5805+ // given input is unchanged: a tool-row read failure still dominates,
5806+ // and a policy or connection read failure still surfaces only where
5807+ // the old code would have executed that read.
5808+ const { row, policyRulesExit, connectionRowExit } = yield* Effect.all(
5809+ {
5810+ row: core.findFirst("tool", {
5811+ where: (b: AnyCb) =>
5812+ b.and(
5813+ byOwner(parsed.owner)(b),
5814+ b("integration", "=", String(parsed.integration)),
5815+ b("connection", "=", String(parsed.connection)),
5816+ b("name", "=", String(parsed.tool)),
5817+ ),
5818+ select: TOOL_INVOCATION_COLUMNS,
5819+ }),
5820+ policyRulesExit: Effect.exit(listActivePolicyRuleSet()),
5821+ connectionRowExit: Effect.exit(
5822+ findConnectionRow({
5823+ owner: parsed.owner,
5824+ integration: parsed.integration,
5825+ name: parsed.connection,
5826+ }),
58075827 ),
5808− select: TOOL_INVOCATION_COLUMNS,
5809− });
5828+ },
5829+ { concurrency: 3 },
5830+ );
58105831 if (!row) {
58115832 const searchMatches = yield* searchToolRowsForConnection(parsed);
58125833 const connectionTools =
@@ -5817,12 +5838,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 4 unmodified lines
58175838 // the reader after a tool that was never the problem, so name the
58185839 // connection and point at the surface that knows the cause.
58195840 const connectionExists =
5820− connectionTools.length === 0 &&
5821− (yield* findConnectionRow({
5822− owner: parsed.owner,
5823− integration: parsed.integration,
5824− name: parsed.connection,
5825− })) !== null;
5841+ connectionTools.length === 0 && (yield* connectionRowExit) !== null;
58265842 return yield* new ToolNotFoundError({
58275843 address,
58285844 suggestions: toolSuggestions(connectionTools),
@@ -5835,7 +5851,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 6 unmodified lines
58355851
58365852 // Resolve policy (owner-ranked).
58375853 const toolForPolicy = rowToTool(row);
5838− const policyRules = yield* listActivePolicyRuleSet();
5854+ const policyRules = yield* policyRulesExit;
58395855 const annotations = decodeJsonColumn(row.annotations) as ToolAnnotations | undefined;
58405856 const policy = yield* resolvePolicyFromRuleSet(
58415857 normalizedPolicyId(toolForPolicy),
@@ -5863,12 +5879,8 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 21 unmodified lines
58635879 });
58645880 }
58655881
5866− // Find the connection row.
5867− const connectionRow = yield* findConnectionRow({
5868− owner: parsed.owner,
5869− integration: parsed.integration,
5870− name: parsed.connection,
5871− });
5882+ // Unwrap the connection row (read concurrently above).
5883+ const connectionRow = yield* connectionRowExit;
58725884 if (!connectionRow) {
58735885 return yield* new ConnectionNotFoundError({
58745886 owner: parsed.owner,
@@ -5903,9 +5915,22 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 28 unmodified lines
59035915 yield* enforceApproval(resolvedAnnotations, address, args, policy, handler);
59045916
59055917 // Resolve every named credential input (`variable → value`); `value` is
5906− // the primary `token` for single-input + OAuth callers.
5907− const values = yield* resolveConnectionValues(connectionRow);
5908− const integrationRow = yield* findIntegrationRow(parsed.integration);
5918+ // the primary `token` for single-input + OAuth callers. The
5919+ // integration-row read is independent of credential resolution, so
5920+ // the two run concurrently. Both start only after `enforceApproval`
5921+ // above completes — a declined call must never trigger the token
5922+ // refresh credential resolution can perform. The integration read is
5923+ // captured as an Exit and unwrapped after `values`, so a credential
5924+ // resolution failure keeps dominating a storage failure exactly as
5925+ // it did when the reads were sequential.
5926+ const { values, integrationRowExit } = yield* Effect.all(
5927+ {
5928+ values: resolveConnectionValues(connectionRow),
5929+ integrationRowExit: Effect.exit(findIntegrationRow(parsed.integration)),
5930+ },
5931+ { concurrency: 2 },
5932+ );
5933+ const integrationRow = yield* integrationRowExit;
59095934 const grantedScopes = grantedScopesFromRow(connectionRow);
59105935 const invokeTool = runtime.plugin.invokeTool;
59115936 const invokeWith = (

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t