diff --git a/__tests__/git/pr.test.ts b/__tests__/git/pr.test.ts index 6d8cc72..18a1949 100644 --- a/__tests__/git/pr.test.ts +++ b/__tests__/git/pr.test.ts @@ -12,7 +12,7 @@ vi.mock('node:util', () => ({ ), })) -import { createPullRequest } from '../../src/git/pr.js' +import { createPullRequest, markPullRequestReady } from '../../src/git/pr.js' beforeEach(() => { vi.clearAllMocks() @@ -66,4 +66,80 @@ describe('createPullRequest', () => { expect(result).toMatchObject({ error: expect.stringContaining('gh pr create failed') }) }) + + it('passes --draft when draft=true en slaat auto-merge over', async () => { + const calls: string[][] = [] + mockExecFile.mockImplementation( + ( + _cmd: string, + args: string[], + _opts: unknown, + cb: (err: null, res: { stdout: string; stderr: string }) => void, + ) => { + calls.push(args) + cb(null, { + stdout: 'Creating draft pull request...\nhttps://github.com/org/repo/pull/100\n', + stderr: '', + }) + }, + ) + + const result = await createPullRequest({ + worktreePath: '/wt/sprint-1', + branchName: 'feat/sprint-12345678', + title: 'Sprint: Cascade-flow live', + body: 'Sprint draft', + draft: true, + enableAutoMerge: false, + }) + + expect(result).toEqual({ url: 'https://github.com/org/repo/pull/100' }) + expect(calls.some((a) => a.includes('--draft'))).toBe(true) + // gh pr merge --auto mag NIET gestart zijn voor draft + auto-merge=false + expect(calls.some((a) => a[0] === 'pr' && a[1] === 'merge')).toBe(false) + }) +}) + +describe('markPullRequestReady', () => { + it('roept gh pr ready aan met de PR-URL', async () => { + const calls: string[][] = [] + mockExecFile.mockImplementation( + ( + _cmd: string, + args: string[], + _opts: unknown, + cb: (err: null, res: { stdout: string; stderr: string }) => void, + ) => { + calls.push(args) + cb(null, { stdout: '', stderr: '' }) + }, + ) + + const result = await markPullRequestReady({ prUrl: 'https://github.com/org/repo/pull/100' }) + + expect(result).toEqual({ ok: true }) + expect(calls[0]).toEqual(['pr', 'ready', 'https://github.com/org/repo/pull/100']) + }) + + it('behandelt "already ready" als success', async () => { + mockExecFile.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: (err: Error) => void) => + cb(Object.assign(new Error(''), { stderr: 'Pull request is not in draft state' })), + ) + + const result = await markPullRequestReady({ prUrl: 'https://github.com/org/repo/pull/100' }) + + expect(result).toEqual({ ok: true }) + }) + + it('retourneert error op onverwachte gh-fout', async () => { + mockExecFile.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: (err: Error) => void) => + cb(new Error('rate limit exceeded')), + ) + + const result = await markPullRequestReady({ prUrl: 'https://github.com/org/repo/pull/100' }) + + expect(result).toMatchObject({ error: expect.stringContaining('gh pr ready failed') }) + }) }) diff --git a/__tests__/update-job-status-auto-pr.test.ts b/__tests__/update-job-status-auto-pr.test.ts index 4a901ad..3218b3e 100644 --- a/__tests__/update-job-status-auto-pr.test.ts +++ b/__tests__/update-job-status-auto-pr.test.ts @@ -4,12 +4,13 @@ vi.mock('../src/prisma.js', () => ({ prisma: { product: { findUnique: vi.fn() }, task: { findUnique: vi.fn() }, - claudeJob: { findFirst: vi.fn() }, + claudeJob: { findFirst: vi.fn(), findUnique: vi.fn() }, }, })) vi.mock('../src/git/pr.js', () => ({ createPullRequest: vi.fn(), + markPullRequestReady: vi.fn(), })) import { prisma } from '../src/prisma.js' @@ -19,7 +20,10 @@ import { maybeCreateAutoPr } from '../src/tools/update-job-status.js' const mockPrisma = prisma as unknown as { product: { findUnique: ReturnType } task: { findUnique: ReturnType } - claudeJob: { findFirst: ReturnType } + claudeJob: { + findFirst: ReturnType + findUnique: ReturnType + } } const mockCreatePr = createPullRequest as ReturnType @@ -40,6 +44,8 @@ beforeEach(() => { story: { id: 'story-1', code: 'SCRUM-42', title: 'Story title' }, }) mockPrisma.claudeJob.findFirst.mockResolvedValue(null) // no sibling PR by default + // Default: legacy job zonder sprint_run (STORY-mode pad). + mockPrisma.claudeJob.findUnique.mockResolvedValue({ sprint_run_id: null, sprint_run: null }) mockCreatePr.mockResolvedValue({ url: 'https://github.com/org/repo/pull/99' }) }) @@ -80,6 +86,41 @@ describe('maybeCreateAutoPr', () => { ) }) + it('SPRINT-mode: maakt een draft-PR aan met sprint-titel, geen auto-merge', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: 'run-1', + sprint_run: { + id: 'run-1', + pr_strategy: 'SPRINT', + sprint: { sprint_goal: 'Cascade-flow live' }, + }, + }) + + const url = await maybeCreateAutoPr(BASE_OPTS) + + expect(url).toBe('https://github.com/org/repo/pull/99') + expect(mockCreatePr).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Sprint: Cascade-flow live', + draft: true, + enableAutoMerge: false, + }), + ) + }) + + it('SPRINT-mode: hergebruikt sibling-PR binnen dezelfde SprintRun', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: 'run-1', + sprint_run: { id: 'run-1', pr_strategy: 'SPRINT', sprint: { sprint_goal: 'Goal' } }, + }) + mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/55' }) + + const url = await maybeCreateAutoPr(BASE_OPTS) + + expect(url).toBe('https://github.com/org/repo/pull/55') + expect(mockCreatePr).not.toHaveBeenCalled() + }) + it('returns null and does not throw when gh fails', async () => { mockCreatePr.mockResolvedValue({ error: 'gh CLI not found' }) const url = await maybeCreateAutoPr(BASE_OPTS) diff --git a/__tests__/wait-for-job-branch-resolution.test.ts b/__tests__/wait-for-job-branch-resolution.test.ts new file mode 100644 index 0000000..b85081f --- /dev/null +++ b/__tests__/wait-for-job-branch-resolution.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('../src/prisma.js', () => ({ + prisma: { + claudeJob: { + findUnique: vi.fn(), + findFirst: vi.fn(), + }, + }, +})) + +import { prisma } from '../src/prisma.js' +import { resolveBranchForJob } from '../src/tools/wait-for-job.js' + +const mockPrisma = prisma as unknown as { + claudeJob: { + findUnique: ReturnType + findFirst: ReturnType + } +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('resolveBranchForJob — sprint-aware', () => { + it('SPRINT-mode: kiest feat/sprint- en marks reused=false bij eerste task', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: 'run-cuid-12345678', + sprint_run: { id: 'run-cuid-12345678', pr_strategy: 'SPRINT' }, + }) + mockPrisma.claudeJob.findFirst.mockResolvedValue(null) + + const result = await resolveBranchForJob('job-1', 'story-anything') + + expect(result.branchName).toBe('feat/sprint-12345678') + expect(result.reused).toBe(false) + }) + + it('SPRINT-mode: marks reused=true wanneer sibling al de branch gebruikt', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: 'run-cuid-12345678', + sprint_run: { id: 'run-cuid-12345678', pr_strategy: 'SPRINT' }, + }) + mockPrisma.claudeJob.findFirst.mockResolvedValue({ branch: 'feat/sprint-12345678' }) + + const result = await resolveBranchForJob('job-2', 'story-anything') + + expect(result.branchName).toBe('feat/sprint-12345678') + expect(result.reused).toBe(true) + }) + + it('STORY-mode (sprint-flow): valt terug op story-branch via legacy-pad', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: 'run-cuid-12345678', + sprint_run: { id: 'run-cuid-12345678', pr_strategy: 'STORY' }, + }) + mockPrisma.claudeJob.findFirst.mockResolvedValue(null) + + const result = await resolveBranchForJob('job-1', 'story-cuid-87654321') + + expect(result.branchName).toBe('feat/story-87654321') + expect(result.reused).toBe(false) + }) + + it('Legacy (geen sprint_run): bestaand gedrag — feat/story-', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: null, + sprint_run: null, + }) + mockPrisma.claudeJob.findFirst.mockResolvedValue(null) + + const result = await resolveBranchForJob('job-1', 'story-cuid-87654321') + + expect(result.branchName).toBe('feat/story-87654321') + expect(result.reused).toBe(false) + }) + + it('Legacy: hergebruik branch wanneer sibling-job in dezelfde story al een branch heeft', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: null, + sprint_run: null, + }) + mockPrisma.claudeJob.findFirst.mockResolvedValue({ branch: 'feat/story-87654321' }) + + const result = await resolveBranchForJob('job-2', 'story-cuid-87654321') + + expect(result.branchName).toBe('feat/story-87654321') + expect(result.reused).toBe(true) + }) +}) diff --git a/__tests__/wait-for-job-worktree.test.ts b/__tests__/wait-for-job-worktree.test.ts index c594fab..c03e91d 100644 --- a/__tests__/wait-for-job-worktree.test.ts +++ b/__tests__/wait-for-job-worktree.test.ts @@ -6,7 +6,7 @@ import * as fs from 'node:fs/promises' vi.mock('../src/prisma.js', () => ({ prisma: { $executeRaw: vi.fn(), - claudeJob: { findFirst: vi.fn() }, + claudeJob: { findFirst: vi.fn(), findUnique: vi.fn() }, product: { findUnique: vi.fn() }, }, })) @@ -21,13 +21,15 @@ import { resolveRepoRoot, rollbackClaim, attachWorktreeToJob } from '../src/tool const mockPrisma = prisma as unknown as { $executeRaw: ReturnType - claudeJob: { findFirst: ReturnType } + claudeJob: { findFirst: ReturnType; findUnique: ReturnType } product: { findUnique: ReturnType } } const mockCreateWorktree = createWorktreeForJob as ReturnType beforeEach(() => { vi.clearAllMocks() + // Default: legacy job zonder sprint_run (oude flow). + mockPrisma.claudeJob.findUnique.mockResolvedValue({ sprint_run_id: null, sprint_run: null }) }) describe('resolveRepoRoot', () => { diff --git a/src/git/pr.ts b/src/git/pr.ts index f30ac8e..1fb72bf 100644 --- a/src/git/pr.ts +++ b/src/git/pr.ts @@ -10,16 +10,18 @@ export async function createPullRequest(opts: { branchName: string title: string body: string + /** Open as draft PR (mens moet 'm later ready-for-review zetten). Default false. */ + draft?: boolean + /** Schakel auto-merge (squash) in. Default true. Voor sprint-mode: false. */ + enableAutoMerge?: boolean }): Promise<{ url: string } | { error: string }> { - const { worktreePath, branchName, title, body } = opts + const { worktreePath, branchName, title, body, draft = false, enableAutoMerge = true } = opts let url: string try { - const { stdout } = await exec( - 'gh', - ['pr', 'create', '--title', title, '--body', body, '--head', branchName], - { cwd: worktreePath }, - ) + const args = ['pr', 'create', '--title', title, '--body', body, '--head', branchName] + if (draft) args.push('--draft') + const { stdout } = await exec('gh', args, { cwd: worktreePath }) // gh prints the PR URL as the last non-empty line const lines = stdout.trim().split('\n').filter(Boolean) url = lines[lines.length - 1]?.trim() ?? '' @@ -43,19 +45,41 @@ export async function createPullRequest(opts: { // gh exits non-zero and we just log. The PR is still valid; auto-merge can // be turned on manually. We do NOT fail the whole createPullRequest call — // the URL was successfully obtained which is the contract this returns. - try { - await exec('gh', ['pr', 'merge', '--auto', '--squash', url], { cwd: worktreePath }) - } catch (err) { - const stderr = - (err as { stderr?: string }).stderr ?? (err as Error).message ?? '' - console.warn( - `[createPullRequest] auto-merge enable failed for ${url}: ${stderr.slice(0, 200)}`, - ) + // Bij draft + sprint-flow slaan we dit over: de PR moet eerst handmatig of + // via markPullRequestReady ready-for-review worden gezet. + if (enableAutoMerge && !draft) { + try { + await exec('gh', ['pr', 'merge', '--auto', '--squash', url], { cwd: worktreePath }) + } catch (err) { + const stderr = + (err as { stderr?: string }).stderr ?? (err as Error).message ?? '' + console.warn( + `[createPullRequest] auto-merge enable failed for ${url}: ${stderr.slice(0, 200)}`, + ) + } } return { url } } +// Zet een draft-PR over naar "ready for review". Gebruikt bij sprint-mode +// wanneer alle stories in de SprintRun DONE zijn — mens reviewt en mergt zelf. +export async function markPullRequestReady(opts: { + prUrl: string + cwd?: string +}): Promise<{ ok: true } | { error: string }> { + try { + await exec('gh', ['pr', 'ready', opts.prUrl], opts.cwd ? { cwd: opts.cwd } : {}) + return { ok: true } + } catch (err) { + const msg = (err as { stderr?: string }).stderr ?? (err as Error).message ?? '' + // gh-CLI fout "Pull request is not in draft state" is benign wanneer de + // PR al ready was (bv. handmatig ready gezet of een tweede call). + if (/not in draft state|already in ready/i.test(msg)) return { ok: true } + return { error: `gh pr ready failed: ${msg.slice(0, 300)}` } + } +} + export type PrState = 'OPEN' | 'MERGED' | 'CLOSED' export type PrInfo = { diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index 41eb9cd..3a24fbd 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -13,7 +13,7 @@ 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' -import { createPullRequest } from '../git/pr.js' +import { createPullRequest, markPullRequestReady } from '../git/pr.js' import { cancelPbiOnFailure } from '../cancel/pbi-cascade.js' import { propagateStatusUpwards } from '../lib/tasks-status-update.js' @@ -223,6 +223,16 @@ export async function maybeCreateAutoPr(opts: { }) if (!product?.auto_pr) return null + const job = await prisma.claudeJob.findUnique({ + where: { id: jobId }, + select: { + sprint_run_id: true, + sprint_run: { + select: { id: true, pr_strategy: true, sprint: { select: { sprint_goal: true } } }, + }, + }, + }) + const task = await prisma.task.findUnique({ where: { id: taskId }, select: { @@ -232,8 +242,41 @@ export async function maybeCreateAutoPr(opts: { }) if (!task) return null - // Branch-per-story: if a sibling job in the same story already opened a PR, - // reuse its URL. This avoids one PR per sub-task. + // PBI-46 SPRINT-mode: hergebruik 1 draft-PR voor de hele SprintRun. + // Mens zet 'm ready-for-review zodra de SprintRun DONE is. + if (job?.sprint_run && job.sprint_run.pr_strategy === 'SPRINT') { + const sprintSibling = await prisma.claudeJob.findFirst({ + where: { + sprint_run_id: job.sprint_run_id, + pr_url: { not: null }, + id: { not: jobId }, + }, + select: { pr_url: true }, + orderBy: { created_at: 'asc' }, + }) + if (sprintSibling?.pr_url) return sprintSibling.pr_url + + // Eerste DONE in deze SprintRun → maak draft-PR aan, geen auto-merge. + const goal = job.sprint_run.sprint.sprint_goal + const sprintTitle = `Sprint: ${goal}`.slice(0, 200) + const body = summary + ? `${summary}\n\n---\n\n*Draft PR voor sprint-run \`${job.sprint_run.id}\`. Wordt ready-for-review zodra alle stories DONE zijn (auto-merge bewust uit voor sprint-mode).*` + : `*Draft PR voor sprint-run \`${job.sprint_run.id}\`. Wordt ready-for-review zodra alle stories DONE zijn (auto-merge bewust uit voor sprint-mode).*` + + const result = await createPullRequest({ + worktreePath, + branchName, + title: sprintTitle, + body, + draft: true, + enableAutoMerge: false, + }) + if ('url' in result) return result.url + console.warn(`[update_job_status] sprint draft-PR skipped for job ${jobId}:`, result.error) + return null + } + + // STORY-mode (default of legacy): branch-per-story, sibling-tasks delen PR. const sibling = await prisma.claudeJob.findFirst({ where: { task: { story_id: task.story.id }, @@ -245,7 +288,6 @@ export async function maybeCreateAutoPr(opts: { }) if (sibling?.pr_url) return sibling.pr_url - // First DONE-task in the story → create a story-scoped PR const storyTitle = task.story.code ? `${task.story.code}: ${task.story.title}` : task.story.title const body = summary ? `${summary}\n\n---\n\n*Auto-generated by Scrum4Me agent (first task in story; PR-body will accumulate as sibling tasks complete).*` @@ -425,13 +467,18 @@ export function registerUpdateJobStatusTool(server: McpServer) { // bij elke task-statusovergang (DONE of FAILED). De helper handelt ook // sibling-cancel binnen dezelfde SprintRun af bij FAILED. // Idea-jobs hebben geen task_id en worden hier overgeslagen. + let sprintRunBecameDone = false if ( (actualStatus === 'done' || actualStatus === 'failed') && job.kind === 'TASK_IMPLEMENTATION' && job.task_id ) { try { - await propagateStatusUpwards(job.task_id, actualStatus === 'done' ? 'DONE' : 'FAILED') + const propagation = await propagateStatusUpwards( + job.task_id, + actualStatus === 'done' ? 'DONE' : 'FAILED', + ) + sprintRunBecameDone = actualStatus === 'done' && propagation.sprintRunChanged } catch (err) { console.warn( `[update_job_status] propagateStatusUpwards error for task ${job.task_id}:`, @@ -440,6 +487,31 @@ export function registerUpdateJobStatusTool(server: McpServer) { } } + // SPRINT-mode: bij sprint-DONE de draft-PR ready-for-review zetten. + // Mens reviewt + mergt zelf — geen auto-merge in deze modus. + if (sprintRunBecameDone && updated.pr_url) { + const sprintRun = await prisma.claudeJob + .findUnique({ + where: { id: job_id }, + select: { + sprint_run: { select: { pr_strategy: true, status: true } }, + }, + }) + .then((j) => j?.sprint_run) + if (sprintRun?.pr_strategy === 'SPRINT' && sprintRun.status === 'DONE') { + try { + const ready = await markPullRequestReady({ prUrl: updated.pr_url }) + if ('error' in ready) { + console.warn( + `[update_job_status] markPullRequestReady failed for ${updated.pr_url}: ${ready.error}`, + ) + } + } catch (err) { + console.warn(`[update_job_status] markPullRequestReady error:`, err) + } + } + } + // M12: bij failed voor IDEA_*-jobs: zet idea.status op // GRILL_FAILED / PLAN_FAILED + log JOB_EVENT. Bij done laten we de // idea-status met rust — die wordt door update_idea_*_md gezet. diff --git a/src/tools/wait-for-job.ts b/src/tools/wait-for-job.ts index 2cb6621..fbc960e 100644 --- a/src/tools/wait-for-job.ts +++ b/src/tools/wait-for-job.ts @@ -111,6 +111,35 @@ export async function resolveBranchForJob( jobId: string, storyId: string, ): Promise<{ branchName: string; reused: boolean }> { + // Sprint-flow (PBI-46): als deze job aan een SprintRun hangt, kies de branch + // op basis van Product.pr_strategy: + // SPRINT → feat/sprint- (één branch voor hele run) + // STORY → feat/story- (één branch per story; sibling-tasks delen 'm) + // Voor legacy task-jobs zonder sprint_run_id valt de logica terug op het + // bestaande feat/story--pad. + const job = await prisma.claudeJob.findUnique({ + where: { id: jobId }, + select: { + sprint_run_id: true, + sprint_run: { select: { id: true, pr_strategy: true } }, + }, + }) + + if (job?.sprint_run && job.sprint_run.pr_strategy === 'SPRINT') { + const branchName = `feat/sprint-${job.sprint_run.id.slice(-8)}` + const sibling = await prisma.claudeJob.findFirst({ + where: { + sprint_run_id: job.sprint_run_id, + branch: branchName, + id: { not: jobId }, + }, + orderBy: { created_at: 'asc' }, + select: { branch: true }, + }) + return { branchName, reused: sibling !== null } + } + + // STORY-mode (default) of legacy: branch per story const sibling = await prisma.claudeJob.findFirst({ where: { task: { story_id: storyId },