Integrate message-anchored comments MVP

[simple-agent-manager]

Summary

Primary integration PR for the message-anchored commenting MVP from idea 01M0JQB842XSJ3W172DYPB37HN.

• Delivers message-anchored comments only; file/markdown anchoring remains deferred.
• Preserves the intended architecture: ProjectData Durable Object SQLite storage, server-authoritative comment state, WebSocket multiplayer broadcasts, MCP pull tools, and send-to-agent directives.
• Keeps send-to-agent directive delivery fail-closed and durable, with deterministic FIFO/idempotency semantics.
• Does not deploy staging and does not merge production.

Primary branch/head

• Branch: sam/execute-task-using-skill-2f0t46
• Head: a1fb44d981ff7654a6b616a020d6f03ab422bc4d
• Target: main
• Current base at validation time: origin/main 728611b878086087a76b8f534af26b3db945e7b6

Constituent PR heads integrated

These exact constituent heads were squash-applied so their commits are not ancestors of this primary branch. This is intentional: keep the constituent PRs open/unmerged and have the coordinator squash-merge this primary PR later.

Constituent	Branch	Exact head integrated
#1878 Backend/multiplayer/API	sam/build-backendmultiplayer-constituent-pr-ys4449	2c980b159128de0e2a02bb639ba348e3735c7be7
#1879 MCP tools + send-to-agent	sam/build-mcpagent-directive-constituent-ekeztr	63508d06a57fe2ba544f7449113dd599097369e7
#1880 Production UI	sam/build-production-web-ui-z72qrz	bc252a91e0ab607569638081a940f1c8819be8af

Verification on the integration branch:

2c980b159128de0e2a02bb639ba348e3735c7be7 not-ancestor
63508d06a57fe2ba544f7449113dd599097369e7 not-ancestor
bc252a91e0ab607569638081a940f1c8819be8af not-ancestor

Integration conflict and contract resolution

• Reconciled apps/api/src/routes/chat.ts so REST comment routes and directive routes are both registered.
• Rebased over main after Add manual session sleep control #1881, preserving the manual session sleep controls and resolving the only conflict in apps/web/src/components/project-message-view/index.tsx by keeping both import sets.
• Reconciled packages/shared/src/types/comments.ts into a single message-only contract used by REST, MCP, WebSocket, and UI code. Author shape supports the integrated name/displayName usage without adding file anchors.
• Wired MCP/directive service code to the ProjectData comment APIs from the backend branch: listCommentThreads, getCommentThread, createCommentThread, createCommentReply, and updateCommentThreadStatus.
• Added the public ProjectData getCommentThread wrapper required by the integrated MCP/directive path.
• Mapped Web UI API calls from the UI contract to the backend contract:
  • create: UI comment payload -> backend messageId, quote, clientMutationId
  • response: backend { thread } -> UI { comment }
  • replies/status/send follow the same mapping.
• Changed the REST /comments/:threadId/send path to call durable sendMessageCommentDirective before returning the thread as sent; delivery failure remains fail-closed.
• Normalized backend comment.thread.changed WebSocket events into the UI comment event cache path.
• Updated route and Playwright mocks to match the integrated backend/UI response contract.

Preflight and specialist evidence

Review	Result	Evidence
Cloudflare/Durable Objects	Pass	D1 schema is append-only through ProjectData DO migration 032-message-comment-threads; comment writes/read models stay inside the DO; list queries are bounded and indexed by session/message/status sequence. No staging/deploy commands were run.
Security	Pass	REST routes require project capabilities; MCP tools derive identity/session from verified context and reject caller-supplied project/user/provenance fields; directive delivery fails closed when durable delivery is disabled; MCP errors are safely mapped.
UI/UX	Pass	Hybrid desktop rail + mobile inline model preserved; selection, empty/error, send, resolve/reopen, and mobile coarse-pointer affordances are covered by Playwright audits with generated screenshots.
Test engineering	Pass	Vertical coverage includes DO migrations/storage/broadcasts, REST routes, MCP tools, durable directive delivery, Web API mapping, React behavior, UI primitives, and Playwright flows.
Constitution / no hardcoded values	Pass	Comment body/quote/list/thread/reply/directive limits are environment-backed with shared defaults; no hardcoded internal URLs or deployment constants added for the feature.
Doc sync	Pass	API contract, configuration reference, API-reference skill, env examples, and implementation notes are updated for message comments and directive delivery.
Task completion	Pass	Scope is message-only MVP integration. Exact constituent heads are integrated, contract mismatches are resolved, local validations pass, and constituent PRs are intentionally left open/unmerged.

