diff --git a/DECISIONS.md b/DECISIONS.md index 48f95c5f..5700c82c 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -582,3 +582,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-27] zizmor gates only on high-severity high-confidence findings while reporting everything to the Security tab: the first run surfaced 56 findings, and a scanner that blocks every merge on day one gets disabled rather than triaged. [2026-07-27] dangerous-triggers suppressed for swedish-compliance-review.yml and docker-image-scan.yml in .github/zizmor.yml rather than left unresolved: both are workflow_run, but the first is the base-repo-only pattern GitHub recommends INSTEAD of pull_request_target and the second checks out no source at all, so leaving two permanent unexplained errors in the Security tab would just train reviewers to ignore it. [2026-07-27] Docker layer cache tag is now per-architecture (buildcache-amd64 / buildcache-arm64): with native runners each job builds one platform, so a shared tag would leave the two racing to overwrite a cache manifest describing layers the other cannot use. +[2026-07-27] Assistant screen-reader announcement fires on turn boundaries, not on the streaming text: a live region over token deltas re-announces on every delta, so the finished answer is announced once (capped at 400 chars) instead of the stream being narrated. +[2026-07-27] Did NOT self-host the dicebear avatars in the PR7 polish pass despite it being on the plan: the Notionists set is third-party artwork with its own licence terms, and vendoring it into an AGPL-3.0 repo is a licence decision for the founder, not a polish item. diff --git a/components/agent/AgentChat.tsx b/components/agent/AgentChat.tsx index 6a30e065..873018c2 100644 --- a/components/agent/AgentChat.tsx +++ b/components/agent/AgentChat.tsx @@ -196,6 +196,21 @@ export default function AgentChat({ const firstTurnFiredRef = useRef(false) const conversationIdRef = useRef(initialConversationId ?? null) const [messages, setMessages] = useState(initialMessages ?? []) + // Read by the announcement effect, which must not re-run on every token: a + // `messages` dependency would fire it hundreds of times per turn. Written in + // an effect rather than during render: React may replay a render, and a + // render-phase ref write can therefore leave the announcement reading a + // snapshot the user never saw. Declared BEFORE the announcement effect so it + // is already current when that one runs for the same commit. + const messagesRef = useRef(messages) + useEffect(() => { + messagesRef.current = messages + }, [messages]) + // Where the current turn's messages start. Without it the announcement + // searches the whole thread, so a turn that produces no text of its own (a + // tool-only turn, an error) finds the PREVIOUS answer and reads it out as + // though it were the new one. + const turnStartRef = useRef(0) const hasAi = useCapability(CAPABILITY.ai) const [input, setInput] = useState('') const [streaming, setStreaming] = useState(false) @@ -204,13 +219,21 @@ export default function AgentChat({ // erroring, aborting or being stopped, and a channel that misses one of // those leaves the trigger claiming the agent is still working forever. const turnOpenRef = useRef(false) + // Screen-reader announcement for the turn. Deliberately NOT the streaming + // text: a live region over token deltas re-announces on every delta and + // renders the chat unusable with a screen reader. Announce the two states + // that matter instead, and the finished answer once, when it is finished. + const [announcement, setAnnouncement] = useState('') useEffect(() => { if (streaming) { turnOpenRef.current = true + turnStartRef.current = messagesRef.current.length onStatus?.({ type: 'turn_start' }) + setAnnouncement('Assistenten skriver ett svar.') } else if (turnOpenRef.current) { turnOpenRef.current = false onStatus?.({ type: 'turn_end' }) + setAnnouncement(announceableAnswer(messagesRef.current.slice(turnStartRef.current))) } }, [streaming, onStatus]) const [errorMessage, setErrorMessage] = useState(null) @@ -717,6 +740,13 @@ export default function AgentChat({ return (
+ {/* The chat had no live region at all, so a screen-reader user got no + signal that the assistant had answered: the reply simply appeared for + people who could see it. role="status" is the polite variant, which + waits for a pause rather than interrupting. */} +
+ {announcement} +
{/* The pill is positioned against THIS box, not the whole component: the composer below grows as the user types, and a fixed offset from the bottom would slide the pill under it. */} @@ -1284,3 +1314,32 @@ export function normalizeStoredMessages( } return out } + +/** + * What to read out when a turn finishes. + * + * Takes only the CURRENT turn's messages: given the whole thread, a turn that + * produced no text of its own would find the previous answer and announce it + * again as if it were new. + * + * The cap exists because a screen reader reads a live region straight through: + * a 900-word bokslut explanation announced in one uninterruptible burst is + * worse than not announcing it. It covers the WHOLE announcement, suffix + * included, so the promise the constant makes is the one the output keeps. + */ +export const ANNOUNCEMENT_LIMIT = 400 +const CONTINUES = '… Svaret fortsätter i meddelandet.' + +export function announceableAnswer(messages: ChatMessage[]): string { + const last = [...messages].reverse().find((m) => m.role === 'assistant') + + // Stop leaves the partial text in place with a visible marker. Reading it + // out as a finished answer would tell a screen-reader user the opposite of + // what the marker tells everyone else. + if (last?.interrupted) return 'Assistenten avbröts. Ett ofullständigt svar står i meddelandet.' + + const text = last?.text?.trim() + if (!text) return 'Assistenten är klar.' + if (text.length <= ANNOUNCEMENT_LIMIT) return text + return text.slice(0, ANNOUNCEMENT_LIMIT - CONTINUES.length).trimEnd() + CONTINUES +} diff --git a/components/agent/AgentSheet.tsx b/components/agent/AgentSheet.tsx index 448b1bb8..63d9620b 100644 --- a/components/agent/AgentSheet.tsx +++ b/components/agent/AgentSheet.tsx @@ -19,6 +19,7 @@ import AgentChat, { import type { StoredStagedOperation } from '@/types' import type { AgentStatusEvent } from './agent-status' import ContextChip from './ContextChip' +import { intentLabel } from './conversation-display' import AgentAvatar from './AgentAvatar' import AgentSessionList from './AgentSessionList' import SandboxAgentPreview from './SandboxAgentPreview' @@ -105,8 +106,8 @@ export default function AgentSheet({ const companyCtx = useCompanyOptional() const isSandbox = companyCtx?.isSandbox ?? false const agentName = identity.displayName?.trim() || null - const sheetTitle = intentToTitle(intentId, agentName) - const displayTitle = loaded ? (loaded.title ?? intentToTitle(loaded.intentId, agentName)) : sheetTitle + const sheetTitle = intentLabel(intentId, agentName) + const displayTitle = loaded ? (loaded.title ?? intentLabel(loaded.intentId, agentName)) : sheetTitle const activeConversationId = loaded?.id ?? conversationId // A resumed thread's stored ref wins: it says what THAT conversation was // about, which is the whole reason to show this. Falls back to the ref the @@ -374,19 +375,4 @@ export default function AgentSheet({ ) } -function intentToTitle(intentId: string, agentName: string | null): string { - switch (intentId) { - case 'general.help': - return agentName ? `Fråga ${agentName}` : 'Fråga din assistent' - case 'transaction.categorization': - return 'Hjälp med transaktion' - case 'verifikation.draft': - return 'Hjälp med verifikation' - case 'invoice.draft': - return 'Hjälp med faktura' - case 'supplier_invoice.review': - return 'Granska leverantörsfaktura' - default: - return agentName ? `Fråga ${agentName}` : 'Fråga din assistent' - } -} + diff --git a/components/agent/MarkdownMessage.tsx b/components/agent/MarkdownMessage.tsx index 37ab3c2b..68016aa4 100644 --- a/components/agent/MarkdownMessage.tsx +++ b/components/agent/MarkdownMessage.tsx @@ -2,6 +2,8 @@ import ReactMarkdown from 'react-markdown' import remarkGfm from 'remark-gfm' +import Link from 'next/link' +import { isInternalHref } from './markdown-links' /** * Isolated so AgentChat can load the markdown parser (react-markdown + @@ -10,5 +12,57 @@ import remarkGfm from 'remark-gfm' * being parsed eagerly whenever the chat surface mounts. */ export default function MarkdownMessage({ text }: { text: string }) { - return {text} + return ( + + {text} + + ) +} + +/** + * Links inside an answer. + * + * These were plain anchors, so following one the agent had written did a full + * document load: the whole app rebooted and the conversation went with it, in + * the panel and in /chat alike. Internal links now route client-side, which + * keeps the thread alive beside the page the user just opened, which is the + * point of docking the panel in the first place. + * + * External links open in a new tab for the same reason, and carry + * rel="noopener noreferrer": the href came out of a model that reads customer + * documents, and target="_blank" without it hands the opened page a + * window.opener handle back into an authenticated session. + */ +function MarkdownLink({ + href, + title, + children, +}: { + href?: string + // Markdown carries an optional title: [text](url "title"). Dropping it here + // would silently discard something the answer's author wrote. + title?: string + children?: React.ReactNode +}) { + if (!href) return <>{children} + + if (isInternalHref(href)) { + return ( + + {children} + + ) + } + + return ( + + {children} + + ) } diff --git a/components/agent/__tests__/markdown-links.test.ts b/components/agent/__tests__/markdown-links.test.ts new file mode 100644 index 00000000..59961710 --- /dev/null +++ b/components/agent/__tests__/markdown-links.test.ts @@ -0,0 +1,124 @@ +import { describe, it, expect } from 'vitest' +import { isInternalHref } from '../markdown-links' +import { ANNOUNCEMENT_LIMIT, announceableAnswer } from '../AgentChat' +import { intentLabel } from '../conversation-display' + +describe('isInternalHref', () => { + it('routes app paths client-side', () => { + expect(isInternalHref('/invoices/abc-123')).toBe(true) + expect(isInternalHref('/bookkeeping/year-end?step=2')).toBe(true) + expect(isInternalHref('/kpi#marginal')).toBe(true) + }) + + it('does not treat a protocol-relative url as internal', () => { + // The href comes out of a model that reads customer documents. "//host" + // starts with a slash but leaves the site, and handing it to the router as + // an app path is the one failure mode worth engineering against. + expect(isInternalHref('//evil.example/login')).toBe(false) + expect(isInternalHref('/\\evil.example')).toBe(false) + }) + + it('leaves absolute and non-http schemes external', () => { + expect(isInternalHref('https://skatteverket.se')).toBe(false) + expect(isInternalHref('http://example.com')).toBe(false) + expect(isInternalHref('mailto:a@b.se')).toBe(false) + expect(isInternalHref('javascript:alert(1)')).toBe(false) + expect(isInternalHref('')).toBe(false) + }) +}) + +describe('announceableAnswer', () => { + const msg = (role: 'user' | 'assistant', text: string) => + ({ role, text }) as Parameters[0][number] + + it('announces the finished answer, not the question', () => { + expect( + announceableAnswer([msg('user', 'Hur gick juli?'), msg('assistant', 'Juli gick 12 % bättre.')]), + ).toBe('Juli gick 12 % bättre.') + }) + + it('reads the LAST assistant turn when the thread has several', () => { + expect( + announceableAnswer([ + msg('assistant', 'Första svaret.'), + msg('user', 'Och augusti?'), + msg('assistant', 'Andra svaret.'), + ]), + ).toBe('Andra svaret.') + }) + + it('caps the WHOLE announcement, suffix included, and says where the rest is', () => { + // A screen reader reads a live region straight through: a full bokslut + // explanation announced in one uninterruptible burst is worse than not + // announcing at all. Asserted against the constant, and at the real limit + // rather than a looser number the suffix could sneak past. + const out = announceableAnswer([msg('assistant', 'a'.repeat(1000))]) + expect(out.length).toBeLessThanOrEqual(ANNOUNCEMENT_LIMIT) + expect(out).toContain('Svaret fortsätter i meddelandet') + }) + + it('does not read an interrupted answer as a finished one', () => { + // Stop leaves the partial text with a visible marker. Announcing it as the + // answer tells a screen-reader user the opposite of what everyone else sees. + const out = announceableAnswer([ + { role: 'assistant', text: 'Momsen för juli blev 12 4', interrupted: true } as Parameters< + typeof announceableAnswer + >[0][number], + ]) + expect(out).toContain('avbröts') + expect(out).not.toContain('12 4') + }) + + it('never re-announces an earlier answer for a turn that produced no text', () => { + // The caller passes only THIS turn's messages. Given the whole thread, a + // tool-only turn would find the previous answer and read it out again as + // though it were new: the user hears a stale answer to a new question. + const thisTurn = [msg('user', 'Boka detta')] + expect(announceableAnswer(thisTurn)).toBe('Assistenten är klar.') + }) + + it('still says something when the turn produced no text', () => { + // Silence would be indistinguishable from the request never having been + // sent. An assistant bubble that only ever held whitespace counts as none. + expect(announceableAnswer([msg('assistant', ' ')])).toBe('Assistenten är klar.') + expect(announceableAnswer([])).toBe('Assistenten är klar.') + }) +}) + +describe('intentLabel', () => { + it('never shows the raw intent id', () => { + // It used to return the id itself, so an unmapped intent put + // "bokslut.step" in front of the user as the name of their conversation. + expect(intentLabel('some.unmapped.intent')).toBe('Fråga din assistent') + expect(intentLabel('some.unmapped.intent', 'Anna')).toBe('Fråga Anna') + }) + + it('gives the panel and the history list the SAME name for a thread', () => { + // The two maps had drifted: the panel titled a bokslut thread "Fråga Anna" + // while the history list called it "Hjälp med bokslut". + expect(intentLabel('bokslut.step')).toBe('Hjälp med bokslut') + expect(intentLabel('bokslut.step', 'Anna')).toBe('Hjälp med bokslut') + expect(intentLabel('kpi.explain', 'Anna')).toBe('Förklara nyckeltal') + }) + + it('personalises general help and falls back without a name', () => { + expect(intentLabel('general.help', 'Anna')).toBe('Fråga Anna') + expect(intentLabel('general.help')).toBe('Fråga din assistent') + expect(intentLabel('general.help', ' ')).toBe('Fråga din assistent') + }) + + it('does not resolve inherited Object keys as labels', () => { + // A plain object literal inherits from Object.prototype, so a lookup of + // 'toString' returned a FUNCTION, passed the truthiness check and reached + // React as a conversation title. intent_id comes from the database. + expect(intentLabel('toString')).toBe('Fråga din assistent') + expect(intentLabel('constructor', 'Anna')).toBe('Fråga Anna') + expect(intentLabel('__proto__')).toBe('Fråga din assistent') + }) + + it('keeps momsdeklaration spelled correctly through the soft hyphen', () => { + // The label carries a U+00AD so it can break across the narrow panel. + // Stripping it must leave a real word. + expect(intentLabel('vat.review').replace(/­/g, '')).toBe('Granska momsdeklaration') + }) +}) diff --git a/components/agent/conversation-display.ts b/components/agent/conversation-display.ts index f43b2b2a..df6759f2 100644 --- a/components/agent/conversation-display.ts +++ b/components/agent/conversation-display.ts @@ -62,27 +62,40 @@ export function relativeTime(iso: string | null | undefined): string { return new Date(iso).toLocaleDateString('sv-SE', { month: 'short', day: 'numeric' }) } -export function intentLabel(intentId: string): string { - switch (intentId) { - case 'general.help': - return 'Fråga din assistent' - case 'transaction.categorization': - return 'Hjälp med transaktion' - case 'invoice.draft': - return 'Hjälp med faktura' - case 'supplier_invoice.review': - return 'Granska leverantörsfaktura' - case 'vat.review': - return 'Granska moms­deklaration' - case 'bokslut.step': - return 'Hjälp med bokslut' - case 'verifikation.draft': - return 'Hjälp med verifikation' - case 'kpi.explain': - return 'Förklara nyckeltal' - default: - return intentId - } +/** + * One label per intent, for every surface that names a conversation. + * + * There were two of these: this map and an intentToTitle in AgentSheet. They + * had already drifted, so the panel opened on the bokslut wizard titled + * "Fråga Anna" while the same thread in the history list read "Hjälp med + * bokslut", and this one's fallback returned the raw intent id, putting + * "bokslut.step" in front of the user as the name of their own conversation. + */ +// Null-prototype: a plain literal inherits from Object.prototype, so +// intentLabel('toString') would resolve to a FUNCTION, pass the truthiness +// check, and be handed to React as a title. intent_id comes from the database. +const INTENT_LABELS: Record = Object.assign(Object.create(null), { + 'transaction.categorization': 'Hjälp med transaktion', + 'invoice.draft': 'Hjälp med faktura', + 'supplier_invoice.review': 'Granska leverantörsfaktura', + 'vat.review': 'Granska moms\u00addeklaration', + 'bokslut.step': 'Hjälp med bokslut', + 'verifikation.draft': 'Hjälp med verifikation', + 'kpi.explain': 'Förklara nyckeltal', + 'settings.help': 'Hjälp med inställningar', + 'inbox.bulk-book': 'Bokför från inkorgen', +}) + +/** + * `agentName` personalises the general-help and unknown cases ("Fråga Anna"). + * Omit it where the agent's name is not to hand: the wording stays correct, + * just less personal. An unknown intent NEVER falls through to its id. + */ +export function intentLabel(intentId: string, agentName?: string | null): string { + const known = INTENT_LABELS[intentId] + if (known) return known + const name = agentName?.trim() + return name ? `Fråga ${name}` : 'Fråga din assistent' } // Group a flat (already server-sorted: pinned first, then last_message_at desc) diff --git a/components/agent/markdown-links.ts b/components/agent/markdown-links.ts new file mode 100644 index 00000000..35fc829b --- /dev/null +++ b/components/agent/markdown-links.ts @@ -0,0 +1,22 @@ +/** + * Is this href a link back into the app? + * + * Kept out of MarkdownMessage so it is testable without the markdown parser + * (that component exists to be lazily imported, and this repo's unit project + * is node-only). + * + * The href comes from a language model that reads customer documents and + * supplier invoices, so "starts with a slash" is not a sufficient test: + * "//evil.example" is protocol-relative and leaves the site entirely. Anything + * this returns false for is rendered as an external link, which is the safe + * direction to be wrong in: an external link that could have been routed + * costs a page load, whereas an external URL treated as internal would be + * handed to the router as an app path. + */ +export function isInternalHref(href: string): boolean { + if (!href.startsWith('/')) return false + // Protocol-relative ("//host/path"), and the backslash variant browsers + // normalise to the same thing. + if (href.startsWith('//') || href.startsWith('/\\')) return false + return true +}