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

Merged

dominik.polakovics merged 2 commits from afk/188 into main

2026-08-03 23:48:45 +02:00

dominik.polakovics commented

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

Owner

Closes #188

An outcome='escalated' run made its PR invisible to the autoland poller forever, and the gate keyed on (repo_id, branch). Since afk/<N> derives from the issue number, a requeued issue reused its branch and the brand-new PR was invisible from birth — a PR never validated, let alone escalated. Keying on the PR number alone would not have been the fix either: the human-fixes-it and manual-description cases are the same PR. All three failed silently — the poller continued at the gate with no log line and nothing in the SPA.

The reframing this implements: escalation is a statement about a moment ("as of these N attempts, agents could not finish this"), not a permanent property of a thing. A human commit or a fresh set of instructions is new information that invalidates the statement.

What landed

Re-arm — new autoland_rearms record (migration 0022). store.RearmPull writes the supersession instant and zeroes the PR's fix/escalate budgets in one transaction. Both halves together, because clearing terminality while leaving the budget spent would escalate again on the first rejection — the worst outcome, since it reads as the re-arm having silently failed.
The gate is relational — terminal iff an escalation signal exists after the last re-arm: the escalated run's ended_at, or the escalate marker comment's CreatedAt. Escalated rows stay history, never deleted or rewritten — supersession, not erasure. A fresh escalation after a re-arm is terminal again, indefinitely repeatable.
PR-scoped, not branch-scoped — new runs.pull_number; autoland_attempts re-keys to (repo_id, pull_number, kind). A new PR on a reused branch starts virgin with no re-arm needed.
DropSupersededEscalations is the poller's one supersession mechanism, feeding the words fold, the marker half of Escalated, and the fix run's work order. Filtering the comments rather than just the words is what makes it whole — RejectionContext re-derives the words internally, so a word-only filter would spawn a fix run with an empty work order. There is a test that fails exactly that way if the filter is dropped from that one call site.
The suppression is logged (repo, pull, branch, source) — worth it on its own merits, per the issue.
Surfaces — one human-authenticated route POST /api/v1/repos/{id}/autoland/pulls/{pull}/rearm, consumed by a History-page SPA action (which also states the consequence, not just the escalated chip) and by lab autoland rearm.

DecideAutoland gains no new states or fields — only the derivation of its Escalated input changed.

The security boundary

No labctl verb exists, deliberately. Escalation means "agents could not finish this"; an agent able to lift its own terminal hand-off would make the bound decorative. internal/agentapi, internal/labctl, and cmd/labctl are untouched by this PR (git diff over them is empty) — the absence there is the feature. The operator verb lives on cmd/lab with a PAT.

Review notes — two defects caught during review, worth knowing about

lab autoland rearm -h leaked the operator PAT. -token defaults to LAB_PAT, and Go's flag package prints default values in its usage — so -h under a normal operator environment printed the token to stderr, into scrollback and CI logs. Fixed by overriding fs.Usage; TestRunAutolandRearmUsageNeverPrintsTheToken covers all six paths that can reach a usage block.
pull_number was omitempty, contradicting runResponse's own pinned "every key, always" rule two lines above it. Changed to a bare nullable key matching issue_number, and the SPA type is number | null accordingly.

Verification

Go: go build ./..., go vet ./..., gofmt -l clean, go test ./... -count=1 green (47 packages). Migration 0022 proven Up → Down → Up against a real sqlite database with DML exercising the PK, the CHECK, and both FK cascades.

Not verified: the SPA. No JavaScript runtime exists in this worktree — no node, npm, bun, deno, nix, or container runtime — so npm run lint, npm run format:check, npm test, and npm run build (which is where tsc --noEmit runs) could not be executed. The web/ changes were reviewed by hand instead: every typed Run/Instance fixture updated (Instance extends Run, so all of them are required), ErrorBanner's default export and prop shape checked against its definition, createLiveResource's [resource, { refetch }] return checked against its overloads, all imports confirmed used, added lines confirmed within the 100-column printWidth, and the new route test matched against RepoCRs.test.tsx's established mount convention. That is review, not proof — please let CI run the web/ job before merging.

Docs

ADR-0048 gains an Amended by issue #188 paragraph superseding the "permanent-terminality gate" and "re-entry ... is a manual, human decision" claims. CONTEXT.md's Escalation entry names re-arm as the second re-entry path, _Avoid_ line untouched.

