Merge dev → main: refactor(triggers) consolidate handler boilerplate (#1237)#1238
Merged
zbigniewsobiecki merged 1 commit intomainfrom Apr 29, 2026
Merged
Merge dev → main: refactor(triggers) consolidate handler boilerplate (#1237)#1238zbigniewsobiecki merged 1 commit intomainfrom
zbigniewsobiecki merged 1 commit intomainfrom
Conversation
… gates (#1237) The trigger/webhook layer had 14+ bug fixes in the past 3 months clustering into 5 recurring classes (skip opacity, dedup collisions, ALS scope leakage, racing writers, gate divergence). The two GitHub handlers most affected by the latest two — check-suite-failure and pr-conflict-detected — had literal copies of the same skip(handler, message) helper. ~30 other return-null self-skip sites still produced the generic "No trigger matched for event" decisionReason despite the structured skipReason plumbing landed last week. This refactor consolidates the common shape into shared modules and pins the invariant with a static-grep guard. New shared modules: - src/triggers/shared/skip.ts — single canonical skip() builder. - src/triggers/shared/gates.ts — composable pure-function gates returning TriggerResult | null: - gateTriggerEnabled — async; wraps checkTriggerEnabled - gateBaseBranch — sync; PR base ref vs project.baseBranch - gateCascadePersona — sync; wraps isCascadeBot() - gateAttemptLimit — sync; per-PR retry counter check - requirePersonaIdentities — type-narrowing variant returning a { ok: true, value } | { ok: false, skip } discriminated union so callers no longer need ctx.personaIdentities! non-null assertions - tests/helpers/triggerAssertions.ts — expectSkip(result, handler, msg) + expectSkipFor(handler) factory for terse per-handler assertions. Static guard: tests/unit/triggers/handler-shape.test.ts walks every handler under src/triggers/ and asserts no file defines a local function skip( AND every GitHub handler that uses return skip(...) imports from ../shared/skip.js. Same pattern as trigger-event-consistency.test.ts and pm-router-adapter-pm-scope.test.ts. Handler-side changes: - 9 GitHub handlers compose gates from shared/gates.ts. Sync gate chains use ?? for short-circuit composition. - ~30 bare return null self-skip sites converted to structured skips with case-specific messages (PR # included, attempt counts, base branches). - pr-comment-mention pre-extracts prNumberHint so the persona-skip carries PR context. - Two integration tests updated to seed both agent + trigger configs so the gate exercises the actual loop-prevention persona check. Net diff: -297 / +823. Most additions are the new shared modules and their dedicated tests; handler files are flatter (typical handler lost ~30 lines of boilerplate). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
R1 trigger-handler refactor: shared skip + composable gates + static-grep guard + 9 GitHub handlers consolidated. Replaces 2 duplicated
skip()helpers and converts ~30 barereturn nullself-skip sites to structured skips with case-specific decisionReason messages. Handler files lose ~30 lines of boilerplate each.🤖 Generated with Claude Code