fix(runtime-host): reconcile retired fallback models

[mikemikimike]

Summary

Fixes #3354.

Reconcile explicitly retired OpenCode Free model ids during Runtime Host bootstrap. The migration updates enabledModelIds, models, and a matching defaultTarget in one catalog write, while preserving user-selected models that are not in the retired-id list. The migration is idempotent and does not alter protocol-discovery providers.

Verification

• git diff --check — passed.
• Added regression coverage for retired inventory entries, default-target repair, and preservation of a user model.
• npm --workspace @maka/runtime-host test -- --test-name-pattern="reconciles retired OpenCode Free models" — blocked by the repository's TypeScript 7 Windows native compiler failing to start with EBUSY before compilation.
• Full lint, format, typecheck, and build remain unrun for the same environment blocker.

AI use

• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex selected the issue, implemented the migration and regression test, and prepared this pull request.

Generated-by: OpenAI Codex

[Astro-Han]

Thanks — the diagnosis is right, and computing the retired set as a diff against the current defaults is the correct shape, so it stays correct as the default list changes.

Reviewed at exact head 94a189691da236de0a188517d759d4686834288d. test is red on this head, and it is your own two new tests that fail.

[P1] Retired models are not all removed.

reconciles retired OpenCode Free models without removing user models fails:

+ actual - expected
  [ 'nemotron-3-ultra-free',
+   'big-pickle',
    'user-model' ]

big-pickle survives the migration. It is in LEGACY_OPENCODE_FREE_SEEDS (bootstrap-runtime-policy.ts:195) and not in the current defaults, so retiredOpencodeFreeModelIds() should be returning it and the filter should be dropping it. a user-modified opencode-free inventory is never migrated fails too.

I have not worked out which side is wrong — whether the retired set is short one entry, or the filter isn't reached in this path. Your tests encode the intent clearly, so whichever it is, they're pointing at it.

Everything else looks sound to me: the retired set is derived in one place and consumed in one place, the legacy-seed branch and the incremental-filter branch are cleanly separated, and defaultTarget is repaired alongside the inventory.

I'll re-review once CI is green. Nothing else is blocking from my side.

---

This review was AI-assisted. Findings were verified against the exact head listed above; any mistakes are mine to correct — please push back where I got it wrong.

[Astro-Han]

Correcting my previous comment — I had the direction backwards, and the code is right where I implied it was wrong.

I said big-pickle "survives the migration" and that the filter should be dropping it. Reading the failure more carefully, + is actual and - is expected: the code keeps big-pickle and the test expects it removed. So the runtime behaviour is correct and the assertion is what's wrong.

And keeping it is right on the merits. In model-metadata.generated.ts, big-pickle is lifecycle: 'active', isFree: true, functionCalling: true, and it isn't in OPENCODE_FREE_BROKEN_MODEL_IDS. It therefore passes every filter in opencodeFreeModelIds, lands in OPENCODE_FREE_DEFAULT_ENABLED_MODELS, and retiredOpencodeFreeModelIds() correctly excludes it. It's a live free tool-calling model — removing it from a user's enabled set would take away something that works.

So the fix is in the test, not the migration. Same for a user-modified opencode-free inventory is never migrated, which I'd guess shares the fixture assumption.

Sorry for pointing you at the wrong half. My earlier praise of the design stands, and I think it's right for the reason I'd underweighted: retiredOpencodeFreeModelIds() computing a diff against the live default set is exactly what keeps a still-active model from being swept up as "retired". That property is worth a test of its own.

---

This review was AI-assisted. Findings were verified against the exact head listed above; any mistakes are mine to correct — please push back where I got it wrong.

[Astro-Han]

For the record, since my earlier comment cast doubt on this function: it behaves correctly.

Computing retired ids as (union of legacy seeds) − (current defaults) means a model that is still active, free and tool-capable can never be classified as retired, however many past seeds mentioned it. big-pickle is exactly that case, which is why it survives — and why it should.

The failing assertion expects it removed, so that's where the fix belongs.

[mikemikimike]

Verified the review correction against model-metadata.generated.ts: big-pickle is active, while deepseek-v4-flash-free is deprecated. Updated the regression to use the deprecated model and retained big-pickle in the user-customization coverage. Pushed c653679; waiting for CI.

