Skip to content

Commit 792dc80

Browse files
committed
fix: recover claims and bind review comments
1 parent 1ef5ef6 commit 792dc80

18 files changed

Lines changed: 264 additions & 85 deletions

loops/issue-dev-loop/LOOP.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ Use a combo trigger. Run `triggers/detect-work.mjs` before starting a Codex impl
6565

6666
### 1. Claim and snapshot
6767

68-
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.
68+
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. If labeling fails, release the reservation only after verifying it still points to the exact base SHA. If a process crash nevertheless leaves a branch without a resumable checkpoint or open PR, the next trigger returns `claim_recovery`, wakes the loop, and immediately escalates the issue and branch to the owner instead of silently excluding it; never delete or reuse that reservation without explicit owner confirmation. 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.
6969

7070
### 2. Isolate
7171

loops/issue-dev-loop/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Run exactly one bounded issue cycle. Treat [`LOOP.md`](./LOOP.md) as the constit
1111
3. Read [`references/github-operations.md`](./references/github-operations.md). Run every operational `loopctl`, executor GitHub command, remote Git command, trigger, and reviewer publication through the installed control plane with the explicit target root. Never use the credential-refusing repository launcher, invoke the `.mjs` router directly, install control code from an issue branch, or alter global `gh` or Git credential configuration.
1212
4. Run `loopctl.mjs reconcile` through the automation wrapper to rebuild verified terminal history, pending/completed evolve state, and active runs from the append-only GitHub state journal. It tombstones local terminal-cache rows with no durable counterpart before recomputing metrics. For returned `workType: resume`, fetch the recorded branch through the wrapper, create a clean isolated worktree at the returned exact head, and run `restore-checkpoint --run-id <id>` inside that worktree. The restore command rejects the wrong branch, a dirty checkout, or any head other than the durable head. Resume it before selecting a new issue.
1313
5. Run `loopctl.mjs evolve-status`. If `evolveDue` is true, start `echo_ui_loop_evolver` with fresh context; do not silently replace it with product work.
14-
6. Run the installed `triggers/detect-work.mjs` through the automation wrapper with `--loop-root "$ECHO_UI_LOOP_TARGET_ROOT"`. The detector paginates every open issue and PR page. Exit without invoking an implementation agent when it reports `hasWork: false`.
14+
6. Run the installed `triggers/detect-work.mjs` through the automation wrapper with `--loop-root "$ECHO_UI_LOOP_TARGET_ROOT"`. The detector paginates every open issue and PR page. Exit without invoking an implementation agent when it reports `hasWork: false`. If it returns `workType: claim_recovery`, a remote claim branch exists without a resumable checkpoint or open PR: immediately notify the owner through the scheduled task and canonical owner channel, include the issue and branch, do not invoke `$implement`, and do not delete or reuse the branch without explicit owner confirmation.
1515
7. Refuse to start when another active run or PR already claims the issue.
1616
8. Create a `codex/issue-<number>` branch and an isolated worktree from `dev`.
1717
9. Start the run with `loopctl.mjs start --issue <number> --title <title> --url <issue-url> --base-sha <full-origin-dev-sha>`.

loops/issue-dev-loop/references/github-operations.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ The shell launcher removes Node preload hooks before starting the router. The ro
2323
## Selection and claim
2424

2525
1. Select only open issues labeled `codex-ready`.
26-
2. Exclude `loop:claimed`, an existing `codex/issue-<number>` branch, and any open PR that references or closes the issue.
27-
3. Let `loopctl start --base-sha <full-origin-dev-sha>` acquire the local claim, recheck every page of open PRs, apply `loop:claimed`, and capture the authoritative issue; do not add the label manually first.
26+
2. Exclude `loop:claimed` and any open PR that references or closes the issue. An existing `codex/issue-<number>` branch without a resumable checkpoint or open PR is `workType: claim_recovery`, not a quiet no-op: notify the owner and require confirmation before cleanup or reuse.
27+
3. Let `loopctl start --base-sha <full-origin-dev-sha>` acquire the local claim, recheck every page of open PRs, atomically create `codex/issue-<number>` at that base SHA, apply `loop:claimed`, and capture the authoritative issue; do not add the label or remote branch manually first. If labeling fails, the trusted start command verifies that the ref is still at the reservation SHA and releases it. A crash that leaves only the remote ref is detected on the next wake and escalated instead of being silently skipped.
2828
4. Record the issue snapshot and claim timestamp before implementation. A second active local run for the issue is rejected.
2929
5. Immediately publish and validate an active checkpoint after the claim. Repeat after each durable phase; the next phase re-fetches the exact state-journal comment and refuses to advance on local-only proof. On a fresh wake, `reconcile` returns `workType: resume`, branch, and expected head before considering a new issue. Fetch that branch, create a clean isolated worktree at the exact head, then run `restore-checkpoint`; restoration blocks on the wrong branch, head, or dirty state.
3030

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,22 @@ export async function defaultGitHubApi(endpoint) {
208208
return JSON.parse(result.stdout)
209209
}
210210

