[itsmiso-ai]

What

renewOwnership committed the fenced lease compare-and-set (TTL extension) and the matching active AgentWork liveness write (lastHeartbeatAt + leaseExpiresAt + history row) as two separate writes. A crash between them left the lease renewed while the AgentWork row kept its old, lapsed 5-minute leaseExpiresAt; sweepStaleWork reclaims candidates on leaseExpiresAt too, so it marked the renewed worker STALE and deleted its lease — the renewal was lost. Since PR #1300 that write is load-bearing for the worker's survival, not best-effort evidence.

Fix: wrap the CAS, the post-CAS lease re-read, and the AgentWork heartbeat/history write in ONE prisma.$transaction, all on the transaction's client (heartbeatMatchingAgentWork now takes the Prisma.TransactionClient). A failure rolls the whole renewal back; the route 500s — a rolled-back renewal never reports success.

• src/lib/worker-ownership.ts — renewOwnership + heartbeatMatchingAgentWork, module header rule 5 rewritten.
• src/app/api/agent-work/ownership/route.test.ts — new renew-atomicity suite (single-transaction-client proof, rollback at the liveness write and at the history write, lost-race CAS commits nothing), plus a new agentWorkHistory.create failure-injection point in the existing harness.
• docs/worker-ownership-capability.md — atomicity section updated.

Fixes #1315

Invariants this change must keep, and every other path enforcing each rule

1. The lease CAS stays fenced on claimToken + disposition: null. Kept verbatim in renewOwnership; it is also the first statement of the transaction, so the count === 0 early return commits an empty transaction (same posture dispositionOwnership documents). Other writers of this rule, all untouched: dispositionOwnership's terminal CAS (same fence), upsertLease claim-token rotation/reset in src/lib/lease.ts, and the stale sweep's claim-epoch-fenced deleteMany on {claimToken, expiredAt, lastRenewalObservedAt} in src/lib/stale-work.ts (The stale-work sweep deletes the lease for the whole (agent, issue) pair when any sibling row is stale #1317) — which still backs off if a renewal commits inside its decision window, now doubly so because the row expiry moves with the lease.
2. Lease and matching AgentWork row move on the operation's own clock and expiry — no split state visible to the sweeper. Enforced by heartbeatMatchingAgentWork riding the transaction client; the values written are the same now/expiresAt the lease received (tests assert equality with db.lease.expiredAt/renewedAt). Other writers of both timestamps: startAgentWork/checkpointAgentWork in src/lib/agent-work.ts maintain their own 5-minute execution lease independently of the ownership capability — untouched and out of scope. The sweeper's candidate predicate (lastHeartbeatAt OR leaseExpiresAt) and its live-sibling complement predicate in src/lib/stale-work.ts are unchanged; the feat(ownership): persist optional worker renewal and fenced claim evidence #1251 suite exercises them through the sweep's real clause, and those tests pass unmodified.
3. A rolled-back write never reports success. The success value is constructed inside the transaction callback; any throw propagates through the guard (which deletes itself on throw) to the route's catch → 500. Same pattern as the already-transactional register/disposition; unchanged.
4. renew's replay semantics unchanged. A renewal is idempotent by nature — a retry re-extends the TTL. No replay branch was added. agentWorkHeartbeated: false still means "no matching active row existed" (absence), never a lost write; a lost write is now impossible (rollback + 500).
5. Serialization unchanged. The per-issue IssueMutationGuard anchor (feat(recovery): add fenced capacity-exhaustion handoffs #1259) still surrounds the whole operation; the transaction narrows the window the sweeper's epoch fence is the belt for.

Validation (run on head 771bf14; code identical at 0b140a0 which received the independent review)

• vitest run src/app/api/agent-work/ownership/route.test.ts src/lib/stale-work.test.ts → 79 passed (incl. 4 new renew-atomicity tests).
• Full suite vitest run → 202 files / 5089 tests passed (24 skipped, 1 todo).
• npm run typecheck → clean. npm run lint → 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched).
• One independent review pass over the pinned diff: no blockers/majors; one minor doc-wording finding, fixed in 771bf14 (docs-only delta; code checks above still apply).
• CI is pending — not claimed green. Remaining external verification: CI on this PR; the crash-window itself is exercised only via the mock journal's rollback (no real DB kill test).

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3.1-Flash (anthropic) — primary route · pr-reviewer-action v3.7.0

Recommendation: Approve. The renewal now commits the fenced lease CAS, matching AgentWork heartbeat, renewed expiry, and history row through one transaction client, satisfying PR 1315 without changing the renewal contract.

Claims and Findings

• Held: 5/5 stated invariants are supported by the implementation. The CAS remains fenced by claimToken and disposition: null and is the transaction’s first write; lease and AgentWork use the same clock and expiry; rollback propagates to the route’s 500 response; retries remain ordinary TTL-extending renewals; and IssueMutationGuard still surrounds the operation.
• The new failure-injection tests cover update failure, history failure, transaction-client usage, retry convergence, and lost-race behavior.

Linked Issue Fit

• Satisfies the issue’s atomicity requirement, honest failure semantics, unchanged fencing contract, and unchanged replay semantics.
• No linked acceptance criterion is missing or misinterpreted.

Tool Harness Findings

• All 24 executed reads/searches succeeded and confirmed the production wiring uses the transaction client for all three renewal writes; the transactional test harness models rollback distinctly from root-client writes.
• The harness exhausted its budget before a broader sibling-path scan, but the supplied CI results report successful tests, database integration, lint, typecheck, and build.

Unknowns or Needs Verification

• No blocker-level unknown remains; the exhaustive tool investigation was truncated by the fixed budget, but the changed production path, its route consumer, transaction harness, mutation guard, stale-sweep integration, and relevant repository history were directly inspected.

[joryirving]

Secondary hard-blocker review on exact head 771bf14a1ef0fb7e69aa274f559caf76120cdd2d: APPROVE. The renewal CAS, lease re-read, matching AgentWork liveness write, and history record now execute through one Prisma transaction client, preserving the existing claim-token/disposition fence and honest rollback semantics. The lost-race path performs no prior write, retry still safely re-extends TTL, and failure-injection tests cover AgentWork/history rollback plus transaction-client usage. The primary reviewer is clean and all exact-head workflows are green. I found no material ownership/concurrency blocker.