[alekspetrov]

Summary

Automated PR created by Pilot for task GH-6327.

Closes #6327

Changes

GitHub Issue GH-6327: fix(autopilot): a PR that failed on a CI timeout with no check reported re-enters the pipeline on a new head or on green checks — purpose-built hold flag, capped redrive, kept across a restart

Context

Autopilot does not take back a PR that failed on a CI timeout when its checks go green later. On 2026-11-11 the GitHub Actions run of PR #6317 stayed queued for 13 hours (a dead run on the GitHub side; no check ever reported). The log shows the platform-outage breaker hold, one re-adoption, and then "CI timeout confirmed by same-tick poll … missing_checks=true" with the stage set to failed. Hours later the branch got a new head and a green CI run; the PR was mergeable and clean; autopilot did nothing, and the PR was merged by hand.

Cause, in internal/autopilot/controller.go: handleWaitingCI sets the failed stage at the confirmed timeout with an error text and a terminal label, and with no hold flag. ProcessPR does nothing for a failed stage. processOnePR keeps the PR tracked, but every redrive there needs a flag that this path never sets (redriveHeldPR, redriveSizeGuardHoldOnGreen, the breaker redrive, the rebase and API-outage redrives). RestoreState drops such a row at the next restart. The existing rule for a revival (memory autopilot-failed-stage-revival-is-a-flag-gated-redrive-before-processpr): a purpose-built boolean set only at the failure site, and a redrive keyed on it; never a match on the error text.

Scope

1. PRState in internal/autopilot/types.go: a boolean for "held after a CI timeout with no check reported", the head SHA at that moment, and a redrive counter. Persist and load them in internal/autopilot/state_store.go (column list, table rebuild list, insert, update and both scans; the columns of fix(autopilot): say it when the token cannot read CI — one PR comment, no pilot-failed label for this cause, and the github adapter reported unhealthy with the reason #6304 and fix(autopilot): a rejected or timed-out approval leaves a comment on the pull request — decision, who, reason, and how to continue #6305 show every place).
2. Set the boolean only at the confirmed-timeout site of handleWaitingCI and only when no check was discovered. Leave the stage failed and the error text as they are; do not set the terminal label on this path. A timeout with discovered checks that stay pending or fail is unchanged.
3. A new redrive, called in processOnePR before ProcessPR, in the shape of redriveHeldPR and redriveSizeGuardHoldOnGreen: when the boolean is set and the PR is open, re-enter the waiting-for-CI stage when the head SHA differs from the stored one, or when the checks of the same SHA are now all successful. On re-entry clear the boolean and the error, start a fresh CI wait clock, and count the redrive.
4. Cap: two redrives per PR (a constant next to maxBreakerReadoptAttempts). At the cap the PR stays failed and gets the terminal label as today.
5. One PR comment at the first hold: the CI run reported no check within the wait time; a push or "Update branch" starts a new run and Pilot continues. One comment on re-entry. Use the existing comment helpers; both texts go through the secret mask.
6. Add the boolean to the rows that RestoreState keeps tracked after a restart.
7. Tests: new head -> re-entry; same SHA turns green -> re-entry; cap reached -> stays failed with the terminal label; a merged or closed PR is ignored; a restart keeps the row and a later new head re-enters; a failure with discovered failing checks never sets the boolean and is never revived; a PR whose merge approval was rejected is untouched.

Acceptance

• go test -race -count=1 ./internal/autopilot/ -run 'CITimeout|Redrive|RestoreState' passes; paste the output into the PR body.
• Remove the call of the new redrive in processOnePR -> the new-head test fails.
• Drop the boolean from the rows RestoreState keeps -> the restart test fails.
• Do not re-run make build, make test or make lint as evidence.

Out of scope

The platform-outage breaker and its thresholds. A cancel or re-run of GitHub Actions runs by Pilot. Revival of any other failed PR.

Refs

• PR GH-6316: fix(review): post-merge minors from PR #6311 for issue #6305 #6317 · fix(autopilot): say it when the token cannot read CI — one PR comment, no pilot-failed label for this cause, and the github adapter reported unhealthy with the reason #6304 · fix(autopilot): a rejected or timed-out approval leaves a comment on the pull request — decision, who, reason, and how to continue #6305 · memory mem-307

Not verified

• go test -race -count=1 ./internal/autopilot/ -run 'CITimeout|Redrive|RestoreState' passes; paste the output into the PR body.
  Reason: Not verified: shell operators are not allowed in evidence commands

• Remove the call of the new redrive in processOnePR -> the new-head test fails.
  Reason: freeform mutation, run manually

• Drop the boolean from the rows RestoreState keeps -> the restart test fails.
  Reason: freeform mutation, run manually

Pre-PR review

Mode: enforce
Verdict: APPROVE-WITH-NOTES
Fix: none

Defects:

• [minor] at internal/autopilot/controller.go:3750 — A held PR has no TerminalLabel, so if a human closes it the issue falls back to pilot-retry-ready. The fix(autopilot): CI wait fires timeout blind on adopted PRs — confirm-poll before deadline, evidence-seeded clock, TerminalLabel on timeout #4851 comment at controller.go:3737 warned about exactly this. The issue asks for this behaviour ("do not set the terminal label on this path"), and the existing HoldActive holds work the same way.
• [minor] at Gate — Results
• [minor] at PR — Body
• [minor] at internal/autopilot/controller.go:3737 — The held path leaves TerminalLabel empty, so if a human closes a held PR the issue goes to pilot-retry-ready and Pilot dispatches it again. The fix(autopilot): CI wait fires timeout blind on adopted PRs — confirm-poll before deadline, evidence-seeded clock, TerminalLabel on timeout #4851 comment just above the change says "without a TerminalLabel here, a later external close of this stranded PR … would default to pilot-retry-ready and silently re-dispatch". At controller.go:11961 issueLabel := github.LabelRetryReady applies whenever prState.TerminalLabel is empty (11967). Scope item 2 asks for exactly this, so it is not blocking, but the old comment no longer describes the code and no test covers a close during a hold.
• [minor] at internal/autopilot/controller.go:3702 — missingChecks := len(prState.DiscoveredChecks) == 0 tracks the PR, not the head SHA. DiscoveredChecks is filled "only once, when first seen" (3639 len(prState.DiscoveredChecks) == 0) and the redrive at 9429-9435 never clears it. So if an earlier head reported checks and a later head times out with none, that later timeout counts as "checks discovered" and fails for good. This leans safe, but it reads "no check was discovered" per PR rather than per head.
• [minor] at internal/autopilot/controller.go:9418 — On the same SHA, every polling tick calls c.ciMonitor.CheckCI for each held PR with no backoff, for as long as the hold lasts (the incident hold lasted 13 hours). redriveSizeGuardHoldOnGreen (9330) does the same, but its revive is one-shot per SHA.

Not-verified:

• Lint failure cause: the gate output is not supplied, so I could not trace it to a file.
• The two mutation acceptance bullets were not run. Reading the tests, both would fail under the mutation: TestRedriveCITimeoutHeldPR_NewHeadReEntersPipeline requires StageWaitingCI after tickPR, and TestRestoreState_CITimeoutHeldPRStaysTrackedAndNewHeadReEnters requires GetPRState to find the PR. This comes from reading the code, not from running it.
• Scope 5: met. One hold comment is posted when CITimeoutRedriveCount == 0, and one comment on each re-entry. Both go through postCITimeoutComment, which applies secretpatterns.Redact. A test checks the mask through the real HTTP comment endpoint. — not traced
• … 3 more not-verified items; 4 requirements marked met but not traced to a file read omitted (capped at 3)

[alekspetrov]

Review — REQUEST-CHANGES (head 86eb711). Closed without merge; #6327 is amended (items 8 and 9) for a second run.

Blocker: a held PR with zero check runs is merged after a daemon restart (internal/autopilot/controller.go:9418-9425). After a restart the new CI monitor has everSeenCheckRuns=false. When the 60 s discovery wait ends, CheckCI falls back to the combined status, and an empty result maps to CISuccess (ci_monitor.go:673-696, 741-744). Reproduction with a throwaway test: hold PR 42 through the no-check timeout → new controller on the same store → RestoreState → tick, wait past the discovery wait, tick. The log shows trigger="green checks" on the held SHA with zero check runs, then waiting_ci → ci_passed → merging → merged.

Fix: the same-SHA redrive needs at least one discovered check run for that head SHA, all passed; plus a restart test with no checks.

Other points

• CI lint is red: gh6327_ci_timeout_redrive_test.go:87 setPR is unused.
• The hold has no backoff: one Warn line per wait cycle per held PR.
• The cap branch at controller.go:9407-9414 is not reachable in normal flow.
• No test that a CheckCI error is not treated as green.

What is correct: items 1, 2, 4, 5, 6, 7 of the contract. go test -race -count=1 ./internal/autopilot/... passes. A database with the schema of main (56 columns) opens under this code three times and ends at 59 columns with old rows intact; hold, SHA and count survive each reopen. The three key mutations fail their tests. No race found.

[alekspetrov]

This PR was closed without merging: closed without merging (no reason recorded)

CI run: https://github.com/qf-studio/pilot/commit/86eb7115dd177fafb44f7fac185af9ecf4db33e7/checks

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 89.15663% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/autopilot/controller.go	83.92%	5 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!