From bc7798af285bfebaaf10cf959266fdc5ea27ef49 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 19 May 2026 19:19:25 +0700 Subject: [PATCH] Allow ACP providers without model lists --- .../combined-model-selector.test.ts | 55 ++++++++ .../components/combined-model-selector.tsx | 122 +++++++++++++----- .../combined-model-selector.utils.ts | 59 +++++++++ .../workspace-draft-agent-tab-core.ts | 51 ++++++++ .../workspace-draft-agent-tab.test.ts | 51 ++++++++ .../workspace/workspace-draft-agent-tab.tsx | 46 +------ .../server/agent/providers/acp-agent.test.ts | 25 ++++ .../src/server/agent/providers/acp-agent.ts | 24 +++- 8 files changed, 353 insertions(+), 80 deletions(-) create mode 100644 packages/app/src/screens/workspace/workspace-draft-agent-tab-core.ts create mode 100644 packages/app/src/screens/workspace/workspace-draft-agent-tab.test.ts diff --git a/packages/app/src/components/combined-model-selector.test.ts b/packages/app/src/components/combined-model-selector.test.ts index 41d06b6ff..49a6a6e73 100644 --- a/packages/app/src/components/combined-model-selector.test.ts +++ b/packages/app/src/components/combined-model-selector.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import type { AgentModelDefinition } from "@server/server/agent/agent-sdk-types"; import { buildModelRows, + buildProviderGroups, buildSelectedTriggerLabel, filterAndRankModelRows, matchesSearch, @@ -24,6 +25,13 @@ describe("combined model selector helpers", () => { defaultModeId: "auto", modes: [], }, + { + id: "deepseek-tui", + label: "DeepSeek TUI", + description: "DeepSeek TUI provider", + defaultModeId: "default", + modes: [], + }, ]; const claudeModels: AgentModelDefinition[] = [ @@ -112,6 +120,53 @@ describe("combined model selector helpers", () => { expect(filterAndRankModelRows(rows, "gpt54").map((row) => row.modelId)).toEqual(["gpt-5.4"]); }); + it("includes providers that expose no models", () => { + const rows = buildModelRows( + providerDefinitions, + new Map([ + ["claude", claudeModels], + ["deepseek-tui", []], + ]), + ); + + const groups = buildProviderGroups( + providerDefinitions, + new Map([ + ["claude", claudeModels], + ["deepseek-tui", []], + ]), + rows, + "", + ); + + expect(groups).toEqual([ + expect.objectContaining({ + providerId: "claude", + hasNoModels: false, + }), + expect.objectContaining({ + providerId: "deepseek-tui", + providerLabel: "DeepSeek TUI", + rows: [], + hasNoModels: true, + }), + ]); + }); + + it("matches model-less providers by provider name", () => { + const groups = buildProviderGroups( + providerDefinitions, + new Map([ + ["claude", claudeModels], + ["deepseek-tui", []], + ]), + [], + "deepseek", + ); + + expect(groups.map((group) => group.providerId)).toEqual(["deepseek-tui"]); + }); + it("keeps the selected trigger label model-only", () => { expect(resolveProviderLabel(providerDefinitions, "codex")).toBe("Codex"); expect(buildSelectedTriggerLabel("GPT-5.4")).toBe("GPT-5.4"); diff --git a/packages/app/src/components/combined-model-selector.tsx b/packages/app/src/components/combined-model-selector.tsx index 081a6b760..b07389225 100644 --- a/packages/app/src/components/combined-model-selector.tsx +++ b/packages/app/src/components/combined-model-selector.tsx @@ -47,9 +47,11 @@ function drillDownRowStyle({ import { getProviderIcon } from "@/components/provider-icons"; import { buildModelRows, + buildProviderGroups, buildSelectedTriggerLabel, filterAndRankModelRows, resolveProviderLabel, + type SelectorProviderGroup, type SelectorModelRow, } from "./combined-model-selector.utils"; @@ -122,31 +124,6 @@ function sortFavoritesFirst( return [...favorites, ...rest]; } -function groupRowsByProvider( - rows: SelectorModelRow[], -): Array<{ providerId: string; providerLabel: string; rows: SelectorModelRow[] }> { - const grouped = new Map< - string, - { providerId: string; providerLabel: string; rows: SelectorModelRow[] } - >(); - - for (const row of rows) { - const existing = grouped.get(row.provider); - if (existing) { - existing.rows.push(row); - continue; - } - - grouped.set(row.provider, { - providerId: row.provider, - providerLabel: row.providerLabel, - rows: [row], - }); - } - - return Array.from(grouped.values()); -} - function ModelRow({ row, isSelected, @@ -317,29 +294,41 @@ interface GroupProviderButtonProps { providerId: string; providerLabel: string; rowCount: number; + hasNoModels: boolean; + disabled?: boolean; onDrillDown: (providerId: string, providerLabel: string) => void; + onSelectDefault: (providerId: string) => void; } function GroupProviderButton({ providerId, providerLabel, rowCount, + hasNoModels, + disabled, onDrillDown, + onSelectDefault, }: GroupProviderButtonProps) { const { theme } = useUnistyles(); const ProvIcon = getProviderIcon(providerId); const handlePress = useCallback(() => { + if (hasNoModels) { + onSelectDefault(providerId); + return; + } onDrillDown(providerId, providerLabel); - }, [onDrillDown, providerId, providerLabel]); + }, [hasNoModels, onDrillDown, onSelectDefault, providerId, providerLabel]); return ( - + {providerLabel} - {rowCount} {rowCount === 1 ? "model" : "models"} + {hasNoModels ? "Default" : `${rowCount} ${rowCount === 1 ? "model" : "models"}`} - + {hasNoModels ? null : ( + + )} ); @@ -348,9 +337,13 @@ function GroupProviderButton({ function GroupedProviderRows({ groupedRows, onDrillDown, + onSelectDefault, + canSelectProvider, }: { - groupedRows: Array<{ providerId: string; providerLabel: string; rows: SelectorModelRow[] }>; + groupedRows: SelectorProviderGroup[]; onDrillDown: (providerId: string, providerLabel: string) => void; + onSelectDefault: (providerId: string) => void; + canSelectProvider: (provider: string) => boolean; }) { return ( @@ -362,7 +355,10 @@ function GroupedProviderRows({ providerId={group.providerId} providerLabel={group.providerLabel} rowCount={group.rows.length} + hasNoModels={group.hasNoModels} + disabled={group.hasNoModels && !canSelectProvider(group.providerId)} onDrillDown={onDrillDown} + onSelectDefault={onSelectDefault} /> ); @@ -371,6 +367,38 @@ function GroupedProviderRows({ ); } +function DefaultProviderRow({ + providerId, + isSelected, + disabled, + onSelect, +}: { + providerId: string; + isSelected: boolean; + disabled?: boolean; + onSelect: (provider: string, modelId: string) => void; +}) { + const { theme } = useUnistyles(); + const ProviderIcon = getProviderIcon(providerId); + const handlePress = useCallback(() => { + onSelect(providerId, ""); + }, [onSelect, providerId]); + const leadingSlot = useMemo( + () => , + [ProviderIcon, theme.iconSize.sm, theme.colors.foregroundMuted], + ); + + return ( + + ); +} + function ProviderModelRows({ rows, selectedProvider, @@ -472,7 +500,16 @@ function SelectorContent({ [favoriteKeys, visibleRows], ); - const allGroupedRows = useMemo(() => groupRowsByProvider(visibleRows), [visibleRows]); + const allGroupedRows = useMemo( + () => buildProviderGroups(providerDefinitions, allProviderModels, visibleRows, normalizedQuery), + [allProviderModels, normalizedQuery, providerDefinitions, visibleRows], + ); + const handleSelectDefaultProvider = useCallback( + (providerId: string) => { + onSelect(providerId, ""); + }, + [onSelect], + ); const hasResults = favoriteRows.length > 0 || allGroupedRows.length > 0; const emptyState = ( @@ -482,6 +519,18 @@ function SelectorContent({ ); if (view.kind === "provider") { + const providerModels = allProviderModels.get(view.providerId); + if (providerModels && providerModels.length === 0 && !normalizedQuery) { + return ( + + ); + } + if (visibleRows.length === 0) { return emptyState; } @@ -513,7 +562,12 @@ function SelectorContent({ /> {allGroupedRows.length > 0 ? ( - + ) : null} {!hasResults ? emptyState : null} @@ -598,6 +652,10 @@ export function CombinedModelSelector({ if (!hasSelectedProvider) { return "Select model"; } + const models = allProviderModels.get(selectedProvider); + if (models && models.length === 0) { + return "Default"; + } return isLoading ? "Loading..." : "Select model"; } const models = allProviderModels.get(selectedProvider); diff --git a/packages/app/src/components/combined-model-selector.utils.ts b/packages/app/src/components/combined-model-selector.utils.ts index 3cea39d62..60c649fbd 100644 --- a/packages/app/src/components/combined-model-selector.utils.ts +++ b/packages/app/src/components/combined-model-selector.utils.ts @@ -5,6 +5,13 @@ import { compareMatchScores, scoreTextFields } from "@/utils/score-match"; export type SelectorModelRow = FavoriteModelRow; +export interface SelectorProviderGroup { + providerId: string; + providerLabel: string; + rows: SelectorModelRow[]; + hasNoModels: boolean; +} + export function resolveProviderLabel( providerDefinitions: AgentProviderDefinition[], providerId: string, @@ -75,3 +82,55 @@ export function filterAndRankModelRows( return scored.map((entry) => entry.row); } + +export function buildProviderGroups( + providerDefinitions: AgentProviderDefinition[], + allProviderModels: Map, + rows: SelectorModelRow[], + normalizedQuery: string, +): SelectorProviderGroup[] { + const rowsByProvider = new Map(); + for (const row of rows) { + const providerRows = rowsByProvider.get(row.provider); + if (providerRows) { + providerRows.push(row); + } else { + rowsByProvider.set(row.provider, [row]); + } + } + + const groups: SelectorProviderGroup[] = []; + for (const definition of providerDefinitions) { + const providerRows = rowsByProvider.get(definition.id) ?? []; + if (providerRows.length > 0) { + groups.push({ + providerId: definition.id, + providerLabel: definition.label, + rows: providerRows, + hasNoModels: false, + }); + continue; + } + + const models = allProviderModels.get(definition.id); + if (!models || models.length > 0) { + continue; + } + + const providerMatches = + !normalizedQuery || + scoreTextFields(normalizedQuery, [definition.label, definition.id]) !== null; + if (!providerMatches) { + continue; + } + + groups.push({ + providerId: definition.id, + providerLabel: definition.label, + rows: [], + hasNoModels: true, + }); + } + + return groups; +} diff --git a/packages/app/src/screens/workspace/workspace-draft-agent-tab-core.ts b/packages/app/src/screens/workspace/workspace-draft-agent-tab-core.ts new file mode 100644 index 000000000..c9d0d55e2 --- /dev/null +++ b/packages/app/src/screens/workspace/workspace-draft-agent-tab-core.ts @@ -0,0 +1,51 @@ +export interface WorkspaceDraftAutoSubmitConfig { + provider: string; + model: string | null; +} + +export function validateDraftSubmission(input: { + text: string; + allowsEmptyAutoSubmit: boolean; + composerState: { + providerDefinitions: unknown[]; + selectedProvider: string | null; + isModelLoading: boolean; + effectiveModelId: string | null; + availableModels: unknown[]; + }; + autoSubmitConfig: WorkspaceDraftAutoSubmitConfig | null; + workspaceDirectory: string | null; + hasClient: boolean; +}): string | null { + const { + text, + allowsEmptyAutoSubmit, + composerState, + autoSubmitConfig, + workspaceDirectory, + hasClient, + } = input; + if (!allowsEmptyAutoSubmit && !text.trim()) { + return "Initial prompt is required"; + } + if (composerState.providerDefinitions.length === 0) { + return "No available providers on the selected host"; + } + if (!(autoSubmitConfig?.provider ?? composerState.selectedProvider)) { + return "Select a model"; + } + if (composerState.isModelLoading) { + return "Model defaults are still loading"; + } + const hasSelectedModel = Boolean(autoSubmitConfig?.model ?? composerState.effectiveModelId); + if (!hasSelectedModel && composerState.availableModels.length > 0) { + return "No model is available for the selected provider"; + } + if (!workspaceDirectory) { + return "Workspace directory not found"; + } + if (!hasClient) { + return "Host is not connected"; + } + return null; +} diff --git a/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.ts b/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.ts new file mode 100644 index 000000000..0d7d5dff8 --- /dev/null +++ b/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "vitest"; + +import { validateDraftSubmission } from "./workspace-draft-agent-tab-core"; + +const baseComposerState = { + providerDefinitions: [{ id: "deepseek-tui" }], + selectedProvider: "deepseek-tui", + isModelLoading: false, + effectiveModelId: "", + availableModels: [], +}; + +function validate(overrides = {}) { + return validateDraftSubmission({ + text: "hello", + allowsEmptyAutoSubmit: false, + composerState: baseComposerState, + autoSubmitConfig: null, + workspaceDirectory: "/tmp/project", + hasClient: true, + ...overrides, + }); +} + +describe("workspace draft agent model validation", () => { + test("allows a ready provider with no models to submit without a selected model", () => { + expect(validate({})).toBeNull(); + }); + + test("keeps waiting while model defaults are loading", () => { + expect( + validate({ + composerState: { + ...baseComposerState, + isModelLoading: true, + }, + }), + ).toBe("Model defaults are still loading"); + }); + + test("still requires a selected model when the provider exposes models", () => { + expect( + validate({ + composerState: { + ...baseComposerState, + availableModels: [{ id: "deepseek/deepseek-v4-pro" }], + }, + }), + ).toBe("No model is available for the selected provider"); + }); +}); diff --git a/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx b/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx index 20236d2c3..28442654e 100644 --- a/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx +++ b/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx @@ -22,6 +22,7 @@ import { useWorkspaceDraftSubmissionStore } from "@/stores/workspace-draft-submi import { encodeImages } from "@/utils/encode-images"; import type { WorkspaceFileOpenRequest } from "@/workspace/file-open"; import { shouldAutoFocusWorkspaceDraftComposer } from "@/screens/workspace/workspace-draft-pane-focus"; +import { validateDraftSubmission } from "@/screens/workspace/workspace-draft-agent-tab-core"; import type { AgentCapabilityFlags } from "@server/server/agent/agent-sdk-types"; import type { AgentSnapshotPayload } from "@server/shared/messages"; import type { DaemonClient } from "@server/client/daemon-client"; @@ -73,51 +74,6 @@ function resolveAutoSubmitConfig( }; } -function validateDraftSubmission(input: { - text: string; - allowsEmptyAutoSubmit: boolean; - composerState: { - providerDefinitions: unknown[]; - selectedProvider: string | null; - isModelLoading: boolean; - effectiveModelId: string | null; - }; - autoSubmitConfig: AutoSubmitConfig | null; - workspaceDirectory: string | null; - hasClient: boolean; -}): string | null { - const { - text, - allowsEmptyAutoSubmit, - composerState, - autoSubmitConfig, - workspaceDirectory, - hasClient, - } = input; - if (!allowsEmptyAutoSubmit && !text.trim()) { - return "Initial prompt is required"; - } - if (composerState.providerDefinitions.length === 0) { - return "No available providers on the selected host"; - } - if (!(autoSubmitConfig?.provider ?? composerState.selectedProvider)) { - return "Select a model"; - } - if (composerState.isModelLoading) { - return "Model defaults are still loading"; - } - if (!(autoSubmitConfig?.model ?? composerState.effectiveModelId)) { - return "No model is available for the selected provider"; - } - if (!workspaceDirectory) { - return "Workspace directory not found"; - } - if (!hasClient) { - return "Host is not connected"; - } - return null; -} - function resolveDraftModeIdOverride(input: { autoSubmitConfig: AutoSubmitConfig | null; modeOptionsCount: number; diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index e80b33f83..b8f889c56 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -731,6 +731,31 @@ describe("ACPAgentSession Zed parity", () => { expect(setSessionConfigOption).not.toHaveBeenCalled(); }); + test("does not fail session start when configured model cannot be applied by ACP", async () => { + const logger = createTestLogger(); + const childLogger = { trace: vi.fn(), warn: vi.fn() }; + vi.spyOn(logger, "child").mockReturnValue(asInternals(childLogger)); + const session = createSessionWithConfig( + { provider: "deepseek-tui", model: "deepseek/v4" }, + logger, + ); + const { internals, setSessionConfigOption, unstableSetSessionModel } = + prepareConfiguredOverrideSession(session, { + currentModel: null, + availableModels: null, + configOptions: [], + connection: { unstable_setSessionModel: undefined }, + }); + + await expect(internals.applyConfiguredOverrides()).resolves.toBeUndefined(); + expect(unstableSetSessionModel).not.toHaveBeenCalled(); + expect(setSessionConfigOption).not.toHaveBeenCalled(); + expect(childLogger.warn).toHaveBeenCalledWith( + { value: "deepseek/v4" }, + "deepseek-tui does not expose ACP model selection; using provider default model", + ); + }); + test("routes config_option_update and refreshes derived mode, model, and thinking state", async () => { const session = createSession(); const internals = asInternals(session); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 64bd89397..583fe88b6 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1376,7 +1376,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { } if (typeof this.connection.unstable_setSessionModel !== "function") { - throw new Error(`${this.provider} does not expose ACP model selection`); + throw new Error(this.modelSelectionUnavailableMessage()); } try { @@ -1398,7 +1398,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { const modelOption = selection.configOption; if (!modelOption) { - throw new Error(`${this.provider} does not expose ACP model selection`); + throw new Error(this.modelSelectionUnavailableMessage()); } if (!selection.configChoice) { this.warnInvalidSelection( @@ -1904,7 +1904,17 @@ export class ACPAgentSession implements AgentSession, ACPClient { availableModels: this.availableModels, configOptions: this.configOptions, }); - await this.setModelWithSelection({ modelId: configuredModelId, selection }); + try { + await this.setModelWithSelection({ modelId: configuredModelId, selection }); + } catch (error) { + if (!this.isModelSelectionUnavailableError(error)) { + throw error; + } + this.logger.warn( + { value: configuredModelId }, + `${this.provider} does not expose ACP model selection; using provider default model`, + ); + } } if (this.config.thinkingOptionId && this.config.thinkingOptionId !== this.thinkingOptionId) { await this.setThinkingOption(this.config.thinkingOptionId); @@ -1915,6 +1925,14 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.logger.warn({ value }, message); } + private modelSelectionUnavailableMessage(): string { + return `${this.provider} does not expose ACP model selection`; + } + + private isModelSelectionUnavailableError(error: unknown): boolean { + return error instanceof Error && error.message === this.modelSelectionUnavailableMessage(); + } + private translateSessionUpdate(update: SessionUpdate): AgentStreamEvent[] { switch (update.sessionUpdate) { case "user_message_chunk": {