From 9ced57bc089942d8e474e92a68f97ba72cf2aadc Mon Sep 17 00:00:00 2001 From: Xiangzhe Date: Tue, 18 Aug 2026 00:36:21 -0300 Subject: [PATCH] refactor(sse): route executor lookup through ExecutorRegistry (R0.3) Adds open-sse/executors/registry.ts (Map-based registry mirroring translator/registry.ts): the built-in table in executors/index.ts stays declarative, every entry is registered at module load, and getExecutor()/hasSpecializedExecutor() resolve through the registry. DefaultExecutor fallback, its memoization, and the cloud-agent (#6699) / search-provider (#10274) guards are unchanged. Also fixes a latent lookup leak: the old object-literal lookup treated Object.prototype names (constructor, toString, ...) as specialized executors; the Map registry resolves them to the DefaultExecutor fallback like any unknown provider. Parity proof: executor-map golden (137 entries, byte-identical before/after), check:known-symbols green, 1018 tests across the 65 executor test files green. Docs: OPEN_SSE_ARCHITECTURE factory section corrected (it claimed generation from providerRegistry). Refs #3501 --- docs/frameworks/OPEN_SSE_ARCHITECTURE.md | 2 +- open-sse/executors/index.ts | 25 ++++++++++- open-sse/executors/registry.ts | 38 ++++++++++++++++ tests/unit/executor-registry.test.ts | 57 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 open-sse/executors/registry.ts create mode 100644 tests/unit/executor-registry.test.ts diff --git a/docs/frameworks/OPEN_SSE_ARCHITECTURE.md b/docs/frameworks/OPEN_SSE_ARCHITECTURE.md index 83ebf35a95..0f265d53bd 100644 --- a/docs/frameworks/OPEN_SSE_ARCHITECTURE.md +++ b/docs/frameworks/OPEN_SSE_ARCHITECTURE.md @@ -368,7 +368,7 @@ const result = await executor.execute({ }); ```` -The factory is generated from `config/providerRegistry.ts` which lists all 338 providers and their executor class. +Resolution goes through the `ExecutorRegistry` (`executors/registry.ts`): every specialized executor is declared in the built-in table of `executors/index.ts` and registered via `registerExecutor(alias, instance)` at module load; `getExecutor()` consults the registry and falls back to a memoized `DefaultExecutor` for any provider without a specialized entry. The full alias → executor mapping is characterized by the golden test `tests/unit/executor-map-golden.test.ts`. --- diff --git a/open-sse/executors/index.ts b/open-sse/executors/index.ts index 39c5880780..faa8c667f5 100644 --- a/open-sse/executors/index.ts +++ b/open-sse/executors/index.ts @@ -1,4 +1,9 @@ import { SEARCH_PROVIDERS } from "../config/searchRegistry.ts"; +import { + registerExecutor, + getRegisteredExecutor, + hasRegisteredExecutor, +} from "./registry.ts"; import { AntigravityExecutor } from "./antigravity.ts"; import { GithubExecutor } from "./github.ts"; import { GheCopilotExecutor } from "./ghe-copilot.ts"; @@ -78,6 +83,12 @@ import { XaiExecutor } from "./xai.ts"; import { PromptQlExecutor } from "./promptql.ts"; import { ConolWebExecutor } from "./conol-web.ts"; +// R0.3 — declarative built-in table. The object literal stays as the single +// place built-ins are declared (compile-time duplicate-key safety; the +// check:known-symbols gate parses this literal from source), but lookup goes +// through the ExecutorRegistry (./registry.ts): every entry is registered at +// module load below, and getExecutor()/hasSpecializedExecutor() consult the +// registry — the literal is never read at request time. const executors = { antigravity: new AntigravityExecutor(), agy: new AntigravityExecutor(), @@ -221,6 +232,13 @@ const executors = { cnl: new ConolWebExecutor(), // Alias }; +// Bootstrap: register every built-in in the ExecutorRegistry. registerExecutor +// throws on duplicates, so an alias collision fails at module load, exactly as +// loudly as a duplicate object key would have failed at lint time. +for (const [alias, executor] of Object.entries(executors)) { + registerExecutor(alias, executor); +} + const defaultCache = new Map(); // #6699 — providers that exist ONLY as Cloud Agent task-API entries @@ -246,7 +264,8 @@ const CHAT_UNSUPPORTED_CLOUD_AGENT_PROVIDERS = new Set(["jules"]); const CHAT_UNSUPPORTED_SEARCH_PROVIDERS = new Set(Object.keys(SEARCH_PROVIDERS)); export function getExecutor(provider) { - if (executors[provider]) return executors[provider]; + const registered = getRegisteredExecutor(provider); + if (registered) return registered; if (CHAT_UNSUPPORTED_CLOUD_AGENT_PROVIDERS.has(provider)) { const err = new Error( `Provider "${provider}" is a cloud-agent provider and does not support direct chat completions; use the Cloud Agents task API instead.` @@ -266,9 +285,11 @@ export function getExecutor(provider) { } export function hasSpecializedExecutor(provider) { - return !!executors[provider]; + return hasRegisteredExecutor(provider); } +export { registerExecutor, listExecutorAliases } from "./registry.ts"; + export { BaseExecutor } from "./base.ts"; export { AntigravityExecutor } from "./antigravity.ts"; export { GithubExecutor } from "./github.ts"; diff --git a/open-sse/executors/registry.ts b/open-sse/executors/registry.ts new file mode 100644 index 0000000000..4d6a002ecd --- /dev/null +++ b/open-sse/executors/registry.ts @@ -0,0 +1,38 @@ +import type { BaseExecutor } from "./base.ts"; + +// R0.3 — ExecutorRegistry: runtime registry for provider executors, mirroring +// open-sse/translator/registry.ts. Built-ins register at module load from +// executors/index.ts; getExecutor() resolves through this map instead of a +// hard-coded object literal. This is the seam the v4 plan (M1.6 +// host.registerProvider) extends — today the surface is internal-only. +// +// The alias → executor mapping is characterized by +// tests/unit/executor-map-golden.test.ts (tests/snapshots/executors/): any +// change to keys, classes or instance sharing shows up as a golden diff. + +const registry = new Map(); + +/** + * Register an executor under an alias. Aliases are unique: registering the + * same alias twice throws, preserving the guarantee the old object literal + * gave at compile time (duplicate keys were impossible). + */ +export function registerExecutor(alias: string, executor: BaseExecutor): void { + if (registry.has(alias)) { + throw new Error(`executor alias already registered: "${alias}"`); + } + registry.set(alias, executor); +} + +export function getRegisteredExecutor(alias: string): BaseExecutor | undefined { + return registry.get(alias); +} + +export function hasRegisteredExecutor(alias: string): boolean { + return registry.has(alias); +} + +/** All registered aliases, in registration order. */ +export function listExecutorAliases(): string[] { + return [...registry.keys()]; +} diff --git a/tests/unit/executor-registry.test.ts b/tests/unit/executor-registry.test.ts new file mode 100644 index 0000000000..c05128681b --- /dev/null +++ b/tests/unit/executor-registry.test.ts @@ -0,0 +1,57 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// R0.3 — unit tests for the ExecutorRegistry seam itself (registration +// semantics + wiring of the built-ins). Behavior parity of the full map is +// covered separately by tests/unit/executor-map-golden.test.ts. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-executor-registry-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const { registerExecutor, getRegisteredExecutor, hasRegisteredExecutor, listExecutorAliases } = + await import("../../open-sse/executors/registry.ts"); +const { getExecutor, hasSpecializedExecutor, BaseExecutor, DefaultExecutor } = await import( + "../../open-sse/executors/index.ts" +); + +test.after(() => { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("built-ins are registered at module load and resolve through the registry", () => { + const aliases = listExecutorAliases(); + assert.ok(aliases.length >= 100, `expected the built-in table, got ${aliases.length} aliases`); + for (const alias of ["antigravity", "kiro", "glm", "9router", "conol-web"]) { + assert.ok(hasRegisteredExecutor(alias), `missing built-in: ${alias}`); + assert.equal(getExecutor(alias), getRegisteredExecutor(alias)); + assert.ok(getExecutor(alias) instanceof BaseExecutor); + } +}); + +test("registerExecutor throws on duplicate alias", () => { + assert.throws(() => registerExecutor("kiro", getRegisteredExecutor("kiro")!), { + message: /already registered: "kiro"/, + }); +}); + +test("registering a new alias makes it resolvable via getExecutor and hasSpecializedExecutor", () => { + const alias = "registry-test-provider"; + assert.equal(hasSpecializedExecutor(alias), false); + const instance = new DefaultExecutor(alias); + registerExecutor(alias, instance); + assert.equal(hasSpecializedExecutor(alias), true); + assert.equal(getExecutor(alias), instance); +}); + +test("registry lookup is exact — Object.prototype names are not executors", () => { + // The old object-literal lookup (`executors[provider]`) leaked prototype + // members: getExecutor("constructor") returned Object's constructor. The Map + // registry must treat these as unknown providers (DefaultExecutor fallback). + for (const name of ["constructor", "toString", "hasOwnProperty", "__proto__"]) { + assert.equal(hasSpecializedExecutor(name), false, name); + assert.ok(getExecutor(name) instanceof DefaultExecutor, name); + } +});