From c489a2ddab2f357693db257e66da43cea607c9d3 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 4 Aug 2026 11:08:21 +0800 Subject: [PATCH] fix(setup): complete prepared model activation --- .../Sources/OpenClaw/OnboardingAISetup.swift | 113 ++++++++++++--- .../OpenClaw/OnboardingAISetupSupport.swift | 10 +- .../OnboardingAISetupTests.swift | 130 +++++++++++++++--- .../model-setup/model-setup-page.test.ts | 22 ++- ui/src/pages/model-setup/model-setup-page.ts | 8 +- .../pages/model-setup/prepare-options.test.ts | 34 ++++- ui/src/pages/model-setup/prepare-options.ts | 8 +- 7 files changed, 279 insertions(+), 46 deletions(-) diff --git a/apps/macos/Sources/OpenClaw/OnboardingAISetup.swift b/apps/macos/Sources/OpenClaw/OnboardingAISetup.swift index f88afc1d0ae8..a271e6e72ecc 100644 --- a/apps/macos/Sources/OpenClaw/OnboardingAISetup.swift +++ b/apps/macos/Sources/OpenClaw/OnboardingAISetup.swift @@ -805,7 +805,7 @@ extension OnboardingAISetupModel { if let preparedChoiceID { // Detection kinds encode the provider-auth choice ID, while // PrepareOption.brandId owns the model-ref namespace. - let preparedKind = "provider-auto:\(preparedChoiceID)" + let preparedKind = Self.providerAutoSetupKind(choiceID: preparedChoiceID) if let prepared = candidates.first(where: { $0.kind == preparedKind && $0.credentials != false }) { @@ -907,8 +907,29 @@ extension OnboardingAISetupModel { private func activate(kind: String, context: AttemptContext) async { guard self.isCurrentAttempt(context), !Task.isCancelled else { return } - guard let candidate = candidates.first(where: { $0.kind == kind }), - let lease = serverLease, + guard let candidate = candidates.first(where: { $0.kind == kind }) + else { + requireFreshDetection(after: Self.transportFailure( + "The Gateway connection changed. Check for AI accounts again.")) + return + } + await self.activate( + kind: kind, + modelRef: candidate.modelRef, + label: candidate.label, + tryNextCandidateOnFailure: true, + context: context) + } + + private func activate( + kind: String, + modelRef: String, + label: String, + tryNextCandidateOnFailure: Bool, + context: AttemptContext) async + { + guard self.isCurrentAttempt(context), !Task.isCancelled else { return } + guard let lease = serverLease, await gateway.isCurrentServerLease(lease) else { requireFreshDetection(after: Self.transportFailure( @@ -934,15 +955,19 @@ extension OnboardingAISetupModel { guard let routeFingerprint = await gateway.activationOwnershipFingerprint( ifCurrentServerLease: lease) else { - self.statuses[kind] = .failed(Self.transportFailure( - "Secure storage is unavailable, so OpenClaw cannot safely resume this AI setup.")) + let failure = Self.transportFailure( + "Secure storage is unavailable, so OpenClaw cannot safely resume this AI setup.") + self.statuses[kind] = .failed(failure) + if !tryNextCandidateOnFailure { + self.detectError = failure + } self.phase = .ready return } guard self.isCurrentAttempt(context), !Task.isCancelled else { return } let params = Self.activationParams( kind: kind, - modelRef: candidate.modelRef, + modelRef: modelRef, supportsExactModel: supportsExactModel) let activationOwner = OnboardingSystemAgentResumeStore.ActivationOwner( id: UUID().uuidString, @@ -957,8 +982,12 @@ extension OnboardingAISetupModel { activationTimeoutMs: requestTimeoutMs, defaults: defaults) else { - self.statuses[kind] = .failed(Self.transportFailure( - "No Gateway is selected. Select a Gateway, then try again.")) + let failure = Self.transportFailure( + "No Gateway is selected. Select a Gateway, then try again.") + self.statuses[kind] = .failed(failure) + if !tryNextCandidateOnFailure { + self.detectError = failure + } self.phase = .ready return } @@ -999,11 +1028,18 @@ extension OnboardingAISetupModel { } else { self.pendingActivationVerification = false self.clearPendingHandoff(ifOwnedBy: context, activationOwner: activationOwner) - self.statuses[kind] = .failed(Self.failure( - label: self.candidates.first { $0.kind == kind }?.label ?? kind, + let failure = Self.failure( + label: label, status: result.status, - error: result.error)) - await tryNextAfterFailure(of: kind, context: context) + error: result.error) + self.statuses[kind] = .failed(failure) + if tryNextCandidateOnFailure { + await tryNextAfterFailure(of: kind, context: context) + } else { + self.phase = .ready + self.detectError = failure + self.showManualEntry = !self.manualProviders.isEmpty + } } } catch { guard self.isCurrentAttempt(context) else { return } @@ -1012,6 +1048,9 @@ extension OnboardingAISetupModel { // this activation and its credential mutation completed safely. let failure = Self.transportFailure(error.localizedDescription) self.statuses[kind] = .failed(failure) + if !tryNextCandidateOnFailure { + self.detectError = failure + } if Self.activationFailureIsDefinitive(error) { self.pendingActivationVerification = false self.clearPendingHandoff(ifOwnedBy: context, activationOwner: activationOwner) @@ -1028,6 +1067,7 @@ extension OnboardingAISetupModel { await !(self.gateway.isCurrentServerLease(lease)), await self.reconcileActivationAfterGatewayRestart( kind: kind, + expectedModel: modelRef, context: context, activationOwner: activationOwner, before: persistedStateBeforeActivation, @@ -1047,6 +1087,7 @@ extension OnboardingAISetupModel { private func reconcileActivationAfterGatewayRestart( kind: String, + expectedModel: String, context: AttemptContext, activationOwner: OnboardingSystemAgentResumeStore.ActivationOwner, before: PersistedActivationState?, @@ -1067,6 +1108,7 @@ extension OnboardingAISetupModel { timeoutMs: Double(leaseTimeoutMs)), await reconcilePersistedActivation( kind: kind, + expectedModel: expectedModel, context: context, activationOwner: activationOwner, before: before, @@ -1096,6 +1138,7 @@ extension OnboardingAISetupModel { private func reconcilePersistedActivation( kind: String, + expectedModel: String, context: AttemptContext, activationOwner: OnboardingSystemAgentResumeStore.ActivationOwner, before: PersistedActivationState?, @@ -1103,7 +1146,6 @@ extension OnboardingAISetupModel { timeoutMs: Int) async -> Bool { guard timeoutMs > 0, - let expectedModel = candidates.first(where: { $0.kind == kind })?.modelRef, isCurrentAttempt(context), !Task.isCancelled, OnboardingSystemAgentResumeStore.isOwned( @@ -1215,7 +1257,8 @@ extension OnboardingAISetupModel { done: result.done, step: result.step, status: wizardStatusString(result.status), - error: result.error) + error: result.error, + preparedModelRef: result.preparedmodelref) } catch { // The Gateway session survives socket loss; cancel by its known // id before reporting failure so it cannot persist config later. @@ -1322,7 +1365,8 @@ extension OnboardingAISetupModel { done: result.done, step: result.step, status: wizardStatusString(result.status), - error: result.error) + error: result.error, + preparedModelRef: result.preparedmodelref) } catch { let cancellation = await self.gateway.cancelWizardSession(sessionID, on: serverLease) guard token == self.attemptToken, authAttemptID == self.authAttemptID else { return } @@ -1347,7 +1391,8 @@ extension OnboardingAISetupModel { done: Bool, step: WizardStep?, status: String?, - error: String?) + error: String?, + preparedModelRef: String?) { self.authBusy = false let validationError = !done && status == "running" && error?.isEmpty == false @@ -1371,8 +1416,30 @@ extension OnboardingAISetupModel { let preparedProvider = self.providerWizardKind == .prepare ? self.activeAuthOption.map { (id: $0.id, label: $0.label) } : nil + let preparedModel = preparedModelRef? + .trimmingCharacters(in: .whitespacesAndNewlines) self.providerAuthReconciliationPending = self.providerWizardKind == .auth self.clearProviderAuth() + if let preparedProvider, + let preparedModel, + !preparedModel.isEmpty + { + guard let context = self.captureAttemptContext() else { + self.failDetectionForMissingRoute() + return + } + let kind = Self.providerAutoSetupKind(choiceID: preparedProvider.id) + self.statuses[kind] = .untried + Task { + await self.activate( + kind: kind, + modelRef: preparedModel, + label: preparedProvider.label, + tryNextCandidateOnFailure: false, + context: context) + } + return + } self.scheduleDetection( preparedChoiceID: preparedProvider?.id, preparedProviderLabel: preparedProvider?.label) @@ -1462,8 +1529,18 @@ extension OnboardingAISetupModel { self.authBusy = true } - func _test_applyAuthWizardResult(done: Bool, status: String?, error: String?) { - self.applyAuthWizardResult(done: done, step: nil, status: status, error: error) + func _test_applyAuthWizardResult( + done: Bool, + status: String?, + error: String?, + preparedModelRef: String? = nil) + { + self.applyAuthWizardResult( + done: done, + step: nil, + status: status, + error: error, + preparedModelRef: preparedModelRef) } var _test_authSessionID: String? { diff --git a/apps/macos/Sources/OpenClaw/OnboardingAISetupSupport.swift b/apps/macos/Sources/OpenClaw/OnboardingAISetupSupport.swift index d100c1f58e5c..1c4c74acdb0f 100644 --- a/apps/macos/Sources/OpenClaw/OnboardingAISetupSupport.swift +++ b/apps/macos/Sources/OpenClaw/OnboardingAISetupSupport.swift @@ -132,9 +132,10 @@ extension OnboardingAISetupModel { website: nil), ] return (advertisedOptions ?? legacyOptions).filter { choice in + let providerKind = self.providerAutoSetupKind(choiceID: choice.id) guard !candidates.contains(where: { $0.credentials != false && - ($0.kind == "provider-auto:\(choice.id)" || + ($0.kind == providerKind || $0.modelRef.hasPrefix("\(choice.brandId ?? choice.id)/")) }) else { return false } return true @@ -196,6 +197,13 @@ extension OnboardingAISetupModel { return params } + static func providerAutoSetupKind(choiceID: String) -> String { + let componentCharacters = CharacterSet( + charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.!~*'()") + let encoded = choiceID.addingPercentEncoding(withAllowedCharacters: componentCharacters) ?? choiceID + return "provider-auto:\(encoded)" + } + static func providerAuthCancellationSessionID(requested: String, returned: String) -> String? { requested == returned ? nil : returned } diff --git a/apps/macos/Tests/OpenClawIPCTests/OnboardingAISetupTests.swift b/apps/macos/Tests/OpenClawIPCTests/OnboardingAISetupTests.swift index 2afe282019a9..7fc6a485c5ea 100644 --- a/apps/macos/Tests/OpenClawIPCTests/OnboardingAISetupTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/OnboardingAISetupTests.swift @@ -365,9 +365,25 @@ private func wizardProgressResponse(id: String, sessionID: String, message: Stri """.utf8) } -private func wizardDoneResponse(id: String, sessionID: String) -> Data { - Data(#"{"type":"res","id":"\#(id)","ok":true,"payload":{"sessionId":"\#(sessionID)","done":true,"status":"done"}}"# - .utf8) +private func wizardDoneResponse( + id: String, + sessionID: String, + preparedModelRef: String? = nil) -> Data +{ + var payload: [String: Any] = [ + "sessionId": sessionID, + "done": true, + "status": "done", + ] + if let preparedModelRef { + payload["preparedModelRef"] = preparedModelRef + } + return try! JSONSerialization.data(withJSONObject: [ + "type": "res", + "id": id, + "ok": true, + "payload": payload, + ]) } private func settleQueuedAISetupTasks() async { @@ -801,6 +817,12 @@ struct OnboardingAISetupTests { detail: "available locally", modelRef: "lmstudio/qwen3-8b-instruct", credentials: true), + OnboardingAISetupModel.Candidate( + kind: "provider-auto:vendor%2Flocal%3Av1%25beta%3Fx%23y", + label: "Vendor Local", + detail: "available locally", + modelRef: "vendor/model", + credentials: true), OnboardingAISetupModel.Candidate( kind: "provider-auto:llama-cpp", label: "Local model (llama.cpp)", @@ -833,6 +855,14 @@ struct OnboardingAISetupTests { brandId: "lmstudio", icon: "https://cdn.simpleicons.org/lmstudio", website: "https://lmstudio.ai/download"), + OnboardingAISetupModel.PrepareOption( + id: "vendor/local:v1%beta?x#y", + label: "Vendor Local", + hint: nil, + actionLabel: nil, + brandId: "different-namespace", + icon: nil, + website: nil), ] let options = OnboardingAISetupModel.prepareOptions( @@ -849,7 +879,6 @@ struct OnboardingAISetupTests { @Test func `prepare starts the shared wizard and polls gateway progress`() async throws { let recorder = AISetupRequestRecorder() let frames = AISetupSocketGeneration() - let detections = AISetupSocketGeneration() let completion = AISetupRequestGate() let preparedModelRef = "llama-cpp/gemma-4-e4b-it-q4_k_m" let session = makeAISetupRequestSession( @@ -857,19 +886,10 @@ struct OnboardingAISetupTests { handler: { task, request in switch request.method { case "openclaw.setup.detect": - if detections.claim() == 0 { - task.emitReceiveSuccess(.data(detectedSetupResponse(id: request.id))) - } else { - let response = String(decoding: detectedSetupResponse( - id: request.id, - kind: "provider-auto:llama-cpp", - modelRef: preparedModelRef), as: UTF8.self) - .replacingOccurrences( - of: #""credentials":false"#, - with: #""credentials":true"#) - task.emitReceiveSuccess(.data(Data(response.utf8))) - } + task.emitReceiveSuccess(.data(detectedSetupResponse(id: request.id))) case "openclaw.setup.activate": + #expect(request.params["kind"] as? String == "provider-auto:llama-cpp") + #expect(request.params["modelRef"] as? String == preparedModelRef) task.emitReceiveSuccess(.data(successfulActivationResponse( id: request.id, modelRef: preparedModelRef, @@ -899,7 +919,8 @@ struct OnboardingAISetupTests { await completion.wait() task.emitReceiveSuccess(.data(wizardDoneResponse( id: request.id, - sessionID: sessionID))) + sessionID: sessionID, + preparedModelRef: preparedModelRef))) } default: break @@ -952,11 +973,86 @@ struct OnboardingAISetupTests { #expect(model.connectedModelRef == preparedModelRef) let completedRequests = await recorder.snapshot() #expect(completedRequests.methods.suffix(2) == [ + "wizard.next", + "openclaw.setup.activate", + ]) + #expect(completedRequests.methods.filter { $0 == "openclaw.setup.detect" }.count == 1) + } + + @Test func `prepare without a model handoff falls back to detection`() async throws { + let recorder = AISetupRequestRecorder() + let detections = AISetupSocketGeneration() + let preparedModelRef = "llama-cpp/gemma-4-e4b-it-q4_k_m" + let session = makeAISetupRequestSession( + recorder: recorder, + handler: { task, request in + switch request.method { + case "openclaw.setup.detect": + if detections.claim() == 0 { + task.emitReceiveSuccess(.data(detectedSetupResponse(id: request.id))) + } else { + let response = String(decoding: detectedSetupResponse( + id: request.id, + kind: "provider-auto:llama-cpp", + modelRef: preparedModelRef), as: UTF8.self) + .replacingOccurrences( + of: #""credentials":false"#, + with: #""credentials":true"#) + task.emitReceiveSuccess(.data(Data(response.utf8))) + } + case "openclaw.setup.prepare.start": + let sessionID = request.params["sessionId"] as? String ?? "prepare-session" + task.emitReceiveSuccess(.data(wizardDoneResponse( + id: request.id, + sessionID: sessionID))) + case "openclaw.setup.activate": + task.emitReceiveSuccess(.data(successfulActivationResponse( + id: request.id, + modelRef: preparedModelRef, + latencyMs: 731))) + default: + break + } + }, + receiveHook: { task, receiveIndex in + if receiveIndex == 0 { + return .data(GatewayWebSocketTestSupport.connectChallengeData()) + } + let id = task.snapshotConnectRequestID() ?? "connect" + return .data(GatewayWebSocketTestSupport.connectOkData( + id: id, + methods: [ + "openclaw.setup.prepare.start", + "openclaw.setup.activate", + ], + capabilities: ["openclaw-setup-model-ref"])) + }) + let url = try #require(URL(string: "ws://example.invalid")) + let gateway = makeAISetupGateway(url: url, session: session) + let model = makeAISetupModel(gateway: gateway) + + await model.detectAndAutoConnect() + let option = try #require(model.prepareOptions.first { $0.id == "llama-cpp" }) + model.startProviderPrepare(option) + for _ in 0..<400 where !model.connected { + try? await Task.sleep(nanoseconds: 5_000_000) + } + + #expect(model.connectedModelRef == preparedModelRef) + #expect(await (recorder.snapshot()).methods == [ + "openclaw.setup.detect", + "openclaw.setup.prepare.start", "openclaw.setup.detect", "openclaw.setup.activate", ]) } + @Test func `provider setup kinds encode reserved choice id characters`() { + #expect(OnboardingAISetupModel.providerAutoSetupKind( + choiceID: "vendor/local:v1%beta?x#y") == + "provider-auto:vendor%2Flocal%3Av1%25beta%3Fx%23y") + } + @Test func `provider auth opens only safe external links`() { let safe = OnboardingProviderAuthLink.safeURL( "https://auth.openai.com/oauth/authorize?client_id=test") diff --git a/ui/src/pages/model-setup/model-setup-page.test.ts b/ui/src/pages/model-setup/model-setup-page.test.ts index 7085a2657da7..5edd67d62bea 100644 --- a/ui/src/pages/model-setup/model-setup-page.test.ts +++ b/ui/src/pages/model-setup/model-setup-page.test.ts @@ -299,6 +299,18 @@ describe("ModelSetupPage catalog icons", () => { }); it("verifies a prepared local provider model before showing success", async () => { + const choiceId = "vendor/local:v1%beta?x#y"; + const preparedDetection: SystemAgentSetupDetectResult = { + ...detection, + prepareOptions: [ + { + id: choiceId, + brandId: "llama-cpp", + label: "llama.cpp", + hint: "Run one private GGUF model directly inside this Gateway", + }, + ], + }; const { context: baseContext, client, request } = createContext(); const runtimeConfig = { runExternalMutation: vi.fn(async (task) => ({ @@ -321,7 +333,7 @@ describe("ModelSetupPage catalog icons", () => { } if (method === "openclaw.setup.detect") { return { - ...detection, + ...preparedDetection, candidates: [ { kind: "existing-model", @@ -332,7 +344,7 @@ describe("ModelSetupPage catalog icons", () => { credentials: true, }, { - kind: "provider-auto:llama-cpp", + kind: "provider-auto:vendor%2Flocal%3Av1%25beta%3Fx%23y", brandId: "llama-cpp", label: "llama.cpp", detail: "Gemma 4 E4B downloaded", @@ -354,18 +366,18 @@ describe("ModelSetupPage catalog icons", () => { return {}; }); const { page } = await mountPage(context, { - state: { phase: "ready", result: detection }, + state: { phase: "ready", result: preparedDetection }, client, firstRun: false, }); - page.querySelector('[data-prepare-choice="llama-cpp"] button')?.click(); + page.querySelector(`[data-prepare-choice="${choiceId}"] button`)?.click(); await vi.waitFor(() => { expect(request).toHaveBeenCalledWith( "openclaw.setup.activate", { - kind: "provider-auto:llama-cpp", + kind: "provider-auto:vendor%2Flocal%3Av1%25beta%3Fx%23y", modelRef: "llama-cpp/gemma-4-e4b-it-q4_k_m", }, expect.objectContaining({ signal: expect.any(AbortSignal) }), diff --git a/ui/src/pages/model-setup/model-setup-page.ts b/ui/src/pages/model-setup/model-setup-page.ts index 5e5ce3ad0fcc..5e5722f39e2e 100644 --- a/ui/src/pages/model-setup/model-setup-page.ts +++ b/ui/src/pages/model-setup/model-setup-page.ts @@ -19,7 +19,11 @@ import { OpenClawLightDomElement } from "../../lit/openclaw-element.ts"; import { SubscriptionsController } from "../../lit/subscriptions-controller.ts"; import { fetchCatalogIconBlobUrl } from "../plugins/icon-loader.ts"; import type { ModelSetupDetectionConnection } from "./detect-cache.ts"; -import { findPreparedModelCandidate, type ModelSetupPrepareOption } from "./prepare-options.ts"; +import { + findPreparedModelCandidate, + type ModelSetupPrepareOption, + providerAutoSetupKind, +} from "./prepare-options.ts"; import { detectModelSetup, verifyModelSetup } from "./rpc.ts"; import { activationTargetId, @@ -553,7 +557,7 @@ export class ModelSetupPage extends OpenClawLightDomElement { startMethod === "openclaw.setup.prepare.start" ? this.pendingPrepareOption : null; this.pendingPrepareOption = null; if (prepareOption && preparedModelRef) { - const kind = `provider-auto:${prepareOption.id}` as const; + const kind = providerAutoSetupKind(prepareOption.id); this.wizard.close(); void this.activate( { kind, modelRef: preparedModelRef }, diff --git a/ui/src/pages/model-setup/prepare-options.test.ts b/ui/src/pages/model-setup/prepare-options.test.ts index bd6b271e66a8..c37f216a74cd 100644 --- a/ui/src/pages/model-setup/prepare-options.test.ts +++ b/ui/src/pages/model-setup/prepare-options.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; import type { SystemAgentSetupDetectResult } from "../../api/types.ts"; -import { findPreparedModelCandidate, listModelSetupPrepareOptions } from "./prepare-options.ts"; +import { + findPreparedModelCandidate, + listModelSetupPrepareOptions, + providerAutoSetupKind, +} from "./prepare-options.ts"; function detection( candidates: SystemAgentSetupDetectResult["candidates"], @@ -19,6 +23,34 @@ function detection( } describe("model setup prepare options", () => { + it("encodes provider choice ids in setup kinds", () => { + const choiceId = "vendor/local:v1%beta?x#y"; + const kind = "provider-auto:vendor%2Flocal%3Av1%25beta%3Fx%23y"; + expect(providerAutoSetupKind(choiceId)).toBe(kind); + const candidate = { + kind, + brandId: "vendor", + label: "Vendor Local", + detail: "available locally", + modelRef: "vendor/model", + recommended: false, + credentials: true, + }; + const result = detection( + [candidate], + [{ id: choiceId, brandId: "vendor", label: "Vendor Local" }], + ); + + expect(listModelSetupPrepareOptions(result)).toEqual([]); + expect(findPreparedModelCandidate(result, choiceId)).toEqual(candidate); + }); + + it("does not treat raw reserved choice ids as canonical kinds", () => { + expect(providerAutoSetupKind("local/provider%beta")).not.toBe( + "provider-auto:local/provider%beta", + ); + }); + it("uses provider identity to hide a usable aliased provider", () => { const result = detection( [ diff --git a/ui/src/pages/model-setup/prepare-options.ts b/ui/src/pages/model-setup/prepare-options.ts index 6494673ebff9..53ca10655bb7 100644 --- a/ui/src/pages/model-setup/prepare-options.ts +++ b/ui/src/pages/model-setup/prepare-options.ts @@ -11,6 +11,10 @@ export type ModelSetupPrepareOption = { website?: string; }; +export function providerAutoSetupKind(choiceId: string): `provider-auto:${string}` { + return `provider-auto:${encodeURIComponent(choiceId)}`; +} + export function listModelSetupPrepareOptions( result: SystemAgentSetupDetectResult, ): ModelSetupPrepareOption[] { @@ -34,7 +38,7 @@ export function listModelSetupPrepareOptions( !result.candidates.some( (candidate) => candidate.credentials !== false && - (candidate.kind === `provider-auto:${choice.id}` || + (candidate.kind === providerAutoSetupKind(choice.id) || candidate.modelRef.startsWith(`${choice.brandId ?? choice.id}/`)), ), ); @@ -45,6 +49,6 @@ export function findPreparedModelCandidate(result: SystemAgentSetupDetectResult, // brandId owns the model-ref namespace and may differ. return result.candidates.find( (candidate) => - candidate.kind === `provider-auto:${choiceId}` && candidate.credentials !== false, + candidate.kind === providerAutoSetupKind(choiceId) && candidate.credentials !== false, ); }