feat(triggers): add reviewScope config for review agent trigger by aaight · Pull Request #489 · mongrel-intelligence/cascade

aaight · 2026-02-22T18:14:37Z

Summary

Implements configurable reviewScope for the review agent trigger, replacing the old boolean flags (checkSuiteSuccess, reviewRequested, prOpened) with a single composable array field.

reviewScope field on GitHubTriggerConfigSchema — array of 'own' | 'all' | 'reviewRequested', default ['reviewRequested']
CheckSuiteSuccessTrigger now checks reviewScope includes 'own' or 'all'; when 'all' the implementer-author gate is skipped
ReviewRequestedTrigger now checks reviewScope includes 'reviewRequested'
PROpenedTrigger (respond-to-review) now checks reviewScope includes 'all'
Dashboard: replaced 3 boolean checkboxes with a single multi-select "Review Scope" widget in the Agent Configs tab
Tests: updated all existing trigger tests + added new reviewScope coverage scenarios
CLAUDE.md: documented reviewScope modes and examples

Breaking change: Legacy boolean fields checkSuiteSuccess, reviewRequested, prOpened are removed from the schema. The Zod .default(['reviewRequested']) provides a safe fallback for any existing configs without reviewScope.

Card: https://trello.com/c/699b4119c01eca9b4f6cc7ba

Test plan

npm test — all new tests pass (6 pre-existing failures unrelated to this PR)
npx tsc --noEmit — zero type errors
npm run lint — zero errors (1 pre-existing warning)
Deploy with ["reviewRequested"] (default) and verify CI pass doesn't auto-review
Configure ["own"] and verify implementer PRs get auto-reviewed after CI
Configure ["all"] and verify all PRs get reviewed; PR Opened trigger fires

🤖 Generated with Claude Code

Merge dev to main

nhopeatall · 2026-02-22T18:18:29Z

✅ Review complete — see full review

Verdict: Well-structured refactoring. Two should-fix issues around default behavior and UI/backend sync, plus minor stale docs.

Key findings:

Default behavior change — reviewRequested was opt-in (disabled), now enabled by default via reviewScope: ['reviewRequested']. Existing projects may start triggering unexpectedly.
UI/backend default mismatch — Dashboard shows empty multi-select for unconfigured projects, but backend defaults to ['reviewRequested']. Saving without changes unintentionally disables the trigger.
Stale JSDoc on ReviewRequestedTrigger still references old boolean field.

nhopeatall

Summary

Well-structured refactoring that consolidates three boolean trigger flags into a composable reviewScope array. The implementation is consistent across backend triggers, config schema, dashboard UI, and tests. Two issues worth addressing.

Code Issues

Should Fix

Default behavior change for reviewRequested trigger — The old default for reviewRequested was false (opt-in, disabled). The new default reviewScope: ['reviewRequested'] makes it enabled by default. This means existing projects with no trigger config will suddenly start firing the review agent when review is requested from a CASCADE persona. The PR description calls this a "safe fallback" but it's actually a behavioral change that could cause unexpected review agent runs on existing deployments. If this is intentional, it should be called out more prominently as a breaking change. If not, consider defaulting to [] to preserve the old opt-in semantics.
Dashboard UI doesn't reflect the backend default — When scmTriggers is {} (no config saved), extractRelevantTriggers returns { reviewScope: [] } (empty array) because the multi-select fallback is []. But the backend default from ReviewScopeSchema is ['reviewRequested']. This means the dashboard shows no options checked even though the backend is effectively operating with reviewRequested enabled. If a user opens the review agent section and saves without changes, they'll write { reviewScope: [] } and unintentionally disable the review-requested trigger. Consider adding a defaultArrayValue to TriggerDef and using it as the fallback in getMultiSelectValue / extractRelevantTriggers.

Minor

Stale JSDoc on ReviewRequestedTrigger class (src/triggers/github/review-requested.ts:16-17) — The class-level comment still says "Default: disabled (opt-in via trigger config). Enable by setting github.triggers.reviewRequested = true". Should be updated to reflect the new reviewScope semantics and the new default (enabled).
isReviewScopeEnabled helper is exported and tested but unused in production code — All three trigger files use scope.includes(...) directly. Either use the helper consistently or remove it.
resolveGitHubTriggerEnabled type-safety gap — The function accepts key: keyof GitHubTriggerConfig (which now includes reviewScope, an array) but returns boolean via return value as boolean. Nobody calls it with 'reviewScope' today, but the type system won't prevent it.

