From f69428472cf8bc4939ba9f2270a65d6d241db4ba Mon Sep 17 00:00:00 2001 From: aaight Date: Thu, 23 Apr 2026 22:20:54 +0200 Subject: [PATCH 1/5] fix(wizard): gate optional PM wizard steps on required steps being complete (#1172) Co-authored-by: Cascade Bot --- tests/unit/web/jira-wizard-isComplete.test.ts | 140 ++++++++++++++++++ .../unit/web/linear-wizard-isComplete.test.ts | 122 +++++++++++++++ .../unit/web/trello-wizard-isComplete.test.ts | 122 +++++++++++++++ .../projects/pm-providers/jira/wizard.ts | 27 +++- .../projects/pm-providers/linear/wizard.ts | 25 +++- .../projects/pm-providers/trello/wizard.ts | 25 +++- 6 files changed, 451 insertions(+), 10 deletions(-) create mode 100644 tests/unit/web/jira-wizard-isComplete.test.ts create mode 100644 tests/unit/web/linear-wizard-isComplete.test.ts create mode 100644 tests/unit/web/trello-wizard-isComplete.test.ts diff --git a/tests/unit/web/jira-wizard-isComplete.test.ts b/tests/unit/web/jira-wizard-isComplete.test.ts new file mode 100644 index 00000000..01aecb06 --- /dev/null +++ b/tests/unit/web/jira-wizard-isComplete.test.ts @@ -0,0 +1,140 @@ +/** + * JIRA wizard — isComplete predicates for optional steps. + * + * Guards that optional steps (labels, custom-fields, issue-types, webhook) + * only show green check marks after the required steps (credentials + + * project + status mapping) are all complete. Prevents the UI bug where a + * brand-new unconfigured integration showed every step as green. + */ + +import { describe, expect, it } from 'vitest'; +import { jiraProviderWizard } from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; + +const getStep = (id: string) => { + const step = jiraProviderWizard.steps.find((s) => s.id === id); + if (!step) throw new Error(`Step ${id} not found`); + return step; +}; + +describe('JIRA optional steps — isComplete gating', () => { + describe('fresh state (createInitialState)', () => { + const state = createInitialState(); + + it('jira-labels is NOT complete on fresh state', () => { + expect(getStep('jira-labels').isComplete(state)).toBe(false); + }); + + it('jira-custom-fields is NOT complete on fresh state', () => { + expect(getStep('jira-custom-fields').isComplete(state)).toBe(false); + }); + + it('jira-issue-types is NOT complete on fresh state', () => { + expect(getStep('jira-issue-types').isComplete(state)).toBe(false); + }); + + it('jira-webhook is NOT complete on fresh state', () => { + expect(getStep('jira-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('partially configured (credentials only, no project)', () => { + const state = { + ...createInitialState(), + jiraEmail: 'user@example.com', + jiraApiToken: 'token123', + jiraBaseUrl: 'https://example.atlassian.net', + verificationResult: { provider: 'jira' as const, display: 'user@example.com' }, + }; + + it('jira-labels is NOT complete when project not selected', () => { + expect(getStep('jira-labels').isComplete(state)).toBe(false); + }); + + it('jira-custom-fields is NOT complete when project not selected', () => { + expect(getStep('jira-custom-fields').isComplete(state)).toBe(false); + }); + + it('jira-issue-types is NOT complete when project not selected', () => { + expect(getStep('jira-issue-types').isComplete(state)).toBe(false); + }); + + it('jira-webhook is NOT complete when project not selected', () => { + expect(getStep('jira-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('credentials + project, but no status mapping', () => { + const state = { + ...createInitialState(), + jiraEmail: 'user@example.com', + jiraApiToken: 'token123', + jiraBaseUrl: 'https://example.atlassian.net', + verificationResult: { provider: 'jira' as const, display: 'user@example.com' }, + jiraProjectKey: 'PROJ', + jiraStatusMappings: {}, + }; + + it('jira-labels is NOT complete without status mapping', () => { + expect(getStep('jira-labels').isComplete(state)).toBe(false); + }); + + it('jira-webhook is NOT complete without status mapping', () => { + expect(getStep('jira-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('fully configured (credentials + project + status mapping)', () => { + const state = { + ...createInitialState(), + jiraEmail: 'user@example.com', + jiraApiToken: 'token123', + jiraBaseUrl: 'https://example.atlassian.net', + verificationResult: { provider: 'jira' as const, display: 'user@example.com' }, + jiraProjectKey: 'PROJ', + jiraStatusMappings: { todo: 'To Do', inProgress: 'In Progress' }, + }; + + it('jira-labels is complete when all required steps done', () => { + expect(getStep('jira-labels').isComplete(state)).toBe(true); + }); + + it('jira-custom-fields is complete when all required steps done', () => { + expect(getStep('jira-custom-fields').isComplete(state)).toBe(true); + }); + + it('jira-issue-types is complete when all required steps done', () => { + expect(getStep('jira-issue-types').isComplete(state)).toBe(true); + }); + + it('jira-webhook is complete when all required steps done', () => { + expect(getStep('jira-webhook').isComplete(state)).toBe(true); + }); + }); + + describe('edit mode with stored credentials (isEditing + hasStoredCredentials)', () => { + const state = { + ...createInitialState(), + isEditing: true, + hasStoredCredentials: true, + jiraProjectKey: 'PROJ', + jiraStatusMappings: { todo: 'To Do' }, + }; + + it('jira-labels is complete in edit mode with stored credentials', () => { + expect(getStep('jira-labels').isComplete(state)).toBe(true); + }); + + it('jira-custom-fields is complete in edit mode with stored credentials', () => { + expect(getStep('jira-custom-fields').isComplete(state)).toBe(true); + }); + + it('jira-issue-types is complete in edit mode with stored credentials', () => { + expect(getStep('jira-issue-types').isComplete(state)).toBe(true); + }); + + it('jira-webhook is complete in edit mode with stored credentials', () => { + expect(getStep('jira-webhook').isComplete(state)).toBe(true); + }); + }); +}); diff --git a/tests/unit/web/linear-wizard-isComplete.test.ts b/tests/unit/web/linear-wizard-isComplete.test.ts new file mode 100644 index 00000000..f480a9be --- /dev/null +++ b/tests/unit/web/linear-wizard-isComplete.test.ts @@ -0,0 +1,122 @@ +/** + * Linear wizard — isComplete predicates for optional steps. + * + * Guards that optional steps (labels, project-scope, webhook) only show + * green check marks after the required steps (credentials + team + status + * mapping) are all complete. Prevents the UI bug where a brand-new + * unconfigured integration showed every step as green. + */ + +import { describe, expect, it } from 'vitest'; +import { linearProviderWizard } from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; + +const getStep = (id: string) => { + const step = linearProviderWizard.steps.find((s) => s.id === id); + if (!step) throw new Error(`Step ${id} not found`); + return step; +}; + +describe('Linear optional steps — isComplete gating', () => { + describe('fresh state (createInitialState)', () => { + const state = createInitialState(); + + it('linear-labels is NOT complete on fresh state', () => { + expect(getStep('linear-labels').isComplete(state)).toBe(false); + }); + + it('linear-project-scope is NOT complete on fresh state', () => { + expect(getStep('linear-project-scope').isComplete(state)).toBe(false); + }); + + it('linear-webhook is NOT complete on fresh state', () => { + expect(getStep('linear-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('partially configured (credentials only, no team)', () => { + const state = { + ...createInitialState(), + linearApiKey: 'lin_api_123', + verificationResult: { provider: 'linear' as const, display: 'user@example.com' }, + }; + + it('linear-labels is NOT complete when team not selected', () => { + expect(getStep('linear-labels').isComplete(state)).toBe(false); + }); + + it('linear-project-scope is NOT complete when team not selected', () => { + expect(getStep('linear-project-scope').isComplete(state)).toBe(false); + }); + + it('linear-webhook is NOT complete when team not selected', () => { + expect(getStep('linear-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('credentials + team, but no status mapping', () => { + const state = { + ...createInitialState(), + linearApiKey: 'lin_api_123', + verificationResult: { provider: 'linear' as const, display: 'user@example.com' }, + linearTeamId: 'team-1', + linearStatusMappings: {}, + }; + + it('linear-labels is NOT complete without status mapping', () => { + expect(getStep('linear-labels').isComplete(state)).toBe(false); + }); + + it('linear-project-scope is NOT complete without status mapping', () => { + expect(getStep('linear-project-scope').isComplete(state)).toBe(false); + }); + + it('linear-webhook is NOT complete without status mapping', () => { + expect(getStep('linear-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('fully configured (credentials + team + status mapping)', () => { + const state = { + ...createInitialState(), + linearApiKey: 'lin_api_123', + verificationResult: { provider: 'linear' as const, display: 'user@example.com' }, + linearTeamId: 'team-1', + linearStatusMappings: { todo: 'state-uuid-1', inProgress: 'state-uuid-2' }, + }; + + it('linear-labels is complete when all required steps done', () => { + expect(getStep('linear-labels').isComplete(state)).toBe(true); + }); + + it('linear-project-scope is complete when all required steps done', () => { + expect(getStep('linear-project-scope').isComplete(state)).toBe(true); + }); + + it('linear-webhook is complete when all required steps done', () => { + expect(getStep('linear-webhook').isComplete(state)).toBe(true); + }); + }); + + describe('edit mode with stored credentials (isEditing + hasStoredCredentials)', () => { + const state = { + ...createInitialState(), + isEditing: true, + hasStoredCredentials: true, + linearTeamId: 'team-1', + linearStatusMappings: { todo: 'state-uuid-1' }, + }; + + it('linear-labels is complete in edit mode with stored credentials', () => { + expect(getStep('linear-labels').isComplete(state)).toBe(true); + }); + + it('linear-project-scope is complete in edit mode with stored credentials', () => { + expect(getStep('linear-project-scope').isComplete(state)).toBe(true); + }); + + it('linear-webhook is complete in edit mode with stored credentials', () => { + expect(getStep('linear-webhook').isComplete(state)).toBe(true); + }); + }); +}); diff --git a/tests/unit/web/trello-wizard-isComplete.test.ts b/tests/unit/web/trello-wizard-isComplete.test.ts new file mode 100644 index 00000000..bc97cbd9 --- /dev/null +++ b/tests/unit/web/trello-wizard-isComplete.test.ts @@ -0,0 +1,122 @@ +/** + * Trello wizard — isComplete predicates for optional steps. + * + * Guards that optional steps (labels, custom-fields, webhook) only show + * green check marks after the required steps (credentials + board + status + * mapping) are all complete. Prevents the UI bug where a brand-new + * unconfigured integration showed every step as green. + */ + +import { describe, expect, it } from 'vitest'; +import { trelloProviderWizard } from '../../../web/src/components/projects/pm-providers/trello/wizard.js'; +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; + +// Grab the optional steps by id +const getStep = (id: string) => { + const step = trelloProviderWizard.steps.find((s) => s.id === id); + if (!step) throw new Error(`Step ${id} not found`); + return step; +}; + +describe('Trello optional steps — isComplete gating', () => { + describe('fresh state (createInitialState)', () => { + const state = createInitialState(); + + it('trello-labels is NOT complete on fresh state', () => { + expect(getStep('trello-labels').isComplete(state)).toBe(false); + }); + + it('trello-custom-fields is NOT complete on fresh state', () => { + expect(getStep('trello-custom-fields').isComplete(state)).toBe(false); + }); + + it('trello-webhook is NOT complete on fresh state', () => { + expect(getStep('trello-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('partially configured (credentials only, no board)', () => { + const state = { + ...createInitialState(), + trelloApiKey: 'key123', + trelloToken: 'token123', + verificationResult: { provider: 'trello' as const, display: 'user@example.com' }, + }; + + it('trello-labels is NOT complete when board not selected', () => { + expect(getStep('trello-labels').isComplete(state)).toBe(false); + }); + + it('trello-custom-fields is NOT complete when board not selected', () => { + expect(getStep('trello-custom-fields').isComplete(state)).toBe(false); + }); + + it('trello-webhook is NOT complete when board not selected', () => { + expect(getStep('trello-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('credentials + board, but no status mapping', () => { + const state = { + ...createInitialState(), + trelloApiKey: 'key123', + trelloToken: 'token123', + verificationResult: { provider: 'trello' as const, display: 'user@example.com' }, + trelloBoardId: 'board-1', + trelloListMappings: {}, + }; + + it('trello-labels is NOT complete without status mapping', () => { + expect(getStep('trello-labels').isComplete(state)).toBe(false); + }); + + it('trello-webhook is NOT complete without status mapping', () => { + expect(getStep('trello-webhook').isComplete(state)).toBe(false); + }); + }); + + describe('fully configured (credentials + board + status mapping)', () => { + const state = { + ...createInitialState(), + trelloApiKey: 'key123', + trelloToken: 'token123', + verificationResult: { provider: 'trello' as const, display: 'user@example.com' }, + trelloBoardId: 'board-1', + trelloListMappings: { todo: 'list-1', inProgress: 'list-2' }, + }; + + it('trello-labels is complete when all required steps done', () => { + expect(getStep('trello-labels').isComplete(state)).toBe(true); + }); + + it('trello-custom-fields is complete when all required steps done', () => { + expect(getStep('trello-custom-fields').isComplete(state)).toBe(true); + }); + + it('trello-webhook is complete when all required steps done', () => { + expect(getStep('trello-webhook').isComplete(state)).toBe(true); + }); + }); + + describe('edit mode with stored credentials (isEditing + hasStoredCredentials)', () => { + const state = { + ...createInitialState(), + isEditing: true, + hasStoredCredentials: true, + trelloBoardId: 'board-1', + trelloListMappings: { todo: 'list-1' }, + }; + + it('trello-labels is complete in edit mode with stored credentials', () => { + expect(getStep('trello-labels').isComplete(state)).toBe(true); + }); + + it('trello-custom-fields is complete in edit mode with stored credentials', () => { + expect(getStep('trello-custom-fields').isComplete(state)).toBe(true); + }); + + it('trello-webhook is complete in edit mode with stored credentials', () => { + expect(getStep('trello-webhook').isComplete(state)).toBe(true); + }); + }); +}); diff --git a/web/src/components/projects/pm-providers/jira/wizard.ts b/web/src/components/projects/pm-providers/jira/wizard.ts index 62693db8..ae40e874 100644 --- a/web/src/components/projects/pm-providers/jira/wizard.ts +++ b/web/src/components/projects/pm-providers/jira/wizard.ts @@ -80,6 +80,25 @@ function isCredentialsComplete(state: { ); } +/** + * Returns true when all required JIRA steps are done: + * credentials + project selected + at least one status mapping. + * Used to gate optional step `isComplete` predicates so they only show + * green after the integration is actually configured. + */ +function areJiraRequiredStepsDone( + state: Parameters[0] & { + jiraProjectKey: string; + jiraStatusMappings: Record; + }, +): boolean { + return ( + isCredentialsComplete(state) && + Boolean(state.jiraProjectKey) && + Object.keys(state.jiraStatusMappings).length > 0 + ); +} + interface JiraProviderHooks { readonly projectOptions: ReadonlyArray<{ readonly id: string; readonly name: string }>; readonly projectsLoading: boolean; @@ -250,25 +269,25 @@ export const jiraProviderWizard: ProviderWizardDefinition = { id: 'jira-labels', title: 'Labels', Component: JiraLabelMappingAdapter, - isComplete: () => true, // labels are optional + isComplete: (state) => areJiraRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'jira-custom-fields', title: 'Custom fields', Component: JiraCustomFieldMappingAdapter, - isComplete: () => true, // cost field optional + isComplete: (state) => areJiraRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'jira-issue-types', title: 'Issue types', Component: JiraIssueTypeAdapter, - isComplete: () => true, // issue-type mapping optional + isComplete: (state) => areJiraRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'jira-webhook', title: 'Webhook', Component: JiraWebhookAdapter, - isComplete: () => true, + isComplete: (state) => areJiraRequiredStepsDone(state), }, ], diff --git a/web/src/components/projects/pm-providers/linear/wizard.ts b/web/src/components/projects/pm-providers/linear/wizard.ts index 2d42755c..fc0369fe 100644 --- a/web/src/components/projects/pm-providers/linear/wizard.ts +++ b/web/src/components/projects/pm-providers/linear/wizard.ts @@ -83,6 +83,25 @@ function isCredentialsComplete(state: { return Boolean(state.linearApiKey && state.verificationResult); } +/** + * Returns true when all required Linear steps are done: + * credentials + team selected + at least one status mapping. + * Used to gate optional step `isComplete` predicates so they only show + * green after the integration is actually configured. + */ +function areLinearRequiredStepsDone( + state: Parameters[0] & { + linearTeamId: string; + linearStatusMappings: Record; + }, +): boolean { + return ( + isCredentialsComplete(state) && + Boolean(state.linearTeamId) && + Object.keys(state.linearStatusMappings).length > 0 + ); +} + interface LinearProviderHooks { readonly teamOptions: ReadonlyArray<{ readonly id: string; @@ -232,19 +251,19 @@ export const linearProviderWizard: ProviderWizardDefinition = { id: 'linear-labels', title: 'Labels', Component: LinearLabelMappingAdapter, - isComplete: () => true, // labels optional + isComplete: (state) => areLinearRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'linear-project-scope', title: 'Project scope', Component: LinearProjectScopeAdapter, - isComplete: () => true, // optional narrowing + isComplete: (state) => areLinearRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'linear-webhook', title: 'Webhook', Component: LinearWebhookAdapter, - isComplete: () => true, + isComplete: (state) => areLinearRequiredStepsDone(state), }, ], diff --git a/web/src/components/projects/pm-providers/trello/wizard.ts b/web/src/components/projects/pm-providers/trello/wizard.ts index 7c34f795..487aec7d 100644 --- a/web/src/components/projects/pm-providers/trello/wizard.ts +++ b/web/src/components/projects/pm-providers/trello/wizard.ts @@ -84,6 +84,25 @@ function isCredentialsComplete(state: { return Boolean(state.trelloApiKey && state.trelloToken && state.verificationResult); } +/** + * Returns true when all required Trello steps are done: + * credentials + board selected + at least one list mapping. + * Used to gate optional step `isComplete` predicates so they only show + * green after the integration is actually configured. + */ +function areTrelloRequiredStepsDone( + state: Parameters[0] & { + trelloBoardId: string; + trelloListMappings: Record; + }, +): boolean { + return ( + isCredentialsComplete(state) && + Boolean(state.trelloBoardId) && + Object.keys(state.trelloListMappings).length > 0 + ); +} + /** * The shape returned by `useProviderHooks`. Each step adapter pulls the * slice it needs from this record. Ports all the mutations + memoized @@ -249,19 +268,19 @@ export const trelloProviderWizard: ProviderWizardDefinition = { id: 'trello-labels', title: 'Label mapping', Component: TrelloLabelMappingAdapter, - isComplete: () => true, // labels are optional + isComplete: (state) => areTrelloRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'trello-custom-fields', title: 'Custom fields', Component: TrelloCustomFieldMappingAdapter, - isComplete: () => true, // cost field is optional + isComplete: (state) => areTrelloRequiredStepsDone(state), // optional, but only green after required steps }, { id: 'trello-webhook', title: 'Webhook', Component: TrelloWebhookAdapter, - isComplete: () => true, + isComplete: (state) => areTrelloRequiredStepsDone(state), }, ], From 0e4a08ecb7d02f7d859bd4ce36249d1137d5f18d Mon Sep 17 00:00:00 2001 From: aaight Date: Thu, 23 Apr 2026 22:22:26 +0200 Subject: [PATCH 2/5] fix(triggers): widen evaluateAuthorMode to recognize reviewer persona as 'own' (#1173) Co-authored-by: Cascade Bot --- src/triggers/github/check-suite-success.ts | 2 +- src/triggers/github/pr-opened.ts | 2 +- src/triggers/github/utils.ts | 17 ++- .../unit/triggers/check-suite-success.test.ts | 62 ++++++++++ tests/unit/triggers/github-utils.test.ts | 109 ++++++++++++++++++ tests/unit/triggers/pr-opened.test.ts | 35 +++++- 6 files changed, 219 insertions(+), 8 deletions(-) diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 2ca5e375..a77e8ffb 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -138,7 +138,7 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { handler: this.name, prNumber, prAuthor: prDetails.user.login, - isImplementerPR: authorResult.isImplementerPR, + isCascadePR: authorResult.isCascadePR, authorMode: authorResult.authorMode, }); return null; diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index ad674a01..edb90908 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -67,7 +67,7 @@ export class PROpenedTrigger implements TriggerHandler { handler: this.name, prNumber, prAuthor, - isImplementerPR: authorResult.isImplementerPR, + isCascadePR: authorResult.isCascadePR, authorMode: authorResult.authorMode, }); return null; diff --git a/src/triggers/github/utils.ts b/src/triggers/github/utils.ts index 9bcda4bb..f5492bef 100644 --- a/src/triggers/github/utils.ts +++ b/src/triggers/github/utils.ts @@ -1,11 +1,12 @@ import { lookupWorkItemForPR } from '../../db/repositories/prWorkItemsRepository.js'; +import type { PersonaIdentities } from '../../github/personas.js'; import type { ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; export interface AuthorModeResult { shouldTrigger: boolean; authorMode: string; - isImplementerPR: boolean; + isCascadePR: boolean; } /** @@ -14,10 +15,13 @@ export interface AuthorModeResult { * * Returns `null` when personaIdentities is missing (caller should return null). * Validates authorMode against known values and falls back to 'own'. + * + * "own" means the PR was authored by any CASCADE persona (implementer OR reviewer). + * This aligns with `isCascadeBot()` which already checks both personas. */ export function evaluateAuthorMode( prAuthorLogin: string, - personaIdentities: { implementer: string } | undefined, + personaIdentities: PersonaIdentities | undefined, parameters: Record, handlerName: string, ): AuthorModeResult | null { @@ -26,7 +30,10 @@ export function evaluateAuthorMode( return null; } const implLogin = personaIdentities.implementer; + const reviewerLogin = personaIdentities.reviewer; const isImplementerPR = prAuthorLogin === implLogin || prAuthorLogin === `${implLogin}[bot]`; + const isReviewerPR = prAuthorLogin === reviewerLogin || prAuthorLogin === `${reviewerLogin}[bot]`; + const isCascadePR = isImplementerPR || isReviewerPR; const rawMode = parameters.authorMode; const authorMode = @@ -41,10 +48,10 @@ export function evaluateAuthorMode( const shouldTrigger = authorMode === 'all' || - (authorMode === 'own' && isImplementerPR) || - (authorMode === 'external' && !isImplementerPR); + (authorMode === 'own' && isCascadePR) || + (authorMode === 'external' && !isCascadePR); - return { shouldTrigger, authorMode, isImplementerPR }; + return { shouldTrigger, authorMode, isCascadePR }; } /** diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index b5390c5f..54e6cfdf 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -880,6 +880,68 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result).toBeNull(); }); + it('triggers when PR authored by reviewer persona and authorMode=own', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'own' }, + }); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Reviewer persona PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/reviewer-authored', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-reviewer' }, + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('review'); + }); + + it('skips reviewer persona PR when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Reviewer persona PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/reviewer-authored', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-reviewer' }, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + it('triggers for both authors when authorMode=all', async () => { vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, diff --git a/tests/unit/triggers/github-utils.test.ts b/tests/unit/triggers/github-utils.test.ts index 9dfca8ca..e9568378 100644 --- a/tests/unit/triggers/github-utils.test.ts +++ b/tests/unit/triggers/github-utils.test.ts @@ -5,7 +5,9 @@ vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import type { PersonaIdentities } from '../../../src/github/personas.js'; import { + evaluateAuthorMode, extractJiraIssueKey, extractTrelloCardId, extractWorkItemId, @@ -200,3 +202,110 @@ describe('parsePrNumberFromRef', () => { expect(parsePrNumberFromRef('pull/42/head')).toBeNull(); }); }); + +describe('evaluateAuthorMode', () => { + const personas: PersonaIdentities = { + implementer: 'cascade-impl', + reviewer: 'cascade-reviewer', + }; + + it('returns null when personaIdentities is undefined', () => { + const result = evaluateAuthorMode('some-user', undefined, {}, 'test-handler'); + expect(result).toBeNull(); + }); + + it('returns shouldTrigger:true + isCascadePR:true for implementer login when authorMode=own', () => { + const result = evaluateAuthorMode('cascade-impl', personas, { authorMode: 'own' }, 'handler'); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:true + isCascadePR:true for reviewer login when authorMode=own (core bug regression)', () => { + const result = evaluateAuthorMode( + 'cascade-reviewer', + personas, + { authorMode: 'own' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:true + isCascadePR:true for implementer[bot] variant when authorMode=own', () => { + const result = evaluateAuthorMode( + 'cascade-impl[bot]', + personas, + { authorMode: 'own' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:true + isCascadePR:true for reviewer[bot] variant when authorMode=own', () => { + const result = evaluateAuthorMode( + 'cascade-reviewer[bot]', + personas, + { authorMode: 'own' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:false for external author when authorMode=own', () => { + const result = evaluateAuthorMode('external-dev', personas, { authorMode: 'own' }, 'handler'); + expect(result).toEqual({ shouldTrigger: false, authorMode: 'own', isCascadePR: false }); + }); + + it('returns shouldTrigger:true for external author when authorMode=external', () => { + const result = evaluateAuthorMode( + 'external-dev', + personas, + { authorMode: 'external' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'external', isCascadePR: false }); + }); + + it('returns shouldTrigger:false for implementer when authorMode=external', () => { + const result = evaluateAuthorMode( + 'cascade-impl', + personas, + { authorMode: 'external' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: false, authorMode: 'external', isCascadePR: true }); + }); + + it('returns shouldTrigger:false for reviewer when authorMode=external (second regression test)', () => { + const result = evaluateAuthorMode( + 'cascade-reviewer', + personas, + { authorMode: 'external' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: false, authorMode: 'external', isCascadePR: true }); + }); + + it('returns shouldTrigger:true for any author when authorMode=all', () => { + for (const login of ['cascade-impl', 'cascade-reviewer', 'external-dev']) { + const result = evaluateAuthorMode(login, personas, { authorMode: 'all' }, 'handler'); + expect(result?.shouldTrigger).toBe(true); + expect(result?.authorMode).toBe('all'); + } + }); + + it('falls back to "own" when authorMode is an invalid string', () => { + const result = evaluateAuthorMode( + 'cascade-impl', + personas, + { authorMode: 'invalid' }, + 'handler', + ); + expect(result?.authorMode).toBe('own'); + expect(result?.shouldTrigger).toBe(true); + }); + + it('falls back to "own" when authorMode is missing from parameters', () => { + const result = evaluateAuthorMode('cascade-impl', personas, {}, 'handler'); + expect(result?.authorMode).toBe('own'); + expect(result?.shouldTrigger).toBe(true); + }); +}); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index 7e9dcfe0..35f67b7d 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -417,7 +417,7 @@ describe('PROpenedTrigger', () => { expect(result?.agentType).toBe('review'); }); - it('fires for reviewer persona PR when authorMode=external (reviewer is not implementer)', async () => { + it('skips reviewer persona PR when authorMode=external (reviewer is own)', async () => { vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ enabled: true, parameters: { authorMode: 'external' }, @@ -446,6 +446,39 @@ describe('PROpenedTrigger', () => { }, }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + + it('fires for reviewer persona PR when authorMode=own', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'own' }, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'feat: add login', + body: 'Implements feature', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/login', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'cascade-review' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'cascade-review' }, + }, + }; + const result = await trigger.handle(ctx); expect(result).not.toBeNull(); expect(result?.agentType).toBe('review'); From 3e555f95397d188ed73380a89ad1b376d4709937 Mon Sep 17 00:00:00 2001 From: aaight Date: Thu, 23 Apr 2026 23:25:36 +0200 Subject: [PATCH 3/5] feat(engine-ux): redesign Engine page with harness-first layout and integrated model (#1175) * feat(engine-ux): redesign Engine page with harness-first layout and integrated model * fix(engine-ux): hide model and maxIterations on non-default engine tabs Model and maxIterations are project-level settings that apply only to the default engine. Rendering their inputs on every tab created a data corruption risk: a user editing these fields while viewing a non-default tab would save incompatible engine/model combinations (e.g. agentEngine=claude-code but model=gpt-4o). Both sections are now wrapped in {isDefault && (...)} so they only appear on the default engine's tab. Co-Authored-By: Claude Sonnet 4.6 * fix(engine-ux): fix activeTab state out-of-sync with async defaultsQuery Initialize activeTab as null so it reactively follows effectiveEngineId until the user manually switches tabs. This prevents the mismatch where defaultsQuery loads after initial render and reveals a different system default engine than the one useState initialized with. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Cascade Bot Co-authored-by: Claude Sonnet 4.6 --- .../projects/project-harness-form.tsx | 386 +++++++++--------- 1 file changed, 193 insertions(+), 193 deletions(-) diff --git a/web/src/components/projects/project-harness-form.tsx b/web/src/components/projects/project-harness-form.tsx index b895cc35..abf65152 100644 --- a/web/src/components/projects/project-harness-form.tsx +++ b/web/src/components/projects/project-harness-form.tsx @@ -17,6 +17,13 @@ import { } from '@/components/ui/card.js'; import { Input } from '@/components/ui/input.js'; import { Label } from '@/components/ui/label.js'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select.js'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs.js'; import { Tooltip, @@ -74,8 +81,9 @@ export function ProjectHarnessForm({ project }: { project: Project }) { // The effective project-level engine: either explicitly set or the system default const effectiveEngineId = agentEngine || systemDefaultEngineId; - // Default tab to show: project's selected engine, or system default - const defaultTab = effectiveEngineId; + // Controlled active tab — null means "follow effectiveEngineId reactively" (handles async defaultsQuery) + const [activeTab, setActiveTab] = useState(null); + const currentTab = activeTab ?? effectiveEngineId; // Resolved engine defaults for EngineSettingsFields function getEngineDefaults(engineId: string): Record | undefined { @@ -84,6 +92,14 @@ export function ProjectHarnessForm({ project }: { project: Project }) { : undefined; } + function handleEngineSelectChange(value: string) { + const newEngine = value === '_system' ? '' : value; + setAgentEngine(newEngine); + // Switch active tab to the newly selected default engine + const newEffective = newEngine || systemDefaultEngineId; + setActiveTab(newEffective); + } + function handleSubmit(e: React.FormEvent) { e.preventDefault(); const activeEngine = agentEngine || null; @@ -107,224 +123,208 @@ export function ProjectHarnessForm({ project }: { project: Project }) {

- {/* Model & Iterations Card — engine-agnostic, always visible */} - Model & Runtime + Engine - Global model and iteration settings applied to all agents unless overridden per-agent. + Choose the default engine, then configure its model, settings, and credentials. -
-
-
- - - - - - - Individual agents can override this in the Agents tab. - - -
- -

- Project default model. Per-agent overrides in the Agents tab. -

-
+ + {/* Default Engine Selector */}
-
- - - - - - - Individual agents can override this in the Agents tab. - - -
- setMaxIterations(e.target.value)} - placeholder={defaults ? `${defaults.maxIterations} (default)` : 'e.g. 50'} - /> + + {engines.length === 0 ? ( +

Loading engines…

+ ) : ( + + )}

- Safety limit on tool-call iterations per run. + Used by all agents unless overridden per-agent.

-
-
-
- {/* Per-engine tabs: credentials + settings + default toggle */} - - - Engine Settings & Credentials - - Configure each engine's credentials and settings. The default engine tab is - highlighted. New engines are added automatically as the catalog expands. - - - - {engines.length === 0 ? ( -

Loading engines…

- ) : ( - - + {/* Per-engine configuration tabs */} + {engines.length > 0 && ( + + + {engines.map((engine) => { + const isDefault = engine.id === effectiveEngineId; + const isUsedByAgents = agentEnginesInUse.includes(engine.id); + return ( + + {engine.label} + {isDefault && ( + + Default + + )} + {!isDefault && isUsedByAgents && ( + + In use + + )} + + ); + })} + + {engines.map((engine) => { const isDefault = engine.id === effectiveEngineId; - const isUsedByAgents = agentEnginesInUse.includes(engine.id); - return ( - - {engine.label} - {isDefault && ( - - Default - - )} - {!isDefault && isUsedByAgents && ( - - In use - - )} - + const engineSecrets = ENGINE_SECRETS.filter((s) => + s.engines?.includes(engine.id), ); - })} - - - {engines.map((engine) => { - const isDefault = engine.id === effectiveEngineId; - const isUsedByAgents = agentEnginesInUse.includes(engine.id); - const engineSecrets = ENGINE_SECRETS.filter((s) => - s.engines?.includes(engine.id), - ); - // Secrets shared with other engines: show a note - const sharedSecretEngines = (envVarKey: string): string[] => { - const secret = ENGINE_SECRETS.find((s) => s.envVarKey === envVarKey); - if (!secret?.engines) return []; - return secret.engines.filter((e) => e !== engine.id); - }; + const sharedSecretEngines = (envVarKey: string): string[] => { + const secret = ENGINE_SECRETS.find((s) => s.envVarKey === envVarKey); + if (!secret?.engines) return []; + return secret.engines.filter((e) => e !== engine.id); + }; + const engineDefaults = getEngineDefaults(engine.id); - const engineDefaults = getEngineDefaults(engine.id); - - return ( - - {/* Engine description */} - {engine.description && ( -

{engine.description}

- )} + return ( + + {/* Engine description */} + {engine.description && ( +

{engine.description}

+ )} - {/* Default engine indicator / Set as Default button */} -
- {isDefault ? ( -
- - ✓ Default engine for this project - {agentEngine === '' && - ` (inheriting system default: ${capitalize(systemDefaultEngineId)})`} - - {agentEngine !== '' && ( - - )} + {/* Model — only shown for the default engine (project-level setting) */} + {isDefault && ( +
+
+ + + + + + + Individual agents can override this in the Agents tab. + + +
+ +

+ Project default. Per-agent overrides in the Agents tab. +

- ) : ( - - )} - {!isDefault && isUsedByAgents && ( - - Used by agent config overrides - )} -
- {/* Engine settings */} - setEngineSettings(next ?? {})} - engineDefaults={engineDefaults} - /> + {/* Engine Settings */} + setEngineSettings(next ?? {})} + engineDefaults={engineDefaults} + /> - {/* Engine credentials */} - {engineSecrets.length > 0 ? ( -
-
-

Credentials

-

- API keys and tokens for {engine.label}. Values are stored encrypted - and never returned to the browser. + {/* Max Iterations — only shown for the default engine (project-level setting) */} + {isDefault && ( +

+
+ + + + + + + Individual agents can override this in the Agents tab. + + +
+ setMaxIterations(e.target.value)} + placeholder={ + defaults ? `${defaults.maxIterations} (default)` : 'e.g. 50' + } + /> +

+ Safety limit on tool-call iterations per run.

- {engineSecrets.map((secret) => { - const sharedWith = sharedSecretEngines(secret.envVarKey); - const sharedNote = - sharedWith.length > 0 - ? `Also used by: ${sharedWith.map((id) => engines.find((e) => e.id === id)?.label ?? id).join(', ')}` - : undefined; - const description = - secret.description + (sharedNote ? ` · ${sharedNote}` : ''); - return ( - c.envVarKey === secret.envVarKey, - )} - /> - ); - })} -
- ) : ( -

- No credentials required for {engine.label}. -

- )} - - ); - })} - - )} + )} + + {/* Credentials */} + {engineSecrets.length > 0 ? ( +
+
+

Credentials

+

+ API keys and tokens for {engine.label}. Values are stored encrypted + and never returned to the browser. +

+
+ {engineSecrets.map((secret) => { + const sharedWith = sharedSecretEngines(secret.envVarKey); + const sharedNote = + sharedWith.length > 0 + ? `Also used by: ${sharedWith.map((id) => engines.find((e) => e.id === id)?.label ?? id).join(', ')}` + : undefined; + const description = + secret.description + (sharedNote ? ` · ${sharedNote}` : ''); + return ( + c.envVarKey === secret.envVarKey, + )} + /> + ); + })} +
+ ) : ( +

+ No credentials required for {engine.label}. +

+ )} + + ); + })} + + )} +