Skip to content

A PR whose deploy succeeds is still evicted, because the heal path holds prod-deploy past the queue timeout #3091

Closed
3 of 3 issues completed

Description

@devantler

🤖 Generated by the Agentic Engineer

One failed prod deploy deterministically costs the next queued PR its slot, even when that next
PR's own deploy succeeds completely. This is a separate mechanism from #3089: there the deploy
failed, here it passed and the PR was evicted anyway.

Evidence

Measured on the 2026-08-11 14:00–15:40Z queue window.

#3085 failed its evidence gate and was evicted — that part is #3089. What matters here is what its
failure did to the PR behind it:

When What
14:19:44Z #3084 added to merge queue
14:20:29Z #3085 deploy fails (evidence gate)
14:20:35Z #3085 🩹 Heal Prod starts, taking the prod-deploy lock
14:21:04Z #3084 merge group builds
14:59:11Z #3085 heal finishes — 38m36s holding the lock
14:59:16Z #3084 deploy finally starts, 5s after the lock frees
15:23:45Z #3084 evicted by github-merge-queue[bot] — 64 min after enqueue
15:40:32Z #3084 deploy succeeds — 17 min after it was already evicted

Run 31500671569 (#3085) and
31501050429 (#3084).
#3084 merge_group conclusion: success. PR state: still open, never merged.

Why it happens

Three settings interact, each individually reasonable:

  • The Require merge queue ruleset sets check_response_timeout_minutes: 60.
  • deploy-prod and heal-prod-on-failure share concurrency: group: prod-deploy with
    cancel-in-progress: false and queue: max — deliberately, so a deploy is never dropped or run
    concurrently against prod.
  • A prod deploy takes 36–64 min, and the heal path takes a comparable ~38 min.

So after any failed deploy the next entry's wall-clock is heal (~38m) + deploy (~41m) ≈ 79m, which
exceeds the 60-minute timeout before its own deploy can finish. The queue gives up on a PR whose
checks were going to pass. Because the evicted PR must then be re-enqueued and re-run a full deploy,
one flaky deploy can keep displacing the queue for hours.

max_entries_to_build: 1 does not help: the entry is built promptly, it is the job that is stalled
behind the lock, and the queue timer runs from enqueue regardless.

Impact

  • A correct, fully green PR is silently dropped from the queue and has to be noticed and re-enqueued
    by hand.
  • Each occurrence burns a complete prod deploy cycle (~41 min of deploy against the real cluster)
    that is thrown away — the deploy ran, succeeded, and merged nothing.
  • Reading the run afterwards is actively misleading: the merge_group run says success, so the
    eviction looks unexplained unless the timeline is checked.

Expected behaviour

A PR whose merge-group checks all succeed is merged, not evicted, even when it waited on the
prod-deploy lock behind an unrelated heal or deploy.

Acceptance criteria

  • The merge queue's check_response_timeout_minutes exceeds the realistic worst case of
    heal + deploy (measured ~79 min; suggest 150 for headroom), or the deploy is taken off
    the critical path so the queue timer is not gated on a lock the entry does not hold.
  • A merge_group run that finishes successfully after its PR was already evicted is surfaced
    rather than silently discarded — the eviction reason is visible without reconstructing the
    timeline by hand.
  • A failed deploy's heal path no longer starves the following entry for its whole duration, or
    the queue accounts for that wait.

Note the first criterion is a repository ruleset change, not a code change, so it needs the
maintainer to apply it.

Related: #3089 (the transient that caused this window's first failure), #3065, #2957.

Activity

  1. added theissue type on Aug 11, 2026
  2. moved this to 📥 Backlog in 🌊 Project Boardon Aug 11, 2026
  3. devantler commented on Aug 11, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Reproduced live ~90 minutes after the window above, and the second run surfaced a worse consequence
    than the delivery cost this issue was filed for.

    Second occurrence

    When What
    16:01:48Z #3086 enqueued (fully green, Codex review clean at head)
    16:02:28Z #3084 enqueued
    ~16:08Z #3090's deploy fails
    16:09:10Z #3090 evicted
    16:09:15Z #3086's merge group builds — 5s later
    16:09:xxZ #3090's 🩹 Heal Prod starts, taking prod-deploy for ~38 min

    Same shape as the first: the heal from a failed deploy starts moments before the next group builds,
    so the next entry's deploy cannot start until ~16:47, needs ~41 min, and finishes ~17:28 — past its
    ~17:09 timeout. Eviction was arithmetically certain before the deploy had run a single step.

    I dequeued #3086 and #3084 rather than let them proceed, because of the following.

    The more serious consequence: a successful deploy that is evicted leaves prod ahead of main

    heal-prod-on-failure runs only when the deploy fails. Its own comment states the reason it
    exists:

    The deploy above runs on the SPECULATIVE merge ref, before the PR lands on main. When it fails
    the merge group is ejected — but the OCI latest tag (and the prod cluster reconciling it) is
    left pointing at that ejected [ref]

    The timeout eviction produces the same end state through the opposite path. In the first window
    #3084's deploy succeeded at 15:40:32Z, so heal was skipped — correctly, by its own
    condition — but the PR had already been evicted at 15:23:45Z and never merged. So prod was deployed
    from a speculative ref that is not on main, and nothing restored it.

    That is the case the heal path does not cover: it guards deploy failed → ejected, and this is
    deploy succeeded → ejected anyway. The drift is self-correcting on the next successful deploy from
    main, but until then prod runs code that was never merged, and no signal is emitted.

    Adjacent to but distinct from #2828, which is about heal overwriting a newer successful deploy.

    Additional acceptance criterion

    • A merge group that is evicted while its deploy is running, or after it has succeeded, restores
      prod to main the same way a failed deploy does — the heal condition keys on "this group was
      ejected", not on "the deploy failed".
  4. devantler commented on Sep 6, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    The existing ejected-group recovery criterion also reproduced for manual dequeue on 2026-09-06. PR #3627 was temporarily removed to complete a review-contract step; this exposed an obsolete deployment starting after removal.

    UTC Observed event
    14:41:29 PR #3627 deliberately dequeued by devantler; complete GraphQL timeline and null queue entry confirmed removal.
    14:42:15 Deploy to Prod started, 46 seconds after removal.
    14:45:09 Explicit run cancellation stopped the deployment.
    14:45:51–14:54:57 Restore-main job ran and succeeded.

    Exact binding: merge-group commit 4ace8380a2bc5b2718e66124baf140e19c0648cc, PR head fe4eba0702d3b61e8663d6eabf387a1738860e1f, base 19642fe908e2503e2792af0dcc2bfb3a21c2c0e1. The merge-group tree matches the inspected source. At that revision, deploy-prod checks the original event and changed paths, while the production composite has no live queue-membership guard. Recovery handles failure/cancellation and explicitly excludes successful deployment.

    This occurrence proves an obsolete deployment start, not successful publication of an unmerged artifact: cancellation intervened, and no observed production revision established that the speculative artifact reached the cluster. After recovery, the source and both infrastructure Kustomizations were Ready and agreed on the restored artifact revision.

    Suggested refinements to the existing acceptance criterion:

    • A group removed before acquiring the production lane cannot begin production mutations or publish its speculative artifact. Bind membership evidence to the exact run/revision/group and fail closed if that evidence is unavailable or incomplete.
    • Removal during or after a successful deploy must not strand an unmerged artifact. Recovery must respect the shared production lock, a later legitimate deployment, and a revision that has already landed.
    • Cover removal before deploy, removal during deploy, successful-but-removed groups, still-current groups, already-landed revisions, and unavailable membership evidence in regressions; retain existing failed/cancelled recovery coverage.

    These extend the existing acceptance comment. The intentional pre-merge production validation remains the delivery model. This is separate from #3097's timeout-specific trigger, and no duplicate issue is needed.

  5. devantler commented on Sep 6, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    The current CI workflow exposes a prerequisite gap relevant to this issue: deploy-prod.needs omits validate-matcher-efficacy, although ci-required-checks includes both jobs. Recovery is conditioned on deployment failure or cancellation.

    A required sibling check can therefore fail without preventing deployment from beginning. Include this case in the recovery regression coverage and require applicable validation prerequisites before deployment. Preserve the shared production lock and protection for newer legitimate or already-landed revisions. The successful-but-ejected acceptance criterion remains open.

  6. added a commit that references this issue on Sep 25, 2026
  7. devantler commented on Sep 27, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Re-checked against main on 2026-09-27, before starting work. Two of the three criteria are now met by later changes. The one left is a ruleset setting.

    Criterion Status now
    A merge group that succeeds after its PR was evicted is surfaced, not silently dropped Met. The 🧾 Check the deployed PR is still queued job (merge-group-queue-membership) runs after a successful deploy. When the PR is no longer queued, heal-prod-on-failure now runs on that success path too, so prod is not left ahead of main.
    The 2026-09-06 gap: deploy-prod did not wait for validate-matcher-efficacy Met. deploy-prod.needs now lists every merge-group validator that ci-required-checks enforces, and scripts/guard-deploy-waits-for-merge-group-validators.sh keeps it that way (#4148).
    The queue timeout covers the worst case of heal plus deploy Open. The main ruleset still has check_response_timeout_minutes: 60, while heal plus deploy measured about 79 minutes.

    The last criterion needs a change to a repository ruleset. Agents cannot make that change, so it is the maintainer's to apply: raise check_response_timeout_minutes on the platform main merge-queue ruleset to about 150. The other way to meet it is to stop a heal from holding the lock the next entry waits on, which is what platform#4192 (held heal) moves toward. The timeout itself is tracked on its own in #3097, which has the later measurements (including #3922 on 2026-09-20).

    Blocker: authority — the merge-queue ruleset timeout (check_response_timeout_minutes) can only be changed by the maintainer.

  8. devantler commented on Sep 27, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Correction: the merge-queue timeout is managed as code in devantler-tech/.github (deploy/repository-rulesets/require-merge-queue-on-platform.yaml, Observe-only today), so it is not maintainer-only. The safe promotion path, and the bypass-actor evidence it needs first, are recorded on #3097, which owns the timeout.

  9. devantler commented on Sep 29, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Status update, 2026-09-29. The authority blocker recorded above no longer holds.

    This issue stays open until #3097 records that evidence. If a merge group is still evicted at 90 minutes, raise the value again in the same ruleset file in devantler-tech/.github. No maintainer action is needed.

  10. devantler commented on Oct 2, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Closing: all three acceptance criteria are met

    Re-checked on 2026-10-02 against the live ruleset and the 2026-09-27 code review above.

    Criterion Status
    Queue timeout covers heal + deploy Met. The live Require merge queue ruleset (active, last updated 2026-09-29 21:30Z) has check_response_timeout_minutes: 90, above the measured worst case of about 79 minutes.
    A merge group that succeeds after its PR was evicted is surfaced Met (2026-09-27 re-check): the queue-membership check runs after a successful deploy, and the heal runs on that path too.
    A failed deploy's heal no longer starves the next entry, or the queue accounts for that wait Met by the second branch. The 90-minute timeout absorbs the about 38-minute heal plus the about 41-minute deploy behind it.

    Whether 90 minutes holds against a real deploy that runs past 60 minutes is an observation, and #3097 already owns it as its open second criterion. If that observation shows an eviction, it is fixed there by raising the value in the ruleset file in devantler-tech/.github. Keeping this issue open as well would only park the same wait twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Fields

    Priority

    None yet

    Effort

    None yet

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions