From baa6e66b4b7782a1fcc3de2a4f8e7be037367c62 Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 19:30:47 +0200 Subject: [PATCH 1/9] feat(pm): add friction slot accessors (#1294) Co-authored-by: Cascade Bot --- src/integrations/pm/jira/config-schema.ts | 1 + src/integrations/pm/linear/config-schema.ts | 1 + src/integrations/pm/trello/config-schema.ts | 4 +- src/pm/config.ts | 51 +++++++ tests/unit/pm/config-friction-slot.test.ts | 143 ++++++++++++++++++++ 5 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 tests/unit/pm/config-friction-slot.test.ts diff --git a/src/integrations/pm/jira/config-schema.ts b/src/integrations/pm/jira/config-schema.ts index e4082e21f..d6630111d 100644 --- a/src/integrations/pm/jira/config-schema.ts +++ b/src/integrations/pm/jira/config-schema.ts @@ -49,6 +49,7 @@ export const jiraConfigSchema = z * * `cascadeAlert` — recognized label for alert work items (spec 019). * `statuses.alerts` is the recognized status key for the alerts slot. + * `statuses.friction` is the recognized status key for the friction report slot. */ labels: z .object({ diff --git a/src/integrations/pm/linear/config-schema.ts b/src/integrations/pm/linear/config-schema.ts index aa8a2bfd5..9d9605e6f 100644 --- a/src/integrations/pm/linear/config-schema.ts +++ b/src/integrations/pm/linear/config-schema.ts @@ -41,6 +41,7 @@ export const linearConfigSchema = z * * `cascadeAlert` — recognized label UUID for alert work items (spec 019). * `statuses.alerts` is the recognized status key for the alerts slot. + * `statuses.friction` is the recognized status key for the friction report slot. */ labels: z .object({ diff --git a/src/integrations/pm/trello/config-schema.ts b/src/integrations/pm/trello/config-schema.ts index 03b838419..d6ac2cf3a 100644 --- a/src/integrations/pm/trello/config-schema.ts +++ b/src/integrations/pm/trello/config-schema.ts @@ -28,7 +28,9 @@ export const trelloConfigSchema = z /** * Mapping from CASCADE status keys (backlog/todo/inProgress/done/...) * to Trello list IDs. Keys are provider-agnostic, values are provider-native. - * Recognized key: `alerts` — the list for incoming alert work items (spec 019). + * Recognized optional keys: + * - `alerts` — the list for incoming alert work items (spec 019). + * - `friction` — the list for incoming friction report work items. */ lists: z.record(z.string(), z.string()), diff --git a/src/pm/config.ts b/src/pm/config.ts index dc1393fa5..38cc1bbf9 100644 --- a/src/pm/config.ts +++ b/src/pm/config.ts @@ -134,6 +134,34 @@ export function getAlertsContainerId(project: ProjectConfig): string | undefined return undefined; } +/** + * Returns the container ID the PM adapter's `createWorkItem.containerId` expects + * for placing friction report work items: + * - Trello → `lists.friction` (the list ID the card will be created in) + * - JIRA → `projectKey` when `statuses.friction` is configured + * - Linear → `teamId` when `statuses.friction` is configured + * + * Returns `undefined` when the project has no PM config or the friction slot + * is not configured. + */ +export function getFrictionContainerId(project: ProjectConfig): string | undefined { + const pmType = project.pm?.type; + if (pmType === 'trello') { + return getTrelloConfig(project)?.lists?.friction; + } + if (pmType === 'jira') { + const jiraConfig = getJiraConfig(project); + if (!jiraConfig?.statuses?.friction) return undefined; + return jiraConfig.projectKey; + } + if (pmType === 'linear') { + const linearConfig = getLinearConfig(project); + if (!linearConfig?.statuses?.friction) return undefined; + return linearConfig.teamId; + } + return undefined; +} + /** * Returns the label identifier to apply to an alert work item: * - Trello → `labels['cascade-alert']` (Trello label ID) @@ -203,3 +231,26 @@ export function getAlertsStatusDestination(project: ProjectConfig): string | und } return undefined; } + +/** + * Returns the actual destination value to pass to `provider.moveWorkItem` for the + * friction slot: + * - Trello → `lists.friction` + * - JIRA → `statuses.friction` + * - Linear → `statuses.friction` + * + * Returns `undefined` when the friction slot is not configured. + */ +export function getFrictionStatusDestination(project: ProjectConfig): string | undefined { + const pmType = project.pm?.type; + if (pmType === 'trello') { + return getTrelloConfig(project)?.lists?.friction; + } + if (pmType === 'jira') { + return getJiraConfig(project)?.statuses?.friction; + } + if (pmType === 'linear') { + return getLinearConfig(project)?.statuses?.friction; + } + return undefined; +} diff --git a/tests/unit/pm/config-friction-slot.test.ts b/tests/unit/pm/config-friction-slot.test.ts new file mode 100644 index 000000000..150a5a927 --- /dev/null +++ b/tests/unit/pm/config-friction-slot.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it } from 'vitest'; +import { jiraConfigSchema } from '../../../src/integrations/pm/jira/config-schema.js'; +import { linearConfigSchema } from '../../../src/integrations/pm/linear/config-schema.js'; +import { trelloConfigSchema } from '../../../src/integrations/pm/trello/config-schema.js'; +import { getFrictionContainerId, getFrictionStatusDestination } from '../../../src/pm/config.js'; +import type { ProjectConfig } from '../../../src/types/index.js'; + +function makeTrelloProject(overrides: Record = {}): ProjectConfig { + return { + id: 'p1', + pm: { type: 'trello' }, + trello: { + boardId: 'board-1', + lists: { todo: 'list-todo', friction: 'list-friction' }, + labels: {}, + }, + ...overrides, + } as unknown as ProjectConfig; +} + +function makeJiraProject(overrides: Record = {}): ProjectConfig { + return { + id: 'p1', + pm: { type: 'jira' }, + jira: { + projectKey: 'PROJ', + baseUrl: 'https://acme.atlassian.net', + statuses: { todo: 'To Do', friction: 'Friction' }, + labels: {}, + }, + ...overrides, + } as unknown as ProjectConfig; +} + +function makeLinearProject(overrides: Record = {}): ProjectConfig { + return { + id: 'p1', + pm: { type: 'linear' }, + linear: { + teamId: 'team-1', + statuses: { todo: 'state-todo', friction: 'state-friction' }, + }, + ...overrides, + } as unknown as ProjectConfig; +} + +describe('getFrictionContainerId', () => { + it('returns Trello list ID from project.trello.lists.friction', () => { + expect(getFrictionContainerId(makeTrelloProject())).toBe('list-friction'); + }); + + it('returns JIRA project key only when statuses.friction is configured', () => { + expect(getFrictionContainerId(makeJiraProject())).toBe('PROJ'); + + const project = makeJiraProject({ + jira: { + projectKey: 'PROJ', + baseUrl: 'https://acme.atlassian.net', + statuses: { todo: 'To Do' }, + labels: {}, + }, + }); + expect(getFrictionContainerId(project)).toBeUndefined(); + }); + + it('returns Linear team ID only when statuses.friction is configured', () => { + expect(getFrictionContainerId(makeLinearProject())).toBe('team-1'); + + const project = makeLinearProject({ + linear: { teamId: 'team-1', statuses: { todo: 'state-todo' } }, + }); + expect(getFrictionContainerId(project)).toBeUndefined(); + }); + + it('returns undefined when no PM config or Trello friction list is present', () => { + expect( + getFrictionContainerId({ id: 'p1', pm: undefined } as unknown as ProjectConfig), + ).toBeUndefined(); + + const project = makeTrelloProject({ + trello: { boardId: 'board-1', lists: { todo: 'list-todo' }, labels: {} }, + }); + expect(getFrictionContainerId(project)).toBeUndefined(); + }); +}); + +describe('getFrictionStatusDestination', () => { + it('returns provider-native friction destination values', () => { + expect(getFrictionStatusDestination(makeTrelloProject())).toBe('list-friction'); + expect(getFrictionStatusDestination(makeJiraProject())).toBe('Friction'); + expect(getFrictionStatusDestination(makeLinearProject())).toBe('state-friction'); + }); + + it('returns undefined when friction is unconfigured', () => { + const trelloProject = makeTrelloProject({ + trello: { boardId: 'board-1', lists: { todo: 'list-todo' }, labels: {} }, + }); + const jiraProject = makeJiraProject({ + jira: { + projectKey: 'PROJ', + baseUrl: 'https://acme.atlassian.net', + statuses: { todo: 'To Do' }, + labels: {}, + }, + }); + const linearProject = makeLinearProject({ + linear: { teamId: 'team-1', statuses: { todo: 'state-todo' } }, + }); + + expect(getFrictionStatusDestination(trelloProject)).toBeUndefined(); + expect(getFrictionStatusDestination(jiraProject)).toBeUndefined(); + expect(getFrictionStatusDestination(linearProject)).toBeUndefined(); + }); +}); + +describe('PM config schemas — friction slot', () => { + it('trelloConfigSchema accepts lists.friction', () => { + const result = trelloConfigSchema.safeParse({ + boardId: 'b1', + lists: { friction: 'list-id-friction', todo: 'list-id-todo' }, + labels: {}, + }); + expect(result.success).toBe(true); + }); + + it('jiraConfigSchema accepts statuses.friction', () => { + const result = jiraConfigSchema.safeParse({ + projectKey: 'P', + baseUrl: 'https://acme.atlassian.net', + statuses: { friction: 'Friction', todo: 'To Do' }, + labels: {}, + }); + expect(result.success).toBe(true); + }); + + it('linearConfigSchema accepts statuses.friction', () => { + const result = linearConfigSchema.safeParse({ + teamId: 'team-1', + statuses: { friction: 'state-uuid-friction', todo: 'state-uuid-todo' }, + }); + expect(result.success).toBe(true); + }); +}); From 23fd00a2c70a3293ccb697425cb2c9e6cd977494 Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 19:44:27 +0200 Subject: [PATCH 2/9] feat(pm-wizard): add friction mapping slot (#1295) Co-authored-by: Cascade Bot --- tests/unit/web/wizard-friction-slot.test.ts | 104 ++++++++++++++++++ .../projects/pm-providers/jira/wizard.ts | 1 + .../projects/pm-providers/linear/wizard.ts | 1 + .../projects/pm-providers/trello/wizard.ts | 1 + 4 files changed, 107 insertions(+) create mode 100644 tests/unit/web/wizard-friction-slot.test.ts diff --git a/tests/unit/web/wizard-friction-slot.test.ts b/tests/unit/web/wizard-friction-slot.test.ts new file mode 100644 index 000000000..5877f44b0 --- /dev/null +++ b/tests/unit/web/wizard-friction-slot.test.ts @@ -0,0 +1,104 @@ +/** + * Regression pin: every PM provider wizard exposes an optional `friction` + * status-mapping slot. + * + * Tests cover: + * 1. Slot-array membership — the UI will render the input if the slot is present. + * 2. Round-trip through buildIntegrationConfig — the value persists to config. + * 3. Optionality — wizard completes without the friction slot being mapped. + */ + +import { describe, expect, it } from 'vitest'; +import { JIRA_STATUS_SLOTS } from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; +import { LINEAR_STATUS_SLOTS } from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; +import { TRELLO_LIST_SLOTS } from '../../../web/src/components/projects/pm-providers/trello/wizard.js'; +import { + buildJiraIntegrationConfig, + buildLinearIntegrationConfig, + buildTrelloIntegrationConfig, + createInitialState, +} from '../../../web/src/components/projects/pm-wizard-state.js'; + +// ============================================================================ +// 1. Slot-array membership +// ============================================================================ + +describe('status-mapping slot arrays include a friction entry', () => { + it('TRELLO_LIST_SLOTS contains a friction entry', () => { + expect(TRELLO_LIST_SLOTS).toContainEqual({ key: 'friction', label: 'Friction' }); + }); + + it('JIRA_STATUS_SLOTS contains a friction entry', () => { + expect(JIRA_STATUS_SLOTS).toContainEqual({ key: 'friction', label: 'Friction' }); + }); + + it('LINEAR_STATUS_SLOTS contains a friction entry', () => { + expect(LINEAR_STATUS_SLOTS).toContainEqual({ key: 'friction', label: 'Friction' }); + }); +}); + +// ============================================================================ +// 2. Round-trip through buildIntegrationConfig +// ============================================================================ + +describe('friction status slot round-trips through buildIntegrationConfig', () => { + it('Trello: lists.friction propagates from trelloListMappings', () => { + const state = { + ...createInitialState(), + trelloListMappings: { friction: 'list-id-friction-123' }, + }; + const config = buildTrelloIntegrationConfig(state); + expect((config.lists as Record).friction).toBe('list-id-friction-123'); + }); + + it('JIRA: statuses.friction propagates from jiraStatusMappings', () => { + const state = { + ...createInitialState(), + jiraStatusMappings: { friction: 'Friction Status' }, + }; + const config = buildJiraIntegrationConfig(state); + expect((config.statuses as Record).friction).toBe('Friction Status'); + }); + + it('Linear: statuses.friction propagates from linearStatusMappings', () => { + const state = { + ...createInitialState(), + linearStatusMappings: { friction: 'state-uuid-friction' }, + }; + const config = buildLinearIntegrationConfig(state); + expect((config.statuses as Record).friction).toBe('state-uuid-friction'); + }); +}); + +// ============================================================================ +// 3. Optionality — wizard completes without friction slot +// ============================================================================ + +describe('friction slot is optional — wizard does not require it', () => { + it('Trello: buildTrelloIntegrationConfig produces no friction entry when not mapped', () => { + const state = { + ...createInitialState(), + trelloListMappings: { todo: 'list-id-todo' }, + }; + const config = buildTrelloIntegrationConfig(state); + expect((config.lists as Record).friction).toBeUndefined(); + }); + + it('JIRA: buildJiraIntegrationConfig produces no friction entry when not mapped', () => { + const state = { + ...createInitialState(), + jiraStatusMappings: { todo: 'To Do' }, + }; + const config = buildJiraIntegrationConfig(state); + expect((config.statuses as Record).friction).toBeUndefined(); + }); + + it('Linear: buildLinearIntegrationConfig produces no friction entry when not mapped', () => { + const state = { + ...createInitialState(), + linearStatusMappings: { todo: 'state-uuid-todo' }, + }; + const config = buildLinearIntegrationConfig(state); + expect((config.statuses as Record).friction).toBeUndefined(); + }); +}); diff --git a/web/src/components/projects/pm-providers/jira/wizard.ts b/web/src/components/projects/pm-providers/jira/wizard.ts index 7d5eff7da..085e43656 100644 --- a/web/src/components/projects/pm-providers/jira/wizard.ts +++ b/web/src/components/projects/pm-providers/jira/wizard.ts @@ -43,6 +43,7 @@ export const JIRA_STATUS_SLOTS = [ { key: 'done', label: 'Done' }, { key: 'merged', label: 'Merged' }, { key: 'alerts', label: 'Alerts' }, + { key: 'friction', label: 'Friction' }, ] as const; // CASCADE labels that map to JIRA labels. JIRA labels are free-form diff --git a/web/src/components/projects/pm-providers/linear/wizard.ts b/web/src/components/projects/pm-providers/linear/wizard.ts index 826a8128f..06a8f6840 100644 --- a/web/src/components/projects/pm-providers/linear/wizard.ts +++ b/web/src/components/projects/pm-providers/linear/wizard.ts @@ -48,6 +48,7 @@ export const LINEAR_STATUS_SLOTS = [ { key: 'done', label: 'Done' }, { key: 'merged', label: 'Merged' }, { key: 'alerts', label: 'Alerts' }, + { key: 'friction', label: 'Friction' }, ] as const; export const LINEAR_LABEL_SLOTS = [ diff --git a/web/src/components/projects/pm-providers/trello/wizard.ts b/web/src/components/projects/pm-providers/trello/wizard.ts index 4804483e6..013640bbc 100644 --- a/web/src/components/projects/pm-providers/trello/wizard.ts +++ b/web/src/components/projects/pm-providers/trello/wizard.ts @@ -47,6 +47,7 @@ export const TRELLO_LIST_SLOTS = [ { key: 'merged', label: 'Merged' }, { key: 'debug', label: 'Debug' }, { key: 'alerts', label: 'Alerts' }, + { key: 'friction', label: 'Friction' }, ] as const; // CASCADE labels that map to Trello labels. Defaults (name + color) From 356f7dd0d8f78f10edd7da65db91ab31590177fe Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 20:02:31 +0200 Subject: [PATCH 3/9] feat(friction): add report materializer and sidecar outbox (#1296) Co-authored-by: Cascade Bot --- src/friction/format.ts | 93 +++++++++++++++ src/friction/index.ts | 20 ++++ src/friction/materialize.ts | 50 ++++++++ src/friction/sidecar.ts | 87 ++++++++++++++ src/friction/types.ts | 104 +++++++++++++++++ tests/unit/friction/format.test.ts | 91 +++++++++++++++ tests/unit/friction/materialize.test.ts | 148 ++++++++++++++++++++++++ tests/unit/friction/sidecar.test.ts | 105 +++++++++++++++++ vitest.config.ts | 1 + 9 files changed, 699 insertions(+) create mode 100644 src/friction/format.ts create mode 100644 src/friction/index.ts create mode 100644 src/friction/materialize.ts create mode 100644 src/friction/sidecar.ts create mode 100644 src/friction/types.ts create mode 100644 tests/unit/friction/format.test.ts create mode 100644 tests/unit/friction/materialize.test.ts create mode 100644 tests/unit/friction/sidecar.test.ts diff --git a/src/friction/format.ts b/src/friction/format.ts new file mode 100644 index 000000000..977684e59 --- /dev/null +++ b/src/friction/format.ts @@ -0,0 +1,93 @@ +import type { FormattedFrictionReport, FrictionReport } from './types.js'; + +const TITLE_LIMIT = 120; + +function truncateTitle(value: string): string { + const normalized = value.replace(/\s+/g, ' ').trim(); + if (normalized.length <= TITLE_LIMIT) return normalized; + return `${normalized.slice(0, TITLE_LIMIT - 3).trimEnd()}...`; +} + +function valueOrMissing(value: string | number | undefined): string { + if (value === undefined || value === '') return '_not provided_'; + return String(value); +} + +function maybeLink(label: string, url: string | undefined): string { + return url ? `[${label}](${url})` : label; +} + +function projectContextLines(report: FrictionReport): string[] { + const { project } = report.context; + return [ + `- Project: ${valueOrMissing(project.name ?? project.id)} (${project.id})`, + `- PM provider: ${valueOrMissing(project.pmType)}`, + ...(project.repo ? [`- Repository: ${project.repo}`] : []), + ]; +} + +function optionalContextLines(report: FrictionReport): string[] { + const { agent, run, workItem, pr } = report.context; + const lines: string[] = []; + + if (agent) { + const agentParts = [ + agent.type, + agent.engine && `engine=${agent.engine}`, + agent.model && `model=${agent.model}`, + ].filter(Boolean); + lines.push(`- Agent: ${agentParts.join(' - ')}`); + } + if (run) lines.push(`- Run: ${maybeLink(valueOrMissing(run.id), run.url)}`); + if (run?.startedAt) lines.push(`- Run started: ${run.startedAt}`); + if (workItem) { + const label = workItem.title + ? `${workItem.title} (${valueOrMissing(workItem.id)})` + : valueOrMissing(workItem.id); + lines.push(`- Work item: ${maybeLink(label, workItem.url)}`); + } + if (pr) lines.push(`- Pull request: ${maybeLink(formatPRLabel(pr), pr.url)}`); + if (pr?.branch) lines.push(`- PR branch: ${pr.branch}`); + if (pr?.headSha) lines.push(`- PR head SHA: ${pr.headSha}`); + + return lines; +} + +function formatPRLabel(pr: NonNullable): string { + if (!pr.number) return valueOrMissing(pr.title); + return `#${pr.number}${pr.title ? ` ${pr.title}` : ''}`; +} + +function contextLines(report: FrictionReport): string[] { + return [...projectContextLines(report), ...optionalContextLines(report)]; +} + +export function formatFrictionReport( + report: FrictionReport, + now: Date = new Date(), +): FormattedFrictionReport { + const timestamp = report.createdAt ?? now.toISOString(); + const title = truncateTitle(`[Friction][${report.severity}] ${report.summary}`); + + return { + title, + descriptionMarkdown: [ + '## What happened', + report.summary, + '', + '## Details', + report.details, + '', + '## Classification', + `- Category: ${report.category}`, + `- Severity: ${report.severity}`, + `- While doing: ${report.whileDoing}`, + '', + '## Context', + ...contextLines(report), + '', + '## Timestamp', + timestamp, + ].join('\n'), + }; +} diff --git a/src/friction/index.ts b/src/friction/index.ts new file mode 100644 index 000000000..6986ddee0 --- /dev/null +++ b/src/friction/index.ts @@ -0,0 +1,20 @@ +export { formatFrictionReport } from './format.js'; +export { materializeFrictionReport } from './materialize.js'; +export { + appendFiledFrictionReport, + appendQueuedFrictionReport, + compactPendingFrictionReports, + readFrictionSidecarEvents, + rewriteFrictionSidecarWithPending, +} from './sidecar.js'; +export type { + FormattedFrictionReport, + FrictionCategory, + FrictionFiledEvent, + FrictionMaterializationResult, + FrictionQueuedEvent, + FrictionReport, + FrictionRuntimeContext, + FrictionSeverity, + FrictionSidecarEvent, +} from './types.js'; diff --git a/src/friction/materialize.ts b/src/friction/materialize.ts new file mode 100644 index 000000000..c7dda5890 --- /dev/null +++ b/src/friction/materialize.ts @@ -0,0 +1,50 @@ +import { getFrictionContainerId, getFrictionStatusDestination } from '../pm/config.js'; +import { pmRegistry } from '../pm/registry.js'; +import type { ProjectConfig } from '../types/index.js'; +import { formatFrictionReport } from './format.js'; +import type { FrictionMaterializationResult, FrictionReport } from './types.js'; + +export interface MaterializeFrictionReportOptions { + project: ProjectConfig; + report: FrictionReport; + now?: Date; +} + +export async function materializeFrictionReport({ + project, + report, + now, +}: MaterializeFrictionReportOptions): Promise { + const containerId = getFrictionContainerId(project); + if (!containerId) { + return { + status: 'skipped', + reportId: report.reportId, + reason: 'friction_slot_missing', + message: + `Project ${project.id} (pm.type=${project.pm?.type ?? 'unknown'}) has no 'friction' slot configured. ` + + `Set lists.friction (Trello) or statuses.friction (JIRA, Linear) in the PM integration config.`, + }; + } + + const provider = pmRegistry.createProvider(project); + const formatted = formatFrictionReport(report, now); + const workItem = await provider.createWorkItem({ + containerId, + title: formatted.title, + description: formatted.descriptionMarkdown, + labels: [], + }); + + const destination = getFrictionStatusDestination(project); + if (destination) { + await provider.moveWorkItem(workItem.id, destination); + } + + return { + status: 'filed', + reportId: report.reportId, + workItemId: workItem.id, + workItemUrl: workItem.url || provider.getWorkItemUrl(workItem.id), + }; +} diff --git a/src/friction/sidecar.ts b/src/friction/sidecar.ts new file mode 100644 index 000000000..866e72b86 --- /dev/null +++ b/src/friction/sidecar.ts @@ -0,0 +1,87 @@ +import { mkdir, readFile, writeFile } from 'node:fs/promises'; +import { dirname } from 'node:path'; +import type { + FrictionFiledEvent, + FrictionQueuedEvent, + FrictionReport, + FrictionSidecarEvent, +} from './types.js'; + +async function appendJsonLine(path: string, event: FrictionSidecarEvent): Promise { + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, `${JSON.stringify(event)}\n`, { encoding: 'utf-8', flag: 'a' }); +} + +export async function appendQueuedFrictionReport( + path: string, + report: FrictionReport, + timestamp = new Date().toISOString(), +): Promise { + await appendJsonLine(path, { + event: 'queued', + reportId: report.reportId, + report, + timestamp, + }); +} + +export async function appendFiledFrictionReport( + path: string, + input: { reportId: string; workItemId: string; workItemUrl?: string }, + timestamp = new Date().toISOString(), +): Promise { + await appendJsonLine(path, { + event: 'filed', + reportId: input.reportId, + workItemId: input.workItemId, + workItemUrl: input.workItemUrl, + timestamp, + }); +} + +export async function readFrictionSidecarEvents(path: string): Promise { + let raw: string; + try { + raw = await readFile(path, 'utf-8'); + } catch (err) { + if (err instanceof Error && 'code' in err && err.code === 'ENOENT') return []; + throw err; + } + + return raw + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .flatMap((line) => { + try { + const parsed = JSON.parse(line) as FrictionSidecarEvent; + return parsed.event === 'queued' || parsed.event === 'filed' ? [parsed] : []; + } catch { + return []; + } + }); +} + +export async function compactPendingFrictionReports(path: string): Promise { + const byReportId = new Map(); + for (const event of await readFrictionSidecarEvents(path)) { + if (event.event === 'queued') { + byReportId.set(event.reportId, event); + } else { + byReportId.delete(event.reportId); + } + } + return [...byReportId.values()]; +} + +export async function rewriteFrictionSidecarWithPending( + path: string, +): Promise { + const pending = await compactPendingFrictionReports(path); + await mkdir(dirname(path), { recursive: true }); + const content = pending.map((event) => JSON.stringify(event)).join('\n'); + await writeFile(path, content ? `${content}\n` : '', 'utf-8'); + return pending; +} + +export type { FrictionFiledEvent, FrictionQueuedEvent }; diff --git a/src/friction/types.ts b/src/friction/types.ts new file mode 100644 index 000000000..b62ea1fb3 --- /dev/null +++ b/src/friction/types.ts @@ -0,0 +1,104 @@ +import type { ProjectConfig } from '../types/index.js'; + +export type FrictionCategory = + | 'tooling' + | 'environment' + | 'permissions' + | 'dependency' + | 'test-failure' + | 'pm-data' + | 'scm-data' + | 'other'; + +export type FrictionSeverity = 'low' | 'medium' | 'high' | 'critical'; + +export interface FrictionReport { + /** Stable id used for sidecar deduplication across queued/filed events. */ + reportId: string; + summary: string; + details: string; + category: FrictionCategory; + severity: FrictionSeverity; + /** Short description of the task or operation in progress when friction occurred. */ + whileDoing: string; + context: FrictionRuntimeContext; + /** ISO-8601 timestamp. Formatting uses this when supplied, otherwise the materializer clock. */ + createdAt?: string; +} + +export interface FrictionRuntimeContext { + project: FrictionProjectContext; + agent?: FrictionAgentContext; + run?: FrictionRunContext; + workItem?: FrictionWorkItemContext; + pr?: FrictionPullRequestContext; +} + +export interface FrictionProjectContext { + id: string; + name?: string; + repo?: string; + pmType?: ProjectConfig['pm']['type']; +} + +export interface FrictionAgentContext { + type: string; + engine?: string; + model?: string; +} + +export interface FrictionRunContext { + id?: string; + url?: string; + startedAt?: string; +} + +export interface FrictionWorkItemContext { + id?: string; + title?: string; + url?: string; +} + +export interface FrictionPullRequestContext { + number?: number; + title?: string; + url?: string; + branch?: string; + headSha?: string; +} + +export interface FormattedFrictionReport { + title: string; + descriptionMarkdown: string; +} + +export type FrictionMaterializationResult = + | { + status: 'filed'; + reportId: string; + workItemId: string; + workItemUrl?: string; + } + | { + status: 'skipped'; + reportId: string; + reason: 'friction_slot_missing'; + message: string; + }; + +export interface FrictionQueuedEvent { + event: 'queued'; + reportId: string; + report: FrictionReport; + timestamp: string; +} + +export interface FrictionFiledEvent { + event: 'filed'; + reportId: string; + workItemId: string; + workItemUrl?: string; + timestamp: string; +} + +export type FrictionSidecarEvent = FrictionQueuedEvent | FrictionFiledEvent; diff --git a/tests/unit/friction/format.test.ts b/tests/unit/friction/format.test.ts new file mode 100644 index 000000000..088589f59 --- /dev/null +++ b/tests/unit/friction/format.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from 'vitest'; +import { formatFrictionReport } from '../../../src/friction/format.js'; +import type { FrictionReport } from '../../../src/friction/types.js'; + +function makeReport(overrides: Partial = {}): FrictionReport { + return { + reportId: 'friction-1', + summary: 'Could not inspect an authenticated Trello attachment', + details: 'The original attachment URL returned an authorization error during analysis.', + category: 'pm-data', + severity: 'medium', + whileDoing: 'reviewing work item screenshots', + context: { + project: { + id: 'proj-1', + name: 'Cascade', + repo: 'acme/cascade', + pmType: 'trello', + }, + agent: { type: 'implementation', engine: 'codex', model: 'gpt-5.4' }, + run: { + id: 'run-1', + url: 'https://ca.sca.de.com/runs/run-1', + startedAt: '2026-05-09T17:46:13.000Z', + }, + workItem: { + id: 'card-1', + title: 'Add friction reports', + url: 'https://trello.com/c/card-1', + }, + pr: { + number: 12, + title: 'feat: example', + url: 'https://github.com/acme/cascade/pull/12', + branch: 'feature/example', + headSha: 'abc123', + }, + }, + ...overrides, + }; +} + +describe('formatFrictionReport', () => { + it('produces a PM-ready title and markdown body with runtime context', () => { + const formatted = formatFrictionReport(makeReport(), new Date('2026-05-09T18:00:00.000Z')); + + expect(formatted.title).toBe( + '[Friction][medium] Could not inspect an authenticated Trello attachment', + ); + expect(formatted.descriptionMarkdown).toContain('## What happened'); + expect(formatted.descriptionMarkdown).toContain( + 'The original attachment URL returned an authorization error during analysis.', + ); + expect(formatted.descriptionMarkdown).toContain('- Category: pm-data'); + expect(formatted.descriptionMarkdown).toContain( + '- While doing: reviewing work item screenshots', + ); + expect(formatted.descriptionMarkdown).toContain('- Project: Cascade (proj-1)'); + expect(formatted.descriptionMarkdown).toContain( + '- Run: [run-1](https://ca.sca.de.com/runs/run-1)', + ); + expect(formatted.descriptionMarkdown).toContain( + '- Work item: [Add friction reports (card-1)](https://trello.com/c/card-1)', + ); + expect(formatted.descriptionMarkdown).toContain( + '- Pull request: [#12 feat: example](https://github.com/acme/cascade/pull/12)', + ); + expect(formatted.descriptionMarkdown).toContain('2026-05-09T18:00:00.000Z'); + expect(formatted.descriptionMarkdown).not.toMatch(/resolution plan/i); + }); + + it('prefers report.createdAt over the formatter clock', () => { + const formatted = formatFrictionReport( + makeReport({ createdAt: '2026-05-09T17:00:00.000Z' }), + new Date('2026-05-09T18:00:00.000Z'), + ); + + expect(formatted.descriptionMarkdown).toContain('2026-05-09T17:00:00.000Z'); + expect(formatted.descriptionMarkdown).not.toContain('2026-05-09T18:00:00.000Z'); + }); + + it('truncates long PM titles', () => { + const formatted = formatFrictionReport( + makeReport({ summary: 'x'.repeat(200), severity: 'high' }), + new Date('2026-05-09T18:00:00.000Z'), + ); + + expect(formatted.title).toHaveLength(120); + expect(formatted.title.endsWith('...')).toBe(true); + }); +}); diff --git a/tests/unit/friction/materialize.test.ts b/tests/unit/friction/materialize.test.ts new file mode 100644 index 000000000..bc159de64 --- /dev/null +++ b/tests/unit/friction/materialize.test.ts @@ -0,0 +1,148 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockCreateWorkItem = vi.fn(); +const mockMoveWorkItem = vi.fn(); +const mockGetWorkItemUrl = vi.fn(); + +const fakePMProvider = { + createWorkItem: (...a: unknown[]) => mockCreateWorkItem(...a), + moveWorkItem: (...a: unknown[]) => mockMoveWorkItem(...a), + getWorkItemUrl: (...a: unknown[]) => mockGetWorkItemUrl(...a), +}; + +vi.mock('../../../src/pm/registry.js', () => ({ + pmRegistry: { createProvider: () => fakePMProvider }, +})); + +import { materializeFrictionReport } from '../../../src/friction/materialize.js'; +import type { FrictionReport } from '../../../src/friction/types.js'; +import type { ProjectConfig } from '../../../src/types/index.js'; + +function makeReport(): FrictionReport { + return { + reportId: 'friction-1', + summary: 'Tool failed while reading logs', + details: 'The log file disappeared before the agent could inspect it.', + category: 'tooling', + severity: 'low', + whileDoing: 'checking CI logs', + context: { + project: { id: 'proj-1', name: 'Cascade', pmType: 'trello' }, + agent: { type: 'implementation' }, + run: { id: 'run-1' }, + workItem: { id: 'card-source' }, + pr: { number: 123 }, + }, + }; +} + +function makeTrelloProject( + lists: Record = { friction: 'list-friction' }, +): ProjectConfig { + return { + id: 'proj-1', + orgId: 'org-1', + name: 'Cascade', + pm: { type: 'trello' }, + trello: { boardId: 'board-1', lists, labels: {} }, + } as unknown as ProjectConfig; +} + +function makeJiraProject( + statuses: Record = { friction: 'Friction' }, +): ProjectConfig { + return { + id: 'proj-1', + orgId: 'org-1', + name: 'Cascade', + pm: { type: 'jira' }, + jira: { + projectKey: 'CAS', + baseUrl: 'https://acme.atlassian.net', + statuses, + labels: {}, + }, + } as unknown as ProjectConfig; +} + +describe('materializeFrictionReport', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockCreateWorkItem.mockResolvedValue({ + id: 'friction-card-1', + title: 'Friction', + description: '', + url: 'https://pm.example/friction-card-1', + labels: [], + }); + mockMoveWorkItem.mockResolvedValue(undefined); + mockGetWorkItemUrl.mockReturnValue('https://pm.example/generated'); + }); + + it('creates a work item in the friction container and moves it to the friction destination', async () => { + const result = await materializeFrictionReport({ + project: makeJiraProject(), + report: makeReport(), + now: new Date('2026-05-09T18:00:00.000Z'), + }); + + expect(result).toEqual({ + status: 'filed', + reportId: 'friction-1', + workItemId: 'friction-card-1', + workItemUrl: 'https://pm.example/friction-card-1', + }); + expect(mockCreateWorkItem).toHaveBeenCalledWith( + expect.objectContaining({ + containerId: 'CAS', + title: '[Friction][low] Tool failed while reading logs', + labels: [], + }), + ); + expect(mockCreateWorkItem.mock.calls[0][0].description).toContain('## Context'); + expect(mockMoveWorkItem).toHaveBeenCalledWith('friction-card-1', 'Friction'); + }); + + it('uses the Trello friction list as both create container and move destination', async () => { + await materializeFrictionReport({ project: makeTrelloProject(), report: makeReport() }); + + expect(mockCreateWorkItem).toHaveBeenCalledWith( + expect.objectContaining({ containerId: 'list-friction' }), + ); + expect(mockMoveWorkItem).toHaveBeenCalledWith('friction-card-1', 'list-friction'); + }); + + it('returns a non-fatal skipped result when the friction slot is missing', async () => { + const result = await materializeFrictionReport({ + project: makeTrelloProject({ todo: 'list-todo' }), + report: makeReport(), + }); + + expect(result).toEqual({ + status: 'skipped', + reportId: 'friction-1', + reason: 'friction_slot_missing', + message: expect.stringContaining("has no 'friction' slot configured"), + }); + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + expect(mockMoveWorkItem).not.toHaveBeenCalled(); + }); + + it('falls back to provider.getWorkItemUrl when createWorkItem returns no URL', async () => { + mockCreateWorkItem.mockResolvedValue({ + id: 'friction-card-1', + title: 'Friction', + description: '', + url: '', + labels: [], + }); + + const result = await materializeFrictionReport({ + project: makeTrelloProject(), + report: makeReport(), + }); + + expect(result).toMatchObject({ status: 'filed', workItemUrl: 'https://pm.example/generated' }); + expect(mockGetWorkItemUrl).toHaveBeenCalledWith('friction-card-1'); + }); +}); diff --git a/tests/unit/friction/sidecar.test.ts b/tests/unit/friction/sidecar.test.ts new file mode 100644 index 000000000..512dc6a04 --- /dev/null +++ b/tests/unit/friction/sidecar.test.ts @@ -0,0 +1,105 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { readFile, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { + appendFiledFrictionReport, + appendQueuedFrictionReport, + compactPendingFrictionReports, + readFrictionSidecarEvents, + rewriteFrictionSidecarWithPending, +} from '../../../src/friction/sidecar.js'; +import type { FrictionReport } from '../../../src/friction/types.js'; + +let tempDir: string; + +function makeReport(reportId: string, summary = 'A thing happened'): FrictionReport { + return { + reportId, + summary, + details: 'Details', + category: 'other', + severity: 'low', + whileDoing: 'running tests', + context: { project: { id: 'proj-1' } }, + }; +} + +describe('friction sidecar helpers', () => { + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'cascade-friction-sidecar-')); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + it('appends queued and filed JSONL events', async () => { + const path = join(tempDir, 'nested', 'friction.jsonl'); + + await appendQueuedFrictionReport(path, makeReport('r1'), '2026-05-09T18:00:00.000Z'); + await appendFiledFrictionReport( + path, + { reportId: 'r1', workItemId: 'card-1', workItemUrl: 'https://pm.example/card-1' }, + '2026-05-09T18:01:00.000Z', + ); + + const events = await readFrictionSidecarEvents(path); + expect(events).toEqual([ + expect.objectContaining({ event: 'queued', reportId: 'r1' }), + expect.objectContaining({ + event: 'filed', + reportId: 'r1', + workItemId: 'card-1', + workItemUrl: 'https://pm.example/card-1', + }), + ]); + + const raw = await readFile(path, 'utf-8'); + expect(raw.trim().split('\n')).toHaveLength(2); + }); + + it('compacts pending reports by reportId and removes filed reports', async () => { + const path = join(tempDir, 'friction.jsonl'); + + await appendQueuedFrictionReport(path, makeReport('r1', 'old'), '2026-05-09T18:00:00.000Z'); + await appendQueuedFrictionReport(path, makeReport('r2'), '2026-05-09T18:01:00.000Z'); + await appendQueuedFrictionReport(path, makeReport('r1', 'new'), '2026-05-09T18:02:00.000Z'); + await appendFiledFrictionReport(path, { reportId: 'r2', workItemId: 'card-2' }); + + const pending = await compactPendingFrictionReports(path); + expect(pending).toHaveLength(1); + expect(pending[0]).toMatchObject({ + event: 'queued', + reportId: 'r1', + report: { summary: 'new' }, + }); + }); + + it('rewrites a sidecar to the compact pending queued events only', async () => { + const path = join(tempDir, 'friction.jsonl'); + await appendQueuedFrictionReport(path, makeReport('r1'), '2026-05-09T18:00:00.000Z'); + await appendQueuedFrictionReport(path, makeReport('r2'), '2026-05-09T18:01:00.000Z'); + await appendFiledFrictionReport(path, { reportId: 'r1', workItemId: 'card-1' }); + + const pending = await rewriteFrictionSidecarWithPending(path); + const events = await readFrictionSidecarEvents(path); + + expect(pending.map((event) => event.reportId)).toEqual(['r2']); + expect(events).toEqual([expect.objectContaining({ event: 'queued', reportId: 'r2' })]); + }); + + it('ignores malformed JSONL lines while reading', async () => { + const path = join(tempDir, 'friction.jsonl'); + await writeFile( + path, + `${JSON.stringify({ event: 'queued', reportId: 'r1', report: makeReport('r1'), timestamp: 't1' })}\nnot json\n`, + 'utf-8', + ); + + const events = await readFrictionSidecarEvents(path); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ event: 'queued', reportId: 'r1' }); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 3738eacf6..38b4568ec 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -125,6 +125,7 @@ export default defineConfig({ 'tests/unit/gadgets/**/*.test.ts', 'tests/unit/config/**/*.test.ts', 'tests/unit/db/**/*.test.ts', + 'tests/unit/friction/**/*.test.ts', 'tests/unit/utils/**/*.test.ts', 'tests/unit/cli/**/*.test.ts', 'tests/unit/pm/**/*.test.ts', From fb91028bec985f202fd5abed8d534dbc25c0170e Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 20:46:58 +0200 Subject: [PATCH 4/9] feat(pm): add report friction command (#1297) * feat(pm): add report friction command * fix(friction): gitignore friction sidecar to prevent accidental staging Add .cascade/friction-reports.jsonl to .gitignore so the runtime artifact written by `cascade-tools pm report-friction` is never staged by stageAndCommit() or surfaced in `git status --porcelain`, which previously blocked the Finish gadget's uncommitted-changes check. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Cascade Bot Co-authored-by: Claude Sonnet 4.6 --- .gitignore | 3 + docs/architecture/07-gadgets.md | 3 +- src/agents/capabilities/index.ts | 2 +- src/agents/capabilities/registry.ts | 1 + src/agents/capabilities/resolver.ts | 2 + src/agents/definitions/toolManifests.ts | 2 + src/backends/secretBuilder.ts | 25 ++- src/cli/pm/report-friction.ts | 13 ++ src/gadgets/pm/ReportFriction.ts | 18 ++ src/gadgets/pm/core/reportFriction.ts | 201 ++++++++++++++++++ src/gadgets/pm/definitions.ts | 71 ++++++- src/gadgets/pm/index.ts | 1 + .../unit/agents/capabilities/resolver.test.ts | 1 + tests/unit/agents/shared/gadgets.test.ts | 2 + tests/unit/backends/adapter.test.ts | 10 + tests/unit/backends/agent-profiles.test.ts | 1 + tests/unit/backends/toolManifests.test.ts | 19 +- tests/unit/cli/file-input-flags.test.ts | 56 +++++ tests/unit/cli/pm/pm-commands.test.ts | 66 ++++++ .../gadgets/pm/core/reportFriction.test.ts | 137 ++++++++++++ tests/unit/gadgets/pm/definitions.test.ts | 50 ++++- 21 files changed, 676 insertions(+), 8 deletions(-) create mode 100644 src/cli/pm/report-friction.ts create mode 100644 src/gadgets/pm/ReportFriction.ts create mode 100644 src/gadgets/pm/core/reportFriction.ts create mode 100644 tests/unit/gadgets/pm/core/reportFriction.test.ts diff --git a/.gitignore b/.gitignore index 65efec9e0..105beb73d 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,6 @@ test-results/ # CASCADE context files (temporary pre-fetched data) .cascade/context/ + +# CASCADE friction report sidecar (runtime artifact — must not be committed or staged) +.cascade/friction-reports.jsonl diff --git a/docs/architecture/07-gadgets.md b/docs/architecture/07-gadgets.md index ef213f217..35ee8fd25 100644 --- a/docs/architecture/07-gadgets.md +++ b/docs/architecture/07-gadgets.md @@ -62,6 +62,7 @@ Todos are stored in `.claude/todos.json` within the repo working directory. | `ListWorkItems` | `pm:read` | List work items with filters | | `UpdateWorkItem` | `pm:write` | Update work item fields | | `CreateWorkItem` | `pm:write` | Create new work item | +| `ReportFriction` | `pm:write` | Queue and file incidental friction reports | | `MoveWorkItem` | `pm:write` | Move work item to a status/list | | `PostComment` | `pm:write` | Post comment on work item | | `AddChecklist` | `pm:write` | Add checklist to work item | @@ -101,7 +102,7 @@ Native-tool engines cannot invoke gadget classes directly (they run as subproces | Category | Commands | Example | |----------|----------|---------| -| PM | `cascade-tools pm read-work-item`, `list-work-items`, `update-work-item`, etc. | `cascade-tools pm read-work-item --workItemId abc123` | +| PM | `cascade-tools pm read-work-item`, `list-work-items`, `update-work-item`, `report-friction`, etc. | `cascade-tools pm report-friction --summary "Missing setup hint" --details-file - --category tooling --severity medium` | | SCM | `cascade-tools scm get-pr-details`, `get-pr-diff`, `post-pr-comment`, etc. | `cascade-tools scm get-pr-details --prNumber 42` | | Alerting | `cascade-tools alerting get-alerting-issue`, `list-alerting-events`, etc. | `cascade-tools alerting get-alerting-issue --organizationId acme --issueId 12345` | | Session | `cascade-tools session finish` | `cascade-tools session finish --comment "Created PR and verified checks"` | diff --git a/src/agents/capabilities/index.ts b/src/agents/capabilities/index.ts index aac4898b3..adfd76a56 100644 --- a/src/agents/capabilities/index.ts +++ b/src/agents/capabilities/index.ts @@ -7,7 +7,7 @@ * Integration Category → Capabilities → Gadgets/Tools * │ │ │ * pm → pm:read → ReadWorkItem, ListWorkItems - * pm:write → CreateWorkItem, UpdateWorkItem, PostComment + * pm:write → CreateWorkItem, ReportFriction, UpdateWorkItem, PostComment * pm:checklist → PMUpdateChecklistItem, PMDeleteChecklistItem * * scm → scm:read → GetPRDetails, GetPRDiff, GetPRChecks diff --git a/src/agents/capabilities/registry.ts b/src/agents/capabilities/registry.ts index 36d196d96..5f6b82c6a 100644 --- a/src/agents/capabilities/registry.ts +++ b/src/agents/capabilities/registry.ts @@ -127,6 +127,7 @@ export const CAPABILITY_REGISTRY: Record = { gadgetNames: [ 'UpdateWorkItem', 'CreateWorkItem', + 'ReportFriction', 'MoveWorkItem', 'PostComment', 'AddChecklist', diff --git a/src/agents/capabilities/resolver.ts b/src/agents/capabilities/resolver.ts index 9cb5618f7..a80186914 100644 --- a/src/agents/capabilities/resolver.ts +++ b/src/agents/capabilities/resolver.ts @@ -30,6 +30,7 @@ import { PMUpdateChecklistItem, PostComment, ReadWorkItem, + ReportFriction, UpdateWorkItem, } from '../../gadgets/pm/index.js'; import { ReadFile } from '../../gadgets/ReadFile.js'; @@ -103,6 +104,7 @@ const GADGET_CONSTRUCTORS: Record any> = { // pm:write UpdateWorkItem, CreateWorkItem, + ReportFriction, MoveWorkItem, PostComment, AddChecklist, diff --git a/src/agents/definitions/toolManifests.ts b/src/agents/definitions/toolManifests.ts index 1a109b744..4d814067b 100644 --- a/src/agents/definitions/toolManifests.ts +++ b/src/agents/definitions/toolManifests.ts @@ -19,6 +19,7 @@ import { pmUpdateChecklistItemDef, postCommentDef, readWorkItemDef, + reportFrictionDef, updateWorkItemDef, } from '../../gadgets/pm/definitions.js'; import { finishDef } from '../../gadgets/session/definitions.js'; @@ -35,6 +36,7 @@ const ALL_DEFINITIONS = [ postCommentDef, updateWorkItemDef, createWorkItemDef, + reportFrictionDef, listWorkItemsDef, addChecklistDef, moveWorkItemDef, diff --git a/src/backends/secretBuilder.ts b/src/backends/secretBuilder.ts index 9bcb83fd0..d93e35e99 100644 --- a/src/backends/secretBuilder.ts +++ b/src/backends/secretBuilder.ts @@ -1,7 +1,7 @@ import type { AgentProfile } from '../agents/definitions/profiles.js'; import { getAllProjectCredentials } from '../config/provider.js'; import { getPersonaToken } from '../github/personas.js'; -import { getJiraConfig, getLinearConfig } from '../pm/config.js'; +import { getJiraConfig, getLinearConfig, getTrelloConfig } from '../pm/config.js'; import type { AgentInput, ProjectConfig } from '../types/index.js'; import { parseRepoFullName } from '../utils/repo.js'; import { ENV_VAR_NAME } from './progressState.js'; @@ -30,6 +30,24 @@ export async function resolveGitHubToken( * Build the per-project secrets map by fetching credentials and injecting * CASCADE-specific env vars (base branch, JIRA config, repo owner/name, agent type, PM type). */ +function injectProjectContext( + projectSecrets: Record, + project: ProjectConfig, +): void { + projectSecrets.CASCADE_PROJECT_ID = project.id; + if (project.orgId) projectSecrets.CASCADE_ORG_ID = project.orgId; + if (project.name) projectSecrets.CASCADE_PROJECT_NAME = project.name; +} + +function injectTrelloConfig(projectSecrets: Record, project: ProjectConfig): void { + const trelloConfig = getTrelloConfig(project); + if (!trelloConfig) return; + + projectSecrets.CASCADE_TRELLO_BOARD_ID = trelloConfig.boardId; + projectSecrets.CASCADE_TRELLO_LISTS = JSON.stringify(trelloConfig.lists); + projectSecrets.CASCADE_TRELLO_LABELS = JSON.stringify(trelloConfig.labels); +} + export async function augmentProjectSecrets( project: ProjectConfig, agentType: string, @@ -37,11 +55,16 @@ export async function augmentProjectSecrets( ): Promise> { const projectSecrets = await getAllProjectCredentials(project.id); + injectProjectContext(projectSecrets, project); + // Inject base branch so cascade-tools create-pr uses the correct target automatically if (project.baseBranch) { projectSecrets.CASCADE_BASE_BRANCH = project.baseBranch; } + // Inject Trello integration config so friction reports can resolve optional PM slots. + injectTrelloConfig(projectSecrets, project); + // Inject JIRA integration config so cascade-tools can construct JiraPMProvider const jiraConfig = getJiraConfig(project); if (jiraConfig) { diff --git a/src/cli/pm/report-friction.ts b/src/cli/pm/report-friction.ts new file mode 100644 index 000000000..ad54d713d --- /dev/null +++ b/src/cli/pm/report-friction.ts @@ -0,0 +1,13 @@ +import { reportFriction } from '../../gadgets/pm/core/reportFriction.js'; +import { reportFrictionDef } from '../../gadgets/pm/definitions.js'; +import { createCLICommand } from '../../gadgets/shared/cliCommandFactory.js'; + +export default createCLICommand(reportFrictionDef, async (params) => { + return reportFriction({ + summary: params.summary as string, + details: params.details as string, + category: params.category as never, + severity: params.severity as never, + whileDoing: params.whileDoing as string | undefined, + }); +}); diff --git a/src/gadgets/pm/ReportFriction.ts b/src/gadgets/pm/ReportFriction.ts new file mode 100644 index 000000000..70624b6e1 --- /dev/null +++ b/src/gadgets/pm/ReportFriction.ts @@ -0,0 +1,18 @@ +import { createGadgetClass } from '../shared/gadgetFactory.js'; +import { reportFriction } from './core/reportFriction.js'; +import { reportFrictionDef } from './definitions.js'; + +export const ReportFriction = createGadgetClass(reportFrictionDef, async (params) => { + const result = await reportFriction({ + summary: params.summary as string, + details: params.details as string, + category: params.category as never, + severity: params.severity as never, + whileDoing: params.whileDoing as string | undefined, + }); + + if (result.status === 'filed') { + return `Friction report filed: ${result.workItemUrl ?? result.workItemId}`; + } + return `${result.status}: ${result.message}`; +}); diff --git a/src/gadgets/pm/core/reportFriction.ts b/src/gadgets/pm/core/reportFriction.ts new file mode 100644 index 000000000..84f4b412e --- /dev/null +++ b/src/gadgets/pm/core/reportFriction.ts @@ -0,0 +1,201 @@ +import { randomUUID } from 'node:crypto'; +import { materializeFrictionReport } from '../../../friction/materialize.js'; +import { + appendFiledFrictionReport, + appendQueuedFrictionReport, +} from '../../../friction/sidecar.js'; +import type { + FrictionCategory, + FrictionReport, + FrictionSeverity, +} from '../../../friction/types.js'; +import type { ProjectConfig } from '../../../types/index.js'; + +const FRICTION_SIDECAR_ENV_VAR = 'CASCADE_FRICTION_SIDECAR_PATH'; +const DEFAULT_FRICTION_SIDECAR_PATH = '.cascade/friction-reports.jsonl'; + +const CATEGORIES = [ + 'tooling', + 'environment', + 'permissions', + 'dependency', + 'test-failure', + 'pm-data', + 'scm-data', + 'other', +] as const satisfies readonly FrictionCategory[]; + +const SEVERITIES = [ + 'low', + 'medium', + 'high', + 'critical', +] as const satisfies readonly FrictionSeverity[]; + +export interface ReportFrictionParams { + summary: string; + details: string; + category: FrictionCategory; + severity: FrictionSeverity; + whileDoing?: string; + project?: ProjectConfig; + sidecarPath?: string; +} + +export type ReportFrictionResult = + | { + status: 'filed'; + reportId: string; + workItemId: string; + workItemUrl?: string; + message: string; + } + | { + status: 'queued_for_retry'; + reportId: string; + message: string; + } + | { + status: 'queued_slot_missing'; + reportId: string; + message: string; + }; + +function parseJsonRecord(value: string | undefined): Record { + if (!value) return {}; + const parsed = JSON.parse(value) as unknown; + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) + ? (parsed as Record) + : {}; +} + +function projectFromEnv(): ProjectConfig { + const pmType = process.env.CASCADE_PM_TYPE as ProjectConfig['pm']['type'] | undefined; + const base = { + id: process.env.CASCADE_PROJECT_ID ?? 'unknown-project', + orgId: process.env.CASCADE_ORG_ID ?? 'unknown-org', + name: process.env.CASCADE_PROJECT_NAME ?? process.env.CASCADE_PROJECT_ID ?? 'Unknown project', + repo: + process.env.CASCADE_REPO_OWNER && process.env.CASCADE_REPO_NAME + ? `${process.env.CASCADE_REPO_OWNER}/${process.env.CASCADE_REPO_NAME}` + : undefined, + pm: { type: pmType ?? 'trello' }, + } as ProjectConfig; + + if (base.pm.type === 'jira') { + return { + ...base, + jira: { + projectKey: process.env.CASCADE_JIRA_PROJECT_KEY ?? '', + baseUrl: process.env.CASCADE_JIRA_BASE_URL ?? process.env.JIRA_BASE_URL ?? '', + statuses: parseJsonRecord(process.env.CASCADE_JIRA_STATUSES), + }, + } as ProjectConfig; + } + if (base.pm.type === 'linear') { + return { + ...base, + linear: { + teamId: process.env.CASCADE_LINEAR_TEAM_ID ?? '', + ...(process.env.CASCADE_LINEAR_PROJECT_ID + ? { projectId: process.env.CASCADE_LINEAR_PROJECT_ID } + : {}), + statuses: parseJsonRecord(process.env.CASCADE_LINEAR_STATUSES), + }, + } as ProjectConfig; + } + return { + ...base, + trello: { + boardId: process.env.CASCADE_TRELLO_BOARD_ID ?? '', + lists: parseJsonRecord(process.env.CASCADE_TRELLO_LISTS), + labels: parseJsonRecord(process.env.CASCADE_TRELLO_LABELS), + }, + } as ProjectConfig; +} + +function requireEnum(value: string, allowed: readonly T[], name: string): T { + if ((allowed as readonly string[]).includes(value)) return value as T; + throw new Error(`${name} must be one of: ${allowed.join(', ')}`); +} + +function buildReport(params: ReportFrictionParams, project: ProjectConfig): FrictionReport { + return { + reportId: randomUUID(), + summary: params.summary, + details: params.details, + category: requireEnum(params.category, CATEGORIES, 'category'), + severity: requireEnum(params.severity, SEVERITIES, 'severity'), + whileDoing: params.whileDoing?.trim() || 'not specified', + createdAt: new Date().toISOString(), + context: { + project: { + id: project.id, + name: project.name, + repo: project.repo, + pmType: project.pm?.type, + }, + agent: { + type: process.env.CASCADE_AGENT_TYPE ?? 'unknown', + engine: process.env.CASCADE_ENGINE_LABEL, + model: process.env.CASCADE_MODEL, + }, + run: { + id: process.env.CASCADE_RUN_ID, + url: + process.env.CASCADE_DASHBOARD_URL && process.env.CASCADE_RUN_ID + ? `${process.env.CASCADE_DASHBOARD_URL.replace(/\/$/, '')}/runs/${process.env.CASCADE_RUN_ID}` + : undefined, + }, + workItem: { + id: process.env.CASCADE_WORK_ITEM_ID, + title: process.env.CASCADE_WORK_ITEM_TITLE, + url: process.env.CASCADE_WORK_ITEM_URL, + }, + pr: { + branch: process.env.CASCADE_PR_BRANCH, + headSha: process.env.CASCADE_INITIAL_HEAD_SHA, + }, + }, + }; +} + +export async function reportFriction(params: ReportFrictionParams): Promise { + const project = params.project ?? projectFromEnv(); + const sidecarPath = + params.sidecarPath ?? process.env[FRICTION_SIDECAR_ENV_VAR] ?? DEFAULT_FRICTION_SIDECAR_PATH; + const report = buildReport(params, project); + + await appendQueuedFrictionReport(sidecarPath, report, report.createdAt); + + try { + const result = await materializeFrictionReport({ project, report }); + if (result.status === 'skipped') { + return { + status: 'queued_slot_missing', + reportId: report.reportId, + message: result.message, + }; + } + + await appendFiledFrictionReport(sidecarPath, { + reportId: report.reportId, + workItemId: result.workItemId, + workItemUrl: result.workItemUrl, + }); + return { + status: 'filed', + reportId: report.reportId, + workItemId: result.workItemId, + workItemUrl: result.workItemUrl, + message: `Friction report filed: ${result.workItemUrl ?? result.workItemId}`, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { + status: 'queued_for_retry', + reportId: report.reportId, + message: `Friction report queued for retry after filing failed: ${message}`, + }; + } +} diff --git a/src/gadgets/pm/definitions.ts b/src/gadgets/pm/definitions.ts index 839a52eb4..2965eeaec 100644 --- a/src/gadgets/pm/definitions.ts +++ b/src/gadgets/pm/definitions.ts @@ -1,5 +1,5 @@ /** - * Unified ToolDefinition objects for all 9 PM tools. + * Unified ToolDefinition objects for all PM tools. * * These definitions are the single source of truth for: * - Gadget classes (generated via createGadgetClass) @@ -165,6 +165,75 @@ export const createWorkItemDef: ToolDefinition = { }, }; +export const reportFrictionDef: ToolDefinition = { + name: 'ReportFriction', + description: + 'File an incidental friction report with structured context. Use this for papercuts, tool issues, missing permissions, flaky dependencies, or confusing PM/SCM data discovered while working.', + timeoutMs: 30000, + parameters: { + summary: { + type: 'string', + describe: 'Short one-line summary of the friction encountered', + required: true, + }, + details: { + type: 'string', + describe: 'Markdown details with concrete symptoms, impact, and useful reproduction context', + required: true, + }, + category: { + type: 'enum', + options: [ + 'tooling', + 'environment', + 'permissions', + 'dependency', + 'test-failure', + 'pm-data', + 'scm-data', + 'other', + ], + describe: + 'Friction category: tooling, environment, permissions, dependency, test-failure, pm-data, scm-data, or other', + required: true, + }, + severity: { + type: 'enum', + options: ['low', 'medium', 'high', 'critical'], + describe: + 'Severity: low annoyance, medium slowdown, high blocker risk, or critical hard blocker', + required: true, + }, + whileDoing: { + type: 'string', + describe: 'Optional short description of the task or operation in progress', + optional: true, + }, + }, + examples: [ + { + params: { + summary: 'Typecheck requires undocumented Redis env var', + details: + '`npm run typecheck` failed until REDIS_URL was set. The setup docs mention Redis for router runtime but not for this command path.', + category: 'environment', + severity: 'medium', + whileDoing: 'Running pre-PR verification', + }, + comment: 'Report a non-blocking setup papercut discovered during implementation', + }, + ], + cli: { + fileInputAlternatives: [ + { + paramName: 'details', + fileFlag: 'details-file', + description: 'Read friction details from file (use - for stdin)', + }, + ], + }, +}; + export const listWorkItemsDef: ToolDefinition = { name: 'ListWorkItems', description: diff --git a/src/gadgets/pm/index.ts b/src/gadgets/pm/index.ts index aae6ed073..a8dbf558c 100644 --- a/src/gadgets/pm/index.ts +++ b/src/gadgets/pm/index.ts @@ -5,5 +5,6 @@ export { ListWorkItems } from './ListWorkItems.js'; export { MoveWorkItem } from './MoveWorkItem.js'; export { PostComment } from './PostComment.js'; export { ReadWorkItem } from './ReadWorkItem.js'; +export { ReportFriction } from './ReportFriction.js'; export { PMUpdateChecklistItem } from './UpdateChecklistItem.js'; export { UpdateWorkItem } from './UpdateWorkItem.js'; diff --git a/tests/unit/agents/capabilities/resolver.test.ts b/tests/unit/agents/capabilities/resolver.test.ts index 6a189b422..e909091d0 100644 --- a/tests/unit/agents/capabilities/resolver.test.ts +++ b/tests/unit/agents/capabilities/resolver.test.ts @@ -54,6 +54,7 @@ vi.mock('../../../../src/gadgets/pm/index.js', () => ({ PMUpdateChecklistItem: mockClass('PMUpdateChecklistItem'), PostComment: mockClass('PostComment'), ReadWorkItem: mockClass('ReadWorkItem'), + ReportFriction: mockClass('ReportFriction'), UpdateWorkItem: mockClass('UpdateWorkItem'), })); vi.mock('../../../../src/gadgets/tmux.js', () => ({ Tmux: mockClass('Tmux') })); diff --git a/tests/unit/agents/shared/gadgets.test.ts b/tests/unit/agents/shared/gadgets.test.ts index ee923be95..95860db6b 100644 --- a/tests/unit/agents/shared/gadgets.test.ts +++ b/tests/unit/agents/shared/gadgets.test.ts @@ -45,6 +45,7 @@ vi.mock('../../../../src/gadgets/pm/index.js', () => ({ PMUpdateChecklistItem: mockClass('PMUpdateChecklistItem'), PostComment: mockClass('PostComment'), ReadWorkItem: mockClass('ReadWorkItem'), + ReportFriction: mockClass('ReportFriction'), UpdateWorkItem: mockClass('UpdateWorkItem'), })); vi.mock('../../../../src/gadgets/tmux.js', () => ({ Tmux: mockClass('Tmux') })); @@ -118,6 +119,7 @@ describe('buildGadgetsFromCapabilities', () => { const gadgets = names(buildGadgetsFromCapabilities(caps)); expect(gadgets).toContain('UpdateWorkItem'); expect(gadgets).toContain('CreateWorkItem'); + expect(gadgets).toContain('ReportFriction'); expect(gadgets).toContain('PostComment'); expect(gadgets).toContain('AddChecklist'); }); diff --git a/tests/unit/backends/adapter.test.ts b/tests/unit/backends/adapter.test.ts index bed054564..6d7bbed9b 100644 --- a/tests/unit/backends/adapter.test.ts +++ b/tests/unit/backends/adapter.test.ts @@ -669,9 +669,14 @@ describe('executeWithEngine', () => { expect(backendInput.projectSecrets).toEqual({ GITHUB_TOKEN: 'proj-gh-token', TRELLO_API_KEY: 'proj-trello-key', + CASCADE_PROJECT_ID: 'test', + CASCADE_PROJECT_NAME: 'Test', CASCADE_BASE_BRANCH: 'main', CASCADE_REPO_OWNER: 'owner', CASCADE_REPO_NAME: 'repo', + CASCADE_TRELLO_BOARD_ID: 'b1', + CASCADE_TRELLO_LISTS: '{}', + CASCADE_TRELLO_LABELS: '{}', CASCADE_AGENT_TYPE: 'implementation', CASCADE_PM_TYPE: 'trello', }); @@ -713,9 +718,14 @@ describe('executeWithEngine', () => { const backendInput = vi.mocked(engine.execute).mock.calls[0][0]; expect(backendInput.projectSecrets).toEqual({ + CASCADE_PROJECT_ID: 'test', + CASCADE_PROJECT_NAME: 'Test', CASCADE_BASE_BRANCH: 'main', CASCADE_REPO_OWNER: 'owner', CASCADE_REPO_NAME: 'repo', + CASCADE_TRELLO_BOARD_ID: 'b1', + CASCADE_TRELLO_LISTS: '{}', + CASCADE_TRELLO_LABELS: '{}', CASCADE_AGENT_TYPE: 'implementation', CASCADE_PM_TYPE: 'trello', }); diff --git a/tests/unit/backends/agent-profiles.test.ts b/tests/unit/backends/agent-profiles.test.ts index 97b2857b8..f3b6f18d8 100644 --- a/tests/unit/backends/agent-profiles.test.ts +++ b/tests/unit/backends/agent-profiles.test.ts @@ -95,6 +95,7 @@ vi.mock('../../../src/gadgets/pm/index.js', () => ({ PMUpdateChecklistItem: mockClass('PMUpdateChecklistItem'), PostComment: mockClass('PostComment'), ReadWorkItem: mockClass('ReadWorkItem'), + ReportFriction: mockClass('ReportFriction'), UpdateWorkItem: mockClass('UpdateWorkItem'), })); diff --git a/tests/unit/backends/toolManifests.test.ts b/tests/unit/backends/toolManifests.test.ts index 4bd96a6a8..d941952e4 100644 --- a/tests/unit/backends/toolManifests.test.ts +++ b/tests/unit/backends/toolManifests.test.ts @@ -8,9 +8,9 @@ describe('getToolManifests', () => { expect(manifests.length).toBeGreaterThan(0); }); - it('returns exactly 20 tools', () => { + it('returns exactly 21 tools', () => { const manifests = getToolManifests(); - expect(manifests).toHaveLength(20); + expect(manifests).toHaveLength(21); }); it('every manifest has required fields: name, description, cliCommand, parameters', () => { @@ -40,6 +40,7 @@ describe('getToolManifests', () => { expect(names).toContain('PostComment'); expect(names).toContain('UpdateWorkItem'); expect(names).toContain('CreateWorkItem'); + expect(names).toContain('ReportFriction'); expect(names).toContain('ListWorkItems'); expect(names).toContain('AddChecklist'); expect(names).toContain('MoveWorkItem'); @@ -47,6 +48,20 @@ describe('getToolManifests', () => { expect(names).toContain('PMDeleteChecklistItem'); }); + it('ReportFriction has enum category/severity and details-file support', () => { + const manifests = getToolManifests(); + const reportFriction = manifests.find((m) => m.name === 'ReportFriction'); + expect(reportFriction).toBeDefined(); + expect(reportFriction?.cliCommand).toBe('cascade-tools pm report-friction'); + expect(reportFriction?.parameters).toMatchObject({ + summary: { type: 'string', required: true }, + details: { type: 'string', required: true }, + category: { type: 'string', required: true }, + severity: { type: 'string', required: true }, + 'details-file': { type: 'string' }, + }); + }); + it('includes GitHub PR tools', () => { const manifests = getToolManifests(); const names = manifests.map((m) => m.name); diff --git a/tests/unit/cli/file-input-flags.test.ts b/tests/unit/cli/file-input-flags.test.ts index d52acda3d..327a78558 100644 --- a/tests/unit/cli/file-input-flags.test.ts +++ b/tests/unit/cli/file-input-flags.test.ts @@ -30,6 +30,9 @@ vi.mock('../../../src/gadgets/pm/core/updateWorkItem.js', () => ({ vi.mock('../../../src/gadgets/pm/core/createWorkItem.js', () => ({ createWorkItem: vi.fn().mockResolvedValue({ id: 'wi-2' }), })); +vi.mock('../../../src/gadgets/pm/core/reportFriction.js', () => ({ + reportFriction: vi.fn().mockResolvedValue({ status: 'filed' }), +})); vi.mock('../../../src/gadgets/pm/core/postComment.js', () => ({ postComment: vi.fn().mockResolvedValue({ id: 'comment-1' }), })); @@ -42,6 +45,7 @@ vi.mock('../../../src/gadgets/github/core/postPRComment.js', () => ({ import CreateWorkItem from '../../../src/cli/pm/create-work-item.js'; import PostComment from '../../../src/cli/pm/post-comment.js'; +import ReportFriction from '../../../src/cli/pm/report-friction.js'; import UpdateWorkItem from '../../../src/cli/pm/update-work-item.js'; import CreatePR from '../../../src/cli/scm/create-pr.js'; import PostPRComment from '../../../src/cli/scm/post-pr-comment.js'; @@ -49,6 +53,7 @@ import { createPR } from '../../../src/gadgets/github/core/createPR.js'; import { postPRComment } from '../../../src/gadgets/github/core/postPRComment.js'; import { createWorkItem } from '../../../src/gadgets/pm/core/createWorkItem.js'; import { postComment } from '../../../src/gadgets/pm/core/postComment.js'; +import { reportFriction } from '../../../src/gadgets/pm/core/reportFriction.js'; import { updateWorkItem } from '../../../src/gadgets/pm/core/updateWorkItem.js'; let tmpDir: string; @@ -169,6 +174,57 @@ describe('CreateWorkItem --description-file', () => { }); }); +describe('ReportFriction --details-file', () => { + it('reads details from file', async () => { + const filePath = writeTempFile('friction.md', 'Friction details from file'); + const cmd = new ReportFriction( + [ + '--summary', + 'Friction summary', + '--details-file', + filePath, + '--category', + 'tooling', + '--severity', + 'medium', + ], + mockConfig as never, + ); + await cmd.run(); + + expect(reportFriction).toHaveBeenCalledWith( + expect.objectContaining({ + summary: 'Friction summary', + details: 'Friction details from file', + category: 'tooling', + severity: 'medium', + }), + ); + }); + + it('prefers --details-file over --details', async () => { + const filePath = writeTempFile('friction.md', 'from file'); + const cmd = new ReportFriction( + [ + '--summary', + 'Friction summary', + '--details', + 'from flag', + '--details-file', + filePath, + '--category', + 'tooling', + '--severity', + 'medium', + ], + mockConfig as never, + ); + await cmd.run(); + + expect(reportFriction).toHaveBeenCalledWith(expect.objectContaining({ details: 'from file' })); + }); +}); + describe('PostComment --text-file', () => { it('reads comment text from file', async () => { const filePath = writeTempFile('comment.md', 'Comment from file'); diff --git a/tests/unit/cli/pm/pm-commands.test.ts b/tests/unit/cli/pm/pm-commands.test.ts index 91b3dcd84..5e4f9a7cf 100644 --- a/tests/unit/cli/pm/pm-commands.test.ts +++ b/tests/unit/cli/pm/pm-commands.test.ts @@ -8,6 +8,7 @@ * - delete-checklist-item * - update-checklist-item * - create-work-item (basic param-passing) + * - report-friction (basic param-passing) * - post-comment (basic param-passing) */ @@ -55,6 +56,11 @@ vi.mock('../../../../src/gadgets/pm/core/updateChecklistItem.js', () => ({ vi.mock('../../../../src/gadgets/pm/core/createWorkItem.js', () => ({ createWorkItem: vi.fn().mockResolvedValue({ id: 'wi-new' }), })); +vi.mock('../../../../src/gadgets/pm/core/reportFriction.js', () => ({ + reportFriction: vi + .fn() + .mockResolvedValue({ status: 'filed', workItemUrl: 'https://pm/friction' }), +})); vi.mock('../../../../src/gadgets/pm/core/postComment.js', () => ({ postComment: vi.fn().mockResolvedValue({ id: 'comment-1' }), })); @@ -65,6 +71,7 @@ import ListWorkItems from '../../../../src/cli/pm/list-work-items.js'; import MoveWorkItem from '../../../../src/cli/pm/move-work-item.js'; import PostComment from '../../../../src/cli/pm/post-comment.js'; import ReadWorkItem from '../../../../src/cli/pm/read-work-item.js'; +import ReportFriction from '../../../../src/cli/pm/report-friction.js'; import UpdateChecklistItem from '../../../../src/cli/pm/update-checklist-item.js'; import { createWorkItem } from '../../../../src/gadgets/pm/core/createWorkItem.js'; import { deleteChecklistItem } from '../../../../src/gadgets/pm/core/deleteChecklistItem.js'; @@ -72,6 +79,7 @@ import { listWorkItems } from '../../../../src/gadgets/pm/core/listWorkItems.js' import { moveWorkItem } from '../../../../src/gadgets/pm/core/moveWorkItem.js'; import { postComment } from '../../../../src/gadgets/pm/core/postComment.js'; import { readWorkItem } from '../../../../src/gadgets/pm/core/readWorkItem.js'; +import { reportFriction } from '../../../../src/gadgets/pm/core/reportFriction.js'; import { updateChecklistItem } from '../../../../src/gadgets/pm/core/updateChecklistItem.js'; /** Create a fresh minimal oclif config to satisfy this.parse() in each test */ @@ -306,6 +314,64 @@ describe('CreateWorkItem command (basic params)', () => { }); }); +// --------------------------------------------------------------------------- +// report-friction (basic param-passing test) +// --------------------------------------------------------------------------- +describe('ReportFriction command (basic params)', () => { + it('passes summary, details, category, severity, and whileDoing to reportFriction', async () => { + const cmd = new ReportFriction( + [ + '--summary', + 'Friction summary', + '--details', + 'Details', + '--category', + 'tooling', + '--severity', + 'medium', + '--whileDoing', + 'Running tests', + ], + makeMockConfig() as never, + ); + await cmd.run(); + + expect(reportFriction).toHaveBeenCalledWith({ + summary: 'Friction summary', + details: 'Details', + category: 'tooling', + severity: 'medium', + whileDoing: 'Running tests', + }); + }); + + it('outputs JSON success result', async () => { + vi.mocked(reportFriction).mockResolvedValue({ + status: 'filed', + workItemUrl: 'https://pm/friction', + } as never); + const cmd = new ReportFriction( + [ + '--summary', + 'Friction summary', + '--details', + 'Details', + '--category', + 'tooling', + '--severity', + 'medium', + ], + makeMockConfig() as never, + ); + const logSpy = vi.spyOn(cmd, 'log'); + await cmd.run(); + + const output = JSON.parse(logSpy.mock.calls[0][0] as string); + expect(output.success).toBe(true); + expect(output.data).toEqual({ status: 'filed', workItemUrl: 'https://pm/friction' }); + }); +}); + // --------------------------------------------------------------------------- // post-comment (basic param-passing test) // --------------------------------------------------------------------------- diff --git a/tests/unit/gadgets/pm/core/reportFriction.test.ts b/tests/unit/gadgets/pm/core/reportFriction.test.ts new file mode 100644 index 000000000..a03dcc5b0 --- /dev/null +++ b/tests/unit/gadgets/pm/core/reportFriction.test.ts @@ -0,0 +1,137 @@ +import { readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const mockMaterializeFrictionReport = vi.fn(); + +vi.mock('../../../../../src/friction/materialize.js', () => ({ + materializeFrictionReport: (...args: unknown[]) => mockMaterializeFrictionReport(...args), +})); + +import { reportFriction } from '../../../../../src/gadgets/pm/core/reportFriction.js'; +import type { ProjectConfig } from '../../../../../src/types/index.js'; + +const project = { + id: 'project-1', + orgId: 'org-1', + name: 'Cascade', + repo: 'owner/repo', + pm: { type: 'trello' }, + trello: { + boardId: 'board-1', + lists: { friction: 'list-friction' }, + labels: {}, + }, +} as ProjectConfig; + +function sidecarPath(): string { + return join(tmpdir(), `cascade-report-friction-${Date.now()}-${Math.random()}.jsonl`); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('reportFriction', () => { + it('queues before filing and appends filed event after successful materialization', async () => { + const path = sidecarPath(); + mockMaterializeFrictionReport.mockResolvedValue({ + status: 'filed', + reportId: 'ignored', + workItemId: 'card-1', + workItemUrl: 'https://pm.example/card-1', + }); + + const result = await reportFriction({ + project, + sidecarPath: path, + summary: 'Missing setup hint', + details: 'The command needs an undocumented env var.', + category: 'environment', + severity: 'medium', + whileDoing: 'Running tests', + }); + + expect(result).toMatchObject({ + status: 'filed', + workItemId: 'card-1', + workItemUrl: 'https://pm.example/card-1', + }); + const events = readFileSync(path, 'utf-8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(events.map((event) => event.event)).toEqual(['queued', 'filed']); + expect(events[0].report.summary).toBe('Missing setup hint'); + expect(events[1].reportId).toBe(events[0].reportId); + rmSync(path, { force: true }); + }); + + it('keeps the queued event and returns queued_slot_missing when friction slot is absent', async () => { + const path = sidecarPath(); + mockMaterializeFrictionReport.mockResolvedValue({ + status: 'skipped', + reportId: 'friction-1', + reason: 'friction_slot_missing', + message: "Project project-1 has no 'friction' slot configured.", + }); + + const result = await reportFriction({ + project, + sidecarPath: path, + summary: 'No destination', + details: 'Cannot route friction.', + category: 'pm-data', + severity: 'low', + }); + + expect(result.status).toBe('queued_slot_missing'); + const events = readFileSync(path, 'utf-8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(events).toHaveLength(1); + expect(events[0].event).toBe('queued'); + rmSync(path, { force: true }); + }); + + it('keeps the queued event and returns queued_for_retry when filing fails', async () => { + const path = sidecarPath(); + mockMaterializeFrictionReport.mockRejectedValue(new Error('PM unavailable')); + + const result = await reportFriction({ + project, + sidecarPath: path, + summary: 'PM outage', + details: 'Create failed.', + category: 'tooling', + severity: 'high', + }); + + expect(result).toMatchObject({ + status: 'queued_for_retry', + message: expect.stringContaining('PM unavailable'), + }); + const events = readFileSync(path, 'utf-8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(events).toHaveLength(1); + expect(events[0].event).toBe('queued'); + rmSync(path, { force: true }); + }); + + it('validates category and severity in the core function', async () => { + await expect( + reportFriction({ + project, + sidecarPath: sidecarPath(), + summary: 'Bad category', + details: 'Invalid classification.', + category: 'invalid' as never, + severity: 'medium', + }), + ).rejects.toThrow('category must be one of'); + }); +}); diff --git a/tests/unit/gadgets/pm/definitions.test.ts b/tests/unit/gadgets/pm/definitions.test.ts index 1ca8e8f55..2d42f2650 100644 --- a/tests/unit/gadgets/pm/definitions.test.ts +++ b/tests/unit/gadgets/pm/definitions.test.ts @@ -8,6 +8,7 @@ import { pmUpdateChecklistItemDef, postCommentDef, readWorkItemDef, + reportFrictionDef, updateWorkItemDef, } from '../../../../src/gadgets/pm/definitions.js'; import type { ToolDefinition } from '../../../../src/gadgets/shared/toolDefinition.js'; @@ -17,6 +18,7 @@ const ALL_PM_DEFINITIONS: ToolDefinition[] = [ postCommentDef, updateWorkItemDef, createWorkItemDef, + reportFrictionDef, listWorkItemsDef, moveWorkItemDef, addChecklistDef, @@ -26,8 +28,8 @@ const ALL_PM_DEFINITIONS: ToolDefinition[] = [ describe('PM gadget definitions', () => { describe('all definitions integrity', () => { - it('exports exactly 9 definitions', () => { - expect(ALL_PM_DEFINITIONS).toHaveLength(9); + it('exports exactly 10 definitions', () => { + expect(ALL_PM_DEFINITIONS).toHaveLength(10); }); it('all definitions have unique names', () => { @@ -107,6 +109,10 @@ describe('PM gadget definitions', () => { expect(ALL_PM_DEFINITIONS.map((d) => d.name)).toContain('CreateWorkItem'); }); + it('includes ReportFriction', () => { + expect(ALL_PM_DEFINITIONS.map((d) => d.name)).toContain('ReportFriction'); + }); + it('includes ListWorkItems', () => { expect(ALL_PM_DEFINITIONS.map((d) => d.name)).toContain('ListWorkItems'); }); @@ -196,6 +202,46 @@ describe('PM gadget definitions', () => { }); }); + describe('reportFrictionDef', () => { + it('has required summary, details, category, and severity parameters', () => { + expect(reportFrictionDef.parameters.summary?.required).toBe(true); + expect(reportFrictionDef.parameters.details?.required).toBe(true); + expect(reportFrictionDef.parameters.category?.required).toBe(true); + expect(reportFrictionDef.parameters.severity?.required).toBe(true); + }); + + it('category and severity are enums with expected options', () => { + const category = reportFrictionDef.parameters.category; + const severity = reportFrictionDef.parameters.severity; + expect(category?.type).toBe('enum'); + expect((category as { options?: string[] }).options).toEqual([ + 'tooling', + 'environment', + 'permissions', + 'dependency', + 'test-failure', + 'pm-data', + 'scm-data', + 'other', + ]); + expect(severity?.type).toBe('enum'); + expect((severity as { options?: string[] }).options).toEqual([ + 'low', + 'medium', + 'high', + 'critical', + ]); + }); + + it('has details file input alternative', () => { + const detailsAlt = reportFrictionDef.cli?.fileInputAlternatives?.find( + (a) => a.paramName === 'details', + ); + expect(detailsAlt).toBeDefined(); + expect(detailsAlt?.fileFlag).toBe('details-file'); + }); + }); + // ─── ListWorkItems specific ──────────────────────────────────────────────── describe('listWorkItemsDef', () => { it('has required containerId parameter', () => { From 249079aeb84811b89e1f32a51d60bc01f4d67f78 Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 21:29:14 +0200 Subject: [PATCH 5/9] feat(agents): scope friction reporting capability (#1298) * feat(agents): scope friction reporting capability * fix(agents): move debug PM capabilities to optional to honour integration gate `debug` explicitly declares `integrations.optional: [pm]`, so all PM capabilities must live in `capabilities.optional` so that `resolveEffectiveCapabilities` can gate them on PM availability. Previously pm:read/write/checklist and pm:friction were all in `capabilities.required`, which caused `buildExecutionPlan` to unconditionally include `ReportFriction` in `availableTools` and append the friction guidance even on projects with no PM integration. Move all four PM capabilities to `capabilities.optional`, matching the pattern used by `review.yaml` and `respond-to-review.yaml`. Update the loader test to reflect that `deriveIntegrations` now correctly derives PM as optional (not required) for the debug profile. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Cascade Bot Co-authored-by: Claude Sonnet 4.6 --- docs/architecture/04-agent-system.md | 2 +- docs/architecture/07-gadgets.md | 4 +- src/agents/capabilities/index.ts | 3 +- src/agents/capabilities/registry.ts | 10 ++- src/agents/capabilities/resolver.ts | 4 +- src/agents/definitions/alerting.yaml | 1 + src/agents/definitions/backlog-manager.yaml | 1 + src/agents/definitions/debug.yaml | 5 +- src/agents/definitions/implementation.yaml | 1 + src/agents/definitions/planning.yaml | 1 + src/agents/definitions/profiles.ts | 17 ++-- src/agents/definitions/resolve-conflicts.yaml | 1 + src/agents/definitions/respond-to-ci.yaml | 1 + .../respond-to-planning-comment.yaml | 1 + .../definitions/respond-to-pr-comment.yaml | 1 + src/agents/definitions/respond-to-review.yaml | 1 + src/agents/definitions/review.yaml | 1 + src/agents/definitions/splitting.yaml | 1 + src/agents/shared/frictionGuidance.ts | 21 +++++ src/backends/secretOrchestrator.ts | 18 +++- .../unit/agents/capabilities/resolver.test.ts | 30 ++++++- tests/unit/agents/definitions/loader.test.ts | 25 +++++- .../agents/shared/frictionGuidance.test.ts | 26 ++++++ tests/unit/agents/shared/gadgets.test.ts | 10 ++- tests/unit/backends/adapter.test.ts | 1 + tests/unit/backends/agent-profiles.test.ts | 35 ++++++++ .../unit/backends/secretOrchestrator.test.ts | 89 +++++++++++++++++++ 27 files changed, 288 insertions(+), 23 deletions(-) create mode 100644 src/agents/shared/frictionGuidance.ts create mode 100644 tests/unit/agents/shared/frictionGuidance.test.ts diff --git a/docs/architecture/04-agent-system.md b/docs/architecture/04-agent-system.md index 89f3931eb..32ef886fb 100644 --- a/docs/architecture/04-agent-system.md +++ b/docs/architecture/04-agent-system.md @@ -135,7 +135,7 @@ const CAPABILITIES = [ // Built-in (always available) 'fs:read', 'fs:write', 'shell:exec', 'session:ctrl', // PM integration - 'pm:read', 'pm:write', 'pm:checklist', + 'pm:read', 'pm:write', 'pm:checklist', 'pm:friction', // SCM integration 'scm:read', 'scm:ci-logs', 'scm:comment', 'scm:review', 'scm:pr', // Alerting integration diff --git a/docs/architecture/07-gadgets.md b/docs/architecture/07-gadgets.md index 35ee8fd25..91d44ab36 100644 --- a/docs/architecture/07-gadgets.md +++ b/docs/architecture/07-gadgets.md @@ -54,7 +54,7 @@ All file gadgets validate paths against allowed directories (working directory + Todos are stored in `.claude/todos.json` within the repo working directory. -### PM (`pm:read`, `pm:write`, `pm:checklist`) +### PM (`pm:read`, `pm:write`, `pm:checklist`, `pm:friction`) | Gadget | Capability | Purpose | |--------|-----------|---------| @@ -62,12 +62,12 @@ Todos are stored in `.claude/todos.json` within the repo working directory. | `ListWorkItems` | `pm:read` | List work items with filters | | `UpdateWorkItem` | `pm:write` | Update work item fields | | `CreateWorkItem` | `pm:write` | Create new work item | -| `ReportFriction` | `pm:write` | Queue and file incidental friction reports | | `MoveWorkItem` | `pm:write` | Move work item to a status/list | | `PostComment` | `pm:write` | Post comment on work item | | `AddChecklist` | `pm:write` | Add checklist to work item | | `PMUpdateChecklistItem` | `pm:checklist` | Update checklist item status | | `PMDeleteChecklistItem` | `pm:checklist` | Delete checklist item | +| `ReportFriction` | `pm:friction` | Queue and file incidental friction reports | PM gadgets use the active `PMProvider` from `AsyncLocalStorage` context, making them provider-agnostic. diff --git a/src/agents/capabilities/index.ts b/src/agents/capabilities/index.ts index adfd76a56..9afddd967 100644 --- a/src/agents/capabilities/index.ts +++ b/src/agents/capabilities/index.ts @@ -7,8 +7,9 @@ * Integration Category → Capabilities → Gadgets/Tools * │ │ │ * pm → pm:read → ReadWorkItem, ListWorkItems - * pm:write → CreateWorkItem, ReportFriction, UpdateWorkItem, PostComment + * pm:write → CreateWorkItem, UpdateWorkItem, PostComment * pm:checklist → PMUpdateChecklistItem, PMDeleteChecklistItem + * pm:friction → ReportFriction * * scm → scm:read → GetPRDetails, GetPRDiff, GetPRChecks * scm:comment → PostPRComment, UpdatePRComment diff --git a/src/agents/capabilities/registry.ts b/src/agents/capabilities/registry.ts index 5f6b82c6a..fed012089 100644 --- a/src/agents/capabilities/registry.ts +++ b/src/agents/capabilities/registry.ts @@ -33,6 +33,7 @@ export const CAPABILITIES = [ 'pm:read', 'pm:write', 'pm:checklist', + 'pm:friction', // SCM integration capabilities 'scm:read', @@ -127,7 +128,6 @@ export const CAPABILITY_REGISTRY: Record = { gadgetNames: [ 'UpdateWorkItem', 'CreateWorkItem', - 'ReportFriction', 'MoveWorkItem', 'PostComment', 'AddChecklist', @@ -144,6 +144,14 @@ export const CAPABILITY_REGISTRY: Record = { cliToolNames: [], }, + 'pm:friction': { + integration: 'pm', + description: 'Report incidental PM-backed friction without general PM write access', + gadgetNames: ['ReportFriction'], + sdkToolNames: [], + cliToolNames: [], + }, + // ------------------------------------------------------------------------- // SCM integration capabilities // ------------------------------------------------------------------------- diff --git a/src/agents/capabilities/resolver.ts b/src/agents/capabilities/resolver.ts index a80186914..d96c0188d 100644 --- a/src/agents/capabilities/resolver.ts +++ b/src/agents/capabilities/resolver.ts @@ -104,7 +104,6 @@ const GADGET_CONSTRUCTORS: Record any> = { // pm:write UpdateWorkItem, CreateWorkItem, - ReportFriction, MoveWorkItem, PostComment, AddChecklist, @@ -113,6 +112,9 @@ const GADGET_CONSTRUCTORS: Record any> = { PMUpdateChecklistItem, PMDeleteChecklistItem, + // pm:friction + ReportFriction, + // scm:read GetPRDetails, GetPRDiff, diff --git a/src/agents/definitions/alerting.yaml b/src/agents/definitions/alerting.yaml index b2ddeaca4..6d5fec23f 100644 --- a/src/agents/definitions/alerting.yaml +++ b/src/agents/definitions/alerting.yaml @@ -20,6 +20,7 @@ capabilities: optional: - pm:read - pm:write + - pm:friction # Supported triggers for this agent triggers: diff --git a/src/agents/definitions/backlog-manager.yaml b/src/agents/definitions/backlog-manager.yaml index c502c6c31..53cf66569 100644 --- a/src/agents/definitions/backlog-manager.yaml +++ b/src/agents/definitions/backlog-manager.yaml @@ -17,6 +17,7 @@ capabilities: - session:ctrl - pm:read - pm:write + - pm:friction optional: [] # Supported triggers for this agent. diff --git a/src/agents/definitions/debug.yaml b/src/agents/definitions/debug.yaml index 6367c68e7..0408f046b 100644 --- a/src/agents/definitions/debug.yaml +++ b/src/agents/definitions/debug.yaml @@ -9,16 +9,17 @@ integrations: required: [] optional: [pm] -# Read-only FS access for log analysis, full PM access for creating debug cards. +# FS/session access always available; PM access gated on configured integration. capabilities: required: - fs:read - shell:exec - session:ctrl + optional: - pm:read - pm:write - pm:checklist - optional: [] + - pm:friction # Debug agent is triggered manually or via internal attachment upload detection. # No external event-based triggers are configured. diff --git a/src/agents/definitions/implementation.yaml b/src/agents/definitions/implementation.yaml index a6d1add6a..bcace71a7 100644 --- a/src/agents/definitions/implementation.yaml +++ b/src/agents/definitions/implementation.yaml @@ -20,6 +20,7 @@ capabilities: - pm:read - pm:write - pm:checklist + - pm:friction - scm:pr optional: [] diff --git a/src/agents/definitions/planning.yaml b/src/agents/definitions/planning.yaml index b8ef9c15e..8c37c505f 100644 --- a/src/agents/definitions/planning.yaml +++ b/src/agents/definitions/planning.yaml @@ -18,6 +18,7 @@ capabilities: - session:ctrl - pm:read - pm:write + - pm:friction optional: [] # Supported triggers for this agent diff --git a/src/agents/definitions/profiles.ts b/src/agents/definitions/profiles.ts index 3a0b94abf..009b73dfe 100644 --- a/src/agents/definitions/profiles.ts +++ b/src/agents/definitions/profiles.ts @@ -41,7 +41,7 @@ export type { AgentCapabilities } from './schema.js'; export interface AgentProfile { /** Filter the full set of tool manifests down to what this agent needs */ - filterTools(allTools: ToolManifest[]): ToolManifest[]; + filterTools(allTools: ToolManifest[], integrationChecker?: IntegrationChecker): ToolManifest[]; /** Engine-neutral capabilities used to derive native tools inside each engine */ allCapabilities: Capability[]; /** Whether this profile needs the GitHub client for context fetching */ @@ -166,9 +166,6 @@ function resolveContextPipeline( function buildProfileFromDefinition(def: AgentDefinition, agentType: string): AgentProfile { const allCapabilities = getAllCapabilities(def.capabilities); - // Derive tool names from capabilities for filtering - const gadgetNames = getGadgetNamesFromCapabilities(allCapabilities); - // Get gadget options from strategies const gadgetOptions = def.strategies.gadgetOptions; @@ -195,9 +192,17 @@ function buildProfileFromDefinition(def: AgentDefinition, agentType: string): Ag const lifecycle = resolveLifecycleHooks(def); const profile: AgentProfile = { - filterTools: (allTools: ToolManifest[]) => { + filterTools: (allTools: ToolManifest[], integrationChecker?: IntegrationChecker) => { + const effectiveCaps = integrationChecker + ? resolveEffectiveCapabilities( + def.capabilities.required, + def.capabilities.optional, + integrationChecker, + ) + : allCapabilities; + // Filter tools by the gadget names derived from capabilities - const nameSet = new Set(gadgetNames); + const nameSet = new Set(getGadgetNamesFromCapabilities(effectiveCaps)); return allTools.filter((t) => nameSet.has(t.name)); }, allCapabilities, diff --git a/src/agents/definitions/resolve-conflicts.yaml b/src/agents/definitions/resolve-conflicts.yaml index 77fcbc7d6..6753d9758 100644 --- a/src/agents/definitions/resolve-conflicts.yaml +++ b/src/agents/definitions/resolve-conflicts.yaml @@ -22,6 +22,7 @@ capabilities: - pm:read - pm:write - pm:checklist + - pm:friction # Supported triggers for this agent triggers: diff --git a/src/agents/definitions/respond-to-ci.yaml b/src/agents/definitions/respond-to-ci.yaml index 680ed1143..d67138f2d 100644 --- a/src/agents/definitions/respond-to-ci.yaml +++ b/src/agents/definitions/respond-to-ci.yaml @@ -23,6 +23,7 @@ capabilities: - pm:read - pm:write - pm:checklist + - pm:friction # Supported triggers for this agent triggers: diff --git a/src/agents/definitions/respond-to-planning-comment.yaml b/src/agents/definitions/respond-to-planning-comment.yaml index 6ee3989b8..3df20b462 100644 --- a/src/agents/definitions/respond-to-planning-comment.yaml +++ b/src/agents/definitions/respond-to-planning-comment.yaml @@ -18,6 +18,7 @@ capabilities: - pm:read - pm:write - pm:checklist + - pm:friction optional: [] # Supported triggers for this agent diff --git a/src/agents/definitions/respond-to-pr-comment.yaml b/src/agents/definitions/respond-to-pr-comment.yaml index da1ac847f..72119f668 100644 --- a/src/agents/definitions/respond-to-pr-comment.yaml +++ b/src/agents/definitions/respond-to-pr-comment.yaml @@ -21,6 +21,7 @@ capabilities: optional: - pm:read - pm:write + - pm:friction # Supported triggers for this agent triggers: diff --git a/src/agents/definitions/respond-to-review.yaml b/src/agents/definitions/respond-to-review.yaml index 2a3a60595..da1c3492f 100644 --- a/src/agents/definitions/respond-to-review.yaml +++ b/src/agents/definitions/respond-to-review.yaml @@ -22,6 +22,7 @@ capabilities: optional: - pm:read - pm:write + - pm:friction # Supported triggers for this agent triggers: diff --git a/src/agents/definitions/review.yaml b/src/agents/definitions/review.yaml index 4cab42201..f042a860f 100644 --- a/src/agents/definitions/review.yaml +++ b/src/agents/definitions/review.yaml @@ -21,6 +21,7 @@ capabilities: - scm:comment optional: - pm:read + - pm:friction # Supported triggers for this agent triggers: diff --git a/src/agents/definitions/splitting.yaml b/src/agents/definitions/splitting.yaml index d6796f6b9..534363e2d 100644 --- a/src/agents/definitions/splitting.yaml +++ b/src/agents/definitions/splitting.yaml @@ -19,6 +19,7 @@ capabilities: - pm:read - pm:write - pm:checklist + - pm:friction optional: [] # Supported triggers for this agent diff --git a/src/agents/shared/frictionGuidance.ts b/src/agents/shared/frictionGuidance.ts new file mode 100644 index 000000000..355cf5679 --- /dev/null +++ b/src/agents/shared/frictionGuidance.ts @@ -0,0 +1,21 @@ +import type { Capability } from '../capabilities/index.js'; + +export const FRICTION_REPORTING_GUIDANCE = `## Friction Reporting + +When the ReportFriction tool is available, use it only for incidental papercuts in the environment, tooling, repository setup, documentation, or developer workflow that make the work harder than it should be. + +Do not report core task difficulty, expected debugging effort, product ambiguity that belongs in the current work item, or issues you can resolve directly as part of the assigned task. + +Keep working after reporting friction unless the issue blocks progress. If blocked, report the friction with concrete context and then explain the blocker in your final response.`; + +export function shouldAppendFrictionGuidance(capabilities: readonly Capability[]): boolean { + return capabilities.includes('pm:friction'); +} + +export function appendFrictionGuidance( + systemPrompt: string, + capabilities: readonly Capability[], +): string { + if (!shouldAppendFrictionGuidance(capabilities)) return systemPrompt; + return `${systemPrompt.trimEnd()}\n\n${FRICTION_REPORTING_GUIDANCE}`; +} diff --git a/src/backends/secretOrchestrator.ts b/src/backends/secretOrchestrator.ts index c2f394428..59f1f2df6 100644 --- a/src/backends/secretOrchestrator.ts +++ b/src/backends/secretOrchestrator.ts @@ -1,8 +1,13 @@ +import { + createIntegrationChecker, + resolveEffectiveCapabilities, +} from '../agents/capabilities/resolver.js'; import { needsGitStateStopHooks } from '../agents/definitions/index.js'; import { getAgentProfile } from '../agents/definitions/profiles.js'; import { getToolManifests } from '../agents/definitions/toolManifests.js'; import type { PromptContext } from '../agents/prompts/index.js'; import type { LogWriter } from '../agents/shared/executionPipeline.js'; +import { appendFrictionGuidance } from '../agents/shared/frictionGuidance.js'; import { resolveModelConfig } from '../agents/shared/modelResolution.js'; import { buildPromptContext } from '../agents/shared/promptContext.js'; import type { createAgentLogger } from '../agents/utils/logging.js'; @@ -89,7 +94,7 @@ export async function buildExecutionPlan( // DB not available — fall back to disk-only partials } - const { + let { systemPrompt, taskPrompt: taskPromptOverride, model: rawModel, @@ -109,6 +114,13 @@ export async function buildExecutionPlan( const model = engine.resolveModel ? engine.resolveModel(rawModel) : rawModel; const profile = await getAgentProfile(agentType); + const integrationChecker = await createIntegrationChecker(project.id); + const effectiveCapabilities = resolveEffectiveCapabilities( + profile.capabilities.required, + profile.capabilities.optional, + integrationChecker, + ); + systemPrompt = appendFrictionGuidance(systemPrompt, effectiveCapabilities); // Use profile to fetch agent-specific context injections const contextInjections = await profile.fetchContext({ @@ -177,14 +189,14 @@ export async function buildExecutionPlan( taskPrompt: taskPromptOverride ?? profile.buildTaskPrompt(input), cliToolsDir, nativeToolShimDir: nativeToolRuntime?.shimDir, - availableTools: profile.filterTools(getToolManifests()), + availableTools: profile.filterTools(getToolManifests(), integrationChecker), contextInjections, maxIterations, budgetUsd: input.remainingBudgetUsd as number | undefined, model, logWriter, agentInput: input, - nativeToolCapabilities: profile.allCapabilities, + nativeToolCapabilities: effectiveCapabilities, completionRequirements, enableStopHooks: needsGitStateStopHooks(profile.finishHooks), blockGitPush: profile.finishHooks.blockGitPush, diff --git a/tests/unit/agents/capabilities/resolver.test.ts b/tests/unit/agents/capabilities/resolver.test.ts index e909091d0..6ad31599b 100644 --- a/tests/unit/agents/capabilities/resolver.test.ts +++ b/tests/unit/agents/capabilities/resolver.test.ts @@ -90,6 +90,11 @@ describe('deriveRequiredIntegrations', () => { expect(deriveRequiredIntegrations(caps)).toEqual(['pm']); }); + it('returns pm for pm:friction capability', () => { + const caps: Capability[] = ['pm:friction']; + expect(deriveRequiredIntegrations(caps)).toEqual(['pm']); + }); + it('returns scm for scm:pr capability', () => { const caps: Capability[] = ['scm:pr']; expect(deriveRequiredIntegrations(caps)).toEqual(['scm']); @@ -150,11 +155,12 @@ describe('resolveEffectiveCapabilities', () => { it('includes optional capabilities when their integration is available', () => { const required: Capability[] = ['fs:read', 'scm:pr']; - const optional: Capability[] = ['pm:read', 'pm:write']; + const optional: Capability[] = ['pm:read', 'pm:write', 'pm:friction']; const hasIntegration = (cat: IntegrationCategory) => cat === 'pm'; const result = resolveEffectiveCapabilities(required, optional, hasIntegration); expect(result).toContain('pm:read'); expect(result).toContain('pm:write'); + expect(result).toContain('pm:friction'); }); it('excludes optional capabilities when their integration is not available', () => { @@ -205,11 +211,12 @@ describe('generateUnavailableCapabilitiesNote', () => { }); it('generates note for unavailable PM capabilities', () => { - const unavailable: Capability[] = ['pm:read', 'pm:write']; + const unavailable: Capability[] = ['pm:read', 'pm:friction']; const note = generateUnavailableCapabilitiesNote(unavailable); expect(note).toContain('PM integration'); expect(note).toContain('not configured'); expect(note).toContain('ReadWorkItem'); + expect(note).toContain('ReportFriction'); }); it('generates note for multiple unavailable integrations', () => { @@ -236,6 +243,11 @@ describe('getGadgetNamesFromCapabilities', () => { const readFileCount = names.filter((n) => n === 'ReadFile').length; expect(readFileCount).toBe(1); }); + + it('keeps ReportFriction scoped to pm:friction', () => { + expect(getGadgetNamesFromCapabilities(['pm:write'])).not.toContain('ReportFriction'); + expect(getGadgetNamesFromCapabilities(['pm:friction'])).toEqual(['ReportFriction']); + }); }); describe('getSdkToolsFromCapabilities', () => { @@ -287,6 +299,20 @@ describe('filterToolManifests', () => { expect(filtered.map((m) => m.name)).toContain('WriteFile'); }); + it('filters ReportFriction by pm:friction instead of pm:write', () => { + const manifests: ToolManifest[] = [ + { name: 'UpdateWorkItem', description: 'Update', inputSchema: {} }, + { name: 'ReportFriction', description: 'Report friction', inputSchema: {} }, + ]; + + expect(filterToolManifests(manifests, ['pm:write']).map((m) => m.name)).toEqual([ + 'UpdateWorkItem', + ]); + expect(filterToolManifests(manifests, ['pm:friction']).map((m) => m.name)).toEqual([ + 'ReportFriction', + ]); + }); + it('logs warning for missing expected tools', () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); const manifests: ToolManifest[] = [ diff --git a/tests/unit/agents/definitions/loader.test.ts b/tests/unit/agents/definitions/loader.test.ts index 3fc3622a0..3908d1a9e 100644 --- a/tests/unit/agents/definitions/loader.test.ts +++ b/tests/unit/agents/definitions/loader.test.ts @@ -304,6 +304,7 @@ describe('YAML agent definitions loader', () => { expect(allCaps.includes('fs:write')).toBe(true); expect(allCaps.includes('scm:pr')).toBe(true); expect(allCaps.includes('pm:checklist')).toBe(true); + expect(allCaps.includes('pm:friction')).toBe(true); expect(def.hooks?.finish?.scm?.requiresPR).toBe(true); expect(def.integrations?.required).toContain('scm'); }); @@ -389,6 +390,24 @@ describe('YAML agent definitions loader', () => { } }); + it('requires pm:friction when PM is required and makes it optional when PM is optional', () => { + for (const agentType of ALL_AGENT_TYPES) { + const def = loadBuiltinDefinition(agentType); + const integrations = deriveIntegrations( + def.capabilities.required, + def.capabilities.optional, + ); + + if (integrations.required.includes('pm')) { + expect(def.capabilities.required, agentType).toContain('pm:friction'); + expect(def.capabilities.optional, agentType).not.toContain('pm:friction'); + } else if (integrations.optional.includes('pm')) { + expect(def.capabilities.required, agentType).not.toContain('pm:friction'); + expect(def.capabilities.optional, agentType).toContain('pm:friction'); + } + } + }); + it('implementation agent requires scm and pm (derived from capabilities)', () => { const def = loadBuiltinDefinition('implementation'); const integrations = deriveIntegrations(def.capabilities.required, def.capabilities.optional); @@ -446,11 +465,11 @@ describe('YAML agent definitions loader', () => { expect(integrations.optional).toEqual([]); }); - it('debug agent requires pm only', () => { + it('debug agent has pm optional (no required integrations)', () => { const def = loadBuiltinDefinition('debug'); const integrations = deriveIntegrations(def.capabilities.required, def.capabilities.optional); - expect(integrations.required).toEqual(['pm']); - expect(integrations.optional).toEqual([]); + expect(integrations.required).toEqual([]); + expect(integrations.optional).toEqual(['pm']); }); it('all derived integration categories are valid', () => { diff --git a/tests/unit/agents/shared/frictionGuidance.test.ts b/tests/unit/agents/shared/frictionGuidance.test.ts new file mode 100644 index 000000000..b38103cdf --- /dev/null +++ b/tests/unit/agents/shared/frictionGuidance.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { + appendFrictionGuidance, + FRICTION_REPORTING_GUIDANCE, + shouldAppendFrictionGuidance, +} from '../../../../src/agents/shared/frictionGuidance.js'; + +describe('friction reporting guidance', () => { + it('is gated on pm:friction capability availability', () => { + expect(shouldAppendFrictionGuidance(['pm:friction'])).toBe(true); + expect(shouldAppendFrictionGuidance(['pm:write'])).toBe(false); + }); + + it('appends central guidance only when ReportFriction is available', () => { + expect(appendFrictionGuidance('Base prompt', ['pm:write'])).toBe('Base prompt'); + expect(appendFrictionGuidance('Base prompt', ['pm:friction'])).toContain( + FRICTION_REPORTING_GUIDANCE, + ); + }); + + it('limits reports to incidental papercuts and tells agents to keep working unless blocked', () => { + expect(FRICTION_REPORTING_GUIDANCE).toContain('incidental papercuts'); + expect(FRICTION_REPORTING_GUIDANCE).toContain('Do not report core task difficulty'); + expect(FRICTION_REPORTING_GUIDANCE).toContain('Keep working after reporting friction unless'); + }); +}); diff --git a/tests/unit/agents/shared/gadgets.test.ts b/tests/unit/agents/shared/gadgets.test.ts index 95860db6b..8e3189af4 100644 --- a/tests/unit/agents/shared/gadgets.test.ts +++ b/tests/unit/agents/shared/gadgets.test.ts @@ -119,9 +119,15 @@ describe('buildGadgetsFromCapabilities', () => { const gadgets = names(buildGadgetsFromCapabilities(caps)); expect(gadgets).toContain('UpdateWorkItem'); expect(gadgets).toContain('CreateWorkItem'); - expect(gadgets).toContain('ReportFriction'); expect(gadgets).toContain('PostComment'); expect(gadgets).toContain('AddChecklist'); + expect(gadgets).not.toContain('ReportFriction'); + }); + + it('pm:friction includes only ReportFriction', () => { + const caps: Capability[] = ['pm:friction']; + const gadgets = names(buildGadgetsFromCapabilities(caps)); + expect(gadgets).toEqual(['ReportFriction']); }); it('pm:checklist includes checklist gadgets', () => { @@ -173,6 +179,7 @@ describe('buildGadgetsFromCapabilities', () => { 'pm:read', 'pm:write', 'pm:checklist', + 'pm:friction', 'scm:pr', ]; const gadgets = names(buildGadgetsFromCapabilities(caps)); @@ -185,6 +192,7 @@ describe('buildGadgetsFromCapabilities', () => { // PM expect(gadgets).toContain('ReadWorkItem'); expect(gadgets).toContain('PMUpdateChecklistItem'); + expect(gadgets).toContain('ReportFriction'); // SCM expect(gadgets).toContain('CreatePR'); // Session diff --git a/tests/unit/backends/adapter.test.ts b/tests/unit/backends/adapter.test.ts index 6d7bbed9b..b84eb72ca 100644 --- a/tests/unit/backends/adapter.test.ts +++ b/tests/unit/backends/adapter.test.ts @@ -497,6 +497,7 @@ describe('executeWithEngine', () => { makeMockProfile({ filterTools, allCapabilities: ['fs:read', 'shell:exec'], + capabilities: { required: ['fs:read', 'shell:exec'], optional: [] }, finishHooks: {}, }), ); diff --git a/tests/unit/backends/agent-profiles.test.ts b/tests/unit/backends/agent-profiles.test.ts index f3b6f18d8..468a0a449 100644 --- a/tests/unit/backends/agent-profiles.test.ts +++ b/tests/unit/backends/agent-profiles.test.ts @@ -239,6 +239,7 @@ describe('getAgentProfile', () => { 'pm:read', 'pm:write', 'pm:checklist', + 'pm:friction', ]), ); }); @@ -317,6 +318,7 @@ describe('getAgentProfile', () => { 'scm:comment', 'pm:read', 'pm:write', + 'pm:friction', ]), ); }); @@ -465,6 +467,39 @@ describe('AgentProfile.getLlmistGadgets', () => { expect(names).toContain('Finish'); }); + it('review exposes optional ReportFriction only when PM is configured', async () => { + const profile = await getAgentProfile('review'); + const tools = [ + { name: 'CreatePRReview', description: '', cliCommand: '', parameters: {} }, + { name: 'ReportFriction', description: '', cliCommand: '', parameters: {} }, + ]; + + expect(profile.filterTools(tools, (category) => category !== 'pm').map((t) => t.name)).toEqual([ + 'CreatePRReview', + ]); + expect(profile.filterTools(tools, (category) => category === 'pm').map((t) => t.name)).toEqual([ + 'CreatePRReview', + 'ReportFriction', + ]); + }); + + it('alerting exposes optional ReportFriction only when PM is configured', async () => { + const profile = await getAgentProfile('alerting'); + const tools = [ + { name: 'GetAlertingIssue', description: '', cliCommand: '', parameters: {} }, + { name: 'ReportFriction', description: '', cliCommand: '', parameters: {} }, + ]; + + expect( + profile.filterTools(tools, (category) => category === 'alerting').map((t) => t.name), + ).toEqual(['GetAlertingIssue']); + expect( + profile + .filterTools(tools, (category) => category === 'alerting' || category === 'pm') + .map((t) => t.name), + ).toEqual(['GetAlertingIssue', 'ReportFriction']); + }); + it('respond-to-review includes file editing and review comment tools', async () => { const profile = await getAgentProfile('respond-to-review'); const names = gadgetNames(profile.getLlmistGadgets()); diff --git a/tests/unit/backends/secretOrchestrator.test.ts b/tests/unit/backends/secretOrchestrator.test.ts index d5f9b720d..76fb8e887 100644 --- a/tests/unit/backends/secretOrchestrator.test.ts +++ b/tests/unit/backends/secretOrchestrator.test.ts @@ -31,9 +31,15 @@ vi.mock('../../../src/agents/definitions/profiles.js', () => ({ fetchContext: vi.fn().mockResolvedValue({}), finishHooks: {}, filterTools: vi.fn().mockReturnValue([]), + capabilities: { required: ['fs:read'], optional: [] }, }), })); +vi.mock('../../../src/agents/capabilities/resolver.js', () => ({ + createIntegrationChecker: vi.fn().mockResolvedValue(() => true), + resolveEffectiveCapabilities: vi.fn((required, optional) => [...required, ...optional]), +})); + vi.mock('../../../src/agents/definitions/toolManifests.js', () => ({ getToolManifests: vi.fn().mockReturnValue([]), })); @@ -57,6 +63,11 @@ vi.mock('../../../src/backends/sidecarManager.js', () => ({ createCompletionArtifacts: vi.fn().mockReturnValue({}), })); +import { + createIntegrationChecker, + resolveEffectiveCapabilities, +} from '../../../src/agents/capabilities/resolver.js'; +import { getAgentProfile } from '../../../src/agents/definitions/profiles.js'; import { buildPromptContext } from '../../../src/agents/shared/promptContext.js'; import { buildExecutionPlan, @@ -70,6 +81,9 @@ import { getDashboardUrl } from '../../../src/utils/runLink.js'; const mockGetDashboardUrl = vi.mocked(getDashboardUrl); const mockGetSentryIntegrationConfig = vi.mocked(getSentryIntegrationConfig); const mockBuildPromptContext = vi.mocked(buildPromptContext); +const mockCreateIntegrationChecker = vi.mocked(createIntegrationChecker); +const mockResolveEffectiveCapabilities = vi.mocked(resolveEffectiveCapabilities); +const mockGetAgentProfile = vi.mocked(getAgentProfile); function makeProject(overrides?: Partial): ProjectConfig { return { @@ -203,6 +217,81 @@ describe('buildExecutionPlan', () => { undefined, ); }); + + it('filters native tool manifests and capabilities through effective capabilities', async () => { + const checker = vi.fn().mockReturnValue(false); + mockCreateIntegrationChecker.mockResolvedValueOnce(checker); + mockResolveEffectiveCapabilities.mockReturnValueOnce(['fs:read']); + const filterTools = vi.fn().mockReturnValue([{ name: 'ReadFile' }]); + mockGetAgentProfile.mockReturnValueOnce({ + fetchContext: vi.fn().mockResolvedValue([]), + finishHooks: {}, + lifecycleHooks: {}, + filterTools, + allCapabilities: ['fs:read', 'pm:friction'], + needsGitHubToken: false, + buildTaskPrompt: () => 'task', + capabilities: { required: ['fs:read'], optional: ['pm:friction'] }, + getLlmistGadgets: vi.fn(), + }); + + const plan = await buildExecutionPlan( + 'review', + makeInput(makeProject(), 'manual'), + '/repo', + noopLogWriter, + noopAgentLogger, + undefined, + false, + 'claude-code', + engine, + ); + + expect(mockResolveEffectiveCapabilities).toHaveBeenCalledWith( + ['fs:read'], + ['pm:friction'], + checker, + ); + expect(filterTools).toHaveBeenCalledWith(expect.any(Array), checker); + expect(plan.nativeToolCapabilities).toEqual(['fs:read']); + expect(plan.availableTools).toEqual([{ name: 'ReadFile' }]); + }); + + it('appends friction guidance only when pm:friction is effective', async () => { + mockResolveEffectiveCapabilities.mockReturnValueOnce(['fs:read', 'pm:friction']); + + const withFriction = await buildExecutionPlan( + 'implementation', + makeInput(makeProject(), 'manual'), + '/repo', + noopLogWriter, + noopAgentLogger, + undefined, + false, + 'claude-code', + engine, + ); + + expect(withFriction.systemPrompt).toContain('Friction Reporting'); + expect(withFriction.systemPrompt).toContain('incidental papercuts'); + expect(withFriction.systemPrompt).toContain('Keep working after reporting friction unless'); + + mockResolveEffectiveCapabilities.mockReturnValueOnce(['fs:read']); + + const withoutFriction = await buildExecutionPlan( + 'review', + makeInput(makeProject(), 'manual'), + '/repo', + noopLogWriter, + noopAgentLogger, + undefined, + false, + 'claude-code', + engine, + ); + + expect(withoutFriction.systemPrompt).not.toContain('Friction Reporting'); + }); }); describe('injectRunLinkSecrets', () => { From a2e43be81f80773a3e7920d7594b5b7a0f11c7c3 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sat, 9 May 2026 19:40:18 +0000 Subject: [PATCH 6/9] fix(triggers): disabled trigger no longer shadows sibling matchers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the prod regression on 2026-05-09 where `PROpenedTrigger`'s structured-skip-on-disabled shadowed `PRConflictDetectedTrigger` for `pull_request: opened` events. On `zbigniewsobiecki/ucho` PR #367 the resolve-conflicts agent never fired even though resolve-conflicts was enabled, because review-on-open was disabled at config and its non-null structured skip halted the registry's first-match-wins loop before the second matcher got a chance. **Root cause.** `TriggerRegistry.dispatch` (`src/triggers/registry.ts`) loops through handlers and stops at the first non-null result. Two distinct semantic classes were collapsed into the same return shape: - "I don't claim this event today" (trigger disabled-at-config) - "I claim this event but my preconditions failed" (wrong base, wrong author, attempt limit) Both produced a structured `{ agentType: null, skipReason: { ... } }` result. Only the second class should halt the registry; the first should continue iteration. Five production sites repeated the wrong pattern (centralized `checkTriggerEnablement` + four inline call sites in `pr-opened`, `pr-merged`, `pr-ready-to-merge`, `check-suite-success`). Six handlers consumed the also-wrong `gateTriggerEnabled` wrapper. **Fix.** - `checkTriggerEnablement` returns `skipResult: null` for disabled, not a structured skip. Operator visibility preserved via the existing INFO-level `Trigger disabled by config, skipping` log. - Inline disabled-skip sites in `pr-opened`, `pr-merged`, `pr-ready-to-merge`, `check-suite-success` flipped to `return null;`. - `gateTriggerEnabled` deleted (now redundant with `checkTriggerEnabled`). Six call sites migrated to `if (!(await checkTriggerEnabled(...))) return null;`. `dispatchRespondToCi` return type widened to `Promise` to match. - New end-to-end regression test `tests/unit/triggers/disabled-trigger-shadowing.test.ts` — wires PROpenedTrigger + PRConflictDetectedTrigger into a real registry, mocks the config-resolver to disable review and enable resolve-conflicts, dispatches a `pull_request: opened` event, asserts resolve-conflicts fires. Sanity-verified against pre-fix code: the test fails loudly with `expected null to be 'resolve-conflicts'`, proving it actually catches the bug class. - Trigger-event-consistency static guard extended to recognize `dispatchRespondToCi(` as a cross-file emission marker (since `check-suite-failure.ts` now gates on `scm:check-suite-failure` via `checkTriggerEnabled` directly and emits through the dispatch helper). **Affected matcher pairs.** `pull_request: opened` is the only known shadowing pair in the registry (PROpened ⊕ PRConflictDetected). PM matchers (Trello/JIRA/Linear) don't overlap on the same payload shape. Other handlers using the same disabled-skip pattern aren't actively biting anything but were fixed for symmetry — same bug class, same fix, one PR. Webhook decisionReasons shift from per-handler "Trigger pr-opened skipped: review trigger is disabled..." to the registry-level "No trigger matched" when no other handler claims. The per-handler INFO log is unchanged, so operators can still grep Loki for disabled-trigger evidence. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/triggers/github/check-suite-failure.ts | 28 ++-- src/triggers/github/check-suite-success.ts | 6 +- src/triggers/github/pr-comment-mention.ts | 23 ++- src/triggers/github/pr-conflict-detected.ts | 22 ++- src/triggers/github/pr-merged.ts | 6 +- src/triggers/github/pr-opened.ts | 7 +- src/triggers/github/pr-ready-to-merge.ts | 6 +- src/triggers/github/pr-review-submitted.ts | 23 ++- src/triggers/github/respond-to-ci-dispatch.ts | 28 ++-- src/triggers/github/review-requested.ts | 16 +- src/triggers/shared/gates.ts | 18 -- src/triggers/shared/trigger-check.ts | 26 ++- tests/helpers/sharedMocks.ts | 17 +- tests/integration/github-personas.test.ts | 2 +- .../unit/triggers/check-suite-failure.test.ts | 6 +- .../unit/triggers/check-suite-success.test.ts | 8 +- .../disabled-trigger-shadowing.test.ts | 158 ++++++++++++++++++ .../github-pr-comment-mention.test.ts | 8 +- .../triggers/pr-conflict-detected.test.ts | 9 +- tests/unit/triggers/pr-merged.test.ts | 8 +- tests/unit/triggers/pr-opened.test.ts | 10 +- tests/unit/triggers/pr-ready-to-merge.test.ts | 8 +- .../unit/triggers/pr-review-submitted.test.ts | 8 +- tests/unit/triggers/review-requested.test.ts | 8 +- tests/unit/triggers/shared/gates.test.ts | 58 +------ .../triggers/shared/trigger-check.test.ts | 20 +-- .../trigger-event-consistency.test.ts | 12 ++ 27 files changed, 375 insertions(+), 174 deletions(-) create mode 100644 tests/unit/triggers/disabled-trigger-shadowing.test.ts diff --git a/src/triggers/github/check-suite-failure.ts b/src/triggers/github/check-suite-failure.ts index 8c3dd152b..f7d9a6995 100644 --- a/src/triggers/github/check-suite-failure.ts +++ b/src/triggers/github/check-suite-failure.ts @@ -2,13 +2,9 @@ import { githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; -import { - gateBaseBranch, - gateCascadePersona, - gateTriggerEnabled, - requirePersonaIdentities, -} from '../shared/gates.js'; +import { gateBaseBranch, gateCascadePersona, requirePersonaIdentities } from '../shared/gates.js'; import { skip } from '../shared/skip.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { decideCheckSuiteOutcome } from './check-suite-decision.js'; import { resolveCheckSuitePRNumber } from './pr-resolution.js'; import { dispatchRespondToCi, resetFixAttempts } from './respond-to-ci-dispatch.js'; @@ -40,13 +36,19 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { // `dispatchRespondToCi` re-checks the same gate (it's the single source of // truth for the success-handler's mixed-state fork too); the redundant call // here is one DB lookup, which the trigger-enabled cache absorbs. - const enabled = await gateTriggerEnabled( - ctx.project.id, - 'respond-to-ci', - 'scm:check-suite-failure', - this.name, - ); - if (enabled) return enabled; + // Disabled-at-config returns null so the registry's first-match loop + // continues to the next matcher — see `src/triggers/shared/trigger-check.ts` + // for the disabled-shadowing contract. + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-ci', + 'scm:check-suite-failure', + this.name, + )) + ) { + return null; + } const payload = ctx.payload as GitHubCheckSuitePayload; const { owner, repo } = parseRepoFullName(payload.repository.full_name); diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index ca12d78e4..946519de9 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -84,7 +84,11 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { this.name, ); if (!triggerConfig.enabled) { - return skip(this.name, 'review trigger is disabled for this project'); + // Disabled-at-config returns null (not a structured skip) so + // the registry's first-match loop can continue to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for the + // disabled-shadowing contract. + return null; } const payload = ctx.payload as GitHubCheckSuitePayload; diff --git a/src/triggers/github/pr-comment-mention.ts b/src/triggers/github/pr-comment-mention.ts index 126c9eb57..14cc27597 100644 --- a/src/triggers/github/pr-comment-mention.ts +++ b/src/triggers/github/pr-comment-mention.ts @@ -3,8 +3,9 @@ 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 { gateTriggerEnabled, requirePersonaIdentities } from '../shared/gates.js'; +import { requirePersonaIdentities } from '../shared/gates.js'; import { skip } from '../shared/skip.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { isGitHubIssueCommentPayload, isGitHubPRReviewCommentPayload } from './types.js'; import { resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js'; @@ -36,13 +37,19 @@ export class PRCommentMentionTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { - const enabled = await gateTriggerEnabled( - ctx.project.id, - 'respond-to-pr-comment', - 'scm:pr-comment-mention', - this.name, - ); - if (enabled) return enabled; + // Disabled-at-config returns null so the registry's first-match loop + // continues to the next matcher — see `src/triggers/shared/trigger-check.ts` + // for the disabled-shadowing contract. + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-pr-comment', + 'scm:pr-comment-mention', + this.name, + )) + ) { + return null; + } // Pre-extract prNumber from whichever payload type matches so subsequent // skip-reasons carry PR context (operator-friendly diagnostics). diff --git a/src/triggers/github/pr-conflict-detected.ts b/src/triggers/github/pr-conflict-detected.ts index 20b80c060..ef8035017 100644 --- a/src/triggers/github/pr-conflict-detected.ts +++ b/src/triggers/github/pr-conflict-detected.ts @@ -6,11 +6,11 @@ import { gateAttemptLimit, gateBaseBranch, gateCascadePersona, - gateTriggerEnabled, requirePersonaIdentities, } from '../shared/gates.js'; import { buildDeferredRecheckResult } from '../shared/result-builders.js'; import { skip } from '../shared/skip.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { buildResolveConflictsResult } from './result-builders.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -64,13 +64,19 @@ export class PRConflictDetectedTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { - const enabled = await gateTriggerEnabled( - ctx.project.id, - 'resolve-conflicts', - 'scm:pr-conflict-detected', - this.name, - ); - if (enabled) return enabled; + // Disabled-at-config returns null so the registry's first-match loop + // continues to the next matcher — see `src/triggers/shared/trigger-check.ts` + // for the disabled-shadowing contract. + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'resolve-conflicts', + 'scm:pr-conflict-detected', + this.name, + )) + ) { + return null; + } const payload = ctx.payload as GitHubPullRequestPayload; const prNumber = payload.pull_request.number; diff --git a/src/triggers/github/pr-merged.ts b/src/triggers/github/pr-merged.ts index 9266d150b..3e8ff4f2f 100644 --- a/src/triggers/github/pr-merged.ts +++ b/src/triggers/github/pr-merged.ts @@ -26,7 +26,11 @@ export class PRMergedTrigger implements TriggerHandler { async handle(ctx: TriggerContext): Promise { // Check lifecycle trigger config (stored in project_integrations.triggers) if (!(await isLifecycleTriggerEnabled(ctx.project.id, 'prMerged', this.name))) { - return skip(this.name, 'prMerged lifecycle trigger is disabled for this project'); + // Disabled-at-config returns null (not a structured skip) so + // the registry's first-match loop can continue to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for + // the disabled-shadowing contract. + return null; } const payload = ctx.payload as GitHubPullRequestPayload; diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index 60c9086c6..ece4b82e6 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -35,7 +35,12 @@ export class PROpenedTrigger implements TriggerHandler { this.name, ); if (!triggerConfig.enabled) { - return skip(this.name, 'review trigger is disabled for this project'); + // Disabled-at-config returns null (not a structured skip) so + // the registry's first-match loop continues to the next matcher + // — `PRConflictDetectedTrigger` also matches `pull_request: opened` + // and must not be shadowed when review is opt-out. + // See `src/triggers/shared/trigger-check.ts` for the contract. + return null; } const payload = ctx.payload as { diff --git a/src/triggers/github/pr-ready-to-merge.ts b/src/triggers/github/pr-ready-to-merge.ts index 8c617be92..b67f08868 100644 --- a/src/triggers/github/pr-ready-to-merge.ts +++ b/src/triggers/github/pr-ready-to-merge.ts @@ -122,7 +122,11 @@ export class PRReadyToMergeTrigger implements TriggerHandler { async handle(ctx: TriggerContext): Promise { // Check lifecycle trigger config (stored in project_integrations.triggers) if (!(await isLifecycleTriggerEnabled(ctx.project.id, 'prReadyToMerge', this.name))) { - return skip(this.name, 'prReadyToMerge lifecycle trigger is disabled for this project'); + // Disabled-at-config returns null (not a structured skip) so + // the registry's first-match loop can continue to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for the + // disabled-shadowing contract. + return null; } let prNumber: number; diff --git a/src/triggers/github/pr-review-submitted.ts b/src/triggers/github/pr-review-submitted.ts index fe56eb3db..dfec65599 100644 --- a/src/triggers/github/pr-review-submitted.ts +++ b/src/triggers/github/pr-review-submitted.ts @@ -1,8 +1,9 @@ import { getPersonaForLogin } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; -import { gateTriggerEnabled, requirePersonaIdentities } from '../shared/gates.js'; +import { requirePersonaIdentities } from '../shared/gates.js'; import { skip } from '../shared/skip.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type GitHubPullRequestReviewPayload, isGitHubPullRequestReviewPayload } from './types.js'; import { resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js'; @@ -24,13 +25,19 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { - const enabled = await gateTriggerEnabled( - ctx.project.id, - 'respond-to-review', - 'scm:pr-review-submitted', - this.name, - ); - if (enabled) return enabled; + // Disabled-at-config returns null so the registry's first-match loop + // continues to the next matcher — see `src/triggers/shared/trigger-check.ts` + // for the disabled-shadowing contract. + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-review', + 'scm:pr-review-submitted', + this.name, + )) + ) { + return null; + } // Type assertion since we validated in matches() const reviewPayload = ctx.payload as GitHubPullRequestReviewPayload; diff --git a/src/triggers/github/respond-to-ci-dispatch.ts b/src/triggers/github/respond-to-ci-dispatch.ts index 2a44ed117..5fa3300d8 100644 --- a/src/triggers/github/respond-to-ci-dispatch.ts +++ b/src/triggers/github/respond-to-ci-dispatch.ts @@ -2,7 +2,8 @@ import { type CheckSuiteStatus, githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; -import { gateAttemptLimit, gateTriggerEnabled } from '../shared/gates.js'; +import { gateAttemptLimit } from '../shared/gates.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { buildRespondToCiResult } from './result-builders.js'; import type { GitHubCheckSuitePayload } from './types.js'; @@ -30,8 +31,10 @@ export interface PRDetails { * a mixed-state SHA — see #1241) call this so the dispatch contract is * single-sourced. * - * Returns either a respond-to-ci `TriggerResult` ready to enqueue, or a - * structured skip when the trigger is disabled or the attempt limit is hit. + * Returns either a respond-to-ci `TriggerResult` ready to enqueue, `null` + * when the trigger is disabled at config (so the registry's first-match loop + * can continue to the next matcher), or a structured skip when the attempt + * limit is hit. */ export async function dispatchRespondToCi(opts: { ctx: TriggerContext; @@ -43,14 +46,17 @@ export async function dispatchRespondToCi(opts: { workItemTitle: string | undefined; checkStatus: CheckSuiteStatus; handlerName: string; -}): Promise { - const enabled = await gateTriggerEnabled( - opts.ctx.project.id, - 'respond-to-ci', - 'scm:check-suite-failure', - opts.handlerName, - ); - if (enabled) return enabled; +}): Promise { + if ( + !(await checkTriggerEnabled( + opts.ctx.project.id, + 'respond-to-ci', + 'scm:check-suite-failure', + opts.handlerName, + )) + ) { + return null; + } const attempts = fixAttempts.get(opts.prNumber) ?? 0; const limitSkip = gateAttemptLimit(attempts, MAX_ATTEMPTS, opts.prNumber, opts.handlerName); diff --git a/src/triggers/github/review-requested.ts b/src/triggers/github/review-requested.ts index 113860393..664416005 100644 --- a/src/triggers/github/review-requested.ts +++ b/src/triggers/github/review-requested.ts @@ -1,8 +1,9 @@ import { isCascadeBot } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; -import { gateTriggerEnabled, requirePersonaIdentities } from '../shared/gates.js'; +import { requirePersonaIdentities } from '../shared/gates.js'; import { skip } from '../shared/skip.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { buildReviewDispatchKey, claimReviewDispatch, @@ -40,13 +41,12 @@ export class ReviewRequestedTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { - const enabled = await gateTriggerEnabled( - ctx.project.id, - 'review', - 'scm:review-requested', - this.name, - ); - if (enabled) return enabled; + // Disabled-at-config returns null so the registry's first-match loop + // continues to the next matcher — see `src/triggers/shared/trigger-check.ts` + // for the disabled-shadowing contract. + if (!(await checkTriggerEnabled(ctx.project.id, 'review', 'scm:review-requested', this.name))) { + return null; + } const payload = ctx.payload as GitHubPullRequestPayload; const prNumber = payload.pull_request.number; diff --git a/src/triggers/shared/gates.ts b/src/triggers/shared/gates.ts index 7f2e5e5f3..018465bf2 100644 --- a/src/triggers/shared/gates.ts +++ b/src/triggers/shared/gates.ts @@ -3,7 +3,6 @@ import { isCascadeBot } from '../../github/personas.js'; import type { ProjectConfig, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { skip } from './skip.js'; -import { checkTriggerEnablement } from './trigger-check.js'; /** * Composable self-skip gates for trigger handlers. @@ -29,23 +28,6 @@ import { checkTriggerEnablement } from './trigger-check.js'; * skip. */ -/** - * Async gate: is the trigger enabled in this project's `agent_trigger_configs`? - * - * Wraps the centralized trigger enablement helper and returns its structured - * skip. Eliminates the repeated `if (!await checkTriggerEnabled(...)) return null;` - * pattern across every handler. - */ -export async function gateTriggerEnabled( - projectId: string, - agentType: string, - triggerEvent: string, - handlerName: string, -): Promise { - const result = await checkTriggerEnablement(projectId, agentType, triggerEvent, handlerName); - return result.skipResult; -} - /** * Sync gate: does the PR target the project's configured base branch? * diff --git a/src/triggers/shared/trigger-check.ts b/src/triggers/shared/trigger-check.ts index 25f77857b..4898c85b9 100644 --- a/src/triggers/shared/trigger-check.ts +++ b/src/triggers/shared/trigger-check.ts @@ -8,7 +8,6 @@ import type { TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { getResolvedTriggerConfig } from '../config-resolver.js'; -import { skip } from './skip.js'; export interface TriggerEnablementCheck { enabled: boolean; @@ -33,8 +32,27 @@ function logDisabledTrigger( } /** - * Resolve trigger enablement, merged parameters, and structured disabled-skip - * output in one config lookup. This is the canonical path for handler gates. + * Resolve trigger enablement and merged parameters in one config lookup. + * Canonical path for handler gates. + * + * **Disabled-at-config returns `skipResult: null`, NOT a structured skip.** + * The registry's `dispatch` loop is first-match-wins on non-null results + * (see `src/triggers/registry.ts`). A handler whose trigger is disabled is + * NOT claiming the event — so it returns null and the registry continues + * to the next matcher. Closes the prod regression on 2026-05-09 where + * `PROpenedTrigger`'s structured skip on `review trigger is disabled` + * shadowed `PRConflictDetectedTrigger` for `pull_request: opened` events + * on `zbigniewsobiecki/ucho` PR #367 (the conflict-resolution agent never + * fired because review was disabled). + * + * The `enabled: false` flag is still returned so callers' explicit branches + * (`if (!result.enabled) return null;`) keep working unchanged. + * + * Operator visibility is preserved via the INFO-level + * `Trigger disabled by config, skipping` log emitted at every disabled + * lookup; webhook-log decisionReasons just shift from a per-handler + * structured message to the registry-level `No trigger matched` (which + * accurately describes the dispatch outcome when no handler claimed). */ export async function checkTriggerEnablement( projectId: string, @@ -48,7 +66,7 @@ export async function checkTriggerEnablement( return { enabled: false, parameters: config?.parameters ?? {}, - skipResult: skip(handlerName, `${agentType} trigger is disabled for this project`), + skipResult: null, }; } diff --git a/tests/helpers/sharedMocks.ts b/tests/helpers/sharedMocks.ts index a26afd769..21fb0f19f 100644 --- a/tests/helpers/sharedMocks.ts +++ b/tests/helpers/sharedMocks.ts @@ -144,19 +144,16 @@ export const mockTriggerCheckModule = { triggerEvent, handlerName, ); + // Disabled-at-config returns skipResult=null so the registry + // continues to the next matcher. Structured skips are reserved + // for "I claim this event but my preconditions failed" — being + // disabled-at-config means "I don't claim this event today". + // Mirrors the production contract in + // `src/triggers/shared/trigger-check.ts:checkTriggerEnablement`. return { enabled, parameters: {}, - skipResult: enabled - ? null - : { - agentType: null, - agentInput: {}, - skipReason: { - handler: handlerName, - message: `${agentType} trigger is disabled for this project`, - }, - }, + skipResult: null, }; }, ), diff --git a/tests/integration/github-personas.test.ts b/tests/integration/github-personas.test.ts index 2eb12b5f0..70c3b9ae4 100644 --- a/tests/integration/github-personas.test.ts +++ b/tests/integration/github-personas.test.ts @@ -274,7 +274,7 @@ describe('GitHub Dual-Persona System (integration)', () => { triggers: { prReviewSubmitted: true }, }); // Agent + trigger configs must both be seeded so the - // gateTriggerEnabled gate passes and we can exercise the + // checkTriggerEnabled gate passes and we can exercise the // loop-prevention persona check. await seedAgentConfig({ agentType: 'respond-to-review' }); await seedTriggerConfig({ diff --git a/tests/unit/triggers/check-suite-failure.test.ts b/tests/unit/triggers/check-suite-failure.test.ts index 457ee1835..9564e9bac 100644 --- a/tests/unit/triggers/check-suite-failure.test.ts +++ b/tests/unit/triggers/check-suite-failure.test.ts @@ -174,7 +174,9 @@ describe('CheckSuiteFailureTrigger', () => { }); describe('handle', () => { - it('returns a structured skip when trigger is disabled', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns null, not a structured skip — see + // `checkTriggerEnablement` contract for the shadowing-bug context. vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { @@ -185,7 +187,7 @@ describe('CheckSuiteFailureTrigger', () => { }; const result = await trigger.handle(ctx); - expectSkip(result, 'respond-to-ci trigger is disabled for this project'); + expect(result).toBeNull(); expect(checkTriggerEnabled).toHaveBeenCalledWith( 'test', 'respond-to-ci', diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index 2192c6c1f..0e881d4eb 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -1031,7 +1031,11 @@ describe('CheckSuiteSuccessTrigger', () => { }); describe('authorMode-aware behavior via trigger parameters', () => { - it('handle returns null when trigger is disabled via checkTriggerEnabledWithParams', async () => { + it('handle returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. Mirror of the contract change in + // `src/triggers/shared/trigger-check.ts:checkTriggerEnablement`. vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ enabled: false, parameters: {}, @@ -1045,7 +1049,7 @@ describe('CheckSuiteSuccessTrigger', () => { }; const result = await trigger.handle(ctx); - expectSkip(result); + expect(result).toBeNull(); expect(checkTriggerEnabledWithParams).toHaveBeenCalledWith( 'test', 'review', diff --git a/tests/unit/triggers/disabled-trigger-shadowing.test.ts b/tests/unit/triggers/disabled-trigger-shadowing.test.ts new file mode 100644 index 000000000..bd282c917 --- /dev/null +++ b/tests/unit/triggers/disabled-trigger-shadowing.test.ts @@ -0,0 +1,158 @@ +/** + * Regression net for the disabled-trigger-shadowing bug class. + * + * Symptom (prod 2026-05-09 — `zbigniewsobiecki/ucho` PR #367): a `pull_request: opened` + * webhook arrived with the project's review trigger DISABLED at config but the + * resolve-conflicts trigger ENABLED. The PR was opened CONFLICTING against `dev`. + * The router logged `decisionReason="Trigger pr-opened skipped: review trigger + * is disabled for this project"` — `resolve-conflicts` never fired. + * + * Root cause: `TriggerRegistry.dispatch` is first-match-wins on non-null results. + * Both `PROpenedTrigger` (review) and `PRConflictDetectedTrigger` (resolve-conflicts) + * match `pull_request: opened`. The disabled-at-config branch in each handler was + * returning a structured skip (non-null), which halted the registry before the + * second matcher got a chance. + * + * Fix: disabled-at-config means "I don't claim this event" — the handler returns + * `null` so the registry continues to the next matcher. Structured skips are + * reserved for "I claim this event but my preconditions failed" (wrong base, + * wrong author, attempt limit, etc.). + * + * Pin the end-to-end behavior so a regression in either layer (registry semantics + * OR per-handler disabled-skip) fails this test loudly. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + mockConfigResolverModule, + mockGitHubClientModule, + mockTriggerCheckModule, +} from '../../helpers/sharedMocks.js'; + +vi.mock('../../../src/triggers/config-resolver.js', () => mockConfigResolverModule); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => mockTriggerCheckModule); +vi.mock('../../../src/github/client.js', () => mockGitHubClientModule); +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn().mockResolvedValue('abc123'), +})); + +import { githubClient } from '../../../src/github/client.js'; +import { PRConflictDetectedTrigger } from '../../../src/triggers/github/pr-conflict-detected.js'; +import { PROpenedTrigger } from '../../../src/triggers/github/pr-opened.js'; +import { createTriggerRegistry } from '../../../src/triggers/registry.js'; +import { + checkTriggerEnabled, + checkTriggerEnabledWithParams, +} from '../../../src/triggers/shared/trigger-check.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; +import { createMockProject } from '../../helpers/factories.js'; +import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; + +describe('disabled-trigger shadowing regression', () => { + const project = createMockProject({ baseBranch: 'main' }); + + const openedPayload = { + action: 'opened', + number: 367, + pull_request: { + number: 367, + title: 'aaight: implementation of MNG-XXX', + body: 'desc', + html_url: 'https://github.com/owner/repo/pull/367', + state: 'open' as const, + draft: false, + merged: false, + mergeable: false, + mergeable_state: 'dirty', + head: { ref: 'feature/test', sha: 'sha-conflicting' }, + base: { ref: 'main' }, + user: { login: 'cascade-impl' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'cascade-impl' }, + }; + + beforeEach(() => { + // Reset the per-test trigger-config mocks so an `mockImplementation` + // from one test doesn't leak into the next. + vi.mocked(checkTriggerEnabled).mockReset(); + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); + vi.mocked(checkTriggerEnabledWithParams).mockReset(); + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, parameters: {} }); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 367, + title: openedPayload.pull_request.title, + state: 'open', + merged: false, + mergeable: false, + mergeable_state: 'dirty', + head: { ref: 'feature/test', sha: 'sha-conflicting' }, + base: { ref: 'main' }, + user: { login: 'cascade-impl' }, + html_url: 'https://github.com/owner/repo/pull/367', + } as never); + vi.mocked(githubClient.createPRComment).mockResolvedValue(undefined); + }); + + it('PROpenedTrigger disabled does NOT shadow PRConflictDetectedTrigger on `pull_request: opened`', async () => { + // Review trigger DISABLED, resolve-conflicts trigger ENABLED. + // PROpenedTrigger reads `checkTriggerEnabledWithParams`; + // PRConflictDetectedTrigger reads `checkTriggerEnabled`. Mock both + // so each handler sees its own trigger's correct state. + vi.mocked(checkTriggerEnabledWithParams).mockImplementation( + async ( + _projectId: string, + agentType: string, + _triggerEvent: string, + _handlerName: string, + ) => ({ enabled: agentType !== 'review', parameters: {} }), + ); + vi.mocked(checkTriggerEnabled).mockImplementation( + async (_projectId: string, agentType: string, _triggerEvent: string, _handlerName: string) => + agentType !== 'review', + ); + + const registry = createTriggerRegistry(); + registry.register(new PROpenedTrigger()); + registry.register(new PRConflictDetectedTrigger()); + + const ctx: TriggerContext = { + project, + source: 'github', + payload: openedPayload, + personaIdentities: mockPersonaIdentities, + }; + + const result = await registry.dispatch(ctx); + + // Before the fix, this assertion failed with `agentType === null` + // (PROpenedTrigger's structured skip halted the registry). + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('resolve-conflicts'); + expect(result?.prNumber).toBe(367); + }); + + it('PROpenedTrigger ENABLED still wins on `pull_request: opened` (no behavior change for the happy path)', async () => { + // Sanity: when the first matcher is enabled, nothing changes — it + // claims the event and the registry returns the review dispatch. + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); + + const registry = createTriggerRegistry(); + registry.register(new PROpenedTrigger()); + registry.register(new PRConflictDetectedTrigger()); + + const ctx: TriggerContext = { + project, + source: 'github', + payload: { + ...openedPayload, + pull_request: { ...openedPayload.pull_request, base: { ref: 'main' } }, + }, + personaIdentities: mockPersonaIdentities, + }; + + const result = await registry.dispatch(ctx); + + expect(result?.agentType).toBe('review'); + }); +}); diff --git a/tests/unit/triggers/github-pr-comment-mention.test.ts b/tests/unit/triggers/github-pr-comment-mention.test.ts index a3560212d..c4ba0dae6 100644 --- a/tests/unit/triggers/github-pr-comment-mention.test.ts +++ b/tests/unit/triggers/github-pr-comment-mention.test.ts @@ -202,11 +202,15 @@ describe('PRCommentMentionTrigger', () => { }); describe('handle — disabled trigger', () => { - it('should return null when trigger is disabled', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for the + // disabled-shadowing contract. vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); const result = await trigger.handle(buildCtx()); - expectSkip(result); + expect(result).toBeNull(); expect(checkTriggerEnabled).toHaveBeenCalledWith( 'test-project', 'respond-to-pr-comment', diff --git a/tests/unit/triggers/pr-conflict-detected.test.ts b/tests/unit/triggers/pr-conflict-detected.test.ts index 96dbe6e9f..dd8bd942e 100644 --- a/tests/unit/triggers/pr-conflict-detected.test.ts +++ b/tests/unit/triggers/pr-conflict-detected.test.ts @@ -121,7 +121,11 @@ describe('PRConflictDetectedTrigger', () => { }); describe('handle', () => { - it('should return null when trigger is disabled', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for the + // disabled-shadowing contract. vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { @@ -132,8 +136,7 @@ describe('PRConflictDetectedTrigger', () => { }; const result = await trigger.handle(ctx); - expect(result?.agentType).toBeNull(); - expect(result?.skipReason?.handler).toBe('pr-conflict-detected'); + expect(result).toBeNull(); expect(checkTriggerEnabled).toHaveBeenCalledWith( 'test', 'resolve-conflicts', diff --git a/tests/unit/triggers/pr-merged.test.ts b/tests/unit/triggers/pr-merged.test.ts index a4615ca7a..352fb359c 100644 --- a/tests/unit/triggers/pr-merged.test.ts +++ b/tests/unit/triggers/pr-merged.test.ts @@ -156,7 +156,11 @@ describe('PRMergedTrigger', () => { }); describe('handle', () => { - it('should return null when trigger is disabled', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. Mirror of the contract change in + // `src/triggers/shared/trigger-check.ts:checkTriggerEnablement`. vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { @@ -171,7 +175,7 @@ describe('PRMergedTrigger', () => { }; const result = await trigger.handle(ctx); - expectSkip(result); + expect(result).toBeNull(); expect(isLifecycleTriggerEnabled).toHaveBeenCalledWith('test', 'prMerged', 'pr-merged'); }); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index dcaa0e065..a7b2127db 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -218,7 +218,13 @@ describe('PROpenedTrigger', () => { expect(result?.workItemId).toBeUndefined(); }); - it('returns null when trigger is disabled via checkTriggerEnabledWithParams', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. Closes the prod regression on 2026-05-09 where this + // handler's structured skip on `review trigger is disabled` + // shadowed PRConflictDetectedTrigger for `pull_request: opened` + // events on `zbigniewsobiecki/ucho` PR #367. vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ enabled: false, parameters: {}, @@ -247,7 +253,7 @@ describe('PROpenedTrigger', () => { }, }; - expectSkip(await trigger.handle(ctx)); + expect(await trigger.handle(ctx)).toBeNull(); expect(checkTriggerEnabledWithParams).toHaveBeenCalledWith( 'test', 'review', diff --git a/tests/unit/triggers/pr-ready-to-merge.test.ts b/tests/unit/triggers/pr-ready-to-merge.test.ts index 22eed418c..027ac875e 100644 --- a/tests/unit/triggers/pr-ready-to-merge.test.ts +++ b/tests/unit/triggers/pr-ready-to-merge.test.ts @@ -207,7 +207,11 @@ describe('PRReadyToMergeTrigger', () => { }); describe('handle', () => { - it('should return null when trigger is disabled', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. Mirror of the contract change in + // `src/triggers/shared/trigger-check.ts:checkTriggerEnablement`. vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { @@ -217,7 +221,7 @@ describe('PRReadyToMergeTrigger', () => { }; const result = await trigger.handle(ctx); - expectSkip(result); + expect(result).toBeNull(); expect(isLifecycleTriggerEnabled).toHaveBeenCalledWith( 'test', 'prReadyToMerge', diff --git a/tests/unit/triggers/pr-review-submitted.test.ts b/tests/unit/triggers/pr-review-submitted.test.ts index d60081ed4..58a8fd4b0 100644 --- a/tests/unit/triggers/pr-review-submitted.test.ts +++ b/tests/unit/triggers/pr-review-submitted.test.ts @@ -130,7 +130,11 @@ describe('PRReviewSubmittedTrigger', () => { }); describe('handle', () => { - it('should return null when trigger is disabled', async () => { + it('returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for the + // disabled-shadowing contract. vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { @@ -141,7 +145,7 @@ describe('PRReviewSubmittedTrigger', () => { }; const result = await trigger.handle(ctx); - expectSkip(result); + expect(result).toBeNull(); expect(checkTriggerEnabled).toHaveBeenCalledWith( 'test', 'respond-to-review', diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts index 7726ae7d6..2ddd8f006 100644 --- a/tests/unit/triggers/review-requested.test.ts +++ b/tests/unit/triggers/review-requested.test.ts @@ -263,7 +263,11 @@ describe('ReviewRequestedTrigger', () => { }); describe('trigger config via checkTriggerEnabled', () => { - it('handle returns null when trigger is disabled', async () => { + it('handle returns null when trigger is disabled (so the registry can try the next matcher)', async () => { + // Disabled-at-config returns bare null, not a structured skip, + // so the registry's first-match loop continues to the next + // matcher. See `src/triggers/shared/trigger-check.ts` for the + // disabled-shadowing contract. vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { @@ -273,7 +277,7 @@ describe('ReviewRequestedTrigger', () => { personaIdentities: mockPersonaIdentities, }; const result = await trigger.handle(ctx); - expectSkip(result); + expect(result).toBeNull(); expect(checkTriggerEnabled).toHaveBeenCalledWith( 'test', 'review', diff --git a/tests/unit/triggers/shared/gates.test.ts b/tests/unit/triggers/shared/gates.test.ts index 5a37b37b0..c510c9dfc 100644 --- a/tests/unit/triggers/shared/gates.test.ts +++ b/tests/unit/triggers/shared/gates.test.ts @@ -1,17 +1,12 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { mockTriggerCheckModule } from '../../../helpers/sharedMocks.js'; - -vi.mock('../../../../src/triggers/shared/trigger-check.js', () => mockTriggerCheckModule); +import { describe, expect, it } from 'vitest'; import type { PersonaIdentities } from '../../../../src/github/personas.js'; import { gateAttemptLimit, gateBaseBranch, gateCascadePersona, - gateTriggerEnabled, requirePersonaIdentities, } from '../../../../src/triggers/shared/gates.js'; -import { checkTriggerEnablement } from '../../../../src/triggers/shared/trigger-check.js'; import type { ProjectConfig } from '../../../../src/types/index.js'; import { createMockProject } from '../../../helpers/factories.js'; @@ -22,52 +17,11 @@ const mockPersonas: PersonaIdentities = { reviewer: 'cascade-rev', }; -describe('gateTriggerEnabled', () => { - beforeEach(() => { - vi.mocked(checkTriggerEnablement).mockReset(); - }); - - it('returns null when checkTriggerEnablement has no skip result', async () => { - vi.mocked(checkTriggerEnablement).mockResolvedValue({ - enabled: true, - parameters: {}, - skipResult: null, - }); - const result = await gateTriggerEnabled( - 'proj', - 'respond-to-ci', - 'scm:check-suite-failure', - 'check-suite-failure', - ); - expect(result).toBeNull(); - }); - - it('returns the centralized structured skip when checkTriggerEnablement resolves disabled', async () => { - vi.mocked(checkTriggerEnablement).mockResolvedValue({ - enabled: false, - parameters: {}, - skipResult: { - agentType: null, - agentInput: {}, - skipReason: { - handler: 'check-suite-failure', - message: 'respond-to-ci trigger is disabled for this project', - }, - }, - }); - const result = await gateTriggerEnabled( - 'proj', - 'respond-to-ci', - 'scm:check-suite-failure', - 'check-suite-failure', - ); - expect(result?.agentType).toBeNull(); - expect(result?.skipReason).toEqual({ - handler: 'check-suite-failure', - message: 'respond-to-ci trigger is disabled for this project', - }); - }); -}); +// `gateTriggerEnabled` was deleted on 2026-05-09 as part of the disabled-trigger +// shadowing fix. Handlers now call `checkTriggerEnabled` (boolean) directly: +// `if (!(await checkTriggerEnabled(...))) return null;` +// — see `src/triggers/shared/trigger-check.ts` for the contract and +// `tests/unit/triggers/shared/trigger-check.test.ts` for the assertions. describe('gateBaseBranch', () => { it('returns null when prBaseRef matches project.baseBranch', () => { diff --git a/tests/unit/triggers/shared/trigger-check.test.ts b/tests/unit/triggers/shared/trigger-check.test.ts index 11e213908..5f901c39b 100644 --- a/tests/unit/triggers/shared/trigger-check.test.ts +++ b/tests/unit/triggers/shared/trigger-check.test.ts @@ -56,7 +56,13 @@ describe('checkTriggerEnablement', () => { }); }); - it('returns enabled=false, default parameters, and structured skip when config is disabled', async () => { + // Disabled-at-config returns skipResult=null so the registry's first-match + // loop continues to the next matcher. Structured skips are reserved for + // "I claim this event but my preconditions failed" (wrong base, wrong + // author, attempt limit). Closes the prod regression on 2026-05-09 where + // PROpenedTrigger's structured skip on `review trigger is disabled` + // shadowed PRConflictDetectedTrigger for `pull_request: opened` events. + it('returns enabled=false, default parameters, and skipResult=null when config is disabled', async () => { mockGetResolvedTriggerConfig.mockResolvedValue({ event: TRIGGER_EVENT, enabled: false, @@ -73,14 +79,10 @@ describe('checkTriggerEnablement', () => { expect(result.enabled).toBe(false); expect(result.parameters).toEqual({ authorMode: 'own' }); - expect(result.skipResult?.agentType).toBeNull(); - expect(result.skipResult?.skipReason).toEqual({ - handler: HANDLER_NAME, - message: `${AGENT_TYPE} trigger is disabled for this project`, - }); + expect(result.skipResult).toBeNull(); }); - it('returns enabled=false, empty parameters, and structured skip when config is missing', async () => { + it('returns enabled=false, empty parameters, and skipResult=null when config is missing', async () => { mockGetResolvedTriggerConfig.mockResolvedValue(null); const result = await checkTriggerEnablement( @@ -92,9 +94,7 @@ describe('checkTriggerEnablement', () => { expect(result.enabled).toBe(false); expect(result.parameters).toEqual({}); - expect(result.skipResult?.skipReason?.message).toBe( - `${AGENT_TYPE} trigger is disabled for this project`, - ); + expect(result.skipResult).toBeNull(); }); it('logs skip message when trigger is disabled', async () => { diff --git a/tests/unit/triggers/trigger-event-consistency.test.ts b/tests/unit/triggers/trigger-event-consistency.test.ts index b40300478..e53ce1723 100644 --- a/tests/unit/triggers/trigger-event-consistency.test.ts +++ b/tests/unit/triggers/trigger-event-consistency.test.ts @@ -131,6 +131,13 @@ function scanHandler(file: string): HandlerScan { if (src.includes('buildRespondToCiResult')) { emittedEvents.add('scm:check-suite-failure'); } + // `check-suite-failure.ts` returns the result of `dispatchRespondToCi(...)`, + // which itself uses `buildRespondToCiResult`. Treat the dispatch helper + // call as a local emission so the cross-file chain stays visible to the + // gating-vs-emitted drift guard. + if (src.includes('dispatchRespondToCi(')) { + emittedEvents.add('scm:check-suite-failure'); + } if (src.includes('buildResolveConflictsResult')) { emittedEvents.add('scm:pr-conflict-detected'); } @@ -153,6 +160,11 @@ const RAW_TRIGGER_LITERAL_EXEMPTIONS = new Set([ "src/triggers/github/pr-conflict-detected.ts :: 'scm:pr-conflict-detected',", "src/triggers/github/review-requested.ts :: 'scm:review-requested',", "src/triggers/github/review-requested.ts :: triggerEvent: 'scm:review-requested',", + // Biome formatter reflowed the `checkTriggerEnabled(...)` call to a single + // line on 2026-05-09 (disabled-trigger-shadowing fix). The literal event + // string still references TRIGGER_EVENTS conceptually but the line content + // changed. Pin the new line so the static guard stays exact. + "src/triggers/github/review-requested.ts :: if (!(await checkTriggerEnabled(ctx.project.id, 'review', 'scm:review-requested', this.name))) {", "src/triggers/github/pr-comment-mention.ts :: 'scm:pr-comment-mention',", "src/triggers/github/pr-comment-mention.ts :: triggerEvent: 'scm:pr-comment-mention',", "src/triggers/github/pr-opened.ts :: 'scm:pr-opened',", From daec048d0d3dc619f896c4f41ef436dd1569e20e Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Sat, 9 May 2026 20:03:24 +0000 Subject: [PATCH 7/9] fix(test): align review-requested disabled assertion --- tests/integration/github-personas.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/github-personas.test.ts b/tests/integration/github-personas.test.ts index 70c3b9ae4..a20a2518a 100644 --- a/tests/integration/github-personas.test.ts +++ b/tests/integration/github-personas.test.ts @@ -364,7 +364,7 @@ describe('GitHub Dual-Persona System (integration)', () => { // scm:review-requested has defaultEnabled: false in definition expect(trigger.matches(ctx)).toBe(true); const result = await trigger.handle(ctx); - expectSkip(result, 'review-requested', /trigger is disabled/); + expect(result).toBeNull(); }); it('triggers review when enabled via DB and persona is requested', async () => { From 91e2af46d196e411633cef0b20fb2f72ffced4c6 Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 23:20:23 +0200 Subject: [PATCH 8/9] feat(runtime): drain friction sidecar reports (#1300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(runtime): drain friction sidecar reports * fix(runtime): propagate url/title context to agentInput and SessionState for LLMist gadgets - Forward workItemUrl/workItemTitle/prUrl/prTitle into agentInput from buildGitHubPRDispatchResult and buildPMDispatchResult so that injectAgentInputContext can emit the corresponding CASCADE_* env vars for native-tool subprocess engines - Extend SessionState with runId, prNumber, prUrl, prTitle fields and expose module-level getters so in-process LLMist gadgets (e.g. ReportFriction) can read per-run metadata without process.env - Pass prNumber/prUrl/prTitle from agentInput into createConfiguredBuilder and initSessionState in the LLMist backend - Update buildReport() in reportFriction to fall back to SessionState getters when the env vars are absent (LLMist in-process path) - Update affected trigger tests for the new agentInput shape Co-Authored-By: Claude Sonnet 4.6 * fix(friction): store project in SessionState so LLMist ReportFriction uses correct context In LLMist (in-process gadgets), projectSecrets are NOT exported into process.env. The production ReportFriction.ts wrapper calls reportFriction() without params.project, causing projectFromEnv() to build a report with id='unknown-project' and empty PM config. - Add `project?: ProjectConfig` to SessionState and InitSessionStateOptions - Pass `project: input.project` from LLMist engine to createConfiguredBuilder → initSessionState - In reportFriction(), fall back to getProject() from SessionState before projectFromEnv() - Add test covering the production wrapper path (no params.project, project in SessionState) Co-Authored-By: Claude Sonnet 4.6 * fix(friction): gate prUrl on prCreated; add SessionState agent context fallbacks - Gate LlmistEngine's returned prUrl on prCreated===true to prevent PR-triggered review/respond-to-ci runs from falsely reporting a "PR created" result when no CreatePR gadget was called - Add SessionState fallbacks for CASCADE_AGENT_TYPE, CASCADE_ENGINE_LABEL, CASCADE_MODEL, CASCADE_PR_BRANCH, CASCADE_INITIAL_HEAD_SHA in buildReport() so LLMist in-process friction reports include accurate agent/PR context instead of "Agent: unknown" with no engine/model/branch/headSha - Store engineLabel and model in InitSessionStateOptions and SessionStateData - Update tests to reflect new prCreated gate and new SessionState fields Co-Authored-By: Claude Sonnet 4.6 * fix(triggers): centralize TriggerResult URL/title merge in prepareAgentWorkItem Several GitHub trigger handlers (PRReviewSubmittedTrigger, PROpenedTrigger, ReviewRequestedTrigger, PRCommentMentionTrigger) set prUrl/prTitle/workItemUrl/ workItemTitle on the top-level TriggerResult but not in agentInput. Since both injectAgentInputContext (secretBuilder.ts) and LlmistEngine read from agentInput, these native-tool and in-process runs were missing CASCADE_PR_URL, CASCADE_PR_TITLE, CASCADE_WORK_ITEM_URL, and CASCADE_WORK_ITEM_TITLE. Centralizes the merge in prepareAgentWorkItem: after resolving workItemId, any top-level TriggerResult URL/title fields absent from agentInput are merged in before agent execution. This covers all handlers without requiring each one to be updated individually. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Cascade Bot Co-authored-by: Claude Sonnet 4.6 --- src/agents/shared/builderFactory.ts | 24 ++ src/backends/adapter.ts | 17 +- src/backends/llmist/index.ts | 21 +- src/backends/secretBuilder.ts | 24 +- src/backends/secretOrchestrator.ts | 26 ++- src/backends/sidecarManager.ts | 145 +++++++++++- src/backends/types.ts | 2 + src/cli/base.ts | 11 +- src/gadgets/pm/core/reportFriction.ts | 82 +++++-- src/gadgets/sessionState.ts | 148 +++++++++++- src/triggers/shared/agent-work-items.ts | 20 +- src/triggers/shared/result-builders.ts | 28 ++- tests/helpers/backendMocks.ts | 16 ++ .../unit/agents/shared/builderFactory.test.ts | 120 ++++++---- tests/unit/backends/adapter.test.ts | 93 ++++++++ tests/unit/backends/llmist.test.ts | 2 + tests/unit/backends/secretBuilder.test.ts | 21 ++ .../unit/backends/secretOrchestrator.test.ts | 23 +- tests/unit/backends/sidecarManager.test.ts | 214 +++++++++++++++++- tests/unit/cli/credential-scoping.test.ts | 33 +++ .../gadgets/pm/core/reportFriction.test.ts | 155 ++++++++++++- .../unit/triggers/check-suite-failure.test.ts | 12 + .../unit/triggers/check-suite-success.test.ts | 6 +- .../triggers/pr-conflict-detected.test.ts | 6 + .../triggers/shared/agent-work-items.test.ts | 59 ++++- .../triggers/shared/result-builders.test.ts | 4 + 26 files changed, 1225 insertions(+), 87 deletions(-) diff --git a/src/agents/shared/builderFactory.ts b/src/agents/shared/builderFactory.ts index d72d9262c..3d701c8ef 100644 --- a/src/agents/shared/builderFactory.ts +++ b/src/agents/shared/builderFactory.ts @@ -11,6 +11,7 @@ import { getIterationTrailingMessage } from '../../config/hintConfig.js'; import { getRateLimitForModel } from '../../config/rateLimits.js'; import { getRetryConfig } from '../../config/retryConfig.js'; import { initSessionState, type SessionHooks, setReadOnlyFs } from '../../gadgets/sessionState.js'; +import type { ProjectConfig } from '../../types/index.js'; import type { LLMCallLogger } from '../../utils/llmLogging.js'; import type { IProgressMonitor } from '../contracts/index.js'; import { getAgentCapabilities } from '../definitions/index.js'; @@ -55,6 +56,21 @@ export interface CreateBuilderOptions { workItemUrl?: string; /** Work item display title for PR ↔ work item enrichment. Passed to session state. */ workItemTitle?: string; + /** JSONL outbox path for incidental friction reports. Passed to session state. */ + frictionSidecarPath?: string; + /** PR number for the current execution. Passed to session state for in-process gadget fallback. */ + prNumber?: number; + /** PR URL for the current execution. Passed to session state for in-process gadget fallback. */ + prUrl?: string; + /** PR title for the current execution. Passed to session state for in-process gadget fallback. */ + prTitle?: string; + /** Full project config. Stored in session state so in-process gadgets (e.g. LLMist ReportFriction) + * can access project context without relying on process.env (projectSecrets are not exported + * to env for in-process runs). */ + project?: ProjectConfig; + /** Engine identifier (e.g. 'llmist'). Stored in session state so in-process gadgets + * (e.g. ReportFriction) can read it without CASCADE_ENGINE_LABEL in process.env. */ + engineLabel?: string; /** Resolved SCM hook flags for finish validation (requiresPR, requiresReview, etc.) */ hooks?: SessionHooks; } @@ -95,10 +111,18 @@ export async function createConfiguredBuilder(options: CreateBuilderOptions): Pr baseBranch: options.baseBranch, prBranch: options.prBranch, projectId: options.projectId, + project: options.project, workItemId: options.workItemId, hooks: options.hooks, workItemUrl: options.workItemUrl, workItemTitle: options.workItemTitle, + frictionSidecarPath: options.frictionSidecarPath, + runId: options.runId, + prNumber: options.prNumber, + prUrl: options.prUrl, + prTitle: options.prTitle, + engineLabel: options.engineLabel, + model: options.model, initialHeadSha, }); diff --git a/src/backends/adapter.ts b/src/backends/adapter.ts index a2a668e43..1871f11ff 100644 --- a/src/backends/adapter.ts +++ b/src/backends/adapter.ts @@ -11,7 +11,11 @@ import { postProcessResult } from './postProcess.js'; import { createProgressMonitor } from './progress.js'; import { buildProgressMonitorConfig, isGitHubAckComment } from './progressLifecycle.js'; import { injectRunLinkSecrets, resolvePartialExecutionPlan } from './secretOrchestrator.js'; -import { cleanupTempFile, hydrateNativeToolSidecars } from './sidecarManager.js'; +import { + cleanupTempFile, + drainFrictionSidecarReports, + hydrateNativeToolSidecars, +} from './sidecarManager.js'; import type { AgentEngine, AgentExecutionPlan } from './types.js'; /** @@ -117,7 +121,8 @@ export async function executeWithEngine( // Plan resolution succeeded — fill in the deferred run-row fields. await tryUpdateRunPlanResolution(runId, partialInput.model, partialInput.maxIterations); - const { reviewSidecarPath, prSidecarPath, nativeToolRuntimeCleanup } = partialInput; + const { reviewSidecarPath, prSidecarPath, frictionSidecarPath, nativeToolRuntimeCleanup } = + partialInput; const { pushedChangesSidecarPath, pmWriteSidecarPath } = partialInput; // Seed session state so GitHub progress updates target the existing ack comment @@ -189,10 +194,18 @@ export async function executeWithEngine( }); } finally { monitor?.stop(); + await drainFrictionSidecarReports({ + sidecarPath: frictionSidecarPath, + project: input.project, + agentType, + runId, + engineId: engine.definition.id, + }); cleanupTempFile(prSidecarPath); cleanupTempFile(reviewSidecarPath); cleanupTempFile(pushedChangesSidecarPath); cleanupTempFile(pmWriteSidecarPath); + cleanupTempFile(frictionSidecarPath); nativeToolRuntimeCleanup?.(); } diff --git a/src/backends/llmist/index.ts b/src/backends/llmist/index.ts index 6a3f68ddd..36ba8dbb0 100644 --- a/src/backends/llmist/index.ts +++ b/src/backends/llmist/index.ts @@ -112,6 +112,20 @@ export class LlmistEngine implements AgentEngine { workItemId: agentInput.workItemId, workItemUrl: agentInput.workItemUrl as string | undefined, workItemTitle: agentInput.workItemTitle as string | undefined, + frictionSidecarPath: input.frictionSidecarPath, + // Pass PR metadata so in-process gadgets (e.g. ReportFriction) can read it + // from SessionState as a fallback when CASCADE_PR_* env vars are not exported + // into process.env (which is the case for all in-process LLMist gadgets). + prNumber: agentInput.prNumber, + prUrl: agentInput.prUrl as string | undefined, + prTitle: agentInput.prTitle as string | undefined, + // Pass full project config so in-process gadgets (e.g. ReportFriction) can + // build accurate reports. In LLMist, projectSecrets are NOT exported into + // process.env, so projectFromEnv() would return 'unknown-project'/empty PM config. + project: input.project, + // Pass engine/model identifiers so ReportFriction context.agent is accurate + // for in-process runs (CASCADE_ENGINE_LABEL / CASCADE_MODEL are not exported). + engineLabel: this.definition.id, // Pass resolved hook flags for finish validation (hook-driven instead of agent-type checks) hooks: profile.finishHooks, // Pass the progress monitor from the adapter so createObserverHooks can call @@ -167,7 +181,12 @@ export class LlmistEngine implements AgentEngine { loopTerminated: result.loopTerminated ?? false, }); - const prUrl = getSessionState().prUrl ?? undefined; + // Only return the prUrl as authoritative "PR created" evidence when CreatePR was actually + // called (prCreated === true). The prUrl field is also populated on init from incoming + // PR context (e.g. for review/respond-to-ci runs), so without this gate a PR-triggered + // run that never calls CreatePR would falsely report a "PR created" result. + const sessionState = getSessionState(); + const prUrl = sessionState.prCreated ? (sessionState.prUrl ?? undefined) : undefined; return { success: !result.loopTerminated, output: result.output, diff --git a/src/backends/secretBuilder.ts b/src/backends/secretBuilder.ts index d93e35e99..0b66c1556 100644 --- a/src/backends/secretBuilder.ts +++ b/src/backends/secretBuilder.ts @@ -48,10 +48,31 @@ function injectTrelloConfig(projectSecrets: Record, project: Pro projectSecrets.CASCADE_TRELLO_LABELS = JSON.stringify(trelloConfig.labels); } +function injectAgentInputContext(projectSecrets: Record, input: AgentInput): void { + const stringFields: Array<[keyof AgentInput, string]> = [ + ['workItemId', 'CASCADE_WORK_ITEM_ID'], + ['workItemUrl', 'CASCADE_WORK_ITEM_URL'], + ['workItemTitle', 'CASCADE_WORK_ITEM_TITLE'], + ['prUrl', 'CASCADE_PR_URL'], + ['prTitle', 'CASCADE_PR_TITLE'], + ]; + + for (const [field, envVar] of stringFields) { + const value = input[field]; + if (typeof value === 'string' && value) { + projectSecrets[envVar] = value; + } + } + + if (typeof input.prNumber === 'number') { + projectSecrets.CASCADE_PR_NUMBER = String(input.prNumber); + } +} + export async function augmentProjectSecrets( project: ProjectConfig, agentType: string, - _input: AgentInput, + input: AgentInput, ): Promise> { const projectSecrets = await getAllProjectCredentials(project.id); @@ -103,6 +124,7 @@ export async function augmentProjectSecrets( // Inject agent type so Finish command can validate without flags projectSecrets.CASCADE_AGENT_TYPE = agentType; + injectAgentInputContext(projectSecrets, input); // Inject PM type so cascade-tools uses the correct provider projectSecrets.CASCADE_PM_TYPE = project.pm?.type ?? 'trello'; diff --git a/src/backends/secretOrchestrator.ts b/src/backends/secretOrchestrator.ts index 59f1f2df6..7ac22f7ed 100644 --- a/src/backends/secretOrchestrator.ts +++ b/src/backends/secretOrchestrator.ts @@ -49,6 +49,7 @@ export async function buildExecutionPlan( prSidecarPath?: string; pushedChangesSidecarPath?: string; pmWriteSidecarPath?: string; + frictionSidecarPath?: string; nativeToolRuntimeCleanup?: () => void; } > { @@ -152,8 +153,13 @@ export async function buildExecutionPlan( isGitHubAck, ); - const { reviewSidecarPath, prSidecarPath, pushedChangesSidecarPath, pmWriteSidecarPath } = - createCompletionArtifacts(profile, agentType, needsNativeToolRuntime, input, projectSecrets); + const { + reviewSidecarPath, + prSidecarPath, + pushedChangesSidecarPath, + pmWriteSidecarPath, + frictionSidecarPath, + } = createCompletionArtifacts(profile, agentType, needsNativeToolRuntime, input, projectSecrets); const completionRequirements = { requiresPR: profile.finishHooks.requiresPR, @@ -206,6 +212,7 @@ export async function buildExecutionPlan( prSidecarPath, pushedChangesSidecarPath, pmWriteSidecarPath, + frictionSidecarPath, nativeToolRuntimeCleanup: nativeToolRuntime?.cleanup, }; } @@ -256,14 +263,7 @@ export function injectRunLinkSecrets( workItemId: string | undefined, runId: string | undefined, ): void { - if (!project.runLinksEnabled) return; - - const dashboardUrl = getDashboardUrl(); - if (!dashboardUrl) return; - partialInput.projectSecrets ??= {}; - partialInput.projectSecrets.CASCADE_RUN_LINKS_ENABLED = 'true'; - partialInput.projectSecrets.CASCADE_DASHBOARD_URL = dashboardUrl; partialInput.projectSecrets.CASCADE_ENGINE_LABEL = engineId; partialInput.projectSecrets.CASCADE_MODEL = partialInput.model ?? ''; partialInput.projectSecrets.CASCADE_PROJECT_ID = project.id; @@ -273,4 +273,12 @@ export function injectRunLinkSecrets( if (runId) { partialInput.projectSecrets.CASCADE_RUN_ID = runId; } + + if (!project.runLinksEnabled) return; + + const dashboardUrl = getDashboardUrl(); + if (!dashboardUrl) return; + + partialInput.projectSecrets.CASCADE_RUN_LINKS_ENABLED = 'true'; + partialInput.projectSecrets.CASCADE_DASHBOARD_URL = dashboardUrl; } diff --git a/src/backends/sidecarManager.ts b/src/backends/sidecarManager.ts index c86806f36..ec27a67ea 100644 --- a/src/backends/sidecarManager.ts +++ b/src/backends/sidecarManager.ts @@ -3,8 +3,14 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import type { getAgentProfile } from '../agents/definitions/profiles.js'; +import { materializeFrictionReport } from '../friction/materialize.js'; +import { + appendFiledFrictionReport, + rewriteFrictionSidecarWithPending, +} from '../friction/sidecar.js'; import { clearInitialComment, + FRICTION_SIDECAR_ENV_VAR, PM_WRITE_SIDECAR_ENV_VAR, PR_SIDECAR_ENV_VAR, PUSHED_CHANGES_SIDECAR_ENV_VAR, @@ -12,7 +18,9 @@ import { recordPRCreation, recordReviewSubmission, } from '../gadgets/sessionState.js'; -import type { AgentInput } from '../types/index.js'; +import { pmRegistry } from '../pm/registry.js'; +import { captureException } from '../sentry.js'; +import type { AgentInput, ProjectConfig } from '../types/index.js'; import { logger } from '../utils/logging.js'; import { readCompletionEvidence } from './completion.js'; import type { AgentEngineResult } from './types.js'; @@ -32,7 +40,14 @@ export function createCompletionArtifacts( pushedChangesSidecarPath: string | undefined; reviewSidecarPath: string | undefined; pmWriteSidecarPath: string | undefined; + frictionSidecarPath: string | undefined; } { + const frictionSidecarPath = join( + tmpdir(), + `cascade-friction-sidecar-${process.pid}-${Date.now()}.jsonl`, + ); + projectSecrets[FRICTION_SIDECAR_ENV_VAR] = frictionSidecarPath; + const reviewSidecarPath = profile.finishHooks.requiresReview ? join(tmpdir(), `cascade-review-sidecar-${process.pid}-${Date.now()}.json`) : undefined; @@ -79,6 +94,7 @@ export function createCompletionArtifacts( pushedChangesSidecarPath, reviewSidecarPath, pmWriteSidecarPath, + frictionSidecarPath, }; } @@ -245,6 +261,133 @@ export async function hydrateNativeToolSidecars( } } +export interface DrainFrictionSidecarOptions { + sidecarPath?: string; + project: ProjectConfig; + agentType: string; + runId?: string; + engineId?: string; +} + +async function withProjectPMCredentials( + project: ProjectConfig, + fn: () => Promise, +): Promise { + const integration = pmRegistry.getOrNull(project.pm?.type ?? 'trello'); + if (!integration) return fn(); + return integration.withCredentials(project.id, fn); +} + +/** + * Best-effort drain of queued friction reports from the JSONL sidecar. + * + * This deliberately never throws. Friction reporting is incidental telemetry: + * a failed materialization should be visible in logs/Sentry, but must not flip + * the primary agent run from success to failure or mask the original error. + */ +export async function drainFrictionSidecarReports({ + sidecarPath, + project, + agentType, + runId, + engineId, +}: DrainFrictionSidecarOptions): Promise { + if (!sidecarPath) return; + + let pending: Awaited>; + try { + pending = await rewriteFrictionSidecarWithPending(sidecarPath); + } catch (err) { + logger.warn('Failed to read friction sidecar before drain', { + sidecarPath, + projectId: project.id, + agentType, + runId, + engine: engineId, + error: String(err), + }); + captureException(err instanceof Error ? err : new Error(String(err)), { + tags: { + source: 'friction_sidecar_drain_failed', + phase: 'read', + agentType, + }, + extra: { sidecarPath, projectId: project.id, runId, engine: engineId }, + }); + return; + } + + if (pending.length === 0) return; + + logger.info('Draining pending friction sidecar reports', { + sidecarPath, + projectId: project.id, + agentType, + runId, + engine: engineId, + pendingCount: pending.length, + }); + + for (const event of pending) { + try { + const result = await withProjectPMCredentials(project, () => + materializeFrictionReport({ project, report: event.report }), + ); + if (result.status === 'filed') { + await appendFiledFrictionReport(sidecarPath, { + reportId: event.reportId, + workItemId: result.workItemId, + workItemUrl: result.workItemUrl, + }); + logger.info('Drained friction sidecar report', { + sidecarPath, + projectId: project.id, + agentType, + runId, + engine: engineId, + reportId: event.reportId, + workItemId: result.workItemId, + }); + } else { + logger.warn('Skipped friction sidecar report during drain', { + sidecarPath, + projectId: project.id, + agentType, + runId, + engine: engineId, + reportId: event.reportId, + reason: result.reason, + message: result.message, + }); + } + } catch (err) { + logger.warn('Failed to drain friction sidecar report', { + sidecarPath, + projectId: project.id, + agentType, + runId, + engine: engineId, + reportId: event.reportId, + error: String(err), + }); + captureException(err instanceof Error ? err : new Error(String(err)), { + tags: { + source: 'friction_sidecar_drain_failed', + phase: 'materialize', + agentType, + }, + extra: { + sidecarPath, + projectId: project.id, + runId, + engine: engineId, + reportId: event.reportId, + }, + }); + } + } +} + /** * Best-effort cleanup of a temp file. Ignores errors silently. */ diff --git a/src/backends/types.ts b/src/backends/types.ts index d2f29c814..744d93871 100644 --- a/src/backends/types.ts +++ b/src/backends/types.ts @@ -80,6 +80,8 @@ export interface AgentExecutionPlan cliToolsDir: string; nativeToolShimDir?: string; completionRequirements?: CompletionRequirements; + /** JSONL outbox path for incidental friction reports. */ + frictionSidecarPath?: string; } export type PrEvidenceSource = 'llmist-session' | 'native-tool-sidecar' | 'text'; diff --git a/src/cli/base.ts b/src/cli/base.ts index 3b984169d..95482d9bc 100644 --- a/src/cli/base.ts +++ b/src/cli/base.ts @@ -110,7 +110,16 @@ function synthesizeProjectFromEnv(pmType: PMType): ProjectConfig { }, } as ProjectConfig; } - return { pm: { type: 'trello' } } as ProjectConfig; + const trelloLists = process.env.CASCADE_TRELLO_LISTS; + const trelloLabels = process.env.CASCADE_TRELLO_LABELS; + return { + pm: { type: 'trello' }, + trello: { + boardId: process.env.CASCADE_TRELLO_BOARD_ID ?? '', + lists: trelloLists ? JSON.parse(trelloLists) : {}, + labels: trelloLabels ? JSON.parse(trelloLabels) : {}, + }, + } as ProjectConfig; } export abstract class CredentialScopedCommand extends Command { diff --git a/src/gadgets/pm/core/reportFriction.ts b/src/gadgets/pm/core/reportFriction.ts index 84f4b412e..8899f0a3f 100644 --- a/src/gadgets/pm/core/reportFriction.ts +++ b/src/gadgets/pm/core/reportFriction.ts @@ -10,8 +10,24 @@ import type { FrictionSeverity, } from '../../../friction/types.js'; import type { ProjectConfig } from '../../../types/index.js'; +import { + FRICTION_SIDECAR_ENV_VAR, + getAgentType, + getEngineLabel, + getFrictionSidecarPath, + getInitialHeadSha, + getModel, + getPrBranch, + getPrNumber, + getProject, + getPrTitle, + getPrUrl, + getRunId, + getWorkItemId, + getWorkItemTitle, + getWorkItemUrl, +} from '../../sessionState.js'; -const FRICTION_SIDECAR_ENV_VAR = 'CASCADE_FRICTION_SIDECAR_PATH'; const DEFAULT_FRICTION_SIDECAR_PATH = '.cascade/friction-reports.jsonl'; const CATEGORIES = [ @@ -119,7 +135,34 @@ function requireEnum(value: string, allowed: readonly T[], nam throw new Error(`${name} must be one of: ${allowed.join(', ')}`); } +function parseOptionalInt(value: string | undefined): number | undefined { + if (!value) return undefined; + const parsed = Number.parseInt(value, 10); + return Number.isSafeInteger(parsed) ? parsed : undefined; +} + function buildReport(params: ReportFrictionParams, project: ProjectConfig): FrictionReport { + // For in-process gadgets (e.g. LLMist / ReportFriction called directly), projectSecrets + // are NOT exported to process.env — only subprocess engines (claude-code, opencode, codex) + // receive them as actual env vars. Fall back to SessionState for fields that are available + // there when the env var is absent. + const runId = process.env.CASCADE_RUN_ID ?? getRunId() ?? undefined; + const dashboardUrl = process.env.CASCADE_DASHBOARD_URL || undefined; + const workItemId = process.env.CASCADE_WORK_ITEM_ID ?? getWorkItemId() ?? undefined; + const workItemTitle = process.env.CASCADE_WORK_ITEM_TITLE ?? getWorkItemTitle() ?? undefined; + const workItemUrl = process.env.CASCADE_WORK_ITEM_URL ?? getWorkItemUrl() ?? undefined; + const prNumber = parseOptionalInt(process.env.CASCADE_PR_NUMBER) ?? getPrNumber() ?? undefined; + const prUrl = process.env.CASCADE_PR_URL ?? getPrUrl() ?? undefined; + const prTitle = process.env.CASCADE_PR_TITLE ?? getPrTitle() ?? undefined; + // The following five values are injected as projectSecrets (CASCADE_*) for subprocess engines + // (claude-code, opencode, codex) but NOT exported to process.env for in-process LLMist gadgets. + // Fall back to SessionState values populated by createConfiguredBuilder when env is absent. + const agentTypeValue = process.env.CASCADE_AGENT_TYPE ?? getAgentType() ?? 'unknown'; + const engineLabelValue = process.env.CASCADE_ENGINE_LABEL ?? getEngineLabel() ?? undefined; + const modelValue = process.env.CASCADE_MODEL ?? getModel() ?? undefined; + const prBranch = process.env.CASCADE_PR_BRANCH ?? getPrBranch() ?? undefined; + const headSha = process.env.CASCADE_INITIAL_HEAD_SHA ?? getInitialHeadSha() ?? undefined; + return { reportId: randomUUID(), summary: params.summary, @@ -136,34 +179,41 @@ function buildReport(params: ReportFrictionParams, project: ProjectConfig): Fric pmType: project.pm?.type, }, agent: { - type: process.env.CASCADE_AGENT_TYPE ?? 'unknown', - engine: process.env.CASCADE_ENGINE_LABEL, - model: process.env.CASCADE_MODEL, + type: agentTypeValue, + engine: engineLabelValue, + model: modelValue, }, run: { - id: process.env.CASCADE_RUN_ID, - url: - process.env.CASCADE_DASHBOARD_URL && process.env.CASCADE_RUN_ID - ? `${process.env.CASCADE_DASHBOARD_URL.replace(/\/$/, '')}/runs/${process.env.CASCADE_RUN_ID}` - : undefined, + id: runId, + url: dashboardUrl && runId ? `${dashboardUrl.replace(/\/$/, '')}/runs/${runId}` : undefined, }, workItem: { - id: process.env.CASCADE_WORK_ITEM_ID, - title: process.env.CASCADE_WORK_ITEM_TITLE, - url: process.env.CASCADE_WORK_ITEM_URL, + id: workItemId, + title: workItemTitle, + url: workItemUrl, }, pr: { - branch: process.env.CASCADE_PR_BRANCH, - headSha: process.env.CASCADE_INITIAL_HEAD_SHA, + number: prNumber, + title: prTitle, + url: prUrl, + branch: prBranch, + headSha, }, }, }; } export async function reportFriction(params: ReportFrictionParams): Promise { - const project = params.project ?? projectFromEnv(); + // Prefer params.project; fall back to SessionState (populated by LLMist createConfiguredBuilder + // from the full AgentExecutionPlan's project) before falling back to the env-var reconstruction. + // In LLMist (in-process gadgets), projectSecrets are NOT exported into process.env, so + // projectFromEnv() would return 'unknown-project' with empty PM placement config. + const project = params.project ?? getProject() ?? projectFromEnv(); const sidecarPath = - params.sidecarPath ?? process.env[FRICTION_SIDECAR_ENV_VAR] ?? DEFAULT_FRICTION_SIDECAR_PATH; + params.sidecarPath ?? + process.env[FRICTION_SIDECAR_ENV_VAR] ?? + getFrictionSidecarPath() ?? + DEFAULT_FRICTION_SIDECAR_PATH; const report = buildReport(params, project); await appendQueuedFrictionReport(sidecarPath, report, report.createdAt); diff --git a/src/gadgets/sessionState.ts b/src/gadgets/sessionState.ts index a43c31fbd..a16fbbef4 100644 --- a/src/gadgets/sessionState.ts +++ b/src/gadgets/sessionState.ts @@ -1,4 +1,5 @@ import type { FinishHookFlags } from '../agents/definitions/schema.js'; +import type { ProjectConfig } from '../types/index.js'; /** Env var holding the temp file path for the review sidecar (written by CLI subprocess, read by adapter). */ export const REVIEW_SIDECAR_ENV_VAR = 'CASCADE_REVIEW_SIDECAR_PATH'; @@ -8,6 +9,8 @@ export const PR_SIDECAR_ENV_VAR = 'CASCADE_PR_SIDECAR_PATH'; export const PUSHED_CHANGES_SIDECAR_ENV_VAR = 'CASCADE_PUSHED_CHANGES_SIDECAR_PATH'; /** Env var holding the temp file path for PM write evidence (written by cascade-tools pm add-checklist). */ export const PM_WRITE_SIDECAR_ENV_VAR = 'CASCADE_PM_WRITE_SIDECAR_PATH'; +/** Env var holding the JSONL outbox for incidental friction reports. */ +export const FRICTION_SIDECAR_ENV_VAR = 'CASCADE_FRICTION_SIDECAR_PATH'; export type SessionHooks = FinishHookFlags; @@ -19,7 +22,29 @@ export interface InitSessionStateOptions { hooks?: SessionHooks; workItemUrl?: string; workItemTitle?: string; + frictionSidecarPath?: string; initialHeadSha?: string; + /** Full project config for in-process gadgets (e.g. LLMist ReportFriction) that need project + * context but cannot access it via process.env (projectSecrets are not exported in-process). */ + project?: ProjectConfig; + /** Run ID for the current agent execution. Used by in-process gadgets as a fallback + * when CASCADE_RUN_ID is not exported to process.env (e.g. LLMist runs). */ + runId?: string; + /** PR number for the current agent execution. Used by in-process gadgets as a fallback + * when CASCADE_PR_NUMBER is not exported to process.env (e.g. LLMist runs). */ + prNumber?: number; + /** PR URL for the current agent execution. Used by in-process gadgets as a fallback + * when CASCADE_PR_URL is not exported to process.env (e.g. LLMist runs). */ + prUrl?: string; + /** PR title for the current agent execution. Used by in-process gadgets as a fallback + * when CASCADE_PR_TITLE is not exported to process.env (e.g. LLMist runs). */ + prTitle?: string; + /** Engine identifier for in-process gadgets (e.g. 'llmist'). Used as a fallback + * when CASCADE_ENGINE_LABEL is not exported to process.env. */ + engineLabel?: string; + /** Resolved AI model name for in-process gadgets (e.g. 'gpt-4o'). Used as a fallback + * when CASCADE_MODEL is not exported to process.env. */ + model?: string; /** * The PR HEAD branch name. Threaded into Finish validation so that * `hasUnpushedCommits` can use ls-remote SHA comparison instead of the @@ -34,14 +59,21 @@ interface SessionStateData { baseBranch: string; prBranch: string | null; projectId: string | null; + project: ProjectConfig | null; workItemId: string | null; workItemUrl: string | null; workItemTitle: string | null; + frictionSidecarPath: string | null; initialHeadSha: string | null; + runId: string | null; + prNumber: number | null; + prUrl: string | null; + prTitle: string | null; + engineLabel: string | null; + model: string | null; hooks: SessionHooks; readOnlyFs: boolean; prCreated: boolean; - prUrl: string | null; reviewSubmitted: boolean; reviewUrl: string | null; reviewBody: string | null; @@ -68,14 +100,21 @@ export class SessionState { baseBranch: 'main', prBranch: null, projectId: null, + project: null, workItemId: null, workItemUrl: null, workItemTitle: null, + frictionSidecarPath: null, initialHeadSha: null, + runId: null, + prNumber: null, + prUrl: null, + prTitle: null, + engineLabel: null, + model: null, hooks: {}, readOnlyFs: false, prCreated: false, - prUrl: null, reviewSubmitted: false, reviewUrl: null, reviewBody: null, @@ -90,25 +129,40 @@ export class SessionState { baseBranch, prBranch, projectId, + project, workItemId, hooks, workItemUrl, workItemTitle, + frictionSidecarPath, initialHeadSha, + runId, + prNumber, + prUrl, + prTitle, + engineLabel, + model, } = options; this.state = { agentType, baseBranch: baseBranch ?? 'main', prBranch: prBranch ?? null, projectId: projectId ?? null, + project: project ?? null, workItemId: workItemId ?? null, workItemUrl: workItemUrl ?? null, workItemTitle: workItemTitle ?? null, + frictionSidecarPath: frictionSidecarPath ?? null, initialHeadSha: initialHeadSha ?? null, + runId: runId ?? null, + prNumber: prNumber ?? null, + prUrl: prUrl ?? null, + prTitle: prTitle ?? null, + engineLabel: engineLabel ?? null, + model: model ?? null, hooks: hooks ?? {}, readOnlyFs: false, prCreated: false, - prUrl: null, reviewSubmitted: false, reviewUrl: null, reviewBody: null, @@ -142,6 +196,50 @@ export class SessionState { return this.state.workItemTitle; } + getProject(): ProjectConfig | null { + return this.state.project; + } + + getFrictionSidecarPath(): string | null { + return this.state.frictionSidecarPath; + } + + getRunId(): string | null { + return this.state.runId; + } + + getPrNumber(): number | null { + return this.state.prNumber; + } + + getPrUrl(): string | null { + return this.state.prUrl; + } + + getPrTitle(): string | null { + return this.state.prTitle; + } + + getAgentType(): string | null { + return this.state.agentType; + } + + getPrBranch(): string | null { + return this.state.prBranch; + } + + getInitialHeadSha(): string | null { + return this.state.initialHeadSha; + } + + getEngineLabel(): string | null { + return this.state.engineLabel; + } + + getModel(): string | null { + return this.state.model; + } + recordPRCreation(prUrl: string): void { this.state.prCreated = true; this.state.prUrl = prUrl; @@ -260,6 +358,50 @@ export function getWorkItemTitle(): string | null { return _defaultInstance.getWorkItemTitle(); } +export function getProject(): ProjectConfig | null { + return _defaultInstance.getProject(); +} + +export function getFrictionSidecarPath(): string | null { + return _defaultInstance.getFrictionSidecarPath(); +} + +export function getRunId(): string | null { + return _defaultInstance.getRunId(); +} + +export function getPrNumber(): number | null { + return _defaultInstance.getPrNumber(); +} + +export function getPrUrl(): string | null { + return _defaultInstance.getPrUrl(); +} + +export function getPrTitle(): string | null { + return _defaultInstance.getPrTitle(); +} + +export function getAgentType(): string | null { + return _defaultInstance.getAgentType(); +} + +export function getPrBranch(): string | null { + return _defaultInstance.getPrBranch(); +} + +export function getInitialHeadSha(): string | null { + return _defaultInstance.getInitialHeadSha(); +} + +export function getEngineLabel(): string | null { + return _defaultInstance.getEngineLabel(); +} + +export function getModel(): string | null { + return _defaultInstance.getModel(); +} + export function recordPRCreation(prUrl: string): void { _defaultInstance.recordPRCreation(prUrl); } diff --git a/src/triggers/shared/agent-work-items.ts b/src/triggers/shared/agent-work-items.ts index c14b6209c..8a5ac5f77 100644 --- a/src/triggers/shared/agent-work-items.ts +++ b/src/triggers/shared/agent-work-items.ts @@ -37,11 +37,29 @@ export async function prepareAgentWorkItem( projectId: string, ): Promise { const workItemId = await resolveWorkItemId(result.workItemId, projectId, result.prNumber); - const agentInput = + let agentInput = workItemId && result.agentInput.workItemId !== workItemId ? { ...result.agentInput, workItemId } : result.agentInput; + // Merge top-level TriggerResult URL/title metadata into agentInput when absent. + // Several GitHub trigger handlers (PRReviewSubmittedTrigger, PROpenedTrigger, + // ReviewRequestedTrigger, PRCommentMentionTrigger) set prUrl/prTitle/workItemUrl/ + // workItemTitle only at the TriggerResult top level without mirroring them into + // agentInput. Both injectAgentInputContext (secretBuilder.ts) and LlmistEngine read + // from agentInput, so we centralize the merge here before agent execution rather + // than patching every handler individually. + const extras: Partial = {}; + if (result.prUrl && !agentInput.prUrl) extras.prUrl = result.prUrl; + if (result.prTitle && !agentInput.prTitle) extras.prTitle = result.prTitle; + if (result.workItemUrl && !agentInput.workItemUrl) extras.workItemUrl = result.workItemUrl; + if (result.workItemTitle && !agentInput.workItemTitle) + extras.workItemTitle = result.workItemTitle; + + if (Object.keys(extras).length > 0) { + agentInput = { ...agentInput, ...extras }; + } + return { workItemId, agentInput }; } diff --git a/src/triggers/shared/result-builders.ts b/src/triggers/shared/result-builders.ts index 2ded64be9..f8c595811 100644 --- a/src/triggers/shared/result-builders.ts +++ b/src/triggers/shared/result-builders.ts @@ -43,17 +43,33 @@ interface DeferredRecheckOptions { agentInput?: AgentInput; } +interface AgentInputExtraContext { + workItemUrl?: string; + workItemTitle?: string; + prUrl?: string; + prTitle?: string; +} + function buildAgentInput( agentInput: AgentInput | undefined, triggerEvent: CanonicalTriggerEvent, workItemId: string | undefined, prNumber?: number, + extraContext?: AgentInputExtraContext, ): AgentInput { return { ...agentInput, triggerEvent, ...(workItemId ? { workItemId } : {}), ...(prNumber !== undefined ? { prNumber } : {}), + // Mirror URL/title fields into agentInput so injectAgentInputContext + // can inject them as CASCADE_WORK_ITEM_URL/TITLE/CASCADE_PR_URL/TITLE + // env vars into the subprocess. Without this, webhook-triggered native-tool + // runs receive IDs but miss the URL/title context. + ...(extraContext?.workItemUrl ? { workItemUrl: extraContext.workItemUrl } : {}), + ...(extraContext?.workItemTitle ? { workItemTitle: extraContext.workItemTitle } : {}), + ...(extraContext?.prUrl ? { prUrl: extraContext.prUrl } : {}), + ...(extraContext?.prTitle ? { prTitle: extraContext.prTitle } : {}), }; } @@ -71,7 +87,10 @@ export function buildPMDispatchResult(options: PMDispatchOptions): TriggerResult return { agentType, - agentInput: buildAgentInput(agentInput, triggerEvent, workItemId), + agentInput: buildAgentInput(agentInput, triggerEvent, workItemId, undefined, { + workItemUrl, + workItemTitle, + }), workItemId, workItemUrl, workItemTitle, @@ -97,7 +116,12 @@ export function buildGitHubPRDispatchResult(options: GitHubPRDispatchOptions): T return { agentType, - agentInput: buildAgentInput(agentInput, triggerEvent, workItemId, prNumber), + agentInput: buildAgentInput(agentInput, triggerEvent, workItemId, prNumber, { + workItemUrl, + workItemTitle, + prUrl, + prTitle, + }), prNumber, prUrl, prTitle, diff --git a/tests/helpers/backendMocks.ts b/tests/helpers/backendMocks.ts index 993ac8d12..0acf912b6 100644 --- a/tests/helpers/backendMocks.ts +++ b/tests/helpers/backendMocks.ts @@ -124,11 +124,27 @@ export const mockSessionStateModule = { PR_SIDECAR_ENV_VAR: 'CASCADE_PR_SIDECAR_PATH', PUSHED_CHANGES_SIDECAR_ENV_VAR: 'CASCADE_PUSHED_CHANGES_SIDECAR_PATH', REVIEW_SIDECAR_ENV_VAR: 'CASCADE_REVIEW_SIDECAR_PATH', + PM_WRITE_SIDECAR_ENV_VAR: 'CASCADE_PM_WRITE_SIDECAR_PATH', + FRICTION_SIDECAR_ENV_VAR: 'CASCADE_FRICTION_SIDECAR_PATH', // Record functions recordInitialComment: vi.fn(), recordPRCreation: vi.fn(), recordReviewSubmission: vi.fn(), clearInitialComment: vi.fn(), + getFrictionSidecarPath: vi.fn(() => null), + getRunId: vi.fn(() => null), + getPrNumber: vi.fn(() => null), + getPrUrl: vi.fn(() => null), + getPrTitle: vi.fn(() => null), + getWorkItemId: vi.fn(() => null), + getWorkItemUrl: vi.fn(() => null), + getWorkItemTitle: vi.fn(() => null), + getProject: vi.fn(() => null), + getAgentType: vi.fn(() => null), + getPrBranch: vi.fn(() => null), + getInitialHeadSha: vi.fn(() => null), + getEngineLabel: vi.fn(() => null), + getModel: vi.fn(() => null), }; // --------------------------------------------------------------------------- diff --git a/tests/unit/agents/shared/builderFactory.test.ts b/tests/unit/agents/shared/builderFactory.test.ts index eac6a65c2..f06c8fe18 100644 --- a/tests/unit/agents/shared/builderFactory.test.ts +++ b/tests/unit/agents/shared/builderFactory.test.ts @@ -166,16 +166,19 @@ describe('createConfiguredBuilder', () => { it('calls initSessionState when skipSessionState is not set', async () => { const options = createBaseOptions(); await createConfiguredBuilder(options); - expect(mockInitSessionState).toHaveBeenCalledWith({ - agentType: 'implementation', - baseBranch: undefined, - projectId: undefined, - workItemId: undefined, - hooks: undefined, - workItemUrl: undefined, - workItemTitle: undefined, - initialHeadSha: 'abc123headsha', - }); + expect(mockInitSessionState).toHaveBeenCalledWith( + expect.objectContaining({ + agentType: 'implementation', + baseBranch: undefined, + projectId: undefined, + workItemId: undefined, + hooks: undefined, + workItemUrl: undefined, + workItemTitle: undefined, + frictionSidecarPath: undefined, + initialHeadSha: 'abc123headsha', + }), + ); }); it('skips initSessionState when skipSessionState is true', async () => { @@ -191,16 +194,19 @@ describe('createConfiguredBuilder', () => { workItemId: 'card-123', }); await createConfiguredBuilder(options); - expect(mockInitSessionState).toHaveBeenCalledWith({ - agentType: 'implementation', - baseBranch: 'main', - projectId: 'project-1', - workItemId: 'card-123', - hooks: undefined, - workItemUrl: undefined, - workItemTitle: undefined, - initialHeadSha: 'abc123headsha', - }); + expect(mockInitSessionState).toHaveBeenCalledWith( + expect.objectContaining({ + agentType: 'implementation', + baseBranch: 'main', + projectId: 'project-1', + workItemId: 'card-123', + hooks: undefined, + workItemUrl: undefined, + workItemTitle: undefined, + frictionSidecarPath: undefined, + initialHeadSha: 'abc123headsha', + }), + ); }); it('passes workItemUrl and workItemTitle to initSessionState', async () => { @@ -212,16 +218,49 @@ describe('createConfiguredBuilder', () => { workItemTitle: 'My Feature Card', }); await createConfiguredBuilder(options); - expect(mockInitSessionState).toHaveBeenCalledWith({ - agentType: 'implementation', - baseBranch: 'main', - projectId: 'project-1', - workItemId: 'card-123', - hooks: undefined, - workItemUrl: 'https://trello.com/c/abc123', - workItemTitle: 'My Feature Card', - initialHeadSha: 'abc123headsha', + expect(mockInitSessionState).toHaveBeenCalledWith( + expect.objectContaining({ + agentType: 'implementation', + baseBranch: 'main', + projectId: 'project-1', + workItemId: 'card-123', + hooks: undefined, + workItemUrl: 'https://trello.com/c/abc123', + workItemTitle: 'My Feature Card', + frictionSidecarPath: undefined, + initialHeadSha: 'abc123headsha', + }), + ); + }); + + it('passes frictionSidecarPath to initSessionState', async () => { + const options = createBaseOptions({ + frictionSidecarPath: '/tmp/friction.jsonl', }); + await createConfiguredBuilder(options); + expect(mockInitSessionState).toHaveBeenCalledWith( + expect.objectContaining({ + frictionSidecarPath: '/tmp/friction.jsonl', + }), + ); + }); + + it('passes runId and PR context fields to initSessionState for in-process gadget fallback', async () => { + const options = createBaseOptions({ + runId: 'run-abc', + prNumber: 42, + prUrl: 'https://github.com/o/r/pull/42', + prTitle: 'fix: runtime metadata', + }); + await createConfiguredBuilder(options); + expect(mockInitSessionState).toHaveBeenCalledWith( + expect.objectContaining({ + runId: 'run-abc', + prNumber: 42, + prUrl: 'https://github.com/o/r/pull/42', + prTitle: 'fix: runtime metadata', + }), + ); }); it('passes undefined initialHeadSha when git rev-parse fails', async () => { @@ -230,16 +269,19 @@ describe('createConfiguredBuilder', () => { }); const options = createBaseOptions(); await createConfiguredBuilder(options); - expect(mockInitSessionState).toHaveBeenCalledWith({ - agentType: 'implementation', - baseBranch: undefined, - projectId: undefined, - workItemId: undefined, - hooks: undefined, - workItemUrl: undefined, - workItemTitle: undefined, - initialHeadSha: undefined, - }); + expect(mockInitSessionState).toHaveBeenCalledWith( + expect.objectContaining({ + agentType: 'implementation', + baseBranch: undefined, + projectId: undefined, + workItemId: undefined, + hooks: undefined, + workItemUrl: undefined, + workItemTitle: undefined, + frictionSidecarPath: undefined, + initialHeadSha: undefined, + }), + ); }); it('calls withBudget when remainingBudgetUsd is positive', async () => { diff --git a/tests/unit/backends/adapter.test.ts b/tests/unit/backends/adapter.test.ts index b84eb72ca..f85210ad5 100644 --- a/tests/unit/backends/adapter.test.ts +++ b/tests/unit/backends/adapter.test.ts @@ -362,6 +362,82 @@ describe('executeWithEngine', () => { expect(result.error).toContain('Backend crashed'); }); + it('drains and cleans the friction sidecar without changing a successful run', async () => { + setupMocks(); + let sidecarPath: string | undefined; + const engine = makeMockBackend('opencode'); + vi.mocked(engine.execute).mockImplementation(async (plan) => { + sidecarPath = plan.frictionSidecarPath; + writeFileSync( + plan.frictionSidecarPath as string, + `${JSON.stringify({ + event: 'queued', + reportId: 'friction-1', + timestamp: '2026-05-09T00:00:00.000Z', + report: { + reportId: 'friction-1', + summary: 'Drain failure stays non-blocking', + details: 'Credential lookup is mocked away in this test.', + category: 'tooling', + severity: 'medium', + whileDoing: 'Testing adapter', + context: { project: { id: 'test' } }, + }, + })}\n`, + ); + return { success: true, output: 'Done' }; + }); + + const result = await executeWithEngine( + engine, + 'implementation', + makeInput({ + project: { + ...makeProject(), + trello: { boardId: 'b1', lists: { friction: 'list-friction' }, labels: {} }, + }, + } as Partial), + ); + + expect(result.success).toBe(true); + expect(sidecarPath).toBeTruthy(); + expect(existsSync(sidecarPath as string)).toBe(false); + }); + + it('cleans the friction sidecar after an ordinary engine failure', async () => { + setupMocks(); + let sidecarPath: string | undefined; + const engine = makeMockBackend('opencode'); + vi.mocked(engine.execute).mockImplementation(async (plan) => { + sidecarPath = plan.frictionSidecarPath; + writeFileSync( + plan.frictionSidecarPath as string, + `${JSON.stringify({ + event: 'queued', + reportId: 'friction-failure', + timestamp: '2026-05-09T00:00:00.000Z', + report: { + reportId: 'friction-failure', + summary: 'Engine failed after reporting friction', + details: 'The sidecar should still be drained before cleanup.', + category: 'tooling', + severity: 'medium', + whileDoing: 'Testing adapter failure cleanup', + context: { project: { id: 'test' } }, + }, + })}\n`, + ); + throw new Error('Backend crashed after friction report'); + }); + + const result = await executeWithEngine(engine, 'implementation', makeInput()); + + expect(result.success).toBe(false); + expect(result.error).toContain('Backend crashed after friction report'); + expect(sidecarPath).toBeTruthy(); + expect(existsSync(sidecarPath as string)).toBe(false); + }); + it('reports engine errors to Sentry via captureException', async () => { setupMocks(); const engine = makeMockBackend(); @@ -672,7 +748,11 @@ describe('executeWithEngine', () => { TRELLO_API_KEY: 'proj-trello-key', CASCADE_PROJECT_ID: 'test', CASCADE_PROJECT_NAME: 'Test', + CASCADE_ENGINE_LABEL: 'test-engine', + CASCADE_MODEL: 'test-model', + CASCADE_RUN_ID: 'run-uuid-123', CASCADE_BASE_BRANCH: 'main', + CASCADE_WORK_ITEM_ID: 'card123', CASCADE_REPO_OWNER: 'owner', CASCADE_REPO_NAME: 'repo', CASCADE_TRELLO_BOARD_ID: 'b1', @@ -680,7 +760,13 @@ describe('executeWithEngine', () => { CASCADE_TRELLO_LABELS: '{}', CASCADE_AGENT_TYPE: 'implementation', CASCADE_PM_TYPE: 'trello', + CASCADE_FRICTION_SIDECAR_PATH: expect.stringMatching( + /cascade-friction-sidecar-\d+-\d+\.jsonl$/, + ), }); + expect(backendInput.frictionSidecarPath).toBe( + backendInput.projectSecrets?.CASCADE_FRICTION_SIDECAR_PATH, + ); }); it('passes PR context fields to promptContext for respond-to-ci agent', async () => { @@ -721,7 +807,11 @@ describe('executeWithEngine', () => { expect(backendInput.projectSecrets).toEqual({ CASCADE_PROJECT_ID: 'test', CASCADE_PROJECT_NAME: 'Test', + CASCADE_ENGINE_LABEL: 'test-engine', + CASCADE_MODEL: 'test-model', + CASCADE_RUN_ID: 'run-uuid-123', CASCADE_BASE_BRANCH: 'main', + CASCADE_WORK_ITEM_ID: 'card123', CASCADE_REPO_OWNER: 'owner', CASCADE_REPO_NAME: 'repo', CASCADE_TRELLO_BOARD_ID: 'b1', @@ -729,6 +819,9 @@ describe('executeWithEngine', () => { CASCADE_TRELLO_LABELS: '{}', CASCADE_AGENT_TYPE: 'implementation', CASCADE_PM_TYPE: 'trello', + CASCADE_FRICTION_SIDECAR_PATH: expect.stringMatching( + /cascade-friction-sidecar-\d+-\d+\.jsonl$/, + ), }); }); diff --git a/tests/unit/backends/llmist.test.ts b/tests/unit/backends/llmist.test.ts index d1f881679..7b6c0de67 100644 --- a/tests/unit/backends/llmist.test.ts +++ b/tests/unit/backends/llmist.test.ts @@ -194,6 +194,7 @@ describe('LlmistEngine.execute', () => { it('reads prUrl from session state regardless of agent output text', async () => { // Reproduces the false positive from run 1391506b: the CreatePR gadget // recorded the URL in session state, but the LLM didn't echo it in output. + // prCreated must be true (set by recordPRCreation) to confirm CreatePR was called. mockRunAgentLoop.mockResolvedValue({ output: 'All done, PR created successfully.', iterations: 9, @@ -202,6 +203,7 @@ describe('LlmistEngine.execute', () => { loopTerminated: false, }); mockGetSessionState.mockReturnValue({ + prCreated: true, prUrl: 'https://github.com/owner/repo/pull/42', } as ReturnType); diff --git a/tests/unit/backends/secretBuilder.test.ts b/tests/unit/backends/secretBuilder.test.ts index 95eb355a5..6075e065d 100644 --- a/tests/unit/backends/secretBuilder.test.ts +++ b/tests/unit/backends/secretBuilder.test.ts @@ -89,6 +89,27 @@ describe('augmentProjectSecrets', () => { expect(secrets.CASCADE_AGENT_TYPE).toBe('review'); }); + it('injects work item and PR runtime metadata from agent input', async () => { + const project = makeProject(); + const secrets = await augmentProjectSecrets(project, 'review', { + workItemId: 'card-123', + workItemUrl: 'https://trello.com/c/card123', + workItemTitle: 'Fix runtime context', + prNumber: 42, + prUrl: 'https://github.com/acme/widgets/pull/42', + prTitle: 'fix: runtime context', + } as AgentInput); + + expect(secrets).toMatchObject({ + CASCADE_WORK_ITEM_ID: 'card-123', + CASCADE_WORK_ITEM_URL: 'https://trello.com/c/card123', + CASCADE_WORK_ITEM_TITLE: 'Fix runtime context', + CASCADE_PR_NUMBER: '42', + CASCADE_PR_URL: 'https://github.com/acme/widgets/pull/42', + CASCADE_PR_TITLE: 'fix: runtime context', + }); + }); + it('injects CASCADE_PM_TYPE defaulting to trello', async () => { const project = makeProject(); const secrets = await augmentProjectSecrets(project, 'implementation', {} as AgentInput); diff --git a/tests/unit/backends/secretOrchestrator.test.ts b/tests/unit/backends/secretOrchestrator.test.ts index 76fb8e887..8cc9829fe 100644 --- a/tests/unit/backends/secretOrchestrator.test.ts +++ b/tests/unit/backends/secretOrchestrator.test.ts @@ -295,7 +295,7 @@ describe('buildExecutionPlan', () => { }); describe('injectRunLinkSecrets', () => { - it('does nothing when runLinksEnabled is false', () => { + it('injects runtime context but no dashboard link flags when runLinksEnabled is false', () => { mockGetDashboardUrl.mockReturnValue('https://dashboard.example.com'); const project = makeProject({ runLinksEnabled: false }); const partialInput: { projectSecrets?: Record; model?: string } = { @@ -304,10 +304,18 @@ describe('injectRunLinkSecrets', () => { injectRunLinkSecrets(partialInput, project, 'claude-code', 'card123', 'run-id-1'); - expect(partialInput.projectSecrets).toBeUndefined(); + expect(partialInput.projectSecrets).toEqual({ + CASCADE_ENGINE_LABEL: 'claude-code', + CASCADE_MODEL: 'test-model', + CASCADE_PROJECT_ID: 'test-project', + CASCADE_WORK_ITEM_ID: 'card123', + CASCADE_RUN_ID: 'run-id-1', + }); + expect(partialInput.projectSecrets?.CASCADE_RUN_LINKS_ENABLED).toBeUndefined(); + expect(partialInput.projectSecrets?.CASCADE_DASHBOARD_URL).toBeUndefined(); }); - it('does nothing when dashboardUrl is absent', () => { + it('injects runtime context when dashboardUrl is absent', () => { mockGetDashboardUrl.mockReturnValue(undefined); const project = makeProject({ runLinksEnabled: true }); const partialInput: { projectSecrets?: Record; model?: string } = { @@ -316,7 +324,14 @@ describe('injectRunLinkSecrets', () => { injectRunLinkSecrets(partialInput, project, 'claude-code', 'card123', 'run-id-1'); - expect(partialInput.projectSecrets).toBeUndefined(); + expect(partialInput.projectSecrets).toEqual({ + CASCADE_ENGINE_LABEL: 'claude-code', + CASCADE_MODEL: 'test-model', + CASCADE_PROJECT_ID: 'test-project', + CASCADE_WORK_ITEM_ID: 'card123', + CASCADE_RUN_ID: 'run-id-1', + }); + expect(partialInput.projectSecrets?.CASCADE_RUN_LINKS_ENABLED).toBeUndefined(); }); it('injects all run link secrets when enabled', () => { diff --git a/tests/unit/backends/sidecarManager.test.ts b/tests/unit/backends/sidecarManager.test.ts index 8a87e1216..1fc21476d 100644 --- a/tests/unit/backends/sidecarManager.test.ts +++ b/tests/unit/backends/sidecarManager.test.ts @@ -1,19 +1,39 @@ -import { existsSync, writeFileSync } from 'node:fs'; +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('../../../src/gadgets/sessionState.js', () => ({ REVIEW_SIDECAR_ENV_VAR: 'CASCADE_REVIEW_SIDECAR_PATH', PR_SIDECAR_ENV_VAR: 'CASCADE_PR_SIDECAR_PATH', PUSHED_CHANGES_SIDECAR_ENV_VAR: 'CASCADE_PUSHED_CHANGES_SIDECAR_PATH', PM_WRITE_SIDECAR_ENV_VAR: 'CASCADE_PM_WRITE_SIDECAR_PATH', + FRICTION_SIDECAR_ENV_VAR: 'CASCADE_FRICTION_SIDECAR_PATH', clearInitialComment: vi.fn(), recordPRCreation: vi.fn(), recordReviewSubmission: vi.fn(), })); +const mockMaterializeFrictionReport = vi.fn(); +const mockWithPMCredentials = vi.fn((_projectId: string, fn: () => Promise) => fn()); + +vi.mock('../../../src/friction/materialize.js', () => ({ + materializeFrictionReport: (...args: unknown[]) => mockMaterializeFrictionReport(...args), +})); + +vi.mock('../../../src/pm/registry.js', () => ({ + pmRegistry: { + getOrNull: vi.fn(() => ({ + withCredentials: mockWithPMCredentials, + })), + }, +})); + +vi.mock('../../../src/sentry.js', () => ({ + captureException: vi.fn(), +})); + vi.mock('../../../src/utils/logging.js', () => ({ logger: { info: vi.fn(), @@ -26,6 +46,7 @@ import type { AgentProfile } from '../../../src/agents/definitions/profiles.js'; import { cleanupTempFile, createCompletionArtifacts, + drainFrictionSidecarReports, hydrateNativeToolSidecars, hydratePrSidecar, hydrateReviewSidecar, @@ -35,11 +56,20 @@ import { recordPRCreation, recordReviewSubmission, } from '../../../src/gadgets/sessionState.js'; -import type { AgentInput } from '../../../src/types/index.js'; +import { captureException } from '../../../src/sentry.js'; +import type { AgentInput, ProjectConfig } from '../../../src/types/index.js'; const mockRecordPRCreation = vi.mocked(recordPRCreation); const mockRecordReviewSubmission = vi.mocked(recordReviewSubmission); const mockClearInitialComment = vi.mocked(clearInitialComment); +const mockCaptureException = vi.mocked(captureException); + +beforeEach(() => { + vi.clearAllMocks(); + mockWithPMCredentials.mockImplementation((_projectId: string, fn: () => Promise) => + fn(), + ); +}); function makeProfile(overrides?: Partial): AgentProfile { return { @@ -58,7 +88,37 @@ function makeSidecarPath(name: string): string { return join(tmpdir(), `test-${name}-${process.pid}-${Date.now()}.json`); } +function makeFrictionSidecarPath(name: string): string { + return join(tmpdir(), `test-${name}-${process.pid}-${Date.now()}.jsonl`); +} + +function makeProject(overrides?: Partial): ProjectConfig { + return { + id: 'project-1', + name: 'Project 1', + pm: { type: 'trello' }, + trello: { boardId: 'board-1', lists: { friction: 'list-friction' }, labels: {} }, + ...overrides, + } as ProjectConfig; +} + describe('createCompletionArtifacts', () => { + it('always creates a friction sidecar path and injects CASCADE_FRICTION_SIDECAR_PATH', () => { + const profile = makeProfile({ finishHooks: {} }); + const projectSecrets: Record = {}; + + const result = createCompletionArtifacts( + profile, + 'implementation', + false, + {} as AgentInput, + projectSecrets, + ); + + expect(result.frictionSidecarPath).toMatch(/cascade-friction-sidecar-\d+-\d+\.jsonl$/); + expect(projectSecrets.CASCADE_FRICTION_SIDECAR_PATH).toBe(result.frictionSidecarPath); + }); + it('creates a review sidecar path when profile.finishHooks.requiresReview is true', () => { const profile = makeProfile({ finishHooks: { requiresReview: true } }); const projectSecrets: Record = {}; @@ -487,6 +547,154 @@ describe('hydrateNativeToolSidecars', () => { }); }); +describe('drainFrictionSidecarReports', () => { + it('materializes pending reports and appends filed events without throwing', async () => { + const sidecarPath = makeFrictionSidecarPath('friction-drain-success'); + writeFileSync( + sidecarPath, + `${JSON.stringify({ + event: 'queued', + reportId: 'friction-1', + timestamp: '2026-05-09T00:00:00.000Z', + report: { + reportId: 'friction-1', + summary: 'Missing hint', + details: 'Need better setup docs', + category: 'tooling', + severity: 'medium', + whileDoing: 'Running tests', + context: { project: { id: 'project-1' } }, + }, + })}\n`, + ); + mockMaterializeFrictionReport.mockResolvedValue({ + status: 'filed', + reportId: 'friction-1', + workItemId: 'card-1', + workItemUrl: 'https://pm/card-1', + }); + + await drainFrictionSidecarReports({ + sidecarPath, + project: makeProject(), + agentType: 'implementation', + runId: 'run-1', + engineId: 'codex', + }); + + expect(mockWithPMCredentials).toHaveBeenCalledWith('project-1', expect.any(Function)); + expect(mockMaterializeFrictionReport).toHaveBeenCalledWith( + expect.objectContaining({ + project: expect.objectContaining({ id: 'project-1' }), + report: expect.objectContaining({ reportId: 'friction-1' }), + }), + ); + const events = readFileSync(sidecarPath, 'utf-8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(events.map((event) => event.event)).toEqual(['queued', 'filed']); + expect(events[1]).toMatchObject({ + event: 'filed', + reportId: 'friction-1', + workItemId: 'card-1', + workItemUrl: 'https://pm/card-1', + }); + }); + + it('does not re-materialize reports already marked filed', async () => { + const sidecarPath = makeFrictionSidecarPath('friction-drain-compact'); + writeFileSync( + sidecarPath, + `${JSON.stringify({ + event: 'queued', + reportId: 'friction-1', + timestamp: '2026-05-09T00:00:00.000Z', + report: { + reportId: 'friction-1', + summary: 'Already filed', + details: 'No retry needed', + category: 'tooling', + severity: 'low', + whileDoing: 'Review', + context: { project: { id: 'project-1' } }, + }, + })}\n${JSON.stringify({ + event: 'filed', + reportId: 'friction-1', + workItemId: 'card-1', + timestamp: '2026-05-09T00:00:01.000Z', + })}\n`, + ); + + await drainFrictionSidecarReports({ + sidecarPath, + project: makeProject(), + agentType: 'review', + }); + + expect(mockMaterializeFrictionReport).not.toHaveBeenCalled(); + expect(readFileSync(sidecarPath, 'utf-8')).toBe(''); + }); + + it('logs and captures materialization failures without throwing', async () => { + const { logger } = await import('../../../src/utils/logging.js'); + const warnSpy = vi.mocked(logger.warn); + const sidecarPath = makeFrictionSidecarPath('friction-drain-failure'); + writeFileSync( + sidecarPath, + `${JSON.stringify({ + event: 'queued', + reportId: 'friction-err', + timestamp: '2026-05-09T00:00:00.000Z', + report: { + reportId: 'friction-err', + summary: 'PM unavailable', + details: 'Create failed', + category: 'pm-data', + severity: 'high', + whileDoing: 'Filing report', + context: { project: { id: 'project-1' } }, + }, + })}\n`, + ); + mockMaterializeFrictionReport.mockRejectedValue(new Error('PM unavailable')); + + await expect( + drainFrictionSidecarReports({ + sidecarPath, + project: makeProject(), + agentType: 'implementation', + runId: 'run-err', + engineId: 'opencode', + }), + ).resolves.not.toThrow(); + + expect(warnSpy).toHaveBeenCalledWith( + 'Failed to drain friction sidecar report', + expect.objectContaining({ + sidecarPath, + projectId: 'project-1', + agentType: 'implementation', + runId: 'run-err', + engine: 'opencode', + reportId: 'friction-err', + error: 'Error: PM unavailable', + }), + ); + expect(mockCaptureException).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ + tags: expect.objectContaining({ + source: 'friction_sidecar_drain_failed', + phase: 'materialize', + agentType: 'implementation', + }), + }), + ); + }); +}); + describe('cleanupTempFile', () => { it('deletes the file when it exists', () => { const filePath = makeSidecarPath('cleanup'); diff --git a/tests/unit/cli/credential-scoping.test.ts b/tests/unit/cli/credential-scoping.test.ts index b40383ac2..94947e86c 100644 --- a/tests/unit/cli/credential-scoping.test.ts +++ b/tests/unit/cli/credential-scoping.test.ts @@ -60,6 +60,7 @@ import { CredentialScopedCommand, resolveJiraBaseUrl } from '../../../src/cli/ba import { withGitHubToken } from '../../../src/github/client.js'; import { withJiraCredentials } from '../../../src/jira/client.js'; import { withLinearCredentials } from '../../../src/linear/client.js'; +import { getPMProvider } from '../../../src/pm/context.js'; import { withTrelloCredentials } from '../../../src/trello/client.js'; class TestCommand extends CredentialScopedCommand { @@ -72,6 +73,16 @@ class TestCommand extends CredentialScopedCommand { } } +class InspectPMProviderCommand extends CredentialScopedCommand { + static override id = 'inspect-pm-provider'; + static override description = 'Inspect PM provider'; + providerConfig: unknown; + + async execute(): Promise { + this.providerConfig = (getPMProvider() as unknown as { config?: unknown }).config; + } +} + describe('CredentialScopedCommand', () => { const originalEnv = process.env; @@ -85,6 +96,9 @@ describe('CredentialScopedCommand', () => { delete process.env.CASCADE_LINEAR_TEAM_ID; delete process.env.CASCADE_LINEAR_PROJECT_ID; delete process.env.CASCADE_LINEAR_STATUSES; + delete process.env.CASCADE_TRELLO_BOARD_ID; + delete process.env.CASCADE_TRELLO_LISTS; + delete process.env.CASCADE_TRELLO_LABELS; // Clear JIRA vars so resolvePmType() falls back to 'trello' when not // explicitly testing JIRA behaviour (env may be set on CI/dev machines). delete process.env.JIRA_EMAIL; @@ -231,4 +245,23 @@ describe('CredentialScopedCommand', () => { await expect(cmd.run()).resolves.not.toThrow(); expect(withLinearCredentials).toHaveBeenCalled(); }); + + it('synthesises Trello board/list/label config from env vars for scoped PM commands', async () => { + process.env.CASCADE_PM_TYPE = 'trello'; + process.env.CASCADE_TRELLO_BOARD_ID = 'board-123'; + process.env.CASCADE_TRELLO_LISTS = JSON.stringify({ + todo: 'list-todo', + friction: 'list-friction', + }); + process.env.CASCADE_TRELLO_LABELS = JSON.stringify({ auto: 'label-auto' }); + + const cmd = new InspectPMProviderCommand([], {} as never); + await cmd.run(); + + expect(cmd.providerConfig).toEqual({ + boardId: 'board-123', + lists: { todo: 'list-todo', friction: 'list-friction' }, + labels: { auto: 'label-auto' }, + }); + }); }); diff --git a/tests/unit/gadgets/pm/core/reportFriction.test.ts b/tests/unit/gadgets/pm/core/reportFriction.test.ts index a03dcc5b0..d73f6ad79 100644 --- a/tests/unit/gadgets/pm/core/reportFriction.test.ts +++ b/tests/unit/gadgets/pm/core/reportFriction.test.ts @@ -1,7 +1,7 @@ import { readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockMaterializeFrictionReport = vi.fn(); @@ -10,6 +10,10 @@ vi.mock('../../../../../src/friction/materialize.js', () => ({ })); import { reportFriction } from '../../../../../src/gadgets/pm/core/reportFriction.js'; +import { + createSessionState, + setDefaultSessionState, +} from '../../../../../src/gadgets/sessionState.js'; import type { ProjectConfig } from '../../../../../src/types/index.js'; const project = { @@ -33,6 +37,29 @@ afterEach(() => { vi.restoreAllMocks(); }); +beforeEach(() => { + mockMaterializeFrictionReport.mockReset(); + setDefaultSessionState(createSessionState()); + delete process.env.CASCADE_FRICTION_SIDECAR_PATH; + delete process.env.CASCADE_RUN_ID; + delete process.env.CASCADE_DASHBOARD_URL; + delete process.env.CASCADE_WORK_ITEM_ID; + delete process.env.CASCADE_WORK_ITEM_TITLE; + delete process.env.CASCADE_WORK_ITEM_URL; + delete process.env.CASCADE_PR_NUMBER; + delete process.env.CASCADE_PR_TITLE; + delete process.env.CASCADE_PR_URL; + delete process.env.CASCADE_PR_BRANCH; + delete process.env.CASCADE_INITIAL_HEAD_SHA; + delete process.env.CASCADE_AGENT_TYPE; + delete process.env.CASCADE_ENGINE_LABEL; + delete process.env.CASCADE_MODEL; + // Clear project env vars so projectFromEnv() would return 'unknown-project' when these are absent + delete process.env.CASCADE_PROJECT_ID; + delete process.env.CASCADE_PM_TYPE; + delete process.env.CASCADE_PROJECT_NAME; +}); + describe('reportFriction', () => { it('queues before filing and appends filed event after successful materialization', async () => { const path = sidecarPath(); @@ -134,4 +161,130 @@ describe('reportFriction', () => { }), ).rejects.toThrow('category must be one of'); }); + + it('uses SessionState for run/work-item/PR metadata when env vars are absent (LLMist in-process path)', async () => { + // In LLMist (in-process gadgets), projectSecrets are NOT exported to process.env. + // All metadata must be sourced from SessionState; env vars are intentionally absent. + const path = sidecarPath(); + const state = createSessionState(); + state.init({ + agentType: 'implementation', + projectId: 'project-1', + workItemId: 'card-123', + workItemUrl: 'https://trello.com/c/card123', + workItemTitle: 'Runtime metadata', + frictionSidecarPath: path, + runId: 'run-123', + prNumber: 77, + prUrl: 'https://github.com/o/r/pull/77', + prTitle: 'fix: runtime metadata', + prBranch: 'feature/runtime-metadata', + initialHeadSha: 'abc123sha', + engineLabel: 'llmist', + model: 'gpt-4o', + }); + setDefaultSessionState(state); + // CASCADE_DASHBOARD_URL is a global infra config, available in process.env even for LLMist + process.env.CASCADE_DASHBOARD_URL = 'https://dashboard.example.com'; + // Confirm none of the per-run secrets are in env (simulating LLMist production environment) + // The beforeEach already deletes CASCADE_RUN_ID, CASCADE_WORK_ITEM_ID, etc. + mockMaterializeFrictionReport.mockResolvedValue({ + status: 'filed', + reportId: 'ignored', + workItemId: 'friction-card', + }); + + await reportFriction({ + project, + summary: 'Context attached', + details: 'The report should carry run/work item/PR metadata from SessionState.', + category: 'tooling', + severity: 'medium', + }); + + const event = JSON.parse(readFileSync(path, 'utf-8').trim().split('\n')[0]); + expect(event.report.context).toMatchObject({ + agent: { + type: 'implementation', + engine: 'llmist', + model: 'gpt-4o', + }, + run: { + id: 'run-123', + url: 'https://dashboard.example.com/runs/run-123', + }, + workItem: { + id: 'card-123', + title: 'Runtime metadata', + url: 'https://trello.com/c/card123', + }, + pr: { + number: 77, + title: 'fix: runtime metadata', + url: 'https://github.com/o/r/pull/77', + branch: 'feature/runtime-metadata', + headSha: 'abc123sha', + }, + }); + rmSync(path, { force: true }); + }); + + it('uses project from SessionState when params.project is absent (production ReportFriction wrapper path)', async () => { + // The production ReportFriction.ts gadget does NOT pass params.project. + // In LLMist, projectSecrets are NOT exported to process.env, so projectFromEnv() + // would produce 'unknown-project'/empty PM config. The project must come from + // SessionState (stored by LLMist via createConfiguredBuilder → initSessionState). + const path = sidecarPath(); + const sessionProject = { + id: 'real-project-id', + orgId: 'org-1', + name: 'Real Project', + repo: 'owner/real-repo', + pm: { type: 'trello' as const }, + trello: { + boardId: 'board-real', + lists: { friction: 'list-friction-real' }, + labels: {}, + }, + } as ProjectConfig; + const state = createSessionState(); + state.init({ + agentType: 'implementation', + projectId: 'real-project-id', + project: sessionProject, + frictionSidecarPath: path, + }); + setDefaultSessionState(state); + // Simulate LLMist env: no project secrets exported + // (beforeEach already clears CASCADE_PROJECT_ID, etc.) + mockMaterializeFrictionReport.mockResolvedValue({ + status: 'filed', + reportId: 'ignored', + workItemId: 'friction-card-real', + }); + + // Crucially: NO params.project — this is what the production wrapper does + await reportFriction({ + summary: 'Missing config', + details: 'Need better docs.', + category: 'tooling', + severity: 'low', + }); + + const event = JSON.parse(readFileSync(path, 'utf-8').trim().split('\n')[0]); + // The report must carry the real project context, not 'unknown-project' + expect(event.report.context.project).toMatchObject({ + id: 'real-project-id', + name: 'Real Project', + repo: 'owner/real-repo', + pmType: 'trello', + }); + // Materialization must receive the real project so drain can place the card correctly + expect(mockMaterializeFrictionReport).toHaveBeenCalledWith( + expect.objectContaining({ + project: expect.objectContaining({ id: 'real-project-id' }), + }), + ); + rmSync(path, { force: true }); + }); }); diff --git a/tests/unit/triggers/check-suite-failure.test.ts b/tests/unit/triggers/check-suite-failure.test.ts index 457ee1835..69d1cdf10 100644 --- a/tests/unit/triggers/check-suite-failure.test.ts +++ b/tests/unit/triggers/check-suite-failure.test.ts @@ -235,11 +235,17 @@ describe('CheckSuiteFailureTrigger', () => { triggerType: 'check-failure', workItemId: 'abc123', triggerEvent: 'scm:check-suite-failure', + prUrl: 'https://github.com/owner/repo/pull/42', + prTitle: 'Test PR', }, prNumber: 42, prUrl: 'https://github.com/owner/repo/pull/42', prTitle: 'Test PR', workItemId: 'abc123', + workItemUrl: undefined, + workItemTitle: undefined, + onBlocked: undefined, + coalesceKey: undefined, }); }); @@ -553,11 +559,17 @@ describe('CheckSuiteFailureTrigger', () => { triggerType: 'check-failure', workItemId: 'abc123', triggerEvent: 'scm:check-suite-failure', + prUrl: 'https://github.com/owner/repo/pull/42', + prTitle: 'Test PR', }, prNumber: 42, prUrl: 'https://github.com/owner/repo/pull/42', prTitle: 'Test PR', workItemId: 'abc123', + workItemUrl: undefined, + workItemTitle: undefined, + onBlocked: undefined, + coalesceKey: undefined, }); }); diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index 2192c6c1f..2113a83d6 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -216,7 +216,7 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result).toEqual( expect.objectContaining({ agentType: 'review', - agentInput: { + agentInput: expect.objectContaining({ prNumber: 42, prBranch: 'feature/test', repoFullName: 'owner/repo', @@ -224,7 +224,9 @@ describe('CheckSuiteSuccessTrigger', () => { triggerType: 'ci-success', workItemId: 'abc123', triggerEvent: 'scm:check-suite-success', - }, + prUrl: 'https://github.com/owner/repo/pull/42', + prTitle: 'Test PR', + }), prNumber: 42, workItemId: 'abc123', }), diff --git a/tests/unit/triggers/pr-conflict-detected.test.ts b/tests/unit/triggers/pr-conflict-detected.test.ts index 96dbe6e9f..2365ff0c6 100644 --- a/tests/unit/triggers/pr-conflict-detected.test.ts +++ b/tests/unit/triggers/pr-conflict-detected.test.ts @@ -176,11 +176,17 @@ describe('PRConflictDetectedTrigger', () => { triggerType: 'conflict-resolution', workItemId: 'abc123', triggerEvent: 'scm:pr-conflict-detected', + prUrl: 'https://github.com/owner/repo/pull/42', + prTitle: 'Test PR', }, prNumber: 42, prUrl: 'https://github.com/owner/repo/pull/42', prTitle: 'Test PR', workItemId: 'abc123', + workItemUrl: undefined, + workItemTitle: undefined, + onBlocked: undefined, + coalesceKey: undefined, }); }); diff --git a/tests/unit/triggers/shared/agent-work-items.test.ts b/tests/unit/triggers/shared/agent-work-items.test.ts index 40bc2dbe1..9f6b5aee2 100644 --- a/tests/unit/triggers/shared/agent-work-items.test.ts +++ b/tests/unit/triggers/shared/agent-work-items.test.ts @@ -121,7 +121,7 @@ describe('agent-work-items', () => { }); }); - it('leaves agentInput untouched when no work item is resolved', async () => { + it('leaves agentInput untouched when no work item is resolved and no URL/title on result', async () => { const agentInput = { prNumber: 42 }; const result = await prepareAgentWorkItem( { agentType: 'review', agentInput, prNumber: 42 }, @@ -131,6 +131,63 @@ describe('agent-work-items', () => { expect(result.workItemId).toBeUndefined(); expect(result.agentInput).toBe(agentInput); }); + + it('merges top-level prUrl/prTitle into agentInput when handler omits them from agentInput', async () => { + // Simulates PRReviewSubmittedTrigger, PROpenedTrigger, ReviewRequestedTrigger: + // prUrl/prTitle are on TriggerResult top level but NOT in agentInput. + const result = await prepareAgentWorkItem( + { + agentType: 'review', + agentInput: { + prNumber: 42, + prBranch: 'feature/foo', + repoFullName: 'acme/myapp', + headSha: 'sha123', + triggerEvent: 'scm:pr-review-submitted', + workItemId: 'card-1', + }, + prNumber: 42, + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'fix: some issue', + workItemId: 'card-1', + workItemUrl: 'https://trello.com/c/card-1', + workItemTitle: 'Do the thing', + }, + 'project-1', + ); + + expect(result.agentInput).toMatchObject({ + prNumber: 42, + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'fix: some issue', + workItemId: 'card-1', + workItemUrl: 'https://trello.com/c/card-1', + workItemTitle: 'Do the thing', + }); + }); + + it('does not overwrite agentInput fields that are already populated', async () => { + // When a handler (via buildGitHubPRDispatchResult) already set prUrl in agentInput, + // the centralized merge must not overwrite with the top-level value. + const result = await prepareAgentWorkItem( + { + agentType: 'review', + agentInput: { + prNumber: 42, + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'feat: already set', + }, + prNumber: 42, + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'feat: already set', + }, + 'project-1', + ); + + // Should not create a new object reference (no extras added) + expect(result.agentInput.prUrl).toBe('https://github.com/acme/myapp/pull/42'); + expect(result.agentInput.prTitle).toBe('feat: already set'); + }); }); describe('persistPreRunWorkItems', () => { diff --git a/tests/unit/triggers/shared/result-builders.test.ts b/tests/unit/triggers/shared/result-builders.test.ts index 196d4a420..b4210b34b 100644 --- a/tests/unit/triggers/shared/result-builders.test.ts +++ b/tests/unit/triggers/shared/result-builders.test.ts @@ -28,6 +28,8 @@ describe('trigger result builders', () => { workItemId: 'card-123', modelOverride: 'gpt-test', triggerEvent: 'pm:status-changed', + workItemUrl: 'https://trello.com/c/abc', + workItemTitle: 'Implement feature', }, workItemId: 'card-123', workItemUrl: 'https://trello.com/c/abc', @@ -61,6 +63,8 @@ describe('trigger result builders', () => { repoFullName: 'acme/repo', triggerEvent: 'scm:pr-opened', workItemId: 'card-456', + prUrl: 'https://github.com/acme/repo/pull/42', + prTitle: 'feat: add thing', }, prNumber: 42, prUrl: 'https://github.com/acme/repo/pull/42', From 197254297314765a1b46916088ee0b8b35c6be87 Mon Sep 17 00:00:00 2001 From: aaight Date: Sat, 9 May 2026 23:35:16 +0200 Subject: [PATCH 9/9] docs: document friction reporting operations (#1301) Co-authored-by: Cascade Bot --- CHANGELOG.md | 2 ++ CLAUDE.md | 2 ++ docs/architecture/07-gadgets.md | 13 ++++++++++ docs/architecture/08-config-credentials.md | 12 +++++++++ docs/architecture/10-resilience.md | 12 +++++++++ src/gadgets/README.md | 16 ++++++++++++ src/integrations/README.md | 21 ++++++++++++++++ tests/unit/architecture-docs.test.ts | 29 ++++++++++++++++++++++ 8 files changed, 107 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d70ab8a28..57416ad13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable user-visible changes to CASCADE are documented here. The format is l ### Documentation +- **Friction reporting is now documented for operators and provider contributors.** Architecture docs cover the optional PM Friction slot (`lists.friction` for Trello, `statuses.friction` for JIRA/Linear), `ReportFriction`, and `cascade-tools pm report-friction --details-file -`. The integration guide explains that friction reports use existing provider `createWorkItem` plus optional `moveWorkItem`, so providers do not need a new adapter method or a DB-backed friction index. Resilience docs describe the JSONL sidecar/outbox retry path, missing-slot behavior, and non-blocking drain failures. See Trello card [Rvv7VVd5](https://trello.com/c/69ff6af3bc5c526cc5faa2d4). + - **Trigger architecture docs now describe the migrated trigger contracts.** Added guidance for canonical `TRIGGER_EVENTS`, shared PM/GitHub result builders, first-match dispatch, structured skip vs bare `null`, no-agent results, deferred bare-job re-checks, router outcome decision reasons, PM coalescing, capacity scope, dispatch failure compensation, and wedged-lock diagnostics. Migration note for future trigger contributors: new handlers should import event constants, use the shared builders, return structured skips for claimed-but-non-dispatched events, and reserve bare `null` for "continue to later handlers." See Trello card [qUbPtALY](https://trello.com/c/69fe2a950699baaf91688a5b). ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index aeab911e6..6134725f0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,6 +127,8 @@ Some triggers take params (e.g. `review` + `scm:check-suite-success` accepts `{" **Worker exit diagnostics** — when a worker container exits non-zero, the router calls `container.inspect()` *before* AutoRemove reaps it and stamps the run record's `error` field with a structured, grep-stable string: `Worker crashed with exit code N · OOMKilled= · reason=""`. The `OOMKilled=true` marker is the definitive cgroup-OOM signal (per Docker's own `State.OOMKilled`); a 137 exit *without* `OOMKilled=true` means the kill came from inside the container or from a non-cgroup signal — *not* memory. The `[WorkerManager] Resolved spawn settings` log emitted at every spawn includes both `projectWatchdogTimeoutMs` and `globalWorkerTimeoutMs` so post-mortems can confirm whether the per-project override actually won. See `src/router/active-workers.ts:formatCrashReason` for the format and `tests/unit/router/container-manager-diagnostics.test.ts` for regression pins. +**Friction reporting** — agents with `pm:friction` can call `cascade-tools pm report-friction` / `ReportFriction` for incidental tooling, environment, permission, dependency, test, PM-data, or SCM-data papercuts. Configure the optional Friction slot in the PM wizard's Status Mapping step: Trello uses `lists.friction`; JIRA and Linear use `statuses.friction`. The feature does not add provider adapter methods or a DB-backed friction index — it materializes a normal PM work item through existing `createWorkItem` plus optional `moveWorkItem`. Reports are first written to `CASCADE_FRICTION_SIDECAR_PATH` as a JSONL outbox, then filed immediately when possible; backend drain retries pending reports after the engine returns, including ordinary failures. Missing friction slot returns a non-fatal `friction_slot_missing`/`queued_slot_missing` result, and drain failures log/capture Sentry under `friction_sidecar_drain_failed` without failing an otherwise successful run. + **Dispatch failure semantics** — spec 015 (verified live in prod via the ucho/MNG-350 incident on 2026-04-26): - **Capacity miss waits, never throws.** When the dispatcher pulls a job and the worker pool is at `maxWorkers`, it `await`s a slot via the in-process slot-waiter (default `slotWaitTimeoutMs` = 5min). The slot is conceptually held by the running container — `slotReleased()` is called once per cleanup from `cleanupWorker`, never from the dispatcher. diff --git a/docs/architecture/07-gadgets.md b/docs/architecture/07-gadgets.md index 91d44ab36..21dedd7ad 100644 --- a/docs/architecture/07-gadgets.md +++ b/docs/architecture/07-gadgets.md @@ -71,6 +71,19 @@ Todos are stored in `.claude/todos.json` within the repo working directory. PM gadgets use the active `PMProvider` from `AsyncLocalStorage` context, making them provider-agnostic. +`ReportFriction` is intentionally narrower than general PM write access. It lets agents file incidental papercuts in tooling, environment, permissions, dependencies, tests, PM data, or SCM data without exposing `CreateWorkItem` / `MoveWorkItem` directly. The CLI form is: + +```bash +cascade-tools pm report-friction \ + --summary "Typecheck requires undocumented Redis env var" \ + --category environment \ + --severity medium \ + --whileDoing "Running pre-PR verification" \ + --details-file - +``` + +`--details-file -` reads Markdown details from stdin; use it for multi-line reproduction notes or shell output. The command always appends a queued event to the friction sidecar before it tries to create the PM work item, so a failed immediate write can be retried by the backend drain. + ### SCM (`scm:read`, `scm:ci-logs`, `scm:comment`, `scm:review`, `scm:pr`) | Gadget | Capability | Purpose | diff --git a/docs/architecture/08-config-credentials.md b/docs/architecture/08-config-credentials.md index 54b456f0e..e3a7923be 100644 --- a/docs/architecture/08-config-credentials.md +++ b/docs/architecture/08-config-credentials.md @@ -55,6 +55,18 @@ interface ProjectConfig { } ``` +### PM workflow slots + +PM provider config maps CASCADE lifecycle concepts onto provider-native lists or statuses. The friction-reporting slot is optional but recognized consistently across providers: + +| Provider | Config key | Meaning | +|---|---|---| +| Trello | `lists.friction` | Trello list ID where friction report cards are created and left | +| JIRA | `statuses.friction` | JIRA status name/ID applied after the issue is created in `projectKey` | +| Linear | `statuses.friction` | Linear workflow state UUID applied after the issue is created in `teamId` | + +If the slot is not configured, `ReportFriction` records the report in the sidecar and returns a non-fatal `queued_slot_missing` result with operator guidance. No run should fail solely because the friction slot is missing. + `maxInFlightItems` is enforced at two points: (a) the `backlog-manager` chain gates (won't auto-pull from BACKLOG when at capacity) and (b) the PM `status-changed` triggers (won't fire `implementation` when a card is moved diff --git a/docs/architecture/10-resilience.md b/docs/architecture/10-resilience.md index 04aa2dd7a..44824bec9 100644 --- a/docs/architecture/10-resilience.md +++ b/docs/architecture/10-resilience.md @@ -67,6 +67,18 @@ Rate limits are enforced by the LLMist SDK for `sdk`-archetype engines. Native-t ## Retry Strategy +### Friction report outbox + +`ReportFriction` uses a JSONL sidecar as a small outbox so incidental agent issues do not block the main run. The gadget appends a queued event to `CASCADE_FRICTION_SIDECAR_PATH` before attempting PM materialization. Native-tool engines receive that path in their environment; in-process engines get the same value through session state. + +On successful immediate materialization, the gadget appends a filed event with the PM work item ID/URL. If the immediate PM write fails, the gadget returns `queued_for_retry` and the agent should keep working unless the underlying issue is a real blocker. + +The backend adapter drains pending sidecar events after the engine returns, including ordinary engine failures. Drain behavior is deliberately non-blocking: + +- A missing `lists.friction` / `statuses.friction` slot produces a skipped report with reason `friction_slot_missing`; operators should configure the Friction row in the PM wizard's Status Mapping step. +- A PM API failure during drain logs a warning and captures Sentry with `source=friction_sidecar_drain_failed`, but it does not change a successful run into a failed run. +- After drain, the sidecar is compacted/cleaned so filed reports are not retried indefinitely. + ### Dispatch retries The router queues `cascade-jobs` and `cascade-dashboard-jobs` with `attempts: 4` and exponential backoff. Dispatch errors before a worker container starts are classified in `src/router/dispatch-error-classifier.ts`: diff --git a/src/gadgets/README.md b/src/gadgets/README.md index e10803b95..d441540b4 100644 --- a/src/gadgets/README.md +++ b/src/gadgets/README.md @@ -159,3 +159,19 @@ Two tuning constants live in `src/gadgets/shared/cli/parseErrors.ts`: `MAX_FLAG_ ## Reference: `createPRReviewDef` `src/gadgets/github/definitions.ts` is the reference gadget for spec 014 — it carries `cliAliases: ['comment']` on `comments`, a `fileInputAlternatives` entry for `--comments-file`, and a well-formed `examples` block. Read it before you add a new gadget with array-of-object parameters. + +## Reference: `ReportFriction` + +`ReportFriction` is the PM-scoped reference for a non-blocking sidecar/outbox command. Its definition lives in `src/gadgets/pm/definitions.ts`; the implementation lives in `src/gadgets/pm/core/reportFriction.ts`. + +Agents invoke it through: + +```bash +cascade-tools pm report-friction \ + --summary "Missing setup hint" \ + --category tooling \ + --severity medium \ + --details-file - +``` + +`--details-file -` reads Markdown details from stdin. The command writes a queued event to `CASCADE_FRICTION_SIDECAR_PATH` before attempting PM materialization, then returns `filed`, `queued_for_retry`, or `queued_slot_missing`. This is why the feature can report friction without granting broad PM write access and without failing the main run when PM filing is unavailable. diff --git a/src/integrations/README.md b/src/integrations/README.md index 4e8ce5840..4b6aef3fd 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -240,6 +240,27 @@ SCM (GitHub) and alerting (Sentry) integrations retain the legacy `IntegrationMo --- +## Friction report materialization + +Friction reports are PM-backed work items filed by the `ReportFriction` gadget. Providers do **not** add a new adapter method for this feature. The shared materializer at `src/friction/materialize.ts` uses the existing `PMProvider` CRUD surface: + +1. Resolve placement from project config with `getFrictionContainerId(project)`. +2. Call `provider.createWorkItem({ containerId, title, description, labels: [] })`. +3. Resolve an optional destination with `getFrictionStatusDestination(project)`. +4. Call `provider.moveWorkItem(workItem.id, destination)` when a destination exists. + +That means a new provider only needs correct implementations of the existing `createWorkItem` and `moveWorkItem` methods plus the normal config schema fields: + +| Provider shape | Friction slot | +|---|---| +| Trello list-based config | `lists.friction` | +| JIRA status-based config | `statuses.friction` | +| Linear status-based config | `statuses.friction` | + +If the slot is missing, the materializer returns a skipped result with reason `friction_slot_missing` instead of throwing. The sidecar/outbox layer can then keep the agent run non-blocking while operators fix configuration. + +--- + ## Checklist implementation by provider Different PM providers have different native concepts of "checklist". The `PMProvider` interface exposes a uniform API (`getChecklists`, `createChecklist`, `addChecklistItem`, `updateChecklistItem`, `deleteChecklistItem`), but adapters implement them differently: diff --git a/tests/unit/architecture-docs.test.ts b/tests/unit/architecture-docs.test.ts index 15bfd70b2..0d882b9aa 100644 --- a/tests/unit/architecture-docs.test.ts +++ b/tests/unit/architecture-docs.test.ts @@ -238,5 +238,34 @@ describe('Architecture documentation', () => { const claude = readDoc(path.join(REPO_ROOT, 'CLAUDE.md')); expect(agents).toBe(claude); }); + + it('documents friction reporting operator and provider contracts', () => { + const requiredFacts = [ + 'lists.friction', + 'statuses.friction', + 'ReportFriction', + 'cascade-tools pm report-friction', + '--details-file -', + 'createWorkItem', + 'moveWorkItem', + 'friction_slot_missing', + 'friction_sidecar_drain_failed', + ]; + const docs = [ + path.join(ARCH_DIR, '07-gadgets.md'), + path.join(ARCH_DIR, '08-config-credentials.md'), + path.join(ARCH_DIR, '10-resilience.md'), + path.join(REPO_ROOT, 'src/integrations/README.md'), + path.join(REPO_ROOT, 'src/gadgets/README.md'), + path.join(REPO_ROOT, 'CLAUDE.md'), + path.join(REPO_ROOT, 'AGENTS.md'), + path.join(REPO_ROOT, 'CHANGELOG.md'), + ]; + const combined = docs.map(readDoc).join('\n'); + + for (const fact of requiredFacts) { + expect(combined, `friction docs should mention ${fact}`).toContain(fact); + } + }); }); });