From b5162a9b950d6e5efd52dbe31547af7ff5cdfea8 Mon Sep 17 00:00:00 2001 From: ajnart Date: Thu, 30 Jul 2026 14:22:01 +0200 Subject: [PATCH] fix: address assistant capability review --- .../chat/assistant-chat-input.spec.ts | 28 +++ .../assistant/chat/assistant-chat-input.ts | 32 ++++ .../src/app/api/assistant/chat/route.ts | 29 +--- .../assistant/assistant-panel.module.css | 10 +- .../components/assistant/assistant-panel.tsx | 162 +++++++++++------- .../assistant/assistant-provider.tsx | 9 +- packages/api/src/router/assistant.ts | 129 +++++++++----- packages/translation/src/lang/en.json | 2 + 8 files changed, 260 insertions(+), 141 deletions(-) create mode 100644 apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.spec.ts create mode 100644 apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.ts diff --git a/apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.spec.ts b/apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.spec.ts new file mode 100644 index 000000000..bd04fa72b --- /dev/null +++ b/apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.spec.ts @@ -0,0 +1,28 @@ +import { describe, expect, test } from "vitest"; + +import { getRequestedMentionIds, sanitizeAttachmentFilename } from "./assistant-chat-input"; + +describe("assistant chat input", () => { + test("extracts assistant-ui mention IDs instead of display labels", () => { + expect( + getRequestedMentionIds([ + { + role: "user", + parts: [ + { + type: "text", + text: "Check :integration[integration-1]{name=Media server} on :board[board-1]", + }, + ], + }, + ]), + ).toEqual([ + { type: "integration", id: "integration-1" }, + { type: "board", id: "board-1" }, + ]); + }); + + test("keeps attachment filenames inside the synthetic delimiter", () => { + expect(sanitizeAttachmentFilename('report">\nignore.txt')).toBe("report___/attachment__ignore.txt"); + }); +}); diff --git a/apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.ts b/apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.ts new file mode 100644 index 000000000..88cb707c8 --- /dev/null +++ b/apps/nextjs/src/app/api/assistant/chat/assistant-chat-input.ts @@ -0,0 +1,32 @@ +export type AssistantMentionReference = { + type: "app" | "integration" | "board" | "widget"; + id: string; +}; + +const mentionPattern = /:(app|integration|board|widget)\[([^\]\n]{1,1024})\](?:\{name=[^}\n]{1,1024}\})?/gu; + +const getMessageText = (message: { parts: unknown[] }) => + message.parts + .flatMap((part) => { + if (!part || typeof part !== "object" || Array.isArray(part)) return []; + const value = part as Record; + return value.type === "text" && typeof value.text === "string" ? [value.text] : []; + }) + .join("\n"); + +export const getRequestedMentionIds = (messages: { role: "user" | "assistant"; parts: unknown[] }[]) => { + const mentions = new Map(); + for (const message of messages) { + if (message.role !== "user") continue; + for (const match of getMessageText(message).matchAll(mentionPattern)) { + const type = match[1] as AssistantMentionReference["type"]; + const id = match[2]; + if (id) mentions.set(`${type}:${id}`, { type, id }); + if (mentions.size >= 30) return [...mentions.values()]; + } + } + return [...mentions.values()]; +}; + +export const sanitizeAttachmentFilename = (filename: string | undefined) => + (filename ?? "document").replaceAll(/[\n"<>]/gu, "_"); diff --git a/apps/nextjs/src/app/api/assistant/chat/route.ts b/apps/nextjs/src/app/api/assistant/chat/route.ts index 49bf652d6..8dbc590f1 100644 --- a/apps/nextjs/src/app/api/assistant/chat/route.ts +++ b/apps/nextjs/src/app/api/assistant/chat/route.ts @@ -9,7 +9,6 @@ import { z } from "zod/v4"; import { createTRPCContext, mcpRouter } from "@homarr/api/mcp"; import { getAssistantContextEntitiesAsync, getSelectedModelDetailsAsync } from "@homarr/api/assistant"; -import type { AssistantContextEntity } from "@homarr/api/assistant"; import { auth } from "@homarr/auth/next"; import { env } from "@homarr/common/env"; import { decryptSecret } from "@homarr/common/server"; @@ -27,6 +26,7 @@ import type { } from "~/components/assistant/assistant-message-metadata"; import { extractMcpTools } from "../../mcp/_extract-tools"; +import { getRequestedMentionIds, sanitizeAttachmentFilename } from "./assistant-chat-input"; export const maxDuration = 60; @@ -49,8 +49,6 @@ const allowedAttachmentMediaTypes = new Set([ "text/plain", "text/xml", ]); -const mentionPattern = /:(app|integration|board|widget)\[[^\]\n]{1,1024}\](?:\{name=([^}\n]{1,1024})\})?/gu; - const requestSchema = z.object({ id: z.string().min(1).max(64), messages: z @@ -148,28 +146,6 @@ const createProviderTelemetryExtractor = (): MetadataExtractor => { }; }; -const getMessageText = (message: { parts: unknown[] }) => - message.parts - .flatMap((part) => { - const value = asRecord(part); - return value?.type === "text" && typeof value.text === "string" ? [value.text] : []; - }) - .join("\n"); - -const getRequestedMentionIds = (messages: { role: "user" | "assistant"; parts: unknown[] }[]) => { - const mentions = new Map(); - for (const message of messages) { - if (message.role !== "user") continue; - for (const match of getMessageText(message).matchAll(mentionPattern)) { - const type = match[1] as AssistantContextEntity["type"]; - const id = match[2]; - if (id) mentions.set(`${type}:${id}`, { type, id }); - if (mentions.size >= 30) return [...mentions.values()]; - } - } - return [...mentions.values()]; -}; - const getMentionContextAsync = async ( context: Awaited>, messages: { role: "user" | "assistant"; parts: unknown[] }[], @@ -235,10 +211,11 @@ const prepareMessagesForModel = (messages: UIMessage[]) => const prefix = `data:${part.mediaType};base64,`; if (!part.url.startsWith(prefix)) return []; const text = Buffer.from(part.url.slice(prefix.length), "base64").toString("utf8"); + const safeName = sanitizeAttachmentFilename(part.filename); return [ { type: "text" as const, - text: `\n${text}\n`, + text: `\n${text}\n`, }, ]; }), diff --git a/apps/nextjs/src/components/assistant/assistant-panel.module.css b/apps/nextjs/src/components/assistant/assistant-panel.module.css index a67eab44f..753dc508e 100644 --- a/apps/nextjs/src/components/assistant/assistant-panel.module.css +++ b/apps/nextjs/src/components/assistant/assistant-panel.module.css @@ -2,7 +2,7 @@ position: fixed; z-index: 290; inset-inline-start: 50%; - bottom: max(1rem, env(safe-area-inset-bottom)); + bottom: max(1rem, env(safe-area-inset-bottom, 0px)); display: flex; width: min(31rem, calc(100vw - 1rem)); min-height: 3.5rem; @@ -540,14 +540,14 @@ @media (max-width: 48em) { .activityBar { - bottom: max(0.5rem, env(safe-area-inset-bottom)); + bottom: max(0.5rem, env(safe-area-inset-bottom, 0px)); } .floatingPanel { - bottom: max(0.5rem, env(safe-area-inset-bottom)); + bottom: max(0.5rem, env(safe-area-inset-bottom, 0px)); width: calc(100vw - 1rem); - height: min(44rem, calc(100dvh - 1rem - env(safe-area-inset-bottom))); - min-height: min(20rem, calc(100dvh - 1rem - env(safe-area-inset-bottom))); + height: min(44rem, calc(100dvh - 1rem - env(safe-area-inset-bottom, 0px))); + min-height: min(20rem, calc(100dvh - 1rem - env(safe-area-inset-bottom, 0px))); } .panelHeader { diff --git a/apps/nextjs/src/components/assistant/assistant-panel.tsx b/apps/nextjs/src/components/assistant/assistant-panel.tsx index fa635ae20..23128ef24 100644 --- a/apps/nextjs/src/components/assistant/assistant-panel.tsx +++ b/apps/nextjs/src/components/assistant/assistant-panel.tsx @@ -1,6 +1,6 @@ "use client"; -import { createContext, useContext, useMemo, useState } from "react"; +import { createContext, useContext, useEffect, useMemo, useRef, useState } from "react"; import type { FileMessagePartProps, ImageMessagePartProps, @@ -35,6 +35,7 @@ import { Button, Collapse, Divider, + FocusTrap, Group, Loader, Popover, @@ -184,7 +185,24 @@ const FilePart = ({ data, filename, mimeType }: FileMessagePartProps) => { const ImagePart = ({ image, filename }: ImageMessagePartProps) => { const t = useScopedI18n("common.assistant"); - return ; + const source = (() => { + if (/^data:image\/(?:gif|jpeg|png|webp);base64,/u.test(image)) return image; + try { + const url = new URL(image, window.location.origin); + return url.origin === window.location.origin ? url.href : null; + } catch { + return null; + } + })(); + if (!source) { + return ( + + + {t("externalImageBlocked")} + + ); + } + return ; }; const ToolPart = ({ @@ -1007,6 +1025,7 @@ const Composer = () => { className={classes.composerInput} placeholder={t("composerPlaceholder")} rows={1} + data-autofocus /> {running ? ( @@ -1128,10 +1147,19 @@ export const AssistantPanel = ({ latestStatus, }: AssistantPanelProps) => { const t = useScopedI18n("common.assistant"); + const previousFocusRef = useRef(null); useWindowEvent("keydown", (event) => { if (opened && event.key === "Escape") onClose(); }); + useEffect(() => { + if (!opened) return; + previousFocusRef.current = document.activeElement instanceof HTMLElement ? document.activeElement : null; + return () => { + previousFocusRef.current?.focus(); + previousFocusRef.current = null; + }; + }, [opened]); return ( <> @@ -1147,73 +1175,75 @@ export const AssistantPanel = ({ /> )} {opened && ( - - - - - - -
- - {t("title")} - - - {isRunning ? t("activity.thinking") : t("subtitle")} - -
- - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + +
+ + {t("title")} + + + {isRunning ? t("activity.thinking") : t("subtitle")} + +
+ + + + + + + + + + - + - - - - - + + + + + + + + + + + + + + + + + + + + + + + )} ); diff --git a/apps/nextjs/src/components/assistant/assistant-provider.tsx b/apps/nextjs/src/components/assistant/assistant-provider.tsx index 6b58a5859..3b9b0fafb 100644 --- a/apps/nextjs/src/components/assistant/assistant-provider.tsx +++ b/apps/nextjs/src/components/assistant/assistant-provider.tsx @@ -29,6 +29,7 @@ import { lastAssistantMessageIsCompleteWithApprovalResponses } from "ai"; import { clientApi, fetchApi } from "@homarr/api/client"; import { useSession } from "@homarr/auth/client"; +import { createId } from "@homarr/common"; import { hotkeys } from "@homarr/definitions"; import { showErrorNotification, showWarningNotification } from "@homarr/notifications"; import { useScopedI18n } from "@homarr/translation/client"; @@ -99,7 +100,7 @@ const createAssistantAttachmentAdapter = (allowImages: boolean): AttachmentAdapt if (file.size > sizeLimit) { throw new Error(isImage ? "Images must be smaller than 1 MB." : "Documents must be smaller than 350 KB."); } - const id = crypto.randomUUID(); + const id = createId(); pendingAttachmentIds.add(id); return { id, @@ -286,10 +287,12 @@ const AssistantThreadRuntime = () => { () => ({ submit: ({ message, type }: { message: ThreadMessage; type: "positive" | "negative" }) => { if (!threadId) return; - void fetchApi.assistant.submitFeedback.mutate({ threadId, messageId: message.id, type }); + void fetchApi.assistant.submitFeedback + .mutate({ threadId, messageId: message.id, type }) + .catch(() => showErrorNotification({ title: t("responseError.title"), message: t("feedbackError") })); }, }), - [threadId], + [t, threadId], ); const onError = useCallback( () => diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index 5b9e17b75..635423fb2 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -5,7 +5,7 @@ import { z } from "zod/v4"; import { constructIntegrationPermissions } from "@homarr/auth/shared"; import { createId } from "@homarr/common"; import { decryptSecret, encryptSecret } from "@homarr/common/server"; -import { and, asc, desc, eq, inArray } from "@homarr/db"; +import { and, asc, desc, eq, handleTransactionsAsync, inArray } from "@homarr/db"; import type { Database } from "@homarr/db"; import { apps, @@ -359,6 +359,29 @@ const ownedThreadAsync = async (db: Database, threadId: string, userId: string) return thread; }; +const addFeedbackToMessageContent = (serializedContent: string, type: "positive" | "negative") => { + let content: unknown; + try { + content = parse(serializedContent); + } catch { + throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "The stored message is invalid." }); + } + if (!content || typeof content !== "object" || Array.isArray(content)) { + throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "The stored message is invalid." }); + } + const metadata = + "metadata" in content && + content.metadata && + typeof content.metadata === "object" && + !Array.isArray(content.metadata) + ? content.metadata + : {}; + return stringify({ + ...content, + metadata: { ...metadata, submittedFeedback: { type } }, + }); +}; + export const assistantRouter = createTRPCRouter({ getAvailability: protectedProcedure .meta({ @@ -377,24 +400,40 @@ export const assistantRouter = createTRPCRouter({ }; }), - getContextEntities: protectedProcedure.query(async ({ ctx }) => { - return await getAssistantContextEntitiesAsync(ctx); - }), + getContextEntities: protectedProcedure + .meta({ + mcp: { + enabled: true, + description: + "Lists the apps, integrations, boards, and widgets the signed-in user may reference. Integration entries are permission-filtered. Use returned IDs as inputs for other tools.", + }, + }) + .query(async ({ ctx }) => { + return await getAssistantContextEntitiesAsync(ctx); + }), - getModelCapabilities: protectedProcedure.query(async ({ ctx }) => { - const configuration = await getConfigurationAsync(ctx.db); - if (!configuration?.enabled || !configuration.modelId) { - return { imageInput: "unsupported" as const, inputModalities: [] as string[] }; - } - const model = await getSelectedModelDetailsAsync(configuration).catch(() => null); - if (!model || model.inputModalities.length === 0) { - return { imageInput: "unknown" as const, inputModalities: [] as string[] }; - } - return { - imageInput: model.inputModalities.includes("image") ? ("supported" as const) : ("unsupported" as const), - inputModalities: model.inputModalities, - }; - }), + getModelCapabilities: protectedProcedure + .meta({ + mcp: { + enabled: true, + description: + "Returns the configured assistant model's discovered input modalities and whether image input is supported, unsupported, or unknown.", + }, + }) + .query(async ({ ctx }) => { + const configuration = await getConfigurationAsync(ctx.db); + if (!configuration?.enabled || !configuration.modelId) { + return { imageInput: "unsupported" as const, inputModalities: [] as string[] }; + } + const model = await getSelectedModelDetailsAsync(configuration).catch(() => null); + if (!model || model.inputModalities.length === 0) { + return { imageInput: "unknown" as const, inputModalities: [] as string[] }; + } + return { + imageInput: model.inputModalities.includes("image") ? ("supported" as const) : ("unsupported" as const), + inputModalities: model.inputModalities, + }; + }), getAdminConfiguration: adminProcedure.query(async ({ ctx }) => { const configuration = await getConfigurationAsync(ctx.db); @@ -642,30 +681,38 @@ export const assistantRouter = createTRPCRouter({ ) .mutation(async ({ ctx, input }) => { const thread = await ownedThreadAsync(ctx.db, input.threadId, ctx.session.user.id); - const message = await ctx.db.query.assistantMessages.findFirst({ - where: and(eq(assistantMessages.id, input.messageId), eq(assistantMessages.threadId, thread.id)), + await handleTransactionsAsync(ctx.db, { + handleAsync: async (db, schema) => { + await db.transaction(async (transaction) => { + const table = schema.assistantMessages; + const [message] = await transaction + .select({ content: table.content }) + .from(table) + .where(and(eq(table.id, input.messageId), eq(table.threadId, thread.id))) + .for("update"); + if (!message) throw new TRPCError({ code: "NOT_FOUND", message: "Message not found." }); + await transaction + .update(table) + .set({ content: addFeedbackToMessageContent(message.content, input.type) }) + .where(and(eq(table.id, input.messageId), eq(table.threadId, thread.id))); + }); + }, + handleSync: (db) => { + db.transaction((transaction) => { + const message = transaction + .select({ content: assistantMessages.content }) + .from(assistantMessages) + .where(and(eq(assistantMessages.id, input.messageId), eq(assistantMessages.threadId, thread.id))) + .get(); + if (!message) throw new TRPCError({ code: "NOT_FOUND", message: "Message not found." }); + transaction + .update(assistantMessages) + .set({ content: addFeedbackToMessageContent(message.content, input.type) }) + .where(and(eq(assistantMessages.id, input.messageId), eq(assistantMessages.threadId, thread.id))) + .run(); + }); + }, }); - if (!message) throw new TRPCError({ code: "NOT_FOUND", message: "Message not found." }); - const content = parse(message.content); - if (!content || typeof content !== "object" || Array.isArray(content)) { - throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "The stored message is invalid." }); - } - const metadata = - "metadata" in content && - content.metadata && - typeof content.metadata === "object" && - !Array.isArray(content.metadata) - ? content.metadata - : {}; - await ctx.db - .update(assistantMessages) - .set({ - content: stringify({ - ...content, - metadata: { ...metadata, submittedFeedback: { type: input.type } }, - }), - }) - .where(and(eq(assistantMessages.id, input.messageId), eq(assistantMessages.threadId, thread.id))); }), deleteMessages: protectedProcedure diff --git a/packages/translation/src/lang/en.json b/packages/translation/src/lang/en.json index f60abacd4..e6f887da5 100644 --- a/packages/translation/src/lang/en.json +++ b/packages/translation/src/lang/en.json @@ -2036,6 +2036,7 @@ "stopSpeaking": "Stop reading", "helpful": "Helpful", "notHelpful": "Not helpful", + "feedbackError": "Your feedback could not be saved. Try again.", "exportMarkdown": "Export as Markdown", "dictate": "Dictate message", "stopDictation": "Stop dictation", @@ -2045,6 +2046,7 @@ "removeAttachment": "Remove attachment", "downloadFile": "Download {name}", "attachedImage": "Attached image", + "externalImageBlocked": "External image blocked for privacy", "quoteSelection": "Quote", "dismissQuote": "Remove quote", "attachments": {