Gabriel Brown
8aa9140191
Phase 1 Stabilize (13), Phase 2 Runtime unification (12), Phase 3 Dev-box surface (11), Phase 4 Sync correctness (12), Phase 5 Notifications & polish (15), plus index.
406 lines
36 KiB
Markdown
406 lines
36 KiB
Markdown
# Spoon Phase 5 — Notifications & Polish: Implementation Plan
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** Ship the one net-new subsystem for Phase 5 — a `notifications` table with an in-app header bell and preference-gated email via the existing UseSend integration — then sweep the audit's polish findings (#14–#20: loading skeletons, fire-and-forget error handling, form re-sync, auth copy, workspace UX, accessibility, dead code) and rewrite the docs for the box-first model.
|
||
|
||
**Architecture:** Notifications are written by a single shared TS helper `emitNotification(ctx, {...})` invoked at the existing event points (maintenance-thread creation, job status transitions, sync failure, connection re-auth). The helper inserts an in-app row and, when the user's per-kind email preference allows, schedules a `'use node'` internal action that sends through UseSend (`usesend-js`, already a dependency). The header bell subscribes to a reactive inbox query; mark-read/mark-all-read are mutations. Preferences live in a per-user `notificationPreferences` row edited from a new Settings → Notifications page. Everything else in the phase is independent, grouped, small fixes to `apps/next` UI and `apps/agent-worker` cleanup.
|
||
|
||
**Tech Stack:** Convex (notifications table + queries/mutations, UseSend email), Next.js 16 (React 19) UI, agent-worker minor cleanup.
|
||
|
||
**Depends on:** Phases 1–4 (notification emit points reference their statuses/events — job status transitions from Phase 1, connection `needs_reauth` from the Phase 4 webhook); polish tasks are independent and can run in any order after the notifications subsystem.
|
||
|
||
## Global Constraints
|
||
|
||
- **Backend tests:** `packages/backend/tests/unit` with `convex-test`. Run `cd packages/backend && bun run test:unit`. The harness pattern is in `packages/backend/tests/unit/harness.test.ts`: `convexTest(schema, import.meta.glob('../../convex/**/*.*s'))`, `createUser`/`authed(t, userId)` helpers, `t.withIdentity({ subject: '<userId>|session', issuer: 'https://convex.test' })`. Add new tests as `packages/backend/tests/unit/<name>.test.ts`.
|
||
- **Next component tests:** jsdom + `@testing-library` in `apps/next/tests/component` (see `render.test.tsx`). Run `cd apps/next && bun run test:component`.
|
||
- **Codegen before typecheck:** after any `convex/schema.ts` or function-signature change run `cd packages/backend && bunx convex codegen` so `_generated` is current, then typecheck.
|
||
- **Conventional commits, one per task** (e.g. `feat(notifications): add notifications + notificationPreferences tables`).
|
||
- **Theme:** use existing semantic tokens only (`text-foreground`, `bg-card`, `text-muted-foreground`, `bg-primary/10`, etc.). No hard-coded hex colors. The codebase is already clean here.
|
||
- **Auth in Convex functions:** queries/mutations get the caller via `getRequiredUserId(ctx)` from `convex/model.ts`. Actions use `getAuthUserId(ctx)` (see `githubSync.ts`).
|
||
|
||
---
|
||
|
||
## Task 1: `notifications` + `notificationPreferences` schema
|
||
|
||
**Files:**
|
||
- `packages/backend/convex/schema.ts` (add two tables to `applicationTables`, ~after `threadMessages` / before `ignoredUpstreamChanges`, line ~820)
|
||
|
||
**Interfaces:**
|
||
- **Produces** table `notifications`:
|
||
```ts
|
||
notifications: defineTable({
|
||
userId: v.id('users'),
|
||
kind: v.union(
|
||
v.literal('maintenance_thread'),
|
||
v.literal('agent_turn_finished'),
|
||
v.literal('agent_needs_input'),
|
||
v.literal('sync_failed'),
|
||
v.literal('connection_needs_reauth'),
|
||
),
|
||
title: v.string(),
|
||
body: v.string(),
|
||
link: v.optional(v.string()), // in-app relative path, e.g. `/threads/<id>`
|
||
spoonId: v.optional(v.id('spoons')),
|
||
threadId: v.optional(v.id('threads')),
|
||
readAt: v.optional(v.number()),
|
||
emailedAt: v.optional(v.number()),
|
||
createdAt: v.number(),
|
||
})
|
||
.index('by_user', ['userId'])
|
||
.index('by_user_unread', ['userId', 'readAt'])
|
||
.index('by_user_created', ['userId', 'createdAt']),
|
||
```
|
||
- **Produces** table `notificationPreferences` (one row per user; unset field = default enabled):
|
||
```ts
|
||
notificationPreferences: defineTable({
|
||
userId: v.id('users'),
|
||
emailEnabled: v.optional(v.boolean()), // master email switch, default true
|
||
emailMaintenance: v.optional(v.boolean()),
|
||
emailAgentTurnFinished: v.optional(v.boolean()),
|
||
emailAgentNeedsInput: v.optional(v.boolean()),
|
||
emailSyncFailed: v.optional(v.boolean()),
|
||
emailConnectionReauth: v.optional(v.boolean()),
|
||
updatedAt: v.number(),
|
||
}).index('by_user', ['userId']),
|
||
```
|
||
|
||
**Steps:**
|
||
- [ ] Write failing test `packages/backend/tests/unit/notifications.test.ts` that inserts a `notifications` row via `t.run(async (ctx) => ctx.db.insert('notifications', {...}))` and reads it back, plus one `notificationPreferences` row. FAIL: tables don't exist (schema validation error).
|
||
- [ ] Add both `defineTable` blocks to `applicationTables` in `schema.ts`.
|
||
- [ ] `cd packages/backend && bunx convex codegen`.
|
||
- [ ] Run `bun run test:unit` — PASS.
|
||
- [ ] Commit: `feat(notifications): add notifications + notificationPreferences tables`.
|
||
|
||
---
|
||
|
||
## Task 2: emit helper, `emit` mutation wrapper, and UseSend email dispatch
|
||
|
||
The single choke point all emit points call. A plain TS helper is callable directly inside mutations; an `internalMutation` wrapper lets Convex **actions** (e.g. `githubSync`) call it via `ctx.runMutation`.
|
||
|
||
**Files:**
|
||
- `packages/backend/convex/notifications.ts` (new — helper + emit wrapper + preference read)
|
||
- `packages/backend/convex/notificationsNode.ts` (new, `'use node'` — email send action reusing `usesend-js`)
|
||
- Reference for UseSend usage: `packages/backend/convex/custom/auth/providers/usesend.ts` (env `USESEND_API_KEY`, `USESEND_URL`, `USESEND_FROM_EMAIL`; `new UseSend(apiKey, useSendUrl)`, `useSend.emails.send({from,to,subject,text,html})`).
|
||
|
||
**Interfaces:**
|
||
- **Produces** exported type + helper (import `MutationCtx` from `./_generated/server`, `Id` from `./_generated/dataModel`):
|
||
```ts
|
||
export type NotificationKind =
|
||
| 'maintenance_thread' | 'agent_turn_finished' | 'agent_needs_input'
|
||
| 'sync_failed' | 'connection_needs_reauth';
|
||
|
||
// Maps a kind to the notificationPreferences field that gates its email.
|
||
const EMAIL_PREF_FIELD: Record<NotificationKind, keyof Doc<'notificationPreferences'>> = {
|
||
maintenance_thread: 'emailMaintenance',
|
||
agent_turn_finished: 'emailAgentTurnFinished',
|
||
agent_needs_input: 'emailAgentNeedsInput',
|
||
sync_failed: 'emailSyncFailed',
|
||
connection_needs_reauth: 'emailConnectionReauth',
|
||
};
|
||
|
||
export async function emitNotification(
|
||
ctx: MutationCtx,
|
||
args: { userId: Id<'users'>; kind: NotificationKind; title: string; body: string;
|
||
link?: string; spoonId?: Id<'spoons'>; threadId?: Id<'threads'> },
|
||
): Promise<Id<'notifications'>>;
|
||
```
|
||
Behavior: insert the `notifications` row (`readAt` unset, `createdAt: Date.now()`). Then load the user's `notificationPreferences` (index `by_user`); email is allowed when `emailEnabled !== false` **and** `prefs[EMAIL_PREF_FIELD[kind]] !== false` (unset = true). If allowed and the user has an `email`, `ctx.scheduler.runAfter(0, internal.notificationsNode.sendNotificationEmail, { notificationId })`. Return the id.
|
||
- **Produces** `internalMutation emit` (thin wrapper for actions) with the same args as the helper, body `return await emitNotification(ctx, args);`.
|
||
- **Produces** `internalMutation markEmailed({ notificationId })` → patches `emailedAt: Date.now()` (called by the email action).
|
||
- **Produces** `internalQuery getForEmail({ notificationId })` → returns `{ to: string | null; title; body; link }` by loading the notification + its user's email.
|
||
- **Produces** in `notificationsNode.ts`: `internalAction sendNotificationEmail({ notificationId })` — runs `getForEmail`; if no `to`, return; else send via UseSend (subject = title, text = body + optional link, minimal HTML mirroring `usesend.ts`); on success `ctx.runMutation(internal.notifications.markEmailed, { notificationId })`. Missing `USESEND_*` env → log and return (do **not** throw; email is best-effort).
|
||
|
||
**Steps:**
|
||
- [ ] In `notifications.test.ts` add failing tests: (a) `emitNotification` inserts an unread row for the user; (b) with `emailEnabled: false` no email is scheduled (assert via a `t.run` count of scheduled functions is impractical — instead assert `emailedAt` stays undefined after `t.finishAllScheduledFunctions()` with UseSend env unset, i.e. row exists and is not emailed); (c) with prefs default (no row) and `USESEND_*` unset, `sendNotificationEmail` runs without throwing and leaves `emailedAt` undefined. FAIL: module doesn't exist.
|
||
- [ ] Implement `notifications.ts` (helper + `emit` + `markEmailed` + `getForEmail`) and `notificationsNode.ts` (`sendNotificationEmail`).
|
||
- [ ] `cd packages/backend && bunx convex codegen`.
|
||
- [ ] `bun run test:unit` — PASS.
|
||
- [ ] Commit: `feat(notifications): add emit helper + UseSend email dispatch`.
|
||
|
||
---
|
||
|
||
## Task 3: wire emit points
|
||
|
||
Call the helper/mutation at each event the spec lists. All are existing functions.
|
||
|
||
**Files & exact emit points:**
|
||
- `packages/backend/convex/threads.ts` — `createMaintenanceThread` (internalMutation, ~line 407). After the **new-thread** insert (~line 475, not the dedup/existing branch), call `await emitNotification(ctx, { userId: args.ownerId, kind: 'maintenance_thread', title: args.title, body: args.summary, link: \`/threads/${threadId}\`, spoonId: args.spoonId, threadId })`. (Import `emitNotification` from `./notifications`.)
|
||
- `packages/backend/convex/agentJobs.ts` — `updateStatus` (mutation, ~line 1108). Inside the `if (job.threadId)` block, after the thread patch: when `args.status === 'changes_ready'` **or** `'draft_pr_opened'`, emit `kind: 'agent_turn_finished'` (title `Agent turn finished`, body = `args.summary ?? job.summary ?? ''`, link `/threads/${job.threadId}`, threadId, spoonId). Emit only on the transition into that status (guard `job.status !== args.status`).
|
||
- `packages/backend/convex/agentJobs.ts` — `applyMaintenanceDecision` (mutation, ~line 1423). When the computed `status === 'waiting_for_user'`, emit `kind: 'agent_needs_input'` (title `Agent needs your input`, body `args.summary`, link `/threads/${job.threadId}`).
|
||
- `packages/backend/convex/githubSync.ts` — this is a `'use node'` action, so use `await ctx.runMutation(internal.notifications.emit, {...})`. Two sites:
|
||
- Terminal `catch` of `refreshOwnedSpoon` (~line 291, the outer catch that sets `syncStatus: 'error'`): emit `kind: 'sync_failed'` (title `Sync failed for ${spoon.name}`, body `message`, link `/spoons/${spoonId}`, spoonId). `spoon.name` is in scope.
|
||
- The maintenance-thread branches already call `createMaintenanceThread` (which now emits) — do **not** double-emit there.
|
||
- **Connection needs re-auth** — the Phase 4 webhook sets `gitConnections.status = 'needs_reauth' | 'revoked'`. Emit from that mutation (created in Phase 4). If that mutation does not yet exist in this branch, add a `// TODO(phase4): emit connection_needs_reauth here` marker at `packages/backend/convex/github.ts` near the connection status writes (~line 181) and emit from the first mutation that patches a connection to `needs_reauth`. Kind `connection_needs_reauth`, title `GitHub connection needs re-authorization`, body `Reconnect your GitHub account to keep syncing.`, link `/settings/integrations`.
|
||
|
||
**Interfaces:**
|
||
- **Consumes** `emitNotification` (mutations) / `internal.notifications.emit` (actions) from Task 2.
|
||
|
||
**Steps:**
|
||
- [ ] Add failing tests in `notifications.test.ts`: (a) calling `createMaintenanceThread` (via `t.run`/internal) for a fresh upstream produces one `maintenance_thread` notification for the owner; a second dedup call does **not** add another. (b) `updateStatus` to `changes_ready` on a claimed job with a thread produces one `agent_turn_finished` notification; a repeat call at the same status does not add a second. Use the harness helpers to seed a spoon/thread/job like existing agentJobs tests. FAIL: no notifications produced.
|
||
- [ ] Add the emit calls at the five points above (imports as noted).
|
||
- [ ] `cd packages/backend && bunx convex codegen`; `bun run test:unit` — PASS.
|
||
- [ ] Commit: `feat(notifications): emit at maintenance/turn/sync/reauth events`.
|
||
|
||
---
|
||
|
||
## Task 4: inbox query + mark-read mutations
|
||
|
||
**Files:**
|
||
- `packages/backend/convex/notifications.ts` (add public query/mutations)
|
||
|
||
**Interfaces:**
|
||
- **Produces** `query listMine({ limit?: number })` → `Doc<'notifications'>[]` for `getRequiredUserId(ctx)`, index `by_user_created` desc, `take(args.limit ?? 30)`.
|
||
- **Produces** `query unreadCount({})` → `number`. Use index `by_user_unread` filtered to `readAt === undefined` (`.withIndex('by_user_unread', q => q.eq('userId', uid).eq('readAt', undefined))`), `.take(100)` then `.length` capped display (return raw count via `.collect().length` bounded by take(100)).
|
||
- **Produces** `mutation markRead({ notificationId })` → ensures the row's `userId === getRequiredUserId(ctx)` (else `ConvexError('Notification not found.')`), patch `readAt: Date.now()` if unset.
|
||
- **Produces** `mutation markAllRead({})` → iterate `by_user_unread` unread rows for the caller, patch each `readAt`.
|
||
|
||
**Steps:**
|
||
- [ ] Failing tests in `notifications.test.ts`: seed 3 notifications for user A + 1 for user B via `t.run`; `authed(t, A).query(api.notifications.listMine, {})` returns exactly A's 3 newest-first; `unreadCount` returns 3; `markRead` on one drops `unreadCount` to 2 and rejects B's row for A; `markAllRead` zeroes it. FAIL: functions missing.
|
||
- [ ] Implement the query + mutations.
|
||
- [ ] `bunx convex codegen`; `bun run test:unit` — PASS.
|
||
- [ ] Commit: `feat(notifications): inbox query + mark-read mutations`.
|
||
|
||
---
|
||
|
||
## Task 5: header notification bell
|
||
|
||
**Files:**
|
||
- `apps/next/src/components/layout/header/controls/notification-bell.tsx` (new)
|
||
- `apps/next/src/components/layout/header/controls/index.tsx` (or wherever `Controls` is composed — confirm by reading `apps/next/src/components/layout/header/controls/`) — mount `<NotificationBell />` before `<AvatarDropdown />`, only when authenticated (use `useConvexAuth().isAuthenticated`, matching `header/index.tsx`).
|
||
|
||
**Interfaces:**
|
||
- **Consumes** `api.notifications.unreadCount`, `api.notifications.listMine`, `api.notifications.markRead`, `api.notifications.markAllRead`.
|
||
- Client component (`'use client'`). Uses `useQuery`/`useMutation` from `convex/react`. Bell icon from `lucide-react` (`Bell`). Wrap in the existing `@spoon/ui` `DropdownMenu` primitives (same set `AvatarDropdown.tsx` imports). Unread badge: small `bg-primary text-primary-foreground` count pill when `unreadCount > 0` (show `9+` above 9). Each item is a `Link` to `notification.link ?? '#'` that calls `markRead({ notificationId })` on click; a "Mark all read" action calls `markAllRead`. Empty state: `No notifications yet.` Relative time via existing date util if present (grep `formatDistanceToNow`/`date-fns` under `apps/next/src`); otherwise a minimal inline formatter.
|
||
|
||
**Steps:**
|
||
- [ ] Failing component test `apps/next/tests/component/notification-bell.test.tsx`: render `<NotificationBell />` with a mocked Convex client (follow `render.test.tsx` for the provider/mock pattern) returning `unreadCount: 2` and two notifications; assert the badge shows `2` and both titles render; clicking one invokes the `markRead` mock. FAIL: component missing.
|
||
- [ ] Implement `notification-bell.tsx`; mount it in the header controls.
|
||
- [ ] `cd apps/next && bun run test:component` — PASS.
|
||
- [ ] Manual verify: `bun run dev`, sign in, confirm the bell renders and the badge updates reactively after seeding a notification. Document the check.
|
||
- [ ] Commit: `feat(notifications): header notification bell`.
|
||
|
||
---
|
||
|
||
## Task 6: Settings → Notifications preferences
|
||
|
||
**Files:**
|
||
- `packages/backend/convex/notifications.ts` (add `getPreferences` query + `updatePreferences` mutation)
|
||
- `apps/next/src/app/(app)/settings/notifications/page.tsx` (new)
|
||
- `apps/next/src/components/settings/notification-preferences-panel.tsx` (new)
|
||
- `apps/next/src/app/(app)/settings/layout.tsx` (add nav item `{ href: '/settings/notifications', label: 'Notifications', icon: Bell }` — import `Bell` from `lucide-react`; insert after Dotfiles)
|
||
|
||
**Interfaces:**
|
||
- **Produces** `query getPreferences({})` → the caller's `notificationPreferences` row or a defaults object `{ emailEnabled: true, emailMaintenance: true, emailAgentTurnFinished: true, emailAgentNeedsInput: true, emailSyncFailed: true, emailConnectionReauth: true }` when no row (unset = enabled).
|
||
- **Produces** `mutation updatePreferences({ emailEnabled?, emailMaintenance?, emailAgentTurnFinished?, emailAgentNeedsInput?, emailSyncFailed?, emailConnectionReauth? })` → upsert the caller's row (patch existing or insert), set `updatedAt`.
|
||
- Panel: `'use client'`, `useQuery(api.notifications.getPreferences)` + `useMutation(api.notifications.updatePreferences)`. **Seed local state from the query using the hydrated-flag pattern** (Task 9 reference) so late-resolving prefs don't clobber. Each toggle is a `Switch`/`Checkbox` from `@spoon/ui`; on change call the mutation in a `try/catch` with `toast.success('Preferences saved.')` on resolve and `toast.error(...)` on failure (see Task 8 pattern). Master `emailEnabled` disables the per-kind toggles when off.
|
||
|
||
**Steps:**
|
||
- [ ] Failing backend test in `notifications.test.ts`: `getPreferences` returns all-true defaults with no row; `updatePreferences({ emailSyncFailed: false })` persists and a subsequent `getPreferences` reflects it; the gating in Task 2 respects it (emit `sync_failed` with pref false → no email scheduled / `emailedAt` stays unset). FAIL.
|
||
- [ ] Implement `getPreferences` + `updatePreferences`; `bunx convex codegen`; `bun run test:unit` — PASS.
|
||
- [ ] Build the page + panel; add the settings nav item.
|
||
- [ ] Manual verify: toggle a preference, reload, confirm it persists. Document the check.
|
||
- [ ] Commit: `feat(notifications): settings notification preferences`.
|
||
|
||
---
|
||
|
||
## Task 7: loading skeletons for dashboard / spoons / threads
|
||
|
||
Kills the zero-state flash where `useQuery(...) ?? []` renders an empty state before data resolves.
|
||
|
||
**Files (read each first for exact `useQuery` lines):**
|
||
- `apps/next/src/app/(app)/dashboard/page.tsx` (~lines 14–16: `?? []` on `listMineWithState`, `syncRuns.listRecent`, `threads.listMine`)
|
||
- `apps/next/src/app/(app)/spoons/page.tsx` (~line 35 `?? []`)
|
||
- `apps/next/src/app/(app)/threads/page.tsx` (`?? []` on list)
|
||
- New: `apps/next/src/components/ui/list-skeleton.tsx` (or reuse an existing `Skeleton` from `@spoon/ui` if present — grep `Skeleton` under `packages/ui`/`apps/next` first).
|
||
|
||
**Interfaces:**
|
||
- **Consumes** existing Convex list queries. Change pattern: keep the raw query result **without** `?? []`, so `undefined` (loading) is distinguishable from `[]` (empty). Render a skeleton block while `data === undefined`; render the real empty-state only when `data.length === 0`.
|
||
|
||
**Steps:**
|
||
- [ ] Component test `apps/next/tests/component/dashboard-loading.test.tsx` (or extend `render.test.tsx`): with the query mock returning `undefined`, assert a skeleton (`data-testid="list-skeleton"` / `role="status"`) is shown and the empty-state copy is **not**; with `[]`, assert the empty state shows and no skeleton. FAIL.
|
||
- [ ] Add the skeleton component; update the three pages to branch on `undefined` vs `[]`. Give the skeleton `role="status"` + `aria-label="Loading"`.
|
||
- [ ] `bun run test:component` — PASS.
|
||
- [ ] Commit: `fix(ui): loading skeletons for dashboard/spoons/threads`.
|
||
|
||
---
|
||
|
||
## Task 8: fire-and-forget mutation error handling across panels
|
||
|
||
Wrap unhandled mutation calls in `try/catch`; only `toast.success` after resolve; `toast.error` on failure. Model on the already-correct `importAll`/`newFile` in `dotfiles-manager.tsx` (lines 172–181, 221–230).
|
||
|
||
**Files & exact sites:**
|
||
- `apps/next/src/components/settings/dotfiles/dotfiles-manager.tsx` — `saveSelected` (lines 161–170: wrap `await putFile(...)` in try/catch; `toast.success('Saved.')` inside try after resolve, `toast.error(...)` in catch) and `deleteSelected` (lines 232–239: same around `await removeFile(...)`).
|
||
- `apps/next/src/components/integrations/ai-provider-profiles-panel.tsx` (~lines 235–240 and 282–285: the save + delete mutations).
|
||
- `apps/next/src/components/spoons/spoon-clone-panel.tsx` (~lines 225–228).
|
||
- `apps/next/src/components/spoons/spoon-secrets-form.tsx` (~lines 312–315; **and** the sequential import at ~134–141 — wrap each iteration so one failure surfaces via `toast.error` and doesn't silently swallow the rest; report count of successes).
|
||
|
||
**Interfaces:** no signature changes; behavior only. Error message: `error instanceof Error ? error.message : '<action> failed.'`.
|
||
|
||
**Steps:**
|
||
- [ ] Read each file at the cited lines to capture exact `await <mutation>(...)` calls.
|
||
- [ ] Component test `apps/next/tests/component/dotfiles-error.test.tsx`: render the dotfiles manager with a `putFile` mock that rejects; trigger save; assert `toast.error` called and `toast.success` **not** called. (Mock `sonner`'s `toast`.) FAIL (currently unconditional success).
|
||
- [ ] Apply try/catch + resolve-gated success to all sites above.
|
||
- [ ] `bun run test:component` — PASS. Manual grep to confirm no remaining unconditional `toast.success(` sits before its `await` in these files.
|
||
- [ ] Commit: `fix(ui): error-handle fire-and-forget mutations in settings panels`.
|
||
|
||
---
|
||
|
||
## Task 9: settings/thread forms re-sync on loaded record id
|
||
|
||
`useState` seeded once from a late-resolving Convex prop can save defaults over real config. Fix with the **hydrated-flag** pattern already in `dotfiles-manager.tsx` RepoPanel (lines 383–394): a `hydrated` boolean, an effect that seeds from the loaded record then sets `hydrated`, keyed on the record's `_id`.
|
||
|
||
**Files:**
|
||
- `apps/next/src/components/spoons/spoon-agent-settings-form.tsx` (~lines 61–119: `useState` seeded from the settings query)
|
||
- `apps/next/src/components/threads/thread-workspace-form.tsx` (~lines 57–74)
|
||
|
||
**Interfaces:**
|
||
- Pattern (per form): `const [hydrated, setHydrated] = useState(false);` + `useEffect(() => { if (!record || hydrated) return; /* seed each useState from record fields */ setHydrated(true); }, [record, hydrated]);`. If the mounting parent can swap records, prefer remounting via `key={record._id}` on the form in the parent **or** reset `hydrated` when `record._id` changes (`useEffect(() => setHydrated(false), [record?._id])`). Choose keyed remount if the parent already has the id; otherwise the id-change effect.
|
||
|
||
**Steps:**
|
||
- [ ] Component test `apps/next/tests/component/form-resync.test.tsx`: render `spoon-agent-settings-form` first with the settings prop `undefined`, then rerender with a resolved record having non-default values; assert the inputs reflect the loaded values (not the initial defaults). FAIL (state seeded once from undefined).
|
||
- [ ] Apply the hydrated-flag (or keyed-remount) pattern to both forms.
|
||
- [ ] `bun run test:component` — PASS.
|
||
- [ ] Commit: `fix(ui): re-sync settings/thread forms on loaded record`.
|
||
|
||
---
|
||
|
||
## Task 10: sign-in + forgot-password copy & input-retention fixes
|
||
|
||
**Files & exact fixes:**
|
||
- `apps/next/src/app/(auth)/sign-in/page.tsx`
|
||
- Remove `signInForm.reset()` from the `finally` on failed sign-in (line 140) — keep the email/password on error. Only reset on success (move a `.reset()` into the success branch if desired, or drop it). Same for `signUpForm.reset()` (line 163) and `verifyEmailForm.reset()` (line 182): do not wipe input in `finally`; reset only after a successful transition.
|
||
- `pendingText='Signing Up...'` on the **Verify Email** button (line 235) → `pendingText='Verifying...'`. Leave the actual sign-up button (line 443) as `Signing Up...`.
|
||
- Typo `Confirm Passsword` (line 426) → `Confirm Password`.
|
||
- `apps/next/src/app/(auth)/forgot-password/page.tsx`
|
||
- `Please enter the one-time password sent to your phone.` (~line 232–233) → `...sent to your email.`
|
||
- `Confirm Passsword` typo (~line 268) → `Confirm Password`.
|
||
|
||
**Steps:**
|
||
- [ ] Component test `apps/next/tests/component/sign-in-retention.test.tsx`: render the sign-in form, type an email + password, make the `signIn` mock reject, submit; assert the email/password inputs still hold the typed values after the rejection settles. FAIL (reset in finally wipes them).
|
||
- [ ] Apply the input-retention + copy fixes above.
|
||
- [ ] `bun run test:component` — PASS. Manual grep: `grep -rn "Passsword\|sent to your phone\|Signing Up" apps/next/src/app/(auth)` returns only the intentional sign-up button.
|
||
- [ ] Commit: `fix(auth): retain input on failure + copy fixes`.
|
||
|
||
---
|
||
|
||
## Task 11: profile page `'use server'` removal + AvatarDropdown fixes
|
||
|
||
**Files:**
|
||
- `apps/next/src/app/(app)/settings/profile/page.tsx` (line 1: remove the stray `'use server';` — this is an async server component using `preloadQuery`; `'use server'` marks it as a Server Actions module and is wrong here).
|
||
- `apps/next/src/components/layout/header/controls/AvatarDropdown.tsx`
|
||
- Line 73: `Link href='/profile'` → `href='/settings/profile'` (the `/profile` route is being deleted in Task 14).
|
||
- Lines 64 & 67: empty-string name falls through `??`. Change `user?.name ?? user?.email` and `user.name?.trim() ?? user.email?.trim()` to use `||` so an empty/whitespace name falls back to email: `(user?.name?.trim() || user?.email)` and label `{user.name?.trim() || user.email?.trim()}`.
|
||
|
||
**Steps:**
|
||
- [ ] Manual verification (test impractical — server component + dropdown wiring):
|
||
- Before: `head -1 apps/next/src/app/(app)/settings/profile/page.tsx` shows `'use server';`. After: first line is `import ...`; `grep -n "use server" apps/next/src/app/(app)/settings/profile/page.tsx` returns nothing.
|
||
- Before: `grep -n "href='/profile'" .../AvatarDropdown.tsx` matches. After: it matches `href='/settings/profile'` and no bare `/profile`.
|
||
- Before: `grep -n "?? user" .../AvatarDropdown.tsx` matches lines 64/67. After: those use `|| user`.
|
||
- [ ] Apply the three edits.
|
||
- [ ] `cd apps/next && bun run typecheck` (or `bun run build` if no dedicated typecheck script) — passes.
|
||
- [ ] Commit: `fix(ui): profile page directive + avatar dropdown links/fallbacks`.
|
||
|
||
---
|
||
|
||
## Task 12: accessibility — remove `role='link'` with nested interactive elements
|
||
|
||
A `role='link'` container wrapping real `<a>`/`<Link>` children is an invalid nested-interactive pattern.
|
||
|
||
**Files:**
|
||
- `apps/next/src/app/(app)/spoons/page.tsx` (~lines 112–124: `TableRow role='link'` containing links)
|
||
- `apps/next/src/app/(app)/threads/page.tsx` (~lines 317–329: `Card role='link'` containing links)
|
||
|
||
**Interfaces:**
|
||
- Pattern: drop the `role='link'` (and any `tabIndex`/`onKeyDown` that simulated link behavior on the container). Make the row/card navigable via a single primary `Link` — either a full-row `Link` wrapping the content, or a "stretched link" overlay (`<Link className="absolute inset-0" ...><span className="sr-only">…</span></Link>` on a `relative` container) so nested action links remain individually clickable. Preserve existing hover styles.
|
||
|
||
**Steps:**
|
||
- [ ] Read both cited blocks to capture current structure.
|
||
- [ ] Component test `apps/next/tests/component/list-a11y.test.tsx`: render the spoons table (mock data) and assert no element has `role="link"` and the row exposes an accessible link to the spoon detail. FAIL (role='link' present).
|
||
- [ ] Refactor both to the stretched-link (or single wrapping Link) pattern; remove `role='link'` and simulated key handlers.
|
||
- [ ] `bun run test:component` — PASS. Manual grep: `grep -rn "role='link'" apps/next/src/app` returns nothing.
|
||
- [ ] Commit: `fix(a11y): remove nested-interactive role='link' on spoon/thread lists`.
|
||
|
||
---
|
||
|
||
## Task 13: agent workspace UX polish
|
||
|
||
Group the Phase-5 workspace items. Read each cited range first.
|
||
|
||
**Files & fixes:**
|
||
- `apps/next/src/components/agent-workspace/agent-workspace-shell.tsx`
|
||
- **Diff/tree load race** (~124–138): guard out-of-order responses. Track a per-request sequence number (or `AbortController`); ignore a response whose request id is stale so a slower earlier fetch can't overwrite newer data.
|
||
- **Editor buffer not refreshed when agent edits the open file** (~252–265): when a workspace change (`agentWorkspaceChanges`) arrives for the currently open file: if the buffer is **not dirty**, reload the file content into the editor; if dirty, set a conflict flag (badge/toast "This file changed on disk") rather than silently overwriting.
|
||
- **Recovery panel over-triggers** (~503–557): require **N consecutive** status/health failures (e.g. `const RECOVERY_THRESHOLD = 3`) before showing the recovery panel; reset the counter on any success. One transient error must not trip it.
|
||
- **Mobile layout** (~476 `min-h-[720px]`, ~562 `grid-cols-1`): replace the fixed `min-h-[720px]` with a viewport-relative min-height (e.g. `min-h-[70vh]`/`min-h-[calc(100dvh-…)]`) and ensure the single-column mobile layout collapses the file tree (collapsible/`Sheet` or a toggle) instead of stacking full-height panels.
|
||
- `apps/next/src/components/agent-workspace/agent-thread.tsx`
|
||
- **Auto-scroll fights the user** (~114–132): only auto-scroll to newest when the user is already near the bottom (compute `scrollHeight - scrollTop - clientHeight < threshold`, e.g. 80px). If scrolled up, don't yank; optionally show a "jump to latest" affordance.
|
||
- `apps/next/src/components/agent-workspace/code-editor.tsx`
|
||
- **editorRef not cleared on unmount** (~53, ~157): in the mount effect's cleanup, dispose the editor and set `editorRef.current = null` to avoid a stale ref / leak across remounts.
|
||
|
||
**Steps:**
|
||
- [ ] Read each cited range.
|
||
- [ ] Component test `apps/next/tests/component/agent-thread-scroll.test.tsx`: simulate the messages list scrolled up (mock `scrollTop`/`scrollHeight`/`clientHeight`), append a new message, assert `scrollTo`/`scrollIntoView` is **not** called; then with near-bottom, assert it **is**. FAIL.
|
||
- [ ] Add a unit-style test for the recovery threshold: extract the consecutive-failure decision into a small pure helper (`shouldShowRecovery(consecutiveFailures, threshold)`) and test it, or assert via a component test that 1–2 failures don't render the recovery panel and the 3rd does. FAIL.
|
||
- [ ] Implement all fixes above (sequencing guard, dirty-aware buffer refresh, consecutive-failure threshold, mobile min-height + collapsible tree, near-bottom auto-scroll, editorRef cleanup).
|
||
- [ ] `bun run test:component` — PASS.
|
||
- [ ] Manual verify in `bun run dev`: open a workspace, scroll the thread up while the agent streams (no yank), trigger an agent edit of an open clean file (buffer refreshes), resize to mobile width (tree collapses). Document the checks.
|
||
- [ ] Commit: `fix(workspace): scroll/diff-race/recovery/editor/mobile polish`.
|
||
|
||
---
|
||
|
||
## Task 14: dead code removal + worker `redact.ts` / `env.ts` cleanup
|
||
|
||
Group all removals + the two worker cleanups. **Note:** `redact.ts` and `env.ts` items are Phase-5-scoped per the audit; some `env.ts` vars may already be gone after Phase 2 — only remove ones that are still present **and** unreferenced.
|
||
|
||
**Files & actions:**
|
||
- Delete route dirs: `apps/next/src/app/(app)/updates/`, `apps/next/src/app/(app)/agents/`, `apps/next/src/app/(app)/settings/ai/`, `apps/next/src/app/(auth)/profile/`.
|
||
- Delete component: `apps/next/src/components/landing/tech-stack.tsx` (grep first for imports; remove any import/usage — likely in a landing page).
|
||
- Delete empty dirs: `apps/next/src/components/agents/`, `apps/next/src/components/updates/` (only if empty after the above).
|
||
- `apps/next/src/proxy.ts` (lines 11–12): remove `'/updates(.*)'`, `'/agents(.*)'`, and `'/profile(.*)'` from `isProtectedRoute` (those routes no longer exist).
|
||
- `apps/agent-worker/src/redact.ts` (lines 1–17): the first three patterns (`ghs_…`, `github_pat_…`, `sk-…`) have **no** capture group, so the shared `'$1=[redacted]'` replacement emits a literal `$1`. Fix: give each pattern its own replacement — group-less patterns replace with `'[redacted]'`; the `(client_secret|…)=(…)` pattern keeps `'$1=[redacted]'`. Restructure `secretPatterns` to `{ pattern, replacement }` objects and map accordingly.
|
||
- `apps/agent-worker/src/env.ts`: remove `terminalImage` and `terminalIdleMs` (and any other audit-listed unused vars — `containerAccess`, `maxConcurrentJobs`) **only if** a repo-wide grep shows no remaining references. Grep before deleting each.
|
||
|
||
**Steps:**
|
||
- [ ] Grep for references to each route/component before deleting: `grep -rn "tech-stack\|/updates\|/agents\|/settings/ai\|(auth)/profile\|href='/profile'" apps/next/src`. Confirm nothing (except the AvatarDropdown link already fixed in Task 11) still points at them.
|
||
- [ ] Failing worker test `apps/agent-worker/src/redact.test.ts` (vitest): `createRedactor([])('token ghs_ABC123 sk-xyz')` must contain `[redacted]` and **not** contain the literal `$1`; the `api_key=secret` case still yields `api_key=[redacted]`. FAIL (current output has `$1`).
|
||
- [ ] Fix `redact.ts` per-pattern replacement; run the worker test (`cd apps/agent-worker && bunx vitest run src/redact.test.ts` or the repo's worker test command) — PASS.
|
||
- [ ] Delete the routes/components/empty dirs; trim `proxy.ts`; remove verified-unused `env.ts` vars (grep each first).
|
||
- [ ] `cd apps/next && bun run build` (or typecheck) passes with no missing-import errors; `cd apps/agent-worker && <typecheck>` passes.
|
||
- [ ] Manual grep confirms removals: `find apps/next/src/app -type d \( -name updates -o -name agents -o -name ai -o -name profile \)` returns nothing under the deleted paths.
|
||
- [ ] Commit: `chore: remove dead routes/components + fix redact/env cleanup`.
|
||
|
||
---
|
||
|
||
## Task 15: docs rewrite for the box-first model
|
||
|
||
**Files:**
|
||
- `README.md`
|
||
- `docs/*.md` (enumerate: `ls docs/*.md` and any relevant subdirs; the spec calls out server-deploy notes)
|
||
|
||
**Interfaces:** documentation only. Rewrite the per-job-container narrative to the Phase-2 **one long-running per-user box** model:
|
||
- Each user owns a persistent Fedora container (`spoon-box-*`) with a persistent home (`homes/<username>/`, `~/Code`), dotfiles, terminal, and agent CLIs; the legacy per-job OpenCode container path is gone.
|
||
- Three agent runtimes exec into the box: Codex, OpenCode, **and Claude Code** (note the new Anthropic provider-profile kind).
|
||
- **GitHub webhooks:** document the webhook URL (Convex `httpAction`) and `GITHUB_APP_WEBHOOK_SECRET`; hourly cron remains a fallback.
|
||
- **New env vars:** list the box/terminal/notification/webhook env (`SPOON_AGENT_BOX_IDLE_MS`, `USESEND_API_KEY`/`USESEND_URL`/`USESEND_FROM_EMAIL`, `GITHUB_APP_WEBHOOK_SECRET`, Claude Code auth) and drop removed ones (`SPOON_AGENT_TERMINAL_IMAGE`, `SPOON_AGENT_TERMINAL_IDLE_MS`, and other Task-14-removed vars).
|
||
- Add a short **Notifications** section (in-app bell + email via UseSend, per-user preferences, web push out of scope).
|
||
|
||
**Steps:**
|
||
- [ ] `ls docs/*.md docs/**/*.md README.md`; read each to find per-job-container language (`grep -rn "per-job\|spoon-agent-job\|container per\|host_port" README.md docs`).
|
||
- [ ] Rewrite the affected sections per the bullets above. No stale references to deleted routes/env.
|
||
- [ ] Manual verification: `grep -rn "spoon-agent-job\|per-job container" README.md docs` returns nothing; the deploy doc lists the webhook URL + `GITHUB_APP_WEBHOOK_SECRET` + `USESEND_*` + Claude Code; a Notifications section exists.
|
||
- [ ] Commit: `docs: rewrite for box-first model + webhooks + notifications`.
|
||
|
||
---
|
||
|
||
## Manual verification checklist (end of phase)
|
||
|
||
- [ ] Trigger a maintenance thread (diverged spoon) → in-app bell increments and (with email pref on + `USESEND_*` set) an email arrives.
|
||
- [ ] Finish an agent turn (`changes_ready`) → `agent_turn_finished` notification; a maintenance decision needing approval → `agent_needs_input`.
|
||
- [ ] Force a sync failure → `sync_failed` notification.
|
||
- [ ] Toggle each preference off → the corresponding email stops while the in-app row still appears.
|
||
- [ ] Mark-read / mark-all-read update the badge reactively.
|
||
- [ ] Dashboard/spoons/threads show skeletons (no empty-state flash) on cold load.
|
||
- [ ] Sign-in keeps input on failure; auth copy fixed; forms re-sync from loaded records.
|
||
- [ ] Deleted routes 404; app builds; worker `redact.ts` no longer emits `$1`.
|