From b86ace5e3d012f8fe05cc19dba3e97a4d2584cee Mon Sep 17 00:00:00 2001 From: wgqqqqq Date: Wed, 29 Jul 2026 15:24:02 +0800 Subject: [PATCH] fix: bundle ACP setup and model selection updates --- .../acp/src/client/session_options.rs | 129 ++++++++++++++++-- .../flow_chat/components/ModelSelector.scss | 4 + .../flow_chat/components/ModelSelector.tsx | 9 +- .../flow_chat/utils/acpSessionConfig.test.ts | 37 ++++- .../src/flow_chat/utils/acpSessionConfig.ts | 24 ++++ .../api/service-api/ACPClientAPI.ts | 1 + .../components/AcpAgentsConfig.test.tsx | 42 ++++++ .../config/components/AcpAgentsConfig.tsx | 101 ++++++++++++-- .../locales/en-US/settings/acp-agents.json | 5 + .../locales/zh-CN/settings/acp-agents.json | 5 + .../locales/zh-TW/settings/acp-agents.json | 5 + 11 files changed, 337 insertions(+), 25 deletions(-) diff --git a/src/crates/interfaces/acp/src/client/session_options.rs b/src/crates/interfaces/acp/src/client/session_options.rs index 6d8b2493cc..b4b79f7e48 100644 --- a/src/crates/interfaces/acp/src/client/session_options.rs +++ b/src/crates/interfaces/acp/src/client/session_options.rs @@ -106,6 +106,8 @@ pub struct AcpSessionOptions { pub struct AcpSessionModelOption { pub id: String, pub name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider_name: Option, #[serde(default)] pub description: Option, } @@ -243,13 +245,41 @@ pub(super) fn model_config_id(config_options: &[SessionConfigOption]) -> Option< } fn model_option_from_model_info(model: &ModelInfo) -> AcpSessionModelOption { + let id = model.model_id.to_string(); AcpSessionModelOption { - id: model.model_id.to_string(), + provider_name: provider_name_from_model_identity(&id, model.description.as_deref()), + id, name: model.name.clone(), description: model.description.clone(), } } +fn provider_name_from_model_identity(id: &str, description: Option<&str>) -> Option { + description + .and_then(provider_prefix) + .or_else(|| provider_prefix(id)) +} + +fn normalized_provider_name(value: &str) -> Option { + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } +} + +fn provider_prefix(value: &str) -> Option { + let (provider, model) = value.trim().split_once('/')?; + let provider = provider.trim(); + let model = model.trim(); + if provider.is_empty() || model.is_empty() { + None + } else { + Some(provider.to_string()) + } +} + fn model_config_option(config_options: &[SessionConfigOption]) -> Option<&SessionConfigOption> { config_options .iter() @@ -274,19 +304,32 @@ fn select_model_values( let models = match &select.options { SessionConfigSelectOptions::Ungrouped(options) => options .iter() - .map(|option| AcpSessionModelOption { - id: option.value.to_string(), - name: option.name.clone(), - description: option.description.clone(), + .map(|option| { + let id = option.value.to_string(); + AcpSessionModelOption { + provider_name: provider_name_from_model_identity( + &id, + option.description.as_deref(), + ), + id, + name: option.name.clone(), + description: option.description.clone(), + } }) .collect(), SessionConfigSelectOptions::Grouped(groups) => groups .iter() .flat_map(|group| { - group.options.iter().map(|option| AcpSessionModelOption { - id: option.value.to_string(), - name: option.name.clone(), - description: option.description.clone(), + group.options.iter().map(|option| { + let id = option.value.to_string(); + AcpSessionModelOption { + provider_name: normalized_provider_name(&group.name).or_else(|| { + provider_name_from_model_identity(&id, option.description.as_deref()) + }), + id, + name: option.name.clone(), + description: option.description.clone(), + } }) }) .collect(), @@ -313,6 +356,21 @@ mod tests { assert!(options.model_config_id.is_none()); } + #[test] + fn extracts_provider_name_from_native_model_identity() { + let state = SessionModelState::new( + "openai/gpt-5.4", + vec![ModelInfo::new("openai/gpt-5.4", "GPT 5.4")], + ); + + let options = session_options_from_state(Some(&state), &[], None); + + assert_eq!( + options.available_models[0].provider_name.as_deref(), + Some("openai") + ); + } + #[test] fn converts_model_config_option_fallback() { let config = SessionConfigOption::select( @@ -334,6 +392,59 @@ mod tests { assert_eq!(options.available_models[1].id, "smart"); } + #[test] + fn extracts_provider_name_from_model_config_description() { + let config = SessionConfigOption::select( + "model", + "Model", + "openai/gpt-5.4", + vec![ + agent_client_protocol::schema::SessionConfigSelectOption::new( + "openai/gpt-5.4", + "GPT 5.4", + ) + .description("openai/gpt-5.4"), + ], + ) + .category(SessionConfigOptionCategory::Model); + + let options = session_options_from_state(None, &[config], None); + + assert_eq!( + options.available_models[0].provider_name.as_deref(), + Some("openai") + ); + } + + #[test] + fn preserves_model_config_group_name_as_provider_name() { + let config = SessionConfigOption::select( + "model", + "Model", + "openai/gpt-5.4", + vec![ + agent_client_protocol::schema::SessionConfigSelectGroup::new( + "openai", + "OpenAI", + vec![ + agent_client_protocol::schema::SessionConfigSelectOption::new( + "openai/gpt-5.4", + "GPT 5.4", + ), + ], + ), + ], + ) + .category(SessionConfigOptionCategory::Model); + + let options = session_options_from_state(None, &[config], None); + + assert_eq!( + options.available_models[0].provider_name.as_deref(), + Some("OpenAI") + ); + } + #[test] fn includes_context_usage() { let state = SessionModelState::new("gpt-5.4", vec![ModelInfo::new("gpt-5.4", "GPT 5.4")]); diff --git a/src/web-ui/src/flow_chat/components/ModelSelector.scss b/src/web-ui/src/flow_chat/components/ModelSelector.scss index 3b91061cf5..02a24dcb10 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelector.scss +++ b/src/web-ui/src/flow_chat/components/ModelSelector.scss @@ -306,7 +306,11 @@ } &__option-provider { + font-size: var(--flowchat-font-size-2xs); color: var(--color-text-muted); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } &__option-meta { diff --git a/src/web-ui/src/flow_chat/components/ModelSelector.tsx b/src/web-ui/src/flow_chat/components/ModelSelector.tsx index 5790d22f23..adfcc340a6 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelector.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelector.tsx @@ -23,7 +23,7 @@ import { Switch, Tooltip } from '@/component-library'; import { FlowChatStore } from '../store/FlowChatStore'; import { getModelMaxTokens } from '../services/flow-chat-manager/SessionModule'; import { acpClientIdFromAgentType } from '../utils/acpSession'; -import { buildAcpFastModeValue, resolveAcpFastModeState } from '../utils/acpSessionConfig'; +import { buildAcpFastModeValue, getAcpModelProviderName, resolveAcpFastModeState } from '../utils/acpSessionConfig'; import { buildContextUsageTooltip, type ContextUsageSource, @@ -386,7 +386,7 @@ export const ModelSelector: React.FC = ({ id: model.id, configName: model.name, modelName: model.name, - providerName: acpClientId ? `${acpClientId} ACP` : 'ACP', + providerName: getAcpModelProviderName(model) ?? (acpClientId ? `${acpClientId} ACP` : 'ACP'), provider: 'acp', })); }, [acpClientId, acpOptions, isAcpSession]); @@ -694,7 +694,7 @@ export const ModelSelector: React.FC = ({ const isSelected = currentAcpModelId === model.id; return ( - +
= ({ {model.modelName} + + {model.providerName} +
{isSelected && ( diff --git a/src/web-ui/src/flow_chat/utils/acpSessionConfig.test.ts b/src/web-ui/src/flow_chat/utils/acpSessionConfig.test.ts index 511df65a9d..c63782b9a3 100644 --- a/src/web-ui/src/flow_chat/utils/acpSessionConfig.test.ts +++ b/src/web-ui/src/flow_chat/utils/acpSessionConfig.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest'; import type { AcpSessionConfigOption } from '@/infrastructure/api/service-api/ACPClientAPI'; -import { buildAcpFastModeValue, resolveAcpFastModeState } from './acpSessionConfig'; +import { + buildAcpFastModeValue, + getAcpModelProviderName, + resolveAcpFastModeState, +} from './acpSessionConfig'; describe('ACP Fast mode config', () => { it('resolves and toggles the select fallback exposed by Codex ACP', () => { @@ -46,3 +50,34 @@ describe('ACP Fast mode config', () => { expect(buildAcpFastModeValue(malformed, true)).toBeNull(); }); }); + +describe('ACP model provider display', () => { + it('uses the explicit provider name when the ACP bridge includes one', () => { + expect(getAcpModelProviderName({ + id: 'openai/gpt-5.4', + name: 'gpt-5.4', + providerName: 'OpenAI', + })).toBe('OpenAI'); + }); + + it('falls back to provider-qualified description or id', () => { + expect(getAcpModelProviderName({ + id: 'openai/gpt-5.4', + name: 'gpt-5.4', + })).toBe('openai'); + + expect(getAcpModelProviderName({ + id: 'gpt-5.4', + name: 'gpt-5.4', + description: 'azure-openai/gpt-5.4', + })).toBe('azure-openai'); + }); + + it('ignores unqualified descriptions', () => { + expect(getAcpModelProviderName({ + id: 'gpt-5.4', + name: 'gpt-5.4', + description: 'Fast reasoning model', + })).toBeUndefined(); + }); +}); diff --git a/src/web-ui/src/flow_chat/utils/acpSessionConfig.ts b/src/web-ui/src/flow_chat/utils/acpSessionConfig.ts index 8730df68dd..fba37115c3 100644 --- a/src/web-ui/src/flow_chat/utils/acpSessionConfig.ts +++ b/src/web-ui/src/flow_chat/utils/acpSessionConfig.ts @@ -1,6 +1,7 @@ import type { AcpSessionConfigOption, AcpSessionConfigValue, + AcpSessionModelOption, } from '@/infrastructure/api/service-api/ACPClientAPI'; const FAST_MODE_CONFIG_ID = 'fast-mode'; @@ -49,3 +50,26 @@ export function buildAcpFastModeValue( ? { type: 'select', value } : null; } + +export function getAcpModelProviderName(model: AcpSessionModelOption): string | undefined { + return normalizedProviderName(model.providerName) + ?? providerNameFromProviderQualifiedValue(model.description) + ?? providerNameFromProviderQualifiedValue(model.id); +} + +function normalizedProviderName(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +function providerNameFromProviderQualifiedValue(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + if (!trimmed) return undefined; + + const slashIndex = trimmed.indexOf('/'); + if (slashIndex <= 0 || slashIndex === trimmed.length - 1) return undefined; + + const provider = trimmed.slice(0, slashIndex).trim(); + const modelId = trimmed.slice(slashIndex + 1).trim(); + return provider && modelId ? provider : undefined; +} diff --git a/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts index 010dc9b712..e6b61238e3 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts @@ -108,6 +108,7 @@ export interface SetAcpSessionConfigOptionRequest { export interface AcpSessionModelOption { id: string; name: string; + providerName?: string; description?: string; } diff --git a/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.test.tsx b/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.test.tsx index ddc9384048..31575d6cfe 100644 --- a/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.test.tsx +++ b/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.test.tsx @@ -13,6 +13,7 @@ const installClientCliMock = vi.hoisted(() => vi.fn()); const predownloadClientAdapterMock = vi.hoisted(() => vi.fn()); const listSavedConnectionsMock = vi.hoisted(() => vi.fn()); const notifyErrorMock = vi.hoisted(() => vi.fn()); +const notifyInfoMock = vi.hoisted(() => vi.fn()); const notifySuccessMock = vi.hoisted(() => vi.fn()); const translate = (_key: string, options?: Record & { defaultValue?: string }) => ( options?.defaultValue ?? _key @@ -122,6 +123,7 @@ vi.mock('@/features/ssh-remote/sshApi', () => ({ vi.mock('@/shared/notification-system', () => ({ useNotification: () => ({ error: notifyErrorMock, + info: notifyInfoMock, success: notifySuccessMock, }), })); @@ -374,6 +376,46 @@ describe('AcpAgentsConfig', () => { expect(container.textContent).not.toContain('registry.configInvalid'); }); + it('labels self-managed missing CLIs as config-only before adding', async () => { + probeClientRequirementsMock.mockResolvedValue([ + { + id: 'opencode', + tool: { name: 'opencode', installed: true }, + runnable: true, + notes: [], + }, + { + id: 'omp', + tool: { name: 'omp', installed: false }, + runnable: false, + notes: ['omp is not available on PATH'], + }, + ]); + + await act(async () => { + root.render(); + }); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + const addConfigButtons = Array.from(container.querySelectorAll('button')) + .filter(button => button.textContent?.includes('actions.addConfig')); + expect(addConfigButtons.length).toBeGreaterThan(0); + + await act(async () => { + addConfigButtons[0].click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(installClientCliMock).not.toHaveBeenCalled(); + expect(saveJsonConfigMock).toHaveBeenCalledWith(expect.stringContaining('"omp"')); + expect(notifySuccessMock).toHaveBeenCalledWith('notifications.configAddedManualCliRequired'); + }); + it('does not downgrade enabled agents on transient probe timeouts during refresh', async () => { probeClientRequirementsMock .mockResolvedValueOnce([ diff --git a/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.tsx b/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.tsx index 77cf591ee8..aba222cada 100644 --- a/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.tsx +++ b/src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.tsx @@ -103,6 +103,21 @@ const PRESETS: AcpClientPreset[] = [ const PRESET_BY_ID = new Map(PRESETS.map(preset => [preset.id, preset])); +interface SelfManagedInstallInfo extends Record { + name: string; + command: string; +} + +function selfManagedInstallInfoForPreset(preset?: AcpClientPreset): SelfManagedInstallInfo | null { + if (!preset || !SELF_MANAGED_INSTALL_PRESET_IDS.has(preset.id)) { + return null; + } + return { + name: preset.name, + command: preset.command, + }; +} + function loadRequirementProbes(options: { force?: boolean } = {}): Promise { return ACPClientAPI.probeClientRequirements({ force: options.force }); } @@ -344,7 +359,7 @@ function AgentStatusBadge({ const AcpAgentsConfig: React.FC = () => { const { t } = useTranslation('settings/acp-agents'); - const { error: notifyError, success: notifySuccess } = useNotification(); + const { error: notifyError, info: notifyInfo, success: notifySuccess } = useNotification(); const jsonEditorRef = useRef(null); const [config, setConfig] = useState({ acpClients: {} }); @@ -682,7 +697,10 @@ const AcpAgentsConfig: React.FC = () => { ), }); - const saveConfig = async (nextConfig = config, options: { mergeEnvDrafts?: boolean } = {}) => { + const saveConfig = async ( + nextConfig = config, + options: { mergeEnvDrafts?: boolean; successMessage?: string } = {} + ) => { savingConfigRef.current = true; try { setSaving(true); @@ -698,7 +716,7 @@ const AcpAgentsConfig: React.FC = () => { await refreshRequirementProbes({ force: true, notifyOnError: false }); loadedRemoteProbeIdsRef.current.clear(); setRemoteProbeRefreshNonce(prev => prev + 1); - notifySuccess(t('notifications.saveSuccess')); + notifySuccess(options.successMessage ?? t('notifications.saveSuccess')); } catch (error) { log.error('Failed to save ACP agent config', error); notifyError(error instanceof Error ? error.message : String(error), { @@ -710,7 +728,10 @@ const AcpAgentsConfig: React.FC = () => { } }; - const addPresetClient = async (preset: AcpClientPreset) => { + const addPresetClient = async ( + preset: AcpClientPreset, + options: { manualCliRequired?: boolean } = {} + ) => { const nextClient = defaultConfigForPreset(preset); const next = { ...config, @@ -726,7 +747,15 @@ const AcpAgentsConfig: React.FC = () => { [preset.id]: formatEnv(nextClient.env), })); setDirty(true); - await saveConfig(next, { mergeEnvDrafts: false }); + await saveConfig(next, { + mergeEnvDrafts: false, + successMessage: options.manualCliRequired + ? t('notifications.configAddedManualCliRequired', { + name: preset.name, + command: preset.command, + }) + : t('notifications.configAdded'), + }); }; const saveJsonConfig = async () => { @@ -824,8 +853,9 @@ const AcpAgentsConfig: React.FC = () => { issueKind: RequirementIssueKind; probe?: AcpClientRequirementProbe; requiresAdapter: boolean; + selfManagedInstallInfo?: SelfManagedInstallInfo | null; }) => { - const { status, issueKind, probe, requiresAdapter } = args; + const { status, issueKind, probe, requiresAdapter, selfManagedInstallInfo } = args; const lines: string[] = []; if (status === 'enabled') { lines.push(t('registry.enabled')); @@ -848,7 +878,11 @@ const AcpAgentsConfig: React.FC = () => { } else if (issueKind === 'adapter_missing' || (requiresAdapter && probe?.adapter && !probe.adapter.installed)) { lines.push(t('registry.acpMissingDetail')); } else if (issueKind === 'cli_missing' || probe?.tool.installed === false) { - lines.push(t('registry.cliMissingDetail')); + lines.push( + selfManagedInstallInfo + ? t('registry.selfManagedCliMissingDetail', selfManagedInstallInfo) + : t('registry.cliMissingDetail') + ); } else if (status === 'invalid') { lines.push(t('registry.configInvalidDetail')); } @@ -875,6 +909,12 @@ const AcpAgentsConfig: React.FC = () => { return t('remote.summary', { available, total }); }, [t]); + const showSelfManagedInstallInfo = useCallback((info: SelfManagedInstallInfo) => { + notifyInfo(t('registry.selfManagedCliMissingDetail', info), { + title: t('registry.cliMissing'), + }); + }, [notifyInfo, t]); + const openLearnMore = useCallback(() => { void systemAPI.openExternal('https://agentclientprotocol.com/get-started/introduction').catch((error) => { log.error('Failed to open ACP documentation', error); @@ -1022,8 +1062,9 @@ const AcpAgentsConfig: React.FC = () => { const hasConfigEntry = Boolean(config.acpClients[preset.id]); const configured = hasConfigEntry; const enabled = clientConfig.enabled; - const requiresAdapter = preset.id !== 'opencode' || Boolean(requirementProbe?.adapter); + const requiresAdapter = Boolean(requirementProbe?.adapter || !NATIVE_ACP_PRESET_IDS.has(preset.id)); const issueKind = getIssueKind({ probe: requirementProbe, requiresAdapter }); + const selfManagedInstallInfo = selfManagedInstallInfoForPreset(preset); const status = getAgentRowStatus({ configured, enabled, @@ -1039,11 +1080,15 @@ const AcpAgentsConfig: React.FC = () => { probe: requirementProbe, requiresAdapter, }); + const selfManagedCliMissing = Boolean(selfManagedInstallInfo) + && status === 'not_installed' + && (issueKind === 'cli_missing' || requirementProbe?.tool.installed === false); const statusTitle = getStatusTitle({ status, issueKind, probe: requirementProbe, requiresAdapter, + selfManagedInstallInfo, }); const installing = installingClientIds.has(preset.id); const configuring = installingClientIds.has(preset.id); @@ -1118,6 +1163,16 @@ const AcpAgentsConfig: React.FC = () => { {t('actions.configureAcp')} + ) : selfManagedCliMissing && hasConfigEntry && selfManagedInstallInfo ? ( + ) : canViewError ? ( - ) : ( + ) : !hasConfigEntry ? ( + ) : ( + null )} @@ -1274,8 +1333,11 @@ const AcpAgentsConfig: React.FC = () => { const hasConfigEntry = Boolean(clientConfig); const effectiveConfig = clientConfig ?? (preset ? defaultConfigForPreset(preset) : undefined); const enabled = effectiveConfig?.enabled ?? true; - const requiresAdapter = Boolean(requirementProbe?.adapter || preset?.id !== 'opencode'); + const requiresAdapter = Boolean( + requirementProbe?.adapter || (preset && !NATIVE_ACP_PRESET_IDS.has(preset.id)) + ); const issueKind = getIssueKind({ probe: requirementProbe, requiresAdapter }); + const selfManagedInstallInfo = selfManagedInstallInfoForPreset(preset); const status = getAgentRowStatus({ configured: hasConfigEntry, enabled, @@ -1300,6 +1362,7 @@ const AcpAgentsConfig: React.FC = () => { enabled, requiresAdapter, issueKind, + selfManagedInstallInfo, status, displayName, description, @@ -1369,9 +1432,13 @@ const AcpAgentsConfig: React.FC = () => { issueKind: row.issueKind, probe: row.requirementProbe, requiresAdapter: row.requiresAdapter, + selfManagedInstallInfo: row.selfManagedInstallInfo, }); const canInstallCli = row.preset && row.status === 'not_installed' && row.issueKind === 'cli_missing' && !SELF_MANAGED_INSTALL_PRESET_IDS.has(row.preset.id); + const selfManagedCliMissing = Boolean(row.selfManagedInstallInfo) + && row.status === 'not_installed' + && (row.issueKind === 'cli_missing' || row.requirementProbe?.tool.installed === false); const canViewError = row.status === 'invalid' || row.status === 'partial' || row.issueKind === 'connection_failed' || row.issueKind === 'permission_denied' @@ -1431,6 +1498,16 @@ const AcpAgentsConfig: React.FC = () => { {t('actions.installCli')} + ) : selfManagedCliMissing && row.selfManagedInstallInfo ? ( + ) : row.status === 'enabled' || row.status === 'ready' ? ( row.clientConfig ? (