From a503b98982f737a24ed46d4b2dc4aee376c2c013 Mon Sep 17 00:00:00 2001 From: Leo Date: Thu, 5 Nov 2026 03:39:59 +0800 Subject: [PATCH] chore: remove dead code and tighten the repository's gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase **P1b** of `docs/improvement-plan.md`, restacked onto 1.8.1. - **`src/adapter-replacement.ts` and its test are gone.** Nothing in `src/` imported it — the map said so and the architecture walk agreed — so it was the next step written down as if it were live behaviour. - **`utils/` is now closed against the clock and the console.** The gate forbids `fetch`, `console`, and a global timer (called or passed as a value, bare or through `globalThis.`/`window.`/`global.`/`self.`), and a relative import that resolves outside the folder. `utils/retry.ts` was the offender: its default `sleep` reached a global timer and its warning reached `console`, so it now takes both from the caller (`backoffOver(timer)`, `warn`). - **`.freebuff/project-id` is deleted and the directory ignored.** A tool's local project id had been committed; it is not source, and it is not anyone else's business. - **The test support files moved under `test/support/`** — `catalogs.ts` (the synthetic catalogue fixture) and `src.ts` — so the fixtures a test imports are named as support rather than as a second kind of test. - **The gates tightened**: the lint warnings the tree had been carrying are addressed, and the docs that describe the `utils/` rule now describe the rule the test enforces. Restacked by rebasing the branch's net change onto 1.8.1, then completing the two file moves `patch` cannot express (a rename and a deletion): `test/fixtures/ catalogs.ts` → `test/support/catalogs.ts`, and the two `adapter-replacement` files removed rather than emptied. --- .freebuff/project-id | 1 - .gitignore | 1 + AGENTS.md | 5 +- CONTRIBUTING.md | 1 + README.md | 2 +- README.zh-CN.md | 2 +- cordis.patch.yml | 22 +- docs/engineering-notes.md | 12 + docs/improvement-plan.md | 19 +- docs/testing.md | 5 +- src/adapter-replacement.ts | 120 ------- src/settings-fields.ts | 11 +- src/settings-usage.tsx | 25 +- src/stream-hook.ts | 6 - src/tool-fallback.ts | 86 +---- src/utils/retry.ts | 58 +++- test/adapter-replacement.test.ts | 214 ------------ test/architecture.test.ts | 365 ++++++++++++++++---- test/fetch-patch.test.ts | 103 +++++- test/free-tier-contract.test.ts | 17 +- test/manifest.test.ts | 111 ++++++- test/plane-divergence.test.ts | 11 +- test/plane-lookup.test.ts | 7 +- test/retry.test.ts | 138 ++++++-- test/settings-page.test.tsx | 26 +- test/settings-usage.test.tsx | 38 ++- test/stream-hook.test.ts | 154 ++++++++- test/{fixtures => support}/catalogs.ts | 39 ++- test/support/src.ts | 442 +++++++++++++++++++++++++ test/test-helpers.ts | 2 +- test/tool-fallback.test.ts | 112 +++++-- 31 files changed, 1539 insertions(+), 616 deletions(-) delete mode 100644 src/adapter-replacement.ts delete mode 100644 test/adapter-replacement.test.ts rename test/{fixtures => support}/catalogs.ts (81%) create mode 100644 test/support/src.ts diff --git a/.freebuff/project-id b/.freebuff/project-id index b164097c..e69de29b 100644 --- a/.freebuff/project-id +++ b/.freebuff/project-id @@ -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 688ee948..712bdfaf 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. @@ -73,7 +73,7 @@ It is the same trick `scripts/check.ts` already uses for the client bundle, at f | `guards.ts` | `src/` | | `session-id.ts` | the rules inside `session.ts` | | `google-replay.ts` | `src/` (pure translation, so the gate polices it) | -| `retry.ts` | new (7b1c5d4): the settings card's usage-read retry loop; its default `sleep` and its warning still reach a global timer and `console`, which P1b moves to the caller | +| `retry.ts` | new (7b1c5d4): the settings card's usage-read retry loop; its default `sleep` reached a global timer and its warning reached `console` until P1b moved both to the caller (`backoffOver(timer)`, `warn`) | - `src/session.ts` · `src/turn-store.ts` — the `ses_<12hex><14base62>` derivation and the `AsyncLocalStorage` turn store. - `src/stream-hook.ts` · `src/fetch-patch.ts` · `src/tool-fallback.ts` — the `llm/stream` hook, the fetch interceptor, and the free-tier `read`/`bash` fallback. @@ -88,7 +88,6 @@ It is the same trick `scripts/check.ts` already uses for the client bundle, at f - `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. - `scripts/freshness-scope.ts` — which of CI's three freshness checks (`catalog:shim`, `limits:shim`, `cli:version`) a pull request is GATED on: the ones whose inputs (the check's script and the local modules it loads, walked from the source as Node loads it — types erased by Node's own stripper, specifiers read by `es-module-lexer` — plus `src/identity.ts` for the CLI version; never the generated files) its own diff touches. The rest only warn, because vendor drift is not the pull request's regression: for the two generated tables the nightly run regenerates them and proposes the refresh, and for the CLI version it warns and the bump is a human pull request, which then gates. The gates fail closed: each runs unless the scope step answered `report`. The CLI version's offline half — every copy in the tree agrees with the pin, `cli:version -- --copies` — gates every pull request, since it skips the generated tables and only the repository can break it. `test/freshness-scope.test.ts` walks the real generators, runs the scope step's own shell and the CLI it calls, and evaluates the workflow's step conditions. - `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 24f5e2b0..09434256 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,6 +68,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; 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; and the setup file `test/support/reset-process-state.ts`, which runs before the jsdom tests too. 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 c74079d0..8d15446a 100644 --- a/README.md +++ b/README.md @@ -371,7 +371,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 | diff --git a/README.zh-CN.md b/README.zh-CN.md index bbec6f19..b60e0a86 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -365,7 +365,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` | 端点覆盖；从组合中自动发现 | | `credentialOrg` | — | 账户凭据所属的组织 id，在部署从带外提供一个时使用 | | `debug` / `debugFile` | `false` / — | 诊断用 JSONL 日志，不是谁会在 UI 里调节的行为 | diff --git a/cordis.patch.yml b/cordis.patch.yml index 75beab27..0b8165eb 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,12 +37,21 @@ # 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'). -# debug: true|false log every streamed call that receives the header. -# debugFile: path optional append target for stream debug JSONL. +# 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 to the console. +# debugFile: path optional JSONL file for the request and stream debug entries; naming +# it writes the file whether or not debug is on. - insert: - id: dsh-opencode-patch name: "dsh-opencode-patch" @@ -58,5 +71,4 @@ injectCoreTools: true enrichModels: true usageEnabled: true - showUsagePrice: true debug: false diff --git a/docs/engineering-notes.md b/docs/engineering-notes.md index ca15e7f0..efe8f783 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 b0bca5b4..f8e016a9 100644 --- a/docs/improvement-plan.md +++ b/docs/improvement-plan.md @@ -22,7 +22,7 @@ Phases land as **stacked pull requests**. Each one is based on the previous phas | :-- | :-- | :-- | :-- | :-- | | P0 | #105 | suite hardening, 10 defect fixes, this plan | in review | **merged `main` 1.8.0**; catalog refreshes and the pricing fix dropped (on `main`) | | P1a | #106 | `fix:` the purge row's refusal and the endpoint copy | in review | **merged P0**; shrunk to the purge row's `RemoteError.message`, the `auto` = `modern` copy and the `opencode-google` regression tests; the warning colour drops its token fallback; the catalog's refusal tests gain three cases `main` lacks; the rest is on `main`, and the debug gate follows D24 | -| P1b | #107 | `chore:` hygiene | in review | pending: merges P1a; drops its tool-fallback variant (D25) | +| P1b | #107 | `chore:` hygiene | in review | **merged P1a**; takes `main`'s tool-fallback plane rule (D25) and keeps only the dead Anthropic branch's deletion; its catalogue fixture and `main`'s `test/fixtures/catalogs.ts` become one module behind the one door | | P2 | #108 | `feat:` a smaller public API | in review | pending: merges P1b; re-retires the wrapper (D9) | | P-T1a | #109 | `build:` pin the host | in review | pending: merges P2; redone on the alpha cluster and pi 1.3.0 (D14', D22') | | P-T1b | #110 | `test:` a harness that is the real host | in review | pending: merges P-T1a; re-measured on both cells | @@ -117,13 +117,13 @@ Phases land as **stacked pull requests**. Each one is based on the previous phas | ~~The Settings spinner animates only if the meter is mounted~~ Fixed on `main` (#133): the card carries its own keyframes. | `settings-usage.tsx:45` | | ~~The stream hook writes `debugFile` with `debug` off~~ Not a defect under D24: naming `debugFile` writes both halves. | `stream-hook.ts:327-340` vs `lifecycle.ts:361` | | `settings-purge`: the `{ ok: false }` branch and the parser are on `main` (#133). ~~Still open: a refused purge whose `error` is the host's `RemoteError` object, not a string, shows the generic message.~~ Fixed by P1a: `settings-purge.ts` reads the `RemoteError`'s `message`. | `settings-purge.ts:56-69` | -| The stream hook's `prepared` handling is dead. The host never reads `options.prepared`. | `stream-hook.ts:44,200-203,258` | -| The partition test misses `debugFile` | `test/settings-page.test.tsx:1223` | -| `cordis.patch.yml`: phantom `showUsagePrice`, wrong `freeModelMarker: ''` text, incomplete field lists | `cordis.patch.yml:23,36,61` | -| `utils/retry.ts` uses `console` and global timers, against the purity gate | `utils/retry.ts:31,55` | -| `tool-fallback`: the Anthropic branch can't be reached. (The Go-first lookup is fixed on `main`, D25.) | `tool-fallback.ts:204,237-242` | +| ~~The stream hook's `prepared` handling is dead. The host never reads `options.prepared`.~~ Fixed by P1b: deleted from `stream-hook.ts`. | `stream-hook.ts:44,200-203,258` | +| ~~The partition test misses `debugFile`~~ Fixed by P1b: `settings-fields.ts` registers it and the partition test walks `Config.dict`. | `test/settings-page.test.tsx:1223` | +| ~~`cordis.patch.yml`: phantom `showUsagePrice`, wrong `freeModelMarker: ''` text, incomplete field lists~~ Fixed by P1b: `test/manifest.test.ts` holds the header to the schema. | `cordis.patch.yml:23,36,61` | +| ~~`utils/retry.ts` uses `console` and global timers, against the purity gate~~ Fixed by P1b: `sleep` and `warn` are parameters (`backoffOver(timer)`). | `utils/retry.ts:31,55` | +| ~~`tool-fallback`: the Anthropic branch can't be reached.~~ Fixed by P1b: the Anthropic schemas are deleted. (The Go-first lookup is fixed on `main`; the plane rule is D25.) | `tool-fallback.ts:204,237-242` | | Hard-coded catalog counts in tests, which the nightly refresh PR breaks by construction | `test/catalog.test.ts:94,200,208` | -| `.freebuff/project-id` is tracked | repo root | +| ~~`.freebuff/project-id` is tracked~~ Fixed by P1b: `.freebuff/` is ignored and untracked. | repo root | ## Routing: the problem, the measured facts, the design @@ -351,6 +351,7 @@ What stays private is pinned by contract tests against the real host packages: - `retry.ts` takes its timer and logger as parameters. The architecture test bans `console` and timers in `utils/`, and walks the folder recursively. - Remove the dead branches in `tool-fallback`. - Add the `test/support/src.ts` indirection. + - On the restack: the tool fallback keeps `main`'s plane rule (D25), so only the unreachable Anthropic schemas go; the catalogue fixture this phase wrote and `main`'s `test/fixtures/catalogs.ts` become one module under `test/support/`, which reaches `src` through the door. 3. **P2 `feat:` a smaller public API.** It ships as a 1.x minor. - `index.ts` exports `apply`, `Config`, `inject`, `name` and the config types. - `./src/*` goes from `exports`, and `check.ts` asserts that every exports target is inside `files`. @@ -452,8 +453,8 @@ How each phase changes now that it sits on `main` 1.8.0 and the alpha host. Each | P0 | Shrinks: the three catalog refreshes and the per-plane pricing fix are on `main`. Keeps the suite hardening, the lifecycle, mount and credential fixes, D23's scoping with `--copies`, and this plan. | | P1a | Shrinks hard: the purge row shows the host's `RemoteError.message`, the endpoint copy says `auto` is `modern`, and `isZenProvider("opencode-google")` gains a regression test. The wrapper fix, refusal passthrough, warning token, spinner and `parsePurgeResult` are on `main` or superseded; P1a only drops the warning token's fallback, checks every token the card names against the host's vocabulary, and adds three refusal-passthrough test cases `main` lacks (the Zen-plane 404 fill, the non-JSON 200 answer, the `retry-after` passthrough). The debug gate follows D24. | | P1b | Takes `main`'s `coreToolDecision` (D25) and ports only the dead Anthropic branch's deletion; its catalogue fixture merges with `main`'s `test/fixtures/catalogs.ts` behind the one door. | -| P2 | Re-retires the wrapper `main` revived (D9); the door list grows by every `src` name `main`'s tests import. | -| P-T1a | Redone on D14' and D22': `main`'s pins, then the pi 1.3.0 override and patch; the 1.1.0 patch is deleted. | +| P2 | Re-retires the wrapper `main` revived (D9); the door list grows by every `src` name `main`'s tests import. On the merge, P1b's and P2's door-rule lines in `CONTRIBUTING.md` conflict; take P2's, which names the same exceptions and adds its gate. | +| P-T1a | Redone on D14' and D22': `main`'s pins, then the pi 1.3.0 override and patch. **Open before it starts:** D14' and D22' verify one channel, while the README's "Verified on" line and `harness-channel.test.ts` name both channels' latest hosts, and the rc host pins an older pi with its own patch and its own defaults (D17), so a peer range cannot stand for it. The owner decides, and the outcome is recorded as D14'' and D22''. The cheapest coverage is a per-channel `HOST_VERSIONS` and an rc row in the e2e-host matrix, which installs into a throwaway prefix: a floating rc install first, and a pinned rc cell with that tag's patch only once the latest rc tag's pi pin is confirmed. The committed install and lockfile stay on the alpha cluster, and the 1.1.0 patch is kept until the decision. | | P-T1b | Re-measured on the pinned cell (pi 1.3.0, patched) and the floating one (1.5.0 today); floating rows are version-agnostic; contract-test citations move to cordis `4.0.6-alpha.1`. | | P-H | Gains the Chinese "Changed in 1.x" bullet the pairing gate requires. | | P-R1 | Restates pi-ai's defaults (D17); deletes `test/google-thinking.test.ts` with `src/google-bridge.ts`, moving its intents to the mock suite; takes `main`'s model lists with its own routing prose. | diff --git a/docs/testing.md b/docs/testing.md index 0623bb9f..65ca32f9 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -47,12 +47,13 @@ Three properties the unit and mock suites keep by construction rather than by ca `test/support/opencode-store-fixture.ts` — the OpenCode store fixture. It needs `node:sqlite` and the filesystem, which is why it lives with the other host-only support files rather than in the shared helpers. -`test/fixtures/catalogs.ts` — **synthetic catalogue rows**. Every other fixture quotes something that exists; this one builds the row a test needs, including rows the vendor has not produced. It exists because the plane-aware decisions are only observable when the two planes DISAGREE, and the real catalogue agrees about the free flag for every model it shares. +`test/support/catalogs.ts` — **synthetic catalogue rows**, reaching `src` through the one door like the other fixtures. Every other fixture quotes something that exists; this one builds the row a test needs, including rows the vendor has not produced. It exists because the plane-aware decisions are only observable when the two planes DISAGREE, and the real catalogue agrees about the free flag for every model it shares. -Two entry points: +Three entry points: - `spec(...)` / `divergent(id, { go, zen })` build a row (or a pair of rows) directly, for a decision that takes the row as its parameter — `coreToolDecision`, `calculateTurnCost`. - `payload({ go, zen })` builds a whole `models.dev` payload, for the paths that read the LIVE catalogue through `refreshCatalog` (`test/plane-lookup.test.ts`). That world is per TEST: `refreshCatalog` merges by id and cannot remove, but the setup file resets the live catalog after every test, so a file installs its world in `beforeEach` and gives its rows ids no other test names. +- `seedDivergentCatalog()` installs one such world for the request-path tests: `ZEN_FREE_PROBE` is free on Zen and paid on Go, `GO_FREE_PROBE` the reverse, so a lookup that reads the wrong plane is red in both directions (`test/tool-fallback.test.ts`, `test/fetch-patch.test.ts`). **A model with no price in a fixture is PAID at a nominal rate, not free.** The parser derives `is_free` from a zero cost, so a "no price" default would silently turn every fixture model into a free tier. 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/settings-fields.ts b/src/settings-fields.ts index dd6f19bd..7a6a6672 100644 --- a/src/settings-fields.ts +++ b/src/settings-fields.ts @@ -54,7 +54,16 @@ export const CONFIG_ONLY_FIELDS: readonly { }, { field: "debug", - reason: "diagnostic JSONL logging, not a behaviour a user tunes in the UI", + reason: + "diagnostic console logging of streamed calls, not a behaviour a user " + + "tunes in the UI", + }, + { + field: "debugFile", + reason: + "the JSONL file both debug halves append to; naming it writes the file " + + "whether or not debug is on, and a server-side path is a deployment fact " + + "rather than a card control", }, { field: "userAgent", diff --git a/src/settings-usage.tsx b/src/settings-usage.tsx index d8ac384a..fa7a63fd 100644 --- a/src/settings-usage.tsx +++ b/src/settings-usage.tsx @@ -33,7 +33,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. */ /** @@ -366,13 +366,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 756f707b..21aa046b 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 43d6f366..bcd25fc9 100644 --- a/src/tool-fallback.ts +++ b/src/tool-fallback.ts @@ -22,55 +22,6 @@ import { isGoPlaneUrl } from "./utils/request-shape.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.", @@ -207,7 +158,7 @@ const toolNames = ( * * **Pure, so the row can be constructed.** The plane-aware lookup that produces * `row` cannot be exercised against a divergence the vendor has not produced - * yet — see `test/fixtures/catalogs.ts` — so the decision takes the row and the + * yet — see `test/support/catalogs.ts` — so the decision takes the row and the * lookup moves to the caller. * * @param row - the model's row on ITS plane, or `undefined` when that plane's @@ -278,7 +229,9 @@ export const maybeInjectCoreTools = ( 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"); @@ -331,36 +284,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 365f20e8..66e697c2 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/catalogs.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,100 @@ 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's path, never from the turn's route", async () => { + // A relay serves its Go plane under a path of the same shape, and the route + // id there is the user's own name, so the path is the one witness both a + // gateway and a relay share. A path that names no Go plane is Zen, whatever + // the turn's provider says. + 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)); + }; + + // A relay's Go-shaped path is the Go plane, even on a Zen turn. + expect( + await send("opencode", "https://relay.example/zen/go/v1/responses") + ).toEqual(["read", "bash"]); + // A path naming no Go plane is Zen, even on a Go turn. + expect( + await send("opencode-go", "https://relay.example/v1/responses") + ).toBeUndefined(); + // On the 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..6b97897d 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,17 @@ 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 because every route below posts to a Zen URL, and the injector + // reads its plane from that URL (the plane-blind row is its fallback), 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 +128,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/plane-divergence.test.ts b/test/plane-divergence.test.ts index c1aac635..300ce887 100644 --- a/test/plane-divergence.test.ts +++ b/test/plane-divergence.test.ts @@ -7,16 +7,19 @@ * real catalogue disagrees about the PRICES today (two models), and agrees * about everything else, so a test built on the real catalogue cannot tell a * plane-aware lookup from a plane-blind one: both pass. That is the gap this - * file closes, using `test/fixtures/catalogs.ts` to build the row. + * file closes, using `test/support/catalogs.ts` to build the row. * * @module test/plane-divergence.test */ import { describe, expect, it } from "vitest"; -import { calculateTurnCost, recordTurnUsage } from "../src/session-cost.ts"; -import { coreToolDecision } from "../src/tool-fallback.ts"; -import { divergent, spec } from "./fixtures/catalogs.ts"; +import { divergent, spec } from "./support/catalogs.ts"; +import { + calculateTurnCost, + coreToolDecision, + recordTurnUsage, +} from "./support/src.ts"; describe("the free flag is the plane's row", () => { it("injects the free tier's tools on the plane that says free, and not on the other", () => { diff --git a/test/plane-lookup.test.ts b/test/plane-lookup.test.ts index 717db0ce..38fbf173 100644 --- a/test/plane-lookup.test.ts +++ b/test/plane-lookup.test.ts @@ -22,13 +22,14 @@ import { beforeEach, describe, expect, it } from "vitest"; +import { payload } from "./support/catalogs.ts"; import { + coreToolDecision, findModelSpec, findModelSpecOn, + isCoreToolModel, refreshCatalog, -} from "../src/models-catalog.ts"; -import { coreToolDecision, isCoreToolModel } from "../src/tool-fallback.ts"; -import { payload } from "./fixtures/catalogs.ts"; +} from "./support/src.ts"; /** A model the two planes describe differently: free on Go, paid on Zen. */ const DIVERGENT = "acme-divergent-scratchpad"; diff --git a/test/retry.test.ts b/test/retry.test.ts index 3699125a..74d43f0c 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,82 @@ 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(async (attempt) => { + await sleep(attempt); + 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 2bd3e912..560d8b66 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 5e86a3c9..e6d3fb3b 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 @@ -358,4 +358,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 ddd6d331..52a61339 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 @@ -122,9 +126,9 @@ describe("stream-hook · the pass-through guards", () => { * serve. */ const hookWith = async (providers: string[]) => { - 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 } as never), @@ -198,6 +202,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", () => { /** * Naming `debugFile` writes the file; `debug` governs the console alone. The @@ -212,9 +348,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({ @@ -298,9 +434,9 @@ describe("stream-hook · a turn is priced at ITS plane's rate", () => { * meter showed the half. */ const hookWith = async (providers: string[]) => { - 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: true } as never), @@ -326,7 +462,7 @@ describe("stream-hook · a turn is priced at ITS plane's rate", () => { it("prices a Zen turn at the Zen rate, and a Go turn at the Go rate", async () => { const { clearSessionUsageStore, getSessionUsage } = - await import("../src/session-cost.ts"); + await import("./support/src.ts"); clearSessionUsageStore(); const hook = await hookWith(["opencode", "opencode-go"]); diff --git a/test/fixtures/catalogs.ts b/test/support/catalogs.ts similarity index 81% rename from test/fixtures/catalogs.ts rename to test/support/catalogs.ts index 673a906a..40e2bc5f 100644 --- a/test/fixtures/catalogs.ts +++ b/test/support/catalogs.ts @@ -17,10 +17,14 @@ * The prices DO disagree for two models, which is why the pricing decision was * testable against the real catalog and the others were not. * - * @module test/fixtures/catalogs + * It reaches `src` through the one door, `./src.ts`, like the other fixtures, + * so a module move edits one file. (The setup file cannot: it loads in the + * jsdom files too, so it names the defining modules directly.) + * + * @module test/support/catalogs */ -import type { CatalogModelSpec } from "../../src/catalog-data.ts"; +import { type CatalogModelSpec, refreshCatalog } from "./src.ts"; /** What a test says about one catalogue row, in the terms the decisions use. */ export interface SpecInput { @@ -168,3 +172,34 @@ export const payload = (world: { }); return { "opencode-go": provider(world.go), opencode: provider(world.zen) }; }; + +/** Free on Zen, paid on Go. Neither probe id contains "free", so only the row decides. */ +export const ZEN_FREE_PROBE = "plane-probe"; + +/** Free on Go, paid on Zen. */ +export const GO_FREE_PROBE = "go-probe"; + +/** + * Refresh the live catalogue with the two plane-divergent probes. + * + * A request-path test needs the world installed where the shipped lookup reads + * it, so this goes through `refreshCatalog` (the stubbed fetch never touches the + * network). `refreshCatalog` merges into the live maps, and + * `test/support/reset-process-state.ts` restores the shim after every test, so + * a case seeds its own world. + * + * @returns once the live maps carry both probes. + */ +export const seedDivergentCatalog = async (): Promise => { + const body = payload({ + go: [ + { free: true, id: GO_FREE_PROBE, name: "Go Probe" }, + { id: ZEN_FREE_PROBE, name: "Plane Probe" }, + ], + zen: [ + { id: GO_FREE_PROBE, name: "Go Probe" }, + { free: true, id: ZEN_FREE_PROBE, name: "Plane Probe" }, + ], + }); + await refreshCatalog(async () => Response.json(body), true); +}; diff --git a/test/support/src.ts b/test/support/src.ts new file mode 100644 index 00000000..c810ea68 --- /dev/null +++ b/test/support/src.ts @@ -0,0 +1,442 @@ +/** + * 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 { + coreToolDecision, + 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 1a310ff9..d79ce118 100644 --- a/test/tool-fallback.test.ts +++ b/test/tool-fallback.test.ts @@ -17,18 +17,23 @@ 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/catalogs.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, + findModelSpecOn, 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}`; @@ -138,6 +143,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(); @@ -459,28 +481,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", () => { @@ -553,3 +553,63 @@ describe("maybeInjectCoreTools: marker scoping", () => { ]); }); }); + +describe("isCoreToolModel: the plane is the request's, with a plane-blind fallback", () => { + // 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, "", "zen")).toBe(true); + expect(isCoreToolModel(ZEN_FREE_PROBE, "", "go")).toBe(false); + expect(isCoreToolModel(GO_FREE_PROBE, "", "go")).toBe(true); + expect(isCoreToolModel(GO_FREE_PROBE, "", "zen")).toBe(false); + }); + + it("falls back to the plane-blind row when the plane is unknown or does not carry the id", () => { + // The id below is free on Zen and absent from the Go catalog. A Go request + // for it finds no Go row, so the plane-blind lookup answers — and that row + // is the Zen one, which says free. No seeding: this is the shipped shim. + // The premise, asserted: a regeneration that lists the id on Go would keep + // the three answers below true while the fallback stopped being taken. + expect(findModelSpecOn("go", "big-pickle")).toBeUndefined(); + expect(findModelSpecOn("zen", "big-pickle")?.is_free).toBe(true); + expect( + isCoreToolModel("big-pickle", DEFAULT_FREE_MODEL_MARKER, "zen") + ).toBe(true); + expect(isCoreToolModel("big-pickle", DEFAULT_FREE_MODEL_MARKER, "go")).toBe( + true + ); + expect(isCoreToolModel("big-pickle", DEFAULT_FREE_MODEL_MARKER)).toBe(true); + }); + + it("maybeInjectCoreTools reads the plane from the URL's path", 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", + }); + 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( + `https://opencode.ai/zen/go/v1${RESPONSES_PATH}`, + body, + goHeaders, + { enabled: true, modelMarker: "none" } + ); + expect(go).toBe(body); + expect(goHeaders.get("content-length")).toBeNull(); + }); +});