Add structured meta chat with feedback, instrumentation, and updated product messaging

[kinwo]

Summary

• add a thread-based /chat experience with hydrated history loading and improved onboarding
• surface structured meta-chat UI for assistant replies, including task framing, plan preview, confidence, limitations, and feedback controls
• add chat lifecycle analytics and feedback endpoints plus shared chat contracts, storage helpers, and metrics utilities
• introduce Mastra planner/critic workflow support, metadata propagation, and fallback handling when meta steps fail
• expand automated coverage for chat routes, workflow logic, component behavior, and chat/landing-page E2E flows
• update the landing page, PRD, and core product roadmap to reflect shipped meta-chat capabilities

Testing

• npm test -- mastra/workflows/meta-chat-workflow.test.ts app/api/chat/route.test.ts app/chat/_components/chat-message.test.tsx
• npm test -- lib/chat/history.test.ts app/api/chat/route.test.ts
• npm test -- app/chat/_components/chat-message-list.test.tsx
• npm run ts-check
• npm run test:e2e -- e2e/chat.spec.ts --project=chromium
• npm run test:e2e -- e2e/landing-page.spec.ts e2e/button-interactions.spec.ts --project=chromium

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
secondorder-web	Ready	Preview, Comment	Mar 28, 2026 10:55pm

[claude]

PR Review: Add structured meta chat with feedback, instrumentation, and updated product messaging

This is a substantial feature addition — well-structured, with good test coverage and a clear product intent. The findings below are meant to help harden it before merge.

---

🔴 Critical

1. Unauthenticated writes allowed when thread doesn't exist yet (app/api/chat/events/route.ts, app/api/chat/feedback/route.ts)

validateThreadAccess returns true unconditionally when getThread returns null:

async function validateThreadAccess(threadId: string, resourceId: string) {
  const thread = await getThread(threadId);
  if (thread && thread.resourceId && thread.resourceId !== resourceId) {
    return false;
  }
  return true; // ← allows any request when thread is null
}

For the feedback and events endpoints, a null thread should be a rejection, not a pass. Any caller can submit arbitrary feedback/events for a guessed threadId as long as it hasn't been persisted yet. The chat POST route (creating new threads) is the only case where a null thread makes sense — this logic shouldn't be shared as-is.

2. Resource ID cookie is not validated as a UUID (lib/chat/session.ts)

getOrCreateResourceId returns the raw cookie value verbatim with no format validation. This value is then used as a Mastra resourceId and in SQL queries. An attacker who sets their own cookie to an unexpected value could send unexpected data into storage layers.

Fix: Validate with z.string().uuid() before accepting the stored value; regenerate a fresh UUID if invalid.

---

🟠 Major

3. Wrong agent used in draftStep (mastra/workflows/meta-chat-workflow.ts)

draftStep calls plannerAgent.generate(...) to produce the user-facing draft answer. The planner's purpose is task decomposition/goal extraction — not drafting answers. This is likely a copy-paste bug. The critic then reviews this planner-generated output, reducing the quality of the full pipeline.

4. Unbounded SQL query with no indexes in getChatEvaluationMetrics (lib/chat/events.ts)

SELECT id, event_type, thread_id, message_id, task_type, metadata, created_at
FROM chat_events

No LIMIT, WHERE, or time window. Also, the CREATE TABLE statement adds no indexes on thread_id or event_type, meaning all queries are full-table scans. This will become a performance problem as events accumulate.

5. initPromise never resets on failure (lib/chat/events.ts, lib/chat/feedback.ts)

let initPromise: Promise<void> | null = null;
if (!initPromise) {
  initPromise = client.execute(`CREATE TABLE IF NOT EXISTS ...`).then(...);
}
return initPromise;

If the DB call fails on first execution, initPromise is set to a rejected promise permanently. All subsequent calls re-await the same rejection silently. Add a .catch(() => { initPromise = null; }) to allow retries.

6. sendMessage errors are silently swallowed in ChatPageClient (app/chat/_components/chat-page-client.tsx)

const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
  setInput(''); // cleared before send
  await sendMessage({ text: messageText }); // errors go nowhere
};

If the request fails after setInput(''), the user sees a blank input field with no feedback and their message is lost. Wrap in try/catch, restore the input on failure, and surface an error state.

7. trackChatEvent does not check response.ok (lib/chat/track-event.ts)

The function sends a fetch but doesn't check for non-2xx responses. Events can be silently dropped on server errors. The call site wraps with .catch(() => {}) for UX resilience, but the tracking function itself should at least log failures.

8. Regex word-boundary bug in classifyTask (mastra/skills/registry.ts)

/\bchoose|decision|trade[- ]?off|compare|which should\b/i

\b only applies to which should (regex alternation has lowest precedence). decision has no leading \b, so substrings like "undecided" will match. Fix: /\b(choose|decision|trade[- ]?off|compare|which should)\b/i

---

🟡 Minor

9. as never type suppressions in chat route (app/api/chat/route.ts)

messages: normalizedMessages as never,
stream: streamWithMetadata as never,

These suppress TypeScript rather than resolving the actual type mismatch between Mastra SDK and AI SDK message shapes. Should be tracked as tech debt and replaced with a proper adapter.

10. PRD.md contains hardcoded local machine paths

[`/Users/henry/workspace/secondorder-web/...`]

These paths only work on the author's machine and shouldn't be in a committed document.

11. afk-ralph.sh committed to repo root

This is a developer automation loop script. It also references progress.md which isn't committed — it would error on a fresh clone. Should be in a scripts/ directory or excluded from the PR.

12. feedbackPositivityRate treats "needs_more_depth" as negative (lib/chat/events.ts)

Only "helpful" is counted as positive. "Needs more depth" is constructive, not negative, and will skew the metric. Worth a comment at minimum to document the intent.

---

🔵 Nits

• jsonResponse and validateThreadAccess are duplicated verbatim across all three route files. Extract to a shared lib/api/ utility.
• 1-year cookie max-age (lib/chat/constants.ts) — worth re-evaluating given this is the sole access control mechanism.
• Missing Secure cookie flag (lib/chat/session.ts) — add in production to prevent transmission over plain HTTP.
• Raw backtick in marketing copy (app/page.tsx): In \/chat`, complex requests...renders as a literal backtick. Use` or plain text.

---

Overall

The architecture is solid and the test coverage for new routes and components is a good foundation. The two critical access control issues are the priority — the rest are hardening/polish items. Happy to discuss any of these in more detail.