Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion src/api/conversations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down Expand Up @@ -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<Response> {
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`
Expand Down
105 changes: 102 additions & 3 deletions src/api/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {}
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', () => {
Expand All @@ -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 () => {
Expand Down
4 changes: 4 additions & 0 deletions src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions src/api/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*
Expand Down
11 changes: 9 additions & 2 deletions src/api/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
})
})

Expand Down
12 changes: 8 additions & 4 deletions src/api/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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\/(?<id>[^/]+)$/,
methods: ['GET', 'PATCH'],
methods: ['GET', 'PATCH', 'DELETE'],
}

/** `/api/v1/conversations/{id}/replies` — the Agent replies (spec §4a), POST only. */
Expand All @@ -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' }
Expand Down Expand Up @@ -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' }
Expand Down
40 changes: 40 additions & 0 deletions src/store/conversations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
32 changes: 32 additions & 0 deletions src/store/conversations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,26 @@ export interface ConversationStore {
conversationId: string,
status: ConversationStatus,
): Promise<ConversationSummary | null>

/**
* 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<boolean>
}

/**
Expand Down Expand Up @@ -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
},
}
}

Expand Down
Loading