From bd9cb93c879fe25acca07ceaa06172c3aa5ac2e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=86=E9=80=8A?= <72533078+UncertaintyDeterminesYou4ndMe@users.noreply.github.com> Date: Sat, 22 Aug 2026 14:25:53 +0800 Subject: [PATCH 1/4] fix(cli): let the TUI wizard create custom relay connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setup wizard filtered out every provider without a built-in base URL — an explicit phase-1 scope cut (#1254) whose phase-2 base-URL prompt never landed — and the onboarding protocol had no field to carry an endpoint anyway, so the three custom relays were creatable from Desktop but unreachable from the TUI. List the category:'custom' relays (cloudflare-workers-ai stays out: its endpoint is an account-id template, not a user-supplied URL), add a base-URL step to the wizard between provider pick and API key with host-mirroring local validation, and thread an always-present 'baseUrl: string | null' through connection.onboarding.verify/save — exact-record wire style like apiKey, normalized by the shared catalog rules, rejected as base_url_not_configured when a relay has no endpoint from input, existing connection, or registry. Blank input on an existing relay reuses its persisted endpoint, mirroring the blank-key contract. Discovery runs against the supplied endpoint and commit persists it: the intent journal gains the field (legacy journals still replay), the upsert resolves input ?? previous ?? registry default, a URL-only change still commits, and a swapped endpoint drops relayModelProfiles and lastTest — the endpoint-keyed contract the update path already enforces. The onboarding wire shapes are closed schemas, so the compatibility epoch moves to 37. Fixes #3405 Generated-by: Claude Code --- .../cli/src/__tests__/pi-tui-runner.test.ts | 77 ++++++++++ packages/cli/src/onboarding-catalog.ts | 33 +++-- packages/cli/src/pi-tui-contracts.ts | 4 + packages/cli/src/pi-tui-pickers.ts | 139 ++++++++++++++++-- packages/cli/src/pi-tui-runner.ts | 18 ++- packages/cli/src/runtime-host-onboarding.ts | 10 +- .../connection-effect-coordinator.test.ts | 84 ++++++++++- .../connection-effects-protocol.test.ts | 19 ++- .../src/__tests__/protocol.test.ts | 6 + .../src/protocol/connection-effects.ts | 31 +++- packages/runtime-host/src/protocol/index.ts | 4 +- .../server/connection-effect-coordinator.ts | 26 +++- .../__tests__/onboarding-transaction.test.ts | 73 +++++++++ .../connection-catalog-document.ts | 24 ++- .../storage/src/runtime-policy/coordinator.ts | 2 + .../runtime-policy/onboarding-transaction.ts | 48 ++++-- .../storage/src/runtime-policy/operations.ts | 2 + 17 files changed, 544 insertions(+), 56 deletions(-) create mode 100644 packages/storage/src/__tests__/onboarding-transaction.test.ts diff --git a/packages/cli/src/__tests__/pi-tui-runner.test.ts b/packages/cli/src/__tests__/pi-tui-runner.test.ts index 930513d668..84d559537f 100644 --- a/packages/cli/src/__tests__/pi-tui-runner.test.ts +++ b/packages/cli/src/__tests__/pi-tui-runner.test.ts @@ -186,10 +186,12 @@ interface FakeOnboardingOpts { verify?: (input: { providerType: ProviderType; apiKey?: string; + baseUrl?: string; }) => Promise; save?: (input: { providerType: ProviderType; apiKey?: string; + baseUrl?: string; enabledModelIds: readonly string[]; models: readonly ModelInfo[]; }) => Promise; @@ -884,6 +886,81 @@ describe('Maka Pi TUI runner', () => { ]); }); + test('wizard collects a base URL for a custom relay and threads it through verify and save', async () => { + const terminal = new FakeTerminal(); + const driver = new SlashCommandDriver(); + const verifyCalls: Array<{ baseUrl?: string }> = []; + const saveCalls: Array<{ baseUrl?: string }> = []; + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'claude-sonnet-4-5', + connectionSlug: 'claude-subscription', + permissionMode: 'bypass', + terminal, + onboarding: fakeOnboardingSurface({ + verify: async (input) => { + verifyCalls.push(input); + return { kind: 'ok', models: [{ id: 'relay/model' }] }; + }, + save: async (input) => { + saveCalls.push(input); + return { kind: 'ok', modelChoices: [] }; + }, + }), + }); + + await waitForTuiPaint(terminal); + terminal.input('/setup'); + terminal.input('\r'); + await waitFor(() => { + try { + return latestPlainLineContaining(terminal.writes.join(''), 'Set Up Provider') !== null; + } catch { + return false; + } + }); + // Filter down to the relay entries and pick the first (OpenAI Chat). + terminal.input('relay'); + terminal.input('\r'); + // The relay flow inserts the base-URL step (2/4) before the key. + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('Base URL')); + assert.ok(plainTerminalOutput(terminal.screenOutput()).includes('2/4')); + // A malformed endpoint is rejected in place, before any host call. + terminal.input('not a url'); + terminal.input('\r'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('不是有效的 URL')); + for (let i = 0; i < 'not a url'.length; i++) terminal.input('\x7f'); // clear the field + terminal.input('https://relay.example.test/v1'); + terminal.input('\r'); + await waitFor(() => { + try { + return latestPlainLineContaining(terminal.writes.join(''), 'API key') !== null; + } catch { + return false; + } + }); + terminal.input('sk-relay'); + terminal.input('\r'); + await waitFor(() => verifyCalls.length === 1); + assert.equal(verifyCalls[0]?.baseUrl, 'https://relay.example.test/v1'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('4/4')); + terminal.input(' '); // toggle the discovered model on + terminal.input('\r'); // save + await waitFor(() => saveCalls.length === 1); + assert.equal(saveCalls[0]?.baseUrl, 'https://relay.example.test/v1'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('已启用')); + + process.emit('SIGTERM'); + await Promise.race([ + run, + delay(CLOSE_BUDGET_MS).then(() => { + throw new Error('TUI did not close after SIGTERM'); + }), + ]); + }); + test('save refreshes the running model choices even when the user backs out during saving', async () => { const terminal = new FakeTerminal(); const driver = new SlashCommandDriver(); diff --git a/packages/cli/src/onboarding-catalog.ts b/packages/cli/src/onboarding-catalog.ts index 95dd6dd5a1..ac8d7d4a63 100644 --- a/packages/cli/src/onboarding-catalog.ts +++ b/packages/cli/src/onboarding-catalog.ts @@ -25,16 +25,25 @@ import { import type { OnboardableProvider } from './pi-tui-contracts.js'; export function listApiKeyOnboardableProviders(): OnboardableProvider[] { - return CATALOG_PROVIDER_TYPES.filter((providerType) => providerAuthSupportsApiKey(providerType)) - .map((providerType) => { - const definition = PROVIDER_DEFAULTS[providerType]; - return { - providerType, - label: definition.label, - authKind: definition.authKind as 'api_key' | 'optional_api_key', - requiresBaseUrl: !definition.baseUrl, - fallbackModels: definition.fallbackModels, - }; - }) - .filter((provider) => !provider.requiresBaseUrl); + // Custom relays have no built-in base URL and stay listed: `requiresBaseUrl` + // tells the wizard to collect an endpoint before the API key. The original + // phase-1 wizard filtered every empty-baseUrl provider out because it had no + // base-URL step to offer (#1254); that step exists now (#3405). Providers + // whose endpoint is derived rather than user-supplied (cloudflare-workers-ai + // interpolates an account id into a URL template) are still excluded — a + // plain base-URL prompt cannot onboard them. + return CATALOG_PROVIDER_TYPES.filter((providerType) => { + if (!providerAuthSupportsApiKey(providerType)) return false; + const definition = PROVIDER_DEFAULTS[providerType]; + return Boolean(definition.baseUrl) || definition.category === 'custom'; + }).map((providerType) => { + const definition = PROVIDER_DEFAULTS[providerType]; + return { + providerType, + label: definition.label, + authKind: definition.authKind as 'api_key' | 'optional_api_key', + requiresBaseUrl: !definition.baseUrl, + fallbackModels: definition.fallbackModels, + }; + }); } diff --git a/packages/cli/src/pi-tui-contracts.ts b/packages/cli/src/pi-tui-contracts.ts index d7dc7901bd..12c16fd560 100644 --- a/packages/cli/src/pi-tui-contracts.ts +++ b/packages/cli/src/pi-tui-contracts.ts @@ -58,6 +58,8 @@ export interface OnboardingProviderEntry extends OnboardableProvider { export interface OnboardingVerifyInput { providerType: ProviderType; apiKey?: string; + /** Endpoint for `requiresBaseUrl` providers; blank reuses the persisted one. */ + baseUrl?: string; } export type OnboardingVerifyResult = @@ -67,6 +69,8 @@ export type OnboardingVerifyResult = export interface OnboardingSaveInput { providerType: ProviderType; apiKey?: string; + /** Endpoint for `requiresBaseUrl` providers; blank reuses the persisted one. */ + baseUrl?: string; enabledModelIds: readonly string[]; models: readonly ModelInfo[]; } diff --git a/packages/cli/src/pi-tui-pickers.ts b/packages/cli/src/pi-tui-pickers.ts index d4ad9867b6..c636bef086 100644 --- a/packages/cli/src/pi-tui-pickers.ts +++ b/packages/cli/src/pi-tui-pickers.ts @@ -841,7 +841,7 @@ function padLine(text: string, width: number): string { return `${trimmed}${' '.repeat(Math.max(0, safeWidth - visibleWidth(trimmed)))}`; } -export type OnboardingWizardPhase = 'search' | 'key' | 'models' | 'success'; +export type OnboardingWizardPhase = 'search' | 'baseUrl' | 'key' | 'models' | 'success'; export type OnboardingWizardStatus = | { kind: 'prompt' } @@ -853,6 +853,10 @@ export interface OnboardingWizardInput { providers: readonly OnboardingProviderEntry[]; /** search→key: the user picked a provider. The runner records it for verify/save. */ onPickProvider: (providerType: ProviderType) => void; + /** baseUrl submit (only for `requiresBaseUrl` providers). Empty means "reuse + * the existing connection's persisted endpoint"; the wizard has already + * rejected an empty value for a provider with no connection. */ + onSubmitBaseUrl: (baseUrl: string) => void; /** key submit. The value may be empty — an existing connection reuses the stored * secret, while a new required-key provider is rejected by verify. */ onSubmitKey: (apiKey: string) => void; @@ -882,6 +886,7 @@ export class OnboardingWizard implements Component { private picked: OnboardingProviderEntry | undefined; private status: OnboardingWizardStatus = { kind: 'prompt' }; private readonly searchEditor: Editor; + private readonly baseUrlEditor: Editor; private readonly keyEditor: Editor; private readonly modelsSearchEditor: Editor; private filtered: readonly OnboardingProviderEntry[]; @@ -907,6 +912,14 @@ export class OnboardingWizard implements Component { // place. SelectList has no setItems, so rebuild it; the next render picks // the new instance up. this.searchEditor.onChange = (text) => this.applyQuery(text); + this.baseUrlEditor = new Editor(tui, editorTheme(), { paddingX: 0 }); + // A fixed typo should not keep showing the old failure. + this.baseUrlEditor.onChange = () => { + if (this.phase === 'baseUrl' && this.status.kind === 'error') { + this.status = { kind: 'prompt' }; + } + }; + this.baseUrlEditor.onSubmit = (value) => this.submitBaseUrl(value); this.keyEditor = new Editor(tui, editorTheme(), { paddingX: 0 }); // Allow a blank submit: an existing connection reuses the stored secret; the // host's verify rejects a blank key for a new required-key provider. @@ -934,8 +947,11 @@ export class OnboardingWizard implements Component { private enterKeyPhase(provider: OnboardingProviderEntry): void { this.picked = provider; - this.phase = 'key'; + // A relay has no registry endpoint, so the wizard must collect one + // before the key — the deferred phase-2 step from #1254 (#3405). + this.phase = provider.requiresBaseUrl ? 'baseUrl' : 'key'; this.status = { kind: 'prompt' }; + this.baseUrlEditor.setText(''); this.keyEditor.setText(''); this.keyEditor.disableSubmit = false; this.searchEditor.setText(''); @@ -950,6 +966,45 @@ export class OnboardingWizard implements Component { this.input.onPickProvider(provider.providerType); } + private submitBaseUrl(value: string): void { + if (!this.picked || this.phase !== 'baseUrl') return; + const trimmed = value.trim(); + const error = this.validateBaseUrl(trimmed); + if (error) { + this.status = { kind: 'error', text: error }; + return; + } + this.input.onSubmitBaseUrl(trimmed); + this.phase = 'key'; + this.status = { kind: 'prompt' }; + } + + /** + * Mirrors the rules the Host's catalog normalizer enforces, so the common + * mistakes fail here with a readable message instead of surfacing as a + * protocol decode error after Enter on the key step. + */ + private validateBaseUrl(trimmed: string): string | null { + if (!trimmed) { + return this.picked?.hasConnection ? null : '需要填写 Base URL'; + } + let parsed: URL; + try { + parsed = new URL(trimmed); + } catch { + return 'Base URL 不是有效的 URL'; + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return 'Base URL 必须使用 http 或 https'; + } + if (parsed.username || parsed.password) return 'Base URL 不能包含账号密码'; + if (trimmed.includes('?') || trimmed.includes('#')) return 'Base URL 不能包含查询串或片段'; + if (new TextEncoder().encode(parsed.toString()).byteLength > 2_048) { + return 'Base URL 不能超过 2048 字节'; + } + return null; + } + private applyQuery(text: string): void { const query = text.trim().toLowerCase(); const next = query @@ -1034,15 +1089,23 @@ export class OnboardingWizard implements Component { invalidate(): void { this.searchEditor.invalidate(); + this.baseUrlEditor.invalidate(); this.keyEditor.invalidate(); this.modelsSearchEditor.invalidate(); this.list.invalidate(); } + /** Step label: relays have four steps (the base-URL one), the rest three. */ + private step(position: number): string { + return `${position}/${this.picked?.requiresBaseUrl ? 4 : 3}`; + } + handleInput(data: string): void { switch (this.phase) { case 'search': return this.handleSearchInput(data); + case 'baseUrl': + return this.handleBaseUrlInput(data); case 'key': return this.handleKeyInput(data); case 'models': @@ -1052,6 +1115,22 @@ export class OnboardingWizard implements Component { } } + private handleBaseUrlInput(data: string): void { + if (matchesKey(data, Key.ctrl('c'))) { + this.input.onCancel(); + return; + } + if (matchesKey(data, Key.escape)) { + this.phase = 'search'; + this.picked = undefined; + this.status = { kind: 'prompt' }; + this.baseUrlEditor.setText(''); + this.input.onBack(); + return; + } + this.baseUrlEditor.handleInput(data); + } + private handleSearchInput(data: string): void { if (matchesKey(data, Key.escape) || matchesKey(data, Key.ctrl('c'))) { this.input.onCancel(); @@ -1081,8 +1160,14 @@ export class OnboardingWizard implements Component { return; } if (matchesKey(data, Key.escape)) { - this.phase = 'search'; - this.picked = undefined; + // One level back: a relay returns to its base-URL step, everything + // else to the provider search. + if (this.picked?.requiresBaseUrl) { + this.phase = 'baseUrl'; + } else { + this.phase = 'search'; + this.picked = undefined; + } this.status = { kind: 'prompt' }; this.keyEditor.setText(''); this.keyEditor.disableSubmit = false; @@ -1179,6 +1264,8 @@ export class OnboardingWizard implements Component { switch (this.phase) { case 'search': return this.renderSearch(safeWidth); + case 'baseUrl': + return this.renderBaseUrl(safeWidth); case 'key': return this.renderKey(safeWidth); case 'models': @@ -1188,13 +1275,37 @@ export class OnboardingWizard implements Component { } } + private renderBaseUrl(width: number): string[] { + this.searchEditor.focused = false; + this.baseUrlEditor.focused = true; + this.keyEditor.focused = false; + this.modelsSearchEditor.focused = false; + const label = this.picked?.label ?? ''; + const hint = this.picked?.hasConnection + ? '留空复用已保存的 Base URL，或输入新地址替换 · Esc 返回选择服务商' + : '输入中转站的 Base URL（http/https）· Esc 返回选择服务商'; + return [ + padLine(`Set Up Provider ${ansi.dim(`· ${this.step(2)}`)} ${ansi.accent(label)}`, width), + padLine(ansi.dim(hint), width), + padLine('', width), + ...this.renderFieldRow(this.baseUrlEditor, 'Base URL', width), + padLine('', width), + padLine( + this.status.kind === 'error' ? ansi.red(`✗ ${this.status.text}`) : ansi.dim('Enter 继续'), + width, + ), + padLine(ansi.accent('-'.repeat(width)), width), + ]; + } + private renderSearch(width: number): string[] { this.searchEditor.focused = true; + this.baseUrlEditor.focused = false; this.keyEditor.focused = false; this.modelsSearchEditor.focused = false; return [ padLine( - `Set Up Provider ${ansi.dim('· 1/3')} ${ansi.accent(String(this.filtered.length))}`, + `Set Up Provider ${ansi.dim(`· ${this.step(1)}`)} ${ansi.accent(String(this.filtered.length))}`, width, ), padLine(ansi.dim('搜索服务商，↑↓ 选择 · Enter 确认 · Esc 取消'), width), @@ -1210,14 +1321,19 @@ export class OnboardingWizard implements Component { private renderKey(width: number): string[] { this.searchEditor.focused = false; + this.baseUrlEditor.focused = false; this.keyEditor.focused = this.status.kind === 'prompt' || this.status.kind === 'error'; this.modelsSearchEditor.focused = false; const label = this.picked?.label ?? ''; + const backTarget = this.picked?.requiresBaseUrl ? 'Esc 返回 Base URL' : 'Esc 返回选择服务商'; const hint = this.picked?.hasConnection - ? '留空复用已保存的 key，或输入新 key 轮换 · Esc 返回选择服务商' - : '输入 API key · 仅本机存储 · Esc 返回选择服务商'; + ? `留空复用已保存的 key，或输入新 key 轮换 · ${backTarget}` + : `输入 API key · 仅本机存储 · ${backTarget}`; return [ - padLine(`Set Up Provider ${ansi.dim('· 2/3')} ${ansi.accent(label)}`, width), + padLine( + `Set Up Provider ${ansi.dim(`· ${this.step(this.picked?.requiresBaseUrl ? 3 : 2)}`)} ${ansi.accent(label)}`, + width, + ), padLine(ansi.dim(hint), width), padLine('', width), ...this.renderFieldRow(this.keyEditor, 'API key', width), @@ -1242,11 +1358,15 @@ export class OnboardingWizard implements Component { private renderModels(width: number): string[] { this.searchEditor.focused = false; + this.baseUrlEditor.focused = false; this.keyEditor.focused = false; this.modelsSearchEditor.focused = this.status.kind !== 'saving'; const label = this.picked?.label ?? ''; const lines = [ - padLine(`Set Up Provider ${ansi.dim('· 3/3')} ${ansi.accent(label)}`, width), + padLine( + `Set Up Provider ${ansi.dim(`· ${this.step(this.picked?.requiresBaseUrl ? 4 : 3)}`)} ${ansi.accent(label)}`, + width, + ), padLine(ansi.dim('搜索模型，↑↓ 选择 · Space 切换 · Enter 保存 · Esc 返回'), width), padLine('', width), ...this.renderFieldRow(this.modelsSearchEditor, '搜索', width), @@ -1288,6 +1408,7 @@ export class OnboardingWizard implements Component { private renderSuccess(width: number): string[] { this.searchEditor.focused = false; + this.baseUrlEditor.focused = false; this.keyEditor.focused = false; this.modelsSearchEditor.focused = false; const label = this.picked?.label ?? ''; diff --git a/packages/cli/src/pi-tui-runner.ts b/packages/cli/src/pi-tui-runner.ts index 79c67bd6ed..e08224b88d 100644 --- a/packages/cli/src/pi-tui-runner.ts +++ b/packages/cli/src/pi-tui-runner.ts @@ -1082,6 +1082,8 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { // The runner holds them so the wizard stays UI-only; the secret never crosses // back into the wizard. let wizardApiKey = ''; + // The relay endpoint from the base-URL step ('' reuses the persisted one). + let wizardBaseUrl = ''; let wizardModels: readonly ModelInfo[] = []; // Authoritative ready model choices for `/model`. A startup snapshot refreshed // in place after `/setup` saves so newly configured models are immediately @@ -1848,6 +1850,7 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { wizard = undefined; wizardProviderType = undefined; wizardApiKey = ''; + wizardBaseUrl = ''; wizardModels = []; }; @@ -1872,7 +1875,7 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { const attempt = ++wizardAttempt; targetWizard.setVerifying(); requestRender(); - void input.onboarding.verify({ providerType, apiKey }).then( + void input.onboarding.verify({ providerType, apiKey, baseUrl: wizardBaseUrl }).then( (result) => { if (closed || wizard !== targetWizard || attempt !== wizardAttempt) return; if (result.kind === 'error') { @@ -1911,7 +1914,13 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { targetWizard.setSaving(); requestRender(); void input.onboarding - .save({ providerType, apiKey: wizardApiKey, enabledModelIds, models: wizardModels }) + .save({ + providerType, + apiKey: wizardApiKey, + baseUrl: wizardBaseUrl, + enabledModelIds, + models: wizardModels, + }) .then( (result) => { if (result.kind === 'error') { @@ -1982,10 +1991,15 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { onPickProvider: (providerType) => { wizardProviderType = providerType; wizardApiKey = ''; + wizardBaseUrl = ''; wizardModels = []; wizardAttempt += 1; // a new pick supersedes any in-flight attempt requestRender(); }, + onSubmitBaseUrl: (baseUrl) => { + wizardBaseUrl = baseUrl; + requestRender(); + }, onSubmitKey: submitWizardKey, onSubmitModels: submitWizardModels, onCancel: () => { diff --git a/packages/cli/src/runtime-host-onboarding.ts b/packages/cli/src/runtime-host-onboarding.ts index 45c26594e3..daca6c4e22 100644 --- a/packages/cli/src/runtime-host-onboarding.ts +++ b/packages/cli/src/runtime-host-onboarding.ts @@ -41,7 +41,8 @@ export function createRuntimeHostOnboardingSurface( try { const result = await connection.request('connection.onboarding.verify', { providerType: input.providerType, - apiKey: normalizedSecret(input.apiKey), + apiKey: trimmedOrNull(input.apiKey), + baseUrl: trimmedOrNull(input.baseUrl), }); return result.kind === 'verified' ? { kind: 'ok', models: [...result.models] } @@ -54,7 +55,8 @@ export function createRuntimeHostOnboardingSurface( try { const result = await connection.request('connection.onboarding.save', { providerType: input.providerType, - apiKey: normalizedSecret(input.apiKey), + apiKey: trimmedOrNull(input.apiKey), + baseUrl: trimmedOrNull(input.baseUrl), enabledModelIds: [...input.enabledModelIds], }); if (result.kind !== 'saved') { @@ -114,7 +116,7 @@ function projectProviders(catalog: ConnectionCatalogSnapshot): OnboardingProvide }); } -function normalizedSecret(value: string | undefined): string | null { +function trimmedOrNull(value: string | undefined): string | null { const secret = value?.trim() ?? ''; return secret.length === 0 ? null : secret; } @@ -128,6 +130,8 @@ function onboardingFailureText(input: { switch (input.reason) { case 'credential_not_configured': return 'API key is required'; + case 'base_url_not_configured': + return 'A base URL is required for this provider'; case 'provider_unsupported': return 'This provider does not support API-key onboarding'; case 'slug_conflict': diff --git a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts index 46c74659fe..d8cd7a3f60 100644 --- a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts @@ -62,7 +62,7 @@ test('verifies a first-run API key without persisting a connection or credential }); const result = await coordinator.handlers['connection.onboarding.verify']( - { providerType: 'openai', apiKey: 'first-run-secret' }, + { providerType: 'openai', apiKey: 'first-run-secret', baseUrl: null }, context, ); @@ -76,6 +76,59 @@ test('verifies a first-run API key without persisting a connection or credential }); }); +test('onboards a custom relay end to end: rejects a missing endpoint, discovers and persists a supplied one', async () => { + await withFixture(async ({ stores }) => { + let observedBaseUrl: string | undefined; + const coordinator = new HostConnectionEffectCoordinator({ + stores, + activation: new RuntimePolicyActivationGate(), + oauthCredentials: new HostOAuthExecutionAuthority(stores), + now: () => 123, + createTransport: () => recordingTransport(() => undefined), + runModelDiscovery: async (connection) => { + observedBaseUrl = connection.baseUrl; + return { ok: true, models: [{ id: 'relay/model' }] }; + }, + }); + + // A relay has no registry endpoint and no existing connection: nothing + // can answer discovery, so the attempt is rejected before any probe. + assert.deepEqual( + await coordinator.handlers['connection.onboarding.verify']( + { providerType: 'openai-compatible', apiKey: 'relay-secret', baseUrl: null }, + context, + ), + { ok: true, result: { kind: 'rejected', reason: 'base_url_not_configured' } }, + ); + assert.equal(observedBaseUrl, undefined); + + const saved = await coordinator.handlers['connection.onboarding.save']( + { + providerType: 'openai-compatible', + apiKey: 'relay-secret', + baseUrl: 'https://relay.example.test/v1', + enabledModelIds: ['relay/model'], + }, + context, + ); + assert.deepEqual(saved, { ok: true, result: { kind: 'saved' } }); + assert.equal(observedBaseUrl, 'https://relay.example.test/v1'); + + const connection = (await stores.connectionCatalog.getSnapshot()).connections.find( + ({ slug }) => slug === 'openai-compatible', + ); + assert.equal(connection?.baseUrl, 'https://relay.example.test/v1'); + // Re-verifying with a blank endpoint now reuses the persisted one. + assert.deepEqual( + await coordinator.handlers['connection.onboarding.verify']( + { providerType: 'openai-compatible', apiKey: '', baseUrl: null }, + context, + ), + { ok: true, result: { kind: 'verified', models: [{ id: 'relay/model' }] } }, + ); + }); +}); + test('saves a verified first-run target through the canonical Host authorities', async () => { await withFixture(async ({ stores }) => { const coordinator = new HostConnectionEffectCoordinator({ @@ -94,6 +147,7 @@ test('saves a verified first-run target through the canonical Host authorities', { providerType: 'openai', apiKey: 'first-run-secret', + baseUrl: null, enabledModelIds: ['second-model'], }, context, @@ -154,6 +208,7 @@ test('re-enables an existing connection without replacing another default target { providerType: 'openai', apiKey: null, + baseUrl: null, enabledModelIds: ['restored-model'], }, context, @@ -191,6 +246,7 @@ test('leaves canonical onboarding state unchanged when the durable intent cannot { providerType: 'openai', apiKey: 'new-secret', + baseUrl: null, enabledModelIds: ['new-model'], }, context, @@ -239,6 +295,7 @@ test('recovers a durable onboarding intent instead of rolling back a partial pub { providerType: 'openai', apiKey: 'new-secret', + baseUrl: null, enabledModelIds: ['new-model'], }, context, @@ -285,6 +342,7 @@ test('invalidates a verified result when onboarding rotates only the credential' { providerType: 'openai', apiKey: 'new-secret', + baseUrl: null, enabledModelIds: ['gpt-5'], }, context, @@ -328,6 +386,7 @@ test('onboarding keeps what its wizard never offered and prunes what it did', as { providerType: 'openai-compatible', apiKey: 'new-secret', + baseUrl: null, enabledModelIds: ['kept-model'], }, context, @@ -350,6 +409,27 @@ test('onboarding keeps what its wizard never offered and prunes what it did', as (await stores.connectionCatalog.getSnapshot()).connections.map(({ slug }) => slug), ['openai-compatible'], ); + + // Declarations are endpoint-keyed, like the update path enforces: a + // re-onboarding that swaps the relay URL must not carry the old relay's + // profile table onto the new one. + assert.deepEqual( + await coordinator.handlers['connection.onboarding.save']( + { + providerType: 'openai-compatible', + apiKey: '', + baseUrl: 'https://relay-b.example.test/v1', + enabledModelIds: ['kept-model'], + }, + context, + ), + { ok: true, result: { kind: 'saved' } }, + ); + const swapped = (await stores.connectionCatalog.getSnapshot()).connections.find( + ({ connectionId }) => connectionId === connection.connectionId, + ); + assert.equal(swapped?.baseUrl, 'https://relay-b.example.test/v1'); + assert.equal(swapped?.relayModelProfiles, undefined); }); }); @@ -385,6 +465,7 @@ test('onboarding drops a declaration for a model the wizard offered and the user { providerType: 'openai-compatible', apiKey: 'new-secret', + baseUrl: null, enabledModelIds: ['kept-model'], }, context, @@ -429,6 +510,7 @@ test('rejects an oversized final catalog before publishing a recovery intent', a { providerType: 'openai', apiKey: 'capacity-secret', + baseUrl: null, enabledModelIds: [discovered[0]!.id], }, context, diff --git a/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts b/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts index dc143e198a..e86a78bf82 100644 --- a/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts @@ -32,11 +32,13 @@ describe('Runtime Host connection effects protocol', () => { const verify = request('connection.onboarding.verify', { providerType: 'openrouter', apiKey: 'transient-secret', + baseUrl: null, }); const save = request('connection.onboarding.save', { - providerType: 'openrouter', + providerType: 'openai-compatible', apiKey: 'transient-secret', - enabledModelIds: ['openrouter/free'], + baseUrl: 'https://relay.example.test/v1', + enabledModelIds: ['relay/model'], }); assert.deepEqual(decodeClientFrame(verify), verify); assert.deepEqual(decodeClientFrame(save), save); @@ -59,8 +61,21 @@ describe('Runtime Host connection effects protocol', () => { assertInvalidRequest('connection.onboarding.save', { providerType: 'openrouter', apiKey: null, + baseUrl: null, enabledModelIds: [], }); + // The endpoint override goes through the shared catalog normalizer, so a + // non-http(s) or credentialed URL never reaches discovery. + assertInvalidRequest('connection.onboarding.verify', { + providerType: 'openai-compatible', + apiKey: 'transient-secret', + baseUrl: 'ftp://relay.example.test/v1', + }); + assertInvalidRequest('connection.onboarding.verify', { + providerType: 'openai-compatible', + apiKey: 'transient-secret', + baseUrl: 'https://user:pass@relay.example.test/v1', + }); assertInvalidResponse('connection.onboarding.verify', { kind: 'verified', models: [], diff --git a/packages/runtime-host/src/__tests__/protocol.test.ts b/packages/runtime-host/src/__tests__/protocol.test.ts index 146c199f07..7b0f47fe9d 100644 --- a/packages/runtime-host/src/__tests__/protocol.test.ts +++ b/packages/runtime-host/src/__tests__/protocol.test.ts @@ -164,6 +164,12 @@ describe('Runtime Host bootstrap protocol', () => { assert.ok(RUNTIME_HOST_COMPATIBILITY_EPOCH > 38); }); + test('publishes a new compatibility epoch for onboarding endpoint overrides', () => { + // Epoch 44 peers reject the required `baseUrl` on onboarding inputs and + // the `base_url_not_configured` rejection on its results. + assert.ok(RUNTIME_HOST_COMPATIBILITY_EPOCH > 44); + }); + test('adds credential rotation without changing existing credential inputs', () => { const issueInput = { principalKind: 'remote_owner', diff --git a/packages/runtime-host/src/protocol/connection-effects.ts b/packages/runtime-host/src/protocol/connection-effects.ts index f4cec26037..a6ec64a71d 100644 --- a/packages/runtime-host/src/protocol/connection-effects.ts +++ b/packages/runtime-host/src/protocol/connection-effects.ts @@ -24,6 +24,7 @@ import { decodeProviderType, decodeConnectionTestSummary, decodeConnectionVersionBasis, + normalizeCatalogConnectionBaseUrl, RuntimePolicyDomainDecodeError, type ConnectionVersionBasis, type ModelDiscoverySource, @@ -86,6 +87,12 @@ export interface ConnectionTestRunInput { export interface ConnectionOnboardingVerifyInput { readonly providerType: ProviderType; readonly apiKey: string | null; + /** + * Endpoint override for providers whose registry entry carries none (the + * custom relays). Always present on the wire, like `apiKey`: `null` means + * "use the registry default or the existing connection's persisted URL". + */ + readonly baseUrl: string | null; } export interface ConnectionOnboardingSaveInput extends ConnectionOnboardingVerifyInput { @@ -96,7 +103,11 @@ export type ConnectionOnboardingVerifyResult = | { readonly kind: 'verified'; readonly models: readonly ModelInfo[] } | { readonly kind: 'rejected'; - readonly reason: 'provider_unsupported' | 'credential_not_configured' | 'slug_conflict'; + readonly reason: + | 'provider_unsupported' + | 'credential_not_configured' + | 'base_url_not_configured' + | 'slug_conflict'; } | { readonly kind: 'failed'; readonly errorClass: ConnectionEffectFailureClass }; @@ -107,6 +118,7 @@ export type ConnectionOnboardingSaveResult = readonly reason: | 'provider_unsupported' | 'credential_not_configured' + | 'base_url_not_configured' | 'slug_conflict' | 'model_unavailable'; } @@ -217,11 +229,13 @@ export function decodeConnectionOnboardingSaveInput(value: unknown): ConnectionO const input = requireExactRecord(value, 'connection onboarding save input', [ 'providerType', 'apiKey', + 'baseUrl', 'enabledModelIds', ]); const verified = decodeConnectionOnboardingVerifyInput({ providerType: input.providerType, apiKey: input.apiKey, + baseUrl: input.baseUrl, }); if ( !Array.isArray(input.enabledModelIds) || @@ -262,6 +276,7 @@ export function decodeConnectionOnboardingSaveResult( rejected.kind !== 'rejected' || (rejected.reason !== 'provider_unsupported' && rejected.reason !== 'credential_not_configured' && + rejected.reason !== 'base_url_not_configured' && rejected.reason !== 'slug_conflict' && rejected.reason !== 'model_unavailable') ) { @@ -276,13 +291,24 @@ export function decodeConnectionOnboardingVerifyInput( const input = requireExactRecord(value, 'connection onboarding verification input', [ 'providerType', 'apiKey', + 'baseUrl', ]); + const providerType = decodeDomain(() => decodeProviderType(input.providerType)); return { - providerType: decodeDomain(() => decodeProviderType(input.providerType)), + providerType, apiKey: input.apiKey === null ? null : requireString(input.apiKey, 'connection onboarding API key', 64 * 1024), + // The shared catalog normalizer owns the URL rules (http/https, no + // credentials/query/fragment, 2048-byte cap) and collapses a value equal + // to the provider default back to null, so the wire never carries a + // redundant override. + baseUrl: + input.baseUrl === null + ? null + : (decodeDomain(() => normalizeCatalogConnectionBaseUrl(input.baseUrl, providerType)) ?? + null), }; } @@ -318,6 +344,7 @@ export function decodeConnectionOnboardingVerifyResult( rejected.kind !== 'rejected' || (rejected.reason !== 'provider_unsupported' && rejected.reason !== 'credential_not_configured' && + rejected.reason !== 'base_url_not_configured' && rejected.reason !== 'slug_conflict') ) { throw invalidProtocolFrame('Invalid connection onboarding rejection'); diff --git a/packages/runtime-host/src/protocol/index.ts b/packages/runtime-host/src/protocol/index.ts index eb63d7d0ab..48542fb081 100644 --- a/packages/runtime-host/src/protocol/index.ts +++ b/packages/runtime-host/src/protocol/index.ts @@ -91,7 +91,9 @@ export const RUNTIME_HOST_REGISTRATION_SCHEMA_VERSION = 1 as const; export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const; // Increment when the same protocol version no longer guarantees safe Client-Host // interoperability. Mismatches are rejected before domain commands are admitted. -export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 44 as const; +export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 45 as const; +// 45: Connection onboarding inputs require `baseUrl` and results can carry the +// `base_url_not_configured` rejection. Older peers reject both shapes. // 44: Session continuity and inspection stop carrying the retired Session // last-used timestamp. Older peers reject those strict projection shapes. // 43: Session tool-start events correlate hidden shell polls with `shellRunRef`. diff --git a/packages/runtime-host/src/server/connection-effect-coordinator.ts b/packages/runtime-host/src/server/connection-effect-coordinator.ts index 132a6f5317..dd9729f48f 100644 --- a/packages/runtime-host/src/server/connection-effect-coordinator.ts +++ b/packages/runtime-host/src/server/connection-effect-coordinator.ts @@ -230,17 +230,23 @@ export class HostConnectionEffectCoordinator { if (PROVIDER_DEFAULTS[input.providerType].authKind === 'api_key' && secret.length === 0) { return { kind: 'rejected', reason: 'credential_not_configured' }; } + // Mirrors the blank-key contract above: a null baseUrl reuses the + // existing connection's persisted endpoint or the registry default. + // A relay provider with no endpoint from any of those sources cannot + // run discovery — reject up front instead of probing an empty URL. + const base = candidate + ? { ...candidate, ...(input.baseUrl ? { baseUrl: input.baseUrl } : {}) } + : transientConnection(input.providerType, input.baseUrl); + if (!base.baseUrl && !PROVIDER_DEFAULTS[input.providerType].baseUrl) { + return { kind: 'rejected', reason: 'base_url_not_configured' }; + } const proxy = await this.#stores.operations.resolveNetworkProxyExecution(); if (proxy.kind !== 'ready') return { kind: 'failed', errorClass: 'network' }; const transport = this.#createTransport( toRuntimePolicyProxy(proxy.networkProxy, proxy.secretMaterial.networkProxy?.secret), ); try { - const effect = await this.#runModelDiscovery( - candidate ?? transientConnection(input.providerType), - secret, - { fetch: transport.fetch }, - ); + const effect = await this.#runModelDiscovery(base, secret, { fetch: transport.fetch }); if (!effect.ok || effect.models.length === 0) { return { kind: 'failed', @@ -265,6 +271,7 @@ export class HostConnectionEffectCoordinator { const committed = await this.#stores.operations.commitConnectionOnboarding({ providerType: input.providerType, suppliedSecret: prepared.suppliedSecret || null, + baseUrl: input.baseUrl, enabledModelIds: input.enabledModelIds, discovery: { models: prepared.models, @@ -439,7 +446,11 @@ type OnboardingDiscovery = } | { readonly kind: 'rejected'; - readonly reason: 'provider_unsupported' | 'credential_not_configured' | 'slug_conflict'; + readonly reason: + | 'provider_unsupported' + | 'credential_not_configured' + | 'base_url_not_configured' + | 'slug_conflict'; } | { readonly kind: 'failed'; readonly errorClass: ConnectionEffectFailureClass }; @@ -554,6 +565,7 @@ function operationFailure< function transientConnection( providerType: ConnectionOnboardingVerifyInput['providerType'], + baseUrl: string | null = null, ): ConnectionCatalogEntry { const definition = PROVIDER_DEFAULTS[providerType]; const models = definition.fallbackModels.map((id) => ({ id })); @@ -563,7 +575,7 @@ function transientConnection( slug: deriveConnectionSlug(providerType), name: definition.label, providerType, - ...(definition.baseUrl ? { baseUrl: definition.baseUrl } : {}), + ...((baseUrl ?? definition.baseUrl) ? { baseUrl: baseUrl ?? definition.baseUrl } : {}), enabled: true, enabledModelIds: models.map(({ id }) => id), models, diff --git a/packages/storage/src/__tests__/onboarding-transaction.test.ts b/packages/storage/src/__tests__/onboarding-transaction.test.ts new file mode 100644 index 0000000000..12c7f160ed --- /dev/null +++ b/packages/storage/src/__tests__/onboarding-transaction.test.ts @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { after, test } from 'node:test'; +import { + readConnectionOnboardingIntent, + writeConnectionOnboardingIntent, + prepareConnectionOnboardingIntent, +} from '../runtime-policy/onboarding-transaction.js'; + +const roots: string[] = []; + +async function root(): Promise { + const directory = await mkdtemp(join(tmpdir(), 'maka-onboarding-intent-')); + roots.push(directory); + return directory; +} + +after(async () => { + await Promise.all(roots.map((directory) => rm(directory, { recursive: true, force: true }))); +}); + +const BASE = { + connectionId: '00000000-0000-4000-8000-000000000001', + providerType: 'openai-compatible', + suppliedSecret: 'relay-secret', + enabledModelIds: ['relay/model'], + discovery: { models: [{ id: 'relay/model' }], source: 'fetched', fetchedAt: 123 }, + invalidateLastTest: false, +}; + +test('an onboarding intent round-trips its endpoint override through the journal', async () => { + const directory = await root(); + const intent = prepareConnectionOnboardingIntent({ + ...BASE, + baseUrl: 'https://relay.example.test/v1', + }); + await writeConnectionOnboardingIntent(directory, intent); + assert.deepEqual(await readConnectionOnboardingIntent(directory), intent); +}); + +test('a journal written before the baseUrl field replays as no override', async () => { + const directory = await root(); + // The exact persisted shape an older build leaves behind on crash: no + // `baseUrl` key at all. Recovery must replay it, not reject the document. + await writeFile( + join(directory, 'runtime-policy-onboarding.json'), + JSON.stringify({ schemaVersion: 1, ...BASE }), + ); + const replayed = await readConnectionOnboardingIntent(directory); + assert.equal(replayed?.baseUrl, null); + assert.deepEqual(replayed?.enabledModelIds, ['relay/model']); +}); diff --git a/packages/storage/src/runtime-policy/connection-catalog-document.ts b/packages/storage/src/runtime-policy/connection-catalog-document.ts index add792364b..d802d4fc4d 100644 --- a/packages/storage/src/runtime-policy/connection-catalog-document.ts +++ b/packages/storage/src/runtime-policy/connection-catalog-document.ts @@ -517,6 +517,7 @@ export class ConnectionCatalogDocumentOwner { current: ConnectionCatalogDocument, rawConnectionId: string, rawProviderType: unknown, + rawBaseUrl: string | null, rawEnabledModelIds: readonly string[], rawResult: ConnectionModelDiscoveryResult, invalidateLastTest: boolean, @@ -550,13 +551,14 @@ export class ConnectionCatalogDocumentOwner { 'Onboarding requires a non-empty model inventory', ); } + // A supplied endpoint replaces the previous one; null preserves the + // existing override or the registry default (blank-reuse, like the key). + const effectiveBaseUrl = rawBaseUrl ?? previous?.baseUrl ?? definition.baseUrl; const changes = decodeConnectionInput(() => normalizeConnectionCatalogEntryUpdateForProvider( { name: previous?.name ?? definition.label, - ...((previous?.baseUrl ?? definition.baseUrl) - ? { baseUrl: previous?.baseUrl ?? definition.baseUrl } - : {}), + ...(effectiveBaseUrl ? { baseUrl: effectiveBaseUrl } : {}), enabled: true, enabledModelIds: rawEnabledModelIds, }, @@ -574,13 +576,18 @@ export class ConnectionCatalogDocumentOwner { (modelId) => !offered.has(modelId) && !changes.enabledModelIds.includes(modelId), ); const enabledModelIds = [...changes.enabledModelIds, ...undisplayed]; + // The endpoint keys the profile table and the last test the same way it + // does on the update path: declarations describe the relay that made + // them, so a swapped URL must not inherit either. + const endpointChanged = previous !== undefined && previous.baseUrl !== changes.baseUrl; // Onboarding installs a new enabledModelIds authority, so a profile keyed // by a model it dropped would violate the subset invariant. Like the // refresh path above, this one bypasses the canonical decoder, so pruning // has to happen here or the document is un-loadable on next read. - const relayModelProfiles = previous - ? pruneRelayModelProfiles(previous.relayModelProfiles, enabledModelIds) - : undefined; + const relayModelProfiles = + previous && !endpointChanged + ? pruneRelayModelProfiles(previous.relayModelProfiles, enabledModelIds) + : undefined; const base: ConnectionCatalogEntry = previous ?? { connectionId, revision: 0, @@ -595,6 +602,7 @@ export class ConnectionCatalogDocumentOwner { const finalized: ConnectionCatalogEntry = { ...baseWithoutProfiles, ...(relayModelProfiles ? { relayModelProfiles } : {}), + ...(changes.baseUrl !== undefined ? { baseUrl: changes.baseUrl } : {}), revision: previous ? nextRevision(previous.revision) : 1, enabled: true, enabledModelIds, @@ -609,6 +617,7 @@ export class ConnectionCatalogDocumentOwner { }; if ( previous?.enabled && + (changes.baseUrl === undefined || changes.baseUrl === previous.baseUrl) && sameStringArray(previous.enabledModelIds, enabledModelIds) && isDeepStrictEqual(previous.models, result.models) && previous.modelSource === result.source && @@ -619,7 +628,8 @@ export class ConnectionCatalogDocumentOwner { return { kind: 'ready', document: current, changed: false }; } const testBasisChanged = previous - ? !sameConnectionTestModelBasis( + ? endpointChanged || + !sameConnectionTestModelBasis( connectionTestModelBasis(previous), connectionTestModelBasis(finalized), ) diff --git a/packages/storage/src/runtime-policy/coordinator.ts b/packages/storage/src/runtime-policy/coordinator.ts index f0be29e378..18013a2df8 100644 --- a/packages/storage/src/runtime-policy/coordinator.ts +++ b/packages/storage/src/runtime-policy/coordinator.ts @@ -972,6 +972,7 @@ export class RuntimePolicyCoordinator { catalog, intent.connectionId, intent.providerType, + intent.baseUrl, intent.enabledModelIds, intent.discovery, intent.invalidateLastTest, @@ -1373,6 +1374,7 @@ export class RuntimePolicyCoordinator { catalog, intent.connectionId, intent.providerType, + intent.baseUrl, intent.enabledModelIds, intent.discovery, intent.invalidateLastTest, diff --git a/packages/storage/src/runtime-policy/onboarding-transaction.ts b/packages/storage/src/runtime-policy/onboarding-transaction.ts index ac864cfb90..d6f94c0429 100644 --- a/packages/storage/src/runtime-policy/onboarding-transaction.ts +++ b/packages/storage/src/runtime-policy/onboarding-transaction.ts @@ -22,6 +22,7 @@ import { join } from 'node:path'; import { decodeProviderType, decodeRuntimePolicyEntityId, + normalizeCatalogConnectionBaseUrl, normalizeConnectionCatalogEntryUpdateForProvider, normalizeConnectionModelDiscoveryResult, normalizeCredentialSecret, @@ -51,6 +52,7 @@ export interface ConnectionOnboardingTransactionInput { readonly connectionId: unknown; readonly providerType: unknown; readonly suppliedSecret: unknown; + readonly baseUrl: unknown; readonly enabledModelIds: unknown; readonly discovery: unknown; readonly invalidateLastTest: unknown; @@ -61,6 +63,7 @@ export interface ConnectionOnboardingIntent { readonly connectionId: string; readonly providerType: ProviderType; readonly suppliedSecret: string | null; + readonly baseUrl: string | null; readonly enabledModelIds: readonly string[]; readonly discovery: ConnectionModelDiscoveryResult; readonly invalidateLastTest: boolean; @@ -90,11 +93,17 @@ export function prepareConnectionOnboardingIntent( 'Onboarding requires a non-empty model inventory', ); } + // Legacy intents predate the field (`undefined` when replayed) and mean + // the same thing as an explicit null: no endpoint override. + const baseUrl = + input.baseUrl === null || input.baseUrl === undefined + ? null + : (decode(() => normalizeCatalogConnectionBaseUrl(input.baseUrl, providerType)) ?? null); const normalized = decode(() => normalizeConnectionCatalogEntryUpdateForProvider( { name: definition.label, - ...(definition.baseUrl ? { baseUrl: definition.baseUrl } : {}), + ...((baseUrl ?? definition.baseUrl) ? { baseUrl: baseUrl ?? definition.baseUrl } : {}), enabled: true, enabledModelIds: input.enabledModelIds, }, @@ -128,6 +137,7 @@ export function prepareConnectionOnboardingIntent( connectionId: decode(() => decodeRuntimePolicyEntityId(input.connectionId)), providerType, suppliedSecret, + baseUrl, enabledModelIds: normalized.enabledModelIds, discovery, invalidateLastTest: input.invalidateLastTest, @@ -139,15 +149,32 @@ export async function readConnectionOnboardingIntent( ): Promise { const value = await readBoundedJsonDocument(root, FILE, MAX_BYTES); if (value === undefined) return undefined; - const raw = record(value, FILE, 'invalid_document', [ - 'schemaVersion', - 'connectionId', - 'providerType', - 'suppliedSecret', - 'enabledModelIds', - 'discovery', - 'invalidateLastTest', - ]); + // `baseUrl` is allowed but not required: an intent journaled by a build + // that predates the field must still replay. + const raw = record( + value, + FILE, + 'invalid_document', + [ + 'schemaVersion', + 'connectionId', + 'providerType', + 'suppliedSecret', + 'baseUrl', + 'enabledModelIds', + 'discovery', + 'invalidateLastTest', + ], + [ + 'schemaVersion', + 'connectionId', + 'providerType', + 'suppliedSecret', + 'enabledModelIds', + 'discovery', + 'invalidateLastTest', + ], + ); if (raw.schemaVersion !== SCHEMA_VERSION) { throw codecError('invalid_document', `${FILE} has an unsupported schema version`); } @@ -156,6 +183,7 @@ export async function readConnectionOnboardingIntent( providerType: raw.providerType, connectionId: raw.connectionId, suppliedSecret: raw.suppliedSecret, + baseUrl: raw.baseUrl, enabledModelIds: raw.enabledModelIds, discovery: raw.discovery, invalidateLastTest: raw.invalidateLastTest, diff --git a/packages/storage/src/runtime-policy/operations.ts b/packages/storage/src/runtime-policy/operations.ts index 2f7074cb65..352b57dd3a 100644 --- a/packages/storage/src/runtime-policy/operations.ts +++ b/packages/storage/src/runtime-policy/operations.ts @@ -216,6 +216,8 @@ export type ConnectionEffectCompletionResult = export interface CommitConnectionOnboardingInput { readonly providerType: ConnectionCatalogEntry['providerType']; readonly suppliedSecret: string | null; + /** Endpoint override; null keeps the existing entry's persisted URL or the registry default. */ + readonly baseUrl: string | null; readonly enabledModelIds: readonly string[]; readonly discovery: ConnectionModelDiscoveryResult; } From c378e56d4cefb7354d9b22827d400443f120cca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=86=E9=80=8A?= <72533078+UncertaintyDeterminesYou4ndMe@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:30:13 +0800 Subject: [PATCH 2/4] fix(cli): carry connection identity through onboarding so custom-slug relays edit in place The wizard recognized an existing connection only at the derived canonical slug, so a relay created in Desktop under a custom slug read as unconfigured in /setup and saving created a second canonical-slug connection, leaving the old credential and default target behind. Onboarding inputs now carry 'connectionId: string | null': the catalog projection resolves the existing connection (canonical slug first, else the provider type's sole connection), the wizard threads its identity through verify/save, the coordinator targets it directly (rejecting a stale id as connection_not_found instead of duplicating), and the storage upsert finds the row by identity first, preserving its slug. A stale-snapshot rejection renders without the retype-the-key framing. Epoch-37 builds from this PR's own review cycle require baseUrl but not connectionId, so the identity shape gets epoch 38 rather than reusing 37 for a second mutually-undecodable frame. Generated-by: Claude Code --- .../__tests__/runtime-host-onboarding.test.ts | 44 ++++++- packages/cli/src/pi-tui-contracts.ts | 15 ++- packages/cli/src/pi-tui-pickers.ts | 8 +- packages/cli/src/pi-tui-runner.ts | 52 ++++---- packages/cli/src/runtime-host-onboarding.ts | 36 +++++- .../connection-effect-coordinator.test.ts | 111 +++++++++++++++++- .../connection-effects-protocol.test.ts | 11 ++ .../src/__tests__/protocol.test.ts | 6 +- .../src/protocol/connection-effects.ts | 17 +++ packages/runtime-host/src/protocol/index.ts | 5 +- .../server/connection-effect-coordinator.ts | 35 ++++-- .../connection-catalog-document.ts | 15 ++- .../storage/src/runtime-policy/coordinator.ts | 21 +++- .../storage/src/runtime-policy/operations.ts | 10 +- 14 files changed, 333 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/__tests__/runtime-host-onboarding.test.ts b/packages/cli/src/__tests__/runtime-host-onboarding.test.ts index e277d14402..b1758babb3 100644 --- a/packages/cli/src/__tests__/runtime-host-onboarding.test.ts +++ b/packages/cli/src/__tests__/runtime-host-onboarding.test.ts @@ -20,7 +20,7 @@ import { describe, test } from 'node:test'; import assert from 'node:assert/strict'; import type { ConnectionCatalogSnapshot } from '@maka/core/runtime-policy'; -import { projectRuntimeHostModelChoices } from '../runtime-host-onboarding.js'; +import { projectProviders, projectRuntimeHostModelChoices } from '../runtime-host-onboarding.js'; function catalog(connections: ConnectionCatalogSnapshot['connections']): ConnectionCatalogSnapshot { return { revision: 1, defaultTarget: null, connections }; @@ -78,3 +78,45 @@ describe('projectRuntimeHostModelChoices', () => { assert.equal(choices[0]?.displayName, 'GPT-5 Mini'); }); }); + +describe('projectProviders', () => { + const relay = { + connectionId: 'relay-custom-id', + revision: 1, + slug: 'my-relay', + name: 'My Relay', + providerType: 'openai-compatible', + baseUrl: 'https://relay.example.test/v1', + enabled: true, + enabledModelIds: ['relay/model'], + models: [{ id: 'relay/model' }], + } as const; + + test('a Desktop-created relay under a custom slug reads as the existing connection', () => { + // Identity must survive the projection: a sole connection of the provider + // type is "the" one to edit even off the canonical slug, or saving would + // duplicate it there (#3467 review). + const entry = projectProviders(catalog([relay])).find( + ({ providerType }) => providerType === 'openai-compatible', + ); + assert.equal(entry?.hasConnection, true); + assert.equal(entry?.connectionId, 'relay-custom-id'); + assert.deepEqual(entry?.enabledModelIds, ['relay/model']); + }); + + test('several non-canonical connections resolve to none — the wizard offers a fresh setup', () => { + const entry = projectProviders( + catalog([relay, { ...relay, connectionId: 'relay-2-id', slug: 'my-relay-2' }]), + ).find(({ providerType }) => providerType === 'openai-compatible'); + assert.equal(entry?.hasConnection, false); + assert.equal(entry?.connectionId, undefined); + }); + + test('the canonical-slug connection wins over other connections of the type', () => { + const canonical = { ...relay, connectionId: 'canonical-id', slug: 'openai-compatible' }; + const entry = projectProviders(catalog([relay, canonical])).find( + ({ providerType }) => providerType === 'openai-compatible', + ); + assert.equal(entry?.connectionId, 'canonical-id'); + }); +}); diff --git a/packages/cli/src/pi-tui-contracts.ts b/packages/cli/src/pi-tui-contracts.ts index 12c16fd560..9e1124182a 100644 --- a/packages/cli/src/pi-tui-contracts.ts +++ b/packages/cli/src/pi-tui-contracts.ts @@ -52,11 +52,15 @@ export interface OnboardableProvider { export interface OnboardingProviderEntry extends OnboardableProvider { hasConnection: boolean; + /** The existing connection's identity, so saving edits it in place. */ + connectionId?: string; enabledModelIds: readonly string[]; } export interface OnboardingVerifyInput { providerType: ProviderType; + /** The existing connection this edit targets; absent creates/updates the canonical-slug one. */ + connectionId?: string; apiKey?: string; /** Endpoint for `requiresBaseUrl` providers; blank reuses the persisted one. */ baseUrl?: string; @@ -64,10 +68,19 @@ export interface OnboardingVerifyInput { export type OnboardingVerifyResult = | { kind: 'ok'; models: ModelInfo[] } - | { kind: 'error'; text: string }; + | { + kind: 'error'; + text: string; + /** The wizard's provider snapshot is outdated (e.g. the targeted + * connection is gone) — retyping the key cannot fix this, so the + * runner shows the text without its retype-the-key framing. */ + stale?: boolean; + }; export interface OnboardingSaveInput { providerType: ProviderType; + /** The existing connection this edit targets; absent creates/updates the canonical-slug one. */ + connectionId?: string; apiKey?: string; /** Endpoint for `requiresBaseUrl` providers; blank reuses the persisted one. */ baseUrl?: string; diff --git a/packages/cli/src/pi-tui-pickers.ts b/packages/cli/src/pi-tui-pickers.ts index c636bef086..18dae33449 100644 --- a/packages/cli/src/pi-tui-pickers.ts +++ b/packages/cli/src/pi-tui-pickers.ts @@ -851,8 +851,10 @@ export type OnboardingWizardStatus = export interface OnboardingWizardInput { providers: readonly OnboardingProviderEntry[]; - /** search→key: the user picked a provider. The runner records it for verify/save. */ - onPickProvider: (providerType: ProviderType) => void; + /** search→key: the user picked a provider. The runner records it — and the + * existing connection's identity, when the catalog resolved one — for + * verify/save, so saving edits that connection in place. */ + onPickProvider: (providerType: ProviderType, existingConnectionId: string | undefined) => void; /** baseUrl submit (only for `requiresBaseUrl` providers). Empty means "reuse * the existing connection's persisted endpoint"; the wizard has already * rejected an empty value for a provider with no connection. */ @@ -963,7 +965,7 @@ export class OnboardingWizard implements Component { this.modelHighlight = 0; this.modelScroll = 0; this.modelsSearchEditor.setText(''); - this.input.onPickProvider(provider.providerType); + this.input.onPickProvider(provider.providerType, provider.connectionId); } private submitBaseUrl(value: string): void { diff --git a/packages/cli/src/pi-tui-runner.ts b/packages/cli/src/pi-tui-runner.ts index e08224b88d..61d335b547 100644 --- a/packages/cli/src/pi-tui-runner.ts +++ b/packages/cli/src/pi-tui-runner.ts @@ -1084,6 +1084,9 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { let wizardApiKey = ''; // The relay endpoint from the base-URL step ('' reuses the persisted one). let wizardBaseUrl = ''; + // The existing connection the picked provider resolved to, so saving edits + // it in place (a Desktop-created relay may live under a custom slug). + let wizardConnectionId: string | undefined; let wizardModels: readonly ModelInfo[] = []; // Authoritative ready model choices for `/model`. A startup snapshot refreshed // in place after `/setup` saves so newly configured models are immediately @@ -1851,6 +1854,7 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { wizardProviderType = undefined; wizardApiKey = ''; wizardBaseUrl = ''; + wizardConnectionId = undefined; wizardModels = []; }; @@ -1875,26 +1879,32 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { const attempt = ++wizardAttempt; targetWizard.setVerifying(); requestRender(); - void input.onboarding.verify({ providerType, apiKey, baseUrl: wizardBaseUrl }).then( - (result) => { - if (closed || wizard !== targetWizard || attempt !== wizardAttempt) return; - if (result.kind === 'error') { - // Probe failed: re-arm the key field in place. The host stores nothing - // during verify, so retrying with a corrected key is clean. - wizard.setKeyError(`API key 验证失败：${result.text}。请检查后重新输入。`); + void input.onboarding + .verify({ providerType, connectionId: wizardConnectionId, apiKey, baseUrl: wizardBaseUrl }) + .then( + (result) => { + if (closed || wizard !== targetWizard || attempt !== wizardAttempt) return; + if (result.kind === 'error') { + // Probe failed: re-arm the key field in place. The host stores nothing + // during verify, so retrying with a corrected key is clean. + // A stale snapshot (the targeted connection is gone) is not a key + // problem — retyping cannot fix it, so skip that framing. + wizard.setKeyError( + result.stale ? result.text : `API key 验证失败：${result.text}。请检查后重新输入。`, + ); + requestRender(); + return; + } + wizardModels = result.models; + wizard.setModels(result.models); // advance to the models step requestRender(); - return; - } - wizardModels = result.models; - wizard.setModels(result.models); // advance to the models step - requestRender(); - }, - (error) => { - if (closed || wizard !== targetWizard || attempt !== wizardAttempt) return; - wizard.setKeyError(`配置失败：${error instanceof Error ? error.message : String(error)}`); - requestRender(); - }, - ); + }, + (error) => { + if (closed || wizard !== targetWizard || attempt !== wizardAttempt) return; + wizard.setKeyError(`配置失败：${error instanceof Error ? error.message : String(error)}`); + requestRender(); + }, + ); }; // Models submit from the wizard: persist the curated enabled set, refresh the @@ -1916,6 +1926,7 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { void input.onboarding .save({ providerType, + connectionId: wizardConnectionId, apiKey: wizardApiKey, baseUrl: wizardBaseUrl, enabledModelIds, @@ -1988,10 +1999,11 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { wizardOverlay?.hide(); wizard = new OnboardingWizard(tui, { providers, - onPickProvider: (providerType) => { + onPickProvider: (providerType, existingConnectionId) => { wizardProviderType = providerType; wizardApiKey = ''; wizardBaseUrl = ''; + wizardConnectionId = existingConnectionId; wizardModels = []; wizardAttempt += 1; // a new pick supersedes any in-flight attempt requestRender(); diff --git a/packages/cli/src/runtime-host-onboarding.ts b/packages/cli/src/runtime-host-onboarding.ts index daca6c4e22..94bbf91cd8 100644 --- a/packages/cli/src/runtime-host-onboarding.ts +++ b/packages/cli/src/runtime-host-onboarding.ts @@ -41,12 +41,18 @@ export function createRuntimeHostOnboardingSurface( try { const result = await connection.request('connection.onboarding.verify', { providerType: input.providerType, + connectionId: input.connectionId ?? null, apiKey: trimmedOrNull(input.apiKey), baseUrl: trimmedOrNull(input.baseUrl), }); - return result.kind === 'verified' - ? { kind: 'ok', models: [...result.models] } - : { kind: 'error', text: onboardingFailureText(result) }; + if (result.kind === 'verified') return { kind: 'ok', models: [...result.models] }; + return { + kind: 'error', + text: onboardingFailureText(result), + ...(result.kind === 'rejected' && result.reason === 'connection_not_found' + ? { stale: true } + : {}), + }; } catch (error) { return { kind: 'error', text: errorText(error) }; } @@ -55,6 +61,7 @@ export function createRuntimeHostOnboardingSurface( try { const result = await connection.request('connection.onboarding.save', { providerType: input.providerType, + connectionId: input.connectionId ?? null, apiKey: trimmedOrNull(input.apiKey), baseUrl: trimmedOrNull(input.baseUrl), enabledModelIds: [...input.enabledModelIds], @@ -103,14 +110,29 @@ export function projectRuntimeHostModelChoices(catalog: ConnectionCatalogSnapsho return choices; } -function projectProviders(catalog: ConnectionCatalogSnapshot): OnboardingProviderEntry[] { +export function projectProviders(catalog: ConnectionCatalogSnapshot): OnboardingProviderEntry[] { const bySlug = new Map(catalog.connections.map((connection) => [connection.slug, connection])); return listApiKeyOnboardableProviders().map((provider) => { - const candidate = bySlug.get(deriveConnectionSlug(provider.providerType)); - const existing = candidate?.providerType === provider.providerType ? candidate : undefined; + // Prefer the canonical-slug connection; failing that, a provider's sole + // connection is unambiguously "the" one to edit — a Desktop-created relay + // under a custom slug must read as configured here, or saving would + // duplicate it at the canonical slug. With several non-canonical + // connections there is no honest single answer, so the wizard offers a + // fresh canonical-slug setup. + const canonical = bySlug.get(deriveConnectionSlug(provider.providerType)); + const ofType = catalog.connections.filter( + (connection) => connection.providerType === provider.providerType, + ); + const existing = + canonical?.providerType === provider.providerType + ? canonical + : ofType.length === 1 + ? ofType[0] + : undefined; return { ...provider, hasConnection: existing !== undefined, + ...(existing ? { connectionId: existing.connectionId } : {}), enabledModelIds: existing ? [...existing.enabledModelIds] : [], }; }); @@ -132,6 +154,8 @@ function onboardingFailureText(input: { return 'API key is required'; case 'base_url_not_configured': return 'A base URL is required for this provider'; + case 'connection_not_found': + return 'The existing connection is gone — reopen /setup and try again'; case 'provider_unsupported': return 'This provider does not support API-key onboarding'; case 'slug_conflict': diff --git a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts index d8cd7a3f60..2d377ae2d0 100644 --- a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts @@ -62,7 +62,7 @@ test('verifies a first-run API key without persisting a connection or credential }); const result = await coordinator.handlers['connection.onboarding.verify']( - { providerType: 'openai', apiKey: 'first-run-secret', baseUrl: null }, + { providerType: 'openai', connectionId: null, apiKey: 'first-run-secret', baseUrl: null }, context, ); @@ -95,7 +95,12 @@ test('onboards a custom relay end to end: rejects a missing endpoint, discovers // can answer discovery, so the attempt is rejected before any probe. assert.deepEqual( await coordinator.handlers['connection.onboarding.verify']( - { providerType: 'openai-compatible', apiKey: 'relay-secret', baseUrl: null }, + { + providerType: 'openai-compatible', + connectionId: null, + apiKey: 'relay-secret', + baseUrl: null, + }, context, ), { ok: true, result: { kind: 'rejected', reason: 'base_url_not_configured' } }, @@ -106,6 +111,7 @@ test('onboards a custom relay end to end: rejects a missing endpoint, discovers { providerType: 'openai-compatible', apiKey: 'relay-secret', + connectionId: null, baseUrl: 'https://relay.example.test/v1', enabledModelIds: ['relay/model'], }, @@ -121,11 +127,101 @@ test('onboards a custom relay end to end: rejects a missing endpoint, discovers // Re-verifying with a blank endpoint now reuses the persisted one. assert.deepEqual( await coordinator.handlers['connection.onboarding.verify']( - { providerType: 'openai-compatible', apiKey: '', baseUrl: null }, + { providerType: 'openai-compatible', connectionId: null, apiKey: '', baseUrl: null }, + context, + ), + { ok: true, result: { kind: 'verified', models: [{ id: 'relay/model' }] } }, + ); + }); +}); + +test('re-onboarding by connection identity edits a Desktop custom-slug relay in place', async () => { + await withFixture(async ({ stores }) => { + // Desktop can create a relay under any slug; the wizard resolves that + // connection's identity and must edit it, not derive a second connection + // at the canonical slug (#3467 review). + const connection = await createConnection(stores, 0, { + ...connectionDraft('my-relay', 'openai-compatible'), + baseUrl: 'https://relay-a.example.test/v1', + enabledModelIds: ['relay/model'], + }); + await setConnectionCredential(stores, connection, 'old-secret'); + let observedBaseUrl: string | undefined; + let observedSecret: string | undefined; + const coordinator = new HostConnectionEffectCoordinator({ + stores, + activation: new RuntimePolicyActivationGate(), + oauthCredentials: new HostOAuthExecutionAuthority(stores), + now: () => 123, + createTransport: () => recordingTransport(() => undefined), + runModelDiscovery: async (target, secret) => { + observedBaseUrl = target.baseUrl; + observedSecret = secret; + return { ok: true, models: [{ id: 'relay/model' }] }; + }, + }); + + // A blank re-verify against the resolved identity reuses the stored + // secret and the persisted custom-slug endpoint. + assert.deepEqual( + await coordinator.handlers['connection.onboarding.verify']( + { + providerType: 'openai-compatible', + connectionId: connection.connectionId, + apiKey: '', + baseUrl: null, + }, context, ), { ok: true, result: { kind: 'verified', models: [{ id: 'relay/model' }] } }, ); + assert.equal(observedBaseUrl, 'https://relay-a.example.test/v1'); + assert.equal(observedSecret, 'old-secret'); + + assert.deepEqual( + await coordinator.handlers['connection.onboarding.save']( + { + providerType: 'openai-compatible', + connectionId: connection.connectionId, + apiKey: 'new-secret', + baseUrl: 'https://relay-b.example.test/v1', + enabledModelIds: ['relay/model'], + }, + context, + ), + { ok: true, result: { kind: 'saved' } }, + ); + const catalog = await stores.connectionCatalog.getSnapshot(); + // Edited in place: still exactly one connection, same identity, custom + // slug preserved, endpoint replaced. + assert.deepEqual( + catalog.connections.map(({ connectionId, slug, baseUrl }) => ({ + connectionId, + slug, + baseUrl, + })), + [ + { + connectionId: connection.connectionId, + slug: 'my-relay', + baseUrl: 'https://relay-b.example.test/v1', + }, + ], + ); + + // A stale identity is rejected instead of silently creating a duplicate. + assert.deepEqual( + await coordinator.handlers['connection.onboarding.verify']( + { + providerType: 'openai-compatible', + connectionId: '00000000-0000-4000-8000-00000000dead', + apiKey: 'x', + baseUrl: null, + }, + context, + ), + { ok: true, result: { kind: 'rejected', reason: 'connection_not_found' } }, + ); }); }); @@ -147,6 +243,7 @@ test('saves a verified first-run target through the canonical Host authorities', { providerType: 'openai', apiKey: 'first-run-secret', + connectionId: null, baseUrl: null, enabledModelIds: ['second-model'], }, @@ -208,6 +305,7 @@ test('re-enables an existing connection without replacing another default target { providerType: 'openai', apiKey: null, + connectionId: null, baseUrl: null, enabledModelIds: ['restored-model'], }, @@ -246,6 +344,7 @@ test('leaves canonical onboarding state unchanged when the durable intent cannot { providerType: 'openai', apiKey: 'new-secret', + connectionId: null, baseUrl: null, enabledModelIds: ['new-model'], }, @@ -295,6 +394,7 @@ test('recovers a durable onboarding intent instead of rolling back a partial pub { providerType: 'openai', apiKey: 'new-secret', + connectionId: null, baseUrl: null, enabledModelIds: ['new-model'], }, @@ -342,6 +442,7 @@ test('invalidates a verified result when onboarding rotates only the credential' { providerType: 'openai', apiKey: 'new-secret', + connectionId: null, baseUrl: null, enabledModelIds: ['gpt-5'], }, @@ -386,6 +487,7 @@ test('onboarding keeps what its wizard never offered and prunes what it did', as { providerType: 'openai-compatible', apiKey: 'new-secret', + connectionId: null, baseUrl: null, enabledModelIds: ['kept-model'], }, @@ -418,6 +520,7 @@ test('onboarding keeps what its wizard never offered and prunes what it did', as { providerType: 'openai-compatible', apiKey: '', + connectionId: null, baseUrl: 'https://relay-b.example.test/v1', enabledModelIds: ['kept-model'], }, @@ -465,6 +568,7 @@ test('onboarding drops a declaration for a model the wizard offered and the user { providerType: 'openai-compatible', apiKey: 'new-secret', + connectionId: null, baseUrl: null, enabledModelIds: ['kept-model'], }, @@ -510,6 +614,7 @@ test('rejects an oversized final catalog before publishing a recovery intent', a { providerType: 'openai', apiKey: 'capacity-secret', + connectionId: null, baseUrl: null, enabledModelIds: [discovered[0]!.id], }, diff --git a/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts b/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts index e86a78bf82..e7408347e3 100644 --- a/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts @@ -31,11 +31,13 @@ describe('Runtime Host connection effects protocol', () => { test('bounds transient onboarding secrets, models, and save selections', () => { const verify = request('connection.onboarding.verify', { providerType: 'openrouter', + connectionId: null, apiKey: 'transient-secret', baseUrl: null, }); const save = request('connection.onboarding.save', { providerType: 'openai-compatible', + connectionId: '00000000-0000-4000-8000-000000000002', apiKey: 'transient-secret', baseUrl: 'https://relay.example.test/v1', enabledModelIds: ['relay/model'], @@ -60,6 +62,7 @@ describe('Runtime Host connection effects protocol', () => { ); assertInvalidRequest('connection.onboarding.save', { providerType: 'openrouter', + connectionId: null, apiKey: null, baseUrl: null, enabledModelIds: [], @@ -68,14 +71,22 @@ describe('Runtime Host connection effects protocol', () => { // non-http(s) or credentialed URL never reaches discovery. assertInvalidRequest('connection.onboarding.verify', { providerType: 'openai-compatible', + connectionId: null, apiKey: 'transient-secret', baseUrl: 'ftp://relay.example.test/v1', }); assertInvalidRequest('connection.onboarding.verify', { providerType: 'openai-compatible', + connectionId: null, apiKey: 'transient-secret', baseUrl: 'https://user:pass@relay.example.test/v1', }); + assertInvalidRequest('connection.onboarding.verify', { + providerType: 'openai-compatible', + connectionId: 42, + apiKey: 'transient-secret', + baseUrl: null, + }); assertInvalidResponse('connection.onboarding.verify', { kind: 'verified', models: [], diff --git a/packages/runtime-host/src/__tests__/protocol.test.ts b/packages/runtime-host/src/__tests__/protocol.test.ts index 7b0f47fe9d..53ba601a36 100644 --- a/packages/runtime-host/src/__tests__/protocol.test.ts +++ b/packages/runtime-host/src/__tests__/protocol.test.ts @@ -165,8 +165,10 @@ describe('Runtime Host bootstrap protocol', () => { }); test('publishes a new compatibility epoch for onboarding endpoint overrides', () => { - // Epoch 44 peers reject the required `baseUrl` on onboarding inputs and - // the `base_url_not_configured` rejection on its results. + // Epoch 44 peers reject the required `baseUrl` and `connectionId` on + // onboarding inputs, and the `base_url_not_configured` / + // `connection_not_found` rejections on their results. Both landed in one + // epoch because neither shape was ever published separately. assert.ok(RUNTIME_HOST_COMPATIBILITY_EPOCH > 44); }); diff --git a/packages/runtime-host/src/protocol/connection-effects.ts b/packages/runtime-host/src/protocol/connection-effects.ts index a6ec64a71d..6357ca2bfe 100644 --- a/packages/runtime-host/src/protocol/connection-effects.ts +++ b/packages/runtime-host/src/protocol/connection-effects.ts @@ -86,6 +86,14 @@ export interface ConnectionTestRunInput { export interface ConnectionOnboardingVerifyInput { readonly providerType: ProviderType; + /** + * The existing connection this onboarding edits, when the client resolved + * one — connection identity stays authoritative instead of being re-derived + * from the provider type, so a relay created under a custom slug is updated + * in place rather than duplicated at the canonical slug. `null` targets the + * canonical-slug connection, creating it if absent. + */ + readonly connectionId: string | null; readonly apiKey: string | null; /** * Endpoint override for providers whose registry entry carries none (the @@ -105,6 +113,7 @@ export type ConnectionOnboardingVerifyResult = readonly kind: 'rejected'; readonly reason: | 'provider_unsupported' + | 'connection_not_found' | 'credential_not_configured' | 'base_url_not_configured' | 'slug_conflict'; @@ -117,6 +126,7 @@ export type ConnectionOnboardingSaveResult = readonly kind: 'rejected'; readonly reason: | 'provider_unsupported' + | 'connection_not_found' | 'credential_not_configured' | 'base_url_not_configured' | 'slug_conflict' @@ -228,12 +238,14 @@ export const CONNECTION_EFFECT_OPERATION_SPECS = { export function decodeConnectionOnboardingSaveInput(value: unknown): ConnectionOnboardingSaveInput { const input = requireExactRecord(value, 'connection onboarding save input', [ 'providerType', + 'connectionId', 'apiKey', 'baseUrl', 'enabledModelIds', ]); const verified = decodeConnectionOnboardingVerifyInput({ providerType: input.providerType, + connectionId: input.connectionId, apiKey: input.apiKey, baseUrl: input.baseUrl, }); @@ -275,6 +287,7 @@ export function decodeConnectionOnboardingSaveResult( if ( rejected.kind !== 'rejected' || (rejected.reason !== 'provider_unsupported' && + rejected.reason !== 'connection_not_found' && rejected.reason !== 'credential_not_configured' && rejected.reason !== 'base_url_not_configured' && rejected.reason !== 'slug_conflict' && @@ -290,12 +303,15 @@ export function decodeConnectionOnboardingVerifyInput( ): ConnectionOnboardingVerifyInput { const input = requireExactRecord(value, 'connection onboarding verification input', [ 'providerType', + 'connectionId', 'apiKey', 'baseUrl', ]); const providerType = decodeDomain(() => decodeProviderType(input.providerType)); return { providerType, + connectionId: + input.connectionId === null ? null : requireEntityId(input.connectionId, 'connectionId'), apiKey: input.apiKey === null ? null @@ -343,6 +359,7 @@ export function decodeConnectionOnboardingVerifyResult( if ( rejected.kind !== 'rejected' || (rejected.reason !== 'provider_unsupported' && + rejected.reason !== 'connection_not_found' && rejected.reason !== 'credential_not_configured' && rejected.reason !== 'base_url_not_configured' && rejected.reason !== 'slug_conflict') diff --git a/packages/runtime-host/src/protocol/index.ts b/packages/runtime-host/src/protocol/index.ts index 48542fb081..1a1b224749 100644 --- a/packages/runtime-host/src/protocol/index.ts +++ b/packages/runtime-host/src/protocol/index.ts @@ -92,8 +92,9 @@ export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const; // Increment when the same protocol version no longer guarantees safe Client-Host // interoperability. Mismatches are rejected before domain commands are admitted. export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 45 as const; -// 45: Connection onboarding inputs require `baseUrl` and results can carry the -// `base_url_not_configured` rejection. Older peers reject both shapes. +// 45: Connection onboarding inputs require `baseUrl` and `connectionId`, and +// results can carry the `base_url_not_configured` / `connection_not_found` +// rejections. Older peers reject all of these shapes. // 44: Session continuity and inspection stop carrying the retired Session // last-used timestamp. Older peers reject those strict projection shapes. // 43: Session tool-start events correlate hidden shell polls with `shellRunRef`. diff --git a/packages/runtime-host/src/server/connection-effect-coordinator.ts b/packages/runtime-host/src/server/connection-effect-coordinator.ts index dd9729f48f..0ff83822d6 100644 --- a/packages/runtime-host/src/server/connection-effect-coordinator.ts +++ b/packages/runtime-host/src/server/connection-effect-coordinator.ts @@ -186,8 +186,10 @@ export class HostConnectionEffectCoordinator { #verifyOnboarding( input: ConnectionOnboardingVerifyInput, ): Promise> { - const slug = deriveConnectionSlug(input.providerType); - return this.#admit(slug, 'connection.onboarding.verify', async () => { + // Same lane a models.fetch on the targeted connection would use; the + // derived slug only keys the create-at-canonical-slug flow. + const lane = input.connectionId ?? deriveConnectionSlug(input.providerType); + return this.#admit(lane, 'connection.onboarding.verify', async () => { const prepared = await this.#discoverOnboarding(input); return prepared.kind === 'ready' ? { kind: 'verified', models: prepared.models } : prepared; }); @@ -196,8 +198,8 @@ export class HostConnectionEffectCoordinator { #saveOnboarding( input: ConnectionOnboardingSaveInput, ): Promise> { - const slug = deriveConnectionSlug(input.providerType); - return this.#admit(slug, 'connection.onboarding.save', async () => { + const lane = input.connectionId ?? deriveConnectionSlug(input.providerType); + return this.#admit(lane, 'connection.onboarding.save', async () => { const prepared = await this.#discoverOnboarding(input); if (prepared.kind !== 'ready') return prepared; const available = new Set(prepared.models.map(({ id }) => id)); @@ -212,11 +214,23 @@ export class HostConnectionEffectCoordinator { if (!providerAuthSupportsApiKey(input.providerType)) { return { kind: 'rejected', reason: 'provider_unsupported' }; } - const slug = deriveConnectionSlug(input.providerType); const catalog = await this.#stores.connectionCatalog.getSnapshot(); - const candidate = catalog.connections.find((connection) => connection.slug === slug); - if (candidate && candidate.providerType !== input.providerType) { - return { kind: 'rejected', reason: 'slug_conflict' }; + let candidate: (typeof catalog.connections)[number] | undefined; + if (input.connectionId) { + // Identity supplied by the client: edit that connection in place — + // whatever slug it lives under — instead of deriving a second one. + candidate = catalog.connections.find( + (connection) => connection.connectionId === input.connectionId, + ); + if (!candidate || candidate.providerType !== input.providerType) { + return { kind: 'rejected', reason: 'connection_not_found' }; + } + } else { + const slug = deriveConnectionSlug(input.providerType); + candidate = catalog.connections.find((connection) => connection.slug === slug); + if (candidate && candidate.providerType !== input.providerType) { + return { kind: 'rejected', reason: 'slug_conflict' }; + } } const supplied = input.apiKey?.trim() ?? ''; const stored = candidate @@ -270,6 +284,7 @@ export class HostConnectionEffectCoordinator { try { const committed = await this.#stores.operations.commitConnectionOnboarding({ providerType: input.providerType, + connectionId: input.connectionId, suppliedSecret: prepared.suppliedSecret || null, baseUrl: input.baseUrl, enabledModelIds: input.enabledModelIds, @@ -282,6 +297,9 @@ export class HostConnectionEffectCoordinator { if (committed.kind === 'slug_conflict') { return { kind: 'rejected', reason: 'slug_conflict' }; } + if (committed.kind === 'target_missing') { + return { kind: 'rejected', reason: 'connection_not_found' }; + } if (committed.changed) this.#onCommittedMutation(); return { kind: 'saved' }; } catch (error) { @@ -448,6 +466,7 @@ type OnboardingDiscovery = readonly kind: 'rejected'; readonly reason: | 'provider_unsupported' + | 'connection_not_found' | 'credential_not_configured' | 'base_url_not_configured' | 'slug_conflict'; diff --git a/packages/storage/src/runtime-policy/connection-catalog-document.ts b/packages/storage/src/runtime-policy/connection-catalog-document.ts index d802d4fc4d..60245c940d 100644 --- a/packages/storage/src/runtime-policy/connection-catalog-document.ts +++ b/packages/storage/src/runtime-policy/connection-catalog-document.ts @@ -525,8 +525,18 @@ export class ConnectionCatalogDocumentOwner { const connectionId = decodeConnectionInput(() => decodeRuntimePolicyEntityId(rawConnectionId)); const providerType = decodeConnectionInput(() => decodeProviderType(rawProviderType)); const definition = PROVIDER_DEFAULTS[providerType]; - const slug = deriveConnectionSlug(providerType); - const index = current.connections.findIndex((connection) => connection.slug === slug); + // Identity first: the intent's connectionId names the connection being + // edited, whatever slug it lives under — a relay created in Desktop under + // a custom slug is updated in place, never duplicated at the canonical + // slug. Only a genuinely new connection lands at the derived slug. + let index = current.connections.findIndex( + (connection) => connection.connectionId === connectionId, + ); + if (index < 0) { + index = current.connections.findIndex( + (connection) => connection.slug === deriveConnectionSlug(providerType), + ); + } const previous = current.connections[index]; if (previous && previous.providerType !== providerType) { return { kind: 'slug_conflict' }; @@ -534,6 +544,7 @@ export class ConnectionCatalogDocumentOwner { if (previous && previous.connectionId !== connectionId) { throw codecError('invalid_document', 'Onboarding intent conflicts with the connection id'); } + const slug = previous?.slug ?? deriveConnectionSlug(providerType); if (!previous && current.connections.length >= CONNECTION_CATALOG_MAX_CONNECTIONS) { throw codecError( 'invalid_connection_input', diff --git a/packages/storage/src/runtime-policy/coordinator.ts b/packages/storage/src/runtime-policy/coordinator.ts index 18013a2df8..f275896787 100644 --- a/packages/storage/src/runtime-policy/coordinator.ts +++ b/packages/storage/src/runtime-policy/coordinator.ts @@ -931,10 +931,23 @@ export class RuntimePolicyCoordinator { ): Promise { return this.inLane(async (root) => { const catalog = await this.catalog.read(root); - const slug = deriveConnectionSlug(input.providerType); - const existing = catalog.connections.find((connection) => connection.slug === slug); - if (existing && existing.providerType !== input.providerType) { - return deepFreeze({ kind: 'slug_conflict' as const }); + let existing: (typeof catalog.connections)[number] | undefined; + if (input.connectionId) { + // Explicit target: edit that connection wherever its slug lives. It + // was resolved from a live snapshot, but this lane is the authority — + // a concurrent delete or provider change surfaces here. + existing = catalog.connections.find( + (connection) => connection.connectionId === input.connectionId, + ); + if (!existing || existing.providerType !== input.providerType) { + return deepFreeze({ kind: 'target_missing' as const }); + } + } else { + const slug = deriveConnectionSlug(input.providerType); + existing = catalog.connections.find((connection) => connection.slug === slug); + if (existing && existing.providerType !== input.providerType) { + return deepFreeze({ kind: 'slug_conflict' as const }); + } } const connectionId = existing?.connectionId ?? randomUUID(); let invalidateLastTest = false; diff --git a/packages/storage/src/runtime-policy/operations.ts b/packages/storage/src/runtime-policy/operations.ts index 352b57dd3a..4f60bb989c 100644 --- a/packages/storage/src/runtime-policy/operations.ts +++ b/packages/storage/src/runtime-policy/operations.ts @@ -215,6 +215,11 @@ export type ConnectionEffectCompletionResult = export interface CommitConnectionOnboardingInput { readonly providerType: ConnectionCatalogEntry['providerType']; + /** + * The existing connection to edit in place (any slug); null targets the + * canonical-slug connection, creating it when absent. + */ + readonly connectionId: string | null; readonly suppliedSecret: string | null; /** Endpoint override; null keeps the existing entry's persisted URL or the registry default. */ readonly baseUrl: string | null; @@ -228,7 +233,10 @@ export type CommitConnectionOnboardingResult = readonly snapshot: ConnectionCatalogSnapshot; readonly changed: boolean; } - | { readonly kind: 'slug_conflict' }; + | { readonly kind: 'slug_conflict' } + // The explicitly targeted connection no longer exists (or changed provider + // type) between the caller's snapshot and this commit. + | { readonly kind: 'target_missing' }; export type ResolveExecutionConnectionResult = | { readonly kind: 'not_found' } From bbe7af7702e00f97adcaa9ba38d91862bc1e96bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=86=E9=80=8A?= <72533078+UncertaintyDeterminesYou4ndMe@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:33:34 +0800 Subject: [PATCH 3/4] fix(runtime-host): bind onboarding discovery to the basis its commit revalidates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Model discovery ran outside the mutation lane and the final commit re-read latest state, so a concurrent supported policy update could persist relay B/key B with the inventory discovered from relay A/key A. Adopt the model-fetch ticket shape: beginConnectionOnboarding locates the target under the write lane and issues a one-shot WeakMap ticket whose basis pins the connection revision (covering the endpoint and every other catalog-visible property), the api-key credential status plus stored secret from one vault read, and the effective proxy with its credential — returning the pinned proxy so discovery runs through the egress the basis certifies rather than re-resolving it. complete revalidates that basis atomically before the durable intent is written: drift returns 'superseded' (a new save rejection, riding this PR's unpublished epoch), a vanished target keeps reporting connection_not_found, and no journal is written on either. Verify abandons its ticket (WeakMap-held); save begins its own. Regression test drives the reviewed race end to end: discovery paused on relay A/key A, endpoint moved and key rotated concurrently, the save supersedes with relay B/key B intact and relay A's inventory never persisted, and a retry commits cleanly. Generated-by: Claude Code --- packages/cli/src/runtime-host-onboarding.ts | 2 + .../connection-effect-coordinator.test.ts | 109 ++++++ .../connection-effects-protocol.test.ts | 7 + .../src/protocol/connection-effects.ts | 8 +- .../server/connection-effect-coordinator.ts | 82 ++-- packages/storage/src/runtime-policy-stores.ts | 7 +- .../storage/src/runtime-policy/coordinator.ts | 353 ++++++++++++++---- .../storage/src/runtime-policy/operations.ts | 58 ++- 8 files changed, 505 insertions(+), 121 deletions(-) diff --git a/packages/cli/src/runtime-host-onboarding.ts b/packages/cli/src/runtime-host-onboarding.ts index 94bbf91cd8..3673c277f2 100644 --- a/packages/cli/src/runtime-host-onboarding.ts +++ b/packages/cli/src/runtime-host-onboarding.ts @@ -156,6 +156,8 @@ function onboardingFailureText(input: { return 'A base URL is required for this provider'; case 'connection_not_found': return 'The existing connection is gone — reopen /setup and try again'; + case 'superseded': + return 'The connection changed while onboarding — reopen /setup and try again'; case 'provider_unsupported': return 'This provider does not support API-key onboarding'; case 'slug_conflict': diff --git a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts index 2d377ae2d0..03ef8c729f 100644 --- a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts @@ -225,6 +225,115 @@ test('re-onboarding by connection identity edits a Desktop custom-slug relay in }); }); +test('a save whose connection changed between discovery and commit is superseded, never mixed', async () => { + await withFixture(async ({ stores }) => { + // The #3467 review race: discovery observes relay A/key A, a supported + // concurrent policy update moves the connection to relay B/key B before + // the commit, and the save must NOT persist relay B with the model + // inventory relay A produced. + const connection = await createConnection(stores, 0, { + ...connectionDraft('openai-compatible', 'openai-compatible'), + baseUrl: 'https://relay-a.example.test/v1', + enabledModelIds: ['relay/original'], + }); + await setConnectionCredential(stores, connection, 'key-a'); + + let releaseDiscovery!: () => void; + const discoveryPaused = new Promise((resolve) => { + releaseDiscovery = resolve; + }); + let observeDiscovery!: (value: { baseUrl?: string; secret: string }) => void; + const discoveryObserved = new Promise<{ baseUrl?: string; secret: string }>((resolve) => { + observeDiscovery = resolve; + }); + const coordinator = new HostConnectionEffectCoordinator({ + stores, + activation: new RuntimePolicyActivationGate(), + oauthCredentials: new HostOAuthExecutionAuthority(stores), + now: () => 999, + createTransport: () => recordingTransport(() => undefined), + runModelDiscovery: async (target, secret) => { + observeDiscovery({ baseUrl: target.baseUrl, secret }); + await discoveryPaused; + return { ok: true, models: [{ id: 'model-from-relay-a' }] }; + }, + }); + + const saving = coordinator.handlers['connection.onboarding.save']( + { + providerType: 'openai-compatible', + connectionId: connection.connectionId, + apiKey: '', + baseUrl: null, + enabledModelIds: ['model-from-relay-a'], + }, + context, + ); + const observed = await discoveryObserved; + assert.equal(observed.baseUrl, 'https://relay-a.example.test/v1'); + assert.equal(observed.secret, 'key-a'); + + // Concurrent, fully supported policy update while discovery is in flight: + // move the endpoint and rotate the credential. + const moved = await stores.connectionCatalog.update({ + expected: { connectionId: connection.connectionId, revision: connection.revision }, + changes: { + name: connection.name, + baseUrl: 'https://relay-b.example.test/v1', + enabled: true, + enabledModelIds: connection.enabledModelIds, + }, + }); + assert.equal(moved.kind, 'committed'); + const keyA = await connectionCredentialStatus(stores, connection); + assert.equal(keyA.configured, true); + const rotated = await stores.credentialVault.set({ + locator: connectionCredential(connection), + expected: + keyA.configured === true + ? { credentialId: keyA.credentialId, revision: keyA.revision } + : null, + secret: 'key-b', + }); + assert.equal(rotated.kind, 'committed'); + + releaseDiscovery(); + assert.deepEqual(await saving, { + ok: true, + result: { kind: 'rejected', reason: 'superseded' }, + }); + + // Relay B and key B stand untouched; relay A's inventory never landed. + const after = (await stores.connectionCatalog.getSnapshot()).connections.find( + ({ connectionId }) => connectionId === connection.connectionId, + ); + assert.equal(after?.baseUrl, 'https://relay-b.example.test/v1'); + assert.equal( + after?.models.some(({ id }) => id === 'model-from-relay-a'), + false, + ); + assert.deepEqual(after?.enabledModelIds, ['relay/original']); + assert.equal( + (await stores.operations.exportCredentialMaterial(connectionCredential(connection)))?.secret, + 'key-b', + ); + + // A retry against the settled state discovers through relay B/key B and + // commits cleanly. + const retried = await coordinator.handlers['connection.onboarding.save']( + { + providerType: 'openai-compatible', + connectionId: connection.connectionId, + apiKey: '', + baseUrl: null, + enabledModelIds: ['model-from-relay-a'], + }, + context, + ); + assert.deepEqual(retried, { ok: true, result: { kind: 'saved' } }); + }); +}); + test('saves a verified first-run target through the canonical Host authorities', async () => { await withFixture(async ({ stores }) => { const coordinator = new HostConnectionEffectCoordinator({ diff --git a/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts b/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts index e7408347e3..ef3bd06012 100644 --- a/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effects-protocol.test.ts @@ -60,6 +60,13 @@ describe('Runtime Host connection effects protocol', () => { decodeHostFrame(response('connection.onboarding.save', { kind: 'saved' })), response('connection.onboarding.save', { kind: 'saved' }), ); + // A save whose discovery basis was concurrently changed is superseded. + assert.deepEqual( + decodeHostFrame( + response('connection.onboarding.save', { kind: 'rejected', reason: 'superseded' }), + ), + response('connection.onboarding.save', { kind: 'rejected', reason: 'superseded' }), + ); assertInvalidRequest('connection.onboarding.save', { providerType: 'openrouter', connectionId: null, diff --git a/packages/runtime-host/src/protocol/connection-effects.ts b/packages/runtime-host/src/protocol/connection-effects.ts index 6357ca2bfe..add65435a1 100644 --- a/packages/runtime-host/src/protocol/connection-effects.ts +++ b/packages/runtime-host/src/protocol/connection-effects.ts @@ -130,7 +130,10 @@ export type ConnectionOnboardingSaveResult = | 'credential_not_configured' | 'base_url_not_configured' | 'slug_conflict' - | 'model_unavailable'; + | 'model_unavailable' + // The connection changed between model discovery and the commit; the + // discovered inventory no longer describes it. Re-run the wizard. + | 'superseded'; } | { readonly kind: 'failed'; readonly errorClass: ConnectionEffectFailureClass }; @@ -291,7 +294,8 @@ export function decodeConnectionOnboardingSaveResult( rejected.reason !== 'credential_not_configured' && rejected.reason !== 'base_url_not_configured' && rejected.reason !== 'slug_conflict' && - rejected.reason !== 'model_unavailable') + rejected.reason !== 'model_unavailable' && + rejected.reason !== 'superseded') ) { throw invalidProtocolFrame('Invalid connection onboarding save rejection'); } diff --git a/packages/runtime-host/src/server/connection-effect-coordinator.ts b/packages/runtime-host/src/server/connection-effect-coordinator.ts index 0ff83822d6..530815b47f 100644 --- a/packages/runtime-host/src/server/connection-effect-coordinator.ts +++ b/packages/runtime-host/src/server/connection-effect-coordinator.ts @@ -50,6 +50,7 @@ import { type BeginConnectionTestResult, type BeginModelFetchResult, type ConnectionEffectCompletionResult, + type ConnectionOnboardingTicket, type RuntimePolicyStoresWriter, } from '@maka/storage/runtime-policy-stores'; import type { @@ -214,33 +215,26 @@ export class HostConnectionEffectCoordinator { if (!providerAuthSupportsApiKey(input.providerType)) { return { kind: 'rejected', reason: 'provider_unsupported' }; } - const catalog = await this.#stores.connectionCatalog.getSnapshot(); - let candidate: (typeof catalog.connections)[number] | undefined; - if (input.connectionId) { - // Identity supplied by the client: edit that connection in place — - // whatever slug it lives under — instead of deriving a second one. - candidate = catalog.connections.find( - (connection) => connection.connectionId === input.connectionId, - ); - if (!candidate || candidate.providerType !== input.providerType) { - return { kind: 'rejected', reason: 'connection_not_found' }; - } - } else { - const slug = deriveConnectionSlug(input.providerType); - candidate = catalog.connections.find((connection) => connection.slug === slug); - if (candidate && candidate.providerType !== input.providerType) { - return { kind: 'rejected', reason: 'slug_conflict' }; - } + // The begin/complete ticket pair binds this discovery to the connection + // revision, credential, and proxy it observed: a concurrent policy update + // between the remote probe and the commit supersedes the save instead of + // pairing the new endpoint with an inventory it never produced. Verify + // simply abandons its ticket (they are WeakMap-held one-shots). + const begun = await this.#stores.operations.beginConnectionOnboarding({ + providerType: input.providerType, + connectionId: input.connectionId, + }); + if (begun.kind === 'target_missing') { + // Identity supplied by the client names a connection that is gone or + // changed provider type: reject instead of deriving a duplicate. + return { kind: 'rejected', reason: 'connection_not_found' }; + } + if (begun.kind === 'slug_conflict') { + return { kind: 'rejected', reason: 'slug_conflict' }; } + const candidate = begun.connection ?? undefined; const supplied = input.apiKey?.trim() ?? ''; - const stored = candidate - ? await this.#stores.operations.exportCredentialMaterial({ - scope: 'connection', - connectionId: candidate.connectionId, - kind: 'api_key', - }) - : null; - const secret = supplied || stored?.secret || ''; + const secret = supplied || begun.storedSecret || ''; if (PROVIDER_DEFAULTS[input.providerType].authKind === 'api_key' && secret.length === 0) { return { kind: 'rejected', reason: 'credential_not_configured' }; } @@ -254,10 +248,12 @@ export class HostConnectionEffectCoordinator { if (!base.baseUrl && !PROVIDER_DEFAULTS[input.providerType].baseUrl) { return { kind: 'rejected', reason: 'base_url_not_configured' }; } - const proxy = await this.#stores.operations.resolveNetworkProxyExecution(); - if (proxy.kind !== 'ready') return { kind: 'failed', errorClass: 'network' }; + // The ticket's basis certifies this exact proxy, so discovery must use + // the pinned value rather than re-resolving it (a flip-and-restore + // between the two reads would otherwise slip past the basis check). + if (begun.proxyCredentialMissing) return { kind: 'failed', errorClass: 'network' }; const transport = this.#createTransport( - toRuntimePolicyProxy(proxy.networkProxy, proxy.secretMaterial.networkProxy?.secret), + toRuntimePolicyProxy(begun.networkProxy, begun.proxySecret ?? undefined), ); try { const effect = await this.#runModelDiscovery(base, secret, { fetch: transport.fetch }); @@ -269,6 +265,7 @@ export class HostConnectionEffectCoordinator { } return { kind: 'ready', + ticket: begun.ticket, suppliedSecret: supplied, models: effect.models, }; @@ -282,24 +279,30 @@ export class HostConnectionEffectCoordinator { prepared: Extract, ): Promise { try { - const committed = await this.#stores.operations.commitConnectionOnboarding({ - providerType: input.providerType, - connectionId: input.connectionId, - suppliedSecret: prepared.suppliedSecret || null, - baseUrl: input.baseUrl, - enabledModelIds: input.enabledModelIds, - discovery: { - models: prepared.models, - source: 'fetched', - fetchedAt: this.#now(), + const committed = await this.#stores.operations.completeConnectionOnboarding( + prepared.ticket, + { + providerType: input.providerType, + connectionId: input.connectionId, + suppliedSecret: prepared.suppliedSecret || null, + baseUrl: input.baseUrl, + enabledModelIds: input.enabledModelIds, + discovery: { + models: prepared.models, + source: 'fetched', + fetchedAt: this.#now(), + }, }, - }); + ); if (committed.kind === 'slug_conflict') { return { kind: 'rejected', reason: 'slug_conflict' }; } if (committed.kind === 'target_missing') { return { kind: 'rejected', reason: 'connection_not_found' }; } + if (committed.kind === 'superseded') { + return { kind: 'rejected', reason: 'superseded' }; + } if (committed.changed) this.#onCommittedMutation(); return { kind: 'saved' }; } catch (error) { @@ -459,6 +462,7 @@ type BeginConnectionTestReady = Extract coordinator.beginModelFetch(connectionId), completeModelFetch: (ticket, result) => coordinator.completeModelFetch(ticket, result), - commitConnectionOnboarding: (input) => coordinator.commitConnectionOnboarding(input), + beginConnectionOnboarding: (input) => coordinator.beginConnectionOnboarding(input), + completeConnectionOnboarding: (ticket, input) => + coordinator.completeConnectionOnboarding(ticket, input), beginConnectionTest: (connectionId, modelId) => coordinator.beginConnectionTest(connectionId, modelId), completeConnectionTest: (ticket, result) => diff --git a/packages/storage/src/runtime-policy/coordinator.ts b/packages/storage/src/runtime-policy/coordinator.ts index f275896787..aa47e4bf40 100644 --- a/packages/storage/src/runtime-policy/coordinator.ts +++ b/packages/storage/src/runtime-policy/coordinator.ts @@ -58,6 +58,7 @@ import { deriveConnectionSlug, effectiveBaseUrl, PROVIDER_DEFAULTS, + providerAuthSupportsApiKey, type ProviderType, } from '@maka/core/llm-connections'; import { deepFreeze } from './codec.js'; @@ -98,8 +99,11 @@ import { type CompareAndSetOAuthCredentialInput, type ConnectionEffectChangedDomain, type ConnectionEffectCompletionResult, + type BeginConnectionOnboardingInput, + type BeginConnectionOnboardingResult, type CommitConnectionOnboardingInput, type CommitConnectionOnboardingResult, + type ConnectionOnboardingTicket, type ConnectionTestTicket, type InteractiveOAuthLoginCompletionResult, type InteractiveOAuthLoginProvider, @@ -181,6 +185,27 @@ interface ConnectionTicketRecord { state: TicketState; } +/** + * What onboarding discovery observed. Unlike the model-fetch/test bases, the + * target may not exist yet (first-time creation at the canonical slug), and + * the connection revision stands in for every catalog-visible property of an + * existing target — a swapped endpoint bumps it. + */ +interface ConnectionOnboardingBasis { + readonly providerType: ProviderType; + readonly slug: string; + readonly target: { readonly connectionId: string; readonly revision: number } | null; + readonly credential: CredentialStatus | null; + readonly effectiveProxy: EffectiveProxyConfigurationBasis; + readonly proxyCredential: CredentialStatus | null; +} + +interface ConnectionOnboardingTicketRecord { + readonly kind: 'connection_onboarding'; + readonly basis: ConnectionOnboardingBasis; + state: TicketState; +} + interface InteractiveOAuthLoginTicketRecord { readonly kind: 'interactive_oauth_login'; readonly connectionBasis: ConnectionVersionBasis; @@ -189,7 +214,10 @@ interface InteractiveOAuthLoginTicketRecord { state: TicketState; } -type OperationTicketRecord = ConnectionTicketRecord | InteractiveOAuthLoginTicketRecord; +type OperationTicketRecord = + | ConnectionTicketRecord + | ConnectionOnboardingTicketRecord + | InteractiveOAuthLoginTicketRecord; export class RuntimePolicyCoordinator { private readonly lane: SerializedOperationLane; @@ -926,93 +954,234 @@ export class RuntimePolicyCoordinator { ); } - commitConnectionOnboarding( - input: CommitConnectionOnboardingInput, - ): Promise { + beginConnectionOnboarding( + input: BeginConnectionOnboardingInput, + ): Promise { return this.inLane(async (root) => { - const catalog = await this.catalog.read(root); - let existing: (typeof catalog.connections)[number] | undefined; - if (input.connectionId) { - // Explicit target: edit that connection wherever its slug lives. It - // was resolved from a live snapshot, but this lane is the authority — - // a concurrent delete or provider change surfaces here. - existing = catalog.connections.find( - (connection) => connection.connectionId === input.connectionId, + // Onboarding guards the api_key credential slot; a provider whose auth + // never uses one has no business here (the Host gates on the same + // predicate, this keeps the storage API honest on its own). + if (!providerAuthSupportsApiKey(input.providerType)) { + throw codecError( + 'invalid_connection_input', + 'Connection onboarding requires an API-key provider', ); - if (!existing || existing.providerType !== input.providerType) { - return deepFreeze({ kind: 'target_missing' as const }); - } - } else { - const slug = deriveConnectionSlug(input.providerType); - existing = catalog.connections.find((connection) => connection.slug === slug); - if (existing && existing.providerType !== input.providerType) { - return deepFreeze({ kind: 'slug_conflict' as const }); - } } - const connectionId = existing?.connectionId ?? randomUUID(); - let invalidateLastTest = false; - if (input.suppliedSecret !== null) { - const locator = { - scope: 'connection', - connectionId, - kind: 'api_key', - } as const; - const vault = await this.vault.read(root); - const credential = findCredential(vault, locator); - if (credential?.secret !== input.suppliedSecret) { - invalidateLastTest = true; - const prepared = this.vault.prepareSet(vault, { - locator, - expected: credential - ? { credentialId: credential.credentialId, revision: credential.revision } - : null, - secret: input.suppliedSecret, - }); - if (prepared.kind !== 'ready') { - throw codecError( - 'invalid_document', - `Onboarding credential preflight returned ${prepared.kind}`, - ); - } + const catalog = await this.catalog.read(root); + const located = locateOnboardingTarget(catalog, input.providerType, input.connectionId); + if (located.kind !== 'ready') return deepFreeze({ kind: located.kind }); + const existing = located.existing; + const policy = await this.policy.read(root); + const networkProxy = structuredClone(policy.policy.networkProxy); + const vault = await this.vault.read(root); + let credential: CredentialStatus | null = null; + let storedSecret: string | null = null; + if (existing) { + const locator = connectionCredentialLocator( + existing.connectionId, + PROVIDER_DEFAULTS[existing.providerType].authKind, + ); + if (locator) { + credential = credentialStatus(vault, locator); + storedSecret = findCredential(vault, locator)?.secret ?? null; } } - const intent = prepareConnectionOnboardingIntent({ - ...input, - connectionId, - invalidateLastTest, + // The proxy discovery will run through is pinned HERE, like + // beginModelFetch pins it — re-resolving it later would let an A→B→A + // proxy flip commit an inventory fetched through egress this basis + // never saw. + const proxyLocator = requiresNetworkProxyCredential(networkProxy) + ? networkProxyCredentialLocator() + : null; + const proxyCredential = proxyLocator ? credentialStatus(vault, proxyLocator) : null; + const proxySecret = proxyLocator + ? (findCredential(vault, proxyLocator)?.secret ?? null) + : null; + const ticket = Object.freeze(Object.create(null)) as object; + this.tickets.set(ticket, { + kind: 'connection_onboarding', + basis: { + providerType: input.providerType, + slug: deriveConnectionSlug(input.providerType), + target: existing + ? { connectionId: existing.connectionId, revision: existing.revision } + : null, + credential, + effectiveProxy: effectiveProxyConfigurationBasis(networkProxy), + proxyCredential, + }, + state: 'available', + }); + return deepFreeze({ + kind: 'ready' as const, + ticket: ticket as ConnectionOnboardingTicket, + connection: existing ? structuredClone(existing) : null, + storedSecret, + networkProxy, + proxySecret, + proxyCredentialMissing: proxyLocator !== null && proxySecret === null, }); - const catalogPreflight = this.catalog.prepareOnboardingUpsert( - catalog, - intent.connectionId, - intent.providerType, - intent.baseUrl, - intent.enabledModelIds, - intent.discovery, - intent.invalidateLastTest, + }); + } + + async completeConnectionOnboarding( + ticket: ConnectionOnboardingTicket, + input: CommitConnectionOnboardingInput, + ): Promise { + const record = ticket && typeof ticket === 'object' ? this.tickets.get(ticket) : undefined; + if (!record || record.kind !== 'connection_onboarding' || record.state !== 'available') { + throw codecError( + 'invalid_connection_input', + 'Expected an authentic available connection onboarding ticket', ); - if (catalogPreflight.kind === 'slug_conflict') { - return deepFreeze({ kind: 'slug_conflict' as const }); + } + record.state = 'in_flight'; + return this.completeClaimedTicket(record, () => + this.inLane(async (root) => { + const catalog = await this.catalog.read(root); + // Revalidate the discovery basis under the write lane: the committed + // inventory must describe the connection state it was discovered + // from, not whatever a concurrent policy update left behind. + const checked = await this.checkOnboardingBasis(root, catalog, record.basis); + if (checked.kind !== 'unchanged') { + return deepFreeze( + checked.kind === 'target_missing' + ? { kind: 'target_missing' as const } + : { kind: 'superseded' as const, changed: checked.changed }, + ); + } + return this.commitConnectionOnboardingInLane(root, catalog, input); + }), + ); + } + + private async checkOnboardingBasis( + root: string, + catalog: Awaited>, + basis: ConnectionOnboardingBasis, + ): Promise< + | { readonly kind: 'unchanged' } + | { readonly kind: 'target_missing' } + | { readonly kind: 'superseded'; readonly changed: ConnectionEffectChangedDomain[] } + > { + const changed: ConnectionEffectChangedDomain[] = []; + if (basis.target) { + const connection = findConnection(catalog, { connectionId: basis.target.connectionId }); + // A vanished target is its own answer — "the connection is gone" beats + // "the connection changed" — while a survived one is compared by + // revision, which covers every catalog-visible property, endpoint + // included. + if (!connection) return { kind: 'target_missing' }; + if (connection.revision !== basis.target.revision) { + changed.push('connection'); + } else if (basis.credential) { + const vault = await this.vault.read(root); + if ( + !sameCredentialStatus(credentialStatus(vault, basis.credential.locator), basis.credential) + ) { + changed.push('credential'); + } } - try { - await writeConnectionOnboardingIntent(root, intent); - } catch (error) { - if (isCommitOutcomeUnknown(error)) this.onboardingRecoveryRequired = true; - throw error; + } else if (catalog.connections.some((connection) => connection.slug === basis.slug)) { + // Discovery ran for a first-time creation; any connection that appeared + // at the canonical slug since supersedes it. + changed.push('connection'); + } + const policy = await this.policy.read(root); + if ( + !sameEffectiveProxyConfiguration( + effectiveProxyConfigurationBasis(policy.policy.networkProxy), + basis.effectiveProxy, + ) + ) { + changed.push('network_proxy'); + } + if (basis.proxyCredential) { + const vault = await this.vault.read(root); + if ( + !sameCredentialStatus( + credentialStatus(vault, basis.proxyCredential.locator), + basis.proxyCredential, + ) && + !changed.includes('credential') + ) { + changed.push('credential'); } - try { - const result = await this.applyConnectionOnboarding(root, intent); - await clearConnectionOnboardingIntent(root); - this.onboardingRecoveryRequired = false; - return deepFreeze({ kind: 'committed' as const, ...result }); - } catch (error) { - this.onboardingRecoveryRequired = true; - if (isCommitOutcomeUnknown(error)) throw error; - throw commitOutcomeUnknown( - 'Connection onboarding has a durable intent and must recover before retrying', - error, - ); + } + return changed.length > 0 ? { kind: 'superseded', changed } : { kind: 'unchanged' }; + } + + private async commitConnectionOnboardingInLane( + root: string, + catalog: Awaited>, + input: CommitConnectionOnboardingInput, + ): Promise { + const located = locateOnboardingTarget(catalog, input.providerType, input.connectionId); + if (located.kind !== 'ready') return deepFreeze({ kind: located.kind }); + const existing = located.existing; + const connectionId = existing?.connectionId ?? randomUUID(); + let invalidateLastTest = false; + if (input.suppliedSecret !== null) { + const locator = { + scope: 'connection', + connectionId, + kind: 'api_key', + } as const; + const vault = await this.vault.read(root); + const credential = findCredential(vault, locator); + if (credential?.secret !== input.suppliedSecret) { + invalidateLastTest = true; + const prepared = this.vault.prepareSet(vault, { + locator, + expected: credential + ? { credentialId: credential.credentialId, revision: credential.revision } + : null, + secret: input.suppliedSecret, + }); + if (prepared.kind !== 'ready') { + throw codecError( + 'invalid_document', + `Onboarding credential preflight returned ${prepared.kind}`, + ); + } } + } + const intent = prepareConnectionOnboardingIntent({ + ...input, + connectionId, + invalidateLastTest, }); + const catalogPreflight = this.catalog.prepareOnboardingUpsert( + catalog, + intent.connectionId, + intent.providerType, + intent.baseUrl, + intent.enabledModelIds, + intent.discovery, + intent.invalidateLastTest, + ); + if (catalogPreflight.kind === 'slug_conflict') { + return deepFreeze({ kind: 'slug_conflict' as const }); + } + try { + await writeConnectionOnboardingIntent(root, intent); + } catch (error) { + if (isCommitOutcomeUnknown(error)) this.onboardingRecoveryRequired = true; + throw error; + } + try { + const result = await this.applyConnectionOnboarding(root, intent); + await clearConnectionOnboardingIntent(root); + this.onboardingRecoveryRequired = false; + return deepFreeze({ kind: 'committed' as const, ...result }); + } catch (error) { + this.onboardingRecoveryRequired = true; + if (isCommitOutcomeUnknown(error)) throw error; + throw commitOutcomeUnknown( + 'Connection onboarding has a durable intent and must recover before retrying', + error, + ); + } } beginConnectionTest( @@ -1419,6 +1588,36 @@ function isObsoleteConnectionOnboardingIntent(error: unknown): boolean { ); } +/** + * The single target-location rule onboarding begin and commit share: an + * explicit connectionId names the connection to edit in place (any slug); + * null targets the canonical slug, creating there when free. + */ +function locateOnboardingTarget( + catalog: { readonly connections: readonly ConnectionCatalogEntry[] }, + providerType: ProviderType, + connectionId: string | null, +): + | { readonly kind: 'target_missing' } + | { readonly kind: 'slug_conflict' } + | { readonly kind: 'ready'; readonly existing: ConnectionCatalogEntry | undefined } { + if (connectionId) { + const existing = catalog.connections.find( + (connection) => connection.connectionId === connectionId, + ); + if (!existing || existing.providerType !== providerType) { + return { kind: 'target_missing' }; + } + return { kind: 'ready', existing }; + } + const slug = deriveConnectionSlug(providerType); + const existing = catalog.connections.find((connection) => connection.slug === slug); + if (existing && existing.providerType !== providerType) { + return { kind: 'slug_conflict' }; + } + return { kind: 'ready', existing }; +} + function commonSemanticConnectionBasis( prepared: PreparedConnectionMaterial, ): CommonSemanticConnectionBasis { diff --git a/packages/storage/src/runtime-policy/operations.ts b/packages/storage/src/runtime-policy/operations.ts index 4f60bb989c..e593795d2d 100644 --- a/packages/storage/src/runtime-policy/operations.ts +++ b/packages/storage/src/runtime-policy/operations.ts @@ -213,6 +213,49 @@ export type ConnectionEffectCompletionResult = readonly changed: readonly ConnectionEffectChangedDomain[]; }; +export interface ConnectionOnboardingTicket { + readonly [operationTicketBrand]: 'connection_onboarding'; +} + +export interface BeginConnectionOnboardingInput { + readonly providerType: ConnectionCatalogEntry['providerType']; + /** + * The existing connection to edit in place (any slug); null targets the + * canonical-slug connection, creating it when absent. + */ + readonly connectionId: string | null; +} + +/** + * Discovery-basis handoff for onboarding: `begin` snapshots the connection + * revision, credential status, and effective proxy the caller will discover + * against and issues a one-shot ticket; `complete` revalidates that exact + * basis under the write lane before committing, so a model inventory can + * never be persisted onto an endpoint or credential it was not discovered + * from (#3467 review). + */ +export type BeginConnectionOnboardingResult = + // The explicitly targeted connection does not exist or changed provider type. + | { readonly kind: 'target_missing' } + | { readonly kind: 'slug_conflict' } + | { + readonly kind: 'ready'; + readonly ticket: ConnectionOnboardingTicket; + /** The targeted connection, or null when onboarding creates one. */ + readonly connection: ConnectionCatalogEntry | null; + /** The target's stored API key, for blank-key reuse during discovery. */ + readonly storedSecret: string | null; + /** + * The proxy discovery must run through — pinned here, like + * beginModelFetch pins it, so the basis certifies the egress the + * inventory actually travelled. + */ + readonly networkProxy: RuntimePolicy['networkProxy']; + readonly proxySecret: string | null; + /** The proxy requires a credential the vault does not hold. */ + readonly proxyCredentialMissing: boolean; + }; + export interface CommitConnectionOnboardingInput { readonly providerType: ConnectionCatalogEntry['providerType']; /** @@ -236,7 +279,14 @@ export type CommitConnectionOnboardingResult = | { readonly kind: 'slug_conflict' } // The explicitly targeted connection no longer exists (or changed provider // type) between the caller's snapshot and this commit. - | { readonly kind: 'target_missing' }; + | { readonly kind: 'target_missing' } + // The discovery basis (connection revision, credential, or proxy) changed + // between begin and complete: committing would bind another endpoint or + // credential to a model inventory it never produced. + | { + readonly kind: 'superseded'; + readonly changed: readonly ConnectionEffectChangedDomain[]; + }; export type ResolveExecutionConnectionResult = | { readonly kind: 'not_found' } @@ -290,7 +340,11 @@ export interface RuntimePolicyOperationCoordinator { ticket: ModelFetchTicket, result: ConnectionModelDiscoveryResult, ): Promise; - commitConnectionOnboarding( + beginConnectionOnboarding( + input: BeginConnectionOnboardingInput, + ): Promise; + completeConnectionOnboarding( + ticket: ConnectionOnboardingTicket, input: CommitConnectionOnboardingInput, ): Promise; beginConnectionTest( From bc3d1e1d3603040ab7993e395a90cc6e30512094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=86=E9=80=8A?= <72533078+UncertaintyDeterminesYou4ndMe@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:11:43 +0800 Subject: [PATCH 4/4] fix(runtime-host): send onboarding discovery through the connection's request customization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The onboarding probe went out on the bare transport fetch, while the models path wraps it with the connection's custom request headers and body overlay — so a connection that authenticates through a custom header verified and fetched models fine but failed re-onboarding. beginConnectionOnboarding now pins the request-headers secret for the probe and adds its credential status to the ticket's basis, so a header rotation between discovery and commit supersedes the save the same way an endpoint or key change does. Generated-by: Claude Code --- .../connection-effect-coordinator.test.ts | 123 ++++++++++++++++++ .../server/connection-effect-coordinator.ts | 12 +- .../storage/src/runtime-policy/coordinator.ts | 57 +++++--- .../storage/src/runtime-policy/operations.ts | 5 + 4 files changed, 178 insertions(+), 19 deletions(-) diff --git a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts index 03ef8c729f..a9f10d071a 100644 --- a/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/connection-effect-coordinator.test.ts @@ -334,6 +334,129 @@ test('a save whose connection changed between discovery and commit is superseded }); }); +test('onboarding probes with the custom request headers the models path sends, and a header rotation supersedes', async () => { + await withFixture(async ({ stores }) => { + // A connection that authenticates through a custom header (plus a body + // overlay) must onboard with the same probe the models path sends — + // otherwise re-onboarding fails against the very provider that + // models.fetch reaches fine (#3467 review). + const headerSecret = 'header-secret-must-not-escape'; + const connection = await createConnection(stores, 0, { + ...connectionDraft('header-relay', 'openai-compatible'), + baseUrl: 'https://relay.example.test/v1', + enabledModelIds: ['relay/model'], + requestBodyOverlay: { tenant: 'acme' }, + }); + await setConnectionCredential(stores, connection, 'api-key'); + const headersLocator = { + scope: 'connection' as const, + connectionId: connection.connectionId, + kind: 'request_headers' as const, + }; + const headersSet = await stores.credentialVault.set({ + locator: headersLocator, + expected: null, + secret: JSON.stringify({ 'X-Relay-Auth': headerSecret }), + }); + assert.equal(headersSet.kind, 'committed'); + + const probes: Array<{ header: string | null; body: unknown }> = []; + let releaseDiscovery!: () => void; + const discoveryPaused = new Promise((resolve) => { + releaseDiscovery = resolve; + }); + let observeDiscovery!: () => void; + const discoveryObserved = new Promise((resolve) => { + observeDiscovery = resolve; + }); + let discoveryRuns = 0; + const coordinator = new HostConnectionEffectCoordinator({ + stores, + activation: new RuntimePolicyActivationGate(), + oauthCredentials: new HostOAuthExecutionAuthority(stores), + now: () => 456, + createTransport: () => ({ + fetch: (async (input, init) => { + const request = new Request(input, init); + probes.push({ + header: request.headers.get('x-relay-auth'), + body: JSON.parse(await request.text()), + }); + return new Response('{}', { status: 200 }); + }) as typeof globalThis.fetch, + close: async () => {}, + }), + runModelDiscovery: async (_target, _secret, options) => { + await options.fetch('https://relay.example.test/v1/models', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ probe: true }), + }); + discoveryRuns += 1; + if (discoveryRuns === 2) { + observeDiscovery(); + await discoveryPaused; + } + return { ok: true, models: [{ id: 'relay/model' }] }; + }, + }); + + const verified = await coordinator.handlers['connection.onboarding.verify']( + { + providerType: 'openai-compatible', + connectionId: connection.connectionId, + apiKey: '', + baseUrl: null, + }, + context, + ); + assert.equal(verified.ok, true); + assert.deepEqual(probes, [{ header: headerSecret, body: { probe: true, tenant: 'acme' } }]); + assertRedacted(verified, [headerSecret]); + + // Rotating the header credential while a save's discovery is in flight + // invalidates its basis: the committed inventory must describe what the + // connection would fetch, and that changed under the probe. + const saving = coordinator.handlers['connection.onboarding.save']( + { + providerType: 'openai-compatible', + connectionId: connection.connectionId, + apiKey: '', + baseUrl: null, + enabledModelIds: ['relay/model'], + }, + context, + ); + await discoveryObserved; + const headerStatus = await stores.credentialVault.getStatus(headersLocator); + assert.equal(headerStatus.kind === 'status' && headerStatus.status.configured, true); + if (headerStatus.kind !== 'status' || !headerStatus.status.configured) return; + const rotated = await stores.credentialVault.set({ + locator: headersLocator, + expected: { + credentialId: headerStatus.status.credentialId, + revision: headerStatus.status.revision, + }, + secret: JSON.stringify({ 'X-Relay-Auth': 'rotated-header-secret' }), + }); + assert.equal(rotated.kind, 'committed'); + // The rotation left the catalog row untouched, so this supersede can only + // come from the header credential joining the discovery basis. + const row = (await stores.connectionCatalog.getSnapshot()).connections.find( + ({ connectionId }) => connectionId === connection.connectionId, + ); + assert.equal(row?.revision, connection.revision); + + releaseDiscovery(); + assert.deepEqual(await saving, { + ok: true, + result: { kind: 'rejected', reason: 'superseded' }, + }); + // The save's own probe carried the same customization as the verify's. + assert.deepEqual(probes[1], probes[0]); + }); +}); + test('saves a verified first-run target through the canonical Host authorities', async () => { await withFixture(async ({ stores }) => { const coordinator = new HostConnectionEffectCoordinator({ diff --git a/packages/runtime-host/src/server/connection-effect-coordinator.ts b/packages/runtime-host/src/server/connection-effect-coordinator.ts index 530815b47f..84e591e2fd 100644 --- a/packages/runtime-host/src/server/connection-effect-coordinator.ts +++ b/packages/runtime-host/src/server/connection-effect-coordinator.ts @@ -256,7 +256,17 @@ export class HostConnectionEffectCoordinator { toRuntimePolicyProxy(begun.networkProxy, begun.proxySecret ?? undefined), ); try { - const effect = await this.#runModelDiscovery(base, secret, { fetch: transport.fetch }); + // The probe must go out the way the models path sends it (#withTransport): + // with the connection's custom request headers and body overlay, both + // pinned by the ticket whose basis the commit revalidates. + const effect = await this.#runModelDiscovery(base, secret, { + fetch: createRequestCustomizationFetch(transport.fetch, { + headers: begun.requestHeadersSecret + ? parseRequestHeaders(begun.requestHeadersSecret) + : {}, + bodyOverlay: base.requestBodyOverlay, + }), + }); if (!effect.ok || effect.models.length === 0) { return { kind: 'failed', diff --git a/packages/storage/src/runtime-policy/coordinator.ts b/packages/storage/src/runtime-policy/coordinator.ts index aa47e4bf40..017beb0921 100644 --- a/packages/storage/src/runtime-policy/coordinator.ts +++ b/packages/storage/src/runtime-policy/coordinator.ts @@ -196,6 +196,7 @@ interface ConnectionOnboardingBasis { readonly slug: string; readonly target: { readonly connectionId: string; readonly revision: number } | null; readonly credential: CredentialStatus | null; + readonly requestHeadersCredential: CredentialStatus | null; readonly effectiveProxy: EffectiveProxyConfigurationBasis; readonly proxyCredential: CredentialStatus | null; } @@ -976,6 +977,8 @@ export class RuntimePolicyCoordinator { const vault = await this.vault.read(root); let credential: CredentialStatus | null = null; let storedSecret: string | null = null; + let requestHeadersCredential: CredentialStatus | null = null; + let requestHeadersSecret: string | null = null; if (existing) { const locator = connectionCredentialLocator( existing.connectionId, @@ -985,6 +988,12 @@ export class RuntimePolicyCoordinator { credential = credentialStatus(vault, locator); storedSecret = findCredential(vault, locator)?.secret ?? null; } + // Discovery must probe with the same header customization the models + // path applies, so the secret is pinned for the probe and its status + // joins the basis the commit revalidates. + const headersLocator = connectionRequestHeadersLocator(existing.connectionId); + requestHeadersCredential = credentialStatus(vault, headersLocator); + requestHeadersSecret = findCredential(vault, headersLocator)?.secret ?? null; } // The proxy discovery will run through is pinned HERE, like // beginModelFetch pins it — re-resolving it later would let an A→B→A @@ -1007,6 +1016,7 @@ export class RuntimePolicyCoordinator { ? { connectionId: existing.connectionId, revision: existing.revision } : null, credential, + requestHeadersCredential, effectiveProxy: effectiveProxyConfigurationBasis(networkProxy), proxyCredential, }, @@ -1017,6 +1027,7 @@ export class RuntimePolicyCoordinator { ticket: ticket as ConnectionOnboardingTicket, connection: existing ? structuredClone(existing) : null, storedSecret, + requestHeadersSecret, networkProxy, proxySecret, proxyCredentialMissing: proxyLocator !== null && proxySecret === null, @@ -1065,6 +1076,8 @@ export class RuntimePolicyCoordinator { | { readonly kind: 'superseded'; readonly changed: ConnectionEffectChangedDomain[] } > { const changed: ConnectionEffectChangedDomain[] = []; + // One vault read serves every credential-status compare below. + const vault = await this.vault.read(root); if (basis.target) { const connection = findConnection(catalog, { connectionId: basis.target.connectionId }); // A vanished target is its own answer — "the connection is gone" beats @@ -1074,19 +1087,29 @@ export class RuntimePolicyCoordinator { if (!connection) return { kind: 'target_missing' }; if (connection.revision !== basis.target.revision) { changed.push('connection'); - } else if (basis.credential) { - const vault = await this.vault.read(root); - if ( - !sameCredentialStatus(credentialStatus(vault, basis.credential.locator), basis.credential) - ) { - changed.push('credential'); - } + } else if ( + basis.credential && + !sameCredentialStatus(credentialStatus(vault, basis.credential.locator), basis.credential) + ) { + changed.push('credential'); } } else if (catalog.connections.some((connection) => connection.slug === basis.slug)) { // Discovery ran for a first-time creation; any connection that appeared // at the canonical slug since supersedes it. changed.push('connection'); } + if ( + basis.requestHeadersCredential && + // The probe went out with these custom headers; a rotation since means + // the inventory no longer describes what the connection would fetch. + !sameCredentialStatus( + credentialStatus(vault, basis.requestHeadersCredential.locator), + basis.requestHeadersCredential, + ) && + !changed.includes('credential') + ) { + changed.push('credential'); + } const policy = await this.policy.read(root); if ( !sameEffectiveProxyConfiguration( @@ -1096,17 +1119,15 @@ export class RuntimePolicyCoordinator { ) { changed.push('network_proxy'); } - if (basis.proxyCredential) { - const vault = await this.vault.read(root); - if ( - !sameCredentialStatus( - credentialStatus(vault, basis.proxyCredential.locator), - basis.proxyCredential, - ) && - !changed.includes('credential') - ) { - changed.push('credential'); - } + if ( + basis.proxyCredential && + !sameCredentialStatus( + credentialStatus(vault, basis.proxyCredential.locator), + basis.proxyCredential, + ) && + !changed.includes('credential') + ) { + changed.push('credential'); } return changed.length > 0 ? { kind: 'superseded', changed } : { kind: 'unchanged' }; } diff --git a/packages/storage/src/runtime-policy/operations.ts b/packages/storage/src/runtime-policy/operations.ts index e593795d2d..367ab293f6 100644 --- a/packages/storage/src/runtime-policy/operations.ts +++ b/packages/storage/src/runtime-policy/operations.ts @@ -245,6 +245,11 @@ export type BeginConnectionOnboardingResult = readonly connection: ConnectionCatalogEntry | null; /** The target's stored API key, for blank-key reuse during discovery. */ readonly storedSecret: string | null; + /** + * The target's custom request-headers secret, so the discovery probe + * carries the same header customization the models path applies. + */ + readonly requestHeadersSecret: string | null; /** * The proxy discovery must run through — pinned here, like * beginModelFetch pins it, so the basis certifies the egress the