From fc663a9bba557f21908b9ee06d0390151eb3ba28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 20 Aug 2026 19:52:28 +0100 Subject: [PATCH] fix(web): compact model availability warning (#2857) --- apps/web/components/combobox.tsx | 5 + .../task-create-dialog-options.test.tsx | 68 +++++++--- .../components/task-create-dialog-options.tsx | 128 ++++++++++++------ .../mobile-no-silent-model-fallback.spec.ts | 11 +- .../settings/no-silent-model-fallback.spec.ts | 15 +- .../plan.md | 103 ++++++++++++++ .../task-01-shared-warning-tooltip.md | 103 ++++++++++++++ docs/specs/no-silent-model-fallback/spec.md | 10 +- 8 files changed, 381 insertions(+), 62 deletions(-) create mode 100644 docs/plans/no-silent-model-fallback-warning-tooltip/plan.md create mode 100644 docs/plans/no-silent-model-fallback-warning-tooltip/task-01-shared-warning-tooltip.md diff --git a/apps/web/components/combobox.tsx b/apps/web/components/combobox.tsx index ea5b05dbda..609d4128d0 100644 --- a/apps/web/components/combobox.tsx +++ b/apps/web/components/combobox.tsx @@ -25,6 +25,8 @@ export type ComboboxOption = { description?: string; keywords?: string[]; renderLabel?: () => React.ReactNode; + /** Optional label renderer for the selected value inside the trigger. */ + renderTriggerLabel?: () => React.ReactNode; /** When true the option renders dimmed and isn't selectable. */ disabled?: boolean; /** Tooltip shown on hover when disabled is true. */ @@ -72,6 +74,9 @@ function TriggerLabel({ plainTrigger: boolean; placeholder: string; }) { + if (!plainTrigger && selectedOption?.renderTriggerLabel) { + return selectedOption.renderTriggerLabel(); + } if (!plainTrigger && selectedOption?.renderLabel) { return selectedOption.renderLabel(); } diff --git a/apps/web/components/task-create-dialog-options.test.tsx b/apps/web/components/task-create-dialog-options.test.tsx index 6a0ca2a25e..192e1339b8 100644 --- a/apps/web/components/task-create-dialog-options.test.tsx +++ b/apps/web/components/task-create-dialog-options.test.tsx @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { AgentProfileOption } from "@/lib/state/slices"; import type { AvailableAgent } from "@/lib/types/http-agents"; import type { Executor } from "@/lib/types/http"; +import { TooltipProvider } from "@kandev/ui/tooltip"; import { computeExecutorHint, useAgentProfileOptions } from "./task-create-dialog-options"; // Minimal store shape consumed by useAvailableAgents. @@ -42,6 +43,7 @@ const AGENT_WITH_GPT: AvailableAgent = { const GONE_MODEL = "claude-gone"; const DATA_DISABLED = "data-disabled"; +const MODEL_PROBE_WARNING_TEST_ID = "agent-profile-model-probe-warning"; function profileOption(overrides: Partial): AgentProfileOption { return { @@ -57,18 +59,20 @@ function profileOption(overrides: Partial): AgentProfileOpti function OptionsProbe({ profiles }: { profiles: AgentProfileOption[] }) { const options = useAgentProfileOptions(profiles); return ( -
- {options.map((option, index) => ( -
- {option.renderLabel()} -
- ))} -
+ +
+ {options.map((option, index) => ( +
+ {option.renderLabel()} +
+ ))} +
+ ); } @@ -77,6 +81,14 @@ function renderOptions(profiles: AgentProfileOption[]) { return screen.getByTestId("option-0"); } +function getModelProbeWarning() { + return screen.getByTestId(MODEL_PROBE_WARNING_TEST_ID); +} + +function getModelProbeWarningLabel() { + return getModelProbeWarning().getAttribute("aria-label"); +} + beforeEach(() => { vi.clearAllMocks(); setAvailableAgents([AGENT_WITH_GPT]); @@ -151,7 +163,7 @@ describe("useAgentProfileOptions executor-authoritative model hint", () => { it("keeps a profile whose start model is absent from the host probe selectable", () => { const option = renderOptions([profileOption({ model: GONE_MODEL })]); expect(option.getAttribute(DATA_DISABLED)).toBeNull(); - expect(screen.getByTitle(/claude-gone/)).not.toBeNull(); + expect(getModelProbeWarningLabel()).toContain(GONE_MODEL); }); it("keeps a profile with an available start model selectable", () => { @@ -169,7 +181,7 @@ describe("useAgentProfileOptions executor-authoritative model hint", () => { expect(option.getAttribute(DATA_DISABLED)).toBeNull(); expect(option.getAttribute("data-reason")).toBeNull(); - expect(screen.getByTitle(/claude-gone/)).not.toBeNull(); + expect(getModelProbeWarningLabel()).toContain(GONE_MODEL); }); it("keeps a profile selectable when both saved models are absent from the host probe", () => { @@ -177,13 +189,37 @@ describe("useAgentProfileOptions executor-authoritative model hint", () => { profileOption({ model: GONE_MODEL, fallback_model: "other-gone" }), ]); expect(option.getAttribute(DATA_DISABLED)).toBeNull(); - expect(screen.getByTitle(/claude-gone/)).not.toBeNull(); + expect(getModelProbeWarningLabel()).toContain(GONE_MODEL); }); it("keeps auto-fallback profiles selectable and shows the host probe hint", () => { const option = renderOptions([profileOption({ model: GONE_MODEL, auto_fallback: true })]); expect(option.getAttribute(DATA_DISABLED)).toBeNull(); - expect(screen.getByTitle(/claude-gone/)).not.toBeNull(); + expect(getModelProbeWarningLabel()).toContain(GONE_MODEL); + }); + + it("renders the host probe hint as one compact warning trigger", () => { + const option = renderOptions([profileOption({ model: GONE_MODEL })]); + + expect(getModelProbeWarning()).toBeTruthy(); + expect(option.textContent).not.toContain( + "The host probe did not advertise claude-gone. The selected executor will decide the model at launch.", + ); + }); + + it("uses a non-interactive warning indicator in the selected trigger", () => { + const { result } = renderHook(() => + useAgentProfileOptions([profileOption({ model: GONE_MODEL })]), + ); + + render( + +
{result.current[0]?.renderTriggerLabel?.()}
+ , + ); + + expect(screen.queryByTestId(MODEL_PROBE_WARNING_TEST_ID)).toBeNull(); + expect(screen.getByTitle(/claude-gone/)).toBeTruthy(); }); it("does not gate when the agent model list is unknown (probe not landed)", () => { diff --git a/apps/web/components/task-create-dialog-options.tsx b/apps/web/components/task-create-dialog-options.tsx index af6797fd5d..a4b7e26c13 100644 --- a/apps/web/components/task-create-dialog-options.tsx +++ b/apps/web/components/task-create-dialog-options.tsx @@ -1,11 +1,20 @@ "use client"; -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import { t } from "@/lib/i18n"; import { IconAlertTriangle, IconGitBranch, IconTerminal2 } from "@tabler/icons-react"; import { Badge } from "@kandev/ui/badge"; +import { + Drawer, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from "@kandev/ui/drawer"; import { ScrollOnOverflow } from "@kandev/ui/scroll-on-overflow"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import type { LocalRepository, Repository, @@ -21,16 +30,62 @@ import { formatUserHomePath, truncateRepoPath } from "@/lib/utils"; import { getExecutorIcon } from "@/lib/executor-icons"; import { AgentLogo } from "@/components/agent-logo"; import { getCapabilityWarning } from "@/lib/capability-warning"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; import { buildBranchKeywords } from "./task-create-dialog-pill"; type OptionItem = { value: string; label: string; renderLabel: () => React.ReactNode; + renderTriggerLabel?: () => React.ReactNode; disabled?: boolean; disabledReason?: string; }; +function ModelProbeWarning({ note }: { note: string }) { + const usesTouchDrawer = useTouchDrawer(); + const [drawerOpen, setDrawerOpen] = useState(false); + const trigger = ( + + ); + + if (usesTouchDrawer) { + return ( + + {trigger} + + + {note} + {note} + + + + ); + } + + return ( + + {trigger} + {note} + + ); +} + +function ModelProbeWarningIndicator({ note }: { note: string }) { + return ; +} + export function useRepositoryOptions( repositories: Repository[], discoveredRepositories: LocalRepository[], @@ -152,49 +207,44 @@ export function useAgentProfileOptions(agentProfiles: AgentProfileOption[]): Opt const modelProbeNote = startModelGone ? t("settings:profileStartModelNotAdvertisedOnHost", { model: profile.model }) : undefined; + const renderProfileLabel = (modelProbeWarning: React.ReactNode) => ( + + + + + {agentLabel} + {warning && ( + + )} + {modelProbeWarning} + + + {isPassthrough && ( + + )} + {profileLabel ? ( + + {profileLabel} + + ) : null} + + + + ); return { value: profile.id, label: profile.label, disabled: undefined, disabledReason: undefined, - renderLabel: () => ( - - - - - {agentLabel} - {warning && ( - - )} - {modelProbeNote && ( - - )} - - - {isPassthrough && ( - - )} - {profileLabel ? ( - - {profileLabel} - - ) : null} - - - {modelProbeNote && ( - - - {modelProbeNote} - - )} - - ), + renderLabel: () => + renderProfileLabel(modelProbeNote ? : null), + renderTriggerLabel: () => + renderProfileLabel( + modelProbeNote ? : null, + ), }; }); }, [agentProfiles, availableAgents, t]); diff --git a/apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts b/apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts index 1978bf32b0..1a9a0e3d2e 100644 --- a/apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts +++ b/apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts @@ -26,7 +26,16 @@ test.describe("executor-authoritative model selection on mobile", () => { .getByRole("option", { name: profile.name, exact: false }); await expect(option).toBeVisible(); await expect(option).toBeEnabled(); - await expect(dialog).toContainText(UNADVERTISED_MODEL); + const warning = option.getByTestId("agent-profile-model-probe-warning"); + await expect(warning).toBeVisible(); + const warningText = `The host probe did not advertise ${UNADVERTISED_MODEL}. The selected executor will decide the model at launch.`; + await expect(dialog).not.toContainText(warningText); + await warning.tap(); + await expect( + testPage + .locator('[data-slot="drawer-content"][data-state="open"]') + .filter({ hasText: warningText }), + ).toBeVisible(); await assertNoDocumentHorizontalOverflow(testPage, "mobile model mismatch selector"); } finally { await apiClient.deleteAgentProfile(profile.id, true).catch(() => {}); diff --git a/apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts b/apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts index e11b1e5e79..382ab9224b 100644 --- a/apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts +++ b/apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts @@ -25,9 +25,18 @@ test.describe("executor-authoritative model selection", () => { .getByRole("option", { name: profile.name, exact: false }); await expect(option).toBeVisible(); await expect(option).toBeEnabled(); - await expect(dialog).toContainText( - `The host probe did not advertise ${UNADVERTISED_MODEL}. The selected executor will decide the model at launch.`, - ); + const warning = option.getByTestId("agent-profile-model-probe-warning"); + await expect(warning).toBeVisible(); + const warningText = `The host probe did not advertise ${UNADVERTISED_MODEL}. The selected executor will decide the model at launch.`; + await expect(dialog).not.toContainText(warningText); + await warning.hover(); + await expect( + testPage + .locator('[data-slot="tooltip-content"]:not([data-state="closed"])') + .filter({ hasText: warningText }), + ).toBeVisible(); + await option.click(); + await expect(selector.locator("button")).toHaveCount(0); } finally { await apiClient.deleteAgentProfile(profile.id, true).catch(() => {}); } diff --git a/docs/plans/no-silent-model-fallback-warning-tooltip/plan.md b/docs/plans/no-silent-model-fallback-warning-tooltip/plan.md new file mode 100644 index 0000000000..ccdf61313f --- /dev/null +++ b/docs/plans/no-silent-model-fallback-warning-tooltip/plan.md @@ -0,0 +1,103 @@ +--- +spec: docs/specs/no-silent-model-fallback/spec.md +created: 2026-08-20 +status: implemented +--- + +# Implementation Plan: Compact host-probe warning + +## Overview + +Replace the always-visible host-probe mismatch sentence in the shared agent +profile option renderer with a single warning icon. The profile remains +selectable, and the existing localized sentence remains available through a +focusable and hoverable tooltip on fine pointers or a drawer on coarse +pointers. The change is frontend-only and covers every consumer of +`useAgentProfileOptions`, including task creation, new sessions, subtasks, +Quick Chat, and Office setup. + +## Frontend + +### Shared profile option renderer + +- `apps/web/components/task-create-dialog-options.tsx` + - Keep `modelProbeNote` derived from the existing host catalog comparison. + - Render it as one amber `IconAlertTriangle` tooltip/drawer trigger instead + of a persistent text row. + - Give the trigger a stable test id, accessible label, focus behavior, and a + touch-usable hit area. Preserve the existing capability warning and + terminal-mode indicators. + - Keep the interactive trigger inside dropdown options, and render a + noninteractive warning indicator in the selected combobox trigger so the + shared option renderer never nests a button inside the combobox button. + +- `apps/web/components/combobox.tsx` + - Allow an option to provide a selected-trigger renderer distinct from its + dropdown renderer. + +## Mobile contract + +- Desktop and mobile keep the existing `AgentSelector` combobox and option + list. The warning is supplementary and never blocks profile selection. +- The warning icon is a focusable/hoverable disclosure in the option row on + fine pointers, and opens the shared coarse-pointer drawer on touch devices. + The option list remains the single scroll owner and keeps its existing + viewport containment. +- Reuse the existing Radix tooltip and `useTouchDrawer`/Drawer patterns from + the shared web UI. + +## Tests + +### Unit test + +- **What:** a host-mismatched profile remains enabled, renders exactly one + warning trigger, and does not render the advisory sentence as persistent + option content. +- **File:** `apps/web/components/task-create-dialog-options.test.tsx` +- **How:** extend the existing `useAgentProfileOptions` render probe; assert + the warning trigger and absence of the always-visible note while retaining + the current selectability assertions. + +### Desktop E2E + +- **Scenario:** a mismatched profile in the create-task picker remains enabled; + the option shows the warning icon; hovering it reveals the localized host + probe message. +- **File:** `apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts` +- **What to verify:** the note is not persistent option text, the tooltip is + visible after hover, and the profile remains enabled. + +### Mobile E2E + +- **Scenario:** the same mismatched profile is reachable in the mobile create + task picker; tapping the warning icon opens the advisory drawer. +- **File:** `apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts` +- **What to verify:** the icon is reachable by touch, the drawer is visible, + the profile remains enabled, and the document has no horizontal overflow. + +## Verification Results + +- `cd apps && pnpm install --frozen-lockfile` — passed. +- `cd apps/web && pnpm vitest run components/task-create-dialog-options.test.tsx` — passed, 17 tests. +- `cd apps/web && pnpm exec eslint components/combobox.tsx components/task-create-dialog-options.tsx components/task-create-dialog-options.test.tsx e2e/tests/settings/no-silent-model-fallback.spec.ts e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts` — passed. +- `cd apps/web && pnpm run typecheck` — passed. +- `cd apps/web && pnpm run i18n:ratchet` — passed, zero new or modified violations. +- `cd apps/web && pnpm e2e:run --project chromium tests/settings/no-silent-model-fallback.spec.ts` — passed, 1 test. +- `cd apps/web && pnpm e2e:run --project mobile-chrome tests/settings/mobile-no-silent-model-fallback.spec.ts` — passed, 1 test. +- `git diff --check` — passed. +- Fresh synthetic desktop and mobile screenshots were captured, inspected, and + compressed for the PR description. +- Review remediation: the selected trigger now uses a noninteractive + indicator, and coarse-pointer inspection uses a Drawer instead of a tooltip. + +## Implementation Waves And Parallel Candidates + +One sequential task: the shared renderer, unit test, and existing desktop/mobile +E2E assertions must stay in sync. + +- [x] [task-01-shared-warning-tooltip](task-01-shared-warning-tooltip.md) + +## Open Questions + +None. The existing host-probe message and selectability contract remain +unchanged; only its presentation changes. diff --git a/docs/plans/no-silent-model-fallback-warning-tooltip/task-01-shared-warning-tooltip.md b/docs/plans/no-silent-model-fallback-warning-tooltip/task-01-shared-warning-tooltip.md new file mode 100644 index 0000000000..28520f95dd --- /dev/null +++ b/docs/plans/no-silent-model-fallback-warning-tooltip/task-01-shared-warning-tooltip.md @@ -0,0 +1,103 @@ +--- +id: "01-shared-warning-tooltip" +title: "Compact shared profile warning" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/no-silent-model-fallback/spec.md" +--- + +# Task 01: Compact shared profile warning + +## Acceptance + +1. A profile whose saved start model is absent from the known host catalog + remains selectable and renders one focusable/hoverable amber warning icon + without nesting an interactive control in the selected combobox trigger. +2. The existing localized host-probe sentence is absent from the persistent + option row and appears in a fine-pointer tooltip or coarse-pointer drawer + when the warning icon is inspected. +3. The create-task desktop and mobile flows retain the warning behavior, + profile selectability, and mobile no-horizontal-overflow guarantee. + +## Verification + +From a fresh worktree, first run: + +```sh +cd apps && pnpm install --frozen-lockfile +``` + +Then run the focused checks: + +```sh +cd apps/web && pnpm vitest run components/task-create-dialog-options.test.tsx +cd apps/web && pnpm run typecheck +cd apps/web && pnpm run i18n:ratchet +cd apps/web && pnpm e2e:run --project chromium tests/settings/no-silent-model-fallback.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome tests/settings/mobile-no-silent-model-fallback.spec.ts +``` + +## Files likely touched + +- `apps/web/components/task-create-dialog-options.tsx` +- `apps/web/components/combobox.tsx` +- `apps/web/components/task-create-dialog-options.test.tsx` +- `apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts` +- `apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts` + +## Dependencies + +None. + +## Parallelism + +Sequential. The unit and E2E assertions share the warning trigger contract. + +## Inputs + +- `docs/specs/no-silent-model-fallback/spec.md`, section 8 and frontend test + requirements. +- `docs/decisions/2026-08-15-executor-authoritative-model-selection.md`, which + keeps the host probe advisory and the executor authoritative. +- `apps/web/AGENTS.md`, especially tooltip, i18n, and mobile guidance. +- Existing `Combobox` disabled-option tooltip pattern in + `apps/web/components/combobox.tsx`. + +## Risks + +- The warning trigger is nested inside a command option. Keep focus and pointer + handling from accidentally selecting or dismissing the profile option while + the user inspects the message. +- Radix tooltip content is portalled. E2E selectors must target the visible + tooltip content, not a hidden or stale portal instance. + +## Output contract + +Report the summary, actual files changed, exact commands and results, any +mobile-rendered verification, blockers, and synchronized task/plan status. + +## Results + +Implemented the compact warning trigger in the shared agent-profile option +renderer. The warning remains selectable, has an accessible focusable trigger, +opens in a fine-pointer tooltip or coarse-pointer Drawer, and uses a +noninteractive indicator in the selected combobox trigger. The advisory +sentence is no longer persistent option content. + +Verification: + +- `cd apps && pnpm install --frozen-lockfile` — passed. +- `cd apps/web && pnpm vitest run components/task-create-dialog-options.test.tsx` — passed, 17 tests. +- `cd apps/web && pnpm exec eslint components/combobox.tsx components/task-create-dialog-options.tsx components/task-create-dialog-options.test.tsx e2e/tests/settings/no-silent-model-fallback.spec.ts e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts` — passed. +- `cd apps/web && pnpm run typecheck` — passed. +- `cd apps/web && pnpm run i18n:ratchet` — passed, zero new or modified violations. +- `cd apps/web && pnpm e2e:run --project chromium tests/settings/no-silent-model-fallback.spec.ts` — passed, 1 test. +- `cd apps/web && pnpm e2e:run --project mobile-chrome tests/settings/mobile-no-silent-model-fallback.spec.ts` — passed, 1 test. +- `git diff --check` — passed. +- Fresh synthetic desktop and mobile screenshots were captured, inspected, and + compressed for the PR description. +- Review remediation completed: the selected trigger no longer nests an + interactive warning button, and coarse-pointer inspection uses the shared + Drawer pattern. diff --git a/docs/specs/no-silent-model-fallback/spec.md b/docs/specs/no-silent-model-fallback/spec.md index b7d3ca64ab..b0314042f0 100644 --- a/docs/specs/no-silent-model-fallback/spec.md +++ b/docs/specs/no-silent-model-fallback/spec.md @@ -368,8 +368,11 @@ judges added lines even in unmigrated files. This difference is advisory only. - Every profile remains selectable. -- A missing host model shows an amber warning and visible secondary text. -- The warning says that the executor decides availability at launch. +- A missing host model shows one amber warning icon beside the profile name. +- On fine pointers, hovering or focusing the warning icon reveals the full + localized advisory that the executor decides availability at launch. On + coarse pointers, tapping the icon opens the same advisory in a drawer. The + advisory is not shown as an always-visible secondary row in the option list. - The warning does not promise that an explicit fallback will be available. - The warning does not change the saved profile model. @@ -439,7 +442,8 @@ E2E (Playwright, `apps/web/e2e`): - Mock backend (`KANDEV_E2E_MOCK=true`): create a profile whose start model is not in the host catalog. Make sure that the task-create picker keeps the - profile selectable and shows the advisory warning. + profile selectable, shows one warning icon, and reveals the advisory warning + through the fine-pointer tooltip or coarse-pointer drawer. - Launch with an executor catalog that omits the profile model. Make sure that no model-selection call occurs, the task continues, and chat shows one warning. - Reload the task page. Make sure that the warning remains in chat.