Repository navigation
feat(autofix): escalate stopped takeover PRs and age out unanswered pauses #8960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5fcabe2
8ea334c
cb0351d
5b6c285
31444cd
f64c176
3758c68
9c8d732
c4ce6e1
41b905d
2de7171
b575b67
bdc360a
175e224
fa2435f
78024d2
9555662
b055a8d
7c76edb
5f24cd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| # Takeover fleet visibility and cap-hit escalation | ||
|
|
||
| ## Problem statement | ||
|
|
||
| As of 2026-08-11, 35 open PRs carry `autofix/takeover`. Two structural gaps: | ||
|
|
||
| 1. **The takeover pool is invisible.** The Fleet Shepherd | ||
| (`qwen-fleet-shepherd.yml`) enumerates only bot-authored PRs (3 today). | ||
| The 35 human-authored takeover PRs appear on no dashboard; their state | ||
| (working / paused / conflicting / idle-for-days) is knowable only by | ||
| opening each PR. | ||
|
|
||
| 2. **Cap-hit PRs die silently.** When a takeover PR reaches its round cap | ||
| (100/100), or a circuit breaker (consecutive-failure, time-budget) stops | ||
| it, the loop posts one comment and goes quiet. Five PRs have been paused | ||
| since 2026-08-06 with no re-arm: #8213, #8396, #8416, #8439, #8443. | ||
| Nothing escalates them — no label, no dashboard entry, no auto-release — | ||
| so they hold the takeover label forever ("zombie takeover"). | ||
|
|
||
| ## Proposed changes | ||
|
|
||
| ### A. `autofix/needs-human` label (qwen-autofix.yml) | ||
|
|
||
| A new maintainer-facing label meaning: _the loop has stopped on this PR; a | ||
| human must act (re-arm, split, merge, or close)_. | ||
|
|
||
| **Applied** in the review scan's cap-notice path (the single funnel every | ||
| terminal state passes through: round cap, consecutive-failure cap, and | ||
| time-budget cap all write a terminal `autofix-eval` marker with | ||
| `round=EFF_MAX_ROUNDS`, which the next scan sees as `ROUND >= EFF_MAX_ROUNDS` | ||
| and lands in the cap-notice branch). The label write is placed so it runs | ||
| even when the once-per-window notice comment is dedup'd — this backfills the | ||
| label onto the already-paused fleet via the regular scan rotation after | ||
| deploy (idle backoff defers PRs idle >24h to ~1 scan in 4 — expect hours, | ||
| not the first scan). | ||
|
|
||
| **Removed** wherever management resumes or a human takes over: | ||
|
|
||
| | Path | Site | | ||
| | ------------------------------------------------------ | -------------------- | | ||
| | `/takeover` re-arm on a managed PR | takeover-command job | | ||
| | `/takeover` fresh engage | takeover-command job | | ||
| | `/takeover stop` | takeover-command job | | ||
| | Manual label engage / release acks | takeover-ack job | | ||
| | `/retry` re-arm marker | retry-command job | | ||
| | Scan first-pickup engage ack (direct-label engagement) | review-scan job | | ||
|
|
||
| Removal is best-effort with a warning on failure, mirroring the existing | ||
| `TAKEOVER_LABEL` DELETE pattern (404 tolerated). A stale `needs-human` left | ||
| behind by a failed removal is cosmetically wrong but harmless; the next | ||
| cap-stop reapplies it anyway. | ||
|
|
||
| A PR closed or merged while paused keeps `needs-human` — deliberately. No | ||
| closure removal path exists (the route drops commands on non-open PRs, every | ||
| enumeration is `--state open`, and there is no `pull_request: closed` | ||
| trigger), and the residue is inert: all consumers filter on open state, so | ||
| the label only marks the resolved escalation in the closed PR's own history. | ||
| All-state label queries should pair the label with a state filter. | ||
|
|
||
| Label creation follows the existing convention: `gh label create` (idempotent, | ||
| fixed color) before the first REST add, so a missing label never gets a random | ||
| color. | ||
|
|
||
| ### B. Shepherd covers the takeover pool (qwen-fleet-shepherd.yml) | ||
|
|
||
| A second enumeration — open PRs with `autofix/takeover`, including forks — | ||
| drives a **second dashboard table** in the same edited-in-place issue: | ||
|
|
||
| | PR | Author | Updated | State | Note | | ||
| | --- | ------ | ------- | ----- | ---- | | ||
|
|
||
| State comes from the list payload (conflicting / ci red / checks in flight / | ||
| idle). PRs carrying `autofix/needs-human` get a `🛑 needs-human` state; for | ||
| those few PRs the shepherd additionally reads the comment stream (fail-closed) | ||
| to recover the terminal timestamp (latest `<!-- takeover-cap-reached -->` | ||
| notice) and the stop reason (first line of the latest terminal "AutoFix | ||
| stopped" headline, else "round cap reached"). | ||
|
|
||
| **NON-GOAL:** the existing levers (conflict dispatch, stale-base sync) stay | ||
| scoped to the bot fleet. Takeover-PR conflicts are already the autofix scan's | ||
| job (`HAS_CONFLICT` selects them as targets), and `update-branch` on | ||
| contributor branches is out of scope for this change. | ||
|
|
||
| ### C. Auto-release lever (qwen-fleet-shepherd.yml) | ||
|
|
||
| When a PR carries **both** `autofix/takeover` and `autofix/needs-human` and | ||
| its terminal timestamp is older than `AUTO_RELEASE_DAYS` (default 3, tunable | ||
| via the `QWEN_SHEPHERD_AUTO_RELEASE_DAYS` repo variable): | ||
|
|
||
| 1. Post one bilingual summary — dedup'd by its | ||
| `<!-- fleet-shepherd auto-release -->` marker (scoped to the current | ||
| pause cycle): why it was released, the stop reason, and the human's | ||
| options (merge / close / split + re-takeover). | ||
| 2. Remove `autofix/takeover` (the loop disengages). A failed removal finds | ||
| the marker and retries only the DELETE; a failed summary leaves both | ||
| labels in place so the whole release retries next tick. | ||
| 3. Keep `autofix/needs-human`: the PR still needs a human decision, and the | ||
| label remains the filterable TODO list. It clears on re-engage/re-arm via | ||
| the paths in (A). | ||
|
|
||
| Idempotency needs no marker comment: the lever's scope condition (both labels) | ||
| is false after the release, so it cannot re-fire. Per-tick cap | ||
| (`MAX_RELEASES_PER_TICK`, default 3) bounds blast radius; `live_skip` is | ||
| re-checked immediately before the mutation, mirroring every existing lever. | ||
|
|
||
| ## Key design decisions | ||
|
|
||
| - **Label write lives in the scan, not the address leg.** Every terminal stop | ||
| converges on `round=EFF_MAX_ROUNDS` markers, which the scan's cap branch | ||
| already observes with comments loaded and PAT identity verified. One hook | ||
| point covers all stop reasons, including future ones. | ||
| - **Pause reason comes from the terminal marker headline**, because the | ||
| scan-side notice always says "round cap (N/N)" even when a breaker fired | ||
| (observed on #8443: both comments present). | ||
| - **Bootstrap without a backfill job:** the label write runs even when the | ||
| notice comment is dedup'd, so currently-paused PRs are labeled by the | ||
| regular scan rotation after deploy — note the scan's idle backoff defers | ||
| PRs idle >24h (exactly the paused population) to ~1 scan in 4, so expect | ||
| the backfill within a few hours (median ~2h, p90 ~6h), not minutes. | ||
| - **Auto-release keyed on the notice timestamp**, not label age: labels carry | ||
| no timestamps, and the notice is written by the same identity-verified path | ||
| that applies the label. Resume evidence newer than the notice vetoes the | ||
| release — the bot's re-arm/engage markers, a re-arm command comment, or a | ||
| fresh `labeled` event. Command comments count only while FRESH | ||
| (`RESUME_COMMAND_GRACE_SEC`, 2h) and UNSUPERSEDED by a refusal ack | ||
| (`fork-refused` / `base-refused` / `skip-blocked`): an accepted command is | ||
| acked within minutes; an ignored one (no route permission) simply expires; | ||
| and no permission check is mirrored into the shepherd — the route's | ||
| collaborator check is the authorization gate, and a mirrored copy would | ||
| only drift. | ||
| - **The release lever gets its own enumeration** of the paused population | ||
| (needs-human ∩ takeover, stalest-first) — not the takeover display window | ||
| and not the needs-human display window: released PRs keep `needs-human` | ||
| and age back into that display window, so feeding the lever from it would | ||
| truncate exactly the fresh pauses that become release-eligible. All three | ||
| enumerations cap at 100 with loud saturation warnings; a display | ||
| enumeration failure degrades to an error row, and a paused-enumeration | ||
| failure skips the lever for that tick — the dashboard write (which | ||
| carries the liveness watermark) always runs. | ||
| - **The summary posts before the label removal**, dedup'd by its own marker | ||
| scoped to the current pause cycle (only markers newer than the latest cap | ||
| notice count), so a failed comment leaves both labels in place and the | ||
| whole release retries next tick; a failed removal finds the marker and | ||
| retries only the DELETE; and a re-armed-and-re-capped PR still gets its | ||
| second summary. | ||
| - **Stale-label heal:** a fork PR released by hand gets no release ack (the | ||
| route suppresses fork `unlabeled` events), so nothing else clears its | ||
| `needs-human`. The shepherd watches the awaiting-human pool for a | ||
| human-actor `unlabeled` event on the takeover label that is NEWER than the | ||
| latest label-apply (a stale unlabel from an earlier takeover cycle must | ||
| never heal this cycle's label), and clears the stale label — bounded per | ||
| tick, skip-vetoed, and never triggered by the bot's own auto-release. | ||
| - **Shepherd timing:** 15-minute tick with a per-tick release cap — a backlog | ||
| of expired PRs drains over a few ticks rather than one burst. | ||
|
|
||
| ## Files affected | ||
|
|
||
| - `.github/workflows/qwen-autofix.yml` — env, cap-notice branch, six | ||
| label-removal sites. | ||
| - `.github/workflows/qwen-fleet-shepherd.yml` — env, takeover enumeration, | ||
| dashboard takeover table plus a read-only "Awaiting human" section | ||
| (released PRs keep `needs-human` and would otherwise vanish from every | ||
| surface), auto-release lever. | ||
|
|
||
| ## Scope boundaries | ||
|
|
||
| - No changes to round caps, breakers, or review-bot behavior. | ||
| - No shepherd levers on takeover PRs other than auto-release. | ||
| - No notification/@-mention of maintainers (comment + label + dashboard only). | ||
| - `autofix/needs-human` on plain (non-takeover) bot PRs is applied by the same | ||
| scan path and shown on the dashboard, but the auto-release lever never | ||
| touches them (they have no takeover label to release). | ||
|
|
||
| ## Open questions | ||
|
|
||
| - Default `AUTO_RELEASE_DAYS=3` — short enough to keep the pool clean, long | ||
| enough for a maintainer to re-arm over a weekend? Adjustable without a | ||
| deploy via the repo variable. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,10 @@ describe('pr-self-report-label', () => { | |
| }); | ||
| expect(race.removed).toBe(true); | ||
| expect(race.out).not.toContain('::warning::'); | ||
| // R7-5: a 404 race means the end state holds → the success claim fires, | ||
| // the "did not land" claim must not. | ||
| expect(race.out).toContain('removed'); | ||
| expect(race.out).not.toContain('removal did not land'); | ||
| // Any other DELETE failure keeps the step green (the job only re-runs | ||
| // on the next PR event) but MUST surface — a silent run would claim | ||
| // "removed" while the label stays on the PR. | ||
|
|
@@ -200,6 +204,26 @@ describe('pr-self-report-label', () => { | |
| expect(failed.out).toContain('::warning::'); | ||
| expect(failed.out).toContain('removal failed'); | ||
| expect(failed.out).toContain('HTTP 500'); | ||
| // R7-5: the LBL_DEL_FAILED branching — a failed DELETE must NOT claim | ||
| // the label was removed (the lying-log shape this change exists to kill). | ||
| expect(failed.out).toContain('removal did not land'); | ||
| expect(failed.out).not.toContain('removed'); | ||
| // R10-1/R10-3: the tolerated token is exactly "HTTP 404" — a transport | ||
| // failure embedding a 404-bearing request URL must warn and report | ||
| // not-landed, never read as the already-off case (the loose *404* | ||
| // match hit the URL's PR number and claimed "removed"). | ||
| const transport = run({ | ||
| prAuthor: 'alice', | ||
| issueAuthors: 'bob', | ||
|
Comment on lines
+215
to
+217
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferred without code change — the exact-token HTTP 404 tolerance is behaviorally pinned at only one of six needs-human removal sites (pr-self-report suite). The workflow's non-convergence brake engaged (diff over the window's growth budget for 2+ rounds, still not shrinking), so this round is a maintainer handoff and all code fixes are suspended pending the landing decision. Whether this lands in-PR or as a follow-up issue is the maintainer's call; thread left open. 中文说明推迟处理、本轮未做代码改动 —— 精确 token HTTP 404 容忍在六个 needs-human 移除点中只有一处有行为引脚(pr-self-report 套件)。工作流的非收敛刹车已触发(diff 连续 2 轮以上超出窗口增长预算且仍在增长),本轮为移交维护者的交接轮,一切代码修复在落地决策前暂停。在 PR 内落地还是作为后续 issue 处理由维护者决定;线程保持开放。 |
||
| hasLabel: true, | ||
| deleteFails: | ||
| 'gh: Delete "https://api.github.com/repos/o/r/issues/4041/labels/review%2Fself-reported": dial tcp 140.82.121.4:443: connect: connection refused', | ||
| }); | ||
| expect(transport.removed).toBe(true); | ||
| expect(transport.out).toContain('::warning::'); | ||
| expect(transport.out).toContain('removal failed'); | ||
| expect(transport.out).toContain('removal did not land'); | ||
| expect(transport.out).not.toContain('removed'); | ||
| // POST carries NO tolerance: a label that fails to apply must fail the | ||
| // step (the runBlock sets -e), not leave the PR unlabeled behind green. | ||
| expect(() => | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R7-5 已修(fa2435f17e):pr-self-report-label.test.js 的失败策略用例补了 LBL_DEL_FAILED 分支断言——HTTP 500 时断言
removal did not land且不出现removed;404 竞态与成功用例做反向断言。