fix(overlay): collapse empty cache to unknown availability status#378
Merged
Conversation
When the API call fails (timeout, throw, or error envelope) and the local
`models.json` cache is readable but produces a zero-size set,
`readFallbackCache` was returning `{ status: 'cache', models: <empty Set> }`.
The downstream gate `availability.status !== 'unknown'` admitted this,
`availabilitySet` became the empty Set, and `resolveSourceModel` fell
through to the 'last-resort: first provider's first model' path —
pinning bundled agents to a provider the user cannot call.
Symmetric with the empty-API collapse from PR #372: both branches in
`readFallbackCache` now check `result.size > 0` before returning
`{ status: 'cache', ... }`, so an empty cache funnels through
`emptyAvailability()` (status: 'unknown') just like the empty-API case.
Downstream consumers stay on a single gate rule rather than
`status !== 'unknown' && models.size > 0` everywhere.
Three regression tests cover the three empty-cache shapes: schema-valid
empty object, providers-with-empty-models, and OPENCODE_MODELS_URL-derived
empty cache. The DiscoveryStatus JSDoc for 'cache' is updated to reflect
the non-empty contract.
Closes #373
fro-bot
approved these changes
May 16, 2026
Collaborator
fro-bot
left a comment
There was a problem hiding this comment.
Verdict: PASS
The fix is correct and minimal. readFallbackCache was the last place a zero-size set could escape to downstream consumers with a non-'unknown' status; the two-line guard added symmetrically to both branches closes that gap. The approach mirrors exactly what PR #372 did for the API path, which is the right pattern.
Blocking issues
None
Non-blocking concerns
readModelsFromCachealready short-circuits onstat.size === 0(returnsnull), so a literal empty file never reaches the new guards — that path is handled upstream. The new guards only fire for schema-valid but zero-model files (e.g.,{}or{anthropic:{models:{}}}). This is correct behavior and the comment in the source makes it clear.- The
OPENCODE_MODELS_URLbranch already returnedemptyAvailability()at its early-exit if the URL-derived file was missing; adding thesize > 0guard now also catches the URL-derived file being present-but-empty without falling through to defaultmodels.json. The trust-domain comment correctly explains why fall-through is intentional not to happen.
Missing tests
None — the three new regression tests cover all three shapes documented in the PR description ({}, providers-with-empty-models, URL-derived empty cache). The sha1Hex helper in the test file matches fastHash in source, so the URL-derived filename computation is exercised correctly.
Risk assessment: LOW
- Blast radius is confined to
readFallbackCache. No API path, no schema, no plugin hook is touched. - The behavior change only affects the previously-bugged case (empty-but-parseable cache). A previously-working non-empty cache is unaffected (
size > 0still returns{ status: 'cache', ... }). - All three new tests were confirmed to fail pre-fix and pass post-fix per the PR description verification table.
- Typecheck, lint, and full test suite reported clean.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | marcusrbrown/systematic |
| Run ID | 25953009159 |
| Cache | hit |
| Session | ses_1d0e83679ffePV3Xwmndqj2y4H |
marcusrbrown
added a commit
that referenced
this pull request
May 16, 2026
Five plans were carrying status: active long after their work shipped. Flipping to status: completed and adding a shipped: field that points at the PR(s) and release(s) that delivered each plan, so future readers can trace each plan to its merge artifact without searching the PR history. - 2026-05-10-001 zod-config-schema → PR #351 (v2.12.0) + #354 + #357 + #363 - 2026-05-12-001 client-api-source-model → PR #358 (v2.13.0) - 2026-05-13-001 bootstrap-message0-skill-catalog → PR #365 - 2026-05-14-001 isolated-opencode-integration → PR #369 - 2026-05-14-002 provider-availability-hardening → PR #372 (v2.14.3) + cache-empty follow-up PR #378 (v2.14.4) closes #373 The provider-availability-hardening plan was previously untracked locally; it now joins the committed history alongside the other four. Status distribution: 19 completed, 5 superseded, 0 active.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
readFallbackCachereturned{ status: 'cache', models: <empty Set> }when the API call failed AND the localmodels.jsoncache was readable but produced a zero-size set. The downstream gateavailability.status !== 'unknown'admitted this case,availabilitySetbecame the empty Set, andresolveSourceModelfell through to the "last-resort: first provider's first model" path — pinning bundled agents to a model the user cannot call. Operationally identical to the empty-API case PR #372 fixed.Three concrete shapes that trigger the bug:
models.jsonis{}(schema-valid empty object)models.jsonhas providers but theirmodelsrecords are{}OPENCODE_MODELS_URLis set and the URL-derivedmodels-<sha1>.jsonis emptyIn all three,
readModelsFromCachereturns a non-null but zero-size Set, theresult !== nullcheck passes, and the bug fires.Fix
Mirror the empty-API collapse from PR #372 in
readFallbackCache. Both branches (URL-derived and defaultmodels.json) now checkresult.size > 0before returning{ status: 'cache', ... }. An empty cache funnels throughemptyAvailability()instead, so the downstream gate stays a single rule (status !== 'unknown') for every consumer.This is the canonical "watch for parallel bugs on parallel paths" pattern documented in
docs/solutions/best-practices/discovery-before-validation-lifecycle-2026-05-15.md(Pattern 3). The API path and the cache path are parallel sources of the same envelope; the collapse rule lives at envelope construction, not at every downstream consumer.Tests
tests/unit/model-availability.test.tsgains a new describe block (edge case: empty cache collapses to unknown) parallel to the existingedge case: empty discovery collapses to unknown. Three regression tests cover:'{}')OPENCODE_MODELS_URL-derived cache that is emptyAll three asserted
status: 'unknown'andmodels.size: 0. The pre-fix run failed all three withReceived: 'cache'; the post-fix run passes.The
DiscoveryStatusJSDoc for'cache'is updated to reflect the non-empty contract.Verification
index.js + chunk + cli.jsexports: ['default'](default-only contract upheld)Out of scope
Larger DX hardening for provider availability (warn-mode validators, process-scoped memoization, 1500ms timeout tuning) remains tracked separately. Smart note will continue to track that work.
Closes #373