Agent Preflight (Required)

• Preflight completed before code changes

Classification

• external-api-change
• cross-component-change
• business-logic-change
• public-surface-change
• docs-sync-change
• security-sensitive-change
• ui-change
• infra-change

External References

N/A: this integration does not change an external provider API; evidence came from the in-repo feature contract and constituent PRs, including specs/035-message-comments/contracts/message-comment-api.md, #1878, #1879, and #1880.

Codebase Impact Analysis

Cross-component integration spans apps/api Durable Object storage/routes/MCP services, packages/shared comment contracts, and apps/web API mapping/WebSocket/UI flows. The integration also reconciles route tests, DO tests, React unit tests, and Playwright coverage for the message-comment path.

Documentation & Specs

The integrated branches include message-comment API/spec/config documentation updates for specs/035-message-comments/contracts/message-comment-api.md, environment/default limit references, and API-reference skill material. This primary PR preserves those docs and verifies the code contracts against them.

Constitution & Risk Check

Checked Principle XI/no hardcoded values and the security-sensitive multi-tenant boundaries. Limits stay environment-backed; comments remain server-authoritative in ProjectData; MCP identity/provenance is derived from verified context; directive delivery fails closed; scope remains message-only with file/markdown anchoring deferred.

Local validation

No bare pnpm format was run.

Passed:

pnpm install

pnpm --filter @simple-agent-manager/shared typecheck
pnpm --filter @simple-agent-manager/shared build
pnpm --filter @simple-agent-manager/providers build
pnpm --filter @simple-agent-manager/cloud-init build
pnpm --filter @simple-agent-manager/api typecheck
pnpm --filter @simple-agent-manager/api test -- tests/unit/services/message-comments.test.ts tests/unit/routes/mcp-message-comments.test.ts tests/unit/routes/chat-comment-directives.test.ts tests/unit/routes/chat-comments.test.ts

pnpm --filter @simple-agent-manager/api test -- tests/unit/routes/chat-comments.test.ts tests/unit/routes/chat-comment-directives.test.ts tests/unit/services/message-comments.test.ts
pnpm --filter @simple-agent-manager/web test -- tests/unit/api/comments.test.ts tests/unit/components/message-comments.test.ts tests/unit/components/project-message-view.test.ts
pnpm --filter @simple-agent-manager/api typecheck
pnpm --filter @simple-agent-manager/web typecheck
pnpm --filter @simple-agent-manager/ui test -- Avatar.test.tsx Popover.test.tsx Textarea.test.tsx
pnpm --filter @simple-agent-manager/api test -- tests/unit/routes/mcp-error-handling.test.ts

npx prettier --write apps/api/src/routes/chat.ts apps/api/src/services/project-data.ts apps/web/tests/playwright/message-comments-audit.spec.ts
npx prettier --write apps/api/tests/unit/routes/mcp.test.ts

pnpm --dir apps/web exec playwright install chromium
pnpm --dir apps/web exec playwright install-deps chromium
pnpm --dir apps/web exec playwright test tests/playwright/message-comments-audit.spec.ts --project="Desktop (1280x800)"
pnpm --dir apps/web exec playwright test tests/playwright/message-comments-audit.spec.ts --project="iPhone SE (375x667)"

