From e487e5fb69a832115a2023d48ac24d25cac5e516 Mon Sep 17 00:00:00 2001 From: hellodk Date: Sat, 29 Aug 2026 04:39:20 +0530 Subject: [PATCH 1/2] fix: persist chat settings to config.yaml, not unregistered VS Code keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-webview settings overlay and Add-Model dialogue posted a saveSettings message whose host handler wrote champ.provider and champ..model/baseUrl to VS Code global settings. Those keys were removed from package.json in the YAML-only migration (#118), so any interaction surfaced "Unable to write to User Settings because champ.provider is not a registered configuration." saveSettings now writes the provider/model/baseUrl block into .champ/config.yaml (workspace or ~/.champ fallback) via upsertProviderInYaml with setActive, then reloads — never touching VS Code settings. Closes #123 --- src/ui/chat-view-provider.ts | 66 +++++++++++++++++++------ test/unit/ui/chat-view-provider.test.ts | 66 +++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 16 deletions(-) diff --git a/src/ui/chat-view-provider.ts b/src/ui/chat-view-provider.ts index f2421e4..f716c4e 100644 --- a/src/ui/chat-view-provider.ts +++ b/src/ui/chat-view-provider.ts @@ -10,7 +10,9 @@ */ import * as vscode from "vscode"; import * as path from "path"; +import * as os from "os"; import { execFile } from "child_process"; +import { upsertProviderInYaml } from "../config/yaml-writer"; import { type AgentController, PromptInjectionError, @@ -486,25 +488,15 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { }); } } else if (isSaveSettingsRequest(msg)) { - // Update provider and model in VS Code global settings, then reload. - const config = vscode.workspace.getConfiguration("champ"); - await config.update( - "provider", + // Persist provider/model/baseUrl into .champ/config.yaml (the YAML-only + // config source since #118), then reload. The legacy champ.* VS Code + // settings were removed from package.json, so writing them here throws + // "not a registered configuration" (#123). + await this.persistProviderSettings( msg.provider, - vscode.ConfigurationTarget.Global, - ); - await config.update( - `${msg.provider}.model`, msg.model, - vscode.ConfigurationTarget.Global, + msg.baseUrl, ); - if (msg.baseUrl) { - await config.update( - `${msg.provider}.baseUrl`, - msg.baseUrl, - vscode.ConfigurationTarget.Global, - ); - } await vscode.commands.executeCommand("champ.reloadProvider"); } else if (isCopyToClipboardRequest(msg)) { // navigator.clipboard is blocked in VS Code webviews — route through extension host @@ -818,6 +810,48 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { } as never); } + /** + * Persist a provider selection (from the in-webview settings overlay or + * Add-Model dialogue) into .champ/config.yaml (#123). The YAML file is + * the single config source since #118; the legacy champ.* VS Code + * settings no longer exist. Targets the workspace .champ/config.yaml, + * falling back to ~/.champ/config.yaml when no workspace is open. + */ + private async persistProviderSettings( + providerId: string, + model: string, + baseUrl?: string, + ): Promise { + const root = + vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? os.homedir(); + const dirUri = vscode.Uri.file(path.join(root, ".champ")); + const fileUri = vscode.Uri.file(path.join(root, ".champ", "config.yaml")); + + let previousText: string | null = null; + try { + previousText = new TextDecoder().decode( + await vscode.workspace.fs.readFile(fileUri), + ); + } catch { + previousText = null; // no file yet + } + + const { yaml: updated } = upsertProviderInYaml( + previousText, + { providerId, baseUrl, model }, + { setActive: true }, + ); + try { + await vscode.workspace.fs.createDirectory(dirUri); + } catch { + // already exists + } + await vscode.workspace.fs.writeFile( + fileUri, + new TextEncoder().encode(updated), + ); + } + /** * Run a shell command (requested from a webview bash code-block "Run" button) * and stream stdout chunks back to the webview as TerminalOutputChunkMessage. diff --git a/test/unit/ui/chat-view-provider.test.ts b/test/unit/ui/chat-view-provider.test.ts index cbd9aea..d0f6151 100644 --- a/test/unit/ui/chat-view-provider.test.ts +++ b/test/unit/ui/chat-view-provider.test.ts @@ -955,4 +955,70 @@ describe("ChatViewProvider", () => { expect(msg.activeSessionId).toBe("s1"); }); }); + + describe("save settings to YAML instead of VS Code settings (#123)", () => { + it("persists provider/model/baseUrl to .champ/config.yaml on saveSettings", async () => { + const vscode = await import("vscode"); + const ws = ( + vscode as unknown as { + workspace: { + fs: { + writeFile: ReturnType; + readFile: ReturnType; + }; + }; + } + ).workspace; + const writeFile = ws.fs.writeFile as ReturnType; + writeFile.mockClear(); + + const view = createMockWebviewView(postMessage); + provider.resolveWebviewView(view as never, {} as never, {} as never); + + view.fireMessage({ + type: "saveSettings", + provider: "openai-compatible", + model: "some-model", + baseUrl: "http://192.168.1.5:8000/v1", + }); + await new Promise((resolve) => setImmediate(resolve)); + + expect(writeFile).toHaveBeenCalled(); + const [uri, bytes] = writeFile.mock.calls[0] as [unknown, Uint8Array]; + expect(String((uri as { fsPath: string }).fsPath)).toContain( + ".champ/config.yaml", + ); + const text = new TextDecoder().decode(bytes); + expect(text).toContain("provider: openai-compatible"); + expect(text).toContain("baseUrl: http://192.168.1.5:8000/v1"); + expect(text).toContain("model: some-model"); + }); + + it("does not call config.update for provider/model/baseUrl on saveSettings", async () => { + const vscode = await import("vscode"); + const cfg = vscode.workspace.getConfiguration("champ") as { + update: ReturnType; + }; + cfg.update.mockClear(); + + const view = createMockWebviewView(postMessage); + provider.resolveWebviewView(view as never, {} as never, {} as never); + + view.fireMessage({ + type: "saveSettings", + provider: "ollama", + model: "llama3.1", + baseUrl: "http://localhost:11434", + }); + await new Promise((resolve) => setImmediate(resolve)); + + const updates = cfg.update.mock.calls + .map((c) => String(c[0])) + .filter( + (k) => + k === "provider" || k.endsWith(".model") || k.endsWith(".baseUrl"), + ); + expect(updates).toEqual([]); + }); + }); }); From 8f8a2ef8a4f12d6bf2cad9bc3a1d8a16a812138c Mon Sep 17 00:00:00 2001 From: hellodk Date: Sun, 30 Aug 2026 18:26:19 +0530 Subject: [PATCH 2/2] fix: attach provider apiKey to the model-discovery probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-webview "Add Model" discovery fetch sent no Authorization header, so a key-required endpoint (MLX/OpenAI-compatible, vLLM) returned 401 and the UI showed "Connected · 0 models found" even though chat worked. The discovery probe now resolves the provider's apiKey (YAML config, then SecretStorage) via a new ChatViewProvider.setApiKeyResolver and sends it as Authorization: Bearer on /v1/models — mirroring the extension's probe fix. Refs #123 --- src/extension.ts | 22 +++++++ src/ui/chat-view-provider.ts | 24 ++++++++ test/unit/ui/chat-view-provider.test.ts | 78 +++++++++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/src/extension.ts b/src/extension.ts index 87bc198..dc369b6 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -969,6 +969,28 @@ export async function activate( }, (template, ctx) => VariableResolver.resolve(template, ctx), ); + // Resolve a provider's apiKey for the model-discovery probe (YAML config, + // then SecretStorage/env fallback). Key-required endpoints (MLX, + // openai-compatible, vLLM) would otherwise return 401 -> "0 models found". + chatViewProvider.setApiKeyResolver(async (providerId) => { + const cfg = await resolveConfig(); + const entry = ( + cfg?.providers as Record | undefined + )?.[providerId]; + if (entry?.apiKey) return entry.apiKey; + const secretKeys: Record = { + claude: "champ.claude.apiKey", + openai: "champ.openai.apiKey", + gemini: "champ.gemini.apiKey", + "openai-compatible": "champ.openaiCompatible.apiKey", + vllm: "champ.vllm.apiKey", + }; + const secretKey = secretKeys[providerId]; + if (secretKey) { + return (await context.secrets.get(secretKey)) || undefined; + } + return undefined; + }); // Auto-label sessions + Smart Router model selection. chatViewProvider.onUserMessage((text) => { const active = agentManager?.getActive(); diff --git a/src/ui/chat-view-provider.ts b/src/ui/chat-view-provider.ts index f716c4e..932f5e9 100644 --- a/src/ui/chat-view-provider.ts +++ b/src/ui/chat-view-provider.ts @@ -179,6 +179,9 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { private skillRegistry: ChatSkillRegistry | undefined; private skillContextProvider: SkillContextProvider | undefined; private skillVariableResolver: SkillVariableResolver | undefined; + private apiKeyResolver: + | ((providerId: string) => Promise) + | undefined; private userMessageCallback: ((text: string) => void) | undefined; private webviewReadyCallback: (() => void) | undefined; private streamCompletedCallback: @@ -251,6 +254,18 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { this.contextResolver = resolver; } + /** + * Attach a resolver that yields the apiKey for a provider id (from YAML + * config / SecretStorage). Used by the model-discovery probe so + * key-required endpoints (e.g. MLX/OpenAI-compatible) return models + * instead of a 401 -> "0 models found" (#123). + */ + setApiKeyResolver( + resolver: (providerId: string) => Promise, + ): void { + this.apiKeyResolver = resolver; + } + /** * Attach a skill registry. When set, user messages starting with * `/` are looked up in the registry and the matching skill's @@ -462,8 +477,17 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { const endpoint = isOllama ? `${msg.baseUrl.replace(/\/$/, "")}/api/tags` : `${msg.baseUrl.replace(/\/$/, "")}/v1/models`; + // Key-required endpoints (MLX/OpenAI-compatible, vLLM) return 401 + // without a Bearer key — attach the provider's stored key so the + // probe returns models instead of "0 models found" (#123). + const headers: Record = {}; + if (!isOllama && this.apiKeyResolver) { + const key = await this.apiKeyResolver(msg.provider); + if (key) headers.Authorization = `Bearer ${key}`; + } const resp = await fetch(endpoint, { signal: AbortSignal.timeout(5000), + headers, }); if (!resp.ok) throw new Error(`HTTP ${resp.status}`); const body = (await resp.json()) as Record; diff --git a/test/unit/ui/chat-view-provider.test.ts b/test/unit/ui/chat-view-provider.test.ts index d0f6151..57924b8 100644 --- a/test/unit/ui/chat-view-provider.test.ts +++ b/test/unit/ui/chat-view-provider.test.ts @@ -1021,4 +1021,82 @@ describe("ChatViewProvider", () => { expect(updates).toEqual([]); }); }); + + describe("model discovery attaches the provider apiKey (#123)", () => { + it("sends an Authorization Bearer header on the /v1/models probe", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + data: [{ id: "mlx-community--Qwen3-1.7B-4bit" }], + }), + }); + vi.stubGlobal("fetch", fetchMock); + + // Attach a key resolver so a known provider key is sent. + ( + provider as unknown as { + setApiKeyResolver: ( + cb: (id: string) => Promise, + ) => void; + } + ).setApiKeyResolver(() => Promise.resolve("dummy")); + + const view = createMockWebviewView(postMessage); + provider.resolveWebviewView(view as never, {} as never, {} as never); + + view.fireMessage({ + type: "discoverModels", + provider: "openai-compatible", + baseUrl: "http://192.168.1.5:8000", + }); + await new Promise((resolve) => setImmediate(resolve)); + + const [url, init] = fetchMock.mock.calls[0] as [ + string, + { headers?: Record }, + ]; + expect(url).toContain("/v1/models"); + expect(init.headers?.Authorization).toBe("Bearer dummy"); + + const posts = postMessage.mock.calls.filter( + (args) => (args[0] as { type: string }).type === "discoveredModels", + ); + expect(posts).toHaveLength(1); + const msg = posts[0][0] as { models: Array<{ name: string }> }; + expect(msg.models[0].name).toBe("mlx-community--Qwen3-1.7B-4bit"); + }); + + it("declares the endpoint as '0 models' when no key is attached and the server 401s", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + status: 401, + }); + vi.stubGlobal("fetch", fetchMock); + + const view = createMockWebviewView(postMessage); + provider.resolveWebviewView(view as never, {} as never, {} as never); + + view.fireMessage({ + type: "discoverModels", + provider: "openai-compatible", + baseUrl: "http://192.168.1.5:8000", + }); + await new Promise((resolve) => setImmediate(resolve)); + + const posts = postMessage.mock.calls.filter( + (args) => (args[0] as { type: string }).type === "discoveredModels", + ); + expect(posts).toHaveLength(1); + const msg = posts[0][0] as { + models: Array<{ name: string }>; + error?: string; + }; + expect(msg.models).toEqual([]); + expect(msg.error).toContain("401"); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + }); });