feat(connections)!: unify custom connections with per-model protocols - #5684
Conversation
The three per-protocol custom provider types (openai-compatible, openai-responses-compatible, anthropic-compatible) become one `custom` type. A custom connection has a required default request protocol fixed at creation and a required base URL; each model may override its wire with modelOverrides[id].apiProtocol. declaredModelApiProtocol (override, then discovered, then connection default) is the single resolution that runtime, Fast mode, hosted web search, connection tests and provider-state identity read. On the Anthropic Messages wire a declared thinking level always sends native adaptive thinking plus output_config.effort; no model-name inference remains for custom connections. connection-catalog.json moves to schema version 3. v1/v2 rows, the onboarding journal and config imports naming a legacy type are upgraded on read to `custom` with the matching default protocol. Settings, CLI onboarding and Eval subjects carry the default protocol on create. Removed: isRelayProviderType, the openai-compatible adapter `name` field and runtimeProviderName, supportsRelayFastServiceTier, pruneModelOverrides, the dead open-responses serviceTier branch and the DeepSeek-V4-Flash hosted-search special case. BREAKING CHANGE: older builds cannot read a v3 connection catalog. Sessions on migrated custom connections, and discovered-wire Copilot or opencode models, start a new provider-state chain once. Custom Responses models no longer send an implicit parallel_tool_calls. Fixes apache#5673 Generated-by: Claude Code
…tion wire Generated-by: Claude Code
…nt state The renderer architecture ledger forbids growing hook debt in legacy files; the protocol and base URL describe one endpoint. Generated-by: Claude Code
…ocol A hosted execution target that names an existing slug reused that connection whenever the provider type matched. An Eval subject asking for custom + anthropic-messages would silently run on a custom connection created with another default protocol. Treat a protocol mismatch like a provider mismatch. Generated-by: Claude Code
Older builds let a relay connection's endpoint be cleared, which left a catalog row without baseUrl. The new codec check that a custom connection carries a base URL ran on the canonical read path too, so one such row made the whole catalog unreadable, and an onboarding journal replaying onto it could never converge. Drop the codec check and go back to where main enforced this: the add form requires the endpoint, onboarding rejects with base_url_not_configured, and the custom chat adapter sets requireBaseUrl again so execution fails with a clear error. Generated-by: Claude Code
The Anthropic SDK accepts only low, medium, high, xhigh and max as effort. A custom model on the Messages wire declaring minimal failed every request with a schema error. Generated-by: Claude Code
… rows main's anthropic-compatible type inferred provider-hosted web search for deepseek-v4-flash. custom honors only a declared webSearch capability and settings has no control for it, so a migrated connection silently lost native search. The upgrade now writes that declaration on catalog rows that enable the model and have not declared webSearch themselves. Generated-by: Claude Code
…f upgrading them Config import is allowed to break with this change. A bundle from an older build that names openai-compatible, openai-responses-compatible or anthropic-compatible now plans those connections as skipped, like a retired provider, and their credentials stay unwritten. This drops the upgrade from the import planner and the credential binding match; the catalog and the onboarding journal still upgrade in place. Generated-by: Claude Code
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head cb4ebf23bd1e5c77e0f62a32cf57928c839a94b1 against current main a87e4520a29bce42b09ac9d07200b5ed3ee7aaf9 (8 commits ahead, 7 behind; clean merge-tree). The change unifies the three legacy custom provider types into custom, adds connection-default and per-model protocol selection, and migrates stored catalog/onboarding state.
Readiness: not ready for approval. I found one P1 and three P2 correctness/migration issues in production paths; see the inline comments.
Validation: the affected core, storage, runtime, runtime-host, eval, and Desktop main builds completed, and 301 focused tests passed. The aggregate build:test later stopped in @maka/ui on type errors in unrelated files outside this diff; GitHub's test and label checks are green at this exact head. I did not perform live vendor API calls.
Automated review notice: This review was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
b983403 meant to restore requireBaseUrl on the custom Chat adapter but added it to opencode's entry instead. A migrated custom Chat row without a base URL then failed as a bare Invalid URL rather than naming the connection. Generated-by: Claude Code
…ol differs Treating a defaultApiProtocol difference like a provider-type difference sent a custom-to-custom import overwrite through remove-then-create. The remove commits first and drops the target's credentials, which a connection-only backup cannot restore, and a failed create leaves nothing. Only a provider-type change needs replacement; the update path keeps the existing connection, its protocol and its credentials. Generated-by: Claude Code
…g a hosted target The reuse check compared only the connection default. A model override or discovered wire on the existing connection could still run the requested model on another protocol than the Eval subject asked for. Generated-by: Claude Code
…4-flash The migration only declared webSearch when the model was enabled, so a listed model enabled after the upgrade lost the search anthropic-compatible used to infer. Generated-by: Claude Code
…otocol again Reverts 29458e7. An Eval subject's defaultApiProtocol names the connection's default, not the wire of one model. Comparing it with the requested model's resolved protocol rejected a subject that stated the connection's real default whenever the user had overridden that model's protocol in settings, which is a normal edit. The override is the user's per-model choice and runs as declared, as it does in the desktop app. Generated-by: Claude Code
|
@hqhq1025 thanks for the review. All four inline points are addressed; head is now
A second round of adversarial review on these commits found nothing else to change. CI failed earlier only because of the |
The models.dev snapshot refresh in apache#5678 lists none, minimal, low, medium, high and xhigh for vercel openai/gpt-5.1-thinking. Main's affected-only CI did not run the runtime suite, so the stale expectation surfaced here. Generated-by: Claude Code
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 8685af278750ca003c8bea2ebb034b1f38fd2a2c, which now contains current main bbeb58c68d5cd7393cfff18f9e37a4297bdfda83 (15 commits ahead, 0 behind).
Two previous findings are fixed: custom Chat again fails clearly without a base URL, and listed-but-disabled legacy catalog rows retain hosted-search metadata. I am also withdrawing the Eval protocol finding after checking the clarified contract: defaultApiProtocol names the connection default, while a per-model override remains authoritative.
Readiness: not ready for approval. Two P2 migration/import issues remain; see the inline comments.
Validation: 254 focused Core, Storage, Runtime, Runtime Host, and Desktop tests passed; Core through CLI and Desktop main compiled; git diff --check passed. The aggregate build still stops in @maka/ui on the existing Astryx API type errors (settledText, autoScroll, menuAnchorRef, and trailingAction). I did not run live vendor API calls or native Windows/macOS execution.
Automated review notice: This review was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| name: connection.name, | ||
| providerType: connection.providerType, | ||
| ...(connection.baseUrl ? { baseUrl: connection.baseUrl } : {}), | ||
| ...(connection.defaultApiProtocol === undefined |
There was a problem hiding this comment.
[P2] Do not report a protocol-mismatched snapshot as overwritten while retaining the old protocol
Creation now restores defaultApiProtocol, but the overwrite branch above cannot apply it: that field is fixed at creation (packages/core/src/runtime-policy.ts:293-294), and catalog updates copy previous.defaultApiProtocol (packages/storage/src/runtime-policy/connection-catalog-document.ts:358-367). The importer nevertheless counts the item as overwritten and replaces its endpoint, selection, and override table. On this exact head, importing an openai-responses snapshot over an existing openai-chat connection changed the base URL to the source value but returned and stored openai-chat. The restored connection is therefore a hybrid that does not match the backup and can send unoverridden models on the wrong wire. Please reject/skip this conflict explicitly, or replace/migrate it atomically while preserving credentials.
There was a problem hiding this comment.
Fixed in 006796c: planConnectionMerge now skips an overwrite whose existing custom connection has a different defaultApiProtocol, so the import neither produces a hybrid nor writes the bundle's credential onto it. Replacing it would drop credentials a connection-only backup cannot restore; the user can remove the connection and import again to take the snapshot's protocol.
| const models: unknown = Reflect.get(row, 'models'); | ||
| const enabled: unknown = Reflect.get(row, 'enabledModelIds'); | ||
| if ( | ||
| !Array.isArray(models) || |
There was a problem hiding this comment.
[P2] Preserve hosted search when replaying a legacy onboarding journal
readConnectionOnboardingIntent applies this upgrader to persisted legacy journals (onboarding-transaction.ts:233-280), but those journals store inventory under discovery.models, so this top-level models check returns without adding the declaration. Recovery then passes only intent.discovery to the catalog upsert (coordinator.ts:2142-2171), and the intent has no override table that could restore it. On this exact head, a schema-v2 anthropic-compatible journal containing and enabling deepseek-v4-flash decodes to custom / anthropic-messages and resolves hosted search to null. The catalog-row case is fixed, but this durable crash-recovery path still silently loses behavior that the legacy provider inferred.
There was a problem hiding this comment.
Leaving this one as is. Reaching it needs an older build to crash mid-onboarding of a new anthropic-compatible relay that lists deepseek-v4-flash, and the upgrade to land before recovery runs. The result is that this one model loses hosted search; no data is lost. A durable fix would add an override table to the onboarding intent format for this single path. Marking the discovered entry instead would not hold, because the next model refresh replaces it.
There was a problem hiding this comment.
Not fixing this one; it is now listed as an accepted cost under Breaking change in the PR description. It takes all four at once: an older build, onboarding a new anthropic-compatible relay that lists deepseek-v4-flash, a crash inside the save window, and upgrading before restarting. Nothing is lost: the connection, key and models are recovered, and only that one model goes without hosted search. That is also exactly how a custom connection added fresh on this build behaves, since custom does not infer hosted search from model names. A durable fix would add an override table to the onboarding intent format for this one path, which is not worth it at this reach.
…tocol A custom connection's defaultApiProtocol is fixed at creation, and the catalog update keeps the stored value. Overwriting one from a snapshot on another protocol therefore applied the snapshot's endpoint, selection and overrides onto the old protocol, a hybrid that matches neither side. Plan that conflict as skipped instead; the skip also keeps the bundle's credential off the existing connection. Generated-by: Claude Code
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 006796cfb846a47701b25921a0948683da5e458c against current main bbeb58c68d5cd7393cfff18f9e37a4297bdfda83 (15 commits ahead, 0 behind).
The protocol-conflict import issue is fixed. planConnectionMerge now skips a same-slug custom connection whose fixed default protocol differs. A production import probe reported one skipped connection and one skipped credential with no connection or credential writes; the same input enters overwrite on parent 8685af278.
Readiness: not ready for approval. One P2 remains from the existing inline thread at packages/storage/src/runtime-policy/legacy-custom-connection.ts:50-60. The PR states that a pending onboarding journal from an older build still replays, but the upgrader reads top-level models while the durable journal stores them under discovery.models. A current-head probe through RuntimePolicyCoordinator.recoverForWrite() recovered an enabled deepseek-v4-flash connection as custom / anthropic-messages with no capability declaration, after which hosted-search resolution returned null. This is a narrow crash-then-upgrade path, but it silently and durably changes behavior, and the current model settings editor has no web-search capability control to restore it.
Validation: 171 focused Storage, Core, Runtime, and Desktop config tests passed; Core through CLI and Desktop main compiled; git diff --check passed. The aggregate build still stops in @maka/ui on the existing Astryx API type errors. Hosted CI was still running and was not awaited, per request. No live vendor API calls or native Windows/macOS execution were run.
Automated review notice: This review was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 006796cfb846a47701b25921a0948683da5e458c against current main bbeb58c68d5cd7393cfff18f9e37a4297bdfda83 (15 ahead, 0 behind). I found no unaccepted P0-P2 issues.
The protocol-conflict import fix now detects an existing same-slug custom connection whose defaultApiProtocol differs and skips it rather than constructing a mixed overwrite (packages/storage/src/config-transfer.ts:171-207). The production applyConfigImport path consequently skips both the connection and its associated credential with no writes; the same probe against parent 8685af278750ca003c8bea2ebb034b1f38fd2a2c entered the overwrite path.
The previously reported legacy onboarding-journal hosted-search gap remains a narrow residual risk: after an older build crashes with a pending anthropic-compatible journal and the application is upgraded before recovery, deepseek-v4-flash can be restored without its hosted-search capability. AstroHan explicitly accepted this behavior as a known product tradeoff for this PR, so it is not treated as a merge blocker in this review.
Validation: 171 focused storage/core/runtime/Desktop tests passed; Core through CLI and Desktop main compiled; hosted test completed successfully; git diff --check passed. The aggregate local build still stops in @maka/ui on existing Astryx API type errors. I did not run live vendor API calls or native Windows/macOS execution.
Review notice: This review was prepared by an automated review agent operated by hqhq1025 and is published at the direction of AstroHan, who has read these findings and is the human accountable for them.
Summary
A relay that serves several model families had to be added once per protocol:
openai-compatible,openai-responses-compatibleandanthropic-compatiblewere three provider types with the same credentials, base URL and model list, differing only in the wire. Thinking declared on the Anthropic type also had to infer the thinking mode from model names.This merges the three into one
customprovider type:openai-chat,openai-responsesoranthropic-messages), fixed at creation. As before, the add form and onboarding require a base URL, and execution without one fails rather than reaching a vendor default endpoint; a migrated row whose endpoint an older build let be cleared still reads.modelOverrides[id].apiProtocol. The wire resolves as the per-model override, then the discoveredmodels[].apiProtocol, then the connection default (declaredModelApiProtocol). Runtime, Fast mode, hosted web search, connection tests and provider-state identity all read that one resolution.customkeeps the chat adapter as its base and addsprotocolAdapters['openai-responses' | 'anthropic-messages']. Model discovery lists through the default protocol's endpoint.thinking: { type: 'adaptive', display: 'summarized' }plusoutput_config.effort; with no declared level nothing is sent. There is no model-name inference for custom connections.Removed along the way:
isRelayProviderTypename: 'provider' | 'connection'andruntimeProviderNamesupportsRelayFastServiceTier, replaced by a wire-basedsupportsCustomFastServiceTierpruneModelOverridesserviceTierbranch on open-responseswebSearchdeclaration insteadFixes #5673
Breaking change
RUNTIME_HOST_COMPATIBILITY_EPOCHmoves to 184 because the catalog and onboarding wire now carrycustomanddefaultApiProtocol. Desktop, CLI and a remote Runtime Host must be upgraded together; a mismatched peer is rejected at the handshake.connection-catalog.jsonmoves to schema version 3. v1/v2 rows naming one of the old types are rewritten on read tocustomwith the matching default protocol, keeping id, slug, credentials and model overrides; the next write persists v3. Older builds cannot read a v3 catalog.anthropic-compatiblejournal does not get the DeepSeek-V4-FlashwebSearchdeclaration, because the journal carries no override table. Reaching this takes an older build crashing mid-onboarding of a new relay that lists that model, then the upgrade landing before restart. The connection, key and models are recovered; only that model goes without hosted search, the same as a custom connection added fresh on this build."providerType": "custom"needsdefaultApiProtocol. Configs naming a legacy type are rejected.parallel_tool_calls: true; the server default applies.Verification
AGENTS.md:/v1/responseswith native reasoning, and Claude to/v1/messageswith adaptive thinking andoutput_config.effortdeclaredModelApiProtocolprecedencecustom/v1/modelswithx-api-keyProduct/Settings/Providersand model-picker stories pass theirplayassertions in light and dark, including the newAddCustomConnectionandCustomModelProtocol.npm run formatandnpm run lintare clean. E2E is left to CI.Add connection catalog
Model parameters
AI use
Tool(s) and scope: Claude Code wrote the design, implementation, tests and this description under the author's direction.
Checklist
Does this PR entail a change in behavior?