Skip to content

Release cancelled review leases on force - #582

Open
prathamdby wants to merge 2 commits into
mainfrom
pd/fix/force-requeue-claim-41ca
Open

Release cancelled review leases on force#582
prathamdby wants to merge 2 commits into
mainfrom
pd/fix/force-requeue-claim-41ca

Conversation

@prathamdby

@prathamdby prathamdby commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • cancelActiveReviews still marks queued and running review rows cancelled.
  • It then calls releasePrActorLeaseHeldByWorkItems with those row ids.
  • releasePrActorLeaseHeldByWorkItems in src/agentWork/prActorLease.ts clears work_item_id and holder_id on matching pr_actor_leases rows for that resource_key and work_type.
  • /review force uses that cancel path, so the replacement can acquire the lease in the same intake transaction.
  • /cancel and pull-request close cancel use the same clear.
  • CONTEXT.md, ADR 0030, docs/operations.md, and docs/agent-work-ops.md record the holder clear.

Details

Cancel clears the holder

cancelActiveReviews in src/agentWork/intake/workItemRepository.ts maps cancelled rows, then releases any lease those ids still hold.

cancelActiveReviews
  update queued reviews to cancelled
  update running reviews to cancelled
  releasePrActorLeaseHeldByWorkItems
    UPDATE pr_actor_leases
      SET work_item_id = NULL, holder_id = NULL, expires_at = now()
      WHERE resource_key AND work_type AND work_item_id = ANY(ids)
 cancelActiveReviews
   update queued
   update running
-  return cancelled rows
+  releasePrActorLeaseHeldByWorkItems(cancelled ids)
+  return cancelled rows
export async function releasePrActorLeaseHeldByWorkItems(
  db: Pool | PoolClient,
  params: PrActorLeaseKey & { readonly workItemIds: readonly string[] },
): Promise<void>
sequenceDiagram
    participant Slash
    participant Cancel
    participant Lease
    participant Replacement
    Slash->>Cancel: /review force
    Cancel->>Cancel: status cancelled
    Cancel->>Lease: clear holder for cancelled ids
    Slash->>Replacement: create queued review
    Replacement->>Lease: acquire
Loading

Docs

CONTEXT, ADR 0030 decision 5, operations /cancel /review force close cancel, and agent-work-ops name the holder clear.

 CONTEXT.md
 docs/
 ├── adr/0030-pr-actor-lease.md
 ├── agent-work-ops.md
 └── operations.md
Open in Web Open in Cursor 

PR Agent Description

PR Type

Enhancement, Tests, Documentation

Description

  • cancelActiveReviews now clears the pr_actor_leases holder for every cancelled review row, inside the same intake transaction.
  • New releasePrActorLeaseHeldByWorkItems matches on work_item_id within one resource key and work type, so it cannot clear another holder's lease.
  • Why: /review force previously waited for the running worker to exit or for PR_ACTOR_LEASE_TTL_SECONDS before the replacement could acquire.
  • The clear matches on work item id, not epoch, so the cancelled worker fails its next fencing check and its writes stay blocked.
  • The lease row is never deleted, so the epoch stays monotonic; the integration test asserts the replacement acquires heldEpoch + 1 and then claims.
  • Scope risk: only review leases are cleared; cancelActiveTriage keeps the cooperative exit plus TTL path for triage work.
  • Tests: reviewCloseCancel expects three queries and asserts the new SQL parameters; slashIntake mocks stub the added pr_actor_leases query.
  • Docs update CONTEXT.md, ADR 0030, agent-work-ops.md, and operations.md to describe the immediate lease handoff for /cancel, /review force, and close cancel.

Changes Diagram

flowchart LR
  A["Slash force, cancel, or close intake"] --> B["Cancel queued and running review rows"]
  B --> C["Clear lease holder by work item id"]
  C --> D["Replacement acquires next lease epoch"]
  D --> E["Worker claims and runs the review"]
  B --> F["Old worker fails fencing check"]
Loading

Review map

  1. src/agentWork/prActorLease.ts: New id-based lease release; verify the match cannot clear a live holder
  2. src/agentWork/intake/workItemRepository.ts: Cancel wiring; confirm the clear runs in the intake transaction
  3. test/integration/slashActiveUniqueness.integration.test.ts: End-to-end proof that a force replacement acquires the next epoch
  4. test/reviewCloseCancel.test.ts: Unit contract for the new SQL call, parameters, and query count
  5. docs/adr/0030-pr-actor-lease.md: Recorded design decision; check scope wording against behaviour

- Add a slash force case that holds a review lease then expects the
  replacement to acquire and claim
- Clear cancelled review lease holders during intake so a force
  replacement can acquire and claim immediately
- Document intake lease clear on force, cancel, and close
@zeus-review

zeus-review Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

PR Agent Review

Note

The orchestrated review completed.

SizeM
FindingsNo issues on this pass.
Relevant testspartial
SecurityNone found on this pass
CI✅ All CI is passing

4d5d35f ⋅ general ⋅ 43m 47s ⋅ glm-5.3

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
pr-agent Ready Ready Preview Sep 8, 2026 2:02pm UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant