Files
spoon/docs/superpowers/plans/2026-07-10-spoon-phase1-stabilize.md
T
Gabriel Brown 8aa9140191 docs: multi-phase implementation plan (63 tasks across 5 phases)
Phase 1 Stabilize (13), Phase 2 Runtime unification (12),
Phase 3 Dev-box surface (11), Phase 4 Sync correctness (12),
Phase 5 Notifications & polish (15), plus index.
2026-07-10 15:18:51 -04:00

81 KiB

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 -rfs 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:

    • 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:
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');
  });
});
    • Run cd apps/agent-worker && bun run test:unit -- cleanup-layout — expect FAIL (planWorkdirCleanup is not exported).
    • Implement the pure helper. Add near the top of apps/agent-worker/src/worker.ts (after the slugify helper, before runClaim):
// 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 };
};
    • Run cd apps/agent-worker && bun run test:unit -- cleanup-layout — expect PASS.
    • 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:
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 };
};
    • Add runningBoxUsernames import to worker.ts: change the import from ./user-container (line ~44) to import { acquireUserBox, releaseUserBox, runningBoxUsernames } from './user-container';.
    • Export runningBoxUsernames from apps/agent-worker/src/user-container.ts (append):
// 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());
    • Update getWorkerHealth (~1888) so workspaceContainers enumerates boxes too. Replace the single listWorkspaceContainerNames('spoon-agent-job-') call with:
  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,.

    • Run cd apps/agent-worker && bun run test:unit and bun run typecheck — expect PASS.
    • 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:

    • 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:
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');
  });
});
    • Run cd apps/agent-worker && bun run test:unit -- user-container — expect FAIL.
    • Rewrite apps/agent-worker/src/user-container.ts in full:
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();
};
    • Run cd apps/agent-worker && bun run test:unit -- user-container — expect PASS.
    • 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',:
    [
      '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.

    • 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:
    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();.
    • 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:
  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.)

    • 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:
import { reconcileExistingBoxes } from './user-container';

and just before await startWorker(); at the bottom add:

await reconcileExistingBoxes();
    • 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.)
    • Commit: git add -A && git commit -m "fix(worker): make per-user box registry concurrency-safe with idempotent handles and startup reconcile"

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.realpaths 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:

    • 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):
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();
  });
});
    • Run cd apps/agent-worker && bun run test:unit -- path-containment — expect FAIL (module missing).
    • Create apps/agent-worker/src/path-containment.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;
};
    • Run cd apps/agent-worker && bun run test:unit -- path-containment — expect PASS.
    • 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).
    • 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.
    • Run cd apps/agent-worker && bun run test:unit && bun run typecheck — expect PASS.
    • 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:

    • 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:
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.)

    • 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).
    • Implement. In packages/backend/convex/agentJobs.ts, add near isTerminalJob (~243):
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:

    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 };
    }
    • Run cd packages/backend && bun run test:unit -- update-status — expect PASS.
    • 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:

    • 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.
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();
    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- claim-validation — expect FAIL.
    • 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:
    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).

    • 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.
    • 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.
    • 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:

    • 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.
    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- recover-stale-jobs — expect FAIL.
    • Implement recoverStaleJobs in agentJobs.ts (add near the other internal mutations; ensure internalMutation is imported — it already is):
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 };
  },
});
    • Update heartbeatWorkspace (~1358) to return cancelRequested. After the patch, replace return { success: true }; with:
    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.)

    • Register the cron in packages/backend/convex/crons.ts (after the existing refresh cron):