211+
export async function postGitHubIssueComment(target, body) {
212+
const result = await execFileAsync(
213+
'gh',
214+
[
215+
'api',
216+
`repos/${target.owner}/${target.repo}/issues/${target.number}/comments`,
217+
'--method',
218+
'POST',
219+
'-f',
220+
`body=${body}`,
221+
],
222+
{ maxBuffer: 1024 * 1024 },
223+
)
224+
return JSON.parse(result.stdout)
225+
}
226+
211227
export async function paginateGitHubApi(
212228
githubApi,
213229
endpoint,

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ export async function recordReview({
536536
const digestMarker = `<!-- issue-dev-loop:${normalizedRunId}:review-result-sha256:${publicationDigest} -->`
537537
const publications = new Map()
538538
const priorFindingIds = new Set()
539+
const publishedInlineCommentIds = new Set()
539540
let previousSubmittedAt = -Infinity
540541
for (const round of reviewSummary.roundDetails) {
541542
const roundTarget = parseReviewUrl(round.reviewUrl)
@@ -568,7 +569,10 @@ export async function recordReview({
568569
) {
569570
throw new Error(`published GitHub review round ${round.round} is not bound to its exact head`)
570571
}
571-
const expectedFindingIds = new Set(round.findings.map((finding) => finding.findingId))
572+
const expectedFindings = new Map(
573+
round.findings.map((finding) => [finding.findingId, finding]),
574+
)
575+
const expectedFindingIds = new Set(expectedFindings.keys())
572576
const publishedFindingIds = new Set(
573577
bodies.flatMap((body) => body.match(/\bRVW-[0-9]+-[0-9]+-[0-9]+\b/g) ?? []),
574578
)
@@ -582,17 +586,38 @@ export async function recordReview({
582586
) {
583587
throw new Error(`published GitHub review round ${round.round} has unrecorded findings`)
584588
}
589+
const inlineFindingIds = new Set()
585590
for (const comment of roundComments) {
591+
const commentId = String(comment.id ?? '')
586592
const commentFindingIds = new Set(comment.body?.match(/\bRVW-[0-9]+-[0-9]+-[0-9]+\b/g) ?? [])
593+
const [commentFindingId] = commentFindingIds
594+
const finding = expectedFindings.get(commentFindingId)
587595
if (
596+
!/^[1-9][0-9]*$/.test(commentId) ||
597+
publishedInlineCommentIds.has(commentId) ||
588598
!sameGitHubLogin(comment.user?.login, reviewerLogin) ||
589-
commentFindingIds.size === 0 ||
590-
[...commentFindingIds].some((findingId) => !expectedFindingIds.has(findingId))
599+
commentFindingIds.size !== 1 ||
600+
!finding ||
601+
inlineFindingIds.has(commentFindingId) ||
602+
finding.path !== comment.path ||
603+
!Number.isInteger(finding.line) ||
604+
![comment.line, comment.original_line].includes(finding.line) ||
605+
![
606+
`<!-- issue-dev-loop:${normalizedRunId}:${commentFindingId} -->`,
607+
commentFindingId,
608+
finding.severity,
609+
finding.confidence,
610+
finding.problem,
611+
finding.evidence,
612+
finding.expectedResolution,
613+
].every((fragment) => comment.body?.includes(fragment))
591614
) {
592615
throw new Error(
593616
`published GitHub review round ${round.round} has an unrecorded reviewer inline comment`,
594617
)
595618
}
619+
publishedInlineCommentIds.add(commentId)
620+
inlineFindingIds.add(commentFindingId)
596621
}
597622
for (const finding of round.findings) {
598623
const findingMarker = `<!-- issue-dev-loop:${normalizedRunId}:${finding.findingId} -->`

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

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import {
99
assertNonEmpty,
1010
defaultGitHubApi,
1111
defaultGitHubPaginatedApi,
12-
execFileAsync,
1312
parsePullCommentUrl,
13+
postGitHubIssueComment,
1414
readJson,
1515
sameGitHubLogin,
1616
sameRepository,
@@ -116,22 +116,6 @@ function isStateJournalComment(url, channel) {
116116
)
117117
}
118118

119-
async function defaultGitHubComment(target, body) {
120-
const result = await execFileAsync(
121-
'gh',
122-
[
123-
'api',
124-
`repos/${target.owner}/${target.repo}/issues/${target.number}/comments`,
125-
'--method',
126-
'POST',
127-
'-f',
128-
`body=${body}`,
129-
],
130-
{ maxBuffer: 1024 * 1024 },
131-
)
132-
return JSON.parse(result.stdout)
133-
}
134-
135119
async function verifyPendingRequestComment({ request, comment, channel }) {
136120
const digest = pendingRequestDigest(request)
137121
const marker = `<!-- issue-dev-loop:evolve-request:${request.requestId}:sha256:${digest} -->`
@@ -312,7 +296,7 @@ export async function completeEvolve({
312296
now = new Date(),
313297
githubApi = defaultGitHubApi,
314298
githubPaginatedApi = defaultGitHubPaginatedApi,
315-
githubComment = defaultGitHubComment,
299+
githubComment = postGitHubIssueComment,
316300
verifyAutomationIdentity = assertAutomationIdentity,
317301
} = {}) {
318302
const normalizedRequestId = assertNonEmpty(requestId, 'requestId')
@@ -394,7 +378,7 @@ export async function completeEvolve({
394378
verified ??= candidate
395379
}
396380
if (!verified) {
397-
if (githubComment === defaultGitHubComment) await verifyAutomationIdentity({ loopRoot })
381+
if (githubComment === postGitHubIssueComment) await verifyAutomationIdentity({ loopRoot })
398382
const comment = await githubComment(journal, body)
399383
const commentUrl = assertHttpUrl(comment?.html_url, 'evolve.completionPublicationUrl')
400384
verified = await verifyPublishedEvolveCompletion({

loops/issue-dev-loop/scripts/lib/finalization-journal.mjs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,9 @@ import {
3434
import { appendValidatedEvent, readEvents, readRun } from './run-store.mjs'
3535
import { createNotification } from './notifications.mjs'
3636
import { observeOwnerApprovedMerge } from './owner-gate.mjs'
37+
import { TERMINAL_STATUSES } from './lifecycle-status.mjs'
3738
import { validateFinalizationHistory } from './validation.mjs'
3839

39-
const TERMINAL_STATUSES = new Set(['completed', 'failed', 'blocked', 'cancelled'])
40-
4140
export const canonicalRecord = canonicalFinalizationRecord
4241
export const recordDigest = finalizationRecordDigest
4342

loops/issue-dev-loop/scripts/lib/finalization-proof.mjs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,9 @@ import {
1414
parseCheckpointRecord,
1515
verifyPublishedCheckpoint,
1616
} from './checkpoint-proof.mjs'
17+
import { TERMINAL_STATUSES } from './lifecycle-status.mjs'
1718
import { observeOwnerApprovedMerge } from './owner-gate.mjs'
1819

19-
const TERMINAL_STATUSES = new Set(['completed', 'failed', 'blocked', 'cancelled'])
20-
2120
export function canonicalFinalizationRecord(record) {
2221
return JSON.stringify({
2322
schemaVersion: 1,

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,16 @@ function automationApiMutationAllowed(
833833
`sha=${authorization.issue.baseSha}`,
834834
])
835835
}
836+
if (
837+
endpoint ===
838+
`repos/${authorization?.expectedRepository}/git/refs/heads/${authorization?.issue?.branch}` &&
839+
method === 'DELETE' &&
840+
fields.length === 0 &&
841+
authorization?.rootIntent === 'start' &&
842+
authorization?.issue?.status === 'starting'
843+
) {
844+
return true
845+
}
836846
const labels = endpoint.match(/^repos\/[^/]+\/[^/]+\/issues\/(\d+)\/labels(?:\/([^/]+))?$/)
837847
if (labels && Number(labels[1]) === authorization?.issue?.issueNumber) {
838848
if (method === 'POST') {

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,25 @@ function issuePriority(issue) {
4747

4848
export function selectIssue({ issues = [], pullRequests = [], branchNames = [] } = {}) {
4949
const branches = new Set(branchNames)
50+
const orphanedClaims = issues.filter(
51+
(issue) =>
52+
labelNames(issue).has('codex-ready') &&
53+
branches.has(`codex/issue-${issue.number}`) &&
54+
!pullRequests.some((pullRequest) => pullRequestClaimsIssue(pullRequest, issue.number)),
55+
)
56+
orphanedClaims.sort((left, right) => {
57+
const priorityDifference = issuePriority(left) - issuePriority(right)
58+
if (priorityDifference !== 0) return priorityDifference
59+
return left.number - right.number
60+
})
61+
if (orphanedClaims[0]) {
62+
return {
63+
hasWork: true,
64+
workType: 'claim_recovery',
65+
issue: orphanedClaims[0],
66+
branch: `codex/issue-${orphanedClaims[0].number}`,
67+
}
68+
}
5069
const eligible = issues.filter((issue) => {
5170
const labels = labelNames(issue)
5271
return (
@@ -136,6 +155,7 @@ export async function detectWork({
136155
runId: result.runId ?? null,
137156
issueNumber: result.issue?.number ?? null,
138157
requestId: result.requestId ?? null,
158+
branch: result.branch ?? null,
139159
})
140160
return result
141161
}

0 commit comments

Comments
 (0)