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.
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.tsrun withcd apps/agent-worker && bun run test:unit(orvitest run --project unit). Backend tests:packages/backend/tests/unit/*.test.tsusingconvex-test, run withcd packages/backend && bun run test:unit. Next component tests: jsdom + @testing-library/react viacd apps/next && bun run test:component; Next unit tests (node env) viacd 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 -nunless a hook blocks; precommit runs lint-staged. - Convex functions live in
packages/backend/convex; codegen viabun codegen:convexfrom repo root before typecheck/test. New Convex functions are auto-discovered by the convex-test harness (import.meta.glob('../../convex/**/*.*s')), but you must runbun codegen:convexfrom the repo root so_generated/apiand_generated/dataModeltypecheck. - Worker unit tests that touch modules importing
./envMUST set the required env vars before the dynamic import and usevi.resetModules()(seeapps/agent-worker/tests/unit/docker-runtime.test.tsfor the exact pattern).env.tsthrows at import time ifSPOON_WORKER_TOKEN,GITHUB_APP_ID, orGITHUB_APP_PRIVATE_KEYare missing. - Worker in-memory concurrency (box registry, cleanup) is tested by
vi.mock-ing../../src/runtime/dockerand../../src/env, then dynamically importing the module under test. Match the style oftests/unit/docker-runtime.test.tsandtests/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(exportrunningBoxUsernames()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 theboxesmap (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 setSPOON_WORKER_TOKEN,GITHUB_APP_ID,GITHUB_APP_PRIVATE_KEYfirst, matchingdocker-runtime.test.ts). Assert onplanWorkdirCleanup:
- Write failing test
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 (planWorkdirCleanupis not exported).
- Run
-
- Implement the pure helper. Add near the top of
apps/agent-worker/src/worker.ts(after theslugifyhelper, beforerunClaim):
- Implement the pure helper. Add near the top of
// 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.
- Run
-
- Wire the planner into
cleanupOrphanedWorkspaces. Replace the body (~1911-1954) so it: enumerates BOTH container prefixes, builds the tree inputs from the filesystem, callsplanWorkdirCleanup, and removes only the planned dirs. Real code:
- Wire the planner into
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
runningBoxUsernamesimport to worker.ts: change the import from./user-container(line ~44) toimport { acquireUserBox, releaseUserBox, runningBoxUsernames } from './user-container';.
- Add
-
- Export
runningBoxUsernamesfromapps/agent-worker/src/user-container.ts(append):
- Export
// 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) soworkspaceContainersenumerates boxes too. Replace the singlelistWorkspaceContainerNames('spoon-agent-job-')call with:
- Update
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:unitandbun run typecheck— expect PASS.
- Run
-
- Commit:
git add -A && git commit -m "fix(worker): make workspace cleanup layout-aware so it never deletes user homes"
- Commit:
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 aboxHandleonActiveWorkspace; release via handle inrunClaimcatch,openWorkspacePullRequest,stopWorkspace). - Modify
apps/agent-worker/src/terminal.ts(use the handle; release on disconnect-during-acquire). - Modify
apps/agent-worker/src/index.ts(callreconcileExistingBoxes()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>(wasPromise<string>).export const runningBoxUsernames = () => Set<string>(kept; now = usernames with refs.size > 0 OR adopted).export const reconcileExistingBoxes = () => Promise<void>— adopt existingspoon-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')andvi.mock('../../src/env'); use fake timers for the idle reaper. Example:
- Write failing test
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.
- Run
-
- Rewrite
apps/agent-worker/src/user-container.tsin full:
- Rewrite
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.
- Run
-
- Add
--initto the box inapps/agent-worker/src/runtime/docker.tsensureUserContainer(~360). In therunargs array, insert'--init',immediately after'-d',:
- Add
[
'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.tsto store and release the handle instead of a username string. - Add to the
ActiveWorkspacetype (afterboxName: string;, ~line 110):boxHandle: BoxHandle; - Import the type: change the
./user-containerimport toimport { acquireUserBox, runningBoxUsernames } from './user-container';andimport type { BoxHandle } from './user-container';(dropreleaseUserBox). - In
runClaim(~1330): changelet acquiredBoxUser: string | undefined;tolet acquiredBoxHandle: BoxHandle | undefined;. Replaceconst boxName = await acquireUserBox({...}); acquiredBoxUser = username;with:
- Update
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) replaceif (acquiredBoxUser) releaseUserBox(acquiredBoxUser);withacquiredBoxHandle?.release();. - In
openWorkspacePullRequest(~1859) replacereleaseUserBox(workspace.username);withworkspace.boxHandle.release();. - In
stopWorkspace(~1876) replacereleaseUserBox(workspace.username);withworkspace.boxHandle.release();.
-
- Update
apps/agent-worker/src/terminal.tsto use the handle and release on disconnect-during-acquire. - Change import (line 9) to
import { acquireUserBox } from './user-container';andimport type { BoxHandle } from './user-container';. - Replace the acquire/cleanup block (~66-98). Real code:
- Update
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 lineimport { startWorker } from './worker';region to also import reconcile and call it beforestartWorker:
- Call reconcile at startup. In
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. (IfreleaseUserBoxis referenced anywhere else,grep -rn releaseUserBox apps/agent-worker/srcreturns nothing.)
- Run
-
- Commit:
git add -A && git commit -m "fix(worker): make per-user box registry concurrency-safe with idempotent handles and startup reconcile"
- Commit:
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 atreadWorkspaceFile,writeWorkspaceFile,materializeEnvFile). - Modify
apps/agent-worker/src/user-environment.ts(safeHomeJoinwrite 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, thenfs.realpaths the deepest existing ancestor; throws if the real path escapesrealpath(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.tsusing real fs in an OS temp dir (no env import needed, so no module mocking):
- Write failing test
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).
- Run
-
- Create
apps/agent-worker/src/path-containment.ts:
- Create
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.
- Run
-
- Use it in
apps/agent-worker/src/worker.ts. Addimport { assertContainedRealPath } from './path-containment';near the top imports. readWorkspaceFile(~1477): replaceconst target = safeWorkspacePath(workspace.repoDir, filePath);withconst target = await assertContainedRealPath(workspace.repoDir, filePath);.writeWorkspaceFile(~1483): replaceconst target = safeWorkspacePath(workspace.repoDir, filePath);withconst target = await assertContainedRealPath(workspace.repoDir, filePath, { forWrite: true });.materializeEnvFile(~1170): replaceconst envPath = safeWorkspacePath(repoDir, claim.job.envFilePath);withconst envPath = await assertContainedRealPath(repoDir, claim.job.envFilePath, { forWrite: true });.- Leave the synchronous
safeWorkspacePathin place only if still referenced;grep -rn safeWorkspacePath apps/agent-worker/src— if no remaining references, delete the function (~1059-1066).
- Use it in
-
- Use it in
apps/agent-worker/src/user-environment.tsoverlay writes. Addimport { assertContainedRealPath } from './path-containment';. In the overlay loop (~113): replaceconst target = safeHomeJoin(homeDir, file.path);withconst target = await assertContainedRealPath(homeDir, file.path, { forWrite: true });. Remove the now-unusedsafeHomeJoin(~35-42) ifgrep -rn safeHomeJoin apps/agent-worker/srcshows no other users.
- Use it in
-
- Run
cd apps/agent-worker && bun run test:unit && bun run typecheck— expect PASS.
- Run
-
- Commit:
git add -A && git commit -m "fix(worker): resolve realpath and reject symlinked targets on all workspace file access"
- Commit:
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(addisTerminalStatus; guard inupdateStatus~1108). - Create
packages/backend/tests/unit/update-status.test.ts.
Interfaces:
- Add
const TERMINAL_STATUSES = new Set(['failed', 'cancelled', 'timed_out', 'draft_pr_opened']);andconst isTerminalStatus = (s: string) => TERMINAL_STATUSES.has(s);inagentJobs.ts. updateStatusreturns{ success: true }or{ success: true, ignored: true }.
Steps:
-
- Write failing test
packages/backend/tests/unit/update-status.test.tsmodeled onharness.test.ts(convexTest,import.meta.glob,authed/createUser). Seed a job withstatus: 'cancelled'andclaimedBy: 'worker-1', callapi.agentJobs.updateStatuswithstatus: 'running', assert the job stayscancelledand the result is{ success: true, ignored: true }. Then a second test: arunningjob acceptschecks_running. Setprocess.env.SPOON_WORKER_TOKEN = 'test-worker-token'in the test and passworkerToken: 'test-worker-token'. Example core:
- Write failing test
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) thencd packages/backend && bun run test:unit -- update-status— expect FAIL (result lacksignored, status flips to running).
- Run
-
- Implement. In
packages/backend/convex/agentJobs.ts, add nearisTerminalJob(~243):
- Implement. In
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.
- Run
-
- Commit:
git add -A && git commit -m "fix(convex): reject status writes to terminal agent jobs so cancel sticks"
- Commit:
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:
claimNextInternalstill returns the sameClaimedJob | nullshape on success; returnsnullafter marking a jobfailedwhen 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 whoseaiProviderProfileIdis undefined and the owner has no configured profile. Callinternal.agentJobs.claimNextInternalviat.mutation(internal.agentJobs.claimNextInternal, { workerId: 'w1' }). Assert it returnsnulland the job is nowstatus: 'failed'with a non-emptyerror. Second test: a fully valid queued job returns a non-null claim and statusclaimed.
- Write failing test
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.
- Run
-
- Implement in
claimNextInternal(~1046). After loadingspoon,aiProviderProfile, andsecretsbut BEFORE thectx.db.patch(job._id, { status: 'claimed', ... }), add a validation block that, on failure, patches the job tofailedand returnsnull:
- Implement in
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
claimNextForWorkerinagentJobsNode.ts: the mutation now guarantees a validspoonandaiProviderProfile, but keep defensive non-throwing handling — replace the threethrow new ConvexError(...)guards (~64-77) with an earlyif (!claimed.spoon || !claimed.aiProviderProfile) return null;(the job is alreadyfailed, so returning null just moves on). Keep the decryption/return mapping.
- Simplify
-
- Run
bun codegen:convex && cd packages/backend && bun run test:unit -- claim-validation— expect PASS. Also re-runbun run test:unit(full) to ensure no regression.
- Run
-
- Commit:
git add -A && git commit -m "fix(convex): validate spoon/profile/secrets inside claim so bad jobs fail instead of wedging"
- Commit:
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 returnscancelRequested; addrecoverStaleJobsinternalMutation). - 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:
heartbeatWorkspacereturns{ success: true, cancelRequested: boolean }wherecancelRequested = job.status === 'cancelled'.export const recoverStaleJobs = internalMutation({ args: { staleMs: v.optional(v.number()) }, ... })— for jobs withstatus ∈ {claimed, preparing, running, checks_running}whoselastHeartbeatAt(orclaimedAtif no heartbeat) is older thanstaleMs(default 180000), setstatus: 'timed_out', threadstatus: 'failed'.- Worker:
startHeartbeat(jobId)/stopHeartbeat(jobId)managingsetIntervalhandles in aMap.
Steps:
-
- Write failing test
packages/backend/tests/unit/recover-stale-jobs.test.ts: seed arunningjob withlastHeartbeatAt = Date.now() - 10*60*1000andclaimedBy: 'w1'(+ its thread). Callt.mutation(internal.agentJobs.recoverStaleJobs, {}). Assert jobstatus === 'timed_out'and threadstatus === 'failed'. Second test: arunningjob with a freshlastHeartbeatAt = Date.now()is left untouched.
- Write failing test
-
- Run
bun codegen:convex && cd packages/backend && bun run test:unit -- recover-stale-jobs— expect FAIL.
- Run
-
- Implement
recoverStaleJobsinagentJobs.ts(add near the other internal mutations; ensureinternalMutationis imported — it already is):
- Implement
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 returncancelRequested. After thepatch, replacereturn { success: true };with:
- Update
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):
- Register the cron in
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.
- Run
-
- Add the worker heartbeat loop + cancel teardown in
apps/agent-worker/src/worker.ts. - Add a wrapper mutation helper near
markWorkspaceStopped(~244):
- Add the worker heartbeat loop + cancel teardown in
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 afterawait markWorkspaceActive({ jobId });(~1388) addstartHeartbeat(jobId);. - In
stopWorkspace(~1866) andopenWorkspacePullRequest(~1851), addstopHeartbeat(jobId);right beforeactiveWorkspaces.delete(jobId);. - In
runClaim'scatch(~1436), addstopHeartbeat(jobId);alongsideacquiredBoxHandle?.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 /cleanupoff; callapi.agentJobs.cancel; within ~30s the worker logs "Cancellation requested" and the box is released.
- Run
-
- Commit:
git add -A && git commit -m "feat(worker,convex): heartbeat active workspaces, recover stale jobs via cron, and tear down on cancel"
- Commit:
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(sendWorkspaceMessagemaintenance 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 arunningmaintenance job with athreadId,claimedBy: 'w1'. Callapi.agentJobs.applyMaintenanceDecisionwith async/low-risk decision (requiresUserApproval: false). Assert the threadstatus === 'resolved'AND the jobworkspaceStatus === 'stopped'with acompletedAtset.
- Write failing test
-
- Run
bun codegen:convex && cd packages/backend && bun run test:unit -- maintenance-decision— expect FAIL.
- Run
-
- Implement in
applyMaintenanceDecision(~1436). After the guardif (!job.threadId) return { success: true };and before/after the thread patch, add a job patch. Insert right afterconst now = Date.now();(~1443):
- Implement in
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.tssendWorkspaceMessage, in themaintenance_reviewbranch (~1742), afterawait applyMaintenanceDecision(claim.job._id, decision);add:
- In the worker, after applying the decision, stop the workspace. In
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-decisionandcd apps/agent-worker && bun run typecheck— expect PASS.
- Run
-
- Commit:
git add -A && git commit -m "fix(convex,worker): close the job and stop the box after a maintenance decision"
- Commit:
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: defaultautoSyncEnabled: 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 whenautoSyncEnabledandstatus === 'behind'andforkAheadBy === 0(a clean compare).export const shouldCreateReviewThread = (settings: { autoReviewEnabled: boolean }, ctx: { status: string }): boolean— true only whenautoReviewEnabledandstatus === 'diverged'.export const backfillAutoSyncDefaults = internalMutation(...)— for everyspoonSettingsrow whose spoon is a GitHub spoon, setautoSyncEnabled: trueif not already.
Steps:
-
- Write failing test
packages/backend/tests/unit/sync-gating.test.ts(pure unit test, node env — put it undertests/unit/):
- Write failing test
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.
- Run
-
- Create
packages/backend/convex/syncGating.ts(NO'use node'; keep it importable by both actions and tests):
- Create
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.
- Run
-
- Wire gating into
refreshOwnedSpoon(githubSync.ts). Add imports:import { shouldAutoSync, shouldCreateReviewThread } from './syncGating';. Immediately after resolvingspoon(~66), load settings:
- Wire gating into
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.tscreateForkSpoonRecord(~193) changeautoSyncEnabled: false,toautoSyncEnabled: true,.
- Flip the default for new GitHub spoons: in
-
- Create
packages/backend/convex/migrations.tswith the backfill:
- Create
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 viabunx convex run migrations:backfillAutoSyncDefaults(self-hosted: use the deployment'sconvex run).
- Run
-
- Commit:
git add -A && git commit -m "feat(convex): gate auto-sync and review threads on spoon settings, backfill GitHub defaults"
- Commit:
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.tsto 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; throwsConvexError('SPOON_WORKER_TOKEN is not configured.')/ConvexError('Invalid worker token.').
Steps:
-
- Write failing test
packages/backend/tests/unit/worker-auth.test.ts:
- Write failing test
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.
- Run
-
- Create
packages/backend/convex/workerAuth.ts:
- Create
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.
- Run
-
- Replace the local
requireWorkerTokenimplementations with the shared helper: agentJobs.ts(~155-161): delete the localgetWorkerToken/requireWorkerToken; addimport { assertWorkerToken } from './workerAuth';; replace everyrequireWorkerToken(args.workerToken)call withassertWorkerToken(args.workerToken).agentJobsNode.ts(~44-48): delete localrequireWorkerToken; import + useassertWorkerToken.userDotfilesNode.ts(~21-25): delete localrequireWorkerToken; import + useassertWorkerToken. (This is the untrimmed one — the actual bug fix.)
- Replace the local
-
- Assert
claimedByinuserEnvironment.getRawEnvironmentForJobInternal(~68). Afterconst job = await ctx.db.get(jobId); if (!job) return null;add:
- Assert
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.
- Run
-
- Commit:
git add -A && git commit -m "fix(convex): unify trimmed worker-token compare and require a claimed job for env access"
- Commit:
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 sharedDockerinstance / 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 singletonnew Docker()using the default socket).
Steps:
-
- Write failing test
apps/agent-worker/tests/unit/terminal-resize.test.ts.terminal.tsimports./env,./worker,./user-container,ws— to unit-test only the pure parser,vi.mockthose heavy imports so importingterminal.tsdoesn't pull docker/ws:
- Write failing test
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.
- Run
-
- In
apps/agent-worker/src/terminal.ts, extract and export the parser (reuse the existingclampDimension):
- In
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.
- Run
-
- Add a dockerode singleton to
apps/agent-worker/src/runtime/docker.ts:
- Add a dockerode singleton to
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
bridgeinterminal.tsto use dockerode. Declare the holders and rewrite the message handler (replacing ~35-64):
- Rewrite
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
cleanuptostream.end()/destroy instead ofprocHolder.current?.kill(); keephandleHolder.current?.release()from Task 2. - in the resize branch of
ws.on('message'), after updatingcols/rows, add:void execHolder.current?.exec.resize({ h: rows, w: cols }).catch(() => {}); - remove the now-unused
spawn,shellQuote,script -qfc,sttylauncher, andChildProcessWithoutNullStreamsimports.
-
- Run
cd apps/agent-worker && bun run test:unit && bun run typecheck— expect PASS.
- Run
-
- 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.
- Manual verification (pure-infra; cannot unit-test dockerode against a real socket). With a running box: open the workspace terminal, run
-
- Commit:
git add -A && git commit -m "fix(worker): drive the terminal through a dockerode TTY exec so resize actually resizes the PTY"
- Commit:
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— runsfit()+sendResize()in the next animation frame, then again afterfontsReady, then again afterloadNerdFont, skipping whenisAborted().
Steps:
-
- Write failing test
apps/next/tests/unit/terminal-fit.test.ts(node env — pure logic, no DOM):
- Write failing test
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.
- Run
-
- Create
apps/next/src/components/agent-workspace/terminal-fit.ts:
- Create
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.
- Run
-
- Use it in
workspace-terminal.tsx. Replace the block afterterm.open(container); fit.fit(); termRef.current = term;and the standalonedocument.fonts.load(...)(~145-156). New code (keepsendResizedefined before this call — move its declaration up if needed):
- Use it in
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.
- Run
-
- Commit:
git add -A && git commit -m "fix(next): re-fit the workspace terminal on fonts.ready and Nerd-Font load"
- Commit:
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 withkey={jobId}and asserts a jobId change remounts it (mount counter increments, state resets). Match the render style ofapps/next/tests/component/render.test.tsx:
- Write failing component test
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 assertmounts).toBe(1)after rerender (wrong) to see it FAIL, then correct to2.
- Run
-
- 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.
- Run
-
- Commit:
git add -A && git commit -m "fix(next): remount AgentWorkspaceShell per job so thread switches don't cross state"
- Commit:
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: returnnullinstead of throw). - Modify
packages/backend/convex/threads.ts(get~209: returnnullinstead of throw). - Create
apps/next/src/app/(app)/error.tsxandapps/next/src/app/(app)/not-found.tsx. - Modify
apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsxand.../threads/[threadId]/page.tsxto handlenull. - Create
packages/backend/tests/unit/detail-null.test.ts.
Interfaces:
agentJobs.get: returnsDoc<'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 callsapi.threads.get/api.agentJobs.getwith A's ids and getsnull(not a throw). Also a genuinely nonexistent id returnsnull. Use theauthed()/createUser()helpers fromharness.test.ts. Assertawait expect(...).resolves.toBeNull().
- Write failing test
-
- Run
bun codegen:convex && cd packages/backend && bun run test:unit -- detail-null— expect FAIL (currently throws).
- Run
-
- Implement:
agentJobs.get(~707): replaceif (job?.ownerId !== ownerId) throw new ConvexError('Agent job not found.');withif (job?.ownerId !== ownerId) return null;.threads.get(~209): replaceif (thread?.ownerId !== ownerId) throw new ConvexError('Thread not found.');withif (thread?.ownerId !== ownerId) return null;. (Leave the other owner-mutating functions throwing; only the read-detailgets change.)
-
- Run
bun codegen:convex && cd packages/backend && bun run test:unit -- detail-null— expect PASS.
- Run
-
- Create
apps/next/src/app/(app)/error.tsx(client component; friendly, matches app shell — the(app)/layout.tsxwraps it inAppShell):
- Create
'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:
- Create
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
nullin the detail pages (distinguish loadingundefinedfrom missingnull): spoons/[spoonId]/agent/[jobId]/page.tsx: afterconst job = useQuery(api.agentJobs.get, { jobId });add, before theuseEffect:
- Handle
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:detailsis already guarded forundefined(~42). Add anullguard right after:if (details === null) { return (<main className='p-6 text-muted-foreground'>This thread was not found.</main>); }and destructure only after. (Notethreads.getnow returnsnull; TypeScript will require this guard beforeconst { thread, spoon, latestJob } = details;.)
-
- Run
bun codegen:convex && cd apps/next && bun run typecheckandcd packages/backend && bun run test:unit— expect PASS.
- Run
-
- Commit:
git add -A && git commit -m "feat(next,convex): return null for missing detail queries and add friendly app error/not-found states"
- Commit:
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/srcreturns no stale references (all replaced). - Post-deploy one-shot:
bunx convex run migrations:backfillAutoSyncDefaultsagainst the target deployment.