You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "rootkiller6788 (via GitHub)" <gi...@apache.org> on 2026/08/24 09:31:26 UTC
[PR] feat(desktop): probe a custom relay's model catalog before creating t… [maka]
rootkiller6788 opened a new pull request, #3700:
URL: https://github.com/apache/maka/pull/3700
## 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:
```text
$ 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:
```text
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
- [x] 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
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
### Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-5976836846
<!-- maka-lifecycle:stale-warning -->
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "rootkiller6788 (via GitHub)" <gi...@apache.org>.
rootkiller6788 commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-5427443507
Please let me know if there are any issues, thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "github-actions[bot] (via GitHub) via commits" <co...@maka.apache.org>.
github-actions[bot] commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-6649204240
<!-- maka-lifecycle:stale-warning -->
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3700:
URL: https://github.com/apache/maka/pull/3700#discussion_r4181531932
##########
apps/desktop/src/renderer/settings/provider-add-submission.ts:
##########
@@ -63,12 +70,24 @@ export function apiKeyOnboardingRoute(input: {
if (input.providerType === 'cloudflare-workers-ai') {
return { kind: 'legacy', reason: 'cloudflare' };
}
- if (!definition.baseUrl) return { kind: 'legacy', reason: 'custom_endpoint' };
- if (input.requestHeaderCount > 0) return { kind: 'legacy', reason: 'request_headers' };
+ if (!definition.baseUrl && !input.hasEndpoint) {
+ return { kind: 'legacy', reason: 'custom_endpoint' };
+ }
if (input.hasRequestBodyOverlay) return { kind: 'legacy', reason: 'request_body' };
Review Comment:
**P1: built-in providers lose their custom request headers.** Removing the `request_headers` branch routes an OpenAI/Anthropic/etc. quick-dialog draft that has headers in the Advanced Request Editor (the quick dialog renders `advancedRequestEditor`) to the Host route. Verify now sends those headers. But the non-custom branch of `saveManagedApiKey` (`provider-add-form.tsx:344-349`) calls `onboarding.save` with only `target/apiKey/baseUrl/enabledModelIds`. The save input deliberately rejects `requestHeaders`, and nothing calls `setRequestHeaders` afterward. So the connection is saved without the headers the user typed and no message is shown. `#saveOnboarding` also re-runs discovery without them, so a provider that needs the header fails at save even though verify passed. On main this draft went to the legacy writer, which persists headers.
Suggestion: keep header-bearing drafts for non-custom providers on the legacy route (restore the `request_headers` reason except for `custom`). Or, after a successful managed save, commit the headers through `connections:setRequestHeaders` and handle failure explicitly. Please add a routing/form test for `openai` plus a header.
##########
apps/desktop/src/renderer/settings/provider-add-form.tsx:
##########
@@ -220,16 +231,47 @@ export function AddProviderForm(props: {
return copy.onboardingUnavailable;
}
- async function verifyManagedApiKey(normalizedApiKey: string) {
+ /**
+ * What the probe must carry beyond the key: a custom relay's endpoint and
+ * whatever headers the advanced editor holds. Both describe the connection
+ * the form is about to create, so probing without them would answer for a
+ * relay nobody asked to add.
+ */
+ function probeMaterial(customization: FormRequestCustomization) {
+ const headerUpdates = probeRequestHeaderUpdates(customization.headers);
+ return {
+ baseUrl: isCustom ? baseUrl.trim() || null : null,
+ ...(headerUpdates.length === 0 ? {} : { requestHeaders: headerUpdates }),
+ };
+ }
+
+ /**
+ * A custom target has to name the protocol it speaks — the Host resolves the
+ * catalog endpoint from it, and the wire closes the field to every other
+ * provider. Only the probe asks for one: a custom save still goes through
+ * create-then-discover, which carries the protocol on its own input.
+ */
+ function probeTarget() {
+ return {
+ kind: 'create' as const,
+ providerType: props.providerType,
+ ...(isCustom ? { defaultApiProtocol } : {}),
+ };
+ }
+
+ async function verifyManagedApiKey(
+ normalizedApiKey: string,
+ customization: FormRequestCustomization,
+ ) {
const onboarding = props.apiKeyOnboardingBridge;
if (!onboarding) return;
submitGuard.begin('submit');
setBusy(true);
try {
const result = await onboarding.verify({
- target: { kind: 'create', providerType: props.providerType },
+ target: probeTarget(),
apiKey: normalizedApiKey || null,
- baseUrl: null,
+ ...probeMaterial(customization),
});
if (!addProviderMountedRef.current) return;
if (result.kind !== 'verified') {
Review Comment:
**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.
##########
apps/desktop/src/renderer/settings/provider-add-form.tsx:
##########
@@ -270,12 +312,25 @@ export function AddProviderForm(props: {
async function saveManagedApiKey(
normalizedApiKey: string,
phase: Extract<ManagedOnboardingPhase, { kind: 'models' }>,
+ customization: FormRequestCustomization,
) {
const onboarding = props.apiKeyOnboardingBridge;
if (!onboarding || phase.selectedIds.length === 0) {
setError({ field: 'form', message: copy.onboardingSelectModel });
return;
}
+ // A custom relay still saves through create-then-discover. The managed
+ // save commits the catalog and the key, but not the endpoint headers this
+ // form just probed with — a connection saved without them would fetch
+ // nothing. The picker's chosen default is the one thing create needs.
+ if (isCustom) {
+ await createConnectionFromForm({
+ normalizedApiKey,
+ createdDefaultModel: phase.defaultId,
Review Comment:
**P2: the picker's model selection is discarded for a custom relay.** The picker shows multi-select with "N of M selected". This branch passes only `phase.defaultId` to `createConnectionFromForm`, and `connections:create` builds `enabledModelIds` from `defaultModel` alone (`runtime-host-connections-ipc-main.ts:214`). A user who selects five models gets one enabled. Either pass `phase.selectedIds` through to create (CreateConnectionInput would need to carry it, or call `update` with `enabledModelIds` after create), or hide the multi-select for custom relays.
##########
packages/runtime-host/src/protocol/connection-effects.ts:
##########
@@ -343,13 +380,20 @@ export function decodeConnectionOnboardingSaveResult(
export function decodeConnectionOnboardingVerifyInput(
value: unknown,
): ConnectionOnboardingVerifyInput {
- const input = requireExactRecord(value, 'connection onboarding verification input', [
- 'target',
- 'apiKey',
- 'baseUrl',
- ]);
+ // `requestHeaders` is optional rather than always-present on the wire: a
+ // caller that never sends one keeps working against a Host that predates
+ // the field, and an older Host rejects a caller that does send one up front,
+ // while it is decoding this frame instead of mid-operation.
+ const input = requireShapedRecord(
Review Comment:
**P2: compatibility epoch not bumped for a field that older Hosts reject.** Before this change, `decodeConnectionOnboardingVerifyInput` used `requireExactRecord`, so a pre-PR Host rejects any frame carrying `requestHeaders`. Desktop calls this through `invokeSelectedRuntimeHost`, which can target a remote Host. A newer Desktop adding a custom relay with headers against an older Host therefore fails mid-operation with `invalid_request`, which the form shows as a generic failure. The repo convention is to bump `RUNTIME_HOST_COMPATIBILITY_EPOCH` exactly when "older peers reject these fields" (`protocol/index.ts:107-109`, for example 204), so the mismatch is caught at handshake. Main is at 204. 205 and 206 are claimed by open PRs, so this needs the next free epoch, with an entry in the epoch comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "rootkiller6788 (via GitHub)" <gi...@apache.org>.
rootkiller6788 commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-5989185917
Rebased onto current main (dcef6427) as a single commit (13a6137d) 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "rootkiller6788 (via GitHub)" <gi...@apache.org>.
rootkiller6788 commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-5523464893
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "rootkiller6788 (via GitHub)" <gi...@apache.org>.
rootkiller6788 commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-5463477812
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "rootkiller6788 (via GitHub)" <gi...@apache.org>.
rootkiller6788 commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-5998922864
All four findings are addressed in 0d9439d4, including the epoch bump to 207 — the built-in header drop and the missing relay fallback were real, and both are fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3700:
URL: https://github.com/apache/maka/pull/3700#discussion_r3889718232
##########
apps/desktop/src/renderer/settings/provider-add-form.tsx:
##########
@@ -355,14 +366,25 @@ export function AddProviderForm(props: {
/>
)}
{showsDefaultModel && (
- <TextInput
- value={defaultModel}
- onChange={setDefaultModel}
- placeholder={copy.defaultModelPlaceholder}
- isDisabled={isExperimental || busy}
- label={copy.defaultModel}
- description={copy.defaultModelHelp}
- />
+ <VStack gap={2}>
+ <TextInput
+ value={defaultModel}
+ onChange={setDefaultModel}
+ placeholder={copy.defaultModelPlaceholder}
+ isDisabled={isExperimental || busy}
+ label={copy.defaultModel}
+ description={copy.defaultModelHelp}
+ />
+ <CatalogProbeField
+ providerType={props.providerType}
+ baseUrl={baseUrl}
+ apiKey={apiKey}
+ value={defaultModel}
+ isDisabled={isExperimental || busy}
+ probeModels={(request) => props.bridge.probeModels(request)}
Review Comment:
[P2] Suggestion (category ① — normal custom-relay setup): could the probe include the normalized request headers from the Advanced Request Editor? The save path sends `requestHeaders`, but this probe contract carries only provider/base URL/API key. Relays whose `/models` endpoint requires a custom header will therefore fail before saving even though the immediately saved connection can discover successfully. I suggest using the same normalized header material in a transient Host-owned probe and adding that case to the probe regression coverage. Please push back if the pre-save action is intentionally narrower than post-save discovery.
##########
apps/desktop/src/main/runtime-host-connections-ipc-main.ts:
##########
@@ -289,6 +295,20 @@ export function registerRuntimeHostConnectionsIpc(
fetchedAt: result.fetchedAt,
};
});
+ // Probe the model catalog an endpoint serves BEFORE a connection exists, so
+ // the custom-relay form can offer a real choice instead of a guess. Runs the
+ // same discovery the post-create fetch uses against a transient connection;
+ // `connectionId` is null because there is nothing to edit yet.
+ deps.ipcMain.handle('connections:probeModels', async (_event, raw: unknown) => {
+ const input = normalizeConnectionCatalogProbeInput(raw);
+ const result = await deps.client.probeConnectionModels({
+ providerType: input.providerType,
+ connectionId: null,
Review Comment:
[P1] Suggestion (category ① — normal add-a-second-relay path; also a credential trust boundary): could this probe use a genuinely transient target rather than `connectionId: null`? In the current Host contract, null targets the provider’s canonical slug. If that connection already exists, `beginConnectionOnboarding()` returns its stored API key and request-header secret; `#discoverOnboarding()` then combines those secrets with the new form’s `baseUrl` and sends the discovery request to that new endpoint. A user adding a second custom relay with an empty/new key can therefore disclose the existing relay’s credentials and get a catalog for the wrong credential/header combination. I suggest a transient probe operation that carries only material explicitly present in this form, plus a regression with an existing canonical relay asserting its key/headers never go to the new endpoint. Please push back if I have misunderstood the null-target invariant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3700:
URL: https://github.com/apache/maka/pull/3700#discussion_r4186722560
##########
apps/desktop/src/renderer/settings/provider-add-form.tsx:
##########
@@ -220,47 +237,88 @@ export function AddProviderForm(props: {
return copy.onboardingUnavailable;
}
- async function verifyManagedApiKey(normalizedApiKey: string) {
+ /**
+ * What the probe must carry beyond the key: a custom relay's endpoint and
+ * whatever headers the advanced editor holds. Both describe the connection
+ * the form is about to create, so probing without them would answer for a
+ * relay nobody asked to add.
+ */
+ function probeMaterial(customization: FormRequestCustomization) {
+ const headerUpdates = probeRequestHeaderUpdates(customization.headers);
+ return {
+ baseUrl: isCustom ? baseUrl.trim() || null : null,
+ ...(headerUpdates.length === 0 ? {} : { requestHeaders: headerUpdates }),
+ };
+ }
+
+ /**
+ * A custom target has to name the protocol it speaks — the Host resolves the
+ * catalog endpoint from it, and the wire closes the field to every other
+ * provider. Only the probe asks for one: a custom save still goes through
+ * create-then-discover, which carries the protocol on its own input.
+ */
+ function probeTarget() {
+ return {
+ kind: 'create' as const,
+ providerType: props.providerType,
+ ...(isCustom ? { defaultApiProtocol } : {}),
+ };
+ }
+
+ /**
+ * Returns false when the caller should create the connection instead: a
+ * probe that cannot answer is not a refusal for a relay whose save is the
+ * create-then-discover writer, which reports a failed discovery rather than
+ * refusing to create. Every other provider keeps the managed route's answer.
+ */
+ async function verifyManagedApiKey(
+ normalizedApiKey: string,
+ customization: FormRequestCustomization,
+ ): Promise<boolean> {
const onboarding = props.apiKeyOnboardingBridge;
- if (!onboarding) return;
+ if (!onboarding) return false;
submitGuard.begin('submit');
setBusy(true);
try {
const result = await onboarding.verify({
- target: { kind: 'create', providerType: props.providerType },
+ target: probeTarget(),
apiKey: normalizedApiKey || null,
- baseUrl: null,
+ ...probeMaterial(customization),
});
- if (!addProviderMountedRef.current) return;
+ if (!addProviderMountedRef.current) return true;
if (result.kind !== 'verified') {
+ if (usesLegacyConnectionWriter(props.providerType)) return false;
Review Comment:
**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`.
##########
apps/desktop/src/renderer/settings/provider-add-form.tsx:
##########
@@ -220,47 +237,88 @@ export function AddProviderForm(props: {
return copy.onboardingUnavailable;
}
- async function verifyManagedApiKey(normalizedApiKey: string) {
+ /**
+ * What the probe must carry beyond the key: a custom relay's endpoint and
+ * whatever headers the advanced editor holds. Both describe the connection
+ * the form is about to create, so probing without them would answer for a
+ * relay nobody asked to add.
+ */
+ function probeMaterial(customization: FormRequestCustomization) {
+ const headerUpdates = probeRequestHeaderUpdates(customization.headers);
+ return {
+ baseUrl: isCustom ? baseUrl.trim() || null : null,
+ ...(headerUpdates.length === 0 ? {} : { requestHeaders: headerUpdates }),
+ };
+ }
+
+ /**
+ * A custom target has to name the protocol it speaks — the Host resolves the
+ * catalog endpoint from it, and the wire closes the field to every other
+ * provider. Only the probe asks for one: a custom save still goes through
+ * create-then-discover, which carries the protocol on its own input.
+ */
+ function probeTarget() {
+ return {
+ kind: 'create' as const,
+ providerType: props.providerType,
+ ...(isCustom ? { defaultApiProtocol } : {}),
+ };
+ }
+
+ /**
+ * Returns false when the caller should create the connection instead: a
+ * probe that cannot answer is not a refusal for a relay whose save is the
+ * create-then-discover writer, which reports a failed discovery rather than
+ * refusing to create. Every other provider keeps the managed route's answer.
+ */
+ async function verifyManagedApiKey(
+ normalizedApiKey: string,
+ customization: FormRequestCustomization,
+ ): Promise<boolean> {
const onboarding = props.apiKeyOnboardingBridge;
- if (!onboarding) return;
+ if (!onboarding) return false;
submitGuard.begin('submit');
setBusy(true);
try {
const result = await onboarding.verify({
- target: { kind: 'create', providerType: props.providerType },
+ target: probeTarget(),
apiKey: normalizedApiKey || null,
- baseUrl: null,
+ ...probeMaterial(customization),
});
- if (!addProviderMountedRef.current) return;
+ if (!addProviderMountedRef.current) return true;
if (result.kind !== 'verified') {
+ if (usesLegacyConnectionWriter(props.providerType)) return false;
setError({
field:
result.kind === 'failed' && result.errorClass === 'auth'
? 'apiKey'
: 'form',
message: onboardingFailureMessage(result),
});
- return;
+ return true;
}
const models = stableOnboardingModels(result.models);
- const selectedIds = initialOnboardingModelIds(models, recommendedDefaultModel);
+ const selectedIds = initialOnboardingModelIds(models, preferredDefaultModel);
Review Comment:
**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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(desktop): read a custom relay's model catalog before saving [maka]
Posted by "rootkiller6788 (via GitHub) via commits" <co...@maka.apache.org>.
rootkiller6788 commented on PR #3700:
URL: https://github.com/apache/maka/pull/3700#issuecomment-6659591622
Rebased onto 5d3a0629 with epoch 239, fixed the Biome formatting failure and both P3s, and re-verified build, typecheck, all targeted suites, and biome check locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org