feat(desktop): read a custom relay's model catalog before saving

[rootkiller6788]

Summary

A custom relay has no published model catalog — the only place its catalog can be read from is the endpoint the user is typing into the form, and there was no way to read it before the connection exists. The form therefore forced a hand-typed model id.

This adds a "read model catalog" action that probes the endpoint and key the user already filled in, then turns the default-model field into a chooser.

Nothing in the runtime host needed changing. The base-url plumbing for onboarding-verify landed upstream for managed-Host discovery, and it already runs the credential → catalog discovery this needs against a transient connection without persisting anything.

This change only wires the desktop side to it:

• an IPC handler
• a shared contract so main/preload/renderer cannot drift
• the row that offers the discovered models

The probe persists nothing and cannot break a reference. It targets connectionId: null against a transient connection — the same shape the post-create discovery fetch uses — so probing never creates, edits, or names a connection.

Staleness

The read is against whatever endpoint and key are in the fields at click time. If either changes while the probe is in flight, the result is discarded rather than attached to the new inputs — a picked model can never belong to a catalog probed from a different endpoint.

Rejected → form copy

The host returns a closed set of rejection reasons. Most map straight onto a sentence in the form:

• credential_not_configured
• base_url_not_configured
• provider_unsupported

One — connection_not_found — cannot describe this probe because it targets no existing connection, so it is folded into a generic failure instead of a reason the form cannot render.

Verification

Eight new tests, each rule pinned by reverting it:

$ node --test apps/desktop/dist/main/**tests**/provider-add-catalog-probe.test.js
ℹ tests 5
ℹ pass 5
ℹ fail 0

$ node --test apps/desktop/dist/main/**tests**/runtime-host-connections-ipc-main.test.js
ℹ tests 13
ℹ pass 13
ℹ fail 0

The following all pass:

npm run typecheck
npm run lint
npm run format:check
git diff --check
npm run check:asf-headers

The full workspace suite does not pass on this machine, and does not pass identically on a clean main — the failures are Windows-environment related:

• EPERM symlink
• EBUSY sqlite
• tar path resolution
• missing native binaries for Rive/computer-use
• macOS-only service tests
• three test files that hang in process isolation

I confirmed the biggest cluster by stashing this branch and rerunning goal-coordinator: it fails 8/8 on clean main exactly as it does here.

Unrelated to this change, reported rather than left unmentioned.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — traced the existing onboarding-verify path to confirm the desktop was the only missing layer, wrote the shared contract, the IPC handler and its projection, the draft logic and the form row, and their tests. The commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[rootkiller6788]

Please let me know if there are any issues, thank you.

[rootkiller6788]

Fixes #3442 — lets the custom-relay add form probe a relay's model catalog (via connection.onboarding.verify against a
transient connection, persisting nothing) and offer the discovered models in a chooser instead of a hand-typed id.

[Astro-Han]

Thanks for adding a pre-save catalog probe; the problem in #3442 is demonstrated and the Desktop → IPC → Runtime Host path is otherwise well aligned with the existing discovery owner. I reviewed exact head b6b1cf51af4a640170409a81a80c123a38a4f2e5 and left two suggestions inline: one P1 credential-boundary issue and one P2 behavior gap. These are suggestions from an adjacent reviewer’s view, so please do push back if the onboarding contract or intended form behavior has context I missed. The exact head currently has no completed CI jobs (action_required), so it will also need a fresh exact-head gate after revision.

中文摘要

谢谢补上保存前读取模型目录。#3442 的问题和整体调用链都成立；当前有两点建议：connectionId: null 会复用 canonical connection 的旧凭据/headers 并把它们发往新 endpoint（P1），以及表单自定义 headers 没有参与 probe（P2）。这些是旁观评审视角，如果 onboarding 合同或产品语义另有上下文，欢迎直接 push back。修改后还需要重新获得 exact-head CI。

AI-assisted review disclosure: Codex ran an independent analysis lane; Astro-Han independently verified the exact head, production path, and severity, and owns this review.

[rootkiller6788]

Thanks for catching both — you were right on each.

• P1 (credential leak): the probe now passes transient: true on connection.onboarding.verify, which skips
  canonical-slug resolution in storage, so a blank key no longer reuses the existing relay's stored credential. A
  blank key now returns credential_not_configured instead of probing with the wrong secret.
• P2 (headers gap): the form's custom request headers ride along on the probe request and are validated at the IPC
  boundary, matching what the save path sends.

Added regression coverage: a transient probe against an existing canonical relay asserts its key and headers never
reach the new endpoint, plus decode coverage for the new transient/requestHeaders fields.

[Astro-Han]

Verified against head ae5236c — and there's a structural problem to surface before line-level findings: this capability already shipped on main. #3443 ('let a relay's model catalog answer for itself', commit 49a54bf) landed a generalized version of the same flow: the host's connection.onboarding.verify operation already supports target: {kind:'create'|'existing'} + baseUrl with transient discovery (fresh candidate, no stored credential/header resolution), and the desktop already has connections:onboardingVerify IPC + verifyOnboarding preload + a managed verify→choose→save form flow in provider-add-form.tsx. This PR is built on a pre-#3443 base and re-implements the same path under different names (transient: true ≈ target.kind:'create', connections:probeModels ≈ connections:onboardingVerify, ConnectionCatalogProbe* ≈ ConnectionOnboardingVerify*).

So the decision before any line review: rebase onto main's machinery and contribute only the delta, or close. The honest delta I can find is requestHeaders forwarding on the probe — worth checking whether main's verify already carries it; if not, that's a small additive PR against the existing operation, not a parallel one.

If this does continue after rebase, the diff's own issues:

P2 — cross-origin redirects forward credentials to the redirect target (③ trust boundary). fetchForConnectionEffect (connection-effect-fetch.ts:82-91) leaves undici's redirect: 'follow' default; per fetch spec only authorization/cookie/host headers are stripped on cross-origin hops — x-api-key (anthropic-compatible relays) and custom headers ride to wherever the 30x points, including https→http downgrades and internal hosts the baseURL gate never vetted. Inherited caveat: connection.models.fetch/test.run on saved connections already have this exposure, so this PR widens a trigger rather than creating the mechanism — but a /models catalog has no legitimate need to redirect, so redirect: 'error' (or manual revalidation like local-web-fetch.ts:79-99) is a one-line hardening worth doing for the whole effect-fetch path.

P3 — parallel DTO layer. shared/connection-catalog-probe.ts + projectConnectionCatalogProbe re-state the verify wire types for one consumer with a cosmetic ready/verified rename; the connection_not_found remap is unreachable under transient. The host types are already renderer-safe — delete the layer and type the IPC with the wire types directly (~150-200 lines).

