diff --git a/modules/software-engineer/__tests__/workers/verify.test.ts b/modules/software-engineer/__tests__/workers/verify.test.ts new file mode 100644 index 00000000..1c1ccb30 --- /dev/null +++ b/modules/software-engineer/__tests__/workers/verify.test.ts @@ -0,0 +1,105 @@ +// @ts-nocheck — vitest harness; the worker is @ts-nocheck'd already. +// +// Regression coverage for issue #57: a per-run cost ceiling trip (`result.costCeiling === true`) +// must route through `blockRun` (status='blocked', a resumable config problem) instead of the +// worker's generic `result.error` → status='failed' dead end. An ordinary agent-session crash +// (no `costCeiling` flag) must still take the unchanged `failed` path. +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +const rs = vi.hoisted(() => ({ + starts: [] as any[], ends: [] as any[], blocks: [] as any[], +})); +vi.mock('../../lib/run-state.js', () => ({ + recordPhaseStart: async (_sb: unknown, run: any, phase: string) => { rs.starts.push({ runId: run.id, phase }); }, + recordPhaseEnd: async (_sb: unknown, run: any, phase: string, status: string, summary?: string) => { + rs.ends.push({ runId: run.id, phase, status, summary }); + }, + blockRun: async (_sb: unknown, run: any, phase: string, gate: string, reason: string) => { + rs.blocks.push({ runId: run.id, phase, gate, reason }); + return { blocked: reason }; + }, + writeGate: async () => {}, + listRunPrs: async () => [], +})); + +vi.mock('../../lib/enqueue.js', () => ({ enqueuePhase: async () => {} })); + +vi.mock('../../lib/worktree.js', () => ({ + makeMultiWorkspace: async () => ({ root: '/tmp/x', repos: [], cleanup: async () => {} }), +})); + +vi.mock('../../lib/github.js', () => ({ + githubClient: () => ({ defaultBranch: async () => 'main', compare: async () => ({ files: [] }) }), +})); + +vi.mock('../../lib/git.js', () => ({ redactToken: (msg: string) => msg })); + +vi.mock('../../lib/credentials.js', () => ({ + getProject: async () => ({ + intakeEnabled: true, githubToken: 'ghp_token', modelCred: 'cred', model: 'sonnet', + }), + getCodeRepos: async () => [], +})); + +const runAgentSession = vi.hoisted(() => vi.fn()); +vi.mock('../../lib/phase-runner.js', () => ({ runAgentSession })); + +vi.mock('@supabase/supabase-js', () => ({ + createClient: () => { throw new Error('createClient should not be called in tests'); }, +})); + +import verify from '../../workers/verify.js'; + +function mockSupabase() { + const updates: any[] = []; + const run = { + id: 'run-1', site_id: 'site-1', project_id: 'proj-1', status: 'running', + repo_owner: 'acme', repo_name: 'issues', issue_number: 57, branch_name: 'se/issue-57', + }; + const from = (table: string) => { + const b: any = { + select() { return b; }, + update(row: any) { updates.push({ table, row }); return b; }, + eq() { return b; }, + maybeSingle() { + if (table === 'se_runs') return Promise.resolve({ data: run, error: null }); + return Promise.resolve({ data: null, error: null }); + }, + then(onF: any) { return Promise.resolve({ error: null }).then(onF); }, + }; + return b; + }; + return { supabase: { from }, updates, run }; +} + +describe('verify worker cost ceiling handling (issue #57)', () => { + beforeEach(() => { + rs.starts.length = 0; rs.ends.length = 0; rs.blocks.length = 0; + runAgentSession.mockReset(); + }); + + it('routes a cost-ceiling trip through blockRun instead of failing the run', async () => { + const msg = 'cost ceiling reached: this run has spent $22.55 of its $20.00 per-run ceiling — raise it in Setup or split the issue'; + runAgentSession.mockImplementation(async () => ({ error: msg, costCeiling: true })); + + const { supabase, updates } = mockSupabase(); + const result = await verify({ data: { runId: 'run-1' } }, { supabase }); + + expect(result).toEqual({ blocked: msg }); + expect(rs.blocks).toEqual([{ runId: 'run-1', phase: 'verify', gate: 'cost_ceiling', reason: msg }]); + expect(rs.ends.find((e) => e.status === 'failed')).toBeUndefined(); + expect(updates.find((u) => u.table === 'se_runs' && u.row?.status === 'failed')).toBeUndefined(); + }); + + it('leaves an ordinary (non-ceiling) error on the unchanged failed path', async () => { + runAgentSession.mockImplementation(async () => ({ error: 'agent session crashed' })); + + const { supabase, updates } = mockSupabase(); + const result = await verify({ data: { runId: 'run-1' } }, { supabase }); + + expect(result).toEqual({ failed: 'agent session crashed' }); + expect(rs.blocks).toEqual([]); + expect(rs.ends.at(-1)).toMatchObject({ phase: 'verify', status: 'failed', summary: 'agent session crashed' }); + expect(updates.find((u) => u.table === 'se_runs')?.row).toMatchObject({ status: 'failed' }); + }); +}); diff --git a/modules/software-engineer/admin/__tests__/resume-route.test.ts b/modules/software-engineer/admin/__tests__/resume-route.test.ts index 1254f37b..7eb22be2 100644 --- a/modules/software-engineer/admin/__tests__/resume-route.test.ts +++ b/modules/software-engineer/admin/__tests__/resume-route.test.ts @@ -250,6 +250,29 @@ describe('POST /runs/:id/resume', () => { expect(msg.row.content).toContain('intake disabled'); }); + it('resumes a cost-ceiling-blocked run into the gated phase (issue #57)', async () => { + const supabase = mockSupabase({ + run: failedRun({ + status: 'blocked', + current_phase: 'verify', + error: 'cost ceiling reached: this run has spent $22.55 of its $20.00 per-run ceiling — raise it in Setup or split the issue', + }), + prs: [], + attemptCount: 1, + }); + const { router, enqueued } = mount(supabase); + const res = mockRes(); + await router.handler('POST /runs/:id/resume')({ params: { id: RID } }, res); + expect(res.statusCode).toBe(200); + expect(res.body).toEqual({ resumed: true, phase: 'verify', attempt: 2 }); + expect(enqueued).toEqual([[ + 'se', 'software-engineer:verify', { runId: RID, attempt: 2 }, + { jobId: `se-run-${RID}-verify`, removeOnComplete: true, removeOnFail: true }, + ]]); + const msg = supabase.__calls.inserts.find((c: any) => c.table === 'se_messages'); + expect(msg.row.content).toContain('cost ceiling reached'); + }); + it('500s when the atomic status-guarded update fails', async () => { const { router } = mount(mockSupabase({ run: failedRun(), diff --git a/modules/software-engineer/lib/__tests__/decision-kind.test.ts b/modules/software-engineer/lib/__tests__/decision-kind.test.ts index c7bec73d..1bcd3e5e 100644 --- a/modules/software-engineer/lib/__tests__/decision-kind.test.ts +++ b/modules/software-engineer/lib/__tests__/decision-kind.test.ts @@ -53,6 +53,15 @@ describe('classifyDecision', () => { const prs = [{ state: 'open' }, { state: 'merged' }]; expect(classifyDecision(run, prs)).toBe('config_blocked'); }); + + it('classifies a cost-ceiling block as config_blocked with the raise-the-ceiling text (issue #57)', () => { + const run = { + status: 'blocked', + error: 'cost ceiling reached: this run has spent $22.55 of its $20.00 per-run ceiling — raise it in Setup or split the issue', + }; + expect(classifyDecision(run, [])).toBe('config_blocked'); + expect(decisionTextFor('config_blocked', run)).toBe(run.error); + }); }); describe('decisionTextFor', () => { diff --git a/modules/software-engineer/lib/__tests__/phase-runner.test.ts b/modules/software-engineer/lib/__tests__/phase-runner.test.ts index 6982ead3..5bf194e8 100644 --- a/modules/software-engineer/lib/__tests__/phase-runner.test.ts +++ b/modules/software-engineer/lib/__tests__/phase-runner.test.ts @@ -123,3 +123,64 @@ describe('runAgentSession heartbeat cost attribution (issue #55)', () => { clearIntervalSpy.mockRestore(); }); }); + +// Pins issue #57: a per-run cost ceiling trip must be machine-distinguishable from an ordinary +// agent-session error, so the five phase workers that hard-fail on `result.error` can instead park +// the run as `blocked` (a config problem, not a crash) rather than dead-ending it as `failed`. +describe('runAgentSession cost ceiling (issue #57)', () => { + const RUN = { id: 'run-1', site_id: 'site-1', project_id: 'proj-1', title: 'fix bug' }; + const SPEC = { cwd: '/tmp/x', prompt: 'do it', repos: [], allowedTools: [] }; + + beforeEach(() => { __runPhase.mockReset(); }); + + // Like fakeSupabase, but se_runs.maybeSingle() resolves to a configurable cost_usd — the ceiling + // check reads this fresh instead of trusting the in-memory `run` object. + function fakeSupabaseWithRunCost(runCostUsd: number) { + const from = (table: string) => { + const b: any = { + select() { return b; }, + insert() { return Promise.resolve({ data: null, error: null }); }, + update() { return b; }, + upsert() { return Promise.resolve({ data: null, error: null }); }, + eq() { return b; }, + maybeSingle() { + return Promise.resolve( + table === 'se_runs' ? { data: { cost_usd: runCostUsd }, error: null } : { data: null, error: null }, + ); + }, + then(onF: any, onR: any) { return Promise.resolve({ data: [], error: null, count: 0 }).then(onF, onR); }, + }; + return b; + }; + return { from }; + } + + it('returns costCeiling: true and the unchanged error message once spend crosses the ceiling, without invoking the runner', async () => { + const supa = fakeSupabaseWithRunCost(22.55); + const PROJECT = { modelCredKind: 'api_key', modelCred: 'x', perRunCostCeilingUSD: 20 }; + + const result = await runAgentSession(supa, {}, RUN, PROJECT, 'verify', SPEC); + + expect(result.costCeiling).toBe(true); + expect(result.error).toBe( + 'cost ceiling reached: this run has spent $22.55 of its $20.00 per-run ceiling — raise it in Setup or split the issue', + ); + expect(__runPhase).not.toHaveBeenCalled(); + }); + + it('does not trip when spend is below the ceiling', async () => { + const supa = fakeSupabaseWithRunCost(5); + const PROJECT = { modelCredKind: 'api_key', modelCred: 'x', perRunCostCeilingUSD: 20 }; + + __runPhase.mockImplementation((opts: any) => { + opts.onUsage?.({}); + return Promise.resolve({ text: 'ok', costUSD: 0.1, tokensInput: 1, tokensOutput: 1 }); + }); + + const result = await runAgentSession(supa, {}, RUN, PROJECT, 'verify', SPEC); + + expect(result.costCeiling).toBeUndefined(); + expect(result.error).toBeUndefined(); + expect(__runPhase).toHaveBeenCalled(); + }); +}); diff --git a/modules/software-engineer/lib/phase-runner.ts b/modules/software-engineer/lib/phase-runner.ts index 8c5dddf6..5a640998 100644 --- a/modules/software-engineer/lib/phase-runner.ts +++ b/modules/software-engineer/lib/phase-runner.ts @@ -165,6 +165,7 @@ export async function runAgentSession(supabase, ctx, run, project, phase, spec) const spent = Number(fresh?.cost_usd) || 0; if (spent >= project.perRunCostCeilingUSD) { return { text: '', tokensInput: 0, tokensOutput: 0, tokensCacheRead: 0, tokensCacheCreation: 0, costUSD: 0, interrupted: false, + costCeiling: true, error: `cost ceiling reached: this run has spent $${spent.toFixed(2)} of its $${project.perRunCostCeilingUSD.toFixed(2)} per-run ceiling — raise it in Setup or split the issue` }; } } catch { /* ceiling check is best-effort — never block a run on a read blip */ } diff --git a/modules/software-engineer/workers/architecture.ts b/modules/software-engineer/workers/architecture.ts index c403284c..9e0b152e 100644 --- a/modules/software-engineer/workers/architecture.ts +++ b/modules/software-engineer/workers/architecture.ts @@ -126,6 +126,7 @@ export default async function architecture(job, ctx) { }); if (result.error) { const msg = redactToken(result.error, token); + if (result.costCeiling) return blockRun(supabase, run, 'architecture', 'cost_ceiling', msg); await recordPhaseEnd(supabase, run, 'architecture', 'failed', msg, { model: result.modelUsed ?? project.model, engine: result.engineUsed ?? 'claude', input: result.tokensInput, output: result.tokensOutput, cacheRead: result.tokensCacheRead, cacheCreation: result.tokensCacheCreation, cost: result.costUSD, modelUsage: result.modelUsage }); await supabase.from('se_runs').update({ status: 'failed', error: msg }).eq('id', run.id); return { failed: msg }; diff --git a/modules/software-engineer/workers/implement.ts b/modules/software-engineer/workers/implement.ts index e7e20982..51b644a8 100644 --- a/modules/software-engineer/workers/implement.ts +++ b/modules/software-engineer/workers/implement.ts @@ -63,6 +63,7 @@ export default async function implement(job, ctx) { }); if (result.error) { const msg = redactToken(result.error, token); + if (result.costCeiling) return blockRun(supabase, run, 'implement', 'cost_ceiling', msg); await recordPhaseEnd(supabase, run, 'implement', 'failed', msg, { model: result.modelUsed ?? project.model, engine: result.engineUsed ?? 'claude', input: result.tokensInput, output: result.tokensOutput, cacheRead: result.tokensCacheRead, cacheCreation: result.tokensCacheCreation, cost: result.costUSD, modelUsage: result.modelUsage }); await supabase.from('se_runs').update({ status: 'failed', error: msg }).eq('id', run.id); return { failed: msg }; diff --git a/modules/software-engineer/workers/review.ts b/modules/software-engineer/workers/review.ts index cc0bb110..ced611f3 100644 --- a/modules/software-engineer/workers/review.ts +++ b/modules/software-engineer/workers/review.ts @@ -76,6 +76,7 @@ export default async function review(job, ctx) { }); if (result.error) { const msg = redactToken(result.error, token); + if (result.costCeiling) return blockRun(supabase, run, 'review', 'cost_ceiling', msg); await recordPhaseEnd(supabase, run, 'review', 'failed', msg, { model: result.modelUsed ?? project.model, engine: result.engineUsed ?? 'claude', input: result.tokensInput, output: result.tokensOutput, cacheRead: result.tokensCacheRead, cacheCreation: result.tokensCacheCreation, cost: result.costUSD, modelUsage: result.modelUsage }); await supabase.from('se_runs').update({ status: 'failed', error: msg }).eq('id', run.id); return { failed: msg }; diff --git a/modules/software-engineer/workers/spec.ts b/modules/software-engineer/workers/spec.ts index 7e706c03..7d4d0a3f 100644 --- a/modules/software-engineer/workers/spec.ts +++ b/modules/software-engineer/workers/spec.ts @@ -81,6 +81,7 @@ export default async function spec(job, ctx) { }); if (result.error) { const msg = redactToken(result.error, token); + if (result.costCeiling) return blockRun(supabase, run, 'spec', 'cost_ceiling', msg); await recordPhaseEnd(supabase, run, 'spec', 'failed', msg, { model: result.modelUsed ?? project.model, engine: result.engineUsed ?? 'claude', input: result.tokensInput, output: result.tokensOutput, cacheRead: result.tokensCacheRead, cacheCreation: result.tokensCacheCreation, cost: result.costUSD, modelUsage: result.modelUsage }); await supabase.from('se_runs').update({ status: 'failed', error: msg }).eq('id', run.id); return { failed: msg }; diff --git a/modules/software-engineer/workers/verify.ts b/modules/software-engineer/workers/verify.ts index 4433c676..0da15d6a 100644 --- a/modules/software-engineer/workers/verify.ts +++ b/modules/software-engineer/workers/verify.ts @@ -81,6 +81,7 @@ export default async function verify(job, ctx) { }); if (result.error) { const msg = redactToken(result.error, token); + if (result.costCeiling) return blockRun(supabase, run, 'verify', 'cost_ceiling', msg); await recordPhaseEnd(supabase, run, 'verify', 'failed', msg, { model: result.modelUsed ?? project.model, engine: result.engineUsed ?? 'claude', input: result.tokensInput, output: result.tokensOutput, cacheRead: result.tokensCacheRead, cacheCreation: result.tokensCacheCreation, cost: result.costUSD, modelUsage: result.modelUsage }); await supabase.from('se_runs').update({ status: 'failed', error: msg }).eq('id', run.id); return { failed: msg };