Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/router/adapters/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,21 @@ export class GitHubRouterAdapter implements RouterPlatformAdapter {
if (this.cachedPersonas !== undefined) return this.cachedPersonas;
try {
this.cachedPersonas = await resolvePersonaIdentities(projectId);
} catch {
// Resolution may fail — leave cachedPersonas undefined so callers can decide
} catch (err) {
// Surface the failure: every downstream gate that depends on
// personaIdentities will silently skip without these logs. Closes
// the 2026-04-29 incident class where ucho/PR#155's check_suite
// failure trigger could have been suppressed by an invisible
// token-resolution error.
logger.error('Failed to resolve cascade persona identities', {
projectId,
error: String(err),
});
captureException(err, {
tags: { source: 'persona_identity_resolution_failed' },
extra: { projectId },
});
// leave cachedPersonas undefined so callers can decide
}
return this.cachedPersonas;
}
Expand Down
19 changes: 19 additions & 0 deletions src/router/webhook-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,25 @@ export async function processRouterWebhook(
};
}

// Structured skip — a matched handler ran but bailed (e.g. precondition
// unmet, dedup claim lost, PR not from cascade persona). Surface the
// handler-specific reason in webhook log decisionReason so operators can
// triage from the dashboard without trawling cascade-router process logs.
if (result.skipReason && result.agentType === null) {
logger.info(`${adapter.type} trigger self-skipped`, {
handler: result.skipReason.handler,
message: result.skipReason.message,
eventType: event.eventType,
workItemId: event.workItemId,
projectId: project.id,
});
return {
shouldProcess: true,
projectId: project.id,
decisionReason: `Trigger ${result.skipReason.handler} skipped: ${result.skipReason.message}`,
};
}

