Cloonar/coding-lab

Fork 0

autoland: escalation is permanently terminal with no way back in — re-arm a PR the humans have moved on #188

New issue

Closed

opened 2026-07-20 18:25:25 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-20 18:25:25 +02:00

Owner

Follow-up from landing #187 (issue #182). Not a regression in that PR — the gate works as designed; the design has no exit.

Problem

An outcome='escalated' run makes its PR invisible to the autoland poller forever (EscalatedRunOnBranch, internal/store/afk.go). The docs say re-entry is "exactly one path — a human running the interactive land-pr skill." Three cases break that:

Requeue reuses the branch. The gate keys on (repo_id, branch) with no PR or time scoping, and afk/<N> is derived from the issue number. Discard the escalated run, let a fresh AFK run re-claim the issue, and the brand-new claim PR is invisible from birth — a PR that has never been validated, let alone escalated.
The human fixes it themselves and wants an agent to re-validate. Same PR, still terminal.
The human hands an agent a manual description to fix it. Same PR, still terminal.

All three fail silently: autolandCandidates continues at the gate with no log line, no marker on the new PR, nothing in the SPA. The symptom is "autoland just ignores this PR" with no diagnosis path short of reading the runs table.

Why keying on the PR number is not the fix

The obvious fix — key terminality on the PR, not the branch — only fixes case 1. Cases 2 and 3 are the same PR.

The real problem is that escalation is modelled as a permanent property of a thing, when it is a statement about a moment: "as of these N attempts, agents could not finish this." A human commit or a fresh set of instructions is new information that invalidates the statement, and nothing in the design can express that. A better key makes the gate more accurate, not less permanent.

What to build

A re-arm operation, not an un-escalate:

Clears the terminality for the PR and zeroes its fix / escalate counters in autoland_attempts (added by #187, migration 0015). Both halves, together: clearing terminality while leaving the fix budget spent would escalate again on the first rejection — the worst outcome, because it looks like the clear silently did not work.
Re-key the terminality gate on the PR rather than the branch while in here. It does not solve the problem on its own, but branch-keying is separately wrong on requeue (case 1) and it is the same gate.
Log the suppression at the gate. Independent of everything above, and worth doing even if the rest is deferred: one line turns a silent mystery into a five-second diagnosis.

Open question — the surface

A product call, not an implementation detail. Options:

a labctl / lab CLI verb
an SPA action in the run history or the PR view
both

Worth deciding alongside: should re-arm be implicit on some signal (a new human commit on the head branch? the issue flipping back to ready-for-agent?) rather than an explicit gesture. Implicit is friendlier but re-introduces the auto-requeue smell ADR-0024 forbids — the point of the fix-forward design is that re-engagement carries new information, and "a human pushed something" is arguably exactly that signal. Needs a decision before implementation.

Notes

ADR-0048 will need a revision pointer once the terminality semantics change.
CONTEXT.md's Escalation entry currently states re-entry is land-pr only; update it with whatever lands.

Follow-up from landing #187 (issue #182). Not a regression in that PR — the gate works as designed; the design has no exit. ## Problem An `outcome='escalated'` run makes its PR invisible to the autoland poller forever (`EscalatedRunOnBranch`, `internal/store/afk.go`). The docs say re-entry is "exactly one path — a human running the interactive land-pr skill." Three cases break that: 1. **Requeue reuses the branch.** The gate keys on `(repo_id, branch)` with no PR or time scoping, and `afk/<N>` is derived from the issue number. Discard the escalated run, let a fresh AFK run re-claim the issue, and the brand-new claim PR is invisible **from birth** — a PR that has never been validated, let alone escalated. 2. **The human fixes it themselves** and wants an agent to re-validate. Same PR, still terminal. 3. **The human hands an agent a manual description** to fix it. Same PR, still terminal. All three fail **silently**: `autolandCandidates` `continue`s at the gate with no log line, no marker on the new PR, nothing in the SPA. The symptom is "autoland just ignores this PR" with no diagnosis path short of reading the runs table. ## Why keying on the PR number is not the fix The obvious fix — key terminality on the PR, not the branch — only fixes case 1. Cases 2 and 3 are the same PR. The real problem is that escalation is modelled as a permanent property of a *thing*, when it is a statement about a *moment*: "as of these N attempts, agents could not finish this." A human commit or a fresh set of instructions is new information that invalidates the statement, and nothing in the design can express that. A better key makes the gate more accurate, not less permanent. ## What to build **A re-arm operation**, not an un-escalate: - Clears the terminality for the PR **and** zeroes its `fix` / `escalate` counters in `autoland_attempts` (added by #187, migration 0015). Both halves, together: clearing terminality while leaving the fix budget spent would escalate again on the first rejection — the worst outcome, because it looks like the clear silently did not work. - Re-key the terminality gate on the PR rather than the branch while in here. It does not solve the problem on its own, but branch-keying is separately wrong on requeue (case 1) and it is the same gate. - **Log the suppression** at the gate. Independent of everything above, and worth doing even if the rest is deferred: one line turns a silent mystery into a five-second diagnosis. ## Open question — the surface A product call, not an implementation detail. Options: - a `labctl` / `lab` CLI verb - an SPA action in the run history or the PR view - both Worth deciding alongside: should re-arm be implicit on some signal (a new human commit on the head branch? the issue flipping back to `ready-for-agent`?) rather than an explicit gesture. Implicit is friendlier but re-introduces the auto-requeue smell ADR-0024 forbids — the point of the fix-forward design is that re-engagement carries new information, and "a human pushed something" is arguably exactly that signal. Needs a decision before implementation. ## Notes - ADR-0048 will need a revision pointer once the terminality semantics change. - CONTEXT.md's **Escalation** entry currently states re-entry is land-pr only; update it with whatever lands.

dominik.polakovics added the

needs-triage

enhancement

labels

2026-07-20 18:25:25 +02:00

dominik.polakovics referenced this issue

2026-07-20 18:25:46 +02:00

autoland fix-forward loop: fix runs on rejection, spawn-count bound, escalation #182

dominik.polakovics commented

2026-08-03 22:24:25 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Add an explicit, human-triggered re-arm operation that returns an escalated PR to autoland's view (terminality cleared and attempt budgets restored), scope escalation terminality to the PR rather than the branch, and log the gate's suppression. Product decisions settled during triage: re-arm is exposed on both the SPA and the operator CLI; re-arm is explicit only — no implicit re-arm on human pushes or label flips.

Current behavior:
Escalation is permanently terminal. The autoland poller treats a PR as escalated when either of two sources says so: an outcome='escalated' run row for the (repo, branch) pair (EscalatedRunOnBranch in the store), or an escalate verdict marker comment at any position in the PR's comment stream (unlike reject/pass/fix-done, which fold last-word-wins). When PullVerdictState.Escalated is set, DecideAutoland returns no action and the poller silently skips the pull — no log line, no SPA marker. Because the run-row half keys on branch (not PR) and afk/<N> branches derive from issue numbers, a requeued issue's brand-new PR is invisible from birth. Fix/escalate budgets live in the autoland_attempts table keyed (repo, branch, kind), so a reused branch also inherits its spent budgets. Re-entry today is exactly one path: a human running the interactive land-pr skill.

Desired behavior:

Re-arm operation. A human can re-arm an escalated PR, after which the autoland poller sees it again exactly as it would a rejected-at-zero-attempts PR: full MaxFixAttempts and MaxEscalateAttempts budgets, no terminality. Re-arm and budget reset are one atomic operation — clearing terminality while leaving budgets spent would re-escalate on the first rejection and read as a silent failure of the re-arm.
Both terminality sources must be superseded. This is the trap in the naive design: clearing the store row alone leaves the escalate marker comment terminal forever. Re-arm must record a durable supersession moment for the PR, and the escalated fold becomes: terminal iff an escalation signal exists after the last re-arm — for the run-row half by comparing against the run's timestamp, for the comment half by comparing against the marker comment's CreatedAt (already on tracker.Comment). A fresh escalation after a re-arm is terminal again, indefinitely repeatable. Escalated run rows are history — never deleted or rewritten; supersession, not erasure.
Terminality and budgets scope to the PR, not the branch. A new PR on a reused branch starts virgin: no inherited terminality, no inherited spent budgets. (This alone does not remove the need for re-arm — the human-fixes-it and manual-description cases are the same PR — but branch-keying is independently wrong on requeue.)
Surfaces. One httpapi endpoint (human-authenticated, following the existing repo-action POST pattern, e.g. the AFK failure-counter reset), consumed by both an SPA action where the escalated PR/run is visible and an operator CLI verb. Re-arm must not be exposed on the run-token agent surface (labctl): escalation's meaning is "agents could not finish this", and an agent able to lift its own terminal hand-off would make the bound decorative. The SPA should also stop rendering the suppression invisibly wherever it already shows run/PR state — at minimum the escalated state should be visible where the re-arm action lives.
Log the suppression. When the poller skips a pull because of terminality, emit one log line naming repo, pull number, branch, and which source (run row or marker comment) tripped the gate. Worth keeping even if everything else here were deferred.
Docs. ADR-0048 gains a revision pointer for the changed terminality semantics (supersedable, PR-scoped, explicit re-arm). CONTEXT.md's Escalation entry currently states re-entry is land-pr only — update it to name re-arm as the second path and keep the Avoid list intact.

Key interfaces:

EscalatedRunOnBranch (store) — becomes a PR-scoped, re-arm-aware query (rename accordingly); callers pass the pull number they already hold.
PullVerdictState / DecideAutoland — the Escalated input's derivation changes (supersession fold); the decision function itself should not need new states.
VerdictWords / the poller's escalated OR — the comment half moves from "contains escalate anywhere" to "escalate not superseded by a later re-arm".
autoland_attempts — keying gains PR scoping; re-arm zeroes (or equivalently re-baselines) the fix and escalate rows for that PR. New migration; follow the existing goose migration conventions including the named-constraint discipline.
A durable re-arm record (new store surface) — must survive restarts and support "escalation after last re-arm" comparisons.
httpapi route + SPA action + operator CLI (lab) verb — all three converge on the same store operation.

Acceptance criteria:

An escalated PR, once re-armed, is picked up again by the autoland poller on the next pass and can spend a full fix budget before escalating again.
Re-arm works when the terminality source is the run row only, the marker comment only, and both (the human-deleted-comment and comment-still-present cases).
A second escalation after a re-arm is terminal again; a second re-arm lifts it again.
A new PR whose head branch previously carried an escalated run (requeue case) is visible to autoland from birth, with zero spent attempts, without any re-arm.
Re-arm resets both the fix and escalate attempt counts in the same operation; a re-armed PR whose first lander verdict is reject spawns a fix run, not an escalate run.
The poller logs every terminality suppression with repo, pull, branch, and source.
Re-arm is reachable from the SPA and the operator CLI, both via the human-authenticated httpapi; no labctl (run-token) verb exists for it.
ADR-0048 carries the revision pointer; CONTEXT.md's Escalation entry names re-arm as a re-entry path.
Store, decide, poller, and httpapi behavior above is covered by tests in the style of the existing autoland/store test suites.

Out of scope:

Implicit re-arm on any signal (human push to the head branch, issue label flips) — explicitly decided against during triage; if wanted later it is its own issue over the same store operation.
Builtin-binding autoland (still gated on builtin PR comments, its own follow-up per ADR-0048).
Any change to the escalation flow itself (digest comment, ready-for-human flip, push notification) or to MaxEscalateAttempts semantics.
Un-pausing three-strikes or any other suppression that is not escalation terminality.
Reworking the verdict-marker grammar beyond what the supersession fold needs.

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Add an explicit, human-triggered **re-arm** operation that returns an escalated PR to autoland's view (terminality cleared *and* attempt budgets restored), scope escalation terminality to the PR rather than the branch, and log the gate's suppression. Product decisions settled during triage: re-arm is exposed on **both** the SPA and the operator CLI; re-arm is **explicit only** — no implicit re-arm on human pushes or label flips. **Current behavior:** Escalation is permanently terminal. The autoland poller treats a PR as escalated when **either** of two sources says so: an `outcome='escalated'` run row for the `(repo, branch)` pair (`EscalatedRunOnBranch` in the store), or an `escalate` verdict marker comment at **any** position in the PR's comment stream (unlike `reject`/`pass`/`fix-done`, which fold last-word-wins). When `PullVerdictState.Escalated` is set, `DecideAutoland` returns no action and the poller silently skips the pull — no log line, no SPA marker. Because the run-row half keys on branch (not PR) and `afk/<N>` branches derive from issue numbers, a requeued issue's brand-new PR is invisible from birth. Fix/escalate budgets live in the `autoland_attempts` table keyed `(repo, branch, kind)`, so a reused branch also inherits its spent budgets. Re-entry today is exactly one path: a human running the interactive land-pr skill. **Desired behavior:** 1. **Re-arm operation.** A human can re-arm an escalated PR, after which the autoland poller sees it again exactly as it would a rejected-at-zero-attempts PR: full `MaxFixAttempts` and `MaxEscalateAttempts` budgets, no terminality. Re-arm and budget reset are one atomic operation — clearing terminality while leaving budgets spent would re-escalate on the first rejection and read as a silent failure of the re-arm. 2. **Both terminality sources must be superseded.** This is the trap in the naive design: clearing the store row alone leaves the `escalate` marker comment terminal forever. Re-arm must record a durable supersession moment for the PR, and the escalated fold becomes: terminal iff an escalation signal exists **after** the last re-arm — for the run-row half by comparing against the run's timestamp, for the comment half by comparing against the marker comment's `CreatedAt` (already on `tracker.Comment`). A fresh escalation after a re-arm is terminal again, indefinitely repeatable. Escalated run rows are history — never deleted or rewritten; supersession, not erasure. 3. **Terminality and budgets scope to the PR, not the branch.** A new PR on a reused branch starts virgin: no inherited terminality, no inherited spent budgets. (This alone does not remove the need for re-arm — the human-fixes-it and manual-description cases are the same PR — but branch-keying is independently wrong on requeue.) 4. **Surfaces.** One httpapi endpoint (human-authenticated, following the existing repo-action POST pattern, e.g. the AFK failure-counter reset), consumed by both an SPA action where the escalated PR/run is visible and an operator CLI verb. Re-arm must **not** be exposed on the run-token agent surface (`labctl`): escalation's meaning is "agents could not finish this", and an agent able to lift its own terminal hand-off would make the bound decorative. The SPA should also stop rendering the suppression invisibly wherever it already shows run/PR state — at minimum the escalated state should be visible where the re-arm action lives. 5. **Log the suppression.** When the poller skips a pull because of terminality, emit one log line naming repo, pull number, branch, and which source (run row or marker comment) tripped the gate. Worth keeping even if everything else here were deferred. 6. **Docs.** ADR-0048 gains a revision pointer for the changed terminality semantics (supersedable, PR-scoped, explicit re-arm). CONTEXT.md's **Escalation** entry currently states re-entry is land-pr only — update it to name re-arm as the second path and keep the _Avoid_ list intact. **Key interfaces:** - `EscalatedRunOnBranch` (store) — becomes a PR-scoped, re-arm-aware query (rename accordingly); callers pass the pull number they already hold. - `PullVerdictState` / `DecideAutoland` — the `Escalated` input's derivation changes (supersession fold); the decision function itself should not need new states. - `VerdictWords` / the poller's escalated OR — the comment half moves from "contains `escalate` anywhere" to "escalate not superseded by a later re-arm". - `autoland_attempts` — keying gains PR scoping; re-arm zeroes (or equivalently re-baselines) the `fix` and `escalate` rows for that PR. New migration; follow the existing goose migration conventions including the named-constraint discipline. - A durable re-arm record (new store surface) — must survive restarts and support "escalation after last re-arm" comparisons. - httpapi route + SPA action + operator CLI (`lab`) verb — all three converge on the same store operation. **Acceptance criteria:** - [ ] An escalated PR, once re-armed, is picked up again by the autoland poller on the next pass and can spend a full fix budget before escalating again. - [ ] Re-arm works when the terminality source is the run row only, the marker comment only, and both (the human-deleted-comment and comment-still-present cases). - [ ] A second escalation after a re-arm is terminal again; a second re-arm lifts it again. - [ ] A new PR whose head branch previously carried an escalated run (requeue case) is visible to autoland from birth, with zero spent attempts, **without** any re-arm. - [ ] Re-arm resets both the fix and escalate attempt counts in the same operation; a re-armed PR whose first lander verdict is reject spawns a fix run, not an escalate run. - [ ] The poller logs every terminality suppression with repo, pull, branch, and source. - [ ] Re-arm is reachable from the SPA and the operator CLI, both via the human-authenticated httpapi; no `labctl` (run-token) verb exists for it. - [ ] ADR-0048 carries the revision pointer; CONTEXT.md's Escalation entry names re-arm as a re-entry path. - [ ] Store, decide, poller, and httpapi behavior above is covered by tests in the style of the existing autoland/store test suites. **Out of scope:** - Implicit re-arm on any signal (human push to the head branch, issue label flips) — explicitly decided against during triage; if wanted later it is its own issue over the same store operation. - Builtin-binding autoland (still gated on builtin PR comments, its own follow-up per ADR-0048). - Any change to the escalation flow itself (digest comment, `ready-for-human` flip, push notification) or to `MaxEscalateAttempts` semantics. - Un-pausing three-strikes or any other suppression that is not escalation terminality. - Reworking the verdict-marker grammar beyond what the supersession fold needs.

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-08-03 22:24:30 +02:00

dominik.polakovics referenced this issue from a commit

2026-08-03 23:30:23 +02:00

feat(autoland): re-arm an escalated PR — PR-scoped, supersedable terminality

dominik.polakovics referenced this issue from a pull request that will close it,

2026-08-03 23:30:50 +02:00

feat(autoland): re-arm an escalated PR — PR-scoped, supersedable terminality #272