From 2da411ee2da9fd106e048c8d23d479558c99deb5 Mon Sep 17 00:00:00 2001 From: GCWing Date: Sun, 11 Oct 2026 16:57:53 +0800 Subject: [PATCH 1/3] feat(ui): add capsule actions and shared glass styling --- design-system/AGENTS.md | 6 ++ .../apps/design-lab/src/i18n/messages.ts | 18 ++-- .../src/pages/ComponentDetailPage.tsx | 8 +- .../design-lab/src/pages/ComponentsPage.tsx | 3 + .../design-lab/src/preview/CapsulePreview.tsx | 22 +++++ .../src/preview/FlowChatMockTools.tsx | 4 +- .../src/preview/FlowChatPreviewRegistry.tsx | 5 +- .../src/preview/componentPresentation.ts | 2 +- .../vite/component-detail-contract.test.mjs | 2 +- .../vite/component-preview-render.test.mjs | 10 ++ design-system/packages/ui/README.md | 39 +++++++- .../ui/src/components/Capsule/Capsule.meta.ts | 15 +++ .../src/components/Capsule/Capsule.module.css | 14 +++ .../ui/src/components/Capsule/Capsule.tsx | 29 ++++++ .../ui/src/components/Capsule/index.ts | 1 + .../components/Combobox/Combobox.module.css | 6 +- .../ui/src/components/Combobox/Combobox.tsx | 8 +- .../packages/ui/src/components/Icon/Icon.tsx | 4 +- .../components/IconButton/IconButton.meta.ts | 7 +- .../src/components/IconButton/IconButton.tsx | 15 ++- .../src/components/TabGroup/TabGroup.meta.ts | 6 +- .../components/TabGroup/TabGroup.module.css | 47 +--------- .../ui/src/components/TabGroup/TabGroup.tsx | 8 +- design-system/packages/ui/src/index.ts | 1 + .../ui/src/internal/FloatingAction.module.css | 50 ++++++++++ .../ui/src/internal/FloatingAction.tsx | 19 ++++ .../ui/src/internal/GlassAction.module.css | 54 +++++++++++ .../ui/src/internal/floatingActionContext.ts | 4 + design-system/packages/ui/src/registry.ts | 2 + .../packages/ui/tests/capsule.test.mjs | 54 +++++++++++ .../packages/ui/tests/registry.test.mjs | 1 + .../packages/ui/tests/tab-group.test.mjs | 23 ++--- .../ai/ModelSettingsPage.loading.test.tsx | 58 +++++++++++- .../settings/pages/ai/ModelSettingsPage.scss | 41 +-------- .../settings/pages/ai/ModelSettingsPage.tsx | 92 +++++++++++++------ .../components/HostPendingQueuePanel.tsx | 22 ++--- .../config/components/ModelTagsField.tsx | 72 +++++++-------- 37 files changed, 557 insertions(+), 215 deletions(-) create mode 100644 design-system/apps/design-lab/src/preview/CapsulePreview.tsx create mode 100644 design-system/packages/ui/src/components/Capsule/Capsule.meta.ts create mode 100644 design-system/packages/ui/src/components/Capsule/Capsule.module.css create mode 100644 design-system/packages/ui/src/components/Capsule/Capsule.tsx create mode 100644 design-system/packages/ui/src/components/Capsule/index.ts create mode 100644 design-system/packages/ui/src/internal/FloatingAction.module.css create mode 100644 design-system/packages/ui/src/internal/FloatingAction.tsx create mode 100644 design-system/packages/ui/src/internal/GlassAction.module.css create mode 100644 design-system/packages/ui/src/internal/floatingActionContext.ts create mode 100644 design-system/packages/ui/tests/capsule.test.mjs diff --git a/design-system/AGENTS.md b/design-system/AGENTS.md index 1e21892aec..49f3d6bb32 100644 --- a/design-system/AGENTS.md +++ b/design-system/AGENTS.md @@ -42,4 +42,10 @@ pnpm run design-system:check Run `pnpm run design-system:check` for cross-package or release-boundary changes. Token and theme changes must also run the repository-level `pnpm run theme:color-audit:all`. +For shared icon rendering or geometry changes, use the focused package checks: + +```bash +pnpm --dir design-system run build:ui && node --test design-system/packages/ui/tests/icon.test.mjs design-system/packages/ui/tests/icon-slots.test.mjs +``` + Do not use browser automation or mock screenshots as visual proof. Design Lab is the manual authoring surface; source, build, HTTP, and package checks do not establish final visual fidelity. diff --git a/design-system/apps/design-lab/src/i18n/messages.ts b/design-system/apps/design-lab/src/i18n/messages.ts index 3c3fd7c1f7..e143ff2acc 100644 --- a/design-system/apps/design-lab/src/i18n/messages.ts +++ b/design-system/apps/design-lab/src/i18n/messages.ts @@ -163,7 +163,8 @@ export const enUSMessages = { "component.GitToolCard.description": "A prominent Git command card with output, warning, failure, and execution metadata.", "component.GlobSearchToolCard.description": "An ambient file-pattern search card with reusable result-list anatomy.", "component.GrepSearchToolCard.description": "An ambient text-search card with query metadata and a scrollable result preview.", - "component.IconButton.description": "A labeled icon-only action with annotation, quiet, outlined, filled, and primary variants. An annotation provides a small, muted entry to supplemental help.", + "component.IconButton.description": "A labeled icon action with annotation, quiet, glass, outlined, filled, and primary variants. Glass actions float over intact content.", + "component.Capsule.description": "A capsule with a circular glass action floating over intact text and equal edge spacing.", "component.Input.description": "A native text field with independent leading and trailing content slots.", "component.KeyHint.description": "A semantic keyboard hint with optional decorative icon content.", "component.LoadingState.description": "A centered loading composition with an activity indicator and optional support copy.", @@ -186,7 +187,7 @@ export const enUSMessages = { "component.SkillToolCard.description": "An ambient trace for skill loading progress and outcomes.", "component.Spinner.description": "A compact activity indicator with matrix and bar presentations.", "component.Switch.description": "Toggles a single setting between on and off with native checkbox semantics.", - "component.TabGroup.description": "A compact single-selection tab list with optional leading icons, end actions, and automatic keyboard activation.", + "component.TabGroup.description": "A compact single-selection tab list with leading icons, glass end actions over intact labels, and automatic keyboard activation.", "component.RollingText.description": "Vertical text replacement with coordinated width, interruption handling, and reduced-motion support. Try repeated clicks in the replacement specimen.", "component.ShimmerText.description": "A soft fade moves slowly across activity text, keeping every glyph visible. Reduced motion, high contrast and print keep the text static and readable.", "components.preview.replaceText": "Replace title", @@ -557,6 +558,7 @@ export const enUSMessages = { "detail.option.primary": "Primary", "detail.option.quiet": "Quiet", "detail.option.annotation": "Annotation", + "detail.option.glass": "Glass", "detail.option.text": "Text", "detail.option.sm": "Small", "detail.option.start": "Start", @@ -969,7 +971,8 @@ export const zhCNMessages = { "component.GitToolCard.description": "关注型 Git 命令卡片，统一呈现输出、警告、失败与执行元数据。", "component.GlobSearchToolCard.description": "复用结果列表结构的低关注文件模式搜索卡片。", "component.GrepSearchToolCard.description": "带查询元数据和可滚动结果预览的低关注文本搜索卡片。", - "component.IconButton.description": "带无障碍标签的纯图标组件，支持标注、安静、描边、填充和主要呈现方式。标注以小尺寸、弱化颜色提供补充说明入口。", + "component.IconButton.description": "带无障碍标签的纯图标组件，支持标注、安静、毛玻璃、描边、填充和主要呈现方式。毛玻璃操作悬浮在完整内容上方。", + "component.Capsule.description": "胶囊内容保持完整，圆形毛玻璃操作悬浮其上，边缘间距一致。", "component.Input.description": "使用原生输入语义，并提供独立前置与后置内容槽的文本字段。", "component.KeyHint.description": "使用键盘提示语义，并支持可选装饰图标的基础组件。", "component.LoadingState.description": "由活动指示器与可选说明文字组成的居中加载状态。", @@ -992,7 +995,7 @@ export const zhCNMessages = { "component.SkillToolCard.description": "低关注 Skill 轨迹，用于呈现加载进度和执行结果。", "component.Spinner.description": "提供点阵与条形样式的紧凑活动指示器。", "component.Switch.description": "通过原生复选框语义在开启与关闭之间切换单项设置。", - "component.TabGroup.description": "紧凑的单选标签组，支持前置图标、尾部操作与方向键自动切换。", + "component.TabGroup.description": "紧凑的单选标签组，支持前置图标、悬浮于完整标签上的毛玻璃尾部操作与方向键自动切换。", "component.RollingText.description": "垂直滚动替换文本，宽度同步过渡，支持连续切换与减少动态效果。可在替换示例中连续点击体验。", "component.ShimmerText.description": "柔和的渐隐在运行文字间缓慢流动，始终保留完整字形；减少动态效果、高对比度和打印时保持静态可读。", "components.preview.replaceText": "替换标题", @@ -1363,6 +1366,7 @@ export const zhCNMessages = { "detail.option.primary": "主要", "detail.option.quiet": "安静", "detail.option.annotation": "标注", + "detail.option.glass": "毛玻璃", "detail.option.text": "文字", "detail.option.sm": "小", "detail.option.start": "起始", @@ -1772,7 +1776,8 @@ export const zhTWMessages = { "meta.category.primitive": "基礎元件", "meta.category.other": "元件", "component.ActionCard.description": "可獨立設定前置內容、標題、說明、選取狀態和同級尾端操作的可互動卡片。", - "component.IconButton.description": "帶無障礙標籤的純圖示元件，支援標註、安靜、描邊、填滿和主要呈現方式。標註以小尺寸、弱化顏色提供補充說明入口。", + "component.IconButton.description": "帶無障礙標籤的純圖示元件，支援標註、安靜、毛玻璃、描邊、填滿和主要呈現方式。毛玻璃操作懸浮在完整內容上方。", + "component.Capsule.description": "膠囊內容保持完整，圓形毛玻璃操作懸浮其上，邊緣間距一致。", "component.MobileActionSheet.description": "用於統一呈現一般和危險操作清單的可關閉行動端彈層。", "component.MobileBadge.description": "用於狀態、身分與中繼資料的緊湊行動端標籤。", "component.MobileBanner.description": "支援語義色調和可選操作的行動端行內狀態橫幅。", @@ -1839,7 +1844,7 @@ export const zhTWMessages = { "component.SkillToolCard.description": "低關注 Skill 軌跡，用於呈現載入進度和執行結果。", "component.Spinner.description": "提供點陣與條形樣式的緊湊活動指示器。", "component.Switch.description": "以原生核取方塊語意在開啟與關閉之間切換單一設定。", - "component.TabGroup.description": "緊湊的單選標籤群組，支援前置圖示、尾端操作與方向鍵自動切換。", + "component.TabGroup.description": "緊湊的單選標籤群組，支援前置圖示、懸浮於完整標籤上的毛玻璃尾端操作與方向鍵自動切換。", "component.RollingText.description": "垂直捲動替換文字，寬度同步過渡，支援連續切換與減少動態效果。可在替換範例中連續點擊體驗。", "component.ShimmerText.description": "柔和的漸隱在執行文字間緩慢流動，始終保留完整字形；減少動態效果、高對比度和列印時保持靜態可讀。", "components.preview.replaceText": "替換標題", @@ -2113,6 +2118,7 @@ export const zhTWMessages = { "detail.iconPosition": "圖示位置", "detail.option.outline": "描邊", "detail.option.annotation": "標註", + "detail.option.glass": "毛玻璃", "detail.option.fill": "填滿", "detail.option.sm": "小", "detail.option.start": "起始", diff --git a/design-system/apps/design-lab/src/pages/ComponentDetailPage.tsx b/design-system/apps/design-lab/src/pages/ComponentDetailPage.tsx index f9aec4b64b..e041290432 100644 --- a/design-system/apps/design-lab/src/pages/ComponentDetailPage.tsx +++ b/design-system/apps/design-lab/src/pages/ComponentDetailPage.tsx @@ -1,3 +1,4 @@ +import { CapsulePreview } from "../preview/CapsulePreview"; import { Fragment, useEffect, useMemo, useState, type CSSProperties } from "react"; import { ComponentOverview } from "../preview/ComponentOverview"; import { getComponentPresentation, getInitialPreviewState, hasComponentOverview } from "../preview/componentPresentation"; @@ -163,7 +164,7 @@ type PageHeaderAlign = "center" | "start"; type PageHeaderSize = "display" | "lg" | "md" | "sm"; const buttonVariants = ["outline", "fill", "secondary", "primary", "text"] as const; -const iconButtonVariants = ["annotation", "quiet", "outline", "fill", "primary"] as const; +const iconButtonVariants = ["annotation", "quiet", "glass", "outline", "fill", "primary"] as const; const iconButtonSizes = ["xs", "standard", "sm", "md", "lg"] as const; const buttonInspectorStates = ["default", "hover", "active"] as const; const fieldOrientations = ["vertical", "horizontal"] as const; @@ -231,6 +232,7 @@ const optionLabelKeys: Readonly> = { open: "detail.option.open", outline: "detail.option.outline", annotation: "detail.option.annotation", + glass: "detail.option.glass", primary: "detail.option.primary", quiet: "detail.option.quiet", raised: "detail.option.raised", @@ -537,6 +539,9 @@ export function ComponentDetailPage({ return `import { Icon } from "@openbitfun/ui";\n\n`; } + if (component.name === "Capsule") { + return `import { Button, Capsule, Icon, IconButton } from "@openbitfun/ui";\n\n} onClick={remove} />\n}>\n \n`; + } if (component.name === "IconButton") { const stateProps = `${inspectorDisabled ? " disabled" : ""}${inspectorLoading ? " loading" : ""}`; const annotation = iconButtonVariant === "annotation"; @@ -904,6 +909,7 @@ export function ComponentDetailPage({ ); } + if (component.name === "Capsule") return ; if (component.name === "Icon") { return ; } diff --git a/design-system/apps/design-lab/src/pages/ComponentsPage.tsx b/design-system/apps/design-lab/src/pages/ComponentsPage.tsx index 77b6dbcc40..41e187d0b5 100644 --- a/design-system/apps/design-lab/src/pages/ComponentsPage.tsx +++ b/design-system/apps/design-lab/src/pages/ComponentsPage.tsx @@ -1,6 +1,7 @@ import { useEffect, useRef, useState } from "react"; import { ComponentDetailPage } from "./ComponentDetailPage"; import { BrandPreview } from "../preview/BrandPreview"; +import { CapsulePreview } from "../preview/CapsulePreview"; import { AppWindow, Heading, Keyboard, List, Rows3, MousePointerClick, PanelTop, ToggleLeft } from "lucide-react"; import { VoiceCallPreview, VoiceParticlePreview } from "../components/VoiceCallPreview"; import { @@ -607,6 +608,8 @@ export function ComponentCardPreview({ component }: { component: ComponentMeta } shortcut={} /> ); + case "Capsule": + return ; case "StatusPill": return ( }> diff --git a/design-system/apps/design-lab/src/preview/CapsulePreview.tsx b/design-system/apps/design-lab/src/preview/CapsulePreview.tsx new file mode 100644 index 0000000000..18844261dd --- /dev/null +++ b/design-system/apps/design-lab/src/preview/CapsulePreview.tsx @@ -0,0 +1,22 @@ +import { useState } from "react"; +import { Button, Capsule, Icon, IconButton, Stack, Tooltip } from "@openbitfun/ui"; +import { useI18n } from "../i18n"; + +export function CapsulePreview({ state = "default" }: { state?: string }) { + const { t } = useI18n(); + const [removed, setRemoved] = useState([]); + const disabled = state === "disabled"; + return + {(["xs", "sm"] as const).filter(size => !removed.includes(size)).map(size => ( + + } + disabled={disabled} onClick={() => setRemoved(current => [...current, size])} /> + + )}> + + + ))} + {removed.length > 0 && } + ; +} diff --git a/design-system/apps/design-lab/src/preview/FlowChatMockTools.tsx b/design-system/apps/design-lab/src/preview/FlowChatMockTools.tsx index 153e2d4a56..29c362a661 100644 --- a/design-system/apps/design-lab/src/preview/FlowChatMockTools.tsx +++ b/design-system/apps/design-lab/src/preview/FlowChatMockTools.tsx @@ -9,14 +9,14 @@ export type MockToolScope = 'all' | ToolGroup; // Only grouping is Lab-owned. Names and component bindings come from production. const groupForTool = { - get_goal: 'collaboration', create_goal: 'collaboration', update_goal: 'collaboration', AgentList: 'collaboration', AgentDelete: 'collaboration', + get_goal: 'collaboration', create_goal: 'collaboration', update_goal: 'collaboration', AgentList: 'collaboration', SessionHistory: 'collaboration', analyze_image: 'files', GetTime: 'execution', ListMCPResources: 'execution', ReadMCPResource: 'execution', ListMCPPrompts: 'execution', GetMCPPrompt: 'execution', Worktree: 'files', PortForward: 'execution', ReviewPlatform: 'collaboration', FrontendWorkbench: 'web', FinalizeMiniApp: 'web', PublishMiniApp: 'web', PublishAppearance: 'web', Playbook: 'execution', Read: 'files', Write: 'files', Edit: 'files', Delete: 'files', Grep: 'files', Glob: 'files', LS: 'files', GetFileDiff: 'files', view_image: 'files', - ListModels: 'execution', ControlHub: 'web', AgentInterrupt: 'collaboration', + ListModels: 'execution', ControlHub: 'web', RunCode: 'execution', ExecCommand: 'execution', WriteStdin: 'execution', ExecControl: 'execution', Cron: 'execution', GetToolSpec: 'execution', Skill: 'execution', WebSearch: 'web', WebFetch: 'web', PageDeploy: 'web', PagePublish: 'web', AgentSpawn: 'collaboration', AgentSendInput: 'collaboration', AgentWait: 'collaboration', TodoWrite: 'collaboration', diff --git a/design-system/apps/design-lab/src/preview/FlowChatPreviewRegistry.tsx b/design-system/apps/design-lab/src/preview/FlowChatPreviewRegistry.tsx index 23cd63c772..c76860ea89 100644 --- a/design-system/apps/design-lab/src/preview/FlowChatPreviewRegistry.tsx +++ b/design-system/apps/design-lab/src/preview/FlowChatPreviewRegistry.tsx @@ -1,4 +1,4 @@ -import { LayersPlusIcon } from "@openbitfun/ui"; +import { Icon, LayersPlusIcon } from "@openbitfun/ui"; import { toolsForComponent } from '@openbitfun/flow-chat-presentation/registry'; import { semanticToolCardPreviews } from './SemanticToolCardPreview'; import { createScenarioClock, execScenarios, execSnapshot, type ExecToolName } from '@openbitfun/flow-chat-presentation/scenarios'; @@ -35,7 +35,6 @@ import { Monitor, MousePointer, Plus, - Pencil, Rocket, Search, SearchCheck, @@ -257,7 +256,7 @@ function ChatComposerPreview({ interactive, state }: PreviewProps) { /> } + icon={} size="xs" /> diff --git a/design-system/apps/design-lab/src/preview/componentPresentation.ts b/design-system/apps/design-lab/src/preview/componentPresentation.ts index 87fa3537a5..55041f0071 100644 --- a/design-system/apps/design-lab/src/preview/componentPresentation.ts +++ b/design-system/apps/design-lab/src/preview/componentPresentation.ts @@ -7,7 +7,7 @@ export type ComponentPresentation = // These are website presentation choices. The public registry still owns the // component inventory, properties and supported states. const presentations: Record = { - Button: "matrix", IconButton: "matrix", + Button: "matrix", IconButton: "matrix", Capsule: "compact", Avatar: "compact", Checkbox: "compact", Radio: "compact", Switch: "compact", StatusPill: "compact", Spinner: "compact", LoadingState: "compact", LauncherButton: "compact", NumberBadge: "compact", KeyHint: "compact", diff --git a/design-system/apps/design-lab/vite/component-detail-contract.test.mjs b/design-system/apps/design-lab/vite/component-detail-contract.test.mjs index 31108cde29..062fc4b94c 100644 --- a/design-system/apps/design-lab/vite/component-detail-contract.test.mjs +++ b/design-system/apps/design-lab/vite/component-detail-contract.test.mjs @@ -201,7 +201,7 @@ test("IconButton preview exposes its icon-only presentation contract", async () assert.ok(declaration); assert.deepEqual( [...declaration[1].matchAll(/"([^"]+)"/g)].map((match) => match[1]), - ["annotation", "quiet", "outline", "fill", "primary"], + ["annotation", "quiet", "glass", "outline", "fill", "primary"], ); assert.match(source, /data-component="icon-button"/); assert.match(source, /aria-label=\{t\(annotation \? "components\.preview\.fieldHelp" : "components\.preview\.listView"\)\}/); diff --git a/design-system/apps/design-lab/vite/component-preview-render.test.mjs b/design-system/apps/design-lab/vite/component-preview-render.test.mjs index 4ee8eb342f..578a9bffe3 100644 --- a/design-system/apps/design-lab/vite/component-preview-render.test.mjs +++ b/design-system/apps/design-lab/vite/component-preview-render.test.mjs @@ -75,6 +75,16 @@ test("color and pattern directories expose one labelled panel while retaining ot } }); +test("Capsule previews publish the glass action composition in every supported state", () => { + const { html, preview } = render("Capsule"); + assert.match(preview, /data-openbitfun-component="capsule"/); + assert.match(preview, /data-openbitfun-variant="glass"/); + assert.match(preview, /data-openbitfun-shape="circle"/); + assert.match(preview, /data-openbitfun-preview-state="disabled"/); + assert.match(html, /import \{ Button, Capsule, Icon, IconButton \}/); + assert.doesNotMatch(html, />component\.Capsule\.description { const { html, preview } = render("Textarea"); assert.equal((preview.match(/data-openbitfun-component="textarea"/g) ?? []).length, 5); diff --git a/design-system/packages/ui/README.md b/design-system/packages/ui/README.md index b4f7593d3c..e27c81804c 100644 --- a/design-system/packages/ui/README.md +++ b/design-system/packages/ui/README.md @@ -133,7 +133,9 @@ by the component that needs them. ## Icons Use `Icon` for shared size, tone and accessible labels. General-purpose symbols -use Lucide. Automatic reasoning uses the authored `reasoning-auto` SVG mark with +use Lucide. The named `edit` icon uses the simple `Pen` outline without Pencil's +cap divider, keeping compact action groups visually light. +Automatic reasoning uses the authored `reasoning-auto` SVG mark with a `currentColor` mask so it follows the active theme. Manual reasoning levels use the product slider and text labels. Product consumers own capability filtering, translated names and @@ -392,6 +394,41 @@ focus ring. Use it for toolbar, dialog, and row utilities. `fill` and `primary` keep an opaque backing surface for persistent emphasis. Disabled quiet actions remain transparent and do not show hover or pressed feedback. +`IconButton variant="glass"` is a translucent, blurred action over existing +content. It uses a low-opacity tint of the shared raised surface, a softened +subtle border and subtle blur, without an outer shadow. Hover and pressed states +retain translucency. Unsupported blur, increased contrast and reduced transparency +use a solid surface. +The button itself owns the glass material and its reveal transition so an opaque +backing or fading parent cannot prevent it from sampling the content behind it. + +### Capsule actions + +Wrap a pill-shaped `Button`, inline `Input`, or tag label in `Capsule`, matching +its `size` to the child. Pass one `IconButton` (optionally inside `Tooltip`) to +`action`. The slot selects circular glass presentation and compact geometry; +the consumer keeps the action's accessible label, tone, disabled/loading state, +and callbacks. The action is a sibling of the primary control, never a nested +button. Pointer hover or focus within the capsule reveals it; touch keeps it +available. Its right inset equals the top and bottom clearance at every density. + +```tsx +} + tone="danger" onClick={requestRemoval} /> +}> + + +``` + +The action floats over the intact label: do not clip, mask, hide, resize, or add +action padding to its text when revealing the icon. Normal constrained-width +`OverflowText` behavior remains available. `TabGroup.endAction` and removable +`MultiSelect` tags share this contract, including existing native tab buttons. +Leading identity icons stay in their label layout; this contract governs the +independent trailing actions. Product CSS owns placement of capsules, not their +action offsets, backgrounds, or visibility. + For supplemental label annotations, compose `Tooltip trigger="hover-focus" openOnClick` with `IconButton variant="annotation" size="xs"` and an `info` icon. The annotation uses `color.content.muted` at rest and diff --git a/design-system/packages/ui/src/components/Capsule/Capsule.meta.ts b/design-system/packages/ui/src/components/Capsule/Capsule.meta.ts new file mode 100644 index 0000000000..10cd8e4d7d --- /dev/null +++ b/design-system/packages/ui/src/components/Capsule/Capsule.meta.ts @@ -0,0 +1,15 @@ +import type { ComponentMeta } from "../../registry.types"; + +export const capsuleMeta = { + category: "action", + description: "A capsule composition with a circular glass action floating over its unmodified label. Pointer hover and keyboard focus reveal the action; touch keeps it available.", + maturity: "stable", + name: "Capsule", + props: [ + { name: "children", type: "ReactNode" }, + { name: "action", type: "ReactNode" }, + { name: "size", type: "xs | sm | md | lg", defaultValue: "sm" }, + ], + states: ["default", "hover", "focus-visible", "disabled"], + tokens: ["control.button.xsHeight", "control.height.sm", "control.height.md", "control.height.lg", "control.iconButton.xsSize", "color.surface.raised", "color.border.subtle", "effect.blur.subtle", "radius.pill"], +} as const satisfies ComponentMeta; diff --git a/design-system/packages/ui/src/components/Capsule/Capsule.module.css b/design-system/packages/ui/src/components/Capsule/Capsule.module.css new file mode 100644 index 0000000000..496a721189 --- /dev/null +++ b/design-system/packages/ui/src/components/Capsule/Capsule.module.css @@ -0,0 +1,14 @@ +@layer openbitfun.components { + .root { + --_floating-action-host-height: var(--openbitfun-control-height-sm); + display: inline-flex; + flex: 0 1 auto; + align-items: center; + min-inline-size: 0; + max-inline-size: 100%; + } + + .root[data-size="xs"] { --_floating-action-host-height: var(--openbitfun-control-button-xs-height); } + .root[data-size="md"] { --_floating-action-host-height: var(--openbitfun-control-height-md); } + .root[data-size="lg"] { --_floating-action-host-height: var(--openbitfun-control-height-lg); } +} diff --git a/design-system/packages/ui/src/components/Capsule/Capsule.tsx b/design-system/packages/ui/src/components/Capsule/Capsule.tsx new file mode 100644 index 0000000000..fe957e7a63 --- /dev/null +++ b/design-system/packages/ui/src/components/Capsule/Capsule.tsx @@ -0,0 +1,29 @@ +import { forwardRef, type HTMLAttributes, type ReactNode } from "react"; +import { classNames } from "../../internal/classNames"; +import { FloatingAction } from "../../internal/FloatingAction"; +import actionStyles from "../../internal/FloatingAction.module.css"; +import styles from "./Capsule.module.css"; + +export interface CapsuleProps extends HTMLAttributes { + /** One IconButton (optionally wrapped in Tooltip), rendered beside the content. */ + action?: ReactNode; + /** Match the size of the child Button, Input, or label surface. */ + size?: "xs" | "sm" | "md" | "lg"; +} + +/** Keeps label layout intact beneath a circular glass action with equal edge gaps. */ +export const Capsule = forwardRef(function Capsule({ + action, + children, + className, + size = "sm", + ...props +}, ref) { + return ( + + {children} + {action != null && {action}} + + ); +}); diff --git a/design-system/packages/ui/src/components/Capsule/index.ts b/design-system/packages/ui/src/components/Capsule/index.ts new file mode 100644 index 0000000000..6262586ca6 --- /dev/null +++ b/design-system/packages/ui/src/components/Capsule/index.ts @@ -0,0 +1 @@ +export { Capsule, type CapsuleProps } from "./Capsule"; diff --git a/design-system/packages/ui/src/components/Combobox/Combobox.module.css b/design-system/packages/ui/src/components/Combobox/Combobox.module.css index 8bc63530f3..1ec8876520 100644 --- a/design-system/packages/ui/src/components/Combobox/Combobox.module.css +++ b/design-system/packages/ui/src/components/Combobox/Combobox.module.css @@ -225,7 +225,7 @@ overflow: hidden; align-items: center; max-inline-size: 10rem; - min-block-size: var(--openbitfun-control-icon-button-xs-size); + block-size: var(--openbitfun-control-button-xs-height); gap: var(--openbitfun-space-1); padding-inline: var(--openbitfun-space-2); border: var(--openbitfun-border-width-default) solid var(--openbitfun-color-border-subtle); @@ -236,10 +236,6 @@ white-space: nowrap; } - .tag:has(button) { - padding-inline-end: 0; - } - .tag > span { overflow: hidden; } diff --git a/design-system/packages/ui/src/components/Combobox/Combobox.tsx b/design-system/packages/ui/src/components/Combobox/Combobox.tsx index e3f73cb43f..5f65da32ea 100644 --- a/design-system/packages/ui/src/components/Combobox/Combobox.tsx +++ b/design-system/packages/ui/src/components/Combobox/Combobox.tsx @@ -22,6 +22,7 @@ import { Portal } from "../../overlay/Portal"; import { useDesignSystem } from "../../overlay/useDesignSystem"; import { useDismissibleLayer } from "../../overlay/useDismissibleLayer"; import { IconButton } from "../IconButton"; +import { Capsule } from "../Capsule"; import { Listbox, ListboxEmpty, @@ -656,8 +657,7 @@ const CollectionPicker = forwardRef(function Collec {showTags && ( {selectedOptions.slice(0, Math.max(1, maxVisibleTags)).map((option) => ( - - {option.label} + (function Collec tabIndex={resolvedOpen ? -1 : undefined} variant="quiet" /> - + )}> + {option.label} + ))} {selectedOptions.length > Math.max(1, maxVisibleTags) && ( +{selectedOptions.length - Math.max(1, maxVisibleTags)} diff --git a/design-system/packages/ui/src/components/Icon/Icon.tsx b/design-system/packages/ui/src/components/Icon/Icon.tsx index 2714bd8588..97badfc451 100644 --- a/design-system/packages/ui/src/components/Icon/Icon.tsx +++ b/design-system/packages/ui/src/components/Icon/Icon.tsx @@ -62,7 +62,7 @@ import { PanelLeft, PanelRight, PanelsTopLeft, - Pencil, + Pen, Pin, Plus, Plug, @@ -270,7 +270,7 @@ const lineGlyphs = { "device-mac": Monitor, "download": ArrowDown, "duplicate": Copy, - "edit": Pencil, + "edit": Pen, "extension": Puzzle, "eye": Eye, "file-search-corner": FileSearchCorner, diff --git a/design-system/packages/ui/src/components/IconButton/IconButton.meta.ts b/design-system/packages/ui/src/components/IconButton/IconButton.meta.ts index 6a044ae694..1c690639e1 100644 --- a/design-system/packages/ui/src/components/IconButton/IconButton.meta.ts +++ b/design-system/packages/ui/src/components/IconButton/IconButton.meta.ts @@ -2,13 +2,13 @@ import type { ComponentMeta } from "../../registry.types"; export const iconButtonMeta = { category: "action", - description: "A labeled icon-only action with annotation, quiet, outlined, filled, and primary variants. Annotation pairs a muted 12px mark with the existing 22px xs target for supplemental label help.", + description: "A labeled icon-only action with annotation, quiet, glass, outlined, filled, and primary variants. Glass floats over intact content; capsule and tab action slots select it automatically.", maturity: "stable", name: "IconButton", props: [ { name: "aria-label", type: "string" }, { name: "icon", type: "ReactNode" }, - { defaultValue: "quiet", name: "variant", type: "annotation | quiet | outline | fill | primary" }, + { defaultValue: "quiet", name: "variant", type: "annotation | quiet | glass | outline | fill | primary" }, { defaultValue: "square", name: "shape", type: "square | circle" }, { defaultValue: "sm", name: "size", type: "xs | standard | sm | md | lg" }, { defaultValue: "neutral", name: "tone", type: "neutral | danger" }, @@ -16,6 +16,9 @@ export const iconButtonMeta = { ], states: ["default", "hover", "active", "focus-visible", "disabled", "loading"], tokens: [ + "color.surface.raised", + "color.border.subtle", + "effect.blur.subtle", "opacity.iconArtwork", "color.content.primary", "color.content.secondary", diff --git a/design-system/packages/ui/src/components/IconButton/IconButton.tsx b/design-system/packages/ui/src/components/IconButton/IconButton.tsx index be5b18a01a..c41a124ffa 100644 --- a/design-system/packages/ui/src/components/IconButton/IconButton.tsx +++ b/design-system/packages/ui/src/components/IconButton/IconButton.tsx @@ -1,9 +1,12 @@ import { forwardRef, + useContext, type ButtonHTMLAttributes, type ReactNode, } from "react"; import { classNames } from "../../internal/classNames"; +import { FloatingActionContext } from "../../internal/floatingActionContext"; +import glassStyles from "../../internal/GlassAction.module.css"; import styles from "./IconButton.module.css"; export interface IconButtonProps @@ -15,7 +18,7 @@ export interface IconButtonProps size?: "xs" | "standard" | "sm" | "md" | "lg"; tone?: "danger" | "neutral"; /** Use annotation with an xs size and Tooltip for supplemental label help. */ - variant?: "annotation" | "fill" | "outline" | "primary" | "quiet"; + variant?: "annotation" | "fill" | "glass" | "outline" | "primary" | "quiet"; } export const IconButton = forwardRef(function IconButton({ @@ -31,18 +34,20 @@ export const IconButton = forwardRef(functio variant = "quiet", ...props }, ref) { + const floating = useContext(FloatingActionContext); + const resolvedVariant = floating ? "glass" : variant; return ( {hasEndAction && ( - + {item.endAction} - + )} ); diff --git a/design-system/packages/ui/src/index.ts b/design-system/packages/ui/src/index.ts index 842ff7c1c1..f511c0784f 100644 --- a/design-system/packages/ui/src/index.ts +++ b/design-system/packages/ui/src/index.ts @@ -57,6 +57,7 @@ export { export { Alert, type AlertProps, type AlertTone } from "./components/Alert"; export { Avatar, AvatarGroup, type AvatarGroupProps, type AvatarProps, type AvatarSize } from "./components/Avatar"; export { Button, type ButtonProps } from "./components/Button"; +export { Capsule, type CapsuleProps } from "./components/Capsule"; export { Checkbox, type CheckboxProps, type CheckboxSize } from "./components/Checkbox"; export { Card, diff --git a/design-system/packages/ui/src/internal/FloatingAction.module.css b/design-system/packages/ui/src/internal/FloatingAction.module.css new file mode 100644 index 0000000000..29985f6673 --- /dev/null +++ b/design-system/packages/ui/src/internal/FloatingAction.module.css @@ -0,0 +1,50 @@ +@layer openbitfun.components { + .host { + --_floating-action-size: var(--openbitfun-control-icon-button-xs-size); + position: relative; + } + + .action { + pointer-events: auto; + position: absolute; + inset-block-start: 50%; + inset-inline-end: calc((var(--_floating-action-host-height) - var(--_floating-action-size)) / 2); + display: inline-flex; + align-items: center; + justify-content: center; + inline-size: var(--_floating-action-size); + block-size: var(--_floating-action-size); + transform: translateY(-50%); + } + + .action > button { + inline-size: 100%; + block-size: 100%; + border-radius: var(--openbitfun-radius-pill); + transition: opacity var(--openbitfun-motion-duration-fast) var(--openbitfun-motion-easing-standard); + } + + @media (hover: hover) and (pointer: fine) { + .action { + pointer-events: none; + } + + .action > button { + opacity: 0; + } + + .host:is(:hover, :focus-within, [data-openbitfun-preview-state="hover"], [data-openbitfun-preview-state="focus-visible"]) > .action { + pointer-events: auto; + } + + .host:is(:hover, :focus-within, [data-openbitfun-preview-state="hover"], [data-openbitfun-preview-state="focus-visible"]) > .action > button { + opacity: 1; + } + } + + @media (prefers-reduced-motion: reduce) { + .action > button { + transition: none; + } + } +} diff --git a/design-system/packages/ui/src/internal/FloatingAction.tsx b/design-system/packages/ui/src/internal/FloatingAction.tsx new file mode 100644 index 0000000000..5525c51f29 --- /dev/null +++ b/design-system/packages/ui/src/internal/FloatingAction.tsx @@ -0,0 +1,19 @@ +import { Children, cloneElement, isValidElement, type HTMLAttributes, type ReactNode } from "react"; +import { classNames } from "./classNames"; +import { FloatingActionContext } from "./floatingActionContext"; +import glassStyles from "./GlassAction.module.css"; +import styles from "./FloatingAction.module.css"; + +export function FloatingAction({ children, className, ...props }: HTMLAttributes) { + return ( + + + {Children.map(children, (child: ReactNode) => ( + isValidElement>(child) && child.type === "button" + ? cloneElement(child, { className: classNames(child.props.className, glassStyles.surface) }) + : child + ))} + + + ); +} diff --git a/design-system/packages/ui/src/internal/GlassAction.module.css b/design-system/packages/ui/src/internal/GlassAction.module.css new file mode 100644 index 0000000000..405ef24d24 --- /dev/null +++ b/design-system/packages/ui/src/internal/GlassAction.module.css @@ -0,0 +1,54 @@ +@layer openbitfun.components { + .surface[data-openbitfun-variant="glass"], + .surface:not([data-openbitfun-component="icon-button"]) { + background: var(--openbitfun-color-surface-raised); + box-shadow: inset 0 0 0 var(--openbitfun-border-width-default) color-mix(in srgb, var(--openbitfun-color-border-subtle) 60%, transparent); + } + + @supports ((backdrop-filter: blur(0)) or (-webkit-backdrop-filter: blur(0))) { + .surface[data-openbitfun-variant="glass"], + .surface:not([data-openbitfun-component="icon-button"]) { + background: color-mix(in srgb, var(--openbitfun-color-surface-raised) 28%, transparent); + -webkit-backdrop-filter: var(--openbitfun-effect-blur-subtle); + backdrop-filter: var(--openbitfun-effect-blur-subtle); + } + + .surface[data-openbitfun-variant="glass"]:where(:not(:disabled):is(:hover, [data-openbitfun-preview-state="hover"])), + .surface:not([data-openbitfun-component="icon-button"]):where(:not(:disabled):is(:hover, [data-openbitfun-preview-state="hover"])) { + background: color-mix(in srgb, var(--openbitfun-color-surface-raised) 40%, transparent); + } + + .surface[data-openbitfun-variant="glass"]:where(:not(:disabled):is(:active, [data-openbitfun-preview-state="active"])), + .surface:not([data-openbitfun-component="icon-button"]):where(:not(:disabled):is(:active, [data-openbitfun-preview-state="active"])) { + background: color-mix(in srgb, var(--openbitfun-color-surface-raised) 48%, transparent); + } + } + + :where([data-contrast="high"]) .surface[data-openbitfun-variant="glass"], + :where([data-contrast="high"]) .surface:not([data-openbitfun-component="icon-button"]) { + background: var(--openbitfun-color-surface-raised); + -webkit-backdrop-filter: none; + backdrop-filter: none; + } + + @media (prefers-reduced-transparency: reduce), (prefers-contrast: more) { + .surface[data-openbitfun-variant="glass"], + .surface:not([data-openbitfun-component="icon-button"]) { + background: var(--openbitfun-color-surface-raised); + -webkit-backdrop-filter: none; + backdrop-filter: none; + } + } + + @media (forced-colors: active) { + .surface[data-openbitfun-variant="glass"], + .surface:not([data-openbitfun-component="icon-button"]) { + color: ButtonText; + background: Canvas; + box-shadow: none; + -webkit-backdrop-filter: none; + backdrop-filter: none; + outline: var(--openbitfun-border-width-default) solid ButtonText; + } + } +} diff --git a/design-system/packages/ui/src/internal/floatingActionContext.ts b/design-system/packages/ui/src/internal/floatingActionContext.ts new file mode 100644 index 0000000000..ec2f7864c0 --- /dev/null +++ b/design-system/packages/ui/src/internal/floatingActionContext.ts @@ -0,0 +1,4 @@ +import { createContext } from "react"; + +/** Capsule and tab action slots own their floating presentation and geometry. */ +export const FloatingActionContext = createContext(false); diff --git a/design-system/packages/ui/src/registry.ts b/design-system/packages/ui/src/registry.ts index cad1627db1..abda186802 100644 --- a/design-system/packages/ui/src/registry.ts +++ b/design-system/packages/ui/src/registry.ts @@ -4,6 +4,7 @@ import { activityItemMeta } from "./components/ActivityItem/ActivityItem.meta"; import { alertMeta } from "./components/Alert/Alert.meta"; import { avatarMeta } from "./components/Avatar/Avatar.meta"; import { buttonMeta } from "./components/Button/Button.meta"; +import { capsuleMeta } from "./components/Capsule/Capsule.meta"; import { checkboxMeta } from "./components/Checkbox/Checkbox.meta"; import { cardMeta } from "./components/Card/Card.meta"; import { composerMeta } from "./components/Composer/Composer.meta"; @@ -127,6 +128,7 @@ export const componentRegistry = [ askUserMeta, avatarMeta, buttonMeta, + capsuleMeta, cardMeta, checkboxMeta, chatComposerMeta, diff --git a/design-system/packages/ui/tests/capsule.test.mjs b/design-system/packages/ui/tests/capsule.test.mjs new file mode 100644 index 0000000000..7de04336c7 --- /dev/null +++ b/design-system/packages/ui/tests/capsule.test.mjs @@ -0,0 +1,54 @@ +import assert from "node:assert/strict"; +import { createElement as h } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import test from "node:test"; +import { Button, Capsule, Icon, IconButton, MultiSelect, TabGroup, Tooltip } from "../dist/index.js"; + +const action = (props = {}) => h(IconButton, { + "aria-label": "Remove provider", icon: h(Icon, { name: "delete" }), ...props, +}); + +test("a tooltip-wrapped capsule action keeps its accessible name and destructive state with shared glass presentation", () => { + const markup = renderToStaticMarkup(h(Capsule, { + size: "sm", + action: h(Tooltip, { content: "Remove provider" }, action({ tone: "danger", variant: "fill", loading: true })), + }, h(Button, { size: "sm", "aria-label": "Edit provider" }, "Long provider name"))); + assert.match(markup, /data-openbitfun-component="capsule"/); + assert.match(markup, /data-openbitfun-part="action"/); + assert.match(markup, /data-openbitfun-variant="glass"/); + assert.match(markup, /data-openbitfun-shape="circle"/); + assert.match(markup, /data-openbitfun-tone="danger"/); + assert.match(markup, /aria-label="Remove provider"/); + assert.match(markup, /aria-busy="true"/); + assert.match(markup, /disabled=""/); + assert.match(markup, /Long provider name/); + assert.equal((markup.match(/") < markup.lastIndexOf(" { + const markup = renderToStaticMarkup(h(Capsule, { size: "xs" }, "A tag")); + assert.match(markup, />A tag<\/span>/); + assert.doesNotMatch(markup, /tabindex|role=|data-openbitfun-part="action"/); +}); + +test("tab actions and removable MultiSelect labels adopt the same glass presentation", () => { + const tabs = renderToStaticMarkup(h(TabGroup, { items: [{ value: "a", label: "A long tab", endAction: action({ variant: "outline" }) }] })); + assert.match(tabs, /data-openbitfun-variant="glass"/); + assert.match(tabs, /role="tab"/); + assert.match(tabs, /A long tab/); + + const select = renderToStaticMarkup(h(MultiSelect, { + "aria-label": "Tags", value: ["a"], options: [{ value: "a", label: "A long tag" }], disabled: true, + })); + assert.match(select, /data-openbitfun-component="capsule"/); + assert.match(select, /data-openbitfun-variant="glass"/); + assert.match(select, /A long tag/); + assert.match(select, /disabled=""/); +}); + +test("a standalone icon retains its chosen variant outside a floating action slot", () => { + const markup = renderToStaticMarkup(action({ variant: "outline" })); + assert.match(markup, /data-openbitfun-variant="outline"/); + assert.doesNotMatch(markup, /data-openbitfun-variant="glass"/); +}); diff --git a/design-system/packages/ui/tests/registry.test.mjs b/design-system/packages/ui/tests/registry.test.mjs index 858094e302..e7539b8c92 100644 --- a/design-system/packages/ui/tests/registry.test.mjs +++ b/design-system/packages/ui/tests/registry.test.mjs @@ -24,6 +24,7 @@ test("registry exposes only the formal stable components", () => { "AskUser", "Avatar", "Button", + "Capsule", "Card", "Checkbox", "ChatComposer", diff --git a/design-system/packages/ui/tests/tab-group.test.mjs b/design-system/packages/ui/tests/tab-group.test.mjs index acaeb2ef83..9c7d6e4eab 100644 --- a/design-system/packages/ui/tests/tab-group.test.mjs +++ b/design-system/packages/ui/tests/tab-group.test.mjs @@ -102,7 +102,7 @@ test("product wrappers preserve the standard tablist, selection and end-action a assert.equal((markup.match(/role="tab"/g) ?? []).length, 2); assert.equal((markup.match(/aria-selected="true"/g) ?? []).length, 1); assert.match(markup, /data-document="settings" data-index="1" draggable="true"/); - assert.match(markup, /<\/button>]+data-openbitfun-part="endAction"> - - removeSelectedModelDraft(draft.modelName)} - disabled={isEditorSaving} - icon={} - /> - - + ))} {isAddingCustomModel && ( {
    {apiProviderGroups.map(group => (
  • - - - + + void requestProviderDelete(group)} + icon={} + /> + + )}> + + + +
    • ))}
@@ -4102,11 +4121,26 @@ const ModelSettingsPage: React.FC = () => {
    {subscriptionProviderGroups.map(group => (
  • - + + void requestProviderDelete(group)} + icon={} + /> + + )}> + +
    • ))}
diff --git a/src/web-ui/src/flow_chat/components/HostPendingQueuePanel.tsx b/src/web-ui/src/flow_chat/components/HostPendingQueuePanel.tsx index 5fcbc7475f..fb1c7e28d3 100644 --- a/src/web-ui/src/flow_chat/components/HostPendingQueuePanel.tsx +++ b/src/web-ui/src/flow_chat/components/HostPendingQueuePanel.tsx @@ -2,8 +2,8 @@ import { pendingQueueManager } from '../services/flow-chat-manager/PendingQueueM import type { QueuedMessage } from '../types/flow-chat'; import { useEffect, useId, useState, useSyncExternalStore } from 'react'; import { useI18n } from '@/infrastructure/i18n'; -import { Button, IconButton, OverflowText, Tooltip } from '@openbitfun/ui'; -import { ArrowUp, ChevronDown, ChevronUp, Copy, Info, Pencil, RotateCw, X } from 'lucide-react'; +import { Button, Icon, IconButton, OverflowText, Tooltip } from '@openbitfun/ui'; +import { RotateCw } from 'lucide-react'; import './HostPendingQueuePanel.scss'; import { HostDialogQueue, observeHostQueue, type QueueOutboxRecord, type QueueItem } from '../../../../shared/dialog-queue/HostDialogQueue'; import { @@ -55,11 +55,11 @@ export function HostPendingQueuePanel({ queue, onRestore }: { queue: HostDialogQ return } onClick={() => setShowHelp(value => !value)} /> + icon={} onClick={() => setShowHelp(value => !value)} /> {showHelp &&

{t('hostQueue.memoryNotice')}

} {visibleError &&
{visibleError}
} @@ -72,13 +72,13 @@ export function HostPendingQueuePanel({ queue, onRestore }: { queue: HostDialogQ {item.reason && {item.reason}} - } disabled={busy || !!view.error || item.status === 'steering_pending'} onClick={() => void run(async () => { + } disabled={busy || !!view.error || item.status === 'steering_pending'} onClick={() => void run(async () => { const saved = await queue.savedDraft(item.turnId); if (!saved || saved.request.action !== 'submit') throw new Error(t('hostQueue.noDraft')); await restoreDraft(saved, item); })} /> - } disabled={busy || !!view.error || item.status === 'steering_pending'} onClick={() => void run(() => queue.act(item, 'promote'))} /> - } disabled={busy || !!view.error || item.status === 'steering_pending'} onClick={() => void run(() => queue.act(item, 'cancel'))} /> + } disabled={busy || !!view.error || item.status === 'steering_pending'} onClick={() => void run(() => queue.act(item, 'promote'))} /> + } disabled={busy || !!view.error || item.status === 'steering_pending'} onClick={() => void run(() => queue.act(item, 'cancel'))} /> )} {view.pending.map(record => @@ -87,9 +87,9 @@ export function HostPendingQueuePanel({ queue, onRestore }: { queue: HostDialogQ {record.restoreIntent && record.request.action === 'submit' - ? } disabled={busy} onClick={() => void run(() => restoreDraft(record))} /> - : } disabled={busy} onClick={() => void run(() => queue.retry(record))} />} - {record.request.action === 'submit' && } disabled={busy} onClick={() => { + ? } disabled={busy} onClick={() => void run(() => restoreDraft(record))} /> + : } disabled={busy} onClick={() => void run(() => queue.retry(record))} />} + {record.request.action === 'submit' && } disabled={busy} onClick={() => { if (record.request.action !== 'submit') return; const message = record.request.message; const cache = record.draft as Partial | undefined; @@ -101,7 +101,7 @@ export function HostPendingQueuePanel({ queue, onRestore }: { queue: HostDialogQ status: 'queued', retryCount: cache?.retryCount ?? 0, userMessageMetadata: message.metadata }); }} />} - } disabled={busy} onClick={() => void run(() => queue.dismiss(record))} /> + } disabled={busy} onClick={() => void run(() => queue.dismiss(record))} /> )} diff --git a/src/web-ui/src/infrastructure/config/components/ModelTagsField.tsx b/src/web-ui/src/infrastructure/config/components/ModelTagsField.tsx index cd7339c9a7..1051f95178 100644 --- a/src/web-ui/src/infrastructure/config/components/ModelTagsField.tsx +++ b/src/web-ui/src/infrastructure/config/components/ModelTagsField.tsx @@ -1,4 +1,4 @@ -import { Button, Field, Icon, IconButton, Input, StatusPill, Tooltip } from '@openbitfun/ui'; +import { Button, Capsule, Field, Icon, IconButton, Input, StatusPill, Tooltip } from '@openbitfun/ui'; import { useRef, useState } from 'react'; import { useI18n } from '@/infrastructure/i18n'; import { MAX_MODEL_USER_TAGS, normalizeModelTags } from '../services/modelTags'; @@ -90,46 +90,46 @@ export default function ModelTagsField({ tags, recommendedTags, disabled, layout
{tags.map(tag => ( -
- {tagDraft?.originalTag === tag ? tagInput : ( - <> - - - { - onChange(tags.filter(value => value !== tag)); - restoreTagFocus(tags.find(value => value !== tag)); - }} - icon={} - /> - - + shape="circle" + variant="quiet" + aria-label={t('pool.removeTag', { tag })} + disabled={disabled} + onClick={() => { + onChange(tags.filter(value => value !== tag)); + restoreTagFocus(tags.find(value => value !== tag)); + }} + icon={} + /> + + )}> + {tagDraft?.originalTag === tag ? tagInput : ( + )} -
+ ))} {tagDraft?.originalTag === null && ( -
+ {tagInput} -
+ )}
From 67c4a635a7dec86c41ae12da9734fce104467573 Mon Sep 17 00:00:00 2001 From: GCWing Date: Sun, 11 Oct 2026 16:58:05 +0800 Subject: [PATCH 2/3] fix(models): retain catalog snapshots and update cold session selectors --- .../cargo-dependency-boundaries.mjs | 3 +- .../cli/src/peer_host/commands/session.rs | 15 +- src/apps/desktop/AGENTS.md | 11 + src/apps/desktop/src/api/agentic_api.rs | 32 +- .../desktop/src/openbitfun_control_host.rs | 18 +- .../src/runtime/session_application.rs | 30 ++ src/crates/assembly/core/AGENTS.md | 14 + .../src/agentic/coordination/coordinator.rs | 35 ++ .../core/src/agentic/persistence/manager.rs | 114 ++++- .../src/agentic/session/session_manager.rs | 440 +++++++++++++++--- .../tools/openbitfun_control_config.rs | 14 +- .../infrastructure/ai/reasoning_catalog.rs | 305 ++++++++---- .../assembly/core/src/product_runtime.rs | 19 + .../core/src/service/config/global.rs | 3 + .../core/src/service/config/manager.rs | 13 + .../core/src/service/config/service.rs | 80 +++- .../core/src/service_agent_runtime.rs | 33 +- .../services/services-integrations/AGENTS.md | 6 + .../services/services-integrations/Cargo.toml | 1 + .../services-integrations/src/models_dev.rs | 165 +++++-- .../flow_chat/components/ModelSelector.tsx | 316 +++++-------- .../components/ModelSelectorAcpMode.test.tsx | 18 +- .../components/ModelSelectorExternal.test.tsx | 21 +- .../ModelSelectorProviderLevels.test.tsx | 184 ++++++-- .../flow-chat-manager/MessageModule.test.ts | 2 + .../src/flow_chat/store/FlowChatStore.ts | 6 +- .../flow_chat/utils/modelResolution.test.ts | 1 + .../src/flow_chat/utils/modelResolution.ts | 53 +-- src/web-ui/src/flow_chat/utils/modelSync.ts | 8 +- .../src/infrastructure/config/AGENTS.md | 11 + .../config/services/ModelCatalogStore.test.ts | 64 +++ .../config/services/ModelCatalogStore.ts | 84 ++++ src/web-ui/src/locales/en-US/flow-chat.json | 1 + src/web-ui/src/locales/zh-CN/flow-chat.json | 1 + src/web-ui/src/locales/zh-TW/flow-chat.json | 1 + 35 files changed, 1610 insertions(+), 512 deletions(-) create mode 100644 src/web-ui/src/infrastructure/config/services/ModelCatalogStore.test.ts create mode 100644 src/web-ui/src/infrastructure/config/services/ModelCatalogStore.ts diff --git a/scripts/core-boundaries/cargo-dependency-boundaries.mjs b/scripts/core-boundaries/cargo-dependency-boundaries.mjs index 8c7f961976..26178ca5de 100644 --- a/scripts/core-boundaries/cargo-dependency-boundaries.mjs +++ b/scripts/core-boundaries/cargo-dependency-boundaries.mjs @@ -145,7 +145,8 @@ function isProcMacroPackage(pkg) { const SERVICES_INTEGRATIONS_TOKIO_FEATURES = new Map([ ['account-identity', ['rt', 'sync']], ['announcement', ['fs', 'sync']], - ['models-dev', ['fs', 'sync', 'time']], + // Large catalog validation/hash work uses spawn_blocking, outside async IO workers. + ['models-dev', ['fs', 'rt', 'sync', 'time']], ['browser-control', ['time']], ['canvas-runtime', ['fs']], ['deep-research', []], diff --git a/src/apps/cli/src/peer_host/commands/session.rs b/src/apps/cli/src/peer_host/commands/session.rs index 57d7af74ba..79eafa5348 100644 --- a/src/apps/cli/src/peer_host/commands/session.rs +++ b/src/apps/cli/src/peer_host/commands/session.rs @@ -757,7 +757,20 @@ pub(crate) async fn update_session_model( .map(|preset| preset.trim().to_string()) .filter(|preset| !preset.is_empty()); if has_session_workspace_scope(request) { - ensure_coordinator_session(state, args).await?; + ensure_session_workspace_runtime_ownership(state, request).await?; + let storage = resolved_session_storage_path(state, request).await?; + state + .compatibility + .update_session_model_selection_from_storage_path( + &storage, + &session_id, + &model_name, + reasoning_preset.as_deref(), + optional_bool(request, "includeInternal").unwrap_or(false), + ) + .await + .map_err(|error| peer_core_session_error("Failed to update session model", error))?; + return Ok(Value::Null); } state .agent_runtime diff --git a/src/apps/desktop/AGENTS.md b/src/apps/desktop/AGENTS.md index 9fa16df9fe..864bfd590e 100644 --- a/src/apps/desktop/AGENTS.md +++ b/src/apps/desktop/AGENTS.md @@ -124,6 +124,17 @@ For session storage directory access, run for session storage directory resolution (these tests do not launch a file manager), and `cargo test -p openbitfun-desktop --lib remote_workspace_policy` after registration changes. +For model selector request compatibility and configuration effect routing: + +```bash +cargo test --locked -p openbitfun-desktop --lib update_session_model_distinguishes_omitted_null_and_explicit_reasoning_presets +cargo test --locked -p openbitfun-desktop --lib backend_effect_routing_is_owned_by_the_control_transaction +``` + +Use the Core guide's session-model tests for cold persistence, no-op writes, +rollback, and ownership; the Desktop adapter must not restore history for a +model selection alone. + For MiniApp Agent session and context changes, run `cargo test --locked -p openbitfun-desktop --lib api::miniapp_agent_api::tests`. This covers legacy payloads, exact context paths, trust instructions, and snapshot cleanup. diff --git a/src/apps/desktop/src/api/agentic_api.rs b/src/apps/desktop/src/api/agentic_api.rs index 5e23546f5f..eea76c20b8 100644 --- a/src/apps/desktop/src/api/agentic_api.rs +++ b/src/apps/desktop/src/api/agentic_api.rs @@ -2102,16 +2102,28 @@ pub async fn update_session_model( if session_id.is_empty() { return Err("session_id is required".to_string()); } - ensure_session_loaded_for_selector_update( - runtime.inner(), - &session_id, - request.workspace_id, - request.workspace_path, - request.remote_connection_id, - request.remote_ssh_host, - request.include_internal, - ) - .await?; + if runtime + .session_application() + .update_unloaded_session_model( + DesktopSessionScopeRequest { + workspace_id: request.workspace_id, + workspace_path: request.workspace_path.unwrap_or_default(), + remote_connection_id: request.remote_connection_id, + remote_ssh_host: request.remote_ssh_host, + }, + &session_id, + &request.model_name, + request + .reasoning_preset + .as_ref() + .and_then(|value| value.as_deref()), + request.include_internal, + ) + .await + .map_err(|error| error.to_string())? + { + return Ok(()); + } let update_result = match request.reasoning_preset { Some(reasoning_preset) => { let reasoning_preset = reasoning_preset diff --git a/src/apps/desktop/src/openbitfun_control_host.rs b/src/apps/desktop/src/openbitfun_control_host.rs index 559dc2bb12..310b2133aa 100644 --- a/src/apps/desktop/src/openbitfun_control_host.rs +++ b/src/apps/desktop/src/openbitfun_control_host.rs @@ -266,8 +266,6 @@ fn option_requires_presentation_commit(option: &ProductCapabilityOption) -> bool fn config_path_invalidates_ai_client(path: &str) -> bool { [ "ai.models", - "ai.default_models", - "ai.agent_model_defaults", "ai.stream_idle_timeout_secs", "ai.stream_ttft_timeout_secs", "ai.proxy", @@ -710,6 +708,17 @@ pub(crate) async fn set_config_from_gui( let state = app.state::(); let previous_value = state.config_service.get_config(Some(path)).await.ok(); let applied = apply_legacy_config_mutation(&state.config_service, path, value).await?; + if !applied.changed { + return Ok(json!({ + "catalogDigest": openbitfun_product_domains::product_control::catalog()?.digest.clone(), + "configured": true, + "changedPaths": [], + "controlledBindings": applied.controlled_bindings, + "effectiveValue": applied.effective_value, + "revision": current_revision(), + "presentationSync": [], + })); + } let backend_effect_result = match apply_backend_config_effects(app, &state, path, &applied.effective_value).await { Ok(()) => { @@ -1233,8 +1242,6 @@ mod tests { for path in [ "ai.models", "ai.models.providers.openai", - "ai.default_models.primary", - "ai.agent_model_defaults", "ai.stream_idle_timeout_secs", "ai.stream_ttft_timeout_secs", "ai.proxy", @@ -1247,6 +1254,9 @@ mod tests { assert!(!config_path_invalidates_ai_client( "ai.tool_execution_timeout_secs" )); + for path in ["ai.default_models.primary", "ai.agent_model_defaults.mode"] { + assert!(!config_path_invalidates_ai_client(path)); + } let appearance = product_capability("setting.application.appearance") .unwrap() diff --git a/src/apps/desktop/src/runtime/session_application.rs b/src/apps/desktop/src/runtime/session_application.rs index 2360c8bc73..55b029ddc8 100644 --- a/src/apps/desktop/src/runtime/session_application.rs +++ b/src/apps/desktop/src/runtime/session_application.rs @@ -739,6 +739,36 @@ impl DesktopSessionApplication { Ok(updated_title) } + pub(crate) async fn update_unloaded_session_model( + &self, + request: DesktopSessionScopeRequest, + session_id: &str, + model_id: &str, + reasoning_preset: Option<&str>, + include_internal: bool, + ) -> DesktopSessionApplicationResult { + if self + .compatibility + .is_session_loaded_in_memory(session_id) + .map_err(desktop_core_session_error)? + { + return Ok(false); + } + let scope = self.resolved_scope(request).await?; + self.ensure_runtime_ownership(&scope)?; + self.compatibility + .update_session_model_selection_from_storage_path( + &self.storage_path(&scope), + session_id, + model_id, + reasoning_preset, + include_internal, + ) + .await + .map_err(desktop_core_session_error)?; + Ok(true) + } + pub(crate) async fn ensure_session_loaded( &self, request: DesktopSessionScopeRequest, diff --git a/src/crates/assembly/core/AGENTS.md b/src/crates/assembly/core/AGENTS.md index 203829187b..d1888c999d 100644 --- a/src/crates/assembly/core/AGENTS.md +++ b/src/crates/assembly/core/AGENTS.md @@ -257,6 +257,20 @@ For built-in provider overlay, trusted endpoint validation, and reasoning catalo cargo test -p openbitfun-core --no-default-features --features ai-adapter-runtime --lib infrastructure::ai:: ``` +For model selector latency, run the catalog and session mutation owner fixtures: + +```bash +cargo test --locked -p openbitfun-core --no-default-features --features model-catalog --lib infrastructure::ai::reasoning_catalog::tests +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git --lib session_model_update +cargo test --locked -p openbitfun-core --no-default-features --features remote-connect,git --lib remote_attach_and_mutation_paths_preserve_workspace_ownership_facts +``` + +Catalog snapshots remain usable during background refresh. Selector mutations +must not restore cold transcripts or reserve active-session capacity; preserve +writer ownership, unknown persisted fields, legacy metadata-only sessions, and +both state/metadata projections on failure. These fixtures cover local storage +and remote routing contracts, not live SSH/relay/peer/dispatch connections. + Configuration persistence, account settings import, backup restore, legacy field/deletion compatibility, local-change notifications, and save/reload/model concurrency regressions have feature-free fixtures: diff --git a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs index 6cb62bca19..62ad18c4a3 100644 --- a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs +++ b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs @@ -2602,6 +2602,41 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet Ok(()) } + /// A selector mutation can edit a historical header without admitting a + /// runtime session. Ownership is checked against the persisted binding + /// while the session mutation and writer permits are held. + pub(crate) async fn update_session_model_selection_from_storage_path( + &self, + storage_path: &Path, + session_id: &str, + model_id: &str, + reasoning_preset: Option<&str>, + include_internal: bool, + ) -> OpenBitFunResult { + let model_id = normalize_model_selection(model_id).await?; + self.session_manager + .update_session_model_selection_in_storage( + Some(storage_path), + session_id, + &model_id, + reasoning_preset, + include_internal, + |session| { + let workspace = session.config.workspace_path.as_deref().ok_or_else(|| { + OpenBitFunError::Validation(format!( + "Session workspace_path is missing: {session_id}" + )) + })?; + self.ensure_runtime_ownership( + Path::new(workspace), + session.config.remote_connection_id.as_deref(), + session.config.remote_ssh_host.as_deref(), + ) + }, + ) + .await + } + /// Re-enable (or disable) the tool loop of an already persisted session. /// /// Session configs are written once at creation, so a host that changes its diff --git a/src/crates/assembly/core/src/agentic/persistence/manager.rs b/src/crates/assembly/core/src/agentic/persistence/manager.rs index 7c150b37cd..c8e4fb64d3 100644 --- a/src/crates/assembly/core/src/agentic/persistence/manager.rs +++ b/src/crates/assembly/core/src/agentic/persistence/manager.rs @@ -1795,7 +1795,7 @@ impl PersistenceManager { &self, workspace_path: &Path, session_id: &str, - state: &StoredSessionStateFile, + state: &(impl serde::Serialize + Sync), ) -> OpenBitFunResult<()> { #[cfg(test)] { @@ -2492,6 +2492,118 @@ impl PersistenceManager { Ok(()) } + /// Persist only selector facts. Historical sessions need neither transcript + /// loading nor runtime restoration, and unknown future state fields survive. + pub(crate) async fn save_session_model_selection( + &self, + workspace_path: &Path, + session: &Session, + ) -> OpenBitFunResult<()> { + let session_id = &session.session_id; + Self::validate_session_id(session_id)?; + let _writer = self.lock_session_write_operation(workspace_path, session_id)?; + self.ensure_runtime_for_write(workspace_path).await?; + let lock = self + .get_session_persistence_lock(workspace_path, session_id) + .await; + let _guard = lock.lock().await; + let original_metadata = self + .load_session_metadata(workspace_path, session_id) + .await? + .ok_or_else(|| OpenBitFunError::NotFound(format!("Session not found: {session_id}")))?; + let state_path = self.state_path(workspace_path, session_id); + let original_state = self + .read_json_optional::(&state_path) + .await?; + let mut state = match original_state.as_ref() { + Some(value) => value.clone(), + None => serde_json::to_value(StoredSessionStateFile { + schema_version: SESSION_STORAGE_SCHEMA_VERSION, + config: session.config.clone(), + snapshot_session_id: session.snapshot_session_id.clone(), + last_user_dialog_agent_type: session.last_user_dialog_agent_type.clone(), + last_submitted_agent_type: session.last_submitted_agent_type.clone(), + compression_state: session.compression_state.clone(), + runtime_state: sanitize_persisted_session_state(&session.state), + })?, + }; + let object = state + .as_object_mut() + .ok_or_else(|| OpenBitFunError::session("Invalid session state object"))?; + let config = object + .entry("config") + .or_insert_with(|| serde_json::json!({})) + .as_object_mut() + .ok_or_else(|| OpenBitFunError::session("Invalid session config object"))?; + config.insert( + "model_id".into(), + serde_json::to_value(&session.config.model_id)?, + ); + config.insert( + "reasoning_preset".into(), + serde_json::to_value(&session.config.reasoning_preset)?, + ); + config.insert( + "max_context_tokens".into(), + serde_json::to_value(session.config.max_context_tokens)?, + ); + let mut metadata = original_metadata.clone(); + metadata.model_name = session.config.model_id.clone().unwrap_or_default(); + metadata.last_active_at = Self::system_time_to_unix_ms(session.last_activity_at); + if original_state.as_ref() == Some(&state) + && original_metadata.model_name == metadata.model_name + && original_metadata.last_active_at == metadata.last_active_at + { + return Ok(()); + } + let write_result = async { + self.save_stored_session_state(workspace_path, session_id, &state) + .await?; + self.save_session_metadata_locked(workspace_path, &metadata) + .await + } + .await; + if let Err(error) = write_result { + // An IO error can be reported before or after an atomic rename. + // Avoid unnecessary rollback writes when the original is intact. + if self + .read_json_optional::(&state_path) + .await + .is_ok_and(|state| state == original_state) + && self + .load_session_metadata(workspace_path, session_id) + .await + .is_ok_and(|value| { + value.is_some_and(|value| { + value.model_name == original_metadata.model_name + && value.last_active_at == original_metadata.last_active_at + }) + }) + { + return Err(error); + } + let state_rollback = match original_state { + Some(state) => { + self.save_stored_session_state(workspace_path, session_id, &state) + .await + } + None => tokio::fs::remove_file(&state_path) + .await + .map_err(Into::into), + }; + let metadata_rollback = self + .save_session_metadata_locked(workspace_path, &original_metadata) + .await; + if state_rollback.is_err() || metadata_rollback.is_err() { + return Err(OpenBitFunError::OutcomeUnknown(format!( + "Session model update rollback failed: session_id={session_id}, error={error}, state={state_rollback:?}, metadata={metadata_rollback:?}" + ))); + } + return Err(error); + } + Ok(()) + } + /// Save session pub async fn save_session( &self, diff --git a/src/crates/assembly/core/src/agentic/session/session_manager.rs b/src/crates/assembly/core/src/agentic/session/session_manager.rs index 579fc76641..c8d98a3fd5 100644 --- a/src/crates/assembly/core/src/agentic/session/session_manager.rs +++ b/src/crates/assembly/core/src/agentic/session/session_manager.rs @@ -2555,6 +2555,9 @@ impl SessionManager { config: manager_config, }; + let mut model_inputs = Self::load_ai_config_for_model_resolution() + .await + .and_then(|ai| serde_json::to_value((&ai.models, &ai.default_models)).ok()); loop { match receiver.recv().await { Ok(ConfigUpdateEvent::ModelsReconciled { @@ -2575,6 +2578,15 @@ impl SessionManager { | ConfigUpdateEvent::DefaultAIModelUpdated { .. } | ConfigUpdateEvent::ConfigReloaded, ) => { + let next_inputs = Self::load_ai_config_for_model_resolution() + .await + .and_then(|ai| { + serde_json::to_value((&ai.models, &ai.default_models)).ok() + }); + if next_inputs.is_some() && next_inputs == model_inputs { + continue; + } + model_inputs = next_inputs; manager .reconcile_loaded_session_reasoning_presets( "model_configuration_updated", @@ -4213,39 +4225,71 @@ impl SessionManager { model_id: &str, reasoning_preset: Option<&str>, ) -> OpenBitFunResult<()> { + self.update_session_model_selection_in_storage( + None, + session_id, + model_id, + reasoning_preset, + true, + |_| Ok(()), + ) + .await + .map(|_| ()) + } + + pub(crate) async fn update_session_model_selection_in_storage( + &self, + storage_path: Option<&Path>, + session_id: &str, + model_id: &str, + reasoning_preset: Option<&str>, + include_internal: bool, + validate_binding: impl FnOnce(&Session) -> OpenBitFunResult<()>, + ) -> OpenBitFunResult { + let _mutation_guard = self.acquire_session_mutation(session_id).await?; + let storage_path = storage_path.map(Path::to_path_buf).or_else(|| { + Self::committed_session_storage_path(&self.session_storage_path_index, session_id) + }); + if let Some(path) = storage_path.as_deref() { + self.validate_session_storage_path_binding(session_id, path)?; + } + let loaded = self.sessions.get(session_id).map(|session| session.clone()); + // Keep the cross-process lease until the complete read/modify/write has + // finished. A cold selector update never reserves active-session capacity. + let _writer = if loaded.is_none() { + storage_path + .as_deref() + .map(|path| { + self.persistence_manager + .lock_session_writes(path, session_id) + }) + .transpose()? + } else { + None + }; + let was_loaded = loaded.is_some(); + let original_session = match loaded { + Some(session) => session, + None => { + let path = storage_path.as_deref().ok_or_else(|| { + OpenBitFunError::NotFound(format!("Session not found: {session_id}")) + })?; + let session = self + .persistence_manager + .load_session_header(path, session_id) + .await?; + if !include_internal && session.kind != SessionKind::Standard { + return Err(OpenBitFunError::NotFound(format!( + "Session not found: {session_id}" + ))); + } + session + } + }; + validate_binding(&original_session)?; let ai_config = Self::load_ai_config_for_model_resolution().await; let mut resolved_context_window = None; let mut auto_cleared_reasoning_preset = None; - - // If the session was evicted from memory (idle > 1h), try to restore it - // using the storage path recorded when it was first created/restored. - if !self.sessions.contains_key(session_id) && self.config.enable_persistence { - let session_storage_path = - Self::committed_session_storage_path(&self.session_storage_path_index, session_id); - if let Some(session_storage_path) = session_storage_path { - debug!( - "Session evicted from memory, restoring for model update: session_id={}", - session_id - ); - let _ = self - .restore_session_from_storage_path(&session_storage_path, session_id) - .await; - } - } - - // Restore owns the same keyed lock internally, so acquire the mutation - // permit only after the optional restore completes. From here through - // persistence, explicit deletion cannot remove and then be recreated by - // a late model update. - let _mutation_guard = self.acquire_session_mutation(session_id).await?; - - let original_session = self - .sessions - .get(session_id) - .map(|session| session.clone()) - .ok_or_else(|| { - OpenBitFunError::NotFound(format!("Session not found: {}", session_id)) - })?; let mut updated_session = original_session.clone(); updated_session.config.model_id = Some(model_id.to_string()); updated_session.config.reasoning_preset = reasoning_preset @@ -4270,43 +4314,44 @@ impl SessionManager { resolved_context_window = Self::sync_session_context_window_from_ai_config(&mut updated_session, ai_config); } + if original_session.config.model_id == updated_session.config.model_id + && original_session.config.reasoning_preset == updated_session.config.reasoning_preset + && original_session.config.max_context_tokens + == updated_session.config.max_context_tokens + { + if !was_loaded && self.should_persist_session_id(session_id) { + if let Some(path) = storage_path.as_deref() { + // Repair a metadata projection left behind by an interrupted + // two-file commit; an aligned snapshot performs no write. + self.persistence_manager + .save_session_model_selection(path, &original_session) + .await?; + } + } + return Ok(original_session); + } let now = SystemTime::now(); updated_session.updated_at = now; updated_session.last_activity_at = now; if self.should_persist_session_id(session_id) { - let effective_path = self.effective_session_storage_path(session_id).await; - if let Some(workspace_path) = effective_path { - if let Err(error) = self - .persistence_manager - .save_session(&workspace_path, &updated_session) - .await - { - if let Err(rollback_error) = self - .persistence_manager - .save_session(&workspace_path, &original_session) - .await - { - return Err(OpenBitFunError::session(format!( - "Session model persistence failed and rollback did not complete: session_id={session_id}, error={error}, rollback_error={rollback_error}" - ))); - } - return Err(error); - } + let effective_path = match storage_path { + Some(path) => Some(path), + None => self.effective_session_storage_path(session_id).await, + }; + if let Some(path) = effective_path { + self.persistence_manager + .save_session_model_selection(&path, &updated_session) + .await?; } } if let Some(mut session) = self.sessions.get_mut(session_id) { - session.config.model_id = updated_session.config.model_id; + session.config.model_id = updated_session.config.model_id.clone(); session.config.reasoning_preset = updated_session.config.reasoning_preset.clone(); session.config.max_context_tokens = updated_session.config.max_context_tokens; session.updated_at = now; session.last_activity_at = now; - } else { - return Err(OpenBitFunError::NotFound(format!( - "Session not found: {}", - session_id - ))); } debug!( @@ -4329,7 +4374,7 @@ impl SessionManager { } } - Ok(()) + Ok(updated_session) } /// Sets the session's own tool permission mode (in-memory + persistence). @@ -4343,7 +4388,7 @@ impl SessionManager { session_id: &str, permission_mode: Option, ) -> OpenBitFunResult<()> { - // Match the model-selection path: an evicted session is restored before + // Permission updates still restore an evicted session before // the mutation permit is taken, because restore owns the same keyed lock. if !self.sessions.contains_key(session_id) && self.config.enable_persistence { let session_storage_path = @@ -12831,6 +12876,289 @@ mod tests { assert!(!manager.context_store.has_session(&session_id)); } + #[tokio::test] + async fn cold_session_model_update_preserves_history_and_unknown_state_without_restore() { + let workspace = TestWorkspace::new(); + let paths = workspace.path_manager(); + let persistence = Arc::new(PersistenceManager::new(paths.clone()).unwrap()); + let manager = test_manager(persistence.clone()); + let id = Uuid::new_v4().to_string(); + let session = Session::new_with_id( + id.clone(), + "Cold selector".into(), + "Standard".into(), + SessionConfig { + workspace_path: Some(workspace.path().to_string_lossy().into_owned()), + model_id: Some("primary".into()), + ..Default::default() + }, + ); + persistence + .save_session(workspace.path(), &session) + .await + .unwrap(); + let directory = paths.project_sessions_dir(workspace.path()).join(&id); + let state_path = directory.join("state.json"); + let mut state: serde_json::Value = + serde_json::from_slice(&std::fs::read(&state_path).unwrap()).unwrap(); + state["future_state"] = serde_json::json!({"preserve": true}); + state["config"]["future_config"] = serde_json::json!(42); + std::fs::write(&state_path, serde_json::to_vec(&state).unwrap()).unwrap(); + // A corrupt turn body would make a full restore fail. Selector writes + // must leave transcript bytes alone, including unknown legacy content. + std::fs::create_dir_all(directory.join("turns")).unwrap(); + let turn_path = directory.join("turns").join("untouched.json"); + std::fs::write(&turn_path, b"unreadable historical turn").unwrap(); + let update = manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "fast", + Some("high"), + false, + |_| Ok(()), + ) + .await + .unwrap(); + assert_eq!(update.config.reasoning_preset.as_deref(), Some("high")); + assert!(manager.get_session(&id).is_none()); + assert!(manager.active_session_permits.is_empty()); + assert!(!manager.context_store.has_session(&id)); + let updated: serde_json::Value = + serde_json::from_slice(&std::fs::read(&state_path).unwrap()).unwrap(); + assert_eq!(updated["future_state"], state["future_state"]); + assert_eq!(updated["config"]["future_config"], 42); + assert_eq!(updated["config"]["model_id"], "fast"); + assert_eq!( + persistence + .load_session_metadata(workspace.path(), &id) + .await + .unwrap() + .unwrap() + .model_name, + "fast" + ); + assert_eq!( + std::fs::read(&turn_path).unwrap(), + b"unreadable historical turn" + ); + let bytes = std::fs::read(&state_path).unwrap(); + manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "fast", + Some("high"), + false, + |_| Ok(()), + ) + .await + .unwrap(); + assert_eq!(std::fs::read(&state_path).unwrap(), bytes); + // Clearing a preset on the same model is a real mutation. + manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "fast", + None, + false, + |_| Ok(()), + ) + .await + .unwrap(); + assert!(persistence + .load_session_header(workspace.path(), &id) + .await + .unwrap() + .config + .reasoning_preset + .is_none()); + persistence.fail_next_session_state_write_for_test(&id); + manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "primary", + None, + false, + |_| Ok(()), + ) + .await + .unwrap_err(); + assert_eq!( + persistence + .load_session_header(workspace.path(), &id) + .await + .unwrap() + .config + .model_id + .as_deref(), + Some("fast") + ); + let state_before_failure = std::fs::read(&state_path).unwrap(); + persistence.fail_next_session_metadata_write_for_test(&id); + assert!(manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "primary", + None, + false, + |_| Ok(()) + ) + .await + .is_err()); + assert_eq!(std::fs::read(&state_path).unwrap(), state_before_failure); + assert_eq!( + persistence + .load_session_metadata(workspace.path(), &id) + .await + .unwrap() + .unwrap() + .model_name, + "fast" + ); + assert!(manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "primary", + None, + false, + |_| Err(OpenBitFunError::Validation("binding denied".into())) + ) + .await + .is_err()); + let writer = persistence + .lock_session_writes(workspace.path(), &id) + .unwrap(); + assert!(manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "primary", + None, + false, + |_| Ok(()) + ) + .await + .is_err()); + drop(writer); + assert_eq!(std::fs::read(&state_path).unwrap(), state_before_failure); + // A restart may encounter a metadata projection from before a commit. + persistence + .update_session_metadata(workspace.path(), &id, |metadata| { + metadata.model_name = "primary".into() + }) + .await + .unwrap(); + manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "fast", + None, + false, + |_| Ok(()), + ) + .await + .unwrap(); + assert_eq!( + persistence + .load_session_metadata(workspace.path(), &id) + .await + .unwrap() + .unwrap() + .model_name, + "fast" + ); + } + + #[tokio::test] + async fn cold_session_model_update_upgrades_metadata_only_and_rejects_missing_or_internal_sessions( + ) { + let workspace = TestWorkspace::new(); + let paths = workspace.path_manager(); + let persistence = Arc::new(PersistenceManager::new(paths.clone()).unwrap()); + let manager = test_manager(persistence.clone()); + let id = Uuid::new_v4().to_string(); + let mut session = Session::new_with_id( + id.clone(), + "Legacy".into(), + "Standard".into(), + SessionConfig { + workspace_path: Some(workspace.path().to_string_lossy().into_owned()), + model_id: Some("primary".into()), + ..Default::default() + }, + ); + persistence + .save_session(workspace.path(), &session) + .await + .unwrap(); + std::fs::remove_file( + paths + .project_sessions_dir(workspace.path()) + .join(&id) + .join("state.json"), + ) + .unwrap(); + manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "fast", + None, + false, + |_| Ok(()), + ) + .await + .unwrap(); + assert_eq!( + persistence + .load_session(workspace.path(), &id) + .await + .unwrap() + .config + .model_id + .as_deref(), + Some("fast") + ); + session.kind = SessionKind::Subagent; + persistence + .save_session(workspace.path(), &session) + .await + .unwrap(); + assert!(manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + &id, + "fast", + None, + false, + |_| Ok(()) + ) + .await + .is_err()); + assert!(manager + .update_session_model_selection_in_storage( + Some(workspace.path()), + "missing-session", + "fast", + None, + false, + |_| Ok(()) + ) + .await + .is_err()); + assert!(!paths + .project_sessions_dir(workspace.path()) + .join("missing-session") + .join("metadata.json") + .exists()); + } + #[tokio::test] async fn session_model_update_is_restored_from_persistence() { let workspace = TestWorkspace::new(); diff --git a/src/crates/assembly/core/src/agentic/tools/openbitfun_control_config.rs b/src/crates/assembly/core/src/agentic/tools/openbitfun_control_config.rs index e6cd475393..3f6f31221c 100644 --- a/src/crates/assembly/core/src/agentic/tools/openbitfun_control_config.rs +++ b/src/crates/assembly/core/src/agentic/tools/openbitfun_control_config.rs @@ -222,12 +222,16 @@ impl SharedProductControlExecutor { pub async fn configure_legacy_path(&self, path: &str, value: Value) -> Result { let _mutation = self.mutation_lock.lock().await; let applied = apply_legacy_config_mutation(&self.config_service, path, value).await?; - let revision = self.revision.fetch_add(1, Ordering::AcqRel) + 1; + let revision = if applied.changed { + self.revision.fetch_add(1, Ordering::AcqRel) + 1 + } else { + self.revision.load(Ordering::Acquire) + }; Ok(json!({ "catalogDigest": product_control_catalog()?.digest.clone(), "configured": true, "changedPath": applied.changed_path.clone(), - "changedPaths": [applied.changed_path], + "changedPaths": if applied.changed { vec![applied.changed_path] } else { vec![] }, "controlledBindings": applied.controlled_bindings, "effectiveValue": applied.effective_value, "revision": revision, @@ -266,6 +270,7 @@ pub struct ProductConfigBinding { #[derive(Debug, Clone, PartialEq)] pub struct AppliedLegacyProductConfig { + pub changed: bool, pub changed_path: String, pub effective_value: Value, pub controlled_bindings: Vec, @@ -454,8 +459,8 @@ pub async fn apply_legacy_config_mutation( ) -> Result { let controlled_bindings = validate_legacy_config_mutation(path, &value)?; validate_config_semantics(path, &value)?; - config_service - .set_config(path, value) + let changed = config_service + .set_config_if_changed(path, value) .await .map_err(|error| error.to_string())?; let effective_value = config_service @@ -463,6 +468,7 @@ pub async fn apply_legacy_config_mutation( .await .map_err(|error| error.to_string())?; Ok(AppliedLegacyProductConfig { + changed, changed_path: path.to_string(), effective_value, controlled_bindings, diff --git a/src/crates/assembly/core/src/infrastructure/ai/reasoning_catalog.rs b/src/crates/assembly/core/src/infrastructure/ai/reasoning_catalog.rs index ed6a1c4f63..6140dc2b08 100644 --- a/src/crates/assembly/core/src/infrastructure/ai/reasoning_catalog.rs +++ b/src/crates/assembly/core/src/infrastructure/ai/reasoning_catalog.rs @@ -1,6 +1,6 @@ use std::sync::Arc; #[cfg(feature = "model-catalog")] -use std::sync::{OnceLock, RwLock}; +use std::sync::{Mutex, OnceLock, RwLock}; #[cfg(feature = "model-catalog")] use std::time::{Duration, Instant}; @@ -41,19 +41,110 @@ pub(crate) struct ModelsDevReasoningCatalogSnapshot { pub(crate) source: ModelsDevSnapshotSource, } +// This is a background check interval, not a snapshot expiration time. #[cfg(feature = "model-catalog")] -const CATALOG_RELOAD_INTERVAL: Duration = Duration::from_secs(60); +const CATALOG_CHECK_INTERVAL: Duration = Duration::from_secs(60); #[cfg(feature = "model-catalog")] struct CachedReasoningCatalogSnapshot { - loaded_at: Instant, snapshot: ModelsDevReasoningCatalogSnapshot, } #[cfg(feature = "model-catalog")] -fn parsed_catalog_cache() -> &'static RwLock> { - static CACHE: OnceLock>> = OnceLock::new(); - CACHE.get_or_init(|| RwLock::new(None)) +#[derive(Default)] +struct ParsedCatalogCache { + snapshot: RwLock>, + // Covers reading, parsing and publication, including manual refreshes. + update: tokio::sync::Mutex<()>, + last_check: Mutex>, +} + +#[cfg(feature = "model-catalog")] +impl ParsedCatalogCache { + fn current(&self) -> Option { + self.snapshot + .read() + .unwrap_or_else(|error| error.into_inner()) + .as_ref() + .map(|cached| cached.snapshot.clone()) + } + + async fn load(&self, service: &ModelsDevCatalogService) -> ModelsDevReasoningCatalogSnapshot { + if let Some(snapshot) = self.current() { + return snapshot; + } + let _update = self.update.lock().await; + if let Some(snapshot) = self.current() { + return snapshot; + } + let source = service.load_cached_or_bundled().await; + self.publish(source).await; + self.current().expect("initial catalog snapshot") + } + + // Caller holds update so a cold read cannot publish over a newer refresh. + async fn publish(&self, source: ModelsDevSnapshot) -> bool { + let current = self.current(); + if let Some(current) = current + .as_ref() + .filter(|current| current.sha256 == source.sha256) + { + if current.source == source.source { + return false; + } + let mut updated = current.clone(); + updated.source = source.source; + return replace_cached_catalog( + &mut self + .snapshot + .write() + .unwrap_or_else(|error| error.into_inner()), + updated, + ); + } + let version = source.version; + let sha256 = source.sha256.clone(); + let source_kind = source.source; + let parsed = tokio::task::spawn_blocking(move || parse_models_dev_snapshot(&source)) + .await + .ok() + .flatten(); + let updated = match parsed { + Some(updated) => updated, + None if current.is_some() => return false, + None => ModelsDevReasoningCatalogSnapshot { + catalog: None, + version, + sha256, + source: source_kind, + }, + }; + replace_cached_catalog( + &mut self + .snapshot + .write() + .unwrap_or_else(|error| error.into_inner()), + updated, + ) + } + + fn claim_background_check(&self) -> bool { + let mut last = self + .last_check + .lock() + .unwrap_or_else(|error| error.into_inner()); + if last.is_some_and(|last| last.elapsed() < CATALOG_CHECK_INTERVAL) { + return false; + } + *last = Some(Instant::now()); + true + } +} + +#[cfg(feature = "model-catalog")] +fn parsed_catalog_cache() -> &'static ParsedCatalogCache { + static CACHE: OnceLock = OnceLock::new(); + CACHE.get_or_init(ParsedCatalogCache::default) } #[cfg(feature = "model-catalog")] @@ -113,100 +204,89 @@ pub(crate) async fn get_models_dev_catalog_status() -> ModelsDevCatalogStatus { #[cfg(feature = "model-catalog")] pub(crate) async fn refresh_models_dev_catalog_now() -> Result { + let cache = parsed_catalog_cache(); let service = models_dev_catalog_service(); - let outcome = service.refresh_now().await; - match outcome { - ModelsDevRefreshOutcome::Updated(snapshot) => { - if let Some(updated) = parse_models_dev_snapshot(&snapshot) { - if replace_parsed_catalog_cache(updated) { - emit_models_dev_catalog_updated(&snapshot).await; - } + // Ensure status reads never have to wait on an in-progress network refresh. + cache.load(service).await; + let outcome = if let Ok(_update) = cache.update.try_lock() { + let outcome = service.refresh_now().await; + match &outcome { + ModelsDevRefreshOutcome::Failed => { + return Err("Failed to refresh models.dev catalog".to_string()) } - Ok(ModelsDevRefreshResult { - outcome: ModelsDevRefreshStatus::Updated, - status: models_dev_catalog_status().await, - }) + ModelsDevRefreshOutcome::Throttled => {} + _ => publish_current_catalog(cache, service).await, } - ModelsDevRefreshOutcome::Unchanged { .. } => { - let snapshot = service.load_cached_or_bundled().await; - if let Some(updated) = parse_models_dev_snapshot(&snapshot) { - replace_parsed_catalog_cache(updated); - } - Ok(ModelsDevRefreshResult { - outcome: ModelsDevRefreshStatus::Unchanged, - status: models_dev_catalog_status().await, - }) - } - ModelsDevRefreshOutcome::NotNeeded => Ok(ModelsDevRefreshResult { - outcome: ModelsDevRefreshStatus::Unchanged, - status: models_dev_catalog_status().await, - }), - ModelsDevRefreshOutcome::Throttled => Ok(ModelsDevRefreshResult { - outcome: ModelsDevRefreshStatus::Throttled, - status: models_dev_catalog_status().await, - }), - ModelsDevRefreshOutcome::Failed => Err("Failed to refresh models.dev catalog".to_string()), - } + outcome + } else { + ModelsDevRefreshOutcome::Throttled + }; + Ok(ModelsDevRefreshResult { + outcome: match outcome { + ModelsDevRefreshOutcome::Updated(_) => ModelsDevRefreshStatus::Updated, + ModelsDevRefreshOutcome::Throttled => ModelsDevRefreshStatus::Throttled, + _ => ModelsDevRefreshStatus::Unchanged, + }, + status: models_dev_catalog_status().await, + }) } #[cfg(feature = "model-catalog")] -pub(crate) async fn load_models_dev_reasoning_catalog_without_refresh( -) -> ModelsDevReasoningCatalogSnapshot { - if let Ok(cache) = parsed_catalog_cache().read() { - if let Some(cached) = cache - .as_ref() - .filter(|cached| cached.loaded_at.elapsed() < CATALOG_RELOAD_INTERVAL) - { - return cached.snapshot.clone(); - } - } - - let service = models_dev_catalog_service(); - let snapshot = service.load_cached_or_bundled().await; - let catalog = match ModelsDevCatalog::parse_str(&snapshot.body) { - Ok(catalog) => Some(Arc::new(catalog)), - Err(error) => { - debug!("Failed to parse models.dev catalog snapshot: {}", error); - None - } +async fn publish_current_catalog(cache: &ParsedCatalogCache, service: &ModelsDevCatalogService) { + let source = service.load_cached_or_bundled().await; + let event = AIModelCatalogUpdatedEvent { + source_version: source.version.to_string(), + sha256: source.sha256.clone(), }; - - let loaded = ModelsDevReasoningCatalogSnapshot { - catalog, - #[cfg(feature = "model-catalog")] - version: snapshot.version, - sha256: snapshot.sha256, - source: snapshot.source, - }; - if let Ok(mut cache) = parsed_catalog_cache().write() { - *cache = Some(CachedReasoningCatalogSnapshot { - loaded_at: Instant::now(), - snapshot: loaded.clone(), - }); + if cache.publish(source).await { + emit_models_dev_catalog_updated(event).await; } +} - loaded +#[cfg(feature = "model-catalog")] +pub(crate) async fn load_models_dev_reasoning_catalog_without_refresh( +) -> ModelsDevReasoningCatalogSnapshot { + parsed_catalog_cache() + .load(models_dev_catalog_service()) + .await } #[cfg(feature = "model-catalog")] pub(crate) async fn load_models_dev_reasoning_catalog() -> ModelsDevReasoningCatalogSnapshot { let loaded = load_models_dev_reasoning_catalog_without_refresh().await; + schedule_catalog_refresh(); + loaded +} - let refresh_service = models_dev_catalog_service().clone(); +#[cfg(feature = "model-catalog")] +fn schedule_catalog_refresh() { + let cache = parsed_catalog_cache(); + if !cache.claim_background_check() { + return; + } tokio::spawn(async move { - let ModelsDevRefreshOutcome::Updated(snapshot) = refresh_service.refresh_if_stale().await - else { + let Ok(_update) = cache.update.try_lock() else { return; }; - let Some(updated) = parse_models_dev_snapshot(&snapshot) else { - return; - }; - if !replace_parsed_catalog_cache(updated) { - return; + let service = models_dev_catalog_service(); + // Observe changes written by another host process, then check the source. + publish_current_catalog(cache, service).await; + if !matches!( + service.refresh_if_stale().await, + ModelsDevRefreshOutcome::Failed | ModelsDevRefreshOutcome::Throttled + ) { + publish_current_catalog(cache, service).await; } - emit_models_dev_catalog_updated(&snapshot).await; }); - loaded +} + +/// Prepare local capabilities once the host configuration is available. Network +/// freshness is optional; readers always retain the last valid local snapshot. +#[cfg(feature = "model-catalog")] +pub(crate) fn prepare_model_catalog() { + tokio::spawn(async { + load_models_dev_reasoning_catalog().await; + }); } #[cfg(feature = "model-catalog")] @@ -231,41 +311,27 @@ fn parse_models_dev_snapshot( }) } -#[cfg(feature = "model-catalog")] -fn replace_parsed_catalog_cache(updated: ModelsDevReasoningCatalogSnapshot) -> bool { - let Ok(mut cache) = parsed_catalog_cache().write() else { - return false; - }; - replace_cached_catalog(&mut cache, updated) -} - #[cfg(feature = "model-catalog")] fn replace_cached_catalog( cache: &mut Option, updated: ModelsDevReasoningCatalogSnapshot, ) -> bool { if cache.as_ref().is_some_and(|cached| { - cached.snapshot.version == updated.version && cached.snapshot.source == updated.source + cached.snapshot.sha256 == updated.sha256 && cached.snapshot.source == updated.source }) { return false; } - *cache = Some(CachedReasoningCatalogSnapshot { - loaded_at: Instant::now(), - snapshot: updated, - }); + *cache = Some(CachedReasoningCatalogSnapshot { snapshot: updated }); true } #[cfg(feature = "model-catalog")] -async fn emit_models_dev_catalog_updated(snapshot: &ModelsDevSnapshot) { +async fn emit_models_dev_catalog_updated(event: AIModelCatalogUpdatedEvent) { crate::service::config::GlobalConfigManager::broadcast_update( crate::service::config::ConfigUpdateEvent::ReasoningCatalogUpdated, ) .await; - let payload = match serde_json::to_value(AIModelCatalogUpdatedEvent { - source_version: snapshot.version.to_string(), - sha256: snapshot.sha256.clone(), - }) { + let payload = match serde_json::to_value(event) { Ok(payload) => payload, Err(error) => { debug!( @@ -847,7 +913,6 @@ mod tests { #[test] fn refreshed_catalog_replaces_projection_without_waiting_for_reload_interval() { let mut cache = Some(super::CachedReasoningCatalogSnapshot { - loaded_at: std::time::Instant::now(), snapshot: super::ModelsDevReasoningCatalogSnapshot { catalog: None, version: 1, @@ -872,7 +937,6 @@ mod tests { fn cache_source_change_replaces_equal_bundled_snapshot() { let catalog = std::sync::Arc::new(catalog()); let mut cache = Some(super::CachedReasoningCatalogSnapshot { - loaded_at: std::time::Instant::now(), snapshot: super::ModelsDevReasoningCatalogSnapshot { catalog: Some(catalog.clone()), version: 1, @@ -894,4 +958,45 @@ mod tests { Some(openbitfun_services_integrations::models_dev::ModelsDevSnapshotSource::Cache) ); } + #[cfg(feature = "model-catalog")] + #[tokio::test] + async fn catalog_readers_share_initial_parse_and_keep_snapshot_until_publication() { + let directory = tempfile::tempdir().unwrap(); + let file = directory.path().join("catalog.json"); + tokio::fs::write(&file, r#"{"openai":{"models":{"a":{"id":"a"}}}}"#) + .await + .unwrap(); + let service = + openbitfun_services_integrations::models_dev::ModelsDevCatalogService::new(&file); + let cache = super::ParsedCatalogCache::default(); + let (a, b) = tokio::join!(cache.load(&service), cache.load(&service)); + assert!(std::sync::Arc::ptr_eq( + a.catalog.as_ref().unwrap(), + b.catalog.as_ref().unwrap() + )); + tokio::fs::write( + &file, + r#"{"openai":{"models":{"new-model":{"id":"new-model"}}}}"#, + ) + .await + .unwrap(); + // A read never revalidates the source, even after the background deadline. + *cache.last_check.lock().unwrap() = + Some(std::time::Instant::now() - std::time::Duration::from_secs(120)); + assert_eq!(cache.load(&service).await.sha256, a.sha256); + assert!(cache.claim_background_check()); + assert!(!cache.claim_background_check()); + let _update = cache.update.lock().await; + assert!(cache.publish(service.load_cached_or_bundled().await).await); + let updated = cache.load(&service).await; + assert_ne!(updated.sha256, a.sha256); + assert!(!cache.publish(service.load_cached_or_bundled().await).await); + assert!(std::sync::Arc::ptr_eq( + updated.catalog.as_ref().unwrap(), + cache.current().unwrap().catalog.as_ref().unwrap() + )); + tokio::fs::write(&file, "corrupt").await.unwrap(); + assert!(!cache.publish(service.load_cached_or_bundled().await).await); + assert_eq!(cache.load(&service).await.sha256, updated.sha256); + } } diff --git a/src/crates/assembly/core/src/product_runtime.rs b/src/crates/assembly/core/src/product_runtime.rs index 44af140a86..97a6346c2c 100644 --- a/src/crates/assembly/core/src/product_runtime.rs +++ b/src/crates/assembly/core/src/product_runtime.rs @@ -1633,6 +1633,25 @@ impl CoreAgentRuntimeCompatibility { Ok(()) } + pub async fn update_session_model_selection_from_storage_path( + &self, + storage_path: &Path, + session_id: &str, + model_id: &str, + reasoning_preset: Option<&str>, + include_internal: bool, + ) -> OpenBitFunResult { + self.coordinator + .update_session_model_selection_from_storage_path( + storage_path, + session_id, + model_id, + reasoning_preset, + include_internal, + ) + .await + } + pub async fn begin_persisted_session_mutation( &self, storage_path: &Path, diff --git a/src/crates/assembly/core/src/service/config/global.rs b/src/crates/assembly/core/src/service/config/global.rs index e4c8d29188..412a4c6cd3 100644 --- a/src/crates/assembly/core/src/service/config/global.rs +++ b/src/crates/assembly/core/src/service/config/global.rs @@ -106,6 +106,9 @@ impl GlobalConfigManager { crate::service::web_search::refresh_global_web_search_runtime(&ai_config).await; } + #[cfg(feature = "model-catalog")] + crate::infrastructure::ai::reasoning_catalog::prepare_model_catalog(); + info!("Global config service initialized"); Ok(()) diff --git a/src/crates/assembly/core/src/service/config/manager.rs b/src/crates/assembly/core/src/service/config/manager.rs index 70bfd88bce..0cda3f06b1 100644 --- a/src/crates/assembly/core/src/service/config/manager.rs +++ b/src/crates/assembly/core/src/service/config/manager.rs @@ -647,7 +647,20 @@ impl ConfigManager { })?; reject_protected_metadata_path(path)?; + if self.get::(path).ok().as_ref() == Some(&json_value) { + return Ok(()); + } let mut config = self.config_with_value(path, json_value)?; + // Selecting a future-session default changes only a reference to an + // already validated model. Keep legacy alias/fallback normalization, + // without constructing clients or revalidating every provider. + if path == "ai.agent_model_defaults.mode" { + reconcile_model_references(&mut config); + if config.ai.agent_model_defaults.mode == self.config.ai.agent_model_defaults.mode { + return Ok(()); + } + return self.commit_config(config, Some(path)).await; + } // Apply capability-driven canonicalization before validation and persistence. // Speech/embedding/image-only models must never carry text-generation sentinels. normalize_typed_config(&mut config); diff --git a/src/crates/assembly/core/src/service/config/service.rs b/src/crates/assembly/core/src/service/config/service.rs index fcf8208d65..78fb11346c 100644 --- a/src/crates/assembly/core/src/service/config/service.rs +++ b/src/crates/assembly/core/src/service/config/service.rs @@ -184,12 +184,26 @@ impl ConfigService { where T: serde::Serialize, { + self.set_config_if_changed(path, value).await.map(|_| ()) + } + + /// Compares and persists under the same lock; unchanged preferences do not + /// trigger validation, disk writes, account sync, or runtime notifications. + pub async fn set_config_if_changed(&self, path: &str, value: T) -> OpenBitFunResult + where + T: serde::Serialize, + { + let value = serde_json::to_value(value)?; { let mut manager = self.manager.write().await; + let before = manager.get::(path).ok(); manager.set(path, value).await?; + if before == manager.get::(path).ok() { + return Ok(false); + } } self.after_config_change(path).await; - Ok(()) + Ok(true) } /// Reads, modifies, validates, and persists a section under one write lock. @@ -225,11 +239,13 @@ impl ConfigService { async fn after_config_change(&self, path: &str) { let model_configuration_changed = Self::path_touches_models(path); if model_configuration_changed { - if let Err(e) = self.reconcile_models("set_config").await { - warn!( - "Model reconcile after set_config failed: path={}, error={}", - path, e - ); + if path != "ai.agent_model_defaults.mode" { + if let Err(e) = self.reconcile_models("set_config").await { + warn!( + "Model reconcile after set_config failed: path={}, error={}", + path, e + ); + } } super::global::GlobalConfigManager::broadcast_update( super::global::ConfigUpdateEvent::ModelConfigurationUpdated, @@ -803,6 +819,53 @@ mod tests { (service, dir) } + #[tokio::test] + async fn model_default_noop_does_not_persist_or_notify() { + let (service, dir) = test_service("model-default-noop").await; + service + .add_ai_model(runtime_model("chosen", "fixture-key")) + .await + .unwrap(); + let mut changes = service.subscribe_local_changes(); + assert!(service + .set_config_if_changed("ai.agent_model_defaults.mode", "chosen") + .await + .unwrap()); + changes.borrow_and_update(); + let before: GlobalConfig = service.get_config(None).await.unwrap(); + assert!(!service + .set_config_if_changed("ai.agent_model_defaults.mode", "chosen") + .await + .unwrap()); + assert!(!changes.has_changed().unwrap()); + let after: GlobalConfig = service.get_config(None).await.unwrap(); + assert_eq!(before.last_modified, after.last_modified); + let restarted = restart_test_service(&dir, "model-default-noop").await; + assert_eq!( + restarted + .get_config::(Some("ai.agent_model_defaults.mode")) + .await + .unwrap(), + "chosen" + ); + // Keep the legacy invalid-reference fallback and reject malformed types. + service + .set_config("ai.agent_model_defaults.mode", "removed-model") + .await + .unwrap(); + assert_eq!( + service + .get_config::(Some("ai.agent_model_defaults.mode")) + .await + .unwrap(), + "primary" + ); + assert!(service + .set_config("ai.agent_model_defaults.mode", 123) + .await + .is_err()); + } + fn realtime_voice_fixture() -> VoiceCallConfig { VoiceCallConfig { api_key: "fixture-realtime-voice-key".to_string(), @@ -1137,9 +1200,10 @@ mod tests { ), ]; for (path, value) in &fixtures { - source.set_config(path, value).await.unwrap(); - assert!( + let changed = source.set_config_if_changed(path, value).await.unwrap(); + assert_eq!( changes.has_changed().unwrap(), + changed, "Missing configuration change signal: {path}" ); changes.borrow_and_update(); diff --git a/src/crates/assembly/core/src/service_agent_runtime.rs b/src/crates/assembly/core/src/service_agent_runtime.rs index 8baca8ac1e..8dfd43836e 100644 --- a/src/crates/assembly/core/src/service_agent_runtime.rs +++ b/src/crates/assembly/core/src/service_agent_runtime.rs @@ -2378,10 +2378,25 @@ impl CoreServiceAgentRuntime { .get_session(session_id) .is_none() { - coordinator - .restore_session_for_workspace_binding(&binding, session_id) + let session = coordinator + .update_session_model_selection_from_storage_path( + &binding.session_storage_dir(), + session_id, + &normalized_model_id, + normalized_reasoning_preset + .as_ref() + .and_then(|value| value.as_deref()), + false, + ) .await - .map_err(|e| format!("Failed to restore session: {e}"))?; + .map_err(|error| format!("Failed to update session model: {error}"))?; + if session_uses_shared_mode_default(&session) { + Self::persist_mode_model(&normalized_model_id).await; + } + return Ok(RemoteSessionModelSelection { + model_id: session.config.model_id.unwrap_or(normalized_model_id), + reasoning_preset: session.config.reasoning_preset, + }); } let previous_model_id = coordinator @@ -4199,7 +4214,6 @@ mod tests { assert!(!open_workspace.contains("initialize_snapshot_manager_for_workspace")); for (start, end) in [ - ("pub(crate) async fn update_remote_session_model", "/// Persist the shared selector"), ("async fn restore_remote_session(\n &self,\n session_id: &str,\n workspace: RemoteDialogWorkspaceBinding", "fn prewarm_remote_terminal"), ("async fn ensure_session_loaded(&self, session_id: &str)", "async fn update_session_title"), ("async fn restore_remote_session(\n &self,\n session_id: &str,\n _restore_path_hint: &str", "async fn cancel_remote_turn"), @@ -4219,6 +4233,17 @@ mod tests { ); } + let selector = source + .split("pub(crate) async fn update_remote_session_model") + .nth(1) + .and_then(|source| source.split("/// Persist the shared selector").next()) + .unwrap(); + assert!(selector.contains("ensure_remote_binding_runtime_ownership")); + assert!(selector.contains( + "coordinator\n .update_session_model_selection_from_storage_path" + )); + assert!(!selector.contains("restore_session")); + let remote_session_host = source .split("impl RemoteSessionRuntimeHost for CoreRemoteSessionRuntimeHost") .nth(1) diff --git a/src/crates/services/services-integrations/AGENTS.md b/src/crates/services/services-integrations/AGENTS.md index f57f00b4e4..70ae078199 100644 --- a/src/crates/services/services-integrations/AGENTS.md +++ b/src/crates/services/services-integrations/AGENTS.md @@ -187,3 +187,9 @@ cargo test --locked -p openbitfun-services-integrations --no-default-features -- These loopback protocol fixtures exercise the serving host's HTTP and filesystem behavior; they do not establish real SSH, relay, peer or dispatch support. + +For models.dev snapshot reuse, last-valid recovery, and background refresh: + +```bash +cargo test --locked -p openbitfun-services-integrations --no-default-features --features models-dev --lib models_dev::tests +``` diff --git a/src/crates/services/services-integrations/Cargo.toml b/src/crates/services/services-integrations/Cargo.toml index 4ceca79f62..40cc5d9b02 100644 --- a/src/crates/services/services-integrations/Cargo.toml +++ b/src/crates/services/services-integrations/Cargo.toml @@ -106,6 +106,7 @@ models-dev = [ "reqwest/system-proxy", "sha2", "tokio/fs", + "tokio/rt", "tokio/sync", "tokio/time", "windows", diff --git a/src/crates/services/services-integrations/src/models_dev.rs b/src/crates/services/services-integrations/src/models_dev.rs index 5158bc0589..eb4a65501c 100644 --- a/src/crates/services/services-integrations/src/models_dev.rs +++ b/src/crates/services/services-integrations/src/models_dev.rs @@ -29,6 +29,19 @@ struct RefreshState { last_attempt: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct FileStamp { + length: u64, + modified: SystemTime, +} + +#[derive(Debug)] +struct ValidatedSnapshot { + stamp: Option, + cache_valid: bool, + snapshot: ModelsDevSnapshot, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ModelsDevSnapshotSource { Cache, @@ -74,6 +87,7 @@ pub struct ModelsDevCatalogService { cache_ttl: Duration, refresh_state: Arc>, refresh_in_progress: Arc, + validated_snapshot: Arc>>, } struct RefreshGuard(Arc); @@ -93,6 +107,7 @@ impl ModelsDevCatalogService { cache_ttl: DEFAULT_CACHE_TTL, refresh_state: Arc::new(Mutex::new(RefreshState::default())), refresh_in_progress: Arc::new(AtomicBool::new(false)), + validated_snapshot: Arc::new(Mutex::new(None)), } } @@ -122,9 +137,21 @@ impl ModelsDevCatalogService { /// Load the best immediately available source without contacting the network. pub async fn load_cached_or_bundled(&self) -> ModelsDevSnapshot { + // Share cold reads across consumers. A timestamp controls revalidation, + // never the lifetime of the last valid document. + let mut cached = self.validated_snapshot.lock().await; + let stamp = self.file_stamp().await; + if let Some(cached) = cached.as_ref().filter(|cached| cached.stamp == stamp) { + return cached.snapshot.clone(); + } if let Ok(body) = fs::read_to_string(&self.cache_file).await { - if is_valid_catalog_document(&body) { - return snapshot(body, ModelsDevSnapshotSource::Cache); + if let Some(snapshot) = checked_snapshot(body, ModelsDevSnapshotSource::Cache).await { + *cached = Some(ValidatedSnapshot { + stamp, + cache_valid: true, + snapshot: snapshot.clone(), + }); + return snapshot; } debug!( "Ignoring invalid models.dev cache at {}", @@ -132,14 +159,29 @@ impl ModelsDevCatalogService { ); } - if is_valid_catalog_document(&self.bundled_snapshot) { - return snapshot( + let snapshot = match cached.as_ref() { + Some(cached) => cached.snapshot.clone(), + None => checked_snapshot( self.bundled_snapshot.to_string(), ModelsDevSnapshotSource::Bundled, - ); - } + ) + .await + .unwrap_or_else(|| snapshot("{}".to_string(), ModelsDevSnapshotSource::Empty)), + }; + *cached = Some(ValidatedSnapshot { + stamp, + cache_valid: false, + snapshot: snapshot.clone(), + }); + snapshot + } - snapshot("{}".to_string(), ModelsDevSnapshotSource::Empty) + async fn file_stamp(&self) -> Option { + let metadata = fs::metadata(&self.cache_file).await.ok()?; + Some(FileStamp { + length: metadata.len(), + modified: metadata.modified().ok()?, + }) } pub async fn cache_metadata(&self) -> ModelsDevCacheMetadata { @@ -181,17 +223,10 @@ impl ModelsDevCatalogService { return ModelsDevRefreshOutcome::Throttled; } let _refresh_guard = RefreshGuard(self.refresh_in_progress.clone()); - let body = match &self.runtime_source { + let incoming = match &self.runtime_source { ModelsDevRuntimeSource::LocalFile(path) => { let body = match fs::read_to_string(path).await { - Ok(body) if is_valid_catalog_document(&body) => body, - Ok(_) => { - warn!( - "Local models.dev catalog at {} failed schema validation", - path.display() - ); - return ModelsDevRefreshOutcome::Failed; - } + Ok(body) => body, Err(error) => { warn!( "Failed to read local models.dev catalog at {}: {}", @@ -201,10 +236,18 @@ impl ModelsDevCatalogService { return ModelsDevRefreshOutcome::Failed; } }; - if self.cache_matches(&body).await { + let Some(incoming) = checked_snapshot(body, ModelsDevSnapshotSource::Cache).await + else { + warn!( + "Local models.dev catalog at {} failed schema validation", + path.display() + ); + return ModelsDevRefreshOutcome::Failed; + }; + if self.cache_matches(&incoming.body).await { return ModelsDevRefreshOutcome::NotNeeded; } - body + incoming } ModelsDevRuntimeSource::Http(endpoint_url) => { if endpoint_url.trim().is_empty() { @@ -272,14 +315,21 @@ impl ModelsDevCatalogService { let previous = self.load_cached_or_bundled().await; - match self.write_cache_atomically(&body).await { - Ok(()) if previous.sha256 == sha256_hex(body.as_bytes()) => { - ModelsDevRefreshOutcome::Unchanged { - version: previous.version, - } - } + match self.write_cache_atomically(&incoming.body).await { Ok(()) => { - ModelsDevRefreshOutcome::Updated(snapshot(body, ModelsDevSnapshotSource::Cache)) + let stamp = self.file_stamp().await; + *self.validated_snapshot.lock().await = Some(ValidatedSnapshot { + stamp, + cache_valid: true, + snapshot: incoming.clone(), + }); + if previous.sha256 == incoming.sha256 { + ModelsDevRefreshOutcome::Unchanged { + version: incoming.version, + } + } else { + ModelsDevRefreshOutcome::Updated(incoming) + } } Err(error) => { warn!( @@ -293,19 +343,22 @@ impl ModelsDevCatalogService { } async fn is_cache_fresh(&self) -> bool { - let Ok(metadata) = fs::metadata(&self.cache_file).await else { - return false; - }; - let Ok(modified) = metadata.modified() else { + let Some(stamp) = self.file_stamp().await else { return false; }; let age = SystemTime::now() - .duration_since(modified) + .duration_since(stamp.modified) .unwrap_or_default(); - age < self.cache_ttl - && fs::read_to_string(&self.cache_file) - .await - .is_ok_and(|body| is_valid_catalog_document(&body)) + if age >= self.cache_ttl { + return false; + } + // Only a document that was validated at this exact file revision is fresh. + self.load_cached_or_bundled().await; + self.validated_snapshot + .lock() + .await + .as_ref() + .is_some_and(|cached| cached.cache_valid && cached.stamp.as_ref() == Some(&stamp)) } async fn cache_matches(&self, expected: &str) -> bool { @@ -347,6 +400,18 @@ impl ModelsDevCatalogService { } } +async fn checked_snapshot( + body: String, + source: ModelsDevSnapshotSource, +) -> Option { + tokio::task::spawn_blocking(move || { + is_valid_catalog_document(&body).then(|| snapshot(body, source)) + }) + .await + .ok() + .flatten() +} + fn runtime_source_from_environment() -> ModelsDevRuntimeSource { runtime_source_from_values( std::env::var(MODELS_DEV_PATH_ENV).ok(), @@ -467,7 +532,7 @@ impl std::fmt::Display for RefreshError { async fn fetch_catalog_body( client: &reqwest::Client, endpoint_url: &str, -) -> Result { +) -> Result { let response = client .get(endpoint_url) .send() @@ -478,10 +543,9 @@ async fn fetch_catalog_body( return Err(RefreshError::Status(status)); } let body = response.text().await.map_err(RefreshError::ResponseBody)?; - if !is_valid_catalog_document(&body) { - return Err(RefreshError::InvalidDocument); - } - Ok(body) + checked_snapshot(body, ModelsDevSnapshotSource::Cache) + .await + .ok_or(RefreshError::InvalidDocument) } fn retry_backoff(attempt: usize) -> Duration { @@ -565,6 +629,29 @@ mod tests { assert!(snapshot.body.contains("gpt-test")); } + #[tokio::test] + async fn loaded_snapshot_survives_corruption_and_observes_replacement() { + let directory = tempfile::tempdir().unwrap(); + let file = directory.path().join("catalog.json"); + tokio::fs::write(&file, VALID).await.unwrap(); + let service = ModelsDevCatalogService::new(&file); + let first = service.load_cached_or_bundled().await; + assert!(service.is_cache_fresh().await); + tokio::fs::write(&file, "invalid").await.unwrap(); + assert_eq!(service.load_cached_or_bundled().await, first); + assert!(!service.is_cache_fresh().await); + tokio::fs::remove_file(&file).await.unwrap(); + assert_eq!(service.load_cached_or_bundled().await, first); + tokio::fs::write( + &file, + r#"{"provider":{"models":{"replacement":{"id":"replacement"}}}}"#, + ) + .await + .unwrap(); + assert_ne!(service.load_cached_or_bundled().await.sha256, first.sha256); + assert!(service.is_cache_fresh().await); + } + #[tokio::test] async fn invalid_cache_falls_back_to_bundled_snapshot() { let directory = tempfile::tempdir().expect("temp directory"); diff --git a/src/web-ui/src/flow_chat/components/ModelSelector.tsx b/src/web-ui/src/flow_chat/components/ModelSelector.tsx index e6e15a706b..89ff84fd42 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelector.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelector.tsx @@ -17,19 +17,17 @@ import { useI18n } from '@/infrastructure/i18n'; import { configManager } from '@/infrastructure/config/services/ConfigManager'; import { agentAPI } from '@/infrastructure/api/service-api/AgentAPI'; import { - aiApi, type AIModelCatalog, type ReasoningCatalogProjection, } from '@/infrastructure/api/service-api/AIApi'; import { ACPClientAPI, type AcpSessionOptions } from '@/infrastructure/api/service-api/ACPClientAPI'; import { getProviderDisplayName, getProviderGroupKey } from '@/infrastructure/config/services/modelConfigs'; -import { globalEventBus } from '@/infrastructure/event-bus'; import type { AIModelConfig, AgentModelDefaultsConfig, DefaultModelsConfig } from '@/infrastructure/config/types'; import { Tooltip, Icon } from '@openbitfun/ui'; import { RetainedMountBoundary } from '@/shared/presence'; import { notificationService } from '@/shared/notification-system'; import { FlowChatStore } from '../store/FlowChatStore'; -import { getModelMaxTokens } from '../services/flow-chat-manager/SessionModule'; +import { modelCatalogStore } from '@/infrastructure/config/services/ModelCatalogStore'; import { acpClientIdFromAgentType } from '../utils/acpSession'; import { buildAcpFastModeValue, @@ -67,6 +65,7 @@ import { import { resolveModelReference, resolveModelSelection, + resolveModelMaxTokens, type ModelAvailabilityStatus, } from '../utils/modelResolution'; import { getRecentManualModel, setRecentManualModel } from '../utils/manualModelSelection'; @@ -75,6 +74,8 @@ import './ModelSelector.scss'; const log = createLogger('ModelSelector'); const ACP_SESSION_OPTIONS_TIMEOUT_MS = 65_000; const MODEL_SELECTOR_EXIT_DURATION_MS = 100; +const EMPTY_MODELS: AIModelConfig[] = []; +const EMPTY_DEFAULT_MODELS: DefaultModelsConfig = {}; export interface ExternalModelSelection { models: string[]; @@ -161,7 +162,7 @@ interface ProviderGroupInfo { models: ModelInfo[]; } -type ModelSelectorPanel = 'modes' | 'models'; +type ModelSelectorPanel = 'modes' | 'models' | 'providers'; type UpcomingModelMode = 'smart' | 'pool'; type ModelSelectorLevelDirection = 'none' | 'forward' | 'back'; @@ -337,23 +338,29 @@ export const ModelSelector: React.FC = ({ const modelSelectionScopeRef = useRef(modelSelectionScopeKey); modelSelectionScopeRef.current = modelSelectionScopeKey; const manualModelRef = useRef<{ scopeKey: string; modelId: string } | null>(null); - const [allModels, setAllModels] = useState([]); - const [modelCatalog, setModelCatalog] = useState(null); - const [defaultModels, setDefaultModels] = useState({}); - const [modeModel, setModeModel] = useState('primary'); - const [configLoadState, setConfigLoadState] = useState<'loading' | 'ready' | 'error'>('loading'); - const [catalogLoadState, setCatalogLoadState] = useState<'loading' | 'ready' | 'error'>('loading'); + const [configEpoch, setConfigEpoch] = useState(deviceSurfaceScope.epoch); + const [loadedModels, setAllModels] = useState([]); + const allModels = configEpoch === deviceSurfaceScope.epoch ? loadedModels : EMPTY_MODELS; + const { catalog: modelCatalog, status: catalogLoadState } = useSyncExternalStore( + modelCatalogStore.subscribe, modelCatalogStore.getSnapshot, modelCatalogStore.getSnapshot, + ); + const [loadedDefaults, setDefaultModels] = useState({}); + const defaultModels = configEpoch === deviceSurfaceScope.epoch ? loadedDefaults : EMPTY_DEFAULT_MODELS; + const [loadedModeModel, setModeModel] = useState('primary'); + const modeModel = configEpoch === deviceSurfaceScope.epoch ? loadedModeModel : 'primary'; + const [configState, setConfigLoadState] = useState<'loading' | 'ready' | 'error'>('loading'); + const configLoadState = configEpoch === deviceSurfaceScope.epoch ? configState : 'loading'; const [acpOptions, setAcpOptions] = useState(null); const [acpOptionsLoadState, setAcpOptionsLoadState] = useState<'loading' | 'ready' | 'error'>('loading'); const [dropdownOpen, setDropdownOpen] = useState(false); const [keyboardNavigationOpen, setKeyboardNavigationOpen] = useState(false); /** Provider whose models the menu is currently showing; null is the provider level. */ const [activeProviderKey, setActiveProviderKey] = useState(null); - /** Models use a detail view; modes expand horizontally inside the summary card. */ + /** Providers and models use detail views; modes expand inside the summary card. */ const [activePanel, setActivePanel] = useState(null); const [modeSelectionPending, setModeSelectionPending] = useState(false); const [modeFocusRevision, setModeFocusRevision] = useState(0); - const [providerFromSummary, setProviderFromSummary] = useState(false); + const [modelsFromSummary, setModelsFromSummary] = useState(false); // Upcoming modes execute through primary until the runtime supports routing. const [upcomingMode, setUpcomingMode] = useState<{ id: UpcomingModelMode; @@ -369,8 +376,9 @@ export const ModelSelector: React.FC = ({ } | null>(null); const acpRestoreToastShownRef = useRef(null); const acpOptionsRef = useRef(null); + const configEpochRef = useRef(configEpoch); + configEpochRef.current = configEpoch; const configLoadRequestRef = useRef(0); - const catalogLoadRequestRef = useRef(0); const acpOptionsLoadRequestRef = useRef(0); const dropdownRef = useRef(null); @@ -382,10 +390,19 @@ export const ModelSelector: React.FC = ({ const modeRetryFocusRef = useRef(undefined); const providerFieldRef = useRef(null); const focusPanelOnOpenRef = useRef(false); - const focusSummaryOnReturnRef = useRef(null); + const focusSummaryOnReturnRef = useRef(null); const triggerRef = useRef(null); const menuId = useId(); + useEffect(() => { + setLoading(false); + setReasoningLoading(false); + setUpcomingMode(null); + setPreSessionReasoningSelection(null); + setDropdownOpen(false); + setActivePanel(null); + }, [deviceSurfaceScope.epoch]); + useEffect(() => { onLoadingChange?.(loading || reasoningLoading); }, [loading, onLoadingChange, reasoningLoading]); @@ -432,28 +449,10 @@ export const ModelSelector: React.FC = ({ const isAcpSession = Boolean(acpClientId && sessionId); const targetIsSubagent = isSubagentSession || activeSession?.sessionKind === 'subagent'; - const loadModelCatalog = useCallback(async () => { - const requestId = ++catalogLoadRequestRef.current; - setCatalogLoadState('loading'); - try { - const catalog = await aiApi.getModelCatalog(); - if (requestId !== catalogLoadRequestRef.current) return false; - setModelCatalog(catalog); - setCatalogLoadState('ready'); - return true; - } catch (error) { - if (requestId !== catalogLoadRequestRef.current) return false; - setModelCatalog(null); - setCatalogLoadState('error'); - log.warn('Failed to load AI model catalog', { error }); - return false; - } - }, []); - // Load configuration data. const loadConfigData = useCallback(async () => { const requestId = ++configLoadRequestRef.current; - setConfigLoadState('loading'); + const scope = getActiveSurfaceScope(); try { const configData = await configManager.getConfigs([ 'ai.models', @@ -466,53 +465,40 @@ export const ModelSelector: React.FC = ({ const defaultModelsData = (configData['ai.default_models'] as DefaultModelsConfig | undefined) || {}; const agentModelDefaults = configData['ai.agent_model_defaults'] as AgentModelDefaultsConfig | undefined; - if (requestId !== configLoadRequestRef.current) return false; + if (requestId !== configLoadRequestRef.current || !scope.isCurrent()) return false; + setConfigEpoch(scope.epoch); setAllModels(models); setDefaultModels(defaultModelsData); setModeModel(agentModelDefaults?.mode?.trim() || 'primary'); setConfigLoadState('ready'); - await loadModelCatalog(); - if (requestId !== configLoadRequestRef.current) return false; + if (requestId !== configLoadRequestRef.current || !scope.isCurrent()) return false; log.debug('Configuration loaded', { modelsCount: models.length }); return true; } catch (error) { - if (requestId !== configLoadRequestRef.current) return false; + if (requestId !== configLoadRequestRef.current || !scope.isCurrent()) return false; + if (configEpochRef.current !== scope.epoch) { + setAllModels([]); setDefaultModels({}); setModeModel('primary'); + } + setConfigEpoch(scope.epoch); setConfigLoadState('error'); log.error('Failed to load configuration', error); return false; } - }, [loadModelCatalog]); + }, []); useEffect(() => { - const unsubscribeCatalog = aiApi.onModelCatalogUpdated(() => { - void loadModelCatalog(); - }); void loadConfigData(); - - const handleConfigUpdate = () => { - log.debug('Configuration update detected, reloading'); - void loadConfigData(); - }; - - globalEventBus.on('mode:config:updated', handleConfigUpdate); - const unsubscribe = configManager.onConfigChange((path) => { - if (path === 'ai' || path.startsWith('ai.')) { - log.debug('AI configuration changed', { path }); - void loadConfigData(); - } + if (!path || path === 'ai' || ['ai.models', 'ai.default_models', 'ai.agent_model_defaults'].some( + owned => path === owned || path.startsWith(`${owned}.`), + )) void loadConfigData(); }); - - return () => { - globalEventBus.off('mode:config:updated', handleConfigUpdate); - unsubscribe(); - unsubscribeCatalog(); - }; - }, [loadConfigData, loadModelCatalog]); + return () => { configLoadRequestRef.current += 1; unsubscribe(); }; + }, [loadConfigData, deviceSurfaceScope.epoch]); const loadAcpOptions = useCallback(async () => { const requestId = ++acpOptionsLoadRequestRef.current; @@ -1009,7 +995,7 @@ export const ModelSelector: React.FC = ({ focusSummaryOnReturnRef.current = null; focusPanelOnOpenRef.current = false; setActiveProviderKey(null); - setProviderFromSummary(false); + setModelsFromSummary(false); setActivePanel(null); setLevelDirection('none'); }, []); @@ -1019,7 +1005,7 @@ export const ModelSelector: React.FC = ({ focusPanelOnOpenRef.current = moveFocus; focusSummaryOnReturnRef.current = null; setActiveProviderKey(null); - setProviderFromSummary(false); + setModelsFromSummary(false); setLevelDirection('forward'); if (activePanel === kind && moveFocus && !activeProviderKey) { focusPanelOnOpenRef.current = false; @@ -1032,7 +1018,7 @@ export const ModelSelector: React.FC = ({ focusSummaryOnReturnRef.current = restoreFocus ? activePanel : null; focusPanelOnOpenRef.current = false; setActiveProviderKey(null); - setProviderFromSummary(false); + setModelsFromSummary(false); setActivePanel(null); setLevelDirection('back'); }, [activePanel]); @@ -1043,25 +1029,24 @@ export const ModelSelector: React.FC = ({ const openProviderLevel = useCallback((providerKey: string, fromSummary = false) => { setActiveProviderKey(providerKey); - setProviderFromSummary(fromSummary); + setModelsFromSummary(fromSummary); setLevelDirection('forward'); }, []); const closeProviderLevel = useCallback(() => { - if (providerFromSummary) { + if (modelsFromSummary) { closePanel(true); - focusSummaryOnReturnRef.current = 'provider'; return; } setActiveProviderKey(null); setLevelDirection('back'); - }, [closePanel, providerFromSummary]); + }, [closePanel, modelsFromSummary]); // Keep the last view intact during exit; trigger handlers reset it on reopening. // Catalog refreshes must not leave an open menu on a missing provider view. useEffect(() => { if (!dropdownOpen) return; - if (activePanel !== 'models' && activeProviderKey) { + if (activePanel !== 'models' && activePanel !== 'providers' && activeProviderKey) { setActiveProviderKey(null); return; } @@ -1072,13 +1057,16 @@ export const ModelSelector: React.FC = ({ const currentNativeModelId = getCurrentModelId(); const concreteModelId = resolveConcreteModelId(currentNativeModelId, defaultModels); - /** Provider that owns the pinned model, so the provider level can mark it. */ + const currentModelId = externalSelection ? externalCurrentModelId + : isAcpSession ? acpOptions?.currentModelId || acpAvailableModels[0]?.id || '' + : currentNativeModelId; + /** Resolve symbolic defaults and target-owned models to their provider. */ const selectedProviderKey = useMemo((): string | null => { - if (isSpecialModel(currentNativeModelId)) return null; + const modelId = externalSelection || isAcpSession ? currentModelId : concreteModelId; return providerGroups.find( - group => group.models.some(model => model.id === currentNativeModelId), + group => group.models.some(model => model.id === modelId), )?.key ?? null; - }, [currentNativeModelId, providerGroups]); + }, [concreteModelId, currentModelId, externalSelection, isAcpSession, providerGroups]); const currentReasoningProjection = useMemo((): ReasoningCatalogProjection | null => { if (!concreteModelId) return null; return modelCatalog?.models.find(model => model.id === concreteModelId)?.reasoning ?? null; @@ -1245,12 +1233,14 @@ export const ModelSelector: React.FC = ({ const previousReasoningPreset = sessionId ? store.getState().sessions.get(sessionId)?.config.reasoningPreset : undefined; + const previousMaxContextTokens = sessionId ? store.getState().sessions.get(sessionId)?.maxContextTokens : undefined; const nextReasoningPreset = mode ? undefined : recentPresetForModel(modelId); let sessionModelWrittenOptimistically = false; try { if (externalSelection) { await externalSelection.onSelect(modelId); + selectionScope.assertCurrent('select external model'); return true; } if (isAcpSession && acpClientId && sessionId) { @@ -1263,6 +1253,7 @@ export const ModelSelector: React.FC = ({ remoteSshHost: activeSession?.remoteSshHost, modelId, }); + selectionScope.assertCurrent('select ACP model'); setAcpOptions(options); syncAcpContextUsageToStore(sessionId, options); store.updateSessionModelName(sessionId, modelId); @@ -1272,13 +1263,14 @@ export const ModelSelector: React.FC = ({ const updateTargetSessionModel = async () => { if (!sessionId) return; + if (previousSessionModelName === modelId && previousReasoningPreset === nextReasoningPreset) return; // Update the frontend session model immediately so the UI reflects the // switch without waiting for the backend IPC round-trip. store.updateSessionModelName(sessionId, modelId); store.updateSessionReasoningPreset(sessionId, nextReasoningPreset); sessionModelWrittenOptimistically = true; - const maxContextTokens = await getModelMaxTokens(modelId, currentMode); + const maxContextTokens = resolveModelMaxTokens(modelId, allModels, defaultModels); selectionScope.assertCurrent('select model'); store.updateSessionMaxContextTokens(sessionId, maxContextTokens); const session = store.getState().sessions.get(sessionId); @@ -1305,13 +1297,26 @@ export const ModelSelector: React.FC = ({ return true; } - if (persistSharedModeDefault) { - await configManager.setConfig('ai.agent_model_defaults.mode', modelId); - selectionScope.assertCurrent('select model'); + await updateTargetSessionModel(); + if (persistSharedModeDefault && modeModel !== modelId) { + const saveDefault = configManager.setConfig('ai.agent_model_defaults.mode', modelId); + if (!sessionId) { + // A selector without a draft/session has no other owner for the next + // submission. Its preference is the selection commit itself. + await saveDefault; + selectionScope.assertCurrent('select initial model'); + } else { + // ConfigManager captures the device and serializes related writes. + // Remembering a future default must not hold the current session UI. + void saveDefault.catch(error => { + if (!selectionScope.isCurrent()) return; + log.warn('Session model selected but default preference was not saved', { error }); + notificationService.warning(t('modelSelector.defaultSaveFailed')); + void loadConfigData(); + }); + } setModeModel(modelId); - globalEventBus.emit('mode:config:updated'); } - await updateTargetSessionModel(); rememberManualModel(); setUpcomingMode(mode ? { id: mode, scopeKey: modelSelectionScopeKey } : null); setPreSessionReasoningSelection({ @@ -1327,20 +1332,22 @@ export const ModelSelector: React.FC = ({ } catch (error) { if (!selectionScope.isCurrent()) return false; log.error('Failed to switch model', error); - // Only a previously pinned selection can be restored: the store has no - // way to express "never pinned" without claiming a session binding. - if (sessionId && sessionModelWrittenOptimistically && previousSessionModelName) { + if (sessionId && sessionModelWrittenOptimistically) { store.updateSessionModelName(sessionId, previousSessionModelName); } if (sessionId && sessionModelWrittenOptimistically) { store.updateSessionReasoningPreset(sessionId, previousReasoningPreset); + store.updateSessionMaxContextTokens(sessionId, previousMaxContextTokens); } notificationService.error(t('modelSelector.switchFailed')); return false; } finally { - setLoading(false); + if (selectionScope.isCurrent()) setLoading(false); } }, [ + allModels, + modeModel, + loadConfigData, activeSession?.config.workspacePath, activeSession?.remoteConnectionId, activeSession?.remoteSshHost, @@ -1411,6 +1418,7 @@ export const ModelSelector: React.FC = ({ const previousPreset = session.config.reasoningPreset; if (previousPreset === normalizedPreset) return true; + const selectionScope = getActiveSurfaceScope(); setReasoningLoading(true); store.updateSessionReasoningPreset(sessionId, normalizedPreset); try { @@ -1426,6 +1434,7 @@ export const ModelSelector: React.FC = ({ includeInternal: shouldIncludeInternalModelSession(session), }); } + selectionScope.assertCurrent('select reasoning preset'); if (!targetIsSubagent) { setRecentReasoningPreset(concreteModelId, normalizedPreset); setPreSessionReasoningSelection({ @@ -1440,12 +1449,13 @@ export const ModelSelector: React.FC = ({ }); return true; } catch (error) { + if (!selectionScope.isCurrent()) return false; store.updateSessionReasoningPreset(sessionId, previousPreset); log.error('Failed to update session reasoning preset', error); notificationService.error(t('reasoningSelector.updateFailed')); return false; } finally { - setReasoningLoading(false); + if (selectionScope.isCurrent()) setReasoningLoading(false); } }, [ concreteModelId, @@ -1727,7 +1737,7 @@ export const ModelSelector: React.FC = ({ if (event.key === 'ArrowRight') { const focusedElement = document.activeElement as HTMLElement | null; const focusedTarget = focusedElement?.dataset?.modelMenuTarget; - if (focusedTarget === 'modes' || focusedTarget === 'models') { + if (focusedTarget === 'modes' || focusedTarget === 'models' || focusedTarget === 'providers') { event.preventDefault(); openPanel(focusedTarget, true); } @@ -1752,7 +1762,7 @@ export const ModelSelector: React.FC = ({ return; } - if (event.key === 'ArrowRight' && activePanel === 'models' && !activeProviderKey) { + if (event.key === 'ArrowRight' && !activeProviderKey) { const focusedElement = document.activeElement as HTMLElement | null; const focusedProviderKey = focusedElement?.dataset?.providerKey; if (focusedProviderKey) { @@ -1801,14 +1811,14 @@ export const ModelSelector: React.FC = ({ const returnTarget = focusSummaryOnReturnRef.current; focusSummaryOnReturnRef.current = null; if (returnTarget) { - const field = returnTarget === 'provider' ? providerFieldRef.current + const field = returnTarget === 'providers' ? providerFieldRef.current : returnTarget === 'modes' ? modeFieldRef.current : modelFieldRef.current; (field ?? modeFieldRef.current ?? modelFieldRef.current)?.focus({ preventScroll: true }); } return; } if (activePanel === 'modes' && (modeSelectionPending || loading || reasoningLoading)) return; - const providerLevelChanged = activePanel === 'models' + const providerLevelChanged = (activePanel === 'models' || activePanel === 'providers') && previousProviderKey !== activeProviderKey; if (!focusPanelOnOpenRef.current && !providerLevelChanged) return; focusPanelOnOpenRef.current = false; @@ -1827,7 +1837,7 @@ export const ModelSelector: React.FC = ({ return; } - if (activePanel === 'models' && previousProviderKey) { + if (previousProviderKey) { const providerRows = Array.from( menu.querySelectorAll('button[data-provider-key]'), ); @@ -1858,9 +1868,6 @@ export const ModelSelector: React.FC = ({ const resolvedContextUsageSource: ContextUsageSource = contextUsageSource ?? (isAcpSession ? 'acp_context' : 'agent_prompt'); - const currentModelId = externalSelection ? externalCurrentModelId - : isAcpSession ? acpOptions?.currentModelId || acpAvailableModels[0]?.id || '' - : currentNativeModelId; const displayedModel = externalSelection ? externalCurrentModel : isAcpSession ? acpCurrentModel ?? acpAvailableModels[0] ?? null : currentModel; const isAcpSelection = isAcpSession && !externalSelection; @@ -1871,7 +1878,7 @@ export const ModelSelector: React.FC = ({ const displayedAvailability = externalSelection ? externalAvailability : isAcpSession ? acpAvailability : nativeAvailability; const showModelLoadingState = modelInformationLoading && ( - isNativeModelSelection ? !modelCatalog : isAcpSelection ? !acpOptions : !displayedModel + isNativeModelSelection ? configLoadState === 'loading' || !modelCatalog : isAcpSelection ? !acpOptions : !displayedModel ); useEffect(() => { // An ACP response may hand model ownership entirely to its separate mode control. @@ -1890,14 +1897,13 @@ export const ModelSelector: React.FC = ({ )); const manualModelLabel = manualModelConfig?.model_name || getAvailabilityLabel(rememberedManualModelId ? 'target-model-unavailable' : nativeAvailability.status, t); - const displayedProviderGroup = providerGroups.find(group => group.models.some(model => ( - model.id === (isNativeModelSelection ? concreteModelId : currentModelId) - ))); - const openCurrentProvider = () => { - if (!displayedProviderGroup || disabled || loading || externalSelection?.disabled) return; + const displayedProviderGroup = providerGroups.find(group => group.key === selectedProviderKey); + const openCurrentProviderModels = () => { + if (disabled || loading || externalSelection?.disabled) return; openPanel('models', true); - openProviderLevel(displayedProviderGroup.key, true); + if (displayedProviderGroup) openProviderLevel(displayedProviderGroup.key, true); }; + const isSelectionPanel = activePanel === 'models' || activePanel === 'providers'; const acpModeLabel = acpMode?.option.options.find(option => option.value === acpMode.currentValue)?.name ?? acpMode?.currentValue ?? ''; const targetTooltip = buildContextUsageTooltip({ @@ -2055,16 +2061,16 @@ export const ModelSelector: React.FC = ({ data-keyboard-open={keyboardNavigationOpen ? 'true' : 'false'} data-placement={resolvedDropdownPlacement} data-open={dropdownOpen ? 'true' : 'false'} - data-menu-level={activeProviderGroup ? 'provider' : activePanel === 'models' ? 'models' : 'settings'} + data-menu-level={activeProviderGroup ? 'provider' : isSelectionPanel ? activePanel : 'settings'} data-model-mode={currentModelMode ?? 'manual'} aria-busy={modelInformationLoading || undefined} aria-hidden={!dropdownOpen} {...(!dropdownOpen ? { inert: '' } : {})} aria-label={activeProviderGroup?.providerName ?? (activePanel === 'modes' ? t('modelSelector.modes.selection') : t('modelSelector.modelSettings'))} - onKeyDown={activePanel === 'models' ? handlePanelKeyDown : handleDropdownKeyDown} + onKeyDown={isSelectionPanel ? handlePanelKeyDown : handleDropdownKeyDown} > - {!showModelLoadingState && activePanel !== 'models' && !activeProviderGroup + {!showModelLoadingState && !isSelectionPanel && !activeProviderGroup && (currentModelMode === 'smart' || currentModelMode === 'pool') && ( )} @@ -2078,7 +2084,7 @@ export const ModelSelector: React.FC = ({ > {t('modelSelector.status.loading')}
- ) : activePanel === 'models' ? ( + ) : isSelectionPanel ? (
= ({ className="openbitfun-model-selector__selection-back" data-testid="chat-model-selector-summary-back" aria-label={t('modelSelector.backToSettings')} - aria-describedby={activePanel === 'models' ? `${menuId}-current-selection` : undefined} + aria-describedby={`${menuId}-current-selection`} leading={} - metadata={activePanel === 'models' ? ( + metadata={( = ({ {settingsModelLabel} - ) : undefined} + )} onClick={() => closePanel(true)} > - {t('modelSelector.model')} + {activePanel === 'providers' + ? displayedModel?.providerName || t('modelSelector.modelSettings') + : t('modelSelector.model')} )} - {(externalSelection || isAcpSession) && !activeProviderGroup ? ( + {(externalSelection || isAcpSession) && activePanel === 'models' && !activeProviderGroup ? ( <> {showModelChoices && ( = ({ data-testid="chat-model-selector-back" data-openbitfun-component="model-selector" data-openbitfun-part="back" - aria-label={t(providerFromSummary ? 'modelSelector.backToSettings' : 'modelSelector.backToProviders')} + aria-label={t(modelsFromSummary ? 'modelSelector.backToSettings' : 'modelSelector.backToProviders')} leading={} onClick={closeProviderLevel} > @@ -2179,77 +2187,10 @@ export const ModelSelector: React.FC = ({ ); })} - ) : availableModels.length === 0 ? ( - <>{renderUnavailableModelMenu(nativeAvailability, true)} + ) : providerGroups.length === 0 ? ( + <>{renderUnavailableModelMenu(displayedAvailability, isNativeModelSelection)} ) : ( <> - {(() => { - const primaryModel = allModels.find( - m => m.id === defaultModels.primary && isSelectableTextChatModel(m), - ); - const primaryTooltip = primaryModel - ? buildResolvedModelTooltipText(primaryModel.model_name, { - providerName: getProviderDisplayName(primaryModel), - contextWindow: primaryModel.context_window - }, t('modelSelector.primaryModelDesc')) - : t('modelSelector.primaryModelDesc'); - return ( - - : null} - disabled={!primaryModel} - onClick={() => handleSelectModelFromMenu('primary')} - > - {t('modelSelector.primaryModel')} - - - ); - })()} - - {(() => { - const fastModel = allModels.find( - m => m.id === defaultModels.fast && isSelectableTextChatModel(m), - ) ?? allModels.find( - m => m.id === defaultModels.primary && isSelectableTextChatModel(m), - ); - const fastTooltip = fastModel - ? buildResolvedModelTooltipText(fastModel.model_name, { - providerName: getProviderDisplayName(fastModel), - contextWindow: fastModel.context_window - }, t('modelSelector.fastModelDesc')) - : t('modelSelector.fastModelDesc'); - return ( - - : null} - onClick={() => handleSelectModelFromMenu('fast')} - > - {t('modelSelector.fastModel')} - - - ); - })()} - - - {providerGroups.map(group => { const isSelected = selectedProviderKey === group.key; @@ -2355,8 +2296,13 @@ export const ModelSelector: React.FC = ({ data-testid="chat-model-selector-settings-model" data-model-menu-target="models" aria-label={`${t('modelSelector.model')}: ${settingsModelLabel}`} - onClick={() => openPanelFromClick('models')} - onKeyDown={(event) => handlePanelTriggerKeyDown('models', event)} + onClick={openCurrentProviderModels} + onKeyDown={event => { + if (event.key !== 'ArrowRight') return; + event.preventDefault(); + event.stopPropagation(); + openCurrentProviderModels(); + }} disabled={disabled || loading || externalSelection?.disabled} > @@ -2397,15 +2343,11 @@ export const ModelSelector: React.FC = ({ className="openbitfun-model-selector__provider-control" labelBehavior="static" data-testid="chat-model-selector-settings-provider" + data-model-menu-target="providers" data-provider-key={displayedProviderGroup?.key} - disabled={disabled || loading || externalSelection?.disabled || !displayedProviderGroup} - onClick={openCurrentProvider} - onKeyDown={event => { - if (event.key !== 'ArrowRight') return; - event.preventDefault(); - event.stopPropagation(); - openCurrentProvider(); - }} + disabled={disabled || loading || externalSelection?.disabled || providerGroups.length === 0} + onClick={() => openPanelFromClick('providers')} + onKeyDown={event => handlePanelTriggerKeyDown('providers', event)} > {displayedModel.providerName} diff --git a/src/web-ui/src/flow_chat/components/ModelSelectorAcpMode.test.tsx b/src/web-ui/src/flow_chat/components/ModelSelectorAcpMode.test.tsx index 1f1ecd0d4e..625c8aa7fe 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelectorAcpMode.test.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelectorAcpMode.test.tsx @@ -8,6 +8,7 @@ * both, and the agent that has fixed the mode and left exactly one choice. */ +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -108,6 +109,7 @@ describe('ModelSelector ACP mode picker', () => { let root: Root; beforeEach(() => { + activateSurface('local'); flowChatStoreMocks.sessions.clear(); flowChatStoreMocks.sessions.set('acp-session', { workspacePath: '/tmp/project', @@ -375,7 +377,7 @@ describe('ModelSelector ACP mode picker', () => { await act(async () => { button?.click(); }); }; - it('opens only the current ACP provider and switches models through the remote adapter', async () => { + it.each(['model', 'provider'])('selects ACP models through the %s entry and its transport', async entry => { Object.assign(flowChatStoreMocks.sessions.get('acp-session')!, { remoteConnectionId: 'ssh-connection', remoteSshHost: 'a100', }); @@ -389,7 +391,17 @@ describe('ModelSelector ACP mode picker', () => { }; await renderWithOptions([reasoningOption], sessionOptions); await click('chat-model-selector-btn'); - await click('chat-model-selector-settings-provider'); + await click(`chat-model-selector-settings-${entry}`); + if (entry === 'provider') { + const providers = document.body.querySelectorAll('[data-testid="chat-model-selector-provider"]'); + expect(providers).toHaveLength(2); + expect(document.body.querySelector('[data-testid="chat-model-selector-option"]')).toBeNull(); + const currentProvider = document.body.querySelector( + '[data-testid="chat-model-selector-provider"][data-provider-key="acp:Acme"]', + )!; + expect(currentProvider.getAttribute('data-selected')).toBe('true'); + await act(async () => currentProvider.click()); + } expect(document.body.querySelector('[data-testid="chat-model-selector-provider"]')).toBeNull(); expect(document.body.querySelector('[data-model-id="other-model"]')).toBeNull(); @@ -420,7 +432,7 @@ describe('ModelSelector ACP mode picker', () => { expect(document.body.querySelector('[data-testid="chat-model-selector-settings-model"]')).not.toBeNull(); await click('chat-model-selector-settings-model'); expect(document.body.querySelectorAll('[data-testid="chat-model-selector-option"]')).toHaveLength(1); - await click('chat-model-selector-summary-back'); + await click('chat-model-selector-back'); await click('chat-model-selector-settings-reasoning'); expect(document.body.querySelector('[data-openbitfun-value="preset:auto"]')).toBeNull(); vi.mocked(ACPClientAPI.setSessionConfigOption).mockResolvedValue({ diff --git a/src/web-ui/src/flow_chat/components/ModelSelectorExternal.test.tsx b/src/web-ui/src/flow_chat/components/ModelSelectorExternal.test.tsx index 056259ddec..19b1769c4b 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelectorExternal.test.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelectorExternal.test.tsx @@ -2,6 +2,7 @@ * @vitest-environment jsdom */ +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -143,6 +144,7 @@ describe('ModelSelector external transport reuse', () => { let catalogUpdated: (() => void) | undefined; beforeEach(() => { + activateSurface('local'); catalogUpdated = undefined; flowChatStoreMocks.sessions.clear(); flowChatStoreMocks.subscribers.clear(); @@ -311,11 +313,6 @@ describe('ModelSelector external transport reuse', () => { '[data-testid="chat-model-selector-settings-model"]', )?.click(); }); - await act(async () => { - document.body.querySelector( - '[data-testid="chat-model-selector-provider"][data-provider-key="provider-shared"]', - )?.click(); - }); await act(async () => { document.body.querySelector( '[data-testid="chat-model-selector-option"][data-model-id="model-b"]', @@ -739,11 +736,21 @@ describe('ModelSelector external transport reuse', () => { await act(async () => { control?.click(); }); }; - it('opens only the current target provider and applies choices through its transport', async () => { + it.each(['model', 'provider'])('selects target models through the %s entry and its transport', async entry => { const selection = targetSelection({ models: ['model-a', 'remote-model', 'remote-plain'] }); await renderTarget(selection); await clickControl('chat-model-selector-btn'); - await clickControl('chat-model-selector-settings-provider'); + await clickControl(`chat-model-selector-settings-${entry}`); + if (entry === 'provider') { + const providers = document.body.querySelectorAll('[data-testid="chat-model-selector-provider"]'); + expect(providers).toHaveLength(2); + expect(document.body.querySelector('[data-testid="chat-model-selector-option"]')).toBeNull(); + const currentProvider = document.body.querySelector( + '[data-testid="chat-model-selector-provider"][data-provider-key="external:a100"]', + )!; + expect(currentProvider.getAttribute('data-selected')).toBe('true'); + await act(async () => currentProvider.click()); + } expect(document.body.querySelector('[data-testid="chat-model-selector-provider"]')).toBeNull(); expect(document.body.querySelector('[data-model-id="model-a"]')).toBeNull(); diff --git a/src/web-ui/src/flow_chat/components/ModelSelectorProviderLevels.test.tsx b/src/web-ui/src/flow_chat/components/ModelSelectorProviderLevels.test.tsx index 68f72bdad1..d08cbfff74 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelectorProviderLevels.test.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelectorProviderLevels.test.tsx @@ -2,6 +2,7 @@ * @vitest-environment jsdom */ +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; import { act, cloneElement } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -147,11 +148,13 @@ describe('ModelSelector provider levels', () => { const openMenu = async () => { await openSettingsMenu(); - const settingsModel = document.body.querySelector( + const entry = document.body.querySelector( + '[data-testid="chat-model-selector-settings-provider"]:not(:disabled)', + ) ?? document.body.querySelector( '[data-testid="chat-model-selector-settings-model"]', ); - if (settingsModel) { - await act(async () => settingsModel.click()); + if (entry) { + await act(async () => entry.click()); } }; @@ -210,6 +213,11 @@ describe('ModelSelector provider levels', () => { }; beforeEach(() => { + activateSurface('local'); + vi.mocked(configManager.getConfigs).mockReset(); + vi.mocked(configManager.setConfig).mockReset().mockResolvedValue(undefined); + aiApiMocks.getModelCatalog.mockReset(); + vi.mocked(agentAPI.updateSessionModel).mockReset().mockResolvedValue(undefined); notificationStore.clearActiveNotifications(); notificationStore.clearHistory(); flowChatStoreMocks.sessions.clear(); @@ -338,7 +346,7 @@ describe('ModelSelector provider levels', () => { })); await act(async () => { aiApiMocks.onModelCatalogUpdated.mock.calls[0][0](); }); - expect(container.querySelector('[data-testid="chat-model-selector-btn"]')?.getAttribute('aria-busy')).toBe('true'); + expect(container.querySelector('[data-testid="chat-model-selector-btn"]')?.hasAttribute('aria-busy')).toBe(false); expect(document.body.querySelector('[data-testid="chat-model-selector-loading"]')).toBeNull(); expect(document.body.querySelector('[data-testid="chat-model-selector-settings-model"]')).toBe(modelField); expect(modelField?.textContent).toContain('acme-fast-native'); @@ -418,40 +426,41 @@ describe('ModelSelector provider levels', () => { }); - it('opens the resolved primary provider directly and returns focus to its shortcut', async () => { - await renderSelector(); + it.each(['primary', 'fast', 'acme-deep'])('opens the current provider models from the model entry for %s', async modelId => { + await renderSelector(CATALOG_MODELS, modelId); await openSettingsMenu(); - const providerControl = () => document.body.querySelector( - '[data-testid="chat-model-selector-settings-provider"]', + const modelControl = () => document.body.querySelector( + '[data-testid="chat-model-selector-settings-model"]', )!; - expect(providerControl().textContent).toBe('Acme'); - await act(async () => providerControl().click()); + const isUmbra = modelId === 'fast'; + const selectedModelId = modelId === 'primary' ? 'acme-fast' : isUmbra ? 'umbra-main' : modelId; + await act(async () => modelControl().click()); expect(document.body.querySelector('[data-testid="chat-model-selector-menu"]') ?.getAttribute('data-menu-level')).toBe('provider'); expect(providerRows()).toHaveLength(0); - expect(modelOption('acme-fast')?.getAttribute('aria-checked')).toBe('true'); - expect(document.activeElement).toBe(modelOption('acme-fast')); - expect(modelOption('acme-deep')).not.toBeNull(); - expect(modelOption('umbra-main')).toBeNull(); + expect(modelOption(selectedModelId)?.getAttribute('aria-checked')).toBe('true'); + expect(document.activeElement).toBe(modelOption(selectedModelId)); + expect(Boolean(modelOption('acme-deep'))).toBe(!isUmbra); + expect(Boolean(modelOption('umbra-main'))).toBe(isUmbra); expect(modelOption('primary')).toBeNull(); await act(async () => document.body.querySelector( '[data-testid="chat-model-selector-back"]', )!.click()); expect(optionsPanel()).toBeNull(); - expect(document.activeElement).toBe(providerControl()); + expect(document.activeElement).toBe(modelControl()); - await act(async () => providerControl().dispatchEvent( + await act(async () => modelControl().dispatchEvent( new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true }), )); - expect(modelOption('acme-deep')).not.toBeNull(); + expect(Boolean(modelOption('acme-deep'))).toBe(!isUmbra); await act(async () => optionsPanel()!.dispatchEvent( new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), )); expect(optionsPanel()).toBeNull(); - expect(document.activeElement).toBe(providerControl()); + expect(document.activeElement).toBe(modelControl()); expect(configManager.setConfig).not.toHaveBeenCalled(); }); @@ -766,7 +775,7 @@ describe('ModelSelector provider levels', () => { { id: 'high', label: 'High', order: 1, source: 'models_dev', actions: [{ type: 'effort', value: 'high' }] }, ] } }], }); - await renderSelector(); + await renderSelector(CATALOG_MODELS, 'acme-fast'); await openSettingsMenu(); const sky = document.body.querySelector('[data-openbitfun-part="reasoningSliderSky"]'); expect(sky).not.toBeNull(); @@ -854,7 +863,7 @@ describe('ModelSelector provider levels', () => { expect(document.activeElement).toBe(autoChoice); }); - it('offers providers first and keeps the symbolic selectors on that level', async () => { + it('opens only providers from the provider entry and marks the resolved default provider', async () => { await renderSelector(); await openMenu(); @@ -867,32 +876,111 @@ describe('ModelSelector provider levels', () => { expect(document.body.querySelector( '[data-testid="chat-model-selector-settings"]', )).toBeNull(); - expect(optionsPanel()?.dataset.panelKind).toBe('models'); + expect(optionsPanel()?.dataset.panelKind).toBe('providers'); expect(sharedPanelItems()).not.toBeNull(); expect(providerRows().every(row => sharedPanelItems()?.contains(row))).toBe(true); - expect(sharedPanelItems()?.contains(modelOption('primary'))).toBe(true); - expect(sharedPanelItems()?.contains(modelOption('fast'))).toBe(true); expect(providerRows().map(row => row.dataset.providerKey)) .toEqual(['provider-acme', 'provider-umbra']); - expect(modelOption('primary')).not.toBeNull(); - expect(modelOption('fast')).not.toBeNull(); - const submenuButtons = Array.from( - sharedPanelItems()?.querySelectorAll('button') ?? [], - ); - expect(submenuButtons.indexOf(modelOption('primary')!)) - .toBeLessThan(submenuButtons.indexOf(modelOption('fast')!)); - expect(submenuButtons.indexOf(modelOption('fast')!)) - .toBeLessThan(submenuButtons.indexOf(providerRows()[0])); + expect(modelOption('primary')).toBeNull(); + expect(modelOption('fast')).toBeNull(); + expect(document.body.querySelectorAll('[data-testid="chat-model-selector-option"]')).toHaveLength(0); + expect(providerRows().find(row => row.dataset.selected === 'true')?.dataset.providerKey) + .toBe('provider-acme'); expect(document.body.querySelector( '[data-testid="chat-model-selector-provider-selected-model"]', )).toBeNull(); // A concrete model is only reachable through its provider now. expect(modelOption('acme-deep')).toBeNull(); expect(modelOption('umbra-main')).toBeNull(); + await act(async () => document.body.querySelector( + '[data-testid="chat-model-selector-summary-back"]', + )!.click()); + const providerControl = document.body.querySelector( + '[data-testid="chat-model-selector-settings-provider"]', + )!; + expect(document.activeElement).toBe(providerControl); + await act(async () => providerControl.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true }), + )); + expect(document.activeElement).toBe(providerRows()[0]); + await act(async () => { + providerRows()[1].focus(); + providerRows()[1].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + }); + expect(modelOption('umbra-main')).not.toBeNull(); + expect(modelOption('acme-fast')).toBeNull(); + await act(async () => optionsPanel()!.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowLeft', bubbles: true }), + )); + expect(document.activeElement).toBe(providerRows()[1]); + expect(configManager.setConfig).not.toHaveBeenCalled(); + }); + + it('commits the session before remembering defaults and leaves the selector usable during that save', async () => { + flowChatStoreMocks.sessions.set('session-a', { config: { modelName: 'umbra-main' } }); + let commitSession!: () => void; + let saveDefault!: () => void; + vi.mocked(agentAPI.updateSessionModel).mockReturnValueOnce(new Promise(resolve => { commitSession = resolve; })); + vi.mocked(configManager.setConfig).mockReturnValueOnce(new Promise(resolve => { saveDefault = resolve; })); + await renderSelector(CATALOG_MODELS, 'umbra-main', 'session-a'); + await openMenu(); + await openProvider('provider-acme'); + await act(async () => modelOption('acme-deep')!.click()); + expect(agentAPI.updateSessionModel).toHaveBeenCalledTimes(1); + expect(configManager.setConfig).not.toHaveBeenCalled(); + await act(async () => commitSession()); + expect(configManager.setConfig).toHaveBeenCalledWith('ai.agent_model_defaults.mode', 'acme-deep'); + expect(optionsPanel()).toBeNull(); + expect(container.querySelector('[data-testid="chat-model-selector-btn"]')!.disabled).toBe(false); + expect(flowChatStoreMocks.sessions.get('session-a')?.config.modelName).toBe('acme-deep'); + await act(async () => saveDefault()); + }); + + it('preserves a committed session selection when remembering the default fails', async () => { + flowChatStoreMocks.sessions.set('session-a', { config: { modelName: 'umbra-main' } }); + vi.mocked(configManager.setConfig).mockRejectedValueOnce(new Error('default save failed')); + await renderSelector(CATALOG_MODELS, 'umbra-main', 'session-a'); + await openMenu(); + await openProvider('provider-acme'); + await act(async () => modelOption('acme-deep')!.click()); + expect(flowChatStoreMocks.sessions.get('session-a')?.config.modelName).toBe('acme-deep'); + expect(notificationStore.getState().activeNotifications.some(item => item.message === 'modelSelector.defaultSaveFailed')).toBe(true); + expect(optionsPanel()).toBeNull(); + }); + + it('skips an identical model and preset but clears a preset when entering an automatic selection mode', async () => { + flowChatStoreMocks.sessions.set('session-a', { config: { modelName: 'acme-fast' } }); + await renderSelector(CATALOG_MODELS, 'acme-fast', 'session-a'); + await openMenu(); + await openProvider('provider-acme'); + await act(async () => modelOption('acme-fast')!.click()); + expect(agentAPI.updateSessionModel).not.toHaveBeenCalled(); + expect(configManager.setConfig).not.toHaveBeenCalled(); + flowChatStoreMocks.sessions.get('session-a')!.config.modelName = 'primary'; + flowChatStoreMocks.sessions.get('session-a')!.config.reasoningPreset = 'high'; + await act(async () => flowChatStoreMocks.subscribers.forEach(callback => callback())); + await act(async () => modeTrigger().click()); + await act(async () => modeOption('smart')!.click()); + expect(agentAPI.updateSessionModel).toHaveBeenCalledWith(expect.objectContaining({ modelName: 'primary', reasoningPreset: null })); + await act(async () => modeTrigger().click()); + await act(async () => modeOption('pool')!.click()); + expect(agentAPI.updateSessionModel).toHaveBeenCalledTimes(1); + }); + + it('rolls back an unpinned selection and context limit when the session update fails', async () => { + flowChatStoreMocks.sessions.set('session-a', { config: {} }); + vi.mocked(agentAPI.updateSessionModel).mockRejectedValueOnce(new Error('session write failed')); + await renderSelector(CATALOG_MODELS, 'umbra-main', 'session-a'); + await openMenu(); + await openProvider('provider-acme'); + await act(async () => modelOption('acme-deep')!.click()); + expect(flowChatStoreMocks.store.updateSessionModelName).toHaveBeenLastCalledWith('session-a', undefined); + expect(flowChatStoreMocks.store.updateSessionMaxContextTokens).toHaveBeenLastCalledWith('session-a', undefined); + expect(configManager.setConfig).not.toHaveBeenCalled(); }); it('shows only the chosen provider\'s models and applies a selection', async () => { - await renderSelector(); + await renderSelector(CATALOG_MODELS, 'umbra-main'); await openMenu(); await openProvider('provider-acme'); @@ -904,7 +992,7 @@ describe('ModelSelector provider levels', () => { expect(sharedPanelItems()).not.toBeNull(); expect(sharedPanelItems()?.contains(modelOption('acme-fast'))).toBe(true); expect(sharedPanelItems()?.contains(modelOption('acme-deep'))).toBe(true); - // The symbolic selectors belong to the provider level and are not repeated. + // Model choices are concrete models owned by this provider. expect(modelOption('primary')).toBeNull(); const card = document.body.querySelector('[data-testid="chat-model-selector-menu"]'); @@ -925,12 +1013,12 @@ describe('ModelSelector provider levels', () => { expect(container.querySelector('[data-testid="chat-model-selector-btn"]')?.getAttribute('aria-expanded')).toBe('true'); expect(document.body.querySelector('[data-testid="chat-model-selector-menu"]')).toBe(card); expect(optionsPanel()).toBeNull(); - expect(document.activeElement).toBe(document.body.querySelector('[data-testid="chat-model-selector-settings-model"]')); + expect(document.activeElement).toBe(document.body.querySelector('[data-testid="chat-model-selector-settings-provider"]')); await act(async () => document.body.querySelector( '[data-testid="chat-model-selector-settings-model"]', )!.click()); - expect(document.body.querySelector('[data-testid="chat-model-selector-current-selection"]')?.textContent) - .toBe('acme-deep-native'); + expect(modelOption('acme-deep')?.getAttribute('aria-checked')).toBe('true'); + expect(modelOption('umbra-main')).toBeNull(); }); it('marks the provider that owns the pinned model without repeating the model beneath it', async () => { @@ -975,7 +1063,7 @@ describe('ModelSelector provider levels', () => { await openProvider('provider-acme'); expect(modelOption('acme-deep')).not.toBeNull(); - // Reopening starts at the summary; opening models resets the provider level. + // Reopening starts at the summary; opening models goes straight to the current provider. await act(async () => { container.querySelector( '[data-testid="chat-model-selector-btn"]', @@ -990,8 +1078,8 @@ describe('ModelSelector provider levels', () => { '[data-testid="chat-model-selector-settings-model"]', )!.click()); expect(optionsPanel()?.dataset.panelKind).toBe('models'); - expect(providerRows()).toHaveLength(2); - expect(modelOption('acme-deep')).toBeNull(); + expect(providerRows()).toHaveLength(0); + expect(modelOption('acme-deep')).not.toBeNull(); }); it('keeps the selector visible and actionable when no model is configured', async () => { @@ -1177,11 +1265,11 @@ describe('ModelSelector provider levels', () => { .toContain('first-model-native'); const listeners = [...flowChatStoreMocks.configChangeListeners]; - expect(listeners).toHaveLength(1); + expect(listeners).toHaveLength(2); await act(async () => { // ModelSettingsPage writes ai.models through ConfigManager's ai-scoped // mutation notification, so this is the post-save event to handle. - listeners[0]?.('ai'); + listeners.forEach(listener => listener('ai')); await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); @@ -1195,7 +1283,7 @@ describe('ModelSelector provider levels', () => { }); }); - it('lets Escape step from a provider to the model list, then the summary, then close', async () => { + it('lets Escape step from provider models to the provider list, then the summary, then close', async () => { await renderSelector(); await openMenu(); await openProvider('provider-acme'); @@ -1215,16 +1303,16 @@ describe('ModelSelector provider levels', () => { expect(document.body.querySelector( '[data-testid="chat-model-selector-settings"]', )).not.toBeNull(); - const modelField = document.body.querySelector( - '[data-testid="chat-model-selector-settings-model"]', + const providerField = document.body.querySelector( + '[data-testid="chat-model-selector-settings-provider"]', )!; - expect(document.activeElement).toBe(modelField); + expect(document.activeElement).toBe(providerField); const trigger = container.querySelector( '[data-testid="chat-model-selector-btn"]', )!; expect(trigger.getAttribute('aria-expanded')).toBe('true'); - await act(async () => modelField.dispatchEvent( + await act(async () => providerField.dispatchEvent( new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), )); expect(trigger.getAttribute('aria-expanded')).toBe('false'); @@ -1316,7 +1404,7 @@ describe('ModelSelector provider levels', () => { expect(document.body.querySelectorAll('[role="menu"]')).toHaveLength(1); await act(async () => document.body.querySelector( - '[data-testid="chat-model-selector-summary-back"]', + '[data-testid="chat-model-selector-back"]', )!.click()); const reasoningRow = document.body.querySelector( '[data-testid="chat-model-selector-settings-reasoning"]', diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.test.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.test.ts index d472c39507..5ffa44f48b 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.test.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.test.ts @@ -1241,6 +1241,8 @@ describe('MessageModule model synchronization', () => { }); const { context, session } = modelSyncContext('asdf'); await syncSessionModelSelection(context, session.sessionId, 'Standard'); + expect(mockGetConfigs).toHaveBeenCalledTimes(1); + expect(mockGetConfigs).toHaveBeenCalledWith(['ai.models', 'ai.default_models']); expect(context.flowChatStore.updateSessionModelName).not.toHaveBeenCalled(); expect(context.flowChatStore.updateSessionMaxContextTokens).toHaveBeenCalledWith(session.sessionId, 64000); expect(mockUpdateSessionModel).toHaveBeenCalledWith(expect.objectContaining({ diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.ts index 893a382bd2..2fbe6ac04a 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.ts @@ -4669,12 +4669,12 @@ export class FlowChatStore { }); } - public updateSessionModelName(sessionId: string, modelName: string): void { + public updateSessionModelName(sessionId: string, modelName: string | undefined): void { this.setState(prev => { const session = prev.sessions.get(sessionId); if (!session) return prev; - const normalizedModelName = modelName.trim() || 'primary'; + const normalizedModelName = modelName === undefined ? undefined : modelName.trim() || 'primary'; if (session.config.modelName?.trim() === normalizedModelName) { return prev; } @@ -6788,7 +6788,7 @@ export class FlowChatStore { public rollbackTokenUsage(): void { } - public updateSessionMaxContextTokens(sessionId: string, maxContextTokens: number): void { + public updateSessionMaxContextTokens(sessionId: string, maxContextTokens: number | undefined): void { this.setState(prev => { const session = prev.sessions.get(sessionId); if (!session) return prev; diff --git a/src/web-ui/src/flow_chat/utils/modelResolution.test.ts b/src/web-ui/src/flow_chat/utils/modelResolution.test.ts index 4576deaf39..4bc8fb1fbd 100644 --- a/src/web-ui/src/flow_chat/utils/modelResolution.test.ts +++ b/src/web-ui/src/flow_chat/utils/modelResolution.test.ts @@ -75,6 +75,7 @@ describe('configured model identity', () => { 'ai.models': models, 'ai.default_models': { primary: 'model-first' }, }); await expect(getModelMaxTokens('asdf')).resolves.toBe(64000); + expect(configManager.getConfigs).toHaveBeenLastCalledWith(['ai.models', 'ai.default_models']); }); }); diff --git a/src/web-ui/src/flow_chat/utils/modelResolution.ts b/src/web-ui/src/flow_chat/utils/modelResolution.ts index 4ffe050eab..b80d8599b0 100644 --- a/src/web-ui/src/flow_chat/utils/modelResolution.ts +++ b/src/web-ui/src/flow_chat/utils/modelResolution.ts @@ -162,44 +162,33 @@ export function resolveModelSelection({ }; } +/** Resolve from already loaded facts so selection and send do not read settings twice. */ +export function resolveModelMaxTokens( + modelName: string | undefined, + models: readonly AIModelConfig[], + defaultModels: DefaultModelsConfig, + modeDefaultModel?: string, +): number { + const selection = modelName?.trim() || modeDefaultModel; + return resolveModelForContextWindow(selection, models, defaultModels)?.context_window + || resolveModelForContextWindow('primary', models, defaultModels)?.context_window + || 128128; +} + export async function getModelMaxTokens(modelName?: string, agentType?: string): Promise { try { const configManager = await import('@/infrastructure/config/services/ConfigManager').then(m => m.configManager); const configData = await configManager.getConfigs([ 'ai.models', 'ai.default_models', - 'ai.agent_model_defaults', + ...(!modelName?.trim() ? ['ai.agent_model_defaults'] : []), ]); - const models = (configData['ai.models'] as AIModelConfig[] | undefined) || []; - const defaultModels = (configData['ai.default_models'] as DefaultModelsConfig | undefined) || {}; - const agentModelDefaults = configData['ai.agent_model_defaults'] as AgentModelDefaultsConfig | undefined; - - const normalizedModelName = modelName?.trim(); - const explicitModel = resolveModelForContextWindow(modelName, models, defaultModels); - if (explicitModel?.context_window) { - return explicitModel.context_window; - } - - // Only legacy sessions without a model selector inherit the current mode - // default. Session-owned selectors are resolved above. - if (!normalizedModelName) { - const modeModel = resolveModelForContextWindow( - agentModelDefaults?.mode, - models, - defaultModels, - ); - if (modeModel?.context_window) { - return modeModel.context_window; - } - } - - const primaryModel = resolveModelForContextWindow('primary', models, defaultModels); - if (primaryModel?.context_window) { - return primaryModel.context_window; - } - - log.debug('Model context_window config not found, using default', { modelName, agentType }); - return 128128; + return resolveModelMaxTokens( + modelName, + (configData['ai.models'] as AIModelConfig[] | undefined) || [], + (configData['ai.default_models'] as DefaultModelsConfig | undefined) || {}, + (configData['ai.agent_model_defaults'] as AgentModelDefaultsConfig | undefined)?.mode, + ); } catch (error) { log.warn('Failed to get model max tokens', { modelName, agentType, error }); return 128128; @@ -214,7 +203,7 @@ export async function resolveReasoningPresetForSessionCreation( const configData = await configManager.getConfigs([ 'ai.models', 'ai.default_models', - 'ai.agent_model_defaults', + ...(!modelName?.trim() ? ['ai.agent_model_defaults'] : []), ]); const models = (configData['ai.models'] as AIModelConfig[] | undefined) || []; const defaultModels = (configData['ai.default_models'] as DefaultModelsConfig | undefined) || {}; diff --git a/src/web-ui/src/flow_chat/utils/modelSync.ts b/src/web-ui/src/flow_chat/utils/modelSync.ts index 639120a43f..4bf1a8bf06 100644 --- a/src/web-ui/src/flow_chat/utils/modelSync.ts +++ b/src/web-ui/src/flow_chat/utils/modelSync.ts @@ -10,7 +10,7 @@ import { configManager } from '@/infrastructure/config/services/ConfigManager'; import type { AIModelConfig, AgentModelDefaultsConfig, DefaultModelsConfig } from '@/infrastructure/config/types'; import { createLogger } from '@/shared/utils/logger'; import type { FlowChatContext } from '../services/flow-chat-manager/types'; -import { getModelMaxTokens, resolveModelReference } from './modelResolution'; +import { resolveModelMaxTokens, resolveModelReference } from './modelResolution'; import { sessionProjectWorkspacePath, sessionWorkspaceId } from './sessionWorkspace'; import { getActiveSurfaceScope, @@ -52,8 +52,9 @@ export async function syncSessionModelSelection( throw new Error(`Session does not exist: ${sessionId}`); } + const sessionModelId = session.config.modelName?.trim(); const configData = (await configManager.getConfigs([ - 'ai.agent_model_defaults', + ...(!sessionModelId ? ['ai.agent_model_defaults'] : []), 'ai.models', 'ai.default_models', ])) ?? {}; @@ -62,10 +63,9 @@ export async function syncSessionModelSelection( const allModels = (configData['ai.models'] as AIModelConfig[] | undefined) || []; const defaultModels = (configData['ai.default_models'] as DefaultModelsConfig | undefined) || {}; - const sessionModelId = session.config.modelName?.trim(); const requestedModelId = sessionModelId || agentModelDefaults?.mode; const desiredModelId = normalizeModelSelection(requestedModelId, allModels, defaultModels); - const desiredMaxContextTokens = await getModelMaxTokens(desiredModelId, agentType); + const desiredMaxContextTokens = resolveModelMaxTokens(desiredModelId, allModels, defaultModels); surfaceScope.assertCurrent('resolve session model context window'); const shouldSyncContextWindow = session.maxContextTokens !== desiredMaxContextTokens; diff --git a/src/web-ui/src/infrastructure/config/AGENTS.md b/src/web-ui/src/infrastructure/config/AGENTS.md index f78ad52237..8ec95b019f 100644 --- a/src/web-ui/src/infrastructure/config/AGENTS.md +++ b/src/web-ui/src/infrastructure/config/AGENTS.md @@ -52,3 +52,14 @@ pnpm --dir src/web-ui run test:run src/infrastructure/config/components/modelEdi These checks cover draft isolation and source contracts, not rendered visuals or remote runtime behavior. + +For the device-scoped model catalog and non-blocking session/default selection: + +```bash +pnpm --dir src/web-ui run test:run src/infrastructure/config/services/ModelCatalogStore.test.ts src/infrastructure/config/services/ConfigManager.test.ts src/flow_chat/components/ModelSelectorProviderLevels.test.tsx src/flow_chat/components/ModelSelectorExternal.test.tsx src/flow_chat/components/ModelSelectorAcpMode.test.tsx src/flow_chat/utils/modelResolution.test.ts src/flow_chat/services/flow-chat-manager/MessageModule.test.ts src/flow_chat/services/flow-chat-manager/SessionModule.test.ts +``` + +Keep warm catalogs while refreshing, invalidate them only for catalog/model +facts, and isolate all snapshots and pending work by surface activation. An +existing session commits its selection before remembering future defaults; +explicit model reads before send must not await the future-default write. diff --git a/src/web-ui/src/infrastructure/config/services/ModelCatalogStore.test.ts b/src/web-ui/src/infrastructure/config/services/ModelCatalogStore.test.ts new file mode 100644 index 0000000000..19a435883b --- /dev/null +++ b/src/web-ui/src/infrastructure/config/services/ModelCatalogStore.test.ts @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; +import { aiApi, type AIModelCatalog } from '@/infrastructure/api/service-api/AIApi'; +import { configManager } from './ConfigManager'; +import { createModelCatalogStore } from './ModelCatalogStore'; + +vi.mock('@/infrastructure/api/service-api/AIApi', () => ({ aiApi: { + getModelCatalog: vi.fn(), onModelCatalogUpdated: vi.fn(() => () => {}), +} })); +vi.mock('./ConfigManager', () => ({ configManager: { + onConfigChange: vi.fn(() => () => {}), +} })); +const catalog = (version: number): AIModelCatalog => ({ version, models: [] }); +const deferred = () => { + let resolve!: (value: AIModelCatalog) => void; + const promise = new Promise(done => { resolve = done; }); + return { promise, resolve }; +}; +describe('shared model catalog', () => { + beforeEach(() => { activateSurface('local'); vi.clearAllMocks(); }); + + it('shares concurrent selectors, retains warm data on errors, and ignores preference writes', async () => { + vi.mocked(aiApi.getModelCatalog).mockResolvedValue(catalog(1)); + const store = createModelCatalogStore(); + const offA = store.subscribe(vi.fn()); + const offB = store.subscribe(vi.fn()); + await vi.waitFor(() => expect(store.getSnapshot().status).toBe('ready')); + expect(aiApi.getModelCatalog).toHaveBeenCalledTimes(1); + vi.mocked(configManager.onConfigChange).mock.calls[0][0]('ai.agent_model_defaults.mode'); + expect(aiApi.getModelCatalog).toHaveBeenCalledTimes(1); + vi.mocked(aiApi.getModelCatalog).mockRejectedValueOnce(new Error('offline')); + await store.refresh(); + expect(store.getSnapshot()).toEqual({ catalog: catalog(1), status: 'ready' }); + offA(); offB(); + }); + + it('replays an invalidation arriving during a read and discards the superseded result', async () => { + const first = deferred(); + vi.mocked(aiApi.getModelCatalog).mockReturnValueOnce(first.promise).mockResolvedValue(catalog(2)); + const store = createModelCatalogStore(); + const off = store.subscribe(vi.fn()); + await Promise.resolve(); + vi.mocked(configManager.onConfigChange).mock.calls[0][0]('ai.models'); + first.resolve(catalog(1)); + await vi.waitFor(() => expect(store.getSnapshot().catalog?.version).toBe(2)); + expect(aiApi.getModelCatalog).toHaveBeenCalledTimes(2); + off(); + }); + + it('never publishes an old host response on a new activation', async () => { + const old = deferred(); + vi.mocked(aiApi.getModelCatalog).mockReturnValueOnce(old.promise).mockResolvedValue(catalog(2)); + const store = createModelCatalogStore(); + const off = store.subscribe(vi.fn()); + await Promise.resolve(); + activateSurface('peer'); + expect(store.getSnapshot().catalog).toBeNull(); + await vi.waitFor(() => expect(store.getSnapshot().catalog?.version).toBe(2)); + old.resolve(catalog(1)); + await Promise.resolve(); + expect(store.getSnapshot().catalog?.version).toBe(2); + off(); + }); +}); diff --git a/src/web-ui/src/infrastructure/config/services/ModelCatalogStore.ts b/src/web-ui/src/infrastructure/config/services/ModelCatalogStore.ts new file mode 100644 index 0000000000..2f023b5153 --- /dev/null +++ b/src/web-ui/src/infrastructure/config/services/ModelCatalogStore.ts @@ -0,0 +1,84 @@ +import { aiApi, type AIModelCatalog } from '@/infrastructure/api/service-api/AIApi'; +import { getActiveSurfaceScope, onSurfaceActivated } from '@/infrastructure/peer-device/deviceSurface'; +import { createLogger } from '@/shared/utils/logger'; +import { configManager } from './ConfigManager'; + +const log = createLogger('ModelCatalogStore'); +type Snapshot = { catalog: AIModelCatalog | null; status: 'loading' | 'ready' | 'error' }; +const EMPTY: Snapshot = { catalog: null, status: 'loading' }; + +/** One catalog projection per device activation. Refreshes keep the last good + * projection usable; a config/catalog event during a read schedules another + * read so an older response cannot consume a newer invalidation. + */ +export function createModelCatalogStore() { + let epoch = -1; + let snapshot = EMPTY; + let revision = 0; + let pending: Promise | undefined; + let stop: (() => void) | undefined; + const listeners = new Set<() => void>(); + const notify = () => listeners.forEach(listener => listener()); + const getSnapshot = () => epoch === getActiveSurfaceScope().epoch ? snapshot : EMPTY; + + const refresh = (): Promise => { + const scope = getActiveSurfaceScope(); + if (epoch !== scope.epoch) { + epoch = scope.epoch; + snapshot = EMPTY; + pending = undefined; + } + revision += 1; + if (pending) return pending; + const read = async () => { + do { + const requestedRevision = revision; + try { + scope.assertCurrent('load model catalog'); + const catalog = await aiApi.getModelCatalog(); + if (!scope.isCurrent()) return; + if (requestedRevision !== revision) continue; + snapshot = { catalog, status: 'ready' }; + } catch (error) { + if (!scope.isCurrent()) return; + if (requestedRevision !== revision) continue; + snapshot = { catalog: snapshot.catalog, status: snapshot.catalog ? 'ready' : 'error' }; + log.warn('Failed to refresh model catalog', { error }); + } + notify(); + return; + } while (scope.isCurrent()); + }; + // Publish the promise before calling any loader that can emit events. + const promise = Promise.resolve().then(read).finally(() => { + if (pending === promise) pending = undefined; + }); + pending = promise; + return promise; + }; + + const subscribe = (listener: () => void) => { + listeners.add(listener); + if (!stop) { + const offCatalog = aiApi.onModelCatalogUpdated(() => { void refresh(); }); + const offConfig = configManager.onConfigChange(path => { + if (!path || path === 'ai' || ['ai.models', 'ai.default_models'].some( + owned => path === owned || path.startsWith(`${owned}.`), + )) void refresh(); + }); + const offSurface = onSurfaceActivated(() => { + void refresh(); + notify(); + }); + stop = () => { offCatalog(); offConfig(); offSurface(); }; + void refresh(); + } + return () => { + listeners.delete(listener); + if (!listeners.size) { stop?.(); stop = undefined; } + }; + }; + return { subscribe, getSnapshot, refresh }; +} + +export const modelCatalogStore = createModelCatalogStore(); diff --git a/src/web-ui/src/locales/en-US/flow-chat.json b/src/web-ui/src/locales/en-US/flow-chat.json index be4f9094c3..43ea76a65d 100644 --- a/src/web-ui/src/locales/en-US/flow-chat.json +++ b/src/web-ui/src/locales/en-US/flow-chat.json @@ -3331,6 +3331,7 @@ "acpMode": "Mode", "acpModeLocked": "This conversation has already started, so its mode is fixed.", "acpModeFailed": "Failed to switch mode", + "defaultSaveFailed": "Model selected, but the default for future chats could not be saved.", "switchFailed": "Failed to switch model", "modelNotConfigured": "Not Configured", "openModelSettings": "Open model settings", diff --git a/src/web-ui/src/locales/zh-CN/flow-chat.json b/src/web-ui/src/locales/zh-CN/flow-chat.json index 335c390e4b..d6b0c1bb44 100644 --- a/src/web-ui/src/locales/zh-CN/flow-chat.json +++ b/src/web-ui/src/locales/zh-CN/flow-chat.json @@ -3331,6 +3331,7 @@ "acpMode": "模式", "acpModeLocked": "会话已经开始，模式不能再改。", "acpModeFailed": "切换模式失败", + "defaultSaveFailed": "模型已切换，但未能保存后续会话的默认选择。", "switchFailed": "切换模型失败", "modelNotConfigured": "未配置", "openModelSettings": "前往模型设置", diff --git a/src/web-ui/src/locales/zh-TW/flow-chat.json b/src/web-ui/src/locales/zh-TW/flow-chat.json index 23f90ac627..0da8ccc993 100644 --- a/src/web-ui/src/locales/zh-TW/flow-chat.json +++ b/src/web-ui/src/locales/zh-TW/flow-chat.json @@ -3331,6 +3331,7 @@ "acpMode": "模式", "acpModeLocked": "對話已經開始，模式不能再改。", "acpModeFailed": "切換模式失敗", + "defaultSaveFailed": "模型已切換，但未能儲存後續對話的預設選擇。", "switchFailed": "切換模型失敗", "modelNotConfigured": "未設定", "openModelSettings": "前往模型設定", From 1377cc6d27dc83c1478d1d4612f97d1fa4020402 Mon Sep 17 00:00:00 2001 From: GCWing Date: Sun, 11 Oct 2026 16:58:27 +0800 Subject: [PATCH 3/3] chore(rust): clean up macOS and atomic API warnings --- .../desktop/src/api/clipboard_file_api.rs | 5 ++ .../src/computer_use/control_session.rs | 1 + .../desktop/src/computer_use/macos_ax_ui.rs | 46 +------------------ .../desktop/src/computer_use/macos_capture.rs | 2 - .../src/computer_use/macos_list_apps.rs | 3 -- src/apps/desktop/src/embedded_relay_host.rs | 7 ++- src/apps/desktop/src/webview_recovery.rs | 10 ++++ .../adapters/agent-runtime-ipc/src/client.rs | 2 +- .../src/miniapp/worker.rs | 17 ++++--- .../src/remote_ssh/port_forward.rs | 10 ++-- third_party/tao/OPENBITFUN-PATCH.md | 8 ++++ third_party/tao/src/platform/macos.rs | 2 +- .../tao/src/platform_impl/macos/app.rs | 2 +- .../src/platform_impl/macos/app_delegate.rs | 8 ++-- .../tao/src/platform_impl/macos/event.rs | 4 +- .../tao/src/platform_impl/macos/event_loop.rs | 12 +---- .../tao/src/platform_impl/macos/icon.rs | 9 +++- .../tao/src/platform_impl/macos/monitor.rs | 2 +- .../tao/src/platform_impl/macos/util/async.rs | 2 +- .../tao/src/platform_impl/macos/util/mod.rs | 8 +--- .../tao/src/platform_impl/macos/window.rs | 36 +++++++-------- .../platform_impl/macos/window_delegate.rs | 5 +- 22 files changed, 84 insertions(+), 117 deletions(-) diff --git a/src/apps/desktop/src/api/clipboard_file_api.rs b/src/apps/desktop/src/api/clipboard_file_api.rs index f5bfc66d44..08d8897930 100644 --- a/src/apps/desktop/src/api/clipboard_file_api.rs +++ b/src/apps/desktop/src/api/clipboard_file_api.rs @@ -342,6 +342,7 @@ impl Default for ClipboardImageResponse { /// Sniffs the image format from magic bytes so a tool that misreports success /// cannot inject arbitrary text as an attachment payload. +#[cfg(any(target_os = "linux", test))] pub(crate) fn sniff_image_mime(bytes: &[u8]) -> Option<&'static str> { if bytes.starts_with(&[0x89, b'P', b'N', b'G', 0x0D, 0x0A, 0x1A, 0x0A]) { Some("image/png") @@ -355,12 +356,14 @@ pub(crate) fn sniff_image_mime(bytes: &[u8]) -> Option<&'static str> { /// Outcome of a Linux clipboard-image probe. enum ClipboardImageRead { /// An image payload, base64-encoded with its sniffed MIME type. + #[cfg(target_os = "linux")] Image(String, String), /// The reader tools ran; the clipboard simply holds no image payload. Empty, /// Neither `wl-paste` nor `xclip` could be spawned. Surfaced to the user /// instead of silently reporting "no image": a plain-text clipboard and a /// machine missing the reader tools must stay distinguishable. + #[cfg(target_os = "linux")] ToolsUnavailable(String), } @@ -440,11 +443,13 @@ fn get_clipboard_image_internal() -> ClipboardImageRead { #[tauri::command] pub async fn get_clipboard_image() -> Result { match get_clipboard_image_internal() { + #[cfg(target_os = "linux")] ClipboardImageRead::Image(base64, mime_type) => Ok(ClipboardImageResponse { base64: Some(base64), mime_type: Some(mime_type), }), ClipboardImageRead::Empty => Ok(ClipboardImageResponse::default()), + #[cfg(target_os = "linux")] ClipboardImageRead::ToolsUnavailable(error) => { log::warn!("Clipboard image read unsupported: {}", error); Err(error) diff --git a/src/apps/desktop/src/computer_use/control_session.rs b/src/apps/desktop/src/computer_use/control_session.rs index 5ceb4299d9..3349f6f42f 100644 --- a/src/apps/desktop/src/computer_use/control_session.rs +++ b/src/apps/desktop/src/computer_use/control_session.rs @@ -288,6 +288,7 @@ fn cleanup_async(ticket: u64) { } /// Native delegates pass the identity captured when their stream was created. /// Never call an unscoped "stop current" from a delayed native callback. +#[cfg(target_os = "linux")] pub(crate) fn native_stopped_generation(generation: u64, reason: &str) { native_stopped_inner(generation, None, reason); } diff --git a/src/apps/desktop/src/computer_use/macos_ax_ui.rs b/src/apps/desktop/src/computer_use/macos_ax_ui.rs index cb571f8126..8f02c62b61 100644 --- a/src/apps/desktop/src/computer_use/macos_ax_ui.rs +++ b/src/apps/desktop/src/computer_use/macos_ax_ui.rs @@ -50,19 +50,6 @@ unsafe extern "C" { const K_AX_VALUE_CGPOINT: u32 = 1; const K_AX_VALUE_CGSIZE: u32 = 2; -/// Pid of the frontmost application, via `NSWorkspace.frontmostApplication`. -/// -/// Reads in-process in microseconds. The previous implementation shelled out to -/// `osascript` on every call — and this is on the hot path for -/// `describe_screen` (which reaches it up to three times per call), so a single -/// observation used to cost several hundred milliseconds of process spawns plus -/// the risk of a System Events AppleEvent timeout. -fn frontmost_pid() -> OpenBitFunResult { - crate::computer_use::macos_bg_input::frontmost_pid_macos().ok_or_else(|| { - OpenBitFunError::tool("NSWorkspace reported no frontmost application.".to_string()) - }) -} - unsafe fn ax_release(v: CFTypeRef) { unsafe { if !v.is_null() { @@ -540,14 +527,6 @@ fn element_short_desc(role: Option<&str>, title: Option<&str>) -> String { const MAX_CANDIDATES: usize = 10; -/// Search the **frontmost** app's accessibility tree (BFS) for elements matching filters. -/// Collects all matches, filters invisible/off-screen ones, ranks by relevance, returns the best. -pub(super) fn locate_ui_element_center( - query: &UiElementLocateQuery, -) -> OpenBitFunResult { - locate_ui_element_center_for_pid(frontmost_pid()?, query) -} - /// Search only the explicitly selected application, even while another app is active. pub(super) fn locate_ui_element_center_for_pid( pid: i32, @@ -890,13 +869,8 @@ unsafe fn is_ax_interactive(elem: AXUIElementRef, role: &str) -> bool { } } -/// Enumerate visible interactive elements in the frontmost app's AX tree -/// and return a condensed text representation of the UI for context (no -/// numbered labels rendered on the screenshot). -pub(super) fn enumerate_ui_tree_text(max_elements: usize) -> Option { - enumerate_ui_tree_text_for_pid(frontmost_pid().ok()?, max_elements) -} - +/// Enumerate visible interactive elements in the selected app's AX tree +/// and return a condensed text representation of the UI for context. pub(super) fn enumerate_ui_tree_text_for_pid(pid: i32, max_elements: usize) -> Option { let root = unsafe { AXUIElementCreateApplication(pid) }; if root.is_null() { @@ -1055,13 +1029,6 @@ unsafe fn ax_parent_context_line(elem: AXUIElementRef) -> Option { } /// Hit-test the accessibility element at global screen coordinates (OCR `move_to_text` disambiguation). -pub(super) fn accessibility_hit_at_global_point(gx: f64, gy: f64) -> Option { - unsafe { - let sys = AXUIElementCreateSystemWide(); - accessibility_hit_from_root(sys, None, gx, gy) - } -} - pub(super) fn accessibility_hit_at_global_point_for_pid( pid: i32, gx: f64, @@ -1116,15 +1083,6 @@ unsafe fn accessibility_hit_from_root( } } -// ── Raw OCR: frontmost window bounds (separate from agent screenshot pipeline) ───────────────── - -/// Bounds of the foreground app's focused or main window in global screen coordinates (same space as pointer / screen capture). -/// Used to crop **raw** pixels for Vision OCR without pointer overlays from the agent screenshot path. -pub(super) fn frontmost_window_bounds_global() -> OpenBitFunResult<(i32, i32, u32, u32)> { - let pid = frontmost_pid()?; - window_bounds_global_for_pid(pid) -} - /// Number of windows the app currently owns, per the AX `AXWindows` attribute. /// /// A launched-but-windowless app — common for Electron clients whose window was diff --git a/src/apps/desktop/src/computer_use/macos_capture.rs b/src/apps/desktop/src/computer_use/macos_capture.rs index 0a5c158d5a..8167ced873 100644 --- a/src/apps/desktop/src/computer_use/macos_capture.rs +++ b/src/apps/desktop/src/computer_use/macos_capture.rs @@ -56,7 +56,6 @@ pub(super) struct CaptureFrame { pub height: u32, pub window_id: u32, pub bounds: [f64; 4], - pub sequence: u64, pub generation: u64, } @@ -144,7 +143,6 @@ impl CaptureSession { height, window_id, bounds, - sequence, generation: self.generation, }) } diff --git a/src/apps/desktop/src/computer_use/macos_list_apps.rs b/src/apps/desktop/src/computer_use/macos_list_apps.rs index 2e5edf55d3..9a3a8ee222 100644 --- a/src/apps/desktop/src/computer_use/macos_list_apps.rs +++ b/src/apps/desktop/src/computer_use/macos_list_apps.rs @@ -127,9 +127,6 @@ pub(super) fn list_running_apps(include_hidden: bool) -> OpenBitFunResult { drop(l); return; } - Err(e) => last_err = Some(e), - } + Err(e) => e, + }; if std::time::Instant::now() >= deadline { panic!("{what}: port {port} never became bindable again: {last_err:?}"); } diff --git a/src/apps/desktop/src/webview_recovery.rs b/src/apps/desktop/src/webview_recovery.rs index 8816038b77..4e8e30d57e 100644 --- a/src/apps/desktop/src/webview_recovery.rs +++ b/src/apps/desktop/src/webview_recovery.rs @@ -1,9 +1,14 @@ +#[cfg(any(target_os = "windows", test))] use serde::{Deserialize, Serialize}; +#[cfg(any(target_os = "windows", test))] use std::time::Duration; +#[cfg(any(target_os = "windows", test))] const RENDERER_FAILURE_WINDOW: Duration = Duration::from_secs(10 * 60); +#[cfg(any(target_os = "windows", test))] const RESTART_FAILURE_WINDOW: Duration = Duration::from_secs(10 * 60); +#[cfg(any(target_os = "windows", test))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum FailureKind { BrowserExited, @@ -13,6 +18,7 @@ enum FailureKind { Other, } +#[cfg(any(target_os = "windows", test))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum RecoveryAction { Reload, @@ -21,6 +27,7 @@ enum RecoveryAction { Observe, } +#[cfg(any(target_os = "windows", test))] #[derive(Debug, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase", default)] struct RecoveryHistory { @@ -28,6 +35,7 @@ struct RecoveryHistory { restart_attempts_ms: Vec, } +#[cfg(any(target_os = "windows", test))] fn decide_recovery( history: &mut RecoveryHistory, failure: FailureKind, @@ -50,6 +58,7 @@ fn decide_recovery( } } +#[cfg(any(target_os = "windows", test))] fn restart_or_block(history: &mut RecoveryHistory, now_ms: u64) -> RecoveryAction { let window_ms = RESTART_FAILURE_WINDOW.as_millis() as u64; history @@ -63,6 +72,7 @@ fn restart_or_block(history: &mut RecoveryHistory, now_ms: u64) -> RecoveryActio } } +#[cfg(any(target_os = "windows", test))] fn restart_after_failed_reload(history: &mut RecoveryHistory, now_ms: u64) -> RecoveryAction { restart_or_block(history, now_ms) } diff --git a/src/crates/adapters/agent-runtime-ipc/src/client.rs b/src/crates/adapters/agent-runtime-ipc/src/client.rs index 8a8affdce3..fe227c5663 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/client.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/client.rs @@ -196,7 +196,7 @@ impl RuntimeIpcClient { let release_gate_after_write = operation.is_interruptible_lineage_read(); let request_id = self .next_request_id - .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |value| { + .try_update(Ordering::Relaxed, Ordering::Relaxed, |value| { value.checked_add(1) }) .map_err(|_| RuntimeIpcClientError::RequestIdExhausted)?; diff --git a/src/crates/services/services-integrations/src/miniapp/worker.rs b/src/crates/services/services-integrations/src/miniapp/worker.rs index 3f24ed2078..a623663a44 100644 --- a/src/crates/services/services-integrations/src/miniapp/worker.rs +++ b/src/crates/services/services-integrations/src/miniapp/worker.rs @@ -84,15 +84,14 @@ impl JsWorker { if line.is_empty() { continue; } - let _ = - last_activity_clone.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |_| { - Some( - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_millis() as i64, - ) - }); + let _ = last_activity_clone.try_update(Ordering::SeqCst, Ordering::SeqCst, |_| { + Some( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as i64, + ) + }); let msg: Value = match serde_json::from_str(&line) { Ok(v) => v, Err(_) => continue, diff --git a/src/crates/services/services-integrations/src/remote_ssh/port_forward.rs b/src/crates/services/services-integrations/src/remote_ssh/port_forward.rs index 5b76fd6898..c5507288bd 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/port_forward.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/port_forward.rs @@ -83,11 +83,11 @@ impl ForwardRuntime { // Saturate rather than wrap. The counter should never reach zero with a // connection still open, but underflowing it would report four billion // active connections in the UI and there is no failure worth that. - let _ = - self.active_connections - .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |active| { - Some(active.saturating_sub(1)) - }); + let _ = self + .active_connections + .try_update(Ordering::SeqCst, Ordering::SeqCst, |active| { + Some(active.saturating_sub(1)) + }); } fn record_error(&self, message: String) { diff --git a/third_party/tao/OPENBITFUN-PATCH.md b/third_party/tao/OPENBITFUN-PATCH.md index 9b7bb1d442..0ca53d76d2 100644 --- a/third_party/tao/OPENBITFUN-PATCH.md +++ b/third_party/tao/OPENBITFUN-PATCH.md @@ -23,6 +23,14 @@ Untitled and fullscreen style masks are excluded from the layout override; windows without a custom inset retain upstream behavior. No notification retry, timer, new private API or product-specific geometry is added. +The vendored macOS sources also receive compiler-warning maintenance: current +event constants and ivar reads, removal of unused private state/helpers and +redundant unsafe blocks, and explicit retention of validated per-window icon +data. The obsolete fullscreen button lookup is omitted because AppKit always +returns nil for it; the zoom button remains in the list. The legacy filename-list +pasteboard format is retained at its three registration/read sites, with local +deprecation annotations, so this cleanup does not change drag payload semantics. + OpenBitFun keeps its original toolbar height and tab spacing. The desktop host derives the configured inset from the shared design height and native metrics; the framework owns applying that inset during its native window lifecycle. diff --git a/third_party/tao/src/platform/macos.rs b/third_party/tao/src/platform/macos.rs index 32b8cbbf3f..1e58321b6a 100644 --- a/third_party/tao/src/platform/macos.rs +++ b/third_party/tao/src/platform/macos.rs @@ -416,7 +416,7 @@ impl EventLoopWindowTargetExtMacOS for EventLoopWindowTarget { fn show_application(&self) { // TODO: Safety. let mtm = unsafe { objc2_foundation::MainThreadMarker::new_unchecked() }; - unsafe { objc2_app_kit::NSApplication::sharedApplication(mtm).unhide(None) } + objc2_app_kit::NSApplication::sharedApplication(mtm).unhide(None) } fn hide_other_applications(&self) { diff --git a/third_party/tao/src/platform_impl/macos/app.rs b/third_party/tao/src/platform_impl/macos/app.rs index a83363eefe..ea218e5c59 100644 --- a/third_party/tao/src/platform_impl/macos/app.rs +++ b/third_party/tao/src/platform_impl/macos/app.rs @@ -35,7 +35,7 @@ extern "C" fn send_event(this: &NSApplication, _sel: Sel, event: &NSEvent) { // but that doesn't really matter here. let event_type = event.r#type(); let modifier_flags = event.modifierFlags(); - if event_type == appkit::NSKeyUp + if event_type == NSEventType::KeyUp && util::has_flag(modifier_flags, appkit::NSEventModifierFlags::Command) { if let Some(key_window) = this.keyWindow() { diff --git a/third_party/tao/src/platform_impl/macos/app_delegate.rs b/third_party/tao/src/platform_impl/macos/app_delegate.rs index 5107814c94..b2b92915da 100644 --- a/third_party/tao/src/platform_impl/macos/app_delegate.rs +++ b/third_party/tao/src/platform_impl/macos/app_delegate.rs @@ -137,11 +137,9 @@ extern "C" fn application_will_terminate(_: &Object, _: Sel, _: id) { extern "C" fn application_open_urls(_: &Object, _: Sel, _: id, urls: &NSArray) { trace!("Trigger `application:openURLs:`"); - let urls = unsafe { - (0..urls.count()) - .flat_map(|i| url::Url::parse(&urls.objectAtIndex(i).absoluteString().unwrap().to_string())) - .collect::>() - }; + let urls = (0..urls.count()) + .flat_map(|i| url::Url::parse(&urls.objectAtIndex(i).absoluteString().unwrap().to_string())) + .collect::>(); trace!("Get `application:openURLs:` URLs: {:?}", urls); AppState::open_urls(urls); trace!("Completed `application:openURLs:`"); diff --git a/third_party/tao/src/platform_impl/macos/event.rs b/third_party/tao/src/platform_impl/macos/event.rs index 6b1ce97f52..6063bc5202 100644 --- a/third_party/tao/src/platform_impl/macos/event.rs +++ b/third_party/tao/src/platform_impl/macos/event.rs @@ -162,7 +162,7 @@ pub fn create_key_event( //#[cfg(debug_assertions)] println!("Couldn't get key from code: {:?}", physical_key); key_without_modifiers = get_modifierless_char(scancode); - let modifiers = unsafe { NSEvent::modifierFlags(ns_event) }; + let modifiers = NSEvent::modifierFlags(ns_event); let has_alt = modifiers.contains(NSEventModifierFlags::Option); let has_ctrl = modifiers.contains(NSEventModifierFlags::Control); if has_alt || has_ctrl || text_with_all_modifiers.is_none() || !is_press { @@ -304,7 +304,7 @@ pub fn extra_function_key_to_code(scancode: u16, string: &str) -> KeyCode { } pub fn event_mods(event: &NSEvent) -> ModifiersState { - let flags = unsafe { NSEvent::modifierFlags(event) }; + let flags = NSEvent::modifierFlags(event); let mut m = ModifiersState::empty(); m.set( ModifiersState::SHIFT, diff --git a/third_party/tao/src/platform_impl/macos/event_loop.rs b/third_party/tao/src/platform_impl/macos/event_loop.rs index 8179ea91fd..58644326dc 100644 --- a/third_party/tao/src/platform_impl/macos/event_loop.rs +++ b/third_party/tao/src/platform_impl/macos/event_loop.rs @@ -35,7 +35,7 @@ use crate::{ observer::*, util::{self, IdRef}, }, - set_badge_label, set_progress_indicator, + set_progress_indicator, }, window::{ProgressBarState, Theme}, }; @@ -123,16 +123,6 @@ impl EventLoopWindowTarget { set_progress_indicator(progress); } - #[inline] - pub fn set_badge_count(&self, count: Option, _desktop_filename: Option) { - set_badge_label(count.map(|c| c.to_string())); - } - - #[inline] - pub fn set_badge_label(&self, label: Option) { - set_badge_label(label); - } - #[inline] pub fn set_theme(&self, theme: Option) { set_ns_theme(theme) diff --git a/third_party/tao/src/platform_impl/macos/icon.rs b/third_party/tao/src/platform_impl/macos/icon.rs index c5c7439d06..edb4b37ac6 100644 --- a/third_party/tao/src/platform_impl/macos/icon.rs +++ b/third_party/tao/src/platform_impl/macos/icon.rs @@ -5,10 +5,15 @@ use crate::icon::{BadIcon, RgbaIcon}; #[derive(Debug, Clone)] -pub struct PlatformIcon(RgbaIcon); +pub struct PlatformIcon { + // macOS ignores per-window icons; retain validated pixels for the shared Icon API. + _rgba: RgbaIcon, +} impl PlatformIcon { pub fn from_rgba(rgba: Vec, width: u32, height: u32) -> Result { - Ok(PlatformIcon(RgbaIcon::from_rgba(rgba, width, height)?)) + Ok(PlatformIcon { + _rgba: RgbaIcon::from_rgba(rgba, width, height)?, + }) } } diff --git a/third_party/tao/src/platform_impl/macos/monitor.rs b/third_party/tao/src/platform_impl/macos/monitor.rs index 7cdbd3986c..c9c60943c8 100644 --- a/third_party/tao/src/platform_impl/macos/monitor.rs +++ b/third_party/tao/src/platform_impl/macos/monitor.rs @@ -20,7 +20,7 @@ use core_graphics::{ }; use objc2::{msg_send, rc::Retained}; use objc2_app_kit::NSScreen; -use objc2_foundation::{ns_string, MainThreadMarker, NSString, NSUInteger}; +use objc2_foundation::{ns_string, MainThreadMarker, NSUInteger}; #[derive(Clone)] pub struct VideoMode { diff --git a/third_party/tao/src/platform_impl/macos/util/async.rs b/third_party/tao/src/platform_impl/macos/util/async.rs index 4513b6bc7c..e584971554 100644 --- a/third_party/tao/src/platform_impl/macos/util/async.rs +++ b/third_party/tao/src/platform_impl/macos/util/async.rs @@ -8,7 +8,7 @@ use std::{ }; use core_graphics::base::CGFloat; -use dispatch2::{DispatchQueue, DispatchQueueAttr}; +use dispatch2::DispatchQueue; use objc2::{rc::autoreleasepool, Message}; use objc2_app_kit::{NSScreen, NSView, NSWindow, NSWindowStyleMask}; use objc2_foundation::{MainThreadMarker, NSPoint, NSSize, NSString}; diff --git a/third_party/tao/src/platform_impl/macos/util/mod.rs b/third_party/tao/src/platform_impl/macos/util/mod.rs index b9a39147e6..32714fbcf2 100644 --- a/third_party/tao/src/platform_impl/macos/util/mod.rs +++ b/third_party/tao/src/platform_impl/macos/util/mod.rs @@ -12,7 +12,7 @@ use std::ops::{BitAnd, Deref}; use core_graphics::display::CGDisplay; use objc2::{ class, - runtime::{AnyClass as Class, AnyObject as Object, Sel}, + runtime::{AnyClass as Class, AnyObject as Object}, }; use objc2_app_kit::{NSApp, NSView, NSWindow, NSWindowStyleMask}; use objc2_foundation::{MainThreadMarker, NSAutoreleasePool, NSPoint, NSRange, NSRect, NSUInteger}; @@ -20,7 +20,7 @@ use objc2_foundation::{MainThreadMarker, NSAutoreleasePool, NSPoint, NSRange, NS use crate::{ dpi::{LogicalPosition, PhysicalPosition}, error::ExternalError, - platform_impl::platform::ffi::{self, id, nil, BOOL, YES}, + platform_impl::platform::ffi::{self, id, nil}, }; // Replace with `!` once stable @@ -123,10 +123,6 @@ pub unsafe fn open_emoji_picker() { let () = msg_send![&NSApp(mtm), orderFrontCharacterPalette: nil]; } -pub extern "C" fn yes(_: &Object, _: Sel) -> BOOL { - YES -} - pub unsafe fn toggle_style_mask( window: &NSWindow, view: &NSView, diff --git a/third_party/tao/src/platform_impl/macos/window.rs b/third_party/tao/src/platform_impl/macos/window.rs index 06067e60d8..7f5d9a25e2 100644 --- a/third_party/tao/src/platform_impl/macos/window.rs +++ b/third_party/tao/src/platform_impl/macos/window.rs @@ -6,7 +6,6 @@ use std::{ collections::VecDeque, convert::TryInto, - f64, ffi::CStr, os::raw::c_void, sync::{ @@ -40,11 +39,7 @@ use crate::{ WindowAttributes, WindowId as RootWindowId, WindowSizeConstraints, }, }; -use core_graphics::{ - base::CGFloat, - display::{CGDisplay, CGDisplayMode}, - geometry::CGPoint, -}; +use core_graphics::{base::CGFloat, display::CGDisplay, geometry::CGPoint}; use objc2::{ msg_send, rc::Retained, @@ -53,8 +48,8 @@ use objc2::{ use objc2_app_kit::{ self as appkit, NSApp, NSApplicationPresentationOptions, NSBackingStoreType, NSColor, NSEvent, NSEventModifierFlags, NSEventSubtype, NSEventType, NSRequestUserAttentionType, NSScreen, NSView, - NSWindow, NSWindowButton, NSWindowCollectionBehavior, NSWindowFullScreenButton, - NSWindowOrderingMode, NSWindowSharingType, NSWindowStyleMask, + NSWindow, NSWindowButton, NSWindowCollectionBehavior, NSWindowOrderingMode, NSWindowSharingType, + NSWindowStyleMask, }; use objc2_foundation::{ ns_string, MainThreadMarker, NSArray, NSAutoreleasePool, NSDictionary, NSInteger, NSPoint, @@ -266,7 +261,6 @@ fn create_window( } if pl_attrs.titlebar_buttons_hidden { for titlebar_button in &[ - NSWindowFullScreenButton, NSWindowButton::MiniaturizeButton, NSWindowButton::CloseButton, NSWindowButton::ZoomButton, @@ -435,9 +429,18 @@ extern "C" fn is_focusable(this: &Object, _: Sel) -> Bool { // Window chrome belongs to the window, not its replaceable content view. // Wry replaces TaoView with its WebView parent during window construction. pub(super) unsafe fn traffic_light_inset(window: &NSWindow) -> Option> { - let configured: Bool = *window.get_ivar("hasTrafficLightInset"); + // These ivars are registered on TaoWindow above and accessed on the AppKit main thread. + let configured = window + .class() + .instance_variable(CStr::from_bytes_with_nul(b"hasTrafficLightInset\0").unwrap()) + .unwrap(); + let configured: Bool = *configured.load(window); if configured.as_bool() { - let position: NSPoint = *window.get_ivar("trafficLightInset"); + let inset = window + .class() + .instance_variable(CStr::from_bytes_with_nul(b"trafficLightInset\0").unwrap()) + .unwrap(); + let position: NSPoint = *inset.load(window); Some(LogicalPosition::new(position.x, position.y)) } else { None @@ -511,7 +514,6 @@ pub struct SharedState { /// bar in exclusive fullscreen but want to restore the original options when /// transitioning back to borderless fullscreen. save_presentation_opts: Option, - pub saved_desktop_display_mode: Option<(CGDisplay, CGDisplayMode)>, pub current_theme: Theme, } @@ -548,7 +550,6 @@ pub struct UnownedWindow { decorations: AtomicBool, cursor_state: Weak>, transparent: bool, - pub inner_rect: Option>, } unsafe impl Send for UnownedWindow {} @@ -615,7 +616,10 @@ impl UnownedWindow { } // register for drag and drop operations. - ns_window.registerForDraggedTypes(&NSArray::arrayWithObject(appkit::NSFilenamesPboardType)); + // Keep the filename-list format paired with the legacy drag delegates. + #[allow(deprecated)] + let filenames_type = appkit::NSFilenamesPboardType; + ns_window.registerForDraggedTypes(&NSArray::arrayWithObject(filenames_type)); } // Since `win_attribs` is put into a mutex below, we'll just copy these @@ -630,9 +634,6 @@ impl UnownedWindow { let focused = win_attribs.focused; let decorations = win_attribs.decorations; let visible_on_all_workspaces = win_attribs.visible_on_all_workspaces; - let inner_rect = win_attribs - .inner_size - .map(|size| size.to_physical(scale_factor)); let cloned_preferred_theme = win_attribs.preferred_theme; @@ -643,7 +644,6 @@ impl UnownedWindow { shared_state: Arc::new(Mutex::new(win_attribs.into())), decorations: AtomicBool::new(decorations), cursor_state, - inner_rect, transparent, }); diff --git a/third_party/tao/src/platform_impl/macos/window_delegate.rs b/third_party/tao/src/platform_impl/macos/window_delegate.rs index fa6e63085d..001f2ad4da 100644 --- a/third_party/tao/src/platform_impl/macos/window_delegate.rs +++ b/third_party/tao/src/platform_impl/macos/window_delegate.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 use std::{ - f64, ffi::CStr, os::raw::c_void, sync::{Arc, Weak}, @@ -420,6 +419,8 @@ extern "C" fn dragging_entered(this: &Object, _: Sel, sender: id) -> BOOL { use std::path::PathBuf; let pb: Retained = unsafe { msg_send![sender, draggingPasteboard] }; + // Match the legacy filename-list format registered by UnownedWindow::new. + #[allow(deprecated)] let filenames = unsafe { NSPasteboard::propertyListForType(&pb, appkit::NSFilenamesPboardType) }.unwrap(); @@ -454,6 +455,8 @@ extern "C" fn perform_drag_operation(this: &Object, _: Sel, sender: id) -> BOOL use std::path::PathBuf; let pb: Retained = unsafe { msg_send![sender, draggingPasteboard] }; + // Match the legacy filename-list format registered by UnownedWindow::new. + #[allow(deprecated)] let filenames = unsafe { NSPasteboard::propertyListForType(&pb, appkit::NSFilenamesPboardType) }.unwrap();