diff --git a/src/api/conversations.ts b/src/api/conversations.ts index 9469d33..2a9c2f1 100644 --- a/src/api/conversations.ts +++ b/src/api/conversations.ts @@ -26,7 +26,7 @@ import { type StoredThread, } from '../store/conversations.js' import { decodeCursor, encodeCursor } from './cursor.js' -import { apiError, json } from './responses.js' +import { apiError, json, noContent } from './responses.js' import { isUuid } from './uuid.js' /** Default page size when the caller omits `limit` (spec §3a). */ @@ -426,6 +426,34 @@ export async function handlePatchConversation( return json(200, toConversationSummaryJson(updated)) } +/** + * Handle `DELETE /api/v1/conversations/{id}` — soft delete (spec §4d, v1.1). + * + * `204` with an empty body on success. `404 not_found` when `{id}` is + * missing, already deleted, or not UUID-shaped — all three identical, per + * §5's no-existence-leak rule (deleting twice reports the second call as a + * plain miss). From this point every endpoint treats the conversation as + * nonexistent — including a keyed replay of a previously-successful reply + * (§4a's replay-vs-delete rule); the store's existing deleted-handling + * covers all of them, so this handler is just the flag flip plus the + * spec's response mapping. + */ +export async function handleDeleteConversation( + id: string, + deps: { store: ConversationStore }, +): Promise { + if (!isUuid(id)) { + return apiError(404, 'not_found', 'No conversation with that id.') + } + + const deleted = await deps.store.deleteConversation(id) + if (!deleted) { + return apiError(404, 'not_found', 'No conversation with that id.') + } + + return noContent() +} + /** * Read and JSON-parse `request`'s body without ever throwing — a malformed * or empty body is `400 validation_failed`, never an uncontrolled `500` diff --git a/src/api/index.test.ts b/src/api/index.test.ts index 37faef5..d0848e6 100644 --- a/src/api/index.test.ts +++ b/src/api/index.test.ts @@ -978,6 +978,105 @@ describe('createInboxApi', () => { }) }) + // --- delete (HT-30, spec §4d v1.1) ---------------------------------------------- + + describe('delete', () => { + /** A `DELETE` request for `path`, Bearer-authenticated unless `token` is explicitly omitted. */ + function del(path: string, ...tokenArg: [string | undefined] | []): Request { + const token = tokenArg.length > 0 ? tokenArg[0] : TOKEN + const headers: Record = {} + if (token !== undefined) { + headers.Authorization = `Bearer ${token}` + } + return new Request(`https://x.example.test${path}`, { method: 'DELETE', headers }) + } + + it('deletes: 204 with an empty body + no-store; afterwards every endpoint treats it as nonexistent', async () => { + const { store, api } = await freshApi() + const { conversationId } = await store.createConversation(newConversation()) + + const res = await api(del(`/api/v1/conversations/${conversationId}`)) + expect(res.status).toBe(204) + expect(res.headers.get('Cache-Control')).toBe('no-store') + expect(await res.text()).toBe('') + + // GET → 404. + const getRes = await api(get(`/api/v1/conversations/${conversationId}`)) + expect(getRes.status).toBe(404) + + // The list never shows it, under any folder. + const listRes = await api(get('/api/v1/conversations')) + const listBody = (await listRes.json()) as { conversations: Array<{ id: string }> } + expect(listBody.conversations.map((c) => c.id)).not.toContain(conversationId) + + // PATCH → 404 (not reachable), reply → 404 (nothing sent). + const patchRes = await api( + patch(`/api/v1/conversations/${conversationId}`, { status: 'active' }), + ) + expect(patchRes.status).toBe(404) + const replyRes = await api( + replyPost(`/api/v1/conversations/${conversationId}/replies`, { text: 'Hello?' }), + ) + expect(replyRes.status).toBe(404) + }) + + it('a second DELETE is 404 — already-deleted is indistinguishable from never-existed', async () => { + const { store, api } = await freshApi() + const { conversationId } = await store.createConversation(newConversation()) + + expect((await api(del(`/api/v1/conversations/${conversationId}`))).status).toBe(204) + const second = await api(del(`/api/v1/conversations/${conversationId}`)) + expect(second.status).toBe(404) + expect(await second.json()).toEqual({ + error: { code: 'not_found', message: expect.any(String) }, + }) + }) + + it('404s for a missing id and for a non-UUID-shaped id', async () => { + const { api } = await freshApi() + expect((await api(del(`/api/v1/conversations/${RANDOM_UUID}`))).status).toBe(404) + expect((await api(del('/api/v1/conversations/not-a-uuid'))).status).toBe(404) + }) + + it('a keyed replay of a previously-successful reply returns 404 after the delete (spec §4a replay-vs-delete)', async () => { + const { store, api } = await freshApi() + const { conversationId } = await store.createConversation(newConversation()) + + const first = await api( + replyPost( + `/api/v1/conversations/${conversationId}/replies`, + { text: 'Original send.' }, + { idempotencyKey: 'replay-vs-delete' }, + ), + ) + expect(first.status).toBe(201) + + expect((await api(del(`/api/v1/conversations/${conversationId}`))).status).toBe(204) + + // The replay does NOT resurrect the original 201 — the conversation is + // gone; there is no mail-safety impact (the original send already + // happened). + const replay = await api( + replyPost( + `/api/v1/conversations/${conversationId}/replies`, + { text: 'Original send.' }, + { idempotencyKey: 'replay-vs-delete' }, + ), + ) + expect(replay.status).toBe(404) + }) + + it('401s without a token', async () => { + const { store, api } = await freshApi() + const { conversationId } = await store.createConversation(newConversation()) + const res = await api(del(`/api/v1/conversations/${conversationId}`, undefined)) + expect(res.status).toBe(401) + + // And nothing was deleted by the unauthenticated call. + expect((await api(get(`/api/v1/conversations/${conversationId}`))).status).toBe(200) + }) + }) + // --- method routing (HT-18 additions) ------------------------------------------ describe('method routing', () => { @@ -994,16 +1093,16 @@ describe('createInboxApi', () => { expect(res.headers.get('Cache-Control')).toBe('no-store') }) - it('DELETE on the item route is 405 with Allow: GET, PATCH', async () => { + it('PUT on the item route is 405 with Allow: GET, PATCH, DELETE', async () => { const { api } = await freshApi() const res = await api( new Request(`https://x.example.test/api/v1/conversations/${RANDOM_UUID}`, { - method: 'DELETE', + method: 'PUT', headers: { Authorization: `Bearer ${TOKEN}` }, }), ) expect(res.status).toBe(405) - expect(res.headers.get('Allow')).toBe('GET, PATCH') + expect(res.headers.get('Allow')).toBe('GET, PATCH, DELETE') }) it('GET on the replies route is 405 with Allow: POST', async () => { diff --git a/src/api/index.ts b/src/api/index.ts index f337ae8..5fa45c9 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -34,6 +34,7 @@ import type { EmailSender } from '../providers/index.js' import type { ConversationStore } from '../store/conversations.js' import { authenticateRequest } from './auth.js' import { + handleDeleteConversation, handleGetConversation, handleListConversations, handlePatchConversation, @@ -144,6 +145,9 @@ export function createInboxApi(deps: InboxApiDeps): (request: Request) => Promis case 'conversation-patch': return await handlePatchConversation(route.id, request, { store: deps.store }) + case 'conversation-delete': + return await handleDeleteConversation(route.id, { store: deps.store }) + case 'conversation-reply': return await handleReply(route.id, request, { store: deps.store, diff --git a/src/api/responses.ts b/src/api/responses.ts index 01fd719..3c49b9e 100644 --- a/src/api/responses.ts +++ b/src/api/responses.ts @@ -41,6 +41,19 @@ export function json(status: number, body: unknown): Response { }) } +/** + * Build an empty `204 No Content` response (spec §4d's successful soft + * delete, v1.1 — the one success in this API with no body). Still carries + * `Cache-Control: no-store` like every other response; no `Content-Type`, + * since there is no content for it to describe. + */ +export function noContent(): Response { + return new Response(null, { + status: 204, + headers: { 'Cache-Control': 'no-store' }, + }) +} + /** * Build an error `Response` in the standard envelope (spec §3). * diff --git a/src/api/router.test.ts b/src/api/router.test.ts index 78597a8..7658d2e 100644 --- a/src/api/router.test.ts +++ b/src/api/router.test.ts @@ -42,10 +42,17 @@ describe('matchRoute', () => { }) }) - it('returns method-not-allowed for a wrong method on the item route, naming GET and PATCH', () => { + it('matches DELETE /api/v1/conversations/{id}, extracting the id (spec §4d, v1.1)', () => { expect(matchRoute('DELETE', '/api/v1/conversations/abc-123')).toEqual({ + kind: 'conversation-delete', + id: 'abc-123', + }) + }) + + it('returns method-not-allowed for a wrong method on the item route, naming GET, PATCH and DELETE', () => { + expect(matchRoute('PUT', '/api/v1/conversations/abc-123')).toEqual({ kind: 'method-not-allowed', - allow: ['GET', 'PATCH'], + allow: ['GET', 'PATCH', 'DELETE'], }) }) diff --git a/src/api/router.ts b/src/api/router.ts index a28b81a..426f825 100644 --- a/src/api/router.ts +++ b/src/api/router.ts @@ -24,10 +24,10 @@ const CONVERSATIONS_LIST: RouteDef = { methods: ['GET'], } -/** `/api/v1/conversations/{id}` — get (spec §3b) and status patch (spec §4b). */ +/** `/api/v1/conversations/{id}` — get (spec §3b), status patch (spec §4b), and soft delete (spec §4d, v1.1). */ const CONVERSATION_ITEM: RouteDef = { pattern: /^\/api\/v1\/conversations\/(?[^/]+)$/, - methods: ['GET', 'PATCH'], + methods: ['GET', 'PATCH', 'DELETE'], } /** `/api/v1/conversations/{id}/replies` — the Agent replies (spec §4a), POST only. */ @@ -44,6 +44,7 @@ export type RouteMatch = | { kind: 'conversations-list' } | { kind: 'conversation-item'; id: string } | { kind: 'conversation-patch'; id: string } + | { kind: 'conversation-delete'; id: string } | { kind: 'conversation-reply'; id: string } | { kind: 'method-not-allowed'; allow: string[] } | { kind: 'not-found' } @@ -83,8 +84,11 @@ export function matchRoute(method: string, pathname: string): RouteMatch { if (route === CONVERSATION_REPLIES) { return { kind: 'conversation-reply', id } } - // route === CONVERSATION_ITEM: GET reads, PATCH updates status. - return method === 'GET' ? { kind: 'conversation-item', id } : { kind: 'conversation-patch', id } + // route === CONVERSATION_ITEM: GET reads, PATCH updates status, DELETE + // soft-deletes (spec §4d, v1.1). + if (method === 'GET') return { kind: 'conversation-item', id } + if (method === 'DELETE') return { kind: 'conversation-delete', id } + return { kind: 'conversation-patch', id } } return { kind: 'not-found' } diff --git a/src/store/conversations.test.ts b/src/store/conversations.test.ts index 40e5bb4..6cb9cfa 100644 --- a/src/store/conversations.test.ts +++ b/src/store/conversations.test.ts @@ -954,4 +954,44 @@ describe('createConversationStore', () => { }) }) }) + + describe('deleteConversation (HT-30, spec §4d v1.1)', () => { + it('soft-deletes a live conversation: true, then invisible to every public path — but the rows survive in storage', async () => { + const { db, store } = await freshStore() + const { conversationId } = await store.createConversation(newConversation()) + + expect(await store.deleteConversation(conversationId)).toBe(true) + + // Public reads treat it as nonexistent… + expect(await store.getConversation(conversationId, { includeDeleted: false })).toBeNull() + expect((await store.listConversations({ limit: 50 })).map((c) => c.id)).not.toContain( + conversationId, + ) + expect(await store.setConversationStatus(conversationId, 'active')).toBeNull() + expect(await store.appendThread(conversationId, newThread())).toEqual({ + ok: false, + reason: 'deleted', + }) + + // …but the mail itself is still in storage (charter invariant #1) — + // soft delete changes visibility, never data. + const raw = await store.getConversation(conversationId) + expect(raw?.status).toBe('deleted') + expect(raw?.threads).toHaveLength(1) + const [{ count }] = await db.query<{ count: number }>( + 'SELECT count(*)::int AS count FROM threads WHERE conversation_id = $1', + [conversationId], + ) + expect(count).toBe(1) + }) + + it('a second delete (and a nonexistent id) return false — indistinguishable misses', async () => { + const { store } = await freshStore() + const { conversationId } = await store.createConversation(newConversation()) + + expect(await store.deleteConversation(conversationId)).toBe(true) + expect(await store.deleteConversation(conversationId)).toBe(false) + expect(await store.deleteConversation(RANDOM_UUID)).toBe(false) + }) + }) }) diff --git a/src/store/conversations.ts b/src/store/conversations.ts index e4c2d09..514ffbc 100644 --- a/src/store/conversations.ts +++ b/src/store/conversations.ts @@ -417,6 +417,26 @@ export interface ConversationStore { conversationId: string, status: ConversationStatus, ): Promise + + /** + * Soft-delete a conversation — the write path behind `DELETE + * /api/v1/conversations/{id}` (specs/api/agent-inbox-v1.md §4d, v1.1). A + * single `UPDATE ... SET status = 'deleted' ... RETURNING`, scoped to + * `status <> 'deleted'` so deleting twice reports the second call as a + * miss. Returns `true` when a live conversation was deleted, `false` when + * no row matched (never existed, or already deleted — indistinguishable, + * per the API's no-existence-leak rule, §5). + * + * Soft, permanently: the row and its threads stay in storage (charter + * invariant #1 — never lose customer mail) but nothing surfaces them + * again. Every read/write path already treats `'deleted'` as nonexistent + * — `getConversation({includeDeleted: false})`, `listConversations` (any + * folder), `appendThread` (returns `{reason: 'deleted'}`; a reply token + * minted against it starts a fresh conversation, threading.md §5), and + * `setConversationStatus` (not reachable) — so this method only has to + * flip the flag, not chase down consumers. + */ + deleteConversation(conversationId: string): Promise } /** @@ -802,6 +822,18 @@ export function createConversationStore(db: Db): ConversationStore { const row = rows[0] return row === undefined ? null : toConversationSummary(row) }, + + async deleteConversation(conversationId) { + // No updated_at bump: a deleted conversation is never surfaced again, + // so its sort key is meaningless — and leaving it untouched keeps the + // row an exact record of its last LIVE activity (charter invariant #1: + // storage keeps the mail; only visibility changes). + const rows = await db.query<{ id: string }>( + `UPDATE conversations SET status = 'deleted' WHERE id = $1 AND status <> 'deleted' RETURNING id`, + [conversationId], + ) + return rows.length === 1 + }, } }