Closes #188 An `outcome='escalated'` run made its PR invisible to the autoland poller forever, and the gate keyed on `(repo_id, branch)`. Since `afk/<N>` derives from the **issue** number, a requeued issue reused its branch and the brand-new PR was invisible **from birth** — a PR never validated, let alone escalated. Keying on the PR number alone would not have been the fix either: the human-fixes-it and manual-description cases are the *same PR*. All three failed silently — the poller `continue`d at the gate with no log line and nothing in the SPA. The reframing this implements: escalation is a statement about a **moment** ("as of these N attempts, agents could not finish this"), not a permanent property of a thing. A human commit or a fresh set of instructions is new information that invalidates the statement. ## What landed - **Re-arm** — new `autoland_rearms` record (migration 0022). `store.RearmPull` writes the supersession instant **and** zeroes the PR's `fix`/`escalate` budgets in one transaction. Both halves together, because clearing terminality while leaving the budget spent would escalate again on the first rejection — the worst outcome, since it reads as the re-arm having silently failed. - **The gate is relational** — terminal iff an escalation signal exists *after* the last re-arm: the escalated run's `ended_at`, or the escalate marker comment's `CreatedAt`. Escalated rows stay history, never deleted or rewritten — supersession, not erasure. A fresh escalation after a re-arm is terminal again, indefinitely repeatable. - **PR-scoped, not branch-scoped** — new `runs.pull_number`; `autoland_attempts` re-keys to `(repo_id, pull_number, kind)`. A new PR on a reused branch starts virgin with no re-arm needed. - **`DropSupersededEscalations`** is the poller's *one* supersession mechanism, feeding the words fold, the marker half of `Escalated`, and the fix run's work order. Filtering the **comments** rather than just the words is what makes it whole — `RejectionContext` re-derives the words internally, so a word-only filter would spawn a fix run with an **empty** work order. There is a test that fails exactly that way if the filter is dropped from that one call site. - **The suppression is logged** (repo, pull, branch, source) — worth it on its own merits, per the issue. - **Surfaces** — one human-authenticated route `POST /api/v1/repos/{id}/autoland/pulls/{pull}/rearm`, consumed by a History-page SPA action (which also states the *consequence*, not just the `escalated` chip) and by `lab autoland rearm`. `DecideAutoland` gains no new states or fields — only the derivation of its `Escalated` input changed. ## The security boundary **No `labctl` verb exists, deliberately.** Escalation means "agents could not finish this"; an agent able to lift its own terminal hand-off would make the bound decorative. `internal/agentapi`, `internal/labctl`, and `cmd/labctl` are untouched by this PR (`git diff` over them is empty) — the absence there *is* the feature. The operator verb lives on `cmd/lab` with a PAT. ## Review notes — two defects caught during review, worth knowing about 1. **`lab autoland rearm -h` leaked the operator PAT.** `-token` defaults to `LAB_PAT`, and Go's `flag` package prints default *values* in its usage — so `-h` under a normal operator environment printed the token to stderr, into scrollback and CI logs. Fixed by overriding `fs.Usage`; `TestRunAutolandRearmUsageNeverPrintsTheToken` covers all six paths that can reach a usage block. 2. **`pull_number` was `omitempty`**, contradicting `runResponse`'s own pinned "every key, always" rule two lines above it. Changed to a bare nullable key matching `issue_number`, and the SPA type is `number | null` accordingly. ## Verification Go: `go build ./...`, `go vet ./...`, `gofmt -l` clean, `go test ./... -count=1` green (47 packages). Migration 0022 proven Up → Down → Up against a real sqlite database with DML exercising the PK, the CHECK, and both FK cascades. **Not verified: the SPA.** No JavaScript runtime exists in this worktree — no node, npm, bun, deno, nix, or container runtime — so `npm run lint`, `npm run format:check`, `npm test`, and `npm run build` (which is where `tsc --noEmit` runs) could **not** be executed. The `web/` changes were reviewed by hand instead: every typed `Run`/`Instance` fixture updated (`Instance extends Run`, so all of them are required), `ErrorBanner`'s default export and prop shape checked against its definition, `createLiveResource`'s `[resource, { refetch }]` return checked against its overloads, all imports confirmed used, added lines confirmed within the 100-column `printWidth`, and the new route test matched against `RepoCRs.test.tsx`'s established mount convention. That is review, not proof — **please let CI run the `web/` job before merging.** ## Docs ADR-0048 gains an `Amended by issue #188` paragraph superseding the "permanent-terminality gate" and "re-entry ... is a manual, human decision" claims. CONTEXT.md's **Escalation** entry names re-arm as the second re-entry path, `_Avoid_` line untouched.

dominik.polakovics added 1 commit

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

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

ci / native (pull_request) Failing after 1m6s

Details

089e5cff4e

An `outcome='escalated'` run made its PR invisible to the autoland poller
forever, and the gate keyed on `(repo_id, branch)`. Since `afk/<N>` derives
from the ISSUE number, a requeued issue reused its branch and the brand-new
PR was invisible from birth — never validated, let alone escalated. The two
other ways back in (a human fixes it and wants re-validation; a human hands
an agent a manual description) are the same PR, so keying on the PR number
alone would not have fixed them either. All three failed silently: the
poller `continue`d at the gate with no log line and nothing in the SPA.

