PBI-49: review-fixes — primary_worktree order, idea-claim rollback, sprint mark-ready fallback

Three findings from PBI-47 review:

P1 — primary_worktree_path scheiden van lock-volgorde
  setupProductWorktrees acquired locks in alphabetical order (deadlock prevention)
  but also returned worktrees in that order, so worktrees[0] could point at a
  secondary product when its id sorted before the primary's. Lock-acquire stays
  sorted; output now preserves caller's input order so worktrees[0] is always
  the primary.

P1 — Idea-claim rollback bij worktree setup failure
  setupProductWorktrees runs after tryClaimJob has already flipped the job to
  CLAIMED. A failure in lock-acquire/git-fetch/reset/sync left the job hanging
  until the 30-min stale-reset and the lock-map populated. Wrapped in try/catch
  with releaseLocksOnTerminal + rollbackClaim mirror of the task-pad behaviour.

P2 — SPRINT mark-ready fallback when last task didn't push
  The mark-ready path used updated.pr_url, which is null when the closing task
  was verify-only or had no diff. Now falls back to a Prisma findFirst on the
  SprintRun's earliest job with pr_url IS NOT NULL.

Tests: 31 files, 243 passing (incl. new input-order regression for setupProductWorktrees).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

__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')
+  })
 })

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 })

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)
             }
           }
         }

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,