From 7669fdabecbbd9688cacb1c9fc8208622982c227 Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 00:01:22 +0800 Subject: [PATCH 1/7] feat(core): add one-shot image modality probe with 3-state verdict --- .../src/services/modalityProbe/probe.test.ts | 174 ++++++++++++++++++ .../core/src/services/modalityProbe/probe.ts | 144 +++++++++++++++ 2 files changed, 318 insertions(+) create mode 100644 packages/core/src/services/modalityProbe/probe.test.ts create mode 100644 packages/core/src/services/modalityProbe/probe.ts diff --git a/packages/core/src/services/modalityProbe/probe.test.ts b/packages/core/src/services/modalityProbe/probe.test.ts new file mode 100644 index 00000000000..307a6a9e484 --- /dev/null +++ b/packages/core/src/services/modalityProbe/probe.test.ts @@ -0,0 +1,174 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + classifyProbeResponse, + probeImageSupport, + RED_PNG_DATA_URL, +} from './probe.js'; + +describe('classifyProbeResponse', () => { + it('accepts an image-bearing 200 as image', () => { + expect(classifyProbeResponse(200, '')).toEqual('image'); + }); + + it('classifies modality-semantic errors as text_only', () => { + expect( + classifyProbeResponse( + 400, + JSON.stringify({ + error: { message: 'This model does not support image' }, + }), + ), + ).toEqual('text_only'); + expect( + classifyProbeResponse( + 400, + JSON.stringify({ + error: { + code: '1210', + message: "messages.content.type 参数非法,取值范围 ['text']", + }, + }), + ), + ).toEqual('text_only'); + expect( + classifyProbeResponse( + 404, + JSON.stringify({ + error: { message: 'No endpoints found that support image input' }, + }), + ), + ).toEqual('text_only'); + expect( + classifyProbeResponse( + 400, + JSON.stringify({ + error: 'this model does not support image input (ref: 9eb0a003)', + }), + ), + ).toEqual('text_only'); + }); + + it('abstains on non-modality errors', () => { + expect( + classifyProbeResponse(401, JSON.stringify({ error: 'Unauthorized' })), + ).toEqual('unknown'); + expect( + classifyProbeResponse( + 429, + JSON.stringify({ error: { message: 'Provider returned error' } }), + ), + ).toEqual('unknown'); + expect(classifyProbeResponse(-1, 'TimeoutError: timeout')).toEqual( + 'unknown', + ); + expect( + classifyProbeResponse( + 400, + JSON.stringify({ error: { message: 'invalid model id' } }), + ), + ).toEqual('unknown'); + }); + + it('abstains on 5xx bodies that merely mention multimodal in tracebacks', () => { + expect( + classifyProbeResponse( + 500, + JSON.stringify({ + error: { + message: 'Traceback ... File "/vllm/multimodal/utils.py", line 42', + }, + }), + ), + ).toEqual('unknown'); + }); +}); + +describe('probeImageSupport', () => { + afterEach(() => vi.unstubAllGlobals()); + + it('sends image_url to the chat completions endpoint and returns the verdict', async () => { + const fetchMock = vi + .fn() + .mockResolvedValue(new Response('{}', { status: 200 })); + vi.stubGlobal('fetch', fetchMock); + const result = await probeImageSupport({ + model: 'm1', + baseUrl: 'https://api.example.com/v1', + apiKey: 'sk-test', + }); + expect(result.verdict).toEqual('image'); + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toEqual('https://api.example.com/v1/chat/completions'); + expect(init.method).toEqual('POST'); + expect((init.headers as Record)['Content-Type']).toEqual( + 'application/json', + ); + expect((init.headers as Record)['Authorization']).toEqual( + 'Bearer sk-test', + ); + const body = JSON.parse(String(init.body)) as { + messages: Array<{ + content: Array<{ type: string; image_url?: { url: string } }>; + }>; + max_tokens: number; + }; + expect( + body.messages[0]!.content.some( + (p) => p.type === 'image_url' && p.image_url!.url === RED_PNG_DATA_URL, + ), + ).toBe(true); + expect(body.max_tokens).toBeLessThanOrEqual(32); + }); + + it('normalizes a trailing slash in baseUrl', async () => { + const fetchMock = vi + .fn() + .mockResolvedValue(new Response('{}', { status: 200 })); + vi.stubGlobal('fetch', fetchMock); + const result = await probeImageSupport({ + model: 'm1', + baseUrl: 'https://api.example.com/v1/', + apiKey: 'k', + }); + expect(result.verdict).toEqual('image'); + const [url] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toEqual('https://api.example.com/v1/chat/completions'); + }); + + it('maps endpoint errors to the three-state verdict without throwing', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + error: { message: 'This model does not support image' }, + }), + { status: 400 }, + ), + ), + ); + const result = await probeImageSupport({ + model: 'm1', + baseUrl: 'https://api.example.com/v1', + apiKey: 'k', + }); + expect(result.verdict).toEqual('text_only'); + expect(result.httpStatus).toEqual(400); + }); + + it('returns unknown on network failure', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('boom'))); + const result = await probeImageSupport({ + model: 'm1', + baseUrl: 'https://api.example.com/v1', + apiKey: 'k', + }); + expect(result.verdict).toEqual('unknown'); + }); +}); diff --git a/packages/core/src/services/modalityProbe/probe.ts b/packages/core/src/services/modalityProbe/probe.ts new file mode 100644 index 00000000000..02eda092ffc --- /dev/null +++ b/packages/core/src/services/modalityProbe/probe.ts @@ -0,0 +1,144 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * One-shot image modality probe (QwenLM/qwen-code#10309, phase 1). + * + * Sends a single chat-completions request carrying a tiny red 8x8 PNG to the + * model's own endpoint and classifies the endpoint's response. Deliberately + * bypasses the content pipeline: the converter's modality gate would replace + * the image with a placeholder for pattern-guessed (text-only) models, which + * is exactly the belief under test. + * + * Verdict is based solely on acceptance — the successful response's CONTENT is + * never inspected: reasoning models routinely return an empty `content` with + * text in `reasoning_content`/`thinking` even when the image was accepted. + * Auth / rate-limit / timeout / ambiguous errors yield `unknown` (no + * conclusion) — never a wrong `text_only`, which would be cached and silently + * strip images from a vision model. + */ + +/** Error-text phrases that express a modality rejection. Observed in the wild + * (2026-08-27, four-endpoint validation — see issue #10309): DeepSeek/Ollama + * reject via error.message; Zhipu phrases it as content.type enum validation; + * OpenRouter's router returns 404 "No endpoints found that support image input". */ +const MODALITY_ERROR_HINTS = [ + 'not support', + 'text-only', + 'text only', + 'multimodal', + 'modalit', + '不支持', + '多模态', + '识图', + '无法处理图片', + 'images are not', + 'does not accept', + 'support image input', + 'content.type 参数非法', + "取值范围 ['text']", +] as const; + +/** The hints pre-lowercased once at module load, so the classification hot + * path does substring checks without re-lowercasing every hint per call. */ +const MODALITY_ERROR_HINTS_LOWER = MODALITY_ERROR_HINTS.map((hint) => + hint.toLowerCase(), +); + +/** Red 8x8 PNG as a data URL. 8x8 rather than 1x1: some endpoints enforce a + * minimum image size and would reject a 1x1 as malformed traffic, corrupting + * the verdict. */ +export const RED_PNG_DATA_URL = + 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAICAIAAABLbSncAAAAEklEQVR4nGP4z8CAFWEXHbQSACj/P8Fu7N9hAAAAAElFTkSuQmCC'; + +export type ModalityProbeVerdict = 'image' | 'text_only' | 'unknown'; + +export interface ModalityProbeInput { + readonly model: string; + readonly baseUrl: string; + readonly apiKey: string; + readonly timeoutMs?: number; +} + +export interface ModalityProbeResult { + readonly verdict: ModalityProbeVerdict; + readonly httpStatus: number; + /** Truncated response/error body — for UI display and debug logging only. */ + readonly snippet: string; +} + +export function classifyProbeResponse( + status: number, + errorText: string, +): ModalityProbeVerdict { + if (status === 200) { + return 'image'; + } + // Hints are only consulted for 4xx client errors: 5xx bodies may contain + // incidental "multimodal" text in server tracebacks (e.g. a vLLM stack + // frame path like vllm/multimodal/utils.py), and trusting those would + // classify a vision model as text_only — a verdict later tasks persist, + // silently stripping images. Anything else non-200 abstains to 'unknown' + // (the safe direction). + if (status < 400 || status >= 500) { + return 'unknown'; + } + const low = (errorText ?? '').toLowerCase(); + // Match against the raw response text, not a parsed error.message field — + // the error payload's shape itself differs per vendor (object-with-message + // vs plain string). + if (MODALITY_ERROR_HINTS_LOWER.some((hint) => low.includes(hint))) { + return 'text_only'; + } + return 'unknown'; +} + +export async function probeImageSupport( + input: ModalityProbeInput, +): Promise { + const body = { + model: input.model, + max_tokens: 24, + messages: [ + { + role: 'user', + content: [ + { type: 'text', text: '这张图片是什么颜色?' }, + { type: 'image_url', image_url: { url: RED_PNG_DATA_URL } }, + ], + }, + ], + }; + try { + const response = await fetch( + `${input.baseUrl.replace(/\/$/, '')}/chat/completions`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${input.apiKey}`, + }, + body: JSON.stringify(body), + signal: AbortSignal.timeout(input.timeoutMs ?? 90_000), + }, + ); + const text = await response.text(); + return { + verdict: classifyProbeResponse(response.status, text), + httpStatus: response.status, + snippet: text.slice(0, 200), + }; + } catch (error) { + return { + verdict: 'unknown', + httpStatus: -1, + snippet: + error instanceof Error + ? `${error.name}: ${error.message}` + : String(error), + }; + } +} From 50788820d87cef848c420f488433539139204a2b Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 00:49:30 +0800 Subject: [PATCH 2/7] feat(settings): persist modality probe verdicts under top-level probeResults key --- packages/cli/src/config/settingsSchema.ts | 14 +++++ packages/core/src/index.ts | 1 + .../modalityProbe/probe-store.test.ts | 63 +++++++++++++++++++ .../src/services/modalityProbe/probe-store.ts | 51 +++++++++++++++ .../schemas/settings.schema.json | 5 ++ 5 files changed, 134 insertions(+) create mode 100644 packages/core/src/services/modalityProbe/probe-store.test.ts create mode 100644 packages/core/src/services/modalityProbe/probe-store.ts diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index dcc405fada5..be2b57c26c6 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -13,6 +13,7 @@ import type { ChatCompressionSettings, ModelProvidersConfig, ProviderProtocolConfig, + ModalityProbeRecord, } from '@qwen-code/qwen-code-core'; import { ApprovalMode, @@ -380,6 +381,19 @@ const SETTINGS_SCHEMA = { mergeStrategy: MergeStrategy.REPLACE, }, + // Persisted modality probe results (QwenLM/qwen-code#10309, phase 1). + probeResults: { + type: 'object', + label: 'Modality Probe Results', + category: 'Model', + requiresRestart: false, + default: {} as Record, + showInDialog: false, + mergeStrategy: MergeStrategy.SHALLOW_MERGE, + description: + 'Persisted one-shot image modality probe verdicts, keyed by "authType|modelId|baseUrl". Written by the "Test image support" action in /model; feeds the modality resolution chain (explicit modalities > probe result > name-pattern table). Records are advisory metadata, never user declarations.', + }, + plansDirectory: { type: 'string', label: 'Plans Directory', diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 457c553c62a..a76bb3c22cb 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -317,6 +317,7 @@ export * from './services/fileHistoryService.js'; export * from './services/fileReadCache.js'; export * from './services/fileSystemService.js'; export * from './services/tool-write-origin.js'; +export type { ModalityProbeRecord } from './services/modalityProbe/probe-store.js'; export { decodeBufferWithEncodingInfo, encodeTextFileContent, diff --git a/packages/core/src/services/modalityProbe/probe-store.test.ts b/packages/core/src/services/modalityProbe/probe-store.test.ts new file mode 100644 index 00000000000..64a06d1d02f --- /dev/null +++ b/packages/core/src/services/modalityProbe/probe-store.test.ts @@ -0,0 +1,63 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + buildProbeKey, + readProbeResult, + withProbeResult, + type ModalityProbeRecord, +} from './probe-store.js'; + +describe('probeStore', () => { + it('builds stable composite keys', () => { + expect(buildProbeKey('openai', 'm1', 'https://api.example.com/v1')).toEqual( + 'openai|m1|https://api.example.com/v1', + ); + expect(buildProbeKey('openai', 'm1', undefined)).toEqual('openai|m1|'); + }); + + it('reads a matching record and ignores non-matching keys', () => { + const store: Record = { + 'openai|m1|https://a.example': { + verdict: 'image', + probedAt: '2026-08-27T00:00:00Z', + }, + }; + expect( + readProbeResult(store, 'openai', 'm1', 'https://a.example')?.verdict, + ).toEqual('image'); + expect( + readProbeResult(store, 'openai', 'm2', 'https://a.example'), + ).toBeUndefined(); + }); + + it('returns a new map on write (immutable read-modify-write)', () => { + const store: Record = {}; + const next = withProbeResult(store, 'openai', 'm1', '', { + verdict: 'text_only', + probedAt: '2026-08-27T00:00:00Z', + }); + expect(next['openai|m1|']).toEqual({ + verdict: 'text_only', + probedAt: '2026-08-27T00:00:00Z', + }); + expect(store).toEqual({}); + }); + + it('lets the last write win for the same key (re-probe overwrites)', () => { + const first = withProbeResult(undefined, 'openai', 'm1', '', { + verdict: 'image', + probedAt: '2026-08-27T00:00:00Z', + }); + const second = withProbeResult(first, 'openai', 'm1', '', { + verdict: 'text_only', + probedAt: '2026-08-28T00:00:00Z', + }); + expect(second['openai|m1|']?.verdict).toEqual('text_only'); + expect(Object.keys(second)).toHaveLength(1); + }); +}); diff --git a/packages/core/src/services/modalityProbe/probe-store.ts b/packages/core/src/services/modalityProbe/probe-store.ts new file mode 100644 index 00000000000..51476c0e96d --- /dev/null +++ b/packages/core/src/services/modalityProbe/probe-store.ts @@ -0,0 +1,51 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { ModalityProbeVerdict } from './probe.js'; + +/** A persisted probe verdict. `verdict` is `image` or `text_only` — `unknown` + * results are never persisted (no conclusion, nothing to cache). */ +export interface ModalityProbeRecord { + readonly verdict: Exclude; + readonly probedAt: string; +} + +export type ProbeResultStore = Record; + +/** `|` is an acceptable separator: realistic authType/modelId/baseUrl values + * never contain it, so the worst case is one wrong advisory verdict that a + * re-probe overwrites — and `\0` (used by modelRegistryKey) would be hostile + * in a human-editable settings.json. */ +export function buildProbeKey( + authType: string, + modelId: string, + baseUrl: string | undefined, +): string { + return `${authType}|${modelId}|${baseUrl ?? ''}`; +} + +export function readProbeResult( + store: ProbeResultStore | undefined, + authType: string, + modelId: string, + baseUrl: string | undefined, +): ModalityProbeRecord | undefined { + return store?.[buildProbeKey(authType, modelId, baseUrl)]; +} + +/** Read-modify-write of the whole map — the composite keys embed dots + * (hostnames in baseUrl), `|`, and `:`, which settings' dotted-path + * addressing would mis-nest, so the caller persists the returned object as + * the whole `probeResults` settings value. */ +export function withProbeResult( + store: ProbeResultStore | undefined, + authType: string, + modelId: string, + baseUrl: string | undefined, + record: ModalityProbeRecord, +): ProbeResultStore { + return { ...store, [buildProbeKey(authType, modelId, baseUrl)]: record }; +} diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index f7b456aa6b7..91688433fd7 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -49,6 +49,11 @@ "type": "object", "additionalProperties": true }, + "probeResults": { + "description": "Persisted one-shot image modality probe verdicts, keyed by \"authType|modelId|baseUrl\". Written by the \"Test image support\" action in /model; feeds the modality resolution chain (explicit modalities > probe result > name-pattern table). Records are advisory metadata, never user declarations.", + "type": "object", + "additionalProperties": true + }, "plansDirectory": { "description": "Custom directory for approved Plan Mode files. Relative paths are resolved from the project root, and the resolved path must stay within the project root. Defaults to ~/.qwen/plans.", "type": "string" From a83184925bfef69ead44ef1b3daaaa89d56b0520 Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 01:32:05 +0800 Subject: [PATCH 3/7] feat(core): probe result layer in modality resolution chain with source stamping --- packages/core/src/config/config.ts | 13 ++ packages/core/src/models/index.ts | 6 + .../src/models/modelConfigResolver.test.ts | 174 ++++++++++++++++++ .../core/src/models/modelConfigResolver.ts | 37 +++- .../core/src/models/modelRegistry.test.ts | 71 +++++++ packages/core/src/models/modelRegistry.ts | 49 ++++- packages/core/src/models/modelsConfig.test.ts | 85 +++++++++ packages/core/src/models/modelsConfig.ts | 53 +++++- packages/core/src/models/types.ts | 14 ++ .../src/services/modalityProbe/probe-store.ts | 11 +- 10 files changed, 501 insertions(+), 12 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 0d097b19574..02334187290 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -297,6 +297,7 @@ import { type AvailableModel, type ResolvedModelConfig, type RuntimeModelSnapshot, + type ProbeResultStore, } from '../models/index.js'; import { resolveModelId } from '../utils/modelId.js'; import type { WebSearchSettings } from '../tools/web-search.js'; @@ -1328,6 +1329,13 @@ export interface ConfigParameters { modelProvidersConfig?: ModelProvidersConfig; /** Maps custom provider ids to their SDK protocol (AuthType) */ providerProtocolConfig?: ProviderProtocolConfig; + /** + * Lazy provider for the persisted modality probe store (settings + * `probeResults`); feeds the modalities resolution chain (explicit > + * probe > pattern). A callback so persisted verdicts are read live rather + * than snapshotted at boot. `undefined` keeps probe-free behavior. + */ + probeResultStoreProvider?: () => ProbeResultStore | undefined; /** Agent and multi-agent collaboration settings */ agents?: AgentsCollabSettings; /** General-purpose worktree settings (Phase D-2). */ @@ -2208,6 +2216,9 @@ export class Config { private modelsConfig!: ModelsConfig; private readonly modelProvidersConfig?: ModelProvidersConfig; private readonly providerProtocolConfig?: ProviderProtocolConfig; + private readonly probeResultStoreProvider?: () => + | ProbeResultStore + | undefined; private readonly sandbox: SandboxConfig | undefined; private targetDir: string; private workspaceContext: WorkspaceContext; @@ -2718,6 +2729,7 @@ export class Config { this.ideMode = params.ideMode ?? false; this.modelProvidersConfig = params.modelProvidersConfig; this.providerProtocolConfig = params.providerProtocolConfig; + this.probeResultStoreProvider = params.probeResultStoreProvider; this.cliVersion = params.cliVersion; this.chatRecordingEnabled = params.chatRecording ?? true; @@ -2854,6 +2866,7 @@ export class Config { initialAuthType: params.authType ?? params.generationConfig?.authType, modelProvidersConfig: this.modelProvidersConfig, providerProtocolConfig: this.providerProtocolConfig, + probeResultStoreProvider: this.probeResultStoreProvider, generationConfig: { model: params.model, ...(params.generationConfig || {}), diff --git a/packages/core/src/models/index.ts b/packages/core/src/models/index.ts index a1a69b57039..4a21bf38743 100644 --- a/packages/core/src/models/index.ts +++ b/packages/core/src/models/index.ts @@ -14,8 +14,14 @@ export { type AvailableModel, type ModelSwitchMetadata, type RuntimeModelSnapshot, + type ModalitySource, } from './types.js'; +export type { + ProbeResultStore, + ModalityProbeRecord, +} from '../services/modalityProbe/probe-store.js'; + export { ModelRegistry, modelRegistryKey, diff --git a/packages/core/src/models/modelConfigResolver.test.ts b/packages/core/src/models/modelConfigResolver.test.ts index 7d8f1aa7cab..9ec8d2a4750 100644 --- a/packages/core/src/models/modelConfigResolver.test.ts +++ b/packages/core/src/models/modelConfigResolver.test.ts @@ -11,6 +11,11 @@ import { } from './modelConfigResolver.js'; import { AuthType } from '../core/contentGenerator.js'; import { DEFAULT_QWEN_MODEL, MAINLINE_CODER_MODEL } from '../config/models.js'; +import { + buildProbeKey, + withProbeResult, + type ProbeResultStore, +} from '../services/modalityProbe/probe-store.js'; describe('modelConfigResolver', () => { describe('resolveModelConfig', () => { @@ -1081,4 +1086,173 @@ describe('modelConfigResolver', () => { expect(result.sources['contextWindowSize'].kind).toBe('settings'); }); }); + + describe('modalities fallback chain (probe layer)', () => { + // Chain under test (QwenLM/qwen-code#10309 phase 1): + // explicit modalities (settings/modelProviders generationConfig) + // > persisted probe verdict (settings `probeResults` store) + // > name-pattern table (defaultModalities). + const PROBED_AT = '2026-08-27T00:00:00.000Z'; + const BASE_URL = 'http://localhost:8000/v1'; + + function probeStore( + modelId: string, + verdict: 'image' | 'text_only', + baseUrl?: string, + ): ProbeResultStore { + return withProbeResult(undefined, AuthType.USE_OPENAI, modelId, baseUrl, { + verdict, + probedAt: PROBED_AT, + }); + } + + it('explicit settings modalities win over a matching probe verdict', () => { + // 'qwen3-coder-plus' pattern-resolves to text-only ({}), so the explicit + // { image: true } below is distinguishable from BOTH lower tiers — and + // the text_only probe entry matches the exact resolution key. + const result = resolveModelConfig({ + authType: AuthType.USE_OPENAI, + cli: {}, + settings: { + generationConfig: { + modalities: { image: true }, + }, + }, + env: { + OPENAI_API_KEY: 'test-key', + OPENAI_BASE_URL: BASE_URL, + OPENAI_MODEL: 'qwen3-coder-plus', + }, + probeResultStore: probeStore('qwen3-coder-plus', 'text_only', BASE_URL), + }); + + expect(result.config.modalities).toEqual({ image: true }); + expect(result.sources['modalities'].kind).toBe('settings'); + }); + + it('probe verdict image wins over the name-pattern table', () => { + // 'qwen3-coder-plus' pattern-resolves to {} (text-only); a persisted + // 'image' verdict must flip it to { image: true }. + const result = resolveModelConfig({ + authType: AuthType.USE_OPENAI, + cli: {}, + settings: {}, + env: { + OPENAI_API_KEY: 'test-key', + OPENAI_BASE_URL: BASE_URL, + OPENAI_MODEL: 'qwen3-coder-plus', + }, + probeResultStore: probeStore('qwen3-coder-plus', 'image', BASE_URL), + }); + + expect(result.config.modalities).toEqual({ image: true }); + expect(result.sources['modalities'].kind).toBe('computed'); + expect(result.sources['modalities'].detail).toBe( + `probe-tested ${PROBED_AT}`, + ); + }); + + it('probe verdict text_only wins over the name-pattern table', () => { + // 'glm-4.5v' pattern-resolves to { image: true }; a persisted + // 'text_only' verdict must flip it to {}. Keyed WITHOUT a baseUrl — + // covers the undefined-baseUrl key spelling (no OPENAI_BASE_URL set). + const result = resolveModelConfig({ + authType: AuthType.USE_OPENAI, + cli: {}, + settings: {}, + env: { + OPENAI_API_KEY: 'test-key', + OPENAI_MODEL: 'glm-4.5v', + }, + probeResultStore: probeStore('glm-4.5v', 'text_only'), + }); + + expect(result.config.modalities).toEqual({}); + expect(result.sources['modalities'].kind).toBe('computed'); + expect(result.sources['modalities'].detail).toBe( + `probe-tested ${PROBED_AT}`, + ); + }); + + it('falls through to the pattern table when no probe entry matches', () => { + const result = resolveModelConfig({ + authType: AuthType.USE_OPENAI, + cli: {}, + settings: {}, + env: { + OPENAI_API_KEY: 'test-key', + OPENAI_BASE_URL: BASE_URL, + OPENAI_MODEL: 'glm-4.5v', + }, + }); + + expect(result.config.modalities).toEqual({ image: true }); + expect(result.sources['modalities'].kind).toBe('computed'); + expect(result.sources['modalities'].detail).toBe( + 'auto-detected from model', + ); + }); + + it('explicit modelProvider modalities win over a matching probe verdict', () => { + // Mirrors the settings-channel test above through the OTHER explicit + // channel (modelProviders): explicit { image: false } vs an 'image' + // probe verdict vs the { image: true } pattern for 'glm-4.5v'. + const result = resolveModelConfig({ + authType: AuthType.USE_OPENAI, + cli: {}, + settings: {}, + env: { + MY_CUSTOM_KEY: 'provider-key', + }, + modelProvider: { + id: 'glm-4.5v', + name: 'GLM 4.5V', + envKey: 'MY_CUSTOM_KEY', + baseUrl: 'https://provider.example.com', + generationConfig: { + modalities: { image: false }, + }, + }, + probeResultStore: probeStore( + 'glm-4.5v', + 'image', + 'https://provider.example.com', + ), + }); + + expect(result.config.modalities).toEqual({ image: false }); + expect(result.sources['modalities'].kind).toBe('modelProviders'); + }); + + it('ignores hand-corrupted probe records (invalid verdict falls through to pattern)', () => { + // Read-side hardening: a hand-edited settings.json can put garbage in + // the store; only verdicts exactly 'image'/'text_only' are honored, so + // a corrupt record abstains to the pattern tier — never a wrong answer. + const corrupted: ProbeResultStore = { + [buildProbeKey(AuthType.USE_OPENAI, 'glm-4.5v', BASE_URL)]: { + // Simulate a hand-edited record; the static type says this can't + // happen, settings.json is not type-checked. + verdict: 'garbage' as 'image', + probedAt: PROBED_AT, + }, + }; + const result = resolveModelConfig({ + authType: AuthType.USE_OPENAI, + cli: {}, + settings: {}, + env: { + OPENAI_API_KEY: 'test-key', + OPENAI_BASE_URL: BASE_URL, + OPENAI_MODEL: 'glm-4.5v', + }, + probeResultStore: corrupted, + }); + + expect(result.config.modalities).toEqual({ image: true }); + expect(result.sources['modalities'].kind).toBe('computed'); + expect(result.sources['modalities'].detail).toBe( + 'auto-detected from model', + ); + }); + }); }); diff --git a/packages/core/src/models/modelConfigResolver.ts b/packages/core/src/models/modelConfigResolver.ts index b14ba12fb59..0550b03f475 100644 --- a/packages/core/src/models/modelConfigResolver.ts +++ b/packages/core/src/models/modelConfigResolver.ts @@ -23,6 +23,8 @@ import type { ContentGeneratorConfig } from '../core/contentGenerator.js'; import { DEFAULT_QWEN_MODEL } from '../config/models.js'; import { defaultModalities } from '../core/modalityDefaults.js'; import { knownTokenLimit } from '../core/tokenLimits.js'; +import { readProbeResult } from '../services/modalityProbe/probe-store.js'; +import type { ProbeResultStore } from '../services/modalityProbe/probe-store.js'; import { resolveField, resolveOptionalField, @@ -93,6 +95,10 @@ export interface ModelConfigSourcesInput { /** Proxy URL (computed from Config) */ proxy?: string; + + /** Persisted modality probe verdicts (settings `probeResults`). Consulted + * between the explicit modalities layer and the name-pattern table. */ + probeResultStore?: ProbeResultStore; } /** @@ -275,6 +281,8 @@ export function resolveModelConfig( authType, modelProvider?.id ?? modelResult.value, sources, + input.probeResultStore, + baseUrlResult?.value, ); // ---- Env override: QWEN_CODE_API_TIMEOUT_MS ---- @@ -356,6 +364,8 @@ function resolveQwenOAuthConfig( AuthType.QWEN_OAUTH, resolvedModel, sources, + input.probeResultStore, + undefined, ); // ---- Env override: QWEN_CODE_API_TIMEOUT_MS ---- @@ -381,6 +391,8 @@ function resolveGenerationConfig( authType: AuthType | undefined, modelId: string | undefined, sources: ConfigSources, + probeResultStore?: ProbeResultStore, + baseUrl?: string, ): Partial { const result: Partial = {}; @@ -426,9 +438,30 @@ function resolveGenerationConfig( // on `modalities === undefined` to mean "unresolved" — use the sources map // (kind === 'computed' vs 'modelProviders'/'settings') if that distinction // matters. + // modalities fallback chain: explicit (handled above via settings/modelProviders + // field copying) > persisted probe result > name-pattern table. + // + // NOTE(#8558/#10309): when the API-backed model metadata catalog (#8558) + // lands, its provider-native metadata layer slots in HERE — between the + // explicit user/provider layer and the probe layer. Final chain per the + // #10309 design discussion: explicit > provider-native catalog (#8558) > + // probe result > regex table > conservative defaults. + // + // Read-side hardening: only a record whose `verdict` is exactly 'image' or + // 'text_only' is honored — hand-edited settings.json garbage abstains to the + // pattern tier, never a wrong verdict. if (result.modalities === undefined && modelId) { - result.modalities = defaultModalities(modelId); - sources['modalities'] = computedSource('auto-detected from model'); + const probe = + authType !== undefined + ? readProbeResult(probeResultStore, authType, modelId, baseUrl) + : undefined; + if (probe?.verdict === 'image' || probe?.verdict === 'text_only') { + result.modalities = probe.verdict === 'image' ? { image: true } : {}; + sources['modalities'] = computedSource(`probe-tested ${probe.probedAt}`); + } else { + result.modalities = defaultModalities(modelId); + sources['modalities'] = computedSource('auto-detected from model'); + } } return result; diff --git a/packages/core/src/models/modelRegistry.test.ts b/packages/core/src/models/modelRegistry.test.ts index 967cf4ee2b1..ffa0749566d 100644 --- a/packages/core/src/models/modelRegistry.test.ts +++ b/packages/core/src/models/modelRegistry.test.ts @@ -13,6 +13,7 @@ import { } from './modelRegistry.js'; import { AuthType } from '../core/contentGenerator.js'; import type { ModelProvidersConfig, ProviderProtocolConfig } from './types.js'; +import { withProbeResult } from '../services/modalityProbe/probe-store.js'; const debugLoggerWarnSpy = vi.hoisted(() => vi.fn()); @@ -323,6 +324,76 @@ describe('ModelRegistry', () => { video: true, }); }); + + it('stamps modalitiesSource probe when a persisted verdict matches', () => { + // The probe store is keyed by (authType, modelId, RESOLVED baseUrl) — + // here the provider entry's own baseUrl, which is already resolved. + const store = withProbeResult( + undefined, + AuthType.USE_OPENAI, + 'qwen3-coder-plus', + 'https://example.invalid', + { verdict: 'image', probedAt: '2026-08-27T00:00:00.000Z' }, + ); + const registry = new ModelRegistry( + { + openai: [ + { + id: 'qwen3-coder-plus', + name: 'Qwen3 Coder Plus', + baseUrl: 'https://example.invalid', + generationConfig: {}, + }, + ], + }, + undefined, + () => store, + ); + + const model = registry.getModel(AuthType.USE_OPENAI, 'qwen3-coder-plus'); + // Pattern table alone would say {} (text-only); the probe verdict wins. + expect(model?.generationConfig.modalities).toEqual({ image: true }); + expect(model?.modalitiesSource).toBe('probe'); + }); + + it('stamps modalitiesSource pattern when no probe verdict matches', () => { + const registry = new ModelRegistry({ + openai: [ + { + id: 'gpt-4-turbo', + name: 'GPT-4 Turbo', + baseUrl: 'https://api.openai.com/v1', + generationConfig: {}, + }, + ], + }); + + const model = registry.getModel(AuthType.USE_OPENAI, 'gpt-4-turbo'); + expect(model?.generationConfig.modalities).toEqual({ image: true }); + expect(model?.modalitiesSource).toBe('pattern'); + }); + + it('stamps modalitiesSource explicit when the provider declares modalities', () => { + // Not a canonical-forced id (MiniMax-M3 et al.), so the provider's own + // declaration survives with an 'explicit' stamp. + const registry = new ModelRegistry({ + openai: [ + { + id: 'gpt-4-turbo', + name: 'GPT-4 Turbo', + baseUrl: 'https://api.openai.com/v1', + generationConfig: { modalities: { image: true, pdf: true } }, + }, + ], + }); + + const model = registry.getModel(AuthType.USE_OPENAI, 'gpt-4-turbo'); + expect(model?.generationConfig.modalities).toEqual({ + image: true, + pdf: true, + }); + expect(model?.modalitiesSource).toBe('explicit'); + }); }); describe('hasModel', () => { diff --git a/packages/core/src/models/modelRegistry.ts b/packages/core/src/models/modelRegistry.ts index df76b55a882..b903b0ff9b7 100644 --- a/packages/core/src/models/modelRegistry.ts +++ b/packages/core/src/models/modelRegistry.ts @@ -8,12 +8,15 @@ import { AuthType } from '../core/contentGenerator.js'; import { defaultModalities } from '../core/modalityDefaults.js'; import { tokenLimit } from '../core/tokenLimits.js'; import { DEFAULT_OPENAI_BASE_URL } from '../core/openaiContentGenerator/constants.js'; +import { readProbeResult } from '../services/modalityProbe/probe-store.js'; +import type { ProbeResultStore } from '../services/modalityProbe/probe-store.js'; import { type ModelConfig, type ModelProvidersConfig, type ProviderProtocolConfig, type ResolvedModelConfig, type AvailableModel, + type ModalitySource, } from './types.js'; import { DEFAULT_QWEN_MODEL } from '../config/models.js'; import { QWEN_OAUTH_MODELS } from './constants.js'; @@ -89,6 +92,19 @@ export class ModelRegistry { /** providerId -> SDK protocol mapping; persists across reloads. */ private providerProtocolConfig: ProviderProtocolConfig; + /** + * Lazy provider for the persisted modality probe store (settings + * `probeResults`), so the CLI layer can supply live settings. Consulted at + * registration/reload time only: entries cache probe-informed modalities + * and `modalitiesSource`, so a later + * `settings.setValue('probeResults', …)` does NOT refresh already + * registered entries until a registry reload. `undefined` keeps probe-free + * behavior (pattern table only). + */ + private readonly probeResultStoreProvider?: () => + | ProbeResultStore + | undefined; + private getDefaultBaseUrl(authType: AuthType): string { switch (authType) { case AuthType.QWEN_OAUTH: @@ -103,9 +119,11 @@ export class ModelRegistry { constructor( modelProvidersConfig?: ModelProvidersConfig, providerProtocolConfig?: ProviderProtocolConfig, + probeResultStoreProvider?: () => ProbeResultStore | undefined, ) { this.modelsByAuthType = new Map(); this.providerProtocolConfig = providerProtocolConfig ?? {}; + this.probeResultStoreProvider = probeResultStoreProvider; // Always register qwen-oauth models (hard-coded, cannot be overridden) this.registerAuthTypeModels(AuthType.QWEN_OAUTH, QWEN_OAUTH_MODELS); @@ -314,11 +332,35 @@ export class ModelRegistry { // them explicitly. Without this, downstream consumers that read straight // from the registry (e.g. sub-agents via getResolvedModel) would inherit // the parent session's modalities instead of the agent's own. + // + // modalities fallback chain: explicit (provider-declared) > persisted + // probe verdict > name-pattern table; provenance is stamped on + // `modalitiesSource` for the /model dialog badge (issue #10309). The + // probe lookup is keyed by the RESOLVED baseUrl — the /model dialog + // probes with the same resolved baseUrl shown on the entry, so keys + // match. Read-side hardening: only verdicts exactly 'image'/'text_only' + // are honored (hand-edited settings.json garbage abstains to pattern). + let modalitiesSource: ModalitySource; if ( - generationConfig.modalities === undefined || - shouldUseCanonicalModalities(config.id) + generationConfig.modalities !== undefined && + !shouldUseCanonicalModalities(config.id) ) { - generationConfig.modalities = defaultModalities(config.id); + modalitiesSource = 'explicit'; + } else { + const probe = readProbeResult( + this.probeResultStoreProvider?.(), + authType, + config.id, + config.baseUrl || this.getDefaultBaseUrl(authType), + ); + if (probe?.verdict === 'image' || probe?.verdict === 'text_only') { + generationConfig.modalities = + probe.verdict === 'image' ? { image: true } : {}; + modalitiesSource = 'probe'; + } else { + generationConfig.modalities = defaultModalities(config.id); + modalitiesSource = 'pattern'; + } } return { @@ -329,6 +371,7 @@ export class ModelRegistry { ...(config.baseUrl ? { registryBaseUrl: config.baseUrl } : {}), generationConfig, capabilities: config.capabilities || {}, + modalitiesSource, }; } diff --git a/packages/core/src/models/modelsConfig.test.ts b/packages/core/src/models/modelsConfig.test.ts index 0f7ddf024dd..927fa3f357b 100644 --- a/packages/core/src/models/modelsConfig.test.ts +++ b/packages/core/src/models/modelsConfig.test.ts @@ -9,6 +9,8 @@ import { ModelsConfig } from './modelsConfig.js'; import { AuthType } from '../core/contentGenerator.js'; import type { ContentGeneratorConfig } from '../core/contentGenerator.js'; import type { ModelProvidersConfig } from './types.js'; +import { withProbeResult } from '../services/modalityProbe/probe-store.js'; +import type { ProbeResultStore } from '../services/modalityProbe/probe-store.js'; describe('ModelsConfig', () => { function deepClone(value: T): T { @@ -1627,6 +1629,89 @@ describe('ModelsConfig', () => { }); }); + it('flips raw model modalities when a persisted probe verdict matches', async () => { + // applyRawModelDerivedDefaults keys the probe by (currentAuthType, + // modelId, this._generationConfig.baseUrl) — the session-resolved + // baseUrl the raw model actually uses. + const makeConfig = (probeResultStoreProvider?: () => ProbeResultStore) => + new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + generationConfig: { + model: 'qwen3.6-plus', + baseUrl: 'https://example.invalid/v1', + }, + ...(probeResultStoreProvider ? { probeResultStoreProvider } : {}), + }); + + // Without a record the pattern tier decides: /^qwen/ → text-only. + const patternOnly = makeConfig(); + await patternOnly.setModel('qwen3.7-max'); + expect(patternOnly.getGenerationConfig().modalities).toEqual({}); + expect(patternOnly.getGenerationConfigSources()['modalities']).toEqual({ + kind: 'computed', + detail: 'auto-detected from model', + }); + + // Same raw switch with a persisted 'image' verdict under that key. + const probeStore = withProbeResult( + undefined, + AuthType.USE_OPENAI, + 'qwen3.7-max', + 'https://example.invalid/v1', + { verdict: 'image', probedAt: '2026-08-27T00:00:00.000Z' }, + ); + const probeBacked = makeConfig(() => probeStore); + await probeBacked.setModel('qwen3.7-max'); + expect(probeBacked.getGenerationConfig().modalities).toEqual({ + image: true, + }); + expect( + probeBacked.getGenerationConfigSources()['modalities'], + ).toMatchObject({ + kind: 'computed', + detail: expect.stringMatching(/^probe-tested/), + }); + }); + + it('does not let a persisted probe verdict override explicit modalities on a raw switch', async () => { + const probeStore = withProbeResult( + undefined, + AuthType.USE_OPENAI, + 'qwen3.7-max', + 'https://example.invalid/v1', + { verdict: 'text_only', probedAt: '2026-08-27T00:00:00.000Z' }, + ); + const modelsConfig = new ModelsConfig({ + initialAuthType: AuthType.USE_OPENAI, + generationConfig: { + model: 'qwen3.6-plus', + baseUrl: 'https://example.invalid/v1', + modalities: { image: true, video: true }, + }, + generationConfigSources: { + modalities: { + kind: 'settings', + settingsPath: 'model.generationConfig.modalities', + }, + }, + probeResultStoreProvider: () => probeStore, + }); + + await modelsConfig.setModel('qwen3.7-max'); + + // Explicit settings modalities outrank the probe layer: the verdict says + // text_only (and the pattern tier alone would say {}), yet neither may + // replace a kind: 'settings' field (shouldUpdateModelDerivedDefault gate). + expect(modelsConfig.getGenerationConfig().modalities).toEqual({ + image: true, + video: true, + }); + expect(modelsConfig.getGenerationConfigSources()['modalities']).toEqual({ + kind: 'settings', + settingsPath: 'model.generationConfig.modalities', + }); + }); + it('refreshes model-derived modalities when hot-switching to the default qwen-oauth model', async () => { // Start on qwen-oauth with a text-only model so modalities are empty. const modelsConfig = new ModelsConfig({ diff --git a/packages/core/src/models/modelsConfig.ts b/packages/core/src/models/modelsConfig.ts index 2c812f83cb8..4f6db84b346 100644 --- a/packages/core/src/models/modelsConfig.ts +++ b/packages/core/src/models/modelsConfig.ts @@ -19,6 +19,8 @@ import { import { createDebugLogger } from '../utils/debugLogger.js'; import { ModelRegistry } from './modelRegistry.js'; +import { readProbeResult } from '../services/modalityProbe/probe-store.js'; +import type { ProbeResultStore } from '../services/modalityProbe/probe-store.js'; import { type ModelProvidersConfig, type ProviderProtocolConfig, @@ -68,6 +70,12 @@ export interface ModelsConfigOptions { initialRegistryBaseUrl?: string | null; /** Callback when model changes require refresh */ onModelChange?: OnModelChangeCallback; + /** + * Lazy provider for the persisted modality probe store (settings + * `probeResults`); consulted between the explicit modalities layer and the + * name-pattern table. `undefined` keeps probe-free behavior. + */ + probeResultStoreProvider?: () => ProbeResultStore | undefined; } /** @@ -111,6 +119,12 @@ export class ModelsConfig { // Callback for notifying Config of model changes private onModelChange?: OnModelChangeCallback; + /** Lazy provider for the persisted modality probe store (settings + * `probeResults`); `undefined` keeps probe-free behavior. */ + private readonly probeResultStoreProvider?: () => + | ProbeResultStore + | undefined; + // Flag indicating whether authType was explicitly provided (not defaulted) private readonly authTypeWasExplicitlyProvided: boolean; @@ -159,8 +173,10 @@ export class ModelsConfig { this.modelRegistry = new ModelRegistry( options.modelProvidersConfig, options.providerProtocolConfig, + options.probeResultStoreProvider, ); this.onModelChange = options.onModelChange; + this.probeResultStoreProvider = options.probeResultStoreProvider; // Initialize generation config // Note: generationConfig.model should already be fully resolved by ModelConfigResolver @@ -438,11 +454,33 @@ export class ModelsConfig { */ private applyRawModelDerivedDefaults(modelId: string): void { if (this.shouldUpdateModelDerivedDefault('modalities')) { - this._generationConfig.modalities = defaultModalities(modelId); - this.generationConfigSources['modalities'] = { - kind: 'computed', - detail: 'auto-detected from model', - }; + // modalities fallback chain: probe verdict > name-pattern table. The + // probe lookup is keyed by the baseUrl this raw model actually uses + // (the resolver-spelled resolved baseUrl). Read-side hardening: only + // verdicts exactly 'image'/'text_only' are honored. + const probe = + this.currentAuthType !== undefined + ? readProbeResult( + this.probeResultStoreProvider?.(), + this.currentAuthType, + modelId, + this._generationConfig.baseUrl, + ) + : undefined; + if (probe?.verdict === 'image' || probe?.verdict === 'text_only') { + this._generationConfig.modalities = + probe.verdict === 'image' ? { image: true } : {}; + this.generationConfigSources['modalities'] = { + kind: 'computed', + detail: `probe-tested ${probe.probedAt}`, + }; + } else { + this._generationConfig.modalities = defaultModalities(modelId); + this.generationConfigSources['modalities'] = { + kind: 'computed', + detail: 'auto-detected from model', + }; + } } if (this.shouldUpdateModelDerivedDefault('contextWindowSize')) { @@ -932,7 +970,10 @@ export class ModelsConfig { }; } - // modalities fallback: auto-detect from model when not set by provider + // modalities fallback: auto-detect from model when not set by provider. + // Defensive only: the registry always pre-fills modalities (the explicit, + // probe, and pattern branches all assign in resolveModelConfig), so this + // never fires for registry-fed models. if (gc.modalities === undefined) { this._generationConfig.modalities = defaultModalities(model.id); this.generationConfigSources['modalities'] = { diff --git a/packages/core/src/models/types.ts b/packages/core/src/models/types.ts index d3a5c5355ed..02f5de86495 100644 --- a/packages/core/src/models/types.ts +++ b/packages/core/src/models/types.ts @@ -104,6 +104,14 @@ export type ProviderProtocolConfig = { [providerId: string]: string; }; +/** + * Provenance of a model's resolved input modalities (issue #10309): + * - 'explicit' — declared by the provider entry or settings generationConfig + * - 'probe' — one-shot endpoint probe verdict (settings `probeResults`) + * - 'pattern' — model-name regex table (`defaultModalities`) + */ +export type ModalitySource = 'explicit' | 'probe' | 'pattern'; + /** * Resolved model config with all defaults applied */ @@ -122,6 +130,12 @@ export interface ResolvedModelConfig extends ModelConfig { generationConfig: ModelGenerationConfig; /** Capabilities (always present, defaults to {}) */ capabilities: ModelCapabilities; + /** + * Provenance annotation for `generationConfig.modalities` (non-persisted): + * which tier of the resolution chain produced it. Consumed by the /model + * dialog badge (issue #10309). Absent when modalities were never resolved. + */ + modalitiesSource?: ModalitySource; } /** diff --git a/packages/core/src/services/modalityProbe/probe-store.ts b/packages/core/src/services/modalityProbe/probe-store.ts index 51476c0e96d..502df2c2673 100644 --- a/packages/core/src/services/modalityProbe/probe-store.ts +++ b/packages/core/src/services/modalityProbe/probe-store.ts @@ -18,7 +18,16 @@ export type ProbeResultStore = Record; /** `|` is an acceptable separator: realistic authType/modelId/baseUrl values * never contain it, so the worst case is one wrong advisory verdict that a * re-probe overwrites — and `\0` (used by modelRegistryKey) would be hostile - * in a human-editable settings.json. */ + * in a human-editable settings.json. + * + * Phase-1 key spelling: registry-listed /model entries are keyed by their + * RESOLVED baseUrl (default-filled, as displayed on the dialog entry); + * raw/session models are keyed by the session-resolved baseUrl (which may be + * undefined, yielding a `''` final segment). Known divergence: QWEN_OAUTH + * resolves to `''` in the resolver path but 'DYNAMIC_QWEN_OAUTH_BASE_URL' in + * the registry path — hard-coded oauth models are therefore poor probe + * candidates in phase 1. + */ export function buildProbeKey( authType: string, modelId: string, From 1753066cbdff36552f744c4ae87afa9df7430ade Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 02:33:15 +0800 Subject: [PATCH 4/7] feat(cli): test-image-support action and provenance badge in /model dialog --- .../cli/src/acp-integration/acpAgent.test.ts | 18 +- packages/cli/src/acp-integration/acpAgent.ts | 3 + .../acp-integration/acpAgent.worktree.test.ts | 1 + packages/cli/src/config/config.test.ts | 10 + packages/cli/src/config/config.ts | 30 ++ packages/cli/src/gemini.test.tsx | 3 + packages/cli/src/gemini.tsx | 3 + packages/cli/src/i18n/locales/en.js | 12 + packages/cli/src/i18n/locales/zh.js | 11 + .../src/ui/components/ModelDialog.test.tsx | 444 +++++++++++++++++- .../cli/src/ui/components/ModelDialog.tsx | 333 ++++++++++--- packages/core/src/index.ts | 13 +- packages/core/src/models/modelRegistry.ts | 3 + packages/core/src/models/types.ts | 6 + 14 files changed, 810 insertions(+), 80 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index c018858a254..b35ad9c0932 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -859,6 +859,7 @@ vi.mock('../config/loadedSettingsAdapter.js', () => ({ vi.mock('../config/config.js', () => ({ loadCliConfig: vi.fn(), buildDisabledSkillNamesProvider: vi.fn(() => () => new Set()), + buildProbeResultStoreProvider: vi.fn(() => () => undefined), SessionIdConflictError: class SessionIdConflictError extends Error { sessionId: string; constructor(sessionId: string, message: string) { @@ -3691,10 +3692,11 @@ describe('QwenAgent MCP SSE/HTTP support', () => { expect(argv).toMatchObject({ sessionId: '550e8400-e29b-41d4-a716-446655440000', }); - // Index 8 is `throwOnSessionIdConflict`: it must be true so a duplicate + // Index 9 is `throwOnSessionIdConflict`: it must be true so a duplicate // caller-supplied id throws (mapped to a RequestError) instead of - // process.exit(1)-ing the shared ACP child. - expect(vi.mocked(loadCliConfig).mock.calls[0]![8]).toBe(true); + // process.exit(1)-ing the shared ACP child. (Index 8 is the settings + // watcher; index 6 is the probe-result store provider.) + expect(vi.mocked(loadCliConfig).mock.calls[0]![9]).toBe(true); mockConnectionState.resolve(); await agentPromise; @@ -10929,6 +10931,8 @@ describe('QwenAgent MCP SSE/HTTP support', () => { '/tmp', undefined, expect.anything(), + // disabledSkillNamesProvider, probeResultStoreProvider + expect.any(Function), expect.any(Function), expect.anything(), undefined, @@ -16488,7 +16492,9 @@ describe('QwenAgent MCP SSE/HTTP support', () => { ], }); - const sessionMcpServers = vi.mocked(loadCliConfig).mock.calls[0]?.[6]; + // Arg index 7 = sessionMcpServers (index 6 is the probe-result store + // provider). + const sessionMcpServers = vi.mocked(loadCliConfig).mock.calls[0]?.[7]; const localConfig = sessionMcpServers?.['local'] as unknown as { _args: unknown[]; }; @@ -18847,7 +18853,9 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { }); vi.mocked(loadSettings).mockReturnValue(makeRestoreSettings()); vi.mocked(loadCliConfig).mockImplementation(async (...args: unknown[]) => { - const hostPolicy = args[9] as + // Index 10 is `hostPolicy` (index 9 is `throwOnSessionIdConflict`; + // index 6 is the probe-result store provider). + const hostPolicy = args[10] as | { sessionRestore?: { projectionSource: (sessionId: string) => Promise; diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 57a8b613dc6..57a26251951 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -234,6 +234,7 @@ import { z } from 'zod'; import type { CliArgs } from '../config/config.js'; import { buildDisabledSkillNamesProvider, + buildProbeResultStoreProvider, loadCliConfig, SessionIdConflictError, } from '../config/config.js'; @@ -3755,6 +3756,7 @@ class QwenAgent implements Agent { projectHooks: settings.getProjectHooks(), }, buildDisabledSkillNamesProvider(settings), + buildProbeResultStoreProvider(settings), ), ); config.setMcpTransportPool(this.mcpPool); @@ -12487,6 +12489,7 @@ class QwenAgent implements Agent { // session. ACP/Zed sessions otherwise leak persisted disabled skills // into the first at cold start. buildDisabledSkillNamesProvider(settings), + buildProbeResultStoreProvider(settings), sessionMcpServers, // The daemon owns the settings watcher lifecycle. undefined, diff --git a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts index b7199f5dc83..52e3de79559 100644 --- a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts @@ -280,6 +280,7 @@ vi.mock('../config/settings-cache.js', async () => { vi.mock('../config/config.js', () => ({ loadCliConfig: vi.fn(), buildDisabledSkillNamesProvider: vi.fn(() => () => new Set()), + buildProbeResultStoreProvider: vi.fn(() => () => undefined), })); vi.mock('./session/Session.js', () => ({ Session: vi.fn(), diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index f5402d4656f..7d49142bcd2 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -1594,6 +1594,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, sessionMcpServers, ); @@ -1626,6 +1627,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, sessionMcpServers, ); @@ -1688,6 +1690,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, sessionMcpServers, ); @@ -1749,6 +1752,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, { 'ide-only': new ServerConfig.MCPServerConfig('ide-cmd'), }, @@ -1818,6 +1822,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, false, { sessionRestore: { projectionSource } }, ); @@ -1876,6 +1881,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, false, { sessionRestore: { projectionSource } }, ); @@ -2006,6 +2012,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, true, ); @@ -2029,6 +2036,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, true, ); @@ -2050,6 +2058,7 @@ describe('loadCliConfig', () => { undefined, undefined, undefined, + undefined, true, ); @@ -3715,6 +3724,7 @@ describe('loadCliConfig with includeDirectories', () => { undefined, undefined, undefined, + undefined, false, { provisionalWorkspace: true }, ); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 6a202329663..f6b33b97fe9 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -42,6 +42,7 @@ import { SchemaValidator, type ConfigParameters, type MCPServerConfig, + type ProbeResultStore, type SkillLevel, type WebSearchSettings, MAX_SUBAGENT_DEPTH_LIMIT, @@ -1481,6 +1482,25 @@ export function buildDisabledSkillNamesProvider( return () => resolveSkillSettings(loadedSettings).disabledNames; } +/** + * Builds the live-read closure for the persisted modality probe results + * (`probeResults` settings key, QwenLM/qwen-code#10309). Forwarded to + * `ConfigParameters.probeResultStoreProvider` so the model registry's + * modality resolution chain (explicit > probe > pattern) reflects verdicts + * written by the /model dialog's "Test image support" action without + * rebuilding `Config`. + * + * Like `buildDisabledSkillNamesProvider`, the closure is over the LIVE + * `LoadedSettings` instance (reading `merged` on every call), NOT over a + * `Settings` snapshot — `LoadedSettings.setValue` replaces `_merged`, so a + * snapshot closure would never observe newly written probe verdicts. + */ +export function buildProbeResultStoreProvider( + loadedSettings: LoadedSettings, +): () => ProbeResultStore | undefined { + return () => loadedSettings.merged.probeResults; +} + /** * Thrown (instead of `process.exit(1)`) when a caller-supplied session id * already exists and `throwOnSessionIdConflict` is set. The interactive CLI @@ -1525,6 +1545,15 @@ export async function loadCliConfig( * correctly. */ disabledSkillNamesProvider?: () => ReadonlySet, + /** + * Live-read provider for the persisted modality probe results. Forwarded + * to `ConfigParameters` so the model registry's modality resolution chain + * sees probe verdicts written by the /model dialog within the same + * process. Callers MUST close over the live `LoadedSettings` instance — + * use `buildProbeResultStoreProvider(loadedSettings)` to construct it + * correctly. + */ + probeResultStoreProvider?: () => ProbeResultStore | undefined, /** * MCP servers injected by the embedding session (e.g. ACP / IDE clients). * Treated as a session-level source at the TOP of the precedence stack — above @@ -2339,6 +2368,7 @@ export async function loadCliConfig( includePartialMessages, modelProvidersConfig, providerProtocolConfig, + probeResultStoreProvider, generationConfigSources: resolvedCliConfig.sources, generationConfig: resolvedCliConfig.generationConfig, initialModelRegistryBaseUrl: resolvedCliConfig.registryBaseUrl, diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 7c68c619187..f5c957d639f 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -138,6 +138,7 @@ vi.mock('./config/config.js', () => ({ parseArguments: vi.fn().mockResolvedValue({}), isDebugMode: vi.fn(() => false), buildDisabledSkillNamesProvider: vi.fn(() => () => new Set()), + buildProbeResultStoreProvider: vi.fn(() => () => undefined), // Mirrors SESSION_ID_REGEX in ./config/config.ts; keep them in sync. isValidSessionId: vi.fn((value: string) => /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}(-agent-[a-zA-Z0-9_.-]+)?$/i.test( @@ -851,6 +852,8 @@ describe('gemini.tsx main function', () => { userHooks: undefined, projectHooks: undefined, }, + // disabledSkillNamesProvider, probeResultStoreProvider + expect.any(Function), expect.any(Function), undefined, // settingsWatcher: not started in bare mode diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 2f626d39e8c..93888ea5089 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -39,6 +39,7 @@ import { scrubAndReportInheritedLoaderEnv } from './config/shared-env-keys.js'; import { QWEN_CODE_SERVE_ENV } from './config/acp-channel-fallback.js'; import { buildDisabledSkillNamesProvider, + buildProbeResultStoreProvider, loadCliConfig, parseArguments, } from './config/config.js'; @@ -576,6 +577,7 @@ export async function main() { projectHooks: settings.getProjectHooks(), }, buildDisabledSkillNamesProvider(settings), + buildProbeResultStoreProvider(settings), ); if (!settings.merged.security?.auth?.useExternal) { @@ -872,6 +874,7 @@ export async function main() { projectHooks: settings.getProjectHooks(), }, buildDisabledSkillNamesProvider(settings), + buildProbeResultStoreProvider(settings), undefined, settingsWatcher, ); diff --git a/packages/cli/src/i18n/locales/en.js b/packages/cli/src/i18n/locales/en.js index a91b8db0220..92957f2ca76 100644 --- a/packages/cli/src/i18n/locales/en.js +++ b/packages/cli/src/i18n/locales/en.js @@ -543,6 +543,18 @@ export default { 'Using {{count}} tools': 'Using {{count}} tools', 'Enter to select, ↑↓ to navigate, Esc to close': 'Enter to select, ↑↓ to navigate, Esc to close', + 't: test image support': 't: test image support', + 'Image probe': 'Image probe', + 'probe-tested': 'probe-tested', + 'auto-detected': 'auto-detected', + manual: 'manual', + 'testing…': 'testing…', + 'accepts images': 'accepts images', + 'text only': 'text only', + 'inconclusive (auth/rate-limit/timeout) — nothing written': + 'inconclusive (auth/rate-limit/timeout) — nothing written', + 'Image probe verdict could not be saved.': + 'Image probe verdict could not be saved.', 'Esc to go back': 'Esc to go back', 'Enter to confirm, Esc to cancel': 'Enter to confirm, Esc to cancel', 'Enter to select, ↑↓ to navigate, Esc to go back': diff --git a/packages/cli/src/i18n/locales/zh.js b/packages/cli/src/i18n/locales/zh.js index 58f179d6a81..e9f54e9cffe 100644 --- a/packages/cli/src/i18n/locales/zh.js +++ b/packages/cli/src/i18n/locales/zh.js @@ -526,6 +526,17 @@ export default { 'Using {{count}} tools': '正在使用 {{count}} 个工具', 'Enter to select, ↑↓ to navigate, Esc to close': 'Enter 选择,↑↓ 导航,Esc 关闭', + 't: test image support': 't: 测试图像支持', + 'Image probe': '图像探测', + 'probe-tested': '已实测', + 'auto-detected': '自动检测', + manual: '手动设置', + 'testing…': '测试中…', + 'accepts images': '支持图像输入', + 'text only': '仅文本', + 'inconclusive (auth/rate-limit/timeout) — nothing written': + '结论不明(鉴权/限流/超时)— 未写入任何结果', + 'Image probe verdict could not be saved.': '图像探测结论保存失败。', 'Esc to go back': '按 Esc 返回', 'Enter to confirm, Esc to cancel': 'Enter 确认,Esc 取消', 'Enter to select, ↑↓ to navigate, Esc to go back': diff --git a/packages/cli/src/ui/components/ModelDialog.test.tsx b/packages/cli/src/ui/components/ModelDialog.test.tsx index 618d97f5b3f..0c1d01077f2 100644 --- a/packages/cli/src/ui/components/ModelDialog.test.tsx +++ b/packages/cli/src/ui/components/ModelDialog.test.tsx @@ -14,7 +14,11 @@ import { ConfigContext } from '../contexts/ConfigContext.js'; import { SettingsContext } from '../contexts/SettingsContext.js'; import { UIStateContext, type UIState } from '../contexts/UIStateContext.js'; import type { Config } from '@qwen-code/qwen-code-core'; -import { AuthType, DEFAULT_QWEN_MODEL } from '@qwen-code/qwen-code-core'; +import { + AuthType, + DEFAULT_QWEN_MODEL, + probeImageSupport, +} from '@qwen-code/qwen-code-core'; import type { LoadedSettings } from '../../config/settings.js'; import { SettingScope } from '../../config/settings.js'; import { getFilteredQwenModels } from '../models/availableModels.js'; @@ -28,6 +32,15 @@ vi.mock('./shared/DescriptiveRadioButtonSelect.js', () => ({ DescriptiveRadioButtonSelect: vi.fn(() => null), })); +// The "Test image support" action must hit a controlled probe, never the +// network. Everything else from core stays real. +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, probeImageSupport: vi.fn() }; +}); +const mockedProbeImageSupport = vi.mocked(probeImageSupport); + // Helper to create getAvailableModelsForAuthType mock const createMockGetAvailableModelsForAuthType = () => vi.fn((t: AuthType) => { @@ -1952,6 +1965,435 @@ describe('', () => { ); expect(mockedSelect.mock.calls[1][0].initialIndex).toBe(expectedCoderIndex); }); + + // --- Modality provenance badge + "Test image support" action (#10309) --- + + const pressT = async () => { + // The dialog registers two useKeypress handlers: [0] escape/left, + // [1] the gated 't' probe action. + await act(async () => { + mockedUseKeypress.mock.calls[1][0]({ + name: 't', + ctrl: false, + meta: false, + shift: false, + paste: false, + sequence: 't', + }); + // The probe handler is fire-and-forget async; flush its microtasks + // inside act so the verdict state updates land in this batch. + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + const patternSourceModel = { + id: 'pattern-model', + label: 'Pattern Model', + description: '', + authType: AuthType.USE_OPENAI, + baseUrl: 'https://api.example.com/v1', + envKey: 'MODEL_DIALOG_PROBE_TEST_KEY', + modalitiesSource: 'pattern', + }; + + it('badges pattern-guessed modalities as auto-detected and offers the t action', () => { + const { getByText } = renderComponent({}, { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial); + + expect(getByText('text-only · auto-detected')).toBeDefined(); + expect(getByText('t: test image support')).toBeDefined(); + }); + + it('badges probe-tested modalities without offering the t action again', () => { + const { getByText, queryByText } = renderComponent({}, { + getModel: vi.fn(() => 'vl-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [ + { + id: 'vl-model', + label: 'VL Model', + description: '', + authType: AuthType.USE_OPENAI, + modalities: { image: true }, + modalitiesSource: 'probe', + }, + ]), + } as unknown as Partial); + + expect(getByText('text · image · probe-tested')).toBeDefined(); + expect(queryByText('t: test image support')).toBeNull(); + }); + + it('badges explicitly declared modalities as manual', () => { + const { getByText } = renderComponent({}, { + getModel: vi.fn(() => 'explicit-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [ + { + id: 'explicit-model', + label: 'Explicit Model', + description: '', + authType: AuthType.USE_OPENAI, + modalities: { image: true }, + modalitiesSource: 'explicit', + }, + ]), + } as unknown as Partial); + + expect(getByText('text · image · manual')).toBeDefined(); + }); + + it('does not run the probe for non-pattern modality sources', async () => { + const { mockSettings } = renderComponent({}, { + getModel: vi.fn(() => 'explicit-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [ + { + id: 'explicit-model', + label: 'Explicit Model', + description: '', + authType: AuthType.USE_OPENAI, + baseUrl: 'https://api.example.com/v1', + envKey: 'MODEL_DIALOG_PROBE_TEST_KEY', + modalities: { image: true }, + modalitiesSource: 'explicit', + }, + ]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial); + + await pressT(); + + expect(mockedProbeImageSupport).not.toHaveBeenCalled(); + expect(mockSettings.setValue).not.toHaveBeenCalled(); + }); + + it('probes a pattern-source entry on t and persists the whole probeResults map', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; + try { + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'image', + httpStatus: 200, + snippet: 'ok', + }); + const existingRecord = { + verdict: 'text_only' as const, + probedAt: '2026-01-01T00:00:00.000Z', + }; + // Scope-targeted read: the write must start from the TARGET scope's + // own map (preserving its records) — not from the merged view. + const userSettingsFile = { + settings: { + probeResults: { + 'openai|older-model|https://older.example.com/v1': existingRecord, + }, + }, + }; + + const { getByText, mockSettings } = renderComponent( + {}, + { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + forScope: () => userSettingsFile, + } as unknown as Partial, + ); + + await pressT(); + + expect(mockedProbeImageSupport).toHaveBeenCalledTimes(1); + expect(mockedProbeImageSupport).toHaveBeenCalledWith({ + model: 'pattern-model', + baseUrl: 'https://api.example.com/v1', + apiKey: 'sk-probe-test', + }); + expect(mockSettings.setValue).toHaveBeenCalledTimes(1); + expect(mockSettings.setValue).toHaveBeenCalledWith( + SettingScope.User, + 'probeResults', + { + 'openai|older-model|https://older.example.com/v1': existingRecord, + 'openai|pattern-model|https://api.example.com/v1': { + verdict: 'image', + probedAt: expect.any(String), + }, + }, + ); + // Registry entries are not reloaded mid-dialog: BOTH the badge and the + // modality value flip to the verdict-consistent presentation from + // local dialog state. + expect(getByText('text · image · probe-tested')).toBeDefined(); + expect(getByText('accepts images')).toBeDefined(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + + it('writes nothing when the probe verdict is unknown', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; + try { + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'unknown', + httpStatus: 401, + snippet: 'unauthorized', + }); + + const { getByText, mockSettings } = renderComponent({}, { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial); + + await pressT(); + + expect(mockedProbeImageSupport).toHaveBeenCalledTimes(1); + expect(mockSettings.setValue).not.toHaveBeenCalled(); + expect( + getByText('inconclusive (auth/rate-limit/timeout) — nothing written'), + ).toBeDefined(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + + it('reports inconclusive without probing when the API key env is unset', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + // No key in the environment: the handler must bail out BEFORE any + // network attempt and write nothing. + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + try { + const { getByText, mockSettings } = renderComponent({}, { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial); + + await pressT(); + + expect(mockedProbeImageSupport).not.toHaveBeenCalled(); + expect(mockSettings.setValue).not.toHaveBeenCalled(); + expect( + getByText('inconclusive (auth/rate-limit/timeout) — nothing written'), + ).toBeDefined(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + + it('hydrates a settings-backed API key before probing', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + // The key exists only in settings.env — NOT in process.env — so the + // probe only works if the handler hydrates the env first. + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + try { + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'text_only', + httpStatus: 400, + snippet: 'does not support images', + }); + + const { getByText, mockSettings } = renderComponent( + {}, + { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + merged: { + env: { MODEL_DIALOG_PROBE_TEST_KEY: 'sk-from-env-42' }, + }, + forScope: () => ({ settings: {} }), + } as unknown as Partial, + ); + + await pressT(); + + expect(mockedProbeImageSupport).toHaveBeenCalledWith( + expect.objectContaining({ apiKey: 'sk-from-env-42' }), + ); + expect(mockSettings.setValue).toHaveBeenCalledTimes(1); + expect(getByText('text only')).toBeDefined(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + + it('displaces the verdict display when the highlight moves to another entry', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; + try { + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'image', + httpStatus: 200, + snippet: 'ok', + }); + + const { getByText, queryByText } = renderComponent( + {}, + { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [ + patternSourceModel, + { + id: 'pattern-model-b', + label: 'Pattern Model B', + description: '', + authType: AuthType.USE_OPENAI, + modalitiesSource: 'pattern', + }, + ]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + forScope: () => ({ settings: {} }), + } as unknown as Partial, + ); + + await pressT(); + // Entry A (highlighted) shows the verdict. + expect(getByText('text · image · probe-tested')).toBeDefined(); + // Move the highlight to entry B: B shows its OWN pattern badge and + // none of A's verdict display leaks onto it. + const selectProps = mockedSelect.mock.calls[0][0]; + const entryB = selectProps.items[1].value; + act(() => { + selectProps.onHighlight?.(entryB); + }); + expect(getByText('text-only · auto-detected')).toBeDefined(); + expect(queryByText('accepts images')).toBeNull(); + expect(queryByText('text · image · probe-tested')).toBeNull(); + + // Moving back to A re-shows A's (uncorrupted) verdict display. + act(() => { + selectProps.onHighlight?.(selectProps.items[0].value); + }); + expect(getByText('text · image · probe-tested')).toBeDefined(); + expect(getByText('accepts images')).toBeDefined(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + + it('surfaces a settings-write failure instead of unhandled success', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; + const setValue = vi.fn(() => { + const error = new Error('settings are read-only'); + Object.assign(error, { code: 'EACCES' }); + throw error; + }); + try { + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'image', + httpStatus: 200, + snippet: 'ok', + }); + + const { getByText, queryByText } = renderComponent( + {}, + { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + setValue, + forScope: () => ({ settings: {} }), + } as unknown as Partial, + ); + + await pressT(); + + expect(setValue).toHaveBeenCalledTimes(1); + // The failure surfaces through the dialog's error channel... + expect( + getByText((text) => + text.includes('Image probe verdict could not be saved.'), + ), + ).toBeDefined(); + // ...and no success feedback or verdict badge is shown (nothing was + // persisted, so the entry's own pattern source stays on display and + // the t action remains available for a retry). + expect(queryByText('accepts images')).toBeNull(); + expect(queryByText('text · image · probe-tested')).toBeNull(); + expect(getByText('text-only · auto-detected')).toBeDefined(); + expect(getByText('t: test image support')).toBeDefined(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); }); describe('encodeAuxModelSelector', () => { diff --git a/packages/cli/src/ui/components/ModelDialog.tsx b/packages/cli/src/ui/components/ModelDialog.tsx index 2ff084b9995..a496d8394fa 100644 --- a/packages/cli/src/ui/components/ModelDialog.tsx +++ b/packages/cli/src/ui/components/ModelDialog.tsx @@ -16,11 +16,15 @@ import { isImageCapable, isImageGenerationCapable, parseVisionModelSetting, + probeImageSupport, resolveModelId, + withProbeResult, type AvailableModel as CoreAvailableModel, type Config, type ContentGeneratorConfig, type InputModalities, + type ModalityProbeVerdict, + type ModalitySource, } from '@qwen-code/qwen-code-core'; import { SettingScope } from '../../config/settings.js'; import { useKeypress } from '../hooks/useKeypress.js'; @@ -48,6 +52,22 @@ function formatModalities(modalities?: InputModalities): string { return `${t('text')} · ${parts.join(' · ')}`; } +/** + * Modality provenance suffix for the details panel (issue #10309): the + * badge reads the resolver's `modalitiesSource` annotation — never the + * settings sources map, which labels probe-derived modalities misleadingly + * as 'modelProviders'. Undefined source renders the plain base value. + */ +function formatModalitiesWithSource( + modalities: InputModalities | undefined, + source: ModalitySource | undefined, +): string { + const base = formatModalities(modalities); + if (source === 'probe') return `${base} · ${t('probe-tested')}`; + if (source === 'pattern') return `${base} · ${t('auto-detected')}`; + return source === 'explicit' ? `${base} · ${t('manual')}` : base; +} + /** * Build a unique selection key for a model entry in the model dialog. * When baseUrl is present, it's appended after a \0 separator to ensure @@ -87,6 +107,29 @@ function parseModelSelectionKey(key: string): { return { authType, modelId: rest }; } +/** + * Selection key for a dialog entry: runtime snapshots are keyed by their + * snapshot id, registry entries by `authType::modelId[\0baseUrl]`. Every + * consumer — the option list, highlight resolution, selection handling, and + * the probe flow's `probeTargetKey` displacement guard — must agree + * byte-for-byte, so they all go through this one helper. + */ +function entrySelectionKey({ + authType, + model, + isRuntime, + snapshotId, +}: { + authType: AuthType; + model: CoreAvailableModel; + isRuntime?: boolean; + snapshotId?: string; +}): string { + return isRuntime && snapshotId + ? snapshotId + : buildModelSelectionKey(authType, model.id, model.baseUrl); +} + /** * Encode a dialog selection key into the `authType:modelId` form persisted for * the fast/vision auxiliary models (baseUrl discarded), so duplicate model ids @@ -129,13 +172,21 @@ interface ModelDialogProps { const MAX_MODEL_ITEMS_TO_SHOW = 10; // Non-list dialog chrome to reserve when capping visible model rows: outer // round border (2) + outer padding (2) + title (1) + gap before the list (1) -// + highlighted-entry detail panel (divider + up to 4 detail rows, ~6) + -// footer gap and hint text (2). The list intentionally omits the ▲/▼ scroll -// indicators other list dialogs enable: they are two always-rendered chrome -// rows, and in a height-capped dialog those rows are better spent on two +// + highlighted-entry detail panel (divider + up to 4 standing detail rows, +// ~6) + footer gap and hint text (2). The list intentionally omits the ▲/▼ +// scroll indicators other list dialogs enable: they are two always-rendered +// chrome rows, and in a height-capped dialog those rows are better spent on two // more entries — the entry numbering already shows where the visible window // sits in the list. Adjust this whenever the surrounding layout changes, and // re-verify with an E2E height sweep rather than guessing. +// +// Two CONDITIONAL rows are deliberately NOT reserved in this budget (phase-1 +// slack; dynamic counting is a phase-2 nicety): a 5th detail row — the Image +// probe feedback, rendered while a probe/verdict for the highlighted entry is +// on screen — and a second footer hint line (`t: test image support`, +// rendered whenever a pattern-source entry is highlighted; that one is NOT +// transient). When either renders, a height-capped dialog grows by that row +// rather than dropping a model entry. const MODEL_DIALOG_FIXED_ROWS = 14; const MODEL_OPTION_ROW_HEIGHT = 1; const MODEL_OPTION_ROW_HEIGHT_WITH_DESCRIPTION = 2; @@ -398,64 +449,58 @@ export function ModelDialog({ const MODEL_OPTIONS = useMemo( () => - availableModelEntries.map( - ({ authType: t2, model, isRuntime, snapshotId }) => { - const value = - isRuntime && snapshotId - ? snapshotId - : buildModelSelectionKey(t2, model.id, model.baseUrl); - - const isQwenOAuth = t2 === AuthType.QWEN_OAUTH; - - const title = ( - - { + const { authType: t2, model, isRuntime } = entry; + const value = entrySelectionKey(entry); + + const isQwenOAuth = t2 === AuthType.QWEN_OAUTH; + + const title = ( + + - [{t2}] - - {` ${model.label}`} - {model.id !== model.label && ( - - {' '} - ({model.id}) - - )} - {isRuntime && ( - (Runtime) - )} - {isQwenOAuth && !isRuntime && ( - ({t('Discontinued')}) - )} + : theme.text.accent + } + > + [{t2}] - ); + {` ${model.label}`} + {model.id !== model.label && ( + + {' '} + ({model.id}) + + )} + {isRuntime && (Runtime)} + {isQwenOAuth && !isRuntime && ( + ({t('Discontinued')}) + )} + + ); - // Include runtime / discontinued indicator in description - let description = model.description || ''; - if (isRuntime) { - description = description - ? `${description} (Runtime)` - : 'Runtime model'; - } - if (isQwenOAuth && !isRuntime) { - description = t('Discontinued — switch to Coding Plan or API Key'); - } + // Include runtime / discontinued indicator in description + let description = model.description || ''; + if (isRuntime) { + description = description + ? `${description} (Runtime)` + : 'Runtime model'; + } + if (isQwenOAuth && !isRuntime) { + description = t('Discontinued — switch to Coding Plan or API Key'); + } - return { - value, - title, - description, - key: value, - }; - }, - ), + return { + value, + title, + description, + key: value, + }; + }), [availableModelEntries], ); const modelOptionRowHeight = MODEL_OPTIONS.some( @@ -734,28 +779,156 @@ export function ModelDialog({ const highlightedEntry = useMemo(() => { const key = highlightedValue ?? preferredKey; return availableModelEntries.find( - ({ authType: t2, model, isRuntime, snapshotId }) => { - const v = - isRuntime && snapshotId - ? snapshotId - : buildModelSelectionKey(t2, model.id, model.baseUrl); - return v === key; - }, + (entry) => entrySelectionKey(entry) === key, ); }, [highlightedValue, preferredKey, availableModelEntries]); + // One-shot image modality probe (issue #10309, phase 1). `probeTargetKey` + // remembers WHICH entry a pending/finished verdict belongs to, so moving + // the highlight mid-probe never shows another entry's result. + const [probeState, setProbeState] = useState< + 'idle' | 'probing' | ModalityProbeVerdict + >('idle'); + const [probeTargetKey, setProbeTargetKey] = useState(null); + + const highlightedEntryKey = highlightedEntry + ? entrySelectionKey(highlightedEntry) + : null; + const activeProbeState = + probeTargetKey !== null && probeTargetKey === highlightedEntryKey + ? probeState + : 'idle'; + + // The `t` action only applies to regex-guessed (pattern-source) + // modalities: explicit declarations need no probe, probe-derived entries + // already carry a persisted verdict, and QWEN_OAUTH's two probe-key + // spellings diverge in phase 1 (see probe-store.ts), so it is excluded. + // Runtime models have no modalitiesSource and are excluded by the same + // check. A probe in flight disables re-trigger globally so two concurrent + // probes can never race the whole-map read-modify-write. + const canTestImageSupport = + !!highlightedEntry && + !highlightedEntry.isRuntime && + highlightedEntry.authType !== AuthType.QWEN_OAUTH && + highlightedEntry.model.modalitiesSource === 'pattern' && + probeState !== 'probing'; + + const handleTestImageSupport = useCallback(async () => { + if (!highlightedEntry || probeState === 'probing') return; + const { model } = highlightedEntry; + setErrorMessage(null); + setProbeState('probing'); + setProbeTargetKey(entrySelectionKey(highlightedEntry)); + // Settings-backed keys are not in process.env until hydrated — the same + // in-file helper handleSelect uses before switching models — so mid- + // session settings.env keys work without a restart. The API key is read + // from the environment at probe time only — never displayed, never + // logged, never persisted. + hydrateApiKeyEnvFromSettings(settings, model.envKey); + const apiKey = model.envKey ? process.env[model.envKey] : undefined; + if (!apiKey || !model.baseUrl) { + setProbeState('unknown'); + return; + } + const result = await probeImageSupport({ + model: model.id, + baseUrl: model.baseUrl, + apiKey, + }); + if (result.verdict !== 'unknown') { + // Read the TARGET scope's own map (not the merged view) so records + // from other scopes never bleed into this write, and always write the + // WHOLE map under the single 'probeResults' key — composite probe + // keys embed dots and '|' that settings' dotted-path addressing would + // mis-nest. + const scope = resolvePersistScope(settings, persistScope); + try { + settings.setValue( + scope, + 'probeResults', + withProbeResult( + settings.forScope(scope).settings.probeResults, + highlightedEntry.authType, + model.id, + model.baseUrl, + { verdict: result.verdict, probedAt: new Date().toISOString() }, + ), + ); + } catch (e) { + // setValue can throw (saveSettings re-throws fs errors) and this + // handler runs fire-and-forget, so an uncaught throw would be an + // unhandled rejection while the UI shows success. Surface the + // failure through the dialog's error channel (the same ✕ box + // handleSelect uses) and reset the probe display: the feedback row + // is hidden and the badge falls back to the entry's own (registry) + // source, which stays truthful because nothing was persisted. + const message = e instanceof Error ? e.message : String(e); + setProbeState('idle'); + setErrorMessage( + `${t('Image probe verdict could not be saved.')}\n\n${message}`, + ); + return; + } + } + setProbeState(result.verdict); + }, [highlightedEntry, persistScope, probeState, settings]); + + // Registered separately from the escape handler above: this one depends + // on `highlightedEntry` and the probe state, which are computed later in + // the component body. + useKeypress( + (key) => { + if ( + key.name === 't' && + !key.ctrl && + !key.meta && + !key.shift && + canTestImageSupport + ) { + void handleTestImageSupport(); + } + }, + { isActive: true }, + ); + + // Registry entries cache modalities/modalitiesSource — a plain + // settings.setValue does not refresh them without a registry reload, which + // we deliberately do NOT trigger mid-dialog. While a final verdict for the + // highlighted entry is on screen, locally derive BOTH the badge source and + // the modality value from it, so the panel never contradicts itself (e.g. + // `text-only · probe-tested` above `accepts images`); other entries' + // badges refresh the next time the dialog opens. + const displayedModalitiesSource: ModalitySource | undefined = + activeProbeState === 'image' || activeProbeState === 'text_only' + ? 'probe' + : highlightedEntry?.model.modalitiesSource; + const displayedModalities: InputModalities | undefined = + activeProbeState === 'image' + ? { ...highlightedEntry?.model.modalities, image: true } + : activeProbeState === 'text_only' && highlightedEntry?.model.modalities + ? { ...highlightedEntry.model.modalities, image: false } + : highlightedEntry?.model.modalities; + + const probeFeedback: { text: string; color: string } | undefined = + activeProbeState === 'probing' + ? { text: t('testing…'), color: theme.text.secondary } + : activeProbeState === 'unknown' + ? { + text: t('inconclusive (auth/rate-limit/timeout) — nothing written'), + color: theme.status.warning, + } + : activeProbeState === 'image' + ? { text: t('accepts images'), color: theme.status.success } + : activeProbeState === 'text_only' + ? { text: t('text only'), color: theme.text.secondary } + : undefined; + const handleSelect = useCallback( async (selected: string) => { if (selectionInFlightRef.current || selectionCommittedRef.current) return; setErrorMessage(null); const selectedEntry = availableModelEntries.find( - ({ authType: t2, model, isRuntime, snapshotId }) => { - const value = - isRuntime && snapshotId - ? snapshotId - : buildModelSelectionKey(t2, model.id, model.baseUrl); - return value === selected; - }, + (entry) => entrySelectionKey(entry) === selected, ); if (isVoiceModelMode) { @@ -1152,8 +1325,19 @@ export function ModelDialog({ )} + {probeFeedback && ( + {probeFeedback.text} + } + /> + )} {t('Enter to select, ↑↓ to navigate, Esc to close')} + {canTestImageSupport && ( + {t('t: test image support')} + )} ); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a76bb3c22cb..70ea2863c7b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -31,6 +31,7 @@ export { type ModelConfigSettingsInput, type ModelConfigSourcesInput, type ModelConfigValidationResult, + type ModalitySource, ModelRegistry, isImageGenerationCapable, modelRegistryKey, @@ -317,7 +318,17 @@ export * from './services/fileHistoryService.js'; export * from './services/fileReadCache.js'; export * from './services/fileSystemService.js'; export * from './services/tool-write-origin.js'; -export type { ModalityProbeRecord } from './services/modalityProbe/probe-store.js'; +export { + type ModalityProbeRecord, + type ProbeResultStore, + withProbeResult, +} from './services/modalityProbe/probe-store.js'; +export { + type ModalityProbeInput, + type ModalityProbeResult, + type ModalityProbeVerdict, + probeImageSupport, +} from './services/modalityProbe/probe.js'; export { decodeBufferWithEncodingInfo, encodeTextFileContent, diff --git a/packages/core/src/models/modelRegistry.ts b/packages/core/src/models/modelRegistry.ts index b903b0ff9b7..f8d501e554b 100644 --- a/packages/core/src/models/modelRegistry.ts +++ b/packages/core/src/models/modelRegistry.ts @@ -246,6 +246,9 @@ export class ModelRegistry { // `modalities` is auto-filled in `resolveModelConfig`, so it is // always defined on `ResolvedModelConfig` — no fallback needed here. modalities: model.generationConfig.modalities, + ...(model.modalitiesSource !== undefined + ? { modalitiesSource: model.modalitiesSource } + : {}), baseUrl: model.baseUrl, ...(model.registryBaseUrl !== undefined ? { registryBaseUrl: model.registryBaseUrl } diff --git a/packages/core/src/models/types.ts b/packages/core/src/models/types.ts index 02f5de86495..b3743f57e05 100644 --- a/packages/core/src/models/types.ts +++ b/packages/core/src/models/types.ts @@ -150,6 +150,12 @@ export interface AvailableModel { isVision?: boolean; contextWindowSize?: number; modalities?: InputModalities; + /** + * Provenance of `modalities` mirrored from `ResolvedModelConfig` so the + * /model dialog can badge probe-tested vs pattern-guessed entries + * (issue #10309). Absent when modalities were never resolved. + */ + modalitiesSource?: ModalitySource; baseUrl?: string; /** Exact optional baseUrl used in the model registry key, before defaults. */ registryBaseUrl?: string; From 0052e789ee2f8bf950cf5bc5be1d9fc5731d813b Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 08:09:26 +0800 Subject: [PATCH 5/7] fix(cli): live probe-result source for badge and gating; tighten dialect hints --- .../src/ui/components/ModelDialog.test.tsx | 155 +++++++++++++++++- .../cli/src/ui/components/ModelDialog.tsx | 80 +++++++-- packages/core/src/index.ts | 1 + .../modalityProbe/probe-store.test.ts | 19 +++ .../src/services/modalityProbe/probe-store.ts | 17 +- .../src/services/modalityProbe/probe.test.ts | 35 ++++ .../core/src/services/modalityProbe/probe.ts | 13 +- 7 files changed, 298 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/ui/components/ModelDialog.test.tsx b/packages/cli/src/ui/components/ModelDialog.test.tsx index 0c1d01077f2..1a002fd0d45 100644 --- a/packages/cli/src/ui/components/ModelDialog.test.tsx +++ b/packages/cli/src/ui/components/ModelDialog.test.tsx @@ -1969,10 +1969,13 @@ describe('', () => { // --- Modality provenance badge + "Test image support" action (#10309) --- const pressT = async () => { - // The dialog registers two useKeypress handlers: [0] escape/left, - // [1] the gated 't' probe action. + // The dialog registers two useKeypress handlers per render: escape/left + // first, then the gated 't' probe action. Handlers accumulate across + // re-renders and remounts, so always fire the LATEST registered one — + // its closure holds the current mount's props and state. + const calls = mockedUseKeypress.mock.calls; await act(async () => { - mockedUseKeypress.mock.calls[1][0]({ + calls[calls.length - 1][0]({ name: 't', ctrl: false, meta: false, @@ -2032,6 +2035,87 @@ describe('', () => { expect(queryByText('t: test image support')).toBeNull(); }); + it('prefers a settings-persisted probe verdict over the registry pattern cache', () => { + // The registry cached modalitiesSource at registration time and a plain + // settings.setValue does not refresh it — but the dialog reads the live + // settings store, so the persisted verdict must drive BOTH the badge and + // the t-action gating even while the entry still says 'pattern'. + const { getByText, queryByText } = renderComponent( + {}, + { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + merged: { + probeResults: { + 'openai|pattern-model|https://api.example.com/v1': { + verdict: 'text_only', + probedAt: '2026-01-01T00:00:00.000Z', + }, + }, + }, + } as unknown as Partial, + ); + + expect(getByText('text-only · probe-tested')).toBeDefined(); + expect(queryByText('t: test image support')).toBeNull(); + }); + + it('shows a hand-written explicit declaration over a stale persisted probe record', () => { + // A wrong verdict was persisted earlier; the phase-1 remediation is to + // hand-write modelProviders modalities and reload the registry, which + // stamps the entry 'explicit'. The live probe read must not shadow + // that: badge shows manual with the hand-written value. + const { getByText, queryByText } = renderComponent( + {}, + { + getModel: vi.fn(() => 'explicit-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [ + { + id: 'explicit-model', + label: 'Explicit Model', + description: '', + authType: AuthType.USE_OPENAI, + baseUrl: 'https://api.example.com/v1', + envKey: 'MODEL_DIALOG_PROBE_TEST_KEY', + modalities: { image: true }, + modalitiesSource: 'explicit', + }, + ]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + merged: { + probeResults: { + // Stale text_only verdict under the SAME key the dialog's live + // read would use — it must not flip the hand-written value. + 'openai|explicit-model|https://api.example.com/v1': { + verdict: 'text_only', + probedAt: '2026-01-01T00:00:00.000Z', + }, + }, + }, + } as unknown as Partial, + ); + + expect(getByText('text · image · manual')).toBeDefined(); + expect(queryByText('text-only · probe-tested')).toBeNull(); + // Explicit entries never offer the t action, record or not. + expect(queryByText('t: test image support')).toBeNull(); + }); + it('badges explicitly declared modalities as manual', () => { const { getByText } = renderComponent({}, { getModel: vi.fn(() => 'explicit-model'), @@ -2154,6 +2238,71 @@ describe('', () => { } }); + it('hides the t action once a verdict concludes and keeps it after unknown', async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; + const renderPatternDialog = () => + renderComponent( + {}, + { + getModel: vi.fn(() => 'pattern-model'), + getAuthType: vi.fn(() => AuthType.USE_OPENAI), + getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getModelsConfig: vi.fn(() => ({ + getGenerationConfig: vi.fn(() => ({ + baseUrl: 'https://api.example.com/v1', + })), + })), + } as unknown as Partial, + { + forScope: () => ({ settings: {} }), + } as unknown as Partial, + ); + try { + // Concluded verdict: the t action must disappear. Note the mocked + // setValue does NOT refresh settings.merged, so the live-settings + // lookup still misses — only the local verdict state gates the action + // here, which is exactly the fallback under test. + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'image', + httpStatus: 200, + snippet: 'ok', + }); + const first = renderPatternDialog(); + + await pressT(); + + expect(first.getByText('text · image · probe-tested')).toBeDefined(); + expect(first.queryByText('t: test image support')).toBeNull(); + first.unmount(); + + // Unknown verdict: nothing was written and no conclusion exists, so + // retry stays available (phase 1 has no other re-probe entry point). + mockedProbeImageSupport.mockResolvedValue({ + verdict: 'unknown', + httpStatus: 429, + snippet: 'rate limited', + }); + const second = renderPatternDialog(); + + await pressT(); + + expect( + second.getByText( + 'inconclusive (auth/rate-limit/timeout) — nothing written', + ), + ).toBeDefined(); + expect(second.getByText('t: test image support')).toBeDefined(); + second.unmount(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + it('writes nothing when the probe verdict is unknown', async () => { const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; diff --git a/packages/cli/src/ui/components/ModelDialog.tsx b/packages/cli/src/ui/components/ModelDialog.tsx index a496d8394fa..eb2653acb9f 100644 --- a/packages/cli/src/ui/components/ModelDialog.tsx +++ b/packages/cli/src/ui/components/ModelDialog.tsx @@ -17,6 +17,7 @@ import { isImageGenerationCapable, parseVisionModelSetting, probeImageSupport, + readProbeResult, resolveModelId, withProbeResult, type AvailableModel as CoreAvailableModel, @@ -799,19 +800,58 @@ export function ModelDialog({ ? probeState : 'idle'; + // Live probe-verdict source: the registry caches modalitiesSource at + // registration/reload time (a plain settings.setValue does NOT refresh + // already-registered entries), so a verdict concluded earlier in this + // session — or written by an earlier dialog session — is visible here only + // through the settings store itself. Keyed the same way the write path + // keys it (declared/resolved entry baseUrl). Read-side hardening mirrors + // the registry: only verdicts exactly 'image'/'text_only' are honored; + // hand-edited garbage abstains to the entry's own source. + // + // The live read only applies to STILL-PATTERN-CACHED entries. An + // 'explicit'-stamped entry got there because the user hand-wrote + // modelProviders modalities — the phase-1 remediation exit for a wrong + // verdict — and a stale probe record must not shadow it in the UI. A + // 'probe'-stamped entry loses nothing either: phase 1 has no re-probe + // path, so the live store can never hold a newer conclusion than the + // registration-time stamp. + const liveRawVerdict = + highlightedEntry && + !highlightedEntry.isRuntime && + highlightedEntry.model.modalitiesSource === 'pattern' + ? readProbeResult( + settings.merged?.probeResults, + highlightedEntry.authType, + highlightedEntry.model.id, + highlightedEntry.model.baseUrl, + )?.verdict + : undefined; + const liveProbeVerdict: 'image' | 'text_only' | undefined = + liveRawVerdict === 'image' || liveRawVerdict === 'text_only' + ? liveRawVerdict + : undefined; + // The `t` action only applies to regex-guessed (pattern-source) - // modalities: explicit declarations need no probe, probe-derived entries - // already carry a persisted verdict, and QWEN_OAUTH's two probe-key + // modalities: explicit declarations need no probe, and entries carrying a + // conclusion — registry-stamped 'probe' source OR a live settings hit for + // a still-pattern-cached entry — need none; QWEN_OAUTH's two probe-key // spellings diverge in phase 1 (see probe-store.ts), so it is excluded. // Runtime models have no modalitiesSource and are excluded by the same // check. A probe in flight disables re-trigger globally so two concurrent - // probes can never race the whole-map read-modify-write. + // probes can never race the whole-map read-modify-write, and a CONCLUDED + // local verdict hides the action too ('unknown' keeps it — retry is the + // only recourse for an inconclusive probe in phase 1). const canTestImageSupport = !!highlightedEntry && !highlightedEntry.isRuntime && highlightedEntry.authType !== AuthType.QWEN_OAUTH && - highlightedEntry.model.modalitiesSource === 'pattern' && - probeState !== 'probing'; + (liveProbeVerdict === undefined + ? highlightedEntry.model.modalitiesSource + : 'probe') === 'pattern' && + probeState !== 'probing' && + activeProbeState !== 'image' && + activeProbeState !== 'text_only'; const handleTestImageSupport = useCallback(async () => { if (!highlightedEntry || probeState === 'probing') return; @@ -891,21 +931,31 @@ export function ModelDialog({ { isActive: true }, ); - // Registry entries cache modalities/modalitiesSource — a plain - // settings.setValue does not refresh them without a registry reload, which - // we deliberately do NOT trigger mid-dialog. While a final verdict for the - // highlighted entry is on screen, locally derive BOTH the badge source and - // the modality value from it, so the panel never contradicts itself (e.g. - // `text-only · probe-tested` above `accepts images`); other entries' - // badges refresh the next time the dialog opens. - const displayedModalitiesSource: ModalitySource | undefined = + // Modality badge/value provenance is a two-layer source. Layer 1 is the + // registry's registration-time cache (`modalitiesSource`), which a plain + // settings.setValue does not refresh without a registry reload — and we + // deliberately do NOT reload mid-dialog. Layer 2 is the live settings + // store (`probeResults`), read on every render for the highlighted entry + // — but ONLY while that entry is still pattern-cached; 'explicit' and + // 'probe' stamps show their own value (a hand-written explicit + // declaration is the phase-1 way out of a wrong verdict, so it must not + // be shadowed by the stale probe record underneath). A local verdict from + // THIS dialog's probe overrides both, so the panel never contradicts + // itself mid-feedback (e.g. `text-only · probe-tested` above + // `accepts images`). + const displayedProbeVerdict: 'image' | 'text_only' | undefined = activeProbeState === 'image' || activeProbeState === 'text_only' + ? activeProbeState + : liveProbeVerdict; + const displayedModalitiesSource: ModalitySource | undefined = + displayedProbeVerdict !== undefined ? 'probe' : highlightedEntry?.model.modalitiesSource; const displayedModalities: InputModalities | undefined = - activeProbeState === 'image' + displayedProbeVerdict === 'image' ? { ...highlightedEntry?.model.modalities, image: true } - : activeProbeState === 'text_only' && highlightedEntry?.model.modalities + : displayedProbeVerdict === 'text_only' && + highlightedEntry?.model.modalities ? { ...highlightedEntry.model.modalities, image: false } : highlightedEntry?.model.modalities; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 70ea2863c7b..a63e8a2bd9b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -321,6 +321,7 @@ export * from './services/tool-write-origin.js'; export { type ModalityProbeRecord, type ProbeResultStore, + readProbeResult, withProbeResult, } from './services/modalityProbe/probe-store.js'; export { diff --git a/packages/core/src/services/modalityProbe/probe-store.test.ts b/packages/core/src/services/modalityProbe/probe-store.test.ts index 64a06d1d02f..ca4948cff43 100644 --- a/packages/core/src/services/modalityProbe/probe-store.test.ts +++ b/packages/core/src/services/modalityProbe/probe-store.test.ts @@ -10,6 +10,7 @@ import { readProbeResult, withProbeResult, type ModalityProbeRecord, + type ProbeResultStore, } from './probe-store.js'; describe('probeStore', () => { @@ -60,4 +61,22 @@ describe('probeStore', () => { expect(second['openai|m1|']?.verdict).toEqual('text_only'); expect(Object.keys(second)).toHaveLength(1); }); + + it('treats a hand-corrupted non-object store as empty on write', () => { + // Spreading a raw string would materialize index keys ("0", "1", ...) + // and persist them; the write must start from an empty map instead. + const record: ModalityProbeRecord = { + verdict: 'image', + probedAt: '2026-08-28T00:00:00Z', + }; + const next = withProbeResult( + 'corrupted' as unknown as ProbeResultStore, + 'openai', + 'm1', + '', + record, + ); + expect(next).toEqual({ 'openai|m1|': record }); + expect(Object.keys(next)).toHaveLength(1); + }); }); diff --git a/packages/core/src/services/modalityProbe/probe-store.ts b/packages/core/src/services/modalityProbe/probe-store.ts index 502df2c2673..273effb1d94 100644 --- a/packages/core/src/services/modalityProbe/probe-store.ts +++ b/packages/core/src/services/modalityProbe/probe-store.ts @@ -27,6 +27,13 @@ export type ProbeResultStore = Record; * resolves to `''` in the resolver path but 'DYNAMIC_QWEN_OAUTH_BASE_URL' in * the registry path — hard-coded oauth models are therefore poor probe * candidates in phase 1. + * + * Another known divergence (phase-1 boundary, see the Draft PR description): + * under baseUrl environment overrides (e.g. OPENAI_BASE_URL), the resolver + * composes keys from the RESOLVED baseUrl while the registry/dialog compose + * keys from the DECLARED value — a record written via one path can be missed + * by the other. Advisory-only impact (a missed record falls back to the + * pattern tier); revisit when a re-probe/reset flow lands. */ export function buildProbeKey( authType: string, @@ -56,5 +63,13 @@ export function withProbeResult( baseUrl: string | undefined, record: ModalityProbeRecord, ): ProbeResultStore { - return { ...store, [buildProbeKey(authType, modelId, baseUrl)]: record }; + // Write-side hardening: settings.json is human-editable, so `probeResults` + // may be hand-corrupted into a non-object (e.g. a bare string). Spreading + // such a value would materialize index keys ("0", "1", ...) and persist + // them, so treat anything that is not a plain object as empty. + const base = + typeof store === 'object' && store !== null && !Array.isArray(store) + ? store + : {}; + return { ...base, [buildProbeKey(authType, modelId, baseUrl)]: record }; } diff --git a/packages/core/src/services/modalityProbe/probe.test.ts b/packages/core/src/services/modalityProbe/probe.test.ts index 307a6a9e484..7b203cab1dc 100644 --- a/packages/core/src/services/modalityProbe/probe.test.ts +++ b/packages/core/src/services/modalityProbe/probe.test.ts @@ -52,6 +52,41 @@ describe('classifyProbeResponse', () => { }), ), ).toEqual('text_only'); + expect( + classifyProbeResponse( + 400, + JSON.stringify({ error: { message: '当前模型不支持图片输入' } }), + ), + ).toEqual('text_only'); + expect( + classifyProbeResponse( + 400, + JSON.stringify({ error: { message: '该模型不支持图像理解' } }), + ), + ).toEqual('text_only'); + }); + + it('abstains on region-unsupported errors despite "not supported" phrasing', () => { + // Objectless "not supported" is entitlement/region vocabulary, not a + // modality rejection — a wrong text_only here would be persisted with no + // re-probe escape hatch in phase 1. + expect( + classifyProbeResponse( + 403, + JSON.stringify({ + error: { message: 'Model o1 is not supported in your region' }, + }), + ), + ).toEqual('unknown'); + }); + + it('abstains on Chinese region-unsupported errors despite "不支持" phrasing', () => { + expect( + classifyProbeResponse( + 400, + JSON.stringify({ error: { message: '此模型在您的区域不受支持' } }), + ), + ).toEqual('unknown'); }); it('abstains on non-modality errors', () => { diff --git a/packages/core/src/services/modalityProbe/probe.ts b/packages/core/src/services/modalityProbe/probe.ts index 02eda092ffc..34d47412a0a 100644 --- a/packages/core/src/services/modalityProbe/probe.ts +++ b/packages/core/src/services/modalityProbe/probe.ts @@ -24,14 +24,21 @@ /** Error-text phrases that express a modality rejection. Observed in the wild * (2026-08-27, four-endpoint validation — see issue #10309): DeepSeek/Ollama * reject via error.message; Zhipu phrases it as content.type enum validation; - * OpenRouter's router returns 404 "No endpoints found that support image input". */ + * OpenRouter's router returns 404 "No endpoints found that support image input". + * + * Phrases carry their object ("...image", "不支持图片/图像"): objectless + * variants ("not supported", "不支持") also appear in region/entitlement + * errors like "not supported in your region" (403) or "此模型在您的区域不受 + * 支持" (400), and matching those would persist a wrong text_only verdict — + * phase 1 has no re-probe escape hatch once a verdict is written. */ const MODALITY_ERROR_HINTS = [ - 'not support', + 'not support image', 'text-only', 'text only', 'multimodal', 'modalit', - '不支持', + '不支持图片', + '不支持图像', '多模态', '识图', '无法处理图片', From e235bb9676a49e7bd922fbe7d001da2f58adb8a9 Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 09:27:55 +0800 Subject: [PATCH 6/7] refactor(cli): extract useImageSupportProbe hook and dedupe dialog test scaffolding --- .../src/ui/components/ModelDialog.test.tsx | 478 +++++++----------- .../cli/src/ui/components/ModelDialog.tsx | 210 +------- .../src/ui/hooks/use-image-support-probe.ts | 272 ++++++++++ 3 files changed, 467 insertions(+), 493 deletions(-) create mode 100644 packages/cli/src/ui/hooks/use-image-support-probe.ts diff --git a/packages/cli/src/ui/components/ModelDialog.test.tsx b/packages/cli/src/ui/components/ModelDialog.test.tsx index 1a002fd0d45..7f26b0cd17a 100644 --- a/packages/cli/src/ui/components/ModelDialog.test.tsx +++ b/packages/cli/src/ui/components/ModelDialog.test.tsx @@ -1999,37 +1999,87 @@ describe('', () => { modalitiesSource: 'pattern', }; - it('badges pattern-guessed modalities as auto-detected and offers the t action', () => { - const { getByText } = renderComponent({}, { - getModel: vi.fn(() => 'pattern-model'), + // The probe suite's explicit-declaration counterpart: same endpoint shape + // as patternSourceModel with hand-written modalities. + const explicitSourceModel = { + id: 'explicit-model', + label: 'Explicit Model', + description: '', + authType: AuthType.USE_OPENAI, + baseUrl: 'https://api.example.com/v1', + envKey: 'MODEL_DIALOG_PROBE_TEST_KEY', + modalities: { image: true }, + modalitiesSource: 'explicit', + }; + + /** Minimal dialog Config around a model list: the FIRST model is the + * current selection, and getModelsConfig mirrors its baseUrl so the + * highlighted row resolves to it. */ + const probeDialogConfig = ( + models: Array>, + ): Partial => + ({ + getModel: vi.fn(() => models[0]!['id']), getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), + getAllConfiguredModels: vi.fn(() => models), getModelsConfig: vi.fn(() => ({ getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', + baseUrl: models[0]!['baseUrl'], })), })), - } as unknown as Partial); + }) as unknown as Partial; + + /** Mount the probe suite's standard dialog over `models`. */ + const renderProbeDialog = ( + models: Array> = [patternSourceModel], + settingsValue?: Partial, + ) => renderComponent({}, probeDialogConfig(models), settingsValue); + + /** `it` wrapper that pins MODEL_DIALOG_PROBE_TEST_KEY to `value` (deleted + * when undefined) for the body and restores the prior value afterwards. */ + const itWithProbeKeyEnv = ( + name: string, + value: string | undefined, + fn: () => Promise | void, + ) => + // The wrapper forwards a literal title from each call site below. + // eslint-disable-next-line vitest/valid-title + it(name, async () => { + const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + if (value === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = value; + } + try { + await fn(); + } finally { + if (previousKey === undefined) { + delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; + } else { + process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; + } + } + }); + + it('badges pattern-guessed modalities as auto-detected and offers the t action', () => { + const { getByText } = renderProbeDialog(); expect(getByText('text-only · auto-detected')).toBeDefined(); expect(getByText('t: test image support')).toBeDefined(); }); it('badges probe-tested modalities without offering the t action again', () => { - const { getByText, queryByText } = renderComponent({}, { - getModel: vi.fn(() => 'vl-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [ - { - id: 'vl-model', - label: 'VL Model', - description: '', - authType: AuthType.USE_OPENAI, - modalities: { image: true }, - modalitiesSource: 'probe', - }, - ]), - } as unknown as Partial); + const { getByText, queryByText } = renderProbeDialog([ + { + id: 'vl-model', + label: 'VL Model', + description: '', + authType: AuthType.USE_OPENAI, + modalities: { image: true }, + modalitiesSource: 'probe', + }, + ]); expect(getByText('text · image · probe-tested')).toBeDefined(); expect(queryByText('t: test image support')).toBeNull(); @@ -2040,29 +2090,16 @@ describe('', () => { // settings.setValue does not refresh it — but the dialog reads the live // settings store, so the persisted verdict must drive BOTH the badge and // the t-action gating even while the entry still says 'pattern'. - const { getByText, queryByText } = renderComponent( - {}, - { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, - { - merged: { - probeResults: { - 'openai|pattern-model|https://api.example.com/v1': { - verdict: 'text_only', - probedAt: '2026-01-01T00:00:00.000Z', - }, + const { getByText, queryByText } = renderProbeDialog(undefined, { + merged: { + probeResults: { + 'openai|pattern-model|https://api.example.com/v1': { + verdict: 'text_only', + probedAt: '2026-01-01T00:00:00.000Z', }, }, - } as unknown as Partial, - ); + }, + } as unknown as Partial); expect(getByText('text-only · probe-tested')).toBeDefined(); expect(queryByText('t: test image support')).toBeNull(); @@ -2073,29 +2110,8 @@ describe('', () => { // hand-write modelProviders modalities and reload the registry, which // stamps the entry 'explicit'. The live probe read must not shadow // that: badge shows manual with the hand-written value. - const { getByText, queryByText } = renderComponent( - {}, - { - getModel: vi.fn(() => 'explicit-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [ - { - id: 'explicit-model', - label: 'Explicit Model', - description: '', - authType: AuthType.USE_OPENAI, - baseUrl: 'https://api.example.com/v1', - envKey: 'MODEL_DIALOG_PROBE_TEST_KEY', - modalities: { image: true }, - modalitiesSource: 'explicit', - }, - ]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, + const { getByText, queryByText } = renderProbeDialog( + [explicitSourceModel], { merged: { probeResults: { @@ -2117,46 +2133,22 @@ describe('', () => { }); it('badges explicitly declared modalities as manual', () => { - const { getByText } = renderComponent({}, { - getModel: vi.fn(() => 'explicit-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [ - { - id: 'explicit-model', - label: 'Explicit Model', - description: '', - authType: AuthType.USE_OPENAI, - modalities: { image: true }, - modalitiesSource: 'explicit', - }, - ]), - } as unknown as Partial); + const { getByText } = renderProbeDialog([ + { + id: 'explicit-model', + label: 'Explicit Model', + description: '', + authType: AuthType.USE_OPENAI, + modalities: { image: true }, + modalitiesSource: 'explicit', + }, + ]); expect(getByText('text · image · manual')).toBeDefined(); }); it('does not run the probe for non-pattern modality sources', async () => { - const { mockSettings } = renderComponent({}, { - getModel: vi.fn(() => 'explicit-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [ - { - id: 'explicit-model', - label: 'Explicit Model', - description: '', - authType: AuthType.USE_OPENAI, - baseUrl: 'https://api.example.com/v1', - envKey: 'MODEL_DIALOG_PROBE_TEST_KEY', - modalities: { image: true }, - modalitiesSource: 'explicit', - }, - ]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial); + const { mockSettings } = renderProbeDialog([explicitSourceModel]); await pressT(); @@ -2164,10 +2156,10 @@ describe('', () => { expect(mockSettings.setValue).not.toHaveBeenCalled(); }); - it('probes a pattern-source entry on t and persists the whole probeResults map', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; - try { + itWithProbeKeyEnv( + 'probes a pattern-source entry on t and persists the whole probeResults map', + 'sk-probe-test', + async () => { mockedProbeImageSupport.mockResolvedValue({ verdict: 'image', httpStatus: 200, @@ -2187,22 +2179,9 @@ describe('', () => { }, }; - const { getByText, mockSettings } = renderComponent( - {}, - { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, - { - forScope: () => userSettingsFile, - } as unknown as Partial, - ); + const { getByText, mockSettings } = renderProbeDialog(undefined, { + forScope: () => userSettingsFile, + } as unknown as Partial); await pressT(); @@ -2229,36 +2208,13 @@ describe('', () => { // local dialog state. expect(getByText('text · image · probe-tested')).toBeDefined(); expect(getByText('accepts images')).toBeDefined(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); - it('hides the t action once a verdict concludes and keeps it after unknown', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; - const renderPatternDialog = () => - renderComponent( - {}, - { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, - { - forScope: () => ({ settings: {} }), - } as unknown as Partial, - ); - try { + itWithProbeKeyEnv( + 'hides the t action once a verdict concludes and keeps it after unknown', + 'sk-probe-test', + async () => { // Concluded verdict: the t action must disappear. Note the mocked // setValue does NOT refresh settings.merged, so the live-settings // lookup still misses — only the local verdict state gates the action @@ -2268,7 +2224,9 @@ describe('', () => { httpStatus: 200, snippet: 'ok', }); - const first = renderPatternDialog(); + const first = renderProbeDialog(undefined, { + forScope: () => ({ settings: {} }), + } as unknown as Partial); await pressT(); @@ -2283,7 +2241,9 @@ describe('', () => { httpStatus: 429, snippet: 'rate limited', }); - const second = renderPatternDialog(); + const second = renderProbeDialog(undefined, { + forScope: () => ({ settings: {} }), + } as unknown as Partial); await pressT(); @@ -2294,35 +2254,20 @@ describe('', () => { ).toBeDefined(); expect(second.getByText('t: test image support')).toBeDefined(); second.unmount(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); - it('writes nothing when the probe verdict is unknown', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; - try { + itWithProbeKeyEnv( + 'writes nothing when the probe verdict is unknown', + 'sk-probe-test', + async () => { mockedProbeImageSupport.mockResolvedValue({ verdict: 'unknown', httpStatus: 401, snippet: 'unauthorized', }); - const { getByText, mockSettings } = renderComponent({}, { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial); + const { getByText, mockSettings } = renderProbeDialog(); await pressT(); @@ -2331,31 +2276,16 @@ describe('', () => { expect( getByText('inconclusive (auth/rate-limit/timeout) — nothing written'), ).toBeDefined(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); - it('reports inconclusive without probing when the API key env is unset', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - // No key in the environment: the handler must bail out BEFORE any - // network attempt and write nothing. - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - try { - const { getByText, mockSettings } = renderComponent({}, { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial); + itWithProbeKeyEnv( + 'reports inconclusive without probing when the API key env is unset', + undefined, + async () => { + // No key in the environment: the handler must bail out BEFORE any + // network attempt and write nothing. + const { getByText, mockSettings } = renderProbeDialog(); await pressT(); @@ -2364,46 +2294,27 @@ describe('', () => { expect( getByText('inconclusive (auth/rate-limit/timeout) — nothing written'), ).toBeDefined(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); - it('hydrates a settings-backed API key before probing', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - // The key exists only in settings.env — NOT in process.env — so the - // probe only works if the handler hydrates the env first. - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - try { + itWithProbeKeyEnv( + 'hydrates a settings-backed API key before probing', + undefined, + async () => { + // The key exists only in settings.env — NOT in process.env — so the + // probe only works if the handler hydrates the env first. mockedProbeImageSupport.mockResolvedValue({ verdict: 'text_only', httpStatus: 400, snippet: 'does not support images', }); - const { getByText, mockSettings } = renderComponent( - {}, - { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, - { - merged: { - env: { MODEL_DIALOG_PROBE_TEST_KEY: 'sk-from-env-42' }, - }, - forScope: () => ({ settings: {} }), - } as unknown as Partial, - ); + const { getByText, mockSettings } = renderProbeDialog(undefined, { + merged: { + env: { MODEL_DIALOG_PROBE_TEST_KEY: 'sk-from-env-42' }, + }, + forScope: () => ({ settings: {} }), + } as unknown as Partial); await pressT(); @@ -2412,46 +2323,30 @@ describe('', () => { ); expect(mockSettings.setValue).toHaveBeenCalledTimes(1); expect(getByText('text only')).toBeDefined(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); - it('displaces the verdict display when the highlight moves to another entry', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; - try { + itWithProbeKeyEnv( + 'displaces the verdict display when the highlight moves to another entry', + 'sk-probe-test', + async () => { mockedProbeImageSupport.mockResolvedValue({ verdict: 'image', httpStatus: 200, snippet: 'ok', }); - const { getByText, queryByText } = renderComponent( - {}, - { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [ - patternSourceModel, - { - id: 'pattern-model-b', - label: 'Pattern Model B', - description: '', - authType: AuthType.USE_OPENAI, - modalitiesSource: 'pattern', - }, - ]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, + const { getByText, queryByText } = renderProbeDialog( + [ + patternSourceModel, + { + id: 'pattern-model-b', + label: 'Pattern Model B', + description: '', + authType: AuthType.USE_OPENAI, + modalitiesSource: 'pattern', + }, + ], { forScope: () => ({ settings: {} }), } as unknown as Partial, @@ -2477,47 +2372,28 @@ describe('', () => { }); expect(getByText('text · image · probe-tested')).toBeDefined(); expect(getByText('accepts images')).toBeDefined(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); - it('surfaces a settings-write failure instead of unhandled success', async () => { - const previousKey = process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = 'sk-probe-test'; - const setValue = vi.fn(() => { - const error = new Error('settings are read-only'); - Object.assign(error, { code: 'EACCES' }); - throw error; - }); - try { + itWithProbeKeyEnv( + 'surfaces a settings-write failure instead of unhandled success', + 'sk-probe-test', + async () => { + const setValue = vi.fn(() => { + const error = new Error('settings are read-only'); + Object.assign(error, { code: 'EACCES' }); + throw error; + }); mockedProbeImageSupport.mockResolvedValue({ verdict: 'image', httpStatus: 200, snippet: 'ok', }); - const { getByText, queryByText } = renderComponent( - {}, - { - getModel: vi.fn(() => 'pattern-model'), - getAuthType: vi.fn(() => AuthType.USE_OPENAI), - getAllConfiguredModels: vi.fn(() => [patternSourceModel]), - getModelsConfig: vi.fn(() => ({ - getGenerationConfig: vi.fn(() => ({ - baseUrl: 'https://api.example.com/v1', - })), - })), - } as unknown as Partial, - { - setValue, - forScope: () => ({ settings: {} }), - } as unknown as Partial, - ); + const { getByText, queryByText } = renderProbeDialog(undefined, { + setValue, + forScope: () => ({ settings: {} }), + } as unknown as Partial); await pressT(); @@ -2535,14 +2411,8 @@ describe('', () => { expect(queryByText('text · image · probe-tested')).toBeNull(); expect(getByText('text-only · auto-detected')).toBeDefined(); expect(getByText('t: test image support')).toBeDefined(); - } finally { - if (previousKey === undefined) { - delete process.env['MODEL_DIALOG_PROBE_TEST_KEY']; - } else { - process.env['MODEL_DIALOG_PROBE_TEST_KEY'] = previousKey; - } - } - }); + }, + ); }); describe('encodeAuxModelSelector', () => { diff --git a/packages/cli/src/ui/components/ModelDialog.tsx b/packages/cli/src/ui/components/ModelDialog.tsx index eb2653acb9f..4f3fd9d63b2 100644 --- a/packages/cli/src/ui/components/ModelDialog.tsx +++ b/packages/cli/src/ui/components/ModelDialog.tsx @@ -5,7 +5,6 @@ */ import type React from 'react'; -import process from 'node:process'; import { useCallback, useContext, useMemo, useRef, useState } from 'react'; import { Box, Text } from 'ink'; import { @@ -16,19 +15,19 @@ import { isImageCapable, isImageGenerationCapable, parseVisionModelSetting, - probeImageSupport, - readProbeResult, resolveModelId, - withProbeResult, type AvailableModel as CoreAvailableModel, type Config, type ContentGeneratorConfig, type InputModalities, - type ModalityProbeVerdict, type ModalitySource, } from '@qwen-code/qwen-code-core'; import { SettingScope } from '../../config/settings.js'; import { useKeypress } from '../hooks/useKeypress.js'; +import { + hydrateApiKeyEnvFromSettings, + useImageSupportProbe, +} from '../hooks/use-image-support-probe.js'; import { theme } from '../semantic-colors.js'; import { DescriptiveRadioButtonSelect } from './shared/DescriptiveRadioButtonSelect.js'; import { ConfigContext } from '../contexts/ConfigContext.js'; @@ -240,24 +239,6 @@ function persistAuthTypeSelection( settings.setValue(scope, 'security.auth.selectedType', authType); } -function hydrateApiKeyEnvFromSettings( - settings: ReturnType, - envKey: string | undefined, -): void { - if (!envKey || process.env[envKey]) { - return; - } - const settingsEnvValue = ( - settings?.merged?.env as Record | undefined - )?.[envKey]; - if ( - typeof settingsEnvValue === 'string' && - settingsEnvValue.trim().length > 0 - ) { - process.env[envKey] = settingsEnvValue; - } -} - interface HandleModelSwitchSuccessParams { config: Config; settings: ReturnType; @@ -784,134 +765,27 @@ export function ModelDialog({ ); }, [highlightedValue, preferredKey, availableModelEntries]); - // One-shot image modality probe (issue #10309, phase 1). `probeTargetKey` - // remembers WHICH entry a pending/finished verdict belongs to, so moving - // the highlight mid-probe never shows another entry's result. - const [probeState, setProbeState] = useState< - 'idle' | 'probing' | ModalityProbeVerdict - >('idle'); - const [probeTargetKey, setProbeTargetKey] = useState(null); - const highlightedEntryKey = highlightedEntry ? entrySelectionKey(highlightedEntry) : null; - const activeProbeState = - probeTargetKey !== null && probeTargetKey === highlightedEntryKey - ? probeState - : 'idle'; - - // Live probe-verdict source: the registry caches modalitiesSource at - // registration/reload time (a plain settings.setValue does NOT refresh - // already-registered entries), so a verdict concluded earlier in this - // session — or written by an earlier dialog session — is visible here only - // through the settings store itself. Keyed the same way the write path - // keys it (declared/resolved entry baseUrl). Read-side hardening mirrors - // the registry: only verdicts exactly 'image'/'text_only' are honored; - // hand-edited garbage abstains to the entry's own source. - // - // The live read only applies to STILL-PATTERN-CACHED entries. An - // 'explicit'-stamped entry got there because the user hand-wrote - // modelProviders modalities — the phase-1 remediation exit for a wrong - // verdict — and a stale probe record must not shadow it in the UI. A - // 'probe'-stamped entry loses nothing either: phase 1 has no re-probe - // path, so the live store can never hold a newer conclusion than the - // registration-time stamp. - const liveRawVerdict = - highlightedEntry && - !highlightedEntry.isRuntime && - highlightedEntry.model.modalitiesSource === 'pattern' - ? readProbeResult( - settings.merged?.probeResults, - highlightedEntry.authType, - highlightedEntry.model.id, - highlightedEntry.model.baseUrl, - )?.verdict - : undefined; - const liveProbeVerdict: 'image' | 'text_only' | undefined = - liveRawVerdict === 'image' || liveRawVerdict === 'text_only' - ? liveRawVerdict - : undefined; - // The `t` action only applies to regex-guessed (pattern-source) - // modalities: explicit declarations need no probe, and entries carrying a - // conclusion — registry-stamped 'probe' source OR a live settings hit for - // a still-pattern-cached entry — need none; QWEN_OAUTH's two probe-key - // spellings diverge in phase 1 (see probe-store.ts), so it is excluded. - // Runtime models have no modalitiesSource and are excluded by the same - // check. A probe in flight disables re-trigger globally so two concurrent - // probes can never race the whole-map read-modify-write, and a CONCLUDED - // local verdict hides the action too ('unknown' keeps it — retry is the - // only recourse for an inconclusive probe in phase 1). - const canTestImageSupport = - !!highlightedEntry && - !highlightedEntry.isRuntime && - highlightedEntry.authType !== AuthType.QWEN_OAUTH && - (liveProbeVerdict === undefined - ? highlightedEntry.model.modalitiesSource - : 'probe') === 'pattern' && - probeState !== 'probing' && - activeProbeState !== 'image' && - activeProbeState !== 'text_only'; - - const handleTestImageSupport = useCallback(async () => { - if (!highlightedEntry || probeState === 'probing') return; - const { model } = highlightedEntry; - setErrorMessage(null); - setProbeState('probing'); - setProbeTargetKey(entrySelectionKey(highlightedEntry)); - // Settings-backed keys are not in process.env until hydrated — the same - // in-file helper handleSelect uses before switching models — so mid- - // session settings.env keys work without a restart. The API key is read - // from the environment at probe time only — never displayed, never - // logged, never persisted. - hydrateApiKeyEnvFromSettings(settings, model.envKey); - const apiKey = model.envKey ? process.env[model.envKey] : undefined; - if (!apiKey || !model.baseUrl) { - setProbeState('unknown'); - return; - } - const result = await probeImageSupport({ - model: model.id, - baseUrl: model.baseUrl, - apiKey, - }); - if (result.verdict !== 'unknown') { - // Read the TARGET scope's own map (not the merged view) so records - // from other scopes never bleed into this write, and always write the - // WHOLE map under the single 'probeResults' key — composite probe - // keys embed dots and '|' that settings' dotted-path addressing would - // mis-nest. - const scope = resolvePersistScope(settings, persistScope); - try { - settings.setValue( - scope, - 'probeResults', - withProbeResult( - settings.forScope(scope).settings.probeResults, - highlightedEntry.authType, - model.id, - model.baseUrl, - { verdict: result.verdict, probedAt: new Date().toISOString() }, - ), - ); - } catch (e) { - // setValue can throw (saveSettings re-throws fs errors) and this - // handler runs fire-and-forget, so an uncaught throw would be an - // unhandled rejection while the UI shows success. Surface the - // failure through the dialog's error channel (the same ✕ box - // handleSelect uses) and reset the probe display: the feedback row - // is hidden and the badge falls back to the entry's own (registry) - // source, which stays truthful because nothing was persisted. - const message = e instanceof Error ? e.message : String(e); - setProbeState('idle'); - setErrorMessage( - `${t('Image probe verdict could not be saved.')}\n\n${message}`, - ); - return; - } - } - setProbeState(result.verdict); - }, [highlightedEntry, persistScope, probeState, settings]); + // One-shot image modality probe (issue #10309, phase 1): the probe state + // machine, the live-verdict read, the t-action gating, and the derived + // badge/modality/feedback presentation all live in the hook; this + // component keeps only rendering and key binding. + const { + canTestImageSupport, + handleTestImageSupport, + displayedModalities, + displayedModalitiesSource, + probeFeedback, + } = useImageSupportProbe({ + highlightedEntry, + highlightedEntryKey, + settings, + scope: resolvePersistScope(settings, persistScope), + setErrorMessage, + }); // Registered separately from the escape handler above: this one depends // on `highlightedEntry` and the probe state, which are computed later in @@ -931,48 +805,6 @@ export function ModelDialog({ { isActive: true }, ); - // Modality badge/value provenance is a two-layer source. Layer 1 is the - // registry's registration-time cache (`modalitiesSource`), which a plain - // settings.setValue does not refresh without a registry reload — and we - // deliberately do NOT reload mid-dialog. Layer 2 is the live settings - // store (`probeResults`), read on every render for the highlighted entry - // — but ONLY while that entry is still pattern-cached; 'explicit' and - // 'probe' stamps show their own value (a hand-written explicit - // declaration is the phase-1 way out of a wrong verdict, so it must not - // be shadowed by the stale probe record underneath). A local verdict from - // THIS dialog's probe overrides both, so the panel never contradicts - // itself mid-feedback (e.g. `text-only · probe-tested` above - // `accepts images`). - const displayedProbeVerdict: 'image' | 'text_only' | undefined = - activeProbeState === 'image' || activeProbeState === 'text_only' - ? activeProbeState - : liveProbeVerdict; - const displayedModalitiesSource: ModalitySource | undefined = - displayedProbeVerdict !== undefined - ? 'probe' - : highlightedEntry?.model.modalitiesSource; - const displayedModalities: InputModalities | undefined = - displayedProbeVerdict === 'image' - ? { ...highlightedEntry?.model.modalities, image: true } - : displayedProbeVerdict === 'text_only' && - highlightedEntry?.model.modalities - ? { ...highlightedEntry.model.modalities, image: false } - : highlightedEntry?.model.modalities; - - const probeFeedback: { text: string; color: string } | undefined = - activeProbeState === 'probing' - ? { text: t('testing…'), color: theme.text.secondary } - : activeProbeState === 'unknown' - ? { - text: t('inconclusive (auth/rate-limit/timeout) — nothing written'), - color: theme.status.warning, - } - : activeProbeState === 'image' - ? { text: t('accepts images'), color: theme.status.success } - : activeProbeState === 'text_only' - ? { text: t('text only'), color: theme.text.secondary } - : undefined; - const handleSelect = useCallback( async (selected: string) => { if (selectionInFlightRef.current || selectionCommittedRef.current) return; diff --git a/packages/cli/src/ui/hooks/use-image-support-probe.ts b/packages/cli/src/ui/hooks/use-image-support-probe.ts new file mode 100644 index 00000000000..268caabf284 --- /dev/null +++ b/packages/cli/src/ui/hooks/use-image-support-probe.ts @@ -0,0 +1,272 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useCallback, useState } from 'react'; +import process from 'node:process'; +import { + AuthType, + probeImageSupport, + readProbeResult, + withProbeResult, + type AvailableModel as CoreAvailableModel, + type InputModalities, + type ModalityProbeVerdict, + type ModalitySource, +} from '@qwen-code/qwen-code-core'; +import { + type LoadedSettings, + type SettingScope, +} from '../../config/settings.js'; +import { theme } from '../semantic-colors.js'; +import { t } from '../../i18n/index.js'; + +/** A /model dialog entry the probe operates on — the subset of the dialog's + * entry shape the hook reads (dialog entries satisfy it structurally). */ +export interface ProbeTargetEntry { + readonly authType: AuthType; + readonly model: CoreAvailableModel; + readonly isRuntime?: boolean; +} + +export interface UseImageSupportProbeParams { + /** The currently highlighted dialog entry, if any. */ + readonly highlightedEntry: ProbeTargetEntry | undefined; + /** Selection key of `highlightedEntry`, owned by the dialog (it keys the + * option list, highlight, and selection handling); reused as the probe's + * displacement-guard key. */ + readonly highlightedEntryKey: string | null; + readonly settings: LoadedSettings; + /** Pre-resolved scope the probe write persists under. */ + readonly scope: SettingScope; + /** The dialog's error channel: cleared when a probe starts, fed when a + * verdict write fails. */ + readonly setErrorMessage: (message: string | null) => void; +} + +export interface UseImageSupportProbeResult { + /** Whether the `t` action may trigger a probe for the highlighted entry. */ + readonly canTestImageSupport: boolean; + /** Fire-and-forget probe handler; own guard clauses make it a no-op when + * `canTestImageSupport` is false. */ + readonly handleTestImageSupport: () => Promise; + /** Modality value + provenance to render in the details panel. */ + readonly displayedModalities: InputModalities | undefined; + readonly displayedModalitiesSource: ModalitySource | undefined; + /** Feedback row for the highlighted entry's probe, if one is on screen. */ + readonly probeFeedback: + | { readonly text: string; readonly color: string } + | undefined; +} + +/** Settings-backed keys are not in process.env until hydrated, so mid-session + * settings.env keys work without a restart. The API key is read from the + * environment at action time only — never displayed, never logged, never + * persisted. */ +export function hydrateApiKeyEnvFromSettings( + settings: LoadedSettings, + envKey: string | undefined, +): void { + if (!envKey || process.env[envKey]) { + return; + } + const settingsEnvValue = ( + settings?.merged?.env as Record | undefined + )?.[envKey]; + if ( + typeof settingsEnvValue === 'string' && + settingsEnvValue.trim().length > 0 + ) { + process.env[envKey] = settingsEnvValue; + } +} + +/** One-shot image modality probe (issue #10309, phase 1) for the /model + * dialog: owns the probe state machine, the live verdict read, the t-action + * gating, and the derived badge/modality/feedback presentation. The dialog + * keeps only rendering and key binding. */ +export function useImageSupportProbe({ + highlightedEntry, + highlightedEntryKey, + settings, + scope, + setErrorMessage, +}: UseImageSupportProbeParams): UseImageSupportProbeResult { + // `probeTargetKey` remembers WHICH entry a pending/finished verdict belongs + // to, so moving the highlight mid-probe never shows another entry's result. + const [probeState, setProbeState] = useState< + 'idle' | 'probing' | ModalityProbeVerdict + >('idle'); + const [probeTargetKey, setProbeTargetKey] = useState(null); + + const activeProbeState = + probeTargetKey !== null && probeTargetKey === highlightedEntryKey + ? probeState + : 'idle'; + + // Live probe-verdict source: the registry caches modalitiesSource at + // registration/reload time (a plain settings.setValue does NOT refresh + // already-registered entries), so a verdict concluded earlier in this + // session — or written by an earlier dialog session — is visible here only + // through the settings store itself. Keyed the same way the write path + // keys it (declared/resolved entry baseUrl). Read-side hardening mirrors + // the registry: only verdicts exactly 'image'/'text_only' are honored; + // hand-edited garbage abstains to the entry's own source. + // + // The live read only applies to STILL-PATTERN-CACHED entries. An + // 'explicit'-stamped entry got there because the user hand-wrote + // modelProviders modalities — the phase-1 remediation exit for a wrong + // verdict — and a stale probe record must not shadow it in the UI. A + // 'probe'-stamped entry loses nothing either: phase 1 has no re-probe + // path, so the live store can never hold a newer conclusion than the + // registration-time stamp. + const liveRawVerdict = + highlightedEntry && + !highlightedEntry.isRuntime && + highlightedEntry.model.modalitiesSource === 'pattern' + ? readProbeResult( + settings.merged?.probeResults, + highlightedEntry.authType, + highlightedEntry.model.id, + highlightedEntry.model.baseUrl, + )?.verdict + : undefined; + const liveProbeVerdict: 'image' | 'text_only' | undefined = + liveRawVerdict === 'image' || liveRawVerdict === 'text_only' + ? liveRawVerdict + : undefined; + + // The `t` action only applies to regex-guessed (pattern-source) + // modalities: explicit declarations need no probe, and entries carrying a + // conclusion — registry-stamped 'probe' source OR a live settings hit for + // a still-pattern-cached entry — need none; QWEN_OAUTH's two probe-key + // spellings diverge in phase 1 (see probe-store.ts), so it is excluded. + // Runtime models have no modalitiesSource and are excluded by the same + // check. A probe in flight disables re-trigger globally so two concurrent + // probes can never race the whole-map read-modify-write, and a CONCLUDED + // local verdict hides the action too ('unknown' keeps it — retry is the + // only recourse for an inconclusive probe in phase 1). + const canTestImageSupport = + !!highlightedEntry && + !highlightedEntry.isRuntime && + highlightedEntry.authType !== AuthType.QWEN_OAUTH && + (liveProbeVerdict === undefined + ? highlightedEntry.model.modalitiesSource + : 'probe') === 'pattern' && + probeState !== 'probing' && + activeProbeState !== 'image' && + activeProbeState !== 'text_only'; + + const handleTestImageSupport = useCallback(async () => { + if (!highlightedEntry || probeState === 'probing') return; + const { model } = highlightedEntry; + setErrorMessage(null); + setProbeState('probing'); + setProbeTargetKey(highlightedEntryKey); + hydrateApiKeyEnvFromSettings(settings, model.envKey); + const apiKey = model.envKey ? process.env[model.envKey] : undefined; + if (!apiKey || !model.baseUrl) { + setProbeState('unknown'); + return; + } + const result = await probeImageSupport({ + model: model.id, + baseUrl: model.baseUrl, + apiKey, + }); + if (result.verdict !== 'unknown') { + // Read the TARGET scope's own map (not the merged view) so records + // from other scopes never bleed into this write, and always write the + // WHOLE map under the single 'probeResults' key — composite probe + // keys embed dots and '|' that settings' dotted-path addressing would + // mis-nest. + try { + settings.setValue( + scope, + 'probeResults', + withProbeResult( + settings.forScope(scope).settings.probeResults, + highlightedEntry.authType, + model.id, + model.baseUrl, + { verdict: result.verdict, probedAt: new Date().toISOString() }, + ), + ); + } catch (e) { + // setValue can throw (saveSettings re-throws fs errors) and this + // handler runs fire-and-forget, so an uncaught throw would be an + // unhandled rejection while the UI shows success. Surface the + // failure through the dialog's error channel (the same ✕ box + // handleSelect uses) and reset the probe display: the feedback row + // is hidden and the badge falls back to the entry's own (registry) + // source, which stays truthful because nothing was persisted. + const message = e instanceof Error ? e.message : String(e); + setProbeState('idle'); + setErrorMessage( + `${t('Image probe verdict could not be saved.')}\n\n${message}`, + ); + return; + } + } + setProbeState(result.verdict); + }, [ + highlightedEntry, + highlightedEntryKey, + probeState, + scope, + setErrorMessage, + settings, + ]); + + // Modality badge/value provenance is a two-layer source. Layer 1 is the + // registry's registration-time cache (`modalitiesSource`), which a plain + // settings.setValue does not refresh without a registry reload — and we + // deliberately do NOT reload mid-dialog. Layer 2 is the live settings + // store (`probeResults`), read on every render for the highlighted entry + // — but ONLY while that entry is still pattern-cached; 'explicit' and + // 'probe' stamps show their own value (a hand-written explicit + // declaration is the phase-1 way out of a wrong verdict, so it must not + // be shadowed by the stale probe record underneath). A local verdict from + // THIS dialog's probe overrides both, so the panel never contradicts + // itself mid-feedback (e.g. `text-only · probe-tested` above + // `accepts images`). + const displayedProbeVerdict: 'image' | 'text_only' | undefined = + activeProbeState === 'image' || activeProbeState === 'text_only' + ? activeProbeState + : liveProbeVerdict; + const displayedModalitiesSource: ModalitySource | undefined = + displayedProbeVerdict !== undefined + ? 'probe' + : highlightedEntry?.model.modalitiesSource; + const displayedModalities: InputModalities | undefined = + displayedProbeVerdict === 'image' + ? { ...highlightedEntry?.model.modalities, image: true } + : displayedProbeVerdict === 'text_only' && + highlightedEntry?.model.modalities + ? { ...highlightedEntry.model.modalities, image: false } + : highlightedEntry?.model.modalities; + + const probeFeedback: { text: string; color: string } | undefined = + activeProbeState === 'probing' + ? { text: t('testing…'), color: theme.text.secondary } + : activeProbeState === 'unknown' + ? { + text: t('inconclusive (auth/rate-limit/timeout) — nothing written'), + color: theme.status.warning, + } + : activeProbeState === 'image' + ? { text: t('accepts images'), color: theme.status.success } + : activeProbeState === 'text_only' + ? { text: t('text only'), color: theme.text.secondary } + : undefined; + + return { + canTestImageSupport, + handleTestImageSupport, + displayedModalities, + displayedModalitiesSource, + probeFeedback, + }; +} From 615b9d5345ed16e6991390e7e8291893817e83d1 Mon Sep 17 00:00:00 2001 From: jarvislee90s-dot Date: Fri, 28 Aug 2026 09:30:30 +0800 Subject: [PATCH 7/7] refactor(cli): fold probe mock boilerplate into shared test helpers --- .../src/ui/components/ModelDialog.test.tsx | 72 +++++++------------ 1 file changed, 26 insertions(+), 46 deletions(-) diff --git a/packages/cli/src/ui/components/ModelDialog.test.tsx b/packages/cli/src/ui/components/ModelDialog.test.tsx index 7f26b0cd17a..48b62adc1dc 100644 --- a/packages/cli/src/ui/components/ModelDialog.test.tsx +++ b/packages/cli/src/ui/components/ModelDialog.test.tsx @@ -2035,6 +2035,20 @@ describe('', () => { settingsValue?: Partial, ) => renderComponent({}, probeDialogConfig(models), settingsValue); + /** Settings value giving every scope an empty settings object — the + * standard "no pre-existing probe records" persistence target. */ + const emptyScopeSettings = { + forScope: () => ({ settings: {} }), + } as unknown as Partial; + + /** Pin the mocked probe endpoint's next verdict. */ + const mockProbeVerdict = ( + verdict: 'image' | 'text_only' | 'unknown', + httpStatus: number, + snippet: string, + ) => + mockedProbeImageSupport.mockResolvedValue({ verdict, httpStatus, snippet }); + /** `it` wrapper that pins MODEL_DIALOG_PROBE_TEST_KEY to `value` (deleted * when undefined) for the body and restores the prior value afterwards. */ const itWithProbeKeyEnv = ( @@ -2160,11 +2174,7 @@ describe('', () => { 'probes a pattern-source entry on t and persists the whole probeResults map', 'sk-probe-test', async () => { - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'image', - httpStatus: 200, - snippet: 'ok', - }); + mockProbeVerdict('image', 200, 'ok'); const existingRecord = { verdict: 'text_only' as const, probedAt: '2026-01-01T00:00:00.000Z', @@ -2219,14 +2229,8 @@ describe('', () => { // setValue does NOT refresh settings.merged, so the live-settings // lookup still misses — only the local verdict state gates the action // here, which is exactly the fallback under test. - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'image', - httpStatus: 200, - snippet: 'ok', - }); - const first = renderProbeDialog(undefined, { - forScope: () => ({ settings: {} }), - } as unknown as Partial); + mockProbeVerdict('image', 200, 'ok'); + const first = renderProbeDialog(undefined, emptyScopeSettings); await pressT(); @@ -2236,14 +2240,8 @@ describe('', () => { // Unknown verdict: nothing was written and no conclusion exists, so // retry stays available (phase 1 has no other re-probe entry point). - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'unknown', - httpStatus: 429, - snippet: 'rate limited', - }); - const second = renderProbeDialog(undefined, { - forScope: () => ({ settings: {} }), - } as unknown as Partial); + mockProbeVerdict('unknown', 429, 'rate limited'); + const second = renderProbeDialog(undefined, emptyScopeSettings); await pressT(); @@ -2261,11 +2259,7 @@ describe('', () => { 'writes nothing when the probe verdict is unknown', 'sk-probe-test', async () => { - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'unknown', - httpStatus: 401, - snippet: 'unauthorized', - }); + mockProbeVerdict('unknown', 401, 'unauthorized'); const { getByText, mockSettings } = renderProbeDialog(); @@ -2303,11 +2297,7 @@ describe('', () => { async () => { // The key exists only in settings.env — NOT in process.env — so the // probe only works if the handler hydrates the env first. - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'text_only', - httpStatus: 400, - snippet: 'does not support images', - }); + mockProbeVerdict('text_only', 400, 'does not support images'); const { getByText, mockSettings } = renderProbeDialog(undefined, { merged: { @@ -2330,11 +2320,7 @@ describe('', () => { 'displaces the verdict display when the highlight moves to another entry', 'sk-probe-test', async () => { - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'image', - httpStatus: 200, - snippet: 'ok', - }); + mockProbeVerdict('image', 200, 'ok'); const { getByText, queryByText } = renderProbeDialog( [ @@ -2347,9 +2333,7 @@ describe('', () => { modalitiesSource: 'pattern', }, ], - { - forScope: () => ({ settings: {} }), - } as unknown as Partial, + emptyScopeSettings, ); await pressT(); @@ -2384,16 +2368,12 @@ describe('', () => { Object.assign(error, { code: 'EACCES' }); throw error; }); - mockedProbeImageSupport.mockResolvedValue({ - verdict: 'image', - httpStatus: 200, - snippet: 'ok', - }); + mockProbeVerdict('image', 200, 'ok'); const { getByText, queryByText } = renderProbeDialog(undefined, { setValue, - forScope: () => ({ settings: {} }), - } as unknown as Partial); + ...emptyScopeSettings, + }); await pressT();