From daec4d6dfa45e33f716edc8ca3d4111cee16ed3c Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 15:55:11 -0400 Subject: [PATCH 01/88] fix(worker): make workspace cleanup layout-aware --- apps/agent-worker/src/user-container.ts | 2 + apps/agent-worker/src/worker.ts | 95 ++++++++++++++++--- .../tests/unit/cleanup-layout.test.ts | 73 ++++++++++++++ 3 files changed, 156 insertions(+), 14 deletions(-) create mode 100644 apps/agent-worker/tests/unit/cleanup-layout.test.ts diff --git a/apps/agent-worker/src/user-container.ts b/apps/agent-worker/src/user-container.ts index c4bb374..5f52764 100644 --- a/apps/agent-worker/src/user-container.ts +++ b/apps/agent-worker/src/user-container.ts @@ -38,3 +38,5 @@ export const releaseUserBox = (username: string) => { boxes.delete(username); }, env.boxIdleMs); }; + +export const runningBoxUsernames = (): Set => new Set(boxes.keys()); diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index d5025ad..831cf2a 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -41,7 +41,11 @@ import { stopWorkspaceContainer, streamExecInContainer, } from './runtime/docker'; -import { acquireUserBox, releaseUserBox } from './user-container'; +import { + acquireUserBox, + releaseUserBox, + runningBoxUsernames, +} from './user-container'; import { fetchUserEnvironment, materializeUserHome } from './user-environment'; type Claim = { @@ -1287,6 +1291,29 @@ const slugify = (value: string) => .replace(/^-+|-+$/g, '') .replace(/-{2,}/g, '-') || 'x'; +export const planWorkdirCleanup = (args: { + root: string; + rootEntries: { name: string; isDirectory: boolean }[]; + homesEntries: Record; + codeLeaves: Record; + activeWorkdirs: Set; + runningBoxUsernames: Set; +}): { removeDirs: string[] } => { + const removeDirs: string[] = []; + for (const entry of args.rootEntries) { + if (!entry.isDirectory || entry.name.startsWith('.')) continue; + if (entry.name === 'homes') continue; + const abs = path.resolve(args.root, entry.name); + if (args.activeWorkdirs.has(abs)) continue; + removeDirs.push(abs); + } + for (const [username, leaves] of Object.entries(args.codeLeaves)) { + if (args.runningBoxUsernames.has(username)) continue; + for (const leaf of leaves) removeDirs.push(leaf); + } + return { removeDirs }; +}; + const runClaim = async (claim: Claim) => { const jobId = claim.job._id; const secretValues = [ @@ -1885,7 +1912,8 @@ export const getWorkerHealth = async () => { workdir: workspace.workdir, agentTurnActive: Boolean(workspace.agentTurnActive), })); - const containerNames = await listWorkspaceContainerNames('spoon-agent-job-'); + const jobContainers = await listWorkspaceContainerNames('spoon-agent-job-'); + const boxContainers = await listWorkspaceContainerNames('spoon-box-'); return { ok: true, buildSha: env.buildSha, @@ -1904,7 +1932,8 @@ export const getWorkerHealth = async () => { jobTimeoutMs: env.jobTimeoutMs, activeWorkspaceCount: active.length, activeWorkspaces: active, - workspaceContainers: containerNames, + workspaceContainers: jobContainers, + boxContainers, }; }; @@ -1928,14 +1957,55 @@ export const cleanupOrphanedWorkspaces = async () => { removedContainers.push(containerName); } - const removedWorkdirs: string[] = []; const root = path.resolve(env.workdir); + const removedWorkdirs: string[] = []; try { - const entries = await readdir(root, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory() || entry.name.startsWith('.')) continue; - const target = path.resolve(root, entry.name); - if (activeWorkdirs.has(target)) continue; + const rootDirents = await readdir(root, { withFileTypes: true }); + const rootEntries = rootDirents.map((d) => ({ + name: d.name, + isDirectory: d.isDirectory(), + })); + const homesRoot = path.join(root, 'homes'); + const homesEntries: Record< + string, + { name: string; isDirectory: boolean }[] + > = {}; + const codeLeaves: Record = {}; + const homesDirents = await readdir(homesRoot, { + withFileTypes: true, + }).catch(() => []); + for (const userDir of homesDirents) { + if (!userDir.isDirectory()) continue; + const username = userDir.name; + const codeRoot = path.join(homesRoot, username, 'Code'); + homesEntries[username] = []; + const leaves: string[] = []; + const spoons = await readdir(codeRoot, { withFileTypes: true }).catch( + () => [], + ); + for (const spoonDir of spoons) { + if (!spoonDir.isDirectory()) continue; + const spoonRoot = path.join(codeRoot, spoonDir.name); + const branches = await readdir(spoonRoot, { + withFileTypes: true, + }).catch(() => []); + for (const branchDir of branches) { + if (branchDir.isDirectory()) { + leaves.push(path.join(spoonRoot, branchDir.name)); + } + } + } + codeLeaves[username] = leaves; + } + const { removeDirs } = planWorkdirCleanup({ + root, + rootEntries, + homesEntries, + codeLeaves, + activeWorkdirs, + runningBoxUsernames: runningBoxUsernames(), + }); + for (const target of removeDirs) { await rm(target, { recursive: true, force: true }); removedWorkdirs.push(target); } @@ -1946,11 +2016,8 @@ export const cleanupOrphanedWorkspaces = async () => { } } - return { - success: true, - removedContainers, - removedWorkdirs, - }; + const boxContainers = await listWorkspaceContainerNames('spoon-box-'); + return { success: true, removedContainers, removedWorkdirs, boxContainers }; }; export const startWorker = async () => { diff --git a/apps/agent-worker/tests/unit/cleanup-layout.test.ts b/apps/agent-worker/tests/unit/cleanup-layout.test.ts new file mode 100644 index 0000000..d30053c --- /dev/null +++ b/apps/agent-worker/tests/unit/cleanup-layout.test.ts @@ -0,0 +1,73 @@ +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', + ); + expect(plan.removeDirs).not.toContain('/work/homes/alice'); + expect(plan.removeDirs).not.toContain('/work/homes/alice/Code'); + }); +}); From 9e7a8722f387df940c4cb03bd76de92351a59cce Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 16:54:55 -0400 Subject: [PATCH 02/88] fix(worker): make user-box lifecycle concurrency-safe --- apps/agent-worker/src/index.ts | 2 + apps/agent-worker/src/runtime/docker.ts | 1 + apps/agent-worker/src/terminal.ts | 18 ++- apps/agent-worker/src/user-container.ts | 116 ++++++++++++++---- apps/agent-worker/src/worker.ts | 22 ++-- .../tests/unit/user-container.test.ts | 85 +++++++++++++ 6 files changed, 203 insertions(+), 41 deletions(-) create mode 100644 apps/agent-worker/tests/unit/user-container.test.ts diff --git a/apps/agent-worker/src/index.ts b/apps/agent-worker/src/index.ts index 2237756..14d5841 100644 --- a/apps/agent-worker/src/index.ts +++ b/apps/agent-worker/src/index.ts @@ -1,5 +1,6 @@ import { env } from './env'; import { startWorkerServer } from './server'; +import { reconcileExistingBoxes } from './user-container'; import { startWorker } from './worker'; // Dev-only watchdog: the dev runner chain (turbo → with-env → dotenv → bash) @@ -26,4 +27,5 @@ if (env.devWatchdog) { } startWorkerServer(); +await reconcileExistingBoxes(); await startWorker(); diff --git a/apps/agent-worker/src/runtime/docker.ts b/apps/agent-worker/src/runtime/docker.ts index 9d79176..f169928 100644 --- a/apps/agent-worker/src/runtime/docker.ts +++ b/apps/agent-worker/src/runtime/docker.ts @@ -362,6 +362,7 @@ export const ensureUserContainer = async (args: { [ 'run', '-d', + '--init', '--name', name, '--memory', diff --git a/apps/agent-worker/src/terminal.ts b/apps/agent-worker/src/terminal.ts index 1a8d3be..c99a5a3 100644 --- a/apps/agent-worker/src/terminal.ts +++ b/apps/agent-worker/src/terminal.ts @@ -4,9 +4,10 @@ import type { Server } from 'node:http'; import type { WebSocket } from 'ws'; import { WebSocketServer } from 'ws'; +import type { BoxHandle } from './user-container'; import { env } from './env'; import { verifyTerminalToken } from './terminal-token'; -import { acquireUserBox, releaseUserBox } from './user-container'; +import { acquireUserBox } from './user-container'; import { getTerminalWorkspace } from './worker'; const clampDimension = (value: unknown) => { @@ -63,7 +64,7 @@ const bridge = async (ws: WebSocket, jobId: string) => { else pendingInput.push(data); }); - let acquired = false; + const handleHolder: { current?: BoxHandle } = {}; let released = false; // Read through a function so TS doesn't narrow `released` to a constant — the // cleanup handler flips it asynchronously when the socket closes. @@ -72,7 +73,7 @@ const bridge = async (ws: WebSocket, jobId: string) => { if (released) return; released = true; procHolder.current?.kill(); - if (acquired) releaseUserBox(workspace.username); + handleHolder.current?.release(); }; ws.on('close', cleanup); ws.on('error', cleanup); @@ -81,12 +82,13 @@ const bridge = async (ws: WebSocket, jobId: string) => { // the terminal share the exact same container (Phase 2). let boxName: string; try { - boxName = await acquireUserBox({ + const handle = await acquireUserBox({ username: workspace.username, workdir: workspace.workdir, containerHome: workspace.containerHome, }); - acquired = true; + handleHolder.current = handle; + boxName = handle.boxName; } catch (error) { ws.close( 1011, @@ -95,7 +97,11 @@ const bridge = async (ws: WebSocket, jobId: string) => { return; } - if (isReleased()) return; // client disconnected during startup; cleanup ran + // Cleanup may have run before the awaited handle existed. + if (isReleased()) { + handleHolder.current.release(); + return; + } // Reattach a persistent tmux session across reconnects when available, else a // plain login shell. `stty` sizes the PTY to the client's viewport up front. diff --git a/apps/agent-worker/src/user-container.ts b/apps/agent-worker/src/user-container.ts index 5f52764..4f119a2 100644 --- a/apps/agent-worker/src/user-container.ts +++ b/apps/agent-worker/src/user-container.ts @@ -1,42 +1,110 @@ import { env } from './env'; import { ensureUserContainer, + listWorkspaceContainerNames, stopWorkspaceContainer, userContainerName, } from './runtime/docker'; -// Phase 2: one persistent "box" container per user that all of their threads -// (agent turns + terminal + commands) exec into. Reference-counted so it stays -// up while any thread workspace is active or a terminal is connected, and is -// reaped after an idle period once nothing holds it. -type Box = { refs: number; idleTimer?: NodeJS.Timeout }; -const boxes = new Map(); +// 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 }; -export const acquireUserBox = async (args: { - username: string; - workdir: string; - containerHome: string; -}): Promise => { - const name = await ensureUserContainer(args); - const box = boxes.get(args.username) ?? { refs: 0 }; - if (box.idleTimer) { - clearTimeout(box.idleTimer); - box.idleTimer = undefined; - } - box.refs += 1; - boxes.set(args.username, box); - return name; +type Box = { + name: string; + refs: Set; + idleTimer?: ReturnType; +}; +const boxes = new Map(); +const locks = new Map>(); + +// Per-username async mutex: chain each operation after the previous one. +const withLock = (username: string, fn: () => Promise): Promise => { + const previous = locks.get(username) ?? Promise.resolve(); + const next = previous.then(fn, fn); + locks.set( + username, + next.then( + () => undefined, + () => undefined, + ), + ); + return next; }; -export const releaseUserBox = (username: string) => { +const scheduleReapIfIdle = (username: string) => { const box = boxes.get(username); - if (!box) return; - box.refs = Math.max(0, box.refs - 1); - if (box.refs > 0) return; + 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; + released = true; + box.refs.delete(token); + scheduleReapIfIdle(username); + }, + }; +}; + +export const acquireUserBox = (args: { + username: string; + workdir: string; + containerHome: string; +}): Promise => + withLock(args.username, async () => { + let box = boxes.get(args.username); + let needsEnsure = false; + if (box?.idleTimer) { + clearTimeout(box.idleTimer); + box.idleTimer = undefined; + } + if (!box) { + box = { name: userContainerName(args.username), refs: new Set() }; + boxes.set(args.username, box); + needsEnsure = true; + } + // Register the ref before the slow Docker call so a failed acquire can + // release it without leaking registry state. + const handle = makeHandle(args.username, box); + try { + if (needsEnsure) box.name = await ensureUserContainer(args); + return handle; + } catch (error) { + handle.release(); + throw error; + } + }); + +// Adopt boxes left by a prior worker process so the idle reaper cleans them up. +export const reconcileExistingBoxes = async (): Promise => { + const names = await listWorkspaceContainerNames('spoon-box-'); + for (const name of names) { + const username = name.replace(/^spoon-box-/, ''); + if (boxes.has(username)) continue; + const box: Box = { name, refs: new Set() }; + boxes.set(username, box); + scheduleReapIfIdle(username); + } +}; + export const runningBoxUsernames = (): Set => new Set(boxes.keys()); + +export const _resetBoxRegistryForTests = () => { + for (const box of boxes.values()) { + if (box.idleTimer) clearTimeout(box.idleTimer); + } + boxes.clear(); + locks.clear(); +}; diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 831cf2a..55b794a 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -16,6 +16,7 @@ import { api } from '@spoon/backend/convex/_generated/api.js'; import type { NormalizedAgentEvent } from './agent-events'; import type { OpenCodeSession } from './opencode-session'; +import type { BoxHandle } from './user-container'; import { normalizeCodexJsonLine } from './agent-events'; import { prepareCodexWorkspaceFiles } from './codex-runtime'; import { env } from './env'; @@ -41,11 +42,7 @@ import { stopWorkspaceContainer, streamExecInContainer, } from './runtime/docker'; -import { - acquireUserBox, - releaseUserBox, - runningBoxUsernames, -} from './user-container'; +import { acquireUserBox, runningBoxUsernames } from './user-container'; import { fetchUserEnvironment, materializeUserHome } from './user-environment'; type Claim = { @@ -111,6 +108,7 @@ type ActiveWorkspace = { repoDir: string; // Phase 2: the per-user box container this thread execs into. boxName: string; + boxHandle: BoxHandle; githubToken: string; redact: (value: string) => string; runtimeMode?: 'opencode_server' | 'codex_exec' | 'legacy_cli'; @@ -1323,7 +1321,7 @@ const runClaim = async (claim: Claim) => { ...claim.secrets.map((secret) => secret.value), ].filter(Boolean); const redact = createRedactor(secretValues); - let acquiredBoxUser: string | undefined; + let acquiredBoxHandle: BoxHandle | undefined; try { if ((claim.job.runtime ?? 'opencode') !== 'opencode') { throw new Error('Legacy OpenAI direct jobs are no longer supported.'); @@ -1354,12 +1352,13 @@ const runClaim = async (claim: Claim) => { // Start (or reuse) the persistent per-user box that this thread — and the // terminal — exec into. It mounts the home, so the clone below is visible. - const boxName = await acquireUserBox({ + const boxHandle = await acquireUserBox({ username, workdir: homeDir, containerHome, }); - acquiredBoxUser = username; + acquiredBoxHandle = boxHandle; + const boxName = boxHandle.boxName; const repoDir = await cloneRepository({ workdir: checkoutParent, @@ -1381,6 +1380,7 @@ const runClaim = async (claim: Claim) => { containerRepo, repoDir, boxName, + boxHandle, githubToken, redact, }; @@ -1460,7 +1460,7 @@ const runClaim = async (claim: Claim) => { ).catch((stopError: unknown) => { console.error(stopError); }); - if (acquiredBoxUser) releaseUserBox(acquiredBoxUser); + acquiredBoxHandle?.release(); } }; @@ -1883,7 +1883,7 @@ export const openWorkspacePullRequest = async (jobId: string) => { activeWorkspaces.delete(jobId); // The persistent per-user home + ~/Code checkouts survive across sessions; // release the box (reaped once no other thread/terminal holds it). - releaseUserBox(workspace.username); + workspace.boxHandle.release(); return { pullRequestUrl: pullRequest.html_url, pullRequestNumber: pullRequest.number, @@ -1900,7 +1900,7 @@ export const stopWorkspace = async (jobId: string) => { activeWorkspaces.delete(jobId); // The persistent per-user home + ~/Code checkouts survive across sessions; // release the box (reaped once no other thread/terminal holds it). - releaseUserBox(workspace.username); + workspace.boxHandle.release(); return { success: true }; }; diff --git a/apps/agent-worker/tests/unit/user-container.test.ts b/apps/agent-worker/tests/unit/user-container.test.ts new file mode 100644 index 0000000..ed33aff --- /dev/null +++ b/apps/agent-worker/tests/unit/user-container.test.ts @@ -0,0 +1,85 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +vi.mock('../../src/env', () => ({ env: { boxIdleMs: 1000 } })); +vi.mock('../../src/runtime/docker', () => ({ + ensureUserContainer: vi.fn((args: { username: string }) => + Promise.resolve(`spoon-box-${args.username}`), + ), + stopWorkspaceContainer: vi.fn(() => Promise.resolve()), + userContainerName: (username: string) => `spoon-box-${username}`, + listWorkspaceContainerNames: vi.fn(() => Promise.resolve([])), +})); + +const load = async () => await import('../../src/user-container'); + +describe('box registry', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(async () => { + const module = await load(); + module._resetBoxRegistryForTests(); + vi.useRealTimers(); + vi.resetModules(); + vi.clearAllMocks(); + }); + + test('serializes concurrent acquire into a single docker run', async () => { + const module = await load(); + const docker = await import('../../src/runtime/docker'); + const [a, b] = await Promise.all([ + module.acquireUserBox({ + username: 'alice', + workdir: '/w', + containerHome: '/home/alice', + }), + module.acquireUserBox({ + username: 'alice', + workdir: '/w', + containerHome: '/home/alice', + }), + ]); + expect(docker.ensureUserContainer).toHaveBeenCalledTimes(1); + expect(a.boxName).toBe('spoon-box-alice'); + 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 module = await load(); + const docker = await import('../../src/runtime/docker'); + const a = await module.acquireUserBox({ + username: 'bob', + workdir: '/w', + containerHome: '/home/bob', + }); + const b = await module.acquireUserBox({ + username: 'bob', + workdir: '/w', + containerHome: '/home/bob', + }); + a.release(); + a.release(); + 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 module = await load(); + const docker = await import('../../src/runtime/docker'); + ( + docker.listWorkspaceContainerNames as unknown as ReturnType + ).mockResolvedValueOnce(['spoon-box-carol']); + await module.reconcileExistingBoxes(); + expect(module.runningBoxUsernames().has('carol')).toBe(true); + vi.advanceTimersByTime(2000); + expect(docker.stopWorkspaceContainer).toHaveBeenCalledWith( + 'spoon-box-carol', + ); + }); +}); From 0a4461eaba32d75d2d5dad40fffffbe0fcf99a10 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 16:53:49 -0400 Subject: [PATCH 03/88] fix(agent-jobs): prevent terminal status resurrection --- .superpowers/sdd/task-4-report.md | 66 +++++++++++ packages/backend/convex/agentJobs.ts | 17 ++- .../backend/tests/unit/update-status.test.ts | 109 ++++++++++++++++++ 3 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 .superpowers/sdd/task-4-report.md create mode 100644 packages/backend/tests/unit/update-status.test.ts diff --git a/.superpowers/sdd/task-4-report.md b/.superpowers/sdd/task-4-report.md new file mode 100644 index 0000000..812e31a --- /dev/null +++ b/.superpowers/sdd/task-4-report.md @@ -0,0 +1,66 @@ +# Task 4 Report: Terminal agent-job status guard + +## Status + +Implemented Phase 1 Task 4. `updateStatus` now treats terminal Convex job state as authoritative and ignores later worker status writes instead of resurrecting a completed or cancelled job. + +## Implementation + +- Added a shared terminal-status set and predicate for `failed`, `cancelled`, `timed_out`, and `draft_pr_opened`. +- Reused the predicate in the existing terminal-job helper. +- Added an early successful no-op in `updateStatus` after worker claim validation. +- Added coverage proving cancelled jobs remain cancelled and running jobs still accept `checks_running`. + +## Files + +- `packages/backend/convex/agentJobs.ts` +- `packages/backend/tests/unit/update-status.test.ts` + +## TDD Evidence + +### RED + +Command: + +```sh +cd packages/backend && bun run test:unit -- update-status +``` + +Result: exit 1. One of two tests failed. The cancelled-job case received `{ success: true }` instead of `{ success: true, ignored: true }`, while the non-terminal transition test passed. This demonstrated that `updateStatus` had no terminal-state guard. + +### GREEN + +Command: + +```sh +cd packages/backend && bun run test:unit -- update-status +``` + +Result: exit 0. One test file passed; both tests passed. + +## Verification + +Commands: + +```sh +cd packages/backend && bun run test:unit +cd packages/backend && bun run lint +cd packages/backend && bun run typecheck +``` + +Results: all exited 0. The backend unit suite passed 2 files and 16 tests; ESLint and `tsc --noEmit` emitted no diagnostics. + +## Codegen Environment + +`bun codegen:convex` was attempted and exited 1 before tests because neither `CONVEX_SELF_HOSTED_URL` plus `CONVEX_SELF_HOSTED_ADMIN_KEY` nor `CONVEX_DEPLOYMENT` is available. To run `convex-test` without modifying generated files, the worktree temporarily used the existing generated snapshot from the primary checkout via a symlink. The symlink was removed before staging and committing. + +## Self-review + +- Confirmed claim ownership is still validated before terminal writes are ignored. +- Confirmed all four specified terminal statuses share the guard. +- Confirmed non-terminal worker transitions retain their prior behavior. +- Confirmed no generated Convex files were edited or committed. + +## Concerns + +Full root-level validation remains unavailable without a configured Convex codegen environment. Focused and package-level backend checks pass against the existing generated snapshot. diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index fb0982f..0dbbda2 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -240,10 +240,18 @@ const isDeletableWorkspace = (job: Doc<'agentJobs'>) => ['failed', 'cancelled', 'timed_out'].includes(job.status) || ['stopped', 'expired', 'failed'].includes(job.workspaceStatus ?? ''); +const TERMINAL_STATUSES = new Set([ + 'failed', + 'cancelled', + 'timed_out', + 'draft_pr_opened', +]); + +const isTerminalStatus = (status: string) => TERMINAL_STATUSES.has(status); + const isTerminalJob = (job: Doc<'agentJobs'>) => - ['failed', 'cancelled', 'timed_out', 'draft_pr_opened'].includes( - job.status, - ) || ['stopped', 'expired', 'failed'].includes(job.workspaceStatus ?? ''); + isTerminalStatus(job.status) || + ['stopped', 'expired', 'failed'].includes(job.workspaceStatus ?? ''); const deleteWorkspaceRows = async (ctx: MutationCtx, job: Doc<'agentJobs'>) => { const messages = await ctx.db @@ -1120,6 +1128,9 @@ export const updateStatus = mutation({ if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); } + if (isTerminalStatus(job.status)) { + return { success: true, ignored: true as const }; + } const now = Date.now(); const patch: Partial> = { status: args.status, diff --git a/packages/backend/tests/unit/update-status.test.ts b/packages/backend/tests/unit/update-status.test.ts new file mode 100644 index 0000000..0758db1 --- /dev/null +++ b/packages/backend/tests/unit/update-status.test.ts @@ -0,0 +1,109 @@ +import { convexTest } from 'convex-test'; +import { beforeAll, describe, expect, test } from 'vitest'; + +import { api } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; + +const modules = import.meta.glob('../../convex/**/*.*s'); + +const spoonInput = { + name: 'Editor Spoon', + provider: 'gitea' as const, + upstreamOwner: 'upstream', + upstreamRepo: 'editor', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://git.example.com/upstream/editor', + forkOwner: 'team', + forkRepo: 'editor-spoon', + forkUrl: 'https://git.example.com/team/editor-spoon', + visibility: 'private' as const, + maintenanceMode: 'watch' as const, + syncCadence: 'daily' as const, + productionRefStrategy: 'default_branch' as const, +}; + +const seedJob = async ( + t: ReturnType, + status: 'cancelled' | 'running', +) => + 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', { + ...spoonInput, + ownerId, + status: 'active', + createdAt: now, + updatedAt: now, + }); + 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: 'editor-spoon', + forkUrl: 'https://git.example.com/team/editor-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'editor', + selectedSecretIds: [], + model: '', + reasoningEffort: 'medium', + claimedBy: 'worker-1', + createdAt: now, + updatedAt: now, + }); + }); + +beforeAll(() => { + process.env.SPOON_WORKER_TOKEN = 'test-worker-token'; +}); + +describe('updateStatus terminal guard', () => { + test('ignores writes to a cancelled job', async () => { + const t = convexTest(schema, modules); + const jobId = await seedJob(t, 'cancelled'); + + const result = await t.mutation(api.agentJobs.updateStatus, { + workerToken: 'test-worker-token', + workerId: 'worker-1', + jobId, + status: 'running', + }); + + expect(result).toEqual({ success: true, ignored: true }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + expect(job?.status).toBe('cancelled'); + }); + + test('accepts writes to a running job', async () => { + const t = convexTest(schema, modules); + const jobId = await seedJob(t, 'running'); + + const result = await t.mutation(api.agentJobs.updateStatus, { + workerToken: 'test-worker-token', + workerId: 'worker-1', + jobId, + status: 'checks_running', + }); + + expect(result).toEqual({ success: true }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + expect(job?.status).toBe('checks_running'); + }); +}); From 490c05329bcc6132319d04ed45644620f1ec3953 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 16:56:07 -0400 Subject: [PATCH 04/88] chore: remove implementation scratch report --- .superpowers/sdd/task-4-report.md | 66 ------------------------------- 1 file changed, 66 deletions(-) delete mode 100644 .superpowers/sdd/task-4-report.md diff --git a/.superpowers/sdd/task-4-report.md b/.superpowers/sdd/task-4-report.md deleted file mode 100644 index 812e31a..0000000 --- a/.superpowers/sdd/task-4-report.md +++ /dev/null @@ -1,66 +0,0 @@ -# Task 4 Report: Terminal agent-job status guard - -## Status - -Implemented Phase 1 Task 4. `updateStatus` now treats terminal Convex job state as authoritative and ignores later worker status writes instead of resurrecting a completed or cancelled job. - -## Implementation - -- Added a shared terminal-status set and predicate for `failed`, `cancelled`, `timed_out`, and `draft_pr_opened`. -- Reused the predicate in the existing terminal-job helper. -- Added an early successful no-op in `updateStatus` after worker claim validation. -- Added coverage proving cancelled jobs remain cancelled and running jobs still accept `checks_running`. - -## Files - -- `packages/backend/convex/agentJobs.ts` -- `packages/backend/tests/unit/update-status.test.ts` - -## TDD Evidence - -### RED - -Command: - -```sh -cd packages/backend && bun run test:unit -- update-status -``` - -Result: exit 1. One of two tests failed. The cancelled-job case received `{ success: true }` instead of `{ success: true, ignored: true }`, while the non-terminal transition test passed. This demonstrated that `updateStatus` had no terminal-state guard. - -### GREEN - -Command: - -```sh -cd packages/backend && bun run test:unit -- update-status -``` - -Result: exit 0. One test file passed; both tests passed. - -## Verification - -Commands: - -```sh -cd packages/backend && bun run test:unit -cd packages/backend && bun run lint -cd packages/backend && bun run typecheck -``` - -Results: all exited 0. The backend unit suite passed 2 files and 16 tests; ESLint and `tsc --noEmit` emitted no diagnostics. - -## Codegen Environment - -`bun codegen:convex` was attempted and exited 1 before tests because neither `CONVEX_SELF_HOSTED_URL` plus `CONVEX_SELF_HOSTED_ADMIN_KEY` nor `CONVEX_DEPLOYMENT` is available. To run `convex-test` without modifying generated files, the worktree temporarily used the existing generated snapshot from the primary checkout via a symlink. The symlink was removed before staging and committing. - -## Self-review - -- Confirmed claim ownership is still validated before terminal writes are ignored. -- Confirmed all four specified terminal statuses share the guard. -- Confirmed non-terminal worker transitions retain their prior behavior. -- Confirmed no generated Convex files were edited or committed. - -## Concerns - -Full root-level validation remains unavailable without a configured Convex codegen environment. Focused and package-level backend checks pass against the existing generated snapshot. From 284f7ea52b043f9ecb372d3d6bdbe25ec8a9add4 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:00:22 -0400 Subject: [PATCH 05/88] fix(worker): serialize box failure and idle reaping --- apps/agent-worker/src/user-container.ts | 14 +++- .../tests/unit/user-container.test.ts | 79 +++++++++++++++++-- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/apps/agent-worker/src/user-container.ts b/apps/agent-worker/src/user-container.ts index 4f119a2..3b6890f 100644 --- a/apps/agent-worker/src/user-container.ts +++ b/apps/agent-worker/src/user-container.ts @@ -38,8 +38,14 @@ const scheduleReapIfIdle = (username: string) => { if (!box || box.refs.size > 0) return; if (box.idleTimer) clearTimeout(box.idleTimer); box.idleTimer = setTimeout(() => { - void stopWorkspaceContainer(userContainerName(username)); - boxes.delete(username); + void withLock(username, async () => { + const current = boxes.get(username); + if (current !== box || current.refs.size > 0) return; + await stopWorkspaceContainer(current.name); + if (boxes.get(username) === current && current.refs.size === 0) { + boxes.delete(username); + } + }); }, env.boxIdleMs); }; @@ -83,6 +89,10 @@ export const acquireUserBox = (args: { return handle; } catch (error) { handle.release(); + if (boxes.get(args.username) === box && box.refs.size === 0) { + if (box.idleTimer) clearTimeout(box.idleTimer); + boxes.delete(args.username); + } throw error; } }); diff --git a/apps/agent-worker/tests/unit/user-container.test.ts b/apps/agent-worker/tests/unit/user-container.test.ts index ed33aff..08b36ad 100644 --- a/apps/agent-worker/tests/unit/user-container.test.ts +++ b/apps/agent-worker/tests/unit/user-container.test.ts @@ -40,10 +40,10 @@ describe('box registry', () => { expect(docker.ensureUserContainer).toHaveBeenCalledTimes(1); expect(a.boxName).toBe('spoon-box-alice'); a.release(); - vi.advanceTimersByTime(2000); + await vi.advanceTimersByTimeAsync(2000); expect(docker.stopWorkspaceContainer).not.toHaveBeenCalled(); b.release(); - vi.advanceTimersByTime(2000); + await vi.advanceTimersByTimeAsync(2000); expect(docker.stopWorkspaceContainer).toHaveBeenCalledTimes(1); }); @@ -62,10 +62,10 @@ describe('box registry', () => { }); a.release(); a.release(); - vi.advanceTimersByTime(2000); + await vi.advanceTimersByTimeAsync(2000); expect(docker.stopWorkspaceContainer).not.toHaveBeenCalled(); b.release(); - vi.advanceTimersByTime(2000); + await vi.advanceTimersByTimeAsync(2000); expect(docker.stopWorkspaceContainer).toHaveBeenCalledTimes(1); }); @@ -77,9 +77,78 @@ describe('box registry', () => { ).mockResolvedValueOnce(['spoon-box-carol']); await module.reconcileExistingBoxes(); expect(module.runningBoxUsernames().has('carol')).toBe(true); - vi.advanceTimersByTime(2000); + await vi.advanceTimersByTimeAsync(2000); expect(docker.stopWorkspaceContainer).toHaveBeenCalledWith( 'spoon-box-carol', ); }); + + test('retries ensure after the first acquire fails', async () => { + const module = await load(); + const docker = await import('../../src/runtime/docker'); + const ensure = vi.mocked(docker.ensureUserContainer); + ensure + .mockRejectedValueOnce(new Error('docker failed')) + .mockResolvedValueOnce('spoon-box-dana'); + + await expect( + module.acquireUserBox({ + username: 'dana', + workdir: '/w', + containerHome: '/home/dana', + }), + ).rejects.toThrow('docker failed'); + + const handle = await module.acquireUserBox({ + username: 'dana', + workdir: '/w', + containerHome: '/home/dana', + }); + expect(ensure).toHaveBeenCalledTimes(2); + expect(handle.boxName).toBe('spoon-box-dana'); + }); + + test('finishes an old idle reap before a new acquire can ensure the box', async () => { + const module = await load(); + const docker = await import('../../src/runtime/docker'); + const ensure = vi.mocked(docker.ensureUserContainer); + const stop = vi.mocked(docker.stopWorkspaceContainer); + const events: string[] = []; + let finishStop!: () => void; + const stopDone = new Promise((resolve) => { + finishStop = resolve; + }); + ensure.mockReset().mockImplementation((args: { username: string }) => { + events.push('ensure'); + return Promise.resolve(`spoon-box-${args.username}`); + }); + stop.mockReset().mockImplementationOnce(() => { + events.push('stop:start'); + return stopDone.then(() => { + events.push('stop:end'); + }); + }); + + const first = await module.acquireUserBox({ + username: 'erin', + workdir: '/w', + containerHome: '/home/erin', + }); + first.release(); + await vi.advanceTimersByTimeAsync(1000); + + const secondPromise = module.acquireUserBox({ + username: 'erin', + workdir: '/w', + containerHome: '/home/erin', + }); + await Promise.resolve(); + await Promise.resolve(); + expect(events).toEqual(['ensure', 'stop:start']); + + finishStop(); + const second = await secondPromise; + expect(second.boxName).toBe('spoon-box-erin'); + expect(events).toEqual(['ensure', 'stop:start', 'stop:end', 'ensure']); + }); }); From 0b5a2c0411ed6bd982b5f87f9e6c1060a1d64a8a Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:04:41 -0400 Subject: [PATCH 06/88] fix(worker): ensure reconciled boxes before acquire --- apps/agent-worker/src/user-container.ts | 16 +++++++++---- .../tests/unit/user-container.test.ts | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/apps/agent-worker/src/user-container.ts b/apps/agent-worker/src/user-container.ts index 3b6890f..97c5763 100644 --- a/apps/agent-worker/src/user-container.ts +++ b/apps/agent-worker/src/user-container.ts @@ -13,6 +13,7 @@ export type BoxHandle = { boxName: string; release: () => void }; type Box = { name: string; + initialized: boolean; refs: Set; idleTimer?: ReturnType; }; @@ -71,21 +72,26 @@ export const acquireUserBox = (args: { }): Promise => withLock(args.username, async () => { let box = boxes.get(args.username); - let needsEnsure = false; if (box?.idleTimer) { clearTimeout(box.idleTimer); box.idleTimer = undefined; } if (!box) { - box = { name: userContainerName(args.username), refs: new Set() }; + box = { + name: userContainerName(args.username), + initialized: false, + refs: new Set(), + }; boxes.set(args.username, box); - needsEnsure = true; } // Register the ref before the slow Docker call so a failed acquire can // release it without leaking registry state. const handle = makeHandle(args.username, box); try { - if (needsEnsure) box.name = await ensureUserContainer(args); + if (!box.initialized) { + box.name = await ensureUserContainer(args); + box.initialized = true; + } return handle; } catch (error) { handle.release(); @@ -103,7 +109,7 @@ export const reconcileExistingBoxes = async (): Promise => { for (const name of names) { const username = name.replace(/^spoon-box-/, ''); if (boxes.has(username)) continue; - const box: Box = { name, refs: new Set() }; + const box: Box = { name, initialized: false, refs: new Set() }; boxes.set(username, box); scheduleReapIfIdle(username); } diff --git a/apps/agent-worker/tests/unit/user-container.test.ts b/apps/agent-worker/tests/unit/user-container.test.ts index 08b36ad..7f6a7a3 100644 --- a/apps/agent-worker/tests/unit/user-container.test.ts +++ b/apps/agent-worker/tests/unit/user-container.test.ts @@ -83,6 +83,30 @@ describe('box registry', () => { ); }); + test('ensures an adopted box once before returning acquire handles', async () => { + const module = await load(); + const docker = await import('../../src/runtime/docker'); + vi.mocked(docker.listWorkspaceContainerNames).mockResolvedValueOnce([ + 'spoon-box-casey', + ]); + await module.reconcileExistingBoxes(); + + const first = await module.acquireUserBox({ + username: 'casey', + workdir: '/w', + containerHome: '/home/casey', + }); + const second = await module.acquireUserBox({ + username: 'casey', + workdir: '/w', + containerHome: '/home/casey', + }); + + expect(docker.ensureUserContainer).toHaveBeenCalledTimes(1); + expect(first.boxName).toBe('spoon-box-casey'); + expect(second.boxName).toBe('spoon-box-casey'); + }); + test('retries ensure after the first acquire fails', async () => { const module = await load(); const docker = await import('../../src/runtime/docker'); From 75d06394cef5f28cb33c912fa8873b98f1d4047c Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 16:53:18 -0400 Subject: [PATCH 07/88] fix(terminal): refit after fonts load --- .superpowers/sdd/task-11-report.md | 24 ++++++++++ .../agent-workspace/terminal-fit.ts | 21 +++++++++ .../agent-workspace/workspace-terminal.tsx | 32 +++++++++---- apps/next/tests/unit/terminal-fit.test.ts | 47 +++++++++++++++++++ 4 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 .superpowers/sdd/task-11-report.md create mode 100644 apps/next/src/components/agent-workspace/terminal-fit.ts create mode 100644 apps/next/tests/unit/terminal-fit.test.ts diff --git a/.superpowers/sdd/task-11-report.md b/.superpowers/sdd/task-11-report.md new file mode 100644 index 0000000..57b55d3 --- /dev/null +++ b/.superpowers/sdd/task-11-report.md @@ -0,0 +1,24 @@ +# Task 11 Report + +## Status + +Implemented terminal fit scheduling for the first animation frame, `document.fonts.ready`, and Nerd Font loading. Each successful, non-aborted fit also sends the current terminal dimensions to the worker. Nerd Font completion still refreshes the terminal glyph rows. + +## TDD evidence + +- RED: `cd apps/next && bun run test:unit -- terminal-fit` failed with `Cannot find package '@/components/agent-workspace/terminal-fit'` before the helper existed. +- GREEN: the same focused command passed 1 file and 2 tests after adding `scheduleTerminalFits`. + +## Verification + +- `cd apps/next && bun run test:unit -- terminal-fit`: PASS, 1 file / 2 tests. +- `cd apps/next && bun run test:unit`: PASS, 4 files / 10 tests. +- `cd apps/next && bun run typecheck`: PASS. +- `cd apps/next && bun run format`: PASS. +- `cd apps/next && bun run lint`: PASS, with the existing stale `baseline-browser-mapping` data warning. +- `cd apps/next && bun run test:component`: BLOCKED before test collection. This isolated worktree's root `node_modules` symlink resolves to `/home/gib/Documents/Code/Spoon/node_modules`, causing Vitest to look for `/home/gib/Documents/Code/Spoon/tools/vitest/setup-jsdom.ts` instead of the worktree setup file. + +## Concerns + +- No implementation concerns found. +- The component-suite worktree dependency-resolution issue remains external to this task. diff --git a/apps/next/src/components/agent-workspace/terminal-fit.ts b/apps/next/src/components/agent-workspace/terminal-fit.ts new file mode 100644 index 0000000..da1d777 --- /dev/null +++ b/apps/next/src/components/agent-workspace/terminal-fit.ts @@ -0,0 +1,21 @@ +export const scheduleTerminalFits = (deps: { + fit: () => void; + sendResize: () => void; + fontsReady: Promise; + loadNerdFont: () => Promise; + raf: (cb: () => void) => void; + isAborted: () => boolean; +}): void => { + const refit = () => { + if (deps.isAborted()) return; + deps.fit(); + deps.sendResize(); + }; + + deps.raf(refit); + void deps.fontsReady.then(refit).catch(() => undefined); + void deps + .loadNerdFont() + .then(refit) + .catch(() => undefined); +}; diff --git a/apps/next/src/components/agent-workspace/workspace-terminal.tsx b/apps/next/src/components/agent-workspace/workspace-terminal.tsx index aee45bf..59d9923 100644 --- a/apps/next/src/components/agent-workspace/workspace-terminal.tsx +++ b/apps/next/src/components/agent-workspace/workspace-terminal.tsx @@ -6,6 +6,8 @@ import { useTheme } from 'next-themes'; import { Button } from '@spoon/ui'; +import { scheduleTerminalFits } from './terminal-fit'; + import '@xterm/xterm/css/xterm.css'; const TERMINAL_FONT = @@ -143,18 +145,8 @@ export const WorkspaceTerminal = ({ term.loadAddon(fit); term.loadAddon(new WebLinksAddon()); term.open(container); - fit.fit(); termRef.current = term; - // Pull in the Nerd Font icon glyphs (loaded lazily by unicode-range) and - // repaint once ready so powerline/oh-my-posh/eza icons render. - void document.fonts - .load("16px 'Symbols Nerd Font Mono'", '\ue0b0') - .then(() => { - if (!isAborted()) term.refresh(0, term.rows - 1); - }) - .catch(() => undefined); - const sendResize = () => { if (ws?.readyState !== WebSocket.OPEN) return; ws.send( @@ -162,6 +154,26 @@ export const WorkspaceTerminal = ({ ); }; + 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, + }); + ws = new WebSocket(url); ws.binaryType = 'arraybuffer'; ws.onopen = () => { diff --git a/apps/next/tests/unit/terminal-fit.test.ts b/apps/next/tests/unit/terminal-fit.test.ts new file mode 100644 index 0000000..5abc592 --- /dev/null +++ b/apps/next/tests/unit/terminal-fit.test.ts @@ -0,0 +1,47 @@ +import { scheduleTerminalFits } from '@/components/agent-workspace/terminal-fit'; +import { describe, expect, test, vi } from 'vitest'; + +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); + 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(); + }); +}); From feb37237536906434379344106a9ae733743c2d9 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 16:56:15 -0400 Subject: [PATCH 08/88] fix(terminal): preserve animation-frame first fit --- .superpowers/sdd/task-11-report.md | 9 +++++++ .../agent-workspace/terminal-fit.ts | 15 ++++++++--- apps/next/tests/unit/terminal-fit.test.ts | 27 +++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/.superpowers/sdd/task-11-report.md b/.superpowers/sdd/task-11-report.md index 57b55d3..e8cc3bb 100644 --- a/.superpowers/sdd/task-11-report.md +++ b/.superpowers/sdd/task-11-report.md @@ -22,3 +22,12 @@ Implemented terminal fit scheduling for the first animation frame, `document.fon - No implementation concerns found. - The component-suite worktree dependency-resolution issue remains external to this task. + +## Reviewer follow-up: preserve the animation-frame first fit + +- RED: `cd apps/next && bun run test:unit -- terminal-fit` exited 1 with 1 failed / 2 passed. The new `waits for the first animation frame before font-triggered fits` regression expected zero calls before rAF, but `fit` had already been called 2 times after flushing the resolved font promises. +- GREEN: after gating both font-triggered refits on the initial rAF fit promise, `cd apps/next && bun run test:unit -- terminal-fit` exited 0 with 1 file / 3 tests passed. +- Final focused verification: `cd apps/next && bun run test:unit -- terminal-fit` exited 0 with 1 file / 3 tests passed. +- Final full unit verification: `cd apps/next && bun run test:unit` exited 0 with 4 files / 11 tests passed. +- Final type verification: `cd apps/next && bun run typecheck` exited 0. +- Final formatting verification: `cd apps/next && bun run format` exited 0; all matched files use Prettier formatting. diff --git a/apps/next/src/components/agent-workspace/terminal-fit.ts b/apps/next/src/components/agent-workspace/terminal-fit.ts index da1d777..9cbeec3 100644 --- a/apps/next/src/components/agent-workspace/terminal-fit.ts +++ b/apps/next/src/components/agent-workspace/terminal-fit.ts @@ -12,10 +12,17 @@ export const scheduleTerminalFits = (deps: { deps.sendResize(); }; - deps.raf(refit); - void deps.fontsReady.then(refit).catch(() => undefined); - void deps - .loadNerdFont() + const initialFit = new Promise((resolve) => { + deps.raf(() => { + refit(); + resolve(); + }); + }); + + void Promise.all([deps.fontsReady, initialFit]) + .then(refit) + .catch(() => undefined); + void Promise.all([deps.loadNerdFont(), initialFit]) .then(refit) .catch(() => undefined); }; diff --git a/apps/next/tests/unit/terminal-fit.test.ts b/apps/next/tests/unit/terminal-fit.test.ts index 5abc592..cc6fffa 100644 --- a/apps/next/tests/unit/terminal-fit.test.ts +++ b/apps/next/tests/unit/terminal-fit.test.ts @@ -7,6 +7,33 @@ const flush = async () => { }; describe('scheduleTerminalFits', () => { + test('waits for the first animation frame before font-triggered fits', 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, + }); + await flush(); + + expect(fit).not.toHaveBeenCalled(); + expect(sendResize).not.toHaveBeenCalled(); + + rafCb?.(); + await flush(); + + expect(fit).toHaveBeenCalledTimes(3); + expect(sendResize).toHaveBeenCalledTimes(3); + }); + test('fits in rAF and again after fonts.ready and nerd-font load', async () => { const fit = vi.fn(); const sendResize = vi.fn(); From 020ffbfb23a4ad287cfeb1321b143514a9903045 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:02:12 -0400 Subject: [PATCH 09/88] fix(worker-auth): unify token checks and bind job environment --- packages/backend/convex/agentJobs.ts | 39 +++---- packages/backend/convex/agentJobsNode.ts | 9 +- packages/backend/convex/userDotfilesNode.ts | 9 +- packages/backend/convex/userEnvironment.ts | 1 + packages/backend/convex/workerAuth.ts | 11 ++ .../backend/tests/unit/worker-auth.test.ts | 106 ++++++++++++++++++ 6 files changed, 138 insertions(+), 37 deletions(-) create mode 100644 packages/backend/convex/workerAuth.ts create mode 100644 packages/backend/tests/unit/worker-auth.test.ts diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 0dbbda2..53d0bb8 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -4,6 +4,7 @@ import type { Doc, Id } from './_generated/dataModel'; import type { MutationCtx } from './_generated/server'; import { internalMutation, mutation, query } from './_generated/server'; import { getOwnedSpoon, getRequiredUserId, optionalText } from './model'; +import { assertWorkerToken } from './workerAuth'; const jobStatus = v.union( v.literal('queued'), @@ -152,14 +153,6 @@ const defaultAgentSettings = { allowUserFileEditing: true, }; -const getWorkerToken = () => process.env.SPOON_WORKER_TOKEN?.trim(); - -const requireWorkerToken = (workerToken: string) => { - const expected = getWorkerToken(); - if (!expected) throw new ConvexError('SPOON_WORKER_TOKEN is not configured.'); - if (workerToken !== expected) throw new ConvexError('Invalid worker token.'); -}; - const mergeMessageMetadata = ( metadata: string | undefined, patch: Record, @@ -1123,7 +1116,7 @@ export const updateStatus = mutation({ summary: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1193,7 +1186,7 @@ export const markWorkspaceActive = mutation({ workspaceExpiresAt: v.optional(v.number()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1223,7 +1216,7 @@ export const setRuntimeSession = mutation({ containerId: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1247,7 +1240,7 @@ export const setCodexSessionId = mutation({ codexSessionId: v.string(), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1275,7 +1268,7 @@ export const createInteractionRequest = mutation({ metadata: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1327,7 +1320,7 @@ export const patchInteractionRequest = mutation({ metadata: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const interaction = await ctx.db.get(args.interactionId); if (!interaction) throw new ConvexError('Interaction request not found.'); const job = await ctx.db.get(interaction.jobId); @@ -1352,7 +1345,7 @@ export const markWorkspaceStopped = mutation({ workspaceStatus: v.optional(workspaceStatus), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1373,7 +1366,7 @@ export const heartbeatWorkspace = mutation({ jobId: v.id('agentJobs'), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1398,7 +1391,7 @@ export const completeWithDraftPr = mutation({ summary: v.string(), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1445,7 +1438,7 @@ export const applyMaintenanceDecision = mutation({ requiresUserApproval: v.boolean(), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1526,7 +1519,7 @@ export const appendEvent = mutation({ metadata: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1555,7 +1548,7 @@ export const appendMessage = mutation({ metadata: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1601,7 +1594,7 @@ export const updateMessage = mutation({ metadata: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const message = await ctx.db.get(args.messageId); if (!message) throw new ConvexError('Agent message not found.'); const job = await ctx.db.get(message.jobId); @@ -1656,7 +1649,7 @@ export const recordWorkspaceChange = mutation({ diff: v.optional(v.string()), }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); @@ -1685,7 +1678,7 @@ export const addArtifact = mutation({ contentType: artifactContentType, }, handler: async (ctx, args) => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const job = await ctx.db.get(args.jobId); if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); diff --git a/packages/backend/convex/agentJobsNode.ts b/packages/backend/convex/agentJobsNode.ts index a22aa0b..eef3810 100644 --- a/packages/backend/convex/agentJobsNode.ts +++ b/packages/backend/convex/agentJobsNode.ts @@ -6,6 +6,7 @@ import type { Doc } from './_generated/dataModel'; import { internal } from './_generated/api'; import { action } from './_generated/server'; import { decryptSecret } from './secretCrypto'; +import { assertWorkerToken } from './workerAuth'; type ClaimedJob = { job: Doc<'agentJobs'>; @@ -41,19 +42,13 @@ type WorkerClaim = { secrets: { name: string; value: string }[]; }; -const requireWorkerToken = (workerToken: string) => { - const expected = process.env.SPOON_WORKER_TOKEN?.trim(); - if (!expected) throw new ConvexError('SPOON_WORKER_TOKEN is not configured.'); - if (workerToken !== expected) throw new ConvexError('Invalid worker token.'); -}; - export const claimNextForWorker = action({ args: { workerId: v.string(), workerToken: v.string(), }, handler: async (ctx, args): Promise => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const claimed = (await ctx.runMutation( internal.agentJobs.claimNextInternal, { diff --git a/packages/backend/convex/userDotfilesNode.ts b/packages/backend/convex/userDotfilesNode.ts index d87f218..9c201f2 100644 --- a/packages/backend/convex/userDotfilesNode.ts +++ b/packages/backend/convex/userDotfilesNode.ts @@ -9,6 +9,7 @@ import { internal } from './_generated/api'; import { action } from './_generated/server'; import { normalizeDotfilePath } from './model'; import { decryptSecret, encryptSecret } from './secretCrypto'; +import { assertWorkerToken } from './workerAuth'; const MAX_FILE_BYTES = 512 * 1024; // 512 KB per dotfile @@ -18,12 +19,6 @@ const getRequiredUserId = async (ctx: ActionCtx): Promise> => { return userId; }; -const requireWorkerToken = (workerToken: string) => { - const expected = process.env.SPOON_WORKER_TOKEN; - if (!expected) throw new ConvexError('Worker token is not configured.'); - if (workerToken !== expected) throw new ConvexError('Invalid worker token.'); -}; - const putOne = async ( ctx: ActionCtx, ownerId: Id<'users'>, @@ -114,7 +109,7 @@ type WorkerEnvironment = { export const getEnvironmentForJob = action({ args: { workerToken: v.string(), jobId: v.id('agentJobs') }, handler: async (ctx, args): Promise => { - requireWorkerToken(args.workerToken); + assertWorkerToken(args.workerToken); const raw = await ctx.runQuery( internal.userEnvironment.getRawEnvironmentForJobInternal, { jobId: args.jobId }, diff --git a/packages/backend/convex/userEnvironment.ts b/packages/backend/convex/userEnvironment.ts index 3c06349..fc30d32 100644 --- a/packages/backend/convex/userEnvironment.ts +++ b/packages/backend/convex/userEnvironment.ts @@ -71,6 +71,7 @@ export const getRawEnvironmentForJobInternal = internalQuery({ handler: async (ctx, { jobId }) => { const job = await ctx.db.get(jobId); if (!job) return null; + if (!job.claimedBy) return null; // only a claimed job's env is exposed const ownerId = job.ownerId; const [user, settings, files] = await Promise.all([ ctx.db.get(ownerId), diff --git a/packages/backend/convex/workerAuth.ts b/packages/backend/convex/workerAuth.ts new file mode 100644 index 0000000..a194b68 --- /dev/null +++ b/packages/backend/convex/workerAuth.ts @@ -0,0 +1,11 @@ +import { ConvexError } from 'convex/values'; + +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.'); + } +}; diff --git a/packages/backend/tests/unit/worker-auth.test.ts b/packages/backend/tests/unit/worker-auth.test.ts new file mode 100644 index 0000000..fd1880c --- /dev/null +++ b/packages/backend/tests/unit/worker-auth.test.ts @@ -0,0 +1,106 @@ +import { convexTest } from 'convex-test'; +import { ConvexError } from 'convex/values'; +import { afterEach, describe, expect, test } from 'vitest'; + +import { internal } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; +import { assertWorkerToken } from '../../convex/workerAuth'; + +const modules = import.meta.glob('../../convex/**/*.*s'); + +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); + }); +}); + +describe('getRawEnvironmentForJobInternal', () => { + test('only exposes environment for a claimed job', async () => { + const t = convexTest(schema, modules); + const jobId = await t.mutation(async (ctx) => { + const now = Date.now(); + const ownerId = await ctx.db.insert('users', { + email: 'worker-auth@example.com', + name: 'Worker Auth', + }); + const spoonId = await ctx.db.insert('spoons', { + ownerId, + name: 'Worker Auth Spoon', + provider: 'gitea', + upstreamOwner: 'upstream', + upstreamRepo: 'worker-auth', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://git.example.com/upstream/worker-auth', + forkOwner: 'team', + forkRepo: 'worker-auth-spoon', + forkUrl: 'https://git.example.com/team/worker-auth-spoon', + visibility: 'private', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + const requestId = await ctx.db.insert('agentRequests', { + spoonId, + ownerId, + prompt: 'Test worker authorization', + status: 'queued', + createdAt: now, + updatedAt: now, + }); + return await ctx.db.insert('agentJobs', { + spoonId, + ownerId, + agentRequestId: requestId, + status: 'queued', + prompt: 'Test worker authorization', + runtime: 'opencode', + baseBranch: 'main', + workBranch: 'spoon/worker-auth', + forkOwner: 'team', + forkRepo: 'worker-auth-spoon', + forkUrl: 'https://git.example.com/team/worker-auth-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'worker-auth', + selectedSecretIds: [], + model: '', + reasoningEffort: 'medium', + createdAt: now, + updatedAt: now, + }); + }); + + expect( + await t.query(internal.userEnvironment.getRawEnvironmentForJobInternal, { + jobId, + }), + ).toBeNull(); + + await t.mutation(async (ctx) => { + await ctx.db.patch(jobId, { claimedBy: 'w1' }); + }); + + expect( + await t.query(internal.userEnvironment.getRawEnvironmentForJobInternal, { + jobId, + }), + ).not.toBeNull(); + }); +}); From d4a38560834dc0532f91f437ad753135df18c505 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:02:26 -0400 Subject: [PATCH 10/88] fix(workspace): key shell by job --- .../spoons/[spoonId]/agent/[jobId]/page.tsx | 2 +- .../src/app/(app)/threads/[threadId]/page.tsx | 2 +- .../agent-workspace-shell-key.test.tsx | 28 +++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 apps/next/tests/component/agent-workspace-shell-key.test.tsx diff --git a/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx b/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx index 06a8883..93fdbfe 100644 --- a/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx +++ b/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx @@ -37,7 +37,7 @@ const AgentWorkspacePage = () => { Back to Spoon - + ); }; diff --git a/apps/next/src/app/(app)/threads/[threadId]/page.tsx b/apps/next/src/app/(app)/threads/[threadId]/page.tsx index f471c58..add0be8 100644 --- a/apps/next/src/app/(app)/threads/[threadId]/page.tsx +++ b/apps/next/src/app/(app)/threads/[threadId]/page.tsx @@ -53,7 +53,7 @@ const ThreadDetailPage = () => { Back to Spoon - + ); } diff --git a/apps/next/tests/component/agent-workspace-shell-key.test.tsx b/apps/next/tests/component/agent-workspace-shell-key.test.tsx new file mode 100644 index 0000000..b6e1178 --- /dev/null +++ b/apps/next/tests/component/agent-workspace-shell-key.test.tsx @@ -0,0 +1,28 @@ +import { useEffect } from 'react'; +import { render } from '@testing-library/react'; +import { describe, expect, test } from 'vitest'; + +let mounts = 0; + +const Child = ({ jobId }: { jobId: string }) => { + useEffect(() => { + mounts += 1; + }, []); + + return {jobId}; +}; + +const Shell = ({ jobId }: { jobId: string }) => ( + +); + +describe('workspace shell keyed by job', () => { + test('remounts when jobId changes', () => { + mounts = 0; + const { rerender } = render(); + expect(mounts).toBe(1); + + rerender(); + expect(mounts).toBe(2); + }); +}); From 2bc090c378dbcee328500ff9af78b22154c3e46b Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:06:44 -0400 Subject: [PATCH 11/88] chore: remove terminal task scratch report --- .superpowers/sdd/task-11-report.md | 33 ------------------------------ 1 file changed, 33 deletions(-) delete mode 100644 .superpowers/sdd/task-11-report.md diff --git a/.superpowers/sdd/task-11-report.md b/.superpowers/sdd/task-11-report.md deleted file mode 100644 index e8cc3bb..0000000 --- a/.superpowers/sdd/task-11-report.md +++ /dev/null @@ -1,33 +0,0 @@ -# Task 11 Report - -## Status - -Implemented terminal fit scheduling for the first animation frame, `document.fonts.ready`, and Nerd Font loading. Each successful, non-aborted fit also sends the current terminal dimensions to the worker. Nerd Font completion still refreshes the terminal glyph rows. - -## TDD evidence - -- RED: `cd apps/next && bun run test:unit -- terminal-fit` failed with `Cannot find package '@/components/agent-workspace/terminal-fit'` before the helper existed. -- GREEN: the same focused command passed 1 file and 2 tests after adding `scheduleTerminalFits`. - -## Verification - -- `cd apps/next && bun run test:unit -- terminal-fit`: PASS, 1 file / 2 tests. -- `cd apps/next && bun run test:unit`: PASS, 4 files / 10 tests. -- `cd apps/next && bun run typecheck`: PASS. -- `cd apps/next && bun run format`: PASS. -- `cd apps/next && bun run lint`: PASS, with the existing stale `baseline-browser-mapping` data warning. -- `cd apps/next && bun run test:component`: BLOCKED before test collection. This isolated worktree's root `node_modules` symlink resolves to `/home/gib/Documents/Code/Spoon/node_modules`, causing Vitest to look for `/home/gib/Documents/Code/Spoon/tools/vitest/setup-jsdom.ts` instead of the worktree setup file. - -## Concerns - -- No implementation concerns found. -- The component-suite worktree dependency-resolution issue remains external to this task. - -## Reviewer follow-up: preserve the animation-frame first fit - -- RED: `cd apps/next && bun run test:unit -- terminal-fit` exited 1 with 1 failed / 2 passed. The new `waits for the first animation frame before font-triggered fits` regression expected zero calls before rAF, but `fit` had already been called 2 times after flushing the resolved font promises. -- GREEN: after gating both font-triggered refits on the initial rAF fit promise, `cd apps/next && bun run test:unit -- terminal-fit` exited 0 with 1 file / 3 tests passed. -- Final focused verification: `cd apps/next && bun run test:unit -- terminal-fit` exited 0 with 1 file / 3 tests passed. -- Final full unit verification: `cd apps/next && bun run test:unit` exited 0 with 4 files / 11 tests passed. -- Final type verification: `cd apps/next && bun run typecheck` exited 0. -- Final formatting verification: `cd apps/next && bun run format` exited 0; all matched files use Prettier formatting. From 57c175aecd4a7edd37b511336b384474ffdce5b7 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:09:17 -0400 Subject: [PATCH 12/88] fix(worker): contain workspace paths against symlinks --- apps/agent-worker/src/path-containment.ts | 39 ++++++++++++++ apps/agent-worker/src/user-environment.ts | 15 ++---- apps/agent-worker/src/worker.ts | 22 ++++---- .../tests/unit/path-containment.test.ts | 54 +++++++++++++++++++ 4 files changed, 107 insertions(+), 23 deletions(-) create mode 100644 apps/agent-worker/src/path-containment.ts create mode 100644 apps/agent-worker/tests/unit/path-containment.test.ts diff --git a/apps/agent-worker/src/path-containment.ts b/apps/agent-worker/src/path-containment.ts new file mode 100644 index 0000000..f53550c --- /dev/null +++ b/apps/agent-worker/src/path-containment.ts @@ -0,0 +1,39 @@ +import { lstat, realpath } from 'node:fs/promises'; +import path from 'node:path'; + +const realpathOrDeepestExisting = async (target: string): Promise => { + let current = target; + 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; + } + } +}; + +export const assertContainedRealPath = async ( + root: string, + requested: string, + opts: { forWrite?: boolean } = {}, +): Promise => { + const lexical = path.resolve(root, requested); + const realRoot = await realpath(root); + const resolved = await realpathOrDeepestExisting(lexical); + if (resolved !== realRoot && !resolved.startsWith(`${realRoot}${path.sep}`)) { + throw new Error(`Refusing to access path outside root: ${requested}`); + } + if (opts.forWrite) { + const info = await lstat(lexical).catch(() => null); + if (info?.isSymbolicLink()) { + throw new Error(`Refusing to write through a symlink: ${requested}`); + } + } + return resolved; +}; diff --git a/apps/agent-worker/src/user-environment.ts b/apps/agent-worker/src/user-environment.ts index 19f6a30..da81712 100644 --- a/apps/agent-worker/src/user-environment.ts +++ b/apps/agent-worker/src/user-environment.ts @@ -7,6 +7,7 @@ import type { Id } from '@spoon/backend/convex/_generated/dataModel.js'; import { api } from '@spoon/backend/convex/_generated/api.js'; import { env } from './env'; +import { assertContainedRealPath } from './path-containment'; import { runExecInContainer } from './runtime/docker'; const client = new ConvexHttpClient(env.convexUrl); @@ -31,16 +32,6 @@ export const fetchUserEnvironment = async ( const shellQuote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; -// Keep a written path inside the home directory. -const safeHomeJoin = (homeDir: string, relPath: string) => { - const target = path.resolve(homeDir, relPath); - const root = path.resolve(homeDir); - if (target !== root && !target.startsWith(`${root}${path.sep}`)) { - throw new Error(`Refusing to write dotfile outside home: ${relPath}`); - } - return target; -}; - /** * Materializes the persistent per-user home: a `.bash_profile` so login shells * load `~/.bashrc`; (when configured and changed) a clone of the public dotfiles @@ -111,7 +102,9 @@ export const materializeUserHome = async (args: { // Editable overlay tree (wins over the repo/setup output). for (const file of userEnv.files) { - const target = safeHomeJoin(homeDir, file.path); + const target = await assertContainedRealPath(homeDir, file.path, { + forWrite: true, + }); await mkdir(path.dirname(target), { recursive: true }); await writeFile(target, file.content); if (file.isExecutable) await chmod(target, 0o755); diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 55b794a..a5f6f15 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -34,6 +34,7 @@ import { promptOpenCodeSession, replyOpenCodePermission, } from './opencode-session'; +import { assertContainedRealPath } from './path-containment'; import { createRedactor, truncate } from './redact'; import { listWorkspaceContainerNames, @@ -1058,15 +1059,6 @@ const resolveWorkspace = (jobId: string) => { return workspace; }; -const safeWorkspacePath = (repoDir: string, filePath: string) => { - const resolved = path.resolve(repoDir, filePath); - const root = path.resolve(repoDir); - if (resolved !== root && !resolved.startsWith(`${root}${path.sep}`)) { - throw new Error(`Refusing to access path outside repository: ${filePath}`); - } - return resolved; -}; - const fileChangedType = async (repoDir: string, filePath: string) => { const status = await run('git', ['status', '--short', '--', filePath], { cwd: repoDir, @@ -1169,7 +1161,11 @@ const recordChangedFiles = async ( const materializeEnvFile = async (workspace: ActiveWorkspace) => { const { claim, repoDir } = workspace; if (!claim.job.materializeEnvFile || !claim.job.envFilePath) return; - const envPath = safeWorkspacePath(repoDir, claim.job.envFilePath); + const envPath = await assertContainedRealPath( + repoDir, + claim.job.envFilePath, + { forWrite: true }, + ); await mkdir(path.dirname(envPath), { recursive: true }); const content = `${claim.secrets .map((secret) => `${secret.name}=${JSON.stringify(secret.value)}`) @@ -1503,7 +1499,7 @@ export const listWorkspaceTree = async (jobId: string) => { export const readWorkspaceFile = async (jobId: string, filePath: string) => { const workspace = resolveWorkspace(jobId); - const target = safeWorkspacePath(workspace.repoDir, filePath); + const target = await assertContainedRealPath(workspace.repoDir, filePath); return await readFile(target, 'utf8'); }; @@ -1513,7 +1509,9 @@ export const writeWorkspaceFile = async ( content: string, ) => { const workspace = resolveWorkspace(jobId); - const target = safeWorkspacePath(workspace.repoDir, filePath); + const target = await assertContainedRealPath(workspace.repoDir, filePath, { + forWrite: true, + }); await mkdir(path.dirname(target), { recursive: true }); await writeFile(target, content); const diff = await getWorktreeDiff(workspace.repoDir, workspace.redact); diff --git a/apps/agent-worker/tests/unit/path-containment.test.ts b/apps/agent-worker/tests/unit/path-containment.test.ts new file mode 100644 index 0000000..02112a3 --- /dev/null +++ b/apps/agent-worker/tests/unit/path-containment.test.ts @@ -0,0 +1,54 @@ +import { mkdir, mkdtemp, rm, symlink, writeFile } 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(); + }); +}); From bdb2964e34ebf5ccb5ce6cc324f14f69dd3e5cd7 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:12:34 -0400 Subject: [PATCH 13/88] fix(sync): honor per-spoon auto-sync settings --- packages/backend/convex/github.ts | 2 +- packages/backend/convex/githubSync.ts | 21 ++- packages/backend/convex/migrations.ts | 25 +++ packages/backend/convex/syncGating.ts | 19 ++ .../backend/tests/unit/sync-gating.test.ts | 167 ++++++++++++++++++ 5 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 packages/backend/convex/migrations.ts create mode 100644 packages/backend/convex/syncGating.ts create mode 100644 packages/backend/tests/unit/sync-gating.test.ts diff --git a/packages/backend/convex/github.ts b/packages/backend/convex/github.ts index 38f91ce..c7886f1 100644 --- a/packages/backend/convex/github.ts +++ b/packages/backend/convex/github.ts @@ -190,7 +190,7 @@ export const createForkSpoonRecord = internalMutation({ ownerId: args.ownerId, autoRefreshEnabled: true, autoReviewEnabled: true, - autoSyncEnabled: false, + autoSyncEnabled: true, requireAiLowRiskForSync: true, requireCleanCompareForSync: true, ignoredFilePatterns: [], diff --git a/packages/backend/convex/githubSync.ts b/packages/backend/convex/githubSync.ts index 8515326..9f07e28 100644 --- a/packages/backend/convex/githubSync.ts +++ b/packages/backend/convex/githubSync.ts @@ -16,6 +16,7 @@ import { listPullRequests, syncForkBranch, } from './githubClient'; +import { shouldAutoSync, shouldCreateReviewThread } from './syncGating'; const getRequiredUserId = async (ctx: ActionCtx) => { const userId = await getAuthUserId(ctx); @@ -69,6 +70,16 @@ const refreshOwnedSpoon = async ( 'GitHub refresh is only available for GitHub Spoons.', ); } + 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; + // requireAiLowRiskForSync applies to the future AI review-to-auto-merge path, + // which is deliberately deferred until Phase 4. const connection = await ctx.runQuery(internal.github.getConnectionForUser, { userId: ownerId, }); @@ -202,7 +213,13 @@ const refreshOwnedSpoon = async ( summary: `GitHub refresh complete: ${upstreamCompare.aheadBy} upstream commit(s), ${forkCompare.aheadBy} fork-only commit(s).`, }); - if (status === 'behind' && forkCompare.aheadBy === 0 && allowAutoSync) { + if ( + allowAutoSync && + shouldAutoSync( + { autoSyncEnabled, requireCleanCompareForSync }, + { status, forkAheadBy: forkCompare.aheadBy }, + ) + ) { try { await syncForkBranch(octokit, { forkOwner, @@ -258,7 +275,7 @@ const refreshOwnedSpoon = async ( } } - if (status === 'diverged') { + if (shouldCreateReviewThread({ autoReviewEnabled }, { status })) { const threadId = await ctx.runMutation( internal.threads.createMaintenanceThread, { diff --git a/packages/backend/convex/migrations.ts b/packages/backend/convex/migrations.ts new file mode 100644 index 0000000..e86db72 --- /dev/null +++ b/packages/backend/convex/migrations.ts @@ -0,0 +1,25 @@ +import { v } from 'convex/values'; + +import { internalMutation } from './_generated/server'; + +// One-shot backfill: preserve existing automatic fast-forward behavior for +// GitHub spoons now that refreshes honor autoSyncEnabled. Idempotent. +export const backfillAutoSyncDefaults = internalMutation({ + args: {}, + returns: v.object({ updated: v.number() }), + 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 }; + }, +}); diff --git a/packages/backend/convex/syncGating.ts b/packages/backend/convex/syncGating.ts new file mode 100644 index 0000000..17eb489 --- /dev/null +++ b/packages/backend/convex/syncGating.ts @@ -0,0 +1,19 @@ +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'; diff --git a/packages/backend/tests/unit/sync-gating.test.ts b/packages/backend/tests/unit/sync-gating.test.ts new file mode 100644 index 0000000..78525f1 --- /dev/null +++ b/packages/backend/tests/unit/sync-gating.test.ts @@ -0,0 +1,167 @@ +import { convexTest } from 'convex-test'; +import { makeFunctionReference } from 'convex/server'; +import { describe, expect, test } from 'vitest'; + +import { internal } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; +import { + shouldAutoSync, + shouldCreateReviewThread, +} from '../../convex/syncGating'; + +const modules = import.meta.glob('../../convex/**/*.*s'); +const backfillAutoSyncDefaults = makeFunctionReference< + 'mutation', + Record, + { updated: number } +>('migrations:backfillAutoSyncDefaults'); + +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); + }); + + test('backfill enables auto-sync only for disabled GitHub spoons', async () => { + const t = convexTest(schema, modules); + const seeded = await t.mutation(async (ctx) => { + const now = Date.now(); + const ownerId = await ctx.db.insert('users', { + email: 'owner@example.com', + name: 'Owner', + }); + const createSpoon = async ( + name: string, + provider: 'github' | 'gitea', + autoSyncEnabled: boolean, + ) => { + const spoonId = await ctx.db.insert('spoons', { + ownerId, + name, + provider, + upstreamOwner: 'upstream', + upstreamRepo: name, + upstreamDefaultBranch: 'main', + upstreamUrl: `https://example.com/upstream/${name}`, + forkOwner: 'owner', + forkRepo: name, + forkUrl: `https://example.com/owner/${name}`, + visibility: 'private', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + const settingsId = await ctx.db.insert('spoonSettings', { + spoonId, + ownerId, + autoRefreshEnabled: true, + autoReviewEnabled: true, + autoSyncEnabled, + requireAiLowRiskForSync: true, + requireCleanCompareForSync: true, + ignoredFilePatterns: [], + importantFilePatterns: [], + createdAt: now, + updatedAt: now, + }); + return settingsId; + }; + + return { + disabledGithub: await createSpoon('disabled-github', 'github', false), + enabledGithub: await createSpoon('enabled-github', 'github', true), + disabledGitea: await createSpoon('disabled-gitea', 'gitea', false), + }; + }); + + const result = await t.mutation(backfillAutoSyncDefaults, {}); + const settings = await t.run(async (ctx) => ({ + disabledGithub: await ctx.db.get(seeded.disabledGithub), + enabledGithub: await ctx.db.get(seeded.enabledGithub), + disabledGitea: await ctx.db.get(seeded.disabledGitea), + })); + + expect(result).toEqual({ updated: 1 }); + expect(settings.disabledGithub?.autoSyncEnabled).toBe(true); + expect(settings.enabledGithub?.autoSyncEnabled).toBe(true); + expect(settings.disabledGitea?.autoSyncEnabled).toBe(false); + }); + + test('new GitHub spoons default auto-sync on', async () => { + const t = convexTest(schema, modules); + const ownerId = await t.mutation(async (ctx) => { + return await ctx.db.insert('users', { + email: 'new-owner@example.com', + name: 'New owner', + }); + }); + + const spoonId = await t.mutation(internal.github.createForkSpoonRecord, { + ownerId, + name: 'github-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'repo', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://github.com/upstream/repo', + forkOwner: 'new-owner', + forkRepo: 'repo', + forkDefaultBranch: 'main', + forkUrl: 'https://github.com/new-owner/repo', + visibility: 'private', + }); + const settings = await t.run(async (ctx) => { + return await ctx.db + .query('spoonSettings') + .withIndex('by_spoon', (q) => q.eq('spoonId', spoonId)) + .unique(); + }); + + expect(settings?.autoSyncEnabled).toBe(true); + }); +}); From 906f906ad38cf730dca2ed5156cb75c57f88927b Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:15:34 -0400 Subject: [PATCH 14/88] fix(backend): use generated import extension --- packages/backend/convex/migrations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/convex/migrations.ts b/packages/backend/convex/migrations.ts index e86db72..8be691e 100644 --- a/packages/backend/convex/migrations.ts +++ b/packages/backend/convex/migrations.ts @@ -1,6 +1,6 @@ import { v } from 'convex/values'; -import { internalMutation } from './_generated/server'; +import { internalMutation } from './_generated/server.js'; // One-shot backfill: preserve existing automatic fast-forward behavior for // GitHub spoons now that refreshes honor autoSyncEnabled. Idempotent. From c4d0f5219a68d1b1743d0bd64d295c7c01f2f0b7 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:19:38 -0400 Subject: [PATCH 15/88] fix(terminal): use real TTY exec and resize --- apps/agent-worker/src/runtime/docker.ts | 8 + apps/agent-worker/src/terminal.ts | 140 +++++++++--------- .../tests/unit/terminal-resize.test.ts | 46 ++++++ 3 files changed, 124 insertions(+), 70 deletions(-) create mode 100644 apps/agent-worker/tests/unit/terminal-resize.test.ts diff --git a/apps/agent-worker/src/runtime/docker.ts b/apps/agent-worker/src/runtime/docker.ts index f169928..f3a23fc 100644 --- a/apps/agent-worker/src/runtime/docker.ts +++ b/apps/agent-worker/src/runtime/docker.ts @@ -1,6 +1,7 @@ import { mkdir } from 'node:fs/promises'; import path from 'node:path'; import type { Readable } from 'node:stream'; +import Docker from 'dockerode'; import { execa } from 'execa'; import { env } from '../env'; @@ -10,6 +11,13 @@ type CommandResult = { output: string; }; +let dockerClient: Docker | undefined; + +export const getDockerClient = () => { + dockerClient ??= new Docker(); + return dockerClient; +}; + const environmentArgs = (environment: Record) => Object.entries(environment).flatMap(([name, value]) => [ '-e', diff --git a/apps/agent-worker/src/terminal.ts b/apps/agent-worker/src/terminal.ts index c99a5a3..b9d4c62 100644 --- a/apps/agent-worker/src/terminal.ts +++ b/apps/agent-worker/src/terminal.ts @@ -1,11 +1,11 @@ -import { spawn } from 'node:child_process'; -import type { ChildProcessWithoutNullStreams } from 'node:child_process'; import type { Server } from 'node:http'; +import type { Duplex } from 'node:stream'; import type { WebSocket } from 'ws'; import { WebSocketServer } from 'ws'; import type { BoxHandle } from './user-container'; import { env } from './env'; +import { getDockerClient } from './runtime/docker'; import { verifyTerminalToken } from './terminal-token'; import { acquireUserBox } from './user-container'; import { getTerminalWorkspace } from './worker'; @@ -16,8 +16,26 @@ const clampDimension = (value: unknown) => { return Math.min(Math.max(n, 1), 1000); }; -// Single-quote a string for a POSIX shell. -const shellQuote = (value: string) => `'${value.replaceAll("'", `'\\''`)}'`; +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; + } +}; const bridge = async (ws: WebSocket, jobId: string) => { const workspace = getTerminalWorkspace(jobId); @@ -26,41 +44,27 @@ const bridge = async (ws: WebSocket, jobId: string) => { return; } - // bun can't load node-pty (native ABI mismatch) and dockerode can't attach to - // podman, so we drive the runtime CLI (` exec -i`) and allocate the PTY - // *inside* the container with `script`, bridging the plain pipes to the socket. - // // Register the message handler immediately and buffer input/size until the exec // is ready (acquiring the box can take seconds on first connect), so the initial // resize and early keystrokes aren't dropped. - const procHolder: { current?: ChildProcessWithoutNullStreams } = {}; + const execHolder: { + current?: { exec: import('dockerode').Exec; stream: Duplex }; + } = {}; const pendingInput: Buffer[] = []; let cols = 80; let rows = 24; ws.on('message', (data: Buffer, isBinary: boolean) => { - if (!isBinary) { - // Text frames are control messages (resize); anything else is raw input. - try { - const message = JSON.parse(data.toString('utf8')) as { - type?: string; - cols?: number; - rows?: number; - }; - if (message.type === 'resize') { - const c = clampDimension(message.cols); - const r = clampDimension(message.rows); - if (c && r) { - cols = c; - rows = r; - } - return; - } - } catch { - // fall through: treat as raw input - } + const resize = parseResizeMessage(data, isBinary); + if (resize) { + cols = resize.cols; + rows = resize.rows; + void execHolder.current?.exec + .resize({ h: rows, w: cols }) + .catch(() => undefined); + return; } - if (procHolder.current) procHolder.current.stdin.write(data); + if (execHolder.current) execHolder.current.stream.write(data); else pendingInput.push(data); }); @@ -72,7 +76,8 @@ const bridge = async (ws: WebSocket, jobId: string) => { const cleanup = () => { if (released) return; released = true; - procHolder.current?.kill(); + execHolder.current?.stream.end(); + execHolder.current?.stream.destroy(); handleHolder.current?.release(); }; ws.on('close', cleanup); @@ -103,54 +108,47 @@ const bridge = async (ws: WebSocket, jobId: string) => { return; } - // Reattach a persistent tmux session across reconnects when available, else a - // plain login shell. `stty` sizes the PTY to the client's viewport up front. - const launcher = - `stty rows ${rows} cols ${cols} 2>/dev/null; ` + - // Reattach a persistent tmux session when tmux is present; otherwise fall back - // to an interactive login shell (`-i` so it prints a prompt and line-edits). - // Check with `command -v` rather than `exec tmux || …`: a failed `exec` makes a - // non-interactive shell exit before the `||`, so the fallback never runs. - 'if command -v tmux >/dev/null 2>&1; then exec tmux new-session -A -s spoon; ' + - 'else exec bash -il; fi'; - const envFlags = [ - '-e', - 'TERM=xterm-256color', - '-e', - `HOME=${workspace.containerHome}`, - ...workspace.secrets.flatMap((s) => ['-e', `${s.name}=${s.value}`]), - ]; - - const proc = spawn( - env.containerRuntime, - [ - 'exec', - '-i', - ...envFlags, - '-w', - workspace.containerRepo, - boxName, + 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', - `exec script -qfc ${shellQuote(launcher)} /dev/null`, + 'if command -v tmux >/dev/null 2>&1; then exec tmux new-session -A -s spoon; else exec bash -il; fi', ], - { stdio: ['pipe', 'pipe', 'pipe'] }, - ); - procHolder.current = proc; + Env: [ + 'TERM=xterm-256color', + `HOME=${workspace.containerHome}`, + ...workspace.secrets.map((secret) => `${secret.name}=${secret.value}`), + ], + WorkingDir: workspace.containerRepo, + }); + const stream = await exec.start({ hijack: true, stdin: true, Tty: true }); + execHolder.current = { exec, stream }; + + if (isReleased()) { + stream.end(); + stream.destroy(); + return; + } + + await exec.resize({ h: rows, w: cols }).catch(() => undefined); // Replay any keystrokes the client sent before the process was ready. - for (const buffered of pendingInput) proc.stdin.write(buffered); + for (const buffered of pendingInput) stream.write(buffered); pendingInput.length = 0; - const forward = (chunk: Buffer) => { + stream.on('data', (chunk: Buffer) => { if (ws.readyState === ws.OPEN) ws.send(chunk, { binary: true }); - }; - proc.stdout.on('data', forward); - proc.stderr.on('data', forward); - proc.on('exit', () => { + }); + stream.on('end', () => { if (ws.readyState === ws.OPEN) ws.close(); }); - proc.on('error', () => { + stream.on('error', () => { if (ws.readyState === ws.OPEN) ws.close(); }); }; @@ -179,7 +177,9 @@ export const attachTerminalServer = (server: Server) => { return; } wss.handleUpgrade(request, socket, head, (ws) => { - void bridge(ws, jobId); + void bridge(ws, jobId).catch(() => { + if (ws.readyState === ws.OPEN) ws.close(); + }); }); }); diff --git a/apps/agent-worker/tests/unit/terminal-resize.test.ts b/apps/agent-worker/tests/unit/terminal-resize.test.ts new file mode 100644 index 0000000..4a7d1fe --- /dev/null +++ b/apps/agent-worker/tests/unit/terminal-resize.test.ts @@ -0,0 +1,46 @@ +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 }); + }); +}); From ed21126eeae74f4f2ed62b3bf165e979bacb8e90 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:11:56 -0400 Subject: [PATCH 16/88] fix(app): add friendly not-found handling --- apps/next/src/app/(app)/error.tsx | 30 +++++ apps/next/src/app/(app)/not-found.tsx | 17 +++ .../spoons/[spoonId]/agent/[jobId]/page.tsx | 25 +++- .../src/app/(app)/threads/[threadId]/page.tsx | 14 ++- packages/backend/convex/agentJobs.ts | 2 +- packages/backend/convex/threads.ts | 2 +- .../backend/tests/unit/detail-null.test.ts | 118 ++++++++++++++++++ 7 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 apps/next/src/app/(app)/error.tsx create mode 100644 apps/next/src/app/(app)/not-found.tsx create mode 100644 packages/backend/tests/unit/detail-null.test.ts diff --git a/apps/next/src/app/(app)/error.tsx b/apps/next/src/app/(app)/error.tsx new file mode 100644 index 0000000..1690291 --- /dev/null +++ b/apps/next/src/app/(app)/error.tsx @@ -0,0 +1,30 @@ +'use client'; + +import { useEffect } from 'react'; +import * as Sentry from '@sentry/nextjs'; + +import { Button } from '@spoon/ui'; + +const AppError = ({ + error, + reset, +}: { + error: Error & { digest?: string }; + reset: () => void; +}) => { + useEffect(() => { + Sentry.captureException(error); + }, [error]); + + return ( +
+

Something went wrong

+

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

+ +
+ ); +}; + +export default AppError; diff --git a/apps/next/src/app/(app)/not-found.tsx b/apps/next/src/app/(app)/not-found.tsx new file mode 100644 index 0000000..0e9a171 --- /dev/null +++ b/apps/next/src/app/(app)/not-found.tsx @@ -0,0 +1,17 @@ +import Link from 'next/link'; + +import { Button } from '@spoon/ui'; + +const NotFound = () => ( +
+

Not found

+

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

+ +
+); + +export default NotFound; diff --git a/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx b/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx index 93fdbfe..29aaef5 100644 --- a/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx +++ b/apps/next/src/app/(app)/spoons/[spoonId]/agent/[jobId]/page.tsx @@ -1,5 +1,6 @@ 'use client'; +import type { FunctionReturnType } from 'convex/server'; import { useEffect } from 'react'; import Link from 'next/link'; import { useParams, useRouter } from 'next/navigation'; @@ -15,13 +16,33 @@ const AgentWorkspacePage = () => { const router = useRouter(); const params = useParams<{ spoonId: string; jobId: string }>(); const jobId = params.jobId as Id<'agentJobs'>; - const job = useQuery(api.agentJobs.get, { jobId }); + const job = useQuery(api.agentJobs.get, { jobId }) as + | FunctionReturnType + | null + | undefined; useEffect(() => { if (job?.threadId) router.replace(`/threads/${job.threadId}`); }, [job?.threadId, router]); - if (job?.threadId) { + if (job === undefined) { + return ( +
Loading workspace...
+ ); + } + + if (job === null) { + return ( +
+

This workspace was not found.

+ +
+ ); + } + + if (job.threadId) { return (
Opening thread workspace... diff --git a/apps/next/src/app/(app)/threads/[threadId]/page.tsx b/apps/next/src/app/(app)/threads/[threadId]/page.tsx index add0be8..b265e0a 100644 --- a/apps/next/src/app/(app)/threads/[threadId]/page.tsx +++ b/apps/next/src/app/(app)/threads/[threadId]/page.tsx @@ -1,5 +1,6 @@ 'use client'; +import type { FunctionReturnType } from 'convex/server'; import { useState } from 'react'; import Link from 'next/link'; import { useParams, useRouter } from 'next/navigation'; @@ -33,7 +34,10 @@ const ThreadDetailPage = () => { const router = useRouter(); const params = useParams<{ threadId: string }>(); const threadId = params.threadId as Id<'threads'>; - const details = useQuery(api.threads.get, { threadId }); + const details = useQuery(api.threads.get, { threadId }) as + | FunctionReturnType + | null + | undefined; const createJob = useMutation(api.agentJobs.createForThread); const markResolved = useMutation(api.threads.markResolved); const cancel = useMutation(api.threads.cancel); @@ -43,6 +47,14 @@ const ThreadDetailPage = () => { return
Loading thread...
; } + if (details === null) { + return ( +
+ This thread was not found. +
+ ); + } + const { thread, spoon, latestJob } = details; if (latestJob && spoon) { return ( diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 53d0bb8..eca649b 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -710,7 +710,7 @@ export const get = query({ handler: async (ctx, { jobId }) => { const ownerId = await getRequiredUserId(ctx); const job = await ctx.db.get(jobId); - if (job?.ownerId !== ownerId) throw new ConvexError('Agent job not found.'); + if (job?.ownerId !== ownerId) return null; return job; }, }); diff --git a/packages/backend/convex/threads.ts b/packages/backend/convex/threads.ts index cd3fc29..5525fa2 100644 --- a/packages/backend/convex/threads.ts +++ b/packages/backend/convex/threads.ts @@ -211,7 +211,7 @@ export const get = query({ handler: async (ctx, { threadId }) => { const ownerId = await getRequiredUserId(ctx); const thread = await ctx.db.get(threadId); - if (thread?.ownerId !== ownerId) throw new ConvexError('Thread not found.'); + if (thread?.ownerId !== ownerId) return null; const spoon = thread.spoonId ? await ctx.db.get(thread.spoonId) : null; const job = thread.latestAgentJobId ? await ctx.db.get(thread.latestAgentJobId) diff --git a/packages/backend/tests/unit/detail-null.test.ts b/packages/backend/tests/unit/detail-null.test.ts new file mode 100644 index 0000000..bfb885f --- /dev/null +++ b/packages/backend/tests/unit/detail-null.test.ts @@ -0,0 +1,118 @@ +import { convexTest } from 'convex-test'; +import { describe, expect, test } from 'vitest'; + +import type { Id } from '../../convex/_generated/dataModel.js'; +import { api } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; + +const modules = import.meta.glob('../../convex/**/*.*s'); + +const createUser = async (t: ReturnType, email: string) => + await t.mutation(async (ctx) => { + return await ctx.db.insert('users', { email, name: email }); + }); + +const authed = (t: ReturnType, userId: string) => + t.withIdentity({ + subject: `${userId}|session`, + issuer: 'https://convex.test', + }); + +const seedDetails = async ( + t: ReturnType, + ownerId: Id<'users'>, +) => + await t.mutation(async (ctx) => { + const now = Date.now(); + const spoonId = await ctx.db.insert('spoons', { + ownerId, + name: 'Detail Spoon', + provider: 'gitea', + upstreamOwner: 'upstream', + upstreamRepo: 'detail', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://git.example.com/upstream/detail', + forkOwner: 'team', + forkRepo: 'detail-spoon', + forkUrl: 'https://git.example.com/team/detail-spoon', + visibility: 'private', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + const threadId = await ctx.db.insert('threads', { + ownerId, + spoonId, + title: 'Private thread', + source: 'user_request', + status: 'open', + priority: 'normal', + createdAt: now, + updatedAt: now, + }); + const requestId = await ctx.db.insert('agentRequests', { + spoonId, + ownerId, + prompt: 'Private work', + status: 'running', + createdAt: now, + updatedAt: now, + }); + const jobId = await ctx.db.insert('agentJobs', { + spoonId, + ownerId, + agentRequestId: requestId, + threadId, + status: 'running', + prompt: 'Private work', + runtime: 'opencode', + baseBranch: 'main', + workBranch: 'spoon/private', + forkOwner: 'team', + forkRepo: 'detail-spoon', + forkUrl: 'https://git.example.com/team/detail-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'detail', + selectedSecretIds: [], + model: 'openai/gpt-5.1-codex', + reasoningEffort: 'medium', + createdAt: now, + updatedAt: now, + }); + await ctx.db.patch(threadId, { latestAgentJobId: jobId }); + return { threadId, jobId }; + }); + +describe('detail queries for unavailable records', () => { + test('return null for records owned by another user', async () => { + const t = convexTest(schema, modules); + const ownerId = (await createUser(t, 'owner@example.com')) as Id<'users'>; + const otherId = await createUser(t, 'other@example.com'); + const { threadId, jobId } = await seedDetails(t, ownerId); + const other = authed(t, otherId); + + await expect( + other.query(api.threads.get, { threadId }), + ).resolves.toBeNull(); + await expect(other.query(api.agentJobs.get, { jobId })).resolves.toBeNull(); + }); + + test('return null for records that do not exist', async () => { + const t = convexTest(schema, modules); + const ownerId = (await createUser(t, 'owner@example.com')) as Id<'users'>; + const owner = authed(t, ownerId); + const { threadId, jobId } = await seedDetails(t, ownerId); + await t.mutation(async (ctx) => { + await ctx.db.delete(jobId); + await ctx.db.delete(threadId); + }); + + await expect( + owner.query(api.threads.get, { threadId }), + ).resolves.toBeNull(); + await expect(owner.query(api.agentJobs.get, { jobId })).resolves.toBeNull(); + }); +}); From 2beeeb17327ed9abe5180a615f5b1a9211226053 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:17:36 -0400 Subject: [PATCH 17/88] fix(app): handle nullable detail consumers --- .../api/threads/[threadId]/message/route.ts | 10 +++- .../agent-workspace/agent-workspace-shell.tsx | 14 +++++- .../component/agent-workspace-shell.test.tsx | 31 ++++++++++++ .../tests/unit/thread-message-route.test.ts | 50 +++++++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 apps/next/tests/component/agent-workspace-shell.test.tsx create mode 100644 apps/next/tests/unit/thread-message-route.test.ts diff --git a/apps/next/src/app/api/threads/[threadId]/message/route.ts b/apps/next/src/app/api/threads/[threadId]/message/route.ts index 2b0626d..d591458 100644 --- a/apps/next/src/app/api/threads/[threadId]/message/route.ts +++ b/apps/next/src/app/api/threads/[threadId]/message/route.ts @@ -1,3 +1,4 @@ +import type { FunctionReturnType } from 'convex/server'; import { NextResponse } from 'next/server'; import { proxyWorker } from '@/lib/agent-worker-proxy'; import { convexAuthNextjsToken } from '@convex-dev/auth/nextjs/server'; @@ -35,7 +36,14 @@ export const POST = async ( { status: 400 }, ); } - const details = await fetchQuery(api.threads.get, { threadId }, { token }); + const details = (await fetchQuery( + api.threads.get, + { threadId }, + { token }, + )) as FunctionReturnType | null; + if (details === null) { + return NextResponse.json({ error: 'Thread not found.' }, { status: 404 }); + } const latestJob = details.latestJob; const canSendToWorker = latestJob && diff --git a/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx b/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx index 923d300..19244f8 100644 --- a/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx +++ b/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx @@ -1,5 +1,6 @@ 'use client'; +import type { FunctionReturnType } from 'convex/server'; import type { CSSProperties, PointerEvent as ReactPointerEvent } from 'react'; import { useCallback, useEffect, useState } from 'react'; import { useMutation, useQuery } from 'convex/react'; @@ -59,7 +60,10 @@ type PendingOverwrite = { }; export const AgentWorkspaceShell = ({ jobId }: { jobId: Id<'agentJobs'> }) => { - const job = useQuery(api.agentJobs.get, { jobId }); + const job = useQuery(api.agentJobs.get, { jobId }) as + | FunctionReturnType + | null + | undefined; const messages = useQuery(api.agentJobs.listMessages, { jobId, limit: 200 }) ?? []; const events = @@ -323,6 +327,14 @@ export const AgentWorkspaceShell = ({ jobId }: { jobId: Id<'agentJobs'> }) => { ); } + if (job === null) { + return ( +
+ This workspace was not found. +
+ ); + } + const activeFile = activeFilePath ? files[activeFilePath] : undefined; const recoverWorkspace = async () => { if (!job.threadId) return; diff --git a/apps/next/tests/component/agent-workspace-shell.test.tsx b/apps/next/tests/component/agent-workspace-shell.test.tsx new file mode 100644 index 0000000..3e61bd9 --- /dev/null +++ b/apps/next/tests/component/agent-workspace-shell.test.tsx @@ -0,0 +1,31 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { AgentWorkspaceShell } from '../../src/components/agent-workspace/agent-workspace-shell'; + +const { mockUseMutation, mockUseQuery } = vi.hoisted(() => ({ + mockUseMutation: vi.fn(() => vi.fn()), + mockUseQuery: vi.fn(() => null), +})); + +vi.mock('convex/react', () => ({ + useMutation: mockUseMutation, + useQuery: mockUseQuery, +})); + +vi.mock('sonner', () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + }, +})); + +describe('AgentWorkspaceShell', () => { + it('renders a missing state when the job query returns null', () => { + render(); + + expect( + screen.getByText('This workspace was not found.'), + ).toBeInTheDocument(); + }); +}); diff --git a/apps/next/tests/unit/thread-message-route.test.ts b/apps/next/tests/unit/thread-message-route.test.ts new file mode 100644 index 0000000..759a27a --- /dev/null +++ b/apps/next/tests/unit/thread-message-route.test.ts @@ -0,0 +1,50 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { POST } from '../../src/app/api/threads/[threadId]/message/route'; + +const { mockAuthToken, mockFetchMutation, mockFetchQuery, mockProxyWorker } = + vi.hoisted(() => ({ + mockAuthToken: vi.fn(), + mockFetchMutation: vi.fn(), + mockFetchQuery: vi.fn(), + mockProxyWorker: vi.fn(), + })); + +vi.mock('@convex-dev/auth/nextjs/server', () => ({ + convexAuthNextjsToken: mockAuthToken, +})); + +vi.mock('convex/nextjs', () => ({ + fetchMutation: mockFetchMutation, + fetchQuery: mockFetchQuery, +})); + +vi.mock('@/lib/agent-worker-proxy', () => ({ + proxyWorker: mockProxyWorker, +})); + +describe('thread message route', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAuthToken.mockResolvedValue('token'); + }); + + it('returns 404 when the thread detail query returns null', async () => { + mockFetchQuery.mockResolvedValue(null); + + const response = await POST( + new Request('http://localhost/api/threads/thread-1/message', { + method: 'POST', + body: JSON.stringify({ content: 'Hello' }), + }), + { params: Promise.resolve({ threadId: 'thread-1' }) }, + ); + + expect(response.status).toBe(404); + await expect(response.json()).resolves.toEqual({ + error: 'Thread not found.', + }); + expect(mockFetchMutation).not.toHaveBeenCalled(); + expect(mockProxyWorker).not.toHaveBeenCalled(); + }); +}); From 926008fd3abfc4b7de6ac6163173e0ef247984a2 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:22:45 -0400 Subject: [PATCH 18/88] fix(workspace): skip queries for missing jobs --- .../agent-workspace/agent-workspace-shell.tsx | 26 +++++++++++++------ .../component/agent-workspace-shell.test.tsx | 23 ++++++++++++++-- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx b/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx index 19244f8..d93cf7a 100644 --- a/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx +++ b/apps/next/src/components/agent-workspace/agent-workspace-shell.tsx @@ -65,17 +65,27 @@ export const AgentWorkspaceShell = ({ jobId }: { jobId: Id<'agentJobs'> }) => { | null | undefined; const messages = - useQuery(api.agentJobs.listMessages, { jobId, limit: 200 }) ?? []; + useQuery( + api.agentJobs.listMessages, + job ? { jobId, limit: 200 } : 'skip', + ) ?? []; const events = - useQuery(api.agentJobs.listEvents, { jobId, limit: 200 }) ?? []; + useQuery(api.agentJobs.listEvents, job ? { jobId, limit: 200 } : 'skip') ?? + []; const workspaceChanges = - useQuery(api.agentJobs.listWorkspaceChanges, { jobId, limit: 200 }) ?? []; + useQuery( + api.agentJobs.listWorkspaceChanges, + job ? { jobId, limit: 200 } : 'skip', + ) ?? []; const interactions = - useQuery(api.agentJobs.listInteractionRequests, { - jobId, - status: 'all', - }) ?? []; - const uiState = useQuery(api.agentJobs.getWorkspaceUiState, { jobId }); + useQuery( + api.agentJobs.listInteractionRequests, + job ? { jobId, status: 'all' } : 'skip', + ) ?? []; + const uiState = useQuery( + api.agentJobs.getWorkspaceUiState, + job ? { jobId } : 'skip', + ); const patchUiState = useMutation(api.agentJobs.patchWorkspaceUiState); const createJobForThread = useMutation(api.agentJobs.createForThread); const deleteWorkspace = useMutation(api.agentJobs.deleteWorkspace); diff --git a/apps/next/tests/component/agent-workspace-shell.test.tsx b/apps/next/tests/component/agent-workspace-shell.test.tsx index 3e61bd9..3da755c 100644 --- a/apps/next/tests/component/agent-workspace-shell.test.tsx +++ b/apps/next/tests/component/agent-workspace-shell.test.tsx @@ -1,11 +1,12 @@ import { render, screen } from '@testing-library/react'; +import { getFunctionName } from 'convex/server'; import { describe, expect, it, vi } from 'vitest'; import { AgentWorkspaceShell } from '../../src/components/agent-workspace/agent-workspace-shell'; const { mockUseMutation, mockUseQuery } = vi.hoisted(() => ({ mockUseMutation: vi.fn(() => vi.fn()), - mockUseQuery: vi.fn(() => null), + mockUseQuery: vi.fn(), })); vi.mock('convex/react', () => ({ @@ -21,11 +22,29 @@ vi.mock('sonner', () => ({ })); describe('AgentWorkspaceShell', () => { - it('renders a missing state when the job query returns null', () => { + it('renders a missing state and skips dependent queries for a null job', () => { + mockUseQuery.mockImplementation((query) => + getFunctionName(query) === 'agentJobs:get' ? null : undefined, + ); + render(); expect( screen.getByText('This workspace was not found.'), ).toBeInTheDocument(); + expect( + mockUseQuery.mock.calls.map(([query]) => getFunctionName(query)), + ).toEqual([ + 'agentJobs:get', + 'agentJobs:listMessages', + 'agentJobs:listEvents', + 'agentJobs:listWorkspaceChanges', + 'agentJobs:listInteractionRequests', + 'agentJobs:getWorkspaceUiState', + ]); + expect(mockUseQuery.mock.calls[0]?.[1]).toEqual({ jobId: 'job-1' }); + for (let call = 2; call <= 6; call += 1) { + expect(mockUseQuery.mock.calls[call - 1]?.[1]).toBe('skip'); + } }); }); From cf45404b7d1628188103eee9b951594360a631a1 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:17:49 -0400 Subject: [PATCH 19/88] fix(agent-jobs): validate claims before claiming --- packages/backend/convex/agentJobs.ts | 62 ++++- packages/backend/convex/agentJobsNode.ts | 17 +- .../tests/unit/claim-validation.test.ts | 219 ++++++++++++++++++ 3 files changed, 281 insertions(+), 17 deletions(-) create mode 100644 packages/backend/tests/unit/claim-validation.test.ts diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index eca649b..0572de4 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -1074,6 +1074,65 @@ export const claimNextInternal = internalMutation({ secrets.push(secret); } } + const ownedProfile = + aiProviderProfile?.ownerId === job.ownerId && aiProviderProfile.enabled + ? 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; + } + if (agentSettings && !agentSettings.enabled) { + await failClaim('Agent jobs are disabled for this Spoon.'); + return null; + } + if ((job.runtime ?? 'opencode') !== 'opencode') { + await failClaim('Legacy OpenAI direct jobs are no longer supported.'); + return null; + } + if (secrets.length !== job.selectedSecretIds.length) { + await failClaim('Selected secrets must belong to this Spoon.'); + return null; + } const now = Date.now(); await ctx.db.patch(job._id, { status: 'claimed', @@ -1098,8 +1157,7 @@ export const claimNextInternal = internalMutation({ job: { ...job, status: 'claimed' as const, claimedBy: workerId }, spoon, aiSettings, - aiProviderProfile: - aiProviderProfile?.ownerId === job.ownerId ? aiProviderProfile : null, + aiProviderProfile: ownedProfile, agentSettings, secrets, }; diff --git a/packages/backend/convex/agentJobsNode.ts b/packages/backend/convex/agentJobsNode.ts index eef3810..4c36a0a 100644 --- a/packages/backend/convex/agentJobsNode.ts +++ b/packages/backend/convex/agentJobsNode.ts @@ -1,6 +1,6 @@ 'use node'; -import { ConvexError, v } from 'convex/values'; +import { v } from 'convex/values'; import type { Doc } from './_generated/dataModel'; import { internal } from './_generated/api'; @@ -56,20 +56,7 @@ export const claimNextForWorker = action({ }, )) as ClaimedJob | null; if (!claimed) return null; - if (!claimed.spoon) { - throw new ConvexError('Claimed job points at a missing Spoon.'); - } - if (!claimed.aiProviderProfile) { - throw new ConvexError( - 'AI is not configured for this user. Add an AI provider in settings.', - ); - } - if ( - claimed.aiProviderProfile.authType !== 'none' && - !claimed.aiProviderProfile.encryptedSecret - ) { - throw new ConvexError('Selected AI provider is missing credentials.'); - } + if (!claimed.spoon || !claimed.aiProviderProfile) return null; const profile = claimed.aiProviderProfile; return { job: claimed.job, diff --git a/packages/backend/tests/unit/claim-validation.test.ts b/packages/backend/tests/unit/claim-validation.test.ts new file mode 100644 index 0000000..8d2f461 --- /dev/null +++ b/packages/backend/tests/unit/claim-validation.test.ts @@ -0,0 +1,219 @@ +import { convexTest } from 'convex-test'; +import { describe, expect, test } from 'vitest'; + +import type { Id } from '../../convex/_generated/dataModel.js'; +import { internal } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; + +const modules = import.meta.glob('../../convex/**/*.*s'); + +type SeedOptions = { + profile?: 'valid' | 'disabled' | 'missing-credentials' | 'none'; + settingsEnabled?: boolean; + runtime?: 'opencode' | 'openai_direct'; + secret?: 'valid' | 'foreign'; +}; + +const seedQueuedJob = async ( + t: ReturnType, + options: SeedOptions = {}, +) => + await t.mutation(async (ctx) => { + const now = Date.now(); + const ownerId = await ctx.db.insert('users', { + email: 'owner@example.com', + name: 'Owner', + }); + const spoonId = await ctx.db.insert('spoons', { + ownerId, + name: 'Editor Spoon', + provider: 'github', + upstreamOwner: 'upstream', + upstreamRepo: 'editor', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://github.com/upstream/editor', + forkOwner: 'team', + forkRepo: 'editor-spoon', + forkUrl: 'https://github.com/team/editor-spoon', + visibility: 'private', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + await ctx.db.insert('spoonAgentSettings', { + spoonId, + ownerId, + enabled: options.settingsEnabled ?? true, + runtime: 'opencode', + defaultBaseBranch: 'main', + branchPrefix: 'spoon/', + agentModel: 'gpt-5.1-codex', + reasoningEffort: 'medium', + maxJobDurationMs: 60_000, + maxOutputBytes: 1_000_000, + createdAt: now, + updatedAt: now, + }); + const profileKind = options.profile ?? 'none'; + const aiProviderProfileId = + profileKind === 'none' + ? undefined + : await ctx.db.insert('aiProviderProfiles', { + ownerId, + name: 'Test provider', + provider: 'openai', + authType: + profileKind === 'missing-credentials' ? 'api_key' : 'none', + defaultModel: 'gpt-5.1-codex', + modelOptions: ['gpt-5.1-codex'], + reasoningEffort: 'medium', + enabled: profileKind !== 'disabled', + createdAt: now, + updatedAt: now, + }); + const selectedSecretIds: Id<'spoonSecrets'>[] = []; + if (options.secret) { + const secretOwnerId = + options.secret === 'foreign' + ? await ctx.db.insert('users', { + email: 'other@example.com', + name: 'Other', + }) + : ownerId; + selectedSecretIds.push( + await ctx.db.insert('spoonSecrets', { + spoonId, + ownerId: secretOwnerId, + name: 'TOKEN', + encryptedValue: 'encrypted', + createdAt: now, + updatedAt: now, + }), + ); + } + const requestId = await ctx.db.insert('agentRequests', { + spoonId, + ownerId, + prompt: 'Fix the editor', + status: 'queued', + createdAt: now, + updatedAt: now, + }); + const jobId = await ctx.db.insert('agentJobs', { + spoonId, + ownerId, + agentRequestId: requestId, + status: 'queued', + prompt: 'Fix the editor', + runtime: options.runtime ?? 'opencode', + workspaceStatus: 'not_started', + baseBranch: 'main', + workBranch: 'spoon/fix-editor', + forkOwner: 'team', + forkRepo: 'editor-spoon', + forkUrl: 'https://github.com/team/editor-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'editor', + selectedSecretIds, + aiProviderProfileId, + model: 'gpt-5.1-codex', + reasoningEffort: 'medium', + createdAt: now, + updatedAt: now, + }); + await ctx.db.patch(requestId, { agentJobId: jobId }); + return { jobId, requestId, spoonId }; + }); + +const expectFailedClaim = async ( + t: ReturnType, + jobId: Id<'agentJobs'>, +) => { + const result = await t.mutation(internal.agentJobs.claimNextInternal, { + workerId: 'w1', + }); + expect(result).toBeNull(); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + expect(job?.status).toBe('failed'); + expect(job?.error).toBeTruthy(); +}; + +describe('claimNextInternal validation', () => { + test('fails a queued job when its owner has no configured profile', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t); + + await expectFailedClaim(t, jobId); + }); + + test('claims a fully valid queued job', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { profile: 'valid' }); + + const result = await t.mutation(internal.agentJobs.claimNextInternal, { + workerId: 'w1', + }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + + expect(result).not.toBeNull(); + expect(result?.aiProviderProfile?.enabled).toBe(true); + expect(job?.status).toBe('claimed'); + }); + + test('fails a queued job when its profile is disabled', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { profile: 'disabled' }); + + await expectFailedClaim(t, jobId); + }); + + test('fails a queued job when its profile is missing credentials', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { + profile: 'missing-credentials', + }); + + await expectFailedClaim(t, jobId); + }); + + test('fails a queued job when Spoon agent jobs are disabled', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { + profile: 'valid', + settingsEnabled: false, + }); + + await expectFailedClaim(t, jobId); + }); + + test('fails a queued job with an unsupported legacy runtime', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { + profile: 'valid', + runtime: 'openai_direct', + }); + + await expectFailedClaim(t, jobId); + }); + + test('fails a queued job with a secret owned by another user', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { + profile: 'valid', + secret: 'foreign', + }); + + await expectFailedClaim(t, jobId); + }); + + test('fails a queued job whose Spoon was deleted', async () => { + const t = convexTest(schema, modules); + const { jobId, spoonId } = await seedQueuedJob(t, { profile: 'valid' }); + await t.mutation(async (ctx) => await ctx.db.delete(spoonId)); + + await expectFailedClaim(t, jobId); + }); +}); From 1e1435e656307be423c7bba7bd85bb949677f180 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:24:15 -0400 Subject: [PATCH 20/88] fix(agent-jobs): recover failed claim decryption --- packages/backend/convex/agentJobs.ts | 107 +++++++---- packages/backend/convex/agentJobsNode.ts | 27 ++- .../tests/unit/claim-validation.test.ts | 170 +++++++++++++++--- 3 files changed, 238 insertions(+), 66 deletions(-) diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 0572de4..46537b1 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -294,6 +294,40 @@ const deleteWorkspaceRows = async (ctx: MutationCtx, job: Doc<'agentJobs'>) => { await ctx.db.delete(job._id); }; +const failJobClaim = async ( + ctx: MutationCtx, + job: Doc<'agentJobs'>, + 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, + }); +}; + const getAgentSettings = async (ctx: MutationCtx, spoon: Doc<'spoons'>) => { const settings = await ctx.db .query('spoonAgentSettings') @@ -1078,59 +1112,48 @@ export const claimNextInternal = internalMutation({ aiProviderProfile?.ownerId === job.ownerId && aiProviderProfile.enabled ? 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.'); + await failJobClaim( + ctx, + job, + 'The Spoon for this job no longer exists.', + ); return null; } if (!ownedProfile) { - await failClaim( + await failJobClaim( + ctx, + job, '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.'); + await failJobClaim( + ctx, + job, + 'Selected AI provider is missing credentials.', + ); return null; } if (agentSettings && !agentSettings.enabled) { - await failClaim('Agent jobs are disabled for this Spoon.'); + await failJobClaim(ctx, job, 'Agent jobs are disabled for this Spoon.'); return null; } if ((job.runtime ?? 'opencode') !== 'opencode') { - await failClaim('Legacy OpenAI direct jobs are no longer supported.'); + await failJobClaim( + ctx, + job, + 'Legacy OpenAI direct jobs are no longer supported.', + ); return null; } if (secrets.length !== job.selectedSecretIds.length) { - await failClaim('Selected secrets must belong to this Spoon.'); + await failJobClaim( + ctx, + job, + 'Selected secrets must belong to this Spoon.', + ); return null; } const now = Date.now(); @@ -1164,6 +1187,20 @@ export const claimNextInternal = internalMutation({ }, }); +export const failClaimInternal = internalMutation({ + args: { + jobId: v.id('agentJobs'), + workerId: v.string(), + reason: v.string(), + }, + handler: async (ctx, { jobId, workerId, reason }) => { + const job = await ctx.db.get(jobId); + if (job?.status !== 'claimed' || job.claimedBy !== workerId) return null; + await failJobClaim(ctx, job, reason); + return null; + }, +}); + export const updateStatus = mutation({ args: { workerToken: v.string(), diff --git a/packages/backend/convex/agentJobsNode.ts b/packages/backend/convex/agentJobsNode.ts index 4c36a0a..6927c48 100644 --- a/packages/backend/convex/agentJobsNode.ts +++ b/packages/backend/convex/agentJobsNode.ts @@ -58,6 +58,24 @@ export const claimNextForWorker = action({ if (!claimed) return null; if (!claimed.spoon || !claimed.aiProviderProfile) return null; const profile = claimed.aiProviderProfile; + let profileSecret: string | undefined; + let secrets: { name: string; value: string }[]; + try { + profileSecret = profile.encryptedSecret + ? decryptSecret(profile.encryptedSecret) + : undefined; + secrets = claimed.secrets.map((secret: Doc<'spoonSecrets'>) => ({ + name: secret.name, + value: decryptSecret(secret.encryptedValue), + })); + } catch { + await ctx.runMutation(internal.agentJobs.failClaimInternal, { + jobId: claimed.job._id, + workerId: args.workerId, + reason: 'Stored AI credentials or Spoon secrets could not be decrypted.', + }); + return null; + } return { job: claimed.job, spoon: claimed.spoon, @@ -70,9 +88,7 @@ export const claimNextForWorker = action({ name: profile.name, provider: profile.provider, authType: profile.authType, - secret: profile.encryptedSecret - ? decryptSecret(profile.encryptedSecret) - : undefined, + secret: profileSecret, baseUrl: profile.baseUrl, model: profile.defaultModel, reasoningEffort: profile.reasoningEffort, @@ -84,10 +100,7 @@ export const claimNextForWorker = action({ claimed.spoon.githubInstallationId ?? process.env.GITHUB_APP_INSTALLATION_ID, }, - secrets: claimed.secrets.map((secret: Doc<'spoonSecrets'>) => ({ - name: secret.name, - value: decryptSecret(secret.encryptedValue), - })), + secrets, }; }, }); diff --git a/packages/backend/tests/unit/claim-validation.test.ts b/packages/backend/tests/unit/claim-validation.test.ts index 8d2f461..78470e0 100644 --- a/packages/backend/tests/unit/claim-validation.test.ts +++ b/packages/backend/tests/unit/claim-validation.test.ts @@ -1,17 +1,24 @@ import { convexTest } from 'convex-test'; -import { describe, expect, test } from 'vitest'; +import { beforeAll, describe, expect, test } from 'vitest'; import type { Id } from '../../convex/_generated/dataModel.js'; -import { internal } from '../../convex/_generated/api.js'; +import { api, internal } from '../../convex/_generated/api.js'; import schema from '../../convex/schema'; +import { encryptSecret } from '../../convex/secretCrypto'; const modules = import.meta.glob('../../convex/**/*.*s'); type SeedOptions = { - profile?: 'valid' | 'disabled' | 'missing-credentials' | 'none'; + profile?: + | 'valid' + | 'disabled' + | 'missing-credentials' + | 'encrypted-valid' + | 'encrypted-malformed' + | 'none'; settingsEnabled?: boolean; runtime?: 'opencode' | 'openai_direct'; - secret?: 'valid' | 'foreign'; + secret?: 'valid' | 'foreign' | 'malformed'; }; const seedQueuedJob = async ( @@ -66,7 +73,16 @@ const seedQueuedJob = async ( name: 'Test provider', provider: 'openai', authType: - profileKind === 'missing-credentials' ? 'api_key' : 'none', + profileKind === 'missing-credentials' || + profileKind.startsWith('encrypted-') + ? 'api_key' + : 'none', + encryptedSecret: + profileKind === 'encrypted-valid' + ? encryptSecret('provider-secret') + : profileKind === 'encrypted-malformed' + ? 'malformed' + : undefined, defaultModel: 'gpt-5.1-codex', modelOptions: ['gpt-5.1-codex'], reasoningEffort: 'medium', @@ -88,7 +104,10 @@ const seedQueuedJob = async ( spoonId, ownerId: secretOwnerId, name: 'TOKEN', - encryptedValue: 'encrypted', + encryptedValue: + options.secret === 'malformed' + ? 'malformed' + : encryptSecret('secret-value'), createdAt: now, updatedAt: now, }), @@ -102,10 +121,22 @@ const seedQueuedJob = async ( createdAt: now, updatedAt: now, }); + const threadId = await ctx.db.insert('threads', { + ownerId, + spoonId, + title: 'Fix the editor', + source: 'user_request', + status: 'queued', + priority: 'normal', + relatedAgentRequestId: requestId, + createdAt: now, + updatedAt: now, + }); const jobId = await ctx.db.insert('agentJobs', { spoonId, ownerId, agentRequestId: requestId, + threadId, status: 'queued', prompt: 'Fix the editor', runtime: options.runtime ?? 'opencode', @@ -125,28 +156,64 @@ const seedQueuedJob = async ( updatedAt: now, }); await ctx.db.patch(requestId, { agentJobId: jobId }); - return { jobId, requestId, spoonId }; + await ctx.db.patch(threadId, { latestAgentJobId: jobId }); + return { jobId, requestId, spoonId, threadId }; }); +const expectFailedClaimState = async ( + t: ReturnType, + jobId: Id<'agentJobs'>, + requestId: Id<'agentRequests'>, + threadId: Id<'threads'>, +) => { + const state = await t.run(async (ctx) => ({ + job: await ctx.db.get(jobId), + request: await ctx.db.get(requestId), + thread: await ctx.db.get(threadId), + events: (await ctx.db.query('agentJobEvents').collect()).filter( + (event) => event.jobId === jobId, + ), + })); + expect(state.job?.status).toBe('failed'); + expect(state.job?.error).toBeTruthy(); + expect(state.request?.status).toBe('failed'); + expect(state.thread?.status).toBe('failed'); + expect(state.thread?.resolvedAt).toBeTypeOf('number'); + expect(state.events).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + level: 'error', + phase: 'queued', + message: expect.stringContaining('Job could not be claimed:'), + }), + ]), + ); +}; + const expectFailedClaim = async ( t: ReturnType, jobId: Id<'agentJobs'>, + requestId: Id<'agentRequests'>, + threadId: Id<'threads'>, ) => { const result = await t.mutation(internal.agentJobs.claimNextInternal, { workerId: 'w1', }); expect(result).toBeNull(); - const job = await t.run(async (ctx) => await ctx.db.get(jobId)); - expect(job?.status).toBe('failed'); - expect(job?.error).toBeTruthy(); + await expectFailedClaimState(t, jobId, requestId, threadId); }; +beforeAll(() => { + process.env.SPOON_WORKER_TOKEN = 'test-worker-token'; + process.env.SPOON_ENCRYPTION_KEY = 'test-encryption-key'; +}); + describe('claimNextInternal validation', () => { test('fails a queued job when its owner has no configured profile', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t); + const { jobId, requestId, threadId } = await seedQueuedJob(t); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('claims a fully valid queued job', async () => { @@ -165,55 +232,110 @@ describe('claimNextInternal validation', () => { test('fails a queued job when its profile is disabled', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { profile: 'disabled' }); + const { jobId, requestId, threadId } = await seedQueuedJob(t, { + profile: 'disabled', + }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job when its profile is missing credentials', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'missing-credentials', }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job when Spoon agent jobs are disabled', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'valid', settingsEnabled: false, }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job with an unsupported legacy runtime', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'valid', runtime: 'openai_direct', }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job with a secret owned by another user', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'valid', secret: 'foreign', }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job whose Spoon was deleted', async () => { const t = convexTest(schema, modules); - const { jobId, spoonId } = await seedQueuedJob(t, { profile: 'valid' }); + const { jobId, requestId, spoonId, threadId } = await seedQueuedJob(t, { + profile: 'valid', + }); await t.mutation(async (ctx) => await ctx.db.delete(spoonId)); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); + }); + + test('fails a claim when provider credential decryption fails', async () => { + const t = convexTest(schema, modules); + const { jobId, requestId, threadId } = await seedQueuedJob(t, { + profile: 'encrypted-malformed', + }); + + const result = await t.action(api.agentJobsNode.claimNextForWorker, { + workerId: 'w1', + workerToken: 'test-worker-token', + }); + + expect(result).toBeNull(); + await expectFailedClaimState(t, jobId, requestId, threadId); + }); + + test('fails a claim when selected secret decryption fails', async () => { + const t = convexTest(schema, modules); + const { jobId, requestId, threadId } = await seedQueuedJob(t, { + profile: 'valid', + secret: 'malformed', + }); + + const result = await t.action(api.agentJobsNode.claimNextForWorker, { + workerId: 'w1', + workerToken: 'test-worker-token', + }); + + expect(result).toBeNull(); + await expectFailedClaimState(t, jobId, requestId, threadId); + }); + + test('returns decrypted provider credentials and selected secrets', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { + profile: 'encrypted-valid', + secret: 'valid', + }); + + const result = await t.action(api.agentJobsNode.claimNextForWorker, { + workerId: 'w1', + workerToken: 'test-worker-token', + }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + + expect(result?.aiProviderProfile?.secret).toBe('provider-secret'); + expect(result?.secrets).toEqual([ + { name: 'TOKEN', value: 'secret-value' }, + ]); + expect(job?.status).toBe('claimed'); }); }); From 00f96a126e3356fde8d50322295c35f0166b1ad1 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:27:07 -0400 Subject: [PATCH 21/88] fix(agent-jobs): close maintenance jobs after decision --- apps/agent-worker/src/worker.ts | 4 + packages/backend/convex/agentJobs.ts | 6 + .../tests/unit/maintenance-decision.test.ts | 112 ++++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 packages/backend/tests/unit/maintenance-decision.test.ts diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index a5f6f15..5426d6f 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -1765,6 +1765,10 @@ export const sendWorkspaceMessage = async ( contentType: 'application/json', }); await applyMaintenanceDecision(claim.job._id, decision); + await stopWorkspace(claim.job._id).catch((error: unknown) => { + console.error(error); + }); + return; } else { await updateStatus(claim.job._id, 'changes_ready', { summary: diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 46537b1..f820f02 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -1540,6 +1540,12 @@ export const applyMaintenanceDecision = mutation({ } if (!job.threadId) return { success: true }; const now = Date.now(); + await ctx.db.patch(args.jobId, { + workspaceStatus: 'stopped', + summary: args.summary, + completedAt: job.completedAt ?? now, + updatedAt: now, + }); const outcome = args.decision === 'sync' ? 'sync_recommended' diff --git a/packages/backend/tests/unit/maintenance-decision.test.ts b/packages/backend/tests/unit/maintenance-decision.test.ts new file mode 100644 index 0000000..c8a7893 --- /dev/null +++ b/packages/backend/tests/unit/maintenance-decision.test.ts @@ -0,0 +1,112 @@ +import { convexTest } from 'convex-test'; +import { afterAll, beforeAll, describe, expect, test } 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'; +}); + +afterAll(() => { + delete process.env.SPOON_WORKER_TOKEN; +}); + +describe('applyMaintenanceDecision', () => { + test('resolves a low-risk sync and stops its maintenance job', async () => { + const t = convexTest(schema, modules); + const { jobId, threadId } = await t.mutation(async (ctx) => { + const now = Date.now(); + const ownerId = await ctx.db.insert('users', { + email: 'maintenance@example.com', + name: 'Maintenance Reviewer', + }); + const spoonId = await ctx.db.insert('spoons', { + ownerId, + name: 'Maintenance Spoon', + provider: 'gitea', + upstreamOwner: 'upstream', + upstreamRepo: 'maintenance', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://git.example.com/upstream/maintenance', + forkOwner: 'team', + forkRepo: 'maintenance-spoon', + forkUrl: 'https://git.example.com/team/maintenance-spoon', + visibility: 'private', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + const requestId = await ctx.db.insert('agentRequests', { + spoonId, + ownerId, + prompt: 'Review upstream maintenance changes', + requestType: 'upstream_review', + status: 'running', + createdAt: now, + updatedAt: now, + }); + const threadId = await ctx.db.insert('threads', { + ownerId, + spoonId, + title: 'Upstream maintenance review', + source: 'upstream_update', + status: 'running', + priority: 'normal', + relatedAgentRequestId: requestId, + createdAt: now, + updatedAt: now, + }); + const jobId = await ctx.db.insert('agentJobs', { + spoonId, + ownerId, + agentRequestId: requestId, + threadId, + jobType: 'maintenance_review', + status: 'running', + workspaceStatus: 'active', + prompt: 'Review upstream maintenance changes', + runtime: 'opencode', + baseBranch: 'main', + workBranch: 'spoon/maintenance-review', + forkOwner: 'team', + forkRepo: 'maintenance-spoon', + forkUrl: 'https://git.example.com/team/maintenance-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'maintenance', + selectedSecretIds: [], + model: '', + reasoningEffort: 'medium', + claimedBy: 'w1', + createdAt: now, + updatedAt: now, + }); + return { jobId, threadId }; + }); + + await t.mutation(api.agentJobs.applyMaintenanceDecision, { + workerToken: 'test-worker-token', + workerId: 'w1', + jobId, + decision: 'sync', + risk: 'low', + summary: 'Safe to sync automatically.', + ignoredCommitShas: [], + recommendedAction: 'Sync the upstream changes.', + requiresUserApproval: false, + }); + + const { job, thread } = await t.run(async (ctx) => ({ + job: await ctx.db.get(jobId), + thread: await ctx.db.get(threadId), + })); + expect(thread?.status).toBe('resolved'); + expect(job?.workspaceStatus).toBe('stopped'); + expect(job?.completedAt).toEqual(expect.any(Number)); + }); +}); From 8af33fc9e0fe42cc125d4cf40b30d18194913618 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:33:38 -0400 Subject: [PATCH 22/88] fix(worker): always release stopped workspaces --- apps/agent-worker/src/worker.ts | 43 +++- .../maintenance-workspace-cleanup.test.ts | 187 ++++++++++++++++++ 2 files changed, 222 insertions(+), 8 deletions(-) create mode 100644 apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 5426d6f..3c6bc3b 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -139,6 +139,13 @@ const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); const client = new ConvexHttpClient(env.convexUrl); const activeWorkspaces = new Map(); +export const _setActiveWorkspaceForTests = ( + jobId: string, + workspace: ActiveWorkspace, +) => activeWorkspaces.set(jobId, workspace); + +export const _resetActiveWorkspacesForTests = () => activeWorkspaces.clear(); + const appendEvent = async ( jobId: Id<'agentJobs'>, level: 'debug' | 'info' | 'warn' | 'error', @@ -1894,15 +1901,35 @@ export const openWorkspacePullRequest = async (jobId: string) => { export const stopWorkspace = async (jobId: string) => { const workspace = resolveWorkspace(jobId); - await markWorkspaceStopped(workspace.claim.job._id); - workspace.opencodeSession?.close(); - if (workspace.containerName) { - await stopWorkspaceContainer(workspace.containerName); + const errors: unknown[] = []; + try { + await markWorkspaceStopped(workspace.claim.job._id); + } catch (error) { + errors.push(error); + } + try { + workspace.opencodeSession?.close(); + } catch (error) { + errors.push(error); + } + try { + if (workspace.containerName) { + await stopWorkspaceContainer(workspace.containerName); + } + } catch (error) { + errors.push(error); + } finally { + activeWorkspaces.delete(jobId); + try { + workspace.boxHandle.release(); + } catch (error) { + errors.push(error); + } + } + if (errors.length === 1) throw errors[0]; + if (errors.length > 1) { + throw new AggregateError(errors, 'Workspace teardown failed.'); } - activeWorkspaces.delete(jobId); - // The persistent per-user home + ~/Code checkouts survive across sessions; - // release the box (reaped once no other thread/terminal holds it). - workspace.boxHandle.release(); return { success: true }; }; diff --git a/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts b/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts new file mode 100644 index 0000000..5e7c6b4 --- /dev/null +++ b/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts @@ -0,0 +1,187 @@ +import { getFunctionName } from 'convex/server'; +import { afterEach, describe, expect, test, vi } from 'vitest'; + +import type { Id } from '@spoon/backend/convex/_generated/dataModel.js'; + +const mocks = vi.hoisted(() => ({ + getWorktreeDiff: vi.fn(() => + Promise.resolve({ exitCode: 0, output: 'unexpected diff' }), + ), + mutation: vi.fn(), + stopWorkspaceContainer: vi.fn(() => Promise.resolve()), + streamExecInContainer: vi.fn(), +})); + +vi.mock('convex/browser', () => ({ + ConvexHttpClient: vi.fn( + class ConvexHttpClient { + mutation = mocks.mutation; + }, + ), +})); +vi.mock('../../src/env', () => ({ + env: { + buildCreatedAt: 'test', + buildSha: 'test', + containerAccess: 'network', + convexUrl: 'http://convex.test', + jobTimeoutMs: 1000, + runtime: 'docker', + workerId: 'worker-1', + workerToken: 'token', + }, +})); +vi.mock('../../src/git', () => ({ + cloneRepository: vi.fn(), + commitAndPush: vi.fn(), + getStatus: vi.fn(), + getWorktreeDiff: mocks.getWorktreeDiff, + run: vi.fn(), +})); +vi.mock('../../src/github', () => ({ + getInstallationToken: vi.fn(), + openDraftPullRequest: vi.fn(), +})); +vi.mock('../../src/opencode-session', () => ({ + abortOpenCodeSession: vi.fn(), + createOpenCodeSession: vi.fn(), + promptOpenCodeSession: vi.fn(), + replyOpenCodePermission: vi.fn(), +})); +vi.mock('../../src/runtime/docker', () => ({ + listWorkspaceContainerNames: vi.fn(() => Promise.resolve([])), + runExecInContainer: vi.fn(), + startWorkspaceContainer: vi.fn(), + stopWorkspaceContainer: mocks.stopWorkspaceContainer, + streamExecInContainer: mocks.streamExecInContainer, +})); +vi.mock('../../src/user-container', () => ({ + acquireUserBox: vi.fn(), + runningBoxUsernames: vi.fn(() => new Set()), +})); +vi.mock('../../src/user-environment', () => ({ + fetchUserEnvironment: vi.fn(), + materializeUserHome: vi.fn(), +})); + +const decision = { + decision: 'sync', + risk: 'low', + summary: 'Safe to sync.', + ignoredCommitShas: [], + ignoredReason: '', + recommendedAction: 'Sync upstream.', + requiresUserApproval: false, +}; + +const load = async () => await import('../../src/worker'); + +afterEach(async () => { + const worker = await load(); + worker._resetActiveWorkspacesForTests(); + vi.restoreAllMocks(); + vi.clearAllMocks(); +}); + +describe('maintenance workspace cleanup', () => { + test.each(['mark', 'container'] as const)( + 'releases and removes the workspace when %s teardown fails', + async (failingStep) => { + const worker = await load(); + const release = vi.fn(); + const close = vi.fn(); + const jobId = `maintenance-${failingStep}`; + mocks.mutation.mockImplementation((reference) => { + if ( + failingStep === 'mark' && + getFunctionName(reference) === 'agentJobs:markWorkspaceStopped' + ) { + return Promise.reject(new Error('status teardown failed')); + } + return Promise.resolve('message-1'); + }); + mocks.stopWorkspaceContainer.mockImplementation(() => + failingStep === 'container' + ? Promise.reject(new Error('container teardown failed')) + : Promise.resolve(), + ); + mocks.streamExecInContainer.mockImplementation(async (args) => { + await args.onStdoutLine( + JSON.stringify({ + type: 'item.completed', + item: { + type: 'agent_message', + text: JSON.stringify(decision), + }, + }), + ); + return { exitCode: 0, output: '' }; + }); + worker._setActiveWorkspaceForTests(jobId, { + claim: { + job: { + _id: jobId as Id<'agentJobs'>, + prompt: 'Review maintenance changes', + jobType: 'maintenance_review', + baseBranch: 'main', + workBranch: 'spoon/maintenance', + forkOwner: 'team', + forkRepo: 'spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'spoon', + }, + spoon: { name: 'Spoon' }, + openai: { + model: 'gpt-5', + reasoningEffort: 'medium', + }, + aiProviderProfile: { + id: 'profile-1', + name: 'Codex', + provider: 'opencode_openai_login', + authType: 'opencode_auth_json', + model: 'gpt-5', + reasoningEffort: 'medium', + }, + github: {}, + secrets: [], + }, + workdir: '/tmp/workspace', + homeDir: '/tmp/workspace', + username: 'tester', + containerHome: '/home/tester', + containerRepo: '/home/tester/Code/spoon', + repoDir: '/tmp/workspace/Code/spoon', + boxName: 'spoon-box-tester', + boxHandle: { boxName: 'spoon-box-tester', release }, + githubToken: 'github-token', + redact: (value: string) => value, + containerName: 'spoon-agent-job-maintenance', + opencodeSession: { + client: {} as never, + sessionId: 'session-1', + close, + }, + }); + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + await expect( + worker.sendWorkspaceMessage(jobId, 'Review maintenance changes'), + ).resolves.toBeUndefined(); + + expect(release).toHaveBeenCalledTimes(1); + expect(close).toHaveBeenCalledTimes(1); + expect(mocks.stopWorkspaceContainer).toHaveBeenCalledTimes(1); + expect(mocks.getWorktreeDiff).not.toHaveBeenCalled(); + expect( + mocks.mutation.mock.calls.some(([, args]) => args.kind === 'diff'), + ).toBe(false); + await expect(worker.stopWorkspace(jobId)).rejects.toThrow( + 'Agent workspace is not active on this worker.', + ); + expect(consoleError).toHaveBeenCalledTimes(1); + }, + ); +}); From cbbb168e7cb2e3ce1c2ab1ebe23685c3293dc6a2 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:54:06 -0400 Subject: [PATCH 23/88] docs: add implementation handoff for Claude --- docs/superpowers/CLAUDE-HANDOFF-2026-07-10.md | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 docs/superpowers/CLAUDE-HANDOFF-2026-07-10.md diff --git a/docs/superpowers/CLAUDE-HANDOFF-2026-07-10.md b/docs/superpowers/CLAUDE-HANDOFF-2026-07-10.md new file mode 100644 index 0000000..2e914ee --- /dev/null +++ b/docs/superpowers/CLAUDE-HANDOFF-2026-07-10.md @@ -0,0 +1,202 @@ +# Spoon implementation handoff for Claude + +## Objective + +Continue and complete every task in the five plans indexed by +`docs/superpowers/plans/2026-07-10-spoon-implementation-plan-index.md`. +The plans contain 63 tasks and must be executed in phase order. Use the +project `AGENTS.md`, test-first implementation, a task-scoped review after +each task, and a whole-branch review at the end. + +## Canonical workspace + +- Integration worktree: `/home/gib/.codex/worktrees/Spoon/spoon-stabilize-unify-build` +- Integration branch: `codex/spoon-stabilize-unify-build` +- Current integration HEAD before this handoff commit: `8af33fc` +- Original checkout: `/home/gib/Documents/Code/Spoon`, still on `main` at + `8aa9140`. Do not implement in the original checkout. +- `main` was already two commits ahead of `origin/main` when work began. + Preserve it and the original checkout's untracked `.agents/` directory. + +## Current state + +Phase 1 Tasks 1-5 and 7-13 are implemented, independently reviewed, fixed +where required, integrated, and verified with focused static/unit checks. +The durable scratch ledger is +`/home/gib/.codex/worktrees/Spoon/spoon-stabilize-unify-build/.superpowers/sdd/progress.md`. + +Integrated Phase 1 commits, newest first: + +```text +8af33fc fix(worker): always release stopped workspaces +00f96a1 fix(agent-jobs): close maintenance jobs after decision +1e1435e fix(agent-jobs): recover failed claim decryption +cf45404 fix(agent-jobs): validate claims before claiming +926008f fix(workspace): skip queries for missing jobs +2beeeb1 fix(app): handle nullable detail consumers +ed21126 fix(app): add friendly not-found handling +c4d0f52 fix(terminal): use real TTY exec and resize +906f906 fix(backend): use generated import extension +bdb2964 fix(sync): honor per-spoon auto-sync settings +57c175a fix(worker): contain workspace paths against symlinks +2bc090c chore: remove terminal task scratch report +d4a3856 fix(workspace): key shell by job +020ffbf fix(worker-auth): unify token checks and bind job environment +feb3723 fix(terminal): preserve animation-frame first fit +75d0639 fix(terminal): refit after fonts load +0b5a2c0 fix(worker): ensure reconciled boxes before acquire +284f7ea fix(worker): serialize box failure and idle reaping +490c053 chore: remove implementation scratch report +0a4461e fix(agent-jobs): prevent terminal status resurrection +9e7a872 fix(worker): make user-box lifecycle concurrency-safe +daec4d6 fix(worker): make workspace cleanup layout-aware +``` + +## Immediate next action: finish Phase 1 Task 6 + +Task 6 is complete on its isolated branch/worktree but has not had its final +post-fix review and has not been integrated. + +- Worktree: `/home/gib/.codex/worktrees/Spoon/phase1-task6` +- Branch: `codex/phase1-task6` +- Commits to review and integrate: + - `bdebacc fix(agent-jobs): add heartbeat cancel and stale recovery` + - `233032a fix(worker): make cancellation teardown failure-safe` +- Report: `/tmp/spoon-phase1-task6-report.md` +- Requirements brief: + `/home/gib/.codex/worktrees/Spoon/spoon-stabilize-unify-build/.superpowers/sdd/task-6-brief.md` + +The first review found that Codex was not actually killed on cancellation and +that fallible event/status operations could bypass timer and ownership cleanup. +Commit `233032a` addresses those findings with: + +- a job-specific in-box Codex process session; +- PID plus `/proc//environ` marker validation before isolated TERM/KILL; +- serialized heartbeat cancellation so ticks cannot overlap; +- failure-safe explicit stop, draft-PR, cancellation, and claim-failure cleanup; +- worker regressions for Codex kill, duplicate ticks, and teardown rejection; +- expanded backend stale-recovery matrix tests. + +Reported checks on that exact branch: worker format/lint/typecheck and 51/51 +unit tests pass; backend 47/47 unit tests pass; stale-recovery matrix 8/8. +The timeout-not-requeue policy is explicitly documented as the plan's deliberate +deferral. Live Docker/Podman isolation remains untested. + +Resume sequence: + +1. Dispatch a fresh read-only reviewer for Task 6 over both commits and the + updated report. Recheck the two prior findings, job-specific kill safety, + timer serialization, all teardown paths, recovery thresholds/statuses, and + tests. +2. Fix and re-review any Critical/Important finding. +3. Cherry-pick `bdebacc` and `233032a` into the integration worktree. Expect + possible conflicts in `worker.ts` and `agentJobs.ts` because Task 7 was + integrated afterward. Preserve Task 7's `finally`-based workspace release + semantics while retaining Task 6's heartbeat cleanup. +4. Run fresh integrated worker focused/full tests and typecheck; run backend + focused tests using the generated-file workaround below. +5. Append Task 6 to `.superpowers/sdd/progress.md`, mark Phase 1 complete, and + perform a broad Phase 1 review before starting Phase 2. + +## Remaining work + +After Task 6, implement all tasks in these plans in order: + +1. `docs/superpowers/plans/2026-07-10-spoon-phase2-runtime-unification.md` + — 12 tasks. +2. `docs/superpowers/plans/2026-07-10-spoon-phase3-dev-box-surface.md` + — 11 tasks. +3. `docs/superpowers/plans/2026-07-10-spoon-phase4-sync-correctness.md` + — 12 tasks. +4. `docs/superpowers/plans/2026-07-10-spoon-phase5-notifications-polish.md` + — 15 tasks. + +The Phase 2 plan has been preflight-read and no conflict with the implemented +Phase 1 APIs was identified. Do not overwrite Phase 1 task briefs in +`.superpowers/sdd/` until Task 6 is fully reviewed and integrated; the +`task-brief` helper uses generic task numbers. + +## Required execution/review workflow + +- Use a fresh implementer per task and a fresh task-scoped reviewer after each + task. Fix all Critical and Important findings, then re-review. +- Isolate concurrent tasks in separate worktrees/branches. Never let agents + commit concurrently in the same worktree. +- Before cherry-picking an isolated commit, inspect `git show --stat` and ensure + it contains only project files. Two earlier agents accidentally committed + `.superpowers/sdd/task-*-report.md`; those scratch reports were removed by + `490c053` and `2bc090c`. Put future reports under `/tmp`. +- After review approval, cherry-pick into the canonical integration worktree, + run fresh controller-side focused checks, and update the scratch ledger. +- Preserve unrelated changes and never edit + `packages/backend/convex/_generated/**` or `packages/backend/convex/http.ts`. +- Commits must use a simple `git commit -m "message"` with no AI attribution. +- Use conventional commit messages and minimal comments. +- Run `bun typecheck`, not a production build, for routine validation. + +## Environment and validation limitations + +The local server, local database, Docker-backed worker, and Convex deployment +are unavailable. Continue implementation and static/unit checks as requested; +record live/manual checks for later. + +Root `bun install --frozen-lockfile` fails on the untouched baseline because +Bun 1.3.10 wants to rewrite the lockfile. The integration worktree currently +uses a root `node_modules` symlink to the original checkout. Do not commit any +dependency artifacts or lockfile rewrite merely to work around this. + +Convex codegen cannot run without `CONVEX_SELF_HOSTED_URL` plus admin key or +`CONVEX_DEPLOYMENT`. For backend unit tests/typecheck that do not require newly +generated API declarations, temporarily link the ignored generated snapshot: + +```bash +cd packages/backend +ln -s /home/gib/Documents/Code/Spoon/packages/backend/convex/_generated convex/_generated +bun run test:unit -- +bun run typecheck +rm convex/_generated +``` + +Never commit that symlink and do not claim fresh codegen succeeded. New API +typing may remain unavailable until deployment credentials return. + +For Next component tests in a worktree, temporarily make the Vitest workspace +package resolve inside that worktree: + +```bash +mkdir -p apps/next/node_modules/@spoon +ln -s ../../../../tools/vitest apps/next/node_modules/@spoon/vitest-config +cd apps/next +bun run test:component +cd ../.. +rm apps/next/node_modules/@spoon/vitest-config +rm -r apps/next/node_modules/.vite apps/next/node_modules/.vite-temp 2>/dev/null || true +rmdir apps/next/node_modules/@spoon apps/next/node_modules 2>/dev/null || true +``` + +## Known deferred/manual checks + +- Inspect a live `spoon-box-*` and confirm Docker `HostConfig.Init` is true. +- Verify real terminal resize, glyph rendering, and tmux reconnect in a browser. +- Verify Task 6 cancellation kills only the selected job's Codex process group + under real Docker and Podman. +- Run each phase's plan-specific live/manual checks when the local stack returns. +- Task 9 has one non-blocking review note: worker-auth tests assert + `ConvexError` type but not both exact error strings. Consider covering this + during final review. +- The realpath containment design has the documented validation-to-I/O TOCTOU + window; descriptor-relative elimination was outside the approved plan. + +## Final completion requirements + +After all 63 tasks: + +1. Run a broad whole-branch review against the design spec and all five plans. +2. Fix all Critical/Important findings in one coordinated pass and re-review. +3. When the environment is available, run `bun typecheck`, + `SKIP_E2E=1 bun run ci:check`, `bun test:e2e`, container smoke, and the + documented live runtime/manual checks. Until then, report them as deferred, + not passing. +4. Use the finishing-development-branch workflow; do not merge, push, or open a + PR unless the user explicitly requests it. + From 470af043fa16b8cb45a6c7cc1fbfadc5edd58da9 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:38:31 -0400 Subject: [PATCH 24/88] fix(agent-jobs): add heartbeat cancel and stale recovery --- apps/agent-worker/src/worker.ts | 51 +++++++ packages/backend/convex/agentJobs.ts | 48 ++++++- packages/backend/convex/crons.ts | 6 + .../tests/unit/recover-stale-jobs.test.ts | 130 ++++++++++++++++++ .../backend/tests/unit/update-status.test.ts | 32 +++++ 5 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 packages/backend/tests/unit/recover-stale-jobs.test.ts diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 3c6bc3b..9715b5a 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -262,6 +262,51 @@ const markWorkspaceStopped = async ( workspaceStatus, }); +const heartbeatWorkspaceMutation = async (jobId: Id<'agentJobs'>) => + (await client.mutation(api.agentJobs.heartbeatWorkspace, { + workerToken: env.workerToken, + workerId: env.workerId, + jobId, + })) as { success: true; cancelRequested: boolean }; + +const heartbeatTimers = new Map>(); + +const stopHeartbeat = (jobId: string) => { + const timer = heartbeatTimers.get(jobId); + if (timer) clearInterval(timer); + heartbeatTimers.delete(jobId); +}; + +const startHeartbeat = (jobId: Id<'agentJobs'>) => { + stopHeartbeat(jobId); + const timer = setInterval(() => { + void (async () => { + try { + const result = await heartbeatWorkspaceMutation(jobId); + if (result.cancelRequested && activeWorkspaces.has(jobId)) { + await appendEvent( + jobId, + 'warn', + 'cleanup', + 'Cancellation requested; stopping workspace.', + ); + await abortWorkspaceAgent(jobId).catch((error: unknown) => { + console.error(error); + }); + await stopWorkspace(jobId).catch((error: unknown) => { + console.error(error); + }); + stopHeartbeat(jobId); + } + } catch (error) { + console.error(error); + } + })(); + }, 30_000); + timer.unref(); + heartbeatTimers.set(jobId, timer); +}; + const appendMessage = async (args: { jobId: Id<'agentJobs'>; role: 'user' | 'assistant' | 'system' | 'tool'; @@ -1416,6 +1461,7 @@ const runClaim = async (claim: Claim) => { }); activeWorkspaces.set(jobId, workspace); await markWorkspaceActive({ jobId }); + startHeartbeat(jobId); await updateStatus(jobId, 'running', { summary: 'Workspace is active.', }); @@ -1463,6 +1509,7 @@ const runClaim = async (claim: Claim) => { ).catch((stopError: unknown) => { console.error(stopError); }); + stopHeartbeat(jobId); acquiredBoxHandle?.release(); } }; @@ -1889,6 +1936,7 @@ export const openWorkspacePullRequest = async (jobId: string) => { if (workspace.containerName) { await stopWorkspaceContainer(workspace.containerName); } + stopHeartbeat(jobId); activeWorkspaces.delete(jobId); // The persistent per-user home + ~/Code checkouts survive across sessions; // release the box (reaped once no other thread/terminal holds it). @@ -1901,6 +1949,9 @@ export const openWorkspacePullRequest = async (jobId: string) => { export const stopWorkspace = async (jobId: string) => { const workspace = resolveWorkspace(jobId); + // Stop the heartbeat timer before teardown so it never outlives the + // workspace even if a teardown step throws. + stopHeartbeat(jobId); const errors: unknown[] = []; try { await markWorkspaceStopped(workspace.claim.job._id); diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index f820f02..019578c 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -1201,6 +1201,52 @@ export const failClaimInternal = internalMutation({ }, }); +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 }; + }, +}); + export const updateStatus = mutation({ args: { workerToken: v.string(), @@ -1471,7 +1517,7 @@ export const heartbeatWorkspace = mutation({ lastHeartbeatAt: Date.now(), updatedAt: Date.now(), }); - return { success: true }; + return { success: true, cancelRequested: job.status === 'cancelled' }; }, }); diff --git a/packages/backend/convex/crons.ts b/packages/backend/convex/crons.ts index 0b65e24..070130e 100644 --- a/packages/backend/convex/crons.ts +++ b/packages/backend/convex/crons.ts @@ -12,6 +12,12 @@ crons.interval( limit: 10, }, ); +crons.interval( + 'Recover stale agent jobs', + { minutes: 1 }, + internal.agentJobs.recoverStaleJobs, + {}, +); /* Example cron jobs crons.cron( // Run at 7:00 AM CST / 8:00 AM CDT diff --git a/packages/backend/tests/unit/recover-stale-jobs.test.ts b/packages/backend/tests/unit/recover-stale-jobs.test.ts new file mode 100644 index 0000000..62285f9 --- /dev/null +++ b/packages/backend/tests/unit/recover-stale-jobs.test.ts @@ -0,0 +1,130 @@ +import { convexTest } from 'convex-test'; +import { describe, expect, test } from 'vitest'; + +import type { Id } from '../../convex/_generated/dataModel.js'; +import { internal } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; + +const modules = import.meta.glob('../../convex/**/*.*s'); + +const seedRunningJob = async ( + t: ReturnType, + lastHeartbeatAt: number, +) => + await t.mutation(async (ctx) => { + const now = Date.now(); + const ownerId = await ctx.db.insert('users', { + email: 'owner@example.com', + name: 'Owner', + }); + const spoonId = await ctx.db.insert('spoons', { + ownerId, + name: 'Recovery Spoon', + provider: 'github', + upstreamOwner: 'upstream', + upstreamRepo: 'recovery', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://github.com/upstream/recovery', + forkOwner: 'team', + forkRepo: 'recovery-spoon', + forkUrl: 'https://github.com/team/recovery-spoon', + visibility: 'private', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + const requestId = await ctx.db.insert('agentRequests', { + spoonId, + ownerId, + prompt: 'Recover the worker job', + status: 'running', + createdAt: now, + updatedAt: now, + }); + const threadId = await ctx.db.insert('threads', { + ownerId, + spoonId, + title: 'Recover the worker job', + source: 'user_request', + status: 'running', + priority: 'normal', + relatedAgentRequestId: requestId, + createdAt: now, + updatedAt: now, + }); + const jobId = await ctx.db.insert('agentJobs', { + spoonId, + ownerId, + agentRequestId: requestId, + threadId, + status: 'running', + prompt: 'Recover the worker job', + runtime: 'opencode', + workspaceStatus: 'active', + baseBranch: 'main', + workBranch: 'spoon/recover-worker-job', + forkOwner: 'team', + forkRepo: 'recovery-spoon', + forkUrl: 'https://github.com/team/recovery-spoon', + upstreamOwner: 'upstream', + upstreamRepo: 'recovery', + selectedSecretIds: [], + model: 'gpt-5.1-codex', + reasoningEffort: 'medium', + claimedBy: 'w1', + claimedAt: now, + lastHeartbeatAt, + createdAt: now, + updatedAt: now, + }); + await ctx.db.patch(requestId, { agentJobId: jobId }); + await ctx.db.patch(threadId, { latestAgentJobId: jobId }); + return { jobId, requestId, threadId }; + }); + +const readState = async ( + t: ReturnType, + ids: { + jobId: Id<'agentJobs'>; + requestId: Id<'agentRequests'>; + threadId: Id<'threads'>; + }, +) => + await t.run(async (ctx) => ({ + job: await ctx.db.get(ids.jobId), + request: await ctx.db.get(ids.requestId), + thread: await ctx.db.get(ids.threadId), + })); + +describe('recoverStaleJobs', () => { + test('times out a running job with a stale heartbeat', async () => { + const t = convexTest(schema, modules); + const ids = await seedRunningJob(t, Date.now() - 10 * 60 * 1000); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 1 }); + expect(state.job?.status).toBe('timed_out'); + expect(state.job?.completedAt).toBeTypeOf('number'); + expect(state.request?.status).toBe('failed'); + expect(state.thread?.status).toBe('failed'); + expect(state.thread?.resolvedAt).toBeTypeOf('number'); + }); + + test('leaves a running job with a fresh heartbeat untouched', async () => { + const t = convexTest(schema, modules); + const ids = await seedRunningJob(t, Date.now()); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 0 }); + expect(state.job?.status).toBe('running'); + expect(state.request?.status).toBe('running'); + expect(state.thread?.status).toBe('running'); + }); +}); diff --git a/packages/backend/tests/unit/update-status.test.ts b/packages/backend/tests/unit/update-status.test.ts index 0758db1..691e5d9 100644 --- a/packages/backend/tests/unit/update-status.test.ts +++ b/packages/backend/tests/unit/update-status.test.ts @@ -107,3 +107,35 @@ describe('updateStatus terminal guard', () => { expect(job?.status).toBe('checks_running'); }); }); + +describe('heartbeatWorkspace cancellation propagation', () => { + test('reports cancellation while refreshing the workspace heartbeat', async () => { + const t = convexTest(schema, modules); + const jobId = await seedJob(t, 'cancelled'); + + const result = await t.mutation(api.agentJobs.heartbeatWorkspace, { + workerToken: 'test-worker-token', + workerId: 'worker-1', + jobId, + }); + + expect(result).toEqual({ success: true, cancelRequested: true }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + expect(job?.status).toBe('cancelled'); + expect(job?.workspaceStatus).toBe('active'); + expect(job?.lastHeartbeatAt).toBeTypeOf('number'); + }); + + test('reports no cancellation for an active job', async () => { + const t = convexTest(schema, modules); + const jobId = await seedJob(t, 'running'); + + const result = await t.mutation(api.agentJobs.heartbeatWorkspace, { + workerToken: 'test-worker-token', + workerId: 'worker-1', + jobId, + }); + + expect(result).toEqual({ success: true, cancelRequested: false }); + }); +}); From ac684840595c89cfb78df7ee9f3ed3cac8838d56 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:52:20 -0400 Subject: [PATCH 25/88] fix(worker): make cancellation teardown failure-safe --- apps/agent-worker/src/codex-process.ts | 88 +++++ apps/agent-worker/src/heartbeat-controller.ts | 42 ++ apps/agent-worker/src/worker.ts | 363 ++++++++++-------- apps/agent-worker/src/workspace-teardown.ts | 35 ++ .../tests/unit/codex-process.test.ts | 37 ++ .../tests/unit/heartbeat-controller.test.ts | 60 +++ .../tests/unit/workspace-teardown.test.ts | 62 +++ .../tests/unit/recover-stale-jobs.test.ts | 79 +++- 8 files changed, 604 insertions(+), 162 deletions(-) create mode 100644 apps/agent-worker/src/codex-process.ts create mode 100644 apps/agent-worker/src/heartbeat-controller.ts create mode 100644 apps/agent-worker/src/workspace-teardown.ts create mode 100644 apps/agent-worker/tests/unit/codex-process.test.ts create mode 100644 apps/agent-worker/tests/unit/heartbeat-controller.test.ts create mode 100644 apps/agent-worker/tests/unit/workspace-teardown.test.ts diff --git a/apps/agent-worker/src/codex-process.ts b/apps/agent-worker/src/codex-process.ts new file mode 100644 index 0000000..c6223a2 --- /dev/null +++ b/apps/agent-worker/src/codex-process.ts @@ -0,0 +1,88 @@ +type CommandResult = { exitCode: number; output: string }; + +const launchScript = ` +import os +import sys + +pid_file, job_id, *command = sys.argv[1:] +if os.getpgrp() != os.getpid(): + os.setsid() +os.environ["SPOON_CODEX_JOB_ID"] = job_id +with open(pid_file, "w", encoding="utf-8") as handle: + handle.write(str(os.getpid())) + handle.flush() + os.fsync(handle.fileno()) +os.execvpe(command[0], command, os.environ) +`; + +const abortScript = ` +import os +import signal +import sys +import time + +pid_file, job_id = sys.argv[1:] +pid = None +for _ in range(20): + try: + with open(pid_file, encoding="utf-8") as handle: + pid = int(handle.read().strip()) + break + except FileNotFoundError: + time.sleep(0.1) +if pid is None: + sys.exit(0) + +try: + with open(f"/proc/{pid}/environ", "rb") as handle: + environment = handle.read().split(b"\\0") + marker = f"SPOON_CODEX_JOB_ID={job_id}".encode() + if marker not in environment or os.getpgid(pid) != pid: + raise RuntimeError("Codex process identity did not match the requested job") + os.killpg(pid, signal.SIGTERM) + for _ in range(20): + time.sleep(0.1) + try: + os.killpg(pid, 0) + except ProcessLookupError: + break + else: + os.killpg(pid, signal.SIGKILL) +except ProcessLookupError: + pass +finally: + try: + os.unlink(pid_file) + except FileNotFoundError: + pass +`; + +export const managedCodexCommand = (args: { + jobId: string; + pidFile: string; + command: string[]; +}) => [ + 'python3', + '-c', + launchScript, + args.pidFile, + args.jobId, + ...args.command, +]; + +export const abortManagedCodexProcess = async (args: { + jobId: string; + pidFile: string; + execute: (command: string[]) => Promise; +}) => { + const result = await args.execute([ + 'python3', + '-c', + abortScript, + args.pidFile, + args.jobId, + ]); + if (result.exitCode !== 0) { + throw new Error(`Failed to abort Codex process: ${result.output}`); + } +}; diff --git a/apps/agent-worker/src/heartbeat-controller.ts b/apps/agent-worker/src/heartbeat-controller.ts new file mode 100644 index 0000000..c6e8380 --- /dev/null +++ b/apps/agent-worker/src/heartbeat-controller.ts @@ -0,0 +1,42 @@ +export const createHeartbeatController = (args: { + intervalMs: number; + heartbeat: (jobId: string) => Promise<{ cancelRequested: boolean }>; + onCancel: (jobId: string) => Promise; + onError: (error: unknown) => void; +}) => { + const timers = new Map>(); + const inFlight = new Set(); + + const stop = (jobId: string) => { + const timer = timers.get(jobId); + if (timer) clearInterval(timer); + timers.delete(jobId); + }; + + const tick = async (jobId: string) => { + if (inFlight.has(jobId) || !timers.has(jobId)) return; + inFlight.add(jobId); + try { + const result = await args.heartbeat(jobId); + if (result.cancelRequested) { + stop(jobId); + await args.onCancel(jobId); + } + } catch (error) { + args.onError(error); + } finally { + inFlight.delete(jobId); + } + }; + + return { + start: (jobId: string) => { + stop(jobId); + const timer = setInterval(() => void tick(jobId), args.intervalMs); + timer.unref(); + timers.set(jobId, timer); + }, + stop, + has: (jobId: string) => timers.has(jobId), + }; +}; diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 9715b5a..4f3b572 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -18,6 +18,7 @@ import type { NormalizedAgentEvent } from './agent-events'; import type { OpenCodeSession } from './opencode-session'; import type { BoxHandle } from './user-container'; import { normalizeCodexJsonLine } from './agent-events'; +import { abortManagedCodexProcess, managedCodexCommand } from './codex-process'; import { prepareCodexWorkspaceFiles } from './codex-runtime'; import { env } from './env'; import { @@ -28,6 +29,7 @@ import { run, } from './git'; import { getInstallationToken, openDraftPullRequest } from './github'; +import { createHeartbeatController } from './heartbeat-controller'; import { abortOpenCodeSession, createOpenCodeSession, @@ -45,6 +47,7 @@ import { } from './runtime/docker'; import { acquireUserBox, runningBoxUsernames } from './user-container'; import { fetchUserEnvironment, materializeUserHome } from './user-environment'; +import { teardownWorkspace } from './workspace-teardown'; type Claim = { job: { @@ -118,7 +121,9 @@ type ActiveWorkspace = { opencodePassword?: string; opencodeSession?: OpenCodeSession; codexSessionId?: string; + codexPidFile?: string; agentTurnActive?: boolean; + cancelRequested?: boolean; resolveTurn?: () => void; lastRecordedDiffSignature?: string; // Captures the most recent Codex `error`/`turn.failed` event for the active @@ -269,43 +274,17 @@ const heartbeatWorkspaceMutation = async (jobId: Id<'agentJobs'>) => jobId, })) as { success: true; cancelRequested: boolean }; -const heartbeatTimers = new Map>(); +const heartbeatController = createHeartbeatController({ + intervalMs: 30_000, + heartbeat: async (jobId) => + await heartbeatWorkspaceMutation(jobId as Id<'agentJobs'>), + onCancel: async (jobId) => await cancelWorkspace(jobId), + onError: (error) => console.error(error), +}); -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((error: unknown) => { - console.error(error); - }); - await stopWorkspace(jobId).catch((error: unknown) => { - console.error(error); - }); - stopHeartbeat(jobId); - } - } catch (error) { - console.error(error); - } - })(); - }, 30_000); - timer.unref(); - heartbeatTimers.set(jobId, timer); -}; +const stopHeartbeat = (jobId: string) => heartbeatController.stop(jobId); +const startHeartbeat = (jobId: Id<'agentJobs'>) => + heartbeatController.start(jobId); const appendMessage = async (args: { jobId: Id<'agentJobs'>; @@ -789,7 +768,10 @@ const runCodexTurn = async (args: { '.codex', outputFileName, ); - const command = workspace.codexSessionId + if (workspace.cancelRequested) { + throw new Error('Workspace cancellation requested.'); + } + const codexCommand = workspace.codexSessionId ? [ 'codex', 'exec', @@ -814,46 +796,65 @@ const runCodexTurn = async (args: { workspace.containerRepo, prompt, ]; + const pidFileName = `codex-turn-${workspace.claim.job._id}.pid`; + const pidFileHostPath = path.join(workspace.workdir, '.codex', pidFileName); + const pidFileContainerPath = path.posix.join( + workspace.containerHome, + '.codex', + pidFileName, + ); + workspace.codexPidFile = pidFileContainerPath; + const command = managedCodexCommand({ + jobId: workspace.claim.job._id, + pidFile: pidFileContainerPath, + command: codexCommand, + }); const aiEnv = providerEnvironment(workspace.claim, workspace.containerHome); const secretEnv = Object.fromEntries( workspace.claim.secrets.map((secret) => [secret.name, secret.value]), ); - const result = await streamExecInContainer({ - containerName: workspace.boxName, - containerCwd: workspace.containerRepo, - command, - environment: { - ...aiEnv, - ...secretEnv, - }, - redact: workspace.redact, - timeoutMs: env.jobTimeoutMs, - onStdoutLine: async (line) => { - for (const event of normalizeCodexJsonLine(line)) { - await handleAgentEvent({ - workspace, - event, - assistantMessageId, - assistantContent, - }); - } - }, - onStderrLine: async (line) => { - const trimmed = line.trim(); - if ( - trimmed && - trimmed !== 'Reading additional input from stdin...' && - !trimmed.includes('`[features].codex_hooks` is deprecated') - ) { - await appendEvent( - workspace.claim.job._id, - 'info', - 'plan', - truncate(trimmed, 10_000), - ); - } - }, - }); + let result; + try { + result = await streamExecInContainer({ + containerName: workspace.boxName, + containerCwd: workspace.containerRepo, + command, + environment: { + ...aiEnv, + ...secretEnv, + }, + redact: workspace.redact, + timeoutMs: env.jobTimeoutMs, + onStdoutLine: async (line) => { + for (const event of normalizeCodexJsonLine(line)) { + await handleAgentEvent({ + workspace, + event, + assistantMessageId, + assistantContent, + }); + } + }, + onStderrLine: async (line) => { + const trimmed = line.trim(); + if ( + trimmed && + trimmed !== 'Reading additional input from stdin...' && + !trimmed.includes('`[features].codex_hooks` is deprecated') + ) { + await appendEvent( + workspace.claim.job._id, + 'info', + 'plan', + truncate(trimmed, 10_000), + ); + } + }, + }); + } finally { + workspace.codexPidFile = undefined; + await rm(pidFileHostPath, { force: true }); + } await appendEvent( workspace.claim.job._id, 'info', @@ -1485,32 +1486,46 @@ const runClaim = async (claim: Claim) => { }); } catch (error) { const message = error instanceof Error ? error.message : String(error); - await appendEvent( - jobId, - 'error', - 'cleanup', - truncate(redact(message), 20_000), - ); - await addArtifact({ - jobId, - kind: 'error', - title: 'Failure', - content: truncate(redact(message), 50_000), - contentType: 'text/plain', - }); - await updateStatus( - jobId, - message.toLowerCase().includes('timed out') ? 'timed_out' : 'failed', - { error: truncate(redact(message), 10_000) }, - ); - await markWorkspaceStopped( - jobId, - message.toLowerCase().includes('timed out') ? 'expired' : 'failed', - ).catch((stopError: unknown) => { - console.error(stopError); - }); - stopHeartbeat(jobId); - acquiredBoxHandle?.release(); + const workspaceStatus = message.toLowerCase().includes('timed out') + ? 'expired' + : 'failed'; + try { + await appendEvent( + jobId, + 'error', + 'cleanup', + truncate(redact(message), 20_000), + ); + await addArtifact({ + jobId, + kind: 'error', + title: 'Failure', + content: truncate(redact(message), 50_000), + contentType: 'text/plain', + }); + await updateStatus( + jobId, + workspaceStatus === 'expired' ? 'timed_out' : 'failed', + { error: truncate(redact(message), 10_000) }, + ); + } finally { + const workspace = activeWorkspaces.get(jobId); + if (workspace) { + const teardown = await teardownActiveWorkspace({ + jobId, + workspace, + abortAgent: true, + workspaceStatus, + }); + reportTeardownErrors(teardown.errors); + } else { + stopHeartbeat(jobId); + await markWorkspaceStopped(jobId, workspaceStatus).catch( + (stopError: unknown) => console.error(stopError), + ); + acquiredBoxHandle?.release(); + } + } } }; @@ -1638,27 +1653,39 @@ export const getWorkspaceAgentStatus = (jobId: string) => { }; }; -export const abortWorkspaceAgent = async (jobId: string) => { - const workspace = resolveWorkspace(jobId); +const abortWorkspaceAgentForWorkspace = async (workspace: ActiveWorkspace) => { if (workspace.opencodeSession) { await abortOpenCodeSession(workspace.opencodeSession); - workspace.agentTurnActive = false; - workspace.resolveTurn?.(); - workspace.resolveTurn = undefined; - await appendEvent( - workspace.claim.job._id, - 'warn', - 'cleanup', - 'Agent turn aborted.', - ); - return { success: true }; - } - if (workspace.runtimeMode === 'codex_exec') { - throw new Error('Codex agent turns cannot be aborted from Spoon yet.'); + } else if (workspace.runtimeMode === 'codex_exec' && workspace.codexPidFile) { + await abortManagedCodexProcess({ + jobId: workspace.claim.job._id, + pidFile: workspace.codexPidFile, + execute: async (command) => + await runExecInContainer({ + containerName: workspace.boxName, + containerCwd: workspace.containerRepo, + command, + environment: {}, + redact: workspace.redact, + timeoutMs: env.jobTimeoutMs, + }), + }); } + workspace.agentTurnActive = false; + workspace.resolveTurn?.(); + workspace.resolveTurn = undefined; + await appendEvent( + workspace.claim.job._id, + 'warn', + 'cleanup', + 'Agent turn aborted.', + ); return { success: true }; }; +export const abortWorkspaceAgent = async (jobId: string) => + await abortWorkspaceAgentForWorkspace(resolveWorkspace(jobId)); + export const replyToInteraction = async ( jobId: string, args: { @@ -1869,6 +1896,60 @@ export const sendWorkspaceMessage = async ( } }; +const teardownActiveWorkspace = async (args: { + jobId: string; + workspace: ActiveWorkspace; + abortAgent: boolean; + appendCancellationEvent?: boolean; + workspaceStatus?: 'stopped' | 'expired' | 'failed'; +}) => { + const { jobId, workspace } = args; + const containerName = workspace.containerName; + if (args.abortAgent) workspace.cancelRequested = true; + return await teardownWorkspace({ + stopHeartbeat: () => stopHeartbeat(jobId), + removeActive: () => activeWorkspaces.delete(jobId), + appendEvent: args.appendCancellationEvent + ? async () => + await appendEvent( + workspace.claim.job._id, + 'warn', + 'cleanup', + 'Cancellation requested; stopping workspace.', + ) + : undefined, + abortAgent: args.abortAgent + ? async () => await abortWorkspaceAgentForWorkspace(workspace) + : undefined, + markStopped: async () => + await markWorkspaceStopped(workspace.claim.job._id, args.workspaceStatus), + closeAgent: () => workspace.opencodeSession?.close(), + stopContainer: containerName + ? async () => await stopWorkspaceContainer(containerName) + : undefined, + release: () => workspace.boxHandle.release(), + }); +}; + +const reportTeardownErrors = (errors: unknown[]) => { + for (const error of errors) console.error(error); +}; + +const cancelWorkspace = async (jobId: string) => { + const workspace = activeWorkspaces.get(jobId); + if (!workspace) { + stopHeartbeat(jobId); + return; + } + const result = await teardownActiveWorkspace({ + jobId, + workspace, + abortAgent: true, + appendCancellationEvent: true, + }); + reportTeardownErrors(result.errors); +}; + export const openWorkspacePullRequest = async (jobId: string) => { const workspace = resolveWorkspace(jobId); const { claim, repoDir, redact } = workspace; @@ -1931,16 +2012,14 @@ export const openWorkspacePullRequest = async (jobId: string) => { pullRequestNumber: pullRequest.number, summary: 'Draft PR opened from interactive workspace.', }); - await markWorkspaceStopped(claim.job._id); - workspace.opencodeSession?.close(); - if (workspace.containerName) { - await stopWorkspaceContainer(workspace.containerName); + const teardown = await teardownActiveWorkspace({ + jobId, + workspace, + abortAgent: false, + }); + if (teardown.errors.length > 0) { + throw new AggregateError(teardown.errors, 'Workspace teardown failed.'); } - stopHeartbeat(jobId); - activeWorkspaces.delete(jobId); - // The persistent per-user home + ~/Code checkouts survive across sessions; - // release the box (reaped once no other thread/terminal holds it). - workspace.boxHandle.release(); return { pullRequestUrl: pullRequest.html_url, pullRequestNumber: pullRequest.number, @@ -1949,37 +2028,13 @@ export const openWorkspacePullRequest = async (jobId: string) => { export const stopWorkspace = async (jobId: string) => { const workspace = resolveWorkspace(jobId); - // Stop the heartbeat timer before teardown so it never outlives the - // workspace even if a teardown step throws. - stopHeartbeat(jobId); - const errors: unknown[] = []; - try { - await markWorkspaceStopped(workspace.claim.job._id); - } catch (error) { - errors.push(error); - } - try { - workspace.opencodeSession?.close(); - } catch (error) { - errors.push(error); - } - try { - if (workspace.containerName) { - await stopWorkspaceContainer(workspace.containerName); - } - } catch (error) { - errors.push(error); - } finally { - activeWorkspaces.delete(jobId); - try { - workspace.boxHandle.release(); - } catch (error) { - errors.push(error); - } - } - if (errors.length === 1) throw errors[0]; - if (errors.length > 1) { - throw new AggregateError(errors, 'Workspace teardown failed.'); + const teardown = await teardownActiveWorkspace({ + jobId, + workspace, + abortAgent: true, + }); + if (teardown.errors.length > 0) { + throw new AggregateError(teardown.errors, 'Workspace teardown failed.'); } return { success: true }; }; diff --git a/apps/agent-worker/src/workspace-teardown.ts b/apps/agent-worker/src/workspace-teardown.ts new file mode 100644 index 0000000..9fac3ab --- /dev/null +++ b/apps/agent-worker/src/workspace-teardown.ts @@ -0,0 +1,35 @@ +type TeardownStep = () => unknown; + +export const teardownWorkspace = async (steps: { + stopHeartbeat: TeardownStep; + removeActive: TeardownStep; + appendEvent?: TeardownStep; + abortAgent?: TeardownStep; + markStopped?: TeardownStep; + closeAgent?: TeardownStep; + stopContainer?: TeardownStep; + release: TeardownStep; +}) => { + const errors: unknown[] = []; + const call = (step: TeardownStep | undefined) => { + if (!step) return Promise.resolve(); + try { + return Promise.resolve(step()).catch((error: unknown) => { + errors.push(error); + }); + } catch (error) { + errors.push(error); + return Promise.resolve(); + } + }; + + const immediate = [call(steps.stopHeartbeat), call(steps.removeActive)]; + await Promise.all(immediate); + await call(steps.appendEvent); + await call(steps.abortAgent); + await call(steps.markStopped); + await call(steps.closeAgent); + await call(steps.stopContainer); + await call(steps.release); + return { errors }; +}; diff --git a/apps/agent-worker/tests/unit/codex-process.test.ts b/apps/agent-worker/tests/unit/codex-process.test.ts new file mode 100644 index 0000000..27cb15f --- /dev/null +++ b/apps/agent-worker/tests/unit/codex-process.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, test, vi } from 'vitest'; + +import { + abortManagedCodexProcess, + managedCodexCommand, +} from '../../src/codex-process'; + +describe('managed Codex process', () => { + test('launches Codex in a job-specific process session', () => { + const command = managedCodexCommand({ + jobId: 'job-1', + pidFile: '/home/alice/.codex/job-1.pid', + command: ['codex', 'exec', '--json', 'fix it'], + }); + + expect(command.slice(0, 2)).toEqual(['python3', '-c']); + expect(command).toContain('/home/alice/.codex/job-1.pid'); + expect(command).toContain('job-1'); + expect(command.slice(-4)).toEqual(['codex', 'exec', '--json', 'fix it']); + }); + + test('invokes an isolated in-box kill for the matching job', async () => { + const execute = vi.fn().mockResolvedValue({ exitCode: 0, output: '' }); + + await abortManagedCodexProcess({ + jobId: 'job-1', + pidFile: '/home/alice/.codex/job-1.pid', + execute, + }); + + expect(execute).toHaveBeenCalledOnce(); + const [command] = execute.mock.calls[0] as [string[]]; + expect(command.slice(0, 2)).toEqual(['python3', '-c']); + expect(command).toContain('/home/alice/.codex/job-1.pid'); + expect(command).toContain('job-1'); + }); +}); diff --git a/apps/agent-worker/tests/unit/heartbeat-controller.test.ts b/apps/agent-worker/tests/unit/heartbeat-controller.test.ts new file mode 100644 index 0000000..d933bee --- /dev/null +++ b/apps/agent-worker/tests/unit/heartbeat-controller.test.ts @@ -0,0 +1,60 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +import { createHeartbeatController } from '../../src/heartbeat-controller'; + +describe('heartbeat controller', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + test('does not overlap ticks while a heartbeat is in flight', async () => { + let resolveHeartbeat!: (value: { cancelRequested: boolean }) => void; + const heartbeat = vi.fn( + () => + new Promise<{ cancelRequested: boolean }>((resolve) => { + resolveHeartbeat = resolve; + }), + ); + const controller = createHeartbeatController({ + intervalMs: 30_000, + heartbeat, + onCancel: vi.fn(), + onError: vi.fn(), + }); + + controller.start('job-1'); + await vi.advanceTimersByTimeAsync(90_000); + expect(heartbeat).toHaveBeenCalledOnce(); + + resolveHeartbeat({ cancelRequested: false }); + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(30_000); + expect(heartbeat).toHaveBeenCalledTimes(2); + controller.stop('job-1'); + }); + + test('removes the timer before awaiting cancellation teardown', async () => { + let finishCancel!: () => void; + const onCancel = vi.fn( + () => + new Promise((resolve) => { + finishCancel = resolve; + }), + ); + const heartbeat = vi.fn().mockResolvedValue({ cancelRequested: true }); + const controller = createHeartbeatController({ + intervalMs: 30_000, + heartbeat, + onCancel, + onError: vi.fn(), + }); + + controller.start('job-1'); + await vi.advanceTimersByTimeAsync(30_000); + expect(onCancel).toHaveBeenCalledOnce(); + expect(controller.has('job-1')).toBe(false); + + await vi.advanceTimersByTimeAsync(90_000); + expect(heartbeat).toHaveBeenCalledOnce(); + finishCancel(); + }); +}); diff --git a/apps/agent-worker/tests/unit/workspace-teardown.test.ts b/apps/agent-worker/tests/unit/workspace-teardown.test.ts new file mode 100644 index 0000000..3bc1fc2 --- /dev/null +++ b/apps/agent-worker/tests/unit/workspace-teardown.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test, vi } from 'vitest'; + +import { abortManagedCodexProcess } from '../../src/codex-process'; +import { teardownWorkspace } from '../../src/workspace-teardown'; + +describe('workspace teardown', () => { + test('event failure does not skip abort, stop, removal, or release', async () => { + const calls: string[] = []; + const record = (name: string) => vi.fn(() => calls.push(name)); + + const result = await teardownWorkspace({ + stopHeartbeat: record('heartbeat'), + removeActive: record('remove'), + appendEvent: vi.fn().mockRejectedValue(new Error('event failed')), + abortAgent: record('abort'), + markStopped: vi.fn().mockRejectedValue(new Error('mutation failed')), + closeAgent: record('close'), + stopContainer: vi.fn().mockRejectedValue(new Error('container failed')), + release: record('release'), + }); + + expect(calls).toEqual(['heartbeat', 'remove', 'abort', 'close', 'release']); + expect(result.errors).toHaveLength(3); + }); + + test('removes active state before asynchronous cancellation work', async () => { + const active = new Set(['job-1']); + let activeDuringAbort = true; + + await teardownWorkspace({ + stopHeartbeat: vi.fn(), + removeActive: () => active.delete('job-1'), + abortAgent: () => { + activeDuringAbort = active.has('job-1'); + }, + release: vi.fn(), + }); + + expect(activeDuringAbort).toBe(false); + expect(active.has('job-1')).toBe(false); + }); + + test('Codex cancellation invokes isolated kill and leaves no active workspace', async () => { + const active = new Set(['job-1']); + const execute = vi.fn().mockResolvedValue({ exitCode: 0, output: '' }); + + await teardownWorkspace({ + stopHeartbeat: vi.fn(), + removeActive: () => active.delete('job-1'), + abortAgent: async () => + await abortManagedCodexProcess({ + jobId: 'job-1', + pidFile: '/home/alice/.codex/job-1.pid', + execute, + }), + release: vi.fn(), + }); + + expect(execute).toHaveBeenCalledOnce(); + expect(active.has('job-1')).toBe(false); + }); +}); diff --git a/packages/backend/tests/unit/recover-stale-jobs.test.ts b/packages/backend/tests/unit/recover-stale-jobs.test.ts index 62285f9..33a4180 100644 --- a/packages/backend/tests/unit/recover-stale-jobs.test.ts +++ b/packages/backend/tests/unit/recover-stale-jobs.test.ts @@ -1,5 +1,5 @@ import { convexTest } from 'convex-test'; -import { describe, expect, test } from 'vitest'; +import { describe, expect, test, vi } from 'vitest'; import type { Id } from '../../convex/_generated/dataModel.js'; import { internal } from '../../convex/_generated/api.js'; @@ -7,9 +7,15 @@ import schema from '../../convex/schema'; const modules = import.meta.glob('../../convex/**/*.*s'); -const seedRunningJob = async ( +type ActiveStatus = 'claimed' | 'preparing' | 'running' | 'checks_running'; + +const seedJob = async ( t: ReturnType, - lastHeartbeatAt: number, + args: { + status?: ActiveStatus; + claimedAt?: number; + lastHeartbeatAt?: number; + }, ) => await t.mutation(async (ctx) => { const now = Date.now(); @@ -60,7 +66,7 @@ const seedRunningJob = async ( ownerId, agentRequestId: requestId, threadId, - status: 'running', + status: args.status ?? 'running', prompt: 'Recover the worker job', runtime: 'opencode', workspaceStatus: 'active', @@ -75,8 +81,10 @@ const seedRunningJob = async ( model: 'gpt-5.1-codex', reasoningEffort: 'medium', claimedBy: 'w1', - claimedAt: now, - lastHeartbeatAt, + claimedAt: args.claimedAt ?? now, + ...(args.lastHeartbeatAt === undefined + ? {} + : { lastHeartbeatAt: args.lastHeartbeatAt }), createdAt: now, updatedAt: now, }); @@ -102,7 +110,9 @@ const readState = async ( describe('recoverStaleJobs', () => { test('times out a running job with a stale heartbeat', async () => { const t = convexTest(schema, modules); - const ids = await seedRunningJob(t, Date.now() - 10 * 60 * 1000); + const ids = await seedJob(t, { + lastHeartbeatAt: Date.now() - 10 * 60 * 1000, + }); const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); const state = await readState(t, ids); @@ -117,7 +127,7 @@ describe('recoverStaleJobs', () => { test('leaves a running job with a fresh heartbeat untouched', async () => { const t = convexTest(schema, modules); - const ids = await seedRunningJob(t, Date.now()); + const ids = await seedJob(t, { lastHeartbeatAt: Date.now() }); const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); const state = await readState(t, ids); @@ -127,4 +137,57 @@ describe('recoverStaleJobs', () => { expect(state.request?.status).toBe('running'); expect(state.thread?.status).toBe('running'); }); + + test('uses claimedAt when a claimed job has no heartbeat', async () => { + const t = convexTest(schema, modules); + const ids = await seedJob(t, { + status: 'claimed', + claimedAt: Date.now() - 10 * 60 * 1000, + }); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 1 }); + expect(state.job?.status).toBe('timed_out'); + }); + + test('times out a heartbeat exactly at the default threshold', async () => { + vi.useFakeTimers(); + try { + const now = new Date('2026-07-10T12:00:00.000Z'); + vi.setSystemTime(now); + const t = convexTest(schema, modules); + const ids = await seedJob(t, { + lastHeartbeatAt: now.getTime() - 3 * 60 * 1000, + }); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 1 }); + expect(state.job?.status).toBe('timed_out'); + } finally { + vi.useRealTimers(); + } + }); + + test.each([ + 'claimed', + 'preparing', + 'running', + 'checks_running', + ])('recovers stale %s jobs', async (status) => { + const t = convexTest(schema, modules); + const ids = await seedJob(t, { + status, + lastHeartbeatAt: Date.now() - 10 * 60 * 1000, + }); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 1 }); + expect(state.job?.status).toBe('timed_out'); + }); }); From 60dd5d383625be219fd9634fb3dac6c6447d268b Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 18:13:28 -0400 Subject: [PATCH 26/88] fix(agent-jobs): stop heartbeat from resurrecting terminated jobs --- packages/backend/convex/agentJobs.ts | 6 ++++- .../tests/unit/recover-stale-jobs.test.ts | 1 + .../backend/tests/unit/update-status.test.ts | 25 ++++++++++++++++--- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 019578c..8cddadd 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -1223,6 +1223,7 @@ export const recoverStaleJobs = internalMutation({ const now = Date.now(); await ctx.db.patch(job._id, { status: 'timed_out', + workspaceStatus: 'expired', error: job.error ?? 'The worker stopped reporting progress; the job timed out.', @@ -1512,12 +1513,15 @@ export const heartbeatWorkspace = mutation({ if (job?.claimedBy !== args.workerId) { throw new ConvexError('Agent job not claimed by this worker.'); } + if (isTerminalStatus(job.status)) { + return { success: true, cancelRequested: true }; + } await ctx.db.patch(args.jobId, { workspaceStatus: 'active', lastHeartbeatAt: Date.now(), updatedAt: Date.now(), }); - return { success: true, cancelRequested: job.status === 'cancelled' }; + return { success: true, cancelRequested: false }; }, }); diff --git a/packages/backend/tests/unit/recover-stale-jobs.test.ts b/packages/backend/tests/unit/recover-stale-jobs.test.ts index 33a4180..89db9ff 100644 --- a/packages/backend/tests/unit/recover-stale-jobs.test.ts +++ b/packages/backend/tests/unit/recover-stale-jobs.test.ts @@ -119,6 +119,7 @@ describe('recoverStaleJobs', () => { expect(result).toEqual({ recovered: 1 }); expect(state.job?.status).toBe('timed_out'); + expect(state.job?.workspaceStatus).toBe('expired'); expect(state.job?.completedAt).toBeTypeOf('number'); expect(state.request?.status).toBe('failed'); expect(state.thread?.status).toBe('failed'); diff --git a/packages/backend/tests/unit/update-status.test.ts b/packages/backend/tests/unit/update-status.test.ts index 691e5d9..5e19be4 100644 --- a/packages/backend/tests/unit/update-status.test.ts +++ b/packages/backend/tests/unit/update-status.test.ts @@ -24,7 +24,7 @@ const spoonInput = { const seedJob = async ( t: ReturnType, - status: 'cancelled' | 'running', + status: 'cancelled' | 'running' | 'timed_out', ) => await t.mutation(async (ctx) => { const now = Date.now(); @@ -109,7 +109,7 @@ describe('updateStatus terminal guard', () => { }); describe('heartbeatWorkspace cancellation propagation', () => { - test('reports cancellation while refreshing the workspace heartbeat', async () => { + test('requests teardown for a cancelled job without resurrecting it', async () => { const t = convexTest(schema, modules); const jobId = await seedJob(t, 'cancelled'); @@ -122,8 +122,25 @@ describe('heartbeatWorkspace cancellation propagation', () => { expect(result).toEqual({ success: true, cancelRequested: true }); const job = await t.run(async (ctx) => await ctx.db.get(jobId)); expect(job?.status).toBe('cancelled'); - expect(job?.workspaceStatus).toBe('active'); - expect(job?.lastHeartbeatAt).toBeTypeOf('number'); + expect(job?.workspaceStatus).not.toBe('active'); + expect(job?.lastHeartbeatAt).toBeUndefined(); + }); + + test('requests teardown for a timed_out job without resurrecting it', async () => { + const t = convexTest(schema, modules); + const jobId = await seedJob(t, 'timed_out'); + + const result = await t.mutation(api.agentJobs.heartbeatWorkspace, { + workerToken: 'test-worker-token', + workerId: 'worker-1', + jobId, + }); + + expect(result).toEqual({ success: true, cancelRequested: true }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + expect(job?.status).toBe('timed_out'); + expect(job?.workspaceStatus).not.toBe('active'); + expect(job?.lastHeartbeatAt).toBeUndefined(); }); test('reports no cancellation for an active job', async () => { From 697fb7286df026979583b58060a9c758d9abf98a Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 18:27:53 -0400 Subject: [PATCH 27/88] fix(agent-jobs): recovery cron skips already-terminal jobs --- packages/backend/convex/agentJobs.ts | 1 + .../tests/unit/recover-stale-jobs.test.ts | 39 ++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 8cddadd..54a79af 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -1218,6 +1218,7 @@ export const recoverStaleJobs = internalMutation({ .withIndex('by_status', (q) => q.eq('status', status)) .collect(); for (const job of jobs) { + if (isTerminalJob(job)) continue; const last = job.lastHeartbeatAt ?? job.claimedAt ?? job.createdAt; if (last > threshold) continue; const now = Date.now(); diff --git a/packages/backend/tests/unit/recover-stale-jobs.test.ts b/packages/backend/tests/unit/recover-stale-jobs.test.ts index 89db9ff..f253add 100644 --- a/packages/backend/tests/unit/recover-stale-jobs.test.ts +++ b/packages/backend/tests/unit/recover-stale-jobs.test.ts @@ -15,6 +15,10 @@ const seedJob = async ( status?: ActiveStatus; claimedAt?: number; lastHeartbeatAt?: number; + workspaceStatus?: string; + completedAt?: number; + threadStatus?: 'running' | 'resolved' | 'ignored' | 'waiting_for_user'; + threadResolvedAt?: number; }, ) => await t.mutation(async (ctx) => { @@ -55,9 +59,12 @@ const seedJob = async ( spoonId, title: 'Recover the worker job', source: 'user_request', - status: 'running', + status: args.threadStatus ?? 'running', priority: 'normal', relatedAgentRequestId: requestId, + ...(args.threadResolvedAt === undefined + ? {} + : { resolvedAt: args.threadResolvedAt }), createdAt: now, updatedAt: now, }); @@ -69,7 +76,7 @@ const seedJob = async ( status: args.status ?? 'running', prompt: 'Recover the worker job', runtime: 'opencode', - workspaceStatus: 'active', + workspaceStatus: args.workspaceStatus ?? 'active', baseBranch: 'main', workBranch: 'spoon/recover-worker-job', forkOwner: 'team', @@ -85,6 +92,9 @@ const seedJob = async ( ...(args.lastHeartbeatAt === undefined ? {} : { lastHeartbeatAt: args.lastHeartbeatAt }), + ...(args.completedAt === undefined + ? {} + : { completedAt: args.completedAt }), createdAt: now, updatedAt: now, }); @@ -126,6 +136,31 @@ describe('recoverStaleJobs', () => { expect(state.thread?.resolvedAt).toBeTypeOf('number'); }); + test('leaves a terminal-workspace maintenance job untouched', async () => { + const t = convexTest(schema, modules); + const resolvedAt = Date.now() - 5 * 60 * 1000; + const ids = await seedJob(t, { + status: 'running', + workspaceStatus: 'stopped', + completedAt: resolvedAt, + threadStatus: 'resolved', + threadResolvedAt: resolvedAt, + // stale heartbeat: the worker stopped its heartbeat after applying the + // maintenance decision, so a naive cron would time this job out + claimedAt: Date.now() - 10 * 60 * 1000, + lastHeartbeatAt: Date.now() - 10 * 60 * 1000, + }); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 0 }); + expect(state.job?.status).toBe('running'); + expect(state.job?.workspaceStatus).toBe('stopped'); + expect(state.thread?.status).toBe('resolved'); + expect(state.thread?.resolvedAt).toBe(resolvedAt); + }); + test('leaves a running job with a fresh heartbeat untouched', async () => { const t = convexTest(schema, modules); const ids = await seedJob(t, { lastHeartbeatAt: Date.now() }); From 38431155b297a906852807a18d518b6fa7983a60 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 18:25:21 -0400 Subject: [PATCH 28/88] refactor(worker): extract AgentRuntime interface, shared provider helpers, and adapter registry --- .../agent-worker/src/runtime/agent-runtime.ts | 49 +++++ apps/agent-worker/src/runtime/provider.ts | 171 +++++++++++++++++ apps/agent-worker/src/worker.ts | 178 ++---------------- .../tests/unit/agent-runtime.test.ts | 42 +++++ .../maintenance-workspace-cleanup.test.ts | 1 + 5 files changed, 278 insertions(+), 163 deletions(-) create mode 100644 apps/agent-worker/src/runtime/agent-runtime.ts create mode 100644 apps/agent-worker/src/runtime/provider.ts create mode 100644 apps/agent-worker/tests/unit/agent-runtime.test.ts diff --git a/apps/agent-worker/src/runtime/agent-runtime.ts b/apps/agent-worker/src/runtime/agent-runtime.ts new file mode 100644 index 0000000..039dab9 --- /dev/null +++ b/apps/agent-worker/src/runtime/agent-runtime.ts @@ -0,0 +1,49 @@ +import type { NormalizedAgentEvent } from '../agent-events'; +import type { streamExecInContainer } from './docker'; +import type { AdapterWorkspace } from './provider'; + +export type AgentRuntimeName = 'codex' | 'opencode' | 'claude'; + +export type ExecStreamFn = typeof streamExecInContainer; + +export type TurnResult = { + // Assistant text the adapter captured out-of-band (Codex --output-last-message + // file, Claude `result` event). Empty/undefined when everything streamed via onEvent. + finalMessage?: string; + // Runtime session id to persist for continuity (resume/--session/--resume). + sessionId?: string; + // Non-empty when the runtime reported a hard failure the caller must surface. + error?: string; +}; + +export type AgentRuntime = { + readonly name: AgentRuntimeName; + prepareAuth(workspace: AdapterWorkspace): Promise; + runTurn( + workspace: AdapterWorkspace, + prompt: string, + onEvent: (event: NormalizedAgentEvent) => Promise, + ): Promise; + abort(workspace: AdapterWorkspace): Promise; +}; + +export type AdapterFactory = (deps?: { + execStream?: ExecStreamFn; +}) => AgentRuntime; + +const factories = new Map(); + +export const registerAdapter = ( + name: AgentRuntimeName, + factory: AdapterFactory, +): void => { + factories.set(name, factory); +}; + +export const getAdapter = (name: AgentRuntimeName): AgentRuntime => { + const factory = factories.get(name); + if (!factory) { + throw new Error(`No agent runtime adapter registered for "${name}".`); + } + return factory(); +}; diff --git a/apps/agent-worker/src/runtime/provider.ts b/apps/agent-worker/src/runtime/provider.ts new file mode 100644 index 0000000..37580df --- /dev/null +++ b/apps/agent-worker/src/runtime/provider.ts @@ -0,0 +1,171 @@ +import path from 'node:path'; + +import type { Id } from '@spoon/backend/convex/_generated/dataModel.js'; + +import type { AgentRuntimeName } from './agent-runtime'; + +export type Claim = { + job: { + _id: Id<'agentJobs'>; + prompt: string; + runtime?: 'codex' | 'opencode' | 'claude'; + jobType?: 'user_change' | 'maintenance_review' | 'conflict_resolution'; + envFilePath?: string; + materializeEnvFile?: boolean; + baseBranch: string; + workBranch: string; + forkOwner: string; + forkRepo: string; + upstreamOwner: string; + upstreamRepo: string; + }; + spoon: { name: string }; + openai: { + apiKey?: string; + model: string; + reasoningEffort: 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh'; + }; + aiProviderProfile?: { + id: string; + name: string; + provider: + | 'openai' + | 'anthropic' + | 'google' + | 'openrouter' + | 'requesty' + | 'litellm' + | 'cloudflare_ai_gateway' + | 'custom_openai_compatible' + | 'opencode_openai_login'; + authType: 'api_key' | 'opencode_auth_json' | 'none'; + secret?: string; + baseUrl?: string; + model: string; + reasoningEffort: 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh'; + }; + github: { installationId?: string }; + agentSettings?: { + installCommand?: string; + checkCommand?: string; + testCommand?: string; + autoDetectCommands?: boolean; + } | null; + secrets: { name: string; value: string }[]; +}; + +// The subset of the worker's active workspace that agent adapters read. +export type AdapterWorkspace = { + claim: Claim; + workdir: string; + homeDir: string; + username: string; + containerHome: string; + containerRepo: string; + repoDir: string; + boxName: string; + redact: (value: string) => string; + runtime: AgentRuntimeName; + codexSessionId?: string; + opencodeSessionId?: string; + claudeSessionId?: string; + turnMarker?: string; +}; + +export const isCodexLoginProfile = (claim: Claim) => + claim.aiProviderProfile?.provider === 'opencode_openai_login' || + claim.aiProviderProfile?.authType === 'opencode_auth_json'; + +export const collectJsonStringValues = (value?: string): string[] => { + if (!value) return []; + try { + const parsed = JSON.parse(value) as unknown; + const values: string[] = []; + const visit = (item: unknown) => { + if (typeof item === 'string') { + if (item.length >= 12) values.push(item); + return; + } + if (Array.isArray(item)) { + item.forEach(visit); + return; + } + if (item && typeof item === 'object') { + Object.values(item).forEach(visit); + } + }; + visit(parsed); + return values; + } catch { + return []; + } +}; + +export const providerEnvironment = ( + claim: Claim, + workspaceRoot?: string, +): Record => { + if (isCodexLoginProfile(claim)) { + if (!workspaceRoot) { + throw new Error('Codex auth profiles require a prepared workspace.'); + } + return { + CODEX_HOME: path.join(workspaceRoot, '.codex'), + HOME: workspaceRoot, + XDG_DATA_HOME: path.join(workspaceRoot, '.local', 'share'), + XDG_CONFIG_HOME: path.join(workspaceRoot, '.config'), + }; + } + const profile = claim.aiProviderProfile; + const secret = profile?.secret ?? claim.openai.apiKey; + if (!secret) { + throw new Error('No AI provider credential is configured for this job.'); + } + const baseUrl: Record = profile?.baseUrl + ? { OPENAI_BASE_URL: profile.baseUrl } + : {}; + if (!profile || profile.provider === 'openai') { + return { OPENAI_API_KEY: secret, ...baseUrl }; + } + if (profile.provider === 'anthropic') return { ANTHROPIC_API_KEY: secret }; + if (profile.provider === 'google') return { GOOGLE_API_KEY: secret }; + if (profile.provider === 'openrouter') { + return { OPENROUTER_API_KEY: secret, ...baseUrl }; + } + if (profile.provider === 'requesty') { + return { REQUESTY_API_KEY: secret, ...baseUrl }; + } + if (profile.provider === 'cloudflare_ai_gateway') { + return { CLOUDFLARE_API_KEY: secret, ...baseUrl }; + } + if ( + profile.provider === 'litellm' || + profile.provider === 'custom_openai_compatible' + ) { + return { OPENAI_API_KEY: secret, ...baseUrl }; + } + throw new Error('Unsupported AI provider profile.'); +}; + +export const opencodeModel = (claim: Claim) => { + const profile = claim.aiProviderProfile; + const model = profile?.model ?? claim.openai.model; + if (model.includes('/')) return model; + if (!profile) return `openai/${model}`; + if ( + profile.provider === 'custom_openai_compatible' || + profile.provider === 'cloudflare_ai_gateway' + ) { + return model; + } + if (profile.provider === 'opencode_openai_login') return `openai/${model}`; + return `${profile.provider}/${model}`; +}; + +export const codexModel = (claim: Claim) => { + const model = claim.aiProviderProfile?.model ?? claim.openai.model; + return model.includes('/') ? (model.split('/').at(-1) ?? model) : model; +}; + +export const codexModelArgs = (claim: Claim) => + isCodexLoginProfile(claim) ? [] : ['--model', codexModel(claim)]; diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 4f3b572..4ee61b8 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -16,6 +16,7 @@ import { api } from '@spoon/backend/convex/_generated/api.js'; import type { NormalizedAgentEvent } from './agent-events'; import type { OpenCodeSession } from './opencode-session'; +import type { AdapterWorkspace, Claim } from './runtime/provider'; import type { BoxHandle } from './user-container'; import { normalizeCodexJsonLine } from './agent-events'; import { abortManagedCodexProcess, managedCodexCommand } from './codex-process'; @@ -45,82 +46,26 @@ import { stopWorkspaceContainer, streamExecInContainer, } from './runtime/docker'; +import { + codexModelArgs, + collectJsonStringValues, + isCodexLoginProfile, + opencodeModel, + providerEnvironment, +} from './runtime/provider'; import { acquireUserBox, runningBoxUsernames } from './user-container'; import { fetchUserEnvironment, materializeUserHome } from './user-environment'; import { teardownWorkspace } from './workspace-teardown'; -type Claim = { - job: { - _id: Id<'agentJobs'>; - prompt: string; - runtime?: 'openai_direct' | 'opencode'; - jobType?: 'user_change' | 'maintenance_review' | 'conflict_resolution'; - envFilePath?: string; - materializeEnvFile?: boolean; - baseBranch: string; - workBranch: string; - forkOwner: string; - forkRepo: string; - upstreamOwner: string; - upstreamRepo: string; - }; - spoon: { name: string }; - openai: { - apiKey?: string; - model: string; - reasoningEffort: 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh'; - }; - aiProviderProfile?: { - id: string; - name: string; - provider: - | 'openai' - | 'anthropic' - | 'google' - | 'openrouter' - | 'requesty' - | 'litellm' - | 'cloudflare_ai_gateway' - | 'custom_openai_compatible' - | 'opencode_openai_login'; - authType: 'api_key' | 'opencode_auth_json' | 'none'; - secret?: string; - baseUrl?: string; - model: string; - reasoningEffort: 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh'; - }; - github: { installationId?: string }; - agentSettings?: { - installCommand?: string; - checkCommand?: string; - testCommand?: string; - autoDetectCommands?: boolean; - } | null; - secrets: { name: string; value: string }[]; -}; - -type ActiveWorkspace = { - claim: Claim; - // Host path of the persistent per-user home (mounted at `containerHome`). - // Equal to `homeDir`; kept as `workdir` for the container mount source. - workdir: string; - homeDir: string; - username: string; - // In-container paths: HOME and the thread's checkout (~/Code/{spoon}/{branch}). - containerHome: string; - containerRepo: string; - repoDir: string; +type ActiveWorkspace = AdapterWorkspace & { // Phase 2: the per-user box container this thread execs into. - boxName: string; boxHandle: BoxHandle; githubToken: string; - redact: (value: string) => string; runtimeMode?: 'opencode_server' | 'codex_exec' | 'legacy_cli'; containerName?: string; containerId?: string; opencodePassword?: string; opencodeSession?: OpenCodeSession; - codexSessionId?: string; codexPidFile?: string; agentTurnActive?: boolean; cancelRequested?: boolean; @@ -381,104 +326,6 @@ const commandToShell = (command: string) => ['bash', '-lc', command]; const workspaceContainerName = (jobId: string) => `spoon-agent-job-${jobId.replace(/[^a-zA-Z0-9_.-]/g, '-')}`; -const isCodexLoginProfile = (claim: Claim) => - claim.aiProviderProfile?.provider === 'opencode_openai_login' || - claim.aiProviderProfile?.authType === 'opencode_auth_json'; - -const collectJsonStringValues = (value?: string): string[] => { - if (!value) return []; - try { - const parsed = JSON.parse(value) as unknown; - const values: string[] = []; - const visit = (item: unknown) => { - if (typeof item === 'string') { - if (item.length >= 12) values.push(item); - return; - } - if (Array.isArray(item)) { - item.forEach(visit); - return; - } - if (item && typeof item === 'object') { - Object.values(item).forEach(visit); - } - }; - visit(parsed); - return values; - } catch { - return []; - } -}; - -const providerEnvironment = ( - claim: Claim, - workspaceRoot?: string, -): Record => { - if (isCodexLoginProfile(claim)) { - if (!workspaceRoot) { - throw new Error('Codex auth profiles require a prepared workspace.'); - } - return { - CODEX_HOME: path.join(workspaceRoot, '.codex'), - HOME: workspaceRoot, - XDG_DATA_HOME: path.join(workspaceRoot, '.local', 'share'), - XDG_CONFIG_HOME: path.join(workspaceRoot, '.config'), - }; - } - const profile = claim.aiProviderProfile; - const secret = profile?.secret ?? claim.openai.apiKey; - if (!secret) { - throw new Error('No AI provider credential is configured for this job.'); - } - const baseUrl: Record = profile?.baseUrl - ? { OPENAI_BASE_URL: profile.baseUrl } - : {}; - if (!profile || profile.provider === 'openai') { - return { OPENAI_API_KEY: secret, ...baseUrl }; - } - if (profile.provider === 'anthropic') return { ANTHROPIC_API_KEY: secret }; - if (profile.provider === 'google') return { GOOGLE_API_KEY: secret }; - if (profile.provider === 'openrouter') { - return { OPENROUTER_API_KEY: secret, ...baseUrl }; - } - if (profile.provider === 'requesty') { - return { REQUESTY_API_KEY: secret, ...baseUrl }; - } - if (profile.provider === 'cloudflare_ai_gateway') { - return { CLOUDFLARE_API_KEY: secret, ...baseUrl }; - } - if ( - profile.provider === 'litellm' || - profile.provider === 'custom_openai_compatible' - ) { - return { OPENAI_API_KEY: secret, ...baseUrl }; - } - throw new Error('Unsupported AI provider profile.'); -}; - -const opencodeModel = (claim: Claim) => { - const profile = claim.aiProviderProfile; - const model = profile?.model ?? claim.openai.model; - if (model.includes('/')) return model; - if (!profile) return `openai/${model}`; - if ( - profile.provider === 'custom_openai_compatible' || - profile.provider === 'cloudflare_ai_gateway' - ) { - return model; - } - if (profile.provider === 'opencode_openai_login') return `openai/${model}`; - return `${profile.provider}/${model}`; -}; - -const codexModel = (claim: Claim) => { - const model = claim.aiProviderProfile?.model ?? claim.openai.model; - return model.includes('/') ? (model.split('/').at(-1) ?? model) : model; -}; - -const codexModelArgs = (claim: Claim) => - isCodexLoginProfile(claim) ? [] : ['--model', codexModel(claim)]; - const writeJsonFile = async (filePath: string, content: string) => { let normalized = content.trim(); try { @@ -1432,6 +1279,8 @@ const runClaim = async (claim: Claim) => { boxHandle, githubToken, redact, + // Placeholder until Task 3 resolves the runtime from the claim/profile. + runtime: 'codex', }; if (userEnv) { await appendEvent( @@ -2167,7 +2016,10 @@ export const startWorker = async () => { await sleep(env.pollMs); continue; } - await runClaim(claim); + // The Convex claim still carries the legacy `openai_direct` runtime + // literal, which `runClaim` rejects. Narrow to the shared `Claim` shape + // (widened to the codex/opencode/claude union) at this boundary. + await runClaim(claim as Claim); } catch (error) { console.error(error); await sleep(env.pollMs); diff --git a/apps/agent-worker/tests/unit/agent-runtime.test.ts b/apps/agent-worker/tests/unit/agent-runtime.test.ts new file mode 100644 index 0000000..af5b972 --- /dev/null +++ b/apps/agent-worker/tests/unit/agent-runtime.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, test } from 'vitest'; + +import type { AgentRuntime } from '../../src/runtime/agent-runtime'; +import type { Claim } from '../../src/runtime/provider'; +import { getAdapter, registerAdapter } from '../../src/runtime/agent-runtime'; +import { isCodexLoginProfile, opencodeModel } from '../../src/runtime/provider'; + +describe('agent runtime registry', () => { + test('registerAdapter/getAdapter resolves a registered runtime', () => { + const fake: AgentRuntime = { + name: 'codex', + prepareAuth: async () => {}, + runTurn: async () => ({}), + abort: async () => {}, + }; + registerAdapter('codex', () => fake); + expect(getAdapter('codex').name).toBe('codex'); + }); + + test('getAdapter throws for an unregistered runtime', () => { + expect(() => getAdapter('opencode')).toThrow(/No agent runtime adapter/); + }); +}); + +describe('shared provider helpers re-exported from runtime/provider', () => { + test('isCodexLoginProfile detects opencode login profiles', () => { + expect( + isCodexLoginProfile({ + aiProviderProfile: { provider: 'opencode_openai_login' }, + } as Claim), + ).toBe(true); + expect(isCodexLoginProfile({} as Claim)).toBe(false); + }); + + test('opencodeModel prefixes the provider', () => { + expect( + opencodeModel({ + aiProviderProfile: { provider: 'anthropic', model: 'claude-x' }, + } as Claim), + ).toBe('anthropic/claude-x'); + }); +}); diff --git a/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts b/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts index 5e7c6b4..f415bd0 100644 --- a/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts +++ b/apps/agent-worker/tests/unit/maintenance-workspace-cleanup.test.ts @@ -156,6 +156,7 @@ describe('maintenance workspace cleanup', () => { boxHandle: { boxName: 'spoon-box-tester', release }, githubToken: 'github-token', redact: (value: string) => value, + runtime: 'codex', containerName: 'spoon-agent-job-maintenance', opencodeSession: { client: {} as never, From 613a6c8a2dee5b22804c9f16184b0c4e933f9459 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 18:34:15 -0400 Subject: [PATCH 29/88] fix(agent-jobs): narrow recovery-cron guard so abandoned stops still recover --- packages/backend/convex/agentJobs.ts | 3 ++- .../tests/unit/recover-stale-jobs.test.ts | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 54a79af..4cdbdce 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -1218,7 +1218,8 @@ export const recoverStaleJobs = internalMutation({ .withIndex('by_status', (q) => q.eq('status', status)) .collect(); for (const job of jobs) { - if (isTerminalJob(job)) continue; + if (isTerminalStatus(job.status) || job.completedAt !== undefined) + continue; const last = job.lastHeartbeatAt ?? job.claimedAt ?? job.createdAt; if (last > threshold) continue; const now = Date.now(); diff --git a/packages/backend/tests/unit/recover-stale-jobs.test.ts b/packages/backend/tests/unit/recover-stale-jobs.test.ts index f253add..02b2944 100644 --- a/packages/backend/tests/unit/recover-stale-jobs.test.ts +++ b/packages/backend/tests/unit/recover-stale-jobs.test.ts @@ -161,6 +161,33 @@ describe('recoverStaleJobs', () => { expect(state.thread?.resolvedAt).toBe(resolvedAt); }); + test('reaps an abandoned-stop job that never completed', async () => { + const t = convexTest(schema, modules); + // The user hit "Stop workspace" (markWorkspaceStopped) while the job was + // still running: workspaceStatus flips to 'stopped' but job.status, + // completedAt, and the thread are never finalized. The heartbeat then goes + // stale, so the cron must still reap this job. + const ids = await seedJob(t, { + status: 'running', + workspaceStatus: 'stopped', + // no completedAt: this is NOT a resolved maintenance job + threadStatus: 'running', + claimedAt: Date.now() - 10 * 60 * 1000, + lastHeartbeatAt: Date.now() - 10 * 60 * 1000, + }); + + const result = await t.mutation(internal.agentJobs.recoverStaleJobs, {}); + const state = await readState(t, ids); + + expect(result).toEqual({ recovered: 1 }); + expect(state.job?.status).toBe('timed_out'); + expect(state.job?.workspaceStatus).toBe('expired'); + expect(state.job?.completedAt).toBeTypeOf('number'); + expect(state.request?.status).toBe('failed'); + expect(state.thread?.status).toBe('failed'); + expect(state.thread?.resolvedAt).toBeTypeOf('number'); + }); + test('leaves a running job with a fresh heartbeat untouched', async () => { const t = convexTest(schema, modules); const ids = await seedJob(t, { lastHeartbeatAt: Date.now() }); From bab87cc2d0c583b6bf0a60be34db778c462f1fa4 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 18:34:07 -0400 Subject: [PATCH 30/88] feat(worker): in-box process-group marker + kill helpers for agent turns --- apps/agent-worker/src/runtime/docker.ts | 36 +++++++++++++++++++ .../tests/unit/box-process-kill.test.ts | 35 ++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 apps/agent-worker/tests/unit/box-process-kill.test.ts diff --git a/apps/agent-worker/src/runtime/docker.ts b/apps/agent-worker/src/runtime/docker.ts index f3a23fc..c1181ba 100644 --- a/apps/agent-worker/src/runtime/docker.ts +++ b/apps/agent-worker/src/runtime/docker.ts @@ -391,6 +391,42 @@ export const ensureUserContainer = async (args: { return name; }; +const shellQuote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; + +// Wraps a CLI argv so the process runs as a new session/process-group leader whose +// bash parent carries the marker in its argv (matchable by `pgrep -f`). We do NOT +// `exec` (bash must survive so the marker stays visible); stdout/stderr fds are +// inherited by the CLI so streaming still works. +export const buildMarkedCommand = ( + marker: string, + command: string[], +): string[] => { + const script = `# ${marker}\nexec 0 + [ + `pids=$(pgrep -f ${shellQuote(marker)} || true)`, + `for pid in $pids; do kill -TERM -"$pid" 2>/dev/null || true; done`, + `sleep 2`, + `for pid in $pids; do kill -KILL -"$pid" 2>/dev/null || true; done`, + ].join('\n'); + +export const killBoxProcessesByMarker = async (args: { + containerName: string; + marker: string; +}): Promise => { + await execa( + containerRuntime(), + ['exec', args.containerName, 'bash', '-lc', buildKillScript(args.marker)], + { reject: false, stdin: 'ignore' }, + ); +}; + export const streamExecInContainer = async (args: { containerName: string; command: string[]; diff --git a/apps/agent-worker/tests/unit/box-process-kill.test.ts b/apps/agent-worker/tests/unit/box-process-kill.test.ts new file mode 100644 index 0000000..573e1d1 --- /dev/null +++ b/apps/agent-worker/tests/unit/box-process-kill.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from 'vitest'; + +const loadDocker = async () => { + 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/runtime/docker'); +}; + +describe('in-box process-group marker + kill helpers', () => { + test('buildMarkedCommand wraps argv as a setsid bash session carrying the marker', async () => { + const { buildMarkedCommand } = await loadDocker(); + const argv = buildMarkedCommand('spoon-turn-abc', [ + 'codex', + 'exec', + '--json', + 'hi', + ]); + expect(argv[0]).toBe('setsid'); + expect(argv[1]).toBe('bash'); + expect(argv[2]).toBe('-lc'); + const script = argv[3] ?? ''; + expect(script).toContain('# spoon-turn-abc'); + expect(script.split('\n').at(-1)).toBe("'codex' 'exec' '--json' 'hi'"); + }); + + test('buildKillScript TERM-then-KILLs every group matching the marker', async () => { + const { buildKillScript } = await loadDocker(); + const script = buildKillScript('spoon-turn-abc'); + expect(script).toContain("pgrep -f 'spoon-turn-abc'"); + expect(script).toContain('kill -TERM -"$pid"'); + expect(script).toContain('kill -KILL -"$pid"'); + }); +}); From c1e816741dbc19f3cb6a7649c5f33a8b8750bc6a Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 18:45:10 -0400 Subject: [PATCH 31/88] fix(worker): marked-command survives tail-exec; kill script excludes itself --- apps/agent-worker/src/runtime/docker.ts | 65 ++++++++++++++++--- .../tests/unit/box-process-kill.test.ts | 46 ++++++++++++- ...-07-10-spoon-phase2-runtime-unification.md | 39 +++++++++-- 3 files changed, 132 insertions(+), 18 deletions(-) diff --git a/apps/agent-worker/src/runtime/docker.ts b/apps/agent-worker/src/runtime/docker.ts index c1181ba..e390c9b 100644 --- a/apps/agent-worker/src/runtime/docker.ts +++ b/apps/agent-worker/src/runtime/docker.ts @@ -393,28 +393,75 @@ export const ensureUserContainer = async (args: { const shellQuote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; +// The marker is embedded literally into a `# comment` line and into a `pgrep` +// pattern, so it must not contain shell metacharacters or a newline (either could +// break out of the comment or the pattern). Callers generate it, but validate +// defensively so a malformed marker fails loudly instead of silently corrupting +// the script. +const MARKER_PATTERN = /^[A-Za-z0-9_-]+$/; +const assertMarker = (marker: string) => { + if (!MARKER_PATTERN.test(marker)) { + throw new Error( + `Invalid process marker ${JSON.stringify(marker)}: must match ${String(MARKER_PATTERN)}.`, + ); + } +}; + // Wraps a CLI argv so the process runs as a new session/process-group leader whose // bash parent carries the marker in its argv (matchable by `pgrep -f`). We do NOT -// `exec` (bash must survive so the marker stays visible); stdout/stderr fds are -// inherited by the CLI so streaming still works. +// `exec` the CLI: bash tail-exec's a final simple command, which would replace the +// marker-carrying bash with the CLI and make `pgrep -f ` miss the running +// turn. Keeping a trailing `rc=$?; exit "$rc"` after the CLI means the CLI is no +// longer the script's final statement, so bash survives as the group leader while +// still propagating the CLI's exit code (downstream `normalizeRunResult` relies on +// it — a bare trailing `wait` would mask a non-zero CLI failure as 0). The CLI runs +// in the same process group as this bash, so the kill helper's group signal reaches +// it. stdout/stderr fds are inherited by the CLI so streaming still works. export const buildMarkedCommand = ( marker: string, command: string[], ): string[] => { - const script = `# ${marker}\nexec 0 - [ +// The negative pid (`kill -TERM -"$pgid"`) targets the whole process group, so the +// CLI and any children it spawned die together. +// +// pgrep self-match: this kill script runs via `bash -lc