spoon/docs/superpowers/plans/2026-07-10-spoon-phase1-stabilize.md

# 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<string, { name: string; isDirectory: boolean }[]>; codeLeaves: Record<string, string[]>; activeWorkdirs: Set<string>; runningBoxUsernames: Set<string>; root: string }) => { removeDirs: string[] }`
- Produces (user-container.ts): `export const runningBoxUsernames = (): Set<string>` — 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<string, { name: string; isDirectory: boolean }[]>;
  codeLeaves: Record<string, string[]>;
  activeWorkdirs: Set<string>;
  runningBoxUsernames: Set<string>;
}): { 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<string, string[]> = {};
    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<string> => 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<BoxHandle>` (was `Promise<string>`).
- `export const runningBoxUsernames = () => Set<string>` (kept; now = usernames with refs.size > 0 OR adopted).
- `export const reconcileExistingBoxes = () => Promise<void>` — 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<typeof vi.fn>).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<symbol>; idleTimer?: ReturnType<typeof setTimeout> };
const boxes = new Map<string, Box>();
const locks = new Map<string, Promise<unknown>>();

// Per-username async mutex: chain each operation after the previous one.
const withLock = <T>(username: string, fn: () => Promise<T>): Promise<T> => {
  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<BoxHandle> =>
  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<void> => {
  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<string> => 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-<user>` 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<string>` — 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<string> => {
  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<string> => {
  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<string, ReturnType<typeof setInterval>>();

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 `<runtime> 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<unknown>; loadNerdFont: () => Promise<unknown>; 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<unknown>;
  loadNerdFont: () => Promise<unknown>;
  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 <span>{jobId}</span>;
};
const Shell = ({ jobId }: { jobId: string }) => <Child key={jobId} jobId={jobId} />;

describe('workspace shell keyed by job', () => {
  test('remounts when jobId changes', () => {
    mounts = 0;
    const { rerender } = render(<Shell jobId='a' />);
    expect(mounts).toBe(1);
    rerender(<Shell jobId='b' />);
    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): `<AgentWorkspaceShell key={latestJob._id} jobId={latestJob._id} />`.
   - `spoons/[spoonId]/agent/[jobId]/page.tsx` (~40): `<AgentWorkspaceShell key={jobId} jobId={jobId} />`.

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 (
    <main className='flex min-h-[50vh] flex-col items-center justify-center gap-4 p-6 text-center'>
      <h1 className='text-2xl font-semibold'>Something went wrong</h1>
      <p className='text-muted-foreground max-w-md text-sm'>
        This page hit an unexpected error. It has been reported.
      </p>
      <Button onClick={() => reset()}>Try again</Button>
    </main>
  );
};

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 = () => (
  <main className='flex min-h-[50vh] flex-col items-center justify-center gap-4 p-6 text-center'>
    <h1 className='text-2xl font-semibold'>Not found</h1>
    <p className='text-muted-foreground max-w-md text-sm'>
      This item does not exist, or you do not have access to it.
    </p>
    <Button asChild>
      <Link href='/dashboard'>Back to dashboard</Link>
    </Button>
  </main>
);

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 <main className='text-muted-foreground p-6'>Loading workspace...</main>;
  }
  if (job === null) {
    return (
      <main className='space-y-4 p-6'>
        <p className='text-muted-foreground'>This workspace was not found.</p>
        <Button asChild variant='outline' size='sm'>
          <Link href={`/spoons/${params.spoonId}`}>Back to Spoon</Link>
        </Button>
      </main>
    );
  }
```
   (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 (<main className='p-6 text-muted-foreground'>This thread was not found.</main>); }` 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.