From 2bbb6737cb9b4c8b7302be6224400152524e7c33 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 11:23:04 +0000 Subject: [PATCH] chore: remove dead code and tighten the repository's gates - Delete adapter-replacement.ts and its test. Nothing imported it, and deleting another plugin's registry entry is not a host API; the engineering notes record why, and what the plugin does instead. A gate fails the build if src/ writes the llm registry map directly. - Delete the stream hook's `prepared` handling: the host passes the prepared dispatch as its own argument and never reads options.prepared. - Untrack .freebuff/ and ignore it. - The settings partition test walks Config.dict, which is how debugFile went missing from it; a manifest test checks cordis.patch.yml against the schema and the field registry, and the manifest's phantom showUsagePrice, wrong marker text and incomplete lists are fixed. - utils/retry.ts takes its timer and logger as parameters, and the utils purity gate now also rejects console and global timers, walking the folder recursively. - tool-fallback reads free-tier ids from the request's own plane (Go iff opencode-go or opencode-go-*) and loses its unreachable Anthropic branch. A Zen-only free model is no longer treated as free on Go. - test/support/src.ts is the one door tests use into src/, so later module moves touch one file; CONTRIBUTING.md states the rule. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0141fKp5k6hiaJcP5ovhGtSD --- .freebuff/project-id | 1 - .gitignore | 1 + AGENTS.md | 3 +- CONTRIBUTING.md | 1 + README.md | 2 +- README.zh-CN.md | 2 +- cordis.patch.yml | 17 +- docs/engineering-notes.md | 12 + docs/improvement-plan.md | 2 +- src/adapter-replacement.ts | 120 --------- src/fetch-patch.ts | 3 + src/settings-fields.ts | 6 + src/settings-usage.tsx | 25 +- src/stream-hook.ts | 6 - src/tool-fallback.ts | 113 +++----- src/utils/retry.ts | 58 +++- test/adapter-replacement.test.ts | 214 --------------- test/architecture.test.ts | 365 ++++++++++++++++++++----- test/fetch-patch.test.ts | 101 ++++++- test/free-tier-contract.test.ts | 16 +- test/manifest.test.ts | 111 +++++++- test/retry.test.ts | 139 ++++++++-- test/settings-page.test.tsx | 26 +- test/settings-usage.test.tsx | 38 ++- test/stream-hook.test.ts | 148 ++++++++++- test/support/catalog-fixture.ts | 50 ++++ test/support/src.ts | 441 +++++++++++++++++++++++++++++++ test/test-helpers.ts | 2 +- test/tool-fallback.test.ts | 113 ++++++-- 29 files changed, 1540 insertions(+), 596 deletions(-) delete mode 100644 .freebuff/project-id delete mode 100644 src/adapter-replacement.ts delete mode 100644 test/adapter-replacement.test.ts create mode 100644 test/support/catalog-fixture.ts create mode 100644 test/support/src.ts diff --git a/.freebuff/project-id b/.freebuff/project-id deleted file mode 100644 index b164097c..00000000 --- a/.freebuff/project-id +++ /dev/null @@ -1 +0,0 @@ -a689d3ef-ace3-4ee0-bdef-783c86931947 diff --git a/.gitignore b/.gitignore index 8c33bb8f..ecb00ca2 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ coverage/ # Agent workspace: session notes and scratch data, not source. .workbuddy-ai/ +.freebuff/ diff --git a/AGENTS.md b/AGENTS.md index 1022cade..382d2acd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,7 +48,7 @@ Host bundle (`lib/index.mjs`), a thin `apply` barrel over small modules: src/utils/ a function of its arguments. No context, no globals, no I/O. src/ effects: read a context, ask a utility, cause something. -**The membership test for `utils/` is a property, not a vibe:** no `node:*`, no `@deepseek-ai/cordis`, no `process.env`, no imports from outside the folder except `guards.ts`. `test/architecture.test.ts` walks the tree and fails the build otherwise — verified by planting a violation of each kind and watching it fail. +**The membership test for `utils/` is a property, not a vibe:** no `node:*`, no `@deepseek-ai/cordis`, no `process.env`, no `fetch`, no reference to `console` or a global timer (`setTimeout`/`setInterval`/`setImmediate` and their `clear*` counterparts, called or passed as a value, bare or through `globalThis.`/`window.`/`global.`/`self.`), no relative import that resolves outside the folder (`guards.ts` lives inside it). A utility that needs a clock or a logger takes it as a parameter, the way `utils/retry.ts` takes its `sleep` and `warn` and leaves the timer to the caller. `test/architecture.test.ts` walks the folder recursively, resolves each import from the importing file's own directory, and fails the build otherwise — verified by planting a violation of each kind and watching it fail. It is the same trick `scripts/check.ts` already uses for the client bundle, at finer grain. @@ -86,7 +86,6 @@ It is the same trick `scripts/check.ts` already uses for the client bundle, at f - `src/go-limits.ts` — the allowance lookup, the label normalization and the money formatting. `allowanceFor` is the one lookup: `attachAllowance` in `usage.ts` calls it rather than inlining a second copy, which it used to. - `src/go-limits-data.ts` — the Go plan allowances, per model and per tier, **generated** from the vendor's own page by `scripts/generate-go-limits.ts`; never hand-edit it, and regenerate with `pnpm run limits:shim -- --write`. The scheduled freshness job is what notices when the page moves, which it did: the docs went under `/v2/docs/console/` and serve HTML now, so the generator reads a table off that page rather than the markdown it used to. - `src/responses-routes.ts` · `src/responses-provider.ts` — the protocol table and the in-process mount of the host's `llm-pi-ai`. -- `src/adapter-replacement.ts` — taking a provider route over from the adapter that already serves it: `llm.registerAdapter` refuses a route the host owns (`DUPLICATE_ADAPTER`), so the current entry is released first, our adapter is registered through the ordinary API, and the original is put back on disposal. The step towards serving a mixed-protocol route as the same provider the user configured, instead of mounting hidden ones. **Not wired yet**: nothing in `src/` imports it, only its own test, so read it as the next step rather than as live behaviour. - `src/purge-service.ts` · `src/purge-contract.ts` — the card's "purge deprecated models" action, and the shape it answers with. It **rewrites the profile**, deleting the declared model entries the live catalog has retired, through `configEditor` (the same service the settings UI writes with). That is deliberate: a read-time filter would leave the declaration in place, and then the only way to explain a missing row is to find the filter. One source of truth, and it is the configuration. - `src/session-cost.ts` · `src/cordis-context.ts` · `src/debug.ts` — per-turn accounting, the typed host interfaces, and stream debug logging. - `src/pi-provider.ts` — the per-protocol base URLs, the pi model object, and the gateway headers. Every value here is measured; see `docs/redirect-planes.md`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4a54c144..f571cf96 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -63,6 +63,7 @@ The web profile wires this checkout with `link:`, so builds are picked up like t - **Restore globals.** Tests that touch `globalThis.fetch` or `process.env` must restore them in `afterEach` (`vi.unstubAllGlobals()`, `delete process.env.…`). - **Offline by construction.** `test/support/offline.ts` refuses every non-loopback request in the unit and mock suites. The catalog's background revalidation used to reach `models.opencode.ai` dozens of times per run, so a networked runner's results could depend on the vendor's live catalog. A case that means to fetch stubs `fetch` itself. - **No order dependence.** `test/support/reset-process-state.ts` clears the plugin's module-level stores (captured keys, session spend, the live catalog) after every test. A new module-level store gets a reset next to its siblings. `pnpm run test:shuffle` is how to check, and CI runs it. +- **Tests reach `src` through `test/support/src.ts`.** It re-exports, from each defining module, everything a test needs, so a module move or a barrel change edits one file instead of thirty. A new test imports from `./support/src.ts` (`../support/src.ts` under `test/mock` and `test/e2e`); add a missing name there rather than importing a `src` path. The exceptions are `vi.mock("../src/x.ts")` targets (and their `importOriginal` type), which must name the module; the architecture tests, whose subject is the `src` layout itself; and the jsdom (client) tests, which import the defining module directly, because `src.ts` re-exports the host graph (`node:sqlite`, pi, `lifecycle.ts`) and the next rule keeps that out of client tests. Importing the door loads the whole `src` graph, host and client, so a module that fails to load fails every converted file; a client-safe door, limited to what the bundle gate admits, is P2's to add if that cost shows. The remaining pre-existing direct imports are converted in P2. - **Host-only fixtures stay out of shared helpers.** `test/test-helpers.ts` is imported by the jsdom (client) tests, so anything that needs `node:sqlite` or the filesystem belongs in `test/support/`, as `opencode-store-fixture.ts` does. - **A test for a fix fails without it.** Before committing a regression test, revert the fix locally and watch it fail. - **Meaningful coverage.** Every config field in the `SPECS` register (`src/settings-fields.ts`) needs both the on and off path where it has one; every passthrough claim needs a non-OpenCode URL test proving headers are untouched; helper functions exported from `usage-ui.ts` are asserted directly, not re-derived in the test. diff --git a/README.md b/README.md index 9a944891..aa7beefa 100644 --- a/README.md +++ b/README.md @@ -369,7 +369,7 @@ These exist in the schema but render no control — each is a literal, a marker, | `userAgent` | empty (= canonical CLI UA) | Literal override; the default is what the gateway expects | | `originClient` | `cli` | Literal `x-opencode-client` value | | `sessionIdEnv` | `OPENCODE_SESSION_ID` | Names an env var only a caller-supplied session id uses | -| `freeModelMarker` | `free` | Model-id substring; `*` forces the fallback, `''` disables it | +| `freeModelMarker` | `free` | Model-id substring; `*` forces the fallback; a blank value inherits the default; catalog-free models always qualify | | `usageBaseURL` | `https://opencode.ai/zen/go/v1` | Endpoint override; auto-discovered from the composition | | `credentialOrg` | — | The org id an account credential belongs to, when the deployment supplies one out of band | | `debug` / `debugFile` | `false` / — | Diagnostic JSONL logging, not a behaviour anyone tunes in the UI; the file is written only while `debug` is on | diff --git a/README.zh-CN.md b/README.zh-CN.md index ea8e086f..b9d1b88b 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -360,7 +360,7 @@ Zen 路线上不出现任何 Go 的数字：根本不画圆环（没有可量的 | `userAgent` | 空（= 规范 CLI UA） | 字面覆盖；默认值就是网关所期望的值 | | `originClient` | `cli` | `x-opencode-client` 的字面值 | | `sessionIdEnv` | `OPENCODE_SESSION_ID` | 指定一个仅当调用方提供会话 id 时才使用的环境变量名 | -| `freeModelMarker` | `free` | 模型 id 子串；`*` 强制启用回退，`''` 关闭回退 | +| `freeModelMarker` | `free` | 模型 id 子串；`*` 强制启用回退；留空则沿用默认值；目录标记为免费的模型始终适用 | | `usageBaseURL` | `https://opencode.ai/zen/go/v1` | 端点覆盖；从组合中自动发现 | | `debug` / `debugFile` | `false` / — | 诊断用 JSONL 日志，不是谁会在 UI 里调节的行为；仅在 `debug` 开启时才写入该文件 | diff --git a/cordis.patch.yml b/cordis.patch.yml index 7e1e7ae7..ca0ce89a 100644 --- a/cordis.patch.yml +++ b/cordis.patch.yml @@ -20,8 +20,12 @@ # model discovery; off leaves both raw (default true). # injectCoreTools: boolean auto-inject read & bash tool schemas on free models (default true). # usageEnabled: boolean enable host-side OpenCode Go usage querying (default true). -# showUsagePrice: boolean show session spend + active model rate in the meter (default true). # keySource: string which credential source wins: auto | request | configured (default 'auto'). +# readOpencodeStore: boolean read the credential an OpenCode installation already holds, as the LAST +# resort after every declared one (default false). +# endpoint: string endpoint family: auto | modern | legacy (default 'auto', an alias of +# modern). modern = the per-protocol inference family; legacy = one base +# per plane. # # Config-only overrides — the card deliberately hides these, because each is a # literal, a marker, or a reference whose default fits every documented setup. @@ -33,10 +37,18 @@ # userAgent: string optional custom User-Agent override (default empty = use OpenCode CLI UA). # originClient: string x-opencode-client value (default 'cli'). # sessionIdEnv: string env var carrying a caller-supplied session id (default 'OPENCODE_SESSION_ID'). -# freeModelMarker: string model-id substring getting the core-tool fallback ('*' = all, '' = none; default 'free'). +# freeModelMarker: string model-id substring getting the core-tool fallback ('*' = all; a blank +# value inherits the default 'free'; models the catalog marks free always +# qualify). # gatewayUrls: [string] URL substrings marking OpenCode gateway traffic (default both # families: ['opencode.ai/zen', 'opencode.ai/inference']). # usageBaseURL: string OpenCode Go gateway base URL (default 'https://opencode.ai/zen/go/v1'). +# credentialOrg: string org id (wrk_...) sent with an account credential that came from elsewhere. +# maxContextWindow: number optional ceiling clamping model context windows (e.g. 131072). +# modelOverrides: object per-model overrides for contextWindow, maxTokens and metadata (default {}). +# modelRoutes: object per-model wire routing: { : { api, baseURL } } (default {}). +# internalRoutes: boolean mount the internal per-protocol routes; off for a deployment that +# declares its own providers for the other protocols (default true). # debug: true|false log every streamed call that receives the header. # debugFile: path optional append target for stream debug JSONL; written only while debug is on. - insert: @@ -58,5 +70,4 @@ injectCoreTools: true enrichModels: true usageEnabled: true - showUsagePrice: true debug: false diff --git a/docs/engineering-notes.md b/docs/engineering-notes.md index deeb841c..8dc1b02d 100644 --- a/docs/engineering-notes.md +++ b/docs/engineering-notes.md @@ -542,6 +542,18 @@ What does NOT work, stated plainly, is the other half. `prepareCall` reads the r The fix is therefore not a mount: it is a route whose adapter the plugin itself owns and whose `models` list carries the catalog's `reasoningEfforts` — the same `models` list the internal routes already use, which works precisely because `configured.length > 0` takes the `entries` from it directly and tolerates ids the installed catalog has never heard of. That means giving the model an SDK so `internalRouteFor` can redirect it, or listing it in the deployment's profile, and neither is something to invent without a measurement of which endpoint the model is actually served on. +## Replacing another plugin's adapter registration is not an API, so the plugin does not try + +`src/adapter-replacement.ts` took a provider route over from the adapter that already served it: it released the llm registry's map entry for a route pi-ai held, registered our adapter through `registerAdapter`, and put the original entry back on disposal. Nothing in `src/` ever imported it, and P1b deleted it with its test. + +It could not have been wired, and the reason is read from the host's llm runtime (`packages/llm/llm/src/index.ts`) rather than assumed: + +1. **`registerAdapter` is all-or-nothing against a private map.** `prepareRoutes` throws `DUPLICATE_ADAPTER` for any route another registration holds (`:440-444`), so the only way in was to delete that registration's entry first. +2. **Only the owner moves its routes.** A registration's routes change through its own `handle.replace`, the method `registerAdapter` returns on its handle (`:421-428`). Nothing else is an API for it. +3. **The owner's disposer deletes what it still believes it holds.** On dispose, the registration deletes every route in its `owned` set and emits `llm/adapters-updated` (`registerAdapter`'s disposer, `:411-416`). An entry deleted from outside leaves pi-ai's registration believing it still owns the route, so its next dispose or restart deletes OUR entry — the plane then answers `NO_ADAPTER` — and its next `replace` registers over us. This failure mode is read from the host's disposer and has not been run. S0's handback failed for a different reason: the prototype released its registration while its own config listener was still live (see the plan's spike record), which is why P-R1 calls its handle and then makes its listener inert in one synchronous section, with no await between them, and restarts pi-ai only after both (the plan's Teardown). + +What the plugin does instead (D13, landing in P-R1 as `src/host/route-claim.ts`): keep pi-ai from claiming `opencode` / `opencode-go` in the first place, through the config waterfall — a prepended `internal/config` listener strips the two plane keys from a copy of the config pi-ai reads — and register its own adapter under the same ids through the ordinary `registerAdapter`. On teardown it releases its own handle and restarts pi-ai, which hands the routes back. It never deletes another plugin's registration; `test/architecture.test.ts` fails the build on a direct `adapters.delete` / `set` / `clear` call (optional chaining included) anywhere in `lifecycle.ts`'s static import graph. + ## A placeholder in a string has no spelling this linter accepts `check.ts` asserts the refresh push carries the PAT by looking for the token URL as a literal, and the literal carries the workflow's own placeholders. Written plainly (`"…${GH_TOKEN}…"`), `no-template-curly-in-string` flags it; written with escapes (`"…\${GH_TOKEN}…"`), `no-useless-escape` flags every escape instead — a plain string has no spelling this oxlint accepts. A regex literal is not an escape hatch either: `prefer-includes` rejects a pattern that is a static string. The one spelling both rules accept is a template literal with `\${`: there the backslash is the documented way to write a literal placeholder, and neither rule applies. diff --git a/docs/improvement-plan.md b/docs/improvement-plan.md index 61178fd8..173018a2 100644 --- a/docs/improvement-plan.md +++ b/docs/improvement-plan.md @@ -20,7 +20,7 @@ Phases land as **stacked pull requests**. Each one is based on the previous phas | :---- | :-------------------------------------------- | :-------- | | P0 | suite hardening, 12 defect fixes, this plan | in review | | P1a | `fix:` shipped breakage | in review | -| P1b | `chore:` hygiene | planned | +| P1b | `chore:` hygiene | in review | | P2 | `feat:` a smaller public API | planned | | P-T1a | `build:` pin the host | planned | | P-T1b | `test:` a harness that is the real host | planned | diff --git a/src/adapter-replacement.ts b/src/adapter-replacement.ts deleted file mode 100644 index 43201074..00000000 --- a/src/adapter-replacement.ts +++ /dev/null @@ -1,120 +0,0 @@ -/** - * Take a provider route over from the adapter that currently serves it. - * - * `llm.registerAdapter` refuses a route that already has an adapter - * (`DUPLICATE_ADAPTER`, `dsh-llm/lib/index.js:273`), which is why this plugin used - * to mount EXTRA routes for the protocols its models speak and redirect calls to - * them. That works, but it costs the thing a user notices: the extra routes are - * providers, so they must be hidden from every listing, and the models have to be - * merged back into the route the user actually configured. - * - * The registry itself is the way out, and it needs no unsupported internals: the - * duplicate check reads `adapters`, so REMOVING the current entry first makes the - * route registrable through the ordinary API. Our registration then carries a - * disposer and fires `llm/adapters-updated`, exactly like one the host made. - * - * @module adapter-replacement - */ - -import type { CordisContext } from "./cordis-context.ts"; - -/** A registration as the registry stores it: the adapter, plus its route. */ -interface RouteRegistration { - readonly adapter: Record; -} - -/** The part of the registry this module reads and writes. */ -interface Registry { - readonly adapters?: Map; - readonly registerAdapter?: ( - providers: readonly string[], - adapter: unknown - ) => unknown; -} - -/** - * Replace the adapter behind `routes` with one this plugin builds. - * - * The swap is two steps and the order matters: release the current entry, then - * register. A failure at either step restores what was there, so a composition - * that cannot take the route over is left exactly as it was rather than with the - * provider missing. - * - * @param ctx - the host context, for the registry and the logging. - * @param routes - the provider routes to take over. - * @param build - builds the adapter that will serve them. - * @returns a disposer that gives the routes back, or `undefined` when the - * composition cannot be taken over. - */ -export const replaceRouteAdapters = ( - ctx: CordisContext, - routes: readonly string[], - build: () => Record -): (() => void) | undefined => { - // The registry is read structurally: `adapters` is a field on the service, not - // part of its declared interface. - const { adapters, registerAdapter } = (ctx.llm ?? {}) as unknown as Registry; - if (!(adapters instanceof Map) || typeof registerAdapter !== "function") { - return undefined; - } - // Both halves are required: an adapter with no routes is refused by the host, - // and this module has nothing to say about a route nobody serves. - const present = routes.filter((route) => adapters.has(route)); - if (present.length === 0 || present.length !== routes.length) { - return undefined; - } - - // What was there, kept so the swap is reversible. Held by value: the registry - // entry is replaced, and the host's own effect still believes it holds it. - const previous = new Map(); - for (const route of present) { - const registration = adapters.get(route); - if (registration === undefined) { - return undefined; - } - previous.set(route, registration); - } - - const restore = (): void => { - for (const [route, registration] of previous) { - adapters.set(route, registration); - } - }; - - let adapter: Record; - try { - adapter = build(); - } catch (error) { - ctx.logger?.warn?.( - "[dsh-opencode-patch] could not build the replacement adapter; leaving the route with its own", - error - ); - return undefined; - } - - // Released only now, so a build failure never leaves the provider unserved. - for (const route of present) { - adapters.delete(route); - } - - let dispose: (() => void) | undefined; - try { - const handle: unknown = registerAdapter(present, adapter); - dispose = typeof handle === "function" ? (handle as () => void) : undefined; - } catch (error) { - restore(); - ctx.logger?.warn?.( - "[dsh-opencode-patch] the replacement adapter was refused; the route keeps its own", - error - ); - return undefined; - } - - return () => { - try { - dispose?.(); - } finally { - restore(); - } - }; -}; diff --git a/src/fetch-patch.ts b/src/fetch-patch.ts index 6ecd614b..e8689da4 100644 --- a/src/fetch-patch.ts +++ b/src/fetch-patch.ts @@ -428,6 +428,9 @@ export const patchFetch = ( const newBody = maybeInjectCoreTools(url, bodyToPatch, headers, { enabled: config.injectCoreTools, modelMarker: config.freeModelMarker, + // The plane is the gateway the request goes to, with the turn's route + // as the fallback off a gateway URL — the credential rule's order. + route: plane.length > 0 ? plane : state?.provider, }); if (newBody !== bodyToPatch) { finalBody = newBody; diff --git a/src/settings-fields.ts b/src/settings-fields.ts index dd6f19bd..78248c82 100644 --- a/src/settings-fields.ts +++ b/src/settings-fields.ts @@ -56,6 +56,12 @@ export const CONFIG_ONLY_FIELDS: readonly { field: "debug", reason: "diagnostic JSONL logging, not a behaviour a user tunes in the UI", }, + { + field: "debugFile", + reason: + "the JSONL path the debug switch appends to; written only while debug " + + "is on, and a server-side path is a deployment fact rather than a card control", + }, { field: "userAgent", reason: diff --git a/src/settings-usage.tsx b/src/settings-usage.tsx index f22023d3..4c2a9757 100644 --- a/src/settings-usage.tsx +++ b/src/settings-usage.tsx @@ -32,7 +32,7 @@ import { import type { GoUsage, UsageWindow } from "./usage-contract.ts"; import { formatRelativeReset, getWindowState, resetLabel } from "./usage-ui.ts"; import { isRecord } from "./utils/guards.ts"; -import { retryUntilAnswered } from "./utils/retry.ts"; +import { backoffOver, retryUntilAnswered } from "./utils/retry.ts"; /** The three windows, in the order the panel lists them. */ const WINDOWS = [ @@ -356,13 +356,32 @@ export interface UsageSummaryProps { * Extracted from the component so the retry is a behaviour a test can perform rather * than a branch inside a React effect. * + * The browser's timer and console are supplied here, at the edge, so + * `utils/retry.ts` stays a function of its arguments. + * * @param read - the read to attempt. * @param attempts - how many times to try before giving up. - * @param sleep - the wait between attempts, called with the attempt number. + * @param sleep - the wait between attempts, called with the attempt number; + * omitted, a linear backoff on the browser's timer. * @param live - asked before each attempt; `false` abandons the loop. * @returns the first defined answer, or `undefined`. */ -export const readUntilAnswered = retryUntilAnswered; +export const readUntilAnswered = ( + read: () => Promise, + attempts = 5, + sleep?: (attempt: number) => Promise, + live: () => boolean = () => true +): Promise => + retryUntilAnswered(read, { + attempts, + live, + sleep: + sleep ?? + backoffOver((callback, delayMs) => setTimeout(callback, delayMs)), + warn: (message, error) => { + console.warn(message, error); + }, + }); /** The container: reads once when the card appears, on window focus, or via manual refresh. */ export const UsageSummary: React.FC = ({ diff --git a/src/stream-hook.ts b/src/stream-hook.ts index d0e4fa44..340dfb95 100644 --- a/src/stream-hook.ts +++ b/src/stream-hook.ts @@ -41,7 +41,6 @@ import { isAsyncIterableLike, isRecord } from "./utils/guards.ts"; interface StreamOptions { messages?: unknown; - prepared?: unknown; model?: unknown; parentId?: unknown; parentSession?: unknown; @@ -197,10 +196,6 @@ export const createStreamHook = ( // nothing about the response has to be rewritten. Returning here rather than // continuing means the redirected call does the session/debug/turn-state // work exactly once — its own pass through this hook. - // - // `prepared` is deliberately dropped. It is bound to the SOURCE route's - // adapter and already-resolved model, so re-resolving on the target route is - // not a loss — it is the only correct thing to do. const redirect = internalRouteFor( providerKey, options.model, @@ -255,7 +250,6 @@ export const createStreamHook = ( if (redirect === GO_GOOGLE_INTERNAL_ROUTE) { const stream = ctx.llm.stream({ ...options, - prepared: undefined, provider: redirect, messages: googleHistoryFor(options.messages, providerKey, redirect), }); diff --git a/src/tool-fallback.ts b/src/tool-fallback.ts index 6fc7acd5..7e8e2d46 100644 --- a/src/tool-fallback.ts +++ b/src/tool-fallback.ts @@ -11,61 +11,12 @@ */ import { ALL_MODELS_MARKER } from "./config-values.ts"; -import { findModelSpec } from "./models-catalog.ts"; +import { catalogPlaneForRoute, findModelSpecOn } from "./models-catalog.ts"; import { isRecord, isUnknownArray } from "./utils/guards.ts"; /** Path segment that marks a Responses-API body worth inspecting. */ export const RESPONSES_PATH = "/responses"; -/** Placeholder `read` schema formatted for Anthropic-compatible /messages. */ -export const DUMMY_READ_TOOL_ANTHROPIC = { - description: "Read a file or directory from the local filesystem.", - input_schema: { - properties: { - filePath: { - description: "The absolute path to the file or directory to read", - type: "string", - }, - offset: { - description: "The line number to start reading from (1-indexed)", - type: "number", - }, - limit: { - description: "The maximum number of lines to read (defaults to 2000)", - type: "number", - }, - }, - required: ["filePath"], - type: "object", - }, - name: "read", -} as const; - -/** Placeholder `bash` schema formatted for Anthropic-compatible /messages. */ -export const DUMMY_BASH_TOOL_ANTHROPIC = { - description: "Execute a bash command.", - input_schema: { - properties: { - command: { - description: "The command to execute", - type: "string", - }, - workdir: { - description: - "The working directory to run the command in. Defaults to the current directory. Use this instead of 'cd' commands.", - type: "string", - }, - timeout: { - description: "Optional timeout in milliseconds", - type: "number", - }, - }, - required: ["command"], - type: "object", - }, - name: "bash", -} as const; - /** Placeholder `read` schema the gateway's validator accepts. */ export const DUMMY_READ_TOOL = { description: "Read a file or directory from the local filesystem.", @@ -197,11 +148,25 @@ const toolNames = ( return { hasBash, hasRead, isOpenAiFormat }; }; -/** Whether `modelId` is in scope for the core-tool fallback. */ -export const isCoreToolModel = (modelId: string, marker: string): boolean => { +/** + * Whether `modelId` is in scope for the core-tool fallback. + * + * @param modelId - the request body's model id. + * @param marker - the configured model-id marker. + * @param route - a route id naming the plane the request is on: the turn's + * provider or the URL's plane id. Go iff it is `opencode-go` or starts with + * `opencode-go-`; anything else, and no route at all, is Zen. + */ +export const isCoreToolModel = ( + modelId: string, + marker: string, + route?: string +): boolean => { // The catalog's free-tier flag is the authority: it catches free models // whose ids carry no marker (big-pickle), where a substring test goes blind. - if (findModelSpec(modelId)?.is_free === true) { + // It is read from the request's own plane: the two planes price the same id + // independently, so a Go-first lookup answered a Zen question with Go data. + if (findModelSpecOn(catalogPlaneForRoute(route), modelId)?.is_free === true) { return true; } if (marker === ALL_MODELS_MARKER) { @@ -220,20 +185,23 @@ export const isCoreToolModel = (modelId: string, marker: string): boolean => { * @param body - the outgoing request body. * @param headers - the mutable header bag whose `content-length` must track a * rewritten body. - * @param options - whether injection is enabled and which model marker scopes it. + * @param options - whether injection is enabled, which model marker scopes it, + * and the route id whose plane's catalog decides whether the model is free. * @returns the (possibly rewritten) body; the original reference when untouched. */ export const maybeInjectCoreTools = ( url: string, body: RequestInit["body"], headers: Headers, - options: { enabled: boolean; modelMarker: string } + options: { enabled: boolean; modelMarker: string; route?: string } ): RequestInit["body"] => { if (!options.enabled) { return body; } // Both JSON routes: the gateway checks read+bash on /responses AND - // /chat/completions (a toolless completions body 403s the same way). + // /chat/completions (a toolless completions body 403s the same way). An + // Anthropic `/messages` URL names neither, so it never gets this far — which + // is why there are no Anthropic-dialect schemas below. const isResponses = url.includes(RESPONSES_PATH); const isCompletions = url.includes("/chat/completions") || url.includes("/completions"); @@ -267,7 +235,7 @@ export const maybeInjectCoreTools = ( if (typeof modelProp !== "string") { return body; } - if (!isCoreToolModel(modelProp, options.modelMarker)) { + if (!isCoreToolModel(modelProp, options.modelMarker, options.route)) { return body; } const toolsProp: unknown = parsed.tools; @@ -282,36 +250,15 @@ export const maybeInjectCoreTools = ( tools = []; } const { hasBash, hasRead, isOpenAiFormat } = toolNames(tools); - const isAnthropic = url.includes("/messages"); - const useOpenAi = - isOpenAiFormat || - url.includes("/chat/completions") || - url.includes("/completions"); - - const getReadTool = () => { - if (isAnthropic) { - return DUMMY_READ_TOOL_ANTHROPIC; - } - if (useOpenAi) { - return DUMMY_READ_TOOL_FUNCTION; - } - return DUMMY_READ_TOOL; - }; - const getBashTool = () => { - if (isAnthropic) { - return DUMMY_BASH_TOOL_ANTHROPIC; - } - if (useOpenAi) { - return DUMMY_BASH_TOOL_FUNCTION; - } - return DUMMY_BASH_TOOL; - }; + const useOpenAi = isOpenAiFormat || isCompletions; + const readTool = useOpenAi ? DUMMY_READ_TOOL_FUNCTION : DUMMY_READ_TOOL; + const bashTool = useOpenAi ? DUMMY_BASH_TOOL_FUNCTION : DUMMY_BASH_TOOL; if (!hasRead) { - tools.push(getReadTool()); + tools.push(readTool); } if (!hasBash) { - tools.push(getBashTool()); + tools.push(bashTool); } parsed.tools = tools; const newBodyStr = JSON.stringify(parsed); diff --git a/src/utils/retry.ts b/src/utils/retry.ts index 11a7c5e9..8dcaca05 100644 --- a/src/utils/retry.ts +++ b/src/utils/retry.ts @@ -2,11 +2,47 @@ * Generic retry loop over an asynchronous operation that may return undefined or throw. * * This is a pure algorithm: it takes the operation, the retry limit, the sleep - * mechanism and an optional liveness probe. No context, no globals, no I/O. + * between attempts, an optional liveness probe and an optional logger as + * parameters. No context, no globals, no I/O — and no clock: the caller owns the + * wait, and {@link backoffOver} builds the usual one over a timer the caller + * supplies. * * @module dsh-opencode-patch/retry */ +/** A `setTimeout`-shaped scheduler: run `callback` after `delayMs`. */ +export type RetryTimer = (callback: () => void, delayMs: number) => unknown; + +/** How {@link retryUntilAnswered} retries, and what it may touch. */ +export interface RetryOptions { + /** Maximum number of attempts (defaults to 5). */ + attempts?: number; + /** Polled before each attempt; returning false aborts. */ + live?: () => boolean; + /** + * Takes the attempt index (0-based) and resolves after the delay. Required, + * because a utility has no clock of its own: pass `backoffOver(timer)` for the + * default schedule. + */ + sleep: (attempt: number) => Promise; + /** Told when every attempt failed with an error; omitted means silent. */ + warn?: (message: string, error: unknown) => void; +} + +/** + * The usual schedule: a linear backoff of `baseMs` per attempt, on `timer`. + * + * @param timer - the scheduler the delay runs on. + * @param baseMs - the first delay; attempt `n` waits `baseMs * (n + 1)`. + * @returns a `sleep` for {@link RetryOptions}. + */ +export const backoffOver = + (timer: RetryTimer, baseMs = 300) => + (attempt: number): Promise => + new Promise((resolve) => { + timer(resolve, baseMs * (attempt + 1)); + }); + /** * Attempt to read a defined value until an answer arrives or attempts are exhausted. * @@ -18,20 +54,17 @@ * - If all attempts are exhausted, it returns `undefined`. * * @param read - the async reader operation. - * @param attempts - maximum number of attempts (defaults to 5). - * @param sleep - callback taking attempt index (0-based) and returning a sleep Promise. - * @param live - optional predicate polled before each attempt; returning false aborts. + * @param options - the attempt limit, the sleep, the liveness probe and the + * logger, all owned by the caller. * @returns the first defined answer, or `undefined` if unfulfilled. */ export const retryUntilAnswered = async ( read: () => Promise, - attempts = 5, - sleep: (attempt: number) => Promise = (attempt) => - new Promise((resolve) => { - setTimeout(resolve, 300 * (attempt + 1)); - }), - live: () => boolean = () => true + options: RetryOptions ): Promise => { + const attempts = options.attempts ?? 5; + const live = options.live ?? (() => true); + const { sleep } = options; let lastError: unknown; for (let attempt = 0; attempt < attempts; attempt += 1) { if (!live()) { @@ -52,7 +85,10 @@ export const retryUntilAnswered = async ( } } if (lastError !== undefined) { - console.warn("[dsh-opencode-patch] retry: all attempts failed", lastError); + options.warn?.( + "[dsh-opencode-patch] retry: all attempts failed", + lastError + ); } return undefined; }; diff --git a/test/adapter-replacement.test.ts b/test/adapter-replacement.test.ts deleted file mode 100644 index d83ecada..00000000 --- a/test/adapter-replacement.test.ts +++ /dev/null @@ -1,214 +0,0 @@ -/** - * `replaceRouteAdapters` — taking a provider route over from the adapter that - * currently serves it. - * - * The duplicate check in `dsh-llm` reads the registry map, so releasing the entry - * first makes the route registrable through the ordinary API. These cases pin the - * three things that must hold for that swap to be safe: the original is restored on - * disposal, a failed build leaves it untouched, and a refusal does not strand the - * provider. - */ -import { describe, expect, it, vi } from "vitest"; - -import { replaceRouteAdapters } from "../src/adapter-replacement.ts"; -import type { CordisContext } from "../src/index.ts"; - -interface FakeLlm { - adapters: Map }>; - registerAdapter: ( - providers: readonly string[], - adapter: unknown - ) => (() => void) | undefined; -} - -/** A registry with one built-in route, and the events a swap would cause. */ -const registryWith = (route: string) => { - const original = { adapter: { name: "builtin" } as Record }; - const adapters = new Map([[route, original]]); - const events: string[] = []; - const llm: FakeLlm = { - adapters, - registerAdapter: (providers, adapter) => { - for (const provider of providers) { - if (adapters.has(provider)) { - throw new Error("DUPLICATE_ADAPTER"); - } - } - for (const provider of providers) { - adapters.set(provider, { adapter: adapter as Record }); - } - events.push(`registered:${providers.join(",")}`); - return () => { - for (const provider of providers) { - adapters.delete(provider); - } - events.push(`disposed:${providers.join(",")}`); - }; - }, - }; - return { adapters, events, llm, original }; -}; - -describe("replaceRouteAdapters", () => { - it("releases the route, registers ours, and restores the original after", () => { - const { adapters, events, llm, original } = registryWith("opencode"); - const ctx = { llm, logger: { warn: () => {} } } as unknown as CordisContext; - const ours = { name: "ours" }; - - const stop = replaceRouteAdapters(ctx, ["opencode"], () => ours); - - expect(stop).toBeTypeOf("function"); - expect(adapters.get("opencode")?.adapter).toBe(ours); - expect(events).toEqual(["registered:opencode"]); - - stop?.(); - expect(adapters.get("opencode")).toBe(original); - expect(events).toEqual(["registered:opencode", "disposed:opencode"]); - }); - - it("does nothing when the route is not registered", () => { - // A composition without the provider has no route to take over, and inventing - // one would give the user a provider the deployment never declared. - const { adapters, events, llm } = registryWith("somewhere-else"); - const ctx = { llm, logger: { warn: () => {} } } as unknown as CordisContext; - expect( - replaceRouteAdapters(ctx, ["opencode"], () => ({ name: "ours" })) - ).toBeUndefined(); - expect(adapters.has("opencode")).toBe(false); - expect(events).toEqual([]); - }); - - it("leaves the route with its own adapter when the build fails", () => { - const { adapters, llm, original } = registryWith("opencode"); - const warn = vi.fn(); - const ctx = { llm, logger: { warn } } as unknown as CordisContext; - const stop = replaceRouteAdapters(ctx, ["opencode"], () => { - throw new Error("no pi-ai here"); - }); - expect(stop).toBeUndefined(); - expect(adapters.get("opencode")).toBe(original); - expect(warn).toHaveBeenCalled(); - }); - - it("restores the original when the host refuses the registration", () => { - const { adapters, llm, original } = registryWith("opencode"); - // A refusal AFTER the release: the provider would otherwise be left unserved. - llm.registerAdapter = () => { - throw new Error("refused"); - }; - const warn = vi.fn(); - const ctx = { llm, logger: { warn } } as unknown as CordisContext; - expect( - replaceRouteAdapters(ctx, ["opencode"], () => ({ name: "ours" })) - ).toBeUndefined(); - expect(adapters.get("opencode")).toBe(original); - expect(warn).toHaveBeenCalled(); - }); -}); - -describe("replaceRouteAdapters: the shapes it declines", () => { - const ctxWith = (llm: unknown) => - ({ llm, logger: { warn: vi.fn() } }) as unknown as CordisContext; - - it("declines a registry without a usable map or registerAdapter", () => { - // Both are read structurally off the service, so a composition that exposes - // one and not the other must answer "cannot take over" rather than throwing - // halfway through a swap. - expect( - replaceRouteAdapters(ctxWith({}), ["opencode"], () => ({})) - ).toBeUndefined(); - expect( - replaceRouteAdapters( - ctxWith({ adapters: {}, registerAdapter: () => {} }), - ["opencode"], - () => ({}) - ) - ).toBeUndefined(); - expect( - replaceRouteAdapters( - ctxWith({ adapters: new Map(), registerAdapter: "nope" }), - ["opencode"], - () => ({}) - ) - ).toBeUndefined(); - // A context with no `llm` at all is the same answer. - expect( - replaceRouteAdapters({} as CordisContext, ["opencode"], () => ({})) - ).toBeUndefined(); - }); - - it("declines when any route of the group is unserved", () => { - // All-or-nothing: replacing half a protocol group would leave the other - // half on the host's adapter, and the two would disagree about the route. - const { llm } = registryWith("opencode"); - expect( - replaceRouteAdapters(ctxWith(llm), ["opencode", "opencode-go"], () => ({ - name: "ours", - })) - ).toBeUndefined(); - }); - - it("declines an empty route list", () => { - const { llm } = registryWith("opencode"); - expect(replaceRouteAdapters(ctxWith(llm), [], () => ({}))).toBeUndefined(); - }); - - it("gives the routes back when the host returns no disposer", () => { - // `registerAdapter` answers a callable or nothing. A composition that - // answers nothing still has to be reversible, or the route is stranded on - // our adapter with no way back. - const original = { - adapter: { name: "builtin" } as Record, - }; - const adapters = new Map([["opencode", original]]); - const llm = { - adapters, - registerAdapter: (providers: readonly string[], adapter: unknown) => { - for (const provider of providers) { - adapters.set(provider, { - adapter: adapter as Record, - }); - } - // Falls off the end: one of the two shapes the host answers with is no - // disposer at all, and a composition that answers that still has to be - // reversible. - }, - }; - const dispose = replaceRouteAdapters(ctxWith(llm), ["opencode"], () => ({ - name: "ours", - })); - expect(dispose).toBeDefined(); - // Ours is in place... - expect(adapters.get("opencode")?.adapter.name).toBe("ours"); - dispose?.(); - // ...and the original is back even though the host gave us no disposer. - expect(adapters.get("opencode")).toBe(original); - }); - - it("restores the original even if its own disposer throws", () => { - // Disposal runs inside a `finally`, so a throwing disposer cannot leave the - // route without its adapter. - const original = { - adapter: { name: "builtin" } as Record, - }; - const adapters = new Map([["opencode", original]]); - const llm = { - adapters, - registerAdapter: (providers: readonly string[], adapter: unknown) => { - for (const provider of providers) { - adapters.set(provider, { - adapter: adapter as Record, - }); - } - return () => { - throw new Error("dispose exploded"); - }; - }, - }; - const dispose = replaceRouteAdapters(ctxWith(llm), ["opencode"], () => ({ - name: "ours", - })); - expect(() => dispose?.()).toThrow("dispose exploded"); - expect(adapters.get("opencode")).toBe(original); - }); -}); diff --git a/test/architecture.test.ts b/test/architecture.test.ts index 802dc2db..cfd06fd5 100644 --- a/test/architecture.test.ts +++ b/test/architecture.test.ts @@ -13,18 +13,38 @@ * * @module test/architecture.test */ -import { readdirSync, readFileSync } from "node:fs"; -import { join } from "node:path"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { join, posix } from "node:path"; import { describe, expect, it } from "vitest"; -const UTILS_DIR = join(import.meta.dirname, "..", "src", "utils"); +const SRC_DIR = join(import.meta.dirname, "..", "src"); +const UTILS_DIR = join(SRC_DIR, "utils"); -/** Every `.ts` file directly inside `src/utils/`. */ -const utilityFiles = (): string[] => - readdirSync(UTILS_DIR) - .filter((name) => name.endsWith(".ts")) - .toSorted(); +/** + * Every `.ts`/`.tsx` file under `dir`, at any depth, relative to `dir`. + * + * Recursive on purpose: a utility in a subfolder is still a utility, and a walk + * that read one level would let `utils/nested/` hold anything at all. + */ +const filesUnder = (dir: string): string[] => { + const files: string[] = []; + const walk = (current: string): void => { + for (const entry of readdirSync(current, { withFileTypes: true })) { + const full = join(current, entry.name); + if (entry.isDirectory()) { + walk(full); + } else if (/\.tsx?$/u.test(entry.name)) { + files.push(full.slice(dir.length + 1).replaceAll("\\", "/")); + } + } + }; + walk(dir); + return files.toSorted(); +}; + +/** Every utility, at any depth under `src/utils/`. */ +const utilityFiles = (): string[] => filesUnder(UTILS_DIR); /** The source with `//` and block comments removed, keeping line structure. */ const stripComments = (source: string): string => @@ -33,6 +53,44 @@ const stripComments = (source: string): string => .replaceAll(/^\s*\/\/.*$/gmu, "") .replaceAll(/\/\/.*$/gmu, ""); +/** + * Runtime imports of one file. A type-only import is erased by the compiler, + * so it is no edge in the bundle and is skipped; a mixed one is a real edge. + */ +const runtimeImportsOf = (source: string): string[] => { + const specifiers: string[] = []; + for (const match of source.matchAll( + /\b(?:import|export)\s+(type\s+)?(?:[^;"']*?\sfrom\s+)?"([^"]+)"/gu + )) { + if (match[1] === undefined && match[2] !== undefined) { + specifiers.push(match[2]); + } + } + return specifiers; +}; + +/** + * Every `src/` file `entry` reaches at runtime, entry included, mapped to its + * source with comments stripped. + */ +const runtimeGraph = (entry: string): Map => { + const seen = new Map(); + const visit = (file: string): void => { + if (seen.has(file)) { + return; + } + const source = stripComments(readFileSync(join(SRC_DIR, file), "utf8")); + seen.set(file, source); + for (const specifier of runtimeImportsOf(source)) { + if (specifier.startsWith("./") || specifier.startsWith("../")) { + visit(join(file, "..", specifier).replaceAll("\\", "/")); + } + } + }; + visit(entry); + return seen; +}; + /** The module specifiers one file imports. */ const importsOf = (source: string): string[] => { const specifiers: string[] = []; @@ -63,13 +121,29 @@ const FORBIDDEN: readonly { "a cordis context is the world: a utility takes its inputs as arguments instead", matches: (s) => s.startsWith("@deepseek-ai/cordis"), }, - { - because: - "only guards.ts and other utilities may be imported; anything else is an effect module", - matches: (s) => s.startsWith("../") || s.startsWith("./../"), - }, ]; +/** + * Why a relative import from the utility at `file` (a path relative to + * `src/utils/`) leaves the folder, or `undefined` when it stays inside. + * + * The specifier is resolved against the importing file's own directory, so a + * nested utility's `../guards.ts` is a sibling, not an escape. Comparing the raw + * specifier would flag every `../` — which, once the walk is recursive, rejects + * exactly the nested utilities the recursion admits. + */ +const escapesUtils = (file: string, specifier: string): string | undefined => { + if (!specifier.startsWith("./") && !specifier.startsWith("../")) { + return undefined; + } + // Resolved under a root named for the folder, so a path that leaves and + // re-enters it (`../../utils/guards.ts`) is judged by where it lands. + const target = posix.join("/utils", posix.dirname(file), specifier); + return target.startsWith("/utils/") + ? undefined + : "only guards.ts and other utilities may be imported; anything else is an effect module"; +}; + describe("architecture · a property read is a declared dependency", () => { const SRC = join(import.meta.dirname, "..", "src"); @@ -133,21 +207,7 @@ describe("architecture · a property read is a declared dependency", () => { return [...names]; }; - const sourceFiles = (): string[] => { - const files: string[] = []; - const walk = (dir: string): void => { - for (const entry of readdirSync(dir, { withFileTypes: true })) { - const full = join(dir, entry.name); - if (entry.isDirectory()) { - walk(full); - } else if (/\.tsx?$/u.test(entry.name)) { - files.push(full.slice(SRC.length + 1)); - } - } - }; - walk(SRC); - return files; - }; + const sourceFiles = (): string[] => filesUnder(SRC); it("reads no host service as a property except the declared `llm`", () => { // Cordis THROWS `cannot get property "X" without inject` for an undeclared @@ -170,7 +230,7 @@ describe("architecture · a property read is a declared dependency", () => { }); it("declares `llm`, the one service the plugin cannot work without", async () => { - const { inject } = await import("../src/identity.ts"); + const { inject } = await import("./support/src.ts"); expect(inject).toContain("llm"); }); }); @@ -217,21 +277,38 @@ describe("architecture · utils/ holds only pure functions", () => { for (const file of utilityFiles()) { const source = readFileSync(join(UTILS_DIR, file), "utf8"); for (const specifier of importsOf(source)) { - // `guards.ts` is dependency-free by construction and is the one - // non-utility a utility may share. - if (specifier === "./guards.ts") { - continue; - } for (const rule of FORBIDDEN) { if (rule.matches(specifier)) { violations.push(`${file} imports "${specifier}" — ${rule.because}`); } } + // `guards.ts` lives in utils/ and is dependency-free by construction, + // so it resolves inside the folder like any other utility. + const escape = escapesUtils(file, specifier); + if (escape !== undefined) { + violations.push(`${file} imports "${specifier}" — ${escape}`); + } } } expect(violations).toEqual([]); }); + it("resolves an import from the importing file's folder, at any depth", () => { + // Known answers, so the rule is seen to allow and to reject. + expect(escapesUtils("retry.ts", "./guards.ts")).toBeUndefined(); + expect(escapesUtils("nested/a.ts", "../guards.ts")).toBeUndefined(); + expect(escapesUtils("nested/a.ts", "./b.ts")).toBeUndefined(); + expect(escapesUtils("nested/a.ts", "../retry.ts")).toBeUndefined(); + expect(escapesUtils("retry.ts", "../config.ts")).toBeDefined(); + expect(escapesUtils("retry.ts", "./../config.ts")).toBeDefined(); + expect(escapesUtils("nested/a.ts", "../../config.ts")).toBeDefined(); + expect( + escapesUtils("nested/a.ts", "../../utils/guards.ts") + ).toBeUndefined(); + expect(escapesUtils("retry.ts", "../utils/guards.ts")).toBeUndefined(); + expect(escapesUtils("nested/a.ts", "../../utilsx/a.ts")).toBeDefined(); + }); + it("never reads process.env, a global the arguments could have carried", () => { // Comments are stripped first: prose ABOUT the environment is exactly what // a rule explaining itself looks like, and a rule that flags its own @@ -253,47 +330,88 @@ describe("architecture · utils/ holds only pure functions", () => { }); expect(violations).toEqual([]); }); -}); - -describe("architecture · the client bundle reaches nothing host-only", () => { - const SRC = join(import.meta.dirname, "..", "src"); - /** The client bundle's entry, as `vite.config.ts` names it. */ - const ENTRY = "settings-page.tsx"; /** - * Runtime imports of one file. A type-only import is erased by the compiler, - * so it is no edge in the bundle and is skipped; a mixed one is a real edge. + * A reference to the console or to a global timer, outside a comment — not + * only a call: `timer = setTimeout` as a default parameter, `backoffOver(setTimeout)` + * and `const log = console.warn` reach the global just as surely, and so do + * the four spellings of the global object (`globalThis.`, `window.`, + * `global.`, `self.`). A member of something else (`options.timer(`, + * `x.setTimeout(`) is not the global. + * + * A tripwire for the honest spelling, not a sandbox: an alias + * (`const g = globalThis; g.setTimeout(…)`) or bracket access on the global + * object gets past it, and catching those is a type-aware lint's job. */ - const runtimeImportsOf = (source: string): string[] => { - const specifiers: string[] = []; - for (const match of source.matchAll( - /\b(?:import|export)\s+(type\s+)?(?:[^;"']*?\sfrom\s+)?"([^"]+)"/gu - )) { - if (match[1] === undefined && match[2] !== undefined) { - specifiers.push(match[2]); + const AMBIENT_EFFECT = + /(? { + // A log line is output and a timer is a clock: both are the world, and a + // utility that needs one takes it as a parameter, the way `retry.ts` takes + // its `sleep` and its `warn` and leaves the timer to the caller. + const violations: string[] = []; + for (const file of utilityFiles()) { + // Block comments are blanked rather than removed, so a line number in a + // violation is the line in the file. + const source = stripComments( + readFileSync(join(UTILS_DIR, file), "utf8").replaceAll( + /\/\*[\s\S]*?\*\//gu, + (comment) => comment.replaceAll(/[^\n]/gu, "") + ) + ); + for (const [index, line] of source.split("\n").entries()) { + const match = AMBIENT_EFFECT.exec(line); + if (match !== null) { + violations.push( + `${file}:${index + 1} — ${match[0].trim()} is an effect the caller owns` + ); + } } } - return specifiers; - }; + expect(violations).toEqual([]); + }); + + it("knows a console call and a timer when it sees one, and walks every level", () => { + // Known answers, so an empty violation list means the rule ran. + expect(AMBIENT_EFFECT.test('console.warn("x")')).toBe(true); + expect(AMBIENT_EFFECT.test("setTimeout(f, 1)")).toBe(true); + expect(AMBIENT_EFFECT.test("globalThis.setInterval(f, 1)")).toBe(true); + expect(AMBIENT_EFFECT.test("global.setTimeout(f, 1)")).toBe(true); + expect(AMBIENT_EFFECT.test("self.console.warn(x)")).toBe(true); + expect(AMBIENT_EFFECT.test("setImmediate(f)")).toBe(true); + expect(AMBIENT_EFFECT.test("clearTimeout(handle)")).toBe(true); + expect( + AMBIENT_EFFECT.test("(timer: RetryTimer = setTimeout, baseMs = 300) =>") + ).toBe(true); + expect(AMBIENT_EFFECT.test("const sleep = backoffOver(setTimeout);")).toBe( + true + ); + expect(AMBIENT_EFFECT.test("const log = console.warn;")).toBe(true); + expect(AMBIENT_EFFECT.test("const { warn } = console;")).toBe(true); + expect(AMBIENT_EFFECT.test('console["warn"](x)')).toBe(true); + expect(AMBIENT_EFFECT.test("(options.warn ?? console.warn)(m, e)")).toBe( + true + ); + expect(AMBIENT_EFFECT.test("options.timer(resolve, ms)")).toBe(false); + expect(AMBIENT_EFFECT.test("x.setTimeout(f, 1)")).toBe(false); + expect(AMBIENT_EFFECT.test("const consoleLike = 1;")).toBe(false); + expect( + AMBIENT_EFFECT.test( + "type RetryTimer = (callback: () => void, delayMs: number) => unknown" + ) + ).toBe(false); + expect(utilityFiles()).toContain("retry.ts"); + expect(filesUnder(SRC_DIR)).toContain("utils/retry.ts"); + }); +}); + +describe("architecture · the client bundle reaches nothing host-only", () => { + /** The client bundle's entry, as `vite.config.ts` names it. */ + const ENTRY = "settings-page.tsx"; /** Every `src/` file the entry reaches at runtime, entry included. */ - const clientGraph = (): Map => { - const seen = new Map(); - const visit = (file: string): void => { - if (seen.has(file)) { - return; - } - const source = stripComments(readFileSync(join(SRC, file), "utf8")); - seen.set(file, source); - for (const specifier of runtimeImportsOf(source)) { - if (specifier.startsWith("./") || specifier.startsWith("../")) { - visit(join(file, "..", specifier).replaceAll("\\", "/")); - } - } - }; - visit(ENTRY); - return seen; - }; + const clientGraph = (): Map => runtimeGraph(ENTRY); /** What a client module may not import, each with the reason it is host-only. */ const HOST_ONLY: readonly { @@ -357,3 +475,114 @@ describe("architecture · the client bundle reaches nothing host-only", () => { expect(HOST_ONLY.some((rule) => rule.matches("node:fs"))).toBe(true); }); }); + +describe("architecture · the host bundle mutates no registry it does not own", () => { + /** The host half's composition root: everything `apply()` can run. */ + const ENTRY = "lifecycle.ts"; + + /** A write to an `adapters` map: the llm registry's private table. */ + const REGISTRY_WRITE = /\badapters\??\.(?:delete|set|clear)\s*\(/u; + + it("walks a graph that actually contains the host modules", () => { + const graph = runtimeGraph(ENTRY); + expect(graph.has("stream-hook.ts")).toBe(true); + expect(graph.has("responses-provider.ts")).toBe(true); + expect(graph.has("settings-page.tsx")).toBe(false); + expect(REGISTRY_WRITE.test('llm.adapters.delete("opencode")')).toBe(true); + expect(REGISTRY_WRITE.test('llm.adapters?.delete("opencode")')).toBe(true); + expect(REGISTRY_WRITE.test("this.adapters.get(route)")).toBe(false); + }); + + it("nothing reachable from lifecycle.ts deletes or sets an llm adapters entry", () => { + // `registerAdapter` is all-or-nothing and only the owner's handle moves a + // registration's routes. An entry deleted from outside leaves the owner + // believing it holds the route, and its next dispose deletes ours. The + // plugin keeps pi-ai off the planes through the config waterfall instead + // (docs/engineering-notes.md, "Replacing another plugin's adapter + // registration is not an API"). + const violations = [...runtimeGraph(ENTRY)] + .filter(([, source]) => REGISTRY_WRITE.test(source)) + .map(([file]) => file); + expect(violations).toEqual([]); + }); +}); + +describe("architecture · tests reach src through one door", () => { + const SUPPORT = join(import.meta.dirname, "support", "src.ts"); + + /** + * Whether `source` DECLARES `name` — an `export const|function|class|…` + * of it — rather than passing it on with `export { name } from "…"`. + */ + const declares = (source: string, name: string): boolean => + new RegExp( + String.raw`export (?:declare )?(?:abstract )?(?:async )?(?:const|let|function\*?|class|interface|type|enum) ${name}\b`, + "u" + ).test(source); + + /** Each `export { names } from "../../src/"` block, names unaliased. */ + const reExportBlocks = ( + source: string + ): { readonly target: string; readonly names: readonly string[] }[] => + [ + ...source.matchAll( + /^export (?:type )?\{([^}]*)\} from "\.\.\/\.\.\/src\/([^"]+)"/gmu + ), + ].map((match) => ({ + names: (match[1] ?? "") + .split(",") + .map((entry) => + entry + .trim() + .replace(/^type\s+/u, "") + .replace(/\s+as\s+[\w$]+$/u, "") + ) + .filter((entry) => entry !== ""), + target: match[2] ?? "", + })); + + it("knows a declaration from a pass-through when it sees one", () => { + // Known answers, so an empty miss list means the rule ran. + const config = readFileSync(join(SRC_DIR, "config.ts"), "utf8"); + expect(declares(config, "Config")).toBe(true); + expect(declares('export { Config } from "./config.ts";', "Config")).toBe( + false + ); + expect(declares("export type ConfigLike = 1;", "Config")).toBe(false); + expect( + reExportBlocks( + 'export { a, type B, c as d } from "../../src/x.ts";\nexport type { E } from "../../src/y.ts";' + ) + ).toEqual([ + { names: ["a", "B", "c"], target: "x.ts" }, + { names: ["E"], target: "y.ts" }, + ]); + }); + + it("test/support/src.ts re-exports each name from the module that declares it", () => { + // Re-exporting through `src/index.ts` would tie every test to the barrel + // P2 shrinks, which is the coupling this file exists to remove; and a name + // taken from a module that only passes it on breaks when P10a moves the + // module that really defines it. + const source = readFileSync(SUPPORT, "utf8"); + expect(source).not.toMatch(/src\/index\.ts/u); + const blocks = reExportBlocks(source); + // Every `from "../../src/…"` in the file is one of the parsed blocks, and + // there is one block per module, so the walk below is not vacuous. + expect(blocks.length).toBe( + source.match(/from "\.\.\/\.\.\/src\//gu)?.length ?? 0 + ); + expect(blocks.length).toBeGreaterThanOrEqual(40); + const missing = blocks + .map(({ target }) => target) + .filter((target) => !existsSync(join(SRC_DIR, target))); + expect(missing).toEqual([]); + const notDeclared = blocks.flatMap(({ names, target }) => { + const defining = readFileSync(join(SRC_DIR, target), "utf8"); + return names + .filter((name) => !declares(defining, name)) + .map((name) => `${name} is not declared in src/${target}`); + }); + expect(notDeclared).toEqual([]); + }); +}); diff --git a/test/fetch-patch.test.ts b/test/fetch-patch.test.ts index 1fe06d7f..4fb042d8 100644 --- a/test/fetch-patch.test.ts +++ b/test/fetch-patch.test.ts @@ -10,6 +10,12 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + GO_FREE_PROBE, + seedDivergentCatalog, + ZEN_FREE_PROBE, +} from "./support/catalog-fixture.ts"; +import { createOpencodeStore } from "./support/opencode-store-fixture.ts"; import { DUMMY_BASH_TOOL, DUMMY_READ_TOOL, @@ -24,8 +30,7 @@ import { patchFetch, resolveConfig, type ActiveTurnState, -} from "../src/index.ts"; -import { createOpencodeStore } from "./support/opencode-store-fixture.ts"; +} from "./support/src.ts"; import { createCaptureFetch, headerOf, @@ -805,6 +810,98 @@ describe("patchFetch", () => { expect(names).toEqual([DUMMY_READ_TOOL.name, DUMMY_BASH_TOOL.name]); }); + it("classifies a free model by the plane the request is on", async () => { + // The catalog's free flag is per plane, and the two planes can disagree. + // The seeded probe is free on Zen and paid on Go, under a marker nothing + // matches, so only the plane's own row can admit it: a Zen request gains + // the schemas, the same body on the Go plane is left alone. + await seedDivergentCatalog(); + const als = new AsyncLocalStorage(); + const config = resolveConfig({ freeModelMarker: "none-of-these" }); + const send = async ( + provider: string, + url: string, + model = ZEN_FREE_PROBE + ): Promise => { + const { capture, mockFetch } = createCaptureFetch(); + const value = openCodeSessionIdFor(`plane-${provider}`); + await als.run({ provider, value }, async () => { + await patchFetch( + mockFetch, + als, + config + )(url, { + body: JSON.stringify({ input: "hi", model }), + headers: { "Content-Type": "application/json" }, + method: "POST", + }); + }); + return toolNamesOf(parseJsonBody(capture.init?.body)); + }; + + expect( + await send("opencode", "https://opencode.ai/zen/v1/responses") + ).toEqual(["read", "bash"]); + expect( + await send("opencode-go", "https://opencode.ai/zen/go/v1/responses") + ).toBeUndefined(); + // The reverse probe, so a Go-first lookup is red in both directions. + expect( + await send( + "opencode-go", + "https://opencode.ai/zen/go/v1/responses", + GO_FREE_PROBE + ) + ).toEqual(["read", "bash"]); + expect( + await send( + "opencode", + "https://opencode.ai/zen/v1/responses", + GO_FREE_PROBE + ) + ).toBeUndefined(); + }); + + it("reads the plane from the URL first and the turn's route off a gateway", async () => { + // `route` has two arms. A gateway URL names its plane and wins over the + // turn's provider; a URL that names no plane (a relay the turn's provider + // claims) falls back to the turn's route. + await seedDivergentCatalog(); + const als = new AsyncLocalStorage(); + const config = resolveConfig({ freeModelMarker: "none-of-these" }); + const send = async (provider: string, url: string): Promise => { + const { capture, mockFetch } = createCaptureFetch(); + const value = openCodeSessionIdFor(`route-${provider}-${url}`); + await als.run({ provider, value }, async () => { + await patchFetch( + mockFetch, + als, + config + )(url, { + body: JSON.stringify({ input: "hi", model: GO_FREE_PROBE }), + headers: { "Content-Type": "application/json" }, + method: "POST", + }); + }); + return toolNamesOf(parseJsonBody(capture.init?.body)); + }; + + // Off a gateway: the turn's route decides. + expect( + await send("opencode-go", "https://relay.example/v1/responses") + ).toEqual(["read", "bash"]); + expect( + await send("opencode", "https://relay.example/v1/responses") + ).toBeUndefined(); + // On a gateway: the URL's plane decides, whatever the turn says. + expect( + await send("opencode", "https://opencode.ai/zen/go/v1/responses") + ).toEqual(["read", "bash"]); + expect( + await send("opencode-go", "https://opencode.ai/zen/v1/responses") + ).toBeUndefined(); + }); + it("skips tool injection for paid models, other paths, and invalid JSON", async () => { const als = new AsyncLocalStorage(); const testSession = openCodeSessionIdFor("skip-cases"); diff --git a/test/free-tier-contract.test.ts b/test/free-tier-contract.test.ts index 804a8176..191f6309 100644 --- a/test/free-tier-contract.test.ts +++ b/test/free-tier-contract.test.ts @@ -13,9 +13,11 @@ import { describe, expect, it } from "vitest"; -import { OPENCODE_ZEN_CATALOG } from "../src/catalog-data.ts"; -import { findModelSpec } from "../src/models-catalog.ts"; -import { maybeInjectCoreTools } from "../src/tool-fallback.ts"; +import { + findModelSpecOn, + maybeInjectCoreTools, + OPENCODE_ZEN_CATALOG, +} from "./support/src.ts"; const ROUTES = [ "https://opencode.ai/zen/v1/chat/completions", @@ -29,14 +31,16 @@ const freeModelIds = (): string[] => { ids.add(spec.id); } } - // findModelSpec covers ids the live map keys differently (prefixes). + // The live map keys some ids differently (prefixes). The lookup reads the + // Zen plane, the plane every route below posts to and the one the injector + // reads with no route, so the set and the injector agree on what is free. for (const id of [ "mimo-v2.6-flash-free", "big-pickle", "beam-free", "muse-spark-1.2-contributor-free", ]) { - if (findModelSpec(id)?.is_free === true) { + if (findModelSpecOn("zen", id)?.is_free === true) { ids.add(id); } } @@ -123,6 +127,6 @@ describe("free-tier contract", () => { } } it("big-pickle is free by catalog flag, not by marker", () => { - expect(findModelSpec("big-pickle")?.is_free).toBe(true); + expect(findModelSpecOn("zen", "big-pickle")?.is_free).toBe(true); }); }); diff --git a/test/manifest.test.ts b/test/manifest.test.ts index 7c8b284d..73c590d0 100644 --- a/test/manifest.test.ts +++ b/test/manifest.test.ts @@ -11,7 +11,14 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { DEFAULT_PROVIDERS, name as PLUGIN_NAME } from "../src/index.ts"; +import { + CARD_FIELDS, + Config, + CONFIG_ONLY_FIELDS, + DEFAULT_PROVIDERS, + name as PLUGIN_NAME, + NS, +} from "./support/src.ts"; import { isRecord } from "./test-helpers.ts"; afterEach(() => { @@ -45,15 +52,8 @@ describe("bundle manifest consistency", () => { it("keeps the settings namespace equal to the cordis row id", async () => { const patch = await readText("cordis.patch.yml"); expect(patch).toContain("id: dsh-opencode-patch"); - // Read the NS constant textually: importing settings-page.tsx would - // drag the React + ui-primitives runtime chain (whose own deps are - // incomplete for node) into a hermetic suite. - const page = await readText("src/settings-page.tsx"); - const ns = /^export const NS = "([^"]+)";/m.exec(page); - if (ns === null || ns[1] === undefined) { - throw new Error("NS constant not found in src/settings-page.tsx"); - } - expect(ns[1]).toBe("dsh-opencode-patch"); + // The constant the client bundle ships, not its spelling in the source. + expect(NS).toBe("dsh-opencode-patch"); }); it("claims every route the layer mounts, in the shipped row", async () => { @@ -107,3 +107,94 @@ describe("bundle manifest consistency", () => { expect(banner[1]).toBe(pkgRaw.name); }); }); + +/** + * The keys `cordis.patch.yml` documents in its header and sets in its row. + * + * The header lists one key per line as `# key: type description`, with + * continuation lines indented deeper, so a key is exactly three spaces after + * the `#`. The card list runs from the line naming `CARD_FIELDS` to the one + * naming `CONFIG_ONLY_FIELDS`, the config-only list from there to the row. The + * row's keys sit eight spaces in under `config:`; its list items sit ten in and + * start with `-`, so they never read as keys. + */ +const manifestKeys = ( + text: string +): { card: string[]; configOnly: string[]; set: string[] } => { + const lines = text.split("\n"); + const insert = lines.findIndex((line) => line.startsWith("- insert:")); + const cardStart = lines.findIndex((line) => line.includes("CARD_FIELDS")); + const configOnlyStart = lines.findIndex((line) => + line.includes("CONFIG_ONLY_FIELDS") + ); + const configStart = lines.findIndex((line) => /^ +config:\s*$/u.test(line)); + if ( + insert === -1 || + cardStart === -1 || + configOnlyStart <= cardStart || + insert <= configOnlyStart || + configStart <= insert + ) { + throw new Error( + "cordis.patch.yml no longer has the layout this test reads" + ); + } + const keysOf = (slice: string[], pattern: RegExp): string[] => + slice + .map((line) => pattern.exec(line)?.[1]) + .filter((key): key is string => key !== undefined); + const documented = /^# {3}([A-Za-z]\w*):/u; + return { + card: keysOf(lines.slice(cardStart, configOnlyStart), documented), + configOnly: keysOf(lines.slice(configOnlyStart, insert), documented), + set: keysOf(lines.slice(configStart + 1), /^ {8}([A-Za-z]\w*):/u), + }; +}; + +describe("cordis.patch.yml agrees with the schema and the field registry", () => { + const root = path.dirname(import.meta.dirname); + const read = async () => + manifestKeys(await readFile(path.join(root, "cordis.patch.yml"), "utf-8")); + // `Config.dict` is the schema's own field table; `Config({})` would omit + // every key with neither a default nor `.volatile()`. + const schemaKeys = Object.keys(Config.dict ?? {}); + + it("documents and sets only keys the schema has", async () => { + const { card, configOnly, set } = await read(); + // Known answers, so a layout change fails loudly rather than vacuously. + expect(schemaKeys).toContain("debugFile"); + expect(set).toContain("providers"); + expect(card).toContain("injectUserAgent"); + + const unknown = [...card, ...configOnly, ...set].filter( + (key) => !schemaKeys.includes(key) + ); + expect(unknown).toEqual([]); + }); + + it("documents exactly the card fields and exactly the config-only fields", async () => { + const { card, configOnly } = await read(); + expect(card).toContain("injectUserAgent"); + expect(configOnly).toContain("providers"); + + expect(card.toSorted()).toEqual( + CARD_FIELDS.map((entry) => entry.field).toSorted() + ); + expect(configOnly.toSorted()).toEqual( + CONFIG_ONLY_FIELDS.map((entry) => entry.field).toSorted() + ); + }); + + it("documents every schema key exactly once", async () => { + const { card, configOnly } = await read(); + const documented = [...card, ...configOnly]; + expect(new Set(documented).size).toBe(documented.length); + expect(documented.toSorted()).toEqual(schemaKeys.toSorted()); + }); + + it("sets each row key at most once", async () => { + const { set } = await read(); + expect(set).toContain("providers"); + expect(new Set(set).size).toBe(set.length); + }); +}); diff --git a/test/retry.test.ts b/test/retry.test.ts index 3699125a..cbb88edb 100644 --- a/test/retry.test.ts +++ b/test/retry.test.ts @@ -1,17 +1,33 @@ import { describe, expect, it, vi } from "vitest"; -import { retryUntilAnswered } from "../src/utils/retry.ts"; +import { + backoffOver, + retryUntilAnswered, + type RetryOptions, + type RetryTimer, +} from "./support/src.ts"; describe("utils/retry: retryUntilAnswered", () => { const noSleep = () => Promise.resolve(); + const run = ( + read: () => Promise, + attempts: number, + extra: Partial = {} + ): Promise => + retryUntilAnswered(read, { + attempts, + sleep: noSleep, + ...extra, + }); + it("returns the first answer without retrying when immediately defined", async () => { let calls = 0; const read = () => { calls += 1; return Promise.resolve("ready"); }; - const result = await retryUntilAnswered(read, 5, noSleep); + const result = await run(read, 5); expect(result).toBe("ready"); expect(calls).toBe(1); }); @@ -22,7 +38,7 @@ describe("utils/retry: retryUntilAnswered", () => { calls += 1; return Promise.resolve(calls < 3 ? undefined : "finally"); }; - const result = await retryUntilAnswered(read, 5, noSleep); + const result = await run(read, 5); expect(result).toBe("finally"); expect(calls).toBe(3); }); @@ -39,7 +55,7 @@ describe("utils/retry: retryUntilAnswered", () => { } return Promise.resolve("recovered"); }; - const result = await retryUntilAnswered(read, 5, noSleep); + const result = await run(read, 5); expect(result).toBe("recovered"); expect(calls).toBe(3); }); @@ -49,28 +65,48 @@ describe("utils/retry: retryUntilAnswered", () => { const read = async () => { calls += 1; }; - const result = await retryUntilAnswered(read, 4, noSleep); + const result = await run(read, 4); expect(result).toBeUndefined(); expect(calls).toBe(4); }); - it("returns undefined and logs a warning when all attempts throw errors", async () => { + it("returns undefined and tells the caller's logger when all attempts throw errors", async () => { let calls = 0; - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const warn = vi.fn<(message: string, error: unknown) => void>(); const read = () => { calls += 1; return Promise.reject(new Error("repeated outage")); }; + const result = await run(read, 3, { warn }); + expect(result).toBeUndefined(); + expect(calls).toBe(3); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + "[dsh-opencode-patch] retry: all attempts failed", + expect.any(Error) + ); + }); + + it("stays silent when every attempt throws and no logger is supplied", async () => { + // Silence is the claim, so the console is watched: a fallback to + // `console.warn` when `warn` is omitted would answer the same result. + const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}); try { - const result = await retryUntilAnswered(read, 3, noSleep); - expect(result).toBeUndefined(); - expect(calls).toBe(3); - expect(warnSpy).toHaveBeenCalledWith( - "[dsh-opencode-patch] retry: all attempts failed", - expect.any(Error) - ); + let calls = 0; + const read = () => { + calls += 1; + return Promise.reject(new Error("repeated outage")); + }; + await expect(run(read, 2)).resolves.toBeUndefined(); + expect(calls).toBe(2); + expect(consoleWarn).not.toHaveBeenCalled(); + expect(consoleError).not.toHaveBeenCalled(); } finally { - warnSpy.mockRestore(); + consoleWarn.mockRestore(); + consoleError.mockRestore(); } }); @@ -81,7 +117,7 @@ describe("utils/retry: retryUntilAnswered", () => { calls += 1; return "unreached"; }; - const result = await retryUntilAnswered(read, 5, sleep, () => false); + const result = await run(read, 5, { live: () => false, sleep }); expect(result).toBeUndefined(); expect(calls).toBe(0); expect(sleep).not.toHaveBeenCalled(); @@ -94,20 +130,83 @@ describe("utils/retry: retryUntilAnswered", () => { calls += 1; alive = false; }; - const result = await retryUntilAnswered(read, 5, noSleep, () => alive); + const result = await run(read, 5, { live: () => alive }); expect(result).toBeUndefined(); expect(calls).toBe(1); }); - it("uses the default sleep calculation when sleep is omitted", async () => { - // Testing that the default sleep parameter can be executed without error + it("defaults to five attempts", async () => { + let calls = 0; + const read = async () => { + calls += 1; + }; + const result = await retryUntilAnswered(read, { sleep: noSleep }); + expect(result).toBeUndefined(); + expect(calls).toBe(5); + }); + + it("waits on the schedule backoffOver builds over the caller's timer", async () => { + // No wall clock: the timer runs its callback at once and records the delay + // it was asked for, so the default schedule is observed rather than waited. let calls = 0; + const delays: number[] = []; + const immediateTimer: RetryTimer = (callback, delayMs) => { + delays.push(delayMs); + callback(); + }; const read = () => { calls += 1; return Promise.resolve(calls < 2 ? undefined : "done"); }; - const result = await retryUntilAnswered(read, 2); + const result = await retryUntilAnswered(read, { + attempts: 2, + sleep: backoffOver(immediateTimer), + }); expect(result).toBe("done"); expect(calls).toBe(2); + expect(delays).toEqual([300]); + }); +}); + +describe("utils/retry: backoffOver", () => { + /** A timer that records each request and runs nothing until told to. */ + const recordingTimer = () => { + const scheduled: { callback: () => void; delayMs: number }[] = []; + const timer: RetryTimer = (callback, delayMs) => { + scheduled.push({ callback, delayMs }); + }; + return { scheduled, timer }; + }; + + it("schedules a linear backoff and resolves only when the timer fires", async () => { + const { scheduled, timer } = recordingTimer(); + const sleep = backoffOver(timer); + const settled: number[] = []; + const pending = [0, 1, 2].map((attempt) => + sleep(attempt).then(() => { + settled.push(attempt); + }) + ); + + expect(scheduled.map((entry) => entry.delayMs)).toEqual([300, 600, 900]); + await Promise.resolve(); + expect(settled).toEqual([]); + + scheduled[1]?.callback(); + await pending[1]; + expect(settled).toEqual([1]); + + scheduled[0]?.callback(); + scheduled[2]?.callback(); + await Promise.all(pending); + expect(settled.toSorted((a, b) => a - b)).toEqual([0, 1, 2]); + }); + + it("takes its base delay as a parameter", () => { + const { scheduled, timer } = recordingTimer(); + const sleep = backoffOver(timer, 100); + void sleep(0); + void sleep(1); + expect(scheduled.map((entry) => entry.delayMs)).toEqual([100, 200]); }); }); diff --git a/test/settings-page.test.tsx b/test/settings-page.test.tsx index 0258994d..055240dd 100644 --- a/test/settings-page.test.tsx +++ b/test/settings-page.test.tsx @@ -3,21 +3,21 @@ import assert from "node:assert/strict"; import { describe, expect, it, vi } from "vitest"; import { + applySettingsPage as apply, + CARD_FIELDS, + Config, + CONFIG_ONLY_FIELDS, + en, ENDPOINT_FAMILIES, KEY_SOURCE_POLICIES, -} from "../src/config-values.ts"; -import { Config } from "../src/index.ts"; -import { en } from "../src/settings-copy.ts"; -import { CARD_FIELDS, CONFIG_ONLY_FIELDS } from "../src/settings-fields.ts"; -import { - apply, LEGACY_NS, - SCOPED_PKG, NS, PKG, + remoteNamespace, + remoteNamespaceOf, + SCOPED_PKG, SPECS, -} from "../src/settings-page.tsx"; -import { remoteNamespace, remoteNamespaceOf } from "../src/settings-remote.ts"; +} from "./support/src.ts"; import { elementName, findAll, @@ -1219,8 +1219,14 @@ describe("settings-page: field specs", () => { // The card deliberately hides the override knobs, so the two lists must // together cover every schema key exactly once. A knob in neither would be // unreachable; a knob in both would contradict CONFIG_ONLY_FIELDS. + // + // The keys come from `Config.dict`, the schema's own field table. + // `Config({})` materialises only the keys that carry a default or are + // `.volatile()`, which is how `debugFile` (neither) went missing from both + // lists without this test noticing. // oxlint-disable-next-line unicorn/no-array-sort -- `Object.keys` returns a fresh array, so in-place sort mutates nothing shared. - const schemaKeys = Object.keys(Config({})).sort(); + const schemaKeys = Object.keys(Config.dict ?? {}).sort(); + expect(schemaKeys).toContain("debugFile"); const cardFields = CARD_FIELDS.map((entry) => entry.field); const configOnly = CONFIG_ONLY_FIELDS.map((entry) => entry.field); const covered = [...cardFields, ...configOnly]; diff --git a/test/settings-usage.test.tsx b/test/settings-usage.test.tsx index 9bfdc834..b602531c 100644 --- a/test/settings-usage.test.tsx +++ b/test/settings-usage.test.tsx @@ -8,7 +8,7 @@ import { render, waitFor } from "@testing-library/react"; */ import type { ReactNode } from "react"; import React from "react"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { zh } from "../src/settings-copy.ts"; // @vitest-environment jsdom @@ -387,4 +387,40 @@ describe("settings-usage: reading until the remote answers", () => { ).toBeUndefined(); expect(calls).toBe(1); }); + + it("waits 300 ms on the real timer when no sleep is supplied", async () => { + // The production card passes no `sleep`, so this is the clock it runs on. + // A timer that never fired would leave the card refreshing for good. + vi.useFakeTimers(); + try { + let calls = 0; + const read = () => { + calls += 1; + return Promise.resolve(calls < 2 ? undefined : answered); + }; + const pending = readUntilAnswered(read, 2); + await vi.advanceTimersByTimeAsync(299); + expect(calls).toBe(1); + await vi.advanceTimersByTimeAsync(1); + expect(calls).toBe(2); + expect(await pending).toBe(answered); + } finally { + vi.useRealTimers(); + } + }); + + it("reports on the console when every attempt fails", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const read = () => Promise.reject(new Error("repeated outage")); + expect(await readUntilAnswered(read, 2, noSleep)).toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + "[dsh-opencode-patch] retry: all attempts failed", + expect.any(Error) + ); + } finally { + warn.mockRestore(); + } + }); }); diff --git a/test/stream-hook.test.ts b/test/stream-hook.test.ts index b16d9dfe..5d63c894 100644 --- a/test/stream-hook.test.ts +++ b/test/stream-hook.test.ts @@ -18,8 +18,12 @@ import { basename, join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { recordDebug } from "../src/debug.ts"; -import { projectNameFrom } from "../src/stream-hook.ts"; +import { + GO_GOOGLE_INTERNAL_ROUTE, + projectNameFrom, + recordDebug, + RESPONSES_ROUTE, +} from "./support/src.ts"; import { collectUnknown, createMockStream } from "./test-helpers.ts"; // The debug sink is observed rather than written: a negative assertion over a @@ -125,9 +129,9 @@ describe("stream-hook · the pass-through guards", () => { providers: string[], overrides: Record = {} ) => { - const { createStreamHook } = await import("../src/stream-hook.ts"); + const { createStreamHook, resolveConfig } = + await import("./support/src.ts"); const { AsyncLocalStorage } = await import("node:async_hooks"); - const { resolveConfig } = await import("../src/config.ts"); return createStreamHook( { logger: { info: () => {} } } as never, resolveConfig({ providers, usageEnabled: false, ...overrides } as never), @@ -172,6 +176,138 @@ describe("stream-hook · the pass-through guards", () => { }); }); +describe("stream-hook · the redirect forwards the caller's request", () => { + /** + * A model the plane serves on another protocol is handed to the route that + * speaks it. The caller's request goes with it whole: only `provider` changes + * (and, for Go-Google, the history is replayed into Google's shape). + * + * `toEqual` deliberately: it ignores a key whose value is `undefined`, so the + * pin holds whether or not the spread carries one. + */ + const hookFor = async ( + providers: string[], + api: string, + route: string, + stream: (options: unknown) => unknown + ) => { + const { createStreamHook, resolveConfig } = + await import("./support/src.ts"); + const { AsyncLocalStorage } = await import("node:async_hooks"); + return createStreamHook( + { + llm: { adapters: new Map([[route, {}]]), stream }, + logger: { info: () => {} }, + } as never, + resolveConfig({ + modelRoutes: { "probe-model": { api } }, + providers, + usageEnabled: false, + } as never), + new AsyncLocalStorage() + ); + }; + + let warn: { mockRestore: () => void } | undefined; + + beforeEach(() => { + // The probe id is in no catalog, so the hook warns once that the picker + // would not offer it. That line is not what these cases are about. + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warn?.mockRestore(); + }); + + it("hands the whole request to the protocol route with only provider replaced", async () => { + const stream = vi.fn((_options: unknown) => "redirected"); + const hook = await hookFor( + ["opencode"], + "openai-responses", + RESPONSES_ROUTE, + stream + ); + const options = { + messages: [], + model: "probe-model", + provider: "opencode", + sessionId: "s", + }; + const next = vi.fn(() => "next-result"); + + expect(hook(options, next)).toBe("redirected"); + expect(next).not.toHaveBeenCalled(); + expect(stream).toHaveBeenCalledTimes(1); + expect(stream.mock.calls[0]?.[0]).toEqual({ + ...options, + provider: RESPONSES_ROUTE, + }); + }); + + it("hands a Go-Google request over with its history in Google's shape", async () => { + const sentinel = { notIterable: true }; + const stream = vi.fn((_options: unknown) => sentinel); + const hook = await hookFor( + ["opencode-go"], + "google-generative-ai", + GO_GOOGLE_INTERNAL_ROUTE, + stream + ); + // The user message is the control: only an assistant turn the Go plane's + // Google adapter answered carries a replay identity to translate. + const user = { content: "hi", role: "user" }; + const replayState = { + response: { + api: "google-generative-ai", + kind: "pi-ai", + provider: "opencode-go", + }, + signature: "sig", + }; + const assistant = { + content: "hello", + role: "assistant", + source: { provider: "opencode-go", replayState }, + }; + const options = { + messages: [user, assistant], + model: "probe-model", + provider: "opencode-go", + sessionId: "s", + }; + const next = vi.fn(() => "next-result"); + + expect(hook(options, next)).toBe(sentinel); + expect(next).not.toHaveBeenCalled(); + expect(stream).toHaveBeenCalledTimes(1); + // A literal expectation, not one re-derived through the function under + // test: forwarding the history raw must fail here. + expect(stream.mock.calls[0]?.[0]).toEqual({ + ...options, + messages: [ + user, + { + content: "hello", + role: "assistant", + source: { + provider: GO_GOOGLE_INTERNAL_ROUTE, + replayState: { + response: { + api: "google-generative-ai", + kind: "pi-ai", + provider: GO_GOOGLE_INTERNAL_ROUTE, + }, + signature: "sig", + }, + }, + }, + ], + provider: GO_GOOGLE_INTERNAL_ROUTE, + }); + }); +}); + describe("stream-hook · the debug gate", () => { /** * `debug` is the switch and `debugFile` is only where the switch writes. The @@ -186,9 +322,9 @@ describe("stream-hook · the debug gate", () => { file: string | undefined, info: (...args: unknown[]) => void ) => { - const { createStreamHook } = await import("../src/stream-hook.ts"); + const { createStreamHook, resolveConfig } = + await import("./support/src.ts"); const { AsyncLocalStorage } = await import("node:async_hooks"); - const { resolveConfig } = await import("../src/config.ts"); return createStreamHook( { logger: { info } } as never, resolveConfig({ diff --git a/test/support/catalog-fixture.ts b/test/support/catalog-fixture.ts new file mode 100644 index 00000000..d262f709 --- /dev/null +++ b/test/support/catalog-fixture.ts @@ -0,0 +1,50 @@ +/** + * A live catalog whose two planes disagree about which models are free. + * + * No id the shipped shim lists on both planes disagrees on `is_free`, so a + * lookup that reads the wrong plane gives the right answer on every shared row + * and the defect stays invisible. (Zen-only free ids are a different case: the + * Go plane does not list them at all — see the `big-pickle` case in + * `test/tool-fallback.test.ts`.) This fixture refreshes the live catalog with two + * probes that flip between the planes: `plane-probe` is free on Zen and paid on + * Go, `go-probe` the reverse. `is_free` is derived from `id.includes("free")` or + * a zero cost, so neither id contains "free" and only the cost decides. + * + * `refreshCatalog` merges into the live maps, and + * `test/support/reset-process-state.ts` restores the shim after every test. The + * stubbed fetch never touches the network. + * + * @module test/support/catalog-fixture + */ +import { refreshCatalog } from "./src.ts"; + +/** Free on Zen, paid on Go. */ +export const ZEN_FREE_PROBE = "plane-probe"; + +/** Free on Go, paid on Zen. */ +export const GO_FREE_PROBE = "go-probe"; + +const FREE = { input: 0, output: 0 } as const; +const PAID = { input: 1, output: 2 } as const; + +/** Refresh the live catalog with the two plane-divergent probes. */ +export const seedDivergentCatalog = async (): Promise => { + await refreshCatalog( + (async () => + Response.json({ + opencode: { + models: { + [GO_FREE_PROBE]: { cost: PAID, name: "Go Probe" }, + [ZEN_FREE_PROBE]: { cost: FREE, name: "Plane Probe" }, + }, + }, + "opencode-go": { + models: { + [GO_FREE_PROBE]: { cost: FREE, name: "Go Probe" }, + [ZEN_FREE_PROBE]: { cost: PAID, name: "Plane Probe" }, + }, + }, + })) as unknown as typeof fetch, + true + ); +}; diff --git a/test/support/src.ts b/test/support/src.ts new file mode 100644 index 00000000..a8fe8132 --- /dev/null +++ b/test/support/src.ts @@ -0,0 +1,441 @@ +/** + * The single indirection through which tests reach `src`. + * + * P2 shrinks the public barrel to `apply`, `Config`, `inject`, `name` and the + * config types, and P10a moves modules into subject folders. A test that names + * a `src` path breaks on every such move, so this file is the one place that + * knows where things live: each name is re-exported from the module that + * DEFINES it, never through the public barrel, so shrinking the barrel does not + * touch a test. + * + * Three kinds of import keep their direct paths: a `vi.mock("../src/x.ts")` + * target (vitest mocks by module id), and the architecture tests, which are + * about the `src` layout itself — for both, the path is their subject; and the + * jsdom (client) tests, which import the defining module directly, because this + * file re-exports the host graph (`node:sqlite`, pi, `lifecycle.ts`) and that + * must stay out of a client test. + * + * Importing this file loads the whole `src` graph, host and client, so a module + * that fails to load fails every test that comes through here, not only its own. + * + * Two names are defined twice under one name, so the second definition is + * aliased: the client bundle's `apply` (`applySettingsPage`) and the Go + * allowance formatter `formatUsd` (`formatAllowanceUsd`). + * + * The file is type-checked with the rest of `test/`, so `vp check` proves that + * every re-export resolves. + * + * @module test/support/src + */ + +// attachment-services.ts +export { attachmentResolvers } from "../../src/attachment-services.ts"; + +// catalog-data.ts +export { + OPENCODE_FAMILY_API, + OPENCODE_GO_CATALOG, + OPENCODE_PLANE_API, + OPENCODE_ZEN_CATALOG, +} from "../../src/catalog-data.ts"; +export type { CatalogModelSpec } from "../../src/catalog-data.ts"; + +// config-values.ts +export { + ALL_MODELS_MARKER, + DEFAULT_ENDPOINT_FAMILY, + DEFAULT_KEY_SOURCE, + DEFAULT_PROVIDERS, + ENDPOINT_FAMILIES, + KEY_SOURCE_POLICIES, + readBoolean, + readString, + readStringList, + unwrapNode, +} from "../../src/config-values.ts"; + +// config.ts +export { + Config, + CONFIG_DEFAULTS, + DEFAULT_FREE_MODEL_MARKER, + DEFAULT_USAGE_BASE_URL, + DEFAULT_USAGE_KEY_ENV, + resolveConfig, +} from "../../src/config.ts"; +export type { + ModelOverrideConfig, + PluginConfig, + ResolvedPluginConfig, +} from "../../src/config.ts"; + +// cordis-context.ts +export { + loaderOfContext, + readCredentialsResolver, + readEntryOptions, + readSessionMetaResolver, +} from "../../src/cordis-context.ts"; +export type { CordisContext } from "../../src/cordis-context.ts"; + +// debug.ts +export { recordDebug } from "../../src/debug.ts"; + +// fetch-patch.ts +export { + hasSessionHeader, + isOpenCodeRequest, + patchFetch, +} from "../../src/fetch-patch.ts"; +export type { FetchPatchHooks } from "../../src/fetch-patch.ts"; + +// go-discovery.ts +export { + declaredProviderModels, + discoverGoConfig, + effectiveGoKeyRef, + resolveGoApiKey, + resolveGoBaseURL, + resolveGoKeyForRef, + resolveRoutedKey, + resolveZenCreditInfo, + toGoBaseURL, +} from "../../src/go-discovery.ts"; +export type { DiscoveredGoConfig } from "../../src/go-discovery.ts"; + +// go-limits-data.ts +export { GO_MODEL_LIMITS } from "../../src/go-limits-data.ts"; + +// go-limits.ts +export { + allowanceFor, + formatUsd as formatAllowanceUsd, + GO_PLAN_TIERS, + normalizeModelLabel, + parseGoLimitsDoc, + WINDOW_SHARE, +} from "../../src/go-limits.ts"; + +// google-bridge.ts +export { + ambientAuthContext, + emptyCredentialStore, + GO_GOOGLE_BASE_URL, + googleProviderFor, + loadHostPi, + registerGoogleBridge, + resolveGatewayKey, +} from "../../src/google-bridge.ts"; +export type { GoogleBridgeInputs, PiRuntime } from "../../src/google-bridge.ts"; + +// identity.ts +export { + inject, + name, + OPENCODE_PATCH_USER_AGENT, + OPENCODE_USER_AGENT, +} from "../../src/identity.ts"; + +// key-capture.ts +export { + clearCapturedApiKeys, + extractApiKeyFromHeaders, + getCapturedApiKey, + isPlaceholderApiKey, + recordCapturedApiKey, + tierForRequest, +} from "../../src/key-capture.ts"; + +// lifecycle.ts +export { apply, mountInternalRoutes } from "../../src/lifecycle.ts"; + +// models-catalog.ts +export { + catalogPlaneForRoute, + enrichModelsResponse, + findModelSpec, + findModelSpecOn, + getLiveGoCatalog, + getLiveZenCatalog, + isDeprecatedModelOn, + isGoModelsListingUrl, + isModelsListingUrl, + parseModelsDevCatalog, + refreshCatalog, + resetCatalogState, + sanitizeModalities, +} from "../../src/models-catalog.ts"; + +// models-discovery.ts +export { + decorateModelDiscovery, + decorateModelInfo, + decoratePlaneAdapters, + hideResponsesRoute, + isRouteRegistered, + isServableModel, + mergeDiscoveredModels, + PI_THINKING_LEVELS, + resolveDiscoveryProvider, + resolveEffectiveContextWindow, +} from "../../src/models-discovery.ts"; + +// opencode-paths.ts +export { + opencodeDatabaseFile, + opencodeDatabasePath, + opencodeRoots, +} from "../../src/opencode-paths.ts"; + +// opencode-store-service.ts +export { OpencodeStoreService } from "../../src/opencode-store-service.ts"; + +// opencode-store.ts +export { + inspectOpencodeStore, + readOpencodeCredentials, + summarizeCredential, +} from "../../src/opencode-store.ts"; + +// pi-provider.ts +export { + BASE_FOR_PROTOCOL, + baseForProtocol, + piModelFor, + planeBaseFor, + protocolFor, + thinkingLevelMapFor, +} from "../../src/pi-provider.ts"; + +// purge-contract.ts +export { parsePurgeResult } from "../../src/purge-contract.ts"; + +// purge-service.ts +export { + ModelPurgeService, + stripRetiredReferences, +} from "../../src/purge-service.ts"; + +// responses-provider.ts +export { + inheritedCredentialRef, + loadPiAi, + loadPiAiAdapterHost, + loadPiAiModule, + modelsForSdk, + narrowToDeclared, + reasoningEffortsFor, + registerResponsesProvider, + undescribedDeclaredModels, + unroutedDeclaredModels, +} from "../../src/responses-provider.ts"; + +// responses-routes.ts +export { + ANTHROPIC_ROUTE, + ANTHROPIC_SDK, + GO_GOOGLE_INTERNAL_ROUTE, + GO_RESPONSES_ROUTE, + GOOGLE_INTERNAL_ROUTE, + GOOGLE_SDK, + INTERNAL_ROUTES, + internalRouteFor, + isServableSdk, + MISTRAL_SDK, + PROTOCOL_FOR_SDK, + RESPONSES_ROUTE, + RESPONSES_SDK, + ROUTE_FOR_PLANE_PROTOCOL, + ROUTE_FOR_PROTOCOL, + SELF_MOUNTED_ROUTES, + UNSERVED_PLANE_SDKS, + UNSERVED_SDKS, +} from "../../src/responses-routes.ts"; + +// session-cost.ts +export { + calculateTurnCost, + clearSessionUsageStore, + describeModel, + formatModelRate, + formatUsd, + getSessionUsage, + recordTurnUsage, +} from "../../src/session-cost.ts"; +export type { + ModelCostRate, + SessionUsageSnapshot, +} from "../../src/session-cost.ts"; + +// session.ts +export { + fallbackSessionId, + headerValueFor, + newOpenCodeMessageId, + OPENCODE_UA, + openCodeSessionIdFor, +} from "../../src/session.ts"; + +// settings-boolean-field.tsx +export { SettingsBooleanField } from "../../src/settings-boolean-field.tsx"; +export type { BooleanFieldProps } from "../../src/settings-boolean-field.tsx"; + +// settings-card.tsx +export { OpencodeCard, purgeLine } from "../../src/settings-card.tsx"; + +// settings-choice-field.tsx +export { SettingsChoiceField } from "../../src/settings-choice-field.tsx"; +export type { ChoiceFieldProps } from "../../src/settings-choice-field.tsx"; + +// settings-copy.ts +export { en, withBundleCopy, zh } from "../../src/settings-copy.ts"; + +// settings-field-shell.tsx +export { SettingsFieldShell } from "../../src/settings-field-shell.tsx"; +export type { FieldShellProps } from "../../src/settings-field-shell.tsx"; + +// settings-fields.ts +export { + CARD_FIELDS, + CONFIG_ONLY_FIELDS, + SPECS, +} from "../../src/settings-fields.ts"; + +// settings-page.tsx +export { + apply as applySettingsPage, + LEGACY_NS, + NS, + patchRemote, + PKG, + SCOPED_PKG, +} from "../../src/settings-page.tsx"; + +// settings-purge.ts +export { purgeDeprecatedModels } from "../../src/settings-purge.ts"; + +// settings-remote.ts +export { + remoteNamespace, + remoteNamespaceOf, +} from "../../src/settings-remote.ts"; + +// settings-store-inspect.tsx +export { + inspectLine, + StoreInspectRow, +} from "../../src/settings-store-inspect.tsx"; + +// settings-store.ts +export { inspectStore } from "../../src/settings-store.ts"; +export type { StoreInspectOutcome } from "../../src/settings-store.ts"; + +// settings-usage.tsx +export { + readGoUsage, + readUntilAnswered, + UsageSummary, + UsageSummaryView, +} from "../../src/settings-usage.tsx"; + +// stream-hook.ts +export { createStreamHook, projectNameFrom } from "../../src/stream-hook.ts"; + +// tool-fallback.ts +export { + DUMMY_BASH_TOOL, + DUMMY_BASH_TOOL_FUNCTION, + DUMMY_READ_TOOL, + DUMMY_READ_TOOL_FUNCTION, + isCoreToolModel, + maybeInjectCoreTools, + RESPONSES_PATH, +} from "../../src/tool-fallback.ts"; + +// turn-store.ts +export { withStore } from "../../src/turn-store.ts"; +export type { ActiveTurnState } from "../../src/turn-store.ts"; + +// usage-contract.ts +export { parseGoUsage, parseUsageQuery } from "../../src/usage-contract.ts"; +export type { GoUsage } from "../../src/usage-contract.ts"; + +// usage-panel.tsx +export { UsagePanel, UsageTrigger } from "../../src/usage-panel.tsx"; +export type { + UsagePanelProps, + UsageTriggerProps, +} from "../../src/usage-panel.tsx"; + +// usage-pill.tsx +export { MIN_FEEDBACK_MS, UsagePill } from "../../src/usage-pill.tsx"; +export type { + ModelDirectoryState, + SnapshotStore, +} from "../../src/usage-pill.tsx"; + +// usage-ui.ts +export { + CIRCUMFERENCE, + CONSOLE_URL, + describeUsage, + formatRelativeReset, + getAffectingWindow, + getWindowColorFor, + GO_PLAN_URL, + matchesAny, + parseFailure, + resetLabel, + ringGeometry, + STYLES, +} from "../../src/usage-ui.ts"; +export type { RelativeReset } from "../../src/usage-ui.ts"; + +// usage.ts +export { GoUsageService } from "../../src/usage.ts"; + +// utils/google-replay.ts +export { + googleHistoryFor, + googleReplayFor, + googleReplayStream, +} from "../../src/utils/google-replay.ts"; + +// utils/guards.ts +export { + getAsyncIterator, + isAsyncIterableLike, + isAsyncIteratorLike, + isFetchFunction, + isFunctionLike, + isRecord, + isUnknownArray, +} from "../../src/utils/guards.ts"; + +// utils/request-shape.ts +export { + ANTHROPIC_VERSION, + authHeaderFor, + gatewayPlaneFor, + headerSourceOf, + isGoPlaneUrl, + isZenProvider, + modelIdOf, + requestUrlOf, + toolNamesOf, +} from "../../src/utils/request-shape.ts"; + +// utils/retry.ts +export { backoffOver, retryUntilAnswered } from "../../src/utils/retry.ts"; +export type { RetryOptions, RetryTimer } from "../../src/utils/retry.ts"; + +// utils/session-id.ts +export { + isOpenCodeMessageId, + isOpenCodeSessionId, + PARENT_SESSION_ALT_HEADER, + PARENT_SESSION_HEADER, + SESSION_AFFINITY_HEADER, + SESSION_HEADER, + sessionIdForRequest, +} from "../../src/utils/session-id.ts"; diff --git a/test/test-helpers.ts b/test/test-helpers.ts index 79893bff..abd62dab 100644 --- a/test/test-helpers.ts +++ b/test/test-helpers.ts @@ -9,7 +9,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { readFile } from "node:fs/promises"; import { setTimeout as sleep } from "node:timers/promises"; -import type { ActiveTurnState } from "../src/index.ts"; +import type { ActiveTurnState } from "./support/src.ts"; export const SESSION_RE = /^ses_[0-9a-f]{12}[0-9A-Za-z]{14}$/; diff --git a/test/tool-fallback.test.ts b/test/tool-fallback.test.ts index b0505989..66d8005f 100644 --- a/test/tool-fallback.test.ts +++ b/test/tool-fallback.test.ts @@ -17,18 +17,22 @@ import { describe, expect, it } from "vitest"; -import { ALL_MODELS_MARKER, DEFAULT_FREE_MODEL_MARKER } from "../src/index.ts"; import { + GO_FREE_PROBE, + seedDivergentCatalog, + ZEN_FREE_PROBE, +} from "./support/catalog-fixture.ts"; +import { + ALL_MODELS_MARKER, + DEFAULT_FREE_MODEL_MARKER, DUMMY_BASH_TOOL, - DUMMY_BASH_TOOL_ANTHROPIC, DUMMY_BASH_TOOL_FUNCTION, DUMMY_READ_TOOL, - DUMMY_READ_TOOL_ANTHROPIC, DUMMY_READ_TOOL_FUNCTION, RESPONSES_PATH, isCoreToolModel, maybeInjectCoreTools, -} from "../src/tool-fallback.ts"; +} from "./support/src.ts"; import { parseJsonBody, toolNamesOf } from "./test-helpers.ts"; const RESPONSES_URL = `https://opencode.ai/zen/v1${RESPONSES_PATH}`; @@ -122,6 +126,23 @@ describe("maybeInjectCoreTools: bodies it must not rewrite", () => { } }); + it("leaves an Anthropic /messages body alone", () => { + // The gate admits only URLs naming `/responses` or `/completions`, and a + // gateway `/messages` URL names neither, so an Anthropic body never reaches + // the injector — which is why it carries no Anthropic schemas at all. + const headers = new Headers(); + const body = JSON.stringify({ model: FREE_MODEL }); + + const out = maybeInjectCoreTools( + "https://opencode.ai/inference/anthropic/v1/messages", + body, + headers, + options() + ); + expect(out).toBe(body); + expect(headers.get("content-length")).toBeNull(); + }); + it("injects on the completions route too", () => { const body = JSON.stringify({ input: "hi", model: FREE_MODEL }); const headers = new Headers(); @@ -443,28 +464,6 @@ describe("maybeInjectCoreTools: injection", () => { tools: [null, "bash", 7, declaredRead, DUMMY_BASH_TOOL], }); }); - - it("prefers the Anthropic schemas when the URL names /messages", () => { - // Both path segments in one URL is not a route this plugin documents: - // `RESPONSES_PATH` gates first, so a plain `/messages` URL never reaches - // the injector. The case pins the precedence between the two dialects - // (Anthropic wins over the OpenAI function form) and keeps the Anthropic - // schemas from rotting untested. - const url = "https://relay.internal/v1/messages?upstream=/responses"; - const headers = new Headers(); - const body = JSON.stringify({ model: FREE_MODEL }); - - const out = maybeInjectCoreTools(url, body, headers, options()); - - expect(parseJsonBody(bodyText(out))).toEqual({ - model: FREE_MODEL, - tools: [DUMMY_READ_TOOL_ANTHROPIC, DUMMY_BASH_TOOL_ANTHROPIC], - }); - // `input_schema`, not `parameters`: the two dialects are not - // interchangeable, and mixing them is the failure the fallback avoids. - expect(DUMMY_READ_TOOL_ANTHROPIC).toHaveProperty("input_schema"); - expect(DUMMY_READ_TOOL_FUNCTION).toHaveProperty("function"); - }); }); describe("maybeInjectCoreTools: marker scoping", () => { @@ -537,3 +536,65 @@ describe("maybeInjectCoreTools: marker scoping", () => { ]); }); }); + +describe("isCoreToolModel: the plane comes from the route", () => { + // The catalog's free flag is read from the plane the request is on. No id + // the shipped shim lists on both planes disagrees on it, so these cases seed a live + // catalog that does: a Go-first lookup reads the paid Go row for a model Zen + // serves free, and the free Go row for a model Zen charges for. + it("admits a model the request's plane calls free, whatever the other plane says", async () => { + await seedDivergentCatalog(); + + expect(isCoreToolModel(ZEN_FREE_PROBE, "", "opencode")).toBe(true); + expect(isCoreToolModel(ZEN_FREE_PROBE, "", "opencode-go")).toBe(false); + // A derived Go route is on the Go plane: the rule is the prefix. + expect(isCoreToolModel(GO_FREE_PROBE, "", "opencode-go-google")).toBe(true); + expect(isCoreToolModel(GO_FREE_PROBE, "", "opencode")).toBe(false); + // No route is the rule's default plane, which is Zen. + expect(isCoreToolModel(ZEN_FREE_PROBE, "")).toBe(true); + expect(isCoreToolModel(GO_FREE_PROBE, "")).toBe(false); + }); + + it("no longer admits a Zen-only free id with no marker on the Go plane", () => { + // An intended change on shipped data, recorded so it is not rediscovered: + // `big-pickle` is free on Zen and absent from the Go catalog. The old + // Go-first lookup fell back to Zen and admitted it on a Go request; the + // plane rule reads the Go plane, which does not serve it, and the default + // marker does not match the id. No seeding: this is the shipped shim. + expect( + isCoreToolModel("big-pickle", DEFAULT_FREE_MODEL_MARKER, "opencode") + ).toBe(true); + expect(isCoreToolModel("big-pickle", DEFAULT_FREE_MODEL_MARKER)).toBe(true); + expect( + isCoreToolModel("big-pickle", DEFAULT_FREE_MODEL_MARKER, "opencode-go") + ).toBe(false); + }); + + it("maybeInjectCoreTools reads the plane from options.route", async () => { + await seedDivergentCatalog(); + const body = JSON.stringify({ model: ZEN_FREE_PROBE }); + + const zenHeaders = new Headers(); + const zen = maybeInjectCoreTools(RESPONSES_URL, body, zenHeaders, { + enabled: true, + modelMarker: "none", + route: "opencode", + }); + expect(parseJsonBody(bodyText(zen))).toEqual({ + model: ZEN_FREE_PROBE, + tools: [DUMMY_READ_TOOL, DUMMY_BASH_TOOL], + }); + expect(zenHeaders.get("content-length")).toBe( + String(Buffer.byteLength(bodyText(zen))) + ); + + const goHeaders = new Headers(); + const go = maybeInjectCoreTools(RESPONSES_URL, body, goHeaders, { + enabled: true, + modelMarker: "none", + route: "opencode-go", + }); + expect(go).toBe(body); + expect(goHeaders.get("content-length")).toBeNull(); + }); +});