pnpm check:fast
pnpm typecheck
pnpm build
pnpm test

Validation notes:

• pnpm check:fast passed after the final minimal patch with existing non-blocking advisory output: format ratchet 2036/2225 unformatted file(s), Oxlint shadow diagnostics, report-only type-boundary findings, and known ESLint warnings.
• pnpm typecheck passed; Astro template validation still prints existing baseline diagnostics while exiting successfully.
• pnpm build passed with known build warnings (advancedChunks deprecation, Lightning CSS @source, Tailwind sourcemap warning).
• Final root pnpm test passed after rebasing and the MCP hook-timeout stabilization: Turbo 21 successful, 21 total; API 591 test files / 7918 tests passed; Web 285 test files / 3419 tests passed.
• The first post-rebase root test exposed a full-suite-only 10s beforeEach timeout in tests/unit/routes/mcp-error-handling.test.ts. The test passed in isolation; the integration branch now gives that setup hook a 20s timeout. The isolated test (1 file / 8 tests) and the full root suite both pass after the change.

Coordination notes

• Keep Backend foundation for message-anchored comments #1878, Add message comment MCP directive layer #1879, and Message-anchored commenting UI MVP (do not merge/deploy) #1880 open/unmerged.
• Do not deploy staging from this branch; coordinator will handle staging/production after this PR is green.
• Do not merge this PR as a merge commit if preserving constituent PR openness matters; coordinator should squash-merge the primary integration PR.

Remediation update — staging verifier blockers

New remediation head: a1fb44d981ff7654a6b616a020d6f03ab422bc4d.

Blockers fixed from verifier task 01M0KQ4XAJCRKGW2E0GPXPH00Y:

1. Live two-browser comment convergence: project-wide comment.thread.changed WebSocket frames now flow through useProjectWebSocket into the same TanStack Query cache updater used by the session socket. Receiving browsers update create/reply/resolve/reopen state without reload.
2. Cross-session comment mutation shaping: serialized Durable Object Comment thread not found misses now map to intentional bounded 404 NOT_FOUND responses instead of falling through to 500.

Root cause:

• ProjectData.broadcastCommentThread() emitted comment.thread.changed to both session:<sessionId> sockets and untagged project sockets. useProjectWebSocket received the project-wide frames but ignored that event type; the cache mutation handler only lived behind the session-specific chat socket. A receiving browser could therefore observe the frame while the rendered comments cache stayed stale.
• Cross-session mutations correctly missed the (sessionId, threadId) row in ProjectData, but Durable Object RPC could surface the typed CommentNotFoundError as a plain Error("Comment thread not found"). apps/api/src/routes/chat-comments.ts only recognized typed/name/code errors, so this serialized domain miss was treated as an unexpected 500.

Files changed in the remediation commit:

• WebSocket/cache convergence: apps/web/src/hooks/useProjectWebSocket.ts, apps/web/src/pages/project-chat/useProjectChatState.ts, apps/web/src/components/project-message-view/comments/useMessageComments.ts, apps/web/src/lib/query-options/comments.ts, apps/web/src/lib/query-options/index.ts
• Error shaping: apps/api/src/routes/chat-comments.ts
• Regression tests: apps/web/tests/unit/hooks/useProjectWebSocket.test.ts, apps/web/tests/unit/lib/message-comment-query-cache.test.ts, apps/api/tests/unit/routes/chat-comments.test.ts
• Process improvement: .claude/rules/13-staging-verification.md

Remediation validation run locally:

pnpm --filter @simple-agent-manager/web test -- tests/unit/hooks/useProjectWebSocket.test.ts tests/unit/lib/message-comment-query-cache.test.ts tests/unit/components/project-message-view.test.tsx
pnpm --filter @simple-agent-manager/api test -- tests/unit/routes/chat-comments.test.ts
pnpm --filter @simple-agent-manager/web typecheck
pnpm --filter @simple-agent-manager/api typecheck
pnpm --dir apps/web exec playwright test tests/playwright/message-comments-audit.spec.ts --project="Desktop (1280x800)" --project="iPhone SE (375x667)"
pnpm check:fast
pnpm --filter @simple-agent-manager/web build
pnpm --filter @simple-agent-manager/api build
env -u GH_TOKEN -u GITHUB_TOKEN git diff --check

Results: focused web tests passed 86/86; focused API route tests passed 6/6; web/api typechecks passed; message-comments Playwright visual audit passed 6/6; pnpm check:fast passed with existing advisory/report-only warnings; web/api builds passed with existing build warnings; git diff --check passed.

Post-Mortem:

• What broke: live comment create/reply/resolve/reopen events arrived in the receiving browser but did not update the visible UI until reload; cross-session status mutation returned a raw 500 instead of a clean isolation error.
• Why it was not caught: existing coverage exercised API and component callback paths, but not the exact project-wide browser WebSocket path that staging used to receive the frame; route tests did not include serialized DO domain errors.
• Class of bug: browser-consumed stream delivery proved only at frame/callback level, not at receiving-browser cache/DOM convergence; typed Durable Object domain errors losing prototype metadata across RPC boundaries.
• Process fix: .claude/rules/13-staging-verification.md now requires browser stream verification/tests to assert receiving-browser UI/cache convergence through the exact socket/hook that receives the production frame, not just frame arrival.

Coordination status: no staging deployment was run for this remediation; no production merge was performed; constituent PRs #1878/#1879/#1880 must remain open/unmerged.

[codspeed-hq]

Merging this PR will not alter performance

✅ 6 untouched benchmarks

---

_{Comparing sam/execute-task-using-skill-2f0t46 (1749f79) with main (728611b)}

[simple-agent-manager]

Remediation follow-up for staging-blocking verifier 01M0KQ4XAJCRKGW2E0GPXPH00Y:

• New head: a1fb44d981ff7654a6b616a020d6f03ab422bc4d on sam/execute-task-using-skill-2f0t46.
• Root cause: ProjectData already broadcast comment.thread.changed frames to untagged project sockets, but useProjectWebSocket dropped those frames. The receiving browser could observe the frame while no comment query-cache mutation ran. Separately, cross-session DO mutation errors could arrive as serialized plain Error("Comment thread not found"), bypassing the typed CommentNotFoundError mapper and falling through to HTTP 500.
• Fix: project-wide websocket handling now maps comment.thread.changed to comment realtime events, and both project-wide and session-specific sockets use a shared query-cache helper to upsert create, reply, resolve, and reopen updates without reload. Comment route error shaping now maps bounded serialized not-found domain messages to a clean 404.
• Local validation passed: focused web realtime/cache tests, focused API comment route tests, web/api typecheck, web/api builds, pnpm check:fast, git diff --check, and desktop/mobile Playwright message-comment audit.
• PR checks are green at this head: CI run 32553242478 completed success, including Playwright Visual Tests; E2E Smoke run 32553242316 completed success; CodSpeed run 32553242346 completed success.
• No staging deployment was performed by this remediation. No production merge was performed. Constituent PRs Backend foundation for message-anchored comments #1878, Add message comment MCP directive layer #1879, and Message-anchored commenting UI MVP (do not merge/deploy) #1880 remain open and unmerged.

Recommendation: #1882 is ready for a fresh staging verification task.

[simple-agent-manager]

Remediation update for cross-session comment mutation 500

New head: 1749f795bc2628fe9a7c392b945652851105216b3

Root cause: Cloudflare ProjectData DO RPC serialized CommentNotFoundError across the isolate boundary as a generic Error with message CommentNotFoundError: Comment thread not found. The route-level exact matcher only covered Comment thread not found, so wrong-session status mutations escaped to the global handler as 500.

