fix(core): preserve fallback models after failed discovery

[mikemikimike]

Summary

Fixes the independent fallback-model half of #3320.

When model discovery has failed or has not run yet, the catalog can contain an empty models: [] array without a successful modelSource. The picker treated that empty array as authoritative and hid the static fallback catalog, even though the connection could still use the configured fallback model.

This change:

• infers fallback, rather than fetched, for an empty model array when no source is persisted;
• keeps fallback models visible for failed/pending discovery;
• preserves an explicitly fetched empty inventory as authoritative;
• adds regression coverage for both states.

The URL-normalization question in #3320 is intentionally out of scope.

Validation

• npm --workspace @maka/core run typecheck
• npm --workspace @maka/core run build
• npm --workspace @maka/core run test:dist — 638 passed
• npx biome check packages/core/src/model-catalog.ts packages/core/src/__tests__/model-catalog.test.ts
• npx biome lint packages/core/src/model-catalog.ts packages/core/src/__tests__/model-catalog.test.ts
• git diff --check

[Astro-Han]

Independent review at exact head 073a9cba387d22314b3446ca13bd218ff363672c.

APPROVE. The distinction this draws is the right one, and it is drawn in the only place that can carry it.

The bug was that an empty array is truthy, so input.modelSource ?? (liveModels ? 'fetched' : 'fallback') turned "discovery has not succeeded" into "the provider enumerated nothing", and the entire static catalog vanished from the picker while authorizeConnectionModel was still happily admitting those same models. The old comment on that line already claimed to use the raw modelSource — the code did not. This PR makes the code match the comment.

I walked the four reachable input shapes at this head rather than taking the tests' word for it:

models	modelSource	inventory	source	entries
undefined	undefined	absent	static_catalog	fallback catalog
[]	undefined (failed/pending discovery)	absent	static_catalog	fallback catalog — the fix
[]	'fetched' (authoritative empty)	live	provider_api	empty; saved default annotated not_in_live_list
non-empty	either	per source	per source	live list

The pairing holds in every row: classifyConnectionModelInventory gates on models === undefined || modelSource === undefined → 'absent', so the fallback catalog can never be labelled provider_api. I specifically probed the shape that would break that — models: undefined with an explicit modelSource: 'fetched' — and it resolves to absent/static_catalog, so provenance stays honest there too.

Worth noting for the record: the defaulted local modelSource also feeds the staleness/annotation paths at model-catalog.ts:207,219,236, so models: [] with no source now reads fallback there where it previously read fetched. That is the same correction, applied consistently, not a side effect.

The two new tests pin behaviour rather than call shape, and the second one is the load-bearing half — without it, "always fall back on empty" would pass just as well and would silently resurrect models a provider has genuinely retired.

Verification: exact-head test is completed/success. Dependency audit is absent on this head by design — it is path-filtered to package.json / lockfile / audit scripts, neither of which this PR touches.

[Astro-Han]

Reviewed on exact head cb3770233099d5ca454effed4ceb3b930964b895. No P0–P2 in the change itself. One pre-existing [P3] inline. Not approving yet, for a gating reason rather than a code reason — details below.

The fix is correct, and for the right reason. Both original sites treated an empty array as "the provider authoritatively answered: none":

• the modelSource inference liveModels ? 'fetched' : 'fallback' — [] is truthy, so "never looked" was scored as "looked and found nothing";
• rawModels = liveModels ?? fallback — ?? only guards undefined, so [] displaced the entire static fallback catalog.

Introducing the "is there an authoritative source" dimension separates the three states cleanly: undefined (never queried) → fallback; [] without an explicit modelSource: 'fetched' (failed or pending discovery) → fallback, which is the bug being fixed; [] with explicit 'fetched' (provider genuinely has none) → stays empty.

I also confirmed classifyConnectionModelInventory still receives the raw input.modelSource rather than the inferred value — that is what the original comment there was protecting, and the change does not lose it.

The three new tests pin both branches of the new condition plus one public-path case, rather than only asserting shape.

On the red check — it is not this PR's fault, but it still blocks. The failing test comes from the branch being cut from an older main that contained a broken test: at the merge base, goal-services-adapter.test.ts:64/70/76 pass a type: 'sessions_changed' field that SessionChangedEvent does not have (its fields are reason/sessionId/modelId/turnId/ts), so that base does not compile. Current main has already dropped those type fields. This PR touches only model-catalog.ts and its test — verified via git diff --name-only merge-base..head.

