From 173529d50e3278b177cedc6165ca9887184511e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Sat, 22 Aug 2026 00:27:11 +0000 Subject: [PATCH 1/7] feat: integrate message comment backend --- .agents/skills/api-reference/SKILL.md | 1 + .claude/skills/api-reference/SKILL.md | 8 + apps/api/.env.example | 7 + apps/api/src/durable-objects/migrations.ts | 91 +++ .../project-data/comment-contracts.ts | 97 +++ .../durable-objects/project-data/comments.ts | 698 ++++++++++++++++++ .../src/durable-objects/project-data/index.ts | 63 +- .../src/durable-objects/project-data/types.ts | 7 + apps/api/src/env.ts | 7 + apps/api/src/routes/chat-comments.ts | 259 +++++++ apps/api/src/routes/chat.ts | 3 + apps/api/src/schemas/comments.ts | 24 + apps/api/src/schemas/index.ts | 7 + apps/api/src/services/project-data.ts | 63 ++ .../unit/durable-objects/comments.test.ts | 394 ++++++++++ .../unit/durable-objects/migrations.test.ts | 3 +- .../project-data-comment-broadcast.test.ts | 192 +++++ .../tests/unit/routes/chat-comments.test.ts | 311 ++++++++ .../unit/routes/chat-prompt-cancel.test.ts | 1 + .../routes/chat-session-agent-routing.test.ts | 1 + .../services/project-data-comments.test.ts | 133 ++++ apps/api/wrangler.toml | 7 + .../docs/docs/reference/configuration.md | 7 + packages/shared/src/constants/defaults.ts | 25 + packages/shared/src/constants/index.ts | 7 + packages/shared/src/types/comments.ts | 81 ++ packages/shared/src/types/index.ts | 21 +- ...-21-message-anchored-commenting-backend.md | 96 +++ 28 files changed, 2607 insertions(+), 7 deletions(-) create mode 100644 apps/api/src/durable-objects/project-data/comment-contracts.ts create mode 100644 apps/api/src/durable-objects/project-data/comments.ts create mode 100644 apps/api/src/routes/chat-comments.ts create mode 100644 apps/api/src/schemas/comments.ts create mode 100644 apps/api/tests/unit/durable-objects/comments.test.ts create mode 100644 apps/api/tests/unit/durable-objects/project-data-comment-broadcast.test.ts create mode 100644 apps/api/tests/unit/routes/chat-comments.test.ts create mode 100644 apps/api/tests/unit/services/project-data-comments.test.ts create mode 100644 packages/shared/src/types/comments.ts create mode 100644 tasks/active/2026-08-21-message-anchored-commenting-backend.md diff --git a/.agents/skills/api-reference/SKILL.md b/.agents/skills/api-reference/SKILL.md index 443f921f8..d982cfdb0 100644 --- a/.agents/skills/api-reference/SKILL.md +++ b/.agents/skills/api-reference/SKILL.md @@ -16,6 +16,7 @@ The reference covers: - MCP orchestration (`wait_for_subtasks`, `dispatch_task`, task inspection) - MCP private incident backlog tools (`list_incident_queue`, `get_incident`, `claim_incident`, `resolve_incident`) - Agent Sessions (`/api/workspaces/:id/agent-sessions/*`) +- Message-anchored chat comments (`/api/projects/:projectId/sessions/:sessionId/comments*`) - Agent Settings (`/api/agent-settings/*`) - Notifications (`/api/notifications/*`) - Automation triggers (`/api/projects/:projectId/triggers/*`, `/api/webhooks/ingest`) diff --git a/.claude/skills/api-reference/SKILL.md b/.claude/skills/api-reference/SKILL.md index 40829f807..ef9c60bd8 100644 --- a/.claude/skills/api-reference/SKILL.md +++ b/.claude/skills/api-reference/SKILL.md @@ -43,11 +43,19 @@ user-invocable: false - `GET /api/projects/:projectId/sessions/:sessionId/state` — Get lightweight ACP activity state for a chat session - `GET /api/projects/:projectId/sessions/:sessionId/messages` — List persisted session messages (supports `roles`, `before`, `limit`, `compact`, `order=asc|desc`) - `GET /api/projects/:projectId/sessions/:sessionId/messages/:messageId/tool-content` — Lazy-load stored tool content for compact messages +- `GET /api/projects/:projectId/sessions/:sessionId/comments` — List message-anchored comment threads (supports `messageId`, `status=open|sent|resolved`, `afterSequence`, `limit`) +- `POST /api/projects/:projectId/sessions/:sessionId/comments` — Create a message-anchored comment thread (`{ messageId, body, quote?, clientMutationId? }`) +- `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/replies` — Append a comment reply (`{ body, clientMutationId? }`) +- `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/send` — Mark a thread `sent` (`{ clientMutationId? }`) +- `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/resolve` — Mark a thread `resolved` (`{ clientMutationId? }`) +- `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/reopen` — Reopen a thread to `open` (`{ clientMutationId? }`) - `POST /api/projects/:projectId/sessions/:sessionId/prompt` — Send a follow-up prompt to the active agent session - `POST /api/projects/:projectId/sessions/:sessionId/attention/:markerId/resolve` — Validate, forward, and record one structured human-input answer (`{ answer }`) - `POST /api/projects/:projectId/sessions/:sessionId/summarize` — Generate a session summary for conversation forking - `POST /api/projects/:projectId/sessions/:sessionId/stop` — Stop a chat session +Comment threads are scoped to the ProjectData Durable Object addressed by `projectId`; route authorization requires project `task:read` for list and `task:write` for mutations, and the DO rejects missing sessions, missing messages, and cross-session message anchors. Mutations return `{ thread, idempotent }` or `{ thread, reply, idempotent }`; successful first writes use HTTP 201 for create/reply and 200 for status transitions. Project session WebSocket listeners receive `{ type: "comment.thread.changed", payload: { sessionId, thread, reason } }` with `reason` in `thread_created | reply_created | marked_sent | resolved | reopened`. + ## Task Management (Project Scoped) - `POST /api/projects/:projectId/tasks` — Create task diff --git a/apps/api/.env.example b/apps/api/.env.example index 875f69833..c9e57972a 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -751,6 +751,13 @@ INFOMANIAK_IP_POLL_INTERVAL_MS=3000 # Project Data Durable Object limits # MAX_SESSIONS_PER_PROJECT=10000 # MAX_MESSAGES_PER_SESSION=100000 +# COMMENT_BODY_MAX_LENGTH=8000 # Max characters per message-anchored comment or reply body +# COMMENT_QUOTE_MAX_LENGTH=2000 # Max characters preserved from quoted message text +# COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH=200 # Max clientMutationId length for comment writes +# COMMENT_LIST_LIMIT_DEFAULT=100 # Default page size for comment thread lists +# COMMENT_LIST_LIMIT_MAX=500 # Max page size for comment thread lists +# COMMENT_THREADS_PER_SESSION_MAX=1000 # Max comment threads per chat session +# COMMENT_REPLIES_PER_THREAD_MAX=200 # Max replies per comment thread # DOCUMENT_CARD_RAW_OUTPUT_MAX_BYTES=16384 # MESSAGE_SIZE_THRESHOLD=102400 # ACTIVITY_RETENTION_DAYS=90 diff --git a/apps/api/src/durable-objects/migrations.ts b/apps/api/src/durable-objects/migrations.ts index f928596dc..962ce177b 100644 --- a/apps/api/src/durable-objects/migrations.ts +++ b/apps/api/src/durable-objects/migrations.ts @@ -964,6 +964,97 @@ export const MIGRATIONS: Migration[] = [ `); }, }, + { + name: '032-message-comment-threads', + run: (sql) => { + sql.exec(` + CREATE TABLE comment_threads ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL REFERENCES chat_sessions(id) ON DELETE CASCADE, + anchor_kind TEXT NOT NULL DEFAULT 'message' CHECK (anchor_kind = 'message'), + message_id TEXT NOT NULL REFERENCES chat_messages(id) ON DELETE CASCADE, + quote TEXT, + body TEXT NOT NULL, + author_type TEXT NOT NULL CHECK (author_type IN ('human', 'agent')), + author_id TEXT NOT NULL, + author_name TEXT, + status TEXT NOT NULL DEFAULT 'open' CHECK (status IN ('open', 'sent', 'resolved')), + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + sequence INTEGER NOT NULL, + version INTEGER NOT NULL DEFAULT 1, + client_mutation_id TEXT, + client_mutation_fingerprint TEXT, + sent_at INTEGER, + sent_by_type TEXT CHECK (sent_by_type IS NULL OR sent_by_type IN ('human', 'agent')), + sent_by_id TEXT, + sent_by_name TEXT, + resolved_at INTEGER, + resolved_by_type TEXT CHECK (resolved_by_type IS NULL OR resolved_by_type IN ('human', 'agent')), + resolved_by_id TEXT, + resolved_by_name TEXT, + reopened_at INTEGER, + reopened_by_type TEXT CHECK (reopened_by_type IS NULL OR reopened_by_type IN ('human', 'agent')), + reopened_by_id TEXT, + reopened_by_name TEXT, + UNIQUE(session_id, client_mutation_id) + ) + `); + sql.exec(` + CREATE INDEX idx_comment_threads_session_sequence + ON comment_threads(session_id, sequence) + `); + sql.exec(` + CREATE INDEX idx_comment_threads_message + ON comment_threads(session_id, message_id, sequence) + `); + sql.exec(` + CREATE INDEX idx_comment_threads_status + ON comment_threads(session_id, status, sequence) + `); + + sql.exec(` + CREATE TABLE comment_replies ( + id TEXT PRIMARY KEY, + thread_id TEXT NOT NULL REFERENCES comment_threads(id) ON DELETE CASCADE, + session_id TEXT NOT NULL REFERENCES chat_sessions(id) ON DELETE CASCADE, + body TEXT NOT NULL, + author_type TEXT NOT NULL CHECK (author_type IN ('human', 'agent')), + author_id TEXT NOT NULL, + author_name TEXT, + created_at INTEGER NOT NULL, + sequence INTEGER NOT NULL, + client_mutation_id TEXT, + client_mutation_fingerprint TEXT, + UNIQUE(thread_id, client_mutation_id) + ) + `); + sql.exec(` + CREATE INDEX idx_comment_replies_thread_sequence + ON comment_replies(thread_id, sequence) + `); + sql.exec(` + CREATE INDEX idx_comment_replies_session + ON comment_replies(session_id, thread_id) + `); + + sql.exec(` + CREATE TABLE comment_status_mutations ( + thread_id TEXT NOT NULL REFERENCES comment_threads(id) ON DELETE CASCADE, + session_id TEXT NOT NULL REFERENCES chat_sessions(id) ON DELETE CASCADE, + client_mutation_id TEXT NOT NULL, + target_status TEXT NOT NULL CHECK (target_status IN ('open', 'sent', 'resolved')), + thread_version INTEGER NOT NULL, + created_at INTEGER NOT NULL, + PRIMARY KEY (thread_id, client_mutation_id) + ) + `); + sql.exec(` + CREATE INDEX idx_comment_status_mutations_session + ON comment_status_mutations(session_id, created_at) + `); + }, + }, ]; /** diff --git a/apps/api/src/durable-objects/project-data/comment-contracts.ts b/apps/api/src/durable-objects/project-data/comment-contracts.ts new file mode 100644 index 000000000..ea073acda --- /dev/null +++ b/apps/api/src/durable-objects/project-data/comment-contracts.ts @@ -0,0 +1,97 @@ +import type { + CommentAuthor, + CommentStatus, + MessageCommentReply, + MessageCommentThread, +} from '@simple-agent-manager/shared'; + +export type CommentActor = CommentAuthor; + +export type CreateCommentThreadInput = { + sessionId: string; + messageId: string; + body: string; + quote?: string | null; + clientMutationId?: string | null; + actor: CommentActor; +}; + +export type CreateCommentReplyInput = { + sessionId: string; + threadId: string; + body: string; + clientMutationId?: string | null; + actor: CommentActor; +}; + +export type ListCommentThreadsInput = { + sessionId: string; + messageId?: string | null; + status?: CommentStatus | null; + afterSequence?: number | null; + limit?: number | null; +}; + +export type UpdateCommentStatusInput = { + sessionId: string; + threadId: string; + status: CommentStatus; + clientMutationId?: string | null; + actor: CommentActor; +}; + +export type CommentThreadMutationResult = { + thread: MessageCommentThread; + idempotent: boolean; + changed: boolean; +}; + +export type CommentReplyMutationResult = CommentThreadMutationResult & { + reply: MessageCommentReply; +}; + +export type ListCommentThreadsResult = { + threads: MessageCommentThread[]; + hasMore: boolean; +}; + +export const COMMENT_NOT_FOUND = 'COMMENT_NOT_FOUND'; +export const COMMENT_VALIDATION = 'COMMENT_VALIDATION'; +export const COMMENT_IDEMPOTENCY_CONFLICT = 'COMMENT_IDEMPOTENCY_CONFLICT'; +export const COMMENT_LIMIT_EXCEEDED = 'COMMENT_LIMIT_EXCEEDED'; + +export class CommentNotFoundError extends Error { + readonly code = COMMENT_NOT_FOUND; + + constructor(readonly resource: 'Chat session' | 'Message' | 'Comment thread') { + super(`${resource} not found`); + this.name = 'CommentNotFoundError'; + } +} + +export class CommentValidationError extends Error { + readonly code = COMMENT_VALIDATION; + + constructor(message: string) { + super(message); + this.name = 'CommentValidationError'; + } +} + +export class CommentIdempotencyConflictError extends Error { + readonly code = COMMENT_IDEMPOTENCY_CONFLICT; + + constructor() { + super('clientMutationId already belongs to a different comment mutation'); + this.name = 'CommentIdempotencyConflictError'; + } +} + +export class CommentLimitExceededError extends Error { + readonly code = COMMENT_LIMIT_EXCEEDED; + + constructor(message: string) { + super(message); + this.name = 'CommentLimitExceededError'; + } +} diff --git a/apps/api/src/durable-objects/project-data/comments.ts b/apps/api/src/durable-objects/project-data/comments.ts new file mode 100644 index 000000000..509ac2265 --- /dev/null +++ b/apps/api/src/durable-objects/project-data/comments.ts @@ -0,0 +1,698 @@ +import type { MessageCommentReply, MessageCommentThread } from '@simple-agent-manager/shared'; +import { + COMMENT_STATUSES, + DEFAULT_COMMENT_BODY_MAX_LENGTH, + DEFAULT_COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH, + DEFAULT_COMMENT_LIST_LIMIT_DEFAULT, + DEFAULT_COMMENT_LIST_LIMIT_MAX, + DEFAULT_COMMENT_QUOTE_MAX_LENGTH, + DEFAULT_COMMENT_REPLIES_PER_THREAD_MAX, + DEFAULT_COMMENT_THREADS_PER_SESSION_MAX, +} from '@simple-agent-manager/shared'; +import * as v from 'valibot'; + +import { createModuleLogger } from '../../lib/logger'; +import { + COMMENT_IDEMPOTENCY_CONFLICT, + COMMENT_LIMIT_EXCEEDED, + COMMENT_NOT_FOUND, + COMMENT_VALIDATION, + type CommentActor, + CommentIdempotencyConflictError, + CommentLimitExceededError, + CommentNotFoundError, + type CommentReplyMutationResult, + type CommentThreadMutationResult, + CommentValidationError, + type CreateCommentReplyInput, + type CreateCommentThreadInput, + type ListCommentThreadsInput, + type ListCommentThreadsResult, + type UpdateCommentStatusInput, +} from './comment-contracts'; +import { parseRow } from './row-schemas'; +import type { Env } from './types'; +import { generateId } from './types'; + +const log = createModuleLogger('project_data.comments'); + +export { + COMMENT_IDEMPOTENCY_CONFLICT, + COMMENT_LIMIT_EXCEEDED, + COMMENT_NOT_FOUND, + COMMENT_VALIDATION, + CommentIdempotencyConflictError, + CommentLimitExceededError, + CommentNotFoundError, + CommentValidationError, +}; +export type { + CommentActor, + CommentReplyMutationResult, + CommentThreadMutationResult, + CreateCommentReplyInput, + CreateCommentThreadInput, + ListCommentThreadsInput, + ListCommentThreadsResult, + UpdateCommentStatusInput, +}; + +type CommentLimits = { + bodyMaxLength: number; + quoteMaxLength: number; + idempotencyKeyMaxLength: number; + listDefaultLimit: number; + listMaxLimit: number; + threadsPerSessionMax: number; + repliesPerThreadMax: number; +}; + +const ThreadRowSchema = v.object({ + id: v.string(), + session_id: v.string(), + message_id: v.string(), + quote: v.nullable(v.string()), + body: v.string(), + author_type: v.picklist(['human', 'agent']), + author_id: v.string(), + author_name: v.nullable(v.string()), + status: v.picklist(COMMENT_STATUSES), + created_at: v.number(), + updated_at: v.number(), + sequence: v.number(), + version: v.number(), + client_mutation_id: v.nullable(v.string()), + sent_at: v.nullable(v.number()), + sent_by_type: v.nullable(v.picklist(['human', 'agent'])), + sent_by_id: v.nullable(v.string()), + sent_by_name: v.nullable(v.string()), + resolved_at: v.nullable(v.number()), + resolved_by_type: v.nullable(v.picklist(['human', 'agent'])), + resolved_by_id: v.nullable(v.string()), + resolved_by_name: v.nullable(v.string()), + reopened_at: v.nullable(v.number()), + reopened_by_type: v.nullable(v.picklist(['human', 'agent'])), + reopened_by_id: v.nullable(v.string()), + reopened_by_name: v.nullable(v.string()), +}); + +const ReplyRowSchema = v.object({ + id: v.string(), + thread_id: v.string(), + session_id: v.string(), + body: v.string(), + author_type: v.picklist(['human', 'agent']), + author_id: v.string(), + author_name: v.nullable(v.string()), + created_at: v.number(), + sequence: v.number(), + client_mutation_id: v.nullable(v.string()), +}); + +type ThreadRow = v.InferOutput; + +function positiveInteger(value: string | undefined, fallback: number): number { + const parsed = Number.parseInt(value || '', 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; +} + +export function resolveCommentLimits(env: Env): CommentLimits { + const listDefaultLimit = positiveInteger( + env.COMMENT_LIST_LIMIT_DEFAULT, + DEFAULT_COMMENT_LIST_LIMIT_DEFAULT + ); + const listMaxLimit = positiveInteger(env.COMMENT_LIST_LIMIT_MAX, DEFAULT_COMMENT_LIST_LIMIT_MAX); + return { + bodyMaxLength: positiveInteger(env.COMMENT_BODY_MAX_LENGTH, DEFAULT_COMMENT_BODY_MAX_LENGTH), + quoteMaxLength: positiveInteger(env.COMMENT_QUOTE_MAX_LENGTH, DEFAULT_COMMENT_QUOTE_MAX_LENGTH), + idempotencyKeyMaxLength: positiveInteger( + env.COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH, + DEFAULT_COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH + ), + listDefaultLimit, + listMaxLimit: Math.max(listDefaultLimit, listMaxLimit), + threadsPerSessionMax: positiveInteger( + env.COMMENT_THREADS_PER_SESSION_MAX, + DEFAULT_COMMENT_THREADS_PER_SESSION_MAX + ), + repliesPerThreadMax: positiveInteger( + env.COMMENT_REPLIES_PER_THREAD_MAX, + DEFAULT_COMMENT_REPLIES_PER_THREAD_MAX + ), + }; +} + +export function resolveCommentListLimit(env: Env, requested?: number | null): number { + const limits = resolveCommentLimits(env); + const limit = + typeof requested === 'number' && Number.isFinite(requested) && requested > 0 + ? Math.floor(requested) + : limits.listDefaultLimit; + return Math.min(limit, limits.listMaxLimit); +} + +function normalizeBody(body: string, limits: CommentLimits): string { + const normalized = body.trim(); + if (!normalized) throw new CommentValidationError('body is required'); + if (normalized.length > limits.bodyMaxLength) { + throw new CommentValidationError(`body must be ${limits.bodyMaxLength} characters or fewer`); + } + return normalized; +} + +function normalizeQuote(quote: string | null | undefined, limits: CommentLimits): string | null { + if (quote === null || quote === undefined) return null; + const normalized = quote.trim(); + if (!normalized) return null; + if (normalized.length > limits.quoteMaxLength) { + throw new CommentValidationError(`quote must be ${limits.quoteMaxLength} characters or fewer`); + } + return normalized; +} + +function normalizeClientMutationId( + clientMutationId: string | null | undefined, + limits: CommentLimits +): string | null { + if (clientMutationId === null || clientMutationId === undefined) return null; + const normalized = clientMutationId.trim(); + if (!normalized) return null; + if (normalized.length > limits.idempotencyKeyMaxLength) { + throw new CommentValidationError( + `clientMutationId must be ${limits.idempotencyKeyMaxLength} characters or fewer` + ); + } + return normalized; +} + +function normalizeActor(actor: CommentActor): CommentActor { + if (actor.kind !== 'human' && actor.kind !== 'agent') { + throw new CommentValidationError('actor kind must be human or agent'); + } + const id = actor.id.trim(); + if (!id) throw new CommentValidationError('actor id is required'); + const name = actor.name?.trim() || null; + return { kind: actor.kind, id, name }; +} + +function fingerprint(value: unknown): string { + return JSON.stringify(value); +} + +function ensureSession(sql: SqlStorage, sessionId: string): void { + const row = sql.exec('SELECT id FROM chat_sessions WHERE id = ? LIMIT 1', sessionId).toArray()[0]; + if (!row) throw new CommentNotFoundError('Chat session'); +} + +function ensureMessageAnchor(sql: SqlStorage, sessionId: string, messageId: string): void { + const row = sql + .exec( + 'SELECT id FROM chat_messages WHERE id = ? AND session_id = ? LIMIT 1', + messageId, + sessionId + ) + .toArray()[0]; + if (!row) throw new CommentNotFoundError('Message'); +} + +function nextThreadSequence(sql: SqlStorage, sessionId: string): number { + const row = sql + .exec( + 'SELECT COALESCE(MAX(sequence), 0) AS max_sequence FROM comment_threads WHERE session_id = ?', + sessionId + ) + .toArray()[0]; + return (typeof row?.max_sequence === 'number' ? row.max_sequence : 0) + 1; +} + +function nextReplySequence(sql: SqlStorage, threadId: string): number { + const row = sql + .exec( + 'SELECT COALESCE(MAX(sequence), 0) AS max_sequence FROM comment_replies WHERE thread_id = ?', + threadId + ) + .toArray()[0]; + return (typeof row?.max_sequence === 'number' ? row.max_sequence : 0) + 1; +} + +function actorFromColumns( + kind: 'human' | 'agent' | null, + id: string | null, + name: string | null +): CommentActor | null { + if (!kind || !id) return null; + return { kind, id, name }; +} + +function mapReply(row: unknown): MessageCommentReply { + const r = parseRow(ReplyRowSchema, row, 'comment_reply'); + return { + id: r.id, + threadId: r.thread_id, + sessionId: r.session_id, + author: { kind: r.author_type, id: r.author_id, name: r.author_name }, + body: r.body, + createdAt: r.created_at, + sequence: r.sequence, + clientMutationId: r.client_mutation_id, + }; +} + +function mapThread(row: unknown, replies: MessageCommentReply[]): MessageCommentThread { + const r = parseRow(ThreadRowSchema, row, 'comment_thread'); + return { + id: r.id, + sessionId: r.session_id, + anchor: { + kind: 'message', + messageId: r.message_id, + quote: r.quote, + }, + author: { kind: r.author_type, id: r.author_id, name: r.author_name }, + body: r.body, + status: r.status, + createdAt: r.created_at, + updatedAt: r.updated_at, + sequence: r.sequence, + version: r.version, + clientMutationId: r.client_mutation_id, + sentAt: r.sent_at, + sentBy: actorFromColumns(r.sent_by_type, r.sent_by_id, r.sent_by_name), + resolvedAt: r.resolved_at, + resolvedBy: actorFromColumns(r.resolved_by_type, r.resolved_by_id, r.resolved_by_name), + reopenedAt: r.reopened_at, + reopenedBy: actorFromColumns(r.reopened_by_type, r.reopened_by_id, r.reopened_by_name), + replies, + }; +} + +function readReplies(sql: SqlStorage, threadIds: string[]): Map { + const byThread = new Map(); + for (const threadId of threadIds) byThread.set(threadId, []); + if (threadIds.length === 0) return byThread; + + const placeholders = threadIds.map(() => '?').join(', '); + const rows = sql + .exec( + `SELECT id, thread_id, session_id, body, author_type, author_id, author_name, + created_at, sequence, client_mutation_id + FROM comment_replies + WHERE thread_id IN (${placeholders}) + ORDER BY thread_id ASC, sequence ASC`, + ...threadIds + ) + .toArray(); + + for (const row of rows) { + try { + const reply = mapReply(row); + byThread.get(reply.threadId)?.push(reply); + } catch (err) { + log.warn('comments.reply_row_skipped', { + rowId: typeof row.id === 'string' ? row.id : null, + threadId: typeof row.thread_id === 'string' ? row.thread_id : null, + error: String(err), + }); + } + } + return byThread; +} + +function readThreadRows(sql: SqlStorage, sessionId: string, threadIds: string[]): ThreadRow[] { + if (threadIds.length === 0) return []; + const placeholders = threadIds.map(() => '?').join(', '); + const rows = sql + .exec( + `SELECT id, session_id, message_id, quote, body, author_type, author_id, author_name, + status, created_at, updated_at, sequence, version, client_mutation_id, + sent_at, sent_by_type, sent_by_id, sent_by_name, + resolved_at, resolved_by_type, resolved_by_id, resolved_by_name, + reopened_at, reopened_by_type, reopened_by_id, reopened_by_name + FROM comment_threads + WHERE session_id = ? AND id IN (${placeholders})`, + sessionId, + ...threadIds + ) + .toArray(); + + const parsed: ThreadRow[] = []; + for (const row of rows) { + try { + parsed.push(parseRow(ThreadRowSchema, row, 'comment_thread')); + } catch (err) { + log.warn('comments.thread_row_skipped', { + rowId: typeof row.id === 'string' ? row.id : null, + sessionId, + error: String(err), + }); + } + } + return parsed; +} + +function hydrateThreads( + sql: SqlStorage, + sessionId: string, + rows: unknown[] +): MessageCommentThread[] { + const parsedRows: ThreadRow[] = []; + for (const row of rows) { + try { + parsedRows.push(parseRow(ThreadRowSchema, row, 'comment_thread')); + } catch (err) { + const record = row && typeof row === 'object' ? (row as Record) : {}; + log.warn('comments.thread_row_skipped', { + rowId: typeof record.id === 'string' ? record.id : null, + sessionId, + error: String(err), + }); + } + } + + const replies = readReplies( + sql, + parsedRows.map((row) => row.id) + ); + return parsedRows.map((row) => mapThread(row, replies.get(row.id) ?? [])); +} + +export function getCommentThread( + sql: SqlStorage, + sessionId: string, + threadId: string +): MessageCommentThread | null { + const rows = readThreadRows(sql, sessionId, [threadId]); + if (rows.length === 0) return null; + const replies = readReplies(sql, [threadId]); + return mapThread(rows[0], replies.get(threadId) ?? []); +} + +export function listCommentThreads( + sql: SqlStorage, + env: Env, + input: ListCommentThreadsInput +): ListCommentThreadsResult { + ensureSession(sql, input.sessionId); + const limit = resolveCommentListLimit(env, input.limit); + const conditions = ['session_id = ?']; + const params: Array = [input.sessionId]; + + if (input.messageId) { + ensureMessageAnchor(sql, input.sessionId, input.messageId); + conditions.push('message_id = ?'); + params.push(input.messageId); + } + if (input.status) { + conditions.push('status = ?'); + params.push(input.status); + } + if (input.afterSequence !== null && input.afterSequence !== undefined) { + conditions.push('sequence > ?'); + params.push(input.afterSequence); + } + + const whereClause = conditions.join(' AND '); + const rows = sql + .exec( + `SELECT id, session_id, message_id, quote, body, author_type, author_id, author_name, + status, created_at, updated_at, sequence, version, client_mutation_id, + sent_at, sent_by_type, sent_by_id, sent_by_name, + resolved_at, resolved_by_type, resolved_by_id, resolved_by_name, + reopened_at, reopened_by_type, reopened_by_id, reopened_by_name + FROM comment_threads + WHERE ${whereClause} + ORDER BY sequence ASC + LIMIT ?`, + ...params, + limit + 1 + ) + .toArray(); + + const hasMore = rows.length > limit; + return { + threads: hydrateThreads(sql, input.sessionId, hasMore ? rows.slice(0, limit) : rows), + hasMore, + }; +} + +export function createCommentThread( + sql: SqlStorage, + env: Env, + input: CreateCommentThreadInput +): CommentThreadMutationResult { + const limits = resolveCommentLimits(env); + const actor = normalizeActor(input.actor); + const body = normalizeBody(input.body, limits); + const quote = normalizeQuote(input.quote, limits); + const clientMutationId = normalizeClientMutationId(input.clientMutationId, limits); + const requestFingerprint = fingerprint([ + 'thread', + input.messageId, + body, + quote, + actor.kind, + actor.id, + ]); + + ensureSession(sql, input.sessionId); + ensureMessageAnchor(sql, input.sessionId, input.messageId); + + if (clientMutationId) { + const existing = sql + .exec( + `SELECT id, client_mutation_fingerprint + FROM comment_threads + WHERE session_id = ? AND client_mutation_id = ? + LIMIT 1`, + input.sessionId, + clientMutationId + ) + .toArray()[0]; + if (existing) { + if (existing.client_mutation_fingerprint !== requestFingerprint) { + throw new CommentIdempotencyConflictError(); + } + const thread = getCommentThread(sql, input.sessionId, String(existing.id)); + if (!thread) throw new CommentNotFoundError('Comment thread'); + return { thread, idempotent: true, changed: false }; + } + } + + const countRow = sql + .exec('SELECT COUNT(*) AS count FROM comment_threads WHERE session_id = ?', input.sessionId) + .toArray()[0]; + if ((typeof countRow?.count === 'number' ? countRow.count : 0) >= limits.threadsPerSessionMax) { + throw new CommentLimitExceededError( + `session comment thread limit of ${limits.threadsPerSessionMax} reached` + ); + } + + const id = generateId(); + const now = Date.now(); + const sequence = nextThreadSequence(sql, input.sessionId); + sql.exec( + `INSERT INTO comment_threads + (id, session_id, anchor_kind, message_id, quote, body, author_type, author_id, author_name, + status, created_at, updated_at, sequence, version, client_mutation_id, + client_mutation_fingerprint) + VALUES (?, ?, 'message', ?, ?, ?, ?, ?, ?, 'open', ?, ?, ?, 1, ?, ?)`, + id, + input.sessionId, + input.messageId, + quote, + body, + actor.kind, + actor.id, + actor.name, + now, + now, + sequence, + clientMutationId, + clientMutationId ? requestFingerprint : null + ); + + const thread = getCommentThread(sql, input.sessionId, id); + if (!thread) throw new CommentNotFoundError('Comment thread'); + return { thread, idempotent: false, changed: true }; +} + +export function createCommentReply( + sql: SqlStorage, + env: Env, + input: CreateCommentReplyInput +): CommentReplyMutationResult { + const limits = resolveCommentLimits(env); + const actor = normalizeActor(input.actor); + const body = normalizeBody(input.body, limits); + const clientMutationId = normalizeClientMutationId(input.clientMutationId, limits); + const requestFingerprint = fingerprint(['reply', input.threadId, body, actor.kind, actor.id]); + + const thread = getCommentThread(sql, input.sessionId, input.threadId); + if (!thread) throw new CommentNotFoundError('Comment thread'); + + if (clientMutationId) { + const existing = sql + .exec( + `SELECT id, client_mutation_fingerprint + FROM comment_replies + WHERE thread_id = ? AND client_mutation_id = ? + LIMIT 1`, + input.threadId, + clientMutationId + ) + .toArray()[0]; + if (existing) { + if (existing.client_mutation_fingerprint !== requestFingerprint) { + throw new CommentIdempotencyConflictError(); + } + const authoritative = getCommentThread(sql, input.sessionId, input.threadId); + const reply = authoritative?.replies.find((candidate) => candidate.id === existing.id); + if (!authoritative || !reply) throw new CommentNotFoundError('Comment thread'); + return { thread: authoritative, reply, idempotent: true, changed: false }; + } + } + + const countRow = sql + .exec('SELECT COUNT(*) AS count FROM comment_replies WHERE thread_id = ?', input.threadId) + .toArray()[0]; + if ((typeof countRow?.count === 'number' ? countRow.count : 0) >= limits.repliesPerThreadMax) { + throw new CommentLimitExceededError( + `comment reply limit of ${limits.repliesPerThreadMax} reached` + ); + } + + const id = generateId(); + const now = Date.now(); + const sequence = nextReplySequence(sql, input.threadId); + sql.exec( + `INSERT INTO comment_replies + (id, thread_id, session_id, body, author_type, author_id, author_name, + created_at, sequence, client_mutation_id, client_mutation_fingerprint) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + id, + input.threadId, + input.sessionId, + body, + actor.kind, + actor.id, + actor.name, + now, + sequence, + clientMutationId, + clientMutationId ? requestFingerprint : null + ); + sql.exec( + `UPDATE comment_threads + SET updated_at = ?, version = version + 1 + WHERE id = ? AND session_id = ?`, + now, + input.threadId, + input.sessionId + ); + + const authoritative = getCommentThread(sql, input.sessionId, input.threadId); + const reply = authoritative?.replies.find((candidate) => candidate.id === id); + if (!authoritative || !reply) throw new CommentNotFoundError('Comment thread'); + return { thread: authoritative, reply, idempotent: false, changed: true }; +} + +export function updateCommentThreadStatus( + sql: SqlStorage, + env: Env, + input: UpdateCommentStatusInput +): CommentThreadMutationResult { + const limits = resolveCommentLimits(env); + const actor = normalizeActor(input.actor); + const clientMutationId = normalizeClientMutationId(input.clientMutationId, limits); + if (!COMMENT_STATUSES.includes(input.status)) { + throw new CommentValidationError('status must be open, sent, or resolved'); + } + + const current = getCommentThread(sql, input.sessionId, input.threadId); + if (!current) throw new CommentNotFoundError('Comment thread'); + + if (clientMutationId) { + const existing = sql + .exec( + `SELECT target_status + FROM comment_status_mutations + WHERE thread_id = ? AND client_mutation_id = ? + LIMIT 1`, + input.threadId, + clientMutationId + ) + .toArray()[0]; + if (existing) { + if (existing.target_status !== input.status) throw new CommentIdempotencyConflictError(); + return { thread: current, idempotent: true, changed: false }; + } + } + + const now = Date.now(); + let changed = false; + if (current.status !== input.status) { + changed = true; + if (input.status === 'sent') { + sql.exec( + `UPDATE comment_threads + SET status = 'sent', sent_at = ?, sent_by_type = ?, sent_by_id = ?, sent_by_name = ?, + updated_at = ?, version = version + 1 + WHERE id = ? AND session_id = ?`, + now, + actor.kind, + actor.id, + actor.name, + now, + input.threadId, + input.sessionId + ); + } else if (input.status === 'resolved') { + sql.exec( + `UPDATE comment_threads + SET status = 'resolved', resolved_at = ?, resolved_by_type = ?, resolved_by_id = ?, + resolved_by_name = ?, updated_at = ?, version = version + 1 + WHERE id = ? AND session_id = ?`, + now, + actor.kind, + actor.id, + actor.name, + now, + input.threadId, + input.sessionId + ); + } else { + sql.exec( + `UPDATE comment_threads + SET status = 'open', reopened_at = ?, reopened_by_type = ?, reopened_by_id = ?, + reopened_by_name = ?, updated_at = ?, version = version + 1 + WHERE id = ? AND session_id = ?`, + now, + actor.kind, + actor.id, + actor.name, + now, + input.threadId, + input.sessionId + ); + } + } + + const authoritative = getCommentThread(sql, input.sessionId, input.threadId); + if (!authoritative) throw new CommentNotFoundError('Comment thread'); + + if (clientMutationId) { + sql.exec( + `INSERT INTO comment_status_mutations + (thread_id, session_id, client_mutation_id, target_status, thread_version, created_at) + VALUES (?, ?, ?, ?, ?, ?)`, + input.threadId, + input.sessionId, + clientMutationId, + input.status, + authoritative.version, + now + ); + } + + return { thread: authoritative, idempotent: false, changed }; +} diff --git a/apps/api/src/durable-objects/project-data/index.ts b/apps/api/src/durable-objects/project-data/index.ts index f98776dee..22ceb009e 100644 --- a/apps/api/src/durable-objects/project-data/index.ts +++ b/apps/api/src/durable-objects/project-data/index.ts @@ -14,6 +14,8 @@ import { type CheckpointEpisodeTransitionInput, type CreateCheckpointEpisodeInput, MAILBOX_DEFAULTS, + type MessageCommentThread, + type MessageCommentThreadEventReason, type SessionActivityTerminalReason, } from '@simple-agent-manager/shared'; import { DurableObject } from 'cloudflare:workers'; @@ -28,6 +30,7 @@ import { computeProjectDataAlarmTime } from './alarm-schedule'; import * as attention from './attention'; import * as attentionExpiry from './attention-expiry'; import * as commands from './commands'; +import * as comments from './comments'; import { stopTimedOutConversationWorkspaces } from './conversation-timeout'; import * as durability from './durability-foundation'; import * as ideas from './ideas'; @@ -244,8 +247,7 @@ export class ProjectData extends DurableObject { ): Promise { sessionWakeProgress.publishSessionWakeProgress( { - broadcastEvent: (type, payload, sessionId) => - this.broadcastEvent(type, payload, sessionId), + broadcastEvent: (type, payload, sessionId) => this.broadcastEvent(type, payload, sessionId), }, input, Date.now() @@ -481,6 +483,40 @@ export class ProjectData extends DurableObject { return messages.searchMessages(this.sql, query, sessionId, roles, limit); } + listCommentThreads(input: comments.ListCommentThreadsInput): comments.ListCommentThreadsResult { + return comments.listCommentThreads(this.sql, this.env, input); + } + + createCommentThread(input: comments.CreateCommentThreadInput) { + const result = this.ctx.storage.transactionSync(() => + comments.createCommentThread(this.sql, this.env, input) + ); + if (result.changed) this.broadcastCommentThread(result.thread, 'thread_created'); + return { thread: result.thread, idempotent: result.idempotent }; + } + + createCommentReply(input: comments.CreateCommentReplyInput) { + const result = this.ctx.storage.transactionSync(() => + comments.createCommentReply(this.sql, this.env, input) + ); + if (result.changed) this.broadcastCommentThread(result.thread, 'reply_created'); + return { + thread: result.thread, + reply: result.reply, + idempotent: result.idempotent, + }; + } + + updateCommentThreadStatus(input: comments.UpdateCommentStatusInput) { + const result = this.ctx.storage.transactionSync(() => + comments.updateCommentThreadStatus(this.sql, this.env, input) + ); + if (result.changed) { + this.broadcastCommentThread(result.thread, this.commentStatusEventReason(input.status)); + } + return { thread: result.thread, idempotent: result.idempotent }; + } + materializeSession(sessionId: string): void { materialization.materializeSession(this.sql, sessionId); } @@ -1598,6 +1634,29 @@ export class ProjectData extends DurableObject { } } + private broadcastCommentThread( + thread: MessageCommentThread, + reason: MessageCommentThreadEventReason + ): void { + this.broadcastEvent( + 'comment.thread.changed', + { + sessionId: thread.sessionId, + thread, + reason, + }, + thread.sessionId + ); + } + + private commentStatusEventReason( + status: comments.UpdateCommentStatusInput['status'] + ): MessageCommentThreadEventReason { + if (status === 'sent') return 'marked_sent'; + if (status === 'resolved') return 'resolved'; + return 'reopened'; + } + private scheduleSummarySync(): void { const debounceMs = parseInt(this.env.DO_SUMMARY_SYNC_DEBOUNCE_MS || '5000', 10); if (this.summarySyncTimer !== null) clearTimeout(this.summarySyncTimer); diff --git a/apps/api/src/durable-objects/project-data/types.ts b/apps/api/src/durable-objects/project-data/types.ts index 091ece82f..8656baedf 100644 --- a/apps/api/src/durable-objects/project-data/types.ts +++ b/apps/api/src/durable-objects/project-data/types.ts @@ -19,6 +19,13 @@ export type Env = { SESSION_INDEX_MAX_ROWS?: string; MAX_SESSIONS_PER_PROJECT?: string; MAX_MESSAGES_PER_SESSION?: string; + COMMENT_BODY_MAX_LENGTH?: string; + COMMENT_QUOTE_MAX_LENGTH?: string; + COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH?: string; + COMMENT_LIST_LIMIT_DEFAULT?: string; + COMMENT_LIST_LIMIT_MAX?: string; + COMMENT_THREADS_PER_SESSION_MAX?: string; + COMMENT_REPLIES_PER_THREAD_MAX?: string; DOCUMENT_CARD_RAW_OUTPUT_MAX_BYTES?: string; PROJECT_DATA_TOOL_METADATA_MAX_BYTES?: string; PROJECT_DATA_STORAGE_TELEMETRY_ENABLED?: string; diff --git a/apps/api/src/env.ts b/apps/api/src/env.ts index f148edcbf..94615867e 100644 --- a/apps/api/src/env.ts +++ b/apps/api/src/env.ts @@ -506,6 +506,13 @@ export interface Env extends WebhookTriggerEnv, TaskRecoveryEnv { CACHED_COMMANDS_MAX_DESC_LENGTH?: string; MAX_SESSIONS_PER_PROJECT?: string; MAX_MESSAGES_PER_SESSION?: string; + COMMENT_BODY_MAX_LENGTH?: string; // Max characters per message-anchored comment or reply body (default: 8000) + COMMENT_QUOTE_MAX_LENGTH?: string; // Max characters preserved from quoted message text (default: 2000) + COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH?: string; // Max clientMutationId length for comment writes (default: 200) + COMMENT_LIST_LIMIT_DEFAULT?: string; // Default page size for comment thread lists (default: 100) + COMMENT_LIST_LIMIT_MAX?: string; // Max page size for comment thread lists (default: 500) + COMMENT_THREADS_PER_SESSION_MAX?: string; // Max comment threads per chat session (default: 1000) + COMMENT_REPLIES_PER_THREAD_MAX?: string; // Max replies per comment thread (default: 200) DOCUMENT_CARD_RAW_OUTPUT_MAX_BYTES?: string; // Max document-card rawOutput bytes preserved in compact message metadata (default: 16384) PROJECT_DATA_TOOL_METADATA_MAX_BYTES?: string; PROJECT_DATA_STORAGE_TELEMETRY_ENABLED?: string; diff --git a/apps/api/src/routes/chat-comments.ts b/apps/api/src/routes/chat-comments.ts new file mode 100644 index 000000000..b85d03287 --- /dev/null +++ b/apps/api/src/routes/chat-comments.ts @@ -0,0 +1,259 @@ +import type { CommentStatus } from '@simple-agent-manager/shared'; +import { COMMENT_STATUSES } from '@simple-agent-manager/shared'; +import { drizzle } from 'drizzle-orm/d1'; +import { type Context, Hono } from 'hono'; + +import * as schema from '../db/schema'; +import type { Env } from '../env'; +import { requireRouteParam } from '../lib/route-helpers'; +import { getAuth, getUserId } from '../middleware/auth'; +import { errors } from '../middleware/error'; +import { requireProjectCapability } from '../middleware/project-auth'; +import { jsonValidator } from '../schemas/_validator'; +import { + CommentStatusMutationSchema, + CreateCommentReplySchema, + CreateCommentThreadSchema, +} from '../schemas/comments'; +import * as projectDataService from '../services/project-data'; + +export const chatCommentRoutes = new Hono<{ Bindings: Env }>(); + +function parseCommentStatus(rawStatus?: string): CommentStatus | null { + if (!rawStatus) return null; + const status = rawStatus.trim().toLowerCase(); + if (COMMENT_STATUSES.includes(status as CommentStatus)) return status as CommentStatus; + throw errors.badRequest('status must be open, sent, or resolved'); +} + +function parsePositiveIntegerQuery(name: string, rawValue?: string): number | null { + if (!rawValue) return null; + const parsed = Number.parseInt(rawValue, 10); + if (!Number.isFinite(parsed) || parsed <= 0) { + throw errors.badRequest(`${name} must be a positive integer`); + } + return parsed; +} + +function parseNonNegativeIntegerQuery(name: string, rawValue?: string): number | null { + if (!rawValue) return null; + const parsed = Number.parseInt(rawValue, 10); + if (!Number.isFinite(parsed) || parsed < 0) { + throw errors.badRequest(`${name} must be a non-negative integer`); + } + return parsed; +} + +function getCommentActor(c: Context<{ Bindings: Env }>) { + const auth = getAuth(c); + return { + kind: 'human' as const, + id: auth.user.id, + name: auth.user.name ?? auth.user.email ?? null, + }; +} + +function getCommentErrorName(err: unknown): string | null { + return err instanceof Error ? err.name : null; +} + +function getCommentErrorCode(err: unknown): string | null { + if (!err || typeof err !== 'object') return null; + const code = (err as { code?: unknown }).code; + return typeof code === 'string' ? code : null; +} + +function getCommentNotFoundResource(err: unknown): string { + if (err && typeof err === 'object') { + const resource = (err as { resource?: unknown }).resource; + if (typeof resource === 'string') return resource; + } + const message = err instanceof Error ? err.message : ''; + if (message.startsWith('Message ')) return 'Message'; + if (message.startsWith('Comment thread ')) return 'Comment thread'; + if (message.startsWith('Chat session ')) return 'Chat session'; + return 'Resource'; +} + +function rethrowCommentError(err: unknown): never { + const code = getCommentErrorCode(err); + const name = getCommentErrorName(err); + if ( + err instanceof projectDataService.CommentValidationError || + code === 'COMMENT_VALIDATION' || + name === 'CommentValidationError' + ) { + throw errors.badRequest(err instanceof Error ? err.message : 'Invalid comment request'); + } + if ( + err instanceof projectDataService.CommentNotFoundError || + code === 'COMMENT_NOT_FOUND' || + name === 'CommentNotFoundError' + ) { + throw errors.notFound(getCommentNotFoundResource(err)); + } + if ( + err instanceof projectDataService.CommentIdempotencyConflictError || + code === 'COMMENT_IDEMPOTENCY_CONFLICT' || + name === 'CommentIdempotencyConflictError' + ) { + throw errors.conflict( + err instanceof Error + ? err.message + : 'clientMutationId already belongs to a different comment mutation' + ); + } + if ( + err instanceof projectDataService.CommentLimitExceededError || + code === 'COMMENT_LIMIT_EXCEEDED' || + name === 'CommentLimitExceededError' + ) { + throw errors.unprocessable(err instanceof Error ? err.message : 'Comment limit exceeded'); + } + throw err; +} + +/** + * GET /api/projects/:projectId/sessions/:sessionId/comments + * List message-anchored comment threads for a chat session. + */ +chatCommentRoutes.get('/:sessionId/comments', async (c) => { + const userId = getUserId(c); + const projectId = requireRouteParam(c, 'projectId'); + const sessionId = requireRouteParam(c, 'sessionId'); + const db = drizzle(c.env.DATABASE, { schema }); + + await requireProjectCapability(db, projectId, userId, 'task:read'); + + try { + const result = await projectDataService.listCommentThreads(c.env, projectId, { + sessionId, + messageId: c.req.query('messageId') ?? null, + status: parseCommentStatus(c.req.query('status')), + afterSequence: parseNonNegativeIntegerQuery('afterSequence', c.req.query('afterSequence')), + limit: parsePositiveIntegerQuery('limit', c.req.query('limit')), + }); + return c.json(result); + } catch (err) { + rethrowCommentError(err); + } +}); + +/** + * POST /api/projects/:projectId/sessions/:sessionId/comments + * Create a message-anchored comment thread. + */ +chatCommentRoutes.post( + '/:sessionId/comments', + jsonValidator(CreateCommentThreadSchema), + async (c) => { + const userId = getUserId(c); + const projectId = requireRouteParam(c, 'projectId'); + const sessionId = requireRouteParam(c, 'sessionId'); + const db = drizzle(c.env.DATABASE, { schema }); + + await requireProjectCapability(db, projectId, userId, 'task:write'); + + const body = c.req.valid('json'); + try { + const result = await projectDataService.createCommentThread(c.env, projectId, { + sessionId, + messageId: body.messageId, + body: body.body, + quote: body.quote ?? null, + clientMutationId: body.clientMutationId ?? null, + actor: getCommentActor(c), + }); + return c.json(result, result.idempotent ? 200 : 201); + } catch (err) { + rethrowCommentError(err); + } + } +); + +/** + * POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/replies + * Append a reply to a message-anchored comment thread. + */ +chatCommentRoutes.post( + '/:sessionId/comments/:threadId/replies', + jsonValidator(CreateCommentReplySchema), + async (c) => { + const userId = getUserId(c); + const projectId = requireRouteParam(c, 'projectId'); + const sessionId = requireRouteParam(c, 'sessionId'); + const threadId = requireRouteParam(c, 'threadId'); + const db = drizzle(c.env.DATABASE, { schema }); + + await requireProjectCapability(db, projectId, userId, 'task:write'); + + const body = c.req.valid('json'); + try { + const result = await projectDataService.createCommentReply(c.env, projectId, { + sessionId, + threadId, + body: body.body, + clientMutationId: body.clientMutationId ?? null, + actor: getCommentActor(c), + }); + return c.json(result, result.idempotent ? 200 : 201); + } catch (err) { + rethrowCommentError(err); + } + } +); + +async function updateCommentStatus( + c: Context<{ Bindings: Env }>, + status: CommentStatus, + clientMutationId: string | null +) { + const userId = getUserId(c); + const projectId = requireRouteParam(c, 'projectId'); + const sessionId = requireRouteParam(c, 'sessionId'); + const threadId = requireRouteParam(c, 'threadId'); + const db = drizzle(c.env.DATABASE, { schema }); + + await requireProjectCapability(db, projectId, userId, 'task:write'); + + try { + return c.json( + await projectDataService.updateCommentThreadStatus(c.env, projectId, { + sessionId, + threadId, + status, + clientMutationId, + actor: getCommentActor(c), + }) + ); + } catch (err) { + rethrowCommentError(err); + } +} + +chatCommentRoutes.post( + '/:sessionId/comments/:threadId/send', + jsonValidator(CommentStatusMutationSchema), + (c) => { + const body = c.req.valid('json'); + return updateCommentStatus(c, 'sent', body.clientMutationId ?? null); + } +); + +chatCommentRoutes.post( + '/:sessionId/comments/:threadId/resolve', + jsonValidator(CommentStatusMutationSchema), + (c) => { + const body = c.req.valid('json'); + return updateCommentStatus(c, 'resolved', body.clientMutationId ?? null); + } +); + +chatCommentRoutes.post( + '/:sessionId/comments/:threadId/reopen', + jsonValidator(CommentStatusMutationSchema), + (c) => { + const body = c.req.valid('json'); + return updateCommentStatus(c, 'open', body.clientMutationId ?? null); + } +); diff --git a/apps/api/src/routes/chat.ts b/apps/api/src/routes/chat.ts index 81e0f6e6e..39b63ada8 100644 --- a/apps/api/src/routes/chat.ts +++ b/apps/api/src/routes/chat.ts @@ -41,6 +41,7 @@ import { isTaskStatus } from '../services/task-status'; import { attachWakeState } from './chat/wake-state'; import { resolveChatAgentState } from './chat-agent-state'; import { registerChatCancelRoute } from './chat-cancel'; +import { chatCommentRoutes } from './chat-comments'; import { chatForkRoutes } from './chat-fork'; import { recordChatSessionLoadFailure } from './chat-load-diagnostics'; import { preparePromptForLiveAgent, sendPreparedPromptToLiveAgent } from './chat-prompt-forward'; @@ -417,6 +418,8 @@ chatRoutes.get('/:sessionId/messages/:messageId/tool-content', async (c) => { return c.json({ content }); }); +chatRoutes.route('/', chatCommentRoutes); + registerChatStopRoute(chatRoutes); registerChatCancelRoute(chatRoutes); diff --git a/apps/api/src/schemas/comments.ts b/apps/api/src/schemas/comments.ts new file mode 100644 index 000000000..f2685f82a --- /dev/null +++ b/apps/api/src/schemas/comments.ts @@ -0,0 +1,24 @@ +import * as v from 'valibot'; + +/** + * Message-anchored comment request schemas. + * + * Length limits are environment-backed and enforced inside ProjectData so HTTP, + * RPC, MCP, and future callers share one authoritative boundary. + */ + +export const CreateCommentThreadSchema = v.object({ + messageId: v.pipe(v.string(), v.trim(), v.minLength(1)), + body: v.string(), + quote: v.optional(v.nullable(v.string())), + clientMutationId: v.optional(v.nullable(v.string())), +}); + +export const CreateCommentReplySchema = v.object({ + body: v.string(), + clientMutationId: v.optional(v.nullable(v.string())), +}); + +export const CommentStatusMutationSchema = v.object({ + clientMutationId: v.optional(v.nullable(v.string())), +}); diff --git a/apps/api/src/schemas/index.ts b/apps/api/src/schemas/index.ts index 131cfe521..5a66ae2dc 100644 --- a/apps/api/src/schemas/index.ts +++ b/apps/api/src/schemas/index.ts @@ -114,6 +114,13 @@ export { CreateAcpSessionSchema, } from './acp-sessions'; +// Message-anchored comment schemas +export { + CommentStatusMutationSchema, + CreateCommentReplySchema, + CreateCommentThreadSchema, +} from './comments'; + // Admin schemas export { AdminUserActionSchema, diff --git a/apps/api/src/services/project-data.ts b/apps/api/src/services/project-data.ts index acc485ca9..dd7e7b347 100644 --- a/apps/api/src/services/project-data.ts +++ b/apps/api/src/services/project-data.ts @@ -11,14 +11,31 @@ import type { AgentMailboxMessage, CheckpointEpisode, CheckpointEpisodeTransitionInput, + CommentAuthor, + CommentStatus, CreateCheckpointEpisodeInput, DeliveryState, MessageClass, + MessageCommentListResponse, + MessageCommentMutationResponse, + MessageCommentReplyMutationResponse, SessionActivityTerminalReason, } from '@simple-agent-manager/shared'; import { resolveHandoffLimits, resolveMissionStateLimits } from '@simple-agent-manager/shared'; import type { ProjectData } from '../durable-objects/project-data'; +import type { + CreateCommentReplyInput, + CreateCommentThreadInput, + ListCommentThreadsInput, + UpdateCommentStatusInput, +} from '../durable-objects/project-data/comment-contracts'; +export { + CommentIdempotencyConflictError, + CommentLimitExceededError, + CommentNotFoundError, + CommentValidationError, +} from '../durable-objects/project-data/comment-contracts'; import type { AcceptedPromptDelivery, AcceptPromptDeliveryInput, @@ -392,6 +409,52 @@ export async function searchMessages( return stub.searchMessages(query, sessionId, roles, limit); } +// ========================================================================= +// Message-Anchored Comments +// ========================================================================= + +export type MessageCommentActor = CommentAuthor; + +export async function listCommentThreads( + env: Env, + projectId: string, + input: ListCommentThreadsInput +): Promise { + return callProjectDataWithRetry(env, projectId, 'listCommentThreads', (stub) => + stub.listCommentThreads(input) + ); +} + +export async function createCommentThread( + env: Env, + projectId: string, + input: CreateCommentThreadInput +): Promise { + return callProjectDataNoRetry(env, projectId, 'createCommentThread', (stub) => + stub.createCommentThread(input) + ); +} + +export async function createCommentReply( + env: Env, + projectId: string, + input: CreateCommentReplyInput +): Promise { + return callProjectDataNoRetry(env, projectId, 'createCommentReply', (stub) => + stub.createCommentReply(input) + ); +} + +export async function updateCommentThreadStatus( + env: Env, + projectId: string, + input: UpdateCommentStatusInput & { status: CommentStatus } +): Promise { + return callProjectDataNoRetry(env, projectId, 'updateCommentThreadStatus', (stub) => + stub.updateCommentThreadStatus(input) + ); +} + /** Materialize all stopped sessions that haven't been indexed yet. */ export async function materializeAllStopped( env: Env, diff --git a/apps/api/tests/unit/durable-objects/comments.test.ts b/apps/api/tests/unit/durable-objects/comments.test.ts new file mode 100644 index 000000000..e67a573cb --- /dev/null +++ b/apps/api/tests/unit/durable-objects/comments.test.ts @@ -0,0 +1,394 @@ +import Database from 'better-sqlite3'; +import { beforeEach, describe, expect, it } from 'vitest'; + +import { runMigrations } from '../../../src/durable-objects/migrations'; +import * as comments from '../../../src/durable-objects/project-data/comments'; +import type { Env } from '../../../src/durable-objects/project-data/types'; +import { createSqlStorage } from './sql-storage-test-utils'; + +const HUMAN = { kind: 'human' as const, id: 'user-1', name: 'Ada' }; + +describe('ProjectData message comments', () => { + let db: Database.Database; + let sql: SqlStorage; + let env: Env; + + beforeEach(() => { + db = new Database(':memory:'); + sql = createSqlStorage(db); + env = {} as Env; + runMigrations(sql); + }); + + function seedSession(sessionId: string): void { + sql.exec( + `INSERT INTO chat_sessions (id, topic, started_at) + VALUES (?, ?, ?)`, + sessionId, + `topic ${sessionId}`, + Date.now() + ); + } + + function seedMessage(sessionId: string, messageId: string, sequence: number): void { + sql.exec( + `INSERT INTO chat_messages + (id, session_id, role, content, tool_metadata, created_at, sequence) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + messageId, + sessionId, + 'assistant', + `message ${messageId}`, + null, + Date.now() + sequence, + sequence + ); + } + + it('adds append-only migration tables and indexes', () => { + const tables = sql + .exec( + `SELECT name FROM sqlite_master + WHERE type = 'table' AND name IN ( + 'comment_threads', + 'comment_replies', + 'comment_status_mutations' + ) + ORDER BY name ASC` + ) + .toArray() + .map((row) => row.name); + + expect(tables).toEqual(['comment_replies', 'comment_status_mutations', 'comment_threads']); + + const indexes = sql + .exec( + `SELECT name FROM sqlite_master + WHERE type = 'index' AND name LIKE 'idx_comment_%' + ORDER BY name ASC` + ) + .toArray() + .map((row) => row.name); + + expect(indexes).toEqual( + expect.arrayContaining([ + 'idx_comment_replies_session', + 'idx_comment_replies_thread_sequence', + 'idx_comment_status_mutations_session', + 'idx_comment_threads_message', + 'idx_comment_threads_session_sequence', + 'idx_comment_threads_status', + ]) + ); + }); + + it('creates, lists, and idempotently replays message-anchored threads', () => { + seedSession('session-a'); + seedMessage('session-a', 'message-a', 1); + + const created = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: ' Needs clarification ', + quote: ' selected quote ', + clientMutationId: 'thread-key-1', + actor: HUMAN, + }); + + expect(created.idempotent).toBe(false); + expect(created.changed).toBe(true); + expect(created.thread).toMatchObject({ + sessionId: 'session-a', + anchor: { kind: 'message', messageId: 'message-a', quote: 'selected quote' }, + author: HUMAN, + body: 'Needs clarification', + status: 'open', + sequence: 1, + version: 1, + clientMutationId: 'thread-key-1', + replies: [], + }); + + const replay = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'Needs clarification', + quote: 'selected quote', + clientMutationId: 'thread-key-1', + actor: HUMAN, + }); + expect(replay.idempotent).toBe(true); + expect(replay.changed).toBe(false); + expect(replay.thread.id).toBe(created.thread.id); + + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'Different body', + clientMutationId: 'thread-key-1', + actor: HUMAN, + }) + ).toThrow(comments.CommentIdempotencyConflictError); + + expect(comments.listCommentThreads(sql, env, { sessionId: 'session-a' })).toMatchObject({ + hasMore: false, + threads: [{ id: created.thread.id, sequence: 1 }], + }); + }); + + it('appends replies and status transitions with stable sequence and version semantics', () => { + seedSession('session-a'); + seedMessage('session-a', 'message-a', 1); + const created = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'Thread', + clientMutationId: 'thread-key', + actor: HUMAN, + }); + + const firstReply = comments.createCommentReply(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + body: 'First reply', + clientMutationId: 'reply-key-1', + actor: HUMAN, + }); + const replay = comments.createCommentReply(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + body: 'First reply', + clientMutationId: 'reply-key-1', + actor: HUMAN, + }); + const secondReply = comments.createCommentReply(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + body: 'Second reply', + clientMutationId: 'reply-key-2', + actor: HUMAN, + }); + + expect(firstReply.idempotent).toBe(false); + expect(replay.idempotent).toBe(true); + expect(replay.reply.id).toBe(firstReply.reply.id); + expect(secondReply.thread.replies.map((reply) => reply.sequence)).toEqual([1, 2]); + expect(secondReply.thread.version).toBe(3); + + const sent = comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + status: 'sent', + clientMutationId: 'status-key-1', + actor: HUMAN, + }); + expect(sent.thread.status).toBe('sent'); + expect(sent.thread.sentBy).toEqual(HUMAN); + expect(sent.thread.version).toBe(4); + + const sentReplay = comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + status: 'sent', + clientMutationId: 'status-key-1', + actor: HUMAN, + }); + expect(sentReplay.idempotent).toBe(true); + expect(sentReplay.changed).toBe(false); + + expect(() => + comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + status: 'resolved', + clientMutationId: 'status-key-1', + actor: HUMAN, + }) + ).toThrow(comments.CommentIdempotencyConflictError); + + const resolved = comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + status: 'resolved', + clientMutationId: 'status-key-2', + actor: HUMAN, + }); + const reopened = comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: created.thread.id, + status: 'open', + clientMutationId: 'status-key-3', + actor: HUMAN, + }); + + expect(resolved.thread.status).toBe('resolved'); + expect(reopened.thread.status).toBe('open'); + expect(reopened.thread.reopenedBy).toEqual(HUMAN); + expect(reopened.thread.version).toBe(6); + }); + + it('enforces message ownership, missing resources, validation, and configured limits', () => { + seedSession('session-a'); + seedSession('session-b'); + seedMessage('session-a', 'message-a', 1); + seedMessage('session-b', 'message-b', 1); + + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-missing', + body: 'Thread', + actor: HUMAN, + }) + ).toThrow(comments.CommentNotFoundError); + + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-b', + body: 'Thread', + actor: HUMAN, + }) + ).toThrow(comments.CommentNotFoundError); + expect(() => + comments.listCommentThreads(sql, env, { + sessionId: 'session-a', + messageId: 'message-b', + }) + ).toThrow(comments.CommentNotFoundError); + + env.COMMENT_BODY_MAX_LENGTH = '5'; + env.COMMENT_QUOTE_MAX_LENGTH = '4'; + env.COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH = '3'; + + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: '', + actor: HUMAN, + }) + ).toThrow(comments.CommentValidationError); + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'too long', + actor: HUMAN, + }) + ).toThrow(comments.CommentValidationError); + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'ok', + quote: 'quote', + actor: HUMAN, + }) + ).toThrow(comments.CommentValidationError); + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'ok', + clientMutationId: 'long', + actor: HUMAN, + }) + ).toThrow(comments.CommentValidationError); + + env = { + ...env, + COMMENT_BODY_MAX_LENGTH: '100', + COMMENT_QUOTE_MAX_LENGTH: '100', + COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH: '100', + COMMENT_THREADS_PER_SESSION_MAX: '1', + COMMENT_REPLIES_PER_THREAD_MAX: '1', + }; + const thread = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'Thread', + actor: HUMAN, + }).thread; + expect(() => + comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'Another', + actor: HUMAN, + }) + ).toThrow(comments.CommentLimitExceededError); + + comments.createCommentReply(sql, env, { + sessionId: 'session-a', + threadId: thread.id, + body: 'Reply', + actor: HUMAN, + }); + expect(() => + comments.createCommentReply(sql, env, { + sessionId: 'session-a', + threadId: thread.id, + body: 'Another reply', + actor: HUMAN, + }) + ).toThrow(comments.CommentLimitExceededError); + }); + + it('lists with status, message, afterSequence, and limit filters in sequence order', () => { + seedSession('session-a'); + seedMessage('session-a', 'message-a', 1); + seedMessage('session-a', 'message-b', 2); + + const first = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'First', + actor: HUMAN, + }).thread; + const second = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-a', + body: 'Second', + actor: HUMAN, + }).thread; + const third = comments.createCommentThread(sql, env, { + sessionId: 'session-a', + messageId: 'message-b', + body: 'Third', + actor: HUMAN, + }).thread; + + comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: second.id, + status: 'resolved', + actor: HUMAN, + }); + comments.updateCommentThreadStatus(sql, env, { + sessionId: 'session-a', + threadId: third.id, + status: 'sent', + actor: HUMAN, + }); + + expect( + comments + .listCommentThreads(sql, env, { sessionId: 'session-a', messageId: 'message-a' }) + .threads.map((thread) => thread.id) + ).toEqual([first.id, second.id]); + expect( + comments.listCommentThreads(sql, env, { sessionId: 'session-a', status: 'sent' }).threads + ).toMatchObject([{ id: third.id, status: 'sent' }]); + expect( + comments + .listCommentThreads(sql, env, { sessionId: 'session-a', afterSequence: 1 }) + .threads.map((thread) => thread.id) + ).toEqual([second.id, third.id]); + + const firstPage = comments.listCommentThreads(sql, env, { sessionId: 'session-a', limit: 2 }); + expect(firstPage.hasMore).toBe(true); + expect(firstPage.threads.map((thread) => thread.id)).toEqual([first.id, second.id]); + }); +}); diff --git a/apps/api/tests/unit/durable-objects/migrations.test.ts b/apps/api/tests/unit/durable-objects/migrations.test.ts index 490ad52a6..fa6b166a9 100644 --- a/apps/api/tests/unit/durable-objects/migrations.test.ts +++ b/apps/api/tests/unit/durable-objects/migrations.test.ts @@ -399,7 +399,8 @@ describe('DO Migrations', () => { // durable task waits: 2 (due + child) from migration 030; the // active-parent (030) and idempotency (031) indexes are CREATE UNIQUE // INDEX and are counted separately - expect(indexes.length).toBe(51); + // message-anchored comments: 6 from migration 032 + expect(indexes.length).toBe(57); }); }); }); diff --git a/apps/api/tests/unit/durable-objects/project-data-comment-broadcast.test.ts b/apps/api/tests/unit/durable-objects/project-data-comment-broadcast.test.ts new file mode 100644 index 000000000..f152524ad --- /dev/null +++ b/apps/api/tests/unit/durable-objects/project-data-comment-broadcast.test.ts @@ -0,0 +1,192 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('drizzle-orm', () => ({ + sql: Object.assign((s: unknown) => s, { raw: (s: unknown) => s }), + eq: (a: unknown, b: unknown) => [a, b], + and: (...args: unknown[]) => args, + desc: (col: unknown) => ({ desc: true, col }), +})); + +vi.mock('cloudflare:workers', () => ({ + DurableObject: class { + ctx: any; + env: any; + constructor(ctx: any, env: any) { + this.ctx = ctx; + this.env = env; + } + }, +})); + +vi.mock('../../../src/durable-objects/migrations', () => ({ + runMigrations: vi.fn(), +})); + +vi.mock('@simple-agent-manager/shared', async (importOriginal) => ({ + ...(await importOriginal()), + ACP_SESSION_VALID_TRANSITIONS: {}, + ACP_SESSION_TERMINAL_STATUSES: new Set(), + ACP_SESSION_DEFAULTS: { + DETECTION_WINDOW_MS: 30000, + MAX_FORK_DEPTH: 5, + }, + DEFAULT_SESSION_IDLE_TIMEOUT_MINUTES: 60, + DEFAULT_WORKSPACE_PROFILE: 'default', + PROVIDER_LOCATIONS: {}, +})); + +vi.mock('../../../src/durable-objects/project-data/comments', () => ({ + createCommentThread: vi.fn(), + createCommentReply: vi.fn(), + listCommentThreads: vi.fn(), + updateCommentThreadStatus: vi.fn(), +})); + +const commentStorage = await import('../../../src/durable-objects/project-data/comments'); +const { ProjectData } = await import('../../../src/durable-objects/project-data'); + +interface MockWebSocket { + send: ReturnType; + close: ReturnType; + tags: string[]; + _sent: string[]; +} + +function createMockWebSocket(tags: string[] = []): MockWebSocket { + const sent: string[] = []; + return { + send: vi.fn((data: string) => sent.push(data)), + close: vi.fn(), + tags, + _sent: sent, + }; +} + +function createMockCtx(websockets: MockWebSocket[] = []) { + return { + storage: { + sql: { + exec: vi.fn(() => ({ + toArray: () => [], + columnNames: [], + rowsRead: 0, + rowsWritten: 0, + })), + }, + transactionSync: vi.fn((fn: () => unknown) => fn()), + get: vi.fn(), + put: vi.fn(), + }, + id: { toString: () => 'test-do-id' }, + blockConcurrencyWhile: vi.fn(async (fn: () => Promise) => fn()), + getWebSockets: vi.fn((tag?: string) => { + if (tag) return websockets.filter((ws) => ws.tags.includes(tag)); + return [...websockets]; + }), + getTags: vi.fn((ws: MockWebSocket) => ws.tags), + acceptWebSocket: vi.fn(), + }; +} + +const thread = { + id: 'thread-1', + sessionId: 'session-a', + anchor: { kind: 'message' as const, messageId: 'message-1', quote: null }, + author: { kind: 'human' as const, id: 'user-1', name: 'Ada' }, + body: 'Needs clarification', + status: 'open' as const, + createdAt: 1, + updatedAt: 1, + sequence: 1, + version: 1, + clientMutationId: 'mutation-1', + sentAt: null, + sentBy: null, + resolvedAt: null, + resolvedBy: null, + reopenedAt: null, + reopenedBy: null, + replies: [], +}; + +describe('ProjectData DO — comment WebSocket broadcasting', () => { + let projectData: InstanceType; + let sessionASocket: MockWebSocket; + let sessionBSocket: MockWebSocket; + let projectSocket: MockWebSocket; + + beforeEach(() => { + vi.resetAllMocks(); + sessionASocket = createMockWebSocket(['session:session-a']); + sessionBSocket = createMockWebSocket(['session:session-b']); + projectSocket = createMockWebSocket([]); + projectData = new ProjectData( + createMockCtx([sessionASocket, sessionBSocket, projectSocket]) as any, + {} as any + ); + }); + + it('broadcasts changed comment threads to matching session and project sockets', () => { + vi.mocked(commentStorage.createCommentThread).mockReturnValue({ + thread, + idempotent: false, + changed: true, + }); + + const result = projectData.createCommentThread({} as never); + + expect(result).toEqual({ thread, idempotent: false }); + expect(sessionASocket.send).toHaveBeenCalledOnce(); + expect(projectSocket.send).toHaveBeenCalledOnce(); + expect(sessionBSocket.send).not.toHaveBeenCalled(); + + const event = JSON.parse(sessionASocket._sent[0]); + expect(event).toEqual({ + type: 'comment.thread.changed', + payload: { + sessionId: 'session-a', + thread, + reason: 'thread_created', + }, + }); + }); + + it('broadcasts reply and status reasons only when the authoritative state changes', () => { + vi.mocked(commentStorage.createCommentReply).mockReturnValue({ + thread, + reply: { + id: 'reply-1', + threadId: 'thread-1', + sessionId: 'session-a', + author: thread.author, + body: 'Reply', + createdAt: 2, + sequence: 1, + clientMutationId: 'reply-key', + }, + idempotent: false, + changed: true, + }); + vi.mocked(commentStorage.updateCommentThreadStatus).mockReturnValueOnce({ + thread: { ...thread, status: 'resolved', version: 2 }, + idempotent: false, + changed: true, + }); + vi.mocked(commentStorage.updateCommentThreadStatus).mockReturnValueOnce({ + thread: { ...thread, status: 'resolved', version: 2 }, + idempotent: true, + changed: false, + }); + + projectData.createCommentReply({} as never); + projectData.updateCommentThreadStatus({ status: 'resolved' } as never); + projectData.updateCommentThreadStatus({ status: 'resolved' } as never); + + const reasons = sessionASocket._sent.map( + (message) => JSON.parse(message).payload.reason as string + ); + expect(reasons).toEqual(['reply_created', 'resolved']); + expect(projectSocket._sent).toHaveLength(2); + expect(sessionBSocket._sent).toHaveLength(0); + }); +}); diff --git a/apps/api/tests/unit/routes/chat-comments.test.ts b/apps/api/tests/unit/routes/chat-comments.test.ts new file mode 100644 index 000000000..0195c418d --- /dev/null +++ b/apps/api/tests/unit/routes/chat-comments.test.ts @@ -0,0 +1,311 @@ +import { drizzle } from 'drizzle-orm/d1'; +import { Hono } from 'hono'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Env } from '../../../src/env'; +import { AppError } from '../../../src/middleware/error'; + +const mocks = vi.hoisted(() => { + class CommentNotFoundError extends Error { + readonly code = 'COMMENT_NOT_FOUND'; + constructor(readonly resource: 'Chat session' | 'Message' | 'Comment thread') { + super(`${resource} not found`); + this.name = 'CommentNotFoundError'; + } + } + + class CommentValidationError extends Error { + readonly code = 'COMMENT_VALIDATION'; + constructor(message: string) { + super(message); + this.name = 'CommentValidationError'; + } + } + + class CommentIdempotencyConflictError extends Error { + readonly code = 'COMMENT_IDEMPOTENCY_CONFLICT'; + constructor() { + super('clientMutationId already belongs to a different comment mutation'); + this.name = 'CommentIdempotencyConflictError'; + } + } + + class CommentLimitExceededError extends Error { + readonly code = 'COMMENT_LIMIT_EXCEEDED'; + constructor(message: string) { + super(message); + this.name = 'CommentLimitExceededError'; + } + } + + return { + createCommentReply: vi.fn(), + createCommentThread: vi.fn(), + getMessageToolContent: vi.fn(), + getMessages: vi.fn(), + getSession: vi.fn(), + listCommentThreads: vi.fn(), + requireProjectAccess: vi.fn(), + requireProjectCapability: vi.fn(), + stopSession: vi.fn(), + updateCommentThreadStatus: vi.fn(), + CommentIdempotencyConflictError, + CommentLimitExceededError, + CommentNotFoundError, + CommentValidationError, + }; +}); + +vi.mock('drizzle-orm/d1'); +vi.mock('../../../src/middleware/auth', () => ({ + requireAuth: () => vi.fn((_c: unknown, next: () => Promise) => next()), + requireApproved: () => vi.fn((_c: unknown, next: () => Promise) => next()), + getUserId: () => 'user-1', + getAuth: () => ({ + user: { + id: 'user-1', + email: 'user@example.com', + name: 'Ada', + avatarUrl: null, + role: 'user', + status: 'active', + }, + session: { id: 'session-1', expiresAt: new Date('2030-01-01T00:00:00Z') }, + }), +})); +vi.mock('../../../src/middleware/project-auth', () => ({ + requireProjectAccess: mocks.requireProjectAccess, + requireProjectCapability: mocks.requireProjectCapability, +})); +vi.mock('../../../src/services/project-data', () => ({ + CommentIdempotencyConflictError: mocks.CommentIdempotencyConflictError, + CommentLimitExceededError: mocks.CommentLimitExceededError, + CommentNotFoundError: mocks.CommentNotFoundError, + CommentValidationError: mocks.CommentValidationError, + createCommentReply: mocks.createCommentReply, + createCommentThread: mocks.createCommentThread, + createSession: vi.fn(), + forwardWebSocket: vi.fn(), + getDurableExecutionSnapshot: vi.fn(), + getMessageToolContent: mocks.getMessageToolContent, + getMessages: mocks.getMessages, + getSession: mocks.getSession, + getSessionState: vi.fn(), + linkSessionIdea: vi.fn(), + listAcpSessions: vi.fn().mockResolvedValue({ sessions: [], total: 0 }), + listCommentThreads: mocks.listCommentThreads, + listSessionIdeas: vi.fn().mockResolvedValue({ ideas: [] }), + listSessions: vi.fn(), + prepareAttentionAnswer: vi.fn(), + completeAttentionAnswer: vi.fn(), + releaseAttentionAnswer: vi.fn(), + resetIdleCleanup: vi.fn(), + stopSession: mocks.stopSession, + unlinkSessionIdea: vi.fn(), + updateCommentThreadStatus: mocks.updateCommentThreadStatus, +})); +vi.mock('../../../src/services/workspace-cleanup', () => ({ + cleanupWorkspaceForDeletion: vi.fn(), +})); +vi.mock('../../../src/services/session-task-repair', () => ({ + ensureSessionTaskBacked: vi.fn(), +})); +vi.mock('../../../src/services/task-terminal-cleanup', () => ({ + cleanupTerminalTaskResources: vi.fn(), +})); + +import { chatRoutes } from '../../../src/routes/chat'; + +const thread = { + id: 'thread-1', + sessionId: 'session-1', + anchor: { kind: 'message' as const, messageId: 'message-1', quote: null }, + author: { kind: 'human' as const, id: 'user-1', name: 'Ada' }, + body: 'Needs clarification', + status: 'open' as const, + createdAt: 1, + updatedAt: 1, + sequence: 1, + version: 1, + clientMutationId: 'mutation-1', + sentAt: null, + sentBy: null, + resolvedAt: null, + resolvedBy: null, + reopenedAt: null, + reopenedBy: null, + replies: [], +}; + +function createApp() { + const app = new Hono<{ Bindings: Env }>(); + app.onError((err, c) => { + if (err instanceof AppError) { + return c.json(err.toJSON(), err.statusCode as never); + } + return c.json({ error: 'INTERNAL_ERROR', message: err.message }, 500); + }); + app.route('/api/projects/:projectId/sessions', chatRoutes); + return app; +} + +describe('chat comment routes', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(drizzle).mockReturnValue({} as never); + mocks.requireProjectCapability.mockResolvedValue({ id: 'project-1' }); + mocks.listCommentThreads.mockResolvedValue({ threads: [thread], hasMore: false }); + mocks.createCommentThread.mockResolvedValue({ thread, idempotent: false }); + mocks.createCommentReply.mockResolvedValue({ + thread: { ...thread, replies: [{ id: 'reply-1' }] }, + reply: { id: 'reply-1' }, + idempotent: true, + }); + mocks.updateCommentThreadStatus.mockResolvedValue({ + thread: { ...thread, status: 'resolved' }, + idempotent: false, + }); + }); + + it('lists comment threads with task:read project authorization and bounded query params', async () => { + const response = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments?messageId=message-1&status=open&afterSequence=3&limit=10', + { method: 'GET' }, + { DATABASE: {} } as Env + ); + + expect(response.status).toBe(200); + expect(mocks.requireProjectCapability).toHaveBeenCalledWith( + expect.anything(), + 'project-1', + 'user-1', + 'task:read' + ); + expect(mocks.listCommentThreads).toHaveBeenCalledWith(expect.anything(), 'project-1', { + sessionId: 'session-1', + messageId: 'message-1', + status: 'open', + afterSequence: 3, + limit: 10, + }); + }); + + it('creates threads and replies with human actor and clientMutationId payloads', async () => { + const createResponse = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + messageId: 'message-1', + body: 'Needs clarification', + clientMutationId: 'thread-key', + }), + }, + { DATABASE: {} } as Env + ); + expect(createResponse.status).toBe(201); + expect(mocks.requireProjectCapability).toHaveBeenLastCalledWith( + expect.anything(), + 'project-1', + 'user-1', + 'task:write' + ); + expect(mocks.createCommentThread).toHaveBeenCalledWith(expect.anything(), 'project-1', { + sessionId: 'session-1', + messageId: 'message-1', + body: 'Needs clarification', + quote: null, + clientMutationId: 'thread-key', + actor: { kind: 'human', id: 'user-1', name: 'Ada' }, + }); + + const replyResponse = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments/thread-1/replies', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ body: 'Reply', clientMutationId: 'reply-key' }), + }, + { DATABASE: {} } as Env + ); + expect(replyResponse.status).toBe(200); + expect(mocks.createCommentReply).toHaveBeenCalledWith(expect.anything(), 'project-1', { + sessionId: 'session-1', + threadId: 'thread-1', + body: 'Reply', + clientMutationId: 'reply-key', + actor: { kind: 'human', id: 'user-1', name: 'Ada' }, + }); + }); + + it('updates status through server-authoritative transition endpoints', async () => { + const response = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments/thread-1/resolve', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ clientMutationId: 'resolve-key' }), + }, + { DATABASE: {} } as Env + ); + + expect(response.status).toBe(200); + expect(mocks.updateCommentThreadStatus).toHaveBeenCalledWith(expect.anything(), 'project-1', { + sessionId: 'session-1', + threadId: 'thread-1', + status: 'resolved', + clientMutationId: 'resolve-key', + actor: { kind: 'human', id: 'user-1', name: 'Ada' }, + }); + }); + + it('does not reach ProjectData when project authorization rejects membership', async () => { + mocks.requireProjectCapability.mockRejectedValueOnce( + new AppError(404, 'NOT_FOUND', 'Project not found') + ); + + const response = await createApp().request( + 'https://api.test/api/projects/project-2/sessions/session-1/comments', + { method: 'GET' }, + { DATABASE: {} } as Env + ); + + expect(response.status).toBe(404); + expect(mocks.listCommentThreads).not.toHaveBeenCalled(); + }); + + it('maps ProjectData validation, missing-message, and idempotency errors to API errors', async () => { + const invalidStatus = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments?status=closed', + { method: 'GET' }, + { DATABASE: {} } as Env + ); + expect(invalidStatus.status).toBe(400); + + mocks.createCommentThread.mockRejectedValueOnce(new mocks.CommentNotFoundError('Message')); + const missingMessage = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ messageId: 'missing-message', body: 'Thread' }), + }, + { DATABASE: {} } as Env + ); + expect(missingMessage.status).toBe(404); + await expect(missingMessage.json()).resolves.toMatchObject({ message: 'Message not found' }); + + mocks.createCommentReply.mockRejectedValueOnce(new mocks.CommentIdempotencyConflictError()); + const conflict = await createApp().request( + 'https://api.test/api/projects/project-1/sessions/session-1/comments/thread-1/replies', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ body: 'Reply', clientMutationId: 'used-key' }), + }, + { DATABASE: {} } as Env + ); + expect(conflict.status).toBe(409); + }); +}); diff --git a/apps/api/tests/unit/routes/chat-prompt-cancel.test.ts b/apps/api/tests/unit/routes/chat-prompt-cancel.test.ts index 4932fdf84..26e6e0ac6 100644 --- a/apps/api/tests/unit/routes/chat-prompt-cancel.test.ts +++ b/apps/api/tests/unit/routes/chat-prompt-cancel.test.ts @@ -23,6 +23,7 @@ vi.mock('drizzle-orm/d1', () => ({ })); vi.mock('@simple-agent-manager/shared', () => ({ + COMMENT_STATUSES: ['open', 'sent', 'resolved'], DEFAULT_CHAT_SESSION_MESSAGE_LIMIT: 500, DEFAULT_CHAT_SESSION_MESSAGE_MAX: 50000, DEFAULT_CHAT_COMPACT_MODE: true, diff --git a/apps/api/tests/unit/routes/chat-session-agent-routing.test.ts b/apps/api/tests/unit/routes/chat-session-agent-routing.test.ts index 4f758039c..537bb4077 100644 --- a/apps/api/tests/unit/routes/chat-session-agent-routing.test.ts +++ b/apps/api/tests/unit/routes/chat-session-agent-routing.test.ts @@ -67,6 +67,7 @@ vi.mock('drizzle-orm/d1', () => ({ })); vi.mock('@simple-agent-manager/shared', () => ({ + COMMENT_STATUSES: ['open', 'sent', 'resolved'], DEFAULT_CHAT_SESSION_DELTA_MESSAGE_LIMIT: 5000, DEFAULT_CHAT_SESSION_MESSAGE_LIMIT: 500, DEFAULT_CHAT_SESSION_MESSAGE_MAX: 50000, diff --git a/apps/api/tests/unit/services/project-data-comments.test.ts b/apps/api/tests/unit/services/project-data-comments.test.ts new file mode 100644 index 000000000..f1975c915 --- /dev/null +++ b/apps/api/tests/unit/services/project-data-comments.test.ts @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Env } from '../../../src/env'; +import * as svc from '../../../src/services/project-data'; +import { resetProjectDataEnsureMemo } from '../../../src/services/project-data-ensure-memo'; + +const actor = { kind: 'human' as const, id: 'user-1', name: 'Ada' }; +const thread = { + id: 'thread-1', + sessionId: 'session-1', + anchor: { kind: 'message' as const, messageId: 'message-1', quote: null }, + author: actor, + body: 'Needs clarification', + status: 'open' as const, + createdAt: 1, + updatedAt: 1, + sequence: 1, + version: 1, + clientMutationId: 'thread-key', + sentAt: null, + sentBy: null, + resolvedAt: null, + resolvedBy: null, + reopenedAt: null, + reopenedBy: null, + replies: [], +}; + +function makeEnv(stub: Record): Env { + return { + DO_RETRY_MAX_ATTEMPTS: '2', + DO_RETRY_BASE_DELAY_MS: '1', + PROJECT_DATA: { + idFromName: vi.fn((name: string) => ({ toString: () => `doid-${name}` })), + get: vi.fn(() => stub), + }, + } as unknown as Env; +} + +beforeEach(() => { + resetProjectDataEnsureMemo(); +}); + +describe('project-data service comment RPC wrappers', () => { + it('forwards list, create, reply, and status inputs to the ProjectData stub', async () => { + const stub = { + ensureProjectId: vi.fn().mockResolvedValue(undefined), + listCommentThreads: vi.fn().mockResolvedValue({ threads: [thread], hasMore: false }), + createCommentThread: vi.fn().mockResolvedValue({ thread, idempotent: false }), + createCommentReply: vi.fn().mockResolvedValue({ + thread: { ...thread, replies: [{ id: 'reply-1' }] }, + reply: { id: 'reply-1' }, + idempotent: false, + }), + updateCommentThreadStatus: vi.fn().mockResolvedValue({ + thread: { ...thread, status: 'resolved' }, + idempotent: false, + }), + }; + const env = makeEnv(stub); + + await expect( + svc.listCommentThreads(env, 'project-1', { + sessionId: 'session-1', + messageId: 'message-1', + status: 'open', + afterSequence: 2, + limit: 10, + }) + ).resolves.toMatchObject({ threads: [thread], hasMore: false }); + + await expect( + svc.createCommentThread(env, 'project-1', { + sessionId: 'session-1', + messageId: 'message-1', + body: 'Needs clarification', + quote: null, + clientMutationId: 'thread-key', + actor, + }) + ).resolves.toMatchObject({ thread, idempotent: false }); + + await expect( + svc.createCommentReply(env, 'project-1', { + sessionId: 'session-1', + threadId: 'thread-1', + body: 'Reply', + clientMutationId: 'reply-key', + actor, + }) + ).resolves.toMatchObject({ reply: { id: 'reply-1' }, idempotent: false }); + + await expect( + svc.updateCommentThreadStatus(env, 'project-1', { + sessionId: 'session-1', + threadId: 'thread-1', + status: 'resolved', + clientMutationId: 'resolve-key', + actor, + }) + ).resolves.toMatchObject({ thread: { status: 'resolved' }, idempotent: false }); + + expect(stub.listCommentThreads).toHaveBeenCalledWith({ + sessionId: 'session-1', + messageId: 'message-1', + status: 'open', + afterSequence: 2, + limit: 10, + }); + expect(stub.createCommentThread).toHaveBeenCalledWith({ + sessionId: 'session-1', + messageId: 'message-1', + body: 'Needs clarification', + quote: null, + clientMutationId: 'thread-key', + actor, + }); + expect(stub.createCommentReply).toHaveBeenCalledWith({ + sessionId: 'session-1', + threadId: 'thread-1', + body: 'Reply', + clientMutationId: 'reply-key', + actor, + }); + expect(stub.updateCommentThreadStatus).toHaveBeenCalledWith({ + sessionId: 'session-1', + threadId: 'thread-1', + status: 'resolved', + clientMutationId: 'resolve-key', + actor, + }); + }); +}); diff --git a/apps/api/wrangler.toml b/apps/api/wrangler.toml index d6062d69a..a1ba0820f 100644 --- a/apps/api/wrangler.toml +++ b/apps/api/wrangler.toml @@ -50,6 +50,13 @@ NODE_HEARTBEAT_STALE_SECONDS = "180" MAX_PROJECTS_PER_USER = "100" MAX_SESSIONS_PER_PROJECT = "10000" MAX_MESSAGES_PER_SESSION = "100000" +COMMENT_BODY_MAX_LENGTH = "8000" +COMMENT_QUOTE_MAX_LENGTH = "2000" +COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH = "200" +COMMENT_LIST_LIMIT_DEFAULT = "100" +COMMENT_LIST_LIMIT_MAX = "500" +COMMENT_THREADS_PER_SESSION_MAX = "1000" +COMMENT_REPLIES_PER_THREAD_MAX = "200" PROJECT_DATA_TOOL_METADATA_MAX_BYTES = "131072" PROJECT_DATA_STORAGE_TELEMETRY_ENABLED = "true" PROJECT_DATA_STORAGE_LIMIT_BYTES = "10000000000" diff --git a/apps/www/src/content/docs/docs/reference/configuration.md b/apps/www/src/content/docs/docs/reference/configuration.md index 5ebe8146c..f0208fa7c 100644 --- a/apps/www/src/content/docs/docs/reference/configuration.md +++ b/apps/www/src/content/docs/docs/reference/configuration.md @@ -769,6 +769,13 @@ ProjectData stores a single prompt-delivery queue and checkpoint episodes keyed | --------------------------------------------- | ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `MAX_SESSIONS_PER_PROJECT` | `10000` | Max chat sessions per project | | `MAX_MESSAGES_PER_SESSION` | `100000` | Max messages per chat session | +| `COMMENT_BODY_MAX_LENGTH` | `8000` | Max characters per message-anchored comment or reply body | +| `COMMENT_QUOTE_MAX_LENGTH` | `2000` | Max characters preserved from quoted message text | +| `COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH` | `200` | Max `clientMutationId` length for message-anchored comment writes | +| `COMMENT_LIST_LIMIT_DEFAULT` | `100` | Default page size for comment thread lists | +| `COMMENT_LIST_LIMIT_MAX` | `500` | Max page size for comment thread lists | +| `COMMENT_THREADS_PER_SESSION_MAX` | `1000` | Max message-anchored comment threads per chat session | +| `COMMENT_REPLIES_PER_THREAD_MAX` | `200` | Max replies per message-anchored comment thread | | `DOCUMENT_CARD_RAW_OUTPUT_MAX_BYTES` | `16384` | Max compact metadata bytes preserved for library document cards | | `PROJECT_DATA_TOOL_METADATA_MAX_BYTES` | `131072` | Max stored `tool_metadata` bytes per message before oversized tool content is stripped into bounded metadata | | `PROJECT_DATA_STORAGE_TELEMETRY_ENABLED` | `true` | Enables ProjectData `databaseSize` alarm measurement and D1 telemetry writes | diff --git a/packages/shared/src/constants/defaults.ts b/packages/shared/src/constants/defaults.ts index d2bc1c3ea..898c8c949 100644 --- a/packages/shared/src/constants/defaults.ts +++ b/packages/shared/src/constants/defaults.ts @@ -93,6 +93,31 @@ export const DEFAULT_CHAT_SESSION_MESSAGE_MAX = 50000; */ export const DEFAULT_CHAT_SESSION_DELTA_MESSAGE_LIMIT = 5000; +// ============================================================================= +// Message-anchored comments +// ============================================================================= + +/** Max comment thread/reply body length. Override via COMMENT_BODY_MAX_LENGTH. */ +export const DEFAULT_COMMENT_BODY_MAX_LENGTH = 8_000; + +/** Max selected quote snapshot length stored on message anchors. Override via COMMENT_QUOTE_MAX_LENGTH. */ +export const DEFAULT_COMMENT_QUOTE_MAX_LENGTH = 2_000; + +/** Max idempotency key length accepted for optimistic comment mutations. Override via COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH. */ +export const DEFAULT_COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH = 200; + +/** Default page size for session comment list reads. Override via COMMENT_LIST_LIMIT_DEFAULT. */ +export const DEFAULT_COMMENT_LIST_LIMIT_DEFAULT = 100; + +/** Max page size for session comment list reads. Override via COMMENT_LIST_LIMIT_MAX. */ +export const DEFAULT_COMMENT_LIST_LIMIT_MAX = 500; + +/** Max message-anchored comment threads per chat session. Override via COMMENT_THREADS_PER_SESSION_MAX. */ +export const DEFAULT_COMMENT_THREADS_PER_SESSION_MAX = 1_000; + +/** Max replies per comment thread. Override via COMMENT_REPLIES_PER_THREAD_MAX. */ +export const DEFAULT_COMMENT_REPLIES_PER_THREAD_MAX = 200; + /** * Safety bound on how many older pages the chat client will fetch while chasing a * timeline jump target that predates the loaded window (the rare oversized-session diff --git a/packages/shared/src/constants/index.ts b/packages/shared/src/constants/index.ts index 2ebf9da6f..7001337c3 100644 --- a/packages/shared/src/constants/index.ts +++ b/packages/shared/src/constants/index.ts @@ -38,6 +38,13 @@ export { DEFAULT_CHAT_SESSION_MESSAGE_LIMIT, DEFAULT_CHAT_SESSION_MESSAGE_MAX, DEFAULT_CHAT_TIMELINE_MAX_PAGES, + DEFAULT_COMMENT_BODY_MAX_LENGTH, + DEFAULT_COMMENT_IDEMPOTENCY_KEY_MAX_LENGTH, + DEFAULT_COMMENT_LIST_LIMIT_DEFAULT, + DEFAULT_COMMENT_LIST_LIMIT_MAX, + DEFAULT_COMMENT_QUOTE_MAX_LENGTH, + DEFAULT_COMMENT_REPLIES_PER_THREAD_MAX, + DEFAULT_COMMENT_THREADS_PER_SESSION_MAX, DEFAULT_DASHBOARD_INACTIVE_THRESHOLD_MS, DEFAULT_DASHBOARD_POLL_INTERVAL_MS, DEFAULT_MAX_AGENT_SESSIONS_PER_WORKSPACE, diff --git a/packages/shared/src/types/comments.ts b/packages/shared/src/types/comments.ts new file mode 100644 index 000000000..935337a0e --- /dev/null +++ b/packages/shared/src/types/comments.ts @@ -0,0 +1,81 @@ +export const COMMENT_STATUSES = ['open', 'sent', 'resolved'] as const; +export type CommentStatus = (typeof COMMENT_STATUSES)[number]; + +export const COMMENT_AUTHOR_KINDS = ['human', 'agent'] as const; +export type CommentAuthorKind = (typeof COMMENT_AUTHOR_KINDS)[number]; + +export type MessageCommentAnchor = { + kind: 'message'; + messageId: string; + quote: string | null; +}; + +export type CommentAuthor = { + kind: CommentAuthorKind; + id: string; + name: string | null; +}; + +export type MessageCommentReply = { + id: string; + threadId: string; + sessionId: string; + author: CommentAuthor; + body: string; + createdAt: number; + sequence: number; + clientMutationId: string | null; +}; + +export type MessageCommentThread = { + id: string; + sessionId: string; + anchor: MessageCommentAnchor; + author: CommentAuthor; + body: string; + status: CommentStatus; + createdAt: number; + updatedAt: number; + sequence: number; + version: number; + clientMutationId: string | null; + sentAt: number | null; + sentBy: CommentAuthor | null; + resolvedAt: number | null; + resolvedBy: CommentAuthor | null; + reopenedAt: number | null; + reopenedBy: CommentAuthor | null; + replies: MessageCommentReply[]; +}; + +export type MessageCommentListResponse = { + threads: MessageCommentThread[]; + hasMore: boolean; +}; + +export type MessageCommentMutationResponse = { + thread: MessageCommentThread; + idempotent: boolean; +}; + +export type MessageCommentReplyMutationResponse = { + thread: MessageCommentThread; + reply: MessageCommentReply; + idempotent: boolean; +}; + +export type MessageCommentThreadEventReason = + | 'thread_created' + | 'reply_created' + | 'marked_sent' + | 'resolved' + | 'reopened'; + +export type MessageCommentThreadEvent = { + type: 'comment.thread.changed'; + payload: { + sessionId: string; + thread: MessageCommentThread; + reason: MessageCommentThreadEventReason; + }; +}; diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 1856ba02a..2adc5fe3d 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -218,6 +218,22 @@ export type { UpdateTaskRequest, UpdateTaskStatusRequest, } from './task'; + +// Message-anchored comments +export type { + CommentAuthor, + CommentAuthorKind, + CommentStatus, + MessageCommentAnchor, + MessageCommentListResponse, + MessageCommentMutationResponse, + MessageCommentReply, + MessageCommentReplyMutationResponse, + MessageCommentThread, + MessageCommentThreadEvent, + MessageCommentThreadEventReason, +} from './comments'; +export { COMMENT_AUTHOR_KINDS, COMMENT_STATUSES } from './comments'; export { ATTACHMENT_DEFAULTS, COMPLETION_EVIDENCE_VERIFICATION_KINDS, @@ -546,10 +562,7 @@ export type { CheckpointProgressEnvelope, CreateCheckpointEpisodeInput, } from './checkpoint'; -export { - CHECKPOINT_EPISODE_STATES, - CHECKPOINT_EPISODE_TRANSITIONS, -} from './checkpoint'; +export { CHECKPOINT_EPISODE_STATES, CHECKPOINT_EPISODE_TRANSITIONS } from './checkpoint'; // Mission (Phase 2: Orchestration Primitives) export type { diff --git a/tasks/active/2026-08-21-message-anchored-commenting-backend.md b/tasks/active/2026-08-21-message-anchored-commenting-backend.md new file mode 100644 index 000000000..6811b2fd9 --- /dev/null +++ b/tasks/active/2026-08-21-message-anchored-commenting-backend.md @@ -0,0 +1,96 @@ +# Message-anchored commenting backend + +## Problem statement + +Build the backend/multiplayer foundation for SAM's message-anchored commenting MVP from idea `01M0JQB842XSJ3W172DYPB37HN`. + +This is a constituent PR for a coordinated multi-PR integration. It must provide the clean server-side contract and persistence layer for sibling UI/MCP tracks, without merging or deploying to staging from this branch. + +## Scope + +In scope: + +- Message-anchored comment threads on `ProjectData.chat_messages`. +- Replies, `open` / `sent` / `resolved` status, resolve/reopen/send status transitions. +- Bounded write validation, idempotency, and deterministic ordering for optimistic clients. +- ProjectData Durable Object migration, RPC/service methods, HTTP API routes, and WebSocket broadcast events. +- Project membership authorization at HTTP boundaries and ProjectData-side session/message/thread consistency checks. +- Tests for migration, CRUD, replies, status transitions, validation, authorization, idempotency, ordering, and events. + +Out of scope: + +- File comments, fuzzy file re-anchoring, mentions, reactions, notification inboxes, and unrelated refactors. +- Actual "send to agent" prompt enqueueing. This backend records and broadcasts `sent` status; sibling work can attach prompt delivery to the explicit API contract. +- Permissions beyond project membership. + +## Research findings + +- Idea `01M0JQB842XSJ3W172DYPB37HN` defines the MVP as message comments first. `chat_messages.id` is stable and immutable; file anchors are deferred because production-safe re-anchoring is a separate hard problem. +- The prototype branch `sam/really-get-feature-talked-5ckt0s` validates behavior only: one anchor-discriminated thread model, replies, status pill, resolve/reopen, and message-row comment markers. Prototype code is UI-only mock data and must not be promoted blindly. +- ProjectData DO migrations live in `apps/api/src/durable-objects/migrations.ts` and must be append-only. Current latest migration is `031-task-wait-replay-hardening`; new work must append after it and avoid table recreation/drop patterns per `.claude/rules/31-migration-safety.md`. +- ProjectData public RPC methods live on `apps/api/src/durable-objects/project-data/index.ts`, with domain logic split into sibling modules. New comment logic should follow the module pattern instead of growing unrelated files. +- `apps/api/src/services/project-data.ts` is the typed Worker-to-DO service boundary. Write calls that can duplicate user intent should use explicit idempotency rather than relying on retry behavior. +- `ProjectData.broadcastEvent()` already fans out to session-tagged and project-wide WebSocket listeners. New comment mutations should use this existing channel with server-authoritative full-thread payloads so clients converge without CRDT/OT. +- `routes/chat.ts` already mounts project-scoped session endpoints under `/api/projects/:projectId/sessions`, protects them with `requireAuth()` and `requireApproved()`, and uses `requireProjectAccess` / `requireProjectCapability` for tenant authorization. +- Existing chat routes sometimes enforce session creator for follow-up prompts, but the commenting MVP explicitly scopes permissions to project membership only. Comment writes should require project `task:write`, reads should require `task:read`, and should not require session creator. +- Storage-safety work warns that ProjectData is low-level, write-hot state with a 10 GB ceiling. Comment writes must bound body, quote, idempotency key, thread count, and reply count at the write boundary via env-backed defaults. +- Rule 50 requires list reads that parse rows to tolerate malformed rows. Comment list mapping must isolate per-row parsing and warn/skip bad rows instead of throwing a whole list response. +- Vertical-slice coverage is required because this crosses HTTP route → D1 membership → ProjectData DO → SQLite → WebSocket event contracts. + +## Implementation checklist + +- [x] Add shared comment API/event types and env-backed default limits. +- [x] Append a ProjectData DO SQLite migration for `comment_threads`, `comment_replies`, and status-idempotency rows. +- [x] Add a `project-data/comments.ts` module with bounded validation, message-anchor verification, idempotent create/reply/status transitions, deterministic sequence ordering, and row parsing isolation. +- [x] Add public ProjectData RPC delegates in `project-data/index.ts` that broadcast server-authoritative comment events on the existing WebSocket channel. +- [x] Add typed service wrappers in `apps/api/src/services/project-data.ts`. +- [x] Add Valibot schemas and HTTP routes under `/api/projects/:projectId/sessions/:sessionId/comments`. +- [x] Enforce project membership/capability authorization at every HTTP route and DO-side rejection for missing sessions, missing/cross-session messages, missing threads, and idempotency conflicts. +- [x] Add/refresh API documentation for the HTTP/RPC/event contract. +- [x] Add focused tests for migrations, CRUD/replies/status transitions, validation/limits, authorization, idempotency, ordering, and WebSocket events. +- [x] Run local quality checks and required specialist reviews. +- [ ] Create an open PR to `main`; do not deploy to staging and do not merge. + +## Acceptance criteria + +- A project member with read access can list message comment threads for a session; non-members cannot. +- A project member with write access can create a message-anchored thread only when the target message exists in that same ProjectData project/session. +- Cross-project or cross-session message/thread IDs are rejected as missing/not found instead of silently creating orphan anchors. +- Thread body, reply body, quote, idempotency key, list limits, per-session thread count, and per-thread reply count are bounded via env-backed defaults. +- Duplicate create/reply/status requests with the same idempotency key return the same authoritative result; reuse of a key with a different intent conflicts. +- Threads and replies have deterministic sequence ordering even when timestamps collide. +- Status transitions support `sent`, `resolved`, and reopening to `open` with server-side actor/timestamp metadata. +- Every mutation emits a ProjectData WebSocket event with enough authoritative state for multiple clients to converge. +- Tests cover the route/service/DO storage path and event payloads; staging is intentionally skipped by explicit instruction. + +## Contract assumptions + +- MVP anchors accepted by this backend are message anchors only: `{ kind: 'message', messageId, quote? }`. +- `sent` means "marked as sent for agent handling" in storage and events. This PR does not enqueue a prompt or wait for agent completion. +- Comment authors for HTTP writes are server-authoritative human actors derived from the authenticated session. Agent-authored comments can be added later through MCP/tool routes using the same ProjectData RPC shape. +- Comment authorization is project membership/capability based, not session-creator based. + +## Validation evidence + +- `pnpm --filter @simple-agent-manager/shared build` +- `pnpm --filter @simple-agent-manager/api typecheck` +- `pnpm --filter @simple-agent-manager/api test -- tests/unit/durable-objects/comments.test.ts tests/unit/durable-objects/project-data-comment-broadcast.test.ts tests/unit/routes/chat-comments.test.ts tests/unit/services/project-data-comments.test.ts` +- `pnpm --filter @simple-agent-manager/api exec eslint src/durable-objects/project-data/comments.ts src/durable-objects/project-data/index.ts src/durable-objects/project-data/types.ts src/env.ts src/routes/chat.ts src/routes/chat-comments.ts src/schemas/comments.ts src/schemas/index.ts src/services/project-data.ts tests/unit/durable-objects/comments.test.ts tests/unit/durable-objects/project-data-comment-broadcast.test.ts tests/unit/routes/chat-comments.test.ts tests/unit/services/project-data-comments.test.ts` +- `pnpm exec prettier --check .agents/skills/api-reference/SKILL.md .claude/skills/api-reference/SKILL.md apps/api/src/durable-objects/project-data/comments.ts apps/api/src/durable-objects/project-data/index.ts apps/api/src/durable-objects/project-data/types.ts apps/api/src/env.ts apps/api/src/routes/chat.ts apps/api/src/routes/chat-comments.ts apps/api/src/schemas/comments.ts apps/api/src/schemas/index.ts apps/api/src/services/project-data.ts apps/api/tests/unit/durable-objects/comments.test.ts apps/api/tests/unit/durable-objects/project-data-comment-broadcast.test.ts apps/api/tests/unit/routes/chat-comments.test.ts apps/api/tests/unit/services/project-data-comments.test.ts apps/www/src/content/docs/docs/reference/configuration.md packages/shared/src/constants/defaults.ts packages/shared/src/constants/index.ts packages/shared/src/types/comments.ts packages/shared/src/types/index.ts tasks/active/2026-08-21-message-anchored-commenting-backend.md` +- `pnpm quality:do-migration-safety` +- `pnpm quality:source-contract-tests` +- `pnpm quality:wrangler-bindings` +- `pnpm quality:type-boundaries` +- `git diff --check` + +## Specialist review evidence + +| Reviewer | Status | Outcome | +| ------------------------- | ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| constitution-validator | PASS | New limits are env-backed shared defaults with Worker/DO Env, Wrangler, `.env.example`, and docs entries; no hardcoded URLs/timeouts/identifiers added. | +| cloudflare-specialist | PASS | ProjectData migration is append-only/non-destructive and passes `quality:do-migration-safety`; write RPCs use synchronous DO transactions and existing WebSocket fan-out. | +| security-auditor | PASS | HTTP routes require project `task:read`/`task:write`; server derives human actor from auth; ProjectData rejects missing/cross-session message anchors and missing threads. | +| test-engineer | PASS | Added storage, route/auth/error, service-wrapper, and DO WebSocket event tests covering migration, CRUD, replies, status, limits, idempotency, ordering, and event payloads. | +| doc-sync-validator | PASS | API reference, shared types, env examples, Wrangler defaults, and configuration docs reflect the new HTTP/RPC/event contract and env knobs. | +| env-validator | PASS | `COMMENT_*` env vars are consistently named and present in Worker Env, ProjectData Env, Wrangler, `.env.example`, docs, and shared defaults. | +| task-completion-validator | PASS | Acceptance criteria checked against the diff and passing tests; staging and merge intentionally skipped by explicit task constraint. | From 738abde359b7228b7e470642efeb977549446654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Sat, 22 Aug 2026 00:37:07 +0000 Subject: [PATCH 2/7] feat: integrate message comment MCP directives --- .claude/skills/api-reference/SKILL.md | 1 + apps/api/.env.example | 5 + .../durable-objects/project-data/comments.ts | 2 +- .../src/durable-objects/project-data/index.ts | 4 + .../durable-objects/project-data/mailbox.ts | 3 +- .../project-data/prompt-delivery.ts | 3 +- apps/api/src/env.ts | 5 + apps/api/src/lib/sanitize-user-input.ts | 11 + .../api/src/routes/chat-comment-directives.ts | 85 +++ apps/api/src/routes/chat.ts | 2 + apps/api/src/routes/mcp/_helpers.ts | 18 +- apps/api/src/routes/mcp/comment-tools.ts | 386 ++++++++++++++ apps/api/src/routes/mcp/index.ts | 32 ++ .../mcp/tool-definitions-comment-tools.ts | 156 ++++++ apps/api/src/routes/mcp/tool-definitions.ts | 4 + apps/api/src/services/message-comments.ts | 491 ++++++++++++++++++ apps/api/src/services/project-data.ts | 12 + .../durable-prompt-delivery.test.ts | 32 +- .../routes/chat-comment-directives.test.ts | 341 ++++++++++++ .../unit/routes/mcp-message-comments.test.ts | 392 ++++++++++++++ apps/api/tests/unit/routes/mcp.test.ts | 92 +++- .../unit/services/message-comments.test.ts | 381 ++++++++++++++ .../docs/docs/reference/configuration.md | 31 +- ...8-21-message-comment-directive-contract.md | 110 ++++ packages/shared/src/types/comments.ts | 133 ++++- packages/shared/src/types/index.ts | 18 +- packages/shared/src/types/mailbox.ts | 1 + ...26-08-21-message-comment-mcp-directives.md | 82 +++ 28 files changed, 2778 insertions(+), 55 deletions(-) create mode 100644 apps/api/src/lib/sanitize-user-input.ts create mode 100644 apps/api/src/routes/chat-comment-directives.ts create mode 100644 apps/api/src/routes/mcp/comment-tools.ts create mode 100644 apps/api/src/routes/mcp/tool-definitions-comment-tools.ts create mode 100644 apps/api/src/services/message-comments.ts create mode 100644 apps/api/tests/unit/routes/chat-comment-directives.test.ts create mode 100644 apps/api/tests/unit/routes/mcp-message-comments.test.ts create mode 100644 apps/api/tests/unit/services/message-comments.test.ts create mode 100644 docs/notes/2026-08-21-message-comment-directive-contract.md create mode 100644 tasks/archive/2026-08-21-message-comment-mcp-directives.md diff --git a/.claude/skills/api-reference/SKILL.md b/.claude/skills/api-reference/SKILL.md index ef9c60bd8..f9f612c39 100644 --- a/.claude/skills/api-reference/SKILL.md +++ b/.claude/skills/api-reference/SKILL.md @@ -50,6 +50,7 @@ user-invocable: false - `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/resolve` — Mark a thread `resolved` (`{ clientMutationId? }`) - `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/reopen` — Reopen a thread to `open` (`{ clientMutationId? }`) - `POST /api/projects/:projectId/sessions/:sessionId/prompt` — Send a follow-up prompt to the active agent session +- `POST /api/projects/:projectId/sessions/:sessionId/comments/:threadId/send-to-agent` — Queue one idempotent comment directive through ProjectData prompt delivery for the explicit human "send to agent" action - `POST /api/projects/:projectId/sessions/:sessionId/attention/:markerId/resolve` — Validate, forward, and record one structured human-input answer (`{ answer }`) - `POST /api/projects/:projectId/sessions/:sessionId/summarize` — Generate a session summary for conversation forking - `POST /api/projects/:projectId/sessions/:sessionId/stop` — Stop a chat session diff --git a/apps/api/.env.example b/apps/api/.env.example index c9e57972a..86dd30fce 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -600,6 +600,11 @@ INFOMANIAK_IP_POLL_INTERVAL_MS=3000 # MCP list/read limits # MCP_MESSAGE_LIST_LIMIT=50 # Default number of raw tokens fetched per request # MCP_MESSAGE_LIST_MAX=200 # Max raw tokens per request (before grouping into logical messages) +# MCP_COMMENT_LIST_LIMIT=10 # Default page size for list_message_comment_threads +# MCP_COMMENT_LIST_MAX=25 # Max page size for list_message_comment_threads +# MCP_COMMENT_BODY_MAX_LENGTH=4000 # Max comment/reply body characters accepted via MCP +# MCP_COMMENT_QUOTE_MAX_LENGTH=1000 # Max message quote characters returned/sent to agents +# COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH=6000 # Max send-to-agent directive prompt length # MCP_TRIGGER_LIST_LIMIT=20 # Default page size for list_triggers # MCP_TRIGGER_LIST_MAX=100 # Max page size for list_triggers # MCP_INCIDENT_LIST_LIMIT=10 # Default page size for private list_incident_queue diff --git a/apps/api/src/durable-objects/project-data/comments.ts b/apps/api/src/durable-objects/project-data/comments.ts index 509ac2265..44ca1f47e 100644 --- a/apps/api/src/durable-objects/project-data/comments.ts +++ b/apps/api/src/durable-objects/project-data/comments.ts @@ -306,7 +306,7 @@ function readReplies(sql: SqlStorage, threadIds: string[]): Map { return comments.listCommentThreads(this.sql, this.env, input); } + getCommentThread(input: { sessionId: string; threadId: string }): MessageCommentThread | null { + return comments.getCommentThread(this.sql, input.sessionId, input.threadId); + } + createCommentThread(input: comments.CreateCommentThreadInput) { const result = this.ctx.storage.transactionSync(() => comments.createCommentThread(this.sql, this.env, input) diff --git a/apps/api/src/durable-objects/project-data/mailbox.ts b/apps/api/src/durable-objects/project-data/mailbox.ts index 0d63864b8..2dc0d75c8 100644 --- a/apps/api/src/durable-objects/project-data/mailbox.ts +++ b/apps/api/src/durable-objects/project-data/mailbox.ts @@ -167,7 +167,8 @@ export function getPendingMessages( WHEN 'notify' THEN 1 ELSE 0 END DESC, - created_at ASC + created_at ASC, + rowid ASC LIMIT ?`, targetSessionId, limit diff --git a/apps/api/src/durable-objects/project-data/prompt-delivery.ts b/apps/api/src/durable-objects/project-data/prompt-delivery.ts index 2b7100b82..4fbe84691 100644 --- a/apps/api/src/durable-objects/project-data/prompt-delivery.ts +++ b/apps/api/src/durable-objects/project-data/prompt-delivery.ts @@ -208,7 +208,8 @@ export function claimDuePromptDeliveries( WHEN 'notify' THEN 1 ELSE 0 END DESC, - created_at ASC + created_at ASC, + rowid ASC LIMIT ?`, now, config.maxAttempts, diff --git a/apps/api/src/env.ts b/apps/api/src/env.ts index 94615867e..680bb33a0 100644 --- a/apps/api/src/env.ts +++ b/apps/api/src/env.ts @@ -657,6 +657,11 @@ export interface Env extends WebhookTriggerEnv, TaskRecoveryEnv { MCP_MESSAGE_LIST_LIMIT?: string; // Default raw tokens per request (default: 50) MCP_MESSAGE_LIST_MAX?: string; // Max raw tokens per request (default: 200) MCP_MESSAGE_SEARCH_MAX?: string; // Max search results for search_messages (default: 20) + MCP_COMMENT_LIST_LIMIT?: string; // Default message-comment threads per request (default: 10) + MCP_COMMENT_LIST_MAX?: string; // Max message-comment threads per request (default: 25) + MCP_COMMENT_BODY_MAX_LENGTH?: string; // Max comment/reply body characters accepted via MCP (default: 4000) + MCP_COMMENT_QUOTE_MAX_LENGTH?: string; // Max message quote characters returned/sent to agents (default: 1000) + COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH?: string; // Max send-to-agent directive prompt length (default: 6000) MCP_TRIGGER_LIST_LIMIT?: string; // Default page size for list_triggers (default: 20) MCP_TRIGGER_LIST_MAX?: string; // Max page size for list_triggers (default: 100) MCP_INCIDENT_LIST_LIMIT?: string; // Default page size for list_incident_queue (default: 10) diff --git a/apps/api/src/lib/sanitize-user-input.ts b/apps/api/src/lib/sanitize-user-input.ts new file mode 100644 index 000000000..da6befb6c --- /dev/null +++ b/apps/api/src/lib/sanitize-user-input.ts @@ -0,0 +1,11 @@ +// Intentional: the character class deliberately targets raw C0/C1 control-code +// ranges (\x00-\x08 etc.) to strip null bytes and control characters from +// user/agent-supplied text. This is the sanitizer itself, not an accidental +// control character left in a regex literal. +const CONTROL_CHAR_PATTERN = + /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F\u200B-\u200F\u202A-\u202E\u2066-\u2069]/g; // eslint-disable-line no-control-regex -- sanitizer intentionally matches control characters + +/** Strip null bytes, Unicode bidi overrides, and C0/C1 control chars except newline and tab. */ +export function sanitizeUserInput(str: string): string { + return str.replace(CONTROL_CHAR_PATTERN, ''); +} diff --git a/apps/api/src/routes/chat-comment-directives.ts b/apps/api/src/routes/chat-comment-directives.ts new file mode 100644 index 000000000..011590977 --- /dev/null +++ b/apps/api/src/routes/chat-comment-directives.ts @@ -0,0 +1,85 @@ +import { drizzle } from 'drizzle-orm/d1'; +import type { Hono } from 'hono'; + +import * as schema from '../db/schema'; +import type { Env } from '../env'; +import { requireRouteParam } from '../lib/route-helpers'; +import { getUserId } from '../middleware/auth'; +import { errors } from '../middleware/error'; +import { requireProjectCapability } from '../middleware/project-auth'; +import { + createProjectDataMessageCommentAdapter, + isMessageCommentServiceError, + sendMessageCommentDirective, +} from '../services/message-comments'; +import * as projectDataService from '../services/project-data'; + +function mapCommentDirectiveError(err: unknown): Error { + if (!isMessageCommentServiceError(err)) { + return errors.internal('Failed to send comment directive'); + } + switch (err.code) { + case 'invalid_request': + return errors.badRequest(err.message); + case 'not_found': + return errors.notFound('Comment thread'); + case 'forbidden': + return errors.forbidden(err.message); + case 'conflict': + return errors.conflict(err.message); + case 'unavailable': + return errors.internal('Comment storage backend is unavailable'); + } +} + +/** + * Register the explicit human "send to agent" action for a message comment. + * + * This route only queues a compact durable prompt delivery for one comment + * thread. It does not inject full comment context into normal session loads. + */ +export function registerChatCommentDirectiveRoute(chatRoutes: Hono<{ Bindings: Env }>): void { + chatRoutes.post('/:sessionId/comments/:threadId/send-to-agent', async (c) => { + const userId = getUserId(c); + const projectId = requireRouteParam(c, 'projectId'); + const sessionId = requireRouteParam(c, 'sessionId'); + const threadId = requireRouteParam(c, 'threadId'); + const db = drizzle(c.env.DATABASE, { schema }); + + await requireProjectCapability(db, projectId, userId, 'task:write'); + + const session = await projectDataService.getSession(c.env, projectId, sessionId); + if (!session) { + throw errors.notFound('Chat session'); + } + + try { + const result = await sendMessageCommentDirective({ + env: c.env, + storage: createProjectDataMessageCommentAdapter(c.env), + projectId, + sessionId, + threadId, + humanUserId: userId, + }); + + return c.json( + { + accepted: result.accepted, + status: result.duplicate ? 'duplicate' : 'queued', + duplicate: result.duplicate, + deliveryId: result.deliveryId, + messageId: result.messageId, + thread: { + id: result.thread.id, + status: result.thread.status, + directive: result.thread.directive, + }, + }, + 202 + ); + } catch (err) { + throw mapCommentDirectiveError(err); + } + }); +} diff --git a/apps/api/src/routes/chat.ts b/apps/api/src/routes/chat.ts index 39b63ada8..4be512d4c 100644 --- a/apps/api/src/routes/chat.ts +++ b/apps/api/src/routes/chat.ts @@ -42,6 +42,7 @@ import { attachWakeState } from './chat/wake-state'; import { resolveChatAgentState } from './chat-agent-state'; import { registerChatCancelRoute } from './chat-cancel'; import { chatCommentRoutes } from './chat-comments'; +import { registerChatCommentDirectiveRoute } from './chat-comment-directives'; import { chatForkRoutes } from './chat-fork'; import { recordChatSessionLoadFailure } from './chat-load-diagnostics'; import { preparePromptForLiveAgent, sendPreparedPromptToLiveAgent } from './chat-prompt-forward'; @@ -463,6 +464,7 @@ chatRoutes.get('/:sessionId/durability', async (c) => { }); registerChatPromptRoute(chatRoutes); +registerChatCommentDirectiveRoute(chatRoutes); /** * POST /api/projects/:projectId/sessions/:sessionId/attention/:markerId/resolve diff --git a/apps/api/src/routes/mcp/_helpers.ts b/apps/api/src/routes/mcp/_helpers.ts index d53e803c4..25c2f22ab 100644 --- a/apps/api/src/routes/mcp/_helpers.ts +++ b/apps/api/src/routes/mcp/_helpers.ts @@ -287,23 +287,7 @@ export function getMcpLimits(env: Env) { }; } -// Intentional: the character class deliberately targets raw C0/C1 -// control-code ranges (\x00-\x08 etc.) to strip null bytes and control -// characters from user/agent-supplied text, per sanitizeUserInput's doc -// comment below. This is the sanitizer itself, not an accidental control -// character left in a regex literal. Declared as its own named constant -// (rather than inline in the .replace() call) with an `eslint-disable-line` -// trailing comment so Prettier's line-wrapping of a long `.replace(...)` -// call cannot separate the disable directive from the regex it targets \u2014 -// see the discriminating incident this pattern replaced during the 2026-08-11 -// ai-slop debt burn-down (tasks/archive/2026-08-10-ai-slop-debt-burndown.md). -const CONTROL_CHAR_PATTERN = - /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F\u200B-\u200F\u202A-\u202E\u2066-\u2069]/g; // eslint-disable-line no-control-regex -- see comment above - -/** Strip null bytes, Unicode bidi overrides, and C0/C1 control chars (except \n, \t) from user/agent input. */ -export function sanitizeUserInput(str: string): string { - return str.replace(CONTROL_CHAR_PATTERN, ''); -} +export { sanitizeUserInput } from '../../lib/sanitize-user-input'; // MCP protocol constants export const MCP_PROTOCOL_VERSION = '2025-03-26'; diff --git a/apps/api/src/routes/mcp/comment-tools.ts b/apps/api/src/routes/mcp/comment-tools.ts new file mode 100644 index 000000000..e87fb633f --- /dev/null +++ b/apps/api/src/routes/mcp/comment-tools.ts @@ -0,0 +1,386 @@ +/** + * MCP message-comment tools. + * + * These handlers are intentionally thin: ProjectData owns durable storage in the + * backend sibling PR, while this layer owns token-derived identity, same-session + * fences, bounded payloads, safe errors, and agent-facing tool contracts. + */ +import type { MessageCommentThreadStatus } from '@simple-agent-manager/shared'; + +import type { Env } from '../../env'; +import { log } from '../../lib/logger'; +import { + boundCommentThread, + boundCommentThreadSummary, + buildAgentCommentAuthor, + buildAgentCommentProvenance, + clampCommentListLimit, + createProjectDataMessageCommentAdapter, + getMessageCommentConfig, + isMessageCommentServiceError, + type MessageCommentStorageAdapter, + normalizeCommentBody, + normalizeCommentQuote, +} from '../../services/message-comments'; +import { + INTERNAL_ERROR, + INVALID_PARAMS, + jsonRpcError, + type JsonRpcResponse, + jsonRpcSuccess, + type McpTokenData, +} from './_helpers'; + +const CALLER_DERIVED_FIELDS = [ + 'projectId', + 'userId', + 'author', + 'authorId', + 'authorKind', + 'authorDisplayName', + 'provenance', +]; + +function textContent(value: unknown): string { + return JSON.stringify(value); +} + +function toolSuccess(requestId: string | number | null, value: unknown): JsonRpcResponse { + return jsonRpcSuccess(requestId, { + content: [ + { + type: 'text', + text: textContent(value), + }, + ], + }); +} + +function rejectCallerDerivedFields( + requestId: string | number | null, + params: Record +): JsonRpcResponse | null { + for (const field of CALLER_DERIVED_FIELDS) { + if (Object.prototype.hasOwnProperty.call(params, field)) { + return jsonRpcError( + requestId, + INVALID_PARAMS, + `${field} is derived from the verified MCP token and must not be supplied` + ); + } + } + return null; +} + +function optionalString(params: Record, field: string): string | null { + const value = params[field]; + return typeof value === 'string' && value.trim() ? value.trim() : null; +} + +function requiredString( + requestId: string | number | null, + params: Record, + field: string +): string | JsonRpcResponse { + const value = optionalString(params, field); + if (!value) return jsonRpcError(requestId, INVALID_PARAMS, `${field} is required`); + return value; +} + +function getStorage( + env: Env, + storage?: MessageCommentStorageAdapter +): MessageCommentStorageAdapter { + return storage ?? createProjectDataMessageCommentAdapter(env); +} + +async function resolveCallerSession( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env +): Promise<{ sessionId: string } | JsonRpcResponse> { + const requestedSessionId = optionalString(params, 'sessionId'); + const currentSessionId = + tokenData.chatSessionId ?? (await resolveWorkspaceChatSession(env, tokenData)); + if (!currentSessionId) { + return jsonRpcError(requestId, INVALID_PARAMS, 'No chat session found for this agent'); + } + if (requestedSessionId && requestedSessionId !== currentSessionId) { + return jsonRpcError( + requestId, + INVALID_PARAMS, + 'sessionId must match the calling agent session' + ); + } + return { sessionId: currentSessionId }; +} + +async function resolveWorkspaceChatSession( + env: Env, + tokenData: McpTokenData +): Promise { + if (!tokenData.workspaceId) return null; + try { + const row = await env.DATABASE.prepare( + 'SELECT chat_session_id FROM workspaces WHERE id = ? AND project_id = ?' + ) + .bind(tokenData.workspaceId, tokenData.projectId) + .first<{ chat_session_id: string | null }>(); + return row?.chat_session_id ?? null; + } catch (err) { + log.warn('mcp.comments.session_lookup_failed', { + projectId: tokenData.projectId, + workspaceId: tokenData.workspaceId, + error: err instanceof Error ? err.message : String(err), + }); + return null; + } +} + +function isRpcError(value: { sessionId: string } | JsonRpcResponse): value is JsonRpcResponse { + return 'jsonrpc' in value; +} + +function mapCommentError( + requestId: string | number | null, + err: unknown, + logTag: string, + logContext: Record +): JsonRpcResponse { + if (isMessageCommentServiceError(err)) { + const code = err.code === 'unavailable' ? INTERNAL_ERROR : INVALID_PARAMS; + return jsonRpcError(requestId, code, err.message); + } + log.warn(logTag, { + ...logContext, + error: err instanceof Error ? err.message : String(err), + }); + return jsonRpcError(requestId, INTERNAL_ERROR, 'Comment tool failed'); +} + +function parseStatus( + requestId: string | number | null, + params: Record +): MessageCommentThreadStatus | 'all' | JsonRpcResponse { + const raw = optionalString(params, 'status') ?? 'open'; + if (raw === 'open' || raw === 'sent' || raw === 'resolved' || raw === 'all') return raw; + return jsonRpcError(requestId, INVALID_PARAMS, 'status must be open, sent, resolved, or all'); +} + +export async function handleListMessageCommentThreads( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + storage?: MessageCommentStorageAdapter +): Promise { + const identityError = rejectCallerDerivedFields(requestId, params); + if (identityError) return identityError; + const session = await resolveCallerSession(requestId, params, tokenData, env); + if (isRpcError(session)) return session; + const status = parseStatus(requestId, params); + if (typeof status !== 'string') return status; + + const config = getMessageCommentConfig(env); + const limit = clampCommentListLimit(params.limit, config); + const messageId = optionalString(params, 'messageId'); + const cursor = optionalString(params, 'cursor'); + + try { + const result = await getStorage(env, storage).listThreads(tokenData.projectId, { + sessionId: session.sessionId, + status, + messageId, + cursor, + limit, + }); + return toolSuccess(requestId, { + ...result, + threads: result.threads.map((thread) => boundCommentThreadSummary(thread, config)), + }); + } catch (err) { + return mapCommentError(requestId, err, 'mcp.comments.list_failed', { + projectId: tokenData.projectId, + sessionId: session.sessionId, + }); + } +} + +export async function handleGetMessageCommentThread( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + storage?: MessageCommentStorageAdapter +): Promise { + const identityError = rejectCallerDerivedFields(requestId, params); + if (identityError) return identityError; + const threadId = requiredString(requestId, params, 'threadId'); + if (typeof threadId !== 'string') return threadId; + const session = await resolveCallerSession(requestId, params, tokenData, env); + if (isRpcError(session)) return session; + const adapter = getStorage(env, storage); + const config = getMessageCommentConfig(env); + const author = buildAgentCommentAuthor(tokenData); + const provenance = buildAgentCommentProvenance(tokenData); + + try { + const thread = await adapter.getThread({ + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + }); + if (!thread) return jsonRpcError(requestId, INVALID_PARAMS, 'Comment thread not found'); + if (thread.sessionId !== session.sessionId) { + return jsonRpcError(requestId, INVALID_PARAMS, 'Comment thread belongs to another session'); + } + await adapter.markThreadObserved({ + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + observer: author, + provenance, + }); + return toolSuccess(requestId, { thread: boundCommentThread(thread, config) }); + } catch (err) { + return mapCommentError(requestId, err, 'mcp.comments.get_failed', { + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + }); + } +} + +export async function handleCreateMessageCommentThread( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + storage?: MessageCommentStorageAdapter +): Promise { + const identityError = rejectCallerDerivedFields(requestId, params); + if (identityError) return identityError; + const messageId = requiredString(requestId, params, 'messageId'); + if (typeof messageId !== 'string') return messageId; + const bodyRaw = requiredString(requestId, params, 'body'); + if (typeof bodyRaw !== 'string') return bodyRaw; + const session = await resolveCallerSession(requestId, params, tokenData, env); + if (isRpcError(session)) return session; + const config = getMessageCommentConfig(env); + const body = normalizeCommentBody(bodyRaw, config); + if (!body) return jsonRpcError(requestId, INVALID_PARAMS, 'body is required'); + + try { + const thread = await getStorage(env, storage).createThread({ + projectId: tokenData.projectId, + sessionId: session.sessionId, + messageId, + quote: normalizeCommentQuote(optionalString(params, 'quote'), config), + body, + author: buildAgentCommentAuthor(tokenData), + provenance: buildAgentCommentProvenance(tokenData), + }); + return toolSuccess(requestId, { thread: boundCommentThread(thread, config) }); + } catch (err) { + return mapCommentError(requestId, err, 'mcp.comments.create_failed', { + projectId: tokenData.projectId, + sessionId: session.sessionId, + messageId, + }); + } +} + +export async function handleReplyToMessageCommentThread( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + storage?: MessageCommentStorageAdapter +): Promise { + const identityError = rejectCallerDerivedFields(requestId, params); + if (identityError) return identityError; + const threadId = requiredString(requestId, params, 'threadId'); + if (typeof threadId !== 'string') return threadId; + const bodyRaw = requiredString(requestId, params, 'body'); + if (typeof bodyRaw !== 'string') return bodyRaw; + const session = await resolveCallerSession(requestId, params, tokenData, env); + if (isRpcError(session)) return session; + const config = getMessageCommentConfig(env); + const body = normalizeCommentBody(bodyRaw, config); + if (!body) return jsonRpcError(requestId, INVALID_PARAMS, 'body is required'); + + try { + const thread = await getStorage(env, storage).replyToThread({ + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + body, + author: buildAgentCommentAuthor(tokenData), + provenance: buildAgentCommentProvenance(tokenData), + }); + return toolSuccess(requestId, { thread: boundCommentThread(thread, config) }); + } catch (err) { + return mapCommentError(requestId, err, 'mcp.comments.reply_failed', { + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + }); + } +} + +async function updateThreadStatus( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + status: Extract, + storage?: MessageCommentStorageAdapter +): Promise { + const identityError = rejectCallerDerivedFields(requestId, params); + if (identityError) return identityError; + const threadId = requiredString(requestId, params, 'threadId'); + if (typeof threadId !== 'string') return threadId; + const session = await resolveCallerSession(requestId, params, tokenData, env); + if (isRpcError(session)) return session; + const config = getMessageCommentConfig(env); + + try { + const thread = await getStorage(env, storage).updateThreadStatus({ + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + status, + actor: buildAgentCommentAuthor(tokenData), + provenance: buildAgentCommentProvenance(tokenData), + }); + return toolSuccess(requestId, { thread: boundCommentThread(thread, config) }); + } catch (err) { + return mapCommentError(requestId, err, 'mcp.comments.status_failed', { + projectId: tokenData.projectId, + sessionId: session.sessionId, + threadId, + status, + }); + } +} + +export function handleResolveMessageCommentThread( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + storage?: MessageCommentStorageAdapter +): Promise { + return updateThreadStatus(requestId, params, tokenData, env, 'resolved', storage); +} + +export function handleReopenMessageCommentThread( + requestId: string | number | null, + params: Record, + tokenData: McpTokenData, + env: Env, + storage?: MessageCommentStorageAdapter +): Promise { + return updateThreadStatus(requestId, params, tokenData, env, 'open', storage); +} diff --git a/apps/api/src/routes/mcp/index.ts b/apps/api/src/routes/mcp/index.ts index be4d90c49..92bcd068e 100644 --- a/apps/api/src/routes/mcp/index.ts +++ b/apps/api/src/routes/mcp/index.ts @@ -26,6 +26,14 @@ import { MCP_TOOLS, METHOD_NOT_FOUND, } from './_helpers'; +import { + handleCreateMessageCommentThread, + handleGetMessageCommentThread, + handleListMessageCommentThreads, + handleReopenMessageCommentThread, + handleReplyToMessageCommentThread, + handleResolveMessageCommentThread, +} from './comment-tools'; import { handleBuildAndPublish, handleGetPublishStatus } from './compose-publish-tools'; import { handleGetDeploymentGuide } from './deployment-guide-tools'; import { @@ -311,6 +319,30 @@ mcpRoutes.post('/', async (c) => { return c.json(await handleGetPendingMessages(requestId, toolArgs, tokenData, c.env)); case 'ack_message': return c.json(await handleAckMessage(requestId, toolArgs, tokenData, c.env)); + case 'list_message_comment_threads': + return c.json( + await handleListMessageCommentThreads(requestId, toolArgs, tokenData, c.env) + ); + case 'get_message_comment_thread': + return c.json( + await handleGetMessageCommentThread(requestId, toolArgs, tokenData, c.env) + ); + case 'create_message_comment_thread': + return c.json( + await handleCreateMessageCommentThread(requestId, toolArgs, tokenData, c.env) + ); + case 'reply_to_message_comment_thread': + return c.json( + await handleReplyToMessageCommentThread(requestId, toolArgs, tokenData, c.env) + ); + case 'resolve_message_comment_thread': + return c.json( + await handleResolveMessageCommentThread(requestId, toolArgs, tokenData, c.env) + ); + case 'reopen_message_comment_thread': + return c.json( + await handleReopenMessageCommentThread(requestId, toolArgs, tokenData, c.env) + ); case 'send_message_to_subtask': return c.json(await handleSendMessageToSubtask(requestId, toolArgs, tokenData, c.env)); case 'stop_subtask': diff --git a/apps/api/src/routes/mcp/tool-definitions-comment-tools.ts b/apps/api/src/routes/mcp/tool-definitions-comment-tools.ts new file mode 100644 index 000000000..e4b81344d --- /dev/null +++ b/apps/api/src/routes/mcp/tool-definitions-comment-tools.ts @@ -0,0 +1,156 @@ +/** + * MCP tool definitions — message-anchored comment tools. + */ + +export const COMMENT_TOOLS = [ + { + name: 'list_message_comment_threads', + description: + 'List bounded message-anchored comment threads for your current SAM chat session. Use this to find open user feedback tied to exact source messages before changing related work. Returns summaries with quoted source-message context; use get_message_comment_thread for full replies.', + inputSchema: { + type: 'object' as const, + properties: { + sessionId: { + type: 'string', + description: + 'Optional safety check. If provided, it must match your current chat session; other sessions are rejected.', + }, + status: { + type: 'string', + enum: ['open', 'sent', 'resolved', 'all'], + description: 'Filter by thread status. Defaults to open.', + }, + messageId: { + type: 'string', + description: 'Optional source message ID filter.', + }, + cursor: { + type: 'string', + description: 'Opaque pagination cursor from the previous response.', + }, + limit: { + type: 'number', + description: 'Max threads to return (default 10, server capped).', + }, + }, + additionalProperties: false, + }, + }, + { + name: 'get_message_comment_thread', + description: + 'Inspect one message-anchored comment thread in your current SAM chat session, including replies and quoted source-message context. Calling this marks the thread observed by this agent through the comment read adapter.', + inputSchema: { + type: 'object' as const, + properties: { + threadId: { + type: 'string', + description: 'The comment thread ID to inspect.', + }, + sessionId: { + type: 'string', + description: + 'Optional safety check. If provided, it must match your current chat session; other sessions are rejected.', + }, + }, + required: ['threadId'], + additionalProperties: false, + }, + }, + { + name: 'create_message_comment_thread', + description: + 'Create an agent-authored message-anchored comment thread in your current SAM chat session. Author identity and provenance are derived from your verified MCP token; do not provide author fields.', + inputSchema: { + type: 'object' as const, + properties: { + messageId: { + type: 'string', + description: 'Source chat message ID to anchor the comment to.', + }, + quote: { + type: 'string', + description: 'Optional short quote from the source message for citation context.', + }, + body: { + type: 'string', + description: 'Comment body (server sanitized and length capped).', + }, + sessionId: { + type: 'string', + description: + 'Optional safety check. If provided, it must match your current chat session; other sessions are rejected.', + }, + }, + required: ['messageId', 'body'], + additionalProperties: false, + }, + }, + { + name: 'reply_to_message_comment_thread', + description: + 'Add an agent-authored reply to an existing message comment thread in your current SAM chat session. Author identity and provenance are derived from your verified MCP token.', + inputSchema: { + type: 'object' as const, + properties: { + threadId: { + type: 'string', + description: 'The comment thread ID to reply to.', + }, + body: { + type: 'string', + description: 'Reply body (server sanitized and length capped).', + }, + sessionId: { + type: 'string', + description: + 'Optional safety check. If provided, it must match your current chat session; other sessions are rejected.', + }, + }, + required: ['threadId', 'body'], + additionalProperties: false, + }, + }, + { + name: 'resolve_message_comment_thread', + description: + 'Resolve a message comment thread in your current SAM chat session after the feedback has been addressed. Actor identity is derived from your verified MCP token.', + inputSchema: { + type: 'object' as const, + properties: { + threadId: { + type: 'string', + description: 'The comment thread ID to resolve.', + }, + sessionId: { + type: 'string', + description: + 'Optional safety check. If provided, it must match your current chat session; other sessions are rejected.', + }, + }, + required: ['threadId'], + additionalProperties: false, + }, + }, + { + name: 'reopen_message_comment_thread', + description: + 'Reopen a resolved message comment thread in your current SAM chat session when more work is needed. Actor identity is derived from your verified MCP token.', + inputSchema: { + type: 'object' as const, + properties: { + threadId: { + type: 'string', + description: 'The comment thread ID to reopen.', + }, + sessionId: { + type: 'string', + description: + 'Optional safety check. If provided, it must match your current chat session; other sessions are rejected.', + }, + }, + required: ['threadId'], + additionalProperties: false, + }, + }, +]; diff --git a/apps/api/src/routes/mcp/tool-definitions.ts b/apps/api/src/routes/mcp/tool-definitions.ts index 2d1c23961..edd6ae486 100644 --- a/apps/api/src/routes/mcp/tool-definitions.ts +++ b/apps/api/src/routes/mcp/tool-definitions.ts @@ -9,10 +9,12 @@ * - tool-definitions-workspace-tools.ts (workspace info, env, CI, cost, onboarding) * - tool-definitions-library-tools.ts (project file library) * - tool-definitions-orchestration-tools.ts (agent-to-agent communication & control) + * - tool-definitions-comment-tools.ts (message comment threads) * - tool-definitions-trigger-tools.ts (trigger management — cron automation) * - tool-definitions-incident-tools.ts (private feedback incident backlog) */ +export { COMMENT_TOOLS } from './tool-definitions-comment-tools'; export { DEPLOYMENT_TOOLS } from './tool-definitions-deployment-tools'; export { INCIDENT_TOOLS } from './tool-definitions-incident-tools'; export { KNOWLEDGE_TOOLS } from './tool-definitions-knowledge-tools'; @@ -29,6 +31,7 @@ export { TASK_LIFECYCLE_TOOLS } from './tool-definitions-task-tools'; export { TRIGGER_TOOLS } from './tool-definitions-trigger-tools'; export { WORKSPACE_TOOLS } from './tool-definitions-workspace-tools'; +import { COMMENT_TOOLS } from './tool-definitions-comment-tools'; import { DEPLOYMENT_TOOLS } from './tool-definitions-deployment-tools'; import { INCIDENT_TOOLS } from './tool-definitions-incident-tools'; import { KNOWLEDGE_TOOLS } from './tool-definitions-knowledge-tools'; @@ -53,6 +56,7 @@ export const MCP_TOOLS = [ ...DEPLOYMENT_TOOLS, ...LIBRARY_TOOLS, ...ORCHESTRATION_TOOLS, + ...COMMENT_TOOLS, ...TRIGGER_TOOLS, ...INCIDENT_TOOLS, ...PROFILE_TOOLS, diff --git a/apps/api/src/services/message-comments.ts b/apps/api/src/services/message-comments.ts new file mode 100644 index 000000000..8cc68db16 --- /dev/null +++ b/apps/api/src/services/message-comments.ts @@ -0,0 +1,491 @@ +import type { + MessageCommentActorProvenance, + MessageCommentAuthor, + MessageCommentDirectiveState, + MessageCommentListRequest, + MessageCommentListResponse, + MessageCommentThread, + MessageCommentThreadStatus, + UpdateMessageCommentThreadStatusRequest, +} from '@simple-agent-manager/shared'; + +import { resolveDurableExecutionConfig } from '../durable-objects/project-data/durable-execution-config'; +import type { Env } from '../env'; +import { parsePositiveInt } from '../lib/route-helpers'; +import { sanitizeUserInput } from '../lib/sanitize-user-input'; +import * as projectDataService from './project-data'; + +const DEFAULT_MCP_COMMENT_LIST_LIMIT = 10; +const DEFAULT_MCP_COMMENT_LIST_MAX = 25; +const DEFAULT_MCP_COMMENT_BODY_MAX_LENGTH = 4_000; +const DEFAULT_MCP_COMMENT_QUOTE_MAX_LENGTH = 1_000; +const DEFAULT_COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH = 6_000; + +export interface MessageCommentConfigEnv { + MCP_COMMENT_LIST_LIMIT?: string; + MCP_COMMENT_LIST_MAX?: string; + MCP_COMMENT_BODY_MAX_LENGTH?: string; + MCP_COMMENT_QUOTE_MAX_LENGTH?: string; + COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH?: string; +} + +export interface MessageCommentConfig { + listLimit: number; + listMax: number; + bodyMaxLength: number; + quoteMaxLength: number; + directiveContextMaxLength: number; +} + +export function getMessageCommentConfig(env: MessageCommentConfigEnv): MessageCommentConfig { + const listLimit = parsePositiveInt(env.MCP_COMMENT_LIST_LIMIT, DEFAULT_MCP_COMMENT_LIST_LIMIT); + const listMax = parsePositiveInt(env.MCP_COMMENT_LIST_MAX, DEFAULT_MCP_COMMENT_LIST_MAX); + return { + listLimit, + listMax: Math.max(listLimit, listMax), + bodyMaxLength: parsePositiveInt( + env.MCP_COMMENT_BODY_MAX_LENGTH, + DEFAULT_MCP_COMMENT_BODY_MAX_LENGTH + ), + quoteMaxLength: parsePositiveInt( + env.MCP_COMMENT_QUOTE_MAX_LENGTH, + DEFAULT_MCP_COMMENT_QUOTE_MAX_LENGTH + ), + directiveContextMaxLength: parsePositiveInt( + env.COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH, + DEFAULT_COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH + ), + }; +} + +export type MessageCommentErrorCode = + | 'invalid_request' + | 'not_found' + | 'forbidden' + | 'conflict' + | 'unavailable'; + +export class MessageCommentServiceError extends Error { + constructor( + readonly code: MessageCommentErrorCode, + message: string + ) { + super(message); + this.name = 'MessageCommentServiceError'; + } +} + +export function isMessageCommentServiceError(err: unknown): err is MessageCommentServiceError { + return err instanceof MessageCommentServiceError; +} + +export interface MessageCommentThreadLookup { + projectId: string; + sessionId: string; + threadId: string; +} + +export interface CreateMessageCommentThreadInput { + projectId: string; + sessionId: string; + messageId: string; + quote: string | null; + body: string; + author: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +} + +export interface ReplyToMessageCommentThreadInput { + projectId: string; + sessionId: string; + threadId: string; + body: string; + author: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +} + +export interface ObserveMessageCommentThreadInput extends MessageCommentThreadLookup { + observer: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +} + +export interface RecordMessageCommentDirectiveInput extends MessageCommentThreadLookup { + delivery: MessageCommentDirectiveState; + sentByUserId: string; + sentAt: number; +} + +export interface MessageCommentStorageAdapter { + listThreads( + projectId: string, + input: MessageCommentListRequest + ): Promise; + getThread(input: MessageCommentThreadLookup): Promise; + createThread(input: CreateMessageCommentThreadInput): Promise; + replyToThread(input: ReplyToMessageCommentThreadInput): Promise; + updateThreadStatus( + input: { projectId: string } & UpdateMessageCommentThreadStatusRequest + ): Promise; + markThreadObserved(input: ObserveMessageCommentThreadInput): Promise<{ observed: boolean }>; + recordDirectiveDelivery( + input: RecordMessageCommentDirectiveInput + ): Promise; +} + +function getAuthorDisplayName(author: MessageCommentAuthor): string | null { + return author.displayName ?? author.name ?? null; +} + +function toStorageActor(author: MessageCommentAuthor): MessageCommentAuthor { + const displayName = getAuthorDisplayName(author); + return { + kind: author.kind, + id: author.id, + name: displayName, + displayName, + }; +} + +function withCommentDisplayNames(thread: MessageCommentThread): MessageCommentThread { + const mapAuthor = (author: MessageCommentAuthor): MessageCommentAuthor => ({ + ...author, + displayName: getAuthorDisplayName(author), + }); + return { + ...thread, + author: mapAuthor(thread.author), + sentBy: thread.sentBy ? mapAuthor(thread.sentBy) : (thread.sentBy ?? null), + resolvedBy: thread.resolvedBy ? mapAuthor(thread.resolvedBy) : (thread.resolvedBy ?? null), + reopenedBy: thread.reopenedBy ? mapAuthor(thread.reopenedBy) : (thread.reopenedBy ?? null), + replies: thread.replies.map((reply) => ({ + ...reply, + author: mapAuthor(reply.author), + })), + }; +} + +function cursorToAfterSequence(cursor: string | null | undefined): number | null { + if (!cursor) return null; + const parsed = Number.parseInt(cursor, 10); + if (!Number.isFinite(parsed) || parsed < 0) { + throw new MessageCommentServiceError('invalid_request', 'cursor must be a comment sequence'); + } + return parsed; +} + +function addNextCursor(result: MessageCommentListResponse): MessageCommentListResponse { + const lastThread = result.threads.at(-1); + const nextCursor = + result.hasMore && typeof lastThread?.sequence === 'number' ? String(lastThread.sequence) : null; + return { + ...result, + threads: result.threads.map(withCommentDisplayNames), + nextCursor, + }; +} + +export function createProjectDataMessageCommentAdapter(env: Env): MessageCommentStorageAdapter { + return { + async listThreads(projectId, input) { + const result = await projectDataService.listCommentThreads(env, projectId, { + sessionId: input.sessionId, + messageId: input.messageId ?? null, + status: input.status === 'all' ? null : (input.status ?? null), + afterSequence: cursorToAfterSequence(input.cursor), + limit: input.limit, + }); + return addNextCursor(result); + }, + async getThread(input) { + const thread = await projectDataService.getCommentThread( + env, + input.projectId, + input.sessionId, + input.threadId + ); + return thread ? withCommentDisplayNames(thread) : null; + }, + async createThread(input) { + const result = await projectDataService.createCommentThread(env, input.projectId, { + sessionId: input.sessionId, + messageId: input.messageId, + quote: input.quote, + body: input.body, + clientMutationId: null, + actor: toStorageActor(input.author), + }); + return withCommentDisplayNames(result.thread); + }, + async replyToThread(input) { + const result = await projectDataService.createCommentReply(env, input.projectId, { + sessionId: input.sessionId, + threadId: input.threadId, + body: input.body, + clientMutationId: null, + actor: toStorageActor(input.author), + }); + return withCommentDisplayNames(result.thread); + }, + async updateThreadStatus(input) { + const result = await projectDataService.updateCommentThreadStatus(env, input.projectId, { + sessionId: input.sessionId, + threadId: input.threadId, + status: input.status, + clientMutationId: null, + actor: toStorageActor(input.actor), + }); + return withCommentDisplayNames(result.thread); + }, + async markThreadObserved() { + return { observed: false }; + }, + async recordDirectiveDelivery(input) { + const result = await projectDataService.updateCommentThreadStatus(env, input.projectId, { + sessionId: input.sessionId, + threadId: input.threadId, + status: 'sent', + clientMutationId: input.delivery.deliveryId, + actor: { + kind: 'human', + id: input.sentByUserId, + name: null, + displayName: null, + }, + }); + return { + ...withCommentDisplayNames(result.thread), + directive: input.delivery, + }; + }, + }; +} + +export function normalizeCommentBody(raw: string, config: MessageCommentConfig): string { + return redactSensitiveText(sanitizeUserInput(raw).trim()).slice(0, config.bodyMaxLength); +} + +export function normalizeCommentQuote( + raw: string | null | undefined, + config: MessageCommentConfig +): string | null { + if (typeof raw !== 'string') return null; + const normalized = redactSensitiveText(sanitizeUserInput(raw).trim()).slice( + 0, + config.quoteMaxLength + ); + return normalized || null; +} + +export function clampCommentListLimit(rawLimit: unknown, config: MessageCommentConfig): number { + if (typeof rawLimit !== 'number' || !Number.isFinite(rawLimit) || rawLimit <= 0) { + return config.listLimit; + } + return Math.min(Math.floor(rawLimit), config.listMax); +} + +export function boundCommentThreadSummary< + T extends { + body: string; + anchor: { quote: string | null }; + sourceMessage?: { quote: string | null } | null; + }, +>(thread: T, config: MessageCommentConfig): T { + return { + ...thread, + body: normalizeCommentBody(thread.body, config), + anchor: { + ...thread.anchor, + quote: normalizeCommentQuote(thread.anchor.quote, config), + }, + ...(Object.prototype.hasOwnProperty.call(thread, 'sourceMessage') + ? { + sourceMessage: thread.sourceMessage + ? { + ...thread.sourceMessage, + quote: normalizeCommentQuote(thread.sourceMessage.quote, config), + } + : null, + } + : {}), + }; +} + +export function boundCommentThread( + thread: MessageCommentThread, + config: MessageCommentConfig +): MessageCommentThread { + const summary = boundCommentThreadSummary(thread, config); + return { + ...summary, + replies: thread.replies.map((reply) => ({ + ...reply, + body: normalizeCommentBody(reply.body, config), + })), + }; +} + +export function buildAgentCommentAuthor(tokenData: { + agentSessionId?: string; + workspaceId?: string; + taskId?: string; +}): MessageCommentAuthor { + const displayName = 'SAM agent'; + return { + kind: 'agent', + id: tokenData.agentSessionId || tokenData.workspaceId || tokenData.taskId || 'agent', + displayName, + }; +} + +export function buildAgentCommentProvenance(tokenData: { + projectId: string; + userId: string; + taskId?: string; + workspaceId?: string; + agentSessionId?: string; +}): MessageCommentActorProvenance { + return { + projectId: tokenData.projectId, + userId: tokenData.userId, + taskId: tokenData.taskId || null, + workspaceId: tokenData.workspaceId || null, + agentSessionId: tokenData.agentSessionId || null, + }; +} + +export function buildCommentDirectiveDeliveryId(threadId: string): string { + return `comment-directive-${threadId}`; +} + +export interface SendMessageCommentDirectiveInput { + env: Env; + storage: MessageCommentStorageAdapter; + projectId: string; + sessionId: string; + threadId: string; + humanUserId: string; + now?: number; +} + +export interface SendMessageCommentDirectiveResult { + accepted: boolean; + deliveryId: string; + messageId: string; + thread: MessageCommentThread; + duplicate: boolean; +} + +export async function sendMessageCommentDirective( + input: SendMessageCommentDirectiveInput +): Promise { + const durableConfig = resolveDurableExecutionConfig(input.env); + if (!durableConfig.deliveryEnabled) { + throw new MessageCommentServiceError( + 'conflict', + 'Durable prompt delivery is disabled for comment directives' + ); + } + + const thread = await input.storage.getThread({ + projectId: input.projectId, + sessionId: input.sessionId, + threadId: input.threadId, + }); + if (!thread) { + throw new MessageCommentServiceError('not_found', 'Comment thread not found'); + } + if (thread.sessionId !== input.sessionId) { + throw new MessageCommentServiceError( + 'forbidden', + 'Comment thread belongs to a different session' + ); + } + if (thread.status === 'resolved') { + throw new MessageCommentServiceError('conflict', 'Resolved comment threads cannot be sent'); + } + + const config = getMessageCommentConfig(input.env); + const deliveryId = buildCommentDirectiveDeliveryId(thread.id); + const content = buildDirectivePrompt(thread, config); + const accepted = await projectDataService.acceptPromptDelivery(input.env, input.projectId, { + deliveryId, + targetSessionId: input.sessionId, + displayContent: content, + deliveryContent: content, + sourceTaskId: thread.taskId ?? null, + senderType: 'human', + senderId: input.humanUserId, + messageClass: 'deliver', + sourceKind: 'comment_directive', + ttlMs: durableConfig.ttlMs, + metadata: { + commentThreadId: thread.id, + sourceMessageId: thread.anchor.messageId, + quote: thread.anchor.quote, + author: { + kind: thread.author.kind, + displayName: getAuthorDisplayName(thread.author), + }, + }, + }); + + const sentAt = input.now ?? Date.now(); + const delivery: MessageCommentDirectiveState = { + deliveryId: accepted.message.id, + deliveryState: accepted.message.deliveryState, + promptMessageId: accepted.message.promptMessageId ?? accepted.transcriptMessageId, + acceptedAt: accepted.message.acceptedAt ?? null, + ackedAt: accepted.message.ackedAt ?? null, + }; + const recordedThread = + (await input.storage.recordDirectiveDelivery({ + projectId: input.projectId, + sessionId: input.sessionId, + threadId: thread.id, + delivery, + sentByUserId: input.humanUserId, + sentAt, + })) ?? thread; + + return { + accepted: true, + deliveryId: accepted.message.id, + messageId: accepted.transcriptMessageId, + thread: recordedThread, + duplicate: !accepted.transcriptInserted, + }; +} + +function buildDirectivePrompt(thread: MessageCommentThread, config: MessageCommentConfig): string { + const author = getAuthorDisplayName(thread.author) || `${thread.author.kind}:${thread.author.id}`; + const quote = normalizeCommentQuote( + thread.anchor.quote ?? thread.sourceMessage?.quote ?? null, + config + ); + const body = normalizeCommentBody(thread.body, config); + const sections = [ + 'SAM comment directive', + `Comment ID: ${thread.id}`, + `Source message ID: ${thread.anchor.messageId}`, + `Author: ${author}`, + quote ? `Quoted context:\n${quote}` : null, + `Feedback:\n${body}`, + ].filter((section): section is string => Boolean(section)); + return sections.join('\n\n').slice(0, config.directiveContextMaxLength); +} + +function redactSensitiveText(value: string): string { + return value + .replace(/\b(Bearer)\s+[A-Za-z0-9._~+/=-]{12,}/gi, '$1 [redacted]') + .replace(/\b(token|api[_-]?key|secret|password)\s*[:=]\s*[^\s'"`<>]{8,}/gi, '$1=[redacted]') + .replace(/\b(ghp|gho|ghu|ghs|ghr)_[A-Za-z0-9_]{20,}\b/g, '[redacted]') + .replace(/\bgithub_pat_[A-Za-z0-9_]{20,}\b/g, '[redacted]') + .replace(/\bsk-[A-Za-z0-9_-]{20,}\b/g, '[redacted]'); +} + +export function assertCommentStatus(value: string): MessageCommentThreadStatus | 'all' | null { + if (value === 'open' || value === 'sent' || value === 'resolved' || value === 'all') { + return value; + } + return null; +} diff --git a/apps/api/src/services/project-data.ts b/apps/api/src/services/project-data.ts index dd7e7b347..9a1733e5b 100644 --- a/apps/api/src/services/project-data.ts +++ b/apps/api/src/services/project-data.ts @@ -18,6 +18,7 @@ import type { MessageClass, MessageCommentListResponse, MessageCommentMutationResponse, + MessageCommentThread, MessageCommentReplyMutationResponse, SessionActivityTerminalReason, } from '@simple-agent-manager/shared'; @@ -425,6 +426,17 @@ export async function listCommentThreads( ); } +export async function getCommentThread( + env: Env, + projectId: string, + sessionId: string, + threadId: string +): Promise { + return callProjectDataWithRetry(env, projectId, 'getCommentThread', (stub) => + stub.getCommentThread({ sessionId, threadId }) + ); +} + export async function createCommentThread( env: Env, projectId: string, diff --git a/apps/api/tests/unit/durable-objects/durable-prompt-delivery.test.ts b/apps/api/tests/unit/durable-objects/durable-prompt-delivery.test.ts index bf9fd6bfa..8062ed510 100644 --- a/apps/api/tests/unit/durable-objects/durable-prompt-delivery.test.ts +++ b/apps/api/tests/unit/durable-objects/durable-prompt-delivery.test.ts @@ -1,3 +1,4 @@ +import type { PromptDeliverySource } from '@simple-agent-manager/shared'; import Database from 'better-sqlite3'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -74,7 +75,7 @@ describe('ProjectData durable prompt delivery', () => { function accept( deliveryId = 'delivery-1', ttlMs = config.ttlMs, - sourceKind: 'user_followup' | 'agent_mailbox' = 'user_followup' + sourceKind: PromptDeliverySource = 'user_followup' ) { return acceptPromptDelivery( sql, @@ -112,6 +113,35 @@ describe('ProjectData durable prompt delivery', () => { expect(first.message.promptMessageId).toBe(first.transcriptMessageId); }); + it('claims same-priority same-timestamp comment directives in insertion order', () => { + accept('comment-directive-thread-1', config.ttlMs, 'comment_directive'); + accept('comment-directive-thread-2', config.ttlMs, 'comment_directive'); + + sql.exec( + `UPDATE session_inbox + SET created_at = ?, next_attempt_at = ? + WHERE id IN ('comment-directive-thread-1', 'comment-directive-thread-2')`, + 10_000, + 10_000 + ); + + expect(mailbox.getPendingMessages(sql, 'chat-1').map((message) => message.id)).toEqual([ + 'comment-directive-thread-1', + 'comment-directive-thread-2', + ]); + + const claims = claimDuePromptDeliveries(sql, config, 10_000); + + expect(claims.map((claim) => claim.message.id)).toEqual([ + 'comment-directive-thread-1', + 'comment-directive-thread-2', + ]); + expect(claims.map((claim) => claim.message.sourceKind)).toEqual([ + 'comment_directive', + 'comment_directive', + ]); + }); + it('rejects reuse of a stable delivery identity for different prompt intent', () => { accept(); expect(() => diff --git a/apps/api/tests/unit/routes/chat-comment-directives.test.ts b/apps/api/tests/unit/routes/chat-comment-directives.test.ts new file mode 100644 index 000000000..cee9d4dc1 --- /dev/null +++ b/apps/api/tests/unit/routes/chat-comment-directives.test.ts @@ -0,0 +1,341 @@ +import type { AgentMailboxMessage, MessageCommentThread } from '@simple-agent-manager/shared'; +import { drizzle } from 'drizzle-orm/d1'; +import { Hono } from 'hono'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Env } from '../../../src/env'; +import { AppError } from '../../../src/middleware/error'; +import { registerChatCommentDirectiveRoute } from '../../../src/routes/chat-comment-directives'; + +vi.mock('drizzle-orm/d1', () => ({ + drizzle: vi.fn(() => ({ mockedDb: true })), +})); + +vi.mock('../../../src/middleware/auth', () => ({ + getUserId: vi.fn(() => 'human-1'), +})); + +vi.mock('../../../src/middleware/project-auth', () => ({ + requireProjectCapability: vi.fn().mockResolvedValue(undefined), +})); + +type ProjectDataCommentTestStub = { + ensureProjectId: ReturnType; + getSession: ReturnType; + getCommentThread: ReturnType; + updateCommentThreadStatus: ReturnType; + acceptPromptDelivery: ReturnType; +}; + +function makeApp(): Hono<{ Bindings: Env }> { + const root = new Hono<{ Bindings: Env }>(); + const routes = new Hono<{ Bindings: Env }>(); + registerChatCommentDirectiveRoute(routes); + root.route('/api/projects/:projectId/sessions', routes); + root.onError((err, c) => { + if (err instanceof AppError) { + return c.json(err.toJSON(), err.statusCode as never); + } + throw err; + }); + return root; +} + +function makeThread(overrides: Partial = {}): MessageCommentThread { + return { + id: 'thread-1', + sessionId: 'session-1', + taskId: 'task-1', + status: 'open', + anchor: { + kind: 'message', + messageId: 'message-1', + quote: 'Quote from the source message', + }, + body: 'Please address this specific comment.', + author: { + kind: 'human', + id: 'reviewer-1', + displayName: 'Reviewer', + }, + createdAt: 1000, + updatedAt: 1000, + resolvedAt: null, + replyCount: 1, + lastReplyAt: 1100, + sourceMessage: { + id: 'message-1', + role: 'assistant', + quote: 'Source message context', + createdAt: 900, + }, + directive: null, + replies: [ + { + id: 'reply-1', + body: 'Existing reply that must not be sent passively.', + author: { kind: 'human', id: 'reviewer-2', displayName: 'Second reviewer' }, + createdAt: 1100, + }, + ], + ...overrides, + }; +} + +function makeMailboxMessage(id: string, duplicate: boolean): AgentMailboxMessage { + return { + id, + targetSessionId: 'session-1', + sourceTaskId: 'task-1', + senderType: 'human', + senderId: 'human-1', + messageClass: 'deliver', + deliveryState: duplicate ? 'delivered' : 'queued', + content: 'comment directive', + metadata: null, + ackRequired: false, + ackTimeoutMs: null, + deliveryAttempts: duplicate ? 1 : 0, + lastDeliveryAt: duplicate ? 2500 : null, + expiresAt: null, + createdAt: 2000, + deliveredAt: duplicate ? 2500 : null, + ackedAt: null, + sourceKind: 'comment_directive', + promptMessageId: `prompt-${id}`, + nextAttemptAt: null, + lastError: null, + terminalReason: null, + attemptId: duplicate ? 'attempt-1' : null, + attemptStartedAt: duplicate ? 2400 : null, + runtimeIdentity: duplicate ? 'agent-runtime-1' : null, + receiptState: duplicate ? 'accepted' : null, + receiptRuntimeIdentity: duplicate ? 'agent-runtime-1' : null, + receiptCheckedAt: duplicate ? 2600 : null, + acceptedAt: 2000, + adapterProtocolVersion: 1, + receiptSupported: true, + }; +} + +function makeEnv( + options: { + duplicate?: boolean; + session?: { id: string; taskId: string; projectId: string } | null; + thread?: MessageCommentThread | null; + durablePromptDeliveryEnabled?: string; + } = {} +): Env & { + _projectDataStub: ProjectDataCommentTestStub; +} { + const duplicate = options.duplicate ?? false; + const session = + options.session === undefined + ? { id: 'session-1', taskId: 'task-1', projectId: 'project-1' } + : options.session; + const thread = options.thread === undefined ? makeThread() : options.thread; + const deliveryId = 'comment-directive-thread-1'; + const projectDataStub: ProjectDataCommentTestStub = { + ensureProjectId: vi.fn().mockResolvedValue(undefined), + getSession: vi.fn().mockResolvedValue(session), + getCommentThread: vi.fn().mockResolvedValue(thread), + updateCommentThreadStatus: vi.fn(async () => ({ + thread: { + ...(thread ?? makeThread()), + status: 'sent', + }, + idempotent: false, + })), + acceptPromptDelivery: vi.fn().mockResolvedValue({ + message: makeMailboxMessage(deliveryId, duplicate), + transcriptMessageId: `prompt-${deliveryId}`, + transcriptInserted: !duplicate, + transcriptCreatedAt: 2000, + transcriptSequence: duplicate ? 0 : 1, + workspaceId: 'workspace-1', + }), + }; + const doId = { toString: () => 'project-data-project-1' }; + return { + DATABASE: { prepare: vi.fn() }, + PROJECT_DATA: { + idFromName: vi.fn().mockReturnValue(doId), + get: vi.fn().mockReturnValue(projectDataStub), + }, + PROMPT_DELIVERY_TTL_MS: '3600000', + PROMPT_DELIVERY_RECEIPT_TIMEOUT_MS: '30000', + PROMPT_DELIVERY_RETRY_BASE_MS: '5000', + PROMPT_DELIVERY_RETRY_MAX_MS: '300000', + PROMPT_DELIVERY_MAX_CANDIDATES_PER_ALARM: '5', + PROMPT_DELIVERY_MAX_ATTEMPTS: '5', + PROMPT_DELIVERY_BACKGROUND_TIMEOUT_MS: '5000', + PROMPT_DELIVERY_MIN_ALARM_DELAY_MS: '1000', + DURABLE_PROMPT_DELIVERY_ENABLED: options.durablePromptDeliveryEnabled, + MCP_COMMENT_BODY_MAX_LENGTH: '4000', + MCP_COMMENT_QUOTE_MAX_LENGTH: '1000', + COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH: '6000', + _projectDataStub: projectDataStub, + } as unknown as Env & { _projectDataStub: ProjectDataCommentTestStub }; +} + +describe('chat comment directive route', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('queues one comment directive for the authenticated project session', async () => { + const app = makeApp(); + const env = makeEnv(); + + const response = await app.request( + '/api/projects/project-1/sessions/session-1/comments/thread-1/send-to-agent', + { method: 'POST' }, + env + ); + + expect(response.status).toBe(202); + expect(drizzle).toHaveBeenCalledWith(env.DATABASE, expect.anything()); + expect(env._projectDataStub.getSession).toHaveBeenCalledWith('session-1'); + expect(env._projectDataStub.getCommentThread).toHaveBeenCalledWith({ + sessionId: 'session-1', + threadId: 'thread-1', + }); + expect(env._projectDataStub.acceptPromptDelivery).toHaveBeenCalledWith( + expect.objectContaining({ + deliveryId: 'comment-directive-thread-1', + targetSessionId: 'session-1', + sourceTaskId: 'task-1', + senderType: 'human', + senderId: 'human-1', + messageClass: 'deliver', + sourceKind: 'comment_directive', + metadata: { + commentThreadId: 'thread-1', + sourceMessageId: 'message-1', + quote: 'Quote from the source message', + author: { kind: 'human', displayName: 'Reviewer' }, + }, + }) + ); + + const deliveryContent = env._projectDataStub.acceptPromptDelivery.mock.calls[0]?.[0] + ?.deliveryContent as string; + expect(deliveryContent).toContain('Comment ID: thread-1'); + expect(deliveryContent).toContain('Source message ID: message-1'); + expect(deliveryContent).not.toContain('Existing reply that must not be sent passively'); + + const body = await response.json(); + expect(body).toEqual({ + accepted: true, + status: 'queued', + duplicate: false, + deliveryId: 'comment-directive-thread-1', + messageId: 'prompt-comment-directive-thread-1', + thread: { + id: 'thread-1', + status: 'sent', + directive: { + deliveryId: 'comment-directive-thread-1', + deliveryState: 'queued', + promptMessageId: 'prompt-comment-directive-thread-1', + acceptedAt: 2000, + ackedAt: null, + }, + }, + }); + }); + + it('surfaces idempotent retries as duplicate without changing route target', async () => { + const env = makeEnv({ duplicate: true }); + + const response = await makeApp().request( + '/api/projects/project-1/sessions/session-1/comments/thread-1/send-to-agent', + { method: 'POST' }, + env + ); + + expect(response.status).toBe(202); + expect(env._projectDataStub.acceptPromptDelivery).toHaveBeenCalledWith( + expect.objectContaining({ deliveryId: 'comment-directive-thread-1' }) + ); + await expect(response.json()).resolves.toMatchObject({ + status: 'duplicate', + duplicate: true, + deliveryId: 'comment-directive-thread-1', + }); + }); + + it('maps a missing chat session to 404 before touching comment storage', async () => { + const env = makeEnv({ session: null }); + + const response = await makeApp().request( + '/api/projects/project-1/sessions/session-1/comments/thread-1/send-to-agent', + { method: 'POST' }, + env + ); + + expect(response.status).toBe(404); + await expect(response.json()).resolves.toEqual({ + error: 'NOT_FOUND', + message: 'Chat session not found', + }); + expect(env._projectDataStub.getSession).toHaveBeenCalledWith('session-1'); + expect(env._projectDataStub.getCommentThread).not.toHaveBeenCalled(); + expect(env._projectDataStub.acceptPromptDelivery).not.toHaveBeenCalled(); + }); + + it('maps wrong-session comment threads to 403 without queuing delivery', async () => { + const env = makeEnv({ thread: makeThread({ sessionId: 'session-2' }) }); + + const response = await makeApp().request( + '/api/projects/project-1/sessions/session-1/comments/thread-1/send-to-agent', + { method: 'POST' }, + env + ); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toEqual({ + error: 'FORBIDDEN', + message: 'Comment thread belongs to a different session', + }); + expect(env._projectDataStub.acceptPromptDelivery).not.toHaveBeenCalled(); + expect(env._projectDataStub.updateCommentThreadStatus).not.toHaveBeenCalled(); + }); + + it('maps resolved comment threads to 409 without queuing delivery', async () => { + const env = makeEnv({ thread: makeThread({ status: 'resolved', resolvedAt: 1500 }) }); + + const response = await makeApp().request( + '/api/projects/project-1/sessions/session-1/comments/thread-1/send-to-agent', + { method: 'POST' }, + env + ); + + expect(response.status).toBe(409); + await expect(response.json()).resolves.toEqual({ + error: 'CONFLICT', + message: 'Resolved comment threads cannot be sent', + }); + expect(env._projectDataStub.acceptPromptDelivery).not.toHaveBeenCalled(); + expect(env._projectDataStub.updateCommentThreadStatus).not.toHaveBeenCalled(); + }); + + it('maps disabled durable delivery to 409 without false queued success', async () => { + const env = makeEnv({ durablePromptDeliveryEnabled: 'false' }); + + const response = await makeApp().request( + '/api/projects/project-1/sessions/session-1/comments/thread-1/send-to-agent', + { method: 'POST' }, + env + ); + + expect(response.status).toBe(409); + await expect(response.json()).resolves.toEqual({ + error: 'CONFLICT', + message: 'Durable prompt delivery is disabled for comment directives', + }); + expect(env._projectDataStub.getCommentThread).not.toHaveBeenCalled(); + expect(env._projectDataStub.acceptPromptDelivery).not.toHaveBeenCalled(); + expect(env._projectDataStub.updateCommentThreadStatus).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/tests/unit/routes/mcp-message-comments.test.ts b/apps/api/tests/unit/routes/mcp-message-comments.test.ts new file mode 100644 index 000000000..84bd56e35 --- /dev/null +++ b/apps/api/tests/unit/routes/mcp-message-comments.test.ts @@ -0,0 +1,392 @@ +import type { + MessageCommentThread, + MessageCommentThreadSummary, +} from '@simple-agent-manager/shared'; +import { describe, expect, it, vi } from 'vitest'; + +import type { Env } from '../../../src/env'; +import type { JsonRpcResponse, McpTokenData } from '../../../src/routes/mcp/_helpers'; +import { + handleCreateMessageCommentThread, + handleGetMessageCommentThread, + handleListMessageCommentThreads, + handleReopenMessageCommentThread, + handleReplyToMessageCommentThread, + handleResolveMessageCommentThread, +} from '../../../src/routes/mcp/comment-tools'; +import { + MessageCommentServiceError, + type MessageCommentStorageAdapter, +} from '../../../src/services/message-comments'; + +function makeToken(overrides: Partial = {}): McpTokenData { + return { + taskId: 'task-1', + projectId: 'project-1', + userId: 'user-1', + workspaceId: 'workspace-1', + chatSessionId: 'session-1', + agentSessionId: 'agent-session-1', + createdAt: '2026-08-21T00:00:00.000Z', + ...overrides, + }; +} + +function makeEnv( + overrides: Partial & { + lookedUpSessionId?: string | null; + } = {} +): Env & { + DATABASE: D1Database & { + _statement: { bind: ReturnType; first: ReturnType }; + }; +} { + const statement = { + bind: vi.fn().mockReturnThis(), + first: vi + .fn() + .mockResolvedValue({ chat_session_id: overrides.lookedUpSessionId ?? 'session-1' }), + }; + return { + MCP_COMMENT_LIST_LIMIT: '5', + MCP_COMMENT_LIST_MAX: '25', + MCP_COMMENT_BODY_MAX_LENGTH: '4000', + MCP_COMMENT_QUOTE_MAX_LENGTH: '1000', + COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH: '6000', + DATABASE: { + prepare: vi.fn().mockReturnValue(statement), + _statement: statement, + }, + ...overrides, + } as unknown as Env & { + DATABASE: D1Database & { + _statement: { bind: ReturnType; first: ReturnType }; + }; + }; +} + +function makeSummary( + overrides: Partial = {} +): MessageCommentThreadSummary { + return { + id: 'thread-1', + sessionId: 'session-1', + taskId: 'task-1', + status: 'open', + anchor: { + kind: 'message', + messageId: 'message-1', + quote: 'Original quoted context', + }, + body: 'Please address this feedback', + author: { + kind: 'human', + id: 'user-2', + displayName: 'Reviewer', + }, + createdAt: 1000, + updatedAt: 1000, + resolvedAt: null, + replyCount: 0, + lastReplyAt: null, + sourceMessage: { + id: 'message-1', + role: 'assistant', + quote: 'Assistant source quote', + createdAt: 900, + }, + directive: null, + ...overrides, + }; +} + +function makeThread(overrides: Partial = {}): MessageCommentThread { + return { + ...makeSummary(overrides), + replies: [], + ...overrides, + }; +} + +function makeStorage( + overrides: Partial = {} +): MessageCommentStorageAdapter { + return { + listThreads: vi.fn().mockResolvedValue({ + threads: [makeSummary()], + nextCursor: null, + hasMore: false, + }), + getThread: vi.fn().mockResolvedValue(makeThread()), + createThread: vi + .fn() + .mockResolvedValue( + makeThread({ author: { kind: 'agent', id: 'agent-session-1', displayName: 'SAM agent' } }) + ), + replyToThread: vi.fn().mockResolvedValue( + makeThread({ + replies: [ + { + id: 'reply-1', + body: 'Agent reply', + author: { kind: 'agent', id: 'agent-session-1', displayName: 'SAM agent' }, + createdAt: 1200, + }, + ], + }) + ), + updateThreadStatus: vi.fn().mockImplementation(async (input) => + makeThread({ + status: input.status, + resolvedAt: input.status === 'resolved' ? 1300 : null, + }) + ), + markThreadObserved: vi.fn().mockResolvedValue({ observed: true }), + recordDirectiveDelivery: vi.fn().mockResolvedValue(null), + ...overrides, + }; +} + +function parseToolResponse(response: JsonRpcResponse): unknown { + const result = response.result as { content: Array<{ text: string }> }; + return JSON.parse(result.content[0]?.text ?? '{}') as unknown; +} + +describe('MCP message comment tools', () => { + it('lists current-session threads with bounded pagination input', async () => { + const env = makeEnv({ MCP_COMMENT_LIST_LIMIT: '1', MCP_COMMENT_LIST_MAX: '2' }); + const storage = makeStorage({ + listThreads: vi.fn().mockResolvedValue({ + threads: [makeSummary(), makeSummary({ id: 'thread-2' })], + nextCursor: 'cursor-2', + hasMore: true, + }), + }); + + const response = await handleListMessageCommentThreads( + 1, + { status: 'all', limit: 99, cursor: 'cursor-1', messageId: 'message-1' }, + makeToken(), + env, + storage + ); + + expect(response.error).toBeUndefined(); + expect(storage.listThreads).toHaveBeenCalledWith('project-1', { + sessionId: 'session-1', + status: 'all', + messageId: 'message-1', + cursor: 'cursor-1', + limit: 2, + }); + expect(parseToolResponse(response)).toMatchObject({ + threads: [{ id: 'thread-1' }, { id: 'thread-2' }], + nextCursor: 'cursor-2', + hasMore: true, + }); + }); + + it('returns an empty list without dumping context', async () => { + const storage = makeStorage({ + listThreads: vi.fn().mockResolvedValue({ threads: [], nextCursor: null, hasMore: false }), + }); + + const response = await handleListMessageCommentThreads(1, {}, makeToken(), makeEnv(), storage); + + expect(response.error).toBeUndefined(); + expect(parseToolResponse(response)).toEqual({ + threads: [], + nextCursor: null, + hasMore: false, + }); + }); + + it('rejects cross-session tool arguments before storage access', async () => { + const storage = makeStorage(); + + const response = await handleListMessageCommentThreads( + 1, + { sessionId: 'session-2' }, + makeToken(), + makeEnv(), + storage + ); + + expect(response.error?.code).toBe(-32602); + expect(response.error?.message).toContain('sessionId must match'); + expect(storage.listThreads).not.toHaveBeenCalled(); + }); + + it('rejects caller-supplied identity and project fields', async () => { + const response = await handleCreateMessageCommentThread( + 1, + { + messageId: 'message-1', + body: 'feedback', + projectId: 'project-2', + author: { kind: 'human', id: 'spoofed' }, + }, + makeToken(), + makeEnv(), + makeStorage() + ); + + expect(response.error?.code).toBe(-32602); + expect(response.error?.message).toContain('projectId is derived'); + }); + + it('resolves the current session from a workspace using the token project fence', async () => { + const env = makeEnv({ lookedUpSessionId: 'session-from-workspace' }); + const storage = makeStorage(); + + await handleListMessageCommentThreads( + 1, + {}, + makeToken({ chatSessionId: undefined }), + env, + storage + ); + + expect(env.DATABASE.prepare).toHaveBeenCalledWith( + 'SELECT chat_session_id FROM workspaces WHERE id = ? AND project_id = ?' + ); + expect(env.DATABASE._statement.bind).toHaveBeenCalledWith('workspace-1', 'project-1'); + expect(storage.listThreads).toHaveBeenCalledWith( + 'project-1', + expect.objectContaining({ sessionId: 'session-from-workspace' }) + ); + }); + + it('marks inspected threads observed with agent provenance', async () => { + const storage = makeStorage(); + + const response = await handleGetMessageCommentThread( + 1, + { threadId: 'thread-1' }, + makeToken(), + makeEnv(), + storage + ); + + expect(response.error).toBeUndefined(); + expect(storage.markThreadObserved).toHaveBeenCalledWith({ + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + observer: { kind: 'agent', id: 'agent-session-1', displayName: 'SAM agent' }, + provenance: { + projectId: 'project-1', + userId: 'user-1', + taskId: 'task-1', + workspaceId: 'workspace-1', + agentSessionId: 'agent-session-1', + }, + }); + expect(parseToolResponse(response)).toMatchObject({ thread: { id: 'thread-1' } }); + }); + + it('creates and replies with token-derived agent provenance', async () => { + const storage = makeStorage(); + + await handleCreateMessageCommentThread( + 1, + { messageId: 'message-1', quote: 'token=secret-value-123', body: 'Use the safer path' }, + makeToken(), + makeEnv(), + storage + ); + await handleReplyToMessageCommentThread( + 2, + { threadId: 'thread-1', body: 'I addressed it' }, + makeToken(), + makeEnv(), + storage + ); + + expect(storage.createThread).toHaveBeenCalledWith( + expect.objectContaining({ + projectId: 'project-1', + sessionId: 'session-1', + messageId: 'message-1', + quote: 'token=[redacted]', + body: 'Use the safer path', + author: { kind: 'agent', id: 'agent-session-1', displayName: 'SAM agent' }, + }) + ); + expect(storage.replyToThread).toHaveBeenCalledWith( + expect.objectContaining({ + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + body: 'I addressed it', + author: { kind: 'agent', id: 'agent-session-1', displayName: 'SAM agent' }, + }) + ); + }); + + it('resolves and reopens threads through the scoped status contract', async () => { + const storage = makeStorage(); + + await handleResolveMessageCommentThread( + 1, + { threadId: 'thread-1' }, + makeToken(), + makeEnv(), + storage + ); + await handleReopenMessageCommentThread( + 2, + { threadId: 'thread-1' }, + makeToken(), + makeEnv(), + storage + ); + + expect(storage.updateThreadStatus).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + status: 'resolved', + projectId: 'project-1', + sessionId: 'session-1', + }) + ); + expect(storage.updateThreadStatus).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ status: 'open', projectId: 'project-1', sessionId: 'session-1' }) + ); + }); + + it('returns deterministic safe errors for backend gaps and unexpected failures', async () => { + const unavailableResponse = await handleListMessageCommentThreads( + 1, + {}, + makeToken(), + makeEnv(), + makeStorage({ + listThreads: vi + .fn() + .mockRejectedValue( + new MessageCommentServiceError('unavailable', 'Comment storage backend is unavailable') + ), + }) + ); + const unexpectedResponse = await handleListMessageCommentThreads( + 2, + {}, + makeToken(), + makeEnv(), + makeStorage({ + listThreads: vi.fn().mockRejectedValue(new Error('raw backend stack with token=leak')), + }) + ); + + expect(unavailableResponse.error).toEqual({ + code: -32603, + message: 'Comment storage backend is unavailable', + }); + expect(unexpectedResponse.error).toEqual({ + code: -32603, + message: 'Comment tool failed', + }); + }); +}); diff --git a/apps/api/tests/unit/routes/mcp.test.ts b/apps/api/tests/unit/routes/mcp.test.ts index a9fb323c7..235b2afc9 100644 --- a/apps/api/tests/unit/routes/mcp.test.ts +++ b/apps/api/tests/unit/routes/mcp.test.ts @@ -229,6 +229,15 @@ const mockDoStub = { updateSessionTopic: vi.fn().mockResolvedValue(true), getAllHighConfidenceKnowledge: vi.fn().mockResolvedValue([]), getActivePolicies: vi.fn().mockResolvedValue([]), + listMessageCommentThreads: vi + .fn() + .mockResolvedValue({ threads: [], nextCursor: null, hasMore: false }), + getMessageCommentThread: vi.fn().mockResolvedValue(null), + createMessageCommentThread: vi.fn(), + replyToMessageCommentThread: vi.fn(), + updateMessageCommentThreadStatus: vi.fn(), + markMessageCommentThreadObserved: vi.fn(), + recordMessageCommentDirectiveDelivery: vi.fn(), createAttentionMarker: vi.fn().mockResolvedValue({ id: 'marker-1', createdAt: Date.now(), @@ -484,6 +493,13 @@ describe('MCP Routes', () => { expect(toolNames).toContain('search_messages'); expect(toolNames).toContain('update_session_topic'); expect(toolNames).toContain('dispatch_task'); + // Message-comment tools + expect(toolNames).toContain('list_message_comment_threads'); + expect(toolNames).toContain('get_message_comment_thread'); + expect(toolNames).toContain('create_message_comment_thread'); + expect(toolNames).toContain('reply_to_message_comment_thread'); + expect(toolNames).toContain('resolve_message_comment_thread'); + expect(toolNames).toContain('reopen_message_comment_thread'); // Orchestration communication tools expect(toolNames).toContain('send_message_to_subtask'); expect(toolNames).toContain('stop_subtask'); @@ -569,7 +585,7 @@ describe('MCP Routes', () => { expect(toolNames).toContain('get_incident'); expect(toolNames).toContain('claim_incident'); expect(toolNames).toContain('resolve_incident'); - expect(body.result.tools).toHaveLength(105); + expect(body.result.tools).toHaveLength(111); }); it('should include MUST call directive in get_instructions description', async () => { @@ -592,6 +608,53 @@ describe('MCP Routes', () => { } }); + it('advertises bounded message-comment schemas without caller identity fields', async () => { + const res = await mcpRequest(app, jsonRpcRequest('tools/list')); + + const body = await res.json(); + const listComments = body.result.tools.find( + (tool: { name: string }) => tool.name === 'list_message_comment_threads' + ); + const createComment = body.result.tools.find( + (tool: { name: string }) => tool.name === 'create_message_comment_thread' + ); + const replyComment = body.result.tools.find( + (tool: { name: string }) => tool.name === 'reply_to_message_comment_thread' + ); + + expect(listComments.description).toContain('current SAM chat session'); + expect(listComments.description).toContain('quoted source-message context'); + expect(listComments.inputSchema.additionalProperties).toBe(false); + expect(listComments.inputSchema.required).toBeUndefined(); + expect(Object.keys(listComments.inputSchema.properties)).toEqual([ + 'sessionId', + 'status', + 'messageId', + 'cursor', + 'limit', + ]); + expect(listComments.inputSchema.properties.status.enum).toEqual([ + 'open', + 'sent', + 'resolved', + 'all', + ]); + expect(listComments.inputSchema.properties.projectId).toBeUndefined(); + expect(listComments.inputSchema.properties.author).toBeUndefined(); + + expect(createComment.inputSchema.required).toEqual(['messageId', 'body']); + expect(Object.keys(createComment.inputSchema.properties)).toEqual([ + 'messageId', + 'quote', + 'body', + 'sessionId', + ]); + expect(createComment.description).toContain('verified MCP token'); + + expect(replyComment.inputSchema.required).toEqual(['threadId', 'body']); + expect(replyComment.description).toContain('Author identity and provenance'); + }); + it('should advertise list_triggers with only optional bounded filter inputs', async () => { const res = await mcpRequest(app, jsonRpcRequest('tools/list')); @@ -741,6 +804,33 @@ describe('MCP Routes', () => { mockEnv ); }); + + it('should dispatch list_message_comment_threads through tools/call', async () => { + mockKV.get.mockResolvedValue({ + ...validTokenData, + chatSessionId: 'chat-session-123', + agentSessionId: 'agent-session-123', + }); + + const res = await mcpRequest( + app, + jsonRpcRequest('tools/call', { + name: 'list_message_comment_threads', + arguments: { status: 'open', limit: 3 }, + }) + ); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.error).toBeUndefined(); + expect(mockDoStub.listMessageCommentThreads).toHaveBeenCalledWith({ + sessionId: 'chat-session-123', + status: 'open', + messageId: null, + cursor: null, + limit: 3, + }); + }); }); // ─── get_instructions ────────────────────────────────────────────── diff --git a/apps/api/tests/unit/services/message-comments.test.ts b/apps/api/tests/unit/services/message-comments.test.ts new file mode 100644 index 000000000..b51548a4d --- /dev/null +++ b/apps/api/tests/unit/services/message-comments.test.ts @@ -0,0 +1,381 @@ +import type { + AgentMailboxMessage, + MessageCommentThread, + MessageCommentThreadSummary, +} from '@simple-agent-manager/shared'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Env } from '../../../src/env'; +import { + buildCommentDirectiveDeliveryId, + MessageCommentServiceError, + type MessageCommentStorageAdapter, + sendMessageCommentDirective, +} from '../../../src/services/message-comments'; +import * as projectDataService from '../../../src/services/project-data'; + +const projectDataMocks = vi.hoisted(() => ({ + acceptPromptDelivery: vi.fn(), +})); + +vi.mock('../../../src/services/project-data', () => ({ + acceptPromptDelivery: projectDataMocks.acceptPromptDelivery, +})); + +function makeEnv(overrides: Partial = {}): Env { + return { + PROMPT_DELIVERY_TTL_MS: '3600000', + PROMPT_DELIVERY_RECEIPT_TIMEOUT_MS: '30000', + PROMPT_DELIVERY_RETRY_BASE_MS: '5000', + PROMPT_DELIVERY_RETRY_MAX_MS: '300000', + PROMPT_DELIVERY_MAX_CANDIDATES_PER_ALARM: '5', + PROMPT_DELIVERY_MAX_ATTEMPTS: '5', + PROMPT_DELIVERY_BACKGROUND_TIMEOUT_MS: '5000', + PROMPT_DELIVERY_MIN_ALARM_DELAY_MS: '1000', + MCP_COMMENT_BODY_MAX_LENGTH: '4000', + MCP_COMMENT_QUOTE_MAX_LENGTH: '1000', + COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH: '6000', + ...overrides, + } as unknown as Env; +} + +function makeSummary( + overrides: Partial = {} +): MessageCommentThreadSummary { + return { + id: 'thread-1', + sessionId: 'session-1', + taskId: 'task-1', + status: 'open', + anchor: { + kind: 'message', + messageId: 'message-1', + quote: 'Please use the project-scoped API.', + }, + body: 'The agent should cite this feedback and avoid broad context. token=secret-value-123', + author: { + kind: 'human', + id: 'user-2', + displayName: 'Reviewer', + }, + createdAt: 1000, + updatedAt: 1000, + resolvedAt: null, + replyCount: 2, + lastReplyAt: 1200, + sourceMessage: { + id: 'message-1', + role: 'assistant', + quote: 'Longer source message context', + createdAt: 900, + }, + directive: null, + ...overrides, + }; +} + +function makeThread(overrides: Partial = {}): MessageCommentThread { + return { + ...makeSummary(overrides), + replies: [ + { + id: 'reply-1', + body: 'This existing full-thread reply must not be injected into the directive payload.', + author: { kind: 'human', id: 'user-3', displayName: 'Second reviewer' }, + createdAt: 1100, + }, + ], + ...overrides, + }; +} + +function makeMailboxMessage(id: string, promptMessageId = `prompt-${id}`): AgentMailboxMessage { + return { + id, + targetSessionId: 'session-1', + sourceTaskId: 'task-1', + senderType: 'human', + senderId: 'human-1', + messageClass: 'deliver', + deliveryState: 'queued', + content: 'directive', + metadata: null, + ackRequired: false, + ackTimeoutMs: null, + deliveryAttempts: 0, + lastDeliveryAt: null, + expiresAt: null, + createdAt: 2000, + deliveredAt: null, + ackedAt: null, + sourceKind: 'comment_directive', + promptMessageId, + nextAttemptAt: null, + lastError: null, + terminalReason: null, + attemptId: null, + attemptStartedAt: null, + runtimeIdentity: null, + receiptState: null, + receiptRuntimeIdentity: null, + receiptCheckedAt: null, + acceptedAt: 2000, + adapterProtocolVersion: null, + receiptSupported: true, + }; +} + +function makeAcceptedDelivery(id: string, inserted: boolean) { + return { + message: makeMailboxMessage(id), + transcriptMessageId: `prompt-${id}`, + transcriptInserted: inserted, + transcriptCreatedAt: 2000, + transcriptSequence: inserted ? 1 : 0, + workspaceId: 'workspace-1', + } satisfies Awaited>; +} + +function makeStorage(threads: Record): MessageCommentStorageAdapter { + return { + listThreads: vi.fn(), + getThread: vi.fn(async ({ threadId }) => threads[threadId] ?? null), + createThread: vi.fn(), + replyToThread: vi.fn(), + updateThreadStatus: vi.fn(), + markThreadObserved: vi.fn(), + recordDirectiveDelivery: vi.fn(async ({ threadId, delivery }) => { + const thread = threads[threadId]; + return thread ? { ...thread, status: 'sent', directive: delivery } : null; + }), + }; +} + +describe('message comment directive service', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('enqueues a minimal comment directive through prompt delivery', async () => { + const thread = makeThread(); + const storage = makeStorage({ 'thread-1': thread }); + projectDataMocks.acceptPromptDelivery.mockResolvedValue( + makeAcceptedDelivery('comment-directive-thread-1', true) + ); + + const result = await sendMessageCommentDirective({ + env: makeEnv(), + storage, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + humanUserId: 'human-1', + now: 3000, + }); + + expect(result).toMatchObject({ + accepted: true, + duplicate: false, + deliveryId: 'comment-directive-thread-1', + messageId: 'prompt-comment-directive-thread-1', + thread: { id: 'thread-1', status: 'sent' }, + }); + + expect(projectDataService.acceptPromptDelivery).toHaveBeenCalledWith( + expect.anything(), + 'project-1', + expect.objectContaining({ + deliveryId: 'comment-directive-thread-1', + targetSessionId: 'session-1', + sourceTaskId: 'task-1', + senderType: 'human', + senderId: 'human-1', + messageClass: 'deliver', + sourceKind: 'comment_directive', + metadata: { + commentThreadId: 'thread-1', + sourceMessageId: 'message-1', + quote: 'Please use the project-scoped API.', + author: { + kind: 'human', + displayName: 'Reviewer', + }, + }, + }) + ); + + const deliveryInput = projectDataMocks.acceptPromptDelivery.mock.calls[0]?.[2] as { + displayContent: string; + deliveryContent: string; + }; + expect(deliveryInput.deliveryContent).toBe(deliveryInput.displayContent); + expect(deliveryInput.deliveryContent).toContain('SAM comment directive'); + expect(deliveryInput.deliveryContent).toContain('Comment ID: thread-1'); + expect(deliveryInput.deliveryContent).toContain('Source message ID: message-1'); + expect(deliveryInput.deliveryContent).toContain('Quoted context:'); + expect(deliveryInput.deliveryContent).toContain('Feedback:'); + expect(deliveryInput.deliveryContent).toContain('token=[redacted]'); + expect(deliveryInput.deliveryContent).not.toContain('full-thread reply'); + expect(storage.recordDirectiveDelivery).toHaveBeenCalledWith({ + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + delivery: expect.objectContaining({ + deliveryId: 'comment-directive-thread-1', + deliveryState: 'queued', + promptMessageId: 'prompt-comment-directive-thread-1', + }), + sentByUserId: 'human-1', + sentAt: 3000, + }); + }); + + it('uses a stable delivery id so browser retries are idempotent', async () => { + const storage = makeStorage({ 'thread-1': makeThread() }); + projectDataMocks.acceptPromptDelivery + .mockResolvedValueOnce(makeAcceptedDelivery('comment-directive-thread-1', true)) + .mockResolvedValueOnce(makeAcceptedDelivery('comment-directive-thread-1', false)); + + const input = { + env: makeEnv(), + storage, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + humanUserId: 'human-1', + }; + + const first = await sendMessageCommentDirective(input); + const second = await sendMessageCommentDirective(input); + + expect(first.duplicate).toBe(false); + expect(second.duplicate).toBe(true); + expect( + projectDataMocks.acceptPromptDelivery.mock.calls.map((call) => call[2].deliveryId) + ).toEqual([ + buildCommentDirectiveDeliveryId('thread-1'), + buildCommentDirectiveDeliveryId('thread-1'), + ]); + expect(storage.recordDirectiveDelivery).toHaveBeenCalledTimes(2); + }); + + it('fails closed when durable prompt delivery is disabled', async () => { + const storage = makeStorage({ 'thread-1': makeThread() }); + + await expect( + sendMessageCommentDirective({ + env: makeEnv({ DURABLE_PROMPT_DELIVERY_ENABLED: 'false' }), + storage, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + humanUserId: 'human-1', + }) + ).rejects.toMatchObject( + new MessageCommentServiceError( + 'conflict', + 'Durable prompt delivery is disabled for comment directives' + ) + ); + + expect(projectDataMocks.acceptPromptDelivery).not.toHaveBeenCalled(); + expect(storage.getThread).not.toHaveBeenCalled(); + expect(storage.recordDirectiveDelivery).not.toHaveBeenCalled(); + }); + + it('preserves FIFO intent for concurrent distinct comment sends', async () => { + const storage = makeStorage({ + 'thread-1': makeThread({ + id: 'thread-1', + anchor: { kind: 'message', messageId: 'message-1', quote: 'one' }, + }), + 'thread-2': makeThread({ + id: 'thread-2', + anchor: { kind: 'message', messageId: 'message-2', quote: 'two' }, + body: 'Second feedback', + }), + }); + projectDataMocks.acceptPromptDelivery + .mockResolvedValueOnce(makeAcceptedDelivery('comment-directive-thread-1', true)) + .mockResolvedValueOnce(makeAcceptedDelivery('comment-directive-thread-2', true)); + + await Promise.all([ + sendMessageCommentDirective({ + env: makeEnv(), + storage, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + humanUserId: 'human-1', + }), + sendMessageCommentDirective({ + env: makeEnv(), + storage, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-2', + humanUserId: 'human-1', + }), + ]); + + expect( + projectDataMocks.acceptPromptDelivery.mock.calls.map((call) => call[2].deliveryId) + ).toEqual(['comment-directive-thread-1', 'comment-directive-thread-2']); + expect( + projectDataMocks.acceptPromptDelivery.mock.calls.map((call) => call[2].sourceKind) + ).toEqual(['comment_directive', 'comment_directive']); + expect( + projectDataMocks.acceptPromptDelivery.mock.calls.map((call) => call[2].messageClass) + ).toEqual(['deliver', 'deliver']); + }); + + it('rejects missing, wrong-session, and resolved threads before delivery', async () => { + const missing = makeStorage({}); + await expect( + sendMessageCommentDirective({ + env: makeEnv(), + storage: missing, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'missing', + humanUserId: 'human-1', + }) + ).rejects.toMatchObject( + new MessageCommentServiceError('not_found', 'Comment thread not found') + ); + + const wrongSession = makeStorage({ + 'thread-1': makeThread({ sessionId: 'session-2' }), + }); + await expect( + sendMessageCommentDirective({ + env: makeEnv(), + storage: wrongSession, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + humanUserId: 'human-1', + }) + ).rejects.toMatchObject( + new MessageCommentServiceError('forbidden', 'Comment thread belongs to a different session') + ); + + const resolved = makeStorage({ + 'thread-1': makeThread({ status: 'resolved' }), + }); + await expect( + sendMessageCommentDirective({ + env: makeEnv(), + storage: resolved, + projectId: 'project-1', + sessionId: 'session-1', + threadId: 'thread-1', + humanUserId: 'human-1', + }) + ).rejects.toMatchObject( + new MessageCommentServiceError('conflict', 'Resolved comment threads cannot be sent') + ); + + expect(projectDataMocks.acceptPromptDelivery).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/www/src/content/docs/docs/reference/configuration.md b/apps/www/src/content/docs/docs/reference/configuration.md index f0208fa7c..30d38d44a 100644 --- a/apps/www/src/content/docs/docs/reference/configuration.md +++ b/apps/www/src/content/docs/docs/reference/configuration.md @@ -904,19 +904,24 @@ Applied via cloud-init on each node: ## MCP Tool Limits -| Variable | Default | Description | -| ----------------------------- | ------- | ------------------------------------------------------- | -| `MCP_IDEA_CONTEXT_MAX_LENGTH` | `500` | Max characters of idea context shown to agents | -| `MCP_IDEA_LIST_LIMIT` | `20` | Default page size for `list_ideas` | -| `MCP_IDEA_LIST_MAX` | `100` | Max page size for `list_ideas` | -| `MCP_IDEA_SEARCH_MAX` | `20` | Max results from `search_ideas` | -| `MCP_MESSAGE_SEARCH_MAX` | `20` | Max results from `search_messages` | -| `MCP_MESSAGE_LIST_LIMIT` | `50` | Default page size for `get_session_messages` | -| `MCP_MESSAGE_LIST_MAX` | `200` | Max messages per `get_session_messages` request | -| `MCP_TRIGGER_LIST_LIMIT` | `20` | Default page size for `list_triggers` | -| `MCP_TRIGGER_LIST_MAX` | `100` | Max triggers per `list_triggers` request | -| `MCP_INCIDENT_LIST_LIMIT` | `10` | Default page size for private `list_incident_queue` | -| `MCP_INCIDENT_LIST_MAX` | `50` | Max private incidents per `list_incident_queue` request | +| Variable | Default | Description | +| -------------------------------------- | ------- | ------------------------------------------------------- | +| `MCP_IDEA_CONTEXT_MAX_LENGTH` | `500` | Max characters of idea context shown to agents | +| `MCP_IDEA_LIST_LIMIT` | `20` | Default page size for `list_ideas` | +| `MCP_IDEA_LIST_MAX` | `100` | Max page size for `list_ideas` | +| `MCP_IDEA_SEARCH_MAX` | `20` | Max results from `search_ideas` | +| `MCP_MESSAGE_SEARCH_MAX` | `20` | Max results from `search_messages` | +| `MCP_MESSAGE_LIST_LIMIT` | `50` | Default page size for `get_session_messages` | +| `MCP_MESSAGE_LIST_MAX` | `200` | Max messages per `get_session_messages` request | +| `MCP_COMMENT_LIST_LIMIT` | `10` | Default page size for `list_message_comment_threads` | +| `MCP_COMMENT_LIST_MAX` | `25` | Max threads per `list_message_comment_threads` request | +| `MCP_COMMENT_BODY_MAX_LENGTH` | `4000` | Max comment/reply body characters accepted through MCP | +| `MCP_COMMENT_QUOTE_MAX_LENGTH` | `1000` | Max quoted source-message characters returned to agents | +| `COMMENT_DIRECTIVE_CONTEXT_MAX_LENGTH` | `6000` | Max send-to-agent comment directive prompt length | +| `MCP_TRIGGER_LIST_LIMIT` | `20` | Default page size for `list_triggers` | +| `MCP_TRIGGER_LIST_MAX` | `100` | Max triggers per `list_triggers` request | +| `MCP_INCIDENT_LIST_LIMIT` | `10` | Default page size for private `list_incident_queue` | +| `MCP_INCIDENT_LIST_MAX` | `50` | Max private incidents per `list_incident_queue` request | ## Web UI (Build-Time) diff --git a/docs/notes/2026-08-21-message-comment-directive-contract.md b/docs/notes/2026-08-21-message-comment-directive-contract.md new file mode 100644 index 000000000..f4a457ed3 --- /dev/null +++ b/docs/notes/2026-08-21-message-comment-directive-contract.md @@ -0,0 +1,110 @@ +# Message comment MCP/directive adapter contract + +This note documents the backend seam used by the MCP/agent-directive constituent +PR for the message-anchored commenting MVP. The backend constituent PR owns +ProjectData SQLite tables, migrations, indexing, and authoritative message +anchor validation. This PR only calls the methods below through +`apps/api/src/services/message-comments.ts`. + +## Scope + +- Message anchors only: `{ kind: "message", messageId, quote }`. +- No file comments, re-anchoring, mentions, reactions, or passive full-thread + context injection. +- All caller identity is derived outside ProjectData from the verified MCP token + or authenticated browser session. Backend RPCs must still enforce `projectId` + by the ProjectData object name and `sessionId` by stored thread rows. + +## ProjectData RPC methods required + +The backend sibling should implement these methods on the ProjectData Durable +Object stub: + +```ts +listMessageCommentThreads(input: { + sessionId: string; + status?: "open" | "sent" | "resolved" | "all"; + messageId?: string | null; + cursor?: string | null; + limit: number; +}): Promise<{ + threads: MessageCommentThreadSummary[]; + nextCursor: string | null; + hasMore: boolean; +}>; + +getMessageCommentThread(input: { + sessionId: string; + threadId: string; +}): Promise; + +createMessageCommentThread(input: { + sessionId: string; + messageId: string; + quote: string | null; + body: string; + author: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +}): Promise; + +replyToMessageCommentThread(input: { + sessionId: string; + threadId: string; + body: string; + author: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +}): Promise; + +updateMessageCommentThreadStatus(input: { + sessionId: string; + threadId: string; + status: "open" | "resolved"; + actor: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +}): Promise; + +markMessageCommentThreadObserved(input: { + sessionId: string; + threadId: string; + observer: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +}): Promise<{ observed: boolean }>; + +recordMessageCommentDirectiveDelivery(input: { + sessionId: string; + threadId: string; + delivery: MessageCommentDirectiveState; + sentByUserId: string; + sentAt: number; +}): Promise; +``` + +The shared TypeScript shapes live in +`packages/shared/src/types/comments.ts`. + +## Backend invariants + +- Every method is scoped to the ProjectData object selected by `projectId`; a + method must not accept or trust a caller-supplied `projectId`. +- `sessionId` must match the stored thread/session. Wrong-session access must + return null or a deterministic authorization error; it must never fall back to + a cross-session lookup. +- `createMessageCommentThread` must verify that `messageId` exists in the same + session and should preserve a bounded quote/source-message snapshot for agent + citation. +- `recordMessageCommentDirectiveDelivery` must be idempotent for the same + `delivery.deliveryId` and thread. Retries from the browser route use the + stable delivery ID `comment-directive-${threadId}`. +- Durable comment directive delivery must fail closed when + `DURABLE_PROMPT_DELIVERY_ENABLED=false`; callers must not report a queued + directive unless the ProjectData prompt-delivery engine is enabled. +- Delivery acknowledgement/read state is the existing ProjectData mailbox + lifecycle: `deliveryState`, `promptMessageId`, `acceptedAt`, and `ackedAt`. + Do not create a second inbox for comment directives. +- Prompt delivery and pending mailbox reads must order same-priority, + same-`created_at` rows by SQLite insertion order (`rowid ASC`) so + same-session comment directives have a deterministic FIFO tie-breaker at the + storage/claim boundary. +- Returned thread summaries must be ordered deterministically, cursor-paginated, + and bounded. The MCP layer applies an additional cap/redaction pass, but the + storage layer should not rely on callers to avoid unbounded scans. diff --git a/packages/shared/src/types/comments.ts b/packages/shared/src/types/comments.ts index 935337a0e..0923328cd 100644 --- a/packages/shared/src/types/comments.ts +++ b/packages/shared/src/types/comments.ts @@ -1,56 +1,110 @@ +/** + * Shared message-comment contracts for the message-anchored commenting MVP. + * + * This contract intentionally models message anchors only. File anchors, + * markdown block anchors, fuzzy re-anchoring, reactions, and mentions are + * outside this MVP slice. + */ + export const COMMENT_STATUSES = ['open', 'sent', 'resolved'] as const; export type CommentStatus = (typeof COMMENT_STATUSES)[number]; +export const MESSAGE_COMMENT_THREAD_STATUSES = COMMENT_STATUSES; +export type MessageCommentThreadStatus = CommentStatus; + export const COMMENT_AUTHOR_KINDS = ['human', 'agent'] as const; export type CommentAuthorKind = (typeof COMMENT_AUTHOR_KINDS)[number]; +export const MESSAGE_COMMENT_AUTHOR_KINDS = COMMENT_AUTHOR_KINDS; +export type MessageCommentAuthorKind = CommentAuthorKind; + +export type CommentAuthor = { + kind: CommentAuthorKind; + id: string; + /** + * Human-readable display name used by the REST/UI storage contract. + */ + name?: string | null; + /** + * Human-readable display name used by the MCP agent-facing contract. + * + * Both names are kept during integration because the backend and MCP + * constituents used different field names for the same concept. + */ + displayName?: string | null; +}; + +export type MessageCommentAuthor = CommentAuthor; + export type MessageCommentAnchor = { kind: 'message'; messageId: string; quote: string | null; }; -export type CommentAuthor = { - kind: CommentAuthorKind; +export type MessageCommentSourceMessageContext = { id: string; - name: string | null; + role: string | null; + quote: string | null; + createdAt: number | null; +}; + +export type MessageCommentDirectiveState = { + deliveryId: string; + deliveryState: string; + promptMessageId: string | null; + acceptedAt: number | null; + ackedAt: number | null; }; export type MessageCommentReply = { id: string; - threadId: string; - sessionId: string; - author: CommentAuthor; + threadId?: string; + sessionId?: string; + author: MessageCommentAuthor; body: string; createdAt: number; - sequence: number; - clientMutationId: string | null; + updatedAt?: number | null; + sequence?: number; + clientMutationId?: string | null; + sentToAgent?: boolean; }; export type MessageCommentThread = { id: string; + projectId?: string; sessionId: string; + taskId?: string | null; anchor: MessageCommentAnchor; - author: CommentAuthor; + author: MessageCommentAuthor; body: string; - status: CommentStatus; + status: MessageCommentThreadStatus; createdAt: number; updatedAt: number; - sequence: number; - version: number; - clientMutationId: string | null; - sentAt: number | null; - sentBy: CommentAuthor | null; - resolvedAt: number | null; - resolvedBy: CommentAuthor | null; - reopenedAt: number | null; - reopenedBy: CommentAuthor | null; + sequence?: number; + version?: number; + clientMutationId?: string | null; + sentAt?: number | null; + sentBy?: MessageCommentAuthor | null; + resolvedAt?: number | null; + resolvedBy?: MessageCommentAuthor | null; + reopenedAt?: number | null; + reopenedBy?: MessageCommentAuthor | null; + replyCount?: number; + lastReplyAt?: number | null; + sourceMessage?: MessageCommentSourceMessageContext | null; + directive?: MessageCommentDirectiveState | null; replies: MessageCommentReply[]; }; +export type MessageCommentThreadSummary = Omit & { + replies?: MessageCommentReply[]; +}; + export type MessageCommentListResponse = { threads: MessageCommentThread[]; hasMore: boolean; + nextCursor?: string | null; }; export type MessageCommentMutationResponse = { @@ -79,3 +133,44 @@ export type MessageCommentThreadEvent = { reason: MessageCommentThreadEventReason; }; }; + +export interface MessageCommentListRequest { + sessionId: string; + status?: MessageCommentThreadStatus | 'all'; + messageId?: string | null; + cursor?: string | null; + limit: number; +} + +export interface MessageCommentActorProvenance { + projectId: string; + userId: string; + taskId: string | null; + workspaceId: string | null; + agentSessionId: string | null; +} + +export interface CreateMessageCommentThreadRequest { + sessionId: string; + messageId: string; + quote?: string | null; + body: string; + author: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +} + +export interface ReplyToMessageCommentThreadRequest { + sessionId: string; + threadId: string; + body: string; + author: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +} + +export interface UpdateMessageCommentThreadStatusRequest { + sessionId: string; + threadId: string; + status: Extract; + actor: MessageCommentAuthor; + provenance: MessageCommentActorProvenance; +} diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 2adc5fe3d..aeb43a19a 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -224,16 +224,32 @@ export type { CommentAuthor, CommentAuthorKind, CommentStatus, + CreateMessageCommentThreadRequest, + MessageCommentActorProvenance, MessageCommentAnchor, + MessageCommentAuthor, + MessageCommentAuthorKind, + MessageCommentDirectiveState, + MessageCommentListRequest, MessageCommentListResponse, MessageCommentMutationResponse, MessageCommentReply, MessageCommentReplyMutationResponse, + MessageCommentSourceMessageContext, MessageCommentThread, MessageCommentThreadEvent, MessageCommentThreadEventReason, + MessageCommentThreadStatus, + MessageCommentThreadSummary, + ReplyToMessageCommentThreadRequest, + UpdateMessageCommentThreadStatusRequest, +} from './comments'; +export { + COMMENT_AUTHOR_KINDS, + COMMENT_STATUSES, + MESSAGE_COMMENT_AUTHOR_KINDS, + MESSAGE_COMMENT_THREAD_STATUSES, } from './comments'; -export { COMMENT_AUTHOR_KINDS, COMMENT_STATUSES } from './comments'; export { ATTACHMENT_DEFAULTS, COMPLETION_EVIDENCE_VERIFICATION_KINDS, diff --git a/packages/shared/src/types/mailbox.ts b/packages/shared/src/types/mailbox.ts index bbbd7c03f..fd2a30b54 100644 --- a/packages/shared/src/types/mailbox.ts +++ b/packages/shared/src/types/mailbox.ts @@ -61,6 +61,7 @@ export const DELIVERY_TERMINAL_STATES: readonly DeliveryState[] = [ export const PROMPT_DELIVERY_SOURCES = [ 'user_followup', + 'comment_directive', 'agent_mailbox', 'orchestration_handoff', 'checkpoint_continuation', diff --git a/tasks/archive/2026-08-21-message-comment-mcp-directives.md b/tasks/archive/2026-08-21-message-comment-mcp-directives.md new file mode 100644 index 000000000..1b3dc4240 --- /dev/null +++ b/tasks/archive/2026-08-21-message-comment-mcp-directives.md @@ -0,0 +1,82 @@ +# Message comment MCP/directives constituent PR + +## Problem + +SAM's message-anchored commenting MVP needs an agent-facing layer and an explicit +human "send to agent" directive path. This constituent PR owns the MCP tools and +directive delivery contract only. The backend sibling will provide ProjectData +comment storage/RPC, and the UI sibling will provide the human interface. + +Constraints: + +- Leave this as an open constituent PR to `main`. +- Do not merge. +- Do not deploy or mutate staging. +- Scope to message-anchored comments only. +- Exclude file comments/re-anchoring, @mentions, reactions, and unrelated tool changes. + +## Research findings + +- The MCP endpoint is centralized in `apps/api/src/routes/mcp/index.ts`, with + tool metadata in `apps/api/src/routes/mcp/tool-definitions*.ts` and shared + auth/rate-limit/sanitization helpers in `apps/api/src/routes/mcp/_helpers.ts`. +- MCP identity comes from the verified opaque token in + `apps/api/src/services/mcp-token.ts`; tools should derive project, user, task, + workspace, and current session from that token instead of accepting caller + supplied project identifiers. +- Existing durable follow-up delivery uses `projectDataService.acceptPromptDelivery` + with a `PromptDeliverySource`, persists a transcript message, enqueues one + mailbox item, and handles idempotency through `deliveryId`. +- Existing read/ack state lives in the durable mailbox and attention/message + lifecycle. Comment directives should reuse those fields instead of adding a + separate inbox. +- ProjectData RPC/storage types for comments are not present on `main`, so this + PR needs an explicit narrow adapter contract that the backend sibling can + implement without rewriting the MCP/directive layer. +- Existing broad MCP route tests assert exact tool discoverability; adding tools + requires updating that contract and focused handler/service tests. + +## Implementation checklist + +- [x] Add shared message-comment types and the comment directive delivery source. +- [x] Add a narrow message-comment service adapter contract with bounded input, + sanitized output, deterministic safe errors, session verification, and + directive delivery helper. +- [x] Add MCP tool definitions for listing, inspecting, creating, replying, + resolving, and reopening message comment threads. +- [x] Add MCP handlers wired to verified token identity and same-session checks. +- [x] Add a REST route for the explicit UI send-to-agent action that enqueues one + durable directive with minimal comment context and idempotent delivery ID. +- [x] Add focused tests for tool discoverability/contracts, empty/pagination, + project isolation, wrong-session rejection, spoof prevention, idempotency, + FIFO, provenance, and minimal-context directive payloads. +- [x] Document backend adapter assumptions. + +## Validation + +- `pnpm --filter @simple-agent-manager/shared build` — passed. +- `pnpm --filter @simple-agent-manager/api typecheck` — passed. +- `pnpm --filter @simple-agent-manager/api lint` — passed. +- `pnpm --filter @simple-agent-manager/api build` — passed. +- `pnpm --filter @simple-agent-manager/api test -- tests/unit/routes/mcp-message-comments.test.ts tests/unit/services/message-comments.test.ts tests/unit/routes/chat-comment-directives.test.ts tests/unit/routes/mcp.test.ts` + — passed, 4 files / 249 tests. +- `pnpm check:fast` — passed. Existing warning-only lint debt was reported in + unrelated UI/client files; no errors and no blocking type-boundary findings. +- `pnpm format:check` — passed after updating the API reference skill doc. +- `git diff --check` — passed. + +## Acceptance criteria + +- Agents can discover bounded open message-comment summaries for their current + session and inspect a full thread without passive full-thread context injection. +- Agents can create/reply/resolve/reopen only through verified-token + project/session-scoped handlers; author provenance is server-derived. +- Human send-to-agent delivery enqueues exactly one durable follow-up per stable + comment directive ID and preserves FIFO ordering under concurrent sends. +- The directive payload contains only comment ID, quoted message context, author, + and minimal thread metadata. +- Tests cover contracts, happy paths, empty/pagination behavior, isolation, + spoof/wrong-session rejection, idempotent send, FIFO, provenance, and bounded + payloads. +- Backend storage dependencies are isolated behind an explicit adapter contract + and documented for the primary integrator. From 671437fd72127279d66e62f1fe33e6115cf45121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Sat, 22 Aug 2026 00:44:39 +0000 Subject: [PATCH 3/7] feat: integrate message comment web UI --- .../api/src/routes/chat-comment-directives.ts | 3 + apps/api/src/routes/chat-comments.ts | 58 ++- apps/api/src/schemas/comments.ts | 5 + apps/api/src/schemas/index.ts | 1 + apps/api/src/services/message-comments.ts | 11 +- .../comments/CommentComposer.tsx | 117 +++++ .../comments/CommentPrimitives.tsx | 134 ++++++ .../comments/CommentThread.tsx | 201 +++++++++ .../comments/CommentableConversationItem.tsx | 116 +++++ .../comments/MessageCommentPanels.tsx | 235 ++++++++++ .../comments/comment-utils.ts | 132 ++++++ .../comments/useCommentSelection.ts | 121 +++++ .../comments/useMessageComments.ts | 257 +++++++++++ .../comments/useProjectMessageCommentUi.tsx | 152 +++++++ .../components/project-message-view/index.tsx | 198 ++++---- .../session-view-utils.tsx | 43 ++ .../useSessionLifecycle.ts | 5 +- apps/web/src/hooks/useChatWebSocket.ts | 64 ++- apps/web/src/lib/api/comments.ts | 287 ++++++++++++ apps/web/src/lib/api/index.ts | 22 + apps/web/src/lib/query-options/comments.ts | 21 + apps/web/src/lib/query-options/index.ts | 1 + .../playwright/message-comments-audit.spec.ts | 423 ++++++++++++++++++ apps/web/tests/unit/api/comments.test.ts | 139 ++++++ .../chat/project-message-view-resume.test.tsx | 14 + .../unit/components/message-comments.test.tsx | 202 +++++++++ .../components/project-message-view.test.tsx | 302 ++++++++++++- .../unit/hooks/useSessionTimeline.test.ts | 4 +- packages/ui/src/components/Avatar.tsx | 57 +++ packages/ui/src/components/Popover.tsx | 94 ++++ packages/ui/src/components/Textarea.tsx | 18 + packages/ui/src/components/index.ts | 3 + packages/ui/tests/Avatar.test.tsx | 27 ++ packages/ui/tests/Popover.test.tsx | 48 ++ packages/ui/tests/Textarea.test.tsx | 26 ++ .../contracts/message-comment-api.md | 209 +++++++++ ...26-08-21-message-anchored-commenting-ui.md | 237 ++++++++++ 37 files changed, 3863 insertions(+), 124 deletions(-) create mode 100644 apps/web/src/components/project-message-view/comments/CommentComposer.tsx create mode 100644 apps/web/src/components/project-message-view/comments/CommentPrimitives.tsx create mode 100644 apps/web/src/components/project-message-view/comments/CommentThread.tsx create mode 100644 apps/web/src/components/project-message-view/comments/CommentableConversationItem.tsx create mode 100644 apps/web/src/components/project-message-view/comments/MessageCommentPanels.tsx create mode 100644 apps/web/src/components/project-message-view/comments/comment-utils.ts create mode 100644 apps/web/src/components/project-message-view/comments/useCommentSelection.ts create mode 100644 apps/web/src/components/project-message-view/comments/useMessageComments.ts create mode 100644 apps/web/src/components/project-message-view/comments/useProjectMessageCommentUi.tsx create mode 100644 apps/web/src/components/project-message-view/session-view-utils.tsx create mode 100644 apps/web/src/lib/api/comments.ts create mode 100644 apps/web/src/lib/query-options/comments.ts create mode 100644 apps/web/tests/playwright/message-comments-audit.spec.ts create mode 100644 apps/web/tests/unit/api/comments.test.ts create mode 100644 apps/web/tests/unit/components/message-comments.test.tsx create mode 100644 packages/ui/src/components/Avatar.tsx create mode 100644 packages/ui/src/components/Popover.tsx create mode 100644 packages/ui/src/components/Textarea.tsx create mode 100644 packages/ui/tests/Avatar.test.tsx create mode 100644 packages/ui/tests/Popover.test.tsx create mode 100644 packages/ui/tests/Textarea.test.tsx create mode 100644 specs/035-message-comments/contracts/message-comment-api.md create mode 100644 tasks/active/2026-08-21-message-anchored-commenting-ui.md diff --git a/apps/api/src/routes/chat-comment-directives.ts b/apps/api/src/routes/chat-comment-directives.ts index 011590977..c5d23bace 100644 --- a/apps/api/src/routes/chat-comment-directives.ts +++ b/apps/api/src/routes/chat-comment-directives.ts @@ -7,6 +7,7 @@ import { requireRouteParam } from '../lib/route-helpers'; import { getUserId } from '../middleware/auth'; import { errors } from '../middleware/error'; import { requireProjectCapability } from '../middleware/project-auth'; +import { parseOptionalBody, SendCommentDirectiveSchema } from '../schemas'; import { createProjectDataMessageCommentAdapter, isMessageCommentServiceError, @@ -47,6 +48,7 @@ export function registerChatCommentDirectiveRoute(chatRoutes: Hono<{ Bindings: E const db = drizzle(c.env.DATABASE, { schema }); await requireProjectCapability(db, projectId, userId, 'task:write'); + const body = await parseOptionalBody(c.req.raw, SendCommentDirectiveSchema, {}); const session = await projectDataService.getSession(c.env, projectId, sessionId); if (!session) { @@ -61,6 +63,7 @@ export function registerChatCommentDirectiveRoute(chatRoutes: Hono<{ Bindings: E sessionId, threadId, humanUserId: userId, + body: body.body ?? null, }); return c.json( diff --git a/apps/api/src/routes/chat-comments.ts b/apps/api/src/routes/chat-comments.ts index b85d03287..c4169e1bd 100644 --- a/apps/api/src/routes/chat-comments.ts +++ b/apps/api/src/routes/chat-comments.ts @@ -14,7 +14,13 @@ import { CommentStatusMutationSchema, CreateCommentReplySchema, CreateCommentThreadSchema, + SendCommentDirectiveSchema, } from '../schemas/comments'; +import { + createProjectDataMessageCommentAdapter, + isMessageCommentServiceError, + sendMessageCommentDirective, +} from '../services/message-comments'; import * as projectDataService from '../services/project-data'; export const chatCommentRoutes = new Hono<{ Bindings: Env }>(); @@ -113,6 +119,24 @@ function rethrowCommentError(err: unknown): never { throw err; } +function rethrowCommentDirectiveError(err: unknown): never { + if (!isMessageCommentServiceError(err)) { + throw err; + } + switch (err.code) { + case 'invalid_request': + throw errors.badRequest(err.message); + case 'not_found': + throw errors.notFound('Comment thread'); + case 'forbidden': + throw errors.forbidden(err.message); + case 'conflict': + throw errors.conflict(err.message); + case 'unavailable': + throw errors.internal('Comment storage backend is unavailable'); + } +} + /** * GET /api/projects/:projectId/sessions/:sessionId/comments * List message-anchored comment threads for a chat session. @@ -233,10 +257,38 @@ async function updateCommentStatus( chatCommentRoutes.post( '/:sessionId/comments/:threadId/send', - jsonValidator(CommentStatusMutationSchema), - (c) => { + jsonValidator(SendCommentDirectiveSchema), + async (c) => { const body = c.req.valid('json'); - return updateCommentStatus(c, 'sent', body.clientMutationId ?? null); + const userId = getUserId(c); + const projectId = requireRouteParam(c, 'projectId'); + const sessionId = requireRouteParam(c, 'sessionId'); + const threadId = requireRouteParam(c, 'threadId'); + const db = drizzle(c.env.DATABASE, { schema }); + + await requireProjectCapability(db, projectId, userId, 'task:write'); + + try { + const result = await sendMessageCommentDirective({ + env: c.env, + storage: createProjectDataMessageCommentAdapter(c.env), + projectId, + sessionId, + threadId, + humanUserId: userId, + body: body.body ?? null, + }); + return c.json( + { + thread: result.thread, + comment: result.thread, + idempotent: result.duplicate, + }, + result.duplicate ? 200 : 202 + ); + } catch (err) { + rethrowCommentDirectiveError(err); + } } ); diff --git a/apps/api/src/schemas/comments.ts b/apps/api/src/schemas/comments.ts index f2685f82a..45ef59538 100644 --- a/apps/api/src/schemas/comments.ts +++ b/apps/api/src/schemas/comments.ts @@ -22,3 +22,8 @@ export const CreateCommentReplySchema = v.object({ export const CommentStatusMutationSchema = v.object({ clientMutationId: v.optional(v.nullable(v.string())), }); + +export const SendCommentDirectiveSchema = v.object({ + body: v.optional(v.string()), + clientMutationId: v.optional(v.nullable(v.string())), +}); diff --git a/apps/api/src/schemas/index.ts b/apps/api/src/schemas/index.ts index 5a66ae2dc..27fcf2d44 100644 --- a/apps/api/src/schemas/index.ts +++ b/apps/api/src/schemas/index.ts @@ -119,6 +119,7 @@ export { CommentStatusMutationSchema, CreateCommentReplySchema, CreateCommentThreadSchema, + SendCommentDirectiveSchema, } from './comments'; // Admin schemas diff --git a/apps/api/src/services/message-comments.ts b/apps/api/src/services/message-comments.ts index 8cc68db16..ba9c04d56 100644 --- a/apps/api/src/services/message-comments.ts +++ b/apps/api/src/services/message-comments.ts @@ -364,6 +364,7 @@ export interface SendMessageCommentDirectiveInput { sessionId: string; threadId: string; humanUserId: string; + body?: string | null; now?: number; } @@ -406,7 +407,7 @@ export async function sendMessageCommentDirective( const config = getMessageCommentConfig(input.env); const deliveryId = buildCommentDirectiveDeliveryId(thread.id); - const content = buildDirectivePrompt(thread, config); + const content = buildDirectivePrompt(thread, config, input.body); const accepted = await projectDataService.acceptPromptDelivery(input.env, input.projectId, { deliveryId, targetSessionId: input.sessionId, @@ -456,13 +457,17 @@ export async function sendMessageCommentDirective( }; } -function buildDirectivePrompt(thread: MessageCommentThread, config: MessageCommentConfig): string { +function buildDirectivePrompt( + thread: MessageCommentThread, + config: MessageCommentConfig, + directiveBody?: string | null +): string { const author = getAuthorDisplayName(thread.author) || `${thread.author.kind}:${thread.author.id}`; const quote = normalizeCommentQuote( thread.anchor.quote ?? thread.sourceMessage?.quote ?? null, config ); - const body = normalizeCommentBody(thread.body, config); + const body = normalizeCommentBody(directiveBody?.trim() ? directiveBody : thread.body, config); const sections = [ 'SAM comment directive', `Comment ID: ${thread.id}`, diff --git a/apps/web/src/components/project-message-view/comments/CommentComposer.tsx b/apps/web/src/components/project-message-view/comments/CommentComposer.tsx new file mode 100644 index 000000000..96e8d357a --- /dev/null +++ b/apps/web/src/components/project-message-view/comments/CommentComposer.tsx @@ -0,0 +1,117 @@ +import { Button, Textarea } from '@simple-agent-manager/ui'; +import { useEffect, useId, useRef, useState } from 'react'; + +import type { MessageCommentAction } from '../../../lib/api/comments'; +import { QuotedAnchor } from './CommentPrimitives'; + +export interface CommentComposerProps { + quote?: string; + placeholder?: string; + submitLabel?: string; + autoFocus?: boolean; + onSubmit: (body: string, action: MessageCommentAction) => Promise | unknown; + onCancel: () => void; +} + +export function CommentComposer({ + quote, + placeholder = 'Add a comment…', + submitLabel = 'Comment', + autoFocus = true, + onSubmit, + onCancel, +}: CommentComposerProps) { + const textareaId = useId(); + const [body, setBody] = useState(''); + const [action, setAction] = useState('note'); + const [submitting, setSubmitting] = useState(false); + const textareaRef = useRef(null); + const canSubmit = body.trim().length > 0 && !submitting; + + useEffect(() => { + if (autoFocus) textareaRef.current?.focus(); + }, [autoFocus]); + + const submit = async (overrideAction = action) => { + const trimmed = body.trim(); + if (!trimmed || submitting) return; + setSubmitting(true); + try { + await onSubmit(trimmed, overrideAction); + setBody(''); + setAction('note'); + } finally { + setSubmitting(false); + } + }; + + return ( +
{ + event.preventDefault(); + void submit(); + }} + > + {quote && } + +