Files
accounted/app/api/agent/__tests__/feedback.test.ts
T
Jakob Wennberg 1dce9227a4 feat(assistant): make the thumbs actually report something (#1236)
* feat(assistant): make the thumbs actually report something

The thumbs up/down under an assistant answer shipped wired to nothing. They lit
up, the vote died in component state, and the code said so in a comment nobody
reading the UI could see. An affordance that looks like it reports something and
does not is worse than no affordance: it spends the user's goodwill once, and
silently.

They now post to a new /api/agent/feedback, which emits the SAME agent.feedback
event the gnubok_feedback MCP tool emits, with actorType 'user'. The product
team already queries event_log for that type, so chat votes land in the backlog
they read rather than in a second place someone has to remember to look at.
event_log takes the payload as jsonb and already treats agent.* as telemetry, so
there is no migration.

The conversation id is caller-supplied, so it gets the same ownership check
/api/agent/invoke got: without it a member could file feedback against a
colleague's thread and the backlog would carry conversations the reporter never
saw. Mutation-checked, three tests fail when the guard is removed.

The pressed state is set only after the server accepts the vote, so the button
never claims a report that never arrived, and a vote does not toggle off: it is
append-only telemetry, and offering an undo we cannot honour would be a control
that lies. Changing your mind sends the other sentiment instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(assistant): drop the unused free-text field from the feedback route

compliance-swarm flagged the `comment` field as an undocumented PII exposure:
free text embedded verbatim into the agent.feedback payload and written to
event_log under telemetry retention, with no data-classification decision, next
to a PostHog policy that treats the same class of data differently.

The finding is right, and the field was worse than it looked: no caller ever
sent one. The UI posts sentiment and a turn index. So this is dead API surface
whose only effect was to accept whatever a user might type, in an accounting
product, into a 180-day log: client names, personnummer, case details.

Removed rather than documented. A comment box is a reasonable thing to want,
but it needs its own classification and redaction decision made with the UI in
front of it, not inherited from an unused parameter.

The test now asserts the property instead of the field's absence: a caller that
posts a comment anyway must not get it stored anywhere in the payload.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-27 15:46:08 +02:00

185 lines
6.3 KiB
TypeScript

/**
* Tests for POST /api/agent/feedback.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { NextResponse } from 'next/server'
import { createQueuedMockSupabase, createMockRequest, parseJsonResponse } from '@/tests/helpers'
const { supabase, enqueue, reset } = createQueuedMockSupabase()
const requireAuthMock = vi.fn()
vi.mock('@/lib/auth/require-auth', () => ({
requireAuth: (...args: unknown[]) => requireAuthMock(...args),
}))
vi.mock('@/lib/company/context', () => ({
getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
requireCompanyId: vi.fn().mockResolvedValue('company-1'),
}))
const requireWriteMock = vi.fn()
vi.mock('@/lib/auth/require-write', () => ({
requireWritePermission: (...args: unknown[]) => requireWriteMock(...args),
}))
vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn() }))
const emitMock = vi.fn()
vi.mock('@/lib/events/bus', () => ({
eventBus: { emit: (...args: unknown[]) => emitMock(...args) },
}))
import { POST } from '../feedback/route'
const routeParams = { params: Promise.resolve({}) }
const CONVERSATION_ID = '11111111-2222-4333-8444-555555555555'
const body = (over: Record<string, unknown> = {}) => ({
conversation_id: CONVERSATION_ID,
sentiment: 'negative',
...over,
})
beforeEach(() => {
vi.clearAllMocks()
reset()
requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase, error: null })
requireWriteMock.mockResolvedValue({ ok: true })
})
describe('POST /api/agent/feedback', () => {
it('returns 401 when not authenticated', async () => {
requireAuthMock.mockResolvedValue({
user: null,
supabase,
error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
})
const req = createMockRequest('/api/agent/feedback', { method: 'POST', body: body() })
expect((await POST(req, routeParams)).status).toBe(401)
expect(emitMock).not.toHaveBeenCalled()
})
it('returns 400 for a sentiment outside the two votes the UI can send', async () => {
const req = createMockRequest('/api/agent/feedback', {
method: 'POST',
body: body({ sentiment: 'furious' }),
})
const { status } = await parseJsonResponse(await POST(req, routeParams))
expect(status).toBe(400)
expect(emitMock).not.toHaveBeenCalled()
})
it('returns 400 when the conversation id is not a uuid', async () => {
const req = createMockRequest('/api/agent/feedback', {
method: 'POST',
body: body({ conversation_id: 'not-a-uuid' }),
})
const { status } = await parseJsonResponse(await POST(req, routeParams))
expect(status).toBe(400)
})
it('404s on a conversation belonging to another user', async () => {
// Same hole that /api/agent/invoke had: the id is caller-supplied, so
// without the ownership check a member could file feedback against a
// colleague's thread and the triage backlog would carry a conversation the
// reporter never saw.
enqueue({
data: {
id: CONVERSATION_ID,
user_id: 'someone-else',
company_id: 'company-1',
intent_id: 'general.help',
},
})
const req = createMockRequest('/api/agent/feedback', { method: 'POST', body: body() })
const { status } = await parseJsonResponse(await POST(req, routeParams))
expect(status).toBe(404)
expect(emitMock).not.toHaveBeenCalled()
})
it('404s on a conversation in another company', async () => {
enqueue({
data: {
id: CONVERSATION_ID,
user_id: 'user-1',
company_id: 'company-2',
intent_id: 'general.help',
},
})
const req = createMockRequest('/api/agent/feedback', { method: 'POST', body: body() })
expect((await parseJsonResponse(await POST(req, routeParams))).status).toBe(404)
expect(emitMock).not.toHaveBeenCalled()
})
it('404s when the conversation does not exist', async () => {
enqueue({ data: null })
const req = createMockRequest('/api/agent/feedback', { method: 'POST', body: body() })
expect((await parseJsonResponse(await POST(req, routeParams))).status).toBe(404)
})
it('emits agent.feedback with actorType user on the happy path', async () => {
enqueue({
data: {
id: CONVERSATION_ID,
user_id: 'user-1',
company_id: 'company-1',
intent_id: 'vat.review',
},
})
const req = createMockRequest('/api/agent/feedback', {
method: 'POST',
body: body({ sentiment: 'positive', message_index: 4 }),
})
const { status } = await parseJsonResponse(await POST(req, routeParams))
expect(status).toBe(200)
expect(emitMock).toHaveBeenCalledOnce()
const event = emitMock.mock.calls[0]![0] as {
type: string
payload: Record<string, unknown>
}
// Same event type the MCP tool emits, so chat votes land in the backlog
// the product team already reads instead of a second place.
expect(event.type).toBe('agent.feedback')
expect(event.payload.sentiment).toBe('positive')
expect(event.payload.actorType).toBe('user')
expect(event.payload.actorId).toBe('user-1')
expect(event.payload.companyId).toBe('company-1')
expect(event.payload.sessionId).toBe(CONVERSATION_ID)
// Traceable to an answer: a thumbs-down that cannot be tied to a turn is a
// number, not a report.
expect(event.payload.context).toContain('vat.review')
expect(event.payload.context).toContain('#4')
})
it('never carries user free text into the telemetry log', async () => {
// A comment box here would put whatever the user typed, in an accounting
// product, verbatim into event_log under telemetry retention: client
// names, personnummer, case details. The field is not accepted, and a
// caller that sends one anyway must not have it stored.
enqueue({
data: {
id: CONVERSATION_ID,
user_id: 'user-1',
company_id: 'company-1',
intent_id: 'general.help',
},
})
const req = createMockRequest('/api/agent/feedback', {
method: 'POST',
body: body({ comment: 'Anna Andersson 19850101-1234 fick fel konto' }),
})
await POST(req, routeParams)
const event = emitMock.mock.calls[0]![0] as { payload: { context: string } }
expect(event.payload.context).not.toContain('Anna Andersson')
expect(event.payload.context).not.toContain('19850101')
expect(JSON.stringify(event.payload)).not.toContain('19850101')
})
})