logger.info(`${adapter.type} trigger matched`, {
agentType: result.agentType || '(no agent)',
workItemId: event.workItemId,
Expand Down
66 changes: 51 additions & 15 deletions src/triggers/github/check-suite-failure.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
import { githubClient } from '../../github/client.js';
import { isCascadeBot } from '../../github/personas.js';
import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js';
import { logger } from '../../utils/logging.js';
import { parseRepoFullName } from '../../utils/repo.js';
import { checkTriggerEnabled } from '../shared/trigger-check.js';
import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js';
import { parsePrNumberFromRef, resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js';

/**
* Build a structured skip result so the router's webhook log decisionReason
* surfaces the real reason this handler bailed (instead of the generic
* "No trigger matched for event"). See `TriggerResult.skipReason`.
*/
function skip(handlerName: string, message: string): TriggerResult {
return {
agentType: null,
agentInput: {},
skipReason: { handler: handlerName, message },
};
}

/**
* Resolve a PR number from a check_suite payload.
* Tries pull_requests[], then refs/pull/{N}/head parsing, then a GitHub API lookup by branch.
Expand Down Expand Up @@ -75,7 +89,7 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
this.name,
))
) {
return null;
return skip(this.name, 'respond-to-ci trigger is disabled for this project');
}

const payload = ctx.payload as GitHubCheckSuitePayload;
Expand All @@ -89,24 +103,33 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
payload.check_suite.head_branch,
this.name,
);
if (prNumber === null) return null;
if (prNumber === null) {
return skip(this.name, 'Could not resolve PR number from check_suite payload');
}
const headSha = payload.check_suite.head_sha;

// Fetch PR details
const prDetails = await githubClient.getPR(owner, repo, prNumber);

// Gate on PR author being the implementer persona
// Gate on PR author being a cascade persona (implementer OR reviewer).
// Loop-prevention: only auto-fix CI on PRs authored by the bot personas;
// human-authored PRs are owned by the human.
if (!ctx.personaIdentities) {
logger.info('No persona identities available, skipping', { handler: this.name, prNumber });
return null;
return skip(
this.name,
'Cascade persona identities could not be resolved (token / GitHub API issue)',
);
}
const implLogin = ctx.personaIdentities.implementer;
if (prDetails.user.login !== implLogin && prDetails.user.login !== `${implLogin}[bot]`) {
logger.info('PR not authored by implementer persona, skipping check failure trigger', {
if (!isCascadeBot(prDetails.user.login, ctx.personaIdentities)) {
logger.info('PR not authored by a cascade persona, skipping check failure trigger', {
prNumber,
prAuthor: prDetails.user.login,
});
return null;
return skip(
this.name,
`PR #${prNumber} not authored by a cascade persona (author: ${prDetails.user.login})`,
);
}

// Only trigger for PRs targeting the project's base branch
Expand All @@ -116,7 +139,10 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
baseRef: prDetails.baseRef,
projectBaseBranch: ctx.project.baseBranch,
});
return null;
return skip(
this.name,
`PR #${prNumber} targets ${prDetails.baseRef}, not project base branch ${ctx.project.baseBranch}`,
);
}

// Resolve work item from DB
Expand All @@ -129,14 +155,18 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
// Verify ALL checks have completed (not still running)
const allComplete = checkStatus.checkRuns.every((cr) => cr.status === 'completed');
if (!allComplete) {
const incomplete = checkStatus.checkRuns
.filter((cr) => cr.status !== 'completed')
.map((cr) => cr.name);
logger.info('Not all checks complete yet, waiting', {
prNumber,
totalChecks: checkStatus.totalCount,
incompleteChecks: checkStatus.checkRuns
.filter((cr) => cr.status !== 'completed')
.map((cr) => cr.name),
incompleteChecks: incomplete,
});
return null;
return skip(
this.name,
`Not all checks complete yet (${incomplete.length}/${checkStatus.totalCount} still running): ${incomplete.join(', ')}`,
);
}

// Verify at least one check failed
Expand All @@ -152,7 +182,10 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
prNumber,
totalChecks: checkStatus.totalCount,
});
return null;
return skip(
this.name,
`All ${checkStatus.totalCount} checks passed for PR #${prNumber} — no action needed`,
);
}

// Check attempt limit to prevent infinite loops
Expand All @@ -168,7 +201,10 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
prNumber,
'⚠️ Unable to automatically fix failing checks after 3 attempts. Manual intervention required.',
);
return null;
return skip(
this.name,
`Max auto-fix attempts (${MAX_ATTEMPTS}) reached for PR #${prNumber} — manual intervention required`,
);
}

// Increment attempt counter
Expand Down
47 changes: 36 additions & 11 deletions src/triggers/github/pr-conflict-detected.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import { githubClient } from '../../github/client.js';
import { isCascadeBot } from '../../github/personas.js';
import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js';
import { logger } from '../../utils/logging.js';
import { parseRepoFullName } from '../../utils/repo.js';
import { checkTriggerEnabled } from '../shared/trigger-check.js';
import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js';
import { resolveWorkItemId } from './utils.js';

function skip(handlerName: string, message: string): TriggerResult {
return {
agentType: null,
agentInput: {},
skipReason: { handler: handlerName, message },
};
}

// Track conflict resolution attempts per PR to prevent infinite loops
const conflictAttempts = new Map<number, number>();
const MAX_ATTEMPTS = 2;
Expand Down Expand Up @@ -44,30 +53,37 @@ export class PRConflictDetectedTrigger implements TriggerHandler {
this.name,
))
) {
return null;
return skip(this.name, 'resolve-conflicts trigger is disabled for this project');
}

const payload = ctx.payload as GitHubPullRequestPayload;
const prNumber = payload.pull_request.number;
const repoFullName = payload.repository.full_name;
const { owner, repo } = parseRepoFullName(repoFullName);

// Gate on PR author being the implementer persona
// Gate on PR author being a cascade persona (implementer OR reviewer).
// Loop-prevention: only auto-resolve conflicts on PRs authored by bot
// personas; human-authored PRs are owned by the human.
if (!ctx.personaIdentities) {
logger.info('No persona identities available, skipping', {
handler: this.name,
prNumber,
});
return null;
return skip(
this.name,
'Cascade persona identities could not be resolved (token / GitHub API issue)',
);
}
const implLogin = ctx.personaIdentities.implementer;
const prAuthorLogin = payload.pull_request.user.login;
if (prAuthorLogin !== implLogin && prAuthorLogin !== `${implLogin}[bot]`) {
logger.info('PR not authored by implementer persona, skipping conflict detection trigger', {
if (!isCascadeBot(prAuthorLogin, ctx.personaIdentities)) {
logger.info('PR not authored by a cascade persona, skipping conflict detection trigger', {
prNumber,
prAuthor: prAuthorLogin,
});
return null;
return skip(
this.name,
`PR #${prNumber} not authored by a cascade persona (author: ${prAuthorLogin})`,
);
}

// Only trigger for PRs targeting the project's base branch
Expand All @@ -77,7 +93,10 @@ export class PRConflictDetectedTrigger implements TriggerHandler {
baseRef: payload.pull_request.base.ref,
projectBaseBranch: ctx.project.baseBranch,
});
return null;
return skip(
this.name,
`PR #${prNumber} targets ${payload.pull_request.base.ref}, not project base branch ${ctx.project.baseBranch}`,
);
}

// Fetch PR details, retrying if mergeable is null (GitHub computes it asynchronously)
Expand All @@ -101,13 +120,16 @@ export class PRConflictDetectedTrigger implements TriggerHandler {
logger.info('mergeable still null after retries, skipping conflict detection trigger', {
prNumber,
});
return null;
return skip(
this.name,
`mergeable still null after ${MERGEABLE_RETRY_COUNT} retries for PR #${prNumber} — cannot determine mergeability`,
);
}

// Only fire if PR is unmergeable (has conflicts)
if (prDetails.mergeable !== false) {
logger.debug('PR is mergeable, no conflict detected', { prNumber });
return null;
return skip(this.name, `PR #${prNumber} is mergeable — no conflict detected`);
}

// Check attempt limit to prevent infinite loops
Expand All @@ -123,7 +145,10 @@ export class PRConflictDetectedTrigger implements TriggerHandler {
prNumber,
'⚠️ Unable to automatically resolve merge conflicts after 2 attempts. Manual intervention required.',
);
return null;
return skip(
this.name,
`Max conflict resolution attempts (${MAX_ATTEMPTS}) reached for PR #${prNumber} — manual intervention required`,
);
}

// Increment attempt counter
Expand Down
18 changes: 18 additions & 0 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,24 @@ export interface TriggerResult {
* Typical key: `${projectId}:${workItemId}`.
*/
coalesceKey?: string;
/**
* Structured self-skip signal — set when a matched handler decided NOT to
* dispatch an agent (e.g. all-checks-not-complete, attempt limit hit, PR
* not authored by a cascade persona). `agentType` MUST be `null` when
* `skipReason` is set.
*
* The router promotes `skipReason.message` into the persisted webhook log's
* `decisionReason` so operators can distinguish "no matcher matched" from
* "matcher claimed the event but bailed" without trawling cascade-router
* process logs. Bare `return null` from `handle()` keeps the legacy
* "try-next-handler" semantic; structured skips terminate dispatch.
*/
skipReason?: {
/** Name of the handler that produced the skip (e.g. `'check-suite-failure'`). */
handler: string;
/** Human-readable explanation. Used verbatim in webhook log decisionReason. */
message: string;
};
}

export interface TriggerHandler {
Expand Down
26 changes: 25 additions & 1 deletion tests/unit/router/webhook-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ describe('processRouterWebhook', () => {
expect(addJob).not.toHaveBeenCalled();
});

it('does not queue when dispatch returns null', async () => {
it('does not queue when dispatch returns null (no matcher matched)', async () => {
const adapter = makeMockAdapter({
dispatchWithCredentials: vi.fn().mockResolvedValue(null),
});
Expand All @@ -206,6 +206,30 @@ describe('processRouterWebhook', () => {
expect(addJob).not.toHaveBeenCalled();
});

// Diagnostic upgrade: a structured skip from a matched handler must be
// surfaced in decisionReason — distinct from "no matcher matched". Closes
// the 2026-04-29 ucho/PR#155 incident where check_suite-failure self-skipped
// for an unknown reason and webhooklogs only said "No trigger matched for
// event", forcing log-trawling instead of dashboard-level diagnosis.
it('surfaces handler-specific skipReason in decisionReason when a matched handler self-skipped', async () => {
const adapter = makeMockAdapter({
dispatchWithCredentials: vi.fn().mockResolvedValue({
agentType: null,
agentInput: {},
skipReason: {
handler: 'check-suite-failure',
message: 'Not all checks complete yet, waiting',
},
}),
});
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toBe(
'Trigger check-suite-failure skipped: Not all checks complete yet, waiting',
);
expect(addJob).not.toHaveBeenCalled();
});

it('queues job when dispatch returns a trigger result', async () => {
const triggerResult = { agentType: 'implementation', agentInput: { cardId: 'card1' } };
vi.mocked(addJob).mockResolvedValue('job-1');
Expand Down
Loading
Loading