Queue and lease

Where an accepted trigger waits, and what stops the same review being paid for twice. Implemented in src/pr_review_agent/queue.py, over the queue table declared in STORAGE.md.

🧷 Why a queue at all

A trigger is not reviewed where it is classified. It is enqueued, and a worker claims it later.

That indirection is what the spending rails need. A claim is the single point at which work becomes expensive, so it is the single point the budget governor has to guard — and the reservation is taken inside the same transaction as the claim, through the admit hook below.

🔑 A trigger is enqueued at most once

The dedupe key is the table's primary key, so enqueue is an INSERT OR IGNORE that returns whether the row was new. A re-poll that sees the same freshly opened pull request, or the same @claude comment, inserts nothing.

Rows are kept after completion. That is the point: the key is what makes "already reviewed" a fact rather than a guess. A pull request whose head moves gets a new key and is genuinely new work; a mention never does, because its key carries the comment id instead of the head.

📮 Not every row is a review

Two kinds of row come from the classifier — pr_opened and mention — and one the worker enqueues for itself: publish, naming a recorded run a post could not deliver. It is an ordinary row, so it takes the ordinary lease and the ordinary dedupe, and it is the only kind the budget governor is not asked about: posting a review reaches no engine. See PUBLISHER.md.

🔒 One pull request, one worker

A claim is refused while any other row for the same (repo, pr_number) holds a live lease. A maintainer's @claude arriving mid-review therefore waits for the run in flight rather than racing it.

🪪 A queue belongs to one repository

ReviewQueue(store, repo=…) is scoped, and _CLAIMABLE filters on it. When several daemons share one store they share this table with it, and each holds a token for its own repository only — so an unscoped claim would hand a trigger to the one process that cannot act on it, which is the boundary the per-process split exists to draw. The sweep that abandons exhausted rows is scoped for the same reason: a daemon never writes to another repository's rows.

Enqueueing is not filtered. A poller only ever sees its own repository, so there is nothing to exclude.

The claim is a read that picks a row followed by a write that leases it. SQLite has no SKIP LOCKED, so the pair runs inside one BEGIN IMMEDIATE transaction — the write lock is taken up front, which is what makes it atomic against another writer.

🔌 The admit hook

claim(now=, owner=, admit=None) takes an optional predicate (connection, claim, now) -> bool, called inside the claim's own transaction. That is what makes the budget reservation atomic with the lease: the governor writes its ledger row on the connection it is handed, and the row and the lease commit together or not at all.

It is a plain predicate rather than anything richer, so no budget type appears in this signature and queue.py imports nothing from the governor.

A refused candidate is skipped, not final. _CLAIMABLE has no LIMIT 1; claim walks the ordered candidates and leases the first one admit accepts.

That matters at the ladder's 85 % rung, where fresh pull requests stop being auto-reviewed but an explicit @claude is still honoured. A refused pull request keeps its pending status and its attempt count — a refusal is about the allowance, not the trigger, and burning an attempt would let three refusals abandon a perfectly good one. So it correctly stays at the head of a FIFO queue, and without skipping it would block the maintainer's mention behind it until the window rolled: days.

The same skip is what makes the pacer a deferral rather than a drop: a pull request reviewed too recently is refused here, keeps its pending row and its attempt count, and is claimed on a later pass once the interval has elapsed — by which time it is reviewed against whatever the head is then.

Every other condition in _CLAIMABLE is a fact about the row, which SQLite can already exclude on. A budget refusal is the first decision it cannot express, because it depends on the ledger, the rung and the trigger's kind.

The candidates are read out with fetchall before one is leased, because committing with a half-consumed cursor still open raises "SQL statements in progress". The claimable set is bounded by the pending queue, which is small.

The hook is optional, and that is the known weakness of the seam. Nothing in the type system stops a caller claiming without a governor; DESIGN.md's one rule, review, and the fact that the only claimer will be the engine phase's worker are what hold it.

⏱ Leases expire; they are not renewed

The lease carries an expiry and no heartbeat. A run has a wall-clock ceiling (BUDGET.md, layer 3), so a lease set above that ceiling cannot lapse under a worker that is still alive, and renewal would be machinery for a case that cannot arise. The default is 30 minutes.

An expired lease makes the row claimable again — that is the crashed-worker path — and release hands work back immediately without waiting the lease out.

complete and release are both guarded on the owner. A worker whose lease lapsed and was re-claimed by somebody else gets False rather than finishing the newer worker's row, which is how it learns to discard its result instead of publishing it late.

🧯 Retries are bounded

Each claim increments attempts, and a row that has used up max_attempts (default 3) is marked abandoned instead of being offered again.

Without that bound, a trigger that crashes its worker every time would be re-reviewed forever, and each attempt spends allowance before it fails. Three is enough to ride out a transient failure and few enough that a poison trigger cannot drain the weekly allowance one retry at a time. tests/test_queue_claim.py::test_retries_are_bounded pins it.

🧬 Statuses

enqueue                     claim()                complete()
(INSERT OR IGNORE)  ──►  pending  ──────────────►  claimed  ──────────────►  done
                      (attempts+1)

claimed returns to pending on release() or when the lease simply lapses; either way attempts is already counted. abandoned is reached from either state: claim() reaches it directly when a row's attempts are already at max_attempts, and a worker holding claimed reaches it by calling abandon() on a failure that will only fail again.

Status	Meaning
pending	Waiting for a worker.
claimed	Leased until leased_until, by owner.
done	Reviewed. Never offered again.
abandoned	Given up on. Never offered again.

abandoned is reached two ways: by using up max_attempts, which claim does for itself, and by a worker calling abandon on a failure that will fail again — an oversized pull request, an unusable payload. See WORKER.md.

complete, release and abandon are the three verbs a worker closes a row with, and all three are guarded on the owner.

🧺 One review answers what was waiting

The agent posts one comment per review. So three maintainers mentioning it on one pull request asked one question: reviewing each mention separately would pay three times to post three comments saying the same thing.

After a review records its findings, the worker calls fold, which closes every other row for that pull request that

• was already pending when the review started — anything enqueued during it may be asking about something the review never read;
• named no other commit (head_sha IS NULL, which every mention is, or the same sha the review actually read);
• is not a publish item — that names a recorded run of its own;
• asked for the same thing — the command column, review or describe. A description does not answer a waiting @claude review (DESCRIBE.md).

Folded rows end done, and the count is logged. They are never acknowledged with 👀, because the acknowledgement happens at claim time and they were never claimed.

Fold is the other half of the pacer. Without it, a burst deferred by the interval would simply be drained one review per interval afterwards; with it, the review the burst was waiting for answers the whole burst.

🔭 What the queue does not do

Claim.trigger.head_sha is the head observed when the trigger was classified, and for a mention it is None — the comment payload does not carry one (POLLER.md).

It is what the review runs against, but it is not what makes the review safe to post. The publisher re-reads the live head immediately before posting and marks a review of a superseded commit as describing that commit. The queue cannot do that check itself: it would have to be a GitHub read, and it has to happen at publish time rather than claim time to be worth anything.

The queue also does not know which recorded run is still owed a comment. That lives in runs, and the outstanding posting is a publish row of its own — admitted without reserving, because publishing reaches no engine and weighing it against a budget window would be refusing to spend nothing. Such a claim is handed back with release_unattempted if the post fails, because max_attempts bounds allowance drained and this drains none. See PUBLISHER.md.