# Spoon Phase 1 — Stabilize: 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:** Eliminate the Critical/High bugs from the 2026-07-10 audit without changing the current architecture: stop the worker cleanup from deleting user homes, make the job state machine crash-safe and cancel-safe, honor upstream-sync settings, close the symlink escape, fix the terminal (client sizing + real PTY resize), key the workspace shell by job, make the per-user box registry concurrency-correct, and add friendly not-found handling. Each task is an independent, test-first commit. **Architecture:** A Convex backend (`packages/backend/convex`) owns the authoritative job/thread state machine and is polled by a single long-running Bun worker (`apps/agent-worker`) that execs into one persistent per-user Docker/Podman "box" (`spoon-box-{username}`) mounting a persistent home at `/home/{username}`; each thread is a checkout at `~/Code/{spoon}/{branch}`. A Next.js 16 app (`apps/next`) renders the workspace UI (xterm terminal, file tree, diff) and proxies worker access after Convex ownership checks. The Convex state machine is authoritative; the worker conforms and never resurrects terminal jobs. **Tech Stack:** TypeScript, Convex, Next.js 16 (React 19), agent-worker (Bun, dockerode, ws), vitest. ## Global Constraints - Tests: vitest projects. Worker unit tests: `apps/agent-worker/tests/unit/*.test.ts` run with `cd apps/agent-worker && bun run test:unit` (or `vitest run --project unit`). Backend tests: `packages/backend/tests/unit/*.test.ts` using `convex-test`, run with `cd packages/backend && bun run test:unit`. Next component tests: jsdom + @testing-library/react via `cd apps/next && bun run test:component`; Next unit tests (node env) via `cd apps/next && bun run test:unit`. - Commit style: conventional commits (feat:/fix:/refactor:/test:/docs:). Frequent commits, one per task. - Do NOT use `git commit -n` unless a hook blocks; precommit runs lint-staged. - Convex functions live in `packages/backend/convex`; codegen via `bun codegen:convex` from repo root before typecheck/test. New Convex functions are auto-discovered by the convex-test harness (`import.meta.glob('../../convex/**/*.*s')`), but you must run `bun codegen:convex` from the repo root so `_generated/api` and `_generated/dataModel` typecheck. - Worker unit tests that touch modules importing `./env` MUST set the required env vars **before** the dynamic import and use `vi.resetModules()` (see `apps/agent-worker/tests/unit/docker-runtime.test.ts` for the exact pattern). `env.ts` throws at import time if `SPOON_WORKER_TOKEN`, `GITHUB_APP_ID`, or `GITHUB_APP_PRIVATE_KEY` are missing. - Worker in-memory concurrency (box registry, cleanup) is tested by `vi.mock`-ing `../../src/runtime/docker` and `../../src/env`, then dynamically importing the module under test. Match the style of `tests/unit/docker-runtime.test.ts` and `tests/unit/terminal-token.test.ts`. --- ## Task 1: Layout-aware `cleanupOrphanedWorkspaces` (stop deleting user homes) **Why first:** This is the only data-loss bug. `POST /cleanup` (Settings → Worker) currently `rm -rf`s every top-level entry under `env.workdir` that is not an active workdir. In the Phase-2 layout every user's persistent home lives under `homes/{username}`, and an active workspace's `workdir` is `homes/{username}` (the home root) — but `activeWorkdirs` only contains homes of *currently running* jobs, so an idle user's entire home is deleted. It also only enumerates `spoon-agent-job-*` containers, ignoring the new `spoon-box-*` boxes. **Files:** - Modify `apps/agent-worker/src/worker.ts` (`cleanupOrphanedWorkspaces` ~1911-1954; `getWorkerHealth` ~1880-1909; add pure helpers near top). - Modify `apps/agent-worker/src/user-container.ts` (export `runningBoxUsernames()` used by cleanup). - Create `apps/agent-worker/tests/unit/cleanup-layout.test.ts`. **Interfaces:** - Produces (worker.ts): `export const planWorkdirCleanup = (args: { rootEntries: { name: string; isDirectory: boolean }[]; homesEntries: Record; codeLeaves: Record; activeWorkdirs: Set; runningBoxUsernames: Set; root: string }) => { removeDirs: string[] }` - Produces (user-container.ts): `export const runningBoxUsernames = (): Set` — usernames whose box currently has refs > 0 OR is registered (adopted). For Task 1 return the set of keys in the `boxes` map (a registered box means the user is/was active). Task 2 refines this. **Steps:** 1. - [ ] Write failing test `apps/agent-worker/tests/unit/cleanup-layout.test.ts`. Set env, `vi.resetModules()`, dynamic-import `../../src/worker` (it imports `./env`, so set `SPOON_WORKER_TOKEN`, `GITHUB_APP_ID`, `GITHUB_APP_PRIVATE_KEY` first, matching `docker-runtime.test.ts`). Assert on `planWorkdirCleanup`: ```ts import { afterEach, describe, expect, test, vi } from 'vitest'; const load = async () => { vi.resetModules(); process.env.SPOON_WORKER_TOKEN = 'test-worker-token'; process.env.GITHUB_APP_ID = '123'; process.env.GITHUB_APP_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\\ntest\\n-----END PRIVATE KEY-----'; return await import('../../src/worker'); }; describe('planWorkdirCleanup', () => { afterEach(() => vi.resetModules()); test('never removes the homes/ root or any home directory', async () => { const { planWorkdirCleanup } = await load(); const plan = planWorkdirCleanup({ root: '/work', rootEntries: [{ name: 'homes', isDirectory: true }], homesEntries: { alice: [{ name: 'Code', isDirectory: true }] }, codeLeaves: {}, activeWorkdirs: new Set(), runningBoxUsernames: new Set(), }); expect(plan.removeDirs).not.toContain('/work/homes'); expect(plan.removeDirs).not.toContain('/work/homes/alice'); }); test('removes legacy top-level job dirs not in the active set', async () => { const { planWorkdirCleanup } = await load(); const plan = planWorkdirCleanup({ root: '/work', rootEntries: [ { name: 'homes', isDirectory: true }, { name: 'legacy-job-123', isDirectory: true }, { name: 'dev', isDirectory: true }, ], homesEntries: {}, codeLeaves: {}, activeWorkdirs: new Set(['/work/dev']), runningBoxUsernames: new Set(), }); expect(plan.removeDirs).toContain('/work/legacy-job-123'); expect(plan.removeDirs).not.toContain('/work/dev'); expect(plan.removeDirs).not.toContain('/work/homes'); }); test('removes per-thread checkouts only for users with no running box', async () => { const { planWorkdirCleanup } = await load(); const plan = planWorkdirCleanup({ root: '/work', rootEntries: [{ name: 'homes', isDirectory: true }], homesEntries: { alice: [{ name: 'Code', isDirectory: true }], bob: [{ name: 'Code', isDirectory: true }], }, codeLeaves: { alice: ['/work/homes/alice/Code/spoon-a/branch-x'], bob: ['/work/homes/bob/Code/spoon-b/branch-y'], }, activeWorkdirs: new Set(), runningBoxUsernames: new Set(['bob']), }); expect(plan.removeDirs).toContain('/work/homes/alice/Code/spoon-a/branch-x'); expect(plan.removeDirs).not.toContain('/work/homes/bob/Code/spoon-b/branch-y'); // never the home root, never Code expect(plan.removeDirs).not.toContain('/work/homes/alice'); expect(plan.removeDirs).not.toContain('/work/homes/alice/Code'); }); }); ``` 2. - [ ] Run `cd apps/agent-worker && bun run test:unit -- cleanup-layout` — expect FAIL (`planWorkdirCleanup` is not exported). 3. - [ ] Implement the pure helper. Add near the top of `apps/agent-worker/src/worker.ts` (after the `slugify` helper, before `runClaim`): ```ts // Pure cleanup planner: decides which host directories under the worker workdir // are safe to remove. NEVER returns `homes/`, a home root, or a home's `Code/` // dir. Removes (a) legacy pre-Phase-2 top-level job dirs and (b) per-thread // checkouts (homes/{user}/Code/{spoon}/{branch}) for users with no running box. export const planWorkdirCleanup = (args: { root: string; rootEntries: { name: string; isDirectory: boolean }[]; homesEntries: Record; codeLeaves: Record; activeWorkdirs: Set; runningBoxUsernames: Set; }): { removeDirs: string[] } => { const removeDirs: string[] = []; for (const entry of args.rootEntries) { if (!entry.isDirectory || entry.name.startsWith('.')) continue; if (entry.name === 'homes') continue; // never touch the homes tree here const abs = path.resolve(args.root, entry.name); if (args.activeWorkdirs.has(abs)) continue; removeDirs.push(abs); // legacy top-level job dir } for (const [username, leaves] of Object.entries(args.codeLeaves)) { if (args.runningBoxUsernames.has(username)) continue; // box is live: keep for (const leaf of leaves) removeDirs.push(leaf); } return { removeDirs }; }; ``` 4. - [ ] Run `cd apps/agent-worker && bun run test:unit -- cleanup-layout` — expect PASS. 5. - [ ] Wire the planner into `cleanupOrphanedWorkspaces`. Replace the body (~1911-1954) so it: enumerates BOTH container prefixes, builds the tree inputs from the filesystem, calls `planWorkdirCleanup`, and removes only the planned dirs. Real code: ```ts export const cleanupOrphanedWorkspaces = async () => { const activeContainers = new Set( [...activeWorkspaces.values()] .map((workspace) => workspace.containerName) .filter((value): value is string => Boolean(value)), ); const activeWorkdirs = new Set( [...activeWorkspaces.values()].map((workspace) => path.resolve(workspace.workdir), ), ); const removedContainers: string[] = []; // Only reap legacy per-job containers; boxes are owned by the registry. for (const containerName of await listWorkspaceContainerNames( 'spoon-agent-job-', )) { if (activeContainers.has(containerName)) continue; await stopWorkspaceContainer(containerName); removedContainers.push(containerName); } const root = path.resolve(env.workdir); const removedWorkdirs: string[] = []; try { const rootDirents = await readdir(root, { withFileTypes: true }); const rootEntries = rootDirents.map((d) => ({ name: d.name, isDirectory: d.isDirectory(), })); const homesRoot = path.join(root, 'homes'); const homesEntries: Record< string, { name: string; isDirectory: boolean }[] > = {}; const codeLeaves: Record = {}; const homesDirents = await readdir(homesRoot, { withFileTypes: true, }).catch(() => []); for (const userDir of homesDirents) { if (!userDir.isDirectory()) continue; const username = userDir.name; const codeRoot = path.join(homesRoot, username, 'Code'); homesEntries[username] = []; const leaves: string[] = []; const spoons = await readdir(codeRoot, { withFileTypes: true }).catch( () => [], ); for (const spoonDir of spoons) { if (!spoonDir.isDirectory()) continue; const spoonRoot = path.join(codeRoot, spoonDir.name); const branches = await readdir(spoonRoot, { withFileTypes: true, }).catch(() => []); for (const branchDir of branches) { if (branchDir.isDirectory()) { leaves.push(path.join(spoonRoot, branchDir.name)); } } } codeLeaves[username] = leaves; } const { removeDirs } = planWorkdirCleanup({ root, rootEntries, homesEntries, codeLeaves, activeWorkdirs, runningBoxUsernames: runningBoxUsernames(), }); for (const target of removeDirs) { await rm(target, { recursive: true, force: true }); removedWorkdirs.push(target); } } catch (error) { const code = error && typeof error === 'object' ? 'code' in error : false; if (!code || (error as { code?: string }).code !== 'ENOENT') throw error; } const boxContainers = await listWorkspaceContainerNames('spoon-box-'); return { success: true, removedContainers, removedWorkdirs, boxContainers }; }; ``` 6. - [ ] Add `runningBoxUsernames` import to worker.ts: change the import from `./user-container` (line ~44) to `import { acquireUserBox, releaseUserBox, runningBoxUsernames } from './user-container';`. 7. - [ ] Export `runningBoxUsernames` from `apps/agent-worker/src/user-container.ts` (append): ```ts // Usernames whose box is currently registered (held or adopted). Cleanup uses // this to avoid deleting a live user's per-thread checkouts. export const runningBoxUsernames = (): Set => new Set(boxes.keys()); ``` 8. - [ ] Update `getWorkerHealth` (~1888) so `workspaceContainers` enumerates boxes too. Replace the single `listWorkspaceContainerNames('spoon-agent-job-')` call with: ```ts const jobContainers = await listWorkspaceContainerNames('spoon-agent-job-'); const boxContainers = await listWorkspaceContainerNames('spoon-box-'); ``` and in the returned object replace `workspaceContainers: containerNames,` with `workspaceContainers: jobContainers,` and add `boxContainers,`. 9. - [ ] Run `cd apps/agent-worker && bun run test:unit` and `bun run typecheck` — expect PASS. 10. - [ ] Commit: `git add -A && git commit -m "fix(worker): make workspace cleanup layout-aware so it never deletes user homes"` --- ## Task 2: Box lifecycle correctness (mutex, idempotent handle release, startup reconcile, `--init`) **Why:** `acquireUserBox`/`releaseUserBox` use a clamped shared counter (`Math.max(0, refs-1)`), so a double-release reaps a box that another thread/terminal still uses. Concurrent acquires race `docker run`. A terminal disconnect *during* acquire leaks a ref forever (`terminal.ts` sets `acquired=false` until the await resolves, so `cleanup` never releases). Worker restarts orphan all `spoon-box-*`. **Files:** - Rewrite `apps/agent-worker/src/user-container.ts`. - Modify `apps/agent-worker/src/runtime/docker.ts` (`ensureUserContainer` ~341-383: add `--init`). - Modify `apps/agent-worker/src/worker.ts` (store a `boxHandle` on `ActiveWorkspace`; release via handle in `runClaim` catch, `openWorkspacePullRequest`, `stopWorkspace`). - Modify `apps/agent-worker/src/terminal.ts` (use the handle; release on disconnect-during-acquire). - Modify `apps/agent-worker/src/index.ts` (call `reconcileExistingBoxes()` at startup). - Create `apps/agent-worker/tests/unit/user-container.test.ts`. **Interfaces:** - `export type BoxHandle = { boxName: string; release: () => void };` - `export const acquireUserBox = (args: { username: string; workdir: string; containerHome: string }) => Promise` (was `Promise`). - `export const runningBoxUsernames = () => Set` (kept; now = usernames with refs.size > 0 OR adopted). - `export const reconcileExistingBoxes = () => Promise` — adopt existing `spoon-box-*` with zero refs so the idle reaper applies. - `export const _resetBoxRegistryForTests = () => void` — clears the map + timers (test-only). **Steps:** 1. - [ ] Write failing test `apps/agent-worker/tests/unit/user-container.test.ts`. `vi.mock('../../src/runtime/docker')` and `vi.mock('../../src/env')`; use fake timers for the idle reaper. Example: ```ts import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; vi.mock('../../src/env', () => ({ env: { boxIdleMs: 1000 } })); vi.mock('../../src/runtime/docker', () => ({ ensureUserContainer: vi.fn(async (a: { username: string }) => `spoon-box-${a.username}`), stopWorkspaceContainer: vi.fn(async () => {}), userContainerName: (u: string) => `spoon-box-${u}`, listWorkspaceContainerNames: vi.fn(async () => []), })); const load = async () => await import('../../src/user-container'); describe('box registry', () => { beforeEach(() => vi.useFakeTimers()); afterEach(async () => { const m = await load(); m._resetBoxRegistryForTests(); vi.useRealTimers(); vi.resetModules(); vi.clearAllMocks(); }); test('serializes concurrent acquire into a single docker run', async () => { const m = await load(); const docker = await import('../../src/runtime/docker'); const [a, b] = await Promise.all([ m.acquireUserBox({ username: 'alice', workdir: '/w', containerHome: '/home/alice' }), m.acquireUserBox({ username: 'alice', workdir: '/w', containerHome: '/home/alice' }), ]); expect(docker.ensureUserContainer).toHaveBeenCalledTimes(1); expect(a.boxName).toBe('spoon-box-alice'); // two refs held -> release one -> not reaped a.release(); vi.advanceTimersByTime(2000); expect(docker.stopWorkspaceContainer).not.toHaveBeenCalled(); b.release(); vi.advanceTimersByTime(2000); expect(docker.stopWorkspaceContainer).toHaveBeenCalledTimes(1); }); test('double-release of one handle does not reap a box held elsewhere', async () => { const m = await load(); const docker = await import('../../src/runtime/docker'); const a = await m.acquireUserBox({ username: 'bob', workdir: '/w', containerHome: '/home/bob' }); const b = await m.acquireUserBox({ username: 'bob', workdir: '/w', containerHome: '/home/bob' }); a.release(); a.release(); // idempotent: must NOT drop b's ref vi.advanceTimersByTime(2000); expect(docker.stopWorkspaceContainer).not.toHaveBeenCalled(); b.release(); vi.advanceTimersByTime(2000); expect(docker.stopWorkspaceContainer).toHaveBeenCalledTimes(1); }); test('reconcileExistingBoxes adopts orphans with zero refs and reaps them', async () => { const m = await load(); const docker = await import('../../src/runtime/docker'); (docker.listWorkspaceContainerNames as unknown as ReturnType).mockResolvedValueOnce(['spoon-box-carol']); await m.reconcileExistingBoxes(); expect(m.runningBoxUsernames().has('carol')).toBe(true); vi.advanceTimersByTime(2000); expect(docker.stopWorkspaceContainer).toHaveBeenCalledWith('spoon-box-carol'); }); }); ``` 2. - [ ] Run `cd apps/agent-worker && bun run test:unit -- user-container` — expect FAIL. 3. - [ ] Rewrite `apps/agent-worker/src/user-container.ts` in full: ```ts import { env } from './env'; import { ensureUserContainer, listWorkspaceContainerNames, stopWorkspaceContainer, userContainerName, } from './runtime/docker'; // Phase 2: one persistent "box" container per user. Reference-counted by opaque // handles (not a shared integer), serialized per-username with an async mutex, // and idle-reaped once no handle is held. export type BoxHandle = { boxName: string; release: () => void }; type Box = { name: string; refs: Set; idleTimer?: ReturnType }; const boxes = new Map(); const locks = new Map>(); // Per-username async mutex: chain each operation after the previous one. const withLock = (username: string, fn: () => Promise): Promise => { const prev = locks.get(username) ?? Promise.resolve(); const next = prev.then(fn, fn); locks.set( username, next.then( () => undefined, () => undefined, ), ); return next; }; const scheduleReapIfIdle = (username: string) => { const box = boxes.get(username); if (!box || box.refs.size > 0) return; if (box.idleTimer) clearTimeout(box.idleTimer); box.idleTimer = setTimeout(() => { void stopWorkspaceContainer(userContainerName(username)); boxes.delete(username); }, env.boxIdleMs); }; const makeHandle = (username: string, box: Box): BoxHandle => { const token = Symbol('box-ref'); box.refs.add(token); let released = false; return { boxName: box.name, release: () => { if (released) return; // idempotent released = true; box.refs.delete(token); scheduleReapIfIdle(username); }, }; }; export const acquireUserBox = (args: { username: string; workdir: string; containerHome: string; }): Promise => withLock(args.username, async () => { let box = boxes.get(args.username); if (box?.idleTimer) { clearTimeout(box.idleTimer); box.idleTimer = undefined; } if (!box) { box = { name: userContainerName(args.username), refs: new Set() }; boxes.set(args.username, box); } // Register the ref BEFORE the (slow) docker call so a disconnect during // acquire has a handle to release and never leaks. const handle = makeHandle(args.username, box); try { box.name = await ensureUserContainer(args); return handle; } catch (error) { handle.release(); throw error; } }); // Adopt any pre-existing spoon-box-* containers (from a prior worker process) // into the registry with zero refs so the idle reaper cleans them up. export const reconcileExistingBoxes = async (): Promise => { const names = await listWorkspaceContainerNames('spoon-box-'); for (const name of names) { const username = name.replace(/^spoon-box-/, ''); if (boxes.has(username)) continue; const box: Box = { name, refs: new Set() }; boxes.set(username, box); scheduleReapIfIdle(username); } }; export const runningBoxUsernames = (): Set => new Set(boxes.keys()); export const _resetBoxRegistryForTests = () => { for (const box of boxes.values()) { if (box.idleTimer) clearTimeout(box.idleTimer); } boxes.clear(); locks.clear(); }; ``` 4. - [ ] Run `cd apps/agent-worker && bun run test:unit -- user-container` — expect PASS. 5. - [ ] Add `--init` to the box in `apps/agent-worker/src/runtime/docker.ts` `ensureUserContainer` (~360). In the `run` args array, insert `'--init',` immediately after `'-d',`: ```ts [ 'run', '-d', '--init', '--name', name, ``` This is pure-infra (needs a real container to observe zombie reaping). Manual verification: `docker inspect --format '{{.HostConfig.Init}}' spoon-box-` prints `true` after a job runs; documented, no unit test. 6. - [ ] Update `apps/agent-worker/src/worker.ts` to store and release the handle instead of a username string. - Add to the `ActiveWorkspace` type (after `boxName: string;`, ~line 110): `boxHandle: BoxHandle;` - Import the type: change the `./user-container` import to `import { acquireUserBox, runningBoxUsernames } from './user-container';` and `import type { BoxHandle } from './user-container';` (drop `releaseUserBox`). - In `runClaim` (~1330): change `let acquiredBoxUser: string | undefined;` to `let acquiredBoxHandle: BoxHandle | undefined;`. Replace `const boxName = await acquireUserBox({...}); acquiredBoxUser = username;` with: ```ts const boxHandle = await acquireUserBox({ username, workdir: homeDir, containerHome, }); acquiredBoxHandle = boxHandle; const boxName = boxHandle.boxName; ``` - In the workspace object literal (~1348) add `boxHandle,`. - In the `catch` (~1436) replace `if (acquiredBoxUser) releaseUserBox(acquiredBoxUser);` with `acquiredBoxHandle?.release();`. - In `openWorkspacePullRequest` (~1859) replace `releaseUserBox(workspace.username);` with `workspace.boxHandle.release();`. - In `stopWorkspace` (~1876) replace `releaseUserBox(workspace.username);` with `workspace.boxHandle.release();`. 7. - [ ] Update `apps/agent-worker/src/terminal.ts` to use the handle and release on disconnect-during-acquire. - Change import (line 9) to `import { acquireUserBox } from './user-container';` and `import type { BoxHandle } from './user-container';`. - Replace the acquire/cleanup block (~66-98). Real code: ```ts const handleHolder: { current?: BoxHandle } = {}; let released = false; const isReleased = () => released; const cleanup = () => { if (released) return; released = true; procHolder.current?.kill(); handleHolder.current?.release(); }; ws.on('close', cleanup); ws.on('error', cleanup); let boxName: string; try { const handle = await acquireUserBox({ username: workspace.username, workdir: workspace.workdir, containerHome: workspace.containerHome, }); handleHolder.current = handle; boxName = handle.boxName; } catch (error) { ws.close( 1011, `Failed to start terminal: ${error instanceof Error ? error.message : 'unknown error'}`, ); return; } // If the socket already closed during acquire, release the box now: cleanup // ran before the handle existed, so nothing has released it. if (isReleased()) { handleHolder.current.release(); return; } ``` (Delete the old `let acquired`/`acquired = true` logic.) 8. - [ ] Call reconcile at startup. In `apps/agent-worker/src/index.ts`, change the import line `import { startWorker } from './worker';` region to also import reconcile and call it before `startWorker`: ```ts import { reconcileExistingBoxes } from './user-container'; ``` and just before `await startWorker();` at the bottom add: ```ts await reconcileExistingBoxes(); ``` 9. - [ ] Run `cd apps/agent-worker && bun run test:unit && bun run typecheck` — expect PASS. (If `releaseUserBox` is referenced anywhere else, `grep -rn releaseUserBox apps/agent-worker/src` returns nothing.) 10. - [ ] Commit: `git add -A && git commit -m "fix(worker): make per-user box registry concurrency-safe with idempotent handles and startup reconcile"` --- ## Task 3: Symlink containment for all worker file access **Why (High):** `safeWorkspacePath` (`worker.ts:1059`) and `safeHomeJoin` (`user-environment.ts:35`) validate the path *lexically* then `readFile`/`writeFile` follow symlinks. A repo containing a symlink (e.g. `link -> /home/otheruser` or `-> /proc/self/environ`) lets the `file` GET/PUT and dotfile-overlay routes read/write outside the checkout — other users' homes and worker secrets. **Files:** - Create `apps/agent-worker/src/path-containment.ts` (shared helper). - Modify `apps/agent-worker/src/worker.ts` (`safeWorkspacePath` → async containment for reads/writes at `readWorkspaceFile`, `writeWorkspaceFile`, `materializeEnvFile`). - Modify `apps/agent-worker/src/user-environment.ts` (`safeHomeJoin` write targets). - Create `apps/agent-worker/tests/unit/path-containment.test.ts`. **Interfaces:** - `export const assertContainedRealPath = async (root: string, requested: string, opts?: { forWrite?: boolean }) => Promise` — resolves the lexical target, then `fs.realpath`s the deepest existing ancestor; throws if the real path escapes `realpath(root)`. For writes, if the final target exists and is a symlink, throw. **Steps:** 1. - [ ] Write failing test `apps/agent-worker/tests/unit/path-containment.test.ts` using real fs in an OS temp dir (no env import needed, so no module mocking): ```ts import { mkdtemp, mkdir, symlink, writeFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, test } from 'vitest'; import { assertContainedRealPath } from '../../src/path-containment'; let dir: string; afterEach(async () => { if (dir) await rm(dir, { recursive: true, force: true }); }); describe('assertContainedRealPath', () => { test('allows a normal file inside the root', async () => { dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); const root = path.join(dir, 'repo'); await mkdir(root, { recursive: true }); await writeFile(path.join(root, 'a.txt'), 'hi'); await expect(assertContainedRealPath(root, 'a.txt')).resolves.toBe( path.join(root, 'a.txt'), ); }); test('rejects a lexical .. escape', async () => { dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); const root = path.join(dir, 'repo'); await mkdir(root, { recursive: true }); await expect(assertContainedRealPath(root, '../secret')).rejects.toThrow(); }); test('rejects reading through a symlink that escapes the root', async () => { dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); const root = path.join(dir, 'repo'); const outside = path.join(dir, 'outside'); await mkdir(root, { recursive: true }); await mkdir(outside, { recursive: true }); await writeFile(path.join(outside, 'secret'), 'S'); await symlink(outside, path.join(root, 'link')); await expect(assertContainedRealPath(root, 'link/secret')).rejects.toThrow(); }); test('rejects a write whose final target is a symlink', async () => { dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); const root = path.join(dir, 'repo'); await mkdir(root, { recursive: true }); await symlink(path.join(dir, 'evil'), path.join(root, 'out')); await expect( assertContainedRealPath(root, 'out', { forWrite: true }), ).rejects.toThrow(); }); }); ``` 2. - [ ] Run `cd apps/agent-worker && bun run test:unit -- path-containment` — expect FAIL (module missing). 3. - [ ] Create `apps/agent-worker/src/path-containment.ts`: ```ts import { lstat, realpath } from 'node:fs/promises'; import path from 'node:path'; const realpathOrDeepestExisting = async (target: string): Promise => { let current = target; // Walk up to the deepest ancestor that exists, realpath it, then re-append // the not-yet-created tail. This lets writes create new files while still // resolving any symlinked directory in the existing prefix. const tail: string[] = []; for (;;) { try { const real = await realpath(current); return path.join(real, ...tail.reverse()); } catch (error) { if ((error as { code?: string }).code !== 'ENOENT') throw error; const parent = path.dirname(current); if (parent === current) throw error; tail.push(path.basename(current)); current = parent; } } }; // Resolve `requested` under `root`, following symlinks, and assert the real // path stays inside the real root. For writes, also reject when the final // target itself is a symlink. export const assertContainedRealPath = async ( root: string, requested: string, opts: { forWrite?: boolean } = {}, ): Promise => { const lexical = path.resolve(root, requested); const realRoot = await realpath(root); const resolved = await realpathOrDeepestExisting(lexical); if (resolved !== realRoot && !resolved.startsWith(`${realRoot}${path.sep}`)) { throw new Error(`Refusing to access path outside root: ${requested}`); } if (opts.forWrite) { const info = await lstat(lexical).catch(() => null); if (info?.isSymbolicLink()) { throw new Error(`Refusing to write through a symlink: ${requested}`); } } return resolved; }; ``` 4. - [ ] Run `cd apps/agent-worker && bun run test:unit -- path-containment` — expect PASS. 5. - [ ] Use it in `apps/agent-worker/src/worker.ts`. Add `import { assertContainedRealPath } from './path-containment';` near the top imports. - `readWorkspaceFile` (~1477): replace `const target = safeWorkspacePath(workspace.repoDir, filePath);` with `const target = await assertContainedRealPath(workspace.repoDir, filePath);`. - `writeWorkspaceFile` (~1483): replace `const target = safeWorkspacePath(workspace.repoDir, filePath);` with `const target = await assertContainedRealPath(workspace.repoDir, filePath, { forWrite: true });`. - `materializeEnvFile` (~1170): replace `const envPath = safeWorkspacePath(repoDir, claim.job.envFilePath);` with `const envPath = await assertContainedRealPath(repoDir, claim.job.envFilePath, { forWrite: true });`. - Leave the synchronous `safeWorkspacePath` in place only if still referenced; `grep -rn safeWorkspacePath apps/agent-worker/src` — if no remaining references, delete the function (~1059-1066). 6. - [ ] Use it in `apps/agent-worker/src/user-environment.ts` overlay writes. Add `import { assertContainedRealPath } from './path-containment';`. In the overlay loop (~113): replace `const target = safeHomeJoin(homeDir, file.path);` with `const target = await assertContainedRealPath(homeDir, file.path, { forWrite: true });`. Remove the now-unused `safeHomeJoin` (~35-42) if `grep -rn safeHomeJoin apps/agent-worker/src` shows no other users. 7. - [ ] Run `cd apps/agent-worker && bun run test:unit && bun run typecheck` — expect PASS. 8. - [ ] Commit: `git add -A && git commit -m "fix(worker): resolve realpath and reject symlinked targets on all workspace file access"` --- ## Task 4: `updateStatus` rejects writes to terminal jobs (fixes cancel-revert) **Why (Critical):** `cancel` sets `status: 'cancelled'`, but the worker's next `updateStatus` (e.g. `running`) flips the job back out of `cancelled` because `updateStatus` accepts any transition as long as `claimedBy` matches. The container keeps running. Fix: `updateStatus` no-ops when the job is already in a terminal status. **Files:** - Modify `packages/backend/convex/agentJobs.ts` (add `isTerminalStatus`; guard in `updateStatus` ~1108). - Create `packages/backend/tests/unit/update-status.test.ts`. **Interfaces:** - Add `const TERMINAL_STATUSES = new Set(['failed', 'cancelled', 'timed_out', 'draft_pr_opened']);` and `const isTerminalStatus = (s: string) => TERMINAL_STATUSES.has(s);` in `agentJobs.ts`. - `updateStatus` returns `{ success: true }` or `{ success: true, ignored: true }`. **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/update-status.test.ts` modeled on `harness.test.ts` (convexTest, `import.meta.glob`, `authed`/`createUser`). Seed a job with `status: 'cancelled'` and `claimedBy: 'worker-1'`, call `api.agentJobs.updateStatus` with `status: 'running'`, assert the job stays `cancelled` and the result is `{ success: true, ignored: true }`. Then a second test: a `running` job accepts `checks_running`. Set `process.env.SPOON_WORKER_TOKEN = 'test-worker-token'` in the test and pass `workerToken: 'test-worker-token'`. Example core: ```ts import { convexTest } from 'convex-test'; import { describe, expect, test, beforeAll } from 'vitest'; import { api } from '../../convex/_generated/api.js'; import schema from '../../convex/schema'; const modules = import.meta.glob('../../convex/**/*.*s'); beforeAll(() => { process.env.SPOON_WORKER_TOKEN = 'test-worker-token'; }); const seedJob = async (t, status) => await t.mutation(async (ctx) => { const now = Date.now(); const ownerId = await ctx.db.insert('users', { email: 'a@b.c', name: 'A' }); const spoonId = await ctx.db.insert('spoons', { /* minimal required spoon fields — copy from harness.test.ts spoonInput + ownerId + status:'active' + createdAt/updatedAt */ }); const requestId = await ctx.db.insert('agentRequests', { spoonId, ownerId, prompt: 'x', status: 'running', createdAt: now, updatedAt: now }); return await ctx.db.insert('agentJobs', { spoonId, ownerId, agentRequestId: requestId, status, prompt: 'x', runtime: 'opencode', baseBranch: 'main', workBranch: 'spoon/x', forkOwner: 'team', forkRepo: 'r', forkUrl: 'u', upstreamOwner: 'up', upstreamRepo: 'r', selectedSecretIds: [], model: '', reasoningEffort: 'medium', claimedBy: 'worker-1', createdAt: now, updatedAt: now, }); }); describe('updateStatus terminal guard', () => { test('ignores writes to a cancelled job', async () => { const t = convexTest(schema, modules); const jobId = await seedJob(t, 'cancelled'); const res = await t.mutation(api.agentJobs.updateStatus, { workerToken: 'test-worker-token', workerId: 'worker-1', jobId, status: 'running', }); expect(res).toEqual({ success: true, ignored: true }); const job = await t.run(async (ctx) => ctx.db.get(jobId)); expect(job?.status).toBe('cancelled'); }); }); ``` (Use the exact `spoonInput` shape from `packages/backend/tests/unit/harness.test.ts` for the spoon insert.) 2. - [ ] Run `bun codegen:convex` (repo root) then `cd packages/backend && bun run test:unit -- update-status` — expect FAIL (result lacks `ignored`, status flips to running). 3. - [ ] Implement. In `packages/backend/convex/agentJobs.ts`, add near `isTerminalJob` (~243): ```ts const TERMINAL_STATUSES = new Set([ 'failed', 'cancelled', 'timed_out', 'draft_pr_opened', ]); const isTerminalStatus = (status: string) => TERMINAL_STATUSES.has(status); ``` In `updateStatus.handler` (~1117) after the `claimedBy` check add: ```ts if (isTerminalStatus(job.status)) { // The Convex state machine is authoritative: never resurrect a terminal // job (fixes the cancel-then-worker-write revert). return { success: true, ignored: true as const }; } ``` 4. - [ ] Run `cd packages/backend && bun run test:unit -- update-status` — expect PASS. 5. - [ ] Commit: `git add -A && git commit -m "fix(convex): reject status writes to terminal agent jobs so cancel sticks"` --- ## Task 5: Validate everything inside `claimNextInternal` before patching to `claimed` **Why (Critical):** `claimNextInternal` patches the job to `claimed` unconditionally, then the *action* `claimNextForWorker` validates spoon/profile/secrets and throws — stranding the job in `claimed` forever (no worker owns it, no recovery). Move validation into the mutation so a bad job goes `failed` with a reason and the poller receives `null`. **Files:** - Modify `packages/backend/convex/agentJobs.ts` (`claimNextInternal` ~1046). - Modify `packages/backend/convex/agentJobsNode.ts` (`claimNextForWorker` ~50 — drop the now-redundant throws; keep decryption). - Create `packages/backend/tests/unit/claim-validation.test.ts`. **Interfaces:** - `claimNextInternal` still returns the same `ClaimedJob | null` shape on success; returns `null` after marking a job `failed` when validation fails (so the poller simply gets no work and does not throw). **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/claim-validation.test.ts`. Seed a queued job whose `aiProviderProfileId` is undefined and the owner has no configured profile. Call `internal.agentJobs.claimNextInternal` via `t.mutation(internal.agentJobs.claimNextInternal, { workerId: 'w1' })`. Assert it returns `null` and the job is now `status: 'failed'` with a non-empty `error`. Second test: a fully valid queued job returns a non-null claim and status `claimed`. ```ts import { internal } from '../../convex/_generated/api'; // ... seed a queued job with aiProviderProfileId undefined ... const res = await t.mutation(internal.agentJobs.claimNextInternal, { workerId: 'w1' }); expect(res).toBeNull(); const job = await t.run((ctx) => ctx.db.get(jobId)); expect(job?.status).toBe('failed'); expect(job?.error).toBeTruthy(); ``` 2. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- claim-validation` — expect FAIL. 3. - [ ] Implement in `claimNextInternal` (~1046). After loading `spoon`, `aiProviderProfile`, and `secrets` but BEFORE the `ctx.db.patch(job._id, { status: 'claimed', ... })`, add a validation block that, on failure, patches the job to `failed` and returns `null`: ```ts const ownedProfile = aiProviderProfile?.ownerId === job.ownerId ? aiProviderProfile : null; const failClaim = async (reason: string) => { const failedAt = Date.now(); await ctx.db.patch(job._id, { status: 'failed', error: reason, completedAt: failedAt, updatedAt: failedAt, }); await ctx.db.patch(job.agentRequestId, { status: 'failed', updatedAt: failedAt, }); if (job.threadId) { await ctx.db.patch(job.threadId, { status: 'failed', updatedAt: failedAt, resolvedAt: failedAt, }); } await ctx.db.insert('agentJobEvents', { jobId: job._id, spoonId: job.spoonId, ownerId: job.ownerId, level: 'error', phase: 'queued', message: `Job could not be claimed: ${reason}`, createdAt: failedAt, }); }; if (!spoon) { await failClaim('The Spoon for this job no longer exists.'); return null; } if (!ownedProfile) { await failClaim( 'AI is not configured for this user. Add an AI provider in settings.', ); return null; } if (ownedProfile.authType !== 'none' && !ownedProfile.encryptedSecret) { await failClaim('Selected AI provider is missing credentials.'); return null; } ``` Then the existing `patch(job._id, { status: 'claimed', ... })` and `return { ..., aiProviderProfile: ownedProfile, ... }` proceed (change the returned `aiProviderProfile` to `ownedProfile`). 4. - [ ] Simplify `claimNextForWorker` in `agentJobsNode.ts`: the mutation now guarantees a valid `spoon` and `aiProviderProfile`, but keep defensive non-throwing handling — replace the three `throw new ConvexError(...)` guards (~64-77) with an early `if (!claimed.spoon || !claimed.aiProviderProfile) return null;` (the job is already `failed`, so returning null just moves on). Keep the decryption/return mapping. 5. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- claim-validation` — expect PASS. Also re-run `bun run test:unit` (full) to ensure no regression. 6. - [ ] Commit: `git add -A && git commit -m "fix(convex): validate spoon/profile/secrets inside claim so bad jobs fail instead of wedging"` --- ## Task 6: Worker heartbeats + recovery cron + cancel propagation **Why (Critical):** `lastHeartbeatAt` is never written by the worker and never read by any recovery path, so a worker crash strands non-terminal jobs forever. And the worker is never told about a user `cancel`. Fix: (a) worker heartbeats every ~30s per active workspace; (b) `heartbeatWorkspace` returns `cancelRequested` so the worker tears down; (c) a recovery cron times out stale non-terminal jobs. **Files:** - Modify `packages/backend/convex/agentJobs.ts` (`heartbeatWorkspace` ~1358 returns `cancelRequested`; add `recoverStaleJobs` internalMutation). - Modify `packages/backend/convex/crons.ts` (register the recovery cron). - Modify `apps/agent-worker/src/worker.ts` (heartbeat loop per active workspace; teardown on cancel). - Create `packages/backend/tests/unit/recover-stale-jobs.test.ts`. **Interfaces:** - `heartbeatWorkspace` returns `{ success: true, cancelRequested: boolean }` where `cancelRequested = job.status === 'cancelled'`. - `export const recoverStaleJobs = internalMutation({ args: { staleMs: v.optional(v.number()) }, ... })` — for jobs with `status ∈ {claimed, preparing, running, checks_running}` whose `lastHeartbeatAt` (or `claimedAt` if no heartbeat) is older than `staleMs` (default 180000), set `status: 'timed_out'`, thread `status: 'failed'`. - Worker: `startHeartbeat(jobId)` / `stopHeartbeat(jobId)` managing `setInterval` handles in a `Map`. **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/recover-stale-jobs.test.ts`: seed a `running` job with `lastHeartbeatAt = Date.now() - 10*60*1000` and `claimedBy: 'w1'` (+ its thread). Call `t.mutation(internal.agentJobs.recoverStaleJobs, {})`. Assert job `status === 'timed_out'` and thread `status === 'failed'`. Second test: a `running` job with a fresh `lastHeartbeatAt = Date.now()` is left untouched. 2. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- recover-stale-jobs` — expect FAIL. 3. - [ ] Implement `recoverStaleJobs` in `agentJobs.ts` (add near the other internal mutations; ensure `internalMutation` is imported — it already is): ```ts export const recoverStaleJobs = internalMutation({ args: { staleMs: v.optional(v.number()) }, handler: async (ctx, { staleMs }) => { const threshold = Date.now() - (staleMs ?? 3 * 60 * 1000); const activeStatuses = [ 'claimed', 'preparing', 'running', 'checks_running', ] as const; let recovered = 0; for (const status of activeStatuses) { const jobs = await ctx.db .query('agentJobs') .withIndex('by_status', (q) => q.eq('status', status)) .collect(); for (const job of jobs) { const last = job.lastHeartbeatAt ?? job.claimedAt ?? job.createdAt; if (last > threshold) continue; const now = Date.now(); await ctx.db.patch(job._id, { status: 'timed_out', error: job.error ?? 'The worker stopped reporting progress; the job timed out.', completedAt: now, updatedAt: now, }); await ctx.db.patch(job.agentRequestId, { status: 'failed', updatedAt: now, }); if (job.threadId) { await ctx.db.patch(job.threadId, { status: 'failed', updatedAt: now, resolvedAt: now, }); } recovered += 1; } } return { recovered }; }, }); ``` 4. - [ ] Update `heartbeatWorkspace` (~1358) to return `cancelRequested`. After the `patch`, replace `return { success: true };` with: ```ts return { success: true, cancelRequested: job.status === 'cancelled' }; ``` (Note: when `job.status === 'cancelled'`, the heartbeat still patches `workspaceStatus`/`lastHeartbeatAt` but never changes `status`, so the cancel stands.) 5. - [ ] Register the cron in `packages/backend/convex/crons.ts` (after the existing refresh cron): ```ts crons.interval( 'Recover stale agent jobs', { minutes: 1 }, internal.agentJobs.recoverStaleJobs, {}, ); ``` 6. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- recover-stale-jobs` — expect PASS. 7. - [ ] Add the worker heartbeat loop + cancel teardown in `apps/agent-worker/src/worker.ts`. - Add a wrapper mutation helper near `markWorkspaceStopped` (~244): ```ts const heartbeatWorkspaceMutation = async (jobId: Id<'agentJobs'>) => await client.mutation(api.agentJobs.heartbeatWorkspace, { workerToken: env.workerToken, workerId: env.workerId, jobId, }); const heartbeatTimers = new Map>(); const stopHeartbeat = (jobId: string) => { const timer = heartbeatTimers.get(jobId); if (timer) clearInterval(timer); heartbeatTimers.delete(jobId); }; const startHeartbeat = (jobId: Id<'agentJobs'>) => { stopHeartbeat(jobId); const timer = setInterval(() => { void (async () => { try { const result = await heartbeatWorkspaceMutation(jobId); if (result?.cancelRequested && activeWorkspaces.has(jobId)) { await appendEvent(jobId, 'warn', 'cleanup', 'Cancellation requested; stopping workspace.'); await abortWorkspaceAgent(jobId).catch(() => {}); await stopWorkspace(jobId).catch(() => {}); stopHeartbeat(jobId); } } catch (error) { console.error(error); } })(); }, 30_000); timer.unref?.(); heartbeatTimers.set(jobId, timer); }; ``` - In `runClaim`, right after `await markWorkspaceActive({ jobId });` (~1388) add `startHeartbeat(jobId);`. - In `stopWorkspace` (~1866) and `openWorkspacePullRequest` (~1851), add `stopHeartbeat(jobId);` right before `activeWorkspaces.delete(jobId);`. - In `runClaim`'s `catch` (~1436), add `stopHeartbeat(jobId);` alongside `acquiredBoxHandle?.release();`. 8. - [ ] Run `cd apps/agent-worker && bun run typecheck` — expect PASS (no new worker unit test; the heartbeat interval is integration-level. The backend behavior is covered by Task 6 step 1-3 and Task 4's terminal guard). Document the manual check: queue a job, `POST /cleanup` off; call `api.agentJobs.cancel`; within ~30s the worker logs "Cancellation requested" and the box is released. 9. - [ ] Commit: `git add -A && git commit -m "feat(worker,convex): heartbeat active workspaces, recover stale jobs via cron, and tear down on cancel"` --- ## Task 7: `applyMaintenanceDecision` closes the job and stops the workspace **Why (Critical):** After a parsed maintenance review, the thread is resolved but the job stays `running` and its box keeps running. Fix: `applyMaintenanceDecision` marks the job terminal (`workspaceStatus: 'stopped'` + `completedAt`), and the worker tears the workspace down after applying the decision. **Files:** - Modify `packages/backend/convex/agentJobs.ts` (`applyMaintenanceDecision` ~1423: patch the job too). - Modify `apps/agent-worker/src/worker.ts` (`sendWorkspaceMessage` maintenance branch ~1742: stop the workspace after applying). - Create `packages/backend/tests/unit/maintenance-decision.test.ts`. **Interfaces:** unchanged signatures; `applyMaintenanceDecision` now also patches the job. Note: `isTerminalJob` already treats `workspaceStatus ∈ {stopped, expired, failed}` as terminal, so setting `workspaceStatus: 'stopped'` closes the job for `createForThread`'s active-job guard and marks it recovered-safe. **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/maintenance-decision.test.ts`: seed a `running` maintenance job with a `threadId`, `claimedBy: 'w1'`. Call `api.agentJobs.applyMaintenanceDecision` with a `sync`/low-risk decision (`requiresUserApproval: false`). Assert the thread `status === 'resolved'` AND the job `workspaceStatus === 'stopped'` with a `completedAt` set. 2. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- maintenance-decision` — expect FAIL. 3. - [ ] Implement in `applyMaintenanceDecision` (~1436). After the guard `if (!job.threadId) return { success: true };` and before/after the thread patch, add a job patch. Insert right after `const now = Date.now();` (~1443): ```ts await ctx.db.patch(args.jobId, { workspaceStatus: 'stopped', summary: args.summary, completedAt: job.completedAt ?? now, updatedAt: now, }); ``` (Keep the rest of the handler as-is.) 4. - [ ] In the worker, after applying the decision, stop the workspace. In `apps/agent-worker/src/worker.ts` `sendWorkspaceMessage`, in the `maintenance_review` branch (~1742), after `await applyMaintenanceDecision(claim.job._id, decision);` add: ```ts await stopWorkspace(claim.job._id).catch((error: unknown) => { console.error(error); }); return; ``` (The early `return` skips the trailing diff/artifact recording, which is unnecessary once the workspace is torn down. `stopWorkspace` is defined later in the module — it is a hoisted `const` exported function, so reference is fine at call time.) 5. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- maintenance-decision` and `cd apps/agent-worker && bun run typecheck` — expect PASS. 6. - [ ] Commit: `git add -A && git commit -m "fix(convex,worker): close the job and stop the box after a maintenance decision"` --- ## Task 8: Auto-sync honors settings + migration **Why (High):** `refreshOwnedSpoon` fast-forwards the fork whenever `status === 'behind' && forkAheadBy === 0`, and creates maintenance-review threads on `diverged`, ignoring `autoSyncEnabled` (default false), `requireCleanCompareForSync`, and `autoReviewEnabled`. Gate both on `spoonSettings`. Migration flips existing/new GitHub spoons to `autoSyncEnabled=true` to preserve today's observed behavior. **Files:** - Create `packages/backend/convex/syncGating.ts` (pure gating helpers, no `'use node'`). - Modify `packages/backend/convex/githubSync.ts` (`refreshOwnedSpoon` ~48: load settings, gate auto-sync and thread creation). - Modify `packages/backend/convex/github.ts` (`createForkSpoonRecord` ~193: default `autoSyncEnabled: true`). - Create `packages/backend/convex/migrations.ts` (internalMutation to backfill existing spoonSettings). - Create `packages/backend/tests/unit/sync-gating.test.ts`. **Interfaces:** - `export const shouldAutoSync = (settings: { autoSyncEnabled: boolean; requireCleanCompareForSync: boolean }, ctx: { status: string; forkAheadBy: number }): boolean` — true only when `autoSyncEnabled` and `status === 'behind'` and `forkAheadBy === 0` (a clean compare). - `export const shouldCreateReviewThread = (settings: { autoReviewEnabled: boolean }, ctx: { status: string }): boolean` — true only when `autoReviewEnabled` and `status === 'diverged'`. - `export const backfillAutoSyncDefaults = internalMutation(...)` — for every `spoonSettings` row whose spoon is a GitHub spoon, set `autoSyncEnabled: true` if not already. **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/sync-gating.test.ts` (pure unit test, node env — put it under `tests/unit/`): ```ts import { describe, expect, test } from 'vitest'; import { shouldAutoSync, shouldCreateReviewThread } from '../../convex/syncGating'; describe('syncGating', () => { test('auto-sync requires the flag and a clean behind compare', () => { expect(shouldAutoSync({ autoSyncEnabled: false, requireCleanCompareForSync: true }, { status: 'behind', forkAheadBy: 0 })).toBe(false); expect(shouldAutoSync({ autoSyncEnabled: true, requireCleanCompareForSync: true }, { status: 'behind', forkAheadBy: 0 })).toBe(true); expect(shouldAutoSync({ autoSyncEnabled: true, requireCleanCompareForSync: true }, { status: 'behind', forkAheadBy: 2 })).toBe(false); expect(shouldAutoSync({ autoSyncEnabled: true, requireCleanCompareForSync: true }, { status: 'diverged', forkAheadBy: 0 })).toBe(false); }); test('review thread requires autoReviewEnabled and diverged', () => { expect(shouldCreateReviewThread({ autoReviewEnabled: false }, { status: 'diverged' })).toBe(false); expect(shouldCreateReviewThread({ autoReviewEnabled: true }, { status: 'diverged' })).toBe(true); expect(shouldCreateReviewThread({ autoReviewEnabled: true }, { status: 'behind' })).toBe(false); }); }); ``` 2. - [ ] Run `cd packages/backend && bun run test:unit -- sync-gating` — expect FAIL. 3. - [ ] Create `packages/backend/convex/syncGating.ts` (NO `'use node'`; keep it importable by both actions and tests): ```ts export const shouldAutoSync = ( settings: { autoSyncEnabled: boolean; requireCleanCompareForSync: boolean }, ctx: { status: string; forkAheadBy: number }, ): boolean => { if (!settings.autoSyncEnabled) return false; if (ctx.status !== 'behind') return false; if (settings.requireCleanCompareForSync && ctx.forkAheadBy !== 0) return false; return ctx.forkAheadBy === 0; }; export const shouldCreateReviewThread = ( settings: { autoReviewEnabled: boolean }, ctx: { status: string }, ): boolean => settings.autoReviewEnabled && ctx.status === 'diverged'; ``` 4. - [ ] Run `cd packages/backend && bun run test:unit -- sync-gating` — expect PASS. 5. - [ ] Wire gating into `refreshOwnedSpoon` (`githubSync.ts`). Add imports: `import { shouldAutoSync, shouldCreateReviewThread } from './syncGating';`. Immediately after resolving `spoon` (~66), load settings: ```ts const settings = await ctx.runQuery(internal.spoonSettings.getInternal, { spoonId, ownerId, }); const autoSyncEnabled = settings?.autoSyncEnabled ?? false; const autoReviewEnabled = settings?.autoReviewEnabled ?? true; const requireCleanCompareForSync = settings?.requireCleanCompareForSync ?? true; ``` - Change the auto-sync condition (~205) from `if (status === 'behind' && forkCompare.aheadBy === 0 && allowAutoSync) {` to: ```ts if ( allowAutoSync && shouldAutoSync( { autoSyncEnabled, requireCleanCompareForSync }, { status, forkAheadBy: forkCompare.aheadBy }, ) ) { ``` - Change the diverged review-thread condition (~261) from `if (status === 'diverged') {` to: ```ts if (shouldCreateReviewThread({ autoReviewEnabled }, { status })) { ``` (The `requireAiLowRiskForSync` flag gates the *AI review → auto-merge* path, which does not exist in Phase 1; leave it for Phase 4. Note this in a code comment.) 6. - [ ] Flip the default for new GitHub spoons: in `packages/backend/convex/github.ts` `createForkSpoonRecord` (~193) change `autoSyncEnabled: false,` to `autoSyncEnabled: true,`. 7. - [ ] Create `packages/backend/convex/migrations.ts` with the backfill: ```ts import { internalMutation } from './_generated/server'; // One-shot backfill: preserve today's observed behavior (auto fast-forward of // clean GitHub forks) by enabling autoSyncEnabled on existing GitHub spoons, // now that refreshOwnedSpoon gates on it. Idempotent. export const backfillAutoSyncDefaults = internalMutation({ args: {}, handler: async (ctx) => { const settings = await ctx.db.query('spoonSettings').collect(); let updated = 0; for (const row of settings) { if (row.autoSyncEnabled) continue; const spoon = await ctx.db.get(row.spoonId); if (spoon?.provider !== 'github') continue; await ctx.db.patch(row._id, { autoSyncEnabled: true, updatedAt: Date.now(), }); updated += 1; } return { updated }; }, }); ``` Add a test in `sync-gating.test.ts` (or a new `packages/backend/tests/unit/migrations.test.ts`) using convex-test: seed two GitHub spoons (one with `autoSyncEnabled: false`, one already `true`) and a non-GitHub spoon with `false`; run `internal.migrations.backfillAutoSyncDefaults`; assert `{ updated: 1 }` and that only the GitHub `false` row flipped to `true`. 8. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit` — expect PASS. Document the deploy step: after deploying, run the migration once via `bunx convex run migrations:backfillAutoSyncDefaults` (self-hosted: use the deployment's `convex run`). 9. - [ ] Commit: `git add -A && git commit -m "feat(convex): gate auto-sync and review threads on spoon settings, backfill GitHub defaults"` --- ## Task 9: Unified trimmed worker-token compare + `getEnvironmentForJob` asserts `claimedBy` **Why (Medium, security hygiene):** `userDotfilesNode.requireWorkerToken` (`userDotfilesNode.ts:22`) compares against an untrimmed `process.env.SPOON_WORKER_TOKEN`, diverging from the trimmed comparisons elsewhere — a trailing newline in the env var silently rejects a valid worker. And `getEnvironmentForJob` returns a user's decrypted dotfiles for any job id without checking the job is actually claimed by a worker. **Files:** - Create `packages/backend/convex/workerAuth.ts` (shared trimming compare). - Modify `packages/backend/convex/agentJobs.ts`, `agentJobsNode.ts`, `userDotfilesNode.ts` to use it. - Modify `packages/backend/convex/userEnvironment.ts` (`getRawEnvironmentForJobInternal` ~68: return null when the job is not claimed). - Create `packages/backend/tests/unit/worker-auth.test.ts`. **Interfaces:** - `export const assertWorkerToken = (provided: string): void` — trims both sides; throws `ConvexError('SPOON_WORKER_TOKEN is not configured.')` / `ConvexError('Invalid worker token.')`. **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/worker-auth.test.ts`: ```ts import { ConvexError } from 'convex/values'; import { afterEach, describe, expect, test } from 'vitest'; import { assertWorkerToken } from '../../convex/workerAuth'; afterEach(() => { delete process.env.SPOON_WORKER_TOKEN; }); describe('assertWorkerToken', () => { test('accepts a token that matches after trimming', () => { process.env.SPOON_WORKER_TOKEN = 'secret\n'; expect(() => assertWorkerToken('secret')).not.toThrow(); expect(() => assertWorkerToken(' secret ')).not.toThrow(); }); test('rejects a mismatched token', () => { process.env.SPOON_WORKER_TOKEN = 'secret'; expect(() => assertWorkerToken('nope')).toThrow(ConvexError); }); test('throws when not configured', () => { expect(() => assertWorkerToken('x')).toThrow(ConvexError); }); }); ``` 2. - [ ] Run `cd packages/backend && bun run test:unit -- worker-auth` — expect FAIL. 3. - [ ] Create `packages/backend/convex/workerAuth.ts`: ```ts import { ConvexError } from 'convex/values'; // Single source of truth for verifying the shared worker token. Trims both the // configured secret and the provided token so trailing newlines never cause a // spurious mismatch. export const assertWorkerToken = (provided: string): void => { const expected = process.env.SPOON_WORKER_TOKEN?.trim(); if (!expected) throw new ConvexError('SPOON_WORKER_TOKEN is not configured.'); if (provided.trim() !== expected) { throw new ConvexError('Invalid worker token.'); } }; ``` 4. - [ ] Run `cd packages/backend && bun run test:unit -- worker-auth` — expect PASS. 5. - [ ] Replace the local `requireWorkerToken` implementations with the shared helper: - `agentJobs.ts` (~155-161): delete the local `getWorkerToken`/`requireWorkerToken`; add `import { assertWorkerToken } from './workerAuth';`; replace every `requireWorkerToken(args.workerToken)` call with `assertWorkerToken(args.workerToken)`. - `agentJobsNode.ts` (~44-48): delete local `requireWorkerToken`; import + use `assertWorkerToken`. - `userDotfilesNode.ts` (~21-25): delete local `requireWorkerToken`; import + use `assertWorkerToken`. (This is the untrimmed one — the actual bug fix.) 6. - [ ] Assert `claimedBy` in `userEnvironment.getRawEnvironmentForJobInternal` (~68). After `const job = await ctx.db.get(jobId); if (!job) return null;` add: ```ts if (!job.claimedBy) return null; // only a claimed job's env is exposed ``` Add a convex-test to `worker-auth.test.ts` (or a `getEnvironmentForJob` test) verifying an unclaimed job yields `null` — seed a queued job with no `claimedBy`, run `internal.userEnvironment.getRawEnvironmentForJobInternal`, assert `null`; then set `claimedBy: 'w1'` and assert non-null. 7. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit` — expect PASS. 8. - [ ] Commit: `git add -A && git commit -m "fix(convex): unify trimmed worker-token compare and require a claimed job for env access"` --- ## Task 10: Terminal worker — real TTY exec via dockerode so resize works **Why (High):** The worker drives ` exec -i … script -qfc …` and only sizes the PTY once via `stty` at launch; resize messages from the client just mutate local `cols`/`rows` and never reach the container, so the terminal stays the initial size. Replace the `script` pipe hack with a dockerode exec that allocates a real TTY (`exec.start({ Tty: true })`) and honors `exec.resize({ h, w })`. `dockerode` is already a dependency; it talks to the Docker socket (this deployment uses Docker for the terminal path — `attachTerminalServer` already early-returns unless `env.runtime === 'docker'`). **Files:** - Modify `apps/agent-worker/src/terminal.ts` (`bridge` ~21-150: dockerode exec + resize). - Modify `apps/agent-worker/src/runtime/docker.ts` (export a shared `Docker` instance / helper). - Create `apps/agent-worker/tests/unit/terminal-resize.test.ts` (unit test the pure resize-dimension extraction; the dockerode exec itself is verified manually). **Interfaces:** - Extract a pure helper in `terminal.ts`: `export const parseResizeMessage = (data: Buffer, isBinary: boolean): { cols: number; rows: number } | null` — returns clamped dims for a `{type:'resize'}` text frame, else null. Unit-testable without docker. - `runtime/docker.ts`: `export const getDockerClient = () => Docker` (a singleton `new Docker()` using the default socket). **Steps:** 1. - [ ] Write failing test `apps/agent-worker/tests/unit/terminal-resize.test.ts`. `terminal.ts` imports `./env`, `./worker`, `./user-container`, `ws` — to unit-test only the pure parser, `vi.mock` those heavy imports so importing `terminal.ts` doesn't pull docker/ws: ```ts import { afterEach, describe, expect, test, vi } from 'vitest'; const load = async () => { vi.resetModules(); process.env.SPOON_WORKER_TOKEN = 'test-worker-token'; process.env.GITHUB_APP_ID = '123'; process.env.GITHUB_APP_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\\ntest\\n-----END PRIVATE KEY-----'; vi.doMock('../../src/worker', () => ({ getTerminalWorkspace: () => null })); vi.doMock('../../src/user-container', () => ({ acquireUserBox: vi.fn(), })); return await import('../../src/terminal'); }; describe('parseResizeMessage', () => { afterEach(() => vi.resetModules()); test('parses and clamps a resize control frame', async () => { const { parseResizeMessage } = await load(); const msg = Buffer.from(JSON.stringify({ type: 'resize', cols: 120, rows: 40 })); expect(parseResizeMessage(msg, false)).toEqual({ cols: 120, rows: 40 }); }); test('returns null for binary input (raw keystrokes)', async () => { const { parseResizeMessage } = await load(); expect(parseResizeMessage(Buffer.from([1, 2, 3]), true)).toBeNull(); }); test('returns null for non-resize JSON', async () => { const { parseResizeMessage } = await load(); expect(parseResizeMessage(Buffer.from('{"type":"other"}'), false)).toBeNull(); }); test('clamps out-of-range dimensions into [1,1000]', async () => { const { parseResizeMessage } = await load(); const msg = Buffer.from(JSON.stringify({ type: 'resize', cols: 99999, rows: 0 })); expect(parseResizeMessage(msg, false)).toEqual({ cols: 1000, rows: 1 }); }); }); ``` 2. - [ ] Run `cd apps/agent-worker && bun run test:unit -- terminal-resize` — expect FAIL. 3. - [ ] In `apps/agent-worker/src/terminal.ts`, extract and export the parser (reuse the existing `clampDimension`): ```ts export const parseResizeMessage = ( data: Buffer, isBinary: boolean, ): { cols: number; rows: number } | null => { if (isBinary) return null; try { const message = JSON.parse(data.toString('utf8')) as { type?: string; cols?: number; rows?: number; }; if (message.type !== 'resize') return null; const cols = clampDimension(message.cols); const rows = clampDimension(message.rows); if (!cols || !rows) return null; return { cols, rows }; } catch { return null; } }; ``` 4. - [ ] Run `cd apps/agent-worker && bun run test:unit -- terminal-resize` — expect PASS. 5. - [ ] Add a dockerode singleton to `apps/agent-worker/src/runtime/docker.ts`: ```ts import Docker from 'dockerode'; let dockerClient: Docker | undefined; export const getDockerClient = () => { dockerClient ??= new Docker(); return dockerClient; }; ``` (Add `import Docker from 'dockerode';` at the top.) 6. - [ ] Rewrite `bridge` in `terminal.ts` to use dockerode. Declare the holders and rewrite the message handler (replacing ~35-64): ```ts const execHolder: { current?: { exec: import('dockerode').Exec; stream: NodeJS.ReadWriteStream }; } = {}; const pendingInput: Buffer[] = []; let cols = 80; let rows = 24; ws.on('message', (data: Buffer, isBinary: boolean) => { const resize = parseResizeMessage(data, isBinary); if (resize) { cols = resize.cols; rows = resize.rows; void execHolder.current?.exec.resize({ h: rows, w: cols }).catch(() => {}); return; } if (execHolder.current) execHolder.current.stream.write(data); else pendingInput.push(data); }); ``` - after acquiring the box, create the exec with a TTY and pipe both directions: ```ts const docker = getDockerClient(); const container = docker.getContainer(boxName); const exec = await container.exec({ AttachStdin: true, AttachStdout: true, AttachStderr: true, Tty: true, Cmd: [ '/bin/bash', '-lc', 'if command -v tmux >/dev/null 2>&1; then exec tmux new-session -A -s spoon; else exec bash -il; fi', ], Env: [ 'TERM=xterm-256color', `HOME=${workspace.containerHome}`, ...workspace.secrets.map((s) => `${s.name}=${s.value}`), ], WorkingDir: workspace.containerRepo, }); const stream = await exec.start({ hijack: true, stdin: true, Tty: true }); execHolder.current = { exec, stream }; await exec.resize({ h: rows, w: cols }).catch(() => {}); // replay buffered input, then bridge for (const buffered of pendingInput) stream.write(buffered); pendingInput.length = 0; stream.on('data', (chunk: Buffer) => { if (ws.readyState === ws.OPEN) ws.send(chunk, { binary: true }); }); stream.on('end', () => { if (ws.readyState === ws.OPEN) ws.close(); }); ``` - update `cleanup` to `stream.end()`/destroy instead of `procHolder.current?.kill()`; keep `handleHolder.current?.release()` from Task 2. - in the resize branch of `ws.on('message')`, after updating `cols`/`rows`, add: `void execHolder.current?.exec.resize({ h: rows, w: cols }).catch(() => {});` - remove the now-unused `spawn`, `shellQuote`, `script -qfc`, `stty` launcher, and `ChildProcessWithoutNullStreams` imports. 7. - [ ] Run `cd apps/agent-worker && bun run test:unit && bun run typecheck` — expect PASS. 8. - [ ] Manual verification (pure-infra; cannot unit-test dockerode against a real socket). With a running box: open the workspace terminal, run `tput cols; tput lines`, resize the browser pane, run again — the values track the pane. `printf '\ue0b0'` renders the powerline glyph. tmux reattach still works across reconnects. Documented as manual because the exec path needs a live Docker daemon. 9. - [ ] Commit: `git add -A && git commit -m "fix(worker): drive the terminal through a dockerode TTY exec so resize actually resizes the PTY"` --- ## Task 11: Terminal client — re-fit on fonts.ready + Nerd-Font load + rAF first fit **Why (High):** xterm measures cell size in `fit()` immediately after `term.open()`, before the Victor Mono webfont loads, so the grid is computed against a fallback metric and renders quarter-size; nothing re-fits when the real font arrives (`workspace-terminal.tsx:145`). Monaco already fixes this via `document.fonts.ready`. **Files:** - Create `apps/next/src/components/agent-workspace/terminal-fit.ts` (pure fit-scheduling helper). - Modify `apps/next/src/components/agent-workspace/workspace-terminal.tsx` (use the helper; first fit in rAF; re-fit + resize on fonts). - Create `apps/next/tests/unit/terminal-fit.test.ts`. **Interfaces:** - `export const scheduleTerminalFits = (deps: { fit: () => void; sendResize: () => void; fontsReady: Promise; loadNerdFont: () => Promise; raf: (cb: () => void) => void; isAborted: () => boolean }) => void` — runs `fit()`+`sendResize()` in the next animation frame, then again after `fontsReady`, then again after `loadNerdFont`, skipping when `isAborted()`. **Steps:** 1. - [ ] Write failing test `apps/next/tests/unit/terminal-fit.test.ts` (node env — pure logic, no DOM): ```ts import { describe, expect, test, vi } from 'vitest'; import { scheduleTerminalFits } from '@/components/agent-workspace/terminal-fit'; const flush = async () => { await Promise.resolve(); await Promise.resolve(); }; describe('scheduleTerminalFits', () => { test('fits in rAF and again after fonts.ready and nerd-font load', async () => { const fit = vi.fn(); const sendResize = vi.fn(); let rafCb: (() => void) | undefined; scheduleTerminalFits({ fit, sendResize, fontsReady: Promise.resolve(), loadNerdFont: () => Promise.resolve(), raf: (cb) => { rafCb = cb; }, isAborted: () => false, }); rafCb?.(); await flush(); expect(fit).toHaveBeenCalledTimes(3); // rAF + fonts.ready + nerd font expect(sendResize).toHaveBeenCalledTimes(3); }); test('does nothing once aborted', async () => { const fit = vi.fn(); scheduleTerminalFits({ fit, sendResize: vi.fn(), fontsReady: Promise.resolve(), loadNerdFont: () => Promise.resolve(), raf: (cb) => cb(), isAborted: () => true, }); await flush(); expect(fit).not.toHaveBeenCalled(); }); }); ``` 2. - [ ] Run `cd apps/next && bun run test:unit -- terminal-fit` — expect FAIL. 3. - [ ] Create `apps/next/src/components/agent-workspace/terminal-fit.ts`: ```ts export const scheduleTerminalFits = (deps: { fit: () => void; sendResize: () => void; fontsReady: Promise; loadNerdFont: () => Promise; raf: (cb: () => void) => void; isAborted: () => boolean; }): void => { const refit = () => { if (deps.isAborted()) return; deps.fit(); deps.sendResize(); }; deps.raf(refit); void deps.fontsReady.then(refit).catch(() => undefined); void deps.loadNerdFont().then(refit).catch(() => undefined); }; ``` 4. - [ ] Run `cd apps/next && bun run test:unit -- terminal-fit` — expect PASS. 5. - [ ] Use it in `workspace-terminal.tsx`. Replace the block after `term.open(container); fit.fit(); termRef.current = term;` and the standalone `document.fonts.load(...)` (~145-156). New code (keep `sendResize` defined before this call — move its declaration up if needed): ```ts term.open(container); termRef.current = term; const sendResize = () => { if (ws?.readyState !== WebSocket.OPEN) return; ws.send( JSON.stringify({ type: 'resize', cols: term.cols, rows: term.rows }), ); }; scheduleTerminalFits({ fit: () => { try { fit.fit(); } catch { // ignore transient layout errors } }, sendResize, fontsReady: document.fonts.ready, loadNerdFont: () => document.fonts .load("16px 'Symbols Nerd Font Mono'", '\ue0b0') .then(() => { if (!isAborted()) term.refresh(0, term.rows - 1); }), raf: (cb) => requestAnimationFrame(cb), isAborted, }); ``` - Delete the later duplicate `const sendResize = () => {...}` (~158-163) now that it is defined above. - Add the import at the top of the file: `import { scheduleTerminalFits } from './terminal-fit';` 6. - [ ] Run `cd apps/next && bun run test:unit -- terminal-fit && bun run typecheck` — expect PASS. 7. - [ ] Commit: `git add -A && git commit -m "fix(next): re-fit the workspace terminal on fonts.ready and Nerd-Font load"` --- ## Task 12: Key `AgentWorkspaceShell` by job on both routes **Why (High):** Navigating thread A → B keeps the same `AgentWorkspaceShell` React instance, so it shows A's files and overwrites B's persisted UI state with A's. Adding `key={jobId}` forces a fresh mount per job. **Files:** - Modify `apps/next/src/app/(app)/threads/[threadId]/page.tsx` (~56). - Modify `apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx` (~40). - Create `apps/next/tests/component/agent-workspace-shell-key.test.tsx`. **Interfaces:** none changed; add `key`. **Steps:** 1. - [ ] Write failing component test `apps/next/tests/component/agent-workspace-shell-key.test.tsx`. Because the real routes pull in Convex/xterm, test the *keying contract* directly: a tiny wrapper that renders a mount-counting child with `key={jobId}` and asserts a jobId change remounts it (mount counter increments, state resets). Match the render style of `apps/next/tests/component/render.test.tsx`: ```ts import { render } from '@testing-library/react'; import { describe, expect, test } from 'vitest'; import { useEffect } from 'react'; let mounts = 0; const Child = ({ jobId }: { jobId: string }) => { useEffect(() => { mounts += 1; }, []); return {jobId}; }; const Shell = ({ jobId }: { jobId: string }) => ; describe('workspace shell keyed by job', () => { test('remounts when jobId changes', () => { mounts = 0; const { rerender } = render(); expect(mounts).toBe(1); rerender(); expect(mounts).toBe(2); }); }); ``` This encodes the expected pattern (a keyed child remounts on id change). It passes once the pattern is present; it is a regression guard for the pattern the routes must use. 2. - [ ] Run `cd apps/next && bun run test:component -- agent-workspace-shell-key` — expect PASS (it validates the keying contract). If your harness requires the failing-first cycle, first assert `mounts).toBe(1)` after rerender (wrong) to see it FAIL, then correct to `2`. 3. - [ ] Apply the key in both routes: - `threads/[threadId]/page.tsx` (~56): ``. - `spoons/[spoonId]/agent/[jobId]/page.tsx` (~40): ``. 4. - [ ] Run `cd apps/next && bun run typecheck && bun run test:component -- agent-workspace-shell-key` — expect PASS. 5. - [ ] Commit: `git add -A && git commit -m "fix(next): remount AgentWorkspaceShell per job so thread switches don't cross state"` --- ## Task 13: Not-found handling — `(app)/error.tsx` + `not-found`, queries return null **Why (Medium):** `threads.get` and `agentJobs.get` throw `ConvexError` for a missing/foreign id, which bubbles to `global-error.tsx` (a full-page crash) instead of a friendly in-app not-found. Change these read queries to return `null` for missing/unowned, add an `(app)/error.tsx` + `not-found.tsx`, and make the two detail pages distinguish loading (`undefined`) from missing (`null`). **Files:** - Modify `packages/backend/convex/agentJobs.ts` (`get` ~707: return `null` instead of throw). - Modify `packages/backend/convex/threads.ts` (`get` ~209: return `null` instead of throw). - Create `apps/next/src/app/(app)/error.tsx` and `apps/next/src/app/(app)/not-found.tsx`. - Modify `apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx` and `.../threads/[threadId]/page.tsx` to handle `null`. - Create `packages/backend/tests/unit/detail-null.test.ts`. **Interfaces:** - `agentJobs.get`: returns `Doc<'agentJobs'> | null`. - `threads.get`: returns `{ thread, spoon, latestJob } | null`. **Steps:** 1. - [ ] Write failing test `packages/backend/tests/unit/detail-null.test.ts`: authed user A creates a thread + a job; authed user B calls `api.threads.get`/`api.agentJobs.get` with A's ids and gets `null` (not a throw). Also a genuinely nonexistent id returns `null`. Use the `authed()`/`createUser()` helpers from `harness.test.ts`. Assert `await expect(...).resolves.toBeNull()`. 2. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- detail-null` — expect FAIL (currently throws). 3. - [ ] Implement: - `agentJobs.get` (~707): replace `if (job?.ownerId !== ownerId) throw new ConvexError('Agent job not found.');` with `if (job?.ownerId !== ownerId) return null;`. - `threads.get` (~209): replace `if (thread?.ownerId !== ownerId) throw new ConvexError('Thread not found.');` with `if (thread?.ownerId !== ownerId) return null;`. (Leave the other owner-mutating functions throwing; only the read-detail `get`s change.) 4. - [ ] Run `bun codegen:convex && cd packages/backend && bun run test:unit -- detail-null` — expect PASS. 5. - [ ] Create `apps/next/src/app/(app)/error.tsx` (client component; friendly, matches app shell — the `(app)/layout.tsx` wraps it in `AppShell`): ```tsx 'use client'; import { useEffect } from 'react'; import * as Sentry from '@sentry/nextjs'; import { Button } from '@spoon/ui'; const AppError = ({ error, reset, }: { error: Error & { digest?: string }; reset: () => void; }) => { useEffect(() => { Sentry.captureException(error); }, [error]); return (

Something went wrong

This page hit an unexpected error. It has been reported.

); }; export default AppError; ``` 6. - [ ] Create `apps/next/src/app/(app)/not-found.tsx`: ```tsx import Link from 'next/link'; import { Button } from '@spoon/ui'; const NotFound = () => (

Not found

This item does not exist, or you do not have access to it.

); export default NotFound; ``` 7. - [ ] Handle `null` in the detail pages (distinguish loading `undefined` from missing `null`): - `spoons/[spoonId]/agent/[jobId]/page.tsx`: after `const job = useQuery(api.agentJobs.get, { jobId });` add, before the `useEffect`: ```tsx if (job === undefined) { return
Loading workspace...
; } if (job === null) { return (

This workspace was not found.

); } ``` (Keep the existing `useEffect`/`threadId` redirect below; note React hooks must run unconditionally — since `useQuery` and `useEffect` are already declared before these returns, move the early returns to AFTER the `useEffect` declaration to preserve hook order. Concretely: keep `const job = useQuery(...)` and the `useEffect(...)` at the top, then the `undefined`/`null` guards, then the render.) - `threads/[threadId]/page.tsx`: `details` is already guarded for `undefined` (~42). Add a `null` guard right after: `if (details === null) { return (
This thread was not found.
); }` and destructure only after. (Note `threads.get` now returns `null`; TypeScript will require this guard before `const { thread, spoon, latestJob } = details;`.) 8. - [ ] Run `bun codegen:convex && cd apps/next && bun run typecheck` and `cd packages/backend && bun run test:unit` — expect PASS. 9. - [ ] Commit: `git add -A && git commit -m "feat(next,convex): return null for missing detail queries and add friendly app error/not-found states"` --- ## Final verification - [ ] From repo root: `bun codegen:convex`. - [ ] `cd apps/agent-worker && bun run test:unit && bun run typecheck`. - [ ] `cd packages/backend && bun run test:unit && bun run typecheck`. - [ ] `cd apps/next && bun run test:unit && bun run test:component && bun run typecheck`. - [ ] Confirm `grep -rn "releaseUserBox\|safeWorkspacePath\|safeHomeJoin" apps/agent-worker/src` returns no stale references (all replaced). - [ ] Post-deploy one-shot: `bunx convex run migrations:backfillAutoSyncDefaults` against the target deployment.