fix: serialize issue claims and paginate reviews

Author: codeacme17

loops/issue-dev-loop/LOOP.md

@@ -65,7 +65,7 @@ Use a combo trigger. Run `triggers/detect-work.mjs` before starting a Codex impl
 
 ### 1. Claim and snapshot
 
-Recheck the issue immediately before mutation. `loopctl start` atomically reserves the issue locally, rechecks GitHub, applies `loop:claimed`, rejects another active run or open issue branch PR, and then creates the run. Capture the issue title/body/labels/URL, base SHA, and acceptance criteria. Use one run ID across logs, handoffs, `screen-shots`, evidence, review comments, notifications, branch metadata, and the PR body.
+Recheck the issue immediately before mutation. `loopctl start` atomically creates the remote `codex/issue-N` branch at the verified base SHA as the cross-worktree reservation, then applies `loop:claimed` and creates the local run. GitHub permits only one creator for a new ref, so a concurrent loser exits before any checkpoint is published. Also reject another active run or open issue branch PR. Capture the issue title/body/labels/URL, base SHA, and acceptance criteria. Use one run ID across logs, handoffs, `screen-shots`, evidence, review comments, notifications, branch metadata, and the PR body.
 
 ### 2. Isolate
 

loops/issue-dev-loop/scripts/lib/evidence.mjs

@@ -549,7 +549,7 @@ export async function recordReview({
     const roundEndpoint = `repos/${roundTarget.owner}/${roundTarget.repo}/pulls/${roundTarget.number}/reviews/${roundTarget.reviewId}`
     const [publishedRound, roundComments] = await Promise.all([
       githubApi(roundEndpoint),
-      githubApi(`${roundEndpoint}/comments?per_page=100`),
+      paginateGitHubApi(githubApi, `${roundEndpoint}/comments`),
     ])
     const bodies = [
       publishedRound.body ?? '',

loops/issue-dev-loop/scripts/lib/github-identity.mjs

@@ -822,6 +822,17 @@ function automationApiMutationAllowed(
   authorization,
 ) {
   if (!endpoint || usesInput || usesFileExpansion) return false
+  if (
+    endpoint.match(/^repos\/[^/]+\/[^/]+\/git\/refs$/) &&
+    method === 'POST' &&
+    authorization?.rootIntent === 'start' &&
+    authorization?.issue?.status === 'starting'
+  ) {
+    return sameArguments(fields, [
+      `ref=refs/heads/${authorization.issue.branch}`,
+      `sha=${authorization.issue.baseSha}`,
+    ])
+  }
   const labels = endpoint.match(/^repos\/[^/]+\/[^/]+\/issues\/(\d+)\/labels(?:\/([^/]+))?$/)
   if (labels && Number(labels[1]) === authorization?.issue?.issueNumber) {
     if (method === 'POST') {
@@ -1333,10 +1344,12 @@ function withRootCommandIntent(authorization, { tool, args, trustedLoopRoot }) {
   }
   const issueNumber = Number(argumentAfter(args, '--issue'))
   const issueUrl = argumentAfter(args, '--url')
+  const baseSha = argumentAfter(args, '--base-sha')
   const target = parseGitHubTarget(issueUrl)
   if (
     !Number.isInteger(issueNumber) ||
     issueNumber < 1 ||
+    !/^[0-9a-f]{40}$/i.test(baseSha ?? '') ||
     target?.kind !== 'issues' ||
     target.number !== issueNumber ||
     !repositoryInScope(`${target.owner}/${target.repo}`, authorization.expectedRepository)
@@ -1351,6 +1364,7 @@ function withRootCommandIntent(authorization, { tool, args, trustedLoopRoot }) {
       prNumber: null,
       runId: null,
       status: 'starting',
+      baseSha: baseSha.toLowerCase(),
       headSha: null,
       implementationCommit: null,
     },

loops/issue-dev-loop/scripts/lib/issue-claim.mjs

@@ -24,6 +24,23 @@ async function defaultAddLabel({ target, issueNumber }) {
   )
 }
 
+async function defaultReserveRemoteBranch({ target, issueNumber, baseSha }) {
+  await execFileAsync(
+    'gh',
+    [
+      'api',
+      `repos/${target.owner}/${target.repo}/git/refs`,
+      '--method',
+      'POST',
+      '-f',
+      `ref=refs/heads/codex/issue-${issueNumber}`,
+      '-f',
+      `sha=${baseSha}`,
+    ],
+    { maxBuffer: 1024 * 1024 },
+  )
+}
+
 async function defaultRemoteBranchExists({ target, issueNumber }) {
   try {
     await execFileAsync(
@@ -55,15 +72,20 @@ export async function defaultClaimIssue({
   loopRoot = DEFAULT_LOOP_ROOT,
   issueUrl,
   issueNumber,
+  baseSha,
   githubApi = defaultGitHubApi,
   githubPaginatedApi = defaultGitHubPaginatedApi,
   addLabel = defaultAddLabel,
+  reserveRemoteBranch = defaultReserveRemoteBranch,
   remoteBranchExists = defaultRemoteBranchExists,
 }) {
   const target = parseGitHubTarget(issueUrl)
   if (!target || target.kind !== 'issues' || target.number !== issueNumber) {
     throw new Error('issueUrl must identify the issue being claimed')
   }
+  if (!/^[0-9a-f]{40}$/i.test(baseSha ?? '')) {
+    throw new Error('issue claim requires a full base SHA')
+  }
   const issue = await githubApi(`repos/${target.owner}/${target.repo}/issues/${issueNumber}`)
   const labels = labelNames(issue)
   if (issue.state !== 'open' || !labels.has('codex-ready') || labels.has('loop:claimed')) {
@@ -78,9 +100,13 @@ export async function defaultClaimIssue({
   if (pulls.some((pullRequest) => pullRequestClaimsIssue(pullRequest, issueNumber))) {
     throw new Error(`an open pull request already claims issue ${issueNumber}`)
   }
-  if (addLabel === defaultAddLabel) {
+  if (
+    addLabel === defaultAddLabel ||
+    reserveRemoteBranch === defaultReserveRemoteBranch
+  ) {
     await assertAutomationIdentity({ loopRoot, githubApi })
   }
+  await reserveRemoteBranch({ target, issueNumber, baseSha })
   await addLabel({ target, issueNumber })
   return issue
 }

loops/issue-dev-loop/scripts/lib/run-store.mjs

@@ -166,6 +166,7 @@ export async function startRun({
       issueUrl: url,
       issueNumber: issue,
       branch: `codex/issue-${issue}`,
+      baseSha: normalizedBaseSha,
       githubApi,
     })
     remoteClaimCreated = true

loops/issue-dev-loop/tests/github-identity-routing.test.mjs

@@ -328,8 +328,23 @@ if (commandArguments[0] === 'spawn') {
 } else if (commandArguments[0] === 'start') {
   const issueIndex = commandArguments.indexOf('--issue')
   const issue = commandArguments[issueIndex + 1]
+  const baseShaIndex = commandArguments.indexOf('--base-sha')
+  const baseSha = commandArguments[baseShaIndex + 1]
+  const reserveIssueIndex = commandArguments.indexOf('--reserve-issue')
+  const reserveIssue =
+    reserveIssueIndex === -1 ? issue : commandArguments[reserveIssueIndex + 1]
   for (const command of [
     ['api', 'user'],
+    [
+      'api',
+      'repos/example/repo/git/refs',
+      '--method',
+      'POST',
+      '-f',
+      \`ref=refs/heads/codex/issue-\${reserveIssue}\`,
+      '-f',
+      \`sha=\${baseSha}\`,
+    ],
     ['api', \`repos/example/repo/issues/\${issue}/labels\`, '--method', 'POST', '-f', 'labels[]=loop:claimed']
   ]) {
     const result = spawnSync('gh', command, { env: process.env, stdio: 'inherit' })
@@ -932,15 +947,49 @@ test('routed loopctl start may claim only its command-line issue', async () => {
       '123',
       '--url',
       'https://github.com/example/repo/issues/123',
+      '--base-sha',
+      '0'.repeat(40),
       '--loop-root',
       fixture.loopRoot,
     ],
     { env: fixture.env },
   )
   assert.match(stdout, /executor-user/)
+  assert.match(stdout, /git\/refs/)
   assert.match(stdout, /issues\/123\/labels/)
 })
 
+test('routed loopctl start rejects a remote reservation for another issue', async () => {
+  const fixture = await createFixture({ activeRun: false })
+  await assert.rejects(
+    execFileAsync(
+      process.execPath,
+      [
+        routerPath,
+        '--loop-root',
+        fixture.loopRoot,
+        'automation',
+        '--',
+        process.execPath,
+        fixture.loopctlPath,
+        'start',
+        '--issue',
+        '123',
+        '--url',
+        'https://github.com/example/repo/issues/123',
+        '--base-sha',
+        '0'.repeat(40),
+        '--reserve-issue',
+        '124',
+        '--loop-root',
+        fixture.loopRoot,
+      ],
+      { env: fixture.env },
+    ),
+    /GitHub action is prohibited for the automation role/,
+  )
+})
+
 test('reviewer identity cannot push code', async () => {
   const fixture = await createFixture()
   await assert.rejects(

loops/issue-dev-loop/tests/runtime.test.mjs

@@ -1154,6 +1154,7 @@ test('authoritative claim rejects any paginated open PR that references the issu
     defaultClaimIssue({
       issueUrl: 'https://github.com/codeacme17/echo-ui/issues/128',
       issueNumber: 128,
+      baseSha: '0'.repeat(40),
       githubApi: async () => ({
         number: 128,
         title: 'Issue',
@@ -1176,6 +1177,7 @@ test('authoritative claim rejects any paginated open PR that references the issu
     defaultClaimIssue({
       issueUrl: 'https://github.com/codeacme17/echo-ui/issues/128',
       issueNumber: 128,
+      baseSha: '0'.repeat(40),
       githubApi: async () => ({
         number: 128,
         title: 'Issue',
@@ -1193,6 +1195,41 @@ test('authoritative claim rejects any paginated open PR that references the issu
   assert.equal(labelAdded, false)
 })
 
+test('authoritative claim atomically reserves one remote issue branch across starters', async () => {
+  let reservationCreated = false
+  let labelCount = 0
+  const claim = () =>
+    defaultClaimIssue({
+      issueUrl: 'https://github.com/codeacme17/echo-ui/issues/128',
+      issueNumber: 128,
+      baseSha: '0'.repeat(40),
+      githubApi: async () => ({
+        number: 128,
+        title: 'Issue',
+        state: 'open',
+        labels: [{ name: 'codex-ready' }],
+      }),
+      githubPaginatedApi: async () => [],
+      remoteBranchExists: async () => false,
+      reserveRemoteBranch: async () => {
+        if (reservationCreated) throw new Error('remote ref already exists')
+        reservationCreated = true
+      },
+      addLabel: async () => {
+        labelCount += 1
+      },
+    })
+
+  const outcomes = await Promise.allSettled([claim(), claim()])
+  assert.equal(outcomes.filter(({ status }) => status === 'fulfilled').length, 1)
+  assert.equal(outcomes.filter(({ status }) => status === 'rejected').length, 1)
+  assert.match(
+    outcomes.find(({ status }) => status === 'rejected').reason.message,
+    /remote ref already exists/,
+  )
+  assert.equal(labelCount, 1)
+})
+
 test('startRun creates one correlated run, handoff, and evidence directories', async () => {
   const { loopRoot } = await createFixture()
   let workspaceValidation
@@ -2757,7 +2794,7 @@ test('review gate verifies published findings and classified replies', async ()
           }),
         ]
       }
-      if (endpoint.endsWith('/reviews/499/comments?per_page=100')) {
+      if (endpoint.endsWith('/reviews/499/comments?per_page=100&page=1')) {
         return [
           {
             user: { login: 'echo-ui-reviewer[bot]' },
@@ -2775,7 +2812,7 @@ test('review gate verifies published findings and classified replies', async ()
           },
         ]
       }
-      if (endpoint.endsWith('/comments?per_page=100')) return []
+      if (endpoint.endsWith('/comments?per_page=100&page=1')) return []
       if (endpoint.includes('/issues/comments/400')) {
         return {
           user: { login: 'echo-ui-loop[bot]' },
@@ -2880,6 +2917,7 @@ test('review gate rejects GitHub findings omitted from the durable result', asyn
     }),
     /include every reviewer publication/,
   )
+  let secondCommentPageFetched = false
   await assert.rejects(
     recordReview({
       loopRoot,
@@ -2897,13 +2935,22 @@ test('review gate rejects GitHub findings omitted from the durable result', asyn
             }),
           ]
         }
-        if (endpoint.endsWith('/comments?per_page=100')) {
+        if (endpoint.endsWith('/comments?per_page=100&page=1')) {
+          return Array.from({ length: 100 }, (_, index) => ({
+            user: { login: 'echo-ui-reviewer[bot]' },
+            path: 'src/context.ts',
+            line: index + 1,
+            body: `Reviewer context ${index + 1}`,
+          }))
+        }
+        if (endpoint.endsWith('/comments?per_page=100&page=2')) {
+          secondCommentPageFetched = true
           return [
             {
               user: { login: 'echo-ui-reviewer[bot]' },
               path: 'src/untracked.ts',
-              line: 3,
-              body: 'This inline finding was omitted and has no stable ID.',
+              line: 101,
+              body: 'RVW-1-1-1: omitted finding beyond the first page.',
             },
           ]
         }
@@ -2921,8 +2968,9 @@ test('review gate rejects GitHub findings omitted from the durable result', asyn
         }
       },
     }),
-    /unrecorded reviewer inline comment/,
+    /unrecorded findings/,
   )
+  assert.equal(secondCommentPageFetched, true)
 })
 
 test('accepted review fix must be after the finding head and inside the final head', async () => {
@@ -3039,7 +3087,7 @@ test('accepted review fix must be after the finding head and inside the final he
           }),
         ]
       }
-      if (endpoint.endsWith('/comments?per_page=100')) return []
+      if (endpoint.endsWith('/comments?per_page=100&page=1')) return []
       if (endpoint.includes('/issues/comments/410')) {
         return {
           user: { login: 'echo-ui-loop[bot]' },
@@ -3168,7 +3216,7 @@ test('review gate binds high-severity adjudication verdict to the correct identi
         },
       ]
     }
-    if (endpoint.endsWith('/comments?per_page=100')) return []
+    if (endpoint.endsWith('/comments?per_page=100&page=1')) return []
     if (endpoint.includes('/issues/comments/401')) {
       return {
         user: { login: 'echo-ui-loop[bot]' },