docs: stabilize/unify/build design spec from full audit
This commit is contained in:
Gabriel Brown
@@ -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.** `<AgentWorkspaceShell key={jobId} .../>` 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).
|
||||||
Reference in New Issue
Block a user