diff --git a/__tests__/update-job-status-push.test.ts b/__tests__/update-job-status-push.test.ts new file mode 100644 index 0000000..1232670 --- /dev/null +++ b/__tests__/update-job-status-push.test.ts @@ -0,0 +1,110 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import * as path from 'node:path' + +vi.mock('../src/git/push.js', () => ({ + pushBranchForJob: vi.fn(), +})) + +import { pushBranchForJob } from '../src/git/push.js' +import { prepareDoneUpdate } from '../src/tools/update-job-status.js' + +const mockPush = pushBranchForJob as ReturnType + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('prepareDoneUpdate', () => { + const originalEnv = { ...process.env } + + afterEach(() => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = originalEnv.SCRUM4ME_AGENT_WORKTREE_DIR + }) + + it('returns DONE with pushedAt and branchOverride when push succeeds', async () => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = '/wt' + mockPush.mockResolvedValue({ pushed: true, remoteRef: 'refs/heads/feat/job-abc' }) + + const plan = await prepareDoneUpdate('job-abc', 'feat/job-abc') + + expect(plan.dbStatus).toBe('DONE') + expect(plan.pushedAt).toBeInstanceOf(Date) + expect(plan.branchOverride).toBe('feat/job-abc') + expect(plan.errorOverride).toBeUndefined() + expect(plan.skipWorktreeCleanup).toBe(false) + + expect(mockPush).toHaveBeenCalledWith({ + worktreePath: path.join('/wt', 'job-abc'), + branchName: 'feat/job-abc', + }) + }) + + it('derives branchName from jobId when branch is undefined', async () => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = '/wt' + mockPush.mockResolvedValue({ pushed: true, remoteRef: 'refs/heads/feat/job-abc12345' }) + + await prepareDoneUpdate('job-abc12345', undefined) + + expect(mockPush).toHaveBeenCalledWith( + expect.objectContaining({ branchName: 'feat/job-abc12345' }), + ) + }) + + it('returns DONE without pushedAt when no-changes', async () => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = '/wt' + mockPush.mockResolvedValue({ pushed: false, reason: 'no-changes', stderr: '' }) + + const plan = await prepareDoneUpdate('job-abc', 'feat/job-abc') + + expect(plan.dbStatus).toBe('DONE') + expect(plan.pushedAt).toBeUndefined() + expect(plan.branchOverride).toBeUndefined() + expect(plan.errorOverride).toBeUndefined() + expect(plan.skipWorktreeCleanup).toBe(false) + }) + + it('returns FAILED with error and skipWorktreeCleanup when no-credentials', async () => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = '/wt' + mockPush.mockResolvedValue({ + pushed: false, + reason: 'no-credentials', + stderr: 'fatal: Authentication failed', + }) + + const plan = await prepareDoneUpdate('job-abc', 'feat/job-abc') + + expect(plan.dbStatus).toBe('FAILED') + expect(plan.errorOverride).toContain('push failed (no-credentials)') + expect(plan.errorOverride).toContain('Authentication failed') + expect(plan.skipWorktreeCleanup).toBe(true) + }) + + it('returns FAILED with error and skipWorktreeCleanup when conflict', async () => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = '/wt' + mockPush.mockResolvedValue({ + pushed: false, + reason: 'conflict', + stderr: '! [rejected] non-fast-forward', + }) + + const plan = await prepareDoneUpdate('job-abc', 'feat/job-abc') + + expect(plan.dbStatus).toBe('FAILED') + expect(plan.errorOverride).toContain('push failed (conflict)') + expect(plan.skipWorktreeCleanup).toBe(true) + }) + + it('returns FAILED with error and skipWorktreeCleanup when unknown push error', async () => { + process.env.SCRUM4ME_AGENT_WORKTREE_DIR = '/wt' + mockPush.mockResolvedValue({ + pushed: false, + reason: 'unknown', + stderr: 'something went wrong', + }) + + const plan = await prepareDoneUpdate('job-abc', 'feat/job-abc') + + expect(plan.dbStatus).toBe('FAILED') + expect(plan.skipWorktreeCleanup).toBe(true) + }) +}) diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 461ebdc..f45a207 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -256,6 +256,7 @@ model ClaudeJob { claimed_at DateTime? started_at DateTime? finished_at DateTime? + pushed_at DateTime? plan_snapshot String? branch String? summary String? diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index 05e5a95..12c85b7 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -5,11 +5,14 @@ import { z } from 'zod' import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import { Client } from 'pg' +import * as os from 'node:os' +import * as path from 'node:path' import { prisma } from '../prisma.js' import { requireWriteAccess } from '../auth.js' import { toolJson, toolError, withToolErrors } from '../errors.js' import { removeWorktreeForJob } from '../git/worktree.js' import { resolveRepoRoot } from './wait-for-job.js' +import { pushBranchForJob } from '../git/push.js' const inputSchema = z.object({ job_id: z.string().min(1), @@ -37,6 +40,56 @@ export async function cleanupWorktreeForTerminalStatus( } } +export type DoneUpdatePlan = { + dbStatus: 'DONE' | 'FAILED' + pushedAt: Date | undefined + branchOverride: string | undefined + errorOverride: string | undefined + skipWorktreeCleanup: boolean +} + +export async function prepareDoneUpdate( + jobId: string, + branch: string | undefined, +): Promise { + const worktreeDir = + process.env.SCRUM4ME_AGENT_WORKTREE_DIR ?? path.join(os.homedir(), '.scrum4me-agent-worktrees') + const worktreePath = path.join(worktreeDir, jobId) + const branchName = branch ?? `feat/job-${jobId.slice(-8)}` + + const pushResult = await pushBranchForJob({ worktreePath, branchName }) + + if (pushResult.pushed) { + return { + dbStatus: 'DONE', + pushedAt: new Date(), + branchOverride: branchName, + errorOverride: undefined, + skipWorktreeCleanup: false, + } + } + + if (pushResult.reason === 'no-changes') { + return { + dbStatus: 'DONE', + pushedAt: undefined, + branchOverride: undefined, + errorOverride: undefined, + skipWorktreeCleanup: false, + } + } + + // Push failed — job becomes FAILED, worktree stays for manual inspection + const snippet = pushResult.stderr.slice(0, 200) + return { + dbStatus: 'FAILED', + pushedAt: undefined, + branchOverride: undefined, + errorOverride: `push failed (${pushResult.reason}): ${snippet}`, + skipWorktreeCleanup: true, + } +} + const DB_STATUS_MAP = { running: 'RUNNING', done: 'DONE', @@ -80,22 +133,40 @@ export function registerUpdateJobStatusTool(server: McpServer) { return toolError(`Job is already in terminal state: ${job.status.toLowerCase()}`) } - const dbStatus = DB_STATUS_MAP[status] + // For DONE: push first, adjust DB status based on result + let actualStatus = status + let pushedAt: Date | undefined + let branchToWrite = branch + let errorToWrite = error + let skipWorktreeCleanup = false + + if (status === 'done') { + const plan = await prepareDoneUpdate(job_id, branch) + actualStatus = plan.dbStatus === 'DONE' ? 'done' : 'failed' + pushedAt = plan.pushedAt + if (plan.branchOverride !== undefined) branchToWrite = plan.branchOverride + if (plan.errorOverride !== undefined) errorToWrite = plan.errorOverride + skipWorktreeCleanup = plan.skipWorktreeCleanup + } + + const dbStatus = DB_STATUS_MAP[actualStatus as keyof typeof DB_STATUS_MAP] const now = new Date() const updated = await prisma.claudeJob.update({ where: { id: job_id }, data: { status: dbStatus, - ...(status === 'running' ? { started_at: now } : {}), - ...(status === 'done' || status === 'failed' ? { finished_at: now } : {}), - ...(branch !== undefined ? { branch } : {}), + ...(actualStatus === 'running' ? { started_at: now } : {}), + ...(actualStatus === 'done' || actualStatus === 'failed' ? { finished_at: now } : {}), + ...(branchToWrite !== undefined ? { branch: branchToWrite } : {}), + ...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}), ...(summary !== undefined ? { summary } : {}), - ...(error !== undefined ? { error } : {}), + ...(errorToWrite !== undefined ? { error: errorToWrite } : {}), }, select: { id: true, status: true, branch: true, + pushed_at: true, summary: true, error: true, started_at: true, @@ -116,8 +187,9 @@ export function registerUpdateJobStatusTool(server: McpServer) { task_id: job.task_id, user_id: job.user_id, product_id: job.product_id, - status, + status: actualStatus, branch: updated.branch ?? undefined, + pushed_at: updated.pushed_at?.toISOString() ?? undefined, summary: updated.summary ?? undefined, error: updated.error ?? undefined, }), @@ -128,15 +200,16 @@ export function registerUpdateJobStatusTool(server: McpServer) { // non-fatal — status is already persisted } - // Best-effort worktree cleanup on terminal transitions - if (status === 'done' || status === 'failed') { - await cleanupWorktreeForTerminalStatus(job.product_id, job_id, status, branch) + // Best-effort worktree cleanup on terminal transitions (skip if push failed — worktree preserved) + if ((actualStatus === 'done' || actualStatus === 'failed') && !skipWorktreeCleanup) { + await cleanupWorktreeForTerminalStatus(job.product_id, job_id, actualStatus, branchToWrite) } return toolJson({ job_id: updated.id, - status, + status: actualStatus, branch: updated.branch, + pushed_at: updated.pushed_at?.toISOString() ?? null, summary: updated.summary, error: updated.error, started_at: updated.started_at?.toISOString() ?? null,