Skip to content

fix(triggers): correct pr-conflict-detected gating event so resolve-conflicts actually fires#1217

Merged
zbigniewsobiecki merged 1 commit intodevfrom
fix/conflict-detection-event-string-mismatch
Apr 27, 2026
Merged

fix(triggers): correct pr-conflict-detected gating event so resolve-conflicts actually fires#1217
zbigniewsobiecki merged 1 commit intodevfrom
fix/conflict-detection-event-string-mismatch

Conversation

@zbigniewsobiecki
Copy link
Copy Markdown
Member

Summary

Conflict detection has been silently disabled in production for every project that "enabled" it via the dashboard, since the same commit that standardized the emitted triggerEvent to 'scm:pr-conflict-detected' everywhere else left one gating call out of the rename.

The handler at src/triggers/github/pr-conflict-detected.ts:43 passed 'scm:conflict-resolution' to checkTriggerEnabled, but every other surface (the agent YAML, the triggerTypes.ts registry the dashboard / CLI write through, and the handler's own emitted triggerEvent on AgentInput) uses 'scm:pr-conflict-detected'. So isTriggerEnabled('resolve-conflicts', 'scm:conflict-resolution') always missed the operator's enabled DB row, fell back to the agent definition's defaultEnabled: false, and silently returned null.

Production confirmation

cascade webhooklogs on 2026-04-27 — six pull_request synchronize events on ucho PRs #81 and #85 in an 11-minute window each returned:

decisionReason: "No trigger matched for event"

despite cascade projects trigger-list ucho showing:

resolve-conflicts | scm:pr-conflict-detected | enabled=yes

The handler's matches() returned true (it only checks payload.action === 'synchronize'); only the gating string was wrong.

Audit

Cross-checked all 20 checkTriggerEnabled / checkTriggerEnabledWithParams call sites in src/triggers/ against each handler's emitted triggerEvent. This was the only mismatch. Every other handler's gating event matches its emitted event matches its agent YAML's declared event:

Handler Status
pr-conflict-detected.ts ❌ → ✅ (this PR)
19 others (alerting, pm:status-changed, pm:label-added, pm:comment-mention, scm:pr-review-submitted, scm:pr-comment-mention, scm:check-suite-{success,failure}, scm:pr-{opened,merged,review-submitted}, scm:review-requested, internal:auto-chain) ✅ Already consistent

Guard against future drift

Added tests/unit/triggers/trigger-event-consistency.test.ts — a static-analysis test that scans every trigger handler file under src/triggers/, extracts the gating event string from each checkTriggerEnabled(...) call and the emitted triggerEvent: '...' property, and asserts every gating event appears in the file's emitted set. The test fails loudly with a precise file:line reference on any future drift, listing the actual events found and pointing the contributor at either fixing the mismatch or adding an explicit EXEMPT_FILES entry with a reason.

The guard test catches this exact bug — when run against the pre-fix code, it produces:

Handler src/triggers/github/pr-conflict-detected.ts calls checkTriggerEnabled(...,
'scm:conflict-resolution', ...) but never emits triggerEvent: 'scm:conflict-resolution'.
Emitted events in this file: [scm:pr-conflict-detected]. This silently disables the
trigger in production: ...

Also

  • Existing handler unit test was codifying the bug (asserted checkTriggerEnabled was called with 'scm:conflict-resolution'). Updated to pin the correct value.

Test plan

  • Guard test fails on pre-fix code, passes on post-fix code (TDD-confirmed locally).
  • Handler unit tests pass (npx vitest run tests/unit/triggers/pr-conflict-detected.test.ts → 16/16).
  • Full unit suite green (npm test → 469 files / 8631 tests pass / 23 skipped).
  • Typecheck clean.
  • Lint clean.
  • CI green on this PR.
  • After merge to dev + main: introduce a real merge conflict on a CASCADE-implementer-authored PR; confirm webhook decision flips from "No trigger matched for event" to "Job queued: resolve-conflicts agent for work item N".

🤖 Generated with Claude Code

…onflicts actually fires

The gating call passed `'scm:conflict-resolution'` to `checkTriggerEnabled`, but
every other surface (the agent YAML, the trigger-types registry the dashboard /
CLI write through, and the handler's own emitted `triggerEvent` on AgentInput)
uses `'scm:pr-conflict-detected'`. So `isTriggerEnabled('resolve-conflicts',
'scm:conflict-resolution')` always missed the operator's enabled DB row, fell
back to the agent definition's `defaultEnabled: false`, and silently returned
null on every `pull_request synchronize` webhook.

Confirmed via `cascade webhooklogs` on 2026-04-27: 6+ synchronize events for
ucho on PRs #81 and #85 in an 11-minute window each returned `decisionReason:
"No trigger matched for event"` despite `cascade projects trigger-list ucho`
showing `resolve-conflicts | scm:pr-conflict-detected | enabled=yes`. The
trigger handler's `matches()` returned true; only the gating string was wrong.

Conflict detection has been silently broken in production for every project
that "enabled" it via the dashboard, since the same commit that standardized
the emitted `triggerEvent` to `'scm:pr-conflict-detected'` everywhere else
left this one gating call out of the rename.

Fix: one-line string change. The existing handler test was codifying the bug
(it asserted `checkTriggerEnabled` was called with `'scm:conflict-resolution'`)
and is updated to pin the correct value.

Audit: cross-checked all 20 `checkTriggerEnabled` / `checkTriggerEnabledWithParams`
call sites in `src/triggers/` against each handler's emitted `triggerEvent`.
This was the only mismatch.

Guard: added `tests/unit/triggers/trigger-event-consistency.test.ts` — a
static-analysis test that scans every trigger handler file under
`src/triggers/`, extracts the gating event string from each
`checkTriggerEnabled(...)` call and the emitted `triggerEvent: '...'`
property, and asserts every gating event appears in the file's emitted set.
The test fails loudly with a precise file:line reference on any future drift,
listing the actual events found and pointing the contributor at either fixing
the mismatch or adding an explicit `EXEMPT_FILES` entry with a reason.

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

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the one-character fix is correct and the guard test is well-designed.

🕵️ claude-code · claude-opus-4-6 · run details

@zbigniewsobiecki zbigniewsobiecki merged commit 0a5dea9 into dev Apr 27, 2026
9 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.

2 participants