Skip to content

Promote dev to main: pr-conflict-detected fires on opened + reopened#1253

Merged
zbigniewsobiecki merged 1 commit intomainfrom
dev
May 2, 2026
Merged

Promote dev to main: pr-conflict-detected fires on opened + reopened#1253
zbigniewsobiecki merged 1 commit intomainfrom
dev

Conversation

@zbigniewsobiecki
Copy link
Copy Markdown
Member

Promotes 1 commit from `dev`:

What

Live regression fix: PRs opened ALREADY conflicting (like ucho/PR #226) never had `resolve-conflicts` dispatched, because the matcher hard-coded `payload.action !== 'synchronize'`. Widen to also accept `opened` and `reopened` — the three actions GitHub emits when the PR head is the candidate state we should check for mergeability.

Reviewed and approved on #1252 by @nhopeatall. CI green on dev (3/3 expected).

Risk

Low. One-line matcher widen + test updates. Handler body unchanged — its `mergeable === null` retry loop and persona/base-branch gates already cover all three event types correctly.

🤖 Generated with Claude Code

…nly synchronize (#1252)

ucho/PR #226 was created by aaight at 2026-05-02T16:51:32 already CONFLICTING
against `dev`. resolve-conflicts never fired. The PR sat open with merge
conflicts and no auto-resolution; the user had to manually intervene.

Trigger config IS enabled for ucho:
  resolve-conflicts  scm:pr-conflict-detected  yes

Root cause: `pr-conflict-detected.matches()` at
`src/triggers/github/pr-conflict-detected.ts:32-42` hard-coded
`payload.action !== 'synchronize'`, rejecting `opened` events. Since PR #226
only fired a `pull_request.opened` event (no commits pushed since), the
matcher returned `false` and `handle()` never ran. The handler body — gates
on cascade persona + base branch, retries on `mergeable === null`, dispatches
when `mergeable === false` — never got the chance.

The existing test at lines 80-88 explicitly codified the bug
(`does not match non-synchronize action` with `action: 'opened'`).

Fix: widen the matcher to accept `opened`, `reopened`, and `synchronize` —
the three actions GitHub emits when the PR head is the candidate state we
should check for mergeability. `closed`, `edited`, `labeled`, `assigned`,
`ready_for_review` correctly stay rejected.

Why the handler body needs no changes: the `mergeable === null` retry loop
already handles GitHub's async mergeability computation (most prominent on
`opened`); cascade-persona + base-branch gates and the `gateAttemptLimit`
remain untouched. The cost of the widening is one extra getPR API call per
opened/reopened cascade-impl PR — small and bounded by the persona gate.

Tests:
- Replaced inverted `does not match non-synchronize action` (was the bug
  pin) with `matches opened action (PR #226 regression pin)` asserting true.
- Added `matches reopened action`.
- Replaced single `does not match closed action` with an `it.each` over
  `closed`, `edited`, `labeled`, `unlabeled`, `assigned`, `ready_for_review`
  to confirm we narrowed the rejection set, not widened carelessly.

Out of scope (follow-up): periodic mergeability re-check for PRs whose base
branch advanced and created a new conflict without any per-PR event. Real
concern for long-lived PRs, but the simpler `opened` fix solves PR #226's
case. Defer.

Verification:
- `npx vitest run --project unit-triggers tests/unit/triggers/pr-conflict-detected.test.ts` → 22/22.
- `npx vitest run --project unit-triggers --project unit-api` → 2503/2503.
- `npm run typecheck` + `npm run lint` clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@zbigniewsobiecki zbigniewsobiecki merged commit 2ca78a4 into main May 2, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant