diff --git a/docs/superpowers/specs/2026-07-10-spoon-stabilize-unify-build-design.md b/docs/superpowers/specs/2026-07-10-spoon-stabilize-unify-build-design.md new file mode 100644 index 0000000..81f7aa1 --- /dev/null +++ b/docs/superpowers/specs/2026-07-10-spoon-stabilize-unify-build-design.md @@ -0,0 +1,278 @@ +# Spoon: Stabilize → Unify → Build + +**Date:** 2026-07-10 +**Status:** Approved (design review with Gib, 2026-07-10) + +## Purpose + +Fix the bugs found in the 2026-07-10 full audit of `apps/next`, `apps/agent-worker`, +and `packages/backend/convex`, finish migrating the system onto the Phase-2 +"one long-running per-user box" architecture, and build the remaining features +that make Spoon the intended product: a self-hosted site where each user owns a +persistent Fedora container (dotfiles, terminal, agent CLIs) and uses it to keep +forks close to upstream and to do agent-driven or manual work on projects. + +## Decisions (from design review) + +- **Tenancy:** single instance shared by the owner + trusted friends. Per-user + boxes/homes must be correct; no hostile-tenant hardening (shared Docker daemon + is acceptable), but no cross-user data exposure either (the symlink escape must + be fixed). +- **Agent runtimes:** Codex, OpenCode, **and Claude Code**, all exec'ing into the + per-user box through one adapter interface. The legacy per-job OpenCode + container path is deleted. +- **Feature scope:** all four tracks — standalone dev-box page, persistent + terminal UX, upstream-sync correctness, notifications & polish. +- **Ordering:** stabilize first, then unify the runtime, then build features. + Each phase ships working. + +## Audit summary (what this design fixes) + +Severity-ranked highlights; file references are as of commit `b092955`. + +### Critical + +1. **Worker cleanup deletes all user homes.** `cleanupOrphanedWorkspaces` + (`apps/agent-worker/src/worker.ts:1931`) rm -rf's any top-level workdir entry + not in the active set; the Phase-2 `homes/` directory is never in that set. + `POST /cleanup` (Settings → Worker) wipes every user's home. +2. **Jobs wedge permanently.** (a) `claimNextInternal` commits `claimed` before + the claim action validates profile/secrets (`convex/agentJobsNode.ts:64`) — a + validation throw strands the job; (b) no server-side timeout: `lastHeartbeatAt` + is never read, the worker never calls `heartbeatWorkspace`, no recovery cron + exists — a worker crash strands jobs and blocks their threads forever; + (c) a parsed maintenance review resolves the thread but leaves the job + `running` (`worker.ts:1732`, `agentJobs.ts:1423`). +3. **Cancel is ignored.** The worker is never told about `cancel`; its next + `updateStatus` write flips the job out of `cancelled` and the container keeps + running (`agentJobs.ts:925`, `1120`). + +### High + +4. **Auto-sync ignores settings.** `autoSyncEnabled` (default false), + `requireAiLowRiskForSync`, `requireCleanCompareForSync`, `autoReviewEnabled` + are never read; `refreshOwnedSpoon` fast-forwards unconditionally + (`convex/githubSync.ts:205`). +5. **Symlink path escape.** `safeWorkspacePath` / `safeHomeJoin` validate + lexically then follow symlinks (`worker.ts:1059`, + `user-environment.ts:35`) — a repo symlink lets file routes read/write worker + secrets and other users' homes. +6. **Terminal renders quarter-size.** xterm measures cell size at `fit()` before + the Victor Mono webfont loads and nothing re-fits + (`apps/next/src/components/agent-workspace/workspace-terminal.tsx:145`). The + ResizeObserver only sees container-box changes. Monaco already has the fix + pattern (`monaco-theme.ts:161` uses `document.fonts.ready`). +7. **Terminal resize after connect is a no-op** worker-side: PTY size is set + once via `stty` at launch; resize messages only mutate local vars + (`apps/agent-worker/src/terminal.ts:40-64,103`). +8. **Box lifecycle races/leaks.** `acquireUserBox`/`ensureUserContainer` aren't + concurrency-safe (duplicate `docker run`, reaper TOCTOU); terminal disconnect + during acquire leaks a ref forever; refcount is a clamped shared counter, so + double-release reaps a box under an active user; worker restarts orphan all + `spoon-box-*` containers (`user-container.ts:15-34`, `terminal.ts:66-98`, + `runtime/docker.ts:341`). +9. **Workspace shell isn't keyed by job.** Thread A → B navigation shows A's + files and overwrites B's persisted UI state with A's + (`threads/[threadId]/page.tsx:56`, `agent-workspace-shell.tsx:88,280`). + +### Medium + +10. Exec timeout/abort kills the local `docker exec` client, not the process in + the box; box lacks `--init`, so zombies accumulate (`runtime/docker.ts:385`). +11. OpenCode still uses the pre-Phase-2 per-job `spoon-agent-job-{jobId}` + container — two containers per OpenCode job (`worker.ts:620-696`). +12. Drift >100 commits yields a wrong head SHA (`githubClient.ts:122`), and the + thread-dedup fallback key `Date.now()` (`githubSync.ts:232`) creates a new + maintenance thread + job every scheduled run. +13. No GitHub webhooks; `gitConnections.status` never leaves `active`; no + rate-limit retry/backoff. +14. Deleted/foreign spoon or thread URLs throw to `global-error.tsx` (no `(app)` + `error.tsx`/`not-found.tsx`). +15. Failed initial turn leaves `activeWorkspaces` entry with released box ref + (`worker.ts:1410-1437`); terminal PTY destroyed on tab switch (Radix tabs + unmount); diff/tree loads race with no ordering guard; editor never refreshes + a file the agent rewrote; recovery panel over-triggers on one transient + status error. +16. Materialized secret env files persist in plaintext in the persistent home + (`worker.ts:1167`). +17. Settings forms seed `useState` from late-resolving queries and can save + defaults over real config (`spoon-agent-settings-form.tsx:61`); several + fire-and-forget mutations toast success unconditionally + (`dotfiles-manager.tsx:161`, `ai-provider-profiles-panel.tsx:235`, etc.). +18. `updateStatus` accepts arbitrary transitions; worker-token compare untrimmed + in one place (`userDotfilesNode.ts:22`); `getEnvironmentForJob` doesn't bind + to the claiming worker; unbounded owner-wide `.collect()`s; + `podman :Z` relabel conflicts on shared mounts; `redact.ts` `$1` replacement + for group-less patterns; podman/host_port dev path diverges from prod. + +### Low / cleanup + +19. Dead code: `/updates`, `/agents`, `/settings/ai`, `(auth)/profile` routes; + `components/landing/tech-stack.tsx`; empty `components/agents|updates` dirs; + unused per-job docker helpers (`runInJobContainer`, `streamInJobContainer`, + `execInWorkspaceContainer`, `inspectWorkspaceContainer`); env vars + `terminalImage`, `terminalIdleMs`; `maxConcurrentJobs` unused (serial loop). +20. UX copy: sign-in form wipes input on failure; "phone" instead of email in + forgot-password; "Confirm Passsword" typo; "Signing Up..." on Verify Email; + avatar label blank for empty-string names; `role='link'` with nested + interactive elements; zero-state flash while dashboards load. + +## Design + +### Phase 1 — Stabilize + +Targeted fixes on the current architecture, in small independent commits. + +- **Cleanup rewrite.** `cleanupOrphanedWorkspaces` becomes layout-aware: it + never removes `homes/` or any `homes/{username}` root; it may remove + (a) legacy pre-Phase-2 top-level job dirs, and (b) per-thread checkouts + `homes/*/Code/{spoon}/{branch}` whose thread/job is terminal — and only for + users with no running box. Health and cleanup enumerate `spoon-box-*`. +- **Job lifecycle safety.** + - Validate everything (spoon exists, profile exists/enabled, secrets + decryptable) inside `claimNextInternal` before patching, so a bad job goes + `failed` with a reason instead of wedging `claimed`. + - Worker heartbeats every ~30s per active workspace (`heartbeatWorkspace`). + - New recovery cron: non-terminal jobs with stale heartbeat (or `claimed` with + no heartbeat within N minutes) → `timed_out`; thread status follows; bounded + automatic requeue for never-started claims. + - `updateStatus` validates transitions; writes to terminal-status jobs are + rejected (fixes cancel-revert). + - Heartbeat response carries `cancelRequested`; the worker tears down the + workspace (abort turn, kill in-box processes for that job, release box). + - `applyMaintenanceDecision` closes the job (terminal status) and stops the + workspace. +- **Sync settings honored.** Auto-sync requires `autoSyncEnabled` (and the + `require*` flags when set); maintenance/AI review gated on `autoReviewEnabled`. + Migration: set `autoSyncEnabled=true` on existing spoons and flip the default + for new GitHub-created spoons, preserving today's observed behavior while + making it controllable. +- **Symlink containment.** All worker file access (`file` GET/PUT, tree, dotfile + overlay writes) resolves `fs.realpath` of the parent and re-checks containment; + reject symlinked targets for writes. +- **Terminal sizing.** + - Client: after `term.open()`, re-run `fit()` + send resize on + `document.fonts.ready` and on Nerd-Font load; first fit in `requestAnimationFrame`. + - Worker: replace the `script -qfc` pipe hack with a real TTY exec via + dockerode (`exec.start({Tty:true})` + `exec.resize({h,w})`), making resize + messages actually resize the PTY. tmux reattach behavior unchanged. +- **Workspace shell keyed by job.** `` on + both mounting routes. +- **Box lifecycle correctness.** Per-username async mutex serializing + acquire/ensure/release/reap; acquisitions return a handle whose release is + idempotent (no shared counter clamp); ref registered before any await + completes the acquire path (disconnect during acquire releases properly); + boxes run with `--init`; on startup the worker reconciles existing + `spoon-box-*` (adopt into the registry with zero refs → idle reap applies). +- **Not-found handling.** `(app)/error.tsx` + friendly not-found states for + spoon/thread detail (queries return `null` for missing/unowned; UI + distinguishes loading `undefined` from missing `null`). +- **Worker-token compare** unified through one trimming helper; + `getEnvironmentForJob` asserts `claimedBy`. + +### Phase 2 — Runtime unification (one box, three agents) + +- **`AgentRuntime` adapter interface** (worker): + `prepareAuth(workspace)`, `runTurn(workspace, prompt, onEvent) → TurnResult`, + `abort(workspace)`. All adapters exec into the user's box via + `streamExecInContainer`; process-group kill on timeout/abort (in-box + `pkill`-by-marker or setsid + kill), never just the local client. +- **Codex adapter:** current `runCodexTurn` refactored behind the interface. +- **OpenCode adapter:** runs OpenCode inside the box (non-interactive + `opencode run --format json`, or `opencode serve` exec'd in-box if session + continuity requires it). Delete: `startWorkspaceContainer` per-job path, + `spoon-agent-job-{jobId}` naming, `containerAccess` host_port/network split + and related env, dead per-job docker helpers. +- **Claude Code adapter:** `claude -p --output-format stream-json` in the box. + New provider-profile kind: Anthropic API key, or pasted OAuth credentials + (same encrypted-snapshot pattern as Codex auth.json). CLI added to the Fedora + job image, version-pinned. +- **Runtime picker:** thread creation + per-spoon agent settings select + codex/opencode/claude; provider profiles declare which runtimes they satisfy. + Queue-time validation rejects runtime/profile mismatches (no more run-time + discovery of legacy `openai_direct`). +- **Secrets hygiene:** materialized env files written `0600`, deleted on + workspace stop, re-materialized per run; never committed (existing staging + guard stays). + +### Phase 3 — Dev box as a first-class surface + +- **`/machine` page:** box status (running/stopped, image, uptime, mem cap), + start/stop/restart actions, full-page terminal rooted at `~`, and a + home-scoped file browser (reuse FileTree/CodeEditor against new worker + `/box/*` file routes). Worker gains user-scoped HMAC terminal tokens and + `/box/tree|file|terminal` routes with the same ownership proxy pattern + (Next mints tokens after Convex auth; browser still never sees worker + secrets). +- **Persistent workspace terminal:** terminal tab `forceMount` + CSS hide, so + the PTY/scrollback survive tab switches; `fit()` + resize-send when the tab + becomes visible; "waiting for workspace" state distinct from "connecting". +- **Terminal QoL:** web links addon, copy-on-select toggle. + +### Phase 4 — Upstream-sync correctness + +- Head SHA fetched from the branch ref (not inferred from the truncated compare + list); paginate compares where counts matter; stable dedup key for maintenance + threads (merge-base + head digest; never `Date.now()`); skip thread creation + when no head is resolvable. +- **GitHub webhooks** (Convex `httpAction`, signature-verified via + `GITHUB_APP_WEBHOOK_SECRET`): `push` on watched repos → immediate targeted + refresh; `installation`/`installation_repositories` lifecycle → + `gitConnections.status = needs_reauth/revoked`, surfaced in + Settings → Integrations. Hourly cron remains as fallback. +- Octokit retry + throttling plugins; sync status distinguishes + "rate-limited (will retry)" from hard errors. +- Fix unbounded `.collect()`s on hot paths (indexed, paginated). + +### Phase 5 — Notifications & polish + +- **Notifications:** `notifications` table + in-app inbox (header bell), and + email via the existing UseSend integration for: upstream moved / maintenance + thread created, agent turn finished / needs input, sync failed, + connection needs re-auth. Per-user preference toggles in settings. Web push + deferred. +- **Polish sweep:** loading skeletons for dashboard/spoons/threads lists; error + handling on all fire-and-forget mutations (toast on failure, success only + after resolve); settings forms remount keyed on loaded record id; sign-in + keeps input on failure + copy fixes ("phone" → email, Passsword typo, + Verify Email pending text); editor buffer refresh when the agent edits an + open non-dirty file (dirty files get a conflict flag); diff/tree request + sequencing guard; recovery panel requires N consecutive failures; auto-scroll + only when already near bottom; mobile workspace layout (collapsible file + tree, viewport-relative min-height); avatar/`role='link'` accessibility fixes; + delete dead routes/components/env vars; `redact.ts` group-less replacement + fix; `'use server'` removed from profile page. +- **Docs:** README + docs/ rewritten for the box-first model (they still + describe the per-job world); server-deploy notes updated (webhook URL, new + env, Claude Code). + +## Error-handling principles (cross-cutting) + +- The Convex job state machine is authoritative; transitions are validated + server-side and the worker conforms (never resurrects terminal jobs). +- UI never reports success before the awaited mutation resolves; background + refresh failures surface after consecutive-failure thresholds, not instantly + and not never. +- Worker spawn failures are always distinguishable from empty successes + (`normalizeRunResult` everywhere; no `exitCode ?? 0`). + +## Testing + +- **Convex (convex-test):** claim validation/rollback, heartbeat-timeout cron, + transition validation incl. cancel-vs-worker races, auto-sync gating matrix, + maintenance-thread dedup, webhook signature handling. +- **Worker (vitest):** box registry mutex/handle semantics (concurrent + acquire/release/reap), path containment incl. symlinks, runtime adapters' + stream parsing (codex/opencode/claude fixtures), cleanup layout rules. +- **Next (component tests):** terminal fit-on-fonts-ready + resize-on-visible, + workspace shell remount on job switch, form re-sync, not-found states. +- **Smoke:** extend `scripts/smoke-agent-container` to run one turn per runtime + inside the box; manual checklist per phase in the plan. + +## Out of scope + +- Hostile multi-tenant isolation (gVisor/rootless per-user daemons). +- Non-GitHub provider automation; pushing agent branches to extra remotes. +- Mobile workspace editing; app-store release flow. +- Long-running preview stacks for forked projects. +- Web push notifications (email + in-app only for now).