diff --git a/apps/client/src/features/ai-chat/components/ai-chat-window.tsx b/apps/client/src/features/ai-chat/components/ai-chat-window.tsx index 122f80ff..90b6e33d 100644 --- a/apps/client/src/features/ai-chat/components/ai-chat-window.tsx +++ b/apps/client/src/features/ai-chat/components/ai-chat-window.tsx @@ -400,7 +400,16 @@ export default function AiChatWindow() { >
setHistoryOpen((o) => !o)} + onKeyDown={(event) => { + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + setHistoryOpen((o) => !o); + } + }} > onTurnFinished(), + // In AI SDK v6 `onFinish` does NOT fire when the stream errors, so a brand + // new chat that fails on its first turn would never invalidate the chat list + // nor adopt the server-created chat id (the server still creates the row and + // saves the error message). Run the same post-turn path on error so the + // failed chat appears in history immediately instead of after a manual + // refresh. The error itself is still surfaced via `error` below. + onError: () => onTurnFinished(), }); const isStreaming = status === "submitted" || status === "streaming"; diff --git a/apps/client/src/features/ai-chat/components/conversation-list.tsx b/apps/client/src/features/ai-chat/components/conversation-list.tsx index c4c566dd..8fe4549c 100644 --- a/apps/client/src/features/ai-chat/components/conversation-list.tsx +++ b/apps/client/src/features/ai-chat/components/conversation-list.tsx @@ -115,7 +115,17 @@ export default function ConversationList({ classes.conversationItem, isActive && classes.conversationItemActive, )} + role="button" + tabIndex={0} onClick={() => onSelect(chat.id)} + onKeyDown={(e) => { + // Activate on Enter/Space like a native button; the inner menu + // button stops propagation so its own keys never reach this row. + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onSelect(chat.id); + } + }} > {chat.title || t("Untitled chat")} diff --git a/apps/client/src/features/ai-chat/components/message-item.tsx b/apps/client/src/features/ai-chat/components/message-item.tsx index 680d4715..4ba1d934 100644 --- a/apps/client/src/features/ai-chat/components/message-item.tsx +++ b/apps/client/src/features/ai-chat/components/message-item.tsx @@ -3,7 +3,7 @@ import { IconAlertTriangle } from "@tabler/icons-react"; import { useTranslation } from "react-i18next"; import type { UIMessage } from "@ai-sdk/react"; import ToolCallCard from "@/features/ai-chat/components/tool-call-card.tsx"; -import { ToolUiPart } from "@/features/ai-chat/utils/tool-parts.tsx"; +import { ToolUiPart, isToolPart } from "@/features/ai-chat/utils/tool-parts.tsx"; import { renderChatMarkdown } from "@/features/ai-chat/utils/markdown.ts"; import { describeChatError } from "@/features/ai-chat/utils/error-message.ts"; import classes from "@/features/ai-chat/components/ai-chat.module.css"; @@ -12,11 +12,6 @@ interface MessageItemProps { message: UIMessage; } -/** True for AI SDK tool parts (static `tool-*` or `dynamic-tool`). */ -function isToolPart(type: string): boolean { - return type.startsWith("tool-") || type === "dynamic-tool"; -} - /** * Render a single UIMessage by iterating its `parts`: * - `text` parts -> sanitized markdown. diff --git a/apps/client/src/features/ai-chat/components/message-list.tsx b/apps/client/src/features/ai-chat/components/message-list.tsx index fb9d137e..ed0fb73d 100644 --- a/apps/client/src/features/ai-chat/components/message-list.tsx +++ b/apps/client/src/features/ai-chat/components/message-list.tsx @@ -4,6 +4,7 @@ import { useTranslation } from "react-i18next"; import type { UIMessage } from "@ai-sdk/react"; import MessageItem from "@/features/ai-chat/components/message-item.tsx"; import TypingIndicator from "@/features/ai-chat/components/typing-indicator.tsx"; +import { isToolPart } from "@/features/ai-chat/utils/tool-parts.tsx"; import classes from "@/features/ai-chat/components/ai-chat.module.css"; interface MessageListProps { @@ -11,11 +12,6 @@ interface MessageListProps { isStreaming: boolean; } -/** True for AI SDK tool parts (static `tool-*` or `dynamic-tool`). */ -function isToolPart(type: string): boolean { - return type.startsWith("tool-") || type === "dynamic-tool"; -} - // Distance (px) from the bottom within which the viewport still counts as // "pinned" — absorbs sub-pixel rounding and small content jitter. const BOTTOM_THRESHOLD = 40; diff --git a/apps/client/src/features/ai-chat/utils/tool-parts.tsx b/apps/client/src/features/ai-chat/utils/tool-parts.tsx index e7705936..be972050 100644 --- a/apps/client/src/features/ai-chat/utils/tool-parts.tsx +++ b/apps/client/src/features/ai-chat/utils/tool-parts.tsx @@ -5,9 +5,11 @@ * * A tool part's `type` is `tool-${toolName}` (AI SDK v6 static tool parts) and * its `state` is one of input-streaming / input-available / output-available / - * output-error (we only surface running / done / error). The server tools are: - * searchPages, getPage, createPage, updatePageContent, renamePage, movePage, - * deletePage, createComment, resolveComment — see ai-chat-tools.service.ts. + * output-error (we only surface running / done / error). The full toolset the + * server exposes lives in `ai-chat-tools.service.ts` (the agent now exposes the + * complete Docmost toolset); friendly action-log labels exist ONLY for the + * tools listed in `toolLabelKey` below — every other tool falls through to the + * generic "Ran tool {{name}}" label. */ /** A tool UI part as it arrives from `useChat` / persisted history. */ @@ -38,6 +40,11 @@ export interface ToolCitation { href: string; } +/** True for AI SDK tool parts (static `tool-*` or `dynamic-tool`). */ +export function isToolPart(type: string): boolean { + return type.startsWith("tool-") || type === "dynamic-tool"; +} + /** Extract the tool name from a part `type` of `tool-${name}` (or dynamic). */ export function getToolName(part: ToolUiPart): string { if (part.type === "dynamic-tool") return part.toolName ?? ""; diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-form.tsx b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-form.tsx index 3e6a8958..1d07e7d5 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-form.tsx +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-form.tsx @@ -47,6 +47,21 @@ interface AiMcpServerFormProps { onClose: () => void; } +// Build the form's field values from a (possibly undefined) server. Used both +// for the initial mount and for re-hydration when the modal is reused for a +// different server, so the two stay in sync. authHeader is always empty: it is +// a write-only secret buffer never echoed back from the server. +function buildInitialValues(server?: IAiMcpServer): FormValues { + return { + name: server?.name ?? "", + transport: server?.transport ?? "http", + url: server?.url ?? "", + authHeader: "", + toolAllowlist: server?.toolAllowlist ?? [], + enabled: server?.enabled ?? true, + }; +} + // Tavily preset (§8.10): the API key goes in the Authorization HEADER, not the URL. const TAVILY_PRESET = { name: "Tavily", @@ -72,26 +87,12 @@ export default function AiMcpServerForm({ const form = useForm({ validate: zod4Resolver(formSchema), - initialValues: { - name: server?.name ?? "", - transport: server?.transport ?? "http", - url: server?.url ?? "", - authHeader: "", - toolAllowlist: server?.toolAllowlist ?? [], - enabled: server?.enabled ?? true, - }, + initialValues: buildInitialValues(server), }); // Re-hydrate when the target server changes (e.g. reusing the modal). useEffect(() => { - form.setValues({ - name: server?.name ?? "", - transport: server?.transport ?? "http", - url: server?.url ?? "", - authHeader: "", - toolAllowlist: server?.toolAllowlist ?? [], - enabled: server?.enabled ?? true, - }); + form.setValues(buildInitialValues(server)); form.resetDirty(); setHasHeaders(server?.hasHeaders ?? false); setHeadersCleared(false); diff --git a/apps/server/src/core/ai-chat/ai-chat.service.spec.ts b/apps/server/src/core/ai-chat/ai-chat.service.spec.ts index 20650457..2756df77 100644 --- a/apps/server/src/core/ai-chat/ai-chat.service.spec.ts +++ b/apps/server/src/core/ai-chat/ai-chat.service.spec.ts @@ -1,9 +1,13 @@ import { compactToolOutput, + assistantParts, + serializeSteps, + rowToUiMessage, prepareAgentStep, MAX_AGENT_STEPS, FINAL_STEP_INSTRUCTION, } from './ai-chat.service'; +import type { AiChatMessage } from '@docmost/db/types/entity.types'; /** * Unit tests for compactToolOutput: the pure helper that shrinks LARGE tool @@ -72,6 +76,124 @@ describe('compactToolOutput', () => { }); }); +/** + * Tests for assistantParts: the pure function that rebuilds the persisted + * UIMessage parts for a turn. Its output decides whether the conversation + * replays correctly on the next turn. The crux: a tool-call WITHOUT a paired + * result must become a synthetic `output-error` part, so convertToModelMessages + * never throws MissingToolResultsError. This test MUST fail on pre-fix logic + * that persisted a bare input-available call. + */ +describe('assistantParts', () => { + type AnyPart = Record; + + it('emits output-available for a tool-call WITH a paired result', () => { + const steps = [ + { + text: '', + toolCalls: [{ toolCallId: 'c1', toolName: 'getPage', input: { id: 'p1' } }], + toolResults: [{ toolCallId: 'c1', toolName: 'getPage', output: { title: 'T' } }], + }, + ]; + const parts = assistantParts(steps, '') as AnyPart[]; + const toolPart = parts.find((p) => p.type === 'tool-getPage'); + expect(toolPart).toBeDefined(); + expect(toolPart!.state).toBe('output-available'); + expect(toolPart!.output).toEqual({ title: 'T' }); + }); + + it('emits a synthetic output-error for an UNPAIRED tool-call (crux)', () => { + const steps = [ + { + text: '', + toolCalls: [{ toolCallId: 'c9', toolName: 'insertNode', input: { node: {} } }], + toolResults: [], + }, + ]; + const parts = assistantParts(steps, '') as AnyPart[]; + const toolPart = parts.find((p) => p.type === 'tool-insertNode'); + expect(toolPart).toBeDefined(); + // The unpaired call MUST become output-error (NOT input-available), so the + // rebuilt history is balanced for convertToModelMessages on the next turn. + expect(toolPart!.state).toBe('output-error'); + expect(toolPart!.errorText).toBeTruthy(); + expect(toolPart).not.toHaveProperty('output'); + }); + + it('skips malformed tool-calls (missing toolName or toolCallId)', () => { + const steps = [ + { + text: '', + toolCalls: [ + { toolCallId: 'c1', input: {} }, // no toolName + { toolName: 'getPage', input: {} }, // no toolCallId + ], + toolResults: [], + }, + ]; + const parts = assistantParts(steps, '') as AnyPart[]; + const toolParts = parts.filter( + (p) => typeof p.type === 'string' && (p.type as string).startsWith('tool-'), + ); + expect(toolParts).toHaveLength(0); + }); + + it('uses per-step text when present', () => { + const steps = [{ text: 'hello', toolCalls: [], toolResults: [] }]; + const parts = assistantParts(steps, 'fallback-ignored') as AnyPart[]; + expect(parts).toEqual([{ type: 'text', text: 'hello' }]); + }); + + it('falls back to a single text part when no step text', () => { + const parts = assistantParts([], 'final answer') as AnyPart[]; + expect(parts).toEqual([{ type: 'text', text: 'final answer' }]); + }); +}); + +describe('serializeSteps', () => { + it('returns null when there are no calls or results', () => { + expect(serializeSteps([])).toBeNull(); + }); + + it('flattens calls and results into a compact trace', () => { + const trace = serializeSteps([ + { + toolCalls: [{ toolName: 'getPage', input: { id: 'p1' } }], + toolResults: [{ toolName: 'getPage', output: { title: 'T' } }], + }, + ]) as Array>; + expect(trace).toHaveLength(2); + expect(trace[0]).toEqual({ toolName: 'getPage', input: { id: 'p1' } }); + expect(trace[1]).toEqual({ toolName: 'getPage', output: { title: 'T' } }); + }); +}); + +describe('rowToUiMessage', () => { + it('prefers metadata.parts over content', () => { + const row = { + id: 'm1', + role: 'assistant', + content: 'plain text', + metadata: { parts: [{ type: 'text', text: 'rich part' }] }, + } as unknown as AiChatMessage; + const ui = rowToUiMessage(row); + expect(ui.role).toBe('assistant'); + expect(ui.parts).toEqual([{ type: 'text', text: 'rich part' }]); + }); + + it('falls back to a single text part from content when no metadata.parts', () => { + const row = { + id: 'm2', + role: 'user', + content: 'hi there', + metadata: null, + } as unknown as AiChatMessage; + const ui = rowToUiMessage(row); + expect(ui.role).toBe('user'); + expect(ui.parts).toEqual([{ type: 'text', text: 'hi there' }]); + }); +}); + /** * Unit tests for prepareAgentStep: the pure helper that decides per-step * overrides for the agent loop. Early steps return undefined (default diff --git a/apps/server/src/core/ai-chat/ai-chat.service.ts b/apps/server/src/core/ai-chat/ai-chat.service.ts index cd62b5f6..bb33a5ef 100644 --- a/apps/server/src/core/ai-chat/ai-chat.service.ts +++ b/apps/server/src/core/ai-chat/ai-chat.service.ts @@ -10,6 +10,7 @@ import { } from 'ai'; import { AiService } from '../../integrations/ai/ai.service'; import { AiSettingsService } from '../../integrations/ai/ai-settings.service'; +import { describeProviderError } from '../../integrations/ai/ai-error.util'; import { AiChatRepo } from '@docmost/db/repos/ai-chat/ai-chat.repo'; import { AiChatMessageRepo } from '@docmost/db/repos/ai-chat/ai-chat-message.repo'; import { User, Workspace, AiChatMessage } from '@docmost/db/types/entity.types'; @@ -313,15 +314,10 @@ export class AiChatService { onError: async ({ error }) => { // NestJS Logger.error(message, stack?, context?): pass the real message // (with statusCode when present) + the stack string, not the Error - // object, so the actual provider cause is clearly logged. - const e = error as { - statusCode?: number; - message?: string; - stack?: string; - }; - const errorText = e?.statusCode - ? `${e.statusCode}: ${e.message ?? String(error)}` - : (e?.message ?? String(error)); + // object, so the actual provider cause is clearly logged. Reuse the + // shared formatter so provider error formatting stays unified. + const e = error as { stack?: string }; + const errorText = describeProviderError(error, String(error)); this.logger.error(`AI chat stream error: ${errorText}`, e?.stack); // Persist whatever text we have (likely empty) so the turn is recorded, // and record the error text in metadata so it is visible in history. @@ -382,10 +378,9 @@ export class AiChatService { result.pipeUIMessageStreamToResponse(res.raw, { headers: { 'X-Accel-Buffering': 'no' }, onError: (error: unknown) => { - const e = error as { statusCode?: number; message?: string }; - return e?.statusCode - ? `${e.statusCode}: ${e.message}` - : (e?.message ?? 'AI stream error'); + // Reuse the shared formatter so provider error formatting stays + // unified between the log line and the streamed error message. + return describeProviderError(error, 'AI stream error'); }, }); @@ -580,7 +575,9 @@ function compactValue(value: unknown, depth: number): unknown { * recovers the name. Falls back to a single `text` part built from * `fallbackText` when the steps carry no text. */ -function assistantParts( +// Exported only so the unit tests can import these pure helpers; exporting +// them does not change runtime behavior. +export function assistantParts( steps: ReadonlyArray | undefined, fallbackText: string, ): UIMessage['parts'] { @@ -638,7 +635,7 @@ function assistantParts( * stored parts when available; assistant messages restore the reconstructable * parts from metadata, falling back to a single text part from `content`. */ -function rowToUiMessage(row: AiChatMessage): Omit & { +export function rowToUiMessage(row: AiChatMessage): Omit & { id: string; } { const role = row.role === 'assistant' ? 'assistant' : 'user'; @@ -655,7 +652,7 @@ function rowToUiMessage(row: AiChatMessage): Omit & { * `tool_calls` column. Stores only what the UI action-log and history need — * never raw provider payloads or keys. */ -function serializeSteps( +export function serializeSteps( steps: ReadonlyArray<{ toolCalls?: ReadonlyArray<{ toolName?: string; input?: unknown }>; toolResults?: ReadonlyArray<{ toolName?: string; output?: unknown }>; diff --git a/apps/server/src/core/ai-chat/external-mcp/ssrf-guard.spec.ts b/apps/server/src/core/ai-chat/external-mcp/ssrf-guard.spec.ts new file mode 100644 index 00000000..b4f3e32e --- /dev/null +++ b/apps/server/src/core/ai-chat/external-mcp/ssrf-guard.spec.ts @@ -0,0 +1,133 @@ +/** + * Unit tests for the SSRF guard protecting admin-configured external MCP URLs. + * + * `isIpAllowed` is pure/sync: every blocked address class must be rejected and a + * public address allowed. `isUrlAllowed` adds scheme/URL validation and, for + * hostnames, a DNS resolve + re-check (the DNS-rebinding defense): a name that + * resolves to a private address must be blocked. We mock `node:dns` `lookup` + * (the guard promisifies it) so the rebinding case is deterministic and offline. + */ + +// Mock node:dns BEFORE importing the guard so promisify(lookup) wraps our mock. +const lookupMock = jest.fn(); +jest.mock('node:dns', () => ({ + __esModule: true, + lookup: (...args: unknown[]) => lookupMock(...args), +})); + +import { isIpAllowed, isUrlAllowed } from './ssrf-guard'; + +// The guard calls promisify(lookup): our mock must honour the (host, opts, cb) +// callback signature. Helper to make it resolve to a given address list. +function dnsResolvesTo(addresses: { address: string }[]) { + lookupMock.mockImplementation( + (_host: string, _opts: unknown, cb: (e: unknown, a: unknown) => void) => { + cb(null, addresses); + }, + ); +} + +describe('isIpAllowed', () => { + const blocked: Array<[string, string]> = [ + ['loopback IPv4', '127.0.0.1'], + ['loopback IPv6', '::1'], + ['link-local / metadata', '169.254.169.254'], + ['private 10/8', '10.0.0.1'], + ['private 172.16/12', '172.16.5.4'], + ['private 192.168/16', '192.168.1.1'], + ['CGNAT 100.64/10', '100.64.1.1'], + ['ULA fc00::/7', 'fc00::1'], + ['unspecified IPv4', '0.0.0.0'], + ['unspecified IPv6', '::'], + ['IPv4-mapped IPv6 (private)', '::ffff:10.0.0.1'], + ]; + + it.each(blocked)('blocks %s (%s)', (_label, ip) => { + expect(isIpAllowed(ip).ok).toBe(false); + }); + + // IP-level bypass vectors ported from the safety-coverage branch. CGNAT + // (100.64/10) and the ULA range (fc00::/7) are already exercised above with + // other sample addresses; the genuinely distinct case is the IPv4-mapped + // IPv6 *loopback* (::ffff:127.0.0.1) — the table above only had the mapped + // *private* variant. fd00::/8 is the commonly-assigned ULA prefix, kept as an + // explicit regression guard. + it.each([ + ['CGNAT', '100.64.0.1'], + ['ULA fd00::/8', 'fd00::1'], + ['IPv4-mapped IPv6 loopback', '::ffff:127.0.0.1'], + ])('blocks bypass vector %s (%s)', (_label, ip) => { + expect(isIpAllowed(ip).ok).toBe(false); + }); + + it('allows a public IPv4 (8.8.8.8)', () => { + expect(isIpAllowed('8.8.8.8').ok).toBe(true); + }); + + it('allows a public IPv6', () => { + expect(isIpAllowed('2001:4860:4860::8888').ok).toBe(true); + }); + + it('blocks an unparseable IP', () => { + expect(isIpAllowed('not-an-ip').ok).toBe(false); + }); +}); + +describe('isUrlAllowed', () => { + beforeEach(() => { + lookupMock.mockReset(); + }); + + it('blocks a non-http(s) scheme', async () => { + const res = await isUrlAllowed('ftp://example.com/'); + expect(res.ok).toBe(false); + expect(lookupMock).not.toHaveBeenCalled(); + }); + + it('blocks an invalid URL', async () => { + const res = await isUrlAllowed('::: not a url :::'); + expect(res.ok).toBe(false); + expect(lookupMock).not.toHaveBeenCalled(); + }); + + it('blocks a private IP literal host without DNS', async () => { + const res = await isUrlAllowed('http://169.254.169.254/latest/meta-data/'); + expect(res.ok).toBe(false); + expect(lookupMock).not.toHaveBeenCalled(); + }); + + it('blocks a bracketed private IPv6 literal host', async () => { + const res = await isUrlAllowed('http://[::1]:8080/'); + expect(res.ok).toBe(false); + expect(lookupMock).not.toHaveBeenCalled(); + }); + + it('blocks a hostname that resolves to a private address (DNS rebinding)', async () => { + dnsResolvesTo([{ address: '10.0.0.5' }]); + const res = await isUrlAllowed('http://rebind.example.com/'); + expect(res.ok).toBe(false); + expect(lookupMock).toHaveBeenCalled(); + }); + + it('blocks when ANY resolved address is private (mixed result)', async () => { + dnsResolvesTo([{ address: '8.8.8.8' }, { address: '127.0.0.1' }]); + const res = await isUrlAllowed('http://mixed.example.com/'); + expect(res.ok).toBe(false); + }); + + it('allows a hostname that resolves only to a public address', async () => { + dnsResolvesTo([{ address: '8.8.8.8' }]); + const res = await isUrlAllowed('https://public.example.com/mcp'); + expect(res.ok).toBe(true); + }); + + it('blocks when the host does not resolve', async () => { + lookupMock.mockImplementation( + (_host: string, _opts: unknown, cb: (e: unknown, a: unknown) => void) => { + cb(new Error('ENOTFOUND'), undefined); + }, + ); + const res = await isUrlAllowed('http://nonexistent.invalid/'); + expect(res.ok).toBe(false); + }); +}); diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts index 65218300..becf082f 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts @@ -211,3 +211,174 @@ describe('AiChatToolsService expanded toolset guardrails', () => { expect(parsed).not.toHaveProperty('deleteComments'); }); }); + +/** + * JSON-string coercion for node arguments (fix 59b99dba): under OpenAI tool + * calls the model sometimes serializes `node`/`content` as a JSON STRING. The + * tools parse a string into an object before forwarding it to the client (which + * type-checks for an object), throw a documented message on invalid JSON, and + * `updatePageJson` distinguishes undefined (title-only) from object/string. + */ +describe('AiChatToolsService node-arg JSON-string coercion', () => { + // Records the positional args forwarded to each write method so we can assert + // the coerced (parsed) value reaches the client. + const patchNodeCalls: unknown[][] = []; + const insertNodeCalls: unknown[][] = []; + const updatePageJsonCalls: unknown[][] = []; + + const fakeClient: Partial = { + patchNode: (...args: unknown[]) => { + patchNodeCalls.push(args); + return Promise.resolve({ ok: true }); + }, + insertNode: (...args: unknown[]) => { + insertNodeCalls.push(args); + return Promise.resolve({ ok: true }); + }, + updatePageJson: (...args: unknown[]) => { + updatePageJsonCalls.push(args); + return Promise.resolve({ ok: true }); + }, + }; + + const tokenServiceStub = { + generateAccessToken: jest.fn().mockResolvedValue('access-token'), + generateCollabToken: jest.fn().mockResolvedValue('collab-token'), + }; + + let service: AiChatToolsService; + + beforeEach(() => { + patchNodeCalls.length = 0; + insertNodeCalls.length = 0; + updatePageJsonCalls.length = 0; + jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({ + DocmostClient: function () { + return fakeClient as DocmostClientLike; + } as unknown as loader.DocmostClientCtor, + }); + service = new AiChatToolsService( + tokenServiceStub as never, + {} as never, + {} as never, + {} as never, + {} as never, + ); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + function buildTools() { + return service.forUser( + { id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never, + 'session-1', + 'ws-1', + 'chat-1', + ); + } + + const NODE_OBJ = { + type: 'paragraph', + content: [{ type: 'text', text: 'Hello' }], + }; + + it('patchNode parses a JSON-string node and forwards it as an object', async () => { + const tools = await buildTools(); + await tools.patchNode.execute( + { pageId: 'p1', nodeId: 'n1', node: JSON.stringify(NODE_OBJ) } as never, + {} as never, + ); + expect(patchNodeCalls).toHaveLength(1); + expect(patchNodeCalls[0]).toEqual(['p1', 'n1', NODE_OBJ]); + }); + + it('patchNode passes an object node through unchanged', async () => { + const tools = await buildTools(); + await tools.patchNode.execute( + { pageId: 'p1', nodeId: 'n1', node: NODE_OBJ } as never, + {} as never, + ); + expect(patchNodeCalls[0]).toEqual(['p1', 'n1', NODE_OBJ]); + }); + + it('patchNode throws the documented message on invalid JSON string', async () => { + const tools = await buildTools(); + await expect( + tools.patchNode.execute( + { pageId: 'p1', nodeId: 'n1', node: '{not json' } as never, + {} as never, + ), + ).rejects.toThrow('node was a string but not valid JSON'); + expect(patchNodeCalls).toHaveLength(0); + }); + + it('insertNode parses a JSON-string node and forwards it as an object', async () => { + const tools = await buildTools(); + await tools.insertNode.execute( + { + pageId: 'p1', + node: JSON.stringify(NODE_OBJ), + position: 'append', + } as never, + {} as never, + ); + expect(insertNodeCalls).toHaveLength(1); + const [pageId, node] = insertNodeCalls[0]; + expect(pageId).toBe('p1'); + expect(node).toEqual(NODE_OBJ); + }); + + it('insertNode throws the documented message on invalid JSON string', async () => { + const tools = await buildTools(); + await expect( + tools.insertNode.execute( + { pageId: 'p1', node: 'nope', position: 'append' } as never, + {} as never, + ), + ).rejects.toThrow('node was a string but not valid JSON'); + expect(insertNodeCalls).toHaveLength(0); + }); + + it('updatePageJson forwards doc=undefined for a title-only update (content undefined)', async () => { + const tools = await buildTools(); + await tools.updatePageJson.execute( + { pageId: 'p1', title: 'New title' } as never, + {} as never, + ); + expect(updatePageJsonCalls).toHaveLength(1); + expect(updatePageJsonCalls[0]).toEqual(['p1', undefined, 'New title']); + }); + + it('updatePageJson passes an object content through unchanged', async () => { + const tools = await buildTools(); + const doc = { type: 'doc', content: [] }; + await tools.updatePageJson.execute( + { pageId: 'p1', content: doc } as never, + {} as never, + ); + expect(updatePageJsonCalls[0]).toEqual(['p1', doc, undefined]); + }); + + it('updatePageJson parses a JSON-string content', async () => { + const tools = await buildTools(); + const doc = { type: 'doc', content: [] }; + await tools.updatePageJson.execute( + { pageId: 'p1', content: JSON.stringify(doc) } as never, + {} as never, + ); + expect(updatePageJsonCalls[0]).toEqual(['p1', doc, undefined]); + }); + + it('updatePageJson throws the documented message on invalid JSON string content', async () => { + const tools = await buildTools(); + await expect( + tools.updatePageJson.execute( + { pageId: 'p1', content: '{bad' } as never, + {} as never, + ), + ).rejects.toThrow('content was a string but not valid JSON'); + expect(updatePageJsonCalls).toHaveLength(0); + }); +}); diff --git a/apps/server/src/database/repos/ai-chat/page-embedding.repo.spec.ts b/apps/server/src/database/repos/ai-chat/page-embedding.repo.spec.ts new file mode 100644 index 00000000..792c8762 --- /dev/null +++ b/apps/server/src/database/repos/ai-chat/page-embedding.repo.spec.ts @@ -0,0 +1,26 @@ +import { PageEmbeddingRepo } from './page-embedding.repo'; +import type { KyselyDB } from '../../types/kysely.types'; + +/** + * Unit test for the pure access-scoping branch of searchByEmbedding: when the + * caller has NO accessible spaces (`spaceIds` empty), the method must early- + * return [] WITHOUT touching the database. We inject a db whose query builder + * throws if invoked, so any DB access fails the test. + * + * NOTE: the dimension-mixing case (filter by model_dimensions) needs a live + * pgvector-enabled Postgres and is intentionally NOT covered here — it requires + * a real DB and is out of scope for this pure unit test. + */ +describe('PageEmbeddingRepo.searchByEmbedding', () => { + it('early-returns [] for empty spaceIds without any DB call', async () => { + const throwingDb = { + selectFrom: () => { + throw new Error('DB should not be queried for empty spaceIds'); + }, + } as unknown as KyselyDB; + + const repo = new PageEmbeddingRepo(throwingDb); + const result = await repo.searchByEmbedding('ws-1', [0.1, 0.2, 0.3], [], 10); + expect(result).toEqual([]); + }); +}); diff --git a/apps/server/src/integrations/ai/ai-error.util.spec.ts b/apps/server/src/integrations/ai/ai-error.util.spec.ts new file mode 100644 index 00000000..c9b7fb3e --- /dev/null +++ b/apps/server/src/integrations/ai/ai-error.util.spec.ts @@ -0,0 +1,61 @@ +import { describeProviderError } from './ai-error.util'; + +/** + * Unit tests for describeProviderError: the shared formatter used both for the + * server log line and for the error text streamed back to the client. This + * pins the behaviour, including the one behaviour change introduced when the + * two inline formatters were unified: a truncated, single-line snippet of the + * provider `responseBody`/`text` is appended (so a misconfigured endpoint's + * HTML error page is diagnosable). The util guarantees the API key is never in + * the response body, so this is safe to surface. + */ +describe('describeProviderError', () => { + it('uses the fallback for a null/empty/undefined error', () => { + expect(describeProviderError(null, 'AI stream error')).toBe( + 'AI stream error', + ); + expect(describeProviderError('', 'AI stream error')).toBe('AI stream error'); + expect(describeProviderError(undefined)).toBe('Unknown error'); + }); + + it('returns a non-empty plain string error as-is', () => { + expect(describeProviderError('boom')).toBe('boom'); + }); + + it('formats statusCode + message', () => { + expect( + describeProviderError({ statusCode: 401, message: 'Unauthorized' }), + ).toBe('401: Unauthorized'); + }); + + it('falls back to message when there is no statusCode', () => { + expect(describeProviderError({ message: 'nope' })).toBe('nope'); + }); + + it('appends a whitespace-collapsed response body snippet', () => { + const out = describeProviderError({ + statusCode: 502, + message: 'Bad Gateway', + responseBody: '\n upstream error\n', + }); + expect(out.startsWith('502: Bad Gateway | response body: ')).toBe(true); + // Newlines and runs of spaces are collapsed to single spaces. + expect(out).toContain(' upstream error '); + }); + + it('reads `text` when responseBody is absent', () => { + expect(describeProviderError({ message: 'e', text: 'body-text' })).toBe( + 'e | response body: body-text', + ); + }); + + it('truncates a long body to 300 chars + ellipsis', () => { + const out = describeProviderError({ + message: 'e', + responseBody: 'x'.repeat(500), + }); + expect(out).toContain('…'); + // 'e | response body: ' + 300 chars + '…' + expect(out.length).toBeLessThan('e | response body: '.length + 305); + }); +}); diff --git a/apps/server/src/integrations/ai/ai-error.util.ts b/apps/server/src/integrations/ai/ai-error.util.ts index 68fa328b..0a0f949b 100644 --- a/apps/server/src/integrations/ai/ai-error.util.ts +++ b/apps/server/src/integrations/ai/ai-error.util.ts @@ -9,10 +9,16 @@ * * None of these fields contain the API key (it is sent as an Authorization * header and never echoed in the response body), so this is safe to log/return. + * + * `fallback` is used when the error carries no usable message (e.g. a bare + * object); defaults to 'Unknown error'. */ -export function describeProviderError(err: unknown): string { +export function describeProviderError( + err: unknown, + fallback = 'Unknown error', +): string { if (typeof err !== 'object' || err === null) { - return typeof err === 'string' ? err : 'Unknown error'; + return typeof err === 'string' && err ? err : fallback; } const e = err as { statusCode?: number; @@ -23,7 +29,7 @@ export function describeProviderError(err: unknown): string { const base = typeof e.statusCode === 'number' ? `${e.statusCode}: ${e.message ?? ''}`.trim() - : (e.message ?? 'Unknown error'); + : (e.message ?? fallback); const body = (e.responseBody ?? e.text ?? '').trim(); if (!body) return base; // Collapse whitespace so a multi-line HTML body stays on one log line. diff --git a/apps/server/src/integrations/crypto/secret-box.spec.ts b/apps/server/src/integrations/crypto/secret-box.spec.ts new file mode 100644 index 00000000..d53d7093 --- /dev/null +++ b/apps/server/src/integrations/crypto/secret-box.spec.ts @@ -0,0 +1,77 @@ +import { SecretBoxService } from './secret-box'; +import { EnvironmentService } from '../environment/environment.service'; + +/** + * Unit tests for SecretBoxService: the AES-256-GCM helper that protects provider + * API keys at rest. The contract is: encrypt -> decrypt round-trips the input; + * two encryptions of the same input yield different blobs (random salt+iv) yet + * both decrypt; a tampered blob or a different APP_SECRET fails decryption with + * the recoverable "APP_SECRET may have changed" message the UI relies on. + */ +describe('SecretBoxService', () => { + // Construct a SecretBoxService whose EnvironmentService.getAppSecret returns a + // fixed 64-hex secret. Only getAppSecret is exercised, so a thin fake suffices. + function makeBox(appSecret: string): SecretBoxService { + const env = { + getAppSecret: () => appSecret, + } as unknown as EnvironmentService; + return new SecretBoxService(env); + } + + const SECRET_A = + '00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff'; + const SECRET_B = + 'ffeeddccbbaa99887766554433221100ffeeddccbbaa99887766554433221100'; + + it('round-trips: decrypt(encrypt(x)) === x', () => { + const box = makeBox(SECRET_A); + const plain = 'sk-super-secret-provider-key-12345'; + const blob = box.encryptSecret(plain); + expect(box.decryptSecret(blob)).toBe(plain); + }); + + it('produces a different blob each time, both of which decrypt', () => { + const box = makeBox(SECRET_A); + const plain = 'identical-input'; + const blob1 = box.encryptSecret(plain); + const blob2 = box.encryptSecret(plain); + // Random per-record salt + iv => the ciphertext blobs must differ. + expect(blob1).not.toBe(blob2); + expect(box.decryptSecret(blob1)).toBe(plain); + expect(box.decryptSecret(blob2)).toBe(plain); + }); + + it('throws the recoverable error on a tampered auth tag', () => { + const box = makeBox(SECRET_A); + const blob = box.encryptSecret('tamper-me'); + + // Layout: base64( salt[16] | iv[12] | authTag[16] | ciphertext ). Flip a bit + // in the auth-tag region so GCM verification (decipher.final) rejects it. + const data = Buffer.from(blob, 'base64'); + const authTagByteIndex = 16 + 12; // first byte of the auth tag + data[authTagByteIndex] = data[authTagByteIndex] ^ 0xff; + const tampered = data.toString('base64'); + + expect(() => box.decryptSecret(tampered)).toThrow(/APP_SECRET may have changed/); + }); + + it('throws the recoverable error on a tampered ciphertext byte', () => { + const box = makeBox(SECRET_A); + const blob = box.encryptSecret('tamper-the-body'); + + const data = Buffer.from(blob, 'base64'); + // Last byte is part of the ciphertext; flipping it must fail GCM auth. + data[data.length - 1] = data[data.length - 1] ^ 0xff; + const tampered = data.toString('base64'); + + expect(() => box.decryptSecret(tampered)).toThrow(/APP_SECRET may have changed/); + }); + + it('throws when decrypting under a different APP_SECRET', () => { + const boxA = makeBox(SECRET_A); + const boxB = makeBox(SECRET_B); + const blob = boxA.encryptSecret('rotate-me'); + // A different APP_SECRET derives a different scrypt key => GCM auth fails. + expect(() => boxB.decryptSecret(blob)).toThrow(/APP_SECRET may have changed/); + }); +}); diff --git a/docs/backlog/ai-chat-review-followups.md b/docs/backlog/ai-chat-review-followups.md deleted file mode 100644 index 3a31d82c..00000000 --- a/docs/backlog/ai-chat-review-followups.md +++ /dev/null @@ -1,202 +0,0 @@ -# Follow-ups код-ревью фичи ai-chat - -Контекст: мульти-аспектное ревью встроенного AI-агента (диапазон коммитов -`6e5d0300..4868ca8e`, вся фича ai-chat) прошло чисто по безопасности, -регрессиям и конвенциям. Ниже — находки, которые НЕ блокируют merge, но -должны быть закрыты: пробелы в тестах на критичном по безопасности коде, -доступность с клавиатуры, устаревшая документация и мелкие рефакторинги. -Сгруппировано по приоритету. Каждая запись: что → где (`file:line`) → почему → -фикс. - -Сознательно НЕ входят в этот файл (вынесены отдельно): warning про неусечённый -реплей tool-выводов в `ai-chat.service.ts` и архитектурное предложение про -дублирование набора инструментов между in-app агентом и `packages/mcp`. - ---- - -## Приоритет 1 — тесты на критичном по безопасности коде (warning) - -### 1.1 Шифрование ключей провайдеров (AES-256-GCM) — ноль тестов - -- **Где:** `apps/server/src/integrations/crypto/secret-box.ts` - — `encryptSecret` (`:36-48`), `decryptSecret` (`:51-81`), сообщение об ошибке - (`:78`). Spec-файла нет (подтверждено grep'ом по `*.spec.ts`). -- **Почему:** это единственная защита API-ключей провайдеров в покое. Не - проверено: round-trip `encrypt → decrypt` возвращает исходный текст; два - шифрования одного текста дают разные блобы (random salt+iv, layout - `base64(salt | iv | authTag | ciphertext)`); ветка `catch` бросает ожидаемую - ошибку «APP_SECRET may have changed» на испорченном/обрезанном блобе или - неверном ключе (на это сообщение опирается UI). Ошибка в смещениях layout или - регресс auth-tag молча испортит все сохранённые креды. -- **Фикс:** `secret-box.spec.ts`, 4 кейса — (1) round-trip equality; (2) два - encrypt одного входа → разные блобы, оба декриптятся; (3) decrypt - подделанного ciphertext / флипнутого байта auth-tag → throw с нужным - сообщением; (4) decrypt под другим `APP_SECRET` → throw. `EnvironmentService` - тривиально стабается (`getAppSecret`). - -### 1.2 SSRF-guard — ветки allow/deny полностью не покрыты - -- **Где:** `apps/server/src/core/ai-chat/external-mcp/ssrf-guard.ts` - — `isIpAllowed` (`:40`), `isUrlAllowed` (`:60-104`); `isIpAllowed` - вызывается для IP-литерала (`:80`) и для каждого DNS-резолва (`:97`). -- **Почему:** единственная защита от SSRF для admin-задаваемых URL внешних - MCP-серверов; тестов нет. Каждая непокрытая ветка = реальный эксплойт: - loopback (127.0.0.1, ::1), link-local/metadata (169.254.169.254), private - (10/172.16/192.168), CGNAT (100.64/10), ULA (fc00::/7), unspecified, - IPv4-mapped IPv6, не-http(s) схема, невалидный URL, DNS-rebinding (любой - резолвнутый адрес приватный ⇒ block). `isIpAllowed` — чистая синхронная - функция. -- **Фикс:** `ssrf-guard.spec.ts` — `isIpAllowed` по каждому блокируемому классу - + публичный IP (allow); `isUrlAllowed` — bad-scheme, invalid-url, - IP-литерал-private и (с моком `dns.lookup`) кейс rebinding, где - резолвнутый адрес приватный. - -### 1.3 `assistantParts()` — логика «сохранить ошибки/tool-calls в истории» без тестов - -- **Где:** `apps/server/src/core/ai-chat/ai-chat.service.ts` - — `assistantParts` (`:430-495`), родственные `serializeSteps` (`:610`), - `rowToUiMessage`. Spec'а у сервиса нет. -- **Почему:** чистая функция, чей вывод определяет, переиграется ли диалог. - Ключевая ветка (`:472-486`) эмитит синтетический `output-error` для tool-call - без пары — чтобы `convertToModelMessages` не бросил `MissingToolResultsError` - на следующем ходу. Это суть фиксов видимости ошибок (`dbd83b5a`/`4868ca8e`). - Регресс, убравший пару, молча вернёт краш. Не покрыты также ветки: step с - текстом vs без (`:451-453`, `:489-492`), call с результатом - (`output-available`, `:463-471`) vs без, skip битого call - (`!toolName || !toolCallId`, `:461`). -- **Фикс:** экспортировать чистые хелперы (или тонкая обёртка) и в spec - проверить: парный вызов → `output-available`; непарный → `output-error`; skip - битых; fallback на единственный `text` при отсутствии step-текста. - `rowToUiMessage` предпочитает `metadata.parts` над `content`. Тест на ветку - непарного вызова обязан падать на pre-fix коде. - -### 1.4 (suggestion) Ветки парсинга JSON-строковых node-аргументов не покрыты - -- **Где:** `apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts` - — `patchNode` (`:686-693`), `insertNode` (`:745-752`), `updatePageJson` - (`:800-809`); сообщения об ошибке `:690`, `:749`, `:804`. Существующий - `ai-chat-tools.service.spec.ts` покрывает только guardrail `deletePage` + - наличие инструментов. -- **Почему:** фикс `59b99dba` добавил coercion string→object (то, что чинило - `insert_node` под OpenAI-tool-calls). Невалидная JSON-строка бросает «node was - a string but not valid JSON» / «content was a string…»; `updatePageJson` - различает undefined/null (title-only) vs object vs string-parse. Регресс, - убравший parse, молча вернёт падение `insert_node` под OpenAI. -- **Фикс:** в существующий spec (он уже стабает фейковый клиент) добавить: - JSON-строковый `node` парсится и форвардится как объект; невалидная строка → - throw с нужным сообщением; `updatePageJson` с `content === undefined` - форвардит `doc === undefined` (title-only), объект проходит как есть. - -### 1.5 (suggestion) Фильтр размерности / пустые spaces в поиске эмбеддингов не покрыты - -- **Где:** `apps/server/src/database/repos/ai-chat/page-embedding.repo.ts` - — `searchByEmbedding` (`:143`), early-return на пустом `spaceIds` (`:149`), - фильтр `model_dimensions = queryEmbedding.length` (`:154` + where в запросе). -- **Почему:** early-return на пустых spaceIds — путь access-scoping с нулевым - результатом; фильтр размерности существует, чтобы избежать pgvector - dimension-mismatch, когда остались строки от ранее настроенной модели - эмбеддингов. Регресс, убравший фильтр, вернёт runtime-краш pgvector. -- **Фикс:** минимум — assert, что `searchByEmbedding(ws, vec, [], n)` → `[]` без - обращения к БД (ветка чистая). При наличии тест-БД — кейс со смешанными - размерностями: скорятся только строки той же размерности. - ---- - -## Приоритет 2 — доступность и документация (suggestion) - -### 2.1 Два новых кликабельных `div` без клавиатурной доступности (a11y) - -- **Где:** `apps/client/src/features/ai-chat/components/ai-chat-window.tsx:342-354` - (заголовок «Chat history») и - `apps/client/src/features/ai-chat/components/conversation-list.tsx:107-119` - (строка диалога, `onClick` на `:118`). -- **Почему:** несемантические элементы с `onClick`, но без - `role`/`tabIndex`/`onKeyDown` — с клавиатуры/скринридером историю не - развернуть и прошлый чат не открыть. Это ниже планки самого проекта: - `apps/client/src/features/comment/components/comment-list-item.tsx` использует - `role="button"`, и бейдж AI-агента, добавленный в этом же изменении - (`apps/client/src/features/page-history/components/history-item.tsx:77-79`), - корректно ставит `role="button"` + `tabIndex={0}` + обработку Enter/Space. -- **Фикс:** применить тот же паттерн к обоим элементам (или Mantine - `UnstyledButton`). - -### 2.2 Устаревший doc-комментарий перечисляет 9 инструментов из текущих ~40 - -- **Где:** `apps/client/src/features/ai-chat/utils/tool-parts.tsx:1-10` - (список инструментов на `:8-10`). -- **Почему:** комментарий описывает старый набор; после «expose full Docmost - toolset» и `drop updateComment` вводит в заблуждение. Не баг — дружелюбные - подписи `toolLabelKey` всё равно только у перечисленных, остальные идут в - generic-ветку «Ran tool {{name}}». -- **Фикс:** заменить жёсткий список на «см. `ai-chat-tools.service.ts`» (или - пометить, что дружелюбные подписи только у инструментов из `toolLabelKey`). - -### 2.3 Реализация `secret-box` противоречит схеме крипто в плане - -- **Где:** `apps/server/src/integrations/crypto/secret-box.ts:11-48` vs - `docs/ai-agent-chat-plan.md` §5.3 / §6.3. -- **Почему:** код использует per-record случайную соль - (`scryptSync(APP_SECRET, salt, 32)`) и layout - `base64(salt | iv | authTag | ciphertext)`; план описывает фиксированную - строковую соль `'ai-provider'` и layout без сегмента соли. Реализация лучше, - но план теперь описывает не те байты на диске — введёт в заблуждение при - написании ротации/отладке decrypt. План помечен «иллюстративным», поэтому - suggestion. -- **Фикс:** обновить §5.3 / §6.3 под фактический layout. - ---- - -## Приоритет 3 — стабильность и рефакторинг (suggestion) - -### 3.1 Новый чат, упавший на первом ходу, не «усыновляет» созданный сервером chat id - -- **Где:** `apps/client/src/features/ai-chat/components/chat-thread.tsx:129-137` - (`useChat` с `onFinish` на `:136`, без `onError`). Целевой колбэк — - `onTurnFinished` в - `apps/client/src/features/ai-chat/components/ai-chat-window.tsx:154-157` - (инвалидирует `AI_CHATS_RQ_KEY`). -- **Почему:** в AI SDK v6 `onFinish` не срабатывает при ошибке стрима, поэтому - `onTurnFinished()` не вызывается. Сервер же уже создал строку чата и сохранил - error-сообщение — но клиент не инвалидирует список чатов и не подхватывает - новый id: ошибочный чат не появляется в истории до постороннего refresh. - Alert с ошибкой показывается, так что это UX-несогласованность, не потеря - данных. -- **Фикс:** передать в `useChat` `onError`, который тоже вызывает - `onTurnFinished()` (или инвалидирует `AI_CHATS_RQ_KEY` + подхватывает новый - id). - -### 3.2 Дублированный хелпер `isToolPart` в двух компонентах - -- **Где:** `apps/client/src/features/ai-chat/components/message-item.tsx:16` и - `apps/client/src/features/ai-chat/components/message-list.tsx:15` — - идентичное `type.startsWith("tool-") || type === "dynamic-tool"`. Оба уже - импортируют из `utils/tool-parts.tsx`. -- **Почему:** копии молча разойдутся, если AI SDK добавит ещё один - tool-part-дискриминатор. -- **Фикс:** экспортировать `isToolPart` один раз из `tool-parts.tsx` (рядом с - `getToolName`), импортировать в оба компонента, локальные определения удалить. - -### 3.3 Объект `initialValues` формы продублирован дословно - -- **Где:** - `apps/client/src/features/workspace/components/settings/components/ai-mcp-server-form.tsx` - — `useForm({ initialValues: {...} })` (`:75-82`) и эффект re-hydration - `form.setValues({...})` (`:87-95`): один и тот же 6-полевой объект из - `server`. -- **Почему:** должны меняться синхронно; добавить поле в одно и забыть второе — - лёгкий баг. (В соседнем `ai-provider-settings.tsx` этой проблемы нет — там - initialValues константны, а эффект мапит из `settings`.) -- **Фикс:** вынести `buildInitialValues(server)` и звать в обоих местах. - -### 3.4 Идиома форматирования ошибки провайдера дублирует существующий хелпер - -- **Где:** `apps/server/src/core/ai-chat/ai-chat.service.ts:274-275` и `:338-339` - — инлайн `e?.statusCode ? \`${e.statusCode}: ${e.message}\` : e.message`. -- **Почему:** в `apps/server/src/integrations/ai/ai-error.util.ts` уже есть - общий `describeProviderError(err)` (импортируется в - `apps/server/src/integrations/ai/ai.service.ts:14`, используется на `:193`, - `:210`). Два места в `ai-chat.service.ts` переизобретают его инлайном — формат - может разойтись. -- **Фикс:** заменить оба инлайн-места на `describeProviderError(err)` (при - необходимости расширив хелпер fallback-аргументом), чтобы формат ошибок - провайдера был единым.