Fix: normalize exact serialized comment not-found RPC strings in apps/api/src/services/project-data.ts, the API-side ProjectData RPC boundary, back into typed CommentNotFoundError. Matching is exact and limited to the three comment not-found domain resources; serialized-looking internal errors still propagate as 500.

Tests/evidence:

• Failing-before-fix proof: pnpm --filter @simple-agent-manager/api test -- tests/unit/services/project-data-comments.test.ts failed with generic Error: CommentNotFoundError: Comment thread not found when the service fix was temporarily reverted.
• Passing focused tests: pnpm --filter @simple-agent-manager/api test -- tests/unit/services/project-data-comments.test.ts tests/unit/durable-objects/comments.test.ts — 2 files / 9 tests passed.
• Passing nearby route/MCP tests: pnpm --filter @simple-agent-manager/api test -- tests/unit/routes/chat-comments.test.ts tests/unit/routes/mcp-message-comments.test.ts — 2 files / 15 tests passed.
• pnpm --filter @simple-agent-manager/api typecheck passed.
• pnpm --filter @simple-agent-manager/api build passed.
• pnpm check:fast passed.
• git diff --check passed.

Specialist review tracker:

• task-completion-validator: PASS — required workflow mapped to diff/tests; no task file created per explicit continuation constraint.
• cloudflare-specialist: PASS — inspected staging observability D1 for request 6b68fcdd-aba9-41f3-acac-d865dc298555; boundary fix is service-side after DO RPC serialization, no direct CF mutations/deploy.
• security-auditor: PASS — preserves fail-closed project/session isolation, returns not-found without disclosing foreign-session thread data, exact matching only.
• test-engineer: PASS — regression proves serialized RPC shape, non-exact internal error remains internal, and cross-session status mutation does not mutate or disclose thread state.
• constitution-validator: PASS — no new configurable business values, URLs, timeouts, limits, or identifiers.
• doc-sync-validator: PASS — no endpoint/env/schema/user-facing contract change requiring docs.

Staging verification intentionally not performed by this agent per task instruction; ready for independent staging verification pinned to this new head once CI is green.

[sonarqubecloud]

Quality Gate passed

Issues
44 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.5% Duplication on New Code

See analysis details on SonarQube Cloud

[simple-agent-manager]

Final independent staging verification PASS for pinned head 1749f79.\n\nEvidence:\n- deploy-staging.yml run 32566266288 completed successfully at headSha 1749f79: https://github.com/raphaeltm/simple-agent-manager/actions/runs/32566266288\n- Pre-deploy checks: PR head matched pinned SHA; checks terminal/green; .codex/config.toml absent from diff; #1878/#1879/#1880 open with mergedAt=null; unfiltered staging run list had no queued/in_progress contention before trigger.\n- Live app/API: authenticated once via token-login then reused storage state; created two distinct sessions in one project; created message-anchored thread, replied, resolved, reopened, reloaded/listed persistence; browser WebSocket receivers observed comment.thread.changed frames for create/reply/resolve/reopen without reload.\n- Former blocker: resolving session A thread through session B returned clean 404 {error: NOT_FOUND, message: Comment thread not found}; no 500/raw internals; session A thread remained unmutated and session B list did not disclose it.\n- Auth/regression: unauth protected API returned 401; /health returned 200; dashboard/settings loaded; no browser console errors. Navigation-aborted /api/t and route-change fetches were observed but no comment-related network failures.\n- Directive/UI: /send returned 202 and status sent with directive; desktop and mobile screenshots captured message badge/count, comment thread/reply/sent badge, directive content, no horizontal overflow. Artifacts are in verifier workspace /tmp/pr1882-verification/.\n- Observability/cleanup: pnpm quality:observability-noise passed (tool skipped unavailable telemetry/D1 checks); D1 cleanup proof active_nodes=0, verifier_active_tasks=0, verifier task statuses cancelled,cancelled,cancelled,cancelled, enabled platform cloud credential=1.\n\nNo merge performed.