AFK reject→requeue: bounce a rejected PR back to the ready queue with feedback for the next run #44

New issue

Open

opened 2026-07-08 10:12:42 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-08 10:12:42 +02:00

Owner

This was generated by AI during triage.

Problem / goal

Today an AFK run's only good ending is a merge. When a run produces a PR/CR a reviewer does not want to land, there is no way to send that work back to an AFK agent to try again. The reviewer's only lever is to close the PR — which, in lab's model, does close to the opposite of "retry":

The claim is the afk/<N> branch. The auto-loop launches only on a claimable issue = ready-for-agent minus issues that already carry a claim branch (ClaimableIssues / FilterClaimable).
The reaper's done-signal is open-or-merged only — a closed-unmerged PR/CR deliberately does not count (tracker.PRPresent).
Closing a PR keeps its head branch (ADR-0011: "closing without merging keeps the head branch and its parked work"). So a rejected (closed) PR leaves the afk/<N> claim in place → the issue stays claimed → not claimable → the auto-loop never re-picks it. The work is stranded as parked, not recycled.
The only requeue action today is deleting the branch — human-only, via the Parked view's Unguarded Discard ("deleting the branch is also the only requeue action").
A retry reads the issue, not the dead PR. A fresh run cuts a new branch and opens a new PR; it never sees the closed one. So feedback that should improve the retry has to live on the issue — labctl issue comment <n> already exists for exactly that.

Goal: a first-class reject → requeue action — a reviewer (and/or an agent) bounces a PR/CR back so its issue returns to the claimable ready queue, carrying the rejection feedback the next run will read.

Why this needs design (not just a labctl verb)

Requeue-on-reject brushes several deliberately-pinned invariants — branch-is-the-claim, closed-is-not-a-done-signal, guarded teardown (ADR-0010, ADR-0011, ADR-0013). It can't be pr close with a side effect bolted on. Open questions:

What releases the claim? Auto-delete the afk/<N> branch on reject, a distinct reviewer action, or lift the Parked-view Unguarded Discard onto the agent/reviewer surface? Guarded teardown currently refuses to delete an unmerged branch on purpose.
Keep the parked worktree/branch for diffing, or discard it? The rejected attempt may be worth reading; requeue needs the claim released without necessarily destroying the artifact.
Where does feedback live, and how does it accumulate? Issue comments are the durable channel (they survive across the new branch/PR). Across several reject→retry rounds, does the next run get the whole thread, the latest note, or a structured "previous attempt rejected because…" block?
Stopping condition. A reject→retry that keeps producing rejectable PRs must not loop forever. How does this interact with the three-strikes pause and the budget clock — is requeue capped, human-gated after N rounds, or does a rejected retry count as a failure?
Which surface initiates a reject? Operator UI (the Parked view already exists), labctl (agent/reviewer-in-session), or both? Reject is a judgment call — does it belong on the agent surface at all, or is it reviewer-only?
State-label semantics. The issue presumably stays ready-for-agent through a requeue; confirm nothing needs a new state.
Relationship to pr close / pr comment. These smaller ops live in this family: a plain pr close (reject without requeue) and pr comment (a PR-thread note, as opposed to issue-level feedback). Decide whether reject is close+requeue as one action or composed from smaller verbs.

Relationship

Sibling of #37 (labctl pr merge, the land half of the same review loop). #37 stays merge-only; this issue owns everything about rejecting and recycling work.

Suggested next step

A design / grill pass — and likely an ADR amending the done-signal / claim invariants (ADR-0010 / 0011 / 0013) — before this is agent-ready.

> *This was generated by AI during triage.* ## Problem / goal Today an AFK run's only good ending is a merge. When a run produces a PR/CR a reviewer does **not** want to land, there is no way to send that work back to an AFK agent to try again. The reviewer's only lever is to close the PR — which, in lab's model, does close to the *opposite* of "retry": - **The claim is the `afk/<N>` branch.** The auto-loop launches only on a *claimable* issue = `ready-for-agent` minus issues that already carry a claim branch (`ClaimableIssues` / `FilterClaimable`). - **The reaper's done-signal is open-or-merged only** — a closed-unmerged PR/CR deliberately does not count (`tracker.PRPresent`). - **Closing a PR keeps its head branch** (ADR-0011: "closing without merging keeps the head branch and its parked work"). So a rejected (closed) PR leaves the `afk/<N>` claim in place → the issue stays *claimed* → **not claimable** → the auto-loop never re-picks it. The work is stranded as parked, not recycled. - **The only requeue action today is deleting the branch** — human-only, via the Parked view's Unguarded Discard ("deleting the branch is also the only requeue action"). - **A retry reads the *issue*, not the dead PR.** A fresh run cuts a new branch and opens a new PR; it never sees the closed one. So feedback that should improve the retry has to live on the **issue** — `labctl issue comment <n>` already exists for exactly that. **Goal:** a first-class **reject → requeue** action — a reviewer (and/or an agent) bounces a PR/CR back so its issue returns to the claimable ready queue, carrying the rejection feedback the next run will read. ## Why this needs design (not just a labctl verb) Requeue-on-reject brushes several deliberately-pinned invariants — branch-is-the-claim, closed-is-not-a-done-signal, guarded teardown (ADR-0010, ADR-0011, ADR-0013). It can't be `pr close` with a side effect bolted on. Open questions: - **What releases the claim?** Auto-delete the `afk/<N>` branch on reject, a distinct reviewer action, or lift the Parked-view Unguarded Discard onto the agent/reviewer surface? Guarded teardown currently refuses to delete an unmerged branch on purpose. - **Keep the parked worktree/branch for diffing, or discard it?** The rejected attempt may be worth reading; requeue needs the *claim* released without necessarily destroying the artifact. - **Where does feedback live, and how does it accumulate?** Issue comments are the durable channel (they survive across the new branch/PR). Across several reject→retry rounds, does the next run get the whole thread, the latest note, or a structured "previous attempt rejected because…" block? - **Stopping condition.** A reject→retry that keeps producing rejectable PRs must not loop forever. How does this interact with the three-strikes pause and the budget clock — is requeue capped, human-gated after N rounds, or does a rejected retry count as a failure? - **Which surface initiates a reject?** Operator UI (the Parked view already exists), `labctl` (agent/reviewer-in-session), or both? Reject is a judgment call — does it belong on the agent surface at all, or is it reviewer-only? - **State-label semantics.** The issue presumably stays `ready-for-agent` through a requeue; confirm nothing needs a new state. - **Relationship to `pr close` / `pr comment`.** These smaller ops live in this family: a plain `pr close` (reject without requeue) and `pr comment` (a PR-thread note, as opposed to issue-level feedback). Decide whether reject is close+requeue as one action or composed from smaller verbs. ## Relationship Sibling of #37 (`labctl pr merge`, the *land* half of the same review loop). #37 stays merge-only; this issue owns everything about rejecting and recycling work. ## Suggested next step A design / grill pass — and likely an ADR amending the done-signal / claim invariants (ADR-0010 / 0011 / 0013) — before this is agent-ready.

