From 50b11f9755255f898df1c945886bab6220b9f54d Mon Sep 17 00:00:00 2001 From: jd <59188306+zhangjiadi225@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:01:23 +0800 Subject: [PATCH 01/10] fix(chat): don't pin a mid-stream follow-up to the viewport top (#16665) Co-authored-by: Claude Opus 4.8 (1M context) Signed-off-by: jd <59188306+zhangjiadi225@users.noreply.github.com> --- .../__tests__/chatVirtualizerRuntime.test.tsx | 40 +++++++++++++++++++ .../messages/list/chatVirtualizerRuntime.tsx | 18 +++++++++ 2 files changed, 58 insertions(+) diff --git a/src/renderer/components/chat/messages/list/__tests__/chatVirtualizerRuntime.test.tsx b/src/renderer/components/chat/messages/list/__tests__/chatVirtualizerRuntime.test.tsx index ebb7463487..36207f0724 100644 --- a/src/renderer/components/chat/messages/list/__tests__/chatVirtualizerRuntime.test.tsx +++ b/src/renderer/components/chat/messages/list/__tests__/chatVirtualizerRuntime.test.tsx @@ -479,6 +479,46 @@ describe('useChatVirtualizerRuntime', () => { expect(handle!.isAtBottom()).toBe(false) }) + it('does not pin a follow-up steered into a still-streaming turn', () => { + const callbacks: ResizeObserverCallback[] = [] + const restoreResizeObserver = installResizeObserverMock(callbacks) + const raf = installQueuedAnimationFrame() + + try { + let runtime: ChatVirtualizerRuntime | undefined + // Render 1: a turn is already streaming (preserveScrollAnchor is true) and no + // new user-message key has arrived yet. + const view = render( + (runtime = nextRuntime)} /> + ) + const getSpacerHeight = () => runtime!.wrappedItems.find((item) => item.kind === 'spacer')?.height ?? 0 + const scroller = runtime!.scrollerRef.current! + Object.defineProperty(scroller, 'scrollTop', { configurable: true, get: () => 0 }) + Object.defineProperty(scroller, 'scrollHeight', { configurable: true, get: () => 900 + getSpacerHeight() }) + Object.defineProperty(scroller, 'clientHeight', { configurable: true, get: () => 400 }) + runtime!.vlistHandleRef.current = createHandle({ getItemOffset: vi.fn(() => 300) }) + + // Render 2: a queued follow-up is steered into the live turn — a new user + // message (`user-b`) arrives while streaming continues. Because a turn was + // already streaming just before it, the message must NOT pin to the top, so + // no anchor spacer is created (the pin path is what created the instability). + view.rerender( + (runtime = nextRuntime)} + /> + ) + raf.tick() + + expect(getSpacerHeight()).toBe(0) + } finally { + restoreResizeObserver() + raf.restore() + } + }) + it('keeps bottom-follow suppressed while the user is still pinned to the top', () => { let runtime: ChatVirtualizerRuntime | undefined let handle: MessageVirtualListHandle | null = null diff --git a/src/renderer/components/chat/messages/list/chatVirtualizerRuntime.tsx b/src/renderer/components/chat/messages/list/chatVirtualizerRuntime.tsx index 8db23b9259..1241ff3bb7 100644 --- a/src/renderer/components/chat/messages/list/chatVirtualizerRuntime.tsx +++ b/src/renderer/components/chat/messages/list/chatVirtualizerRuntime.tsx @@ -402,6 +402,11 @@ export function useChatVirtualizerRuntime({ const lastScrollToTopKeyRef = useRef(undefined) const didMountForScrollKeyRef = useRef(false) + // The committed `preserveScrollAnchor` from the previous render — i.e. whether a + // turn was already streaming just before the current commit. Lets the pin effect + // tell a fresh idle→new-turn send from a mid-stream insertion. A trailing effect + // (below) keeps it in sync AFTER the pin effect has read the prior value. + const wasStreamingBeforeUserMessageRef = useRef(preserveScrollAnchor) useEffect(() => { const previous = lastScrollToTopKeyRef.current @@ -411,6 +416,12 @@ export function useChatVirtualizerRuntime({ return } if (!scrollToTopKey || scrollToTopKey === previous) return + // A new user message appeared. Only pin it to the top when it STARTS a fresh + // turn (the topic was idle just before it). If a turn was already streaming — + // a queued follow-up steered into the live turn — pinning the new message to + // the top would yank the view and fight the previous assistant's still-growing + // response (the instability we're fixing). Leave scroll to bottom-follow. + if (wasStreamingBeforeUserMessageRef.current) return const idx = findDataIndexByKey(scrollToTopKey) if (idx < 0) return anchor.pinTo(idx) @@ -420,6 +431,13 @@ export function useChatVirtualizerRuntime({ userTookControlRef.current = false }, [anchor, atBottom, findDataIndexByKey, scrollToTopKey]) + // Sync the "was a turn already streaming" marker AFTER the pin effect above has + // read the previous render's value. Runs every commit so the next new-user- + // message commit sees whether streaming was in progress when it arrived. + useEffect(() => { + wasStreamingBeforeUserMessageRef.current = preserveScrollAnchor + }) + // Initial scroll on mount is owned by `useScrollPositionMemory` above: it // restores the saved anchor for this topic, or scrolls to the newest message // when there is nothing to restore. From 4d9ce40efd6efbce7d8a9d267ef1a4f51aa2d836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A7=91=E5=9B=BF=E8=84=91=E8=A2=8B?= <70054568+eeee0717@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:07:10 +0800 Subject: [PATCH 02/10] fix(knowledge-tools): fix kb_manage defer/approval deadlock (#16625) Co-authored-by: Claude Sonnet 5 Signed-off-by: eeee0717 --- .../aiSdk/builtin/KnowledgeManageTool.ts | 11 +++- .../__tests__/KnowledgeManageTool.test.ts | 5 +- .../aiSdk/builtin/__tests__/index.test.ts | 16 +++++ src/main/ai/tools/knowledgeLookup.ts | 19 ++++-- src/shared/ai/__tests__/builtinTools.test.ts | 37 ++++++++++++ src/shared/ai/builtinTools.ts | 59 ++++++++++++++++++- 6 files changed, 138 insertions(+), 9 deletions(-) diff --git a/src/main/ai/tools/adapters/aiSdk/builtin/KnowledgeManageTool.ts b/src/main/ai/tools/adapters/aiSdk/builtin/KnowledgeManageTool.ts index ce8e3f127f..6927f3c8c9 100644 --- a/src/main/ai/tools/adapters/aiSdk/builtin/KnowledgeManageTool.ts +++ b/src/main/ai/tools/adapters/aiSdk/builtin/KnowledgeManageTool.ts @@ -9,9 +9,14 @@ * mutation itself lives in the shared `knowledgeLookup` core so the Claude Code * MCP bridge runs identical logic (gated there by Claude Code's own permission * prompt); this file is just the AI-SDK `tool()` wrapper. + * + * `defer: 'never'` (kept inline, never behind `tool_search`/`tool_invoke`): the same rule + * `mcp/mcpTools.ts` applies to force-prompt MCP tools. Deferring an approval-gated tool would strip + * it from the SDK's tool-set, so the SDK's native `needsApproval` gate never fires and `tool_invoke` + * refuses it too (it never runs an approval-gated tool blind) — an unreachable tool either way. */ -import { KB_MANAGE_TOOL_NAME, kbManageInputSchema, kbManageOutputSchema } from '@shared/ai/builtinTools' +import { KB_MANAGE_TOOL_NAME, kbManageOutputSchema, kbManageStrictInputSchema } from '@shared/ai/builtinTools' import { type InferToolInput, type InferToolOutput, tool } from 'ai' import * as z from 'zod' @@ -31,7 +36,7 @@ const knowledgeManageResultSchema = z.union([kbManageOutputSchema, knowledgeLook const kbManageTool = tool({ description: KNOWLEDGE_MANAGE_DESCRIPTION, - inputSchema: kbManageInputSchema, + inputSchema: kbManageStrictInputSchema, outputSchema: knowledgeManageResultSchema, strict: true, // Every action (add / delete / refresh) modifies the base; gate on explicit user approval. @@ -48,7 +53,7 @@ export function createKbManageToolEntry(): ToolEntry { name: KB_MANAGE_TOOL_NAME, namespace: 'kb', description: 'Add, delete, or re-index documents in a knowledge base (requires approval)', - defer: 'always', + defer: 'never', tool: kbManageTool, applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.assistant?.knowledgeBaseIds?.length ?? 0) > 0 } diff --git a/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/KnowledgeManageTool.test.ts b/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/KnowledgeManageTool.test.ts index 3d1819ff39..435457df33 100644 --- a/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/KnowledgeManageTool.test.ts +++ b/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/KnowledgeManageTool.test.ts @@ -69,7 +69,10 @@ describe('kb_manage', () => { it('builds an entry with the agreed namespace + defer policy and is approval-gated', () => { expect(entry.name).toBe(KB_MANAGE_TOOL_NAME) expect(entry.namespace).toBe('kb') - expect(entry.defer).toBe('always') + // Approval-gated tools must stay inline (never deferred) — see applyDeferExposition/toolInvoke: + // a deferred approval-gated tool is unreachable (stripped from the inline set, and tool_invoke + // refuses to run an approval-gated tool blind). + expect(entry.defer).toBe('never') // Every action mutates the base, so the tool must require user approval. expect(entry.tool.needsApproval).toBe(true) }) diff --git a/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/index.test.ts b/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/index.test.ts index 9389ccdad7..56bb957abf 100644 --- a/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/index.test.ts +++ b/src/main/ai/tools/adapters/aiSdk/builtin/__tests__/index.test.ts @@ -34,4 +34,20 @@ describe('registerBuiltinTools', () => { expect(readFile?.applies?.({ mcpToolIds: new Set(), hasFileAttachments: false })).toBe(false) expect(readFile?.applies?.({ mcpToolIds: new Set(), hasFileAttachments: true })).toBe(true) }) + + it( + 'never defers an approval-gated entry (would strip it from the inline set with no way back — ' + + 'see mcp/mcpTools.ts and toolInvoke.ts for the same rule on MCP force-prompt tools)', + () => { + const reg = new ToolRegistry() + registerBuiltinTools(reg) + for (const entry of reg.getAll()) { + if (entry.tool.needsApproval) { + expect(entry.defer).toBe('never') + } + } + // Sanity: this loop is only meaningful while at least one builtin entry is approval-gated. + expect(reg.getAll().some((e) => e.tool.needsApproval)).toBe(true) + } + ) }) diff --git a/src/main/ai/tools/knowledgeLookup.ts b/src/main/ai/tools/knowledgeLookup.ts index 26b9f5cf3b..1767a2bbf5 100644 --- a/src/main/ai/tools/knowledgeLookup.ts +++ b/src/main/ai/tools/knowledgeLookup.ts @@ -25,7 +25,6 @@ import type { KbGrepOutput, KbListOutput, KbListOutputItem, - KbManageInput, KbManageOutput, KbReadInput, KbReadOutput, @@ -415,6 +414,18 @@ export async function listOrOutlineKnowledge( /** Longest a derived note title (its first line) may be before it is truncated. */ const NOTE_TITLE_MAX_CHARS = 80 +/** kb_manage input shape shared by both callers: MCP omits an unused field, AI-SDK strict passes null. */ +type ManageKnowledgeInput = { + baseId: string + action: 'add' | 'delete' | 'refresh' + type?: 'file' | 'url' | 'note' | null + path?: string | null + url?: string | null + content?: string | null + title?: string | null + conceptIds?: string[] | null +} + /** * Apply a destructive knowledge-base change (add / delete / refresh). Like the * read cores it never throws: an out-of-scope base, a missing required field, an @@ -425,7 +436,7 @@ const NOTE_TITLE_MAX_CHARS = 80 * executes the mutation unconditionally once invoked. */ export async function manageKnowledge( - input: KbManageInput, + input: ManageKnowledgeInput, allowedIds: string[] ): Promise { if (allowedIds.length > 0 && !allowedIds.includes(input.baseId)) { @@ -493,7 +504,7 @@ type AddInputResult = { ok: true; input: KnowledgeAddItemInput; source: string } * invalid value (e.g. a non-absolute file path) is rejected before it reaches the * filesystem boundary. `source` is the identifier reported back as `added`. */ -function buildAddInput(input: KbManageInput): AddInputResult { +function buildAddInput(input: ManageKnowledgeInput): AddInputResult { switch (input.type) { case 'file': { if (!input.path) { @@ -540,7 +551,7 @@ function firstNonEmptyLine(content: string): string | undefined { } /** A note's display source: the caller-supplied title, else its first non-empty line (truncated), else a placeholder. */ -function deriveNoteSource(content: string, title?: string): string { +function deriveNoteSource(content: string, title?: string | null): string { const explicit = title?.trim() if (explicit) return explicit // Truncation here differs by role from deriveSampleSource's note branch (a stored id, plain-clipped; diff --git a/src/shared/ai/__tests__/builtinTools.test.ts b/src/shared/ai/__tests__/builtinTools.test.ts index 532ad4052a..5a0bbda254 100644 --- a/src/shared/ai/__tests__/builtinTools.test.ts +++ b/src/shared/ai/__tests__/builtinTools.test.ts @@ -6,6 +6,8 @@ import { KB_SEARCH_TOOL_NAME, kbListInputSchema, kbListStrictInputSchema, + kbManageInputSchema, + kbManageStrictInputSchema, kbSearchInputSchema, REPORT_ARTIFACTS_DESCRIPTION, REPORT_ARTIFACTS_TOOL_NAME, @@ -62,6 +64,41 @@ describe('builtin tool contracts', () => { expect(kbListInputSchema.safeParse({ query: 'recipes' }).success).toBe(true) }) + it('keeps kb_manage strict-path fields in `required` so strict providers accept the schema', () => { + // Same regression as kb_list above: the AI-SDK path (KnowledgeManageTool) runs strict:true, so + // an all-optional object would serialize `required` away to nothing and a strict OpenAI-compatible + // provider would reject the whole request. The strict variant makes every optional field + // `.nullable()` (null = unused for this action/type) so they all stay in `required`. + const json = z.toJSONSchema(kbManageStrictInputSchema) as { required?: unknown } + + expect(Array.isArray(json.required)).toBe(true) + expect(json.required).toEqual( + expect.arrayContaining(['baseId', 'action', 'type', 'path', 'url', 'content', 'title', 'conceptIds']) + ) + // null is the "unused" signal for every optional field; an explicit all-null payload must still parse. + expect( + kbManageStrictInputSchema.safeParse({ + baseId: 'kb-1', + action: 'delete', + type: null, + path: null, + url: null, + content: null, + title: null, + conceptIds: null + }).success + ).toBe(true) + }) + + it('lets the MCP kb_manage path omit unused fields', () => { + // The Claude Code bridge parses raw args with kbManageInputSchema; an agent may omit every + // field but `baseId`/`action`, so the optional shape must accept that without erroring. + expect(kbManageInputSchema.safeParse({ baseId: 'kb-1', action: 'delete' }).success).toBe(true) + expect(kbManageInputSchema.safeParse({ baseId: 'kb-1', action: 'add', type: 'note', content: 'hi' }).success).toBe( + true + ) + }) + it('validates final report artifacts', () => { const result = reportArtifactsInputSchema.parse({ artifacts: [{ path: 'dist/report.pdf', description: 'Final report' }], diff --git a/src/shared/ai/builtinTools.ts b/src/shared/ai/builtinTools.ts index 62c861a756..0be8ef34ee 100644 --- a/src/shared/ai/builtinTools.ts +++ b/src/shared/ai/builtinTools.ts @@ -279,6 +279,11 @@ export const KB_MANAGE_ADD_TYPES = ['file', 'url', 'note'] as const // One flat object, not a discriminated union: which fields apply depends on `action` // (and, for add, on `type`). The core validates the combination and returns a steer // string on a missing field, so the model gets a clear error rather than a schema reject. +// +// kb_manage is consumed by two paths with conflicting schema needs, same split as kb_list. +// +// MCP / Claude Code bridge (cherryBuiltinTools): the agent parses raw args with this schema and may +// omit any field, so they are `.optional()`. export const kbManageInputSchema = z.object({ baseId: z.string().trim().min(1).describe('ID of the knowledge base to modify — a base id from kb_list.'), action: z @@ -319,6 +324,59 @@ export const kbManageInputSchema = z.object({ ) }) +// AI-SDK path (KnowledgeManageTool) runs with `strict: true` — same `.nullable()` treatment as +// `kbListStrictInputSchema` and for the same reason (an all-optional shape serializes `required` +// away to nothing, which a strict OpenAI-compatible provider rejects). `manageKnowledge` treats +// null (like undefined) as "field not set for this action/type". +export const kbManageStrictInputSchema = z.object({ + baseId: z.string().trim().min(1).describe('ID of the knowledge base to modify — a base id from kb_list.'), + action: z + .enum(KB_MANAGE_ACTIONS) + .describe( + 'add: import a new source (set `type` + its field). delete: remove documents by `conceptIds`. ' + + 'refresh: re-index documents by `conceptIds`. All actions modify the base and require user approval.' + ), + type: z + .enum(KB_MANAGE_ADD_TYPES) + .nullable() + .describe( + 'For action="add" only: the source kind — "file" (set `path`), "url" (set `url`), or "note" (set `content`). ' + + 'Pass null otherwise.' + ), + path: z + .string() + .trim() + .min(1) + .nullable() + .describe('For action="add", type="file": absolute local filesystem path of the file to import. Else null.'), + url: z + .string() + .trim() + .min(1) + .nullable() + .describe('For action="add", type="url": the URL to fetch and index. Else null.'), + content: z + .string() + .min(1) + .nullable() + .describe('For action="add", type="note": the plain-text note content to index. Else null.'), + title: z + .string() + .trim() + .min(1) + .nullable() + .describe( + 'For action="add", type="note": optional display title (defaults to the note\'s first line). Pass null to omit.' + ), + conceptIds: z + .array(z.string().trim().min(1)) + .nullable() + .describe( + 'For action="delete"/"refresh": Concept IDs (the `conceptId` field of a kb_search hit or a kb_list result) ' + + 'to operate on. Else null.' + ) +}) + export const kbManageOutputSchema = z.object({ action: z.enum(KB_MANAGE_ACTIONS), // add: the source identifiers that were imported (one per add call). @@ -330,7 +388,6 @@ export const kbManageOutputSchema = z.object({ notFound: z.array(z.string()).optional() }) -export type KbManageInput = z.infer export type KbManageOutput = z.infer // ── web_search ─────────────────────────────────────────────────── From 5e6d0278b8555aec258b831cda8f696ac0924ee7 Mon Sep 17 00:00:00 2001 From: Pleasure1234 <3196812536@qq.com> Date: Thu, 2 Jul 2026 18:44:31 +0800 Subject: [PATCH 03/10] fix(provider-settings): open pull-reconcile sidebar instead of auto-pulling all models on API key entry (#16632) Signed-off-by: Pleasurecruise <3196812536@qq.com> --- .../useAutoPullOnApiKeyChange.test.tsx | 78 ++++++++++++++++- .../ModelList/useAutoPullOnApiKeyChange.ts | 32 ++++--- .../useProviderAutoModelSync.test.tsx | 84 +++++++++++-------- .../useProviderAutoModelSync.ts | 7 ++ 4 files changed, 154 insertions(+), 47 deletions(-) diff --git a/src/renderer/pages/settings/ProviderSettings/ModelList/__tests__/useAutoPullOnApiKeyChange.test.tsx b/src/renderer/pages/settings/ProviderSettings/ModelList/__tests__/useAutoPullOnApiKeyChange.test.tsx index 9a4d92eec2..8602c8d14f 100644 --- a/src/renderer/pages/settings/ProviderSettings/ModelList/__tests__/useAutoPullOnApiKeyChange.test.tsx +++ b/src/renderer/pages/settings/ProviderSettings/ModelList/__tests__/useAutoPullOnApiKeyChange.test.tsx @@ -122,16 +122,92 @@ describe('useAutoPullOnApiKeyChange', () => { expect(onTrigger).not.toHaveBeenCalled() }) - it('does not fire when no models exist locally yet', () => { + it('fires on first render + key change for API-key providers with no models (auto-sync is disabled)', () => { const onTrigger = vi.fn() useProviderApiKeysMock.mockReturnValue(apiKeys('sk-one')) useModelsMock.mockReturnValue({ models: [] }) const { rerender } = renderHook(() => useAutoPullOnApiKeyChange('openai', onTrigger)) + // First render with keys + no models: opens pull reconcile. + expect(onTrigger).toHaveBeenCalledTimes(1) + useProviderApiKeysMock.mockReturnValue(apiKeys('sk-two')) rerender() + // Key change with no models: opens pull reconcile again. + expect(onTrigger).toHaveBeenCalledTimes(2) + }) + + it('does not fire for non-key providers when no models exist (auto-sync handles bootstrap)', () => { + const onTrigger = vi.fn() + useProviderApiKeysMock.mockReturnValue(apiKeys('sk-one')) + useModelsMock.mockReturnValue({ models: [] }) + useProviderMock.mockReturnValue(providerWithHost('http://localhost:11434', 'ollama')) + + const { rerender } = renderHook(() => useAutoPullOnApiKeyChange('ollama', onTrigger)) + + useProviderApiKeysMock.mockReturnValue(apiKeys('sk-two')) + rerender() + + expect(onTrigger).not.toHaveBeenCalled() + }) + + it('fires on first render for API-key providers with no models and enabled keys', () => { + const onTrigger = vi.fn() + useProviderApiKeysMock.mockReturnValue(apiKeys('sk-one')) + useModelsMock.mockReturnValue({ models: [] }) + + renderHook(() => useAutoPullOnApiKeyChange('openai', onTrigger)) + + expect(onTrigger).toHaveBeenCalledTimes(1) + }) + + it('does not fire on first render for API-key providers that already have models', () => { + const onTrigger = vi.fn() + useProviderApiKeysMock.mockReturnValue(apiKeys('sk-one')) + useModelsMock.mockReturnValue({ models: [{ id: 'openai::gpt-4o' }] }) + + renderHook(() => useAutoPullOnApiKeyChange('openai', onTrigger)) + + expect(onTrigger).not.toHaveBeenCalled() + }) + + it('does not fire on first render for API-key providers without enabled keys', () => { + const onTrigger = vi.fn() + useProviderApiKeysMock.mockReturnValue(emptyApiKeys()) + useModelsMock.mockReturnValue({ models: [] }) + + renderHook(() => useAutoPullOnApiKeyChange('openai', onTrigger)) + + expect(onTrigger).not.toHaveBeenCalled() + }) + + it('does not fire on first render for non-key providers with no models (auto-sync handles it)', () => { + const onTrigger = vi.fn() + useProviderApiKeysMock.mockReturnValue(emptyApiKeys()) + useModelsMock.mockReturnValue({ models: [] }) + useProviderMock.mockReturnValue(providerWithHost('http://localhost:11434', 'ollama')) + + renderHook(() => useAutoPullOnApiKeyChange('ollama', onTrigger)) + + expect(onTrigger).not.toHaveBeenCalled() + }) + + it('waits for models to finish loading before deciding first-render pull reconcile', () => { + const onTrigger = vi.fn() + // api-keys resolve first, models are still loading. + useProviderApiKeysMock.mockReturnValue(apiKeys('sk-one')) + useModelsMock.mockReturnValue({ models: [], isLoading: true }) + + const { rerender } = renderHook(() => useAutoPullOnApiKeyChange('openai', onTrigger)) + + expect(onTrigger).not.toHaveBeenCalled() + + // models resolve later — the provider already has local models. + useModelsMock.mockReturnValue({ models: [{ id: 'openai::gpt-4o' }], isLoading: false }) + rerender() + expect(onTrigger).not.toHaveBeenCalled() }) diff --git a/src/renderer/pages/settings/ProviderSettings/ModelList/useAutoPullOnApiKeyChange.ts b/src/renderer/pages/settings/ProviderSettings/ModelList/useAutoPullOnApiKeyChange.ts index 0aca90c052..da6486fb49 100644 --- a/src/renderer/pages/settings/ProviderSettings/ModelList/useAutoPullOnApiKeyChange.ts +++ b/src/renderer/pages/settings/ProviderSettings/ModelList/useAutoPullOnApiKeyChange.ts @@ -7,16 +7,16 @@ import { providerNeedsApiKeyForModelSync } from './providerModelSyncRequirements /** * Fires `onTrigger` once whenever the provider's enabled API-key fingerprint OR - * its host (endpoint/baseUrl/authType) changes — but only after the first render - * and only when local models already exist (first-time bootstrap is owned by - * `useProviderAutoModelSync`). A pull still requires at least one enabled key - * for providers whose model sync needs API-key auth, so disabling the only key - * never fires for those providers. + * its host (endpoint/baseUrl/authType) changes. For API-key providers this also + * fires on first render when no local models exist (first-time bootstrap uses + * the pull-reconcile sidebar instead of direct auto-sync). A pull still requires + * at least one enabled key for providers whose model sync needs API-key auth, + * so disabling the only key never fires for those providers. */ export function useAutoPullOnApiKeyChange(providerId: string, onTrigger: () => void | Promise) { const { provider } = useProvider(providerId) const { data: apiKeysData } = useProviderApiKeys(providerId) - const { models } = useModels({ providerId }) + const { models, isLoading } = useModels({ providerId }) const enabledKeySignature = useMemo( () => @@ -47,12 +47,12 @@ export function useAutoPullOnApiKeyChange(providerId: string, onTrigger: () => v }, [onTrigger]) useEffect(() => { - // Until provider/api-keys resolve the signature is a cold-cache placeholder; - // recording that as the baseline would make the later undefined→loaded - // transition look like a user-initiated change and auto-fire the pull. - if (!provider || apiKeysData === undefined) return + if (!provider || apiKeysData === undefined || isLoading) return if (lastSignatureRef.current === null) { lastSignatureRef.current = changeSignature + if (models.length === 0 && requiresApiKeyForModelSync && enabledKeySignature) { + void onTriggerRef.current() + } return } if (lastSignatureRef.current === changeSignature) { @@ -61,7 +61,15 @@ export function useAutoPullOnApiKeyChange(providerId: string, onTrigger: () => v lastSignatureRef.current = changeSignature // Key-required providers still need an enabled key; disabling the only key must not fire. if (requiresApiKeyForModelSync && !enabledKeySignature) return - if (models.length === 0) return + if (models.length === 0 && !requiresApiKeyForModelSync) return void onTriggerRef.current() - }, [apiKeysData, changeSignature, enabledKeySignature, models.length, provider, requiresApiKeyForModelSync]) + }, [ + apiKeysData, + changeSignature, + enabledKeySignature, + isLoading, + models.length, + provider, + requiresApiKeyForModelSync + ]) } diff --git a/src/renderer/pages/settings/ProviderSettings/hooks/__tests__/useProviderAutoModelSync.test.tsx b/src/renderer/pages/settings/ProviderSettings/hooks/__tests__/useProviderAutoModelSync.test.tsx index db42cd1ef4..9ddec3ec5a 100644 --- a/src/renderer/pages/settings/ProviderSettings/hooks/__tests__/useProviderAutoModelSync.test.tsx +++ b/src/renderer/pages/settings/ProviderSettings/hooks/__tests__/useProviderAutoModelSync.test.tsx @@ -80,47 +80,58 @@ describe('useProviderAutoModelSync', () => { expect(useProviderModelSyncMock).toHaveBeenCalledWith('openai', { existingModels: [] }) }) - it('enables a disabled provider when auto sync returns at least one model', async () => { - syncProviderModelsMock.mockResolvedValueOnce([{ id: 'openai::gpt-4o' }]) - + it('skips auto sync for API-key providers (uses pull reconcile instead)', async () => { renderHook(() => useProviderAutoModelSync('openai')) - await waitFor(() => expect(updateProviderMock).toHaveBeenCalledWith({ isEnabled: true })) + await waitFor(() => + expect(loggerInfoMock).toHaveBeenCalledWith('Skipping provider auto model sync', { + providerId: 'openai', + reason: 'uses_pull_reconcile' + }) + ) + expect(syncProviderModelsMock).not.toHaveBeenCalled() }) - it('keeps a disabled provider disabled when auto sync returns zero models', async () => { - syncProviderModelsMock.mockResolvedValueOnce([]) - - renderHook(() => useProviderAutoModelSync('openai')) - - await waitFor(() => expect(syncProviderModelsMock).toHaveBeenCalledTimes(1)) - await Promise.resolve() - expect(updateProviderMock).not.toHaveBeenCalled() - }) - - it('does not patch an already enabled provider after successful auto sync', async () => { + it('auto syncs for non-key providers (e.g. Ollama) when models are missing', async () => { + syncProviderModelsMock.mockResolvedValueOnce([{ id: 'ollama::llama3.2' }]) useProviderMock.mockReturnValue({ provider: { - id: 'openai', - isEnabled: true, - defaultChatEndpoint: 'openai_chat_completions', + id: 'ollama', + isEnabled: false, + defaultChatEndpoint: 'ollama_chat', endpointConfigs: { - openai_chat_completions: { baseUrl: 'https://api.openai.com/v1' } + ollama_chat: { baseUrl: 'http://localhost:11434' } } }, updateProvider: updateProviderMock }) - syncProviderModelsMock.mockResolvedValueOnce([{ id: 'openai::gpt-4o' }]) + useProviderApiKeysMock.mockReturnValue({ + data: { keys: [] } + }) - renderHook(() => useProviderAutoModelSync('openai')) + renderHook(() => useProviderAutoModelSync('ollama')) - await waitFor(() => expect(syncProviderModelsMock).toHaveBeenCalledTimes(1)) - await Promise.resolve() - expect(updateProviderMock).not.toHaveBeenCalled() + await waitFor(() => expect(updateProviderMock).toHaveBeenCalledWith({ isEnabled: true })) }) - it('syncs only once for the same initial eligible configuration', async () => { - const { rerender } = renderHook(() => useProviderAutoModelSync('openai')) + it('syncs only once for the same initial eligible configuration (non-key provider)', async () => { + useProviderMock.mockReturnValue({ + provider: { + id: 'ollama', + isEnabled: false, + defaultChatEndpoint: 'ollama_chat', + endpointConfigs: { + ollama_chat: { baseUrl: 'http://localhost:11434' } + } + }, + updateProvider: updateProviderMock + }) + useProviderApiKeysMock.mockReturnValue({ + data: { keys: [] } + }) + syncProviderModelsMock.mockResolvedValue([]) + + const { rerender } = renderHook(() => useProviderAutoModelSync('ollama')) await waitFor(() => expect(syncProviderModelsMock).toHaveBeenCalledTimes(1)) @@ -154,19 +165,18 @@ describe('useProviderAutoModelSync', () => { expect(syncProviderModelsMock).not.toHaveBeenCalled() }) - it('logs auto sync failures and allows retrying when the same signature becomes eligible again', async () => { - const syncError = new Error('sync down') - syncProviderModelsMock.mockRejectedValueOnce(syncError).mockResolvedValueOnce([]) - + it('skips auto sync for API-key provider even after key rotation (pull reconcile handles it)', async () => { const { rerender } = renderHook(() => useProviderAutoModelSync('openai')) + // First render with keys → uses_pull_reconcile await waitFor(() => - expect(loggerErrorMock).toHaveBeenCalledWith('Provider auto model sync failed', { + expect(loggerInfoMock).toHaveBeenCalledWith('Skipping provider auto model sync', { providerId: 'openai', - error: syncError + reason: 'uses_pull_reconcile' }) ) + // Keys removed useProviderApiKeysMock.mockReturnValue({ data: { keys: [] } }) @@ -179,12 +189,18 @@ describe('useProviderAutoModelSync', () => { }) ) + // Keys restored — still uses pull reconcile, no direct sync useProviderApiKeysMock.mockReturnValue({ data: { keys: [{ id: 'key-1', key: 'sk-test', isEnabled: true }] } }) rerender() - await waitFor(() => expect(syncProviderModelsMock).toHaveBeenCalledTimes(2)) - expect(updateProviderMock).not.toHaveBeenCalled() + await waitFor(() => + expect(loggerInfoMock).toHaveBeenCalledWith('Skipping provider auto model sync', { + providerId: 'openai', + reason: 'uses_pull_reconcile' + }) + ) + expect(syncProviderModelsMock).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/pages/settings/ProviderSettings/hooks/providerSetting/useProviderAutoModelSync.ts b/src/renderer/pages/settings/ProviderSettings/hooks/providerSetting/useProviderAutoModelSync.ts index 3f672c535e..81dc2040d8 100644 --- a/src/renderer/pages/settings/ProviderSettings/hooks/providerSetting/useProviderAutoModelSync.ts +++ b/src/renderer/pages/settings/ProviderSettings/hooks/providerSetting/useProviderAutoModelSync.ts @@ -69,6 +69,13 @@ export function useProviderAutoModelSync(providerId: string) { } as const } + if (requiresApiKeyForModelSync) { + return { + shouldSync: false, + reason: 'uses_pull_reconcile' + } as const + } + if (!initialModelSyncSignature) { return { shouldSync: false, From b65d85905de4d58062d948b9bc57129c0ae4ad2e Mon Sep 17 00:00:00 2001 From: jd <59188306+zhangjiadi225@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:45:52 +0800 Subject: [PATCH 04/10] feat(resource-list): tag grouping & single-tag UI (#16561) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What this PR does > **Stacked PR.** Base is `jd/resource-list-config`, not `main` — this builds on the old-view resource-list rails from that branch. It should merge after its base. Before this PR: - The old-view assistant rail showed a flat assistant list with no way to group it. - A resource (assistant) could carry multiple tags, and the resource-card tag picker rendered each tag with a multi-select checkbox. After this PR: - The old-view assistant rail has a right-click / "more" menu toggle that groups the non-pinned assistants into collapsible per-tag sections (the "已固定" pinned section stays on top). The state reuses the existing `assistant.tab.sort_type` preference (`list` | `tags`). Drag-reorder is disabled while grouping. Untagged assistants collapse under the existing "未分组" section. Agents and the topic list are untouched. - A resource now carries a single tag. The resource-card tag picker drops the checkbox and shows a single trailing check on the selected tag (`menuitemradio`), matching the single-select tag combobox in the edit dialog. Fixes # ### Why we need it and why it was done in this way The following tradeoffs were made: - Tag grouping reuses the existing collapsible `SectionHeader` primitive instead of adding a new visual, so it only allows expand/collapse and matches the current rail look. - Grouping reuses the v1 `assistant.tab.sort_type` preference (cross-window, persisted) rather than introducing a new cache key. - The single-tag model keeps the backend tag relation many-to-many and only sends a 0/1-element array at the UI boundary. The following alternatives were considered: - A new `GroupHeader` (with chevron/actions) for tags — rejected to keep "expand/collapse only" and reuse the existing section visual. - Storing the grouping toggle in the Cache layer — rejected in favor of the existing Preference, which is the idiomatic home for a synced UI setting. Links to places where the discussion took place: N/A ### Breaking changes Collapsing assistant tags from many to one is a behavior change, but no multi-tag assistant data exists at this pre-release stage, so there is no user-visible data impact. ### Special notes for your reviewer - Intentionally stacked on `jd/resource-list-config`; the diff is the three commits on top of that branch (single-tag collapse, tag grouping, single-select tag picker visual). Opened as **draft** since it cannot merge before its base. ### Checklist This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR. Approvers are expected to review this list. - [x] Branch: This PR targets the correct branch — stacked on `jd/resource-list-config` (its base feature branch), not `main` - [x] PR: The PR description is expressive enough and will help future contributors - [x] Code: [Write code that humans can understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans) and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle) - [x] Refactor: You have [left the code cleaner than you found it (Boy Scout Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html) - [x] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com) was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior. - [x] Self-review: I have reviewed my own code (e.g., via [`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`, or GitHub UI) before requesting review from others ### Release note ```release-note Group assistants by tag in the chat sidebar (old view) via the assistant right-click menu, and use a single tag per assistant with a single-select tag picker. ``` --------- Signed-off-by: jd <59188306+zhangjiadi225@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) --- .../variants/AssistantResourceList.tsx | 25 +++- .../resources/variants/ResourceEntityRail.tsx | 79 ++++++++--- .../EntityResourceListActions.test.tsx | 38 ++++++ .../__tests__/ResourceEntityRail.test.tsx | 90 ++++++++++++- .../components/resource/AssistantSelector.tsx | 11 +- .../resource/ResourceSelectorShell.tsx | 36 ++--- .../__tests__/AssistantSelector.test.tsx | 2 +- .../__tests__/ResourceSelectorShell.test.tsx | 32 ++++- .../dialogs/__tests__/EditDialogs.test.tsx | 60 ++++----- .../dialogs/components/TagSelector.tsx | 95 +++++++++----- .../dialogs/edit/AssistantEditDialog.tsx | 8 +- .../dialogs/form/__tests__/assistant.test.ts | 22 ++-- .../resource/dialogs/form/assistant.ts | 24 ++-- src/renderer/i18n/locales/en-us.json | 6 +- src/renderer/i18n/locales/zh-cn.json | 2 + src/renderer/i18n/locales/zh-tw.json | 2 + src/renderer/pages/library/LibraryPage.tsx | 6 +- .../library/__tests__/LibraryPage.test.tsx | 7 - .../__tests__/assistantAdapter.test.ts | 4 +- .../library/adapters/assistantAdapter.ts | 3 +- src/renderer/pages/library/adapters/types.ts | 2 +- .../pages/library/list/ResourceCardMenu.tsx | 115 ++++++++++------ .../pages/library/list/ResourceCards.tsx | 31 ++--- .../pages/library/list/ResourceGrid.tsx | 9 +- .../list/__tests__/ResourceGrid.test.tsx | 124 +++++++++++------- .../list/__tests__/useResourceLibrary.test.ts | 5 +- .../pages/library/list/useResourceLibrary.ts | 23 ++-- src/renderer/pages/library/types.ts | 9 +- .../utils/__tests__/assistantTransfer.test.ts | 7 +- .../pages/library/utils/assistantTransfer.ts | 11 +- 30 files changed, 569 insertions(+), 319 deletions(-) diff --git a/src/renderer/components/chat/resources/variants/AssistantResourceList.tsx b/src/renderer/components/chat/resources/variants/AssistantResourceList.tsx index 81ea1a457a..6a2ecff39b 100644 --- a/src/renderer/components/chat/resources/variants/AssistantResourceList.tsx +++ b/src/renderer/components/chat/resources/variants/AssistantResourceList.tsx @@ -1,3 +1,4 @@ +import { usePreference } from '@data/hooks/usePreference' import { loggerService } from '@logger' import type { ResolvedAction } from '@renderer/components/chat/actions/actionTypes' import { @@ -17,7 +18,7 @@ import { usePins } from '@renderer/hooks/usePins' import { mapApiTopicToRendererTopic, useTopicMutations } from '@renderer/hooks/useTopic' import type { Topic } from '@renderer/types/topic' import { formatErrorMessageWithPrefix } from '@renderer/utils/error' -import { Bot, Edit3, PinIcon, PinOffIcon, Plus, Trash2 } from 'lucide-react' +import { Bot, Edit3, PinIcon, PinOffIcon, Plus, Tags, Trash2 } from 'lucide-react' import { useCallback, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -27,6 +28,7 @@ const logger = loggerService.withContext('AssistantResourceList') const ASSISTANT_ENTITY_EDIT_ACTION_ID = 'assistant-entity.edit' const ASSISTANT_ENTITY_TOGGLE_PIN_ACTION_ID = 'assistant-entity.toggle-pin' +const ASSISTANT_ENTITY_TOGGLE_TAG_GROUPING_ACTION_ID = 'assistant-entity.toggle-tag-grouping' const ASSISTANT_ENTITY_DELETE_ACTION_ID = 'assistant-entity.delete' type AssistantResourceListProps = { @@ -52,6 +54,8 @@ export function AssistantResourceList({ onActiveAssistantDeleted }: AssistantResourceListProps) { const { t } = useTranslation() + const [assistantSortType, setAssistantSortType] = usePreference('assistant.tab.sort_type') + const isTagGrouping = assistantSortType === 'tags' const { assistants, isLoading: isAssistantsLoading, @@ -95,6 +99,7 @@ export function AssistantResourceList({ name: assistant.name, orderKey: assistant.orderKey, pinned: assistantPinnedIdSet.has(assistant.id), + tag: assistant.tags?.[0]?.name, icon: assistant.emoji ? ( ) : ( @@ -227,6 +232,15 @@ export function AssistantResourceList({ availability: { visible: true, enabled: !isAssistantPinActionDisabled }, children: [] }, + { + id: ASSISTANT_ENTITY_TOGGLE_TAG_GROUPING_ACTION_ID, + label: isTagGrouping ? t('assistants.tags.ungroup') : t('assistants.tags.group_by'), + icon: , + order: 25, + danger: false, + availability: { visible: true, enabled: true }, + children: [] + }, { id: ASSISTANT_ENTITY_DELETE_ACTION_ID, label: t('assistants.delete.title'), @@ -239,7 +253,7 @@ export function AssistantResourceList({ } ] }, - [assistantPinnedIdSet, deletingAssistantId, isAssistantPinActionDisabled, t] + [assistantPinnedIdSet, deletingAssistantId, isAssistantPinActionDisabled, isTagGrouping, t] ) const handleContextMenuAction = useCallback( @@ -252,11 +266,15 @@ export function AssistantResourceList({ void handleToggleAssistantPin(item.id) return } + if (action.id === ASSISTANT_ENTITY_TOGGLE_TAG_GROUPING_ACTION_ID) { + void setAssistantSortType(isTagGrouping ? 'list' : 'tags') + return + } if (action.id === ASSISTANT_ENTITY_DELETE_ACTION_ID) { void handleDeleteAssistant(item.id) } }, - [handleDeleteAssistant, handleToggleAssistantPin, openAssistantEditor] + [handleDeleteAssistant, handleToggleAssistantPin, isTagGrouping, openAssistantEditor, setAssistantSortType] ) return ( @@ -268,6 +286,7 @@ export function AssistantResourceList({ status={listStatus} ariaLabel={t('assistants.abbr')} defaultGroupLabel={t('assistants.abbr')} + groupByTag={isTagGrouping} addIcon={} addLabel={t('chat.add.assistant.title')} onAdd={onAddAssistant ?? (() => onStartDraftAssistant(null))} diff --git a/src/renderer/components/chat/resources/variants/ResourceEntityRail.tsx b/src/renderer/components/chat/resources/variants/ResourceEntityRail.tsx index 6374591b04..5d238b3bed 100644 --- a/src/renderer/components/chat/resources/variants/ResourceEntityRail.tsx +++ b/src/renderer/components/chat/resources/variants/ResourceEntityRail.tsx @@ -26,6 +26,8 @@ export type ResourceEntityRailItem = { * It does not affect visibility — an entity with no resources stays hidden whether pinned or not. */ pinned?: boolean + /** Single user tag name. Only consulted when the rail runs with `groupByTag`; undefined → "未分组". */ + tag?: string } // Pinned entities float into a "已固定" section at the top; the rest sit under the "助手" / "智能体" @@ -36,6 +38,27 @@ const ENTITY_RAIL_PINNED_SECTION_ID = 'resource-entity-rail:section:pinned' const ENTITY_RAIL_DEFAULT_SECTION_ID = 'resource-entity-rail:section:default' const ENTITY_RAIL_PINNED_GROUP_ID = 'resource-entity-rail:group:pinned' const ENTITY_RAIL_DEFAULT_GROUP_ID = 'resource-entity-rail:group:default' +// When `groupByTag` is on, each tag name becomes its own collapsible section below the pinned one; +// untagged entities collapse together under a distinct internal bucket. +const ENTITY_RAIL_TAG_SECTION_PREFIX = 'resource-entity-rail:section:' +const ENTITY_RAIL_TAG_GROUP_PREFIX = 'resource-entity-rail:group:' +const ENTITY_RAIL_UNTAGGED_KEY = JSON.stringify(['untagged']) + +function getEntityRailTagBucketKey(tag: string | undefined) { + return tag ? JSON.stringify(['tag', tag]) : ENTITY_RAIL_UNTAGGED_KEY +} + +function getEntityRailTagGroupingRank(item: ResourceEntityRailItem) { + if (item.pinned) return 0 + return item.tag ? 2 : 1 +} + +function sortEntityRailItemsForTagGrouping(items: readonly T[]): T[] { + return items + .map((item, index) => ({ item, index, rank: getEntityRailTagGroupingRank(item) })) + .sort((a, b) => a.rank - b.rank || a.index - b.index) + .map(({ item }) => item) +} export type ResourceEntityRailProps = { addIcon?: ReactNode @@ -43,6 +66,12 @@ export type ResourceEntityRailProps readonly ResolvedAction[] listRef?: RefObject @@ -82,6 +111,7 @@ export function ResourceEntityRail) { const { t } = useTranslation() + // Tag grouping splits the flat order across sections, so dragging an item between tags would have + // no meaningful `orderKey` target — disable reorder entirely while grouping by tag. + const reorderEnabled = !!onReorder && !groupByTag const fallbackListRef = useRef(null) const effectiveListRef = listRef ?? fallbackListRef const runContextMenuAction = useCallback( @@ -173,23 +206,39 @@ export function ResourceEntityRail emptyFallback ??
, [emptyFallback]) + const providerItems = useMemo( + () => (groupByTag ? sortEntityRailItemsForTagGrouping(items) : items), + [groupByTag, items] + ) // Collapsible sections matching the modern layout's left assistant/agent layout (minus the nested // topics/sessions): pinned entities float into "已固定" at the top, the rest sit under the // "助手" / "智能体" section below. Section headers stay flush-left; the entity rows keep their // avatar and read as indented beneath. The single-section case (nothing pinned) renders the flat // list with no header, exactly like the modern layout. const sectionBy = useMemo<(item: T) => ResourceListSection>( - () => (item) => - item.pinned - ? { id: ENTITY_RAIL_PINNED_SECTION_ID, label: t('selector.common.pinned_title') } - : { id: ENTITY_RAIL_DEFAULT_SECTION_ID, label: defaultGroupLabel ?? '' }, - [defaultGroupLabel, t] + () => (item) => { + if (item.pinned) return { id: ENTITY_RAIL_PINNED_SECTION_ID, label: t('selector.common.pinned_title') } + if (groupByTag) { + const tagBucketKey = getEntityRailTagBucketKey(item.tag) + return item.tag + ? { id: `${ENTITY_RAIL_TAG_SECTION_PREFIX}${tagBucketKey}`, label: item.tag } + : { id: `${ENTITY_RAIL_TAG_SECTION_PREFIX}${tagBucketKey}`, label: t('assistants.tags.untagged') } + } + return { id: ENTITY_RAIL_DEFAULT_SECTION_ID, label: defaultGroupLabel ?? '' } + }, + [defaultGroupLabel, groupByTag, t] ) // Header-less groups (one per section, distinct ids) keep entity avatars visible and stop - // drag-reorder from crossing the pinned/non-pinned boundary. + // drag-reorder from crossing the pinned/non-pinned (or per-tag) boundary. const groupBy = useMemo<(item: T) => ResourceListGroup>( - () => (item) => ({ id: item.pinned ? ENTITY_RAIL_PINNED_GROUP_ID : ENTITY_RAIL_DEFAULT_GROUP_ID, label: '' }), - [] + () => (item) => { + if (item.pinned) return { id: ENTITY_RAIL_PINNED_GROUP_ID, label: '' } + if (groupByTag) { + return { id: `${ENTITY_RAIL_TAG_GROUP_PREFIX}${getEntityRailTagBucketKey(item.tag)}`, label: '' } + } + return { id: ENTITY_RAIL_DEFAULT_GROUP_ID, label: '' } + }, + [groupByTag] ) // Alias the compound provider to a local before rendering — same pattern as TopicResourceList/SessionResourceList. @@ -200,7 +249,7 @@ export function ResourceEntityRail !!onReorder && !item.pinned} + canDragItem={({ item }) => reorderEnabled && !item.pinned} canDropItem={({ activeItem, targetGroupId }) => - !!onReorder && !activeItem.pinned && targetGroupId !== ENTITY_RAIL_PINNED_GROUP_ID + reorderEnabled && !activeItem.pinned && targetGroupId !== ENTITY_RAIL_PINNED_GROUP_ID } - onReorder={onReorder}> + onReorder={reorderEnabled ? onReorder : undefined}> listRef={effectiveListRef} - draggable={!!onReorder} + draggable={reorderEnabled} ariaLabel={ariaLabel} virtualClassName="pt-1 pb-3" errorFallback={} diff --git a/src/renderer/components/chat/resources/variants/__tests__/EntityResourceListActions.test.tsx b/src/renderer/components/chat/resources/variants/__tests__/EntityResourceListActions.test.tsx index 1230fb35a1..416766472b 100644 --- a/src/renderer/components/chat/resources/variants/__tests__/EntityResourceListActions.test.tsx +++ b/src/renderer/components/chat/resources/variants/__tests__/EntityResourceListActions.test.tsx @@ -17,12 +17,21 @@ const agentDataMocks = vi.hoisted(() => ({ refetchAgents: vi.fn() })) +const preferenceMocks = vi.hoisted(() => ({ + sortType: 'list' as 'list' | 'tags', + setSortType: vi.fn() +})) + vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) +vi.mock('@data/hooks/usePreference', () => ({ + usePreference: () => [preferenceMocks.sortType, preferenceMocks.setSortType] +})) + vi.mock('@logger', () => ({ loggerService: { withContext: () => ({ @@ -192,6 +201,8 @@ vi.mock('@renderer/utils/error', () => ({ describe('classic layout entity resource list actions', () => { beforeEach(() => { + preferenceMocks.sortType = 'list' + preferenceMocks.setSortType.mockClear() assistantDataMocks.deleteAssistant.mockResolvedValue(undefined) assistantDataMocks.refreshTopics.mockResolvedValue(undefined) assistantDataMocks.refetchAssistants.mockResolvedValue(undefined) @@ -239,6 +250,33 @@ describe('classic layout entity resource list actions', () => { expect(onStartDraftAssistant).not.toHaveBeenCalled() }) + it('toggles assistant tag grouping from the context menu (list → tags)', () => { + render( + + ) + + // sort_type === 'list' → the menu offers "group by tag". + const menu = screen.getByTestId('assistant-1-context-menu') + expect(menu).toHaveTextContent('assistants.tags.group_by') + expect(menu).not.toHaveTextContent('assistants.tags.ungroup') + + fireEvent.click(screen.getAllByRole('button', { name: 'assistants.tags.group_by' })[0]) + expect(preferenceMocks.setSortType).toHaveBeenCalledWith('tags') + }) + + it('offers turning tag grouping off when already grouping (tags → list)', () => { + preferenceMocks.sortType = 'tags' + + render( + + ) + + expect(screen.getByTestId('assistant-1-context-menu')).toHaveTextContent('assistants.tags.ungroup') + + fireEvent.click(screen.getAllByRole('button', { name: 'assistants.tags.ungroup' })[0]) + expect(preferenceMocks.setSortType).toHaveBeenCalledWith('list') + }) + it('uses delete-agent actions for the classic layout agent context and more menus', async () => { const onStartMissingAgentDraft = vi.fn() const onActiveAgentDeleted = vi.fn() diff --git a/src/renderer/components/chat/resources/variants/__tests__/ResourceEntityRail.test.tsx b/src/renderer/components/chat/resources/variants/__tests__/ResourceEntityRail.test.tsx index fe8d60fdc1..440b5c0ffa 100644 --- a/src/renderer/components/chat/resources/variants/__tests__/ResourceEntityRail.test.tsx +++ b/src/renderer/components/chat/resources/variants/__tests__/ResourceEntityRail.test.tsx @@ -65,7 +65,8 @@ vi.mock('@renderer/components/VirtualList', () => { return rows } - const GroupedVirtualList = ({ + const GroupedVirtualListContent = ({ + dragEnabled, ref, className, groups, @@ -91,6 +92,7 @@ vi.mock('@renderer/components/VirtualList', () => { }} role={role} className={className} + data-draggable={dragEnabled ? 'true' : 'false'} {...scrollerProps}> {rows.map((row, index) => { if (row.type === 'group-header') { @@ -114,8 +116,8 @@ vi.mock('@renderer/components/VirtualList', () => { return { buildGroupedVirtualRows, DynamicVirtualList: () => null, - GroupedSortableVirtualList: GroupedVirtualList, - GroupedVirtualList + GroupedSortableVirtualList: (props) => , + GroupedVirtualList: (props) => } }) @@ -261,6 +263,88 @@ describe('ResourceEntityRail', () => { expect(screen.getByTestId('assistant-a-icon')).toBeInTheDocument() }) + it('groups non-pinned entities into per-tag sections while keeping pinned on top', () => { + render( + , pinned: true, tag: 'work' }, + { id: 'work-a', name: 'Work A', icon: , tag: 'work' }, + { id: 'home-a', name: 'Home A', icon: , tag: 'home' }, + { id: 'loose', name: 'Loose', icon: , tag: undefined } + ]} + variant="assistant" + onAdd={vi.fn()} + onReorder={vi.fn()} + onSelect={vi.fn()} + /> + ) + + // Pinned section stays on top; non-pinned entities split into tag sections + an untagged section. + expect(screen.getByText('selector.common.pinned_title')).toBeInTheDocument() + expect(screen.getByText('work')).toBeInTheDocument() + expect(screen.getByText('home')).toBeInTheDocument() + expect(screen.getByText('assistants.tags.untagged')).toBeInTheDocument() + expect( + Array.from( + screen.getByRole('listbox', { name: 'Assistants list' }).querySelectorAll('button[aria-expanded]') + ).map((header) => header.textContent) + ).toEqual(['selector.common.pinned_title', 'assistants.tags.untagged', 'work', 'home']) + // A pinned entity stays under the pinned section even though it carries a tag — its tag must not + // spawn a second "work" header. + expect(screen.getAllByText('work')).toHaveLength(1) + // The flat default "Assistants" header never appears while grouping by tag. + expect(screen.queryByText('Assistants')).not.toBeInTheDocument() + expect(screen.getByTestId('work-a-icon')).toBeInTheDocument() + expect(screen.getByRole('listbox', { name: 'Assistants list' })).toHaveAttribute('data-draggable', 'false') + }) + + it('keeps a real tag named like the untagged sentinel separate from untagged entities', () => { + render( + , tag: '__untagged__' }, + { id: 'loose', name: 'Loose', icon: , tag: undefined } + ]} + variant="assistant" + onAdd={vi.fn()} + onSelect={vi.fn()} + /> + ) + + expect(screen.getByText('__untagged__')).toBeInTheDocument() + expect(screen.getByText('assistants.tags.untagged')).toBeInTheDocument() + }) + + it('ignores entity tags when groupByTag is off', () => { + render( + , tag: 'work' }, + { id: 'home-a', name: 'Home A', icon: , tag: 'home' } + ]} + variant="assistant" + onAdd={vi.fn()} + onReorder={vi.fn()} + onSelect={vi.fn()} + /> + ) + + expect(screen.queryByText('work')).not.toBeInTheDocument() + expect(screen.queryByText('home')).not.toBeInTheDocument() + expect(screen.queryByText('assistants.tags.untagged')).not.toBeInTheDocument() + }) + it('renders a flat list with no section header when nothing is pinned', () => { render( /** * Row shape the selector operates on — derived from the Assistant DTO. `selectionType: 'item'` * returns values of this shape (not the raw Assistant) so the selector never leaks DB columns the - * caller didn't ask about. User tag names may be present so the selector can filter by assistant - * tags. + * caller didn't ask about. A user tag name may be present so the selector can filter by assistant + * tag. */ export type AssistantSelectorItem = ResourceSelectorShellItem @@ -118,7 +118,7 @@ export function AssistantSelector(props: AssistantSelectorProps) { name: a.name, emoji: a.emoji, description: a.description, - tags: (a.tags ?? []).map((tag) => tag.name) + tag: a.tags?.[0]?.name })), [data] ) @@ -126,7 +126,8 @@ export function AssistantSelector(props: AssistantSelectorProps) { const tags = useMemo(() => { const byName = new Map() for (const assistant of data?.items ?? []) { - for (const tag of assistant.tags ?? []) { + const tag = assistant.tags?.[0] + if (tag) { if (!byName.has(tag.name)) { byName.set(tag.name, tag.color ?? undefined) } @@ -200,7 +201,7 @@ export function AssistantSelector(props: AssistantSelectorProps) { name: created.name, emoji: created.emoji, description: created.description, - tags: (created.tags ?? []).map((tag) => tag.name) + tag: created.tags?.[0]?.name }) } else { props.onChange(created.id) diff --git a/src/renderer/components/resource/ResourceSelectorShell.tsx b/src/renderer/components/resource/ResourceSelectorShell.tsx index 2ac3112bb2..b572b91b49 100644 --- a/src/renderer/components/resource/ResourceSelectorShell.tsx +++ b/src/renderer/components/resource/ResourceSelectorShell.tsx @@ -30,7 +30,7 @@ export type ResourceSelectorShellItem = { name: string emoji?: string description?: string - tags?: string[] + tag?: string disabled?: boolean } @@ -273,7 +273,7 @@ export function ResourceSelectorShell(props ) const [searchValue, setSearchValue] = useState('') - const [selectedTagIds, setSelectedTagIds] = useState([]) + const [selectedTagName, setSelectedTagName] = useState(null) const listboxId = useId() const listRef = useRef(null) const searchInputRef = useRef(null) @@ -320,9 +320,8 @@ export function ResourceSelectorShell(props const { pinnedItems, unpinnedItems } = useMemo(() => { let filtered = items - if (selectedTagIds.length > 0) { - const wanted = new Set(selectedTagIds) - filtered = filtered.filter((item) => item.tags?.some((tag) => wanted.has(tag))) + if (selectedTagName) { + filtered = filtered.filter((item) => item.tag === selectedTagName) } const query = searchValue.trim().toLowerCase() @@ -338,7 +337,7 @@ export function ResourceSelectorShell(props const unpinned = filtered.filter((item) => !pinnedSet.has(item.id)) const pinnedOrdered = pinnedIds.map((id) => pinned.find((item) => item.id === id)).filter(Boolean) as T[] return { pinnedItems: pinnedOrdered, unpinnedItems: unpinned } - }, [items, pinnedIds, pinnedSet, searchValue, selectedTagIds]) + }, [items, pinnedIds, pinnedSet, searchValue, selectedTagName]) const sections = useMemo[]>(() => { const nextSections: ResourceSelectorSection[] = [] @@ -569,18 +568,14 @@ export function ResourceSelectorShell(props <> {labels.tagFilter ? {labels.tagFilter} : null} {tagOptions.map((tag) => { - const active = selectedTagIds.includes(tag.name) + const active = selectedTagName === tag.name return ( - setSelectedTagIds((prev) => - prev.includes(tag.name) ? prev.filter((value) => value !== tag.name) : [...prev, tag.name] - ) - } + onClick={() => setSelectedTagName((prev) => (prev === tag.name ? null : tag.name))} /> ) })} @@ -607,16 +602,13 @@ export function ResourceSelectorShell(props {fallbackIcon} ) : null - const trailing = - item.tags && item.tags.length > 0 ? ( -
- {item.tags.map((tag) => ( - - ))} -
- ) : null + const trailing = item.tag ? ( +
+ +
+ ) : null return (
diff --git a/src/renderer/components/resource/__tests__/AssistantSelector.test.tsx b/src/renderer/components/resource/__tests__/AssistantSelector.test.tsx index 0da4301a32..c22e775c8e 100644 --- a/src/renderer/components/resource/__tests__/AssistantSelector.test.tsx +++ b/src/renderer/components/resource/__tests__/AssistantSelector.test.tsx @@ -92,7 +92,7 @@ vi.mock('react-i18next', async (importOriginal) => { 'library.config.basic.model_pick': 'Pick model', 'library.config.basic.model_not_found': 'Model {{id}} is unavailable.', 'library.config.basic.tag_empty': 'No tags', - 'library.config.basic.tag_placeholder': 'Select tags', + 'library.config.basic.tag_placeholder': 'Select tag', 'library.config.basic.tag_search': 'Search tags', 'library.config.prompt.label': 'Prompt', 'library.config.prompt.placeholder': 'Tell this assistant how to respond', diff --git a/src/renderer/components/resource/__tests__/ResourceSelectorShell.test.tsx b/src/renderer/components/resource/__tests__/ResourceSelectorShell.test.tsx index dc4161acf9..7ed7f6b014 100644 --- a/src/renderer/components/resource/__tests__/ResourceSelectorShell.test.tsx +++ b/src/renderer/components/resource/__tests__/ResourceSelectorShell.test.tsx @@ -636,7 +636,7 @@ describe('ResourceSelectorShell', () => { describe('edit button', () => { it('places edit and pin together in the row action area', () => { - const taggedItems: Item[] = [{ ...ITEMS[0], tags: ['Cherry', 'DEV'] }, ...ITEMS.slice(1)] + const taggedItems: Item[] = [{ ...ITEMS[0], tag: 'Cherry' }, ...ITEMS.slice(1)] render( { expect(screen.queryByRole('option', { name: /Alpha/ })).toBeInTheDocument() expect(screen.queryByRole('option', { name: /Beta/ })).not.toBeInTheDocument() }) + + it('uses a single active tag filter at a time', () => { + render( + Open} + items={[ + { ...ITEMS[0], tag: 'Cherry' }, + { ...ITEMS[1], tag: 'DEV' }, + { ...ITEMS[2], tag: 'Cherry' } + ]} + tags={['Cherry', 'DEV']} + pinnedIds={[]} + onTogglePin={vi.fn()} + labels={LABELS} + value={null} + onChange={vi.fn()} + /> + ) + openPopover() + + fireEvent.click(screen.getByRole('button', { name: 'Cherry' })) + expect(screen.queryByRole('option', { name: /Alpha/ })).toBeInTheDocument() + expect(screen.queryByRole('option', { name: /Gamma/ })).toBeInTheDocument() + expect(screen.queryByRole('option', { name: /Beta/ })).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'DEV' })) + expect(screen.queryByRole('option', { name: /Beta/ })).toBeInTheDocument() + expect(screen.queryByRole('option', { name: /Alpha/ })).not.toBeInTheDocument() + expect(screen.queryByRole('option', { name: /Gamma/ })).not.toBeInTheDocument() + }) }) }) diff --git a/src/renderer/components/resource/dialogs/__tests__/EditDialogs.test.tsx b/src/renderer/components/resource/dialogs/__tests__/EditDialogs.test.tsx index cad37f70df..3b7dbe209c 100644 --- a/src/renderer/components/resource/dialogs/__tests__/EditDialogs.test.tsx +++ b/src/renderer/components/resource/dialogs/__tests__/EditDialogs.test.tsx @@ -295,8 +295,9 @@ vi.mock('react-i18next', async (importOriginal) => { 'library.config.basic.model_not_found': 'Model {{id}} is unavailable.', 'library.config.basic.precise': 'Precise', 'library.config.basic.stream_output': 'Stream output', + 'library.config.basic.tags': 'Tags', 'library.config.basic.tag_empty': 'No tags', - 'library.config.basic.tag_placeholder': 'Select tags', + 'library.config.basic.tag_placeholder': 'Select tag', 'library.config.basic.tag_search': 'Search tags', 'library.config.basic.mcp_mode': 'MCP Mode', 'library.config.basic.temperature': 'Temperature', @@ -558,13 +559,10 @@ async function expectVariablesHelpOnHover() { await waitFor(() => expect(screen.getAllByText('{{date}}').length).toBeGreaterThan(0)) } -function openTagCombobox() { - const removeTagButton = screen.getByRole('button', { name: 'Remove work' }) - const combobox = removeTagButton.closest('[role="combobox"]') - if (!combobox) { - throw new Error('Tag combobox trigger not found') - } - fireEvent.click(combobox) +function openTagSelect() { + const select = screen.getByRole('combobox', { name: 'Tags' }) + fireEvent.pointerDown(select) + fireEvent.click(select) } describe('edit dialogs', () => { @@ -616,57 +614,45 @@ describe('edit dialogs', () => { }) it('submits assistant tag changes through ensureTags', async () => { - ensureTagsMock.mockResolvedValueOnce([ - { id: 'tag-work', name: 'work', color: '#8b5cf6' }, - { id: 'tag-personal', name: 'personal', color: '#10b981' } - ]) + ensureTagsMock.mockResolvedValueOnce([{ id: 'tag-personal', name: 'personal', color: '#10b981' }]) render() - openTagCombobox() - fireEvent.click(await screen.findByText('personal')) + openTagSelect() + fireEvent.click(await screen.findByRole('option', { name: 'personal' })) fireEvent.click(screen.getByRole('button', { name: 'Save' })) - await waitFor(() => expect(ensureTagsMock).toHaveBeenCalledWith(['work', 'personal'])) + await waitFor(() => expect(ensureTagsMock).toHaveBeenCalledWith(['personal'])) expect(updateAssistantMock).toHaveBeenCalledWith({ body: expect.objectContaining({ - tagIds: ['tag-work', 'tag-personal'] + tagIds: ['tag-personal'] }) }) }) - it('creates and binds a new tag typed in assistant editing', async () => { - ensureTagsMock.mockResolvedValueOnce([ - { id: 'tag-work', name: 'work', color: '#8b5cf6' }, - { id: 'tag-new', name: 'new-tag', color: '#10b981' } - ]) + it('clears the assistant tag from the single-select tag field', async () => { + ensureTagsMock.mockResolvedValueOnce([]) render() - openTagCombobox() - fireEvent.change(screen.getByPlaceholderText('Search tags'), { target: { value: 'new-tag' } }) - fireEvent.click(await screen.findByText('new-tag')) + const clearButton = screen.getByRole('button', { name: 'Tags Clear' }) + expect(clearButton).toHaveClass('focus-visible:pointer-events-auto', 'focus-visible:opacity-100') + fireEvent.click(clearButton) fireEvent.click(screen.getByRole('button', { name: 'Save' })) - await waitFor(() => expect(ensureTagsMock).toHaveBeenCalledWith(['work', 'new-tag'])) + await waitFor(() => expect(ensureTagsMock).toHaveBeenCalledWith([])) expect(updateAssistantMock).toHaveBeenCalledWith({ body: expect.objectContaining({ - tagIds: ['tag-work', 'tag-new'] + tagIds: [] }) }) }) - it('does not expose typed tag names beyond the server-side max length', async () => { + it('limits assistant tag editing to existing tags', async () => { render() - // Open the combobox popover (CommandInput only mounts once the trigger is active) - openTagCombobox() - const atLimit = 'y'.repeat(64) // TagNameSchema.max(64) - const tooLong = 'x'.repeat(65) // one over the limit — server would reject - const searchInput = screen.getByPlaceholderText('Search tags') - fireEvent.change(searchInput, { target: { value: tooLong } }) - expect(screen.queryByText(tooLong)).not.toBeInTheDocument() - - fireEvent.change(searchInput, { target: { value: atLimit } }) - expect(await screen.findByText(atLimit)).toBeInTheDocument() + openTagSelect() + expect(screen.queryByPlaceholderText('Search tags')).not.toBeInTheDocument() + expect(screen.queryByRole('option', { name: 'No tag' })).not.toBeInTheDocument() + expect(screen.queryByText('new-tag')).not.toBeInTheDocument() }) it('submits agent instructions and model changes as a PATCH', async () => { diff --git a/src/renderer/components/resource/dialogs/components/TagSelector.tsx b/src/renderer/components/resource/dialogs/components/TagSelector.tsx index 5e01fc2e31..90a29208a6 100644 --- a/src/renderer/components/resource/dialogs/components/TagSelector.tsx +++ b/src/renderer/components/resource/dialogs/components/TagSelector.tsx @@ -1,53 +1,80 @@ -import { Combobox, type ComboboxOption } from '@cherrystudio/ui' -import { TagNameSchema } from '@shared/data/types/tag' +import { Button, Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@cherrystudio/ui' +import { cn } from '@renderer/utils/style' +import { X } from 'lucide-react' import type { FC } from 'react' -import { useMemo, useState } from 'react' +import { useMemo } from 'react' import { useTranslation } from 'react-i18next' interface Props { - value: string[] - onChange: (tags: string[]) => void + value: string | null + onChange: (tag: string | null) => void allTagNames: string[] disabled?: boolean portalContainer?: HTMLElement | null } +const TAG_SELECT_VALUE_PREFIX = 'tag:' + +function encodeTagSelectValue(name: string) { + return `${TAG_SELECT_VALUE_PREFIX}${name}` +} + +function decodeTagSelectValue(value: string) { + if (!value.startsWith(TAG_SELECT_VALUE_PREFIX)) return null + return value.slice(TAG_SELECT_VALUE_PREFIX.length) +} + export const TagSelector: FC = ({ value, onChange, allTagNames, disabled, portalContainer }) => { const { t } = useTranslation() - const [search, setSearch] = useState('') - // `value` may contain names not present in `/tags` yet, for example while a - // caller waits for SWR refresh. Keep selected names visible in the options. - const tagOptions = useMemo(() => { - const trimmedSearch = search.trim() - const names = new Set([...allTagNames, ...value]) - // Mirror the server-side TagNameSchema (z.string().trim().min(1).max(64)) - // so a user cannot select a name the create endpoint would reject. - if (trimmedSearch && TagNameSchema.safeParse(trimmedSearch).success) { - names.add(trimmedSearch) - } + // `value` may be a name not present in `/tags` yet, for example while a + // caller waits for SWR refresh. Keep the selected name visible in the options. + const tagNames = useMemo(() => { + const names = new Set(allTagNames) + if (value) names.add(value) const sortedNames = Array.from(names) sortedNames.sort((a, b) => a.localeCompare(b, 'zh')) - return sortedNames.map((name) => ({ - value: name, - label: name - })) - }, [allTagNames, search, value]) + return sortedNames + }, [allTagNames, value]) return ( - onChange(Array.isArray(v) ? v : v ? [v] : [])} - onSearch={setSearch} - placeholder={t('library.config.basic.tag_placeholder')} - searchPlaceholder={t('library.config.basic.tag_search')} - emptyText={t('library.config.basic.tag_empty')} - portalContainer={portalContainer ?? undefined} - /> +
+ + {value && !disabled ? ( + + ) : null} +
) } diff --git a/src/renderer/components/resource/dialogs/edit/AssistantEditDialog.tsx b/src/renderer/components/resource/dialogs/edit/AssistantEditDialog.tsx index efdd2f4bd9..a3350475a0 100644 --- a/src/renderer/components/resource/dialogs/edit/AssistantEditDialog.tsx +++ b/src/renderer/components/resource/dialogs/edit/AssistantEditDialog.tsx @@ -63,7 +63,7 @@ type AssistantEditFormValues = { name: string description: string modelId: UniqueModelId | null - tags: string[] + tagName: string | null prompt: string temperature: number enableTemperature: boolean @@ -99,7 +99,7 @@ function defaultValuesForAssistant(resource: AssistantEditDialogResource): Assis name: form.name, description: form.description, modelId: form.modelId ?? null, - tags: form.tags, + tagName: form.tagName, prompt: form.prompt, temperature: form.temperature, enableTemperature: form.enableTemperature, @@ -132,7 +132,7 @@ function buildAssistantFormState(baseline: AssistantFormState, values: Assistant name: values.name, description: values.description, modelId: values.modelId, - tags: values.tags, + tagName: values.tagName, prompt: values.prompt, temperature: values.temperature, enableTemperature: values.enableTemperature, @@ -361,7 +361,7 @@ function AssistantBasicFields({
( {t('library.config.basic.tags')} diff --git a/src/renderer/components/resource/dialogs/form/__tests__/assistant.test.ts b/src/renderer/components/resource/dialogs/form/__tests__/assistant.test.ts index 2d7292f5d0..ccfa62a838 100644 --- a/src/renderer/components/resource/dialogs/form/__tests__/assistant.test.ts +++ b/src/renderer/components/resource/dialogs/form/__tests__/assistant.test.ts @@ -69,9 +69,9 @@ describe('initialAssistantFormState', () => { }) }) - it('extracts tag names from embedded tag rows', () => { + it('extracts a single tag name from embedded tag rows', () => { const assistant = createAssistant({ tags: [tag('t1', 'alpha', '#f00'), tag('t2', 'beta', '#0f0')] }) - expect(initialAssistantFormState(assistant).tags).toEqual(['alpha', 'beta']) + expect(initialAssistantFormState(assistant).tagName).toBe('alpha') }) }) @@ -128,22 +128,24 @@ describe('diffAssistantUpdate', () => { expect(result?.dto.settings).toMatchObject({ reasoning_effort: 'high' }) }) - it('flags tag changes and passes form.tags through as tagNames', () => { + it('flags tag changes and passes one form tag through as tagNames', () => { const assistant = createAssistant({ tags: [tag('t1', 'alpha', '#f00')] }) const baseline = initialAssistantFormState(assistant) - const form = { ...baseline, tags: ['alpha', 'new'] } + const form = { ...baseline, tagName: 'new' } const result = diffAssistantUpdate(form, baseline, assistant) expect(result?.tagsChanged).toBe(true) - expect(result?.tagNames).toEqual(['alpha', 'new']) + expect(result?.tagNames).toEqual(['new']) }) - it('treats tag reorder (same set) as unchanged', () => { + it('flags clearing the assistant tag', () => { const assistant = createAssistant({ tags: [tag('t1', 'alpha', '#f00'), tag('t2', 'beta', '#0f0')] }) const baseline = initialAssistantFormState(assistant) - const form = { ...baseline, tags: ['beta', 'alpha'] } + const form = { ...baseline, tagName: null } - expect(diffAssistantUpdate(form, baseline, assistant)).toBeNull() + const result = diffAssistantUpdate(form, baseline, assistant) + expect(result?.tagsChanged).toBe(true) + expect(result?.tagNames).toEqual([]) }) it('emits knowledgeBaseIds only when the set changes, ignoring order', () => { @@ -187,12 +189,12 @@ describe('diffAssistantSaveIntent', () => { it('wraps update diffs for the edit dialog save handler', () => { const assistant = createAssistant({ tags: [tag('t1', 'alpha')] }) const baseline = initialAssistantFormState(assistant) - const form = { ...baseline, tags: ['alpha', 'beta'] } + const form = { ...baseline, tagName: 'beta' } expect(diffAssistantSaveIntent(form, baseline, assistant)).toEqual({ kind: 'update', payload: {}, - tagNames: ['alpha', 'beta'], + tagNames: ['beta'], tagsChanged: true }) }) diff --git a/src/renderer/components/resource/dialogs/form/assistant.ts b/src/renderer/components/resource/dialogs/form/assistant.ts index 7cba873ba4..915b2cca4f 100644 --- a/src/renderer/components/resource/dialogs/form/assistant.ts +++ b/src/renderer/components/resource/dialogs/form/assistant.ts @@ -21,9 +21,9 @@ const UI_DEFAULT_MAX_TOOL_CALLS = 20 * Flat form state for the Assistant edit dialog. Every editable field lives * here so the dialog commits in a single PATCH. * - * `tags` stores user-facing names, not ids — tag-id resolution happens - * at save time via `ensureTags` so the user can freely type new tags without - * paying a network round-trip per keystroke. + * `tagName` stores one user-facing name, not an id — tag-id resolution happens + * at save time via `ensureTags`, keeping the form state independent from + * backend tag ids. */ export interface AssistantFormState { // columns @@ -46,11 +46,15 @@ export interface AssistantFormState { customParameters: CustomParameter[] mcpMode: AssistantSettings['mcpMode'] // relations - tags: string[] + tagName: string | null knowledgeBaseIds: string[] mcpServerIds: string[] } +function normalizeAssistantTagName(tags: readonly string[]): string | null { + return tags[0] ?? null +} + function buildAssistantSettingsFromForm( form: AssistantFormState, baseSettings: AssistantSettings = DEFAULT_ASSISTANT_SETTINGS @@ -90,7 +94,7 @@ export function initialAssistantFormState(assistant: Assistant): AssistantFormSt enableMaxToolCalls: settings.enableMaxToolCalls ?? true, customParameters: settings.customParameters ?? [], mcpMode: settings.mcpMode ?? 'auto', - tags: (assistant.tags ?? []).map((t) => t.name), + tagName: normalizeAssistantTagName((assistant.tags ?? []).map((t) => t.name)), knowledgeBaseIds: assistant.knowledgeBaseIds ?? [], mcpServerIds: assistant.mcpServerIds ?? [] } @@ -160,7 +164,7 @@ export function diffAssistantUpdate( baseline.mcpMode !== form.mcpMode || customParametersChanged - const tagsChanged = !sameStringSet(baseline.tags, form.tags) + const tagsChanged = baseline.tagName !== form.tagName const knowledgeBaseIdsChanged = !sameIdSet(baseline.knowledgeBaseIds, form.knowledgeBaseIds) const mcpServerIdsChanged = !sameIdSet(baseline.mcpServerIds, form.mcpServerIds) @@ -183,7 +187,7 @@ export function diffAssistantUpdate( ...(mcpServerIdsChanged ? { mcpServerIds: form.mcpServerIds } : {}) } - return { dto, tagsChanged, tagNames: form.tags } + return { dto, tagsChanged, tagNames: form.tagName ? [form.tagName] : [] } } export function diffAssistantSaveIntent( @@ -208,9 +212,3 @@ function sameIdSet(a: readonly string[], b: readonly string[]): boolean { const set = new Set(a) return b.every((id) => set.has(id)) } - -function sameStringSet(a: readonly string[], b: readonly string[]): boolean { - if (a.length !== b.length) return false - const set = new Set(a) - return b.every((v) => set.has(v)) -} diff --git a/src/renderer/i18n/locales/en-us.json b/src/renderer/i18n/locales/en-us.json index 0d3a228a73..743fb5f0d8 100644 --- a/src/renderer/i18n/locales/en-us.json +++ b/src/renderer/i18n/locales/en-us.json @@ -1333,12 +1333,14 @@ "add": "Add Tag", "delete": "Delete Tag", "deleteConfirm": "Are you sure to delete this tag?", + "group_by": "Group by tag", "manage": "Tag Management", "modify": "Modify Tag", "none": "No tags", "settings": { "title": "Tag Settings" }, + "ungroup": "Turn off tag grouping", "untagged": "Untagged" }, "title": "Assistants", @@ -3154,9 +3156,9 @@ "stream_output": "Stream output", "tag_empty": "No tags available", "tag_hint": "To add a new tag, use the \"+ Tag\" entry in the library top bar", - "tag_placeholder": "Select tags", + "tag_placeholder": "Select tag", "tag_search": "Search tags", - "tags": "Tags", + "tags": "Tag", "temperature": "Temperature", "title": "Basic settings", "top_p": "Top-P", diff --git a/src/renderer/i18n/locales/zh-cn.json b/src/renderer/i18n/locales/zh-cn.json index 2c6ef3ff49..5bc94253d0 100644 --- a/src/renderer/i18n/locales/zh-cn.json +++ b/src/renderer/i18n/locales/zh-cn.json @@ -1333,12 +1333,14 @@ "add": "添加标签", "delete": "删除标签", "deleteConfirm": "确定要删除这个标签吗？", + "group_by": "按照标签分组", "manage": "标签管理", "modify": "修改标签", "none": "暂无标签", "settings": { "title": "标签设置" }, + "ungroup": "关闭标签分组", "untagged": "未分组" }, "title": "助手", diff --git a/src/renderer/i18n/locales/zh-tw.json b/src/renderer/i18n/locales/zh-tw.json index 2e7fa7566c..65262fc3b9 100644 --- a/src/renderer/i18n/locales/zh-tw.json +++ b/src/renderer/i18n/locales/zh-tw.json @@ -1333,12 +1333,14 @@ "add": "新增標籤", "delete": "刪除標籤", "deleteConfirm": "確定要刪除這個標籤嗎？", + "group_by": "依標籤分組", "manage": "標籤管理", "modify": "修改標籤", "none": "暫無標籤", "settings": { "title": "標籤設定" }, + "ungroup": "關閉標籤分組", "untagged": "未分組" }, "title": "助手", diff --git a/src/renderer/pages/library/LibraryPage.tsx b/src/renderer/pages/library/LibraryPage.tsx index 091512f49e..bb78cc694a 100644 --- a/src/renderer/pages/library/LibraryPage.tsx +++ b/src/renderer/pages/library/LibraryPage.tsx @@ -65,8 +65,10 @@ function buildTags(resources: ResourceItem[], backendTags: Tag[], filterType?: R for (const tag of r.raw.tags ?? []) { if (!backendTagByName.has(tag.name)) backendTagByName.set(tag.name, tag) } + if (r.tag) { + tagMap.set(r.tag, (tagMap.get(r.tag) || 0) + 1) + } } - r.tags.forEach((t) => tagMap.set(t, (tagMap.get(t) || 0) + 1)) }) return Array.from(tagMap.entries()) .sort((a, b) => b[1] - a[1]) @@ -155,7 +157,6 @@ export default function LibraryPage() { [tagList.tags] ) - const noop = useCallback(() => {}, []) const handleClosePromptDialog = useCallback(() => { setPromptDialog(null) }, []) @@ -474,7 +475,6 @@ export default function LibraryPage() { // rows; binding stays inside card/dialog tag hooks. await ensureTags([tagName]) }} - onUpdateResourceTags={noop /* binding is executed inside FixedCardMenu via the tag hooks */} allTagNames={allTagNames} allTags={tagList.tags} assistantCatalog={assistantCatalogProp} diff --git a/src/renderer/pages/library/__tests__/LibraryPage.test.tsx b/src/renderer/pages/library/__tests__/LibraryPage.test.tsx index b72279102f..a951a0a52e 100644 --- a/src/renderer/pages/library/__tests__/LibraryPage.test.tsx +++ b/src/renderer/pages/library/__tests__/LibraryPage.test.tsx @@ -516,7 +516,6 @@ describe('LibraryPage create flow', () => { name: 'Assistant to duplicate', description: '', avatar: '💬', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'assistant-to-duplicate', name: 'Assistant to duplicate', tags: [] } @@ -630,7 +629,6 @@ describe('LibraryPage create flow', () => { name: 'Selector Agent', description: '', avatar: '', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'agent-from-selector' } @@ -652,7 +650,6 @@ describe('LibraryPage create flow', () => { name: 'Selector Agent', description: '', avatar: '', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'agent-from-selector' } @@ -675,7 +672,6 @@ describe('LibraryPage create flow', () => { name: 'Selector Assistant', description: '', avatar: '💬', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'assistant-from-selector' } @@ -697,7 +693,6 @@ describe('LibraryPage create flow', () => { name: 'Stale Assistant', description: '', avatar: '💬', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'assistant-stale-tags', name: 'Stale Assistant' } @@ -717,7 +712,6 @@ describe('LibraryPage create flow', () => { name: 'Grid Prompt', description: '', avatar: 'Aa', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'prompt-from-grid' } @@ -746,7 +740,6 @@ describe('LibraryPage create flow', () => { name: 'Grid Skill', description: '', avatar: 'S', - tags: [], createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', raw: { id: 'skill-from-grid', name: 'Grid Skill' } diff --git a/src/renderer/pages/library/adapters/__tests__/assistantAdapter.test.ts b/src/renderer/pages/library/adapters/__tests__/assistantAdapter.test.ts index 523a09816e..fab60965bc 100644 --- a/src/renderer/pages/library/adapters/__tests__/assistantAdapter.test.ts +++ b/src/renderer/pages/library/adapters/__tests__/assistantAdapter.test.ts @@ -78,7 +78,7 @@ describe('useAssistantMutations', () => { }) }) - it('forwards tag ids to the create endpoint when duplicating an assistant', async () => { + it('forwards only one tag id to the create endpoint when duplicating an assistant', async () => { const created = createAssistant({ id: 'ast-copy', tags: [] }) createTriggerMock.mockResolvedValue(created) @@ -106,7 +106,7 @@ describe('useAssistantMutations', () => { settings: source.settings, mcpServerIds: ['mcp-1'], knowledgeBaseIds: ['kb-1'], - tagIds: ['tag-1', 'tag-2'] + tagIds: ['tag-1'] } }) }) diff --git a/src/renderer/pages/library/adapters/assistantAdapter.ts b/src/renderer/pages/library/adapters/assistantAdapter.ts index 96aefd3023..73be14a6a6 100644 --- a/src/renderer/pages/library/adapters/assistantAdapter.ts +++ b/src/renderer/pages/library/adapters/assistantAdapter.ts @@ -66,6 +66,7 @@ export function useAssistantMutations() { const duplicateAssistant = useCallback( async (source: Assistant): Promise => { const duplicateName = t('library.duplicate_name', { name: source.name }) + const tagId = source.tags[0]?.id return createTrigger({ body: { @@ -77,7 +78,7 @@ export function useAssistantMutations() { settings: source.settings, mcpServerIds: source.mcpServerIds, knowledgeBaseIds: source.knowledgeBaseIds, - tagIds: source.tags.map((tag) => tag.id) + tagIds: tagId ? [tagId] : [] } }) }, diff --git a/src/renderer/pages/library/adapters/types.ts b/src/renderer/pages/library/adapters/types.ts index 640cb5c411..182ea0183b 100644 --- a/src/renderer/pages/library/adapters/types.ts +++ b/src/renderer/pages/library/adapters/types.ts @@ -3,7 +3,7 @@ import type { ResourceType } from '../types' export interface ResourceListQuery { /** Free-text match against name OR description (passed through to the API). */ search?: string - /** Union (OR) tag filter — kept if the resource is bound to ANY of these tag ids. */ + /** Backend tag-id filter transport shape; current assistant UI passes at most one id. */ tagIds?: string[] limit?: number offset?: number diff --git a/src/renderer/pages/library/list/ResourceCardMenu.tsx b/src/renderer/pages/library/list/ResourceCardMenu.tsx index 75d2239677..aa5368dd2f 100644 --- a/src/renderer/pages/library/list/ResourceCardMenu.tsx +++ b/src/renderer/pages/library/list/ResourceCardMenu.tsx @@ -1,6 +1,5 @@ import { Button, - Checkbox, Input, MenuDivider, MenuItem, @@ -12,8 +11,8 @@ import { } from '@cherrystudio/ui' import { loggerService } from '@logger' import { useEnsureTags, useTagList } from '@renderer/hooks/useTags' -import { ChevronDown, Copy, Download, Plus, Tag, Trash2 } from 'lucide-react' -import { useCallback, useRef, useState } from 'react' +import { Check, ChevronDown, Copy, Download, Plus, Tag, Trash2 } from 'lucide-react' +import { type KeyboardEvent, useCallback, useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { useAssistantMutationsById } from '../adapters/assistantAdapter' @@ -32,7 +31,6 @@ interface ResourceCardMenuProps { onDuplicate: (r: ResourceItem) => void onDelete: (r: ResourceItem) => void onExport: (r: ResourceItem) => void - onUpdateResourceTags: (resourceId: string, tags: string[]) => void allTagNames: string[] } @@ -42,16 +40,19 @@ export function ResourceCardMenu({ onDuplicate, onDelete, onExport, - onUpdateResourceTags, allTagNames }: ResourceCardMenuProps) { const { t } = useTranslation() const [showTagPicker, setShowTagPicker] = useState(false) - const [localTags, setLocalTags] = useState(resource.tags) + const [localTag, setLocalTag] = useState(() => + resource.type === 'assistant' ? (resource.tag ?? null) : null + ) const [tagInput, setTagInput] = useState('') const [bindingError, setBindingError] = useState(null) const [bindingPending, setBindingPending] = useState(false) const bindingPendingRef = useRef(false) + const tagOptionRefs = useRef>([]) + const [activeTagIndex, setActiveTagIndex] = useState(0) const { ensureTags } = useEnsureTags({ getDefaultColor: getRandomTagColor }) const { updateAssistant } = useAssistantMutationsById(resource.id) @@ -65,22 +66,28 @@ export function ResourceCardMenu({ const tagList = useTagList() const colorFor = (name: string): string => tagList.tags.find((tag) => tag.name === name)?.color ?? DEFAULT_TAG_COLOR - const persistTags = useCallback( - async (nextNames: string[], previousNames: string[]) => { + useEffect(() => { + if (!showTagPicker) return + const selectedIndex = localTag ? allTagNames.indexOf(localTag) : -1 + setActiveTagIndex(selectedIndex >= 0 ? selectedIndex : 0) + }, [allTagNames, localTag, showTagPicker]) + + const persistTag = useCallback( + async (nextName: string | null, previousName: string | null) => { if (!canBindTags) return if (bindingPendingRef.current) return bindingPendingRef.current = true setBindingPending(true) try { + const nextNames = nextName ? [nextName] : [] const tags = await ensureTags(nextNames) const tagIds = tags.map((tag) => tag.id) if (resource.type === 'assistant') { await updateAssistant({ tagIds }) } - onUpdateResourceTags(resource.id, nextNames) } catch (e) { // Roll back optimistic state on failure. - setLocalTags(previousNames) + setLocalTag(previousName) const message = e instanceof Error ? e.message : t('library.tag_sync_failed') setBindingError(message) // The inline error text only renders while the popup is open. Toast + @@ -96,31 +103,60 @@ export function ResourceCardMenu({ setBindingPending(false) } }, - [canBindTags, ensureTags, updateAssistant, onUpdateResourceTags, resource.id, resource.type, t] + [canBindTags, ensureTags, updateAssistant, resource.id, resource.type, t] ) const toggleTag = (tag: string) => { if (bindingPendingRef.current) return - const prev = localTags - const next = prev.includes(tag) ? prev.filter((item) => item !== tag) : [...prev, tag] - setLocalTags(next) + const prev = localTag + const next = prev === tag ? null : tag + setLocalTag(next) setBindingError(null) - void persistTags(next, prev) + void persistTag(next, prev) + } + + const focusTagOption = (index: number) => { + setActiveTagIndex(index) + tagOptionRefs.current[index]?.focus() + } + + const handleTagOptionKeyDown = (e: KeyboardEvent, index: number, tag: string) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + if (!bindingPending) toggleTag(tag) + return + } + + if (allTagNames.length === 0) return + + if (e.key === 'ArrowDown') { + e.preventDefault() + focusTagOption((index + 1) % allTagNames.length) + } else if (e.key === 'ArrowUp') { + e.preventDefault() + focusTagOption((index - 1 + allTagNames.length) % allTagNames.length) + } else if (e.key === 'Home') { + e.preventDefault() + focusTagOption(0) + } else if (e.key === 'End') { + e.preventDefault() + focusTagOption(allTagNames.length - 1) + } } const addNewTag = () => { if (bindingPendingRef.current) return const tag = tagInput.trim() - if (!tag || localTags.includes(tag)) { + if (!tag || localTag === tag) { setTagInput('') return } - const prev = localTags - const next = [...prev, tag] - setLocalTags(next) + const prev = localTag + const next = tag + setLocalTag(next) setTagInput('') setBindingError(null) - void persistTags(next, prev) + void persistTag(next, prev) } return ( @@ -137,9 +173,7 @@ export function ResourceCardMenu({ label={t('library.action.manage_tags')} suffix={ <> - {localTags.length > 0 && ( - {localTags.length} - )} + {localTag && 1} } @@ -174,43 +208,38 @@ export function ResourceCardMenu({ )}
-
+
{allTagNames.length === 0 && !tagInput.trim() && (

{t('library.tag_picker.no_tags')}

)} - {allTagNames.map((tag) => { - const checked = localTags.includes(tag) + {allTagNames.map((tag, index) => { + const checked = localTag === tag return (
{ + tagOptionRefs.current[index] = node + }} + role="menuitemradio" + aria-checked={checked} + tabIndex={!bindingPending && index === activeTagIndex ? 0 : -1} aria-disabled={bindingPending || undefined} onClick={() => toggleTag(tag)} - onKeyDown={(e) => { - if (bindingPending) return - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - toggleTag(tag) - } - }} + onFocus={() => setActiveTagIndex(index)} + onKeyDown={(e) => handleTagOptionKeyDown(e, index, tag)} className={`flex w-full items-center gap-2 rounded-md px-2.5 py-1 text-foreground-secondary text-xs transition-colors ${ bindingPending ? 'cursor-not-allowed opacity-60' : 'cursor-pointer hover:bg-accent hover:text-foreground' }`}> - e.stopPropagation()}> - toggleTag(tag)} - /> - {tag} + {checked && }
) })} diff --git a/src/renderer/pages/library/list/ResourceCards.tsx b/src/renderer/pages/library/list/ResourceCards.tsx index 5503bdf669..afd7f46c82 100644 --- a/src/renderer/pages/library/list/ResourceCards.tsx +++ b/src/renderer/pages/library/list/ResourceCards.tsx @@ -214,22 +214,13 @@ interface ResourceCardProps { onDuplicate: (resource: ResourceItem) => void onEdit: (resource: ResourceItem) => void onExport: (resource: ResourceItem) => void - onUpdateResourceTags: (resourceId: string, tags: string[]) => void } function hasOverflowActions(resource: ResourceItem) { return resource.type === 'assistant' } -export function ResourceCard({ - resource: r, - allTagNames, - onDelete, - onDuplicate, - onEdit, - onExport, - onUpdateResourceTags -}: ResourceCardProps) { +export function ResourceCard({ resource: r, allTagNames, onDelete, onDuplicate, onEdit, onExport }: ResourceCardProps) { const { t } = useTranslation() const [menuOpen, setMenuOpen] = useState(false) const cfg = RESOURCE_TYPE_META[r.type] @@ -237,8 +228,7 @@ export function ResourceCard({ // other resources keep their own avatar on the neutral accent block. const useTypedAvatarBg = r.type === 'skill' const showOverflowMenu = hasOverflowActions(r) - const visibleTags = r.type === 'assistant' ? r.tags.slice(0, 2) : [] - const extraTagCount = r.type === 'assistant' ? r.tags.length - visibleTags.length : 0 + const visibleTag = r.type === 'assistant' ? r.tag : undefined return (

{r.name}

{r.description}

- {visibleTags.length > 0 && ( + {visibleTag && (
- {visibleTags.map((tag, i) => ( - - {tag} - - ))} - {extraTagCount > 0 && +{extraTagCount}} + + {visibleTag} +
)}
@@ -299,7 +285,6 @@ export function ResourceCard({ onDuplicate={onDuplicate} onDelete={onDelete} onExport={onExport} - onUpdateResourceTags={onUpdateResourceTags} allTagNames={allTagNames} /> diff --git a/src/renderer/pages/library/list/ResourceGrid.tsx b/src/renderer/pages/library/list/ResourceGrid.tsx index 90ea056eb9..40b9852a45 100644 --- a/src/renderer/pages/library/list/ResourceGrid.tsx +++ b/src/renderer/pages/library/list/ResourceGrid.tsx @@ -69,8 +69,6 @@ interface Props { onTagFilter: (tagName: string | null) => void /** Create a new tag (POST /tags). Does not bind the tag to any resource. */ onAddTag: (tagName: string) => Promise | void - /** Replace the tag-name set for a single resource. Caller handles ensure-tag + bind. */ - onUpdateResourceTags: (resourceId: string, tags: string[]) => Promise | void allTagNames: string[] /** Full backend tag records (id + name + color). Distinct from `allTagNames` (names only). */ allTags: BackendTag[] @@ -128,7 +126,6 @@ export const ResourceGrid: FC = ({ activeTag, onTagFilter, onAddTag, - onUpdateResourceTags, allTagNames, allTags, assistantCatalog @@ -494,7 +491,6 @@ export const ResourceGrid: FC = ({ onDuplicate={onDuplicate} onEdit={onEdit} onExport={onExport} - onUpdateResourceTags={onUpdateResourceTags} /> )}
@@ -534,7 +530,6 @@ interface VirtualizedResourceGridProps { onDuplicate: (r: ResourceItem) => void onEdit: (r: ResourceItem) => void onExport: (r: ResourceItem) => void - onUpdateResourceTags: (resourceId: string, tags: string[]) => void } function VirtualizedResourceGrid({ @@ -545,8 +540,7 @@ function VirtualizedResourceGrid({ onDelete, onDuplicate, onEdit, - onExport, - onUpdateResourceTags + onExport }: VirtualizedResourceGridProps) { const rows = useMemo(() => { const nextRows: ResourceItem[][] = [] @@ -590,7 +584,6 @@ function VirtualizedResourceGrid({ onDuplicate={onDuplicate} onEdit={onEdit} onExport={onExport} - onUpdateResourceTags={onUpdateResourceTags} /> ))}
diff --git a/src/renderer/pages/library/list/__tests__/ResourceGrid.test.tsx b/src/renderer/pages/library/list/__tests__/ResourceGrid.test.tsx index af62c9ecc6..ce598c46c9 100644 --- a/src/renderer/pages/library/list/__tests__/ResourceGrid.test.tsx +++ b/src/renderer/pages/library/list/__tests__/ResourceGrid.test.tsx @@ -75,27 +75,6 @@ vi.mock('@cherrystudio/ui', async () => { ) }, - Checkbox: ({ - checked = false, - onCheckedChange, - size, - ...props - }: Omit, 'onChange'> & { - checked?: boolean - onCheckedChange?: (checked: boolean) => void - size?: string - }) => { - void size - return ( - + } + /> + @@ -783,14 +829,20 @@ const CreateForm: FC<{ const [promptModalOpen, setPromptModalOpen] = useState(false) const [schedule, setSchedule] = useState({ kind: 'interval', value: '', timeoutMinutes: '' }) const [channelIds, setChannelIds] = useState([]) - // TODO(agent-workspace-picker): wire the workspace picker before re-enabling task creation. - const [workspaceSource] = useState(null) + // `null` = "No work directory" (system workspace); a string binds the task to that user workspace. + const [workspaceId, setWorkspaceId] = useState(null) + const { data: workspaces } = useQuery('/agent-workspaces') const [saving, setSaving] = useState(false) - const isValid = agentId && name.trim() && prompt.trim() && schedule.value.trim() && workspaceSource + const isSystemWorkspace = workspaceId === null + const workspaceLabel = isSystemWorkspace + ? t('agent.session.workspace_selector.no_project') + : (workspaces?.find((w) => w.id === workspaceId)?.name ?? workspaceId) + + const isValid = agentId && name.trim() && prompt.trim() && schedule.value.trim() const handleCreate = useCallback(async () => { - if (!agentId || !name.trim() || !prompt.trim() || !schedule.value.trim() || !workspaceSource) return + if (!agentId || !name.trim() || !prompt.trim() || !schedule.value.trim()) return const trigger = formStateToTrigger(schedule.kind, schedule.value.trim()) if (!trigger) return setSaving(true) @@ -800,14 +852,17 @@ const CreateForm: FC<{ name: name.trim(), prompt: prompt.trim(), trigger, - workspace: workspaceSource, + workspace: + workspaceId === null + ? { type: AGENT_WORKSPACE_TYPE.SYSTEM } + : { type: AGENT_WORKSPACE_TYPE.USER, workspaceId }, timeoutMinutes: timeout && timeout > 0 ? timeout : undefined, channelIds: channelIds.length > 0 ? channelIds : undefined }) } finally { setSaving(false) } - }, [agentId, name, prompt, schedule, workspaceSource, channelIds, onCreate]) + }, [agentId, name, prompt, schedule, workspaceId, channelIds, onCreate]) return ( @@ -880,6 +935,23 @@ const CreateForm: FC<{ + {/* Workspace is a secondary detail — scheduled tasks default to "No work directory". */} +
+ {t('agent.session.display.workdir')} + + {isSystemWorkspace ? : } + {workspaceLabel} + + + } + /> +
+