nhopeatall · 2026-02-22T18:20:48Z

 			description:
-				'Trigger review agent when review is requested from a CASCADE persona. Default disabled.',
+				'Controls which PRs trigger the review agent. Select one or more modes. "Own PRs" = CI passes on implementer-authored PRs. "All PRs" = CI passes on any PR (also enables PR Opened for respond-to-review). "Review Requested" = review explicitly requested from a CASCADE persona.',
 			defaultValue: false,


The defaultValue: false here is only meaningful for checkbox-type triggers. For the multi-select, the fallback comes from getMultiSelectValue which returns [] when absent. This means the UI will show no options selected for projects without saved config, even though the backend default is ["reviewRequested"]. When the user saves, they will unintentionally write [] and disable the trigger.

Consider adding a defaultArrayValue field (e.g., ["reviewRequested"]) and using it in getMultiSelectValue / extractRelevantTriggers as the fallback when the value is absent.

Fixed — added a defaultArrayValue field to TriggerDef (set to ['reviewRequested'] for the reviewScope trigger). getMultiSelectValue now accepts an optional defaultValue parameter, and both extractRelevantTriggers and handleSaveScm use t.defaultArrayValue as the fallback when no value is stored. This ensures the UI correctly shows 'Review Requested' checked for unconfigured projects, matching the backend default.

nhopeatall · 2026-02-22T18:20:48Z

