Skip to content

Commit c3287b8

Browse files
authored
fix(overlay): memoize getAvailableModels per OpencodeClient instance (#383)
* fix(overlay): memoize getAvailableModels per client identity Adds a module-scope WeakMap<OpencodeClientLike, ModelAvailability> cache so multi-source plugin loads share the discovery result instead of each running a fresh HTTP round-trip. Within one OpenCode process lifecycle, repeated calls with the same client reference return the cached envelope by reference. Only successful discovery ('api' and 'cache' status) populates the cache. 'unknown' outcomes are intentionally left uncached so that a transient failure on the first call does not pin every subsequent call into restart-required mode for the rest of the process lifetime - the next call gets a fresh attempt instead. The cache invalidates automatically when the OpencodeClient reference is collected (i.e., on OpenCode restart), so no TTL is needed. The OpencodeClient identity is the natural memoization key because the OpenCode plugin loader builds it once per process and shares the same reference across every plugin factory (verified empirically against anomalyco/opencode v1.15.1 plugin loader source). Adds four memoization tests covering: same-client api cache hit, distinct-client miss, no-cache for unknown status, and the cache-skip behavior on second call when the first returned 'cache' status. * docs(config): describe availability memoization and restart contract Documents the new WeakMap-based memoization of getAvailableModels in the Availability-Aware Resolution subsection of the configuration guide. Three contracts are spelled out: - Memoization scope: successful discovery ('api' and 'cache') is memoized for the OpenCode process lifetime; 'unknown' outcomes are intentionally left uncached so transient failures can retry. - Restart contract: provider-state changes (auth login/logout, env var edits) are not reflected until OpenCode is restarted. - No-mutation contract: callers receive the cached envelope by reference; the returned models set is ReadonlySet<string> and must not be mutated. Also adds the originating plan to the durable record under docs/plans/.
1 parent c9cb642 commit c3287b8

4 files changed

Lines changed: 422 additions & 7 deletions

File tree

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
---
2+
title: 'fix(overlay): memoize getAvailableModels per OpencodeClient instance'
3+
type: fix
4+
status: active
5+
date: 2026-05-16
6+
origin: docs/brainstorms/2026-05-14-provider-availability-dx-hardening-requirements.md
7+
---
8+
9+
# fix(overlay): memoize getAvailableModels per OpencodeClient instance
10+
11+
## Overview
12+
13+
Add module-scope memoization to `getAvailableModels` keyed on `OpencodeClientLike` identity via `WeakMap`. Within one OpenCode process lifecycle, repeated calls with the same `client` reuse the result. Multi-source Systematic plugin loads collapse N HTTP round-trips to one for **successful discovery outcomes**; `'unknown'` results are not cached so transient failures can retry.
14+
15+
Two distinct caches exist in this code path. To avoid ambiguity, this plan uses precise terminology throughout:
16+
- **WeakMap memoization cache** — the new module-scope per-process cache added by this plan
17+
- **`models.json` disk cache** — the existing on-disk fallback at `~/.cache/opencode/models.json` (unchanged)
18+
19+
Ships as v2.14.5 (`fix:` patch).
20+
21+
## Problem Frame
22+
23+
PR #370 made dual-Systematic loads a real and supported contributor configuration. Each plugin source invokes its own config hook, and each config hook calls `getAvailableModels` afresh — N HTTP round-trips with N independent 1500ms timeout budgets. The empty-`'api'` collapse fix shipped in v2.14.3 made second-source calls cheap-to-fail (returns `unknown` quickly when discovery is empty), but the duplicate work itself is still wasteful when discovery succeeds.
24+
25+
Empirical inspection of `anomalyco/opencode@v1.15.1` plugin loader at `packages/opencode/src/plugin/index.ts:128-150` proves that `PluginInput.client` is built once per OpenCode process and passed by reference to every plugin factory. That shared `client` identity is the natural memoization key.
26+
27+
## Requirements Trace
28+
29+
- R1. `getAvailableModels` MUST be memoized at module scope using `WeakMap<OpencodeClientLike, ModelAvailability>` for **successful discovery outcomes** (`'api'` and `'cache'` status). The WeakMap auto-invalidates when `client` is collected. `'unknown'` outcomes MUST NOT be cached so transient failures can retry on the next call (origin: brainstorm R1, refined by document review).
30+
- R2. The `docs/src/content/docs/getting-started/configuration.mdx` "Availability-Aware Resolution" subsection MUST be updated to describe: (a) process-scoped memoization for successful results only, (b) the explicit restart contract for provider-state changes, and (c) the no-mutation contract for callers (the cached envelope is returned by reference). All three clauses verified in Unit 2's acceptance.
31+
32+
## Scope Boundaries
33+
34+
- No TTL on memoization — process lifetime is the contract for cached entries
35+
- No promise-dedupe for concurrent in-flight calls — not needed under today's FIFO loader topology
36+
- No subprocess/MCP isolation — module scope only covers the main OpenCode process
37+
- No auth-event hook integration — OpenCode side change required
38+
39+
### Deferred to Separate Tasks
40+
41+
- Typed config validation (build-time agent/skill name enumeration + Zod enums + IDE autocomplete): separate brainstorm at `docs/brainstorms/2026-05-16-typed-config-validation-requirements.md` (status `draft`)
42+
- Solution-doc cross-reference edits: separate `docs(solutions):` PR after this PR ships (matches PR #376 pattern)
43+
- 1500ms timeout symptom on first call: smart note `#95` (this plan reduces N timeouts to 1, doesn't eliminate the first call's latency)
44+
- Mid-process invalidation on auth changes: explicit non-goal; restart contract documented in R2
45+
46+
## Context & Research
47+
48+
### Relevant Code and Patterns
49+
50+
- `src/lib/model-availability.ts:266-340` — current `getAvailableModels(client, options?)` implementation; **6 return paths** to handle (enumerated in Unit 1)
51+
- `src/lib/model-availability.ts:51-61``ModelAvailability` interface with `ReadonlySet` `models` contract (memory `#2963`)
52+
- `src/lib/model-availability.ts:69-71``emptyAvailability()` factory (each caller gets a fresh Set, not a shared singleton)
53+
- `src/lib/config-handler.ts:557` — single call site for `getAvailableModels`; verified to consume the return value only with no signature dependency
54+
- `tests/unit/model-availability.test.ts` — existing 857-line test file; the empty-discovery test block is the structural template for new memoization tests
55+
56+
### Institutional Learnings
57+
58+
- Memory `#2963` (factory + ReadonlySet pattern): `ReadonlySet` is a type-level trust contract, not a runtime immutability guarantee. The cached envelope is safe to return by reference because the existing codebase honors the contract; downstream code that casts and mutates would corrupt the cache, but no such code exists today.
59+
- Memory `#3005` (contributor DX invariant): local checkout wins over npm-installed Systematic. Multi-source plugin loads are a supported configuration.
60+
- Memory `#2065` (scope-discipline split signal): typed-validation work was split into its own brainstorm. This plan honors the split.
61+
- Memory `#2767` (TDD): RED-GREEN cycle, failing test first.
62+
- Memory `#2685` (`docs:build` in pre-PR gate): Unit 2's docs change must be verified.
63+
- Memory `#2734` (`fix:` triggers patch): `fix(overlay):` → v2.14.5.
64+
65+
### External References
66+
67+
- `anomalyco/opencode@v1.15.1` plugin loader source: `PluginInput.client` is built once at `packages/opencode/src/plugin/index.ts:128-150` and shared across all plugin factories. (Single sentence: this is the empirical anchor for keying the cache on `client` identity; full inspection notes are in the origin brainstorm.)
68+
69+
## Key Technical Decisions
70+
71+
- **WeakMap<OpencodeClientLike, ModelAvailability> at module scope**: The shared `client` reference is the only stable cross-source identity (module-scope state is per-source, not shared).
72+
- **Cache only successful discovery (`'api'` and `'cache'`); skip caching `'unknown'`**: A transient failure on the first call (network blip, 1500ms timeout exceeded) should not pin every subsequent call into `'unknown'` for the rest of the process lifetime. The cost of one extra HTTP call on retry is acceptable; sticky-failure UX is not.
73+
- **No TTL**: `WeakMap` auto-invalidates on `client` collection (OpenCode restart). TTL adds UX contract surface without providing better invalidation than the natural process boundary.
74+
- **Cached envelope returned by reference, not clone**: Cloning every cache hit defeats the win. The existing `ReadonlySet` type contract (memory `#2963`) is a trust-level guarantee; this plan extends the same contract to cached returns. **This is type-level, not runtime: callers that cast and mutate would corrupt the cache.** No such code exists today and the contract is documented.
75+
- **Single canonical statement of reference-identity contract**: The cached envelope is the same `ModelAvailability` reference as the originally-computed result. All test scenarios and acceptance assertions trace to this one contract.
76+
- **No promise-dedupe**: The empirical FIFO finding means concurrent calls do not happen in today's topology. The multi-source test scenario is **sequential** — Source B's call happens after Source A's completes.
77+
78+
## Open Questions
79+
80+
### Resolved During Planning
81+
82+
- Cache shape: `WeakMap<OpencodeClientLike, ModelAvailability>`
83+
- TTL: none
84+
- Cached value type: `ModelAvailability` envelope by reference
85+
- What gets cached: `'api'` and `'cache'` results only; `'unknown'` is skipped (refined by document review)
86+
- Whether to expose a public reset/invalidation API: NO — process restart is the contract
87+
88+
### Deferred to Implementation
89+
90+
- (None — test isolation strategy is now explicit in Unit 1.)
91+
92+
## Implementation Units
93+
94+
- [ ] **Unit 1: Add WeakMap memoization to getAvailableModels (successful results only)**
95+
96+
**Goal:** Memoize successful `getAvailableModels` results (`'api'` and `'cache'` status) at module scope using `WeakMap<OpencodeClientLike, ModelAvailability>`. Sequential calls with the same `client` reuse the cached envelope. `'unknown'` results are not cached so transient failures can retry on the next call.
97+
98+
**Requirements:** R1
99+
100+
**Dependencies:** None
101+
102+
**Files:**
103+
- Modify: `src/lib/model-availability.ts`
104+
- Test: `tests/unit/model-availability.test.ts`
105+
106+
**Approach:**
107+
- Declare a module-scope `WeakMap<OpencodeClientLike, ModelAvailability>` (suggested name: `availabilityCache`)
108+
- At the top of `getAvailableModels`, check the WeakMap for a cached envelope keyed on `client`. On hit, return the cached envelope by reference.
109+
- The current function has **6 distinct return paths**. Cache-population happens at the two successful paths only:
110+
1. Defensive guard (no `client.config.providers`) → returns `readFallbackCache()` → cache if status is `'cache'`, skip if `'unknown'`
111+
2. Timeout branch → returns `readFallbackCache()` → cache if `'cache'`, skip if `'unknown'`
112+
3. Thrown error branch → returns `readFallbackCache()` → cache if `'cache'`, skip if `'unknown'`
113+
4. Error-envelope / undefined-data branch → returns `readFallbackCache()` → cache if `'cache'`, skip if `'unknown'`
114+
5. Empty-discovery branch (`models.size === 0`) → returns `emptyAvailability()` (`'unknown'`) → **skip cache**
115+
6. Successful API return → returns `{ status: 'api', models }`**cache**
116+
- Implementation pattern: wrap the existing function body in a cache-check prelude + a single `cacheAndReturn(envelope)` helper that only populates the WeakMap when `envelope.status !== 'unknown'`. This makes the cache rule visible at every return path without scattering the conditional throughout.
117+
- All cached values are full `ModelAvailability` envelopes, not promises. The FIFO loader topology means we never have concurrent in-flight calls on the same client; promise-dedupe is not needed.
118+
119+
**Execution note:** Write failing tests for the memoization behavior first (RED), then add the cache structure (GREEN). Memory `#2767` discipline.
120+
121+
**Patterns to follow:**
122+
- The existing factory-based `emptyAvailability()` pattern — `'unknown'` results still construct via the factory; they just don't enter the cache
123+
- The single call site at `src/lib/config-handler.ts:557` should continue to work with zero changes (verified)
124+
125+
**Test scenarios:**
126+
127+
The scope-guardian noted that 7 scenarios was padded for a 5-15 line change. Trimmed to 4 high-signal scenarios that cover the contract:
128+
129+
- **Happy path — same-client cache hit (the core invariant):** Two consecutive `getAvailableModels(client)` calls with the same client where the first returns `status: 'api'` produce exactly one provider-API HTTP call. The second call returns the cached envelope by reference (`===`-equal). Spy on `client.config.providers` to count invocations.
130+
- **Happy path — different-client miss:** `getAvailableModels(clientA)` followed by `getAvailableModels(clientB)` (distinct `OpencodeClientLike` instances) each produce their own provider-API call. No false sharing — the WeakMap correctly distinguishes by reference identity.
131+
- **Edge case — `'unknown'` does not cache (the retry contract):** A first call that returns `status: 'unknown'` (either via empty-discovery collapse or via failed-cache fallthrough) is followed by a second call with the same client. The second call MUST re-invoke the underlying provider-API path (or cache-fallback path) — proving the failure is not sticky. This is the refinement that emerged from document review.
132+
- **Edge case — `'cache'` status caches too (sequential multi-source integration scenario):** Simulate the multi-source contributor scenario sequentially: configure `client.config.providers` to throw, but pre-write a valid `models.json` to a temp `XDG_CACHE_HOME`. The first call returns `status: 'cache'`. The second call (still sequential, same shared `client`) hits the WeakMap, skipping both the failing API call AND the `fs.readFileSync` of `models.json`. Spy on `fs.readFileSync` to assert it fires exactly once.
133+
134+
**Test isolation strategy:**
135+
- Tests MUST construct a fresh `OpencodeClientLike` per test case. Module-scope `WeakMap` state cannot leak between tests because each fresh client is its own key, and the test-scoped client goes out of scope when the test ends (no shared module state visible across tests).
136+
- Tests that assert on cache behavior across multiple calls within ONE test use the same `client` reference deliberately and document the intent. No test should share a `client` across `it()` blocks.
137+
- This is **not** relying on garbage-collection timing — the WeakMap auto-invalidation is a production property; in tests, the isolation comes from never sharing the key (client) across test cases.
138+
139+
**Verification:**
140+
- All new tests pass; existing 857 lines of `tests/unit/model-availability.test.ts` continue to pass unchanged
141+
- The provider-availability timeout warning at `src/lib/model-availability.ts:299-301` fires at most once per `(client, process)` pair on successful paths, and continues to fire on retries when discovery returns `'unknown'`
142+
- `bun typecheck` passes; the `ReadonlySet` contract holds at the type level
143+
- `bun run lint` passes; no new Biome warnings
144+
- `bun run build` produces a clean dist; no dead code
145+
146+
- [ ] **Unit 2: Document memoization semantics**
147+
148+
**Goal:** Update the user-facing configuration docs to describe (a) process-scoped memoization for successful results, (b) the restart contract, and (c) the no-mutation contract for callers.
149+
150+
**Requirements:** R2
151+
152+
**Dependencies:** Unit 1 (the behavior must exist before the doc describes it)
153+
154+
**Files:**
155+
- Modify: `docs/src/content/docs/getting-started/configuration.mdx`
156+
157+
**Approach:**
158+
- In the existing "Availability-Aware Resolution" subsection, add a sub-paragraph describing the WeakMap memoization cache (per-process, no TTL, auto-invalidates on restart)
159+
- Make the cache scope explicit: successful results (`'api'` and `'cache'`) are cached; `'unknown'` results are not cached (next call retries)
160+
- Add explicit "Restart Contract" framing: provider-state changes (auth login/logout, env var changes) require OpenCode restart to be reflected in cached availability
161+
- Add a brief note that callers receive the same envelope reference and MUST NOT mutate the returned `models` Set — referencing the existing `ReadonlySet` type contract
162+
- Keep prose tight; this is a docs delta, not a redesign
163+
164+
**Patterns to follow:**
165+
- The existing "Availability-Aware Resolution" subsection style — tight, plain prose, no embedded code examples
166+
- Other docs sections that describe runtime contracts (search for "restart" or "process lifetime" mentions in the same MDX file)
167+
168+
**Test scenarios:**
169+
- Test expectation: none — pure documentation, no behavioral change. Verified via `bun run docs:build` per memory `#2685`.
170+
171+
**Verification:**
172+
- `bun run docs:build` produces 110+ pages without MDX errors
173+
- The rendered page contains paragraphs covering ALL THREE R2 clauses: (a) memoization scope (successful results only), (b) restart contract, (c) no-mutation contract on cached envelope
174+
- The doc reads cleanly to a senior engineer who hasn't followed the v2.14.x arc
175+
176+
## System-Wide Impact
177+
178+
- **Interaction graph:** `getAvailableModels` is called from a single site (`src/lib/config-handler.ts:557`). The memoization is transparent to that caller — no signature change. Multi-source plugin loads now hit the WeakMap memoization cache for successful discovery; single-source loads are unchanged.
179+
- **Error propagation:** Existing "never rejects, always returns a `ModelAvailability` envelope" contract is preserved. Transient `'unknown'` outcomes are not cached, so a flaky first call doesn't poison subsequent calls within the same process.
180+
- **State lifecycle risks:** WeakMap state is per-OpenCode-process. No partial-write or cross-process concerns. Tests that construct fresh clients per case see fresh state; tests that share a client within one `it()` block see the shared cached state, which is intended for multi-source coherency assertions.
181+
- **API surface parity:** `getAvailableModels` is internal to `src/lib/`. Not exported from the package entry. No public API change.
182+
- **Integration coverage:** Existing 857-line `tests/unit/model-availability.test.ts` covers empty-discovery, cache fallback, timeout behavior, and defensive client guards. The 4 new memoization tests add coverage for the WeakMap behavior without disturbing existing scenarios.
183+
- **Unchanged invariants:** The `ReadonlySet<string>` contract for `models` (memory `#2963`); the `emptyAvailability()` factory pattern; the 1500ms `apiTimeoutMs` default; the empty-discovery collapse to `'unknown'` (v2.14.3); the cache-empty collapse to `'unknown'` (v2.14.4); the defensive guard for partial client shapes; the "never rejects" contract.
184+
185+
## Risks & Dependencies
186+
187+
Scope-guardian rightly noted the original risk table padded decision echoes. Trimmed to genuine open risks that could materially affect implementation:
188+
189+
| Risk | Mitigation |
190+
|------|------------|
191+
| If OpenCode ever parallelizes config-hook execution, concurrent calls would race on the WeakMap and could trigger duplicate HTTP work. | Explicit non-goal documented in plan + brainstorm. The FIFO empirical finding from v1.15.1 loader source is the anchor. If OpenCode ships parallelism in a future version, this plan must be revisited (promise-dedupe in the cache becomes necessary). |
192+
| If OpenCode reuses a `client` reference after a transport teardown (hypothetical future behavior), the WeakMap returns stale data. | Empirical loader source shows `client` is built once per process and not torn down mid-process. If a future OpenCode version adds live-reload, the cache contract still holds because a new `client` is a new WeakMap key. |
193+
194+
## Documentation / Operational Notes
195+
196+
- No release notes beyond the standard `fix:` commit message — semantic-release picks up `fix(overlay):` and bumps to v2.14.5 patch automatically.
197+
- The PR body should note that this is an internal optimization with no observable behavior change beyond N→1 HTTP calls on multi-source loads. Per memory `#2632`, no agent/session/memory refs in PR body.
198+
- Post-merge: smart note `#95` updates to reflect the memoization-shipped status (typed-validation work remains the next surface).
199+
200+
## Sources & References
201+
202+
- **Origin document:** [docs/brainstorms/2026-05-14-provider-availability-dx-hardening-requirements.md](../brainstorms/2026-05-14-provider-availability-dx-hardening-requirements.md)
203+
- **Companion (deferred):** docs/brainstorms/2026-05-16-typed-config-validation-requirements.md (status `draft`)
204+
- Related code: `src/lib/model-availability.ts:266-340`, `src/lib/config-handler.ts:557`
205+
- Related PRs (parent arc): #372 (v2.14.3), #378 (v2.14.4), #380 (compound docs)
206+
- External docs: `.slim/clonedeps/repos/anomalyco__opencode/packages/opencode/src/plugin/index.ts:128-150` at v1.15.1
207+
- Memories: `#2065`, `#2627`, `#2685`, `#2734`, `#2762`, `#2767`, `#2963`, `#3005`, `#3043`, `#3061`
208+
- Smart notes: `#95` (active, updates post-merge)

0 commit comments

Comments
 (0)