From 9ccdeb3e4f7e1ae6f5a3b2b3f3129fa36d18dcdb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 09:40:44 +0000 Subject: [PATCH 1/2] refactor: one way to patch a host method Add src/host/patch-method.ts (patchMethod), now the only way the plugin wraps a method on a host object, and move the four instance patch sites in src/models-discovery.ts onto it: the three listings hidden by hideResponsesRoute, decorateModelDiscovery, decorateModelInfo and decoratePlaneAdapters. On the real host the three llm-level disposers never restored: ctx.llm is cordis's traceable proxy, every method read returns a new shadow proxy, so the `===` identity checks were never true and each reload stacked a wrapper over the last. The helper reads identity from own descriptors instead. A dispose restores only while its wrapper is still current (otherwise it becomes a pass-through), deletes the own property when the original was inherited, and a same-site reinstall wraps the unpatched method rather than the earlier install, so either disposal order ends at the exact original. decoratePlaneAdapters becomes one patch per (adapter, method) that composes the adapter's plane routes in registry order, which keeps today's nesting on pi-ai's shared adapter. rewrapOnAdaptersUpdated is built and tested but wired to no site yet. test/reload.test.ts runs apply A, apply B, dispose A, dispose B (and the reverse order) for every site on own and inherited originals, on the real host (tier A) and through apply(); test/patch-method.test.ts covers the helper's contract. The existing per-site pins in test/models-discovery.test.ts stay unchanged; five new cases there record the deliberate edge changes on hosts that refuse the patch. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0141fKp5k6hiaJcP5ovhGtSD --- AGENTS.md | 1 + CONTRIBUTING.md | 2 +- README.md | 1 + docs/engineering-notes.md | 18 + docs/improvement-plan.md | 29 +- src/cordis-context.ts | 9 +- src/host/patch-method.ts | 364 ++++++++++++ src/models-discovery.ts | 1018 +++++++++++++++++---------------- test/models-discovery.test.ts | 333 +++++++++++ test/patch-method.test.ts | 782 +++++++++++++++++++++++++ test/reload.test.ts | 702 +++++++++++++++++++++++ test/support/src.ts | 7 + vite.config.ts | 7 + 13 files changed, 2783 insertions(+), 490 deletions(-) create mode 100644 src/host/patch-method.ts create mode 100644 test/patch-method.test.ts create mode 100644 test/reload.test.ts diff --git a/AGENTS.md b/AGENTS.md index ec36e0f4..0eff0cdc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,6 +94,7 @@ It is the same trick `scripts/check.ts` already uses for the client bundle, at f - `src/responses-routes.ts` · `src/responses-provider.ts` — the protocol table and the in-process mount of the host's `llm-pi-ai`. - `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/host/patch-method.ts` — `patchMethod`, the one way to wrap a method on a host object. A dispose restores only while its wrapper is still the current value (otherwise it becomes a pass-through under whatever wrapped on top), and deletes the own property when the original was inherited rather than assigning it back; a second install of the same site wraps the unpatched method instead of the first install. Identity is read from own descriptors, never from a property read, because cordis's traceable proxy answers a fresh proxy for every method read (`docs/engineering-notes.md`). `rewrapOnAdaptersUpdated` reinstalls a site on `llm/adapters-updated`, for a site that must follow the registry; none uses it yet. Every instance patch in `src/` goes through this module — today the four sites in `models-discovery.ts` — and `test/reload.test.ts` runs apply A, apply B, dispose A, dispose B on each. - `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`. - `src/attachment-services.ts` — optional attachment and filesystem services for Google image requests, captured through Cordis child scopes. - `src/google-bridge.ts` — the Google models, served through the plugin's own adapter on the real `llm` service. It is the ONLY module that imports pi at runtime, and its route is `opencode-google` so the `google` name stays free for a real deployment. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 66529591..b0b4b2c0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -127,7 +127,7 @@ The web profile wires this checkout with `link:`, so builds are picked up like t - **Raise them when you genuinely add coverage** — a PR that lifts statements by a few points should lift the threshold with it, or the next contributor inherits a gate nobody re-measured. - **Never set one above the current measurement.** That does not make the suite better; it makes every subsequent PR fail until someone deletes tests. -`src/index.ts` is excluded from the report — it is a pure re-export barrel, and the coverage tools attribute an untaken re-export line to whichever file re-exports it, so leaving it in reports a hole nobody can fill while hiding real ones. `responses-provider.ts` and `responses-routes.ts` carry their own per-file floors: the mount has four host contracts to survive, and a well-covered average must not be able to hide it. +`src/index.ts` is excluded from the report — it is a pure re-export barrel, and the coverage tools attribute an untaken re-export line to whichever file re-exports it, so leaving it in reports a hole nobody can fill while hiding real ones. `responses-provider.ts` and `responses-routes.ts` carry their own per-file floors: the mount has four host contracts to survive, and a well-covered average must not be able to hide it. `host/patch-method.ts` carries a 100% floor, because it is the one way to patch a host method and each of its branches is a reload case. ## Release process (maintainers) diff --git a/README.md b/README.md index 9553efda..593ee789 100644 --- a/README.md +++ b/README.md @@ -428,6 +428,7 @@ These exist in the schema but render no control — each is a literal, a marker, - **A smaller public API.** The package's entry exports `apply`, `Config`, `inject` and `name` — the names the host reads — and the config types (`PluginConfig`, `ResolvedPluginConfig`, `ModelOverrideConfig`, `ModelRouteConfig`, `EndpointFamily`, `KeySourcePolicy`). Everything else it used to re-export is internal now. - **`./src/*` is no longer an export.** `src/` was never in the published files, so the subpath resolved in the repository and nowhere else. - **`internalRoutes` was removed.** The per-protocol routes always mount. A profile row that still carries `internalRoutes` validates and the key is ignored. +- **Disabling or reloading the plugin restores the host methods it wrapped.** The model listing, model discovery and model-info decorations, and the plane adapters' fill, come off on a disable; a live reload replaces them instead of stacking another layer, so a setting removed in the reload stops applying. - **`@viztor/dsh-opencode` is no longer published.** Its last version's "Renamed to @viztor/dsh-opencode-patch" deprecation stands as the final message; install `dsh-opencode-patch` or `@viztor/dsh-opencode-patch` instead. ## 📜 Compatibility & Verification diff --git a/docs/engineering-notes.md b/docs/engineering-notes.md index 8dc1b02d..63d2d03a 100644 --- a/docs/engineering-notes.md +++ b/docs/engineering-notes.md @@ -572,3 +572,21 @@ Two properties decided the blast radius, and both pointed the same way: The fix is `Reflect.apply(original, adapter, [provider, model, signal])`, the idiom `decorateModelInfo` already used for the service method, plus a `route in PLANES` gate on the loop. The shared pi-ai adapter still receives the wrap through its plane routes, so the fill keeps applying to its models on every route; an adapter behind no plane route keeps its own methods. The regressions are two cases that fail on the previous code: a class-based adapter whose method reads `this`, and a foreign route that must stay unwrapped. Measured on the running deployment after the rebuild: `session/modelCatalog` answers with every group populated and `failures: []`. + +## A service method read through cordis is a new function every time + +Every disposer for the three `llm`-level patches (the listing, discovery and model-info wrappers) checked that it was still installed by reading the property back and comparing: `llm.discoverModels === decorated`. On the real host that comparison is never true. `ctx.llm` is cordis's traceable proxy: `createTraceable` answers each read of a function-valued property with `createShadowMethod`, a NEW `Proxy` around the function (the harness's `vendor/cordis/src/utils.ts`), and only a tracker marked `noShadow` is exempt, which a `Service`'s is not (`vendor/cordis/src/service.ts`, where the tracker is built). So the read is a fresh proxy every time, the check fails, and the disposer silently restores nothing. + +Measured with a real `LlmRuntime` and pi-ai mounted (tier A): after every dispose each wrapper was still an own property of the service, and a second activation's wrapper wrapped a shadow of the first's, so a live reload stacked one more layer each time. The plain-object fixtures the unit suite used could not see it, because a plain object's read IS the function. The plane adapters had a different form of the same defect: their disposer assigned the original back without checking, so disposing the first of two activations wrote the prototype's method over the second's, and every dispose left an own property shadowing the class method. + +What can be trusted on that proxy is what it does not trap. Its handler has `get`, `set` and `apply` and nothing else, so `Object.hasOwn`, `Reflect.getOwnPropertyDescriptor`, `Reflect.getPrototypeOf`, `Reflect.defineProperty` and `Reflect.deleteProperty` reach the instance itself. + +The rule that followed is `src/host/patch-method.ts`, the one way the plugin wraps a host method: + +- **Compare descriptors, not reads.** A dispose restores only when the own descriptor still holds the exact function it defined; otherwise someone wrapped on top, and ours turns into a pass-through. +- **Delete when the original was inherited.** Assigning the prototype's method back is not a restore: it leaves an own property that shadows the prototype for good. +- **A same-site install replaces instead of wrapping.** A second activation wraps the true unpatched method (the prototype's, or the recorded own original), and remembers the install it replaced, so either disposal order ends at the exact original. + +One limit is kept on purpose. A foreign plugin that reads one of these methods through `ctx.llm`, wraps it, and later puts back what it read puts back a shadow proxy, not our function. The helper finds its records by exact identity, so it treats the proxy as foreign: the proxy stays an own property and passes the call through correctly, and a later install wraps it. A symbol tag would see through the shadow proxy, but it would equally see through a foreign `new Proxy(ours, { apply })` that decorates ours, and a dispose would then remove that live wrapper. One extra pass-through is the cheaper failure. `test/patch-method.test.ts` pins it on tier A. + +`test/reload.test.ts` drives apply A, apply B, dispose A, dispose B (and the reverse) on every site, on own and inherited shapes and on the real host; it drives `apply()` itself twice, first disposed first, on a stand-in context. On the code before the helper, all four sites failed it. diff --git a/docs/improvement-plan.md b/docs/improvement-plan.md index 1ca3f82f..cb3939d8 100644 --- a/docs/improvement-plan.md +++ b/docs/improvement-plan.md @@ -24,7 +24,7 @@ Phases land as **stacked pull requests**. Each one is based on the previous phas | P2 | `feat:` a smaller public API | in review | | P-T1a | `build:` pin the host | in review | | P-T1b | `test:` a harness that is the real host | in review | -| P-H | `refactor:` one way to patch a host method | planned | +| P-H | `refactor:` one way to patch a host method | in review | | P-R1 | `feat:` the plugin owns the plane routes | planned | | P-R2 | `fix:` the endpoint family, measured | planned | | P3 | `feat:` Settings complements pi's list | planned | @@ -318,7 +318,7 @@ What stays private is pinned by contract tests against the real host packages: **How each phase opens:** - **Behaviour phases open with a red test:** P1a, P1b's fixes, P2, P-R1, P-R2, P3, P6 and P10b. -- **Pinning phases open with a green pin**, which the PR shows unchanged: P-T1a/b, P-H, P5, P7, P8, P9, P10a and P-T2. +- **Pinning phases open with a green pin**, which the PR shows unchanged: P-T1a/b, P-H, P5, P7, P8, P9, P10a and P-T2. P-H's exceptions are red on the base by design: its reload test (the defect the phase removes) and the five cases in `test/models-discovery.test.ts` marked "a deliberate change in P-H" (a refused listing define, a reinstall's `registeredRoutes`, a frozen adapter, a refused adapter method, and an inherited adapter method left with no own property after dispose). - **P-T3 and P12 are exempt.** **Characterization rows are tagged when they are recorded**, so a later phase flips named rows. The authoritative per-facet table is `S0_TODAY` in `test/support/host.ts` (each model's info, call-config verdict and wire carry their own tag); a phase's PR body cites the rows it flips from there. The three tags: @@ -377,7 +377,18 @@ What stays private is pinned by contract tests against the real host packages: - **A stored gap id blocks every edit of its plane**, `displayName` included: the strict check names the stored id first, ahead of the one being saved. - **Shapes.** A resolved profile also carries the profile's pass-through fields (`compat`, `defaultContextWindow`, `defaultInput`, `defaultMaxTokens`, `headers`, `modelOverrides`, `thinkingBudgets`), and a built pi provider exposes `getModels()` but no `api` map, so F10 is pinned by behaviour (pi's own "no API implementation" error) rather than by its keys. -6. **P-H `refactor:` one way to patch a host method.** `host/patch-method.ts` and `test/reload.test.ts` (apply A → apply B → dispose A → B intact → dispose B → originals). +6. **P-H `refactor:` one way to patch a host method.** Landed as: + - **The helper.** `src/host/patch-method.ts` (`patchMethod`) is the only way the plugin wraps a method on a host object. A dispose restores only while its own descriptor still holds its wrapper, deletes the own property when the original was inherited, and a same-site reinstall wraps the unpatched method. It carries a 100% per-file coverage floor. + - **The four migrated sites,** all in `models-discovery.ts`: `hideResponsesRoute` (the three listings), `decorateModelDiscovery`, `decorateModelInfo` and `decoratePlaneAdapters`. No wrapper's answer changed on a host that accepts the patch. The existing green pins of each site's behaviour in `test/models-discovery.test.ts` opened the phase and stayed unchanged; the five new cases there marked "a deliberate change in P-H" were added red: four over the three edge changes below (two for the adapter bullet: a frozen adapter and one refused method), and one for the helper's deletion of an inherited original (a class adapter's `resolveModel`, which the base assigned back as an own property). + - **The reload test.** `test/reload.test.ts` runs apply A → apply B → dispose A → B intact → dispose B → originals restored exactly, and the reverse disposal order, for every site on own and inherited originals and on the real host (tier A). cordis itself disposes before it re-applies (`Fiber.update` → `restart` → `_unload` → `_reload`), so the overlapping sequence models two activations live at once (two rows) and the base's stacking, where a dispose restored nothing and the next activation wrapped the leftover. A stacked install leaks wherever the decoration reads config, which every site does: a reload whose config changed (the live answer is B's alone) pins discovery (`gatewayUrls`), model-info, both plane-adapter patches (`resolveModel`'s window and `modelOf`'s levels) and the listing's `listModels` (its per-activation keyless option, `hasCredential`, rather than config); the listing's `listProviders` capture is pinned by the `isRouteRegistered` reinstall case in `test/models-discovery.test.ts`. Structurally, for every key of every site, the second install is checked to have REPLACED the first (two live entries of one site id). A foreign wrapper on top with the newest install disposed first is run too. `apply()` itself is driven twice, first disposed first, on a stand-in context. All four sites failed it before the helper. + - **Deliberate edge changes**, each pinned; the first three are not reachable with the real `LlmRuntime` and pi-ai adapter (both extensible class instances): + - **The listing install is all or nothing on a refused define.** Before, a silent refusal (`Reflect.set` answering false on a non-writable `listModels`) left the other two listings filtered and returned a disposer; now all three stay as they were, which is what a throwing refusal already did. The three listings must agree on what is hidden. + - **A refused plane-adapter method is skipped.** Before, `adapter[method] = wrapped` threw a `TypeError` out of the fetch-patch effect on a frozen adapter; now that method keeps its own, the others are patched, and a fully frozen adapter answers `undefined`. Each adapter fill is independent, so this site is per method. + - **A reinstall's `registeredRoutes` reads the host's own listing**, not the previous install's filtered one, so `isRouteRegistered` keeps finding the hidden route after a reload. + - **The measured identity trap.** On the real host the three `llm` disposers never restored: `ctx.llm` is cordis's traceable proxy, every method read answers a new shadow proxy, so `llm.x === decorated` was never true and reloads stacked wrappers. The helper reads identity from own descriptors (`docs/engineering-notes.md`). So on the real host a disable or a reload now restores the listing, discovery and model-info methods; before, each reload stacked a wrapper that kept the previous config's decisions, and a disable left them installed. The limit, recorded and pinned on tier A: a foreign plugin that wraps and later unwraps through `ctx.llm` puts back a shadow proxy, which the helper treats as foreign, so one correct pass-through stays as an own property. + - **The plane adapters' composition.** pi-ai serves both planes from one adapter, so the per-route wraps became one patch per (adapter, method) that composes the adapter's plane routes in registry order. That keeps today's nesting, including the pi-ai-first `[INTENDED_CHANGE]` row where the Go plane's repaired base is Zen's. + - **Re-wrap built, not wired.** `rewrapOnAdaptersUpdated` is tested but used by no site: on `decoratePlaneAdapters` it would wrap pi-ai's adapter in the plugin-first order and flip the `[KNOWN_DEFECT]` rows, and P-R1 deletes that site. + - **Left for P6:** the global `fetch` patch in `lifecycle.ts` stacks the same way (A, B, dispose A, dispose B leaves A's patched fetch installed), but it is a global, not a host service method. 7. **P-R1 `feat:` the plugin owns the plane routes.** - **New:** - the design above; @@ -417,6 +428,7 @@ What stays private is pinned by contract tests against the real host packages: - One resolver, for an empty or placeholder wire key only. - A parity test over the deciders, with the adapter path as its own row: a rotated key takes effect, and a service failure propagates. - A `CaptureStore` per root, evicting a key on 401. + - The global `fetch` patch reads its original inside the effect and restores only while it is still current, so apply A, apply B, dispose A, dispose B ends at the native `fetch`, in both orders. Today `lifecycle.ts` captures it in `apply()`, and the plugin's interceptor outlives a full disable after two overlapping activations (recorded in P-H). 14. **P7 `refactor:` one source per table.** One `planeForRoute` (four copies today), `formatUsd` and `specFor`; delete the duplicate `ALL_THINKING_LEVELS` and the capitalisation copies. 15. **P8 `refactor:` settings table.** It starts with a snapshot test of the derived `Config`. Then one field table drives the schema, resolver, types, card, copy and manifest. 16. **P9 `refactor:` remote seam.** `defineRemote`, `markRemote` and `callRemote`, with the parity test reading the contracts. @@ -440,7 +452,7 @@ P0 → P1a → P1b → P2 → P-T1a → P-T1b → P-H → P-R1 → P-R2 → P3 - **P1a first:** it fixes what already shipped broken. - **P-T1a and P-T1b before any routing change:** the real-host harness records today's behaviour, and P-R1 must keep it except for the rows it names. -- **P-H before P-R1:** P-R1 needs the `describe` wrapper. +- **P-H before P-R1:** P-R1 builds its `settings.describe` wrapper on `patchMethod`. - **P10a only after P5–P9:** a module is moved only once tests pin what it does. ## Test matrix @@ -482,6 +494,11 @@ P0 → P1a → P1b → P2 → P-T1a → P-T1b → P-H → P-R1 → P-R2 → P3 - **Characterization rows**, tagged, on pi 1.1.0, in both apply orders. - **Binary e2e and the keyless matrix**, as below. +**P-H** + +- **Unit:** each `patchMethod` clause, at 100% coverage; every site through apply A, apply B and both disposal orders, on own and inherited originals; a changed-config reload; a foreign wrapper with the newest install disposed first; the refusal edges. +- **Real host, tier A:** every site in both disposal orders on the traceable proxy, and the re-wrap over pi-ai's adapter. + **P-R1** - **Unit:** @@ -557,6 +574,10 @@ P0 → P1a → P1b → P2 → P-T1a → P-T1b → P-H → P-R1 → P-R2 → P3 - **Contract:** the wrapper is seen through the gateway and survives a restart. - **Binary e2e:** the Settings fetch action. +**P6** + +- **Unit:** apply A, apply B, dispose A, dispose B ends at the native `fetch`, in both orders (red on the base). + ## End-to-end infrastructure ### The `e2e-host` job (`ci.yml`) diff --git a/src/cordis-context.ts b/src/cordis-context.ts index 952495cb..2029dfa4 100644 --- a/src/cordis-context.ts +++ b/src/cordis-context.ts @@ -133,6 +133,11 @@ export interface CordisContext { info?: (msg: string, ...args: unknown[]) => void; warn?: (msg: string, ...args: unknown[]) => void; }; + /** + * Listen for a host event. Cordis answers the listener's disposer + * (`() => boolean`), which `rewrapOnAdaptersUpdated` calls to stop listening; + * a stand-in may answer nothing, so the answer is `unknown` until checked. + */ on?: ( event: string, callback: ( @@ -140,8 +145,8 @@ export interface CordisContext { next: () => unknown, ...rest: unknown[] ) => unknown, - options?: { prepend?: boolean } - ) => void; + options?: { global?: boolean; prepend?: boolean } + ) => unknown; /** * Start a plugin in this context and return its fiber. A plugin is a function * or an object with an `apply` method — which is exactly what `llm-pi-ai` diff --git a/src/host/patch-method.ts b/src/host/patch-method.ts new file mode 100644 index 00000000..15f1d19e --- /dev/null +++ b/src/host/patch-method.ts @@ -0,0 +1,364 @@ +/** + * The one way this plugin wraps a method on a host object. + * + * Every instance patch in `src/` goes through {@link patchMethod}: the listing, + * discovery and model-info wrappers on the `llm` service and the plane adapters' + * `resolveModel`, `prepareCall` and `modelOf` (all in `models-discovery.ts`). + * Wrapping an INSTANCE is enough because the host resolves a service method per + * call (F20: the gateway reads it with `Reflect.get` at dispatch), so a wrapper + * set on the instance is what every later caller runs. + * + * A host activation is not always the only one. cordis disposes before it + * re-applies (a config update is `Fiber.update` → `restart` → `_unload`, which + * awaits every disposer, → `_reload`), but two activations are live at once + * when two rows load the plugin, and on the base a dispose that silently + * restored nothing left its wrapper for the next activation to wrap. Another + * plugin may also wrap the same method on top of ours. Three rules follow, and + * the module exists to keep them in one place: + * + * - **A dispose restores only what is still ours.** When the own property no + * longer holds the wrapper this install defined, something wrapped on top of + * it, and that wrapper captured ours: ours turns into a pass-through instead + * of being torn out from under it. A pass-through hands the call to the + * newest same-site install it replaced that is still live, and to the method + * it wrapped only when none is. + * - **An inherited original is restored by deletion.** Assigning the prototype's + * method back leaves an own property that shadows the prototype for good; the + * exact original state is "no own property", so the property is deleted. An + * own original gets its exact descriptor back, attributes included. + * - **A second install of the same site wraps the UNPATCHED method**, never the + * previous install: the prototype's method when the original was inherited, + * the recorded own original otherwise. Each install remembers the newest live + * same-site install it replaced, so either disposal order ends at the exact + * original. + * + * A retired install keeps no wrapper alive: its record drops the function `wrap` + * built (which closes over that activation's context and config), and the + * links between same-site records drop the retired ones the next time a call, + * an install or a dispose walks them, so a long-running host that reloads the + * plugin holds the live installs and nothing else. A retired + * wrapper of ours that becomes the current value again (a foreign wrapper put + * back the EXACT function on its own teardown, read through an untrapped handle + * or an own descriptor) is nobody's: the next dispose of this key collapses it + * to what it replaced. + * + * The limit of that collapse: a foreign plugin that reads and later puts back + * through `ctx.llm` puts back a cordis shadow-method proxy, not our function, + * and a record is found by exact identity, so the proxy reads as foreign. It + * stays an own property and a correct pass-through (to the retired trampoline, + * which falls through as any retired install does), and a later install wraps + * it; `test/patch-method.test.ts` pins this on tier A. + * Seeing through it is not offered on purpose: a tag read through the proxy + * would read just as well through a foreign `new Proxy(ours, { apply })` that + * DECORATES ours, and a dispose would then tear that live wrapper out. + * + * **Identity is read from own descriptors, never with `===` on a property + * read.** On the real host `ctx.llm` is cordis's traceable proxy: a read of a + * function-valued property answers a NEW shadow-method proxy every time + * (`createTraceable` / `createShadowMethod`, the harness's + * `vendor/cordis/src/utils.ts:156-218`), and a `Service`'s tracker carries no + * `noShadow` (`vendor/cordis/src/service.ts:46-49`). So `llm.discoverModels === + * decorated` is never true there, and a disposer that checked it never + * restored: measured with a real `LlmRuntime`, every wrapper stayed installed + * after its dispose and a reload stacked the next one on top. + * `Reflect.getOwnPropertyDescriptor`, `Reflect.getPrototypeOf`, + * `Reflect.defineProperty` and `Reflect.deleteProperty` are not trapped by that + * proxy, so they see the instance itself. + * + * @module dsh-opencode-patch/host/patch-method + */ + +import type { CordisContext } from "../cordis-context.ts"; +import { isFunctionLike } from "../utils/guards.ts"; + +/** The method a wrapper delegates to: the host's own, before any of ours. */ +export type UnpatchedMethod = (...args: unknown[]) => unknown; + +/** What names one patch site. */ +export interface PatchMethodOptions { + /** + * The SITE, not the install: two activations of the same site share it, which + * is how the second one knows to replace the first rather than wrap it. + */ + readonly id: string; +} + +/** Where the true original lives: on the prototype, or in this own descriptor. */ +type Slot = + | { readonly inherited: true } + | { readonly inherited: false; readonly descriptor: PropertyDescriptor }; + +/** One install, keyed by the function it defined. */ +interface PatchRecord { + readonly id: string; + readonly key: string; + readonly slot: Slot; + readonly installed: (...args: never[]) => unknown; + /** What `wrap` built: held while live, dropped on retire. */ + impl: ((...args: never[]) => unknown) | undefined; + /** The newest same-site install this one replaced, which it hands back to. */ + replaced: PatchRecord | undefined; + live: boolean; +} + +/** + * Every install's record, by the function it defined. A WeakMap, so an entry + * lives exactly as long as its function is reachable from some host object. + */ +const records = new WeakMap(); + +/** The record of a function this module installed under `key`, if it is one. */ +const recordOf = (value: unknown, key: string): PatchRecord | undefined => { + if (!isFunctionLike(value)) { + return undefined; + } + const record = records.get(value); + return record?.key === key ? record : undefined; +}; + +/** + * The method the slot names: the own original's value, or the value of the + * first data property up the prototype chain. An accessor, own or inherited, + * has no value and so answers `undefined`, which refuses the install: a getter + * is never run, and a snapshot of what it answered is never installed. + */ +const unpatchedFor = (target: object, key: string, slot: Slot): unknown => { + if (!slot.inherited) { + return slot.descriptor.value; + } + for ( + let proto = Reflect.getPrototypeOf(target); + proto !== null; + proto = Reflect.getPrototypeOf(proto) + ) { + const descriptor = Reflect.getOwnPropertyDescriptor(proto, key); + if (descriptor !== undefined) { + return descriptor.value; + } + } + return undefined; +}; + +/** + * The newest live record from `start` down its `replaced` chain, relinking each + * live record in it past the retired ones, so a retired record is reachable from + * no live one and is released with the function that defined it. + */ +const compactFrom = ( + start: PatchRecord | undefined +): PatchRecord | undefined => { + let head: PatchRecord | undefined; + let last: PatchRecord | undefined; + for (let record = start; record !== undefined; record = record.replaced) { + if (!record.live) { + continue; + } + if (last === undefined) { + head = record; + } else { + last.replaced = record; + } + last = record; + } + if (last !== undefined) { + last.replaced = undefined; + } + return head; +}; + +/** + * Put back what was there before `record`: the newest live same-site install it + * replaced, else the true original. A retired wrapper of ours found as the + * original is nobody's original, so it is collapsed to what IT replaced. + */ +const restoreFor = ( + target: object, + key: string, + record: PatchRecord, + current: PropertyDescriptor +): void => { + const heir = compactFrom(record.replaced); + record.replaced = heir; + if (heir !== undefined) { + Reflect.defineProperty(target, key, { ...current, value: heir.installed }); + return; + } + const { slot } = record; + if (slot.inherited) { + Reflect.deleteProperty(target, key); + return; + } + const retired = recordOf(slot.descriptor.value, key); + if (retired !== undefined && !retired.live) { + restoreFor(target, key, retired, current); + return; + } + Reflect.defineProperty(target, key, slot.descriptor); +}; + +/** + * Wrap `target[key]` with what `wrap` builds from the unpatched method. + * + * `wrap` runs once, before the property is defined, and its result is called + * with the caller's receiver. What is defined is a plain (never `async`) + * trampoline in front of it, so a synchronous method stays synchronous and a + * retired install can fall through: to the newest live same-site install it + * replaced, else to the method it wrapped. + * + * @param target - the host object (a service instance, or an adapter). + * @param key - the method's name. + * @param wrap - builds the wrapper from the unpatched method. + * @param options - the patch site's id. + * @returns the disposer, or `undefined` when nothing was installed: the key + * holds no method (an accessor included, own or inherited), or the target + * refused the define (a frozen method, a non-extensible object, a refusing + * proxy). + */ +export const patchMethod = ( + target: object, + key: string, + wrap: (unpatched: UnpatchedMethod) => (...args: never[]) => unknown, + options: PatchMethodOptions +): (() => void) | undefined => { + const own = Reflect.getOwnPropertyDescriptor(target, key); + const ours = recordOf(own?.value, key); + const same = ours?.id === options.id ? ours : undefined; + const slot: Slot = + same?.slot ?? + (own === undefined + ? { inherited: true } + : { descriptor: own, inherited: false }); + const unpatched = unpatchedFor(target, key, slot); + if (!isFunctionLike(unpatched)) { + return undefined; + } + // Called only after `record` below exists: the define comes after it. + const installed = function installed( + this: unknown, + ...args: never[] + ): unknown { + if (record.impl !== undefined) { + return Reflect.apply(record.impl, this, args); + } + const heir = compactFrom(record.replaced); + record.replaced = heir; + return heir === undefined + ? Reflect.apply(unpatched, this, args) + : Reflect.apply(heir.installed, this, args); + }; + const record: PatchRecord = { + id: options.id, + impl: wrap(unpatched), + installed, + key, + live: true, + replaced: compactFrom(same), + slot, + }; + let defined: boolean; + try { + defined = Reflect.defineProperty( + target, + key, + own === undefined + ? { + configurable: true, + enumerable: true, + value: installed, + writable: true, + } + : { ...own, value: installed } + ); + } catch { + defined = false; + } + if (!defined) { + return undefined; + } + records.set(installed, record); + return () => { + record.live = false; + record.impl = undefined; + // Whatever is current now: ours (this install, or a retired one of ours a + // foreign wrapper put back) is collapsed; anything else wrapped on top and + // keeps what it captured. + const current = Reflect.getOwnPropertyDescriptor(target, key); + const top = recordOf(current?.value, key); + if (current === undefined || top === undefined || top.live) { + return; + } + restoreFor(target, key, top, current); + }; +}; + +/** + * What this module knows about a function it installed: its site, whether it is + * live, whether it still holds the wrapper `wrap` built, and the same-site + * installs it would hand back to, newest first. `undefined` for any other value. + * For tests and diagnostics; it changes nothing. + */ +export const inspectPatch = ( + value: unknown, + key: string +): + | readonly { id: string; live: boolean; holdsWrapper: boolean }[] + | undefined => { + const chain: { id: string; live: boolean; holdsWrapper: boolean }[] = []; + for ( + let record = recordOf(value, key); + record !== undefined; + record = record.replaced + ) { + chain.push({ + holdsWrapper: record.impl !== undefined, + id: record.id, + live: record.live, + }); + } + return chain.length === 0 ? undefined : chain; +}; + +/** + * Install now, and again after every `llm/adapters-updated`, for a site that + * has to follow the adapter registry rather than the registry it saw first. + * + * Wired to no site yet: on the plane-adapter decoration it would wrap pi-ai's + * shared adapter in the plugin-first order too, and flip the host + * characterization's `[KNOWN_DEFECT]` rows. + * + * @param ctx - the context whose (global) registry events to follow. + * @param install - one install; answers its disposer, or `undefined`. + * @returns the disposer: stops listening and disposes the current install. + */ +export const rewrapOnAdaptersUpdated = ( + ctx: Pick, + install: () => (() => void) | undefined +): (() => void) => { + let active = true; + let current = install(); + const off: unknown = ctx.on?.( + "llm/adapters-updated", + () => { + if (!active) { + return; + } + current?.(); + current = install(); + }, + // Defensive: today's emitter dispatches this event with no `thisArg`, so + // cordis applies no filter and a local listener hears it too. `global` + // keeps it heard if the emitter ever passes a filtered context. + { global: true } + ); + return () => { + if (!active) { + return; + } + active = false; + if (isFunctionLike(off)) { + off(); + } + current?.(); + current = undefined; + }; +}; diff --git a/src/models-discovery.ts b/src/models-discovery.ts index d5e4a731..6e114369 100644 --- a/src/models-discovery.ts +++ b/src/models-discovery.ts @@ -19,6 +19,7 @@ import type { ResolvedPluginConfig } from "./config.ts"; import type { CordisContext } from "./cordis-context.ts"; import { declaredProviderModels } from "./go-discovery.ts"; +import { patchMethod, type UnpatchedMethod } from "./host/patch-method.ts"; import { catalogPlaneForRoute, type CatalogPlane, @@ -45,6 +46,18 @@ import { import { isRecord } from "./utils/guards.ts"; import { isGoPlaneUrl } from "./utils/request-shape.ts"; +/** + * The patch sites this module installs, one id each. Two activations of the + * same site share it, which is how a reload replaces a wrapper instead of + * stacking another one on it (see `host/patch-method.ts`). + */ +const PATCH_SITE = { + discovery: "dsh-opencode-patch/model-discovery", + listing: "dsh-opencode-patch/listing", + modelInfo: "dsh-opencode-patch/model-info", + planeAdapter: "dsh-opencode-patch/plane-adapter", +} as const; + export interface DiscoveryCandidate { contextWindow?: number; id: string; @@ -252,6 +265,12 @@ const withoutRoute = (value: unknown, key: "id" | "provider"): unknown => ? value.filter((entry) => !(isRecord(entry) && isInternalRoute(entry[key]))) : value; +/** The configurable directory, without the internal routes. */ +const wrapListConfigurableProviders = (unpatched: UnpatchedMethod) => + function listConfigurableProviders(this: unknown): unknown { + return withoutRoute(Reflect.apply(unpatched, this, []), "provider"); + }; + /** * Whether a model can be served, judged by the SDK the PLANE IN HAND declares. * @@ -384,6 +403,13 @@ const costsMoney = (plane: CatalogPlane, id: string): boolean => { return cost !== undefined && cost.input > 0; }; +/** The listing methods the patch wraps, in the order it installs them. */ +const LISTING_KEYS = [ + "listProviders", + "listConfigurableProviders", + "listModels", +] as const; + export const hideResponsesRoute = ( ctx: CordisContext, config: ResolvedPluginConfig, @@ -393,64 +419,47 @@ export const hideResponsesRoute = ( if (llm === undefined) { return undefined; } - const { - listConfigurableProviders: originalListConfigurableProviders, - listModels: originalListModels, - listProviders: originalListProviders, - } = llm; - if ( - typeof originalListProviders !== "function" && - typeof originalListConfigurableProviders !== "function" && - typeof originalListModels !== "function" - ) { + // Today's install order, and only the methods the host has. + const keys = LISTING_KEYS.filter((key) => typeof llm[key] === "function"); + if (keys.length === 0) { return undefined; } - if (typeof originalListProviders === "function") { - registeredRoutes = { list: originalListProviders, receiver: llm }; - } - const patched: Record = {}; - if (typeof originalListProviders === "function") { - patched.listProviders = function listProviders(this: unknown): unknown { - return withoutRoute(Reflect.apply(originalListProviders, this, []), "id"); - }; - } - if (typeof originalListConfigurableProviders === "function") { - patched.listConfigurableProviders = function listConfigurableProviders( - this: unknown - ): unknown { - return withoutRoute( - Reflect.apply(originalListConfigurableProviders, this, []), - "provider" - ); + const wrapListProviders = (unpatched: UnpatchedMethod) => { + // Captured as the wrapper is built, which is the TRUE unfiltered listing: + // a reload's second install is handed the host's own method, not the first + // install's filtered one. + registeredRoutes = { list: unpatched, receiver: llm }; + return function listProviders(this: unknown): unknown { + return withoutRoute(Reflect.apply(unpatched, this, []), "id"); }; - } - if (typeof originalListModels === "function") { - // **The deployment's model list is the whole list.** This route's `models:` - // array is a decision the operator made, and the picker must show it and - // nothing else. - // - // An earlier revision merged the plane's whole catalog in here, so that - // models living on the hidden protocol routes would be reachable. That was - // wrong, and the way it failed is worth keeping: the join was invisible on - // screen (nobody can tell 27 rows from 88 unless they counted the config), it - // overrode every explicit `models:` list, and it put models the operator never - // offered in front of them — which is how a request for a model the account - // cannot use became the reported bug. A route's list is not ours to widen. - // - // What remains is a filter, and only that: a model the plane cannot serve is - // dropped, because offering one produces a failure with nothing in the row to - // explain it. `internalRouteFor` still redirects a DECLARED model whose - // protocol differs, so a Responses / Messages / Google id the operator lists - // keeps working — it just has to be listed. - patched.listModels = async function listModels( + }; + // **The deployment's model list is the whole list.** This route's `models:` + // array is a decision the operator made, and the picker must show it and + // nothing else. + // + // An earlier revision merged the plane's whole catalog in here, so that + // models living on the hidden protocol routes would be reachable. That was + // wrong, and the way it failed is worth keeping: the join was invisible on + // screen (nobody can tell 27 rows from 88 unless they counted the config), it + // overrode every explicit `models:` list, and it put models the operator never + // offered in front of them — which is how a request for a model the account + // cannot use became the reported bug. A route's list is not ours to widen. + // + // What remains is a filter, and only that: a model the plane cannot serve is + // dropped, because offering one produces a failure with nothing in the row to + // explain it. `internalRouteFor` still redirects a DECLARED model whose + // protocol differs, so a Responses / Messages / Google id the operator lists + // keeps working — it just has to be listed. + const wrapListModels = (unpatched: UnpatchedMethod) => + async function listModels( this: unknown, provider: string ): Promise { if (isInternalRoute(provider)) { return []; } - const models = await Reflect.apply(originalListModels, this, [provider]); + const models = await Reflect.apply(unpatched, this, [provider]); // **Only the routes this plugin claims.** A provider the deployment did // not hand it — a direct `anthropic` or `openai` route — is answered by its // own adapter and nothing else: the servability rule below reads OUR @@ -501,30 +510,30 @@ export const hideResponsesRoute = ( return typeof id !== "string" || !costsMoney(plane, id); }); }; - } - const installed: string[] = []; - const restoreFrom: Record = { - listConfigurableProviders: originalListConfigurableProviders, - listModels: originalListModels, - listProviders: originalListProviders, + const factories = { + listConfigurableProviders: wrapListConfigurableProviders, + listModels: wrapListModels, + listProviders: wrapListProviders, }; - try { - for (const [key, value] of Object.entries(patched)) { - Reflect.set(llm, key, value); - installed.push(key); - } - } catch { - for (const key of installed) { - Reflect.set(llm, key, restoreFrom[key]); + + const stops: (() => void)[] = []; + for (const key of keys) { + const stop = patchMethod(llm, key, factories[key], { + id: PATCH_SITE.listing, + }); + if (stop === undefined) { + // All or nothing: a host that refuses one listing keeps all three. + for (const undo of stops.toReversed()) { + undo(); + } + return undefined; } - return undefined; + stops.push(stop); } return () => { - for (const key of installed) { - if (Reflect.get(llm, key) === patched[key]) { - Reflect.set(llm, key, restoreFrom[key]); - } + for (const undo of stops.toReversed()) { + undo(); } }; }; @@ -548,35 +557,30 @@ export const decorateModelDiscovery = ( if (llm === undefined || typeof llm.discoverModels !== "function") { return undefined; } - const discover = llm.discoverModels; - const decorated = async ( - settingsNs: string, - request?: unknown, - signal?: AbortSignal - ): Promise => { - const discovered = await Reflect.apply(discover, llm, [ - settingsNs, - request, - signal, - ]); - const provider = resolveDiscoveryProvider(request, config); - if (provider === undefined) { - return discovered; - } - return mergeDiscoveredModels(discovered, provider); - }; - - try { - llm.discoverModels = decorated; - } catch { - // A non-extensible Host service must not prevent request patching. - return undefined; - } - return () => { - if (llm.discoverModels === decorated) { - llm.discoverModels = discover; - } - }; + // A non-extensible Host service answers `undefined` here, and must not + // prevent request patching. + return patchMethod( + llm, + "discoverModels", + (discover) => + async ( + settingsNs: string, + request?: unknown, + signal?: AbortSignal + ): Promise => { + const discovered: unknown = await Reflect.apply(discover, llm, [ + settingsNs, + request, + signal, + ]); + const provider = resolveDiscoveryProvider(request, config); + if (provider === undefined) { + return discovered; + } + return mergeDiscoveredModels(discovered, provider); + }, + { id: PATCH_SITE.discovery } + ); }; /** Every pi thinking level, in escalation order, `off` first. */ @@ -707,6 +711,13 @@ export const resolveEffectiveContextWindow = ( * the original runs with the adapter as its receiver, because all three are * class methods that read `this`. * + * Each (adapter, method) is ONE patch, installed through `patchMethod` + * (`host/patch-method.ts`). pi-ai serves both plane routes from one adapter + * instance, so that patch composes the adapter's plane routes in registry + * order, each fill wrapping the previous one's - the nesting the per-route wraps + * always had, and the one the host characterization pins - and a reload + * replaces the whole composition instead of wrapping it again. + * * @param llm - the Host's LLM service. * @param config - the resolved config, for the ids a deployment withholds. * @returns the disposer restoring the original methods, or `undefined`. @@ -724,7 +735,12 @@ export const decoratePlaneAdapters = ( if (!(adapters instanceof Map)) { return undefined; } - const restored: (() => void)[] = []; + // pi-ai serves both planes from ONE adapter instance, so the routes are + // grouped per adapter, in registry order, and each plane's fill wraps the + // previous plane's: the later route's fill runs outermost and sees what the + // earlier one answered. One patch per (adapter, method) composes them, which + // is what a reload replaces as a whole. + const planeRoutes = new Map, string[]>(); for (const [route, registration] of adapters) { const adapter = registration?.adapter; if (adapter === undefined || adapter === null) { @@ -737,191 +753,216 @@ export const decoratePlaneAdapters = ( if (!(route in PLANES)) { continue; } - // The plane comes from the ROUTE, so a Go model is filled from the Go - // catalog and a Zen one from Zen's - the same rule `findModelSpecOn` applies. - const plane = catalogPlaneForRoute(route); + planeRoutes.set(adapter, [...(planeRoutes.get(adapter) ?? []), route]); + } + + const stops: (() => void)[] = []; + for (const [adapter, routes] of planeRoutes) { for (const method of ["resolveModel", "prepareCall"] as const) { - const original = adapter[method]; - if (typeof original !== "function") { - continue; - } - const wrapped = async ( - provider: string, - model: string, - signal?: AbortSignal - ): Promise => { - // Go's shipped catalog cannot resolve Gemini before the stream hook runs. - // Ask the native bridge during model preparation as well as generation. - if ( - provider === "opencode-go" && - internalRouteFor( - provider, - model, - findModelSpecOn("go", model)?.provider_npm, - config.modelRoutes[model]?.api - ) === GO_GOOGLE_INTERNAL_ROUTE - ) { - const target = adapters.get(GO_GOOGLE_INTERNAL_ROUTE)?.adapter; - const resolve = target?.[method]; - if (target !== undefined && typeof resolve === "function") { - const prepared: unknown = await Reflect.apply(resolve, target, [ - GO_GOOGLE_INTERNAL_ROUTE, - model, - signal, - ]); - if (isRecord(prepared)) - return method === "prepareCall" && isRecord(prepared.model) - ? { ...prepared, model: { ...prepared.model, provider } } - : { ...prepared, provider }; - return prepared; - } - } - // The original with the adapter as its receiver: both methods are class - // methods that read `this` (pi-ai's open with `this.current()`), and a - // detached call fails the provider's whole catalog load. - const answered: unknown = await Reflect.apply(original, adapter, [ - provider, - model, - signal, - ]); - const spec = findModelSpecOn(plane, model); - if (spec === undefined) { - return answered; - } - // Only what the adapter left BLANK. An adapter that answers keeps its - // answer, because that is what dispatch will honour. - const override = config.modelOverrides[model]; - const record = isRecord(answered) ? answered : {}; - const existingContext = isRecord(record.context) ? record.context : {}; - const requestedWindow = - override?.contextWindow ?? - (typeof existingContext.contextWindow === "number" - ? existingContext.contextWindow - : undefined); - // What the deployment declared outranks the catalog; the catalog is the - // fallback when nothing did. - const declaredWindow = requestedWindow ?? spec.context_window; - const effectiveWindow = - declaredWindow > 0 - ? resolveEffectiveContextWindow( - declaredWindow, - config.maxContextWindow - ) - : undefined; - const modelName = - override?.name ?? - (typeof record.name === "string" ? record.name : spec.name); - const { defaultMaxTokens: recordMaxTokens } = record; - let inputModalities: ("text" | "image")[] = sanitizeModalities( - spec.input_modalities - ); - if (override?.inputModalities !== undefined) { - inputModalities = [...override.inputModalities]; - } else if (Array.isArray(record.inputModalities)) { - inputModalities = sanitizeModalities(record.inputModalities); - } + const stop = patchMethod( + adapter, + method, + (unpatched) => { + // The original with the adapter as its receiver: both methods are + // class methods that read `this` (pi-ai's open with + // `this.current()`), and a detached call fails the provider's whole + // catalog load. + let chain = ( + provider: string, + model: string, + signal?: AbortSignal + ): unknown => + Reflect.apply(unpatched, adapter, [provider, model, signal]); + for (const route of routes) { + // Captured per route: `chain` itself is reassigned below, and a + // fill that called it would call itself. + const inner = chain; + // The plane comes from the ROUTE, so a Go model is filled from the + // Go catalog and a Zen one from Zen's - the same rule + // `findModelSpecOn` applies. + const plane = catalogPlaneForRoute(route); + chain = async ( + provider: string, + model: string, + signal?: AbortSignal + ): Promise => { + // Go's shipped catalog cannot resolve Gemini before the stream hook runs. + // Ask the native bridge during model preparation as well as generation. + if ( + provider === "opencode-go" && + internalRouteFor( + provider, + model, + findModelSpecOn("go", model)?.provider_npm, + config.modelRoutes[model]?.api + ) === GO_GOOGLE_INTERNAL_ROUTE + ) { + const target = adapters.get(GO_GOOGLE_INTERNAL_ROUTE)?.adapter; + const resolve = target?.[method]; + if (target !== undefined && typeof resolve === "function") { + const prepared: unknown = await Reflect.apply( + resolve, + target, + [GO_GOOGLE_INTERNAL_ROUTE, model, signal] + ); + if (isRecord(prepared)) + return method === "prepareCall" && isRecord(prepared.model) + ? { ...prepared, model: { ...prepared.model, provider } } + : { ...prepared, provider }; + return prepared; + } + } + // What the earlier plane's fill answered, or the adapter's own method + // when this is the first plane on the adapter. + const answered: unknown = await inner(provider, model, signal); + const spec = findModelSpecOn(plane, model); + if (spec === undefined) { + return answered; + } + // Only what the adapter left BLANK. An adapter that answers keeps its + // answer, because that is what dispatch will honour. + const override = config.modelOverrides[model]; + const record = isRecord(answered) ? answered : {}; + const existingContext = isRecord(record.context) + ? record.context + : {}; + const requestedWindow = + override?.contextWindow ?? + (typeof existingContext.contextWindow === "number" + ? existingContext.contextWindow + : undefined); + // What the deployment declared outranks the catalog; the catalog is the + // fallback when nothing did. + const declaredWindow = requestedWindow ?? spec.context_window; + const effectiveWindow = + declaredWindow > 0 + ? resolveEffectiveContextWindow( + declaredWindow, + config.maxContextWindow + ) + : undefined; + const modelName = + override?.name ?? + (typeof record.name === "string" ? record.name : spec.name); + const { defaultMaxTokens: recordMaxTokens } = record; + let inputModalities: ("text" | "image")[] = sanitizeModalities( + spec.input_modalities + ); + if (override?.inputModalities !== undefined) { + inputModalities = [...override.inputModalities]; + } else if (Array.isArray(record.inputModalities)) { + inputModalities = sanitizeModalities(record.inputModalities); + } - let defaultMaxTokens: number | undefined; - if (override?.maxTokens !== undefined) { - defaultMaxTokens = override.maxTokens; - } else if (typeof recordMaxTokens === "number") { - defaultMaxTokens = recordMaxTokens; - } else if (spec.max_output_tokens > 0) { - defaultMaxTokens = spec.max_output_tokens; - } + let defaultMaxTokens: number | undefined; + if (override?.maxTokens !== undefined) { + defaultMaxTokens = override.maxTokens; + } else if (typeof recordMaxTokens === "number") { + defaultMaxTokens = recordMaxTokens; + } else if (spec.max_output_tokens > 0) { + defaultMaxTokens = spec.max_output_tokens; + } - // The same pair the picker reads: `thinkingLevelMapFor` decides which - // levels exist, `effortsFromLevelMap` renders them, so a level offered - // here is a level the composer shows. - let supports: { id: string; name: string }[] | undefined; - if (override?.reasoningEfforts !== undefined) { - supports = override.reasoningEfforts.map((id) => ({ - id, - name: - id === "off" - ? "Off" - : `${id.charAt(0).toUpperCase()}${id.slice(1)}`, - })); - } else if (record.reasoning === undefined) { - const map = thinkingLevelMapFor(spec); - if (map !== undefined) { - supports = effortsFromLevelMap(map); - } - } + // The same pair the picker reads: `thinkingLevelMapFor` decides which + // levels exist, `effortsFromLevelMap` renders them, so a level offered + // here is a level the composer shows. + let supports: { id: string; name: string }[] | undefined; + if (override?.reasoningEfforts !== undefined) { + supports = override.reasoningEfforts.map((id) => ({ + id, + name: + id === "off" + ? "Off" + : `${id.charAt(0).toUpperCase()}${id.slice(1)}`, + })); + } else if (record.reasoning === undefined) { + const map = thinkingLevelMapFor(spec); + if (map !== undefined) { + supports = effortsFromLevelMap(map); + } + } - const outRecord: Record = { - ...record, - inputModalities, - name: modelName, - }; - if (record.provider === undefined) { - outRecord.provider = provider; - } - if (record.id === undefined) { - outRecord.id = model; - } - if (effectiveWindow !== undefined) { - outRecord.context = { - ...existingContext, - contextWindow: effectiveWindow, - }; - } - if (defaultMaxTokens !== undefined) { - outRecord.defaultMaxTokens = defaultMaxTokens; - } - if (supports !== undefined && supports.length > 0) { - outRecord.reasoning = { efforts: supports }; - } - // **Route the model on the object the host dispatches on.** - // `dsh-llm-pi-ai` selects the implementation per model — - // `stream: (model, …) => input.api.stream(model, …)`, with the protocol - // materialized as `request.api ?? base?.api ?? routeApi` — so a single - // provider route serves every protocol its models name. That makes this - // object the place to say it, and it is why a model needing Anthropic does - // not need a provider of its own. - // - // The catalog's protocol is the default; `modelRoutes` overrides it, which - // is the layer for an endpoint the shim gets wrong. - const declaredRoute = config.modelRoutes[model]; - const protocol = declaredRoute?.api ?? protocolFor(spec); - if (protocol !== undefined) { - outRecord.api = protocol; - } - // **The endpoint: a declaration outranks our measurement, except where the - // declaration is the vendor's own retired path.** pi's installed catalog - // still says `https://opencode.ai/zen`, and a profile written for the old - // gateway says `…/zen/v1`; both are aliases today (measured: `/zen/v1/…` - // and `/inference/openai/v1/…` both answer 200), but the per-family - // addresses are what the gateway serves, and an anthropic model on a - // `/zen` base is simply wrong. - // - // Anything else is left alone: a deployment pointing this route at its own - // proxy must not have that endpoint rewritten by a bundled snapshot. - const existingBase = - typeof record.baseUrl === "string" - ? record.baseUrl.trim().replace(/\/+$/u, "") - : undefined; - const legacyBase = - existingBase !== undefined && LEGACY_PLANE_BASES.has(existingBase); - // The catalog's per-family endpoint is a REPAIR for a retired base and a - // FALLBACK when none was given; an endpoint the deployment wrote is kept. - const keepExisting = - existingBase !== undefined && !legacyBase && protocol !== undefined; - const measured = - protocol === undefined - ? undefined - : baseForProtocol(route, protocol, config.endpoint); - const baseURL = - declaredRoute?.baseURL ?? (keepExisting ? existingBase : measured); - if (baseURL !== undefined) { - outRecord.baseUrl = baseURL; - } - return outRecord; - }; - adapter[method] = wrapped; - restored.push(() => { - adapter[method] = original; - }); + const outRecord: Record = { + ...record, + inputModalities, + name: modelName, + }; + if (record.provider === undefined) { + outRecord.provider = provider; + } + if (record.id === undefined) { + outRecord.id = model; + } + if (effectiveWindow !== undefined) { + outRecord.context = { + ...existingContext, + contextWindow: effectiveWindow, + }; + } + if (defaultMaxTokens !== undefined) { + outRecord.defaultMaxTokens = defaultMaxTokens; + } + if (supports !== undefined && supports.length > 0) { + outRecord.reasoning = { efforts: supports }; + } + // **Route the model on the object the host dispatches on.** + // `dsh-llm-pi-ai` selects the implementation per model — + // `stream: (model, …) => input.api.stream(model, …)`, with the protocol + // materialized as `request.api ?? base?.api ?? routeApi` — so a single + // provider route serves every protocol its models name. That makes this + // object the place to say it, and it is why a model needing Anthropic does + // not need a provider of its own. + // + // The catalog's protocol is the default; `modelRoutes` overrides it, which + // is the layer for an endpoint the shim gets wrong. + const declaredRoute = config.modelRoutes[model]; + const protocol = declaredRoute?.api ?? protocolFor(spec); + if (protocol !== undefined) { + outRecord.api = protocol; + } + // **The endpoint: a declaration outranks our measurement, except where the + // declaration is the vendor's own retired path.** pi's installed catalog + // still says `https://opencode.ai/zen`, and a profile written for the old + // gateway says `…/zen/v1`; both are aliases today (measured: `/zen/v1/…` + // and `/inference/openai/v1/…` both answer 200), but the per-family + // addresses are what the gateway serves, and an anthropic model on a + // `/zen` base is simply wrong. + // + // Anything else is left alone: a deployment pointing this route at its own + // proxy must not have that endpoint rewritten by a bundled snapshot. + const existingBase = + typeof record.baseUrl === "string" + ? record.baseUrl.trim().replace(/\/+$/u, "") + : undefined; + const legacyBase = + existingBase !== undefined && + LEGACY_PLANE_BASES.has(existingBase); + // The catalog's per-family endpoint is a REPAIR for a retired base and a + // FALLBACK when none was given; an endpoint the deployment wrote is kept. + const keepExisting = + existingBase !== undefined && + !legacyBase && + protocol !== undefined; + const measured = + protocol === undefined + ? undefined + : baseForProtocol(route, protocol, config.endpoint); + const baseURL = + declaredRoute?.baseURL ?? + (keepExisting ? existingBase : measured); + if (baseURL !== undefined) { + outRecord.baseUrl = baseURL; + } + return outRecord; + }; + } + return chain; + }, + { id: PATCH_SITE.planeAdapter } + ); + if (stop !== undefined) { + stops.push(stop); + } } // The half dispatch validates against, and the reason a filled picker was @@ -935,89 +976,99 @@ export const decoratePlaneAdapters = ( // The map is the one the composer already renders: `thinkingLevelMapFor` is // the same derivation `decorateModelInfo` uses to build the picker's Effort // row, so the two read one source rather than two that have to agree. - const originalModelOf = adapter.modelOf; - if (typeof originalModelOf === "function") { - const wrappedModelOf = ( - snapshot: unknown, - provider: string, - model: string - ): unknown => { - const answered: unknown = Reflect.apply(originalModelOf, adapter, [ - snapshot, - provider, - model, - ]); - if (!isRecord(answered)) { - return answered; - } - const spec = findModelSpecOn(plane, model); - if (spec === undefined) { - return answered; - } - const override = config.modelOverrides[model]; - const levels = - override?.reasoningEfforts === undefined - ? thinkingLevelMapFor(spec) - : Object.fromEntries( - override.reasoningEfforts.map((id) => [id, id] as const) - ); - // No opinion: the catalog does not call the model a reasoning one, and - // the deployment named no level for it either. - if (levels === undefined || Object.keys(levels).length === 0) { - return answered; - } - const declared = isRecord(answered.thinkingLevelMap) - ? answered.thinkingLevelMap - : {}; - // pi's acceptance rule, which is what `resolveReasoningLevel` throws - // against (`getSupportedThinkingLevels`): a model that reasons takes a - // level whose entry is not `null`, and `xhigh`/`max` need an entry to - // exist at all - an absent entry there is not a level, while for the - // base levels it means the level's own name on the wire. - const taken = (level: string): boolean => { - if (answered.reasoning !== true) { - return false; - } - const entry = declared[level]; - if (entry === null) { - return false; - } - return level === "xhigh" || level === "max" - ? entry !== undefined - : true; - }; - const merged: Record = { ...declared }; - let filled = false; - for (const [level, wire] of Object.entries(levels)) { - // A level the model already takes keeps the adapter's own answer: one - // that spells the level itself keeps that spelling, and one that leaves - // it out keeps the level's name on the wire. Only what it REFUSES is - // filled, so the levels that work today keep working byte for byte. - if (wire === null || taken(level)) { - continue; - } - merged[level] = wire; - filled = true; - } - // Nothing to add - and so nothing to say about `reasoning` either, since - // every level is only "taken" for a model that already reasons: hand back - // the adapter's own object rather than an equal copy of it. - if (!filled) { - return answered; + const stopModelOf = patchMethod( + adapter, + "modelOf", + (unpatched) => { + let chain = ( + snapshot: unknown, + provider: string, + model: string + ): unknown => + Reflect.apply(unpatched, adapter, [snapshot, provider, model]); + for (const route of routes) { + const inner = chain; + const plane = catalogPlaneForRoute(route); + chain = ( + snapshot: unknown, + provider: string, + model: string + ): unknown => { + const answered: unknown = inner(snapshot, provider, model); + if (!isRecord(answered)) { + return answered; + } + const spec = findModelSpecOn(plane, model); + if (spec === undefined) { + return answered; + } + const override = config.modelOverrides[model]; + const levels = + override?.reasoningEfforts === undefined + ? thinkingLevelMapFor(spec) + : Object.fromEntries( + override.reasoningEfforts.map((id) => [id, id] as const) + ); + // No opinion: the catalog does not call the model a reasoning one, and + // the deployment named no level for it either. + if (levels === undefined || Object.keys(levels).length === 0) { + return answered; + } + const declared = isRecord(answered.thinkingLevelMap) + ? answered.thinkingLevelMap + : {}; + // pi's acceptance rule, which is what `resolveReasoningLevel` throws + // against (`getSupportedThinkingLevels`): a model that reasons takes a + // level whose entry is not `null`, and `xhigh`/`max` need an entry to + // exist at all - an absent entry there is not a level, while for the + // base levels it means the level's own name on the wire. + const taken = (level: string): boolean => { + if (answered.reasoning !== true) { + return false; + } + const entry = declared[level]; + if (entry === null) { + return false; + } + return level === "xhigh" || level === "max" + ? entry !== undefined + : true; + }; + const merged: Record = { ...declared }; + let filled = false; + for (const [level, wire] of Object.entries(levels)) { + // A level the model already takes keeps the adapter's own answer: one + // that spells the level itself keeps that spelling, and one that leaves + // it out keeps the level's name on the wire. Only what it REFUSES is + // filled, so the levels that work today keep working byte for byte. + if (wire === null || taken(level)) { + continue; + } + merged[level] = wire; + filled = true; + } + // Nothing to add - and so nothing to say about `reasoning` either, since + // every level is only "taken" for a model that already reasons: hand back + // the adapter's own object rather than an equal copy of it. + if (!filled) { + return answered; + } + return { ...answered, reasoning: true, thinkingLevelMap: merged }; + }; } - return { ...answered, reasoning: true, thinkingLevelMap: merged }; - }; - adapter.modelOf = wrappedModelOf; - restored.push(() => { - adapter.modelOf = originalModelOf; - }); + return chain; + }, + { id: PATCH_SITE.planeAdapter } + ); + if (stopModelOf !== undefined) { + stops.push(stopModelOf); } } - if (restored.length === 0) { + if (stops.length === 0) { return undefined; } return () => { - for (const undo of restored.toReversed()) { + for (const undo of stops.toReversed()) { undo(); } }; @@ -1071,133 +1122,134 @@ export const decorateModelInfo = ( if (llm === undefined || typeof llm.resolveModelInfo !== "function") { return undefined; } - const original = llm.resolveModelInfo; - const decorated = async ( - provider: string, - model: string, - signal?: AbortSignal - ): Promise => { - // A provider this plugin does not claim is answered by its own adapter, - // unchanged and un-caught. Our catalog is OpenCode's: substituting it for a - // foreign route's failure, or overwriting a foreign route's limits with it, - // describes a model the user is not calling. - if (!config.providers.has(provider) && !isInternalRoute(provider)) { - return Reflect.apply(original, llm, [provider, model, signal]); - } - let resolved: unknown; - try { - resolved = await Reflect.apply(original, llm, [provider, model, signal]); - } catch (error) { - // **One throw removes the WHOLE provider group from the picker.** The Host - // builds a group with `Promise.all` over `listModels`, and a rejection - // there is caught per PROVIDER, not per model — so the user loses every - // model on that route, not one row. - // - // That became reachable when `listModels` started publishing the catalog on - // the parent route: the route's own adapter never carried the Responses, - // Messages or Google models, so `resolveModelInfo` may refuse an id it does - // not know. Our catalog does know it, so answer from there rather than - // letting one unknown id take the group down. - // - // A model the catalog cannot describe either is rethrown: inventing an - // answer for an id nobody knows would hide a real problem. - const spec = findModelSpecOn(catalogPlaneForRoute(provider), model); - if (isInternalRoute(provider) || spec === undefined) { - throw error; - } - resolved = {}; - } - // Our own routes carry the real metadata already. - if (isInternalRoute(provider)) { - return resolved; - } - const spec = findModelSpecOn(catalogPlaneForRoute(provider), model); - if (spec === undefined) { - return resolved; - } - // A NON-RECORD answer is the same hazard as a throw, one step quieter: the - // Host reads `resolved.reasoning` immediately after this returns, so - // `undefined` becomes `TypeError: Cannot read properties of undefined` inside - // the group's `Promise.all` — and the whole provider group goes with it. When - // the catalog knows the model, the answer is built from the catalog instead of - // passed through. - const out: Record = isRecord(resolved) - ? { ...resolved } - : {}; - // Our catalog is the FRESHER VINTAGE of the same data, and the divergence is - // measured rather than assumed. pi ships an installed `opencode` catalog the - // plane routes fall back to, and it is a different snapshot of models.dev: - // it says 272k for `gpt-5.4` and 200k for `claude-sonnet-4-5` where the - // vendor and our shim both say 1.05M and 1M, and it carries no - // `thinkingLevelMap` for the `budget_tokens` Claude models where models.dev - // declares a token minimum. - // - // Filling only what the adapter left BLANK let every one of those stale - // values survive, which is what the surfaces then reported — so the fields - // our catalog carries are authoritative here rather than advisory. - const override = config.modelOverrides[model]; - const existingContext = isRecord(out.context) ? out.context : {}; - const requestedWindow = override?.contextWindow; - - if (typeof spec.context_window === "number" && spec.context_window > 0) { - const effectiveWindow = resolveEffectiveContextWindow( - requestedWindow ?? spec.context_window, - config.maxContextWindow - ); - out.context = { - ...existingContext, - contextWindow: effectiveWindow, - }; - } - // The output cap is the same class of staleness, and a wrong one truncates - // rather than merely mis-reports. - if (override?.maxTokens !== undefined) { - out.defaultMaxTokens = override.maxTokens; - } else if ( - typeof spec.max_output_tokens === "number" && - spec.max_output_tokens > 0 - ) { - out.defaultMaxTokens = spec.max_output_tokens; - } - if (override?.name !== undefined) { - out.name = override.name; - } - if (override?.inputModalities !== undefined) { - out.inputModalities = [...override.inputModalities]; - } - if (override?.reasoningEfforts === undefined) { - const map = thinkingLevelMapFor(spec); - const efforts = map === undefined ? [] : effortsFromLevelMap(map); - if (efforts.length > 0) { - out.reasoning = { efforts }; - } else { - // The model takes no thinking level. Leaving the adapter's `reasoning` - // here would offer a control the model refuses — and for a model our - // catalog marks non-reasoning, that is exactly the stale shape. - delete out.reasoning; - } - } else { - out.reasoning = { - efforts: override.reasoningEfforts.map((id) => ({ - id, - name: - id === "off" - ? "Off" - : `${id.charAt(0).toUpperCase()}${id.slice(1)}`, - })), - }; - } - return out; - }; + return patchMethod( + llm, + "resolveModelInfo", + (original) => + async ( + provider: string, + model: string, + signal?: AbortSignal + ): Promise => { + // A provider this plugin does not claim is answered by its own adapter, + // unchanged and un-caught. Our catalog is OpenCode's: substituting it for a + // foreign route's failure, or overwriting a foreign route's limits with it, + // describes a model the user is not calling. + if (!config.providers.has(provider) && !isInternalRoute(provider)) { + return Reflect.apply(original, llm, [provider, model, signal]); + } + let resolved: unknown; + try { + resolved = await Reflect.apply(original, llm, [ + provider, + model, + signal, + ]); + } catch (error) { + // **One throw removes the WHOLE provider group from the picker.** The Host + // builds a group with `Promise.all` over `listModels`, and a rejection + // there is caught per PROVIDER, not per model — so the user loses every + // model on that route, not one row. + // + // That became reachable when `listModels` started publishing the catalog on + // the parent route: the route's own adapter never carried the Responses, + // Messages or Google models, so `resolveModelInfo` may refuse an id it does + // not know. Our catalog does know it, so answer from there rather than + // letting one unknown id take the group down. + // + // A model the catalog cannot describe either is rethrown: inventing an + // answer for an id nobody knows would hide a real problem. + const spec = findModelSpecOn(catalogPlaneForRoute(provider), model); + if (isInternalRoute(provider) || spec === undefined) { + throw error; + } + resolved = {}; + } + // Our own routes carry the real metadata already. + if (isInternalRoute(provider)) { + return resolved; + } + const spec = findModelSpecOn(catalogPlaneForRoute(provider), model); + if (spec === undefined) { + return resolved; + } + // A NON-RECORD answer is the same hazard as a throw, one step quieter: the + // Host reads `resolved.reasoning` immediately after this returns, so + // `undefined` becomes `TypeError: Cannot read properties of undefined` inside + // the group's `Promise.all` — and the whole provider group goes with it. When + // the catalog knows the model, the answer is built from the catalog instead of + // passed through. + const out: Record = isRecord(resolved) + ? { ...resolved } + : {}; + // Our catalog is the FRESHER VINTAGE of the same data, and the divergence is + // measured rather than assumed. pi ships an installed `opencode` catalog the + // plane routes fall back to, and it is a different snapshot of models.dev: + // it says 272k for `gpt-5.4` and 200k for `claude-sonnet-4-5` where the + // vendor and our shim both say 1.05M and 1M, and it carries no + // `thinkingLevelMap` for the `budget_tokens` Claude models where models.dev + // declares a token minimum. + // + // Filling only what the adapter left BLANK let every one of those stale + // values survive, which is what the surfaces then reported — so the fields + // our catalog carries are authoritative here rather than advisory. + const override = config.modelOverrides[model]; + const existingContext = isRecord(out.context) ? out.context : {}; + const requestedWindow = override?.contextWindow; - try { - llm.resolveModelInfo = decorated; - } catch { - return undefined; - } - return () => { - if (llm.resolveModelInfo === decorated) { - llm.resolveModelInfo = original; - } - }; + if ( + typeof spec.context_window === "number" && + spec.context_window > 0 + ) { + const effectiveWindow = resolveEffectiveContextWindow( + requestedWindow ?? spec.context_window, + config.maxContextWindow + ); + out.context = { + ...existingContext, + contextWindow: effectiveWindow, + }; + } + // The output cap is the same class of staleness, and a wrong one truncates + // rather than merely mis-reports. + if (override?.maxTokens !== undefined) { + out.defaultMaxTokens = override.maxTokens; + } else if ( + typeof spec.max_output_tokens === "number" && + spec.max_output_tokens > 0 + ) { + out.defaultMaxTokens = spec.max_output_tokens; + } + if (override?.name !== undefined) { + out.name = override.name; + } + if (override?.inputModalities !== undefined) { + out.inputModalities = [...override.inputModalities]; + } + if (override?.reasoningEfforts === undefined) { + const map = thinkingLevelMapFor(spec); + const efforts = map === undefined ? [] : effortsFromLevelMap(map); + if (efforts.length > 0) { + out.reasoning = { efforts }; + } else { + // The model takes no thinking level. Leaving the adapter's `reasoning` + // here would offer a control the model refuses — and for a model our + // catalog marks non-reasoning, that is exactly the stale shape. + delete out.reasoning; + } + } else { + out.reasoning = { + efforts: override.reasoningEfforts.map((id) => ({ + id, + name: + id === "off" + ? "Off" + : `${id.charAt(0).toUpperCase()}${id.slice(1)}`, + })), + }; + } + return out; + }, + { id: PATCH_SITE.modelInfo } + ); }; diff --git a/test/models-discovery.test.ts b/test/models-discovery.test.ts index c313e4b8..4fb11ff3 100644 --- a/test/models-discovery.test.ts +++ b/test/models-discovery.test.ts @@ -12,6 +12,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { + baseForProtocol, type CordisContext, PI_THINKING_LEVELS, RESPONSES_ROUTE, @@ -25,6 +26,7 @@ import { isServableModel, mergeDiscoveredModels, parseModelsDevCatalog, + protocolFor, resolveConfig, resolveDiscoveryProvider, resolveEffectiveContextWindow, @@ -319,6 +321,44 @@ describe("decorateModelDiscovery", () => { stop?.(); expect(ctx.llm?.discoverModels).toBe(original); }); + + it("calls the host's method with the service as its receiver", async () => { + // The real registry is a class whose `discoverModels` reads its own + // discoveries through `this`, so a wrapper that calls it detached fails + // before any adapter is asked. + class DiscoveryHost { + readonly discoveries = new Map([ + ["llm-pi-ai", async () => [{ id: "adapter-model" }]], + ]); + async discoverModels(ns: string, _request?: unknown): Promise { + return this.discoveries.get(ns)?.(); + } + } + const llm = new DiscoveryHost(); + const stop = decorateModelDiscovery( + { llm } as unknown as CordisContext, + resolveConfig() + ); + + await expect( + llm.discoverModels("llm-pi-ai", { provider: "other" }) + ).resolves.toEqual([{ id: "adapter-model" }]); + stop?.(); + }); + + it("leaves a wrapper installed on top of it in place on dispose", () => { + const original = async () => [{ id: "adapter-model" }]; + const llm: Record = { discoverModels: original }; + const stop = decorateModelDiscovery( + { llm } as unknown as CordisContext, + resolveConfig() + ); + const foreign = async () => []; + llm.discoverModels = foreign; + stop?.(); + + expect(llm.discoverModels).toBe(foreign); + }); }); describe("models-discovery · an empty list from a route that declares models", () => { @@ -515,6 +555,123 @@ describe("models-discovery: hiding the internal Responses route", () => { const ctx = { llm: {} } as unknown as CordisContext; expect(hideResponsesRoute(ctx, resolveConfig())).toBeUndefined(); }); + + it("restores each listing method it replaced, by identity, on dispose", () => { + const { ctx } = hostWith(); + const llm = ctx.llm as Record; + const keys = [ + "listProviders", + "listConfigurableProviders", + "listModels", + ] as const; + const orig = Object.fromEntries(keys.map((key) => [key, llm[key]])); + const stop = hideResponsesRoute(ctx, resolveConfig()); + for (const key of keys) { + expect(llm[key], key).not.toBe(orig[key]); + } + + stop?.(); + for (const key of keys) { + expect(llm[key], key).toBe(orig[key]); + } + }); + + it("patches only the listing methods the host has", () => { + const listModels = async () => []; + const llm: Record = { listModels }; + const stop = hideResponsesRoute( + { llm } as unknown as CordisContext, + resolveConfig() + ); + expect(stop).toBeTypeOf("function"); + expect("listProviders" in llm).toBe(false); + expect("listConfigurableProviders" in llm).toBe(false); + expect(llm.listModels).not.toBe(listModels); + + stop?.(); + expect(llm.listModels).toBe(listModels); + }); + + it("the patched listing passes the caller's receiver to the host's method", async () => { + // Both methods read their state through `this`, so either would throw if + // the patch called the host's method detached. + const registry = { + routes: [ + { id: "opencode", name: "opencode" }, + { id: RESPONSES_ROUTE, name: RESPONSES_ROUTE }, + ], + listProviders(this: { routes: unknown[] }): unknown[] { + return [...this.routes]; + }, + async listModels( + this: { routes: unknown[] }, + provider: string + ): Promise { + return this.routes.length > 0 ? [{ id: `${provider}-model` }] : []; + }, + }; + const stop = hideResponsesRoute( + { llm: registry } as unknown as CordisContext, + resolveConfig() + ); + + expect(registry.listProviders()).toEqual([ + { id: "opencode", name: "opencode" }, + ]); + await expect(registry.listModels("opencode")).resolves.toEqual([ + { id: "opencode-model" }, + ]); + stop?.(); + }); + + it("keeps every listing when the host refuses one of them", () => { + // All or nothing, a deliberate change in P-H: the three listings must agree + // on what is hidden, so a host that will not take the `listModels` wrapper + // keeps all three as they were. Before, a THROWING refusal already rolled + // back like this, but a SILENT one (`Reflect.set` answering false on a + // non-writable method) left the other two filtered and returned a disposer. + const { ctx } = hostWith(); + const llm = ctx.llm as Record; + const { listConfigurableProviders, listModels, listProviders } = llm; + Object.defineProperty(llm, "listModels", { + configurable: false, + value: listModels, + writable: false, + }); + + expect(hideResponsesRoute(ctx, resolveConfig())).toBeUndefined(); + expect(llm.listProviders).toBe(listProviders); + expect(llm.listConfigurableProviders).toBe(listConfigurableProviders); + expect(llm.listModels).toBe(listModels); + expect(ctx.llm?.listProviders?.()).toHaveLength(2); + }); + + it("a reinstall still reads the host's own listing, so the redirect keeps firing", () => { + // A deliberate change in P-H. Before, the second install destructured the + // first install's filtered `listProviders` into `registeredRoutes`, so + // `isRouteRegistered` answered false. A reload's second install is handed + // the UNPATCHED listing, not the first install's filtered one, so + // `isRouteRegistered` keeps seeing the hidden route. A second install that stacked on the first would capture the + // filtered listing, and the redirect would never find its route. (The host + // here has no `adapters` map, so the listing is the only evidence.) + const { ctx } = hostWith(); + const a = hideResponsesRoute(ctx, resolveConfig()); + const b = hideResponsesRoute(ctx, resolveConfig()); + expect(ctx.llm?.listProviders?.()).toHaveLength(1); + expect(isRouteRegistered(RESPONSES_ROUTE)).toBe(true); + + a?.(); + expect(ctx.llm?.listProviders?.()).toHaveLength(1); + expect(isRouteRegistered(RESPONSES_ROUTE)).toBe(true); + b?.(); + expect(ctx.llm?.listProviders?.()).toHaveLength(2); + }); + + it("stands down without an llm service", () => { + expect( + hideResponsesRoute({} as CordisContext, resolveConfig()) + ).toBeUndefined(); + }); }); describe("models-discovery: the unfiltered registry keeps its receiver", () => { @@ -839,6 +996,58 @@ describe("models-discovery: the adapter behind a plane route", () => { ); }); + it("stands down on a frozen adapter: nothing installed, nothing thrown", () => { + // A deliberate change in P-H. Before, `adapter[method] = wrapped` threw a + // TypeError out of the fetch-patch effect on a frozen adapter; now a refused + // method is skipped, and with every method refused there is nothing to + // dispose. + const adapter = Object.freeze({ + modelOf: () => ({}), + prepareCall: async () => ({}), + resolveModel: async () => ({}), + }); + const before = { ...adapter }; + const llm = { adapters: new Map([["opencode-go", { adapter }]]) }; + + expect( + decoratePlaneAdapters(llm as never, resolveConfig({ enrichModels: true })) + ).toBeUndefined(); + expect({ ...adapter }).toEqual(before); + }); + + it("skips one method the adapter refuses and patches the rest, per method", () => { + // A deliberate change in P-H. Before, `adapter[method] = wrapped` threw a + // TypeError on the non-writable `prepareCall`. Each adapter fill is + // independent, so a refused method costs only itself: unlike the listings, + // which must agree and are all or nothing. + const prepareCall = async () => ({}); + const adapter: Record = { + modelOf: () => ({}), + resolveModel: async () => ({}), + }; + Object.defineProperty(adapter, "prepareCall", { + configurable: false, + enumerable: true, + value: prepareCall, + writable: false, + }); + const { modelOf, resolveModel } = adapter; + const llm = { adapters: new Map([["opencode-go", { adapter }]]) }; + const stop = decoratePlaneAdapters( + llm as never, + resolveConfig({ enrichModels: true }) + ); + + expect(stop).toBeTypeOf("function"); + expect(adapter.prepareCall).toBe(prepareCall); + expect(adapter.resolveModel).not.toBe(resolveModel); + expect(adapter.modelOf).not.toBe(modelOf); + stop?.(); + expect(adapter.resolveModel).toBe(resolveModel); + expect(adapter.modelOf).toBe(modelOf); + expect(adapter.prepareCall).toBe(prepareCall); + }); + it("calls the original on the adapter, because it is a class method that reads `this`", async () => { // The registry holds CLASS instances, not bags of arrows: pi-ai's real // `resolveModel` opens with `this.current()`. A wrapper that calls the @@ -1062,6 +1271,84 @@ describe("models-discovery: the adapter behind a plane route", () => { stop?.(); }); + it("one adapter serving both planes: the later plane's fill wraps the earlier one, in registry order", async () => { + // pi-ai serves both plane routes from ONE adapter instance, so the wrap is + // applied once per route and the later route's fill runs over the earlier + // one's answer. The base the earlier plane wrote is not a retired one, so + // the later fill keeps it: in registry order [opencode, opencode-go] both + // routes answer Zen's base. The host characterization's pi-ai-first + // `[INTENDED_CHANGE]` row pins the same nesting on the real host. + const specOn = (plane: "go" | "zen") => { + const spec = findModelSpecOn(plane, "deepseek-v4-pro"); + const protocol = spec === undefined ? undefined : protocolFor(spec); + if (protocol === undefined) { + throw new TypeError(`no protocol for deepseek-v4-pro on ${plane}`); + } + return protocol; + }; + const { endpoint } = resolveConfig(); + const zen = baseForProtocol("opencode", specOn("zen"), endpoint); + const go = baseForProtocol("opencode-go", specOn("go"), endpoint); + expect(zen).not.toBe(go); + + const baseFor = async ( + order: readonly string[] + ): Promise<{ calls: number; bases: unknown[] }> => { + const prepareCall = vi.fn(async () => ({})); + const adapter: Record = { prepareCall }; + const llm = { + adapters: new Map(order.map((route) => [route, { adapter }])), + }; + const stop = decoratePlaneAdapters(llm as never, resolveConfig()); + const call = adapter.prepareCall as ( + provider: string, + model: string + ) => Promise<{ baseUrl?: unknown }>; + const onGo = await call("opencode-go", "deepseek-v4-pro"); + const onZen = await call("opencode", "deepseek-v4-pro"); + const bases = [onGo.baseUrl, onZen.baseUrl]; + stop?.(); + return { bases, calls: prepareCall.mock.calls.length }; + }; + + await expect(baseFor(["opencode", "opencode-go"])).resolves.toEqual({ + bases: [zen, zen], + calls: 2, + }); + await expect(baseFor(["opencode-go", "opencode"])).resolves.toEqual({ + bases: [go, go], + calls: 2, + }); + }); + + it("a class adapter answers from its prototype again after dispose", async () => { + // A deliberate change in P-H. Before, the disposer assigned the prototype's + // method back as an OWN property, which answered the same but shadowed the + // prototype for good; now the own property is deleted. + class Adapter { + async resolveModel(_provider: string, model: string): Promise { + return { id: model, marker: "raw" }; + } + } + const adapter = new Adapter(); + const llm = { adapters: new Map([["opencode-go", { adapter }]]) }; + const stop = decoratePlaneAdapters(llm as never, resolveConfig()); + + const decorated = (await adapter.resolveModel( + "opencode-go", + "beam-free" + )) as { + context?: { contextWindow?: number }; + }; + expect(decorated.context?.contextWindow).toBeGreaterThan(0); + + stop?.(); + expect(Object.hasOwn(adapter, "resolveModel")).toBe(false); + await expect( + adapter.resolveModel("opencode-go", "beam-free") + ).resolves.toEqual({ id: "beam-free", marker: "raw" }); + }); + it("stands down when the host exposes no adapter registry", () => { expect(decoratePlaneAdapters({} as never, resolveConfig())).toBeUndefined(); expect( @@ -1313,6 +1600,52 @@ describe("models-discovery: the Effort row a model reports", () => { expect(patched(ctx)).toBe(before); }); + it("calls the host's method with the service as its receiver", async () => { + class InfoHost { + answers = new Map([ + ["hand-declared-model", { id: "hand-declared-model", name: "Mine" }], + ]); + async resolveModelInfo( + _provider: string, + model: string + ): Promise { + return this.answers.get(model); + } + } + const llm = new InfoHost(); + const stop = decorateModelInfo( + { llm } as unknown as CordisContext, + enrichConfig + ); + + // Uncatalogued, so the answer passes through - read off `this`. + await expect( + llm.resolveModelInfo("opencode", "hand-declared-model") + ).resolves.toEqual({ id: "hand-declared-model", name: "Mine" }); + stop?.(); + }); + + it("leaves a wrapper installed on top of it in place on dispose", () => { + const before = async () => ({ id: "x" }); + const ctx = { llm: { resolveModelInfo: before } } as never as CordisContext; + const stop = decorateModelInfo(ctx, enrichConfig); + const foreign = async () => ({ id: "foreign" }); + (ctx.llm as Record).resolveModelInfo = foreign; + stop?.(); + + expect(patched(ctx)).toBe(foreign); + }); + + it("leaves a non-extensible service installed", () => { + const before = async () => ({ id: "x" }); + const ctx = { + llm: Object.freeze({ resolveModelInfo: before }), + } as never as CordisContext; + + expect(decorateModelInfo(ctx, enrichConfig)).toBeUndefined(); + expect(patched(ctx)).toBe(before); + }); + it("installs nothing when the plugin is not enriching models", () => { const before = async () => ({ id: "x" }); const ctx = { llm: { resolveModelInfo: before } } as never as CordisContext; diff --git a/test/patch-method.test.ts b/test/patch-method.test.ts new file mode 100644 index 00000000..a6df3da8 --- /dev/null +++ b/test/patch-method.test.ts @@ -0,0 +1,782 @@ +/** + * The one way the plugin wraps a method on a host object. + * + * Each case is one clause of the contract: (a) a dispose restores only while its + * wrapper is still the current value, (b) an inherited original comes back by + * deleting the own property, (c) a second install of the same site wraps the + * unpatched method rather than the first install, and (d) a site can follow the + * adapter registry. Identity is read from own descriptors throughout, because on + * the real host a property read is a fresh proxy every time (the tier-A case). + * + * @module test/patch-method.test + */ + +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + adaptersOf, + mountTierA, + s0Providers, + type TierA, +} from "./support/host.ts"; +import { + type CordisContext, + inspectPatch, + patchMethod, + rewrapOnAdaptersUpdated, +} from "./support/src.ts"; + +/** A host whose method is a class method: inherited, and reading `this`. */ +class Host { + calls = 0; + greet(this: Host, name: string): string { + this.calls += 1; + return `hi ${name}`; + } +} + +type Unpatched = (...args: unknown[]) => unknown; + +/** A wrapper that appends `suffix`, keeping the caller's receiver. */ +const suffixed = (suffix: string) => (unpatched: Unpatched) => + function greet(this: unknown, name: string): string { + return `${String(Reflect.apply(unpatched, this, [name]))}${suffix}`; + }; + +const bang = suffixed("!"); + +/** Call `greet` on any target, with the target as its receiver. */ +const greetOn = (target: object, name = "a"): unknown => { + const method: unknown = Reflect.get(target, "greet"); + if (typeof method !== "function") { + throw new TypeError("no greet on the target"); + } + return Reflect.apply(method, target, [name]); +}; + +/** Call a method by name on its host, with the host as the receiver. */ +const greetLike = ( + target: object, + key: string, + ...args: unknown[] +): unknown => { + const method: unknown = Reflect.get(target, key); + if (typeof method !== "function") { + throw new TypeError(`no ${key} on the target`); + } + return Reflect.apply(method, target, args); +}; + +/** An object whose `greet` is an own data property with these attributes. */ +const ownGreet = (attributes: Partial = {}): object => { + const target = {}; + Object.defineProperty(target, "greet", { + configurable: true, + enumerable: true, + value(name: string): string { + return `hi ${name}`; + }, + writable: true, + ...attributes, + }); + return target; +}; + +const SHAPES = { + inherited: (): object => new Host(), + own: (): object => ownGreet(), +} as const; + +let tier: TierA | undefined; + +afterEach(async () => { + await tier?.dispose(); + tier = undefined; +}); + +describe("patchMethod", () => { + it("(b) an inherited method is wrapped with an own property, keeps its receiver, and is deleted on dispose", () => { + const host = new Host(); + const stop = patchMethod(host, "greet", bang, { id: "t" }); + + expect(stop).toBeTypeOf("function"); + expect(Object.hasOwn(host, "greet")).toBe(true); + // Synchronous in, synchronous out: the trampoline is not `async`. + expect(host.greet("a")).toBe("hi a!"); + // It ran ON the instance: the counter lives on `this`. + expect(host.calls).toBe(1); + + stop?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + expect(Reflect.get(host, "greet")).toBe( + Reflect.get(Host.prototype, "greet") + ); + }); + + it("an own method keeps its attributes while installed and gets its exact descriptor back", () => { + const target = ownGreet({ enumerable: false }); + const original = Reflect.getOwnPropertyDescriptor(target, "greet"); + const stop = patchMethod(target, "greet", bang, { id: "t" }); + + const installed = Reflect.getOwnPropertyDescriptor(target, "greet"); + expect(installed?.value).not.toBe(original?.value); + expect(installed?.enumerable).toBe(false); + expect(greetOn(target)).toBe("hi a!"); + + stop?.(); + const restored = Reflect.getOwnPropertyDescriptor(target, "greet"); + expect(restored).toEqual(original); + expect(restored?.value).toBe(original?.value); + }); + + it("(a) leaves a wrapper set on top of it in place, and becomes a pass-through under it", () => { + const host = new Host(); + const stop = patchMethod(host, "greet", bang, { id: "t" }); + const ours: unknown = Reflect.getOwnPropertyDescriptor( + host, + "greet" + )?.value; + if (typeof ours !== "function") { + throw new TypeError("nothing installed"); + } + const foreign = function foreign(this: unknown, name: string): string { + return `[${String(Reflect.apply(ours, this, [name]))}]`; + }; + Reflect.set(host, "greet", foreign); + + stop?.(); + expect(Reflect.getOwnPropertyDescriptor(host, "greet")?.value).toBe( + foreign + ); + // Ours still runs under the foreign wrapper, and no longer decorates. + expect(greetOn(host)).toBe("[hi a]"); + }); + + it.each(Object.entries(SHAPES))( + "(c) a second install of the same site wraps the unpatched method, never the first install (%s)", + (_shape, make) => { + const target = make(); + const original: unknown = Reflect.get(target, "greet"); + const first = patchMethod(target, "greet", bang, { id: "t" }); + const seen: unknown[] = []; + const second = patchMethod( + target, + "greet", + (unpatched) => { + seen.push(unpatched); + return bang(unpatched); + }, + { id: "t" } + ); + + expect(seen).toEqual([original]); + expect(seen[0]).toBe(original); + // One decoration, not two. + expect(greetOn(target)).toBe("hi a!"); + second?.(); + first?.(); + } + ); + + it.each( + Object.entries(SHAPES).flatMap(([shape, make]) => [ + [shape, "first-in first-out", make, true] as const, + [shape, "last-in first-out", make, false] as const, + ]) + )("%s · %s ends at the exact original", (_shape, _order, make, fifo) => { + const target = make(); + const before = Reflect.getOwnPropertyDescriptor(target, "greet"); + const a = patchMethod(target, "greet", bang, { id: "t" }); + const installedA: unknown = Reflect.getOwnPropertyDescriptor( + target, + "greet" + )?.value; + const b = patchMethod(target, "greet", bang, { id: "t" }); + const installedB: unknown = Reflect.getOwnPropertyDescriptor( + target, + "greet" + )?.value; + expect(installedB).not.toBe(installedA); + + (fifo ? a : b)?.(); + // The survivor is installed and decorates exactly once. + expect(Reflect.getOwnPropertyDescriptor(target, "greet")?.value).toBe( + fifo ? installedB : installedA + ); + expect(greetOn(target)).toBe("hi a!"); + + (fifo ? b : a)?.(); + expect(Reflect.getOwnPropertyDescriptor(target, "greet")).toEqual(before); + expect(Reflect.getOwnPropertyDescriptor(target, "greet")?.value).toBe( + before?.value + ); + expect(greetOn(target)).toBe("hi a"); + }); + + it.each([ + ["the outer first", true], + ["the inner first", false], + ] as const)( + "two different sites compose, and disposing %s ends with no own property", + (_order, outerFirst) => { + const host = new Host(); + const inner = patchMethod(host, "greet", bang, { id: "inner" }); + const outer = patchMethod(host, "greet", suffixed("?"), { id: "outer" }); + expect(greetOn(host)).toBe("hi a!?"); + + if (outerFirst) { + outer?.(); + expect(greetOn(host)).toBe("hi a!"); + inner?.(); + } else { + inner?.(); + // The inner one is retired under the outer one: a pass-through. + expect(greetOn(host)).toBe("hi a?"); + outer?.(); + } + expect(Object.hasOwn(host, "greet")).toBe(false); + expect(greetOn(host)).toBe("hi a"); + } + ); + + it("collapses a retired same-site wrapper it finds as the original, back to what that one replaced", () => { + // A (site t) is replaced by B (site t); a foreign site wraps B; B retires + // under it. Retired, B hands the call to A, which is still live, so the + // answer stays decorated. When the foreign site goes, B is nobody's + // original: A is what comes back. + const host = new Host(); + const a = patchMethod(host, "greet", bang, { id: "t" }); + const installedA: unknown = Reflect.getOwnPropertyDescriptor( + host, + "greet" + )?.value; + const b = patchMethod(host, "greet", bang, { id: "t" }); + const other = patchMethod(host, "greet", suffixed("?"), { id: "other" }); + b?.(); + expect(greetOn(host)).toBe("hi a!?"); + + other?.(); + expect(Reflect.getOwnPropertyDescriptor(host, "greet")?.value).toBe( + installedA + ); + expect(greetOn(host)).toBe("hi a!"); + a?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + }); + + it("refuses a key that holds no method, without calling wrap", () => { + const wrap = vi.fn(bang); + const missing = new Host(); + const notAMethod = { greet: 42 }; + + expect(patchMethod(missing, "absent", wrap, { id: "t" })).toBeUndefined(); + expect(Object.hasOwn(missing, "absent")).toBe(false); + expect(patchMethod(notAMethod, "greet", wrap, { id: "t" })).toBeUndefined(); + expect(notAMethod.greet).toBe(42); + expect( + patchMethod(Object.create(null), "greet", wrap, { id: "t" }) + ).toBeUndefined(); + expect(wrap).not.toHaveBeenCalled(); + }); + + it("refuses an inherited accessor without running it, and adds no own property", () => { + let reads = 0; + class Getter { + get greet(): () => string { + reads += 1; + return () => "hi"; + } + } + const target = new Getter(); + + expect(patchMethod(target, "greet", bang, { id: "t" })).toBeUndefined(); + expect(reads).toBe(0); + expect(Object.hasOwn(target, "greet")).toBe(false); + }); + + it("finds an inherited method more than one prototype up", () => { + class Child extends Host {} + const host = new Child(); + const stop = patchMethod(host, "greet", bang, { id: "t" }); + + expect(host.greet("a")).toBe("hi a!"); + stop?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + expect(host.greet("a")).toBe("hi a"); + }); + + it("refuses an own accessor, and leaves it untouched", () => { + const target = { + get greet(): () => string { + return () => "hi"; + }, + }; + const before = Reflect.getOwnPropertyDescriptor(target, "greet"); + + expect(patchMethod(target, "greet", bang, { id: "t" })).toBeUndefined(); + expect(Reflect.getOwnPropertyDescriptor(target, "greet")).toEqual(before); + }); + + it.each([ + ["a frozen own method", () => Object.freeze(ownGreet())], + [ + "a non-extensible object over an inherited method", + () => Object.preventExtensions(new Host()), + ], + [ + "a proxy whose defineProperty trap throws", + () => + new Proxy(ownGreet(), { + defineProperty: () => { + throw new TypeError("refused"); + }, + }), + ], + ])("refuses %s, and leaves it untouched", (_case, make) => { + const target = make(); + const before = Reflect.getOwnPropertyDescriptor(target, "greet"); + + expect(patchMethod(target, "greet", bang, { id: "t" })).toBeUndefined(); + expect(Reflect.getOwnPropertyDescriptor(target, "greet")).toEqual(before); + expect(greetOn(target)).toBe("hi a"); + }); + + it("treats a wrapper of ours copied under another key as foreign there", () => { + const host = new Host(); + const stop = patchMethod(host, "greet", bang, { id: "t" }); + const copied: unknown = Reflect.getOwnPropertyDescriptor( + host, + "greet" + )?.value; + Reflect.set(host, "other", copied); + + const seen: unknown[] = []; + const stopOther = patchMethod( + host, + "other", + (unpatched) => { + seen.push(unpatched); + return suffixed("?")(unpatched); + }, + { id: "t" } + ); + // It wraps the copy as it found it, rather than reaching for a slot that + // belongs to `greet`. + expect(seen).toEqual([copied]); + stopOther?.(); + expect(Reflect.getOwnPropertyDescriptor(host, "other")?.value).toBe(copied); + stop?.(); + }); + + it("a repeated dispose leaves a later install alone", () => { + const host = new Host(); + const first = patchMethod(host, "greet", bang, { id: "t" }); + first?.(); + const later = patchMethod(host, "greet", bang, { id: "t" }); + const installed: unknown = Reflect.getOwnPropertyDescriptor( + host, + "greet" + )?.value; + + first?.(); + expect(Reflect.getOwnPropertyDescriptor(host, "greet")?.value).toBe( + installed + ); + expect(greetOn(host)).toBe("hi a!"); + later?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + }); + + it("a retired wrapper a foreign one puts back is collapsed by the next dispose", () => { + // Ours retires under a foreign wrapper, which then restores what it + // captured: ours, retired. That is nobody's wrapper any more, so a dispose + // that finds it current collapses it to the original. + const host = new Host(); + const stop = patchMethod(host, "greet", bang, { id: "t" }); + const ours: unknown = Reflect.getOwnPropertyDescriptor( + host, + "greet" + )?.value; + Reflect.set(host, "greet", (): string => "foreign"); + stop?.(); + Reflect.set(host, "greet", ours); + expect(greetOn(host)).toBe("hi a"); + + stop?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + }); + + it("disposes quietly when the property was deleted under it", () => { + const host = new Host(); + const stop = patchMethod(host, "greet", bang, { id: "t" }); + Reflect.deleteProperty(host, "greet"); + + stop?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + expect(greetOn(host)).toBe("hi a"); + }); + + it("three live installs of one site hand back newest first, and end at the original", () => { + // Each install answers with its own suffix, so the answer names which one + // ran; a same-site install wraps the unpatched method, never another one. + const host = new Host(); + const current = (): unknown => + Reflect.getOwnPropertyDescriptor(host, "greet")?.value; + const a = patchMethod(host, "greet", suffixed("A"), { id: "t" }); + const b = patchMethod(host, "greet", suffixed("B"), { id: "t" }); + const installedB = current(); + const c = patchMethod(host, "greet", suffixed("C"), { id: "t" }); + expect(greetOn(host)).toBe("hi aC"); + + // The newest goes first while TWO predecessors are live: the newest of + // them, B, is the heir, not the oldest. + c?.(); + expect(current()).toBe(installedB); + expect(greetOn(host)).toBe("hi aB"); + a?.(); + expect(current()).toBe(installedB); + expect(greetOn(host)).toBe("hi aB"); + b?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + expect(greetOn(host)).toBe("hi a"); + }); + + it("the middle install of three goes first: the newest stays and hands back to the oldest", () => { + const host = new Host(); + const current = (): unknown => + Reflect.getOwnPropertyDescriptor(host, "greet")?.value; + const liveIn = (value: unknown): number => + (inspectPatch(value, "greet") ?? []).filter((entry) => entry.live).length; + const a = patchMethod(host, "greet", suffixed("A"), { id: "t" }); + const installedA = current(); + const b = patchMethod(host, "greet", suffixed("B"), { id: "t" }); + const c = patchMethod(host, "greet", suffixed("C"), { id: "t" }); + const installedC = current(); + expect(liveIn(installedC)).toBe(3); + + b?.(); + expect(current()).toBe(installedC); + expect(liveIn(installedC)).toBe(2); + expect(greetOn(host)).toBe("hi aC"); + c?.(); + expect(current()).toBe(installedA); + expect(greetOn(host)).toBe("hi aA"); + a?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + }); + + it("under a foreign wrapper, a retired newest install falls through to the newest live one", () => { + // The trampoline twin of the heir choice above: C is captured by a foreign + // wrapper, so its dispose cannot restore and C turns into a pass-through. + const host = new Host(); + const a = patchMethod(host, "greet", suffixed("A"), { id: "t" }); + const b = patchMethod(host, "greet", suffixed("B"), { id: "t" }); + const c = patchMethod(host, "greet", suffixed("C"), { id: "t" }); + const inner: unknown = Reflect.getOwnPropertyDescriptor( + host, + "greet" + )?.value; + if (typeof inner !== "function") { + throw new TypeError("nothing installed"); + } + Reflect.set( + host, + "greet", + function foreign(this: unknown, name: string): string { + return `[${String(Reflect.apply(inner, this, [name]))}]`; + } + ); + expect(greetOn(host)).toBe("[hi aC]"); + + c?.(); + expect(greetOn(host)).toBe("[hi aB]"); + b?.(); + expect(greetOn(host)).toBe("[hi aA]"); + a?.(); + expect(greetOn(host)).toBe("[hi a]"); + }); + + it("a retired install keeps no wrapper alive, and live installs skip it", () => { + // A long-running host reloads the plugin many times. A retired record + // drops what `wrap` built (which closes over an activation's context and + // config), and the chain from the current install no longer reaches it. + const host = new Host(); + const current = (): unknown => + Reflect.getOwnPropertyDescriptor(host, "greet")?.value; + const a = patchMethod(host, "greet", bang, { id: "t" }); + const installedA = current(); + const b = patchMethod(host, "greet", bang, { id: "t" }); + expect(inspectPatch(current(), "greet")).toEqual([ + { holdsWrapper: true, id: "t", live: true }, + { holdsWrapper: true, id: "t", live: true }, + ]); + + a?.(); + expect(inspectPatch(installedA, "greet")).toEqual([ + { holdsWrapper: false, id: "t", live: false }, + ]); + const c = patchMethod(host, "greet", bang, { id: "t" }); + // C, then B: A is not in the chain. + expect(inspectPatch(current(), "greet")).toEqual([ + { holdsWrapper: true, id: "t", live: true }, + { holdsWrapper: true, id: "t", live: true }, + ]); + expect(greetOn(host)).toBe("hi a!"); + + c?.(); + b?.(); + expect(Object.hasOwn(host, "greet")).toBe(false); + expect(inspectPatch(Reflect.get(host, "greet"), "greet")).toBeUndefined(); + expect(inspectPatch(installedA, "other")).toBeUndefined(); + }); + + it( + "through the real cordis service proxy (tier A): a method read is never the same function twice, and the helper still restores", + { timeout: 15_000 }, + async () => { + tier = await mountTierA({ providers: s0Providers() }); + const { llm } = tier; + // The host fact the helper is built around. + expect(Reflect.get(llm, "listProviders")).not.toBe( + Reflect.get(llm, "listProviders") + ); + const base = llm.listProviders(); + + const stop = patchMethod( + llm, + "listProviders", + (unpatched) => + function listProviders(this: unknown): unknown { + const listed: unknown = Reflect.apply(unpatched, this, []); + return [ + ...(Array.isArray(listed) ? listed : []), + { id: "x", name: "x" }, + ]; + }, + { id: "t" } + ); + expect(llm.listProviders()).toContainEqual({ id: "x", name: "x" }); + + stop?.(); + expect(Object.hasOwn(llm, "listProviders")).toBe(false); + expect(llm.listProviders()).toEqual(base); + } + ); + + it( + "a foreign wrap and unwrap through ctx.llm (tier A) puts back a shadow proxy: the recorded limit, a correct pass-through left as an own property that the next install wraps once", + { timeout: 15_000 }, + async () => { + tier = await mountTierA({ providers: s0Providers() }); + const { llm } = tier; + const base = llm.listProviders(); + const stop = patchMethod( + llm, + "listProviders", + (unpatched) => + function listProviders(this: unknown): unknown { + const listed: unknown = Reflect.apply(unpatched, this, []); + return [ + ...(Array.isArray(listed) ? listed : []), + { id: "x", name: "x" }, + ]; + }, + { id: "t" } + ); + const installed: unknown = Reflect.getOwnPropertyDescriptor( + llm, + "listProviders" + )?.value; + + // Another plugin, through the same proxy: read, wrap, later put back. + const captured: unknown = Reflect.get(llm, "listProviders"); + expect(captured).not.toBe(installed); + Reflect.set(llm, "listProviders", (): unknown => []); + Reflect.set(llm, "listProviders", captured); + // What came back is cordis's shadow proxy around ours, not ours. + expect( + Reflect.getOwnPropertyDescriptor(llm, "listProviders")?.value + ).not.toBe(installed); + expect(llm.listProviders()).toContainEqual({ id: "x", name: "x" }); + + // Our dispose sees a foreign value and leaves it; the answer is the + // host's own, through the retired trampoline. + stop?.(); + expect(Object.hasOwn(llm, "listProviders")).toBe(true); + expect(llm.listProviders()).toEqual(base); + + // The next activation, which is how the limit reaches a real host + // (cordis disposes, then re-applies): it wraps the proxy as a foreign + // original, decorates exactly once, and its dispose puts the proxy back. + const leftover: unknown = Reflect.getOwnPropertyDescriptor( + llm, + "listProviders" + )?.value; + const again = patchMethod( + llm, + "listProviders", + (unpatched) => + function listProviders(this: unknown): unknown { + const listed: unknown = Reflect.apply(unpatched, this, []); + return [ + ...(Array.isArray(listed) ? listed : []), + { id: "x", name: "x" }, + ]; + }, + { id: "t" } + ); + const listed = llm.listProviders() as { id?: unknown }[]; + expect(listed.filter((entry) => entry.id === "x")).toHaveLength(1); + again?.(); + expect( + Reflect.getOwnPropertyDescriptor(llm, "listProviders")?.value + ).toBe(leftover); + expect(llm.listProviders()).toEqual(base); + } + ); +}); + +describe("rewrapOnAdaptersUpdated", () => { + /** A stand-in `on` that records each subscription and counts disposals. */ + const fakeOn = () => { + const subscriptions: { + event: string; + listener: () => unknown; + }[] = []; + let offCalls = 0; + const ctx: Pick = { + on: (event, listener) => { + subscriptions.push({ + event, + listener: () => listener({}, () => null), + }); + return () => { + offCalls += 1; + return true; + }; + }, + }; + return { ctx, offCalls: () => offCalls, subscriptions }; + }; + + it("reinstalls on llm/adapters-updated and stops listening on dispose", () => { + const { ctx, offCalls, subscriptions } = fakeOn(); + const t1 = new Host(); + const t2 = new Host(); + let target: object = t1; + const stop = rewrapOnAdaptersUpdated(ctx, () => + patchMethod(target, "greet", bang, { id: "r" }) + ); + + expect(subscriptions).toHaveLength(1); + const [subscription] = subscriptions; + if (subscription === undefined) { + throw new TypeError("no subscription"); + } + const { event, listener } = subscription; + expect(event).toBe("llm/adapters-updated"); + expect(Object.hasOwn(t1, "greet")).toBe(true); + + target = t2; + listener(); + expect(Object.hasOwn(t1, "greet")).toBe(false); + expect(Object.hasOwn(t2, "greet")).toBe(true); + expect(t2.greet("a")).toBe("hi a!"); + + listener(); + expect(t2.greet("a")).toBe("hi a!"); + + stop(); + expect(offCalls()).toBe(1); + expect(Object.hasOwn(t2, "greet")).toBe(false); + + listener(); + expect(Object.hasOwn(t2, "greet")).toBe(false); + stop(); + expect(offCalls()).toBe(1); + }); + + it("without an event source it installs once, and dispose restores", () => { + const host = new Host(); + const install = vi.fn(() => patchMethod(host, "greet", bang, { id: "r" })); + const stop = rewrapOnAdaptersUpdated({}, install); + + expect(install).toHaveBeenCalledTimes(1); + expect(host.greet("a")).toBe("hi a!"); + stop(); + expect(Object.hasOwn(host, "greet")).toBe(false); + }); + + it("an install that found nothing is retried on the next update", () => { + const { ctx, subscriptions } = fakeOn(); + let target: object = Object.create(null); + const stop = rewrapOnAdaptersUpdated(ctx, () => + patchMethod(target, "greet", bang, { id: "r" }) + ); + + const host = new Host(); + target = host; + subscriptions[0]?.listener(); + expect(host.greet("a")).toBe("hi a!"); + stop(); + expect(Object.hasOwn(host, "greet")).toBe(false); + }); + + it( + "on the real host (tier A): a registry change reinstalls over pi-ai's plane adapter, once", + { timeout: 15_000 }, + async () => { + tier = await mountTierA({ providers: s0Providers() }); + const adapter = adaptersOf(tier.llm).get("opencode")?.adapter; + if (adapter === undefined) { + throw new TypeError("no adapter behind the opencode route"); + } + let installs = 0; + let decorations = 0; + const stop = rewrapOnAdaptersUpdated( + tier.ctx as unknown as CordisContext, + () => { + installs += 1; + return patchMethod( + adapter, + "resolveModel", + (unpatched) => + function resolveModel( + this: unknown, + ...args: unknown[] + ): unknown { + decorations += 1; + return Reflect.apply(unpatched, this, args); + }, + { id: "rewrap-probe" } + ); + } + ); + expect(installs).toBe(1); + + const handle = tier.llm.registerAdapter(["reload-probe"], { + providerInfo: (provider: string) => ({ id: provider, name: provider }), + providerRetryPolicy: () => null, + } as never); + expect(installs).toBe(2); + handle(); + expect(installs).toBe(3); + + // An emit whose `thisArg` carries a cordis filter that rejects every + // context still reaches the listener: it is registered `global`. + // Today's emitter passes no `thisArg`; this is the case `global` is for. + tier.ctx.emit( + { [Symbol.for("cordis.filter")]: () => false }, + "llm/adapters-updated" + ); + expect(installs).toBe(4); + + await greetLike(adapter, "resolveModel", "opencode", "deepseek-v4-pro"); + // One layer, though it was reinstalled on every update. + expect(decorations).toBe(1); + + stop(); + expect(Object.hasOwn(adapter, "resolveModel")).toBe(false); + } + ); +}); diff --git a/test/reload.test.ts b/test/reload.test.ts new file mode 100644 index 00000000..4d8416c6 --- /dev/null +++ b/test/reload.test.ts @@ -0,0 +1,702 @@ +/** + * A reload, at every site where the plugin wraps a method on a host object. + * + * The host runs `apply` once per activation. cordis disposes an activation + * before it re-applies one (a config update is `Fiber.update` → `restart` → + * `_unload`, which awaits every disposer, → `_reload`), so a reload alone never + * overlaps two activations. Two are live at once when two rows load the plugin, + * or when a dispose restores nothing, which is how the base stacked: its + * disposers silently failed the identity check below, and the next activation + * wrapped the leftover wrapper. So the sequence a patch has to survive is: + * apply A → apply B → dispose A → B intact → dispose B → the originals restored + * EXACTLY, and the same with the disposals reversed. + * Two harder shapes follow: a reload whose config (for the listing, its keyless + * option) CHANGED, where the live answer must be B's alone, which a site that + * stacked B on A would get wrong; and a foreign wrapper set on top while the + * newest install is disposed first. + * "Exactly" is two facts: an own method comes back as the same function with + * the same attributes, and an inherited one leaves no own property behind, so + * the prototype answers again. + * + * Identity is read from OWN DESCRIPTORS, never by comparing a property read with + * `===`. On the real host `ctx.llm` is cordis's traceable proxy, which answers a + * fresh proxy for every read of a method, so a read is never the function that + * was installed; `Reflect.getOwnPropertyDescriptor` is not trapped and sees the + * instance itself. The tier-A block is that host. + * + * @module test/reload.test + */ + +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + adaptersOf, + mountTierA, + s0Providers, + type TierA, +} from "./support/host.ts"; +import { + apply, + baseForProtocol, + type CordisContext, + decorateModelDiscovery, + decorateModelInfo, + decoratePlaneAdapters, + findModelSpecOn, + hideResponsesRoute, + inspectPatch, + mergeDiscoveredModels, + protocolFor, + type ResolvedPluginConfig, + RESPONSES_ROUTE, + resolveConfig, +} from "./support/src.ts"; + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +/** Where a fixture's methods live: on the object, or on its prototype. */ +type Shape = "inherited" | "own"; + +type Method = (...args: never[]) => unknown; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null; + +/** + * The methods as own data properties of an object literal, or as methods on the + * prototype of a fresh object, which is where a class keeps them. + */ +const shaped = ( + shape: Shape, + methods: Record +): Record => + shape === "own" ? { ...methods } : Object.create({ ...methods }); + +/** Call a method by name, with its host as the receiver. */ +const call = async ( + target: object, + key: string, + ...args: unknown[] +): Promise => { + const method: unknown = Reflect.get(target, key); + if (typeof method !== "function") { + throw new TypeError(`no ${key} on the fixture`); + } + return await Reflect.apply(method, target, args); +}; + +/** The own descriptor of each key, which is what identity is read from. */ +const ownDescriptors = ( + target: object, + keys: readonly string[] +): Map => + new Map( + keys.map((key) => [key, Reflect.getOwnPropertyDescriptor(target, key)]) + ); + +/** The own value of each key, or `undefined` where there is no own property. */ +const ownValues = ( + target: object, + keys: readonly string[] +): Map => + new Map( + keys.map((key) => [ + key, + Reflect.getOwnPropertyDescriptor(target, key)?.value, + ]) + ); + +const contextWindowOf = (value: unknown): unknown => + isRecord(value) && isRecord(value.context) + ? value.context.contextWindow + : undefined; + +const nameOf = (value: unknown): unknown => + isRecord(value) ? value.name : undefined; + +const baseUrlOf = (value: unknown): unknown => + isRecord(value) ? value.baseUrl : undefined; + +/** The base a plane's catalog dispatches `id` to, under the default endpoint. */ +const planeBase = ( + route: "opencode" | "opencode-go", + id: string +): string | undefined => { + const spec = findModelSpecOn(route === "opencode" ? "zen" : "go", id); + const protocol = spec === undefined ? undefined : protocolFor(spec); + if (protocol === undefined) { + throw new TypeError(`no protocol for ${id} on ${route}`); + } + return baseForProtocol(route, protocol, resolveConfig().endpoint); +}; + +const asContext = (llm: object): CordisContext => + ({ llm }) as unknown as CordisContext; + +/** One patch site, as the reload sequence drives it. */ +interface Site { + readonly name: string; + /** The keys the site wraps on its target. */ + readonly keys: readonly string[]; + /** A fresh host `llm` service of the given shape. */ + readonly host: (shape: Shape) => Record; + /** The object whose methods the site wraps. */ + readonly targetOf: (llm: Record) => object; + readonly install: ( + llm: object, + config?: ResolvedPluginConfig + ) => (() => void) | undefined; + /** A call whose decorated and raw answers differ. */ + readonly probe: (llm: Record) => Promise; + readonly raw: unknown; + readonly decorated: unknown; + /** + * An activation A whose decoration differs from the default's, and a call + * that tells them apart: a stacked install would leak the first activation's + * answer through the second's wherever the decoration reads per-activation + * state, which every site does. Most sites read it from config; the listing's + * `listModels` reads its `hasCredential` option (the keyless filter), so its + * A is an install with that option rather than a config. The listing's + * `listProviders` capture is pinned separately, by the `isRouteRegistered` + * reinstall case in `models-discovery.test.ts` (a stacked listing would + * capture the previous install's filtered `listProviders`). The main + * sequence also checks, for every key of every site, that the second + * install REPLACED the first (two live entries of one site id). + */ + readonly reconfigured?: { + /** A's config, installed through the site's own `install`. */ + readonly config?: ResolvedPluginConfig; + /** A's install, where what differs is not config. */ + readonly installA?: (llm: object) => (() => void) | undefined; + readonly probe: (llm: Record) => Promise; + }; +} + +const LISTED = [ + { id: "opencode", name: "opencode" }, + { id: RESPONSES_ROUTE, name: RESPONSES_ROUTE }, +]; + +/** A priced Zen id and a free one, for the keyless filter. */ +const PRICED_AND_FREE = ["gpt-5.4", "beam-free"] as const; + +const listingMethods = (): Record => ({ + listConfigurableProviders: () => [ + { provider: "opencode" }, + { provider: RESPONSES_ROUTE }, + ], + listModels: async () => PRICED_AND_FREE.map((id) => ({ id })), + listProviders: () => LISTED.map((entry) => ({ ...entry })), +}); + +/** The one adapter both plane routes share, as pi-ai registers it. */ +const adapterMethods = (): Record => ({ + modelOf: () => ({ id: "x", reasoning: false }), + prepareCall: async () => ({}), + resolveModel: async () => ({}), +}); + +const adapterOf = (llm: Record): object => { + const { adapters } = llm; + const row: unknown = + adapters instanceof Map ? adapters.get("opencode") : undefined; + if (!isRecord(row) || !isRecord(row.adapter)) { + throw new TypeError("no adapter behind the opencode route"); + } + return row.adapter; +}; + +const SITES: readonly Site[] = [ + { + decorated: [{ id: "opencode", name: "opencode" }], + host: (shape) => shaped(shape, listingMethods()), + install: (llm) => hideResponsesRoute(asContext(llm), resolveConfig()), + keys: ["listProviders", "listConfigurableProviders", "listModels"], + name: "listing", + probe: (llm) => call(llm, "listProviders"), + raw: LISTED, + // The keyless filter is per activation: under A (no credential) a priced + // id is dropped, under the default (nobody asked) the listing is kept, so + // a stacked default install would keep A's narrowing. + reconfigured: { + installA: (llm) => + hideResponsesRoute(asContext(llm), resolveConfig(), { + hasCredential: () => false, + }), + probe: (llm) => call(llm, "listModels", "opencode"), + }, + targetOf: (llm) => llm, + }, + { + decorated: mergeDiscoveredModels([{ id: "minimax-m3" }], "go"), + host: (shape) => + shaped(shape, { discoverModels: async () => [{ id: "minimax-m3" }] }), + install: (llm, config = resolveConfig()) => + decorateModelDiscovery(asContext(llm), config), + keys: ["discoverModels"], + name: "discovery", + probe: (llm) => + call(llm, "discoverModels", "llm-pi-ai", { + provider: "opencode-go", + }), + raw: [{ id: "minimax-m3" }], + // Which requests are merged is read from `gatewayUrls`: under A a + // relay.example base is the Zen gateway and is merged, under the default it + // is nobody's and passes through, so a stacked default install would keep + // A's merge. + reconfigured: { + config: resolveConfig({ gatewayUrls: ["relay.example"] }), + probe: (llm) => + call(llm, "discoverModels", "llm-pi-ai", { + baseURL: "https://relay.example/v1", + }), + }, + targetOf: (llm) => llm, + }, + { + decorated: findModelSpecOn("go", "minimax-m3")?.context_window, + host: (shape) => shaped(shape, { resolveModelInfo: async () => ({}) }), + install: (llm, config = resolveConfig()) => + decorateModelInfo(asContext(llm), config), + keys: ["resolveModelInfo"], + name: "model-info", + probe: async (llm) => + contextWindowOf( + await call(llm, "resolveModelInfo", "opencode-go", "minimax-m3") + ), + raw: undefined, + // A name override is applied only where one is declared, so a stacked + // default install would keep the first activation's name. + reconfigured: { + config: resolveConfig({ + modelOverrides: { "minimax-m3": { name: "Activation A" } }, + }), + probe: async (llm) => + nameOf( + await call(llm, "resolveModelInfo", "opencode-go", "minimax-m3") + ), + }, + targetOf: (llm) => llm, + }, + { + decorated: planeBase("opencode", "deepseek-v4-pro"), + host: (shape) => { + const adapter = shaped(shape, adapterMethods()); + return { + adapters: new Map([ + ["opencode", { adapter }], + ["opencode-go", { adapter }], + ]), + }; + }, + install: (llm, config = resolveConfig()) => + decoratePlaneAdapters(llm as never, config), + keys: ["resolveModel", "prepareCall", "modelOf"], + name: "plane-adapters", + probe: async (llm) => + baseUrlOf( + await call( + adapterOf(llm), + "resolveModel", + "opencode", + "deepseek-v4-pro" + ) + ), + raw: undefined, + // The fill keeps a window the adapter already answered, so a stacked + // default install would keep the first activation's clamp; and `modelOf` + // (its own patch) fills only the levels the adapter refuses, so a stacked + // default install would build on A's declared levels instead of the + // catalog's. + reconfigured: { + config: resolveConfig({ + maxContextWindow: 1000, + modelOverrides: { "deepseek-v4-pro": { reasoningEfforts: ["low"] } }, + }), + probe: async (llm) => { + const adapter = adapterOf(llm); + const resolved = await call( + adapter, + "resolveModel", + "opencode", + "deepseek-v4-pro" + ); + const model = await call( + adapter, + "modelOf", + {}, + "opencode", + "deepseek-v4-pro" + ); + return { + contextWindow: contextWindowOf(resolved), + thinkingLevelMap: isRecord(model) + ? model.thinkingLevelMap + : undefined, + }; + }, + }, + targetOf: adapterOf, + }, +]; + +const CASES = SITES.flatMap((site) => + (["own", "inherited"] as const).map((shape) => ({ + shape, + site, + title: `${site.name} · original is ${shape}`, + })) +); + +describe("reload · every patch site", () => { + it.each(CASES.map((entry) => [entry.title, entry] as const))( + "%s · apply A → apply B → dispose A → B intact → dispose B → originals restored exactly", + async (_title, { shape, site }) => { + // The sequence reads "decorated" and "raw" off one call, so the two must + // differ, or a patch that never ran would pass it. + expect(site.decorated).toBeDefined(); + expect(site.decorated).not.toEqual(site.raw); + const llm = site.host(shape); + const target = site.targetOf(llm); + const before = ownDescriptors(target, site.keys); + await expect(site.probe(llm)).resolves.toEqual(site.raw); + + const a = site.install(llm); + const b = site.install(llm); + expect(a).toBeTypeOf("function"); + expect(b).toBeTypeOf("function"); + const installedB = ownValues(target, site.keys); + for (const key of site.keys) { + expect(installedB.get(key), key).toBeTypeOf("function"); + expect(installedB.get(key), key).not.toBe(before.get(key)?.value); + // B REPLACED A rather than wrapping it: both are live entries of one + // site id. A per-install id would leave B with no same-site + // predecessor, and B would wrap A's wrapper (one entry). + const chain = inspectPatch(installedB.get(key), key); + expect(chain, `same-site replacement: ${key}`).toHaveLength(2); + expect(chain?.[0], key).toEqual({ + holdsWrapper: true, + id: chain?.[1]?.id, + live: true, + }); + expect(chain?.[1], key).toEqual({ + holdsWrapper: true, + id: chain?.[0]?.id, + live: true, + }); + } + await expect(site.probe(llm)).resolves.toEqual(site.decorated); + + a?.(); + for (const key of site.keys) { + expect( + Reflect.getOwnPropertyDescriptor(target, key)?.value, + `B intact: ${key}` + ).toBe(installedB.get(key)); + } + await expect(site.probe(llm)).resolves.toEqual(site.decorated); + + b?.(); + for (const key of site.keys) { + const after = Reflect.getOwnPropertyDescriptor(target, key); + expect(after, `restored: ${key}`).toEqual(before.get(key)); + expect(after?.value, `restored: ${key}`).toBe(before.get(key)?.value); + } + await expect(site.probe(llm)).resolves.toEqual(site.raw); + } + ); + + it.each(CASES.map((entry) => [entry.title, entry] as const))( + "%s · apply A → apply B → dispose B → A intact → dispose A → originals restored exactly", + async (_title, { shape, site }) => { + expect(site.decorated).not.toEqual(site.raw); + const llm = site.host(shape); + const target = site.targetOf(llm); + const before = ownDescriptors(target, site.keys); + + const a = site.install(llm); + const installedA = ownValues(target, site.keys); + const b = site.install(llm); + expect(a).toBeTypeOf("function"); + expect(b).toBeTypeOf("function"); + + b?.(); + for (const key of site.keys) { + expect( + Reflect.getOwnPropertyDescriptor(target, key)?.value, + `A intact: ${key}` + ).toBe(installedA.get(key)); + } + await expect(site.probe(llm)).resolves.toEqual(site.decorated); + + a?.(); + for (const key of site.keys) { + const after = Reflect.getOwnPropertyDescriptor(target, key); + expect(after, `restored: ${key}`).toEqual(before.get(key)); + expect(after?.value, `restored: ${key}`).toBe(before.get(key)?.value); + } + await expect(site.probe(llm)).resolves.toEqual(site.raw); + } + ); +}); + +describe("reload · a changed config", () => { + const RECONFIGURED = CASES.filter(({ site }) => site.reconfigured); + + it.each(RECONFIGURED.map((entry) => [entry.title, entry] as const))( + "%s · A under another config or option, B under the default: the live answer is B's alone", + async (_title, { shape, site }) => { + const { reconfigured } = site; + if (reconfigured === undefined) { + throw new TypeError("filtered above"); + } + // A's install: its own, where what differs is not config. + const installA = + reconfigured.installA ?? + ((llm: object) => site.install(llm, reconfigured.config)); + // What each activation answers on its own, each on a fresh host. + const answerUnder = async ( + install: (llm: object) => (() => void) | undefined + ): Promise => { + const llm = site.host(shape); + const stop = install(llm); + const answer = await reconfigured.probe(llm); + stop?.(); + return answer; + }; + const answerA = await answerUnder(installA); + const answerB = await answerUnder((llm) => site.install(llm)); + expect(answerA).not.toEqual(answerB); + + const llm = site.host(shape); + const a = installA(llm); + await expect(reconfigured.probe(llm)).resolves.toEqual(answerA); + const b = site.install(llm); + await expect(reconfigured.probe(llm)).resolves.toEqual(answerB); + a?.(); + await expect(reconfigured.probe(llm)).resolves.toEqual(answerB); + b?.(); + } + ); +}); + +describe("reload · under a foreign wrapper, newest disposed first", () => { + it.each(CASES.map((entry) => [entry.title, entry] as const))( + "%s · B retires under it and hands the call to A, and the last dispose restores exactly", + async (_title, { shape, site }) => { + const llm = site.host(shape); + const target = site.targetOf(llm); + const before = ownDescriptors(target, site.keys); + const a = site.install(llm); + const b = site.install(llm); + + // Another plugin wraps every key on top of B, and remembers what it + // found so its own teardown can put it back. + const captured = ownValues(target, site.keys); + for (const key of site.keys) { + const inner = captured.get(key); + if (typeof inner !== "function") { + throw new TypeError(`nothing installed at ${key}`); + } + Reflect.set( + target, + key, + function foreign(this: unknown, ...args: unknown[]): unknown { + return Reflect.apply(inner, this, args); + } + ); + } + + b?.(); + // A is still live, so the answer stays decorated through B's retired + // wrapper under the foreign one. + await expect(site.probe(llm)).resolves.toEqual(site.decorated); + + for (const key of site.keys) { + Reflect.set(target, key, captured.get(key)); + } + await expect(site.probe(llm)).resolves.toEqual(site.decorated); + + a?.(); + for (const key of site.keys) { + const after = Reflect.getOwnPropertyDescriptor(target, key); + expect(after, `restored: ${key}`).toEqual(before.get(key)); + expect(after?.value, `restored: ${key}`).toBe(before.get(key)?.value); + } + await expect(site.probe(llm)).resolves.toEqual(site.raw); + } + ); +}); + +describe("reload · on the real host (tier A)", { timeout: 15_000 }, () => { + let tier: TierA | undefined; + + afterEach(async () => { + await tier?.dispose(); + tier = undefined; + }); + + it.each( + SITES.flatMap((site) => [ + [site.name, "first disposed first", site, true] as const, + [site.name, "last disposed first", site, false] as const, + ]) + )("%s · %s", async (name, _order, site, fifo) => { + tier = await mountTierA({ providers: s0Providers() }); + const { llm } = tier; + let target: object = llm; + if (name === "plane-adapters") { + const shared = adaptersOf(llm).get("opencode")?.adapter; + expect(shared).toBeDefined(); + // pi-ai serves both planes from one instance, which is the nesting the + // plane-adapter patch composes over. + expect(adaptersOf(llm).get("opencode-go")?.adapter).toBe(shared); + target = shared ?? {}; + } + const listed = llm.listProviders(); + const info = await llm.resolveModelInfo("opencode", "deepseek-v4-pro"); + for (const key of site.keys) { + expect(Object.hasOwn(target, key), key).toBe(false); + } + + const a = site.install(llm); + const installedA = ownValues(target, site.keys); + const b = site.install(llm); + expect(a).toBeTypeOf("function"); + expect(b).toBeTypeOf("function"); + const installedB = ownValues(target, site.keys); + for (const key of site.keys) { + expect(Object.hasOwn(target, key), key).toBe(true); + } + + (fifo ? a : b)?.(); + const survivor = fifo ? installedB : installedA; + for (const key of site.keys) { + expect( + Reflect.getOwnPropertyDescriptor(target, key)?.value, + `survivor intact: ${key}` + ).toBe(survivor.get(key)); + } + if (name === "plane-adapters") { + expect( + baseUrlOf( + await call(target, "resolveModel", "opencode", "deepseek-v4-pro") + ) + ).toBe(planeBase("opencode", "deepseek-v4-pro")); + } + + (fifo ? b : a)?.(); + for (const key of site.keys) { + expect(Object.hasOwn(target, key), `restored: ${key}`).toBe(false); + } + expect(llm.listProviders()).toEqual(listed); + await expect( + llm.resolveModelInfo("opencode", "deepseek-v4-pro") + ).resolves.toEqual(info); + }); +}); + +describe("reload · through apply()", () => { + it("apply() twice, the first disposed first: every host method the plugin patched is restored exactly", async () => { + // The global fetch is patched too, and stacks the same way; it is not a host + // service method and is not asserted here (P6 owns it). + vi.stubGlobal("fetch", vi.fn()); + vi.spyOn(console, "info").mockImplementation(() => { + // Silenced: the activation line and the hidden-route stand-down. + }); + class Adapter { + modelOf(): unknown { + return { id: "x", reasoning: false }; + } + async prepareCall(): Promise { + return {}; + } + async resolveModel(): Promise { + return {}; + } + } + const adapter = new Adapter(); + class HostLlm { + readonly adapters = new Map([ + ["opencode", { adapter }], + ["opencode-go", { adapter }], + ]); + async discoverModels(): Promise { + return [{ id: "minimax-m3" }]; + } + listConfigurableProviders(): unknown { + return [{ provider: "opencode" }, { provider: RESPONSES_ROUTE }]; + } + async listModels(): Promise { + return [{ id: "opencode-model" }]; + } + listProviders(): unknown { + return LISTED.map((entry) => ({ ...entry })); + } + async resolveModelInfo(): Promise { + return {}; + } + } + const llm = new HostLlm(); + const llmKeys = [ + "listProviders", + "listConfigurableProviders", + "listModels", + "discoverModels", + "resolveModelInfo", + ]; + const adapterKeys = ["resolveModel", "prepareCall", "modelOf"]; + + const cleanups: (() => void)[] = []; + const ctxFor = (): CordisContext => + ({ + effect: (fn: () => unknown) => { + const cleanup = fn(); + if (typeof cleanup === "function") { + cleanups.push(() => { + cleanup(); + }); + } + }, + llm, + on: vi.fn(), + }) as unknown as CordisContext; + + apply(ctxFor()); + apply(ctxFor()); + expect(cleanups).toHaveLength(2); + const llmB = ownValues(llm, llmKeys); + const adapterB = ownValues(adapter, adapterKeys); + for (const key of llmKeys) { + expect(llmB.get(key), key).toBeTypeOf("function"); + } + for (const key of adapterKeys) { + expect(adapterB.get(key), key).toBeTypeOf("function"); + } + + cleanups[0]?.(); + for (const key of llmKeys) { + expect(Reflect.getOwnPropertyDescriptor(llm, key)?.value, key).toBe( + llmB.get(key) + ); + } + for (const key of adapterKeys) { + expect(Reflect.getOwnPropertyDescriptor(adapter, key)?.value, key).toBe( + adapterB.get(key) + ); + } + expect(llm.listProviders()).toEqual([{ id: "opencode", name: "opencode" }]); + + cleanups[1]?.(); + for (const key of llmKeys) { + expect(Object.hasOwn(llm, key), key).toBe(false); + } + for (const key of adapterKeys) { + expect(Object.hasOwn(adapter, key), key).toBe(false); + } + expect(llm.listProviders()).toEqual(LISTED); + }); +}); diff --git a/test/support/src.ts b/test/support/src.ts index b42ae157..e38648ab 100644 --- a/test/support/src.ts +++ b/test/support/src.ts @@ -136,6 +136,13 @@ export { } from "../../src/google-bridge.ts"; export type { GoogleBridgeInputs, PiRuntime } from "../../src/google-bridge.ts"; +// host/patch-method.ts +export { + inspectPatch, + patchMethod, + rewrapOnAdaptersUpdated, +} from "../../src/host/patch-method.ts"; + // identity.ts export { inject, diff --git a/vite.config.ts b/vite.config.ts index c9ff8205..a6d9d376 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -273,6 +273,13 @@ export default defineConfig({ functions: 100, lines: 100, }, + // The one way to patch a host method: every branch is a reload case. + "src/host/patch-method.ts": { + statements: 100, + branches: 100, + functions: 100, + lines: 100, + }, }, }, }, From 3cfacd9e993f1de5b437ef009255895a8de122f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 15:11:16 +0000 Subject: [PATCH 2/2] fix: address the second review of P-H What the review found, and what changed: - rewrapOnAdaptersUpdated reinstalls its site inside the plugin's own teardown and on every foreign registry change. Its JSDoc in src/host/patch-method.ts now says so, counts the reinstalls a release causes, says a caller disposes it before releasing its adapters, and notes that a throw from a reinstall is logged by the host. - The plan of record said all three P-H edge changes are unreachable on the real host for one reason; the third (the live-reload listing) is unreachable for a different one. docs/improvement-plan.md now gives each its reason. - The tier-A reload cases for the listing and discovery sites proved the row only structurally. test/reload.test.ts now gives every site a tier-A probe whose decorated answer must differ from the host's own, so "survivor still decorates" and "last dispose restores" can fail. - "patchMethod is the only way the plugin wraps a host method" was asserted in docs and enforced by nothing. test/architecture.test.ts now has a textual tripwire over everything apply() reaches, with a self-check of planted violations, and the apply()-twice reload case requires a patch record on every own function apply() leaves on the stand-in. - The tripwire missed several write forms (Object.assign, Object.defineProperty/defineProperties, Reflect.setPrototypeOf, Object.setPrototypeOf, and an Object.* target reached through a property path such as registration?.adapter or this.llm). The patterns now catch them, and the self-check plants each one (17 planted, up from 13). Nothing new in src/ matches. - The docs said "any Reflect.* write" where the pattern matches a fixed set. The test docstring, AGENTS.md and the plan's P-H section now name the four Reflect and four Object methods matched, and why a prototype swap is listed. No runtime behaviour changed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0141fKp5k6hiaJcP5ovhGtSD --- AGENTS.md | 2 +- docs/improvement-plan.md | 4 +- src/host/patch-method.ts | 12 ++++ test/architecture.test.ts | 106 ++++++++++++++++++++++++++++++++++ test/reload.test.ts | 116 +++++++++++++++++++++++++++++++++++--- 5 files changed, 229 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0eff0cdc..3f1d5417 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,7 +94,7 @@ It is the same trick `scripts/check.ts` already uses for the client bundle, at f - `src/responses-routes.ts` · `src/responses-provider.ts` — the protocol table and the in-process mount of the host's `llm-pi-ai`. - `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/host/patch-method.ts` — `patchMethod`, the one way to wrap a method on a host object. A dispose restores only while its wrapper is still the current value (otherwise it becomes a pass-through under whatever wrapped on top), and deletes the own property when the original was inherited rather than assigning it back; a second install of the same site wraps the unpatched method instead of the first install. Identity is read from own descriptors, never from a property read, because cordis's traceable proxy answers a fresh proxy for every method read (`docs/engineering-notes.md`). `rewrapOnAdaptersUpdated` reinstalls a site on `llm/adapters-updated`, for a site that must follow the registry; none uses it yet. Every instance patch in `src/` goes through this module — today the four sites in `models-discovery.ts` — and `test/reload.test.ts` runs apply A, apply B, dispose A, dispose B on each. +- `src/host/patch-method.ts` — `patchMethod`, the one way to wrap a method on a host object. A dispose restores only while its wrapper is still the current value (otherwise it becomes a pass-through under whatever wrapped on top), and deletes the own property when the original was inherited rather than assigning it back; a second install of the same site wraps the unpatched method instead of the first install. Identity is read from own descriptors, never from a property read, because cordis's traceable proxy answers a fresh proxy for every method read (`docs/engineering-notes.md`). `rewrapOnAdaptersUpdated` reinstalls a site on `llm/adapters-updated`, for a site that must follow the registry; none uses it yet. Every instance patch in `src/` goes through this module — today the four sites in `models-discovery.ts` — and `test/reload.test.ts` runs apply A, apply B, dispose A, dispose B on each. Two tripwires hold the rule. `test/architecture.test.ts` is textual: anywhere `apply()` reaches but the helper, it fails on a `Reflect.set`/`defineProperty`/`deleteProperty`/`setPrototypeOf`, an `Object.assign`/`defineProperty`/`defineProperties`/`setPrototypeOf` onto `llm`, an adapter, `settings` or `globalThis` (bare or through a property path such as `registration?.adapter`; a prototype swap shadows a method without leaving an own property), a dotted or bracketed assignment (through `?.` or `!` too) onto the first three, and `globalThis.x =` (the global `fetch` patch is the one named exemption, until P6); a write through a local alias or a cast receiver escapes it. The `apply()`-twice reload case is behavioural: it requires the helper's record on every own function `apply()` leaves on the stand-in's `llm` and adapter, and P-R1 adds `settings` to the stand-in together with its site. - `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`. - `src/attachment-services.ts` — optional attachment and filesystem services for Google image requests, captured through Cordis child scopes. - `src/google-bridge.ts` — the Google models, served through the plugin's own adapter on the real `llm` service. It is the ONLY module that imports pi at runtime, and its route is `opencode-google` so the `google` name stays free for a real deployment. diff --git a/docs/improvement-plan.md b/docs/improvement-plan.md index 0957f17d..2ba93fcf 100644 --- a/docs/improvement-plan.md +++ b/docs/improvement-plan.md @@ -378,10 +378,10 @@ What stays private is pinned by contract tests against the real host packages: - **Shapes.** A resolved profile also carries the profile's pass-through fields (`compat`, `defaultContextWindow`, `defaultInput`, `defaultMaxTokens`, `headers`, `modelOverrides`, `thinkingBudgets`), and a built pi provider exposes `getModels()` but no `api` map, so F10 is pinned by behaviour (pi's own "no API implementation" error) rather than by its keys. 6. **P-H `refactor:` one way to patch a host method.** Landed as: - - **The helper.** `src/host/patch-method.ts` (`patchMethod`) is the only way the plugin wraps a method on a host object. A dispose restores only while its own descriptor still holds its wrapper, deletes the own property when the original was inherited, and a same-site reinstall wraps the unpatched method. It carries a 100% per-file coverage floor. + - **The helper.** `src/host/patch-method.ts` (`patchMethod`) is the only way the plugin wraps a method on a host object. A dispose restores only while its own descriptor still holds its wrapper, deletes the own property when the original was inherited, and a same-site reinstall wraps the unpatched method. It carries a 100% per-file coverage floor, and two tripwires hold the "only way". The architecture rule is textual, over everything `apply()` reaches: a `Reflect.set`/`defineProperty`/`deleteProperty`/`setPrototypeOf`, an `Object.assign`/`defineProperty`/`defineProperties`/`setPrototypeOf` onto `llm`, an adapter, `settings` or `globalThis` (bare or through a property path such as `registration?.adapter`; a prototype swap shadows a method without leaving an own property), a dotted or bracketed assignment (through `?.` or `!` too) onto the first three, and `globalThis.x =` (the global `fetch` patch named as the one exemption, until P6); a write through a local alias or a cast receiver escapes it. The `apply()`-twice reload case is behavioural: a patch record required on every own function `apply()` leaves on the stand-in's `llm` and adapter, with `settings` added to the stand-in by P-R1 together with its site. - **The four migrated sites,** all in `models-discovery.ts`: `hideResponsesRoute` (the three listings), `decorateModelDiscovery`, `decorateModelInfo` and `decoratePlaneAdapters`. No wrapper's answer changed on a host that accepts the patch. The existing green pins of each site's behaviour in `test/models-discovery.test.ts` opened the phase and stayed unchanged; the five new cases there marked "a deliberate change in P-H" were added red: four over the three edge changes below (two for the adapter bullet: a frozen adapter and one refused method), and one for the helper's deletion of an inherited original (a class adapter's `resolveModel`, which the base assigned back as an own property). - **The reload test.** `test/reload.test.ts` runs apply A → apply B → dispose A → B intact → dispose B → originals restored exactly, and the reverse disposal order, for every site on own and inherited originals and on the real host (tier A). cordis itself disposes before it re-applies (`Fiber.update` → `restart` → `_unload` → `_reload`), so the overlapping sequence models two activations live at once (two rows) and the base's stacking, where a dispose restored nothing and the next activation wrapped the leftover. A stacked install leaks wherever the decoration reads config, which every site does: a reload whose config changed (the live answer is B's alone) pins discovery (`gatewayUrls`), model-info, both plane-adapter patches (`resolveModel`'s window and `modelOf`'s levels) and the listing's `listModels` (its per-activation keyless option, `hasCredential`, rather than config); the listing's `listProviders` capture is pinned by the `isRouteRegistered` reinstall case in `test/models-discovery.test.ts`. Structurally, for every key of every site, the second install is checked to have REPLACED the first (two live entries of one site id). A foreign wrapper on top with the newest install disposed first is run too. `apply()` itself is driven twice, first disposed first, on a stand-in context. All four sites failed it before the helper. - - **Deliberate edge changes**, each pinned; the first three are not reachable with the real `LlmRuntime` and pi-ai adapter (both extensible class instances): + - **Deliberate edge changes**, each pinned; none of the three is reachable with the real `LlmRuntime` and pi-ai adapter: the first two because both are extensible class instances, the third because the real `LlmRuntime` keeps its `adapters` map as an own property the traceable proxy hands through, and `isRouteRegistered` answers from that map before it reads the captured listing (the listing is the evidence only on a host without one): - **The listing install is all or nothing on a refused define.** Before, a silent refusal (`Reflect.set` answering false on a non-writable `listModels`) left the other two listings filtered and returned a disposer; now all three stay as they were, which is what a throwing refusal already did. The three listings must agree on what is hidden. - **A refused plane-adapter method is skipped.** Before, `adapter[method] = wrapped` threw a `TypeError` out of the fetch-patch effect on a frozen adapter; now that method keeps its own, the others are patched, and a fully frozen adapter answers `undefined`. Each adapter fill is independent, so this site is per method. - **A reinstall's `registeredRoutes` reads the host's own listing**, not the previous install's filtered one, so `isRouteRegistered` keeps finding the hidden route after a reload. diff --git a/src/host/patch-method.ts b/src/host/patch-method.ts index 15f1d19e..2e6cc471 100644 --- a/src/host/patch-method.ts +++ b/src/host/patch-method.ts @@ -326,6 +326,18 @@ export const inspectPatch = ( * shared adapter in the plugin-first order too, and flip the host * characterization's `[KNOWN_DEFECT]` rows. * + * Two things the first caller has to know. The listener reinstalls on EVERY + * registry change while it is live, the caller's own included, so a caller that + * also registers adapters disposes this BEFORE it releases them: the release + * emits `llm/adapters-updated` synchronously, and a live listener would + * reinstall the site once per registration the release withdraws (today each + * Google bridge handle and the mounted pi-ai's adapter handle; its + * configurable-provider registration is answered locally by the mount's facade + * and emits nothing). The fetch-patch cleanup in `lifecycle.ts` releases the + * hidden routes first, so order it there when this is wired. And `install` runs synchronously inside the host's per-listener + * `try`/`catch` in `LlmRuntime.emitAdaptersUpdated`, so a throw from a + * reinstall is logged by the host, not propagated to the plugin. + * * @param ctx - the context whose (global) registry events to follow. * @param install - one install; answers its disposer, or `undefined`. * @returns the disposer: stops listening and disposes the current install. diff --git a/test/architecture.test.ts b/test/architecture.test.ts index 2f9474fd..467290f4 100644 --- a/test/architecture.test.ts +++ b/test/architecture.test.ts @@ -520,6 +520,112 @@ describe("architecture · the host bundle mutates no registry it does not own", }); }); +describe("architecture · host methods are wrapped only through patchMethod", () => { + /** The host half's composition root: everything `apply()` can run. */ + const ENTRY = "lifecycle.ts"; + /** The one module that may write a method onto a host object. */ + const HELPER = "host/patch-method.ts"; + + /** + * A write onto a host object by any route but the helper: a + * `Reflect.set`/`defineProperty`/`deleteProperty`/`setPrototypeOf`; an + * `Object.assign`/`defineProperty`/`defineProperties`/`setPrototypeOf` whose + * target is `llm`, an adapter, `settings` (the next site, P-R1's + * `settings.describe`) or `globalThis`, bare or reached through a property + * path (`registration?.adapter`, `this.llm`); a dotted or bracketed + * assignment onto one of the first three, through `?.` or `!` too; or + * `globalThis.x =`. A prototype swap is listed because it shadows a method + * without leaving the own property the behavioural half inspects. + * A textual rule, so a write through a local alias or a cast receiver + * escapes it. The behavioural half is the `apply()`-twice case in + * `test/reload.test.ts`, which requires a patch record on every own function + * `apply()` leaves on its stand-in `llm` and adapter (P-R1 adds `settings` to + * the stand-in together with its site). + */ + const HOST_WRITES = [ + /\bReflect\.(?:set|defineProperty|deleteProperty|setPrototypeOf)\s*\(/gu, + /\bObject\.(?:assign|defineProperty|defineProperties|setPrototypeOf)\(\s*(?:[\w$]+[?!]?\.)*(?:llm|adapter|settings|globalThis)\b/gu, + /\b(?:llm|adapter|settings)[!?]?\.\w+\s*=(?![=>])/gu, + /\b(?:llm|adapter|settings)!?\[[^\]\n]+\]\s*=(?![=>])/gu, + /\bglobalThis\.\w+\s*=(?![=>])/gu, + ]; + + /** + * Named exemptions, each `: `. The global `fetch` + * patch stacks the same way but is a global, not a host service method, and + * P6 owns it (docs/improvement-plan.md). + */ + const EXEMPT = new Set(["lifecycle.ts: globalThis.fetch ="]); + + /** Every host write in `source`, as `: `. */ + const hostWritesIn = (file: string, source: string): string[] => + HOST_WRITES.flatMap((pattern) => + [...source.matchAll(pattern)].map((match) => `${file}: ${match[0]}`) + ); + + it("flags a planted violation of each kind, so a pass means something", () => { + const planted = [ + "llm.listModels = wrapped;", + "ctx.llm.resolveModelInfo = async () => ({});", + 'adapter["resolveModel"] = wrapped;', + "settings.describe = describe;", + "Reflect.set(llm, key, wrapped);", + "Reflect.defineProperty(adapter, key, { value: wrapped });", + "Reflect.deleteProperty(llm, key);", + "globalThis.fetch = patched;", + "ctx.llm!.listModels = wrapped;", + "adapter![key] = wrapped;", + 'Object.defineProperty(settings, "describe", { value: wrapped });', + "Object.assign(ctx.llm, { listModels: wrapped });", + "Object.defineProperties(adapter, { modelOf: { value: wrapped } });", + // A prototype swap shadows a method without leaving an own property. + "Reflect.setPrototypeOf(llm, proto);", + "Object.setPrototypeOf(adapter, proto);", + // The receiver reached through a property path, as decoratePlaneAdapters + // reaches the adapter. + "Object.assign(registration?.adapter, { modelOf: wrapped });", + 'Object.defineProperty(this.llm, "listModels", { value: wrapped });', + ].join("\n"); + expect(hostWritesIn("planted.ts", planted)).toHaveLength(17); + // Reads, comparisons and a define onto the plugin's own prototype are not + // host writes. + expect( + hostWritesIn( + "planted.ts", + "if (llm.listModels === x) {}\nconst f = (adapter) => adapter.modelOf;\nconst g = llm.x == null;\n" + + 'Object.defineProperty(UsageService.prototype, "inspect", marker);' + ) + ).toEqual([]); + }); + + it("walks a graph that contains the helper and the sites it serves", () => { + const graph = runtimeGraph(ENTRY); + expect(graph.has(HELPER)).toBe(true); + expect(graph.has("models-discovery.ts")).toBe(true); + // The helper itself is what the rule exempts, and it does match it. + expect(hostWritesIn(HELPER, graph.get(HELPER) ?? "")).not.toEqual([]); + }); + + it("nothing reachable from lifecycle.ts but the helper writes a method onto a host object", () => { + const violations = [...runtimeGraph(ENTRY)] + .filter(([file]) => file !== HELPER) + .flatMap(([file, source]) => hostWritesIn(file, source)) + .filter((write) => !EXEMPT.has(write)); + expect(violations).toEqual([]); + }); + + it("every named exemption is still there, so a stale one is noticed", () => { + const writes = new Set( + [...runtimeGraph(ENTRY)].flatMap(([file, source]) => + hostWritesIn(file, source) + ) + ); + for (const exempt of EXEMPT) { + expect(writes.has(exempt), exempt).toBe(true); + } + }); +}); + describe("architecture · tests reach src through one door", () => { const SUPPORT = join(import.meta.dirname, "support", "src.ts"); diff --git a/test/reload.test.ts b/test/reload.test.ts index 4d8416c6..e2172373 100644 --- a/test/reload.test.ts +++ b/test/reload.test.ts @@ -31,7 +31,9 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { adaptersOf, + type HostLlm, mountTierA, + planeKeysOnly, s0Providers, type TierA, } from "./support/host.ts"; @@ -120,6 +122,14 @@ const nameOf = (value: unknown): unknown => const baseUrlOf = (value: unknown): unknown => isRecord(value) ? value.baseUrl : undefined; +const idsOf = (value: unknown): unknown => + Array.isArray(value) + ? value.map((entry: unknown) => (isRecord(entry) ? entry.id : entry)) + : value; + +/** The settings namespace the tier-A discovery probe registers under. */ +const PROBE_NAMESPACE = "reload-probe"; + /** The base a plane's catalog dispatches `id` to, under the default endpoint. */ const planeBase = ( route: "opencode" | "opencode-go", @@ -173,6 +183,21 @@ interface Site { readonly installA?: (llm: object) => (() => void) | undefined; readonly probe: (llm: Record) => Promise; }; + /** + * The same site on the real host (tier A), where the stand-in's probe does not + * apply: a call whose answer through the site's decoration differs from the + * host's own THERE, so the block's "survivor still decorates" and "last + * dispose restores" checks can fail. Without one, the listing would be probed + * by a filter with nothing to filter (tier A mounts no hidden route) and + * discovery not at all. + */ + readonly tierA: { + /** The install the block drives, where the site's default decorates nothing there. */ + readonly install?: (llm: object) => (() => void) | undefined; + /** Host state the probe needs, set before the first install. */ + readonly setup?: (llm: HostLlm) => void; + readonly probe: (llm: HostLlm) => Promise; + }; } const LISTED = [ @@ -228,6 +253,15 @@ const SITES: readonly Site[] = [ }), probe: (llm) => call(llm, "listModels", "opencode"), }, + // pi-ai lists the plane's catalog once a key is stored; with no credential + // the keyless filter drops the priced ids the host's own answer keeps. + tierA: { + install: (llm) => + hideResponsesRoute(asContext(llm), resolveConfig(), { + hasCredential: () => false, + }), + probe: async (llm) => idsOf(await llm.listModels("opencode")), + }, targetOf: (llm) => llm, }, { @@ -254,6 +288,17 @@ const SITES: readonly Site[] = [ baseURL: "https://relay.example/v1", }), }, + // A discovery registered on the real runtime, answering one id with no + // network: the decoration merges by the request's route alone. + tierA: { + probe: (llm) => + llm.discoverModels(PROBE_NAMESPACE, { provider: "opencode-go" }), + setup: (llm) => { + llm.registerModelDiscovery(PROBE_NAMESPACE, async () => [ + { id: "minimax-m3" }, + ]); + }, + }, targetOf: (llm) => llm, }, { @@ -279,6 +324,12 @@ const SITES: readonly Site[] = [ await call(llm, "resolveModelInfo", "opencode-go", "minimax-m3") ), }, + // pi's installed catalog is an older snapshot than ours for this id, and the + // decoration's window is authoritative. + tierA: { + probe: async (llm) => + contextWindowOf(await llm.resolveModelInfo("opencode", "gpt-5.4")), + }, targetOf: (llm) => llm, }, { @@ -339,6 +390,17 @@ const SITES: readonly Site[] = [ }; }, }, + tierA: { + probe: async (llm) => + baseUrlOf( + await call( + adaptersOf(llm).get("opencode")?.adapter ?? {}, + "resolveModel", + "opencode", + "deepseek-v4-pro" + ) + ), + }, targetOf: adapterOf, }, ]; @@ -545,7 +607,11 @@ describe("reload · on the real host (tier A)", { timeout: 15_000 }, () => { [site.name, "last disposed first", site, false] as const, ]) )("%s · %s", async (name, _order, site, fifo) => { - tier = await mountTierA({ providers: s0Providers() }); + // Plane keys stored, so pi-ai lists the planes' catalogs (the listing probe). + tier = await mountTierA({ + credentials: planeKeysOnly(), + providers: s0Providers(), + }); const { llm } = tier; let target: object = llm; if (name === "plane-adapters") { @@ -556,21 +622,28 @@ describe("reload · on the real host (tier A)", { timeout: 15_000 }, () => { expect(adaptersOf(llm).get("opencode-go")?.adapter).toBe(shared); target = shared ?? {}; } + site.tierA.setup?.(llm); + const install = site.tierA.install ?? site.install; const listed = llm.listProviders(); const info = await llm.resolveModelInfo("opencode", "deepseek-v4-pro"); + const raw = await site.tierA.probe(llm); for (const key of site.keys) { expect(Object.hasOwn(target, key), key).toBe(false); } - const a = site.install(llm); + const a = install(llm); const installedA = ownValues(target, site.keys); - const b = site.install(llm); + const b = install(llm); expect(a).toBeTypeOf("function"); expect(b).toBeTypeOf("function"); const installedB = ownValues(target, site.keys); for (const key of site.keys) { expect(Object.hasOwn(target, key), key).toBe(true); } + // The probe has to tell the decoration from the host's own answer here, + // or the two checks against `raw` below could not fail. + const decorated = await site.tierA.probe(llm); + expect(decorated).not.toEqual(raw); (fifo ? a : b)?.(); const survivor = fifo ? installedB : installedA; @@ -580,18 +653,19 @@ describe("reload · on the real host (tier A)", { timeout: 15_000 }, () => { `survivor intact: ${key}` ).toBe(survivor.get(key)); } + await expect(site.tierA.probe(llm), "survivor decorates").resolves.toEqual( + decorated + ); if (name === "plane-adapters") { - expect( - baseUrlOf( - await call(target, "resolveModel", "opencode", "deepseek-v4-pro") - ) - ).toBe(planeBase("opencode", "deepseek-v4-pro")); + expect(decorated).toBe(planeBase("opencode", "deepseek-v4-pro")); } (fifo ? b : a)?.(); for (const key of site.keys) { expect(Object.hasOwn(target, key), `restored: ${key}`).toBe(false); } + await expect(site.tierA.probe(llm), "restored").resolves.toEqual(raw); + // Smoke: other calls through the restored receiver still answer as before. expect(llm.listProviders()).toEqual(listed); await expect( llm.resolveModelInfo("opencode", "deepseek-v4-pro") @@ -665,9 +739,35 @@ describe("reload · through apply()", () => { on: vi.fn(), }) as unknown as CordisContext; + const ownBefore = { + adapter: new Set(Reflect.ownKeys(adapter)), + llm: new Set(Reflect.ownKeys(llm)), + }; apply(ctxFor()); apply(ctxFor()); expect(cleanups).toHaveLength(2); + // The behavioural tripwire for "patchMethod is the only way": whatever + // syntax a later site used, every function `apply()` left as an own property + // of a host object carries the helper's record. A bare `llm.x = wrapped` + // would leave an own function with none. + for (const [label, host] of [ + ["llm", llm], + ["adapter", adapter], + ] as const) { + const added = Reflect.ownKeys(host).filter( + (key) => !ownBefore[label].has(key) + ); + expect(added.length, label).toBeGreaterThan(0); + for (const key of added) { + const value = Reflect.getOwnPropertyDescriptor(host, key)?.value; + if (typeof value === "function" && typeof key === "string") { + expect( + inspectPatch(value, key), + `${label}.${key} installed through patchMethod` + ).toBeDefined(); + } + } + } const llmB = ownValues(llm, llmKeys); const adapterB = ownValues(adapter, adapterKeys); for (const key of llmKeys) {