History

Merge pull request #322 from RhysSullivan/rs/perf-description-drop-tools-list

perf(execution): drop executor.tools.list from buildExecuteDescription in favor of sources list

Rhys Sullivan committed 6 months ago

Commit e382e752 parents bcebd01 0a22ad63 files · +122 −20Browse files

Unified · Split

3 files
  1. apps/cloud/src/mcp-session.ts +5 −5
  2. packages/core/execution/src/description.test.ts +109 −0
  3. packages/core/execution/src/description.ts +8 −15
apps/cloud/src/mcp-session.ts+5−5
@@ -227,11 +227,11 @@ export class McpSessionDO extends DurableObject { · 226 unmodified lines
227 sessionMeta.organizationId,227 sessionMeta.organizationId,
228 sessionMeta.organizationName,228 sessionMeta.organizationName,
229 );229 );
230 // Build the description here so the two postgres queries it runs230 // Build the description here so the postgres query it runs
231 // (`executor.sources.list` + `executor.tools.list`) land as231 // (`executor.sources.list`) lands as a child of
232 // children of `McpSessionDO.createRuntime`. host-mcp would232 // `McpSessionDO.createRuntime`. host-mcp would otherwise call
233 // otherwise call `Effect.runPromise(engine.getDescription)` at233 // `Effect.runPromise(engine.getDescription)` at its async
234 // its async MCP-SDK boundary and orphan those sub-spans.234 // MCP-SDK boundary and orphan the sub-span.
235 const description = yield* buildExecuteDescription(executor);235 const description = yield* buildExecuteDescription(executor);
236 const mcpServer = yield* Effect.promise(() =>236 const mcpServer = yield* Effect.promise(() =>
237 createExecutorMcpServer({ engine, description }),237 createExecutorMcpServer({ engine, description }),
packages/core/execution/src/description.test.ts+109−0
@@ -0,0 +1,109 @@
1import { describe, expect, it } from "@effect/vitest";
2import { Effect } from "effect";
3
4import { createExecutor, definePlugin, makeTestConfig } from "@executor/sdk";
5
6import { buildExecuteDescription } from "./description";
7
8const EmptyInputSchema = {
9 type: "object",
10 properties: {},
11 additionalProperties: false,
12} as const;
13
14// Two plugins registering static sources whose ids are distinct from their
15// pluginIds/names. If `buildExecuteDescription` ever renders the wrong field
16// (e.g. pluginId, an internal UUID, or the source name), these assertions
17// fail — which is the class of bug a hand-rolled fake `Executor` would miss.
18const githubPlugin = definePlugin(() => ({
19 id: "github-plugin" as const,
20 storage: () => ({}),
21 staticSources: () => [
22 {
23 id: "github",
24 kind: "in-memory",
25 name: "GitHub",
26 tools: [
27 {
28 name: "noop",
29 description: "noop",
30 inputSchema: EmptyInputSchema,
31 handler: () => Effect.succeed(null),
32 },
33 ],
34 },
35 ],
36}));
37
38const slackPlugin = definePlugin(() => ({
39 id: "slack-plugin" as const,
40 storage: () => ({}),
41 staticSources: () => [
42 {
43 id: "slack",
44 kind: "in-memory",
45 name: "Slack Workspace",
46 tools: [
47 {
48 name: "noop",
49 description: "noop",
50 inputSchema: EmptyInputSchema,
51 handler: () => Effect.succeed(null),
52 },
53 ],
54 },
55 ],
56}));
57
58describe("buildExecuteDescription", () => {
59 it.effect(
60 "renders real source ids as namespaces (sorted) through the real executor flow",
61 () =>
62 Effect.gen(function* () {
63 // Intentionally register in non-alphabetical order — the formatter
64 // is expected to sort by source id.
65 const executor = yield* createExecutor(
66 makeTestConfig({ plugins: [slackPlugin(), githubPlugin()] as const }),
67 );
68
69 const description = yield* buildExecuteDescription(executor);
70
71 // Stable anchor from the workflow preamble.
72 expect(description).toContain(
73 "Execute TypeScript in a sandboxed runtime",
74 );
75 // The namespaces section header.
76 expect(description).toContain("## Available namespaces");
77 // Each source renders with its ACTUAL id (not pluginId / name / UUID).
78 expect(description).toContain("`github` — GitHub");
79 expect(description).toContain("`slack` — Slack Workspace");
80 // And the plugin ids must NOT leak into the namespace list.
81 expect(description).not.toContain("`github-plugin`");
82 expect(description).not.toContain("`slack-plugin`");
83
84 // Sort order: `github` before `slack`.
85 const githubIdx = description.indexOf("`github`");
86 const slackIdx = description.indexOf("`slack`");
87 expect(githubIdx).toBeGreaterThan(-1);
88 expect(slackIdx).toBeGreaterThan(-1);
89 expect(githubIdx).toBeLessThan(slackIdx);
90 }),
91 );
92
93 it.effect(
94 "omits the Available namespaces section when no plugins register sources",
95 () =>
96 Effect.gen(function* () {
97 const executor = yield* createExecutor(
98 makeTestConfig({ plugins: [] as const }),
99 );
100
101 const description = yield* buildExecuteDescription(executor);
102
103 expect(description).toContain(
104 "Execute TypeScript in a sandboxed runtime",
105 );
106 expect(description).not.toContain("## Available namespaces");
107 }),
108 );
109});
packages/core/execution/src/description.ts+8−15
@@ -1,5 +1,5 @@
1import { Effect } from "effect";1import { Effect } from "effect";
2import type { Executor, Tool, Source } from "@executor/sdk";2import type { Executor, Source } from "@executor/sdk";
3 3
4/**4/**
5 * Builds a tool description dynamically.5 * Builds a tool description dynamically.
@@ -13,17 +13,11 @@ export const buildExecuteDescription = (executor: Executor): Effect.Effect<strin · 7 unmodified lines
13 const sources: readonly Source[] = yield* executor.sources13 const sources: readonly Source[] = yield* executor.sources
14 .list()14 .list()
15 .pipe(Effect.orDie, Effect.withSpan("executor.sources.list"));15 .pipe(Effect.orDie, Effect.withSpan("executor.sources.list"));
16 const tools: readonly Tool[] = yield* executor.tools
17 .list()
18 .pipe(Effect.orDie, Effect.withSpan("executor.tools.list"));
19
20 const namespaces = new Set<string>();
21 for (const tool of tools) namespaces.add(tool.sourceId);
22 16
23 return formatDescription([...namespaces], sources);17 return formatDescription(sources);
24 }).pipe(Effect.withSpan("buildExecuteDescription"));18 }).pipe(Effect.withSpan("buildExecuteDescription"));
25 19
26const formatDescription = (namespaces: readonly string[], sources: readonly Source[]): string => {20const formatDescription = (sources: readonly Source[]): string => {
27 const lines: string[] = [21 const lines: string[] = [
28 "Execute TypeScript in a sandboxed runtime with access to configured API tools.",22 "Execute TypeScript in a sandboxed runtime with access to configured API tools.",
29 "",23 "",
@@ -50,15 +44,14 @@ const formatDescription = (namespaces: readonly string[], sources: readonly Sour · 20 unmodified lines
50 "- If execution pauses for interaction, resume it with the returned `resumePayload`.",44 "- If execution pauses for interaction, resume it with the returned `resumePayload`.",
51 ];45 ];
52 46
53 if (namespaces.length > 0) {47 if (sources.length > 0) {
54 lines.push("");48 lines.push("");
55 lines.push("## Available namespaces");49 lines.push("## Available namespaces");
56 lines.push("");50 lines.push("");
57 const sorted = [...namespaces].sort();51 const sorted = [...sources].sort((a, b) => a.id.localeCompare(b.id));
58 for (const ns of sorted) {52 for (const source of sorted) {
59 const source = sources.find((s) => s.id === ns);53 const label = source.name;
60 const label = source?.name ?? ns;54 lines.push(`- \`${source.id}\`${label !== source.id ? ` — ${label}` : ""}`);
61 lines.push(`- \`${ns}\`${label !== ns ? ` — ${label}` : ""}`);
62 }55 }
63 }56 }
64 57