diff --git a/__tests__/git/job-locks.test.ts b/__tests__/git/job-locks.test.ts index 3ed3f03..125620d 100644 --- a/__tests__/git/job-locks.test.ts +++ b/__tests__/git/job-locks.test.ts @@ -118,4 +118,19 @@ describe('job-locks: setupProductWorktrees', () => { // Lock was still acquired and registered — release cleans up await releaseLocksOnTerminal('j3') }) + + it('output preserves input order regardless of alphabetical lock-acquire order', async () => { + // 'z-primary' sorts AFTER 'a-secondary' alphabetically, but caller passes + // primary first → output[0] must be 'z-primary' so wait_for_job's + // primary_worktree_path = worktrees[0]?.worktreePath points at the right repo. + const result = await setupProductWorktrees( + 'j4', + ['z-primary', 'a-secondary'], + async () => originRepo, + ) + expect(result).toHaveLength(2) + expect(result[0].productId).toBe('z-primary') + expect(result[1].productId).toBe('a-secondary') + await releaseLocksOnTerminal('j4') + }) }) diff --git a/src/git/job-locks.ts b/src/git/job-locks.ts index 446f183..a7c1a05 100644 --- a/src/git/job-locks.ts +++ b/src/git/job-locks.ts @@ -23,15 +23,19 @@ export async function setupProductWorktrees( // Ensure parent dir exists so lockfile creation succeeds await fs.mkdir(path.join(getWorktreeRoot(), '_products'), { recursive: true }) - // Lock-first, alphabetically sorted (deadlock prevention for multi-product idea-jobs) + // Lock-first, alphabetically sorted (deadlock prevention for multi-product idea-jobs). + // Locks acquired in sorted order; output preserves caller's input order so that + // worktrees[0] is the primary product (Idea.product_id), regardless of how its + // id sorts alphabetically against secondary products. const sorted = [...productIds].sort() const lockPaths = sorted.map(getProductWorktreeLockPath) const releaseAll = await acquireFileLocksOrdered(lockPaths) registerJobLockReleases(jobId, [releaseAll]) - // After lock-acquire, create/reuse worktrees and sync + // After lock-acquire, create/reuse worktrees and sync — iterate input order + // so callers get back [primary, ...secondaries] in their original sequence. const out: Array<{ productId: string; worktreePath: string }> = [] - for (const productId of sorted) { + for (const productId of productIds) { const repoRoot = await resolveRepoRoot(productId) if (!repoRoot) continue const { worktreePath } = await getOrCreateProductWorktree({ repoRoot, productId }) diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index 3ca0ebb..9f60006 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -590,25 +590,41 @@ 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 + // PBI-49 P2: gebruik niet alleen updated.pr_url — als de laatste task + // verify-only is of geen wijzigingen pusht, krijgt die geen pr_url. + // Zoek de eerst aangemaakte PR op binnen de SprintRun als fallback. + if (sprintRunBecameDone) { + const ctx = await prisma.claudeJob .findUnique({ where: { id: job_id }, select: { + sprint_run_id: true, 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}`, - ) + if ( + ctx?.sprint_run?.pr_strategy === 'SPRINT' + && ctx.sprint_run.status === 'DONE' + && ctx.sprint_run_id + ) { + const sprintPrUrl = updated.pr_url + ?? (await prisma.claudeJob.findFirst({ + where: { sprint_run_id: ctx.sprint_run_id, pr_url: { not: null } }, + orderBy: { created_at: 'asc' }, + select: { pr_url: true }, + }))?.pr_url + ?? null + if (sprintPrUrl) { + try { + const ready = await markPullRequestReady({ prUrl: sprintPrUrl }) + if ('error' in ready) { + console.warn( + `[update_job_status] markPullRequestReady failed for ${sprintPrUrl}: ${ready.error}`, + ) + } + } catch (err) { + console.warn(`[update_job_status] markPullRequestReady error:`, err) } - } catch (err) { - console.warn(`[update_job_status] markPullRequestReady error:`, err) } } } diff --git a/src/tools/wait-for-job.ts b/src/tools/wait-for-job.ts index 0958498..5741ec5 100644 --- a/src/tools/wait-for-job.ts +++ b/src/tools/wait-for-job.ts @@ -428,9 +428,27 @@ async function getFullJobContext(jobId: string) { involvedProductIds.push(ip.product_id) } } - const worktrees = involvedProductIds.length > 0 - ? await setupProductWorktrees(job.id, involvedProductIds, (pid) => resolveRepoRoot(pid)) - : [] + // PBI-49 P1: rollback the claim if worktree setup fails so the job + // doesn't hang in CLAIMED until the 30-min stale-reset, and any partial + // locks are released. Mirrors attachWorktreeToJob's task-pad behaviour. + let worktrees: Array<{ productId: string; worktreePath: string }> = [] + if (involvedProductIds.length > 0) { + try { + worktrees = await setupProductWorktrees( + job.id, + involvedProductIds, + (pid) => resolveRepoRoot(pid), + ) + } catch (err) { + console.warn( + `[wait-for-job] product-worktree setup failed for idea-job ${job.id}; rolling back claim:`, + err, + ) + await releaseLocksOnTerminal(job.id) + await rollbackClaim(job.id) + return null + } + } return { job_id: job.id,