[Astro-Han]

[P1] The non-legacy branch restores modelSource without modelsFetchedAt, writing a document that fails its own decoder on the next read.

const { lastTest: _lastTest, modelSource: _modelSource, modelsFetchedAt: _modelsFetchedAt, ...retained } = previous;
...
  ...(isLegacySeed && fallbackModels.length > 0
    ? { modelSource: 'fallback' as const, modelsFetchedAt: 0 }
    : previous.modelSource === undefined
      ? {}
      : { modelSource: previous.modelSource }),

Both fields are destructured out of retained, but the non-legacy path puts only modelSource back. The canonical decoder requires them to travel together:

if ((item.modelSource === undefined) !== (item.modelsFetchedAt === undefined)) {
  throw domainError('connection model source and fetched time must occur together');
}

— packages/core/src/runtime-policy/connection-catalog-codec.ts:418-420

An ordinary OpenCode Free catalog row carries modelSource: 'fallback', modelsFetchedAt: 0, so as soon as this migration touches such a row it persists an invalid document. The next getSnapshot() throws invalid_document, and Host/catalog operations from that point on are unavailable. The user does not hit a degraded state; they hit a hard read failure on a document the upgrade itself produced.

This is not hypothetical — this PR's own new test already fails on it. Building the exact head and running:

node --test packages/runtime-host/dist/__tests__/bootstrap-runtime-policy.test.js

gives 8 pass / 1 fail, and the failure is reconciles retired OpenCode Free models without removing user models, throwing at the first getSnapshot() after bootstrap.

Why the existing guards did not stop this: migrateSystemSeed() assembles and writes the entry directly, bypassing the canonical decoder on the write side. The invariant is only enforced on read — which is exactly why the failure lands on the next startup rather than on the one that caused it.

Minimal fix: restore or reconstruct modelsFetchedAt alongside modelSource on this branch, the way the legacy path already does with its { modelSource: 'fallback', modelsFetchedAt: 0 } pair.

Independent blind line, provisional judgment sealed before any existing review was read. Verified at the gate 2026-08-23 12:58 UTC.

[mikemikimike]

Fixed in c5ba13e. The non-legacy migration now restores modelSource and modelsFetchedAt together, preserving the decoder invariant. The focused bootstrap regression passes 9/9.

[Astro-Han]

[P2] The same branch filters enabledModelIds but carries relayModelProfiles through untouched, breaking the second catalog invariant.

relayModelProfiles arrives via ...retained exactly as it was, while enabledModelIds is filtered down by !retired.has(id) on the line below. If the user had declared a relay profile for a model that is now retired, the persisted profile key is no longer in the enabled set — and the decoder requires containment:

if (!enabledModelIds.includes(modelId)) {
  throw domainError(`relay model profile for ${modelId} is not an enabled model`);
}

— packages/core/src/runtime-policy/connection-catalog-codec.ts:418 region, profile decode at :245-250

This is independent of the P1 above: fixing modelsFetchedAt still leaves this one, and it fails the next read the same way.

What makes it clearly an omission rather than a design choice: every other selection-changing path in this same file already prunes — :283, :472, :566 all call pruneRelayModelProfiles(...) when the enabled set moves. The new migration path is the only place that narrows enabledModelIds without it.

Fix: run pruneRelayModelProfiles(previous.relayModelProfiles, migratedEnabledModelIds) on this branch, matching what the neighbouring paths do.

---

Direction and scope, since they are worth saying separately from the findings. Subtracting current defaults from historical seeds to derive the retired set, and passing it in at bootstrap, is a sound way to bound historical fallback drift — and big-pickle correctly stays active because it is still a current default. Widening from "replace the exact legacy seed wholesale" to "incrementally filter user-modified rows against a tombstone set" is the right generalization.

But both findings live in that new hand-assembled row, and both come from the same cause: it rebuilds the document by hand and bypasses two canonicalization invariants that every neighbouring path respects. On net, until they are addressed, this adds entropy to the persistence path rather than removing it — which is the opposite of the change's own goal.

Two smaller notes, neither a blocker: the legacy exact-seed atomic upgrade and the null-default handling are both still correct, and a user-modified row whose retired default is not in the retained selection fails closed to null as it should. The comment claiming "user selection is never touched" is now stale — documentation debt, not a defect.

