From 7c59b368371d7a91e7a391aea4e74e006836052e Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Wed, 2 Sep 2026 05:19:22 +0000 Subject: [PATCH 1/2] test: pin the client provider mirrors against their server originals (#5673) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `client/src/utils/providers.js` hand-mirrors three server modules plus the vendored toolkit, and every one of those mirrors was pinned by prose alone — each side carried its own hardcoded expectations, so a row added to one copy left both suites green. That is user-visible in both directions: an effort level the picker offers but the server's ladder rejects gets clamped or dropped, and a tool-use family the server recognises but the client doesn't puts a "no known tool use" warning on a model that works fine. Three parity suites now read the client file as TEXT (never import it, so no client deps leak into the server CI job): - `providerModels.mirror.test.js` pins the six effort ladders, `EFFORT_RANK`, `CODEX_ULTRA_MODELS`, `CONFIGURED_DEFAULT_SENTINELS` and the Antigravity effort-suffix split. - `localModelHeuristics.mirror.test.js` pins the embedding / vision / tool-use capability regexes. The server spells them as a commented array of alternatives and the client as one inline literal, so declaration text can't be diffed — the new `compareRegexDeclaration` primitive compares what each pattern MATCHES instead. The same suite pins the toolkit's third `TOOL_USE_RE` copy as text. - `providerGateways.parity.test.js` gains the client registry as a third leg, scoped to the `id`/`label`/`apiKeyEnv`/`legacyMarker` fields the browser actually carries (it omits `baseURL`/`legacyApiKeyField`, so `toEqual` against the server rows can't be used). Also refreshes the mirror pointers that had already rotted to `server/lib/stageRunner.js`, a path that has not existed since the module moved to `server/services/`. Claude-Session: https://claude.ai/code/session_01GMxEz43s3YCLaVZV9KmVwE --- client/src/utils/providers.js | 12 ++- server/lib/README.md | 2 +- server/lib/aiToolkit/providers.js | 3 +- server/lib/localModelHeuristics.js | 10 +- .../lib/localModelHeuristics.mirror.test.js | 93 +++++++++++++++++++ server/lib/mirrorParity.js | 61 ++++++++++++ server/lib/mirrorParity.test.js | 39 +++++++- server/lib/pipelineValidation.js | 4 +- server/lib/providerGateways.js | 3 +- server/lib/providerGateways.parity.test.js | 70 ++++++++++++-- server/lib/providerModels.js | 3 +- server/lib/providerModels.mirror.test.js | 77 +++++++++++++++ 12 files changed, 353 insertions(+), 24 deletions(-) create mode 100644 server/lib/localModelHeuristics.mirror.test.js create mode 100644 server/lib/providerModels.mirror.test.js diff --git a/client/src/utils/providers.js b/client/src/utils/providers.js index ac1b92f4fc..3f0e446dd5 100644 --- a/client/src/utils/providers.js +++ b/client/src/utils/providers.js @@ -44,7 +44,7 @@ export const GEMINI_CONTEXT_WINDOW = 1_048_576; export const GROK_CONTEXT_WINDOW = 256_000; export const KIMI_CONTEXT_WINDOW = 256_000; -// Keep in sync with server/lib/stageRunner.js. +// Keep in sync with server/services/stageRunner.js. const KNOWN_MODEL_CONTEXT_WINDOWS = Object.freeze([ [/gpt[-_.:/]?5\.5(?:[-_.:/]|\b)/i, CODEX_CONTEXT_WINDOW], [/gpt[-_.:/]?5\.4[-_.:/]?mini(?:[-_.:/]|\b)/i, 400_000], @@ -648,7 +648,8 @@ export const isVisionCapableCliProvider = (provider) => * Tool-use (function-calling) capable model detector — mirror of `isToolUseModel` * in server/lib/localModelHeuristics.js (and the TOOL_USE_RE inlined in * server/lib/aiToolkit/providers.js). Keep all three in lockstep (the server libs - * can't be imported here). Ollama's /api/show `tools` capability is authoritative + * can't be imported here) — server/lib/localModelHeuristics.mirror.test.js reads + * this file as text and fails when the patterns stop matching the same ids. Ollama's /api/show `tools` capability is authoritative * when known; this id regex is the fallback for bare model-id strings. The CoS * agent harness depends on reliable tool-calling, so only these families should * be selectable for a local-model-backed coding provider. @@ -964,7 +965,7 @@ export const CONTEXT_WINDOW_SOURCE = Object.freeze({ * The window this provider's own `/models` catalog reported for this model, or * `null` when it never mentioned it. Recorded by model refresh — the serving * side's own declaration, so it outranks the hand-maintained regex table. - * Mirror of `catalogModelContextWindow` in server/lib/stageRunner.js. + * Mirror of `catalogModelContextWindow` in server/services/stageRunner.js. */ export function catalogModelContextWindow(provider, model) { const windows = provider?.modelContextWindows; @@ -976,7 +977,7 @@ export function catalogModelContextWindow(provider, model) { /** * The planning context window for a provider/model AND where it came from. - * Mirror of `effectiveContextWindow` in server/lib/stageRunner.js — the two + * Mirror of `effectiveContextWindow` in server/services/stageRunner.js — the two * must resolve identically, or the card promises a budget the budgeter won't use. * * `{ tokens: null, source: null }` means nothing is known (an unrecognized model @@ -1276,7 +1277,8 @@ export const modelCapabilityInfo = (provider, model, { * front-end. MIRROR of `PROVIDER_GATEWAYS` in `server/lib/providerGateways.js` * (and its vendored twin `server/lib/aiToolkit/internal/gateways.js`) — the * browser cannot import server code, so the table is duplicated; keep the three - * in lockstep. `id` is simultaneously the OpenCode namespace, the + * in lockstep (server/lib/providerGateways.parity.test.js pins this copy's + * `id`/`label`/`apiKeyEnv`/`legacyMarker` rows against the server registry). `id` is simultaneously the OpenCode namespace, the * `gatewayBacked` marker value, and the id of the sibling `api` record that * owns the key. */ diff --git a/server/lib/README.md b/server/lib/README.md index 885a9a2182..ba8957befc 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -491,7 +491,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub |---|---| | `dbTestGate.js` | `requireDbOrSkip(label, dbReady, reason)` keeps a missing local test database as a visible skipped suite, but throws when `PORTOS_REQUIRE_DB` is set so CI cannot pass after DB-backed suites disappear. | | `gitTestRepo.js` | Shared real-git sandbox for integration tests (#4394): one initialized template (working tree + bare origin) per worker, then `fs.cp` into a fresh temp dir. `makeGitSandbox({ origin })`, `attachBareOrigin(scratch, repo)`, `materializeGitRepo(dest)`, `destroyGitSandbox`, plus `SKIP_HEAVY_INTEGRATION` (`VITEST_FAST=1`). Every entry point runs `assertTempPath` first, so a path outside `os.tmpdir()` throws instead of `git init`-ing or `rm -rf`-ing a real checkout (#4554). Still real git — just not rebuilt from `init`+`commit`+`push` in every `beforeEach`. | -| `mirrorParity.js` | Source-comparison primitives for the `*.mirror.test.js` server↔client parity tests: `stripCommentsAndNormalize` (so per-side commentary may diverge but logic may not), `extractDeclaration(src, name)` (balanced `{}`/`()`/`[]` walk over `function` / `async function` / `const`), and `compareDeclaration(serverSrc, clientSrc, name)`. Use these instead of hand-rolling a brace-walker per mirror. Pure — no `vitest` import — so callers own the assertions. | +| `mirrorParity.js` | Source-comparison primitives for the `*.mirror.test.js` server↔client parity tests: `stripCommentsAndNormalize` (so per-side commentary may diverge but logic may not), `extractDeclaration(src, name)` (balanced `{}`/`()`/`[]` walk over `function` / `async function` / `const`), `compareDeclaration(serverSrc, clientSrc, name)`, and `compareRegexDeclaration(serverSrc, clientSrc, serverName, clientName?)` (for a regex spelled as a `new RegExp([…].join('|'), 'i')` array on one side and an inline `/…/i` literal on the other — compares what it matches, not how it is typeset). Use these instead of hand-rolling a brace-walker per mirror. Pure — no `vitest` import — so callers own the assertions. | | `mockPathsDataRoot.js` | Shared Vitest helpers for `PATHS.data → temp dir` and no-peer record creation guards. | | `settingsTestUtil.js` | `bindSettingsFile(dataRoot)` → `writeSettingsFile`/`mergeSettingsFile`: direct settings.json disk writes that also drop the `getSettings()` read cache (dynamic-import reset) so a stale cache can't survive a bypass-`save()` write. | | `testHelper.js` | Test helpers: `request()` (supertest-style HTTP) + `mockJsonResponse`/`mockTextResponse` (fetch `Response` mocks with `.text()`, `.json()`, and a `headers.get` content-type), `startLoopbackServer(app)`/`closeLoopbackServer(server)`/`waitForAbort(signal)` for tests that need a real socket (raw disconnects, SSE streaming) that `request()`'s run-to-completion fetch harness can't model, plus the source-scan pair `collectServerSources()` / `readServerSource(rel)` (and `SERVER_DIR`) used by the whole-tree guard suites — `spawnCwd.test.js` (#3193) and `cliChildEnv.test.js` (#3194). Those guards overlap deliberately, so they share one definition of "a source file"; change the ignore rules here and both move together. Cross-platform trio: `posixPath(v)` normalizes a RECEIVED path before comparing it to a POSIX-spelled literal (no-op on POSIX — never normalize the expectation, which would hide a genuinely wrong path), and `resolveTestPython()` returns an interpreter that actually runs, probing by execution because Windows ships a `python` Store-alias stub that exists but fails; `null` when there is none, for `describe.skipIf`; `pinPlatform(value)` pins `process.platform` and returns a restore that reinstates the ORIGINAL descriptor (deleting the pin when there was none) — it carries the one hazard every hand-rolled pin had to rediscover: never pin above an import that loads a native addon, which picks its prebuilt binary off the platform at load time (#4085). | diff --git a/server/lib/aiToolkit/providers.js b/server/lib/aiToolkit/providers.js index 92d2ba7b07..a5815ac221 100644 --- a/server/lib/aiToolkit/providers.js +++ b/server/lib/aiToolkit/providers.js @@ -143,7 +143,8 @@ const execFileAsync = (file, args, options) => // Tool-use (function-calling) capable model families. Inlined here because the // aiToolkit is self-contained (no imports out to server/lib). MIRROR of // TOOL_USE_RE in server/lib/localModelHeuristics.js and isToolUseModel in -// client/src/utils/providers.js — keep all three in lockstep. +// client/src/utils/providers.js — keep all three in lockstep +// (server/lib/localModelHeuristics.mirror.test.js fails when they drift). const TOOL_USE_RE = new RegExp([ 'qwen', 'llama-?3\\.[1-9]', 'llama-?4', diff --git a/server/lib/localModelHeuristics.js b/server/lib/localModelHeuristics.js index 695bf0512a..be7acd47e8 100644 --- a/server/lib/localModelHeuristics.js +++ b/server/lib/localModelHeuristics.js @@ -9,9 +9,10 @@ * generation/fallback run — the cause of the nomic-embed-text fallback bug) * - localLlm.getStatus (recommend a best-fit editorial model) * - * The client mirrors `isEmbeddingModel` + `isVisionModel` in - * client/src/utils/providers.js — keep the regexes in lockstep (the - * aiToolkit/lib dirs can't be imported there). + * The client mirrors `isEmbeddingModel` + `isVisionModel` + `isToolUseModel` + * in client/src/utils/providers.js — keep the regexes in lockstep (the + * aiToolkit/lib dirs can't be imported there). `localModelHeuristics.mirror.test.js` + * enforces that, by what each pattern matches rather than by its text. */ // Embedding-only models — never valid for chat/generation. The bge/nomic/e5/gte @@ -171,7 +172,8 @@ export function isVisionModel(model) { // Gemma 4), so the gemma rule is anchored to the family AND the version. // // MIRRORED in client/src/utils/providers.js (isToolUseModel) and inlined in -// server/lib/aiToolkit/providers.js (TOOL_USE_RE) — keep all three in lockstep. +// server/lib/aiToolkit/providers.js (TOOL_USE_RE) — keep all three in lockstep; +// `localModelHeuristics.mirror.test.js` fails when any of them drifts. const TOOL_USE_RE = new RegExp([ 'qwen', 'llama-?3\\.[1-9]', 'llama-?4', diff --git a/server/lib/localModelHeuristics.mirror.test.js b/server/lib/localModelHeuristics.mirror.test.js new file mode 100644 index 0000000000..91d8c106a5 --- /dev/null +++ b/server/lib/localModelHeuristics.mirror.test.js @@ -0,0 +1,93 @@ +/** + * Mirror parity test for the local-model capability regexes, which exist in + * three copies by architecture: + * 1. server/lib/localModelHeuristics.js — authoritative; + * 2. client/src/utils/providers.js — the browser cannot import server code; + * 3. server/lib/aiToolkit/providers.js — the vendored toolkit may not import + * out of its own directory (see aiToolkit/AGENTS.md), so TOOL_USE_RE is + * inlined there too. + * + * All three were pinned by a "keep all three in lockstep" comment and by + * per-side hardcoded id lists (`providers.test.js`'s + * `describe('isToolUseModel (mirror of server localModelHeuristics)')` never + * reads the server file), so a family added to one copy alone left every suite + * green. The consequence is user-visible in both directions: a model the server + * accepts for tool use gets a "no known tool use" warning in the agent picker, + * and an embedding model the client hasn't learned to recognise is offered in a + * generation picker, where the daemon answers `400 … does not support chat`. + * + * The server spells VISION_RE / TOOL_USE_RE as a multi-line array of + * alternatives so each one can carry its own comment; the client inlines the + * same pattern as a single literal. Declaration TEXT therefore cannot be + * compared — `compareRegexDeclaration` compares what the two patterns match + * instead. Typesetting is irrelevant; the accepted id set is not. The toolkit + * copy is the array form on both sides, so it is compared as text. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import { resolve, dirname } from 'path'; +import { fileURLToPath } from 'url'; +import { compareDeclaration, compareRegexDeclaration } from './mirrorParity.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +const SERVER_PATH = resolve(__dirname, 'localModelHeuristics.js'); +const CLIENT_PATH = resolve(__dirname, '../../client/src/utils/providers.js'); +const TOOLKIT_PATH = resolve(__dirname, 'aiToolkit/providers.js'); + +// [server declaration, the client predicate that inlines it] +const MIRRORED_REGEXES = [ + ['EMBEDDING_RE', 'isEmbeddingModel'], + ['VISION_RE', 'isVisionModel'], + ['TOOL_USE_RE', 'isToolUseModel'], +]; + +describe('localModelHeuristics↔client providers capability-regex mirror parity', () => { + const serverSrc = readFileSync(SERVER_PATH, 'utf8'); + const clientSrc = readFileSync(CLIENT_PATH, 'utf8'); + + it('both files are non-empty', () => { + expect(serverSrc.length).toBeGreaterThan(100); + expect(clientSrc.length).toBeGreaterThan(100); + }); + + for (const [serverName, clientName] of MIRRORED_REGEXES) { + it(`${serverName} accepts the same ids as the client's ${clientName}`, () => { + const { serverDecl, clientDecl, serverSource, clientSource } = + compareRegexDeclaration(serverSrc, clientSrc, serverName, clientName); + + expect(serverDecl, `server/lib/localModelHeuristics.js is missing: ${serverName}`).not.toBeNull(); + expect(clientDecl, `client/src/utils/providers.js is missing: ${clientName}`).not.toBeNull(); + expect( + serverSource, + `server/lib/localModelHeuristics.js#${serverName} is neither a /…/i literal nor the new RegExp([…].join('|'), 'i') form`, + ).not.toBeNull(); + expect( + clientSource, + `client/src/utils/providers.js#${clientName} no longer inlines a /…/i literal`, + ).not.toBeNull(); + expect( + clientSource, + `"${serverName}" diverged from the client's ${clientName} — the server copy is authoritative; port the alternatives verbatim`, + ).toBe(serverSource); + }); + } +}); + +describe('localModelHeuristics↔aiToolkit TOOL_USE_RE mirror parity', () => { + const serverSrc = readFileSync(SERVER_PATH, 'utf8'); + const toolkitSrc = readFileSync(TOOLKIT_PATH, 'utf8'); + + it('is present and identical in both server copies (code only)', () => { + const { serverDecl, clientDecl, serverNorm, clientNorm } = + compareDeclaration(serverSrc, toolkitSrc, 'TOOL_USE_RE'); + + expect(serverDecl, 'server/lib/localModelHeuristics.js is missing: TOOL_USE_RE').not.toBeNull(); + expect(clientDecl, 'server/lib/aiToolkit/providers.js is missing: TOOL_USE_RE').not.toBeNull(); + expect( + clientNorm, + 'TOOL_USE_RE diverged — the localModelHeuristics copy is authoritative; port the change verbatim', + ).toBe(serverNorm); + }); +}); diff --git a/server/lib/mirrorParity.js b/server/lib/mirrorParity.js index 8269cbdef8..b3b97a7c80 100644 --- a/server/lib/mirrorParity.js +++ b/server/lib/mirrorParity.js @@ -117,3 +117,64 @@ export function compareDeclaration(serverSrc, clientSrc, name) { match: serverNorm != null && serverNorm === clientNorm, }; } + +// A single-quoted JS string literal, escapes included, as written in source. +const STRING_LITERAL_RE = /'((?:[^'\\]|\\.)*)'/g; +// `new RegExp([ 'a', 'b' ].join('|'), 'i')` — the multi-line, per-alternative +// array form the server files use so each alternative can carry its own comment. +const ARRAY_FORM_RE = /new RegExp\(\s*\[(.*)\]\.join\('\|'\)\s*,\s*'i'\s*\)/; +// A `/…/i` regex literal, terminated by the statement's `;` or a `.test(` call. +// Greedy on purpose: `[-_/:]` puts an unescaped `/` mid-pattern, so a lazy walk +// would stop inside a character class. +const REGEX_LITERAL_RE = /\/(.*)\/i(?=\.test\(|;)/; + +/** + * The alternation source of a capability regex, whichever of the two forms the + * declaration is typeset in — the array form above, or a plain `/…/i` literal. + * + * `/` is normalized (an escaped `\/` and a bare `/` mean the same thing to the + * engine, and only one of them is legal inside a literal outside a character + * class), so the two sides may differ in slash escaping but not in what they + * accept. Returns null when the declaration is neither form. + */ +export function regexAlternationSource(declText) { + if (declText == null) return null; + const norm = stripCommentsAndNormalize(declText); + const array = ARRAY_FORM_RE.exec(norm); + const source = array + ? [...array[1].matchAll(STRING_LITERAL_RE)] + // Source text → string VALUE: `'llama-?3\\.[1-9]'` is the regex fragment + // `llama-?3\.[1-9]`. Comparing the raw literals instead would report a + // divergence on every escaped metacharacter. + .map(([, body]) => body.replace(/\\(.)/g, '$1')) + .join('|') + : REGEX_LITERAL_RE.exec(norm)?.[1] ?? null; + return source == null ? null : source.replace(/\\\//g, '/'); +} + +/** + * Compare a case-insensitive regex mirrored across two files that spell it + * differently — an array of per-alternative strings on one side, a single + * inline literal on the other — by what it MATCHES rather than by its text. + * + * `compareDeclaration` can't be used there: the two typesettings never compare + * equal even when they accept exactly the same ids, and forcing one side to + * adopt the other's form is churn for no behavioural gain. + * + * `clientName` defaults to `serverName`; pass it when the client inlines the + * pattern inside a differently-named predicate (`isToolUseModel` wrapping what + * the server declares as `TOOL_USE_RE`). + */ +export function compareRegexDeclaration(serverSrc, clientSrc, serverName, clientName = serverName) { + const serverDecl = extractDeclaration(serverSrc, serverName); + const clientDecl = extractDeclaration(clientSrc, clientName); + const serverSource = regexAlternationSource(serverDecl); + const clientSource = regexAlternationSource(clientDecl); + return { + serverDecl, + clientDecl, + serverSource, + clientSource, + match: serverSource != null && serverSource === clientSource, + }; +} diff --git a/server/lib/mirrorParity.test.js b/server/lib/mirrorParity.test.js index 03b985710c..d890ee0f37 100644 --- a/server/lib/mirrorParity.test.js +++ b/server/lib/mirrorParity.test.js @@ -1,5 +1,10 @@ import { describe, it, expect } from 'vitest'; -import { stripCommentsAndNormalize, extractDeclaration, compareDeclaration } from './mirrorParity.js'; +import { + stripCommentsAndNormalize, + extractDeclaration, + compareDeclaration, + compareRegexDeclaration, +} from './mirrorParity.js'; describe('stripCommentsAndNormalize', () => { it('drops block and line comments and collapses whitespace', () => { @@ -93,3 +98,35 @@ describe('compareDeclaration', () => { expect(match).toBe(false); }); }); + +describe('compareRegexDeclaration', () => { + // The shape this exists for: the server spells a capability regex as an array + // of per-alternative strings (so each alternative can carry its own comment) + // while the browser copy inlines the same pattern as one literal. + const server = "const TOOL_USE_RE = new RegExp([\n 'qwen',\n 'llama-?3\\\\.[1-9]',\n].join('|'), 'i');\n"; + const client = "export const isToolUseModel = (id) =>\n /qwen|llama-?3\\.[1-9]/i.test(id);\n"; + + it('matches the array form against the inline literal it compiles to', () => { + const { serverSource, match } = compareRegexDeclaration(server, client, 'TOOL_USE_RE', 'isToolUseModel'); + expect(serverSource).toBe('qwen|llama-?3\\.[1-9]'); + expect(match).toBe(true); + }); + + it('reports a mismatch when one side gains an alternative', () => { + const drifted = "export const isToolUseModel = (id) =>\n /qwen|llama-?3\\.[1-9]|newfamily/i.test(id);\n"; + expect(compareRegexDeclaration(server, drifted, 'TOOL_USE_RE', 'isToolUseModel').match).toBe(false); + }); + + it('treats an escaped and a bare slash as the same pattern', () => { + // `[-_/:]` is legal bare inside a character class and must be escaped + // outside one; that typesetting difference is not a divergence. + const bare = 'const RE = /a[-_/:]b/i;\n'; + const escaped = 'const RE = /a[-_\\/:]b/i;\n'; + expect(compareRegexDeclaration(bare, escaped, 'RE').match).toBe(true); + }); + + it('does not report a match when a side is missing or is not a regex', () => { + expect(compareRegexDeclaration(server, 'const other = 1;', 'TOOL_USE_RE').match).toBe(false); + expect(compareRegexDeclaration(server, 'const TOOL_USE_RE = 42;', 'TOOL_USE_RE').match).toBe(false); + }); +}); diff --git a/server/lib/pipelineValidation.js b/server/lib/pipelineValidation.js index 91cb568010..f2f4042762 100644 --- a/server/lib/pipelineValidation.js +++ b/server/lib/pipelineValidation.js @@ -609,7 +609,7 @@ export const objectBibleUpdateSchema = writersRoomObjectUpdateSchema; // Per-call timeout bounds: STAGE_TIMEOUT_MIN_MS / STAGE_TIMEOUT_MAX_MS are // imported (aliased) from aiToolkit/constants.js at the top of this file so -// the route validator, the runner (server/lib/stageRunner.js), and the +// the route validator, the runner (server/services/stageRunner.js), and the // toolkit's own provider/run validation all share one source of truth. The // client mirror in client/src/utils/formatters.js can't import across the // server boundary — comments on both sides flag the requirement to keep @@ -674,7 +674,7 @@ export const stageConfigUpdateSchema = z.object({ // string so the inner `.number()` check fails. The digit-only rule // (and the `.trim()` before it) mirror `parseTimeoutMs` in // client/src/utils/formatters.js and `normalizeTimeout` in - // server/lib/stageRunner.js so all three reject the same shapes. + // server/services/stageRunner.js so all three reject the same shapes. (v) => { if (v === '' || v === null) return null; if (v === undefined) return undefined; diff --git a/server/lib/providerGateways.js b/server/lib/providerGateways.js index 6c8c335bae..9a7e740c08 100644 --- a/server/lib/providerGateways.js +++ b/server/lib/providerGateways.js @@ -30,7 +30,8 @@ * import out of its own directory (see `aiToolkit/AGENTS.md`); * `providerGateways.parity.test.js` fails when the two drift; * 3. `client/src/utils/providers.js` — the browser, which cannot import server - * code at all. + * code at all; `providerGateways.parity.test.js` reads it as TEXT (never + * imports it) and pins the fields it carries. */ /** diff --git a/server/lib/providerGateways.parity.test.js b/server/lib/providerGateways.parity.test.js index d2c2a4d7f8..0ce9bd0974 100644 --- a/server/lib/providerGateways.parity.test.js +++ b/server/lib/providerGateways.parity.test.js @@ -1,20 +1,58 @@ /** * The gateway registry exists in three places by architecture — the vendored * `aiToolkit/` may not import out of its own directory, and the browser cannot - * import server code at all. This suite pins the two SERVER copies together so - * a new gateway added to one is never silently missing from the other (which - * would show up as a wrapper that spawns fine but can never refresh its models, - * or vice versa). + * import server code at all. This suite pins all three together so a new + * gateway added to one is never silently missing from another (which would show + * up as a wrapper that spawns fine but can never refresh its models, or a + * gateway the server supports that no picker ever offers). * - * The client copy (`client/src/utils/providers.js`) is deliberately NOT imported - * here: a server suite that imports a client module drags the client's deps into - * the server CI job. It carries a "keep in lockstep" comment naming both server - * copies instead. + * The two SERVER copies are compared as VALUES — the toolkit module imports + * cleanly here, so `toEqual` pins every field including `baseURL`. + * + * The client copy (`client/src/utils/providers.js`) is compared as TEXT: it is + * read with `readFileSync` and its rows are parsed out of the source, never + * imported, so the client's dependency tree stays out of the server CI job. + * That comparison is deliberately field-scoped — the browser omits `baseURL` + * and `legacyApiKeyField` (it never dials the gateway, and it never handles the + * key), so `toEqual` against the server rows cannot be used. The fields it does + * carry are all user-visible or dispatch-critical: `id` is the OpenCode + * namespace AND the sibling-key lookup, `label` and `apiKeyEnv` are rendered in + * the provider form, and `legacyMarker` is how a pre-registry stored record + * still resolves. */ import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import { resolve, dirname } from 'path'; +import { fileURLToPath } from 'url'; import { PROVIDER_GATEWAYS as SERVER_GATEWAYS } from './providerGateways.js'; import { PROVIDER_GATEWAYS as TOOLKIT_GATEWAYS, gatewayForProvider as toolkitGatewayFor } from './aiToolkit/internal/gateways.js'; import { gatewayForProvider as serverGatewayFor } from './providerGateways.js'; +import { extractDeclaration, stripCommentsAndNormalize } from './mirrorParity.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const CLIENT_PATH = resolve(__dirname, '../../client/src/utils/providers.js'); + +// The fields the browser copy carries, and the only ones it can be held to. +const CLIENT_FIELDS = ['id', 'label', 'apiKeyEnv', 'legacyMarker']; + +const pickClientFields = (row) => Object.fromEntries( + CLIENT_FIELDS.filter((field) => row[field] !== undefined).map((field) => [field, row[field]]), +); + +/** + * The client registry's rows, parsed from source. Each row is a flat + * `Object.freeze({ key: 'value', … })` with no nested braces, so a + * brace-delimited split over the comment-stripped declaration is enough — and + * anything more structural would mean importing the module, which is the thing + * this file exists to avoid. + */ +function parseClientGatewayRows() { + const declaration = extractDeclaration(readFileSync(CLIENT_PATH, 'utf8'), 'PROVIDER_GATEWAYS'); + if (declaration == null) return null; + return [...stripCommentsAndNormalize(declaration).matchAll(/\{([^{}]*)\}/g)].map(([, body]) => + Object.fromEntries([...body.matchAll(/(\w+):\s*'([^']*)'/g)].map(([, key, value]) => [key, value])), + ); +} describe('providerGateways ↔ aiToolkit/internal/gateways parity', () => { it('declares the same rows, in the same order', () => { @@ -35,3 +73,19 @@ describe('providerGateways ↔ aiToolkit/internal/gateways parity', () => { } }); }); + +describe('providerGateways ↔ client/src/utils/providers.js parity', () => { + const clientRows = parseClientGatewayRows(); + + it('the client declares a parseable PROVIDER_GATEWAYS table', () => { + expect(clientRows, 'client/src/utils/providers.js is missing: PROVIDER_GATEWAYS').not.toBeNull(); + expect(clientRows.length).toBeGreaterThan(0); + }); + + it('declares the same rows, in the same order (browser-visible fields)', () => { + expect( + clientRows, + 'the gateway registry diverged — server/lib/providerGateways.js is authoritative; port the row verbatim (minus baseURL / legacyApiKeyField, which the browser omits)', + ).toEqual(SERVER_GATEWAYS.map(pickClientFields)); + }); +}); diff --git a/server/lib/providerModels.js b/server/lib/providerModels.js index d20639b92a..0db62639cf 100644 --- a/server/lib/providerModels.js +++ b/server/lib/providerModels.js @@ -77,7 +77,8 @@ export const resolveCliModel = (model) => isConfiguredDefaultModel(model) ? null // values (`none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, plus // model-gated `ultra`) and agy // (`--help`: "Reasoning effort for the current CLI session (low|medium|high)"). Mirrored in -// client/src/utils/providers.js — keep in sync. +// client/src/utils/providers.js — keep in sync +// (`providerModels.mirror.test.js` fails when the two copies drift). // // Codex Ultra adds automatic task delegation on the models that advertise it. // Keep it model-gated: older Codex models and Luna top out at `max`. diff --git a/server/lib/providerModels.mirror.test.js b/server/lib/providerModels.mirror.test.js new file mode 100644 index 0000000000..f848ca53b4 --- /dev/null +++ b/server/lib/providerModels.mirror.test.js @@ -0,0 +1,77 @@ +/** + * Mirror parity test for the effort ladders and model sentinels shared by + * server/lib/providerModels.js and client/src/utils/providers.js (the browser + * cannot import server code, so the tables are duplicated on each side behind a + * "keep in sync" comment). + * + * Until now that comment was the only pin: each side had its own hardcoded + * expectations (`providerModels.test.js` / `providers.test.js`), so an effort + * level or Codex Ultra model added to one copy alone left BOTH suites green. + * The two ends then disagree about what the user may select — an effort the + * picker offers and the server's ladder rejects is clamped to something else or + * dropped, and an ultra-capable model the client hasn't heard of loses its + * `ultra` rung entirely. + * + * Only the DATA is mirrored. `effortLevelsForProvider` / `resolveCliEffort` are + * legitimately `function` server-side and arrow consts client-side (and the + * client's ladder resolution falls back to the server-published + * `effortLevels`/`effortLevelsByModel` fields, which have no server-side + * counterpart), so they are not compared. Comparison strips comments, so + * per-side commentary may diverge — logic can't. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import { resolve, dirname } from 'path'; +import { fileURLToPath } from 'url'; +import { compareDeclaration } from './mirrorParity.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +const SERVER_PATH = resolve(__dirname, 'providerModels.js'); +const CLIENT_PATH = resolve(__dirname, '../../client/src/utils/providers.js'); + +const MIRRORED_NAMES = [ + 'CLAUDE_EFFORT_LEVELS', + 'CODEX_EFFORT_LEVELS', + 'CODEX_ULTRA_EFFORT_LEVELS', + 'ANTIGRAVITY_EFFORT_LEVELS', + 'OPENCODE_LOCAL_EFFORT_LEVELS', + 'CURSOR_EFFORT_LEVELS', + // The ladder a Codex model gets is model-gated, so the gate's membership is + // as load-bearing as the ladders themselves: a new Ultra model added + // server-side only tops the client picker out at `max`. + 'CODEX_ULTRA_MODELS', + // The clamp order. Divergence here is silent — every value stays "known", it + // just resolves to a different rung than the run will actually use. + 'EFFORT_RANK', + 'CONFIGURED_DEFAULT_SENTINELS', + // Derived FROM ANTIGRAVITY_EFFORT_LEVELS on both sides, and pinned anyway: + // the suffix split is what turns `gemini-3.6-flash-high` into a base model + // plus an effort, so a divergence strands a stored model id. + 'ANTIGRAVITY_EFFORT_SUFFIX_RE', +]; + +describe('providerModels↔client providers effort-ladder mirror parity', () => { + const serverSrc = readFileSync(SERVER_PATH, 'utf8'); + const clientSrc = readFileSync(CLIENT_PATH, 'utf8'); + + it('both files are non-empty', () => { + expect(serverSrc.length).toBeGreaterThan(100); + expect(clientSrc.length).toBeGreaterThan(100); + }); + + for (const name of MIRRORED_NAMES) { + it(`${name} is present and identical on both sides (code only)`, () => { + const { serverDecl, clientDecl, serverNorm, clientNorm } = + compareDeclaration(serverSrc, clientSrc, name); + + expect(serverDecl, `server/lib/providerModels.js is missing: ${name}`).not.toBeNull(); + expect(clientDecl, `client/src/utils/providers.js is missing: ${name}`).not.toBeNull(); + expect( + clientNorm, + `"${name}" diverged — the server copy is authoritative; port the change verbatim`, + ).toBe(serverNorm); + }); + } +}); From 94d117d055e4ed1aa912a1e301a14fff29021297 Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Wed, 2 Sep 2026 05:31:18 +0000 Subject: [PATCH 2/2] test: make the provider mirror readers fail closed on a shape they can't account for (#5673) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review pass on the new parity guards. Each reader scanned its input for the parts it recognised and ignored the rest, so a mirror that had genuinely changed could still be reported intact — the worst possible outcome for a guard test. All three now consume their input to the end and return null on any leftover: - the alternative reader accepts only a comma-separated run of string literals, so `[...SHARED, 'extra']` or a named fragment can no longer silently vanish from the comparison; - escapes other than `\\` and `\'` are refused rather than copied through as their own text — `'\x2e'` is `.`, which matches anything; - both the capability-regex and gateway-table readers are anchored to the WHOLE declaration, so a decoy array or row list left inside a wrapped or conditional assignment no longer reads as the exported value; - gateway rows reject spreads, computed values and duplicate keys. Each rule gets a probe asserting the reader returns null rather than a partial read. Claude-Session: https://claude.ai/code/session_01GMxEz43s3YCLaVZV9KmVwE --- server/lib/README.md | 2 +- server/lib/mirrorParity.js | 104 ++++++++++++++++----- server/lib/mirrorParity.test.js | 35 +++++++ server/lib/providerGateways.parity.test.js | 88 +++++++++++++++-- 4 files changed, 197 insertions(+), 32 deletions(-) diff --git a/server/lib/README.md b/server/lib/README.md index ba8957befc..ce2c53d948 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -491,7 +491,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub |---|---| | `dbTestGate.js` | `requireDbOrSkip(label, dbReady, reason)` keeps a missing local test database as a visible skipped suite, but throws when `PORTOS_REQUIRE_DB` is set so CI cannot pass after DB-backed suites disappear. | | `gitTestRepo.js` | Shared real-git sandbox for integration tests (#4394): one initialized template (working tree + bare origin) per worker, then `fs.cp` into a fresh temp dir. `makeGitSandbox({ origin })`, `attachBareOrigin(scratch, repo)`, `materializeGitRepo(dest)`, `destroyGitSandbox`, plus `SKIP_HEAVY_INTEGRATION` (`VITEST_FAST=1`). Every entry point runs `assertTempPath` first, so a path outside `os.tmpdir()` throws instead of `git init`-ing or `rm -rf`-ing a real checkout (#4554). Still real git — just not rebuilt from `init`+`commit`+`push` in every `beforeEach`. | -| `mirrorParity.js` | Source-comparison primitives for the `*.mirror.test.js` server↔client parity tests: `stripCommentsAndNormalize` (so per-side commentary may diverge but logic may not), `extractDeclaration(src, name)` (balanced `{}`/`()`/`[]` walk over `function` / `async function` / `const`), `compareDeclaration(serverSrc, clientSrc, name)`, and `compareRegexDeclaration(serverSrc, clientSrc, serverName, clientName?)` (for a regex spelled as a `new RegExp([…].join('|'), 'i')` array on one side and an inline `/…/i` literal on the other — compares what it matches, not how it is typeset). Use these instead of hand-rolling a brace-walker per mirror. Pure — no `vitest` import — so callers own the assertions. | +| `mirrorParity.js` | Source-comparison primitives for the `*.mirror.test.js` server↔client parity tests: `stripCommentsAndNormalize` (so per-side commentary may diverge but logic may not), `extractDeclaration(src, name)` (balanced `{}`/`()`/`[]` walk over `function` / `async function` / `const`), `compareDeclaration(serverSrc, clientSrc, name)`, and `compareRegexDeclaration(serverSrc, clientSrc, serverName, clientName?)` / `regexAlternationSource(declText)` (for a regex spelled as a `new RegExp([…].join('|'), 'i')` array on one side and an inline `/…/i` literal on the other — compares what it matches, not how it is typeset, and returns `null` rather than a partial read on any shape it can't decode). Use these instead of hand-rolling a brace-walker per mirror. Pure — no `vitest` import — so callers own the assertions. | | `mockPathsDataRoot.js` | Shared Vitest helpers for `PATHS.data → temp dir` and no-peer record creation guards. | | `settingsTestUtil.js` | `bindSettingsFile(dataRoot)` → `writeSettingsFile`/`mergeSettingsFile`: direct settings.json disk writes that also drop the `getSettings()` read cache (dynamic-import reset) so a stale cache can't survive a bypass-`save()` write. | | `testHelper.js` | Test helpers: `request()` (supertest-style HTTP) + `mockJsonResponse`/`mockTextResponse` (fetch `Response` mocks with `.text()`, `.json()`, and a `headers.get` content-type), `startLoopbackServer(app)`/`closeLoopbackServer(server)`/`waitForAbort(signal)` for tests that need a real socket (raw disconnects, SSE streaming) that `request()`'s run-to-completion fetch harness can't model, plus the source-scan pair `collectServerSources()` / `readServerSource(rel)` (and `SERVER_DIR`) used by the whole-tree guard suites — `spawnCwd.test.js` (#3193) and `cliChildEnv.test.js` (#3194). Those guards overlap deliberately, so they share one definition of "a source file"; change the ignore rules here and both move together. Cross-platform trio: `posixPath(v)` normalizes a RECEIVED path before comparing it to a POSIX-spelled literal (no-op on POSIX — never normalize the expectation, which would hide a genuinely wrong path), and `resolveTestPython()` returns an interpreter that actually runs, probing by execution because Windows ships a `python` Store-alias stub that exists but fails; `null` when there is none, for `describe.skipIf`; `pinPlatform(value)` pins `process.platform` and returns a restore that reinstates the ORIGINAL descriptor (deleting the pin when there was none) — it carries the one hazard every hand-rolled pin had to rediscover: never pin above an import that loads a native addon, which picks its prebuilt binary off the platform at load time (#4085). | diff --git a/server/lib/mirrorParity.js b/server/lib/mirrorParity.js index b3b97a7c80..12688d5313 100644 --- a/server/lib/mirrorParity.js +++ b/server/lib/mirrorParity.js @@ -118,37 +118,99 @@ export function compareDeclaration(serverSrc, clientSrc, name) { }; } -// A single-quoted JS string literal, escapes included, as written in source. -const STRING_LITERAL_RE = /'((?:[^'\\]|\\.)*)'/g; -// `new RegExp([ 'a', 'b' ].join('|'), 'i')` — the multi-line, per-alternative -// array form the server files use so each alternative can carry its own comment. -const ARRAY_FORM_RE = /new RegExp\(\s*\[(.*)\]\.join\('\|'\)\s*,\s*'i'\s*\)/; -// A `/…/i` regex literal, terminated by the statement's `;` or a `.test(` call. -// Greedy on purpose: `[-_/:]` puts an unescaped `/` mid-pattern, so a lazy walk -// would stop inside a character class. -const REGEX_LITERAL_RE = /\/(.*)\/i(?=\.test\(|;)/; +// One single-quoted JS string literal at the head of the remaining array body, +// plus its trailing separator. +const NEXT_STRING_LITERAL_RE = /^'((?:[^'\\]|\\.)*)'\s*(?:,\s*)?/; +// Each accepted spelling of a mirrored capability regex, anchored to the WHOLE +// normalized declaration. Anchoring is what makes an unrecognized shape fail +// closed: a search-anywhere pattern would happily read a decoy array out of a +// declaration that assigns something else entirely, and report the mirror as +// intact. Group 1 is the alternation body in every one of them. +// +// 1. `const NAME = new RegExp([ 'a', 'b' ].join('|'), 'i');` — the server's +// array form, so each alternative can carry its own comment; +// 2. `const NAME = /…/i;` — a plain literal; +// 3. `export const isX = (id) => && /…/i.test(id);` — the browser's +// predicate form, where the literal is inlined at the end of a guard chain. +const DECLARATION_FORMS = [ + /^(?:export\s+)?const\s+\w+\s*=\s*new RegExp\(\s*\[(.*)\]\s*\.join\('\|'\)\s*,\s*'i'\s*\)\s*;$/, + // Greedy on purpose: `[-_/:]` puts an unescaped `/` mid-pattern, so a lazy + // walk would end the literal inside a character class. + /^(?:export\s+)?const\s+\w+\s*=\s*\/(.*)\/i\s*;$/, + /^(?:export\s+)?const\s+\w+\s*=\s*\(\w+\)\s*=>\s*(?:[^/]*&&\s*)?\/(.*)\/i\.test\(\w+\)\s*;$/, +]; +// Only the first form's body is a list of alternatives; the other two capture +// the finished pattern. +const [ARRAY_FORM_RE] = DECLARATION_FORMS; /** - * The alternation source of a capability regex, whichever of the two forms the - * declaration is typeset in — the array form above, or a plain `/…/i` literal. + * The regex fragment a single-quoted source literal denotes, or null when it + * uses an escape this reader cannot decode by inspection. + * + * Only `\\` (a backslash the regex engine will actually see, which is how every + * alternative spells `\d` / `\.`) and `\'` are decodable here. `\x2e`, `\u002e` + * and `\n` all denote something OTHER than their own text — `'\x2e'` is `.`, + * which matches ANY character — so copying them through verbatim would let a + * client literal that means something different compare equal. These guards + * exist to catch drift, so an unreadable literal fails CLOSED rather than + * being guessed at. + */ +function decodeRegexFragmentLiteral(body) { + let out = ''; + for (let i = 0; i < body.length; i++) { + const ch = body[i]; + if (ch !== '\\') { + out += ch; + continue; + } + const escaped = body[i + 1]; + if (escaped !== '\\' && escaped !== "'") return null; + out += escaped; + i++; + } + return out; +} + +/** + * Every alternative in an array body, or null when the body holds anything + * other than a comma-separated run of decodable string literals. + * + * Failing closed on the leftovers is the point: a `[...SHARED, 'extra']` or a + * `[NAMED_FRAGMENT]` element read by a scan-for-literals pass would silently + * vanish from the comparison, and the mirror it is supposed to pin would drift + * green. + */ +function parseAlternativeList(body) { + const alternatives = []; + let rest = body.trim(); + while (rest.length > 0) { + const match = NEXT_STRING_LITERAL_RE.exec(rest); + if (!match) return null; + const decoded = decodeRegexFragmentLiteral(match[1]); + if (decoded === null) return null; + alternatives.push(decoded); + rest = rest.slice(match[0].length); + } + return alternatives; +} + +/** + * The alternation source of a capability regex, whichever of the accepted + * spellings above the declaration is typeset in. * * `/` is normalized (an escaped `\/` and a bare `/` mean the same thing to the * engine, and only one of them is legal inside a literal outside a character * class), so the two sides may differ in slash escaping but not in what they - * accept. Returns null when the declaration is neither form. + * accept. Returns null for any shape not in `DECLARATION_FORMS` — a mirror the + * reader cannot fully account for must fail its guard, not skip past it. */ export function regexAlternationSource(declText) { if (declText == null) return null; const norm = stripCommentsAndNormalize(declText); - const array = ARRAY_FORM_RE.exec(norm); - const source = array - ? [...array[1].matchAll(STRING_LITERAL_RE)] - // Source text → string VALUE: `'llama-?3\\.[1-9]'` is the regex fragment - // `llama-?3\.[1-9]`. Comparing the raw literals instead would report a - // divergence on every escaped metacharacter. - .map(([, body]) => body.replace(/\\(.)/g, '$1')) - .join('|') - : REGEX_LITERAL_RE.exec(norm)?.[1] ?? null; + const form = DECLARATION_FORMS.find((re) => re.test(norm)); + if (!form) return null; + const [, body] = form.exec(norm); + const source = form === ARRAY_FORM_RE ? parseAlternativeList(body)?.join('|') ?? null : body; return source == null ? null : source.replace(/\\\//g, '/'); } diff --git a/server/lib/mirrorParity.test.js b/server/lib/mirrorParity.test.js index d890ee0f37..8a8199c32a 100644 --- a/server/lib/mirrorParity.test.js +++ b/server/lib/mirrorParity.test.js @@ -4,6 +4,7 @@ import { extractDeclaration, compareDeclaration, compareRegexDeclaration, + regexAlternationSource, } from './mirrorParity.js'; describe('stripCommentsAndNormalize', () => { @@ -130,3 +131,37 @@ describe('compareRegexDeclaration', () => { expect(compareRegexDeclaration(server, 'const TOOL_USE_RE = 42;', 'TOOL_USE_RE').match).toBe(false); }); }); + +describe('regexAlternationSource fails closed', () => { + // A scan-for-literals pass would silently DROP the non-literal element and + // report the surviving alternatives as the whole pattern — a mirror that has + // genuinely diverged then compares equal. Refuse the array instead. + it('refuses an array holding anything but string literals', () => { + expect(regexAlternationSource("const RE = new RegExp(['a', 'b'].join('|'), 'i');")).toBe('a|b'); + expect(regexAlternationSource("const RE = new RegExp(['a', SHARED].join('|'), 'i');")).toBeNull(); + expect(regexAlternationSource("const RE = new RegExp([...SHARED, 'b'].join('|'), 'i');")).toBeNull(); + }); + + // `'\\x2e'` is the single character `.`, which matches ANYTHING — copying the + // escape through as the text `x2e` would compare equal to a client `/x2e/i` + // that means something entirely different. + it('refuses an escape it cannot decode by inspection', () => { + expect(regexAlternationSource("const RE = new RegExp(['a\\\\.b'].join('|'), 'i');")).toBe('a\\.b'); + expect(regexAlternationSource("const RE = new RegExp(['\\x2e'].join('|'), 'i');")).toBeNull(); + expect(regexAlternationSource("const RE = new RegExp(['\\n'].join('|'), 'i');")).toBeNull(); + }); + + // A search-anywhere read would find the array and report the mirror intact + // while the declaration assigns something else entirely. + it('refuses a declaration that merely CONTAINS a recognizable form', () => { + const array = "new RegExp(['a', 'b'].join('|'), 'i')"; + expect(regexAlternationSource(`const RE = ${array};`)).toBe('a|b'); + expect(regexAlternationSource(`const RE = FLAG ? ${array} : OTHER_RE;`)).toBeNull(); + expect(regexAlternationSource(`const RE = widen(${array});`)).toBeNull(); + }); + + it('returns null for a declaration that is neither supported form', () => { + expect(regexAlternationSource(null)).toBeNull(); + expect(regexAlternationSource('const RE = buildPattern();')).toBeNull(); + }); +}); diff --git a/server/lib/providerGateways.parity.test.js b/server/lib/providerGateways.parity.test.js index 0ce9bd0974..82f8c0aa44 100644 --- a/server/lib/providerGateways.parity.test.js +++ b/server/lib/providerGateways.parity.test.js @@ -39,19 +39,64 @@ const pickClientFields = (row) => Object.fromEntries( CLIENT_FIELDS.filter((field) => row[field] !== undefined).map((field) => [field, row[field]]), ); +// The whole normalized declaration, anchored: `export const PROVIDER_GATEWAYS = +// Object.freeze([ … ]);`. Anchoring is what makes an unrecognized shape fail +// closed — a scan for `{…}` anywhere in the declaration would keep reading the +// old row literals out of a table that had been wrapped in a transform, and +// report a registry that no longer exists as intact. +const TABLE_RE = /^(?:export\s+)?const\s+PROVIDER_GATEWAYS\s*=\s*Object\.freeze\(\s*\[(.*)\]\s*\)\s*;$/; +// One `Object.freeze({ … })` row at the head of the remaining array body, plus +// its trailing separator. +const NEXT_ROW_RE = /^Object\.freeze\(\s*\{([^{}]*)\}\s*\)\s*(?:,\s*)?/; +// One `key: 'value'` property at the head of the remaining row body, plus its +// trailing separator. Escapes are rejected outright — no gateway field has one, +// and decoding them by inspection is how a parser starts guessing. +const NEXT_ROW_FIELD_RE = /^(\w+)\s*:\s*'([^'\\]*)'\s*(?:,\s*)?/; + +/** + * Consume `text` with `nextRe` until nothing is left, collecting what each + * match yields — or null the moment a leftover doesn't match. + * + * Failing closed on the leftover is the whole point: a `{ id: 'x', ...overrides }` + * row, or a table wrapped in a helper, read by a scan-for-what-I-recognize pass + * would compare equal to the static shape it stopped being, and the drift this + * guard exists to catch would pass green. + */ +function consumeAll(text, nextRe, valueOf) { + const collected = []; + let rest = text.trim(); + while (rest.length > 0) { + const match = nextRe.exec(rest); + if (!match) return null; + const value = valueOf(match, collected); + if (value === null) return null; + collected.push(value); + rest = rest.slice(match[0].length); + } + return collected; +} + +/** One registry row as a plain object, or null when it is not a static table. */ +function parseGatewayRow(body) { + const fields = consumeAll(body, NEXT_ROW_FIELD_RE, ([, key, value], seen) => + (seen.some(([existing]) => existing === key) ? null : [key, value])); + return fields && Object.fromEntries(fields); +} + /** - * The client registry's rows, parsed from source. Each row is a flat - * `Object.freeze({ key: 'value', … })` with no nested braces, so a - * brace-delimited split over the comment-stripped declaration is enough — and - * anything more structural would mean importing the module, which is the thing - * this file exists to avoid. + * A registry declaration's rows, or null when it — or any row in it — is no + * longer a flat run of static literals. Anything more structural would mean + * importing the module, which is the thing this file exists to avoid. */ +function parseGatewayTable(declaration) { + const table = TABLE_RE.exec(stripCommentsAndNormalize(declaration)); + return table && consumeAll(table[1], NEXT_ROW_RE, ([, body]) => parseGatewayRow(body)); +} + +/** `parseGatewayTable` over the browser copy's `PROVIDER_GATEWAYS`. */ function parseClientGatewayRows() { const declaration = extractDeclaration(readFileSync(CLIENT_PATH, 'utf8'), 'PROVIDER_GATEWAYS'); - if (declaration == null) return null; - return [...stripCommentsAndNormalize(declaration).matchAll(/\{([^{}]*)\}/g)].map(([, body]) => - Object.fromEntries([...body.matchAll(/(\w+):\s*'([^']*)'/g)].map(([, key, value]) => [key, value])), - ); + return declaration == null ? null : parseGatewayTable(declaration); } describe('providerGateways ↔ aiToolkit/internal/gateways parity', () => { @@ -78,10 +123,33 @@ describe('providerGateways ↔ client/src/utils/providers.js parity', () => { const clientRows = parseClientGatewayRows(); it('the client declares a parseable PROVIDER_GATEWAYS table', () => { - expect(clientRows, 'client/src/utils/providers.js is missing: PROVIDER_GATEWAYS').not.toBeNull(); + expect( + clientRows, + 'client/src/utils/providers.js#PROVIDER_GATEWAYS is missing, or a row is no longer a flat table of static `key: \'value\'` properties — this guard cannot read it, so re-shape the row or teach the parser', + ).not.toBeNull(); expect(clientRows.length).toBeGreaterThan(0); }); + // The parser must refuse what it cannot see through, not skip it: a row that + // pulls fields in at runtime would otherwise compare equal to the static row + // it stopped being. + it('refuses a row carrying anything but static single-quoted properties', () => { + expect(parseGatewayRow("id: 'openrouter', label: 'OpenRouter'")).toEqual({ id: 'openrouter', label: 'OpenRouter' }); + expect(parseGatewayRow("id: 'openrouter', ...overrides")).toBeNull(); + expect(parseGatewayRow("id: 'openrouter', label: LABELS.openrouter")).toBeNull(); + expect(parseGatewayRow("id: 'openrouter', id: 'orcarouter'")).toBeNull(); + }); + + // …and the table reader must refuse a declaration whose rows it can still + // see but whose exported value they no longer are. + it('refuses a table that is not a plain frozen array of frozen rows', () => { + const rows = "Object.freeze({ id: 'openrouter', label: 'OpenRouter' })"; + expect(parseGatewayTable(`export const PROVIDER_GATEWAYS = Object.freeze([${rows}]);`)) + .toEqual([{ id: 'openrouter', label: 'OpenRouter' }]); + expect(parseGatewayTable(`export const PROVIDER_GATEWAYS = Object.freeze([${rows}].map(withBaseURL));`)).toBeNull(); + expect(parseGatewayTable(`export const PROVIDER_GATEWAYS = Object.freeze([${rows}, ...EXTRA]);`)).toBeNull(); + }); + it('declares the same rows, in the same order (browser-visible fields)', () => { expect( clientRows,