dominik.polakovics added the

needs-triage

enhancement

labels

2026-07-08 10:12:42 +02:00

dominik.polakovics commented

2026-07-09 00:33:10 +02:00

Author

Owner

This was generated by AI during triage.

Triage: ready-for-human

Category: enhancement
Summary: confirmed the invariants this issue leans on are still exactly as
described in the current code — ClaimableIssues/FilterClaimable
(internal/afk/decide.go, internal/afk/launch.go) filter the ready queue by
claim-branch presence, and tracker.PRPresent (internal/tracker/pulls.go) is
the reaper's open-or-merged-only done-signal. A closed-unmerged PR/CR still
leaves the afk/<N> claim branch in place, and the only requeue action today
is still the human-only Unguarded Discard. Nothing here has drifted since
filing.

Why this is ready-for-human, not ready-for-agent: this issue itself
lists seven open design questions (what releases the claim, whether to keep or
discard the parked worktree, where/how rejection feedback accumulates across
retries, the stopping condition against the three-strikes pause and budget
clock, which surface can initiate a reject, state-label semantics, and the
relationship to pr close/pr comment) and explicitly flags that this
"brushes several deliberately-pinned invariants" (ADR-0010, ADR-0011,
ADR-0013) and likely needs an ADR amendment before it's agent-ready. These are
product/architecture trade-offs on invariants you pinned deliberately — not
gaps an agent should fill by guessing your intent, and picking wrong here
would mean re-litigating pinned ADR semantics later. This matches the issue's
own "Suggested next step: a design/grill pass" — that pass needs you, not an
agent brief.

Out of scope (for now):

Any implementation — this stays a design/ADR question until the seven open
questions above are resolved.
#37 (labctl pr merge) — sibling issue, stays merge-only regardless of how
this is decided.

Relabeling needs-triage → ready-for-human.

> *This was generated by AI during triage.* ## Triage: ready-for-human **Category:** enhancement **Summary:** confirmed the invariants this issue leans on are still exactly as described in the current code — `ClaimableIssues`/`FilterClaimable` (`internal/afk/decide.go`, `internal/afk/launch.go`) filter the ready queue by claim-branch presence, and `tracker.PRPresent` (`internal/tracker/pulls.go`) is the reaper's open-or-merged-only done-signal. A closed-unmerged PR/CR still leaves the `afk/<N>` claim branch in place, and the only requeue action today is still the human-only Unguarded Discard. Nothing here has drifted since filing. **Why this is `ready-for-human`, not `ready-for-agent`:** this issue itself lists seven open design questions (what releases the claim, whether to keep or discard the parked worktree, where/how rejection feedback accumulates across retries, the stopping condition against the three-strikes pause and budget clock, which surface can initiate a reject, state-label semantics, and the relationship to `pr close`/`pr comment`) and explicitly flags that this "brushes several deliberately-pinned invariants" (ADR-0010, ADR-0011, ADR-0013) and likely needs an ADR amendment before it's agent-ready. These are product/architecture trade-offs on invariants you pinned deliberately — not gaps an agent should fill by guessing your intent, and picking wrong here would mean re-litigating pinned ADR semantics later. This matches the issue's own "Suggested next step: a design/grill pass" — that pass needs you, not an agent brief. **Out of scope (for now):** - Any implementation — this stays a design/ADR question until the seven open questions above are resolved. - #37 (`labctl pr merge`) — sibling issue, stays merge-only regardless of how this is decided. Relabeling `needs-triage` → `ready-for-human`.

dominik.polakovics added

ready-for-human

and removed

needs-triage

labels

2026-07-09 00:33:15 +02:00