diff --git a/.qwen/e2e-tests/electron-desktop/model-configuration-workflow.md b/.qwen/e2e-tests/electron-desktop/model-configuration-workflow.md new file mode 100644 index 000000000..63a73998d --- /dev/null +++ b/.qwen/e2e-tests/electron-desktop/model-configuration-workflow.md @@ -0,0 +1,55 @@ +# Model Configuration Workflow + +- Slice date: 2026-04-26 +- Executable harness: `packages/desktop/scripts/e2e-cdp-smoke.mjs` +- Command: + `cd packages/desktop && npm run e2e:cdp` +- Result: pass +- Artifact directory: + `.qwen/e2e-tests/electron-desktop/artifacts/2026-04-26T03-47-01-812Z/` + +## Scenario + +1. Launch the real Electron app with isolated HOME, runtime, user-data, fake + ACP, and a dirty temporary Git workspace. +2. Open the fake project, type a first composer prompt, create a thread, and + approve the fake command request. +3. Complete the existing review, discard-cancel safety, stage, and commit path. +4. Open Settings, save `qwen-e2e-cdp` with a fake API key and base URL, and + assert the saved model is visible without exposing the secret. +5. Return to Conversation and select `qwen-e2e-cdp` from the composer model + picker. +6. Continue through terminal expand, command execution, attach-to-composer, and + send verification. + +## Assertions + +- The Settings default view includes product sections and keeps diagnostics + behind Advanced. +- The API key input remains `type="password"` and is cleared after save. +- The fake API key is absent from settings text, composer text, input values, + advanced diagnostics, and the model-switch artifact. +- The composer model picker is enabled for the active thread, includes both the + fake ACP runtime model and the saved configured model, and switches to + `qwen-e2e-cdp`. +- The conversation view after returning from Settings does not expose the local + server URL. +- The composer stays contained in the chat panel and does not overflow after + the model switch. +- Console errors: 0. +- Failed local network requests: 0. + +## Artifacts + +- `settings-page.png` +- `settings-product-state.json` +- `settings-advanced-diagnostics.json` +- `composer-model-switch.json` +- `electron.log` +- `summary.json` + +## Known Uncovered Risk + +The harness verifies the API-key model save and active-session model switch +using fake ACP. It does not yet cover Coding Plan model switching, invalid API +key validation, or keyboard-only navigation through the model picker. diff --git a/design/qwen-code-electron-desktop-implementation-plan.md b/design/qwen-code-electron-desktop-implementation-plan.md index d7751b590..992fbcb40 100644 --- a/design/qwen-code-electron-desktop-implementation-plan.md +++ b/design/qwen-code-electron-desktop-implementation-plan.md @@ -22,6 +22,110 @@ execution order, verification, decisions, and remaining work. ## Codex Alignment Progress +### Completed Slice: Composer Model Provider Promotion + +Status: completed in iteration 27. + +Goal: make a model saved in desktop settings immediately available from the +composer model picker for the active thread, so the settings flow connects to +the first-viewport task controls instead of ending on the settings page. + +User-visible value: after adding or editing an API-key model configuration, +users can return to the conversation, choose that saved model from the composer, +and see the active thread model update without restarting the desktop app or +creating a new thread. + +Expected files: + +- `packages/desktop/src/renderer/App.tsx` +- `packages/desktop/src/renderer/stores/modelStore.ts` +- `packages/desktop/src/renderer/stores/modelStore.test.ts` +- `packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx` +- `packages/desktop/scripts/e2e-cdp-smoke.mjs` +- `.qwen/e2e-tests/electron-desktop/model-configuration-workflow.md` +- `design/qwen-code-electron-desktop-implementation-plan.md` + +Acceptance criteria: + +- Saving an API-key model provider merges the saved model into the active + session model options without leaking the API key into DOM text, input values, + screenshots, logs, or diagnostics. +- The composer model picker stays disabled before a thread exists, then shows + the runtime model and the saved configured model once a session is active. +- Selecting the saved model from the composer calls the existing token-protected + session model route and updates the visible composer selection. +- Settings remains a supporting surface; returning to Conversation restores the + conversation-first workbench, terminal strip, and compact composer. +- No raw ACP/session IDs or server URLs are introduced into the main + conversation or composer. + +Verification: + +- Unit/component test command: + `cd packages/desktop && SHELL=/bin/bash npx vitest run src/renderer/stores/modelStore.test.ts src/renderer/components/layout/WorkspacePage.test.tsx` +- Syntax command: `node --check packages/desktop/scripts/e2e-cdp-smoke.mjs` +- Build/typecheck/lint commands: + `cd packages/desktop && npm run typecheck && npm run lint && npm run build` +- Real Electron harness: + `cd packages/desktop && npm run e2e:cdp` +- Harness path: `packages/desktop/scripts/e2e-cdp-smoke.mjs` +- E2E scenario steps: launch real Electron with isolated HOME/runtime/user-data + and fake ACP, open the fake Git project, create a thread, complete the + command-approval path, open Settings, save `qwen-e2e-cdp` with a fake API key, + return to Conversation, select `qwen-e2e-cdp` from the composer model picker, + assert the composer selection changes and the secret is absent, then continue + terminal attach/send verification. +- E2E assertions: saved configured model appears in composer options, selecting + it updates the active select value and visible text, the API key remains + hidden, the composer remains contained, and no console errors or failed local + requests are recorded. +- Diagnostic artifacts: `settings-product-state.json`, + `composer-model-switch.json`, `settings-page.png`, Electron log, and summary + JSON under `.qwen/e2e-tests/electron-desktop/artifacts/`. +- Required skills applied: `brainstorming` for selecting the smallest + settings-to-composer workflow slice without asking routine product questions, + `frontend-design` for keeping the picker compact and prototype-constrained, + and `electron-desktop-dev` for real Electron CDP verification. + +Notes and decisions: + +- Chosen approach: promote saved provider models into renderer model state as + selectable session candidates, then continue using the existing + `/api/sessions/:id/model` route for the actual thread switch. This keeps the + server ACP session model route as the authority for runtime state while making + the settings result visible in the first viewport. +- Alternatives rejected for this slice: rebuilding the full model provider UI + as a composer popover, or automatically switching the active session when + settings are saved. Both are broader than needed and risk surprising users. +- Configured model options are replaced when settings change rather than + accumulated indefinitely. Session resets preserve the configured option cache + so the next loaded runtime model list can be merged without another settings + fetch. + +Verification results: + +- `node --check packages/desktop/scripts/e2e-cdp-smoke.mjs` passed. +- `git diff --check` passed. +- `cd packages/desktop && SHELL=/bin/bash npx vitest run src/renderer/stores/modelStore.test.ts src/renderer/components/layout/WorkspacePage.test.tsx` + passed with 21 tests. +- `cd packages/desktop && npm run typecheck` passed. +- `cd packages/desktop && npm run lint` passed. +- `cd packages/desktop && npm run build` passed. +- `cd packages/desktop && npm run e2e:cdp` passed through real Electron at + `.qwen/e2e-tests/electron-desktop/artifacts/2026-04-26T03-47-01-812Z/`. +- Key recorded model-switch metrics: composer model picker enabled, selected + value `qwen-e2e-cdp`, options `e2e/qwen-code` and `qwen-e2e-cdp`, composer + height `101` px, no composer overflow, no fake API key exposure, no local + server URL exposure in the conversation view, no console errors, and no + failed local requests. + +Next work: + +- Continue the model configuration workflow by adding inline validation and + clearer disabled/save reasons for missing model, base URL, or API key states. +- Continue prototype fidelity by checking whether the settings page needs a + narrower modal/drawer treatment instead of a full workbench replacement. + ### Completed Slice: Sidebar and Topbar Chrome Density Pass Status: completed in iteration 26. diff --git a/packages/desktop/scripts/e2e-cdp-smoke.mjs b/packages/desktop/scripts/e2e-cdp-smoke.mjs index 76248c0b0..87989e212 100644 --- a/packages/desktop/scripts/e2e-cdp-smoke.mjs +++ b/packages/desktop/scripts/e2e-cdp-smoke.mjs @@ -188,6 +188,7 @@ async function main() { await clickButton('Conversation'); await waitForSelector('[data-testid="terminal-drawer"]'); + await assertComposerModelSwitch('composer-model-switch.json', 'qwen-e2e-cdp'); await clickButton('Expand Terminal'); await waitForSelector('[data-testid="terminal-body"]'); await assertTerminalExpandedLayout('terminal-expanded-layout.json'); @@ -4164,6 +4165,123 @@ async function assertSettingsAdvancedDiagnostics(fileName) { } } +async function assertComposerModelSwitch(fileName, modelId) { + await waitFor( + 'configured composer model option', + async () => + evaluate(`(() => { + const select = document.querySelector('select[aria-label="Model"]'); + return Boolean( + select && + !select.disabled && + [...select.options].some( + (option) => option.value === ${JSON.stringify(modelId)} + ) + ); + })()`), + 15_000, + ); + + await setFieldByAriaLabel('Model', modelId); + await waitFor( + 'composer model switch', + async () => + evaluate(`(() => { + const select = document.querySelector('select[aria-label="Model"]'); + return Boolean(select && select.value === ${JSON.stringify(modelId)}); + })()`), + 15_000, + ); + + const snapshot = await evaluate(`(() => { + const rectFor = (selector) => { + const element = document.querySelector(selector); + if (!element) { + return null; + } + const rect = element.getBoundingClientRect(); + return { + top: rect.top, + right: rect.right, + bottom: rect.bottom, + left: rect.left, + width: rect.width, + height: rect.height + }; + }; + const select = document.querySelector('select[aria-label="Model"]'); + const options = select + ? [...select.options].map((option) => ({ + value: option.value, + text: option.textContent.trim(), + selected: option.selected + })) + : []; + const selected = options.find((option) => option.selected) ?? null; + const bodyText = document.body.innerText; + return { + composer: rectFor('[data-testid="message-composer"]'), + chat: rectFor('[data-testid="chat-thread"]'), + terminal: rectFor('[data-testid="terminal-drawer"]'), + disabled: select?.disabled ?? null, + value: select?.value ?? null, + options, + selected, + hasSavedModel: options.some( + (option) => option.value === ${JSON.stringify(modelId)} + ), + hasSecret: + bodyText.includes('sk-desktop-e2e') || + [...document.querySelectorAll('input, textarea')].some((field) => + field.value.includes('sk-desktop-e2e') + ), + hasServerUrl: /http:\\/\\/127\\.0\\.0\\.1:/u.test(bodyText), + composerOverflow: + Boolean( + document.querySelector('[data-testid="message-composer"]') && + document.querySelector('[data-testid="message-composer"]').scrollWidth > + document.querySelector('[data-testid="message-composer"]').clientWidth + 4 + ) + }; + })()`); + + await writeFile( + join(artifactDir, fileName), + `${JSON.stringify(snapshot, null, 2)}\n`, + 'utf8', + ); + + if (snapshot.disabled !== false) { + throw new Error('Composer model picker should be enabled for active thread.'); + } + + if (!snapshot.hasSavedModel || snapshot.value !== modelId) { + throw new Error( + `Composer did not switch to configured model: ${JSON.stringify(snapshot)}`, + ); + } + + if (snapshot.hasSecret) { + throw new Error('Composer model workflow exposed the fake API key.'); + } + + if (snapshot.hasServerUrl) { + throw new Error('Conversation view exposed the local server URL.'); + } + + if ( + !snapshot.composer || + !snapshot.chat || + snapshot.composer.bottom > snapshot.chat.bottom + 1 + ) { + throw new Error('Composer model picker is not contained in chat panel.'); + } + + if (snapshot.composerOverflow) { + throw new Error('Composer overflows after model switch.'); + } +} + async function waitForText(text, timeoutMs = 15_000) { await waitFor( `text "${text}"`, diff --git a/packages/desktop/src/renderer/App.tsx b/packages/desktop/src/renderer/App.tsx index 4d5a6f946..8d299453c 100644 --- a/packages/desktop/src/renderer/App.tsx +++ b/packages/desktop/src/renderer/App.tsx @@ -175,6 +175,7 @@ export function App() { .then((settings) => { if (!disposed) { dispatchSettings({ type: 'load_success', settings }); + dispatchModel({ type: 'settings_models_loaded', settings }); } }) .catch((error: unknown) => { @@ -850,6 +851,7 @@ export function App() { buildSettingsUpdateRequest(settingsState.form), ); dispatchSettings({ type: 'save_success', settings }); + dispatchModel({ type: 'settings_models_loaded', settings }); } catch (error) { dispatchSettings({ type: 'save_error', message: getErrorMessage(error) }); } diff --git a/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx b/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx index 5678e51c4..88add4359 100644 --- a/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx +++ b/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx @@ -463,6 +463,43 @@ describe('WorkspacePage', () => { expect((model as HTMLSelectElement).disabled).toBe(true); }); + it('shows configured settings models in the active composer picker', () => { + const onModelChange = vi.fn(); + const renderedContainer = renderWorkspace({ + modelState: { + ...createInitialModelState(), + models: { + currentModelId: 'e2e/qwen-code', + availableModels: [ + { modelId: 'e2e/qwen-code', name: 'Qwen Code E2E' }, + { + modelId: 'qwen-e2e-cdp', + name: 'qwen-e2e-cdp', + description: 'Configured in desktop settings', + }, + ], + }, + }, + onModelChange, + }); + const model = renderedContainer.querySelector('select[aria-label="Model"]'); + + expect(model).toBeInstanceOf(HTMLSelectElement); + expect((model as HTMLSelectElement).disabled).toBe(false); + expect((model as HTMLSelectElement).value).toBe('e2e/qwen-code'); + expect( + [...(model as HTMLSelectElement).options].map((option) => option.value), + ).toEqual(['e2e/qwen-code', 'qwen-e2e-cdp']); + expect(renderedContainer.textContent).toContain('qwen-e2e-cdp'); + expect(renderedContainer.textContent).not.toContain('sk-desktop-e2e'); + + act(() => { + setSelectValue(model as HTMLSelectElement, 'qwen-e2e-cdp'); + }); + + expect(onModelChange).toHaveBeenCalledWith('qwen-e2e-cdp'); + }); + it('bounds the inline changed-files summary before opening review', () => { const manyFileDiff: DesktopGitDiff = { ...gitDiff, @@ -1002,6 +1039,15 @@ function setInputValue(input: HTMLInputElement, value: string): void { input.dispatchEvent(new Event('change', { bubbles: true })); } +function setSelectValue(select: HTMLSelectElement, value: string): void { + const descriptor = Object.getOwnPropertyDescriptor( + HTMLSelectElement.prototype, + 'value', + ); + descriptor?.set?.call(select, value); + select.dispatchEvent(new Event('change', { bubbles: true })); +} + const project: DesktopProject = { id: 'project-1', name: 'example-workspace', diff --git a/packages/desktop/src/renderer/stores/modelStore.test.ts b/packages/desktop/src/renderer/stores/modelStore.test.ts index 7ecd85f94..6f3fffec2 100644 --- a/packages/desktop/src/renderer/stores/modelStore.test.ts +++ b/packages/desktop/src/renderer/stores/modelStore.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it } from 'vitest'; import { createInitialModelState, modelReducer } from './modelStore.js'; +import type { DesktopUserSettings } from '../api/client.js'; describe('modelStore', () => { it('tracks loaded session model and mode state', () => { @@ -58,4 +59,96 @@ describe('modelStore', () => { expect(modeChanged.models?.currentModelId).toBe('openai/qwen-max'); expect(modeChanged.modes?.currentModeId).toBe('yolo'); }); + + it('promotes configured settings models into active session options', () => { + const withSettings = modelReducer(createInitialModelState(), { + type: 'settings_models_loaded', + settings: createSettings('qwen-e2e-cdp'), + }); + const loaded = modelReducer(withSettings, { + type: 'session_runtime_loaded', + models: { + currentModelId: 'e2e/qwen-code', + availableModels: [{ modelId: 'e2e/qwen-code', name: 'Qwen Code E2E' }], + }, + }); + + expect(loaded.models?.currentModelId).toBe('e2e/qwen-code'); + expect(loaded.models?.availableModels).toEqual([ + { modelId: 'e2e/qwen-code', name: 'Qwen Code E2E' }, + { + modelId: 'qwen-e2e-cdp', + name: 'qwen-e2e-cdp', + description: 'Configured in desktop settings', + }, + ]); + }); + + it('keeps configured settings models available across session resets', () => { + const withSettings = modelReducer(createInitialModelState(), { + type: 'settings_models_loaded', + settings: createSettings('qwen-e2e-cdp'), + }); + const reset = modelReducer(withSettings, { type: 'reset' }); + const loaded = modelReducer(reset, { + type: 'session_runtime_loaded', + models: { + currentModelId: 'e2e/qwen-code', + availableModels: [{ modelId: 'e2e/qwen-code', name: 'Qwen Code E2E' }], + }, + }); + + expect(reset.models).toBeNull(); + expect( + loaded.models?.availableModels.map((model) => model.modelId), + ).toEqual(['e2e/qwen-code', 'qwen-e2e-cdp']); + }); + + it('replaces stale configured options when settings change', () => { + const withOldSettings = modelReducer(createInitialModelState(), { + type: 'settings_models_loaded', + settings: createSettings('qwen-old'), + }); + const loaded = modelReducer(withOldSettings, { + type: 'session_runtime_loaded', + models: { + currentModelId: 'e2e/qwen-code', + availableModels: [{ modelId: 'e2e/qwen-code', name: 'Qwen Code E2E' }], + }, + }); + const withNewSettings = modelReducer(loaded, { + type: 'settings_models_loaded', + settings: createSettings('qwen-new'), + }); + + expect( + withNewSettings.models?.availableModels.map((model) => model.modelId), + ).toEqual(['e2e/qwen-code', 'qwen-new']); + }); }); + +function createSettings(model: string): DesktopUserSettings { + return { + ok: true, + settingsPath: '/tmp/settings.json', + provider: 'api-key', + selectedAuthType: 'openai', + model: { name: model }, + codingPlan: { + region: 'china', + hasApiKey: false, + version: null, + }, + openai: { + hasApiKey: true, + providers: [ + { + id: model, + name: model, + baseUrl: 'https://example.invalid/v1', + envKey: 'OPENAI_API_KEY', + }, + ], + }, + }; +} diff --git a/packages/desktop/src/renderer/stores/modelStore.ts b/packages/desktop/src/renderer/stores/modelStore.ts index 970835e04..b2efc02d0 100644 --- a/packages/desktop/src/renderer/stores/modelStore.ts +++ b/packages/desktop/src/renderer/stores/modelStore.ts @@ -6,13 +6,16 @@ import type { DesktopApprovalMode, + DesktopModelInfo, DesktopSessionModeState, DesktopSessionModelState, } from '../../shared/desktopProtocol.js'; +import type { DesktopUserSettings } from '../api/client.js'; export interface ModelState { models: DesktopSessionModelState | null; modes: DesktopSessionModeState | null; + configuredModels: DesktopModelInfo[]; savingModel: boolean; savingMode: boolean; error: string | null; @@ -24,6 +27,7 @@ export type ModelAction = models?: DesktopSessionModelState; modes?: DesktopSessionModeState; } + | { type: 'settings_models_loaded'; settings: DesktopUserSettings } | { type: 'model_save_start' } | { type: 'model_saved'; models: DesktopSessionModelState } | { type: 'mode_save_start' } @@ -37,6 +41,7 @@ export function createInitialModelState(): ModelState { return { models: null, modes: null, + configuredModels: [], savingModel: false, savingMode: false, error: null, @@ -51,17 +56,33 @@ export function modelReducer( case 'session_runtime_loaded': return { ...state, - models: action.models ?? state.models, + models: mergeConfiguredModels( + action.models ?? state.models, + state.configuredModels, + ), modes: action.modes ?? state.modes, error: null, }; + case 'settings_models_loaded': { + const configuredModels = extractConfiguredModels(action.settings); + const sessionModels = removeConfiguredModels( + state.models, + state.configuredModels, + ); + return { + ...state, + configuredModels, + models: mergeConfiguredModels(sessionModels, configuredModels), + error: null, + }; + } case 'model_save_start': return { ...state, savingModel: true, error: null }; case 'model_saved': return { ...state, savingModel: false, - models: action.models, + models: mergeConfiguredModels(action.models, state.configuredModels), error: null, }; case 'mode_save_start': @@ -95,8 +116,99 @@ export function modelReducer( error: action.message, }; case 'reset': - return createInitialModelState(); + return { + ...createInitialModelState(), + configuredModels: state.configuredModels, + }; default: return state; } } + +function mergeConfiguredModels( + models: DesktopSessionModelState | null, + configuredModels: DesktopModelInfo[], +): DesktopSessionModelState | null { + if (!models) { + return null; + } + + const availableModels = [...models.availableModels]; + for (const configuredModel of configuredModels) { + if ( + !availableModels.some( + (candidate) => candidate.modelId === configuredModel.modelId, + ) + ) { + availableModels.push(configuredModel); + } + } + + if ( + !availableModels.some( + (candidate) => candidate.modelId === models.currentModelId, + ) + ) { + availableModels.unshift({ + modelId: models.currentModelId, + name: models.currentModelId, + }); + } + + return { + ...models, + availableModels, + }; +} + +function removeConfiguredModels( + models: DesktopSessionModelState | null, + configuredModels: DesktopModelInfo[], +): DesktopSessionModelState | null { + if (!models || configuredModels.length === 0) { + return models; + } + + const configuredIds = new Set(configuredModels.map((model) => model.modelId)); + return { + ...models, + availableModels: models.availableModels.filter( + (model) => + model.modelId === models.currentModelId || + !configuredIds.has(model.modelId), + ), + }; +} + +function extractConfiguredModels( + settings: DesktopUserSettings, +): DesktopModelInfo[] { + const providers = settings.openai.providers + .map((provider) => ({ + modelId: provider.id.trim(), + name: (provider.name || provider.id).trim(), + description: 'Configured in desktop settings', + })) + .filter((model) => model.modelId.length > 0); + const activeModel = settings.model.name?.trim(); + if ( + activeModel && + !providers.some((provider) => provider.modelId === activeModel) + ) { + providers.unshift({ + modelId: activeModel, + name: activeModel, + description: 'Configured in desktop settings', + }); + } + + const seen = new Set(); + return providers.filter((provider) => { + if (seen.has(provider.modelId)) { + return false; + } + + seen.add(provider.modelId); + return true; + }); +}