Skip to content

A merge-group heal can leave prod behind a merge that lands just after itΒ #2838

Closed
2 of 3 issues completed
Closed

Description

@devantler

πŸ€– Generated by the Agentic Engineer

Evidence

Raised as a P1 by Codex on #2825 and confirmed
as a real system property while reviewing that PR β€” but it is not introduced by that PR, so it is
filed here rather than folded into it.

The problem

deploy-prod and its heal job share the prod-deploy concurrency lock. A merge group's deploy is
speculative β€” it deploys code that has not merged yet. That admits this interleaving:

  1. Merge group A's deploy fails; A is evicted and its heal is queued.
  2. Merge group B's deploy runs and succeeds, putting B's (unmerged) content on prod.
  3. A's heal acquires the lock, checks out the current tip of main β€” which still predates B β€” and
    deploys it, overwriting B's deployment.
  4. B then merges. Nothing re-deploys it, so prod sits behind main until the next deploy.

Step 3 is individually correct: the heal deliberately deploys the last state that actually landed in
Git, and if B were later evicted instead of merged, that overwrite would be exactly right. The defect
is the absence of a deploy after step 4, not the heal's choice of ref.

Why this is not a #2825 regression

Without queue: max the same interleaving occurs whenever the heal is not dropped; queue: max only
stops the heal being silently discarded, which is the failure that job exists to prevent. The gap is
in the heal contract, and it predates the queue setting.

Expected behaviour

After a merge group lands, prod converges on main without requiring an unrelated later deploy.

Acceptance criteria

  • A merged revision is always deployed, even when a heal overwrote its speculative deploy beforehand.
  • The heal keeps deploying main's tip (no reintroduction of base_sha).
  • Covered by scripts/validate-merge-group-heal or an equivalent executable check.

Rough size: S–M. Likely shapes: a post-merge convergence deploy on push: main, or a heal that
records the sha it deployed and re-fires when main advances past it.

Activity

  1. added theissue type on Jul 28, 2026
  2. moved this to πŸ“₯ Backlog in 🌊 Project Boardon Jul 28, 2026
  3. coderabbitai commented on Jul 28, 2026

    @coderabbitai
    Contributor
    ⚠️ Possible Duplicate Issue(s)
    πŸ”— Related PRs

    #2330 - ci: heal prod when a merge-group deploy fails (restore main) [merged]
    #2339 - docs(agents): record merge-queue mechanics + root-cause-before-requeue [merged]
    #2622 - fix(ci): heal cancelled merge-group deploys [merged]
    #2633 - fix(deploy): verify incoming image on Talos nodes [merged]
    #2793 - feat(cilium): activate homogeneous device selection [merged]


    πŸ“ Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    • Create Plan

    πŸ§ͺ Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    πŸ’¬ Have feedback or questions? Drop into our discord!

  4. devantler commented on Sep 15, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Decision: fix this with a convergence check after merge, not in the heal

    I selected this as the oldest open bug with nothing parked on it, together with #2828, which describes the same race from the heal's side. I closed #2828 as a duplicate of this issue.

    Why not the heal. When the older group's heal runs, the newer group has not merged yet. Its artifact is therefore no more reachable from main than an ejected one, and no rule inside the heal can tell them apart. The state only becomes decidable once the newer group lands.

    Why a convergence check works. Measured 2026-09-15 on the deploy for #3845:

    • :latest's digest carries a signed SLSA provenance attestation that names its source commit.
    • A merge-group commit is exactly the commit main advances to: the attested commit reads identical against main.

    So "is prod on main?" is answerable from signed evidence we already publish.

    Decomposition:

    1. Tell whether prod has converged on main from the published artifact's signed provenanceΒ #3846: a read-only resolver that reports CONVERGED, BEHIND, DIVERGED or UNKNOWN from :latest's provenance and main. It is not wired anywhere, so it changes nothing in production.
    2. Follow-up, filed once the resolver's verdicts are proven: after a merge, run a deploy of main only when the verdict is BEHIND or DIVERGED. That child reaches production, so it carries the deploy lane's usual gates.

    Both acceptance criteria here map onto that sequence: a merged revision is always deployed (child 2 on BEHIND), and the heal keeps deploying main's tip unchanged.

  5. devantler commented on Sep 19, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Delivered in #3870 (merged 2026-09-17 10:48Z). This issue now waits on production evidence, not on code.

    Evidence so far (2026-09-19): all 31 validate-main runs on main since #3870 merged report CONVERGED from the post-merge convergence job. None has hit the BEHIND β†’ redeploy path yet. The scripted test covers that path (only BEHIND redeploys), but no real merge-group heal race has happened since the fix.

    Next step: close this issue once one real BEHIND reading on main is followed by a redeploy that ends CONVERGED. If a BEHIND reading is not followed by a converged prod, reopen the work. Moving this to πŸ“Š Verifying.

  6. moved this from πŸ“₯ Backlog to πŸ“Š Verifying in 🌊 Project Boardon Sep 19, 2026
  7. devantler commented on Sep 22, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Still Verifying (read 2026-09-22T00:45Z): the last 60 validate-main runs on main (2026-09-19T23:43Z onward) all ran 🧭 Observe Production Convergence successfully with πŸ” Redeploy main when prod is behind skipped β€” every reading converged, so no real heal race has occurred yet and there were no false BEHIND readings either. The close condition in the previous comment is unchanged.

  8. devantler commented on Sep 23, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Design pass (2026-09-23) β€” picking the "post-merge convergence deploy" shape, and why it is smaller than it looks.

    Three facts, each read from the current tree:

    1. A merge group's head commit is the commit that lands on main (the queue fast-forwards to the tested commit). So "did prod last deploy the revision now at main's tip?" has a single, exact comparison key β€” no speculative/landed mapping is needed.
    2. cd.yaml on refs/heads/main is already a trusted signer in the Flux OCI verification subject (and in ksail.prod.yaml's bootstrap copy), independent of the triggering event. A push: main trigger on cd.yaml therefore publishes an artifact Flux already accepts β€” no verifier or validate-flux-verify change.
    3. cd.yaml already carries the full gate set a non-merge-group deploy needs (PVC retirement, Talos/Kubernetes compatibility, nested RGD scan, authorization), and shares the prod-deploy lock with queue: max, so a convergence run queues behind any in-flight deploy or heal rather than racing it.

    Proposed first slice: add push: branches: [main] to cd.yaml, gated by a cheap first job that exits early unless prod's last published revision differs from github.sha. That keeps the original reason the release-tag trigger was dropped (no redundant re-deploy of what the queue already shipped) while closing step 4 of the interleaving above. The heal is untouched and keeps deploying main's tip.

    Open question the slice must settle first β€” the comparison source. The staging tag embeds GITHUB_SHA, but :latest is a digest alias, and the GitHub deployment record for the heal job carries the merge group's sha rather than the main revision it actually deployed, so it is not a safe key. Candidates: record the source revision as an annotation on the published artifact, or resolve which staging tag :latest's digest matches. The check must fail open to deploying (an unknown revision deploys; it never skips).

    Acceptance evidence: extend scripts/validate-merge-group-heal with the interleaving as a fixture (heal after a later successful group β†’ the landed revision still gets a deploy), RED on today's workflows.

  9. devantler commented on Sep 23, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Correction to the 2026-09-23 design pass: the post-merge convergence deploy it proposes has already shipped. Don't implement it again.

    This issue stays in πŸ“Š Verifying on the close condition recorded on 2026-09-19: close it after one real BEHIND reading on main is followed by a redeploy that ends CONVERGED. If the design pass found a gap in #3870's approach, file it as a separate issue with that evidence.

  10. self-assigned this
    on Sep 25, 2026
  11. added a commit that references this issue on Sep 25, 2026
  12. devantler commented on Sep 28, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    The missing post-merge deployment path was delivered by #3870, merged as 1dff62393cb4d1a5d11f2c2ccdc118e5251b82a2. Every push to main runs the production convergence observer under the shared deploy lock; a BEHIND result dispatches the normal gated CD workflow, while a queued speculative deploy is not rolled back on a DIVERGED result. The observer succeeded on current main in run 36359379463.

    I reran the focused convergence-report and redeploy contracts (31 and 7 assertions), plus the merge-group heal and queue-membership contracts; all passed. This verifies the shipped path and its tests, not a live replay of the rare failure. The later draft #4192 is being closed because its queue-drain wait is redundant with this path and can postpone healing a failed deploy while the queue remains busy. Closing this issue as delivered by #3870.

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Fields

Priority

None yet

Effort

None yet

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions