feat: integrate push into update_job_status DONE transition
On status=done, calls pushBranchForJob before DB write: - pushed=true → DONE + pushed_at + branch set + worktree cleanup (keepBranch=true) - no-changes → DONE without pushed_at + worktree cleanup - push failure → FAILED with error message + worktree preserved for manual inspection Also adds pushed_at to vendored prisma schema + regenerates client. 6 unit tests for prepareDoneUpdate covering all push outcomes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
fbfaf905c8
commit
8ebf4ff895
3 changed files with 194 additions and 10 deletions
110
__tests__/update-job-status-push.test.ts
Normal file
110
__tests__/update-job-status-push.test.ts
Normal file
|
|
@ -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<typeof vi.fn>
|
||||
|
||||
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)
|
||||
})
|
||||
})
|
||||
|
|
@ -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?
|
||||
|
|
|
|||
|
|
@ -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<DoneUpdatePlan> {
|
||||
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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue