Repository navigation
fix(agent-work): checkpoint selects current claim's work via shared ordering rule (#1328) - #1330
Conversation
…rdering rule (#1328) checkpointAgentWork used an unordered findFirst with no issueId filter, so a checkpoint could stamp an older active sibling (e.g. a surviving BLOCKED row) with the freshest heartbeat while the current attempt stayed behind it, and could heartbeat another issue's active row entirely. Selection now goes through findCurrentClaimWork (CURRENT_CLAIM_WORK_ORDER_BY: freshest lastHeartbeatAt, then createdAt, then id), now exported and accepting an optional issueId. The checkpoint request accepts an optional issueId that scopes selection to that issue's active rows when present.
Review follow-up for #1328: an empty/whitespace issueId would otherwise fall through to the unscoped selection branch. Treat it as absent instead, and fix test indentation.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M3.1-Flash (anthropic) — primary route · pr-reviewer-action v3.7.0
Recommendation: approve — the implementation satisfies PR 1328 and the shared current-claim-work contract.
Change-by-change: checkpointAgentWork now delegates selection to findCurrentClaimWork, applies the shared CURRENT_CLAIM_WORK_ORDER_BY, and scopes by issueId when supplied; parsing trims valid IDs and normalizes invalid, empty, or whitespace IDs to absent. The shared helper’s issue filter is conditional, while renewal and terminal disposition continue passing their required issue IDs.
Claims: all three held. Claim 1 held: the helper is exported and issueId is optional. Claim 2 held: repository search found three call sites—checkpoint, renewal heartbeat, and terminal disposition—and all supply an issue ID to the helper. Claim 3 held: parsing and selection preserve or normalize issueId as described. The 404/fail-closed and orphan-lease paths also remain consistent with the route and issue context.
Linked Issue Fit: the implementation directly addresses both failure modes in PR 1328: deterministic selection among active siblings and cross-issue scoping when the request identifies an issue. The added tests assert the shared ordering, scoped selection, unscoped compatibility, and input normalization required by the issue.
Tool Harness Findings: 24 read-only calls verified the changed implementation, route, tests, shared helper, all helper call sites, ownership consumers, Prisma adapter, and repository worker contract. No concrete defect was found. The harness exhausted its investigation budget, so no additional paths were independently explored beyond those checks and repository-wide searches.
Unknowns or Needs Verification: no database-backed test specifically demonstrated two competing rows in one transaction; correctness instead rests on the shared ordering constant and its direct use, with the reported database-integration CI check successful.
joryirving
left a comment
There was a problem hiding this comment.
Blocker-only review on exact head e1ce98e42878cf0410748aa3693acdf0edb9c1c3: APPROVE. checkpointAgentWork now uses the shared deterministic current-claim-work ordering, scopes to issueId when supplied, and preserves backward compatibility for callers that omit it. The parser trims valid IDs and normalizes absent/invalid optional values; regression tests cover ordered selection, issue scoping, omitted-ID behavior, and normalization. The shared helper's existing renewal/disposition callers still pass their required issue IDs. All current workflows are green. The related unordered selection in finishAgentWork is a separate pre-existing follow-up, not a blocker for this focused #1328 fix.
Summary
Fixes #1328.
checkpointAgentWorkselected the activeAgentWorkrow with an unorderedfindFirstand noissueIdfilter, diverging from the shared current-claim-work rule established in #1251/#1300 (CURRENT_CLAIM_WORK_ORDER_BY/findCurrentClaimWorkinsrc/lib/worker-ownership.ts). Two failure modes:BLOCKEDsibling and a currentIN_PROGRESSrow for the same (agent, issue), a checkpoint could land on the older row and give it the freshest heartbeat — after which the capability's freshest-row rule made renewal heartbeat and terminal disposition settle/follow the stale sibling while the real attempt stayed active.issueIdscoping, a checkpoint for issue A could heartbeat the agent's active row on issue B.Changes
src/lib/worker-ownership.ts: exportfindCurrentClaimWorkand makeissueIdoptional (where clause scoped to the issue only when provided). The three existing internal callers (renewal heartbeat, terminal disposition) all pass a requiredissueId, so their behavior is unchanged.src/lib/agent-work.ts:checkpointAgentWorknow selects throughfindCurrentClaimWork(tx, agentName, issueId ?? undefined)instead of an unorderedfindFirst.CheckpointAgentWorkInputaccepts an optionalissueId;parseCheckpointAgentWorkInputpasses it through and normalizes a non-string or empty/whitespace value to absent (so an emptyissueIdcannot silently degrade to unscoped selection).src/app/api/agent-work/checkpoint/route.ts: request-schema doc comment updated for the optionalissueId.src/lib/agent-work.test.ts: regression tests — selection goes through the shared ordering rule (orderBy: CURRENT_CLAIM_WORK_ORDER_BY), selection is scoped to the checkpoint's issue when the request identifies one and unscoped otherwise; parse passthrough / non-string / empty-string cases.Backward compatibility
Old checkpoint clients that omit
issueIdkeep working: with a single active row the selection is identical to before; with multiple active rows the freshest row now wins deterministically. A checkpoint with anissueIdmatching no active row returns 404 (fail-closed) instead of heartbeating a sibling — the pre-existing orphan-lease cleanup path is untouched.Validation (local, revision e1ce98e)
npx vitest run src/lib/agent-work.test.ts src/app/api/agent-work src/lib/worker-ownership.test.ts— 7 files, 185 tests passednpm run lint— 0 errors (1 pre-existing warning insrc/app/login/page.tsx, unrelated)npm run typecheck— cleanA fresh reviewer pass approved the diff; its in-scope nits (empty-string
issueIdnormalization, test indentation) are addressed in the second commit. Out-of-scope observation:finishAgentWorkhas the same unordered-findFirstshape — follow-up issue to be filed separately.Remaining external verification
CI (lint/typecheck/test workflows) and human review are pending on GitHub; not verified locally.