From 5ceef06b4b73e547f2f80bb10fcd47740534fb79 Mon Sep 17 00:00:00 2001 From: ajnart Date: Sun, 2 Aug 2026 16:45:14 +0200 Subject: [PATCH] fix: address assistant review feedback --- apps/docs/docs/widgets/assistant/index.ts | 2 +- .../_components/assistant-configuration.tsx | 20 ++++---- .../chat/assistant-model-lookup.spec.ts | 49 +++++++++++++++++++ .../assistant/chat/assistant-model-lookup.ts | 17 +++++++ .../src/app/api/assistant/chat/route.ts | 31 ++++++++++-- .../assistant/assistant-auto-approval.spec.ts | 27 ++++++++++ .../assistant/assistant-auto-approval.tsx | 4 +- .../assistant-board-settings-tool.tsx | 12 ++--- .../assistant-board-settings.spec.ts | 15 +++++- .../assistant/assistant-board-settings.ts | 24 +++++++++ .../components/assistant/assistant-panel.tsx | 21 ++++++-- .../assistant/assistant-preferences.spec.ts | 40 ++++++++++++++- .../assistant/assistant-preferences.ts | 18 +++++++ .../assistant/assistant-provider.tsx | 21 +++++--- .../assistant-tool-contracts.spec.ts | 5 ++ .../assistant/assistant-tool-contracts.ts | 5 +- .../assistant/assistant-tool-result.spec.ts | 24 +++++++++ .../assistant/assistant-tool-result.ts | 13 ++++- .../items/actions/test/empty-position.spec.ts | 1 + packages/api/src/router/assistant.ts | 3 +- .../api/src/router/test/assistant.spec.ts | 10 ++++ packages/translation/src/lang/en.json | 10 +++- .../_inputs/widget-dynamic-select-input.tsx | 5 +- packages/widgets/src/assistant/component.tsx | 8 +-- packages/widgets/src/assistant/index.ts | 3 +- packages/widgets/src/options.ts | 1 + 26 files changed, 340 insertions(+), 49 deletions(-) create mode 100644 apps/nextjs/src/app/api/assistant/chat/assistant-model-lookup.spec.ts create mode 100644 apps/nextjs/src/app/api/assistant/chat/assistant-model-lookup.ts diff --git a/apps/docs/docs/widgets/assistant/index.ts b/apps/docs/docs/widgets/assistant/index.ts index afbaf21a5..d465261bf 100644 --- a/apps/docs/docs/widgets/assistant/index.ts +++ b/apps/docs/docs/widgets/assistant/index.ts @@ -18,7 +18,7 @@ export const assistantWidget: WidgetDefinition = { name: "Pinned conversation", description: "Search for a conversation. This field is hidden when Current conversation is selected.", values: { type: "string" }, - defaultValue: "Hidden", + defaultValue: "", }, ], }, diff --git a/apps/nextjs/src/app/[locale]/manage/assistant/_components/assistant-configuration.tsx b/apps/nextjs/src/app/[locale]/manage/assistant/_components/assistant-configuration.tsx index a7b2db536..3a2250842 100644 --- a/apps/nextjs/src/app/[locale]/manage/assistant/_components/assistant-configuration.tsx +++ b/apps/nextjs/src/app/[locale]/manage/assistant/_components/assistant-configuration.tsx @@ -101,8 +101,8 @@ const ConfigurationSection = ({ number, title, description, status, actions, chi ); -const ConfigurationSkeleton = () => ( - +const ConfigurationSkeleton = ({ label }: { label: string }) => ( + @@ -357,7 +357,7 @@ export const AssistantConfiguration = () => { }; if (isLoading) { - return ; + return ; } return ( @@ -376,12 +376,12 @@ export const AssistantConfiguration = () => { - {connectionReady - ? t("overview.connectionReady") - : connectionChanged - ? t("overview.unsavedChanges") + {connectionChanged + ? t("overview.unsavedChanges") + : connectionReady + ? t("overview.connectionReady") : t("overview.setupRequired")} @@ -815,9 +815,7 @@ export const AssistantConfiguration = () => { {selectedModel.contextLength && ( - - {t("model.context", { count: selectedModel.contextLength.toLocaleString() })} - + {t("model.context", { count: selectedModel.contextLength })} )} { + test("keeps the configured default usable when discovery is temporarily unreachable", () => { + expect( + getAssistantModelLookupStatus({ + configuredModelId: "provider/default", + requestedModelId: "provider/default", + hasModel: false, + failed: true, + }), + ).toBe("available"); + }); + + test("treats an alternate model discovery failure as retryable", () => { + expect( + getAssistantModelLookupStatus({ + configuredModelId: "provider/default", + requestedModelId: "provider/alternate", + hasModel: false, + failed: true, + }), + ).toBe("unreachable"); + }); + + test("rejects an alternate model only after discovery confirms it is absent", () => { + expect( + getAssistantModelLookupStatus({ + configuredModelId: "provider/default", + requestedModelId: "provider/alternate", + hasModel: false, + failed: false, + }), + ).toBe("unavailable"); + }); + + test("accepts a discovered alternate model", () => { + expect( + getAssistantModelLookupStatus({ + configuredModelId: "provider/default", + requestedModelId: "provider/alternate", + hasModel: true, + failed: false, + }), + ).toBe("available"); + }); +}); diff --git a/apps/nextjs/src/app/api/assistant/chat/assistant-model-lookup.ts b/apps/nextjs/src/app/api/assistant/chat/assistant-model-lookup.ts new file mode 100644 index 000000000..b78ae470f --- /dev/null +++ b/apps/nextjs/src/app/api/assistant/chat/assistant-model-lookup.ts @@ -0,0 +1,17 @@ +export type AssistantModelLookupStatus = "available" | "unavailable" | "unreachable"; + +export const getAssistantModelLookupStatus = ({ + configuredModelId, + requestedModelId, + hasModel, + failed, +}: { + configuredModelId: string; + requestedModelId: string; + hasModel: boolean; + failed: boolean; +}): AssistantModelLookupStatus => { + if (requestedModelId === configuredModelId) return "available"; + if (failed) return "unreachable"; + return hasModel ? "available" : "unavailable"; +}; diff --git a/apps/nextjs/src/app/api/assistant/chat/route.ts b/apps/nextjs/src/app/api/assistant/chat/route.ts index f3e38dad2..ef84df1c2 100644 --- a/apps/nextjs/src/app/api/assistant/chat/route.ts +++ b/apps/nextjs/src/app/api/assistant/chat/route.ts @@ -31,6 +31,7 @@ import type { import { extractMcpTools } from "../../mcp/_extract-tools"; import { getRequestedMentionIds, sanitizeAttachmentFilename } from "./assistant-chat-input"; +import { getAssistantModelLookupStatus } from "./assistant-model-lookup"; import { getAssistantStreamErrorMessage } from "./assistant-stream-error"; import { getForcedAssistantToolName, withAssistantToolPolicy } from "./assistant-tool-policy"; @@ -361,11 +362,35 @@ export async function POST(request: Request) { const requestStartedAt = Date.now(); const requestId = crypto.randomUUID(); const requestedModelId = parsed.data.modelId ?? configuration.modelId; - const [selectedModel, mentionContext] = await Promise.all([ - getSelectedModelDetailsAsync(configuration, requestedModelId).catch(() => null), + const [modelLookup, mentionContext] = await Promise.all([ + getSelectedModelDetailsAsync(configuration, requestedModelId).then( + (model) => ({ model, error: null }), + (error: unknown) => ({ model: null, error }), + ), getMentionContextAsync(context, parsed.data.messages), ]); - if (requestedModelId !== configuration.modelId && !selectedModel) { + if (modelLookup.error !== null) { + logger.warn("Assistant model discovery failed before the response started", { + requestId, + provider: configuration.provider, + modelId: requestedModelId, + error: modelLookup.error, + }); + } + const selectedModel = modelLookup.model; + const modelLookupStatus = getAssistantModelLookupStatus({ + configuredModelId: configuration.modelId, + requestedModelId, + hasModel: selectedModel !== null, + failed: modelLookup.error !== null, + }); + if (modelLookupStatus === "unreachable") { + return Response.json( + { error: "The configured model endpoint could not be reached. Try again in a moment." }, + { status: 503 }, + ); + } + if (modelLookupStatus === "unavailable") { return Response.json( { error: "The selected model is not available from the configured provider." }, { status: 400 }, diff --git a/apps/nextjs/src/components/assistant/assistant-auto-approval.spec.ts b/apps/nextjs/src/components/assistant/assistant-auto-approval.spec.ts index 0de56119b..794c38148 100644 --- a/apps/nextjs/src/components/assistant/assistant-auto-approval.spec.ts +++ b/apps/nextjs/src/components/assistant/assistant-auto-approval.spec.ts @@ -96,4 +96,31 @@ describe("assistant automatic approval tracker", () => { await act(async () => root.unmount()); }); + + test("returns a failed approval to the caller and releases its claim", async () => { + const container = document.createElement("div"); + containers.push(container); + document.body.append(container); + const root = createRoot(container); + + await act(async () => { + root.render( + createElement( + AssistantAutoApprovalProvider, + { conversationId: "local-thread-1" }, + createElement(AutoApprovalProbe), + ), + ); + }); + act(() => currentAutoApproval?.setEnabled(true)); + + expect( + currentAutoApproval?.requestApproval("app-create-1", () => { + throw new Error("Approval transport failed"); + }), + ).toBe(false); + expect(currentAutoApproval?.requestApproval("app-create-1", () => undefined)).toBe(true); + + await act(async () => root.unmount()); + }); }); diff --git a/apps/nextjs/src/components/assistant/assistant-auto-approval.tsx b/apps/nextjs/src/components/assistant/assistant-auto-approval.tsx index af9bd6cbe..69675396c 100644 --- a/apps/nextjs/src/components/assistant/assistant-auto-approval.tsx +++ b/apps/nextjs/src/components/assistant/assistant-auto-approval.tsx @@ -58,9 +58,9 @@ export const AssistantAutoApprovalProvider = ({ try { approve(); return true; - } catch (error) { + } catch { trackerRef.current.release(toolCallId); - throw error; + return false; } }, [enabled], diff --git a/apps/nextjs/src/components/assistant/assistant-board-settings-tool.tsx b/apps/nextjs/src/components/assistant/assistant-board-settings-tool.tsx index c8d198673..4c71874ec 100644 --- a/apps/nextjs/src/components/assistant/assistant-board-settings-tool.tsx +++ b/apps/nextjs/src/components/assistant/assistant-board-settings-tool.tsx @@ -43,7 +43,11 @@ import { useZodForm } from "@homarr/form"; import { useI18n, useScopedI18n } from "@homarr/translation/client"; import { boardSavePartialSettingsSchema } from "@homarr/validation/board"; -import { getChangedBoardSettings, getCustomCssWarnings } from "./assistant-board-settings"; +import { + getAssistantBoardSettingsDefaultTab, + getChangedBoardSettings, + getCustomCssWarnings, +} from "./assistant-board-settings"; import { hasCompleteAssistantToolArguments, hasFailedAssistantToolArguments } from "./assistant-human-tool-status"; import { AssistantHumanToolError } from "./assistant-human-tools"; import classes from "./assistant-panel.module.css"; @@ -183,8 +187,7 @@ const BoardSettingsForm = ({ const changedCount = Object.keys(changes).length; const cssWarnings = getCustomCssWarnings(form.values.customCss); const proposedKeys = Object.keys(args.changes); - const defaultTab = - args.changes.customCss !== undefined ? "css" : proposedKeys.some(isAppearanceKey) ? "appearance" : "general"; + const defaultTab = getAssistantBoardSettingsDefaultTab(args.changes); const optionData = ( values: readonly TValue[], key: "backgroundImageAttachment" | "backgroundImageRepeat" | "backgroundImageSize", @@ -404,6 +407,3 @@ const BoardSettingsForm = ({ ); }; - -const isAppearanceKey = (key: string) => - ["primaryColor", "secondaryColor", "opacity", "iconColor", "itemRadius"].includes(key); diff --git a/apps/nextjs/src/components/assistant/assistant-board-settings.spec.ts b/apps/nextjs/src/components/assistant/assistant-board-settings.spec.ts index 39c74114b..369221f7f 100644 --- a/apps/nextjs/src/components/assistant/assistant-board-settings.spec.ts +++ b/apps/nextjs/src/components/assistant/assistant-board-settings.spec.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "vitest"; -import { getChangedBoardSettings, getCustomCssWarnings } from "./assistant-board-settings"; +import { + getAssistantBoardSettingsDefaultTab, + getChangedBoardSettings, + getCustomCssWarnings, +} from "./assistant-board-settings"; describe("getChangedBoardSettings", () => { test("returns only changed settings and treats empty nullable values equally", () => { @@ -27,3 +31,12 @@ describe("getCustomCssWarnings", () => { }); }); }); + +describe("getAssistantBoardSettingsDefaultTab", () => { + test("opens the tab containing the proposed settings", () => { + expect(getAssistantBoardSettingsDefaultTab({ pageTitle: "Home" })).toBe("general"); + expect(getAssistantBoardSettingsDefaultTab({ primaryColor: "#112233" })).toBe("appearance"); + expect(getAssistantBoardSettingsDefaultTab({ backgroundImageSize: "cover" })).toBe("background"); + expect(getAssistantBoardSettingsDefaultTab({ customCss: ".item { opacity: 0.9; }" })).toBe("css"); + }); +}); diff --git a/apps/nextjs/src/components/assistant/assistant-board-settings.ts b/apps/nextjs/src/components/assistant/assistant-board-settings.ts index c42998462..a939159fa 100644 --- a/apps/nextjs/src/components/assistant/assistant-board-settings.ts +++ b/apps/nextjs/src/components/assistant/assistant-board-settings.ts @@ -31,3 +31,27 @@ export const getCustomCssWarnings = (css: string) => ({ importsStylesheet: /@import\b/iu.test(css), loadsRemoteResource: /url\(\s*["']?https?:\/\//iu.test(css), }); + +const appearanceKeys = new Set([ + "primaryColor", + "secondaryColor", + "opacity", + "iconColor", + "itemRadius", +]); +const backgroundKeys = new Set([ + "backgroundImageUrl", + "backgroundImageAttachment", + "backgroundImageRepeat", + "backgroundImageSize", +]); + +export const getAssistantBoardSettingsDefaultTab = ( + changes: AssistantBoardSettingsChanges, +): "general" | "appearance" | "background" | "css" => { + if (changes.customCss !== undefined) return "css"; + const keys = Object.keys(changes) as (keyof AssistantBoardSettingsChanges)[]; + if (keys.some((key) => appearanceKeys.has(key))) return "appearance"; + if (keys.some((key) => backgroundKeys.has(key))) return "background"; + return "general"; +}; diff --git a/apps/nextjs/src/components/assistant/assistant-panel.tsx b/apps/nextjs/src/components/assistant/assistant-panel.tsx index 1f76f4edb..085d2976a 100644 --- a/apps/nextjs/src/components/assistant/assistant-panel.tsx +++ b/apps/nextjs/src/components/assistant/assistant-panel.tsx @@ -357,6 +357,8 @@ const ToolPart = ({ const autoApproval = useAssistantAutoApproval(); const [opened, setOpened] = useState(false); const [approvalResponse, setApprovalResponse] = useState<"approve" | "deny" | null>(null); + const [autoApprovalInProgress, setAutoApprovalInProgress] = useState(false); + const autoApprovalAttemptRef = useRef(null); const completed = status?.type === "complete"; const awaitingApproval = approval !== undefined && approval.approved === undefined && !approval.resolution; const denied = approval?.approved === false; @@ -369,12 +371,23 @@ const ToolPart = ({ const duration = timing?.completedAt !== undefined ? Math.max(0, timing.completedAt - timing.startedAt) : undefined; useEffect(() => { - if (!awaitingApproval || !autoApproval.enabled) return; + if (!awaitingApproval) { + autoApprovalAttemptRef.current = null; + setAutoApprovalInProgress(false); + return; + } + if (!autoApproval.enabled) { + setAutoApprovalInProgress(false); + return; + } + if (autoApprovalAttemptRef.current === toolCallId) return; + autoApprovalAttemptRef.current = toolCallId; - autoApproval.requestApproval(toolCallId, () => { - setApprovalResponse("approve"); + const requested = autoApproval.requestApproval(toolCallId, () => { respondToApproval({ approved: true, reason: "Approved automatically by the user." }); + setApprovalResponse("approve"); }); + setAutoApprovalInProgress(requested); }, [autoApproval, awaitingApproval, respondToApproval, toolCallId]); return ( @@ -429,7 +442,7 @@ const ToolPart = ({ {t("approvalDescription")} - {autoApproval.enabled ? ( + {autoApprovalInProgress ? ( diff --git a/apps/nextjs/src/components/assistant/assistant-preferences.spec.ts b/apps/nextjs/src/components/assistant/assistant-preferences.spec.ts index 846a8e1ae..c0dd0352d 100644 --- a/apps/nextjs/src/components/assistant/assistant-preferences.spec.ts +++ b/apps/nextjs/src/components/assistant/assistant-preferences.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "vitest"; -import { resolveAssistantPreferenceModelId } from "./assistant-preferences"; +import { resolveAssistantPreferenceModelId, resolveAssistantThreadPreferenceModelId } from "./assistant-preferences"; const models = [ { id: "provider/default", name: "Default", inputModalities: ["text"] }, @@ -52,3 +52,41 @@ describe("resolveAssistantPreferenceModelId", () => { ).toBe("provider/default"); }); }); + +describe("resolveAssistantThreadPreferenceModelId", () => { + test("waits for an existing remote conversation's metadata", () => { + expect( + resolveAssistantThreadPreferenceModelId({ + isRemote: true, + metadataLoaded: false, + threadModelId: undefined, + defaultModelId: "provider/default", + models, + }), + ).toBeUndefined(); + }); + + test("uses the default immediately for a new local conversation", () => { + expect( + resolveAssistantThreadPreferenceModelId({ + isRemote: false, + metadataLoaded: false, + threadModelId: undefined, + defaultModelId: "provider/default", + models, + }), + ).toBe("provider/default"); + }); + + test("restores a valid persisted conversation model", () => { + expect( + resolveAssistantThreadPreferenceModelId({ + isRemote: true, + metadataLoaded: true, + threadModelId: "provider/alternate", + defaultModelId: "provider/default", + models, + }), + ).toBe("provider/alternate"); + }); +}); diff --git a/apps/nextjs/src/components/assistant/assistant-preferences.ts b/apps/nextjs/src/components/assistant/assistant-preferences.ts index 8da15989e..54f6b6468 100644 --- a/apps/nextjs/src/components/assistant/assistant-preferences.ts +++ b/apps/nextjs/src/components/assistant/assistant-preferences.ts @@ -21,3 +21,21 @@ export const resolveAssistantPreferenceModelId = ({ if (currentModelId && models.some((model) => model.id === currentModelId)) return currentModelId; return defaultModelId; }; + +export const resolveAssistantThreadPreferenceModelId = ({ + isRemote, + metadataLoaded, + threadModelId, + defaultModelId, + models, +}: { + isRemote: boolean; + metadataLoaded: boolean; + threadModelId: unknown; + defaultModelId: string | null; + models: readonly AssistantRuntimeModelOption[]; +}) => { + if (isRemote && !metadataLoaded) return undefined; + if (typeof threadModelId === "string" && models.some((model) => model.id === threadModelId)) return threadModelId; + return defaultModelId; +}; diff --git a/apps/nextjs/src/components/assistant/assistant-provider.tsx b/apps/nextjs/src/components/assistant/assistant-provider.tsx index 7d30b0581..442e87bc8 100644 --- a/apps/nextjs/src/components/assistant/assistant-provider.tsx +++ b/apps/nextjs/src/components/assistant/assistant-provider.tsx @@ -32,7 +32,7 @@ import { AssistantAskUserTool, AssistantConfigureAppTool } from "./assistant-hum import { AssistantPanel } from "./assistant-panel"; import { getPendingAssistantAction } from "./assistant-pending-action"; import type { AssistantReasoningMode, AssistantRuntimeModelOption } from "./assistant-preferences"; -import { resolveAssistantPreferenceModelId } from "./assistant-preferences"; +import { resolveAssistantPreferenceModelId, resolveAssistantThreadPreferenceModelId } from "./assistant-preferences"; import { AssistantRuntimeProviderWithTools } from "./assistant-runtime-provider"; import { sendAssistantPrompt as sendPromptThroughComposer } from "./assistant-send"; import { createAssistantPromptInteraction } from "./assistant-spotlight"; @@ -481,7 +481,9 @@ const AssistantPreferenceSync = () => { const preferences = useAssistantPreferences(); // The local id distinguishes every thread immediately, including threads that do not have a remote id yet. const conversationId = useAuiState((state) => state.threadListItem.id); - const threadModelId = useAuiState((state) => state.threadListItem.custom?.modelId); + const remoteId = useAuiState((state) => state.threadListItem.remoteId); + const threadCustom = useAuiState((state) => state.threadListItem.custom); + const threadModelId = threadCustom?.modelId; const previousSyncKeyRef = useRef(null); const modelCatalogKey = preferences.models.map((model) => model.id).join("\0"); @@ -489,14 +491,17 @@ const AssistantPreferenceSync = () => { if (preferences.models.length === 0) return; const syncKey = `${conversationId}:${modelCatalogKey}`; if (previousSyncKeyRef.current === syncKey) return; + const nextModelId = resolveAssistantThreadPreferenceModelId({ + isRemote: remoteId !== undefined, + metadataLoaded: threadCustom !== undefined, + threadModelId, + defaultModelId: preferences.defaultModelId, + models: preferences.models, + }); + if (nextModelId === undefined) return; previousSyncKeyRef.current = syncKey; - const storedModelId = typeof threadModelId === "string" ? threadModelId : null; - const nextModelId = - storedModelId && preferences.models.some((model) => model.id === storedModelId) - ? storedModelId - : preferences.defaultModelId; if (nextModelId) preferences.setModelId(nextModelId); - }, [conversationId, modelCatalogKey, preferences, threadModelId]); + }, [conversationId, modelCatalogKey, preferences, remoteId, threadCustom, threadModelId]); return null; }; diff --git a/apps/nextjs/src/components/assistant/assistant-tool-contracts.spec.ts b/apps/nextjs/src/components/assistant/assistant-tool-contracts.spec.ts index 270f357fc..b9414bc74 100644 --- a/apps/nextjs/src/components/assistant/assistant-tool-contracts.spec.ts +++ b/apps/nextjs/src/components/assistant/assistant-tool-contracts.spec.ts @@ -113,5 +113,10 @@ describe("assistant human tool contracts", () => { expect(normalizeAssistantAppIconUrl("youtube")).toBe(""); expect(normalizeAssistantAppIconUrl("/api/media/youtube.svg")).toBe("/api/media/youtube.svg"); expect(normalizeAssistantAppIconUrl("https://icons.example/youtube.svg")).toBe("https://icons.example/youtube.svg"); + expect(normalizeAssistantAppIconUrl("http://icons.example/youtube.svg")).toBe("http://icons.example/youtube.svg"); + expect(normalizeAssistantAppIconUrl("//icons.example/youtube.svg")).toBe(""); + expect(normalizeAssistantAppIconUrl("javascript:alert(1)")).toBe(""); + expect(normalizeAssistantAppIconUrl("data:image/svg+xml,")).toBe(""); + expect(normalizeAssistantAppIconUrl("file:///etc/passwd")).toBe(""); }); }); diff --git a/apps/nextjs/src/components/assistant/assistant-tool-contracts.ts b/apps/nextjs/src/components/assistant/assistant-tool-contracts.ts index 8ce0e6bf3..c8282b876 100644 --- a/apps/nextjs/src/components/assistant/assistant-tool-contracts.ts +++ b/apps/nextjs/src/components/assistant/assistant-tool-contracts.ts @@ -111,5 +111,8 @@ export type ConfigureBoardSettingsResult = export const normalizeAssistantAppIconUrl = (value: string | undefined) => { if (!value) return ""; - return value.startsWith("/") || URL.canParse(value) ? value : ""; + if (value.startsWith("/") && !value.startsWith("//")) return value; + if (!URL.canParse(value)) return ""; + const protocol = new URL(value).protocol; + return protocol === "http:" || protocol === "https:" ? value : ""; }; diff --git a/apps/nextjs/src/components/assistant/assistant-tool-result.spec.ts b/apps/nextjs/src/components/assistant/assistant-tool-result.spec.ts index 733585db3..ad441384f 100644 --- a/apps/nextjs/src/components/assistant/assistant-tool-result.spec.ts +++ b/apps/nextjs/src/components/assistant/assistant-tool-result.spec.ts @@ -73,6 +73,30 @@ describe("getToolResultPresentation", () => { }); }); + test("removes URL credentials and query parameters from inline descriptions", () => { + expect( + getToolResultPresentation({ + apps: [ + { + name: "Sonarr", + url: "https://user:password@sonarr.example/api/v3/series?apikey=secret#calendar", + }, + ], + }), + ).toEqual({ + type: "collection", + totalCount: 1, + items: [ + { + title: "Sonarr", + description: "https://sonarr.example/api/v3/series#calendar", + badges: [], + fields: [], + }, + ], + }); + }); + test("limits large collections while preserving their total", () => { const result = getToolResultPresentation({ items: Array.from({ length: 8 }, (_, index) => ({ name: `Item ${index + 1}` })), diff --git a/apps/nextjs/src/components/assistant/assistant-tool-result.ts b/apps/nextjs/src/components/assistant/assistant-tool-result.ts index 82da513e5..ffce65859 100644 --- a/apps/nextjs/src/components/assistant/assistant-tool-result.ts +++ b/apps/nextjs/src/components/assistant/assistant-tool-result.ts @@ -31,6 +31,17 @@ const maximumFields = 6; type ToolResultPrimitive = string | number | boolean; +const redactUrlCredentials = (value: string) => { + if (!URL.canParse(value)) return value; + const url = new URL(value); + if (url.protocol !== "http:" && url.protocol !== "https:") return value; + if (!url.username && !url.password && !url.search) return value; + url.username = ""; + url.password = ""; + url.search = ""; + return url.toString(); +}; + export interface ToolResultField { label: string; value: ToolResultPrimitive; @@ -65,7 +76,7 @@ const toDisplayValue = (value: unknown): ToolResultPrimitive | undefined => { if (typeof value === "string") { const trimmed = value.trim(); if (trimmed.length === 0 || trimmed.length > 160) return undefined; - return trimmed; + return redactUrlCredentials(trimmed); } if (typeof value === "number" && Number.isFinite(value)) return value; if (typeof value === "boolean") return value; diff --git a/apps/nextjs/src/components/board/items/actions/test/empty-position.spec.ts b/apps/nextjs/src/components/board/items/actions/test/empty-position.spec.ts index 848ea9ac2..1325f41cc 100644 --- a/apps/nextjs/src/components/board/items/actions/test/empty-position.spec.ts +++ b/apps/nextjs/src/components/board/items/actions/test/empty-position.spec.ts @@ -18,6 +18,7 @@ describe("get first empty position", () => { 0, ], [[["a", "a", "a", "a"]], [1, 1], 0, 1], + [[["a", " ", " ", " "]], [4, 1], 0, 1], [ [ ["a", "a", "a", "a"], diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index 11752f2f1..d68ec65e0 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -237,8 +237,9 @@ const fetchModelsAsync = async (configuration: AssistantConfiguration) => { .flatMap((model) => { const id = typeof model.id === "string" ? model.id : typeof model.model === "string" ? model.model : null; if (!id || id.length > 256) return []; - const name = + const rawName = typeof model.name === "string" ? model.name : typeof model.display_name === "string" ? model.display_name : id; + const name = rawName.trim() || id; const contextLength = typeof model.context_length === "number" && Number.isSafeInteger(model.context_length) && diff --git a/packages/api/src/router/test/assistant.spec.ts b/packages/api/src/router/test/assistant.spec.ts index f71b5b26c..4c37afebc 100644 --- a/packages/api/src/router/test/assistant.spec.ts +++ b/packages/api/src/router/test/assistant.spec.ts @@ -170,6 +170,12 @@ describe("assistant conversation features", () => { supported_parameters: [], architecture: { output_modalities: ["text"] }, }, + { + id: "example/unnamed", + name: "", + supported_parameters: ["tools"], + architecture: { output_modalities: ["text"] }, + }, ], }), ), @@ -187,6 +193,10 @@ describe("assistant conversation features", () => { name: "Example Model", inputModalities: ["text", "image"], }), + expect.objectContaining({ + id: "example/unnamed", + name: "example/unnamed", + }), ], }); expect(JSON.stringify(options)).not.toContain("encrypted"); diff --git a/packages/translation/src/lang/en.json b/packages/translation/src/lang/en.json index 0ca29a2ec..4e9fa851e 100644 --- a/packages/translation/src/lang/en.json +++ b/packages/translation/src/lang/en.json @@ -1970,6 +1970,7 @@ "management": "Management", "assistant": "Homarr Assistant", "assistantThinking": "Thinking", + "modifier": "Mod", "docker": "Docker", "preferences": "Your preferences", "logout": "Logout", @@ -2493,6 +2494,10 @@ } }, "widget": { + "dynamicSelect": { + "noResults": "No results found", + "loadError": "Failed to load options" + }, "integrationSelect": { "configureIntegrationFirst": "Connect an integration first", "loading": "Loading...", @@ -6084,6 +6089,7 @@ } }, "assistant": { + "loading": "Loading assistant configuration", "title": "Assistant", "description": "Connect an OpenAI-compatible model endpoint for the instance-wide assistant.", "security": { @@ -6245,9 +6251,9 @@ "manualFallback": "Discovery is unavailable. You can still enter a model ID manually.", "saveConnectionFirst": "Save the provider connection before choosing a model.", "discovering": "Refreshing the models available from this provider…", - "discovered": "Discovered {count} models. Tool support depends on the selected endpoint and model.", + "discovered": "{count, plural, one {Discovered # model. Tool support depends on the selected endpoint and model.} other {Discovered # models. Tool support depends on the selected endpoint and model.}}", "discoveryFailed": "Model discovery did not complete", - "context": "{count} token context", + "context": "{count, number} token context", "toolsConfirmed": "Tools advertised", "toolsUnknown": "Verify tool support" }, diff --git a/packages/widgets/src/_inputs/widget-dynamic-select-input.tsx b/packages/widgets/src/_inputs/widget-dynamic-select-input.tsx index 398f1df72..27ea719cc 100644 --- a/packages/widgets/src/_inputs/widget-dynamic-select-input.tsx +++ b/packages/widgets/src/_inputs/widget-dynamic-select-input.tsx @@ -6,7 +6,7 @@ import { useDebouncedValue } from "@mantine/hooks"; import { IconCheck } from "@tabler/icons-react"; import { translateIfNecessary } from "@homarr/translation"; -import { useI18n } from "@homarr/translation/client"; +import { useI18n, useScopedI18n } from "@homarr/translation/client"; import type { CommonWidgetInputProps } from "./common"; import { useWidgetInputTranslation } from "./common"; @@ -25,6 +25,7 @@ export const WidgetDynamicSelectInput = ({ boardId, }: CommonWidgetInputProps<"dynamicSelect">) => { const t = useI18n(); + const tSelect = useScopedI18n("widget.dynamicSelect"); const tWidget = useWidgetInputTranslation(kind, property); const form = useFormContext(); const inputProps = form.getInputProps(`options.${property}`); @@ -77,7 +78,7 @@ export const WidgetDynamicSelectInput = ({ searchValue={search} onSearchChange={setSearch} placeholder={tWidget("placeholder")} - nothingFoundMessage={t("common.noResults")} + nothingFoundMessage={tSelect("noResults")} leftSection={isPending && } renderOption={({ option, checked }) => { return ( diff --git a/packages/widgets/src/assistant/component.tsx b/packages/widgets/src/assistant/component.tsx index 00aa4c7ba..da79d418c 100644 --- a/packages/widgets/src/assistant/component.tsx +++ b/packages/widgets/src/assistant/component.tsx @@ -3,14 +3,14 @@ import { Center, Stack, Text, ThemeIcon } from "@mantine/core"; import { IconRobot } from "@tabler/icons-react"; -import { useI18n } from "@homarr/translation/client"; +import { useScopedI18n } from "@homarr/translation/client"; import type { WidgetComponentProps } from "../definition"; import { useAssistantWidgetRenderer } from "./context"; export default function AssistantWidget(props: WidgetComponentProps<"assistant">) { const Renderer = useAssistantWidgetRenderer(); - const t = useI18n(); + const t = useScopedI18n("widget.assistant.unavailable"); if (Renderer) return ; @@ -21,10 +21,10 @@ export default function AssistantWidget(props: WidgetComponentProps<"assistant"> - {t("widget.assistant.unavailable.title")} + {t("title")} - {t("widget.assistant.unavailable.description")} + {t("description")} diff --git a/packages/widgets/src/assistant/index.ts b/packages/widgets/src/assistant/index.ts index 0d8d99397..fad37db51 100644 --- a/packages/widgets/src/assistant/index.ts +++ b/packages/widgets/src/assistant/index.ts @@ -21,10 +21,11 @@ export const { definition, componentLoader } = createWidgetDefinition("assistant conversation: factory.dynamicSelect({ withDescription: true, useOptions(query) { - const { data: threads = [], isPending } = clientApi.assistant.listThreads.useQuery(); + const { data: threads = [], isPending, isError } = clientApi.assistant.listThreads.useQuery(); const normalizedQuery = query.trim().toLocaleLowerCase(); return { isPending, + isError, options: threads .filter((thread) => { if (normalizedQuery.length === 0) return true; diff --git a/packages/widgets/src/options.ts b/packages/widgets/src/options.ts index bdcb715be..43428fd4b 100644 --- a/packages/widgets/src/options.ts +++ b/packages/widgets/src/options.ts @@ -61,6 +61,7 @@ interface DynamicSelectInput extends CommonInput { ) => { error?: string; isPending: boolean; + isError?: boolean; options: DynamicSelectOption[]; }; }