History

refactor(openapi,graphql): store invocation config on source, not per-tool (#222)

* refactor(openapi,graphql): store invocation config on source, not per-tool Per-tool binding rows were duplicating the full InvocationConfig (baseUrl/endpoint + headers) on every entry. A source with N operations stored N copies of the same config in KV, and every updateSource call read + rewrote every binding row. Move invocationConfig to the source row (one per namespace) alongside the user-facing SourceConfig. Per-tool rows now store only { namespace, binding }. The invoker resolves config via getSource on the hot path; updateSource is a single putSource (O(1) writes). * feat(openapi,graphql): self-heal legacy source rows on read Pre-refactor source rows don't carry invocationConfig. Mark the field decode-optional on StoredSourceSchema, and in the KV stores rehydrate missing invocationConfig on read: - GraphQL: synthesize from source.config.endpoint + headers (lossless, endpoint is always user-provided). - OpenAPI: scan for a legacy binding row whose inlined config carries the resolved baseUrl. Fall back to source.config.baseUrl. getSource writes the healed row back; listSources rehydrates in memory only to avoid N writes per list. All migration paths are tagged with TODO(migration) for a later cleanup pass.

Rhys Sullivan committed 7 months ago

Commit b28b4641 parent 3efffbb10 files · +146 −67Browse files

Unified · Split

10 files
  1. packages/plugins/graphql/src/sdk/invoke.ts +11 −1
  2. packages/plugins/graphql/src/sdk/kv-operation-store.ts +30 −7
  3. packages/plugins/graphql/src/sdk/operation-store.ts +3 −2
  4. packages/plugins/graphql/src/sdk/plugin.ts +7 −21
  5. packages/plugins/graphql/src/sdk/stored-source.ts +5 −1
  6. packages/plugins/openapi/src/sdk/invoke.ts +11 −1
  7. packages/plugins/openapi/src/sdk/kv-operation-store.ts +64 −7
  8. packages/plugins/openapi/src/sdk/operation-store.ts +3 −2
  9. packages/plugins/openapi/src/sdk/plugin.ts +7 −24
  10. packages/plugins/openapi/src/sdk/stored-source.ts +5 −1
packages/plugins/graphql/src/sdk/invoke.ts+11−1
@@ -170,7 +170,17 @@ export const makeGraphqlInvoker = (opts: { · 169 unmodified lines
170170 });
171171 }
172172
173− const { binding, config } = entry;
173+ const source = yield* opts.operationStore.getSource(entry.namespace);
174+ if (!source) {
175+ return yield* new ToolInvocationError({
176+ toolId,
177+ message: `No source found for namespace "${entry.namespace}"`,
178+ cause: undefined,
179+ });
180+ }
181+
182+ const { binding } = entry;
183+ const { invocationConfig: config } = source;
174184
175185 // Resolve secret-backed headers
176186 const resolvedHeaders = yield* resolveHeaders(config.headers, opts.secrets, opts.scopeId);
packages/plugins/graphql/src/sdk/kv-operation-store.ts+30−7
@@ -6,7 +6,7 @@ import { Effect, Schema } from "effect"; · 5 unmodified lines
66 import { scopeKv, makeInMemoryScopedKv, type Kv, type ToolId, type ScopedKv } from "@executor/sdk";
77
88 import type { GraphqlOperationStore, StoredSource } from "./operation-store";
9−import { OperationBinding, InvocationConfig } from "./types";
9+import { InvocationConfig, OperationBinding } from "./types";
1010 import { StoredSourceSchema } from "./stored-source";
1111
1212 // ---------------------------------------------------------------------------
@@ -16,7 +16,6 @@ import { StoredSourceSchema } from "./stored-source"; · 3 unmodified lines
1616 class StoredEntry extends Schema.Class<StoredEntry>("StoredEntry")({
1717 namespace: Schema.String,
1818 binding: OperationBinding,
19− config: InvocationConfig,
2019 }) {}
2120
2221 const encodeEntry = Schema.encodeSync(Schema.parseJson(StoredEntry));
@@ -29,18 +28,36 @@ const decodeSource = Schema.decodeUnknownSync(Schema.parseJson(StoredSourceSchem · 6 unmodified lines
2928 // Implementation
3029 // ---------------------------------------------------------------------------
3130
31+// TODO(migration): remove DecodedSource + rehydrate once all source rows
32+// have been migrated to carry invocationConfig. For GraphQL the endpoint
33+// is always user-provided in SourceConfig, so rehydration is lossless.
34+type DecodedSource = Omit<StoredSource, "invocationConfig"> & {
35+ invocationConfig?: InvocationConfig;
36+};
37+
38+const rehydrate = (source: DecodedSource): StoredSource =>
39+ source.invocationConfig
40+ ? (source as StoredSource)
41+ : {
42+ ...source,
43+ invocationConfig: new InvocationConfig({
44+ endpoint: source.config.endpoint,
45+ headers: source.config.headers ?? {},
46+ }),
47+ };
48+
3249 const makeStore = (bindings: ScopedKv, sources: ScopedKv): GraphqlOperationStore => ({
3350 get: (toolId) =>
3451 Effect.gen(function* () {
3552 const raw = yield* bindings.get(toolId);
3653 if (!raw) return null;
3754 const entry = decodeEntry(raw);
38− return { binding: entry.binding, config: entry.config };
55+ return { binding: entry.binding, namespace: entry.namespace };
3956 }),
4057
41− put: (toolId, namespace, binding, config) =>
58+ put: (toolId, namespace, binding) =>
4259 bindings.set([
43− { key: toolId, value: encodeEntry(new StoredEntry({ namespace, binding, config })) },
60+ { key: toolId, value: encodeEntry(new StoredEntry({ namespace, binding })) },
4461 ]),
4562
4663 remove: (toolId) => bindings.delete([toolId]).pipe(Effect.asVoid),
@@ -75,14 +92,20 @@ const makeStore = (bindings: ScopedKv, sources: ScopedKv): GraphqlOperationStore · 28 unmodified lines
7592 listSources: () =>
7693 Effect.gen(function* () {
7794 const entries = yield* sources.list();
78− return entries.map((e) => decodeSource(e.value) as StoredSource);
95+ // TODO(migration): rehydrate in memory only — avoid N writes per list.
96+ return entries.map((e) => rehydrate(decodeSource(e.value) as DecodedSource));
7997 }),
8098
8199 getSource: (namespace) =>
82100 Effect.gen(function* () {
83101 const raw = yield* sources.get(namespace);
84102 if (!raw) return null;
85− return decodeSource(raw) as StoredSource;
103+ const source = decodeSource(raw) as DecodedSource;
104+ if (source.invocationConfig) return source as StoredSource;
105+ // TODO(migration): self-heal — rehydrate and write back once.
106+ const healed = rehydrate(source);
107+ yield* sources.set([{ key: namespace, value: encodeSource(healed) }]);
108+ return healed;
86109 }),
87110
88111 getSourceConfig: (namespace) =>
packages/plugins/graphql/src/sdk/operation-store.ts+3−2
@@ -18,18 +18,19 @@ export interface StoredSource { · 17 unmodified lines
1818 readonly namespace: string;
1919 readonly name: string;
2020 readonly config: SourceConfig;
21+ /** Pre-resolved runtime invocation config (endpoint, headers). */
22+ readonly invocationConfig: InvocationConfig;
2123 }
2224
2325 export interface GraphqlOperationStore {
2426 readonly get: (
2527 toolId: ToolId,
26− ) => Effect.Effect<{ binding: OperationBinding; config: InvocationConfig } | null>;
28+ ) => Effect.Effect<{ binding: OperationBinding; namespace: string } | null>;
2729
2830 readonly put: (
2931 toolId: ToolId,
3032 namespace: string,
3133 binding: OperationBinding,
32− config: InvocationConfig,
3334 ) => Effect.Effect<void>;
3435
3536 readonly remove: (toolId: ToolId) => Effect.Effect<void>;
packages/plugins/graphql/src/sdk/plugin.ts+7−21
@@ -370,7 +370,7 @@ export const graphqlPlugin = (options?: { · 369 unmodified lines
370370 variableNames: extractedField.arguments.map((a) => a.name),
371371 });
372372
373− return operationStore.put(reg.id, namespace, binding, invocationConfig);
373+ return operationStore.put(reg.id, namespace, binding);
374374 },
375375 { discard: true },
376376 );
@@ -386,6 +386,7 @@ export const graphqlPlugin = (options?: { · 9 unmodified lines
386386 namespace: config.namespace,
387387 headers: config.headers,
388388 },
389+ invocationConfig,
389390 });
390391
391392 return { sourceId: namespace, toolCount: registrations.length };
@@ -438,11 +439,11 @@ export const graphqlPlugin = (options?: { · 46 unmodified lines
438439
439440 updateSource: (namespace: string, input: GraphqlUpdateSourceInput) =>
440441 Effect.gen(function* () {
441− const existingConfig = yield* operationStore.getSourceConfig(namespace);
442− if (!existingConfig) return;
442+ const existing = yield* operationStore.getSource(namespace);
443+ if (!existing) return;
443444
444445 const updatedConfig = {
445− ...existingConfig,
446+ ...existing.config,
446447 ...(input.endpoint !== undefined ? { endpoint: input.endpoint } : {}),
447448 ...(input.headers !== undefined
448449 ? { headers: input.headers as Record<string, HeaderValueValue> }
@@ -454,26 +455,11 @@ export const graphqlPlugin = (options?: { · 5 unmodified lines
454455 headers: (updatedConfig.headers ?? {}) as Record<string, HeaderValueValue>,
455456 });
456457
457− const toolIds = yield* operationStore.listByNamespace(namespace);
458− for (const toolId of toolIds) {
459− const entry = yield* operationStore.get(toolId);
460− if (entry) {
461− yield* operationStore.put(
462− toolId,
463− namespace,
464− entry.binding,
465− newInvocationConfig,
466− );
467− }
468− }
469−
470− const sources = yield* operationStore.listSources();
471− const existingMeta = sources.find((s) => s.namespace === namespace);
472−
473458 yield* operationStore.putSource({
474459 namespace,
475− name: existingMeta?.name ?? namespace,
460+ name: existing.name,
476461 config: updatedConfig,
462+ invocationConfig: newInvocationConfig,
477463 });
478464 }),
479465 },
packages/plugins/graphql/src/sdk/stored-source.ts+5−1
@@ -1,6 +1,6 @@
11 import { Schema } from "effect";
22
3−import { HeaderValue } from "./types";
3+import { HeaderValue, InvocationConfig } from "./types";
44
55 // ---------------------------------------------------------------------------
66 // Stored source — the shape persisted by the operation store and exposed
@@ -16,6 +16,10 @@ export class StoredSourceSchema extends Schema.Class<StoredSourceSchema>("Graphq · 9 unmodified lines
1616 namespace: Schema.optional(Schema.String),
1717 headers: Schema.optional(Schema.Record({ key: Schema.String, value: HeaderValue })),
1818 }),
19+ // TODO(migration): make required once all rows have been migrated to
20+ // carry invocationConfig. Left optional for decode compat with rows
21+ // written before the source-level invocationConfig refactor.
22+ invocationConfig: Schema.optional(InvocationConfig),
1923 }) {}
2024
2125 export type StoredSourceSchemaType = typeof StoredSourceSchema.Type;
packages/plugins/openapi/src/sdk/invoke.ts+11−1
@@ -292,7 +292,17 @@ export const makeOpenApiInvoker = (opts: { · 291 unmodified lines
292292 });
293293 }
294294
295− const { binding, config } = entry;
295+ const source = yield* opts.operationStore.getSource(entry.namespace);
296+ if (!source) {
297+ return yield* new ToolInvocationError({
298+ toolId,
299+ message: `No source found for namespace "${entry.namespace}"`,
300+ cause: undefined,
301+ });
302+ }
303+
304+ const { binding } = entry;
305+ const { invocationConfig: config } = source;
296306 const baseUrl = config.baseUrl;
297307
298308 // Resolve secret-backed headers
packages/plugins/openapi/src/sdk/kv-operation-store.ts+64−7
@@ -8,7 +8,7 @@ import { Effect, Schema } from "effect"; · 7 unmodified lines
88 import { scopeKv, makeInMemoryScopedKv, type Kv, type ToolId, type ScopedKv } from "@executor/sdk";
99
1010 import type { OpenApiOperationStore, StoredOperation, StoredSource } from "./operation-store";
11−import { OperationBinding, InvocationConfig } from "./types";
11+import { InvocationConfig, OperationBinding } from "./types";
1212 import { StoredSourceSchema } from "./stored-source";
1313
1414 // ---------------------------------------------------------------------------
@@ -18,12 +18,23 @@ import { StoredSourceSchema } from "./stored-source"; · 3 unmodified lines
1818 class StoredEntry extends Schema.Class<StoredEntry>("StoredEntry")({
1919 namespace: Schema.String,
2020 binding: OperationBinding,
21− config: InvocationConfig,
2221 }) {}
2322
2423 const encodeEntry = Schema.encodeSync(Schema.parseJson(StoredEntry));
2524 const decodeEntry = Schema.decodeUnknownSync(Schema.parseJson(StoredEntry));
2625
26+// TODO(migration): remove LegacyStoredEntry + rehydrateInvocationConfig
27+// once all source rows have been migrated off the pre-refactor schema.
28+// Old binding rows inlined the resolved InvocationConfig, which is the
29+// only place the server-derived baseUrl was persisted.
30+class LegacyStoredEntry extends Schema.Class<LegacyStoredEntry>("LegacyStoredEntry")({
31+ namespace: Schema.String,
32+ binding: OperationBinding,
33+ config: Schema.optional(InvocationConfig),
34+}) {}
35+
36+const decodeLegacyEntry = Schema.decodeUnknownSync(Schema.parseJson(LegacyStoredEntry));
37+
2738 const encodeSource = Schema.encodeSync(Schema.parseJson(StoredSourceSchema));
2839 const decodeSource = Schema.decodeUnknownSync(Schema.parseJson(StoredSourceSchema));
2940
@@ -37,22 +48,53 @@ const makeStore = (bindings: ScopedKv, sources: ScopedKv): OpenApiOperationStore · 7 unmodified lines
3748 effect: Effect.Effect<A, E, never>,
3849 ): Effect.Effect<A, E, never> => kv.withTransaction?.(effect) ?? effect;
3950
51+ // TODO(migration): remove along with LegacyStoredEntry.
52+ // Rebuild invocationConfig for a source row that predates the refactor.
53+ // We try to recover the resolved baseUrl from any legacy binding row
54+ // (old rows inlined the full InvocationConfig); if none is found, fall
55+ // back to the user-provided baseUrl from the source config.
56+ type DecodedSource = Omit<StoredSource, "invocationConfig"> & {
57+ invocationConfig?: InvocationConfig;
58+ };
59+ const rehydrateInvocationConfig = (
60+ source: DecodedSource,
61+ ): Effect.Effect<StoredSource> =>
62+ Effect.gen(function* () {
63+ if (source.invocationConfig) return source as StoredSource;
64+ let recovered: InvocationConfig | null = null;
65+ const entries = yield* bindings.list();
66+ for (const e of entries) {
67+ const legacy = decodeLegacyEntry(e.value);
68+ if (legacy.namespace === source.namespace && legacy.config) {
69+ recovered = legacy.config;
70+ break;
71+ }
72+ }
73+ const invocationConfig =
74+ recovered ??
75+ new InvocationConfig({
76+ baseUrl: source.config.baseUrl ?? "",
77+ headers: source.config.headers ?? {},
78+ });
79+ return { ...source, invocationConfig };
80+ });
81+
4082 return {
4183 get: (toolId) =>
4284 Effect.gen(function* () {
4385 const raw = yield* bindings.get(toolId);
4486 if (!raw) return null;
4587 const entry = decodeEntry(raw);
46− return { binding: entry.binding, config: entry.config };
88+ return { binding: entry.binding, namespace: entry.namespace };
4789 }),
4890
4991 put: (entries: readonly StoredOperation[]) =>
5092 withKvTransaction(
5193 bindings,
5294 bindings.set(
53− entries.map(({ toolId, namespace, binding, config }) => ({
95+ entries.map(({ toolId, namespace, binding }) => ({
5496 key: toolId,
55− value: encodeEntry(new StoredEntry({ namespace, binding, config })),
97+ value: encodeEntry(new StoredEntry({ namespace, binding })),
5698 })),
5799 ),
58100 ),
@@ -89,14 +131,29 @@ const makeStore = (bindings: ScopedKv, sources: ScopedKv): OpenApiOperationStore · 30 unmodified lines
89131 listSources: () =>
90132 Effect.gen(function* () {
91133 const entries = yield* sources.list();
92− return entries.map((e) => decodeSource(e.value) as StoredSource);
134+ const out: StoredSource[] = [];
135+ for (const e of entries) {
136+ const raw = decodeSource(e.value) as DecodedSource;
137+ // TODO(migration): rehydrate in memory only — avoid N writes per list.
138+ out.push(
139+ raw.invocationConfig
140+ ? (raw as StoredSource)
141+ : yield* rehydrateInvocationConfig(raw),
142+ );
143+ }
144+ return out;
93145 }),
94146
95147 getSource: (namespace) =>
96148 Effect.gen(function* () {
97149 const raw = yield* sources.get(namespace);
98150 if (!raw) return null;
99− return decodeSource(raw) as StoredSource;
151+ const source = decodeSource(raw) as DecodedSource;
152+ if (source.invocationConfig) return source as StoredSource;
153+ // TODO(migration): self-heal — rehydrate and write back once.
154+ const healed = yield* rehydrateInvocationConfig(source);
155+ yield* sources.set([{ key: namespace, value: encodeSource(healed) }]);
156+ return healed;
100157 }),
101158
102159 getSourceConfig: (namespace) =>
packages/plugins/openapi/src/sdk/operation-store.ts+3−2
@@ -18,19 +18,20 @@ export interface StoredSource { · 17 unmodified lines
1818 readonly namespace: string;
1919 readonly name: string;
2020 readonly config: SourceConfig;
21+ /** Pre-resolved runtime invocation config (baseUrl resolved from servers, headers). */
22+ readonly invocationConfig: InvocationConfig;
2123 }
2224
2325 export interface StoredOperation {
2426 readonly toolId: ToolId;
2527 readonly namespace: string;
2628 readonly binding: OperationBinding;
27− readonly config: InvocationConfig;
2829 }
2930
3031 export interface OpenApiOperationStore {
3132 readonly get: (
3233 toolId: ToolId,
33− ) => Effect.Effect<{ binding: OperationBinding; config: InvocationConfig } | null>;
34+ ) => Effect.Effect<{ binding: OperationBinding; namespace: string } | null>;
3435
3536 readonly put: (entries: readonly StoredOperation[]) => Effect.Effect<void>;
3637
packages/plugins/openapi/src/sdk/plugin.ts+7−24
@@ -275,7 +275,6 @@ export const openApiPlugin = (options?: { · 274 unmodified lines
275275 toolId: ToolId.make(`${namespace}.${def.toolPath}`),
276276 namespace,
277277 binding: toBinding(def),
278− config: invocationConfig,
279278 })),
280279 );
281280
@@ -291,6 +290,7 @@ export const openApiPlugin = (options?: { · 9 unmodified lines
291290 namespace: config.namespace,
292291 headers: config.headers,
293292 },
293+ invocationConfig,
294294 });
295295
296296 return { sourceId: namespace, toolCount: registrations.length };
@@ -345,11 +345,11 @@ export const openApiPlugin = (options?: { · 48 unmodified lines
345345
346346 updateSource: (namespace: string, input: OpenApiUpdateSourceInput) =>
347347 Effect.gen(function* () {
348− const existingSource = yield* operationStore.getSourceConfig(namespace);
349− if (!existingSource) return;
348+ const existing = yield* operationStore.getSource(namespace);
349+ if (!existing) return;
350350
351351 const updatedConfig = {
352− ...existingSource,
352+ ...existing.config,
353353 ...(input.baseUrl !== undefined ? { baseUrl: input.baseUrl } : {}),
354354 ...(input.headers !== undefined
355355 ? { headers: input.headers as Record<string, HeaderValueValue> }
@@ -357,32 +357,15 @@ export const openApiPlugin = (options?: { · 1 unmodified line
357357 };
358358
359359 const newInvocationConfig = new InvocationConfig({
360− baseUrl: updatedConfig.baseUrl ?? resolveBaseUrl([]),
360+ baseUrl: updatedConfig.baseUrl ?? existing.invocationConfig.baseUrl,
361361 headers: (updatedConfig.headers ?? {}) as Record<string, HeaderValueValue>,
362362 });
363363
364− const toolIds = yield* operationStore.listByNamespace(namespace);
365− for (const toolId of toolIds) {
366− const entry = yield* operationStore.get(toolId);
367− if (entry) {
368− yield* operationStore.put([
369− {
370− toolId,
371− namespace,
372− binding: entry.binding,
373− config: newInvocationConfig,
374− },
375− ]);
376− }
377− }
378−
379− const sources = yield* operationStore.listSources();
380− const existingMeta = sources.find((s) => s.namespace === namespace);
381−
382364 yield* operationStore.putSource({
383365 namespace,
384− name: existingMeta?.name ?? namespace,
366+ name: existing.name,
385367 config: updatedConfig,
368+ invocationConfig: newInvocationConfig,
386369 });
387370 }),
388371 },
packages/plugins/openapi/src/sdk/stored-source.ts+5−1
@@ -1,6 +1,6 @@
11 import { Schema } from "effect";
22
3−import { HeaderValue } from "./types";
3+import { HeaderValue, InvocationConfig } from "./types";
44
55 // ---------------------------------------------------------------------------
66 // Stored source — the shape persisted by the operation store and exposed
@@ -16,6 +16,10 @@ export class StoredSourceSchema extends Schema.Class<StoredSourceSchema>("OpenAp · 9 unmodified lines
1616 namespace: Schema.optional(Schema.String),
1717 headers: Schema.optional(Schema.Record({ key: Schema.String, value: HeaderValue })),
1818 }),
19+ // TODO(migration): make required once all rows have been migrated to
20+ // carry invocationConfig. Left optional for decode compat with rows
21+ // written before the source-level invocationConfig refactor.
22+ invocationConfig: Schema.optional(InvocationConfig),
1923 }) {}
2024
2125 export type StoredSourceSchemaType = typeof StoredSourceSchema.Type;