So: no finding recorded for it, but an unrelated red is still not green, and the approval bar is terminal-green expected checks on the exact head. Merging current main into the branch should clear it, and I will re-check the terminal state after that.

[Astro-Han]

[P3] Pre-existing, not introduced here (anchored to this hunk because the affected line at :270 is unchanged and cannot take an inline comment) — but it sits directly adjacent to what this PR fixes, so it is worth naming while the area is being touched.

A connection that succeeded at discovery (modelSource === 'fetched') but whose models were all later quarantined ends up here with [] after the filter, still carrying 'fetched'. That takes the "authoritatively empty" branch, so the picker renders empty and no fallback catalog is offered — even though the static fallback may contain usable models.

Trigger → path → outcome: a connection works normally, every one of its model ids later lands in brokenModelIds, the user opens the model picker and sees nothing.

From the user's side this is indistinguishable from the failed-discovery case this PR is fixing, but only the latter is covered. Behavior is unchanged from before this PR (the old code also reduced to rawModels = liveModels = [] here), so this is not a regression — hence P3 rather than a blocker.

Smallest fix: check for emptiness after filtering — if the pre-filter list was non-empty and the post-filter list is empty, treat it as having no usable discovery result and fall back. A production-seam test would set models such that every id hits brokenModelIds with modelSource: 'fetched', and assert fallback catalog entries are still returned.

[Astro-Han]

Incremental re-review at exact head d117414f138188886bb78394038aea91e9a59d5b. APPROVE — no P0–P3 introduced.

This confirms the earlier approval, which was bound to 073a9cba. I did not re-review the whole change; I verified what changed since that approval and that the added test asserts real production behaviour.

What changed since 073a9cba

git range-diff against each side's own merge-base reports the fix commit as equivalent (=), not merely similar: d7698bbf is the same patch that was approved. The branch then adds one commit, cb377023, which is 32 added lines in packages/core/src/__tests__/model-catalog.test.ts and nothing else — no production file moved.

The new test asserts the behaviour the fix actually implements

a persisted empty discovery result preserves the connection fallback through the public catalog path exercises buildConnectionModelCatalogEntries — the connection-level entry point — where the existing coverage only went through buildModelCatalogEntries. It passes models: [] with no modelSource, and expects the two fallback ids back with modelSource: 'fallback' and unavailableReason: 'none'.

That is not in tension with an explicitly fetched empty inventory remains authoritative two tests above it. The discriminator is the raw modelSource, and I checked it holds at the persistence boundary, not just in the unit under test:

• runtime-policy/connection-catalog-codec.ts:368 rejects any record where modelSource and modelsFetchedAt disagree about being present, and :371 rejects modelSource === undefined together with a non-empty models.
• So a genuinely empty successful fetch persists modelSource: 'fetched' alongside modelsFetchedAt and stays authoritative, while "discovery never ran or failed" persists neither. The new test is the second case.

Still open, not introduced here

The [P3] thread at model-catalog.ts:191 remains open. It was filed as pre-existing and is not a blocker.

Gates at this exact head

• test — terminal success
• mergeable — MERGEABLE / CLEAN
• the check set is the correct triggered set for these paths

中文

在 exact head d117414f138188886bb78394038aea91e9a59d5b 上的增量复审，APPROVE，未引入 P0–P3。原批准绑在 073a9cba。

自那次批准以来：range-diff 判定修复提交与已批准的那个等价（=），另多一个提交 cb377023，只有 32 行新增测试，没有动任何生产文件。

新测试走的是 buildConnectionModelCatalogEntries（连接层入口），原覆盖只走 buildModelCatalogEntries。它与上面那条"显式取回的空清单仍然权威"不矛盾——判别依据是原始的 modelSource，而且我核到持久化边界上也成立：connection-catalog-codec.ts:368/371 保证"真的取回但为空"会同时写下 modelSource: 'fetched' 和 modelsFetchedAt，而"从没取过/取失败"两者都不写。

model-catalog.ts:191 那条 [P3] 仍开着，它是既有问题，非本 PR 引入，不阻断。

门禁：test 终态绿、CLEAN。