fix: give "what can this connection run" one authority - #3330
Open
Astro-Han wants to merge 4 commits into
Open
Conversation
Astro-Han
force-pushed
the
fix/1584-model-selection-authority
branch
from
August 20, 2026 14:21
770e89f to
7f41aaf
Compare
Astro-Han
changed the title
Astro-Han
force-pushed
the
fix/1584-model-selection-authority
branch
from
August 20, 2026 16:07
f795551 to
ea45a70
Compare
Astro-Han
changed the title
…n run" one authority Nine places answered that question with their own criteria and no shared source, so a Volcengine Ark Agent Plan key could not select the models its own plan serves (#1584). Every one of them read `modelSource === 'fetched'` as "a provider enumerated this account". It does not: `modelSource` records write provenance. A provider whose `modelDiscovery.kind` is `'fallback'` still runs discovery — it just replays the array this build shipped — and the row honestly says `'fetched'`. Ark has no model-list endpoint at all, so every gate treated a release snapshot as an allowlist and refused the models the plan demonstrably serves. `classifyConnectionModelInventory` is now the one place that conjunction lives, and it returns what the catalog IS rather than where it came from: - `live` — a provider with a model-list endpoint enumerated this account. Evidence, and an allowlist. An empty one is still evidence. - `snapshot` — the array this build shipped. It describes the provider at release, not the account, so it cannot rule on what a plan serves. - `absent` — no catalog, and none asked for. `modelSource` is what tells this apart from an empty live list; the codec keeps it present exactly when a run has written the row. `authorizeConnectionModel` builds the verdict on top: the user's `enabledModelIds` is the authorization, and only a `live` catalog overrules it. It returns the inventory alongside, because callers legitimately differ on `absent` — sending a message needs something to send against, while resolving an id for execution does not (#2896). Each states that policy for itself; none restate the rest. Converted, and their old criteria deleted rather than left beside it: readiness, Runtime Host execution and Session admission, the hosted execution target, catalog projection, the connection test probe, and the storage test-model gate. `connection-model-admission.ts` was a thin shell over the same question and is gone; its two callers ask the authority. Two onboarding guards required `source === 'fetched'`. The only writer of that field always writes `'fetched'`, so the clause never fired — it was an assertion that provenance is a quality bar, next to the check that actually matters. Removed; the non-empty requirement stands. Catalog projection also stopped hiding a snapshot provider's `models` from itself, which had been discarding the capabilities in those rows: an image-only model read as selectable. Capabilities are facts wherever they came from; only inventory claims lose authority when the list is a snapshot. Staleness now follows the same rule — a snapshot is as current as the build, so it cannot expire. Generated-by: Claude Code
… keys `relayModelProfiles` is scoped to `enabledModelIds` — the canonical decoder rejects a table keyed by a model the selection dropped. `prepareOnboardingUpsert` replaced `enabledModelIds` while spreading the previous entry, so a re-onboard that narrowed the selection carried the old table across intact. The write succeeded; the next read of the catalog failed, because this path bypasses the decoder that would have caught it. The refresh path a few lines above prunes for exactly this reason and says so. Generated-by: Claude Code
…not learn The authority now lets an enabled model through on a provider whose catalog is a shipped snapshot, but nothing could put one there. A provider with no model-list endpoint never grows its list, so a model the user's Ark plan already serves had no way into the connection at all (#1584). An add-model dialog writes the id into `enabledModelIds` — the same user authority a catalogued model uses, so nothing pretends the provider advertised it — and a context window into `relayModelProfiles`, in one write, because the store requires every declaration to key an enabled model. The window is required rather than defaulted: an unknown one silently falls back to a flat 32k history budget, three percent of a 1M-token window, and a silent downgrade is worse than a request the provider rejects outright. Whoever types an exact model id is reading the provider's own model page, where the number is stated. `relayModelProfiles` was gated whole, by provider, which is why only relays could carry any declaration. The gate is now per field, because the fields differ in kind: `contextWindow` and `vision` state facts about a model and are legal anywhere, while `thinkingLevels` and `serviceTier` shape an OpenAI-compatible request body and stay relay-only. The codec enforces that split, so a declaration that cannot be honoured is rejected at the boundary rather than silently ignored downstream. The capability section shows a row for exactly the models Maka cannot describe — every model added here, the moment it is added — and hides the thinking-level control where it cannot apply. Two details the section got wrong on the way: adding a model now merges the declaration into the open editor draft, which does not reseed from props while dirty and would otherwise have dropped the window on the next save; and the dialog's submit is disabled while another write is in flight, where the store would have discarded the submission and the typed id with it. Generated-by: Claude Code
Generated-by: Claude Code
Astro-Han
force-pushed
the
fix/1584-model-selection-authority
branch
from
August 20, 2026 17:26
ea45a70 to
a9eb6c8
Compare
Astro-Han
changed the title
Astro-Han
marked this pull request as ready for review
August 20, 2026 19:41
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
1 participant
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.
Summary
A Volcengine Ark Agent Plan key cannot select the models its own plan serves (#1584). The cause is not in that provider: nine places across core, storage, runtime and Runtime Host each decided "what models can this connection run" on their own, and every one read
modelSource === 'fetched'as "a provider enumerated this account".It does not mean that.
modelSourcerecords write provenance. A provider whosemodelDiscovery.kindis'fallback'still runs discovery — it just replays the array this build shipped — and the row honestly says'fetched'. Ark has no model-list endpoint at all, so every gate read a release snapshot as an allowlist.This PR gives the question one authority and converts all nine callers to it.
classifyConnectionModelInventoryreturns what the catalog IS:live(a provider enumerated this account — an allowlist, even when empty),snapshot(the array this build shipped — describes the provider, not the account),absent(no catalog, none asked for).authorizeConnectionModelbuilds the verdict on top:enabledModelIdsis the authorization, and onlyliveoverrules it.The other half is that nothing could put a model into such a connection. An add-model dialog writes the id into
enabledModelIdsand a context window intorelayModelProfilesin one write, which required splitting therelayModelProfilesgate from whole-table-by-provider to per-field —contextWindow/visionare facts about a model,thinkingLevels/serviceTiershape an OpenAI-compatible request body and stay relay-only.Two live defects fell out of the same audit: the connection test probed a snapshot model instead of the one the user chose, and onboarding carried a stale
relayModelProfilesacross a narrowed selection, writing a catalog the decoder rejects on next read (separate commit,e86603052).Fixes #1584
Verification
@maka/core571,@maka/storage842,@maka/runtime-host1031,@maka/desktop978,maka-agent336, Storybook smoke 144 stories,format:checkandtypecheckclean.@maka/runtime2968 pass / 1 fail —does not carry queued Unix PTY writes past native exit, which spawns an independent node-pty child, imports nothing this PR touches, fails the same way onmainhere, and is green in CI.Every new test was checked by mutation: reverting the production change makes it fail. The end-to-end one drives a real
volcengine-agent-planconnection throughtestConnectionand asserts the wire body names the user's model.Fixtures across the suites were corrected to the state the write path actually produces — a snapshot provider's row says
modelSource: 'fetched'. Several existing tests asserted'fallback'there, which is why none of them caught this.Known gap
The CLI onboarding wizard still offers only what discovery returned, so the fix is reachable from desktop connection settings only. A CLI entry point for declaring a model is not in this PR.
AI use
Tool(s) and scope: Claude Code (Opus 5) wrote the implementation, tests and this description under my direction, and ran the suites above. An earlier revision of this branch fixed the write path instead and introduced a P0 of its own; adversarial review caught it and the work was redone against the authority described here. I reviewed the final diff and commit messages and own the result.
Checklist
Does this PR entail a change in behavior?