Independent blind line, same seal. Verified at the gate 2026-08-23 12:58 UTC. Exact head had no check runs at all when reviewed; the stalled workflow has now been approved so CI can actually report.

[mikemikimike]

Fixed in c5ba13e. The non-legacy migration now prunes relayModelProfiles against the migrated enabledModelIds using the existing canonical helper. The focused bootstrap regression passes 9/9.

[Astro-Han]

Independent review of c6536797668c07ce906291da293059341f9d9924. One [P1] and one [P2], both inline, both in the new non-legacy migration branch.

• fix(runtime-host): reconcile retired fallback models #3517 (comment) — modelSource is restored without modelsFetchedAt, producing a document that fails getSnapshot() on the next read. This PR's own new test already fails on it (8 pass / 1 fail at exact head).
• fix(runtime-host): reconcile retired fallback models #3517 (comment) — relayModelProfiles is carried through unpruned while enabledModelIds is filtered, breaking the containment invariant independently of the first.

On CI: this head had zero check runs — one workflow sitting at action_required, which renders like "pending" but means it never started. Approved as a maintainer so it can actually report. The absence of a red mark here was not evidence of green.

The direction is right and worth saying so. Deriving the retired set by subtracting current defaults from historical seeds is a principled way to bound fallback drift, and generalizing from "replace the exact legacy seed wholesale" to "incrementally filter user-modified rows against a tombstone set" is the correct move. big-pickle correctly stays active because it is still a current default, and the revised historical test using it instead is a genuine improvement over the previous fixture.

Both findings share one cause, which is why they are worth fixing together rather than patching individually: the new branch hand-assembles the persisted row and, in doing so, steps around two canonicalization invariants that every neighbouring path in the same file honours. The write side never sees the decoder, so both violations surface on the next startup instead of the one that created them — which is the worst place for a migration bug to land.

Credit where it is due: the new regression test is what exposed the P1, reproducibly and without extra setup. A test that catches the change's own defect before review does is doing its job.

Reviewed at 2026-08-23 12:58 UTC. Blind line — the provisional judgment was sealed before any existing review on this PR was read.

[Astro-Han]

CI has now run on this head and confirms the [P1] independently. Run 32612843956, Run Runtime Host tests:

✖ reconciles retired OpenCode Free models without removing user models (24.842023ms)
✖ failing tests:
    code: 'invalid_document',

That is the same test, the same failure, and the same invalid_document error the review reproduced locally before CI had ever executed.

Worth separating two facts that looked identical from the outside: this head previously showed no red mark, because the only workflow was parked at action_required with zero jobs — the tests had never started. After approving the run, the red appeared immediately. The absence of a failure signal was an absence of any signal.

No further action needed from the review side; the fix is described in the inline comments.

[Astro-Han]

Approving c5ba13eea1653354f7b643df089a103786054c73. Required test is completed / success bound to that exact SHA. No P0–P3.

Re-review at the current head. Both earlier findings were re-derived rather than accepted as fixed — the [P1] in particular, since CI had independently reproduced it on the previous head.

The [P1] is closed. In connection-catalog-document.ts the non-legacy migration branch now carries modelSource and modelsFetchedAt as a pair, in a single spread, so it can no longer write a document that fails its own decoder on the next getSnapshot(). The legacy-seed branch sets both together too. Restoring one half of a two-field invariant was the whole defect, and the fix is shaped so the two cannot be separated again by accident.

The [P2] is closed at the same place. relayModelProfiles is no longer carried through untouched: it goes through the existing pruneRelayModelProfiles against migratedEnabledModelIds, which restores the containment invariant between the two lists. The default target is retargeted in the same branch when the migration drops the model it pointed at, so the document stays internally consistent rather than merely decodable.

Checked rather than assumed: retiredModelIds is still derived as a difference between the bootstrap seed and the current default set, so an active model such as big-pickle survives and user-customised enabled ids are preserved — the migration removes retired models, not unfamiliar ones. Bootstrap regression 9/9, storage runtime-policy 52/52.

Disclosure, because it changes what this approval is worth: this is an AI review. Under CONTRIBUTING.md §Review it does not count as the required independent human review. It means the code has been checked, not that the gate is open — merge still needs a committer other than the author to give LGTM and to decide.