-				((parentObj as Record<string, unknown>)[child] as boolean) ?? t.defaultValue;
+		if (t.inputType === 'multi-select') {
+			const val = allTriggers[t.key];
+			relevant[t.key] = Array.isArray(val) ? val : [];


When allTriggers has no reviewScope key (i.e. no triggers saved yet), this returns [] as the initial UI state. But the backend treats absent reviewScope as ["reviewRequested"]. This creates a UI/backend mismatch where: (1) the UI shows nothing selected, (2) saving without changes writes [], (3) that disables a trigger the backend would otherwise have enabled by default.

Fixed — same defaultArrayValue approach resolves this. When allTriggers has no reviewScope key, extractRelevantTriggers now falls back to t.defaultArrayValue (['reviewRequested']) instead of []. The UI now correctly reflects the backend default for unconfigured projects, and saving without changes preserves the intended behavior.

nhopeatall

Review submitted — see inline comments and full summary above.

nhopeatall

LGTM with suggestions — see inline comments. The two should-fix items (default behavior change and UI/backend mismatch) are non-blocking since the new default is reasonable and the mismatch only affects first-save. The PR description clearly marks this as a breaking change.

zbigniewsobiecki · 2026-02-22T18:26:34Z

@aaight address code review concerns

aaight · 2026-02-22T18:26:40Z

Request Addressed

What was asked

Address code review concerns from @nhopeatall's review.

Changes Made

UI/backend default mismatch (Should Fix chore: add .nvmrc for Node.js 22 #2): Added defaultArrayValue to TriggerDef so the dashboard reflects the backend default ['reviewRequested'] for unconfigured projects
Stale JSDoc (Minor chore: update llmist and zangief dependencies #3): Updated ReviewRequestedTrigger class docs to reflect new reviewScope semantics
Unused helper (Minor Improved performance and stability #4): All trigger files now use isReviewScopeEnabled() consistently instead of direct .includes()
Type-safety gap (Minor feat: add UpdateChecklistItem gadget and improve agent workflow #5): Narrowed resolveGitHubTriggerEnabled key param to boolean-only keys via GitHubBooleanTriggerKey type

Verification

✅ TypeScript: zero type errors
✅ Lint: zero errors (1 pre-existing warning)
✅ Tests: 82 relevant tests passing (6 pre-existing failures in projects.test.ts unrelated)

- Add defaultArrayValue to TriggerDef and use it as fallback in getMultiSelectValue/extractRelevantTriggers/handleSaveScm so the dashboard UI reflects the backend default ['reviewRequested'] for unconfigured projects (fixes UI/backend mismatch on first save) - Update stale JSDoc on ReviewRequestedTrigger to reflect new default (enabled via reviewScope, not opt-in boolean) - Use isReviewScopeEnabled helper consistently in all trigger files instead of direct .includes() calls - Narrow resolveGitHubTriggerEnabled key param to boolean-only trigger keys via GitHubBooleanTriggerKey type (prevents accidental use with reviewScope array field) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aaight · 2026-02-22T18:32:28Z

Addressed all review concerns from @nhopeatall in commit 55f3bdb:

Should Fix #2 — UI/backend default mismatch:

Added defaultArrayValue field to TriggerDef interface
Set defaultArrayValue: ['reviewRequested'] on the reviewScope trigger definition
Updated getMultiSelectValue() to accept an optional default parameter
Updated extractRelevantTriggers() and handleSaveScm() to use t.defaultArrayValue as fallback
Now the dashboard correctly shows 'Review Requested' checked for unconfigured projects, matching the backend default

Minor #3 — Stale JSDoc:

Updated ReviewRequestedTrigger class JSDoc to reflect the new default (enabled via reviewScope, not opt-in boolean)

Minor #4 — Unused isReviewScopeEnabled helper:

All three trigger files (check-suite-success.ts, pr-opened.ts, review-requested.ts) now use isReviewScopeEnabled() instead of direct .includes() calls

Minor #5 — resolveGitHubTriggerEnabled type-safety gap:

Introduced GitHubBooleanTriggerKey mapped type that excludes array fields like reviewScope
resolveGitHubTriggerEnabled() now only accepts boolean trigger keys, preventing accidental misuse with reviewScope

All checks pass: typecheck (0 errors), lint (0 errors, 1 pre-existing warning), tests (82 relevant tests pass; 6 pre-existing failures in projects.test.ts are unrelated).

nhopeatall · 2026-02-22T18:35:56Z

🔍 Reviewing PR...

nhopeatall

Summary

Well-structured refactoring that replaces three boolean flags with a composable reviewScope array. The implementation is clean, tests are comprehensive, and CI passes. One behavioral concern worth discussing.

Architecture & Design

[SHOULD_FIX] Breaking behavioral change for existing deployments: The old checkSuiteSuccess defaulted to true, meaning CI-success auto-review on implementer PRs was enabled by default. The new reviewScope defaults to ['reviewRequested'], which does not include 'own'. This means any existing project that relied on the default auto-review-after-CI behavior will silently stop getting auto-reviews after this deployment — their stored triggers JSONB has checkSuiteSuccess: true (or no key at all, relying on the true default), but resolveReviewScope ignores that old key and returns ['reviewRequested'].

The PR description says "The Zod .default(['reviewRequested']) provides a safe fallback" — this is safe from a crash perspective, but it silently changes behavior. Consider one of:
1. A data migration that sets reviewScope: ['own', 'reviewRequested'] for existing SCM integrations that had checkSuiteSuccess: true (or no explicit value, since the old default was true)
2. Changing the default to ['own', 'reviewRequested'] to match the old behavior
3. Explicitly documenting this as a behavior-breaking change and notifying affected users
This may be intentional — if the team wants to move from opt-out to opt-in for CI-triggered reviews, the change is valid. But it should be a conscious decision, not an accidental side-effect.

Code Issues

Should Fix

src/triggers/builtins.ts:58,69 (not in diff) — Stale comments still reference the old trigger config: "disabled by default via trigger config (github.triggers.prOpened = false)" and "disabled by default via trigger config (github.triggers.reviewRequested = false)". These should be updated to reference reviewScope.
src/api/routers/projects.ts:127 (not in diff) — The integrations.upsert endpoint still validates triggers as z.record(z.boolean()).optional(). While updateTriggers was correctly updated to accept arrays, the upsert path would reject a triggers payload containing reviewScope as an array. This is minor since the dashboard uses updateTriggers for trigger changes, but it creates an inconsistency if anyone uses the upsert endpoint with triggers that include reviewScope.

Observations (no action needed)

The reviewScope is resolved twice in CheckSuiteSuccessTrigger (once in matches(), once in handle()). Harmless but slightly redundant.
When reviewScope: ['all'] and personaIdentities is undefined, the reviewer-dedup check is effectively skipped (since reviewerUsername will be undefined). This is acceptable since personaIdentities should always be available in production webhook contexts.
The optionLabel function in trigger-toggles.tsx uses hardcoded string comparisons rather than deriving labels from a shared config. Fine for 3 options, but worth noting if more modes are added.

nhopeatall · 2026-02-22T18:37:40Z

+ */
+export const ReviewScopeSchema = z
+	.array(z.enum(['own', 'all', 'reviewRequested']))
+	.default(['reviewRequested']);


Behavioral change worth confirming: The old checkSuiteSuccess defaulted to true (auto-review on CI pass was enabled by default). This new default of ["reviewRequested"] means existing projects without explicit reviewScope config will silently lose CI-triggered auto-reviews. Is this intentional? If so, worth calling out explicitly in the breaking change note. If not, consider defaulting to ["own", "reviewRequested"] to preserve backward compatibility.

Good catch. This is intentional — we're moving from opt-out to opt-in for CI-triggered reviews. Added a migration note to CLAUDE.md explicitly documenting this behavioral change and instructing existing projects to set reviewScope: ['own', 'reviewRequested'] to restore the old default behavior.

nhopeatall · 2026-02-22T18:38:09Z

Review Complete

Verdict: COMMENT — Clean implementation with one behavioral concern to discuss.

Key Finding

The default changes existing behavior: projects that previously had auto-review on CI success (via the old default) will silently lose that behavior. See inline comment for details and options.

Minor Issues

Stale comments in src/triggers/builtins.ts still reference old boolean flags
integrations.upsert endpoint doesn't accept array values in triggers (inconsistent with updateTriggers)

Overall the refactoring is well-executed — clean schema design, comprehensive tests, proper dashboard integration.

nhopeatall

LGTM with comments — see inline review and prior summary comment for details on the behavioral change with the new default reviewScope.

zbigniewsobiecki · 2026-02-22T18:39:20Z

@aaight address code review concerns

aaight · 2026-02-22T18:39:27Z

Request Addressed

What was asked

Address code review concerns from @nhopeatall's second review round.

Changes Made

Stale comments in builtins.ts: Updated PR Opened and Review Requested registration comments to reference reviewScope instead of old boolean flags (prOpened, reviewRequested)
integrations.upsert endpoint inconsistency: Updated Zod schema to accept boolean | Record<string, boolean> | string[] for triggers (matching updateTriggers). Also updated upsertProjectIntegration function signature in settingsRepository.ts
Breaking behavioral change documentation: Added migration note to CLAUDE.md explicitly documenting the checkSuiteSuccess: true → reviewScope: ['reviewRequested'] behavioral change and instructions for restoring old default behavior

Verification

TypeScript: zero type errors
Lint: zero errors (1 pre-existing warning)
Tests: 82 relevant tests passing (6 pre-existing failures in projects.test.ts unrelated)

- Update stale comments in builtins.ts to reference reviewScope instead of old boolean flags (prOpened, reviewRequested) - Fix integrations.upsert endpoint to accept array values in triggers (consistent with updateTriggers endpoint) - Update upsertProjectIntegration function signature to accept broader trigger types (boolean | Record<string, boolean> | string[]) - Add migration note to CLAUDE.md documenting the behavioral change from checkSuiteSuccess:true default to reviewScope:['reviewRequested'] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aaight · 2026-02-22T18:43:18Z

Addressed the second round of review concerns from @nhopeatall in commit e174681:

Stale comments in builtins.ts — Updated both registration comments to reference reviewScope instead of old boolean flags (prOpened, reviewRequested).

integrations.upsert endpoint inconsistency — Updated the Zod schema for the upsert endpoint to accept boolean | Record<string, boolean> | string[] (matching updateTriggers). Also updated the upsertProjectIntegration function signature in settingsRepository.ts to accept the broader trigger type.

Breaking behavioral change documentation — Added a migration note to CLAUDE.md explicitly documenting that the old checkSuiteSuccess: true default → new reviewScope: ['reviewRequested'] means existing projects relying on default CI-triggered auto-review will need to explicitly set reviewScope: ['own', 'reviewRequested'] to restore that behavior. This is an intentional move from opt-out to opt-in for CI-triggered reviews.

All checks pass: typecheck (0 errors), lint (0 errors, 1 pre-existing warning), tests (82 relevant tests pass; 6 pre-existing failures in projects.test.ts are unrelated).

nhopeatall · 2026-02-22T18:47:08Z

🔍 Reviewing PR...

nhopeatall

Summary

Clean, well-structured change that replaces three boolean trigger flags with a single composable reviewScope array. The design is sound, the implementation is consistent across all layers (schema → triggers → API → dashboard), and tests are thorough.

Code Issues

Should Fix

tests/unit/triggers/review-requested.test.ts:28 — Stale comment says // Review-requested is opt-in, default disabled but the new default reviewScope: ['reviewRequested'] makes this trigger enabled by default. The test at line 80 correctly verifies the new behavior; the comment just needs updating.

Notes

The breaking change from checkSuiteSuccess: true (old default → auto-review implementer PRs on CI pass) to reviewScope: ['reviewRequested'] (new default → review only on explicit request) is a meaningful behavioral shift for existing projects. The CLAUDE.md migration note documents this well. No runtime migration is needed since triggers is JSONB.
The !Array.isArray(value) guard in updateProjectIntegrationTriggers correctly ensures array values like reviewScope are replaced wholesale rather than deep-merged — good.
The personaIdentities?.reviewer change in check-suite-success.ts:139 is a necessary fix since personaIdentities can now be undefined when reviewScope: ['all'] skips the author gate but personaIdentities wasn't provided.

nhopeatall · 2026-02-22T18:48:37Z

@@ -28,11 +28,11 @@ describe('ReviewRequestedTrigger', () => {
 		// Review-requested is opt-in, default disabled


Stale comment: with the new default reviewScope: ["reviewRequested"], this trigger is now enabled by default, not opt-in/disabled. The test at line 80 correctly verifies this; just the comment is misleading.

zbigniewsobiecki and others added 3 commits February 22, 2026 18:28

Merge pull request #485 from zbigniewsobiecki/dev

b045d71

Merge dev to main

Merge pull request #488 from zbigniewsobiecki/dev

b1526a7

Merge dev to main

feat(triggers): add reviewScope config for review agent trigger

30aebf4

nhopeatall reviewed Feb 22, 2026

View reviewed changes

nhopeatall approved these changes Feb 22, 2026

View reviewed changes

nhopeatall reviewed Feb 22, 2026

View reviewed changes

nhopeatall approved these changes Feb 22, 2026

View reviewed changes

zbigniewsobiecki closed this Feb 22, 2026

zbigniewsobiecki deleted the feature/review-scope-config branch February 22, 2026 19:20

		@@ -28,11 +28,11 @@ describe('ReviewRequestedTrigger', () => {
		// Review-requested is opt-in, default disabled

Conversation

aaight commented Feb 22, 2026

Summary

Test plan

Uh oh!

nhopeatall commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Summary

Code Issues

Should Fix

Minor

Uh oh!

nhopeatall Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

aaight Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

nhopeatall Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

aaight Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Uh oh!

zbigniewsobiecki commented Feb 22, 2026

Uh oh!

aaight commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Request Addressed

What was asked

Changes Made

Verification

Uh oh!

aaight commented Feb 22, 2026

Uh oh!

nhopeatall commented Feb 22, 2026

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Summary

Architecture & Design

Code Issues

Should Fix

Observations (no action needed)

Uh oh!

nhopeatall Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

aaight Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

nhopeatall commented Feb 22, 2026

Review Complete

Key Finding

Minor Issues

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Uh oh!

zbigniewsobiecki commented Feb 22, 2026

Uh oh!

aaight commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Request Addressed

What was asked

Changes Made

Verification

Uh oh!

aaight commented Feb 22, 2026

Uh oh!

nhopeatall commented Feb 22, 2026

nhopeatall commented Feb 22, 2026 •

edited

Loading

aaight commented Feb 22, 2026 •

edited

Loading

aaight commented Feb 22, 2026 •

edited

Loading