crons.interval(
  'Recover stale agent jobs',
  { minutes: 1 },
  internal.agentJobs.recoverStaleJobs,
  {},
);
    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- recover-stale-jobs — expect PASS.
    • Add the worker heartbeat loop + cancel teardown in apps/agent-worker/src/worker.ts.
    • Add a wrapper mutation helper near markWorkspaceStopped (~244):
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();.
    • 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.
    • 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:

    • 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.
    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- maintenance-decision — expect FAIL.
    • 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):
    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.)

    • 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:
        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.)

    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- maintenance-decision and cd apps/agent-worker && bun run typecheck — expect PASS.
    • 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:

    • Write failing test packages/backend/tests/unit/sync-gating.test.ts (pure unit test, node env — put it under tests/unit/):
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);
  });
});
    • Run cd packages/backend && bun run test:unit -- sync-gating — expect FAIL.
    • Create packages/backend/convex/syncGating.ts (NO 'use node'; keep it importable by both actions and tests):
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';
    • Run cd packages/backend && bun run test:unit -- sync-gating — expect PASS.
    • Wire gating into refreshOwnedSpoon (githubSync.ts). Add imports: import { shouldAutoSync, shouldCreateReviewThread } from './syncGating';. Immediately after resolving spoon (~66), load settings:
  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:
    if (
      allowAutoSync &&
      shouldAutoSync(
        { autoSyncEnabled, requireCleanCompareForSync },
        { status, forkAheadBy: forkCompare.aheadBy },
      )
    ) {
  • Change the diverged review-thread condition (~261) from if (status === 'diverged') { to:
    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.)

    • Flip the default for new GitHub spoons: in packages/backend/convex/github.ts createForkSpoonRecord (~193) change autoSyncEnabled: false, to autoSyncEnabled: true,.
    • Create packages/backend/convex/migrations.ts with the backfill:
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.

    • 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).
    • 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:

    • Write failing test packages/backend/tests/unit/worker-auth.test.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);
  });
});
    • Run cd packages/backend && bun run test:unit -- worker-auth — expect FAIL.
    • Create packages/backend/convex/workerAuth.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.');
  }
};
    • Run cd packages/backend && bun run test:unit -- worker-auth — expect PASS.
    • 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.)
    • Assert claimedBy in userEnvironment.getRawEnvironmentForJobInternal (~68). After const job = await ctx.db.get(jobId); if (!job) return null; add:
    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.

    • Run bun codegen:convex && cd packages/backend && bun run test:unit — expect PASS.
    • 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:

    • 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:
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 });
  });
});
    • Run cd apps/agent-worker && bun run test:unit -- terminal-resize — expect FAIL.
    • In apps/agent-worker/src/terminal.ts, extract and export the parser (reuse the existing clampDimension):
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;
  }
};
    • Run cd apps/agent-worker && bun run test:unit -- terminal-resize — expect PASS.
    • Add a dockerode singleton to apps/agent-worker/src/runtime/docker.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.)

    • Rewrite bridge in terminal.ts to use dockerode. Declare the holders and rewrite the message handler (replacing ~35-64):
  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:
  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.
    • Run cd apps/agent-worker && bun run test:unit && bun run typecheck — expect PASS.
    • 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.
    • 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:

    • Write failing test apps/next/tests/unit/terminal-fit.test.ts (node env — pure logic, no DOM):
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();
  });
});
    • Run cd apps/next && bun run test:unit -- terminal-fit — expect FAIL.
    • Create apps/next/src/components/agent-workspace/terminal-fit.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);
};
    • Run cd apps/next && bun run test:unit -- terminal-fit — expect PASS.
    • 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):
      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';
    • Run cd apps/next && bun run test:unit -- terminal-fit && bun run typecheck — expect PASS.
    • 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:

    • 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:
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.

    • 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.
    • 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} />.
    • Run cd apps/next && bun run typecheck && bun run test:component -- agent-workspace-shell-key — expect PASS.
    • 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:

    • 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().
    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- detail-null — expect FAIL (currently throws).
    • 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 gets change.)
    • Run bun codegen:convex && cd packages/backend && bun run test:unit -- detail-null — expect PASS.
    • Create apps/next/src/app/(app)/error.tsx (client component; friendly, matches app shell — the (app)/layout.tsx wraps it in AppShell):
'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;
    • Create apps/next/src/app/(app)/not-found.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;
    • 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:
  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;.)
    • Run bun codegen:convex && cd apps/next && bun run typecheck and cd packages/backend && bun run test:unit — expect PASS.
    • 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.