P3 — ConnectionOnboardingSaveInput extends the verify input, re-admitting transient/requestHeaders to in-process save callers (#3299's review closed exactly this gap by keeping save input standalone — spell the shared fields instead).

P3 sweep: connections-ipc-validation.ts:163 PROVIDER_DEFAULTS bracket lookup admits inherited members (use providerDefaultsOf per the codebase's own rule); probe apiKey lacks the 4096 cap/control-char check the create path applies; decode bounds models only to non-empty (add the catalog cap for symmetry); probeCatalogHelp copy key is defined but never rendered; a non-string display_name fails the entire discovery where a typeof guard would degrade gracefully; the 'local relay may accept a catalog read without credentials' comment only holds for optional_api_key providers.

Needs one check: the PR adds keys to the closed verify wire schema — confirm the compatibility epoch bumps accordingly, or an older Host rejects the fields mid-operation rather than at handshake.

[github-actions]

This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed.

Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the pinned label.

[rootkiller6788]

Rebased onto current main (dcef642) as a single commit (13a6137) built on the existing target {kind:'create'|'existing'} verify machinery, so the only delta left is optional requestHeaders forwarding on a create target plus the desktop routing that uses it — and no RUNTIME_HOST_COMPATIBILITY_EPOCH bump, since the field is optional and follows the same shape as #4605/#4621's optional slug/name, where an older Host rejects the extra field at frame decode rather than mid-operation.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review: ae5236cc (our last reviewed head) to 13a6137d (one commit, "probe a custom relay with its endpoint and headers"; 9 files, +415/-78). The branch is now a single commit on current main dcef6427, so comparing (old head vs its merge-base 00a15abd) with (new head vs dcef6427) shows a full rewrite, not an increment. GitHub reports MERGEABLE (merge state BLOCKED). CI: the only workflow run on this head is action_required, so no job has run yet.

Prior findings. The structural point from the last round is resolved. The parallel connections:probeModels / ConnectionCatalogProbe* layer is gone. The PR now adds the one real delta to main's #3443 machinery: optional requestHeaders on a create verify target. The earlier P1 (a connectionId: null probe reusing the canonical connection's stored credentials) is fixed because the probe uses a transient create target. The earlier P2 (form headers not sent by the probe) is the point of this change and is fixed. ConnectionOnboardingSaveInput no longer extends the verify input (P3, fixed). The DTO-layer and connections-ipc-validation P3s no longer apply because that code is gone. The redirect hardening (connection-effect-fetch.ts still follows redirects) is unchanged. It comes from main and this PR does not create it, so I now grade it P3. Caller-supplied custom headers do now follow a cross-origin redirect, though, so redirect: 'error' is still worth adding.

New findings (inline):

• P1: provider-add-submission.ts:73-76. Built-in providers with advanced request headers now take the managed route, and the managed save (provider-add-form.tsx:344-349) does not carry the headers. The user's headers are silently dropped.
• P2: provider-add-form.tsx:277. A custom relay with an endpoint can now only be added after verify succeeds. If its catalog fails or comes back empty, nothing falls back to the old create path. The typed Default model is also ignored.
• P2: provider-add-form.tsx:326-332. For a custom relay, the picker's model selection is discarded. Create enables only the chosen default.
• P2: connection-effects.ts:387. This adds a field to the verify wire that an older Host rejects, but the compatibility epoch is not bumped. That goes against the convention at protocol/index.ts:107-109.

P3 (not inline):

• createConnectionFromForm validation errors with field: 'slug' | 'baseUrl' | 'apiKey' (provider-add-form.tsx:405) are not shown in the models picker, which renders only form errors (:686). Clicking Back also clears them.
• The custom form's primary button still reads Save / Saving (:899), but it now starts a verify step.

Verification and limits. I read the diff and traced the call chains at this exact head: the IPC handler reuses CONNECTION_EFFECT_OPERATION_SPECS[...].decodeInput, so headers do reach the Host, and #saveOnboarding re-runs discovery from the save input without headers. git diff --check is clean. I did not run tests or build locally, and CI has not run on this head. Protocol epoch: main is 204. #5709/#5902/#5548 claim 205 and #4138 claims 206, so this PR would need the next free epoch when it merges.

[Astro-Han]

P2: a custom relay can no longer be added unless verify succeeds. With hasEndpoint, every custom relay without a body overlay now goes through verifyManagedApiKey. If discovery fails, or the catalog is empty (the Host maps empty to failed/invalid_response, connection-effect-coordinator.ts:199), this line shows an error and stays in the input phase. Nothing falls back to create. On main, a relay without a working /models could still be created with a typed Default model (createProviderWithDiscovery treats discovery failure as non-fatal and returns modelDiscoveryError). Now the only workaround is adding a request body overlay. The form still shows the Default model field for custom (showsDefaultModel), but the managed path ignores it.

Suggestion: when verify fails or returns no models for a custom relay, offer the create-then-discover path with the typed default model, like main does. Please add a test for a relay whose catalog read fails.

[rootkiller6788]

All four findings are addressed in 0d9439d, including the epoch bump to 207 — the built-in header drop and the missing relay fallback were real, and both are fixed.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review: 13a6137d (our last reviewed head) to 0d9439d4 (one new commit, "carry a relay's selection and headers through its own writers"; 7 files, +319/-38). The merge-base is unchanged (dcef6427, current main), so the comparison is exactly 13a6137d..0d9439d4.

Prior findings

• P1 (built-in providers with advanced headers lose them on the managed save): fixed. apiKeyOnboardingRoute takes hasRequestHeaders and sends a header-bearing draft for any provider except custom to the legacy writer (provider-add-submission.ts:106-108, reason request_headers). The legacy writer persists headers through replaceConnectionRequestHeaders. custom stays on the Host probe because its save is already the create writer. Covered by the new openai + headers routing assertion.
• P2 (a relay whose catalog fails or is empty cannot be added; typed Default model ignored): fixed. verifyManagedApiKey now returns false for custom on a non-verified result or an empty selection (provider-add-form.tsx:290, :303). submit then falls through to createConnectionFromForm, which reports modelDiscoveryError through onCreated as main does. The typed default is used as preferredDefaultModel for both the picker preselection and the fallback create. Transport exceptions still show an error and do not fall back, which seems right.
• P2 (multi-select discarded for custom relays): fixed. The picker's selection, default first, is sent as enabledModelIds on connections:create. The handler merges it through connectionEnabledModelIds (runtime-host-connections-ipc-main.ts:214-220). IPC validation is bounded: the list cap, the id length, control characters and no duplicates (connections-ipc-validation.ts:71-95). The follow-up fetchModels keeps the selection, because custom is non-authoritative and reconcileConnectionAfterModelFetch preserves user choices. Tests cover the create path and the validation.
• P2 (new verify wire field without an epoch bump): fixed. RUNTIME_HOST_COMPATIBILITY_EPOCH is now 207, with a note (protocol/index.ts:107-112). Main is 204, 205 is claimed by #5548/#5709/#5902 and 206 by #4138, so 207 is the next free number. Whichever of these merges later must renumber.
• P3 (create errors not shown in the models picker; button labelled Save): fixed. The picker banner now shows any error, and the quick dialog shows every error except the fields it renders inline. The custom form's primary button now reads "Verify and choose" when it starts onboarding.
• P3 (connection-effect fetch follows redirects with caller headers): unchanged. This comes from main and is not in this delta.

New findings (inline, both P3)

• provider-add-form.tsx:290: for a custom relay, an auth verify failure also falls back to create. A wrong key is saved, and the user only sees a discovery error afterwards.
• provider-add-form.tsx:301: a typed Default model that the catalog does not list is silently replaced by the first catalog model.

No P0-P2 open. CI: the only workflow run on this head is action_required (the fork run needs maintainer approval), so no job has run. GitHub reports MERGEABLE (merge state BLOCKED). git merge-tree against main and git diff --check are both clean. Not run locally: the tests, the build, and the Electron UI. Not covered by a test: the form-level fallback (verify fails, so create runs), which I traced by hand. This is not merge approval.

[Astro-Han]

P3: an auth failure also falls back to create for a relay. This early return covers every non-verified result, including failed with errorClass: 'auth'. A relay whose key the probe just rejected is therefore created anyway. The user pressed "Verify and choose" and gets a saved connection plus a discovery error, instead of the apiKey field error that other providers get. Consider falling back only for catalog-shaped failures (no endpoint, empty or invalid response), and keeping the apiKey error for auth.

[Astro-Han]

P3: a typed Default model outside the catalog is silently replaced. initialOnboardingModelIds only preselects preferredDefaultModel when /models lists it, and otherwise picks the first catalog model. A relay that serves a model its /models omits is a common case. For that relay, the id the user typed vanishes from the picker, and nothing says why. Consider adding the typed id to the picker as an unlisted entry, or showing a short notice that the catalog does not list it.