Escalation is modelled as a statement about a MOMENT — "as of these N
attempts, agents could not finish this" — rather than a permanent property
of a thing. A human commit or a fresh set of instructions is new information
that invalidates the statement, and re-arm is the gesture that says so.

- **Re-arm** (`autoland_rearms`, migration 0022) records the supersession
  instant and zeroes the PR's `fix`/`escalate` budgets in ONE transaction.
  Both halves together: clearing terminality while leaving the budget spent
  would escalate again on the first rejection — the worst outcome, because
  it reads as the re-arm having silently failed.
- **The gate is relational**: terminal iff an escalation signal exists AFTER
  the last re-arm — the escalated run's `ended_at`, or the escalate marker
  comment's `CreatedAt`. Escalated rows stay history, never deleted or
  rewritten: supersession, not erasure. A fresh escalation after a re-arm is
  terminal again, indefinitely repeatable.
- **PR-scoped, not branch-scoped**: new `runs.pull_number`, and
  `autoland_attempts` re-keys to `(repo_id, pull_number, kind)`. A new PR on
  a reused branch starts virgin with no re-arm needed.
- `DropSupersededEscalations` is the poller's ONE supersession mechanism,
  feeding the words fold, the marker half of `Escalated`, and the fix run's
  work order. Filtering the comments rather than just the words is what
  makes it whole: `RejectionContext` re-derives the words itself, so a
  word-only filter would spawn a fix run with an EMPTY work order.
- **The suppression is logged** (repo, pull, branch, source). Worth doing on
  its own: it turns an undiagnosable "autoland just ignores this PR" into a
  five-second answer.
- **Surfaces**: one human-authenticated route, consumed by a History-page
  SPA action that also states the consequence, and `lab autoland rearm`.
  Deliberately NO `labctl` verb — an agent able to lift its own terminal
  hand-off would make the bound decorative. `internal/agentapi` and
  `internal/labctl` are untouched; the absence there is the feature.

`DecideAutoland` gains no states — only the derivation of its `Escalated`
input changed. ADR-0048 carries the amendment; CONTEXT.md names re-arm as
the second re-entry path.

Closes #188

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

dominik.polakovics added 1 commit

2026-08-03 23:39:38 +02:00

style(web): satisfy prettier in History.test.tsx

ci / native (pull_request) Successful in 7m41s

Details

32e96fb1bf

The repos-stub return collapses to exactly 100 columns, which fits
printWidth, so `prettier --check` rejected the wrapped form and the
`ci / native` SPA step failed before reaching any other gate.

Formatting only — no behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented

2026-08-03 23:48:40 +02:00

Author

Owner

[autoland] verdict: pass

Validated against the validation core. CI ci / native green on head 32e96fb (run 376, 7m41s), plus the full gate re-run locally: eslint, prettier --check, vitest 990/990, tsc --noEmit + vite build, go build/test -tags ui, golangci-lint 2.12.2 (0 issues), and the untagged go build/vet/gofmt/test suite (the nix gate is path-gated off for this diff). Title is Conventional Commits; body carries Closes #188 matching head afk/188; 1 ahead / 0 behind main, no conflict. Diff scope matches the issue brief, including the security-relevant criterion: no labctl (run-token) rearm verb exists — the internal/agentapi change is comment-only, correcting a stale description of permanent terminality. Lander pushed one formatting-only commit (32e96fb) to fix the prettier violation in web/src/routes/History.test.tsx:93 that was the sole CI failure.

[autoland] verdict: pass Validated against the validation core. CI `ci / native` green on head 32e96fb (run 376, 7m41s), plus the full gate re-run locally: eslint, prettier --check, vitest 990/990, tsc --noEmit + vite build, go build/test -tags ui, golangci-lint 2.12.2 (0 issues), and the untagged go build/vet/gofmt/test suite (the nix gate is path-gated off for this diff). Title is Conventional Commits; body carries Closes #188 matching head afk/188; 1 ahead / 0 behind main, no conflict. Diff scope matches the issue brief, including the security-relevant criterion: no labctl (run-token) rearm verb exists — the internal/agentapi change is comment-only, correcting a stale description of permanent terminality. Lander pushed one formatting-only commit (32e96fb) to fix the prettier violation in web/src/routes/History.test.tsx:93 that was the sole CI failure.

dominik.polakovics merged commit 3f302362a3 into main

2026-08-03 23:48:45 +02:00

dominik.polakovics referenced this pull request from a commit

2026-08-03 23:48:46 +02:00

Merge pull request 'feat(autoland): re-arm an escalated PR — PR-scoped, supersedable terminality' (#272) from afk/188 into main