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'); + }); +});