Skip to content

A conflicting PR is never recovered: the rebase sentinel re-fires ~15x per PR reporting success while resolving nothing, and auto-rebase-retry has no manual trigger #1890

Closed

Description

@don-petry

Summary

A PR that goes CONFLICTING/DIRTY is never recovered by the fleet. The designed path exists and fires — auto-rebase posts an <!-- auto-rebase-conflict: --> sentinel, dev-lead-intent.sh maps it to emit_intent "rebase" — but it does not converge. It re-fires indefinitely, reports success, and changes nothing. There is also no way to invoke the retry path by hand, so an operator cannot even nudge it.

Both halves were hit on 2026-09-22 while unblocking #1799 and #1800.

Evidence — the automation fired ~15× per PR and resolved nothing

#1799 and #1800 became CONFLICTING on 2026-09-21T02:02–02:23Z (conflicts created by four unrelated PRs merging) and were still CONFLICTING at 2026-09-22T22:04Z — ~44 hours.

In that window:

PR auto-rebase-conflict sentinel comments
#1799 16
#1800 15

Two of them landed at 17:20Z and 17:43Z on 2026-09-22 — after needs-human-review had been removed from both PRs, so the suppression gate (pr_resume_suppressed) was not what stopped them. The dev-lead runs triggered by those sentinels completed with conclusion: success while the PRs remained CONFLICTING. This is the false-success shape of #1567, on the rebase surface: a run that reports done having done nothing.

The net effect is worse than no automation: ~31 sentinel comments of pure noise across two PRs, each one an issue_comment event that spawns further dev-lead runs, while the actual blocker never moves.

Prior art: #865 — "rebase handler times out (exit 124) on hard conflicts instead of aborting; sentinel bursts" — landed as PR #1802 on 2026-09-21T00:18Z. The bursts documented above are all after that fix, so #865 did not close this.

The conflicts were mechanical, not semantic

This is the part that makes the gap indefensible. A human resolved both in minutes, and neither required judgement about intent:

Both are the "two sides independently added adjacent things" pattern. An agent that can implement a feature can certainly do this.

Half two — the retry path cannot be triggered manually

$ gh workflow run auto-rebase-retry.yml --repo petry-projects/.github-private
could not create workflow dispatch event: HTTP 422:
Workflow does not have 'workflow_dispatch' trigger

auto-rebase-retry.yml triggers only on workflow_run of "Auto-rebase non-Dependabot PRs" (types: [completed], gated on conclusion == 'failure').

Two consequences:

  1. No manual entry point. An operator watching a stuck PR cannot ask the retry path to run.
  2. A conflict is not in its scope at all. It retries failed auto-rebase runs — infrastructure flakes. scripts/auto-rebase-retry.sh says so: "The Auto-rebase workflow handles merge conflicts gracefully (it posts a …)". A conflict is a successful run with a sentinel, so the retry workflow is never even considered.

And auto-rebase.yml, which does have workflow_dispatch, is pure git — zero engine references — so dispatching it cannot resolve a real conflict. The only manually-invocable rebase surface is structurally incapable of the job.

Acceptance Criteria

  1. A PR in CONFLICTING/DIRTY state converges without a human: either the rebase intent resolves the conflict, or it escalates once with a clear reason. The current outcome — neither resolving nor escalating, while re-firing indefinitely — is not acceptable.
  2. The rebase intent cannot report success while the PR is still CONFLICTING. Assert the post-condition against the PR's real mergeable/mergeStateStatus, not the script's exit code. A run that did not change the state reports failure.
  3. Sentinel re-posting is bounded per head SHA. A PR must never accumulate 15 identical conflict sentinels; after N unsuccessful attempts at the same head, stop posting and escalate to needs-human-review with the reason.
  4. auto-rebase-retry.yml gains a workflow_dispatch trigger taking a PR number, so an operator can invoke the recovery path directly.
  5. Either the retry workflow's scope widens to cover conflict sentinels (not only failed runs), or a separate manually-invocable surface exists that routes a DIRTY PR into the rebase intent. Document which, because today the naming implies a coverage that does not exist.
  6. Mechanical adjacency conflicts — both sides adding independent, non-overlapping content at the same location — are resolved rather than escalated. The two cases above are the regression fixtures.
  7. Post-resolution integrity is verified before pushing: bash -n, shellcheck, and check-duplicate-decls.sh (the [Phase 4] Post-conflict-resolution integrity check — catch duplicated/corrupted content before it's trusted #1482/[Incident] Trunk corruption: main's scripts carry 3–6x duplicated function definitions (merged via #1420, #1292) — surgical restoration required #1485 corruption class), since a bad conflict resolution has corrupted trunk here before.

Notes for the implementer

References

Activity

  1. github-actions commented on Sep 22, 2026

    @github-actions
    Contributor

    Thank you for this exceptionally thorough report — the evidence table, exact timestamps, and worked conflict examples make the failure mode very clear. One question before implementation begins: you note the dev-lead runs triggered by the sentinels completed in seconds with conclusion: success while the PR stayed CONFLICTING — could you share a link to (or paste the step-level log of) one of those runs so we can confirm exactly where dev-lead-intent.sh exits early, whether that is before the engine is ever invoked or inside it after a no-op? Knowing which short-circuit path is responsible will let us sequence the fixes in AC #2 and #3 correctly and write a regression test against the right exit point.

  2. don-petry commented on Sep 22, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #1890 — A conflicting PR is never recovered: the rebase sentinel re-fires ~15x per PR reporting success while resolving nothing, and auto-rebase-retry has no manual trigger

    Scope

    Make the rebase intent converge (resolve the mechanical conflict class or escalate once, loudly), stop it reporting success while the PR is still CONFLICTING by asserting GitHub's real mergeable/mergeStateStatus, bound the failure loop into a single needs-human-review escalation, verify post-resolution integrity before reporting applied, and give auto-rebase-retry a manual workflow_dispatch entry point that routes a DIRTY PR into the rebase intent.

    Root cause

    The rebase intent determined success from the engine exit code + a local trial-merge (detect_conflicting_paths). GitHub's mergeable state is computed asynchronously and can disagree with a locally-clean merge, so a run could report status=applied in seconds while the PR remained CONFLICTING. Because it never recorded a failure, the #865 exhaustion counter never advanced and nothing escalated — the sentinel re-fired indefinitely.

    Implementation Checklist

    Tests to Write

    • tests/dev-lead/unit/test_rebase_exhaustion.bats — rebase_conflict_state truth table (CONFLICTING / DIRTY / MERGEABLE / UNKNOWN / empty)
    • tests/dev-lead/unit/test_auto_rebase_retry.bats — manual workflow_dispatch with a PR number fires the rebase repository_dispatch and does NOT run rerun

    Test Command

    bats tests/dev-lead/unit/test_rebase_exhaustion.bats tests/dev-lead/unit/test_auto_rebase_retry.bats and bash .dev-lead/scripts/dev-lead-lint.sh

    Note on scope / AC #6

    The mechanical-conflict resolution itself is performed by the engine driven by prompts/dev-lead/rebase.md; the deterministic, testable fixes here are the convergence/verification machinery around it plus the sharpened prompt. The two hand-fixed commits (f828bbe9, e11169ce) are the worked examples for the adjacency class.

  3. don-petry commented on Sep 22, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead — issue #1890 (in progress)

    Implementation is on the branch dev-lead/issue-1890-20260922-2207; CI opens the PR. This is a work-in-progress status note, not a completion report — the changes still need PR review and a live conflict-recovery run to confirm behavior end-to-end.

    What the change set does so far

    • Authoritative post-condition for the rebase intent. The rebase handler no longer infers success from the engine exit code or a local trial-merge — both lied here (a locally-clean merge that GitHub still reported CONFLICTING let a run post status=applied in seconds while resolving nothing). A new pure helper rebase_conflict_state(mergeable, mergeStateStatus) classifies GitHub's real state as resolved / conflicting / indeterminate, wrapped by a short polling read (gh pr view --json mergeable,mergeStateStatus) that tolerates the transient UNKNOWN right after a force-push. A run that did not change the state is now recorded as a failure, not success.

    • Bounded escalation. A conflicting verdict feeds the existing per-PR failure counter; once the threshold is reached the handler adds the needs-human-review label (which the hold gate honors, so subsequent sentinels for that PR are skipped) and disables auto-merge. The loop converges to a single escalation with a stated reason instead of re-firing.

    • Post-resolution integrity gate. Before reporting applied, the resolved tree must pass bash -n, shellcheck --severity=warning -x, and check-duplicate-decls.sh on changed shell files. A failure escalates to a human rather than merging a possibly-corrupted resolution (the [Phase 4] Post-conflict-resolution integrity check — catch duplicated/corrupted content before it's trusted #1482/[Incident] Trunk corruption: main's scripts carry 3–6x duplicated function definitions (merged via #1420, #1292) — surgical restoration required #1485 class).

    • Manual recovery surface. auto-rebase-retry.yml gains a workflow_dispatch trigger taking a pr_number. Rather than widening the workflow_run scope to conflict sentinels, the manual path routes a PR stuck in CONFLICTING/DIRTY straight into dev-lead's rebase intent via a dev-lead-reviews-retry repository_dispatch. Documented in AGENTS.md.

    • Mechanical adjacency guidance. prompts/dev-lead/rebase.md now instructs the engine to resolve (union, keep both) independent non-overlapping additions at the same location rather than escalate, while still escalating genuinely contradictory edits — with the two hand-fixed commits called out as the reference.

    Verification run locally

    Unit tests for the two touched libraries pass (test_rebase_exhaustion.bats, test_auto_rebase_retry.bats), dev-lead-lint.sh passes, and shellcheck/bash -n are clean on all changed scripts. Pre-existing, environment-dependent failures in test_engine_fallback.bats / test_fix_issue.bats (engine-binary/auth fixtures) reproduce identically with my changes stashed, so they are unrelated to this work.

    Still pending: PR review and a real conflicting-PR recovery to confirm the mergeable-state assertion and escalation behave as intended against live GitHub timing.

  4. don-petry commented on Sep 22, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete — PR #1891

    The implementation for issue #1890 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.

  5. added a commit that references this issue on Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickupenhancementFeature requests

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions