diff --git a/CHANGELOG.md b/CHANGELOG.md index 242b44cb8..1ef8181c3 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 ### Added +- **Sentry alerts now materialize as real PM work items in the configured `alerts` slot** (spec 019). The alerting trigger previously minted a synthetic `sentry:issue:` workItemId, which caused a Trello 400 error on the budget gate for projects with a cost custom field, silently killing every alerting run. The trigger now calls `materializeAlertWorkItem('sentry', issueId, project, hints)`, which creates (or idempotently retrieves) a Trello card / JIRA issue / Linear issue in the PM `alerts` slot and returns its native ID. Budget tracking, lifecycle transitions, and label writes all work correctly on the resulting card. A partial UNIQUE index on `(project_id, external_source, external_id)` in `work_items` ensures a second Sentry alert on the same issue produces the same PM card, not a duplicate. Configure the `alerts` slot in the PM wizard's **Status Mapping** step; validation pre-flight emits a `pm`-category error when the slot is unset and an alerting trigger is enabled. See [spec 019](docs/specs/019-sentry-alert-pm-materialization.md). + - **Alerting agent now investigates Sentry alerts and files bug investigation work items** (spec 018, plan 1 of 2). The `alerting` agent had been wired end-to-end except for its system prompt template — definition YAML, capabilities, trigger handlers, context pipeline, and Sentry integration were all in place, but `src/agents/prompts/templates/alerting.eta` was missing, so the worker crashed at agent boot with `ENOENT` when the first prod-traffic Sentry alert arrived (cascade project, 2026-05-06). This plan ships the prompt: a three-phase investigator (parse pre-loaded event → confirm root cause via source reads → file or comment) with an explicit `INVESTIGATE-AND-FILE-ONLY` guardrail. The agent does not edit source, commit, push, or open PRs — that property is enforced at the capability layer (no `fs:write`, no `scm:*`), pinned by a static test that asserts the resolved gadget allowlist excludes `WriteFile`, `CreatePR`, and `CreatePRReview`. When the trigger context provides an existing work item, the agent comments on it; otherwise it creates a new bug investigation work item in the configured backlog. Output structure is predictable: `Investigate: in (:)` title and a 4-6 sentence + bullets description. Engine-agnostic prose; reuses `partials/environment` for the shared preamble. See [spec 018](docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done). Plan 2 of 2 closes the silent-failure path that masked this gap (worker boot failures will produce visible failed run rows, exit code 2, Sentry capture under `worker_boot_failure`). ### Changed diff --git a/docs/plans/019-sentry-alert-pm-materialization/1-schema-and-pm-config.md.done b/docs/plans/019-sentry-alert-pm-materialization/1-schema-and-pm-config.md.done new file mode 100644 index 000000000..adf3bbe2e --- /dev/null +++ b/docs/plans/019-sentry-alert-pm-materialization/1-schema-and-pm-config.md.done @@ -0,0 +1,196 @@ +--- +id: 019 +slug: sentry-alert-pm-materialization +plan: 1 +plan_slug: schema-and-pm-config +level: plan +parent_spec: docs/specs/019-sentry-alert-pm-materialization.md +depends_on: [] +status: done +--- + +# 019/1: Schema migration + PM-config slot recognition (foundation) + +> Part 1 of 4 in the 019-sentry-alert-pm-materialization plan. See [parent spec](../../specs/019-sentry-alert-pm-materialization.md). + +## Summary + +Land the dormant DB and config foundations for the alert-materialization feature. Two parallel pieces: + +1. A **DB migration** adds `external_source` + `external_id` columns to `pr_work_items` plus a partial UNIQUE index on `(project_id, external_source, external_id) WHERE external_source IS NOT NULL`. The columns are nullable; existing rows default to NULL; legacy reads/writes are unaffected. The index is the race-free idempotency primitive plan 2 will rely on. +2. **PM-config recognition of the `alerts` slot** for all three providers. Trello/JIRA/Linear already store `lists` / `statuses` as open `Record`, so `lists.alerts` (Trello) / `statuses.alerts` (JIRA, Linear) are valid today. This plan adds: + - A typed `cascadeAlert?: string` slot in JIRA's and Linear's `labels` object (Trello's `labels` is already open). The materializer in plan 2 reads from this slot to apply the alert label. + - JSDoc + Zod `.describe` annotations identifying `alerts` as a recognized status-key and `cascadeAlert` as a recognized label-key. + - A small typed accessor `getAlertsContainerId(project)` and `getAlertLabelId(project)` in `src/pm/config.ts` for the materializer to consume. + +This plan ships **dormant** code: the migration + accessors are unused until plan 2 wires them in. Reviewers can validate the migration semantics and the accessor contract independently of the materialization logic. + +**Components delivered:** +- New migration file: `src/db/migrations/0051_pr_work_items_external_source.sql` +- Updated `src/db/migrations/meta/_journal.json` (idx 51) +- Updated `src/db/schema/prWorkItems.ts` (two new nullable text columns + a JSDoc note about the partial unique index) +- Updated `src/integrations/pm/{trello,jira,linear}/config-schema.ts` (Zod `.describe` annotations + JIRA/Linear `cascadeAlert` label slot) +- Updated `src/pm/config.ts` (`JiraConfig` / `LinearConfig` `cascadeAlert?: string` field + new `getAlertsContainerId` and `getAlertLabelId` accessors) +- Tests: schema-round-trip pin per provider; accessor unit tests; migration smoke test + +**Deferred to later plans in this spec:** +- The materializer that uses these accessors (plan 2) +- Wiring the Sentry trigger to call the materializer (plan 3) +- Wizard UI for configuring `alerts` and `cascadeAlert` (plan 4) +- `validateIntegrations` requirement that `alerts` is set when alerting trigger is enabled (plan 3) + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #2 (concurrent dedupe → one card) — **partial**: this plan provides the partial UNIQUE index that enforces it; plan 2 provides the `INSERT … ON CONFLICT DO NOTHING RETURNING` query that uses it. +- Spec AC #3 (per-project mapping) — **partial**: the UNIQUE includes `project_id`; plan 2 wires the lookup. +- (Plan 1 carries no full-coverage spec ACs by itself — it ships dormant. This is expected and called out in the proposal.) + +--- + +## Depends On + +- (none — foundation plan) + +--- + +## Detailed Task List (TDD) + +### 1. DB migration: add `external_source` + `external_id` to `pr_work_items` with partial UNIQUE + +**Tests first** (`tests/integration/db/pr-work-items-external-source.test.ts`): + +- `pr_work_items.external_source and external_id columns exist and accept NULL` — integration — set up the test DB with migrations applied; query `information_schema.columns`. Expected red: `expected to find 2 columns ('external_source', 'external_id') in pr_work_items, found 0`. +- `partial UNIQUE index uq_pr_work_items_project_external blocks a duplicate (project_id, external_source, external_id) insert` — integration — insert one row with `(project_id='p1', external_source='sentry', external_id='S1')`, then attempt a second identical insert; expect a Postgres error with code `23505` (unique_violation). Expected red: `expected unique_violation (23505), got: rows inserted successfully`. +- `partial UNIQUE index allows multiple rows where external_source IS NULL` — integration — insert three rows with `external_source=NULL` for the same `project_id`; all three succeed. Expected red: `expected 3 rows, got: insert failed with unique_violation`. (Confirms the index is partial.) +- `partial UNIQUE index allows the same external_id across different projects` — integration — insert `(project_id='p1', external_source='sentry', external_id='S1')` and `(project_id='p2', external_source='sentry', external_id='S1')`; both succeed. Expected red: `expected 2 rows, got: unique_violation on second insert`. +- `partial UNIQUE index allows the same external_id across different sources within a project` — integration — insert `(p1, sentry, S1)` and `(p1, pagerduty, S1)`; both succeed. Expected red: `unique_violation on second insert`. + +**Implementation** (`src/db/migrations/0051_pr_work_items_external_source.sql`): +```sql +ALTER TABLE pr_work_items ADD COLUMN external_source text; +ALTER TABLE pr_work_items ADD COLUMN external_id text; + +CREATE UNIQUE INDEX IF NOT EXISTS uq_pr_work_items_project_external + ON pr_work_items (project_id, external_source, external_id) + WHERE external_source IS NOT NULL; +``` + +`src/db/migrations/meta/_journal.json` — append entry `{ idx: 51, version: '7', when: , tag: '0051_pr_work_items_external_source', breakpoints: false }`. + +`src/db/schema/prWorkItems.ts` — add two nullable text columns and a JSDoc note about the partial unique index (drizzle does not natively support partial unique indexes; the migration enforces it directly, mirroring the pattern at lines 22–25). + +### 2. Per-provider config-schema annotations + JIRA/Linear `cascadeAlert` label slot + +**Tests first** (`tests/unit/pm/config-alert-slot.test.ts`): + +- `trelloConfigSchema accepts lists.alerts and labels.cascade-alert without error` — unit — parse `{ boardId: 'b', lists: { alerts: 'list-id' }, labels: { 'cascade-alert': 'lbl-id' } }`. Expected red: `expected schema.parse to succeed, got ZodError`. +- `jiraConfigSchema accepts statuses.alerts and labels.cascadeAlert` — unit — parse `{ projectKey: 'P', baseUrl: 'https://x', statuses: { alerts: 'In Triage' }, labels: { cascadeAlert: 'cascade-alert' } }`. Expected red: `ZodError: unrecognized key cascadeAlert`. +- `linearConfigSchema accepts statuses.alerts and labels.cascadeAlert` — unit — analogous. Expected red: same ZodError. +- `JiraConfig.labels has optional cascadeAlert field` — unit (typecheck via `expectTypeOf().toEqualTypeOf<{ ... cascadeAlert?: string }>()`). Expected red: typecheck failure: missing `cascadeAlert` property. +- `LinearConfig.labels has optional cascadeAlert field` — unit (typecheck). Expected red: same. + +**Implementation:** + +- `src/integrations/pm/trello/config-schema.ts` — add `.describe` text on `lists` field calling out `alerts` as a recognized key. No type change (lists is already an open record). +- `src/integrations/pm/jira/config-schema.ts` — extend the `labels` object to include `cascadeAlert: z.string().optional()`; add `.describe` on `statuses` calling out `alerts` as a recognized key. +- `src/integrations/pm/linear/config-schema.ts` — same pattern as JIRA: add `cascadeAlert` to `labels`; document `alerts` on `statuses`. +- `src/pm/config.ts` — add `cascadeAlert?: string` to `JiraConfig['labels']` and `LinearConfig['labels']`. Trello's `labels` is `Record` and needs no type change; the materializer reads `labels['cascade-alert']` for Trello. + +### 3. Typed accessors for the alerts container and label + +**Tests first** (`tests/unit/pm/config-alert-accessors.test.ts`): + +- `getAlertsContainerId returns Trello list ID from project.trello.lists.alerts` — unit — fixture project with `pm.type='trello'` and `trello.lists.alerts='list-123'`. Expected red: `getAlertsContainerId is not a function` / `expected 'list-123', got undefined`. +- `getAlertsContainerId returns JIRA project key from statuses.alerts` (note: JIRA's container is the project key from `projectKey`, NOT a status; for the materializer this means we return `projectKey` as the createWorkItem container, while `statuses.alerts` is what the lifecycle/move uses to put the issue in the alerts state). Document the asymmetry in the accessor's JSDoc; return `projectKey` for JIRA. Expected red: same. +- `getAlertsContainerId returns Linear team ID for Linear projects` — unit — `pm.type='linear'`, `linear.teamId='team-1'`. The Linear materializer creates an issue in the team and moves it to `statuses.alerts`. Expected red: same. +- `getAlertsContainerId returns undefined when no PM config is present` — unit. Expected red: `expected undefined, got `. +- `getAlertLabelId returns Trello label ID from labels['cascade-alert']` — unit. Expected red: function not found. +- `getAlertLabelId returns JIRA label string from labels.cascadeAlert` — unit. +- `getAlertLabelId returns Linear label ID from labels.cascadeAlert` — unit. +- `getAlertLabelId returns undefined when label slot is not configured` — unit (covers all three PM types). +- `getAlertsStatusKey returns 'alerts' literal when statuses.alerts is configured` — unit. (Used by plan 3's lifecycle move.) +- `getAlertsStatusKey returns undefined when statuses.alerts is not configured` — unit (Trello: lists.alerts; JIRA: statuses.alerts; Linear: statuses.alerts). + +**Implementation** (`src/pm/config.ts`): +- `getAlertsContainerId(project: ProjectConfig): string | undefined` — returns the value the PM adapter's `createWorkItem.containerId` expects: Trello → `lists.alerts`, JIRA → `projectKey`, Linear → `teamId`. JSDoc explains why JIRA/Linear deviate from "look up alerts in lists/statuses" (their createWorkItem container is project/team-scoped, not status-scoped; the alerts state is applied via a follow-up move, see plan 2). +- `getAlertLabelId(project: ProjectConfig): string | undefined` — returns Trello `labels['cascade-alert']` or JIRA/Linear `labels.cascadeAlert`. +- `getAlertsStatusKey(project: ProjectConfig): 'alerts' | undefined` — returns the literal `'alerts'` if the project's PM config has the alerts slot populated (in `lists.alerts` for Trello, `statuses.alerts` for JIRA/Linear); otherwise undefined. Plan 2's materializer feeds this into the existing `lifecycle.moveTo(statusKey)` machinery. + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/pm/config-alert-slot.test.ts`: 5 tests (Zod round-trip + typecheck pins per provider) +- [ ] `tests/unit/pm/config-alert-accessors.test.ts`: ~10 tests (the three accessors across three PM types + missing-config branches) + +### Integration tests +- [ ] `tests/integration/db/pr-work-items-external-source.test.ts`: 5 tests covering column existence, partial UNIQUE enforcement (positive + negative + cross-project + cross-source + null-allowed branches) + +### Acceptance tests +- [ ] (none for this plan — dormant code; ACs roll up into plans 2/3/4) + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +n/a — all ACs auto-tested. + +--- + +## Acceptance Criteria (per-plan, testable) + +1. `npm run db:migrate` applies `0051_pr_work_items_external_source.sql` cleanly against an integration DB seeded by prior migrations; the two columns and the partial unique index are present. +2. The partial UNIQUE index blocks a duplicate insert with the same `(project_id, external_source, external_id)` and allows distinct rows across projects, distinct sources within a project, and any number of `external_source IS NULL` rows. +3. All three PM config schemas parse the new keys (`lists.alerts` / `statuses.alerts`, `labels.cascade-alert` / `labels.cascadeAlert`) without error and reject unrelated unknown keys consistent with their existing strictness. +4. `getAlertsContainerId`, `getAlertLabelId`, and `getAlertsStatusKey` return the documented values for each PM type and `undefined` when unconfigured. +5. **Partial-state criterion**: the new columns exist with NULL defaults; no application code reads or writes them yet. Running the existing test suite is unaffected. +6. All new/modified code has corresponding tests. +7. `npm run build` passes. +8. `npm test` (all 4 unit projects) passes. +9. `npm run test:integration` passes (covers the migration test). +10. `npm run lint` and `npm run typecheck` pass. +11. All documentation listed in Documentation Impact has been updated. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `src/integrations/pm/trello/config-schema.ts` | JSDoc + Zod `.describe` annotations identifying `lists.alerts` and `labels['cascade-alert']` as recognized keys | +| `src/integrations/pm/jira/config-schema.ts` | Same + adds `cascadeAlert` to typed `labels` block | +| `src/integrations/pm/linear/config-schema.ts` | Same pattern as JIRA | +| `src/pm/config.ts` | JSDoc on the three new accessors explaining the per-PM container asymmetry | + +(Top-level docs — README, spec backlink — land in plan 4.) + +--- + +## Out of Scope (this plan) + +- The materializer entrypoint that uses the new schema columns and accessors (plan 2). +- Wiring the Sentry alerting trigger handler to call the materializer (plan 3). +- `validateIntegrations` requiring `alerts` to be set when the alerting trigger is enabled (plan 3). +- Wizard UI for configuring the `alerts` slot and `cascade-alert` label (plan 4). +- `src/integrations/README.md` updates (plan 4). +- All items originally out of scope for the spec (auto-close on resolution, bidirectional sync, non-Sentry alert sources, DB-loss DR breadcrumb, etc.). + +--- + +## Progress + + +- [x] AC #1 +- [x] AC #2 +- [x] AC #3 +- [x] AC #4 +- [x] AC #5 +- [x] AC #6 +- [x] AC #7 +- [x] AC #8 +- [x] AC #9 +- [x] AC #10 +- [x] AC #11 diff --git a/docs/plans/019-sentry-alert-pm-materialization/2-materializer-core.md.done b/docs/plans/019-sentry-alert-pm-materialization/2-materializer-core.md.done new file mode 100644 index 000000000..a492eca23 --- /dev/null +++ b/docs/plans/019-sentry-alert-pm-materialization/2-materializer-core.md.done @@ -0,0 +1,215 @@ +--- +id: 019 +slug: sentry-alert-pm-materialization +plan: 2 +plan_slug: materializer-core +level: plan +parent_spec: docs/specs/019-sentry-alert-pm-materialization.md +depends_on: [1-schema-and-pm-config.md] +status: done +--- + +# 019/2: Generic alert→PM materializer (race-free, lazy-heal) + +> Part 2 of 4 in the 019-sentry-alert-pm-materialization plan. See [parent spec](../../specs/019-sentry-alert-pm-materialization.md). + +## Summary + +Build the generic, source-agnostic materializer that turns an `(source, externalId, project, hints)` tuple into a real PM work item id. The implementation: + +1. **Resolves the existing mapping**, if any, via `pr_work_items.external_source = $1 AND external_id = $2 AND project_id = $3`. If found, verifies the PM card still exists; on PM 404, lazy-heals by creating a fresh card and atomically updating the row in place. +2. **Atomically gets-or-creates** a new mapping using Postgres `INSERT … ON CONFLICT (project_id, external_source, external_id) WHERE external_source IS NOT NULL DO NOTHING RETURNING id, work_item_id`. When the conflict path returns no row (concurrent insert won), re-selects to obtain the winner's `work_item_id`. This is the race-free primitive — no application-level lock. +3. **Calls the project's PM provider** (`createWorkItem` against the `alerts` container with the formatted title, description, and optional alert label) and writes the resulting native id back into the row. +4. **Posts a follow-up "move to alerts state"** through the existing `PMLifecycleManager` machinery if the provider's `createWorkItem` does not support an initial state argument (Linear: state set on create; Trello: list set on create; JIRA: status applied via transition after create). +5. **Propagates PM errors** untouched — the trigger handler in plan 3 catches them and returns `null` so BullMQ retry semantics apply. + +This plan ships a unit-testable seam with no caller wired in yet. Plan 3 wires the Sentry trigger to it. + +**Components delivered:** +- New module: `src/integrations/alerting/_shared/materialize.ts` — exports `materializeAlertWorkItem(source, externalId, project, hints): Promise`. +- New module: `src/integrations/alerting/_shared/types.ts` — exports the `AlertSource` union, `AlertHints` shape (title, descriptionMarkdown, labelKey). +- New module: `src/integrations/alerting/_shared/format.ts` — exports `formatSentryCardBody(payload): { title, descriptionMarkdown }` (the only Sentry-specific helper this plan ships; consumed in plan 3). Trivially extensible to PagerDuty/Datadog later via separate format helpers. +- New repository methods on `src/db/repositories/prWorkItemsRepository.ts`: + - `findByExternal(projectId, source, externalId): Promise<{ id, workItemId } | null>` + - `claimExternalMapping(projectId, source, externalId): Promise<{ ownedHere: true } | { ownedHere: false, existing: { id, workItemId } }>` — does the ON-CONFLICT-DO-NOTHING insert with a follow-up SELECT. + - `attachWorkItemId(rowId, workItemId): Promise` — writes the native id back into the claimed row. + - `replaceWorkItemId(rowId, oldWorkItemId, newWorkItemId): Promise` — used by the lazy-heal path; logs the orphan transition. +- Tests: unit tests for the materializer with a fake PM provider (concurrency simulation, lazy-heal, terminal error propagation, hints translation); integration tests against a real Postgres for the repository methods. + +**Deferred to later plans in this spec:** +- The Sentry trigger calling the materializer (plan 3). +- `validateIntegrations` requiring the `alerts` slot when alerting is enabled (plan 3). +- Coalescing wiring on the post-materialization workItemId (plan 3 — the materializer itself is one-shot; the coalesce window applies at job-dispatch time). +- Wizard / dashboard / docs (plan 4). + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #2 (concurrent dedupe → 1 card) — **full** at the data layer; plan 3 wires the call site. +- Spec AC #3 (per-project mapping) — **full**. +- Spec AC #4 (orphan lazy-heal + log) — **full**. +- Spec AC #9 (card title/description/label format) — **full** for the formatting helper; plan 3 ensures the trigger passes the right `hints`. +- Spec AC #10 (new alert source pluggability) — **full** at the interface level; the materializer accepts any `source` literal, and `format.ts` shows the per-source formatter pattern. + +--- + +## Depends On + +- Plan 1 (`1-schema-and-pm-config.md`) — provides the `external_source` + `external_id` columns, the partial UNIQUE index, and the `getAlertsContainerId` / `getAlertLabelId` / `getAlertsStatusKey` accessors. + +--- + +## Detailed Task List (TDD) + +### 1. Repository: `findByExternal`, `claimExternalMapping`, `attachWorkItemId`, `replaceWorkItemId` + +**Tests first** (`tests/integration/db/pr-work-items-external-mapping.test.ts`): + +- `findByExternal returns null when no row matches` — integration. Expected red: `findByExternal is not a function` / `expected null, got `. +- `findByExternal returns { id, workItemId } when an exact match exists` — integration — seed a row with `(project='p', source='sentry', external_id='S1', work_item_id='trello-card-1')`; expect `{ id, workItemId: 'trello-card-1' }`. Expected red: `expected { id, workItemId: 'trello-card-1' }, got null`. +- `claimExternalMapping inserts a new row when none exists, returns ownedHere=true` — integration — start with empty table; call with `(p, sentry, S1)`; verify the row is inserted with `work_item_id=NULL`. Expected red: `claimExternalMapping is not a function` / `expected ownedHere=true, got false`. +- `claimExternalMapping returns ownedHere=false with existing row when conflict` — integration — seed a row first; call again; verify only one row exists in the DB and the returned `existing.workItemId` matches the seed. Expected red: `expected ownedHere=false with existing { id, workItemId }, got ownedHere=true`. +- `claimExternalMapping is race-free under simulated concurrency` — integration — fire 5 parallel `Promise.all` calls; verify exactly one returns `ownedHere=true` and the other four return `ownedHere=false` pointing at the same row id, and that `SELECT count(*) FROM pr_work_items WHERE ... = 1`. Expected red: `expected exactly 1 ownedHere=true, got >1` (or `expected 1 row, got N`). +- `attachWorkItemId writes work_item_id into the claimed row` — integration. Expected red: `attachWorkItemId is not a function` / `expected work_item_id='card-1', got NULL`. +- `replaceWorkItemId atomically updates work_item_id only when old value matches` — integration — seed `work_item_id='card-old'`; call `replaceWorkItemId(rowId, 'card-old', 'card-new')`; expect success + row now has `work_item_id='card-new'`. Then call `replaceWorkItemId(rowId, 'card-old', 'card-newer')` again with a stale old value; expect a no-op (returns false) and row stays at `card-new`. Expected red: `expected stale-old to be rejected, got row updated`. + +**Implementation** (`src/db/repositories/prWorkItemsRepository.ts`): +- `findByExternal(projectId, source, externalId)` — single SELECT with `WHERE project_id = $1 AND external_source = $2 AND external_id = $3 LIMIT 1`. +- `claimExternalMapping(projectId, source, externalId)` — `INSERT INTO pr_work_items (project_id, external_source, external_id) VALUES (...) ON CONFLICT (project_id, external_source, external_id) WHERE external_source IS NOT NULL DO NOTHING RETURNING id, work_item_id`. If `RETURNING` produces a row → `{ ownedHere: true }`. If empty → follow-up SELECT to fetch the winning row → `{ ownedHere: false, existing: { id, workItemId } }`. +- `attachWorkItemId(rowId, workItemId)` — `UPDATE pr_work_items SET work_item_id = $2, updated_at = now() WHERE id = $1`. (No optimistic-concurrency check needed — only the claimer reaches here.) +- `replaceWorkItemId(rowId, oldWorkItemId, newWorkItemId)` — `UPDATE … SET work_item_id = $3, updated_at = now() WHERE id = $1 AND work_item_id = $2 RETURNING id`. Returns `true` when one row was updated, `false` when zero (stale or already replaced). + +### 2. The materializer entrypoint + +**Tests first** (`tests/unit/integrations/alerting/materialize.test.ts`): + +Use a fake PM provider that records `createWorkItem` calls and a fake repository that simulates the four DB methods. + +- `materializeAlertWorkItem returns existing native id when a healthy mapping is present` — unit — repo returns `{ id, workItemId: 'card-1' }`; PM `getWorkItem('card-1')` resolves; expect return value `'card-1'`; expect `createWorkItem` was NOT called. Expected red: `materializeAlertWorkItem is not a function` / `expected createWorkItem not called, got 1 call`. +- `materializeAlertWorkItem creates and attaches when no mapping exists` — unit — repo returns `null` then `{ ownedHere: true, rowId }`; PM `createWorkItem` returns `{ id: 'card-new' }`; expect repo `attachWorkItemId(rowId, 'card-new')` called; expect return value `'card-new'`. Expected red: `expected createWorkItem to be called with containerId from getAlertsContainerId, got `. +- `materializeAlertWorkItem awaits the winning concurrent claim's result when ownedHere=false` — unit — repo returns `null` from `findByExternal`, then `{ ownedHere: false, existing: { workItemId: 'card-winner' } }`; expect return value `'card-winner'`; expect `createWorkItem` was NOT called. Expected red: `expected return 'card-winner', got `. +- `materializeAlertWorkItem awaits a concurrent winner whose row has work_item_id NOT YET set, then re-reads` — unit — repo returns `{ ownedHere: false, existing: { workItemId: null } }` once, then on a short retry returns `{ workItemId: 'card-winner' }`; expect a bounded retry with backoff (e.g. 50ms × 5 attempts) and a final return of `'card-winner'`. After the budget is exhausted, throw a structured `MaterializationRetryExhausted` error. Expected red: `expected bounded retry with eventual success, got immediate failure or infinite hang`. +- `materializeAlertWorkItem lazy-heals on PM 404 by creating a new card and replacing the work_item_id` — unit — repo returns `{ workItemId: 'card-stale' }`; PM `getWorkItem('card-stale')` rejects with a 404; expect `createWorkItem` is called; expect repo `replaceWorkItemId(rowId, 'card-stale', )` called; expect return value ``; expect a log line at WARN with the literal prefix `[alert-materializer] orphan card detected` containing the prior id. Expected red: `expected lazy-heal flow, got error propagated` / `expected log line not found`. +- `materializeAlertWorkItem propagates terminal PM errors untouched (auth/validation/5xx)` — unit — fake PM `createWorkItem` rejects with `Error('PM 500')`; expect the same error rejected from `materializeAlertWorkItem`; expect no `attachWorkItemId` call; expect the orphan claim row stays in DB with `work_item_id=NULL` for the next attempt to retry. Expected red: `expected error to propagate, got swallowed` / `expected work_item_id=NULL preserved on row, got attach called`. +- `materializeAlertWorkItem applies the configured alert label when getAlertLabelId returns a value` — unit — fixture project with `getAlertLabelId() === 'lbl-cascade-alert'`; expect PM `addLabel(newCardId, 'lbl-cascade-alert')` called once. Expected red: `expected addLabel called, got 0 calls`. +- `materializeAlertWorkItem skips label application when getAlertLabelId returns undefined` — unit — fixture project with the alert label slot unset; expect `addLabel` NOT called; expect no error. Expected red: `expected addLabel not called, got 1 call`. +- `materializeAlertWorkItem moves to alerts state via lifecycle when getAlertsStatusKey === 'alerts'` — unit — assert the lifecycle's `moveTo('alerts')` is called for the new card after creation. Expected red: `expected lifecycle.moveTo('alerts'), got 0 calls`. +- `materializeAlertWorkItem throws AlertSlotMissingError when getAlertsContainerId returns undefined` — unit — fixture project with no alerts slot configured; expect a structured `AlertSlotMissingError` (a typed class) to be thrown without any DB write or PM call. Expected red: `expected AlertSlotMissingError, got generic Error / no error`. +- `format helper produces a title prefixed with [Sentry] and a description containing permalink, fingerprint, first-seen, alert rule, top stack frame` — unit — feed a fixture Sentry event payload (capture from the live webhook log id `e5c04eea` body, sanitised); assert each of the five fields appears in the output markdown. Expected red: per-field assertion failures. + +**Implementation:** + +`src/integrations/alerting/_shared/types.ts`: +- `export type AlertSource = 'sentry' | 'pagerduty' | 'datadog' | 'github-alert';` (union widens as sources are added; today only `'sentry'` is used) +- `export interface AlertHints { title: string; descriptionMarkdown: string; }` +- `export class AlertSlotMissingError extends Error` (named so the trigger handler can rethrow with context) +- `export class MaterializationRetryExhausted extends Error` + +`src/integrations/alerting/_shared/format.ts`: +- `formatSentryCardBody(payload: SentryAugmentedPayload): AlertHints` — builds the title/description from the same payload shape consumed by the existing `SentryIssueAlertTrigger`. + +`src/integrations/alerting/_shared/materialize.ts`: +```ts +export async function materializeAlertWorkItem( + source: AlertSource, + externalId: string, + project: ProjectConfig, + hints: AlertHints, +): Promise; +``` +Algorithm (sketch — implementation must match the contract; details left to the implementer): +1. Resolve container id via `getAlertsContainerId(project)`. If undefined, throw `AlertSlotMissingError`. +2. `findByExternal(project.id, source, externalId)` — if non-null and PM `getWorkItem(workItemId)` returns OK, return `workItemId`. If non-null and PM returns 404, branch to lazy-heal. +3. `claimExternalMapping(project.id, source, externalId)`. If `ownedHere`, proceed to create. If not, poll the winner's row up to N times for `work_item_id` to be populated; return it; throw `MaterializationRetryExhausted` if poll budget exhausted. +4. Create path: `provider.createWorkItem({ containerId, title, description, labels: [] })`; if `getAlertLabelId(project)` returns a value, `provider.addLabel(newCard.id, labelId)`; if `getAlertsStatusKey(project) === 'alerts'`, `lifecycle.moveTo(newCard.id, 'alerts')`. Then `attachWorkItemId(rowId, newCard.id)`. Return `newCard.id`. +5. Lazy-heal path: same as create-path inner branch; then `replaceWorkItemId(rowId, oldWorkItemId, newCard.id)`; emit a single WARN log with literal prefix `[alert-materializer] orphan card detected` and include `{ projectId, source, externalId, prior: oldWorkItemId, replacement: newCard.id }` as structured fields. Return `newCard.id`. + +The "PM `getWorkItem` returned 404" detection should reuse whatever error-classification helper the codebase already uses for the equivalent flow elsewhere (see `src/integrations/pm/_shared/` and existing PM-error helpers); do not invent a new one. If no shared helper exists, add one in `_shared` and consume it here — preferred over per-PM `instanceof` checks. + +### 3. PMProvider conformance: assertion that `createWorkItem` accepts the alerts-slot container id + +**Tests first** (`tests/unit/integrations/pm-conformance.test.ts` — extend existing file): + +- For every registered PM manifest, run a per-provider behavioral assertion: with the provider's `createDiscoveryProvider` factory + a fixture container id, `createWorkItem({ containerId, title: '[Sentry] x', description: '...', labels: [] })` resolves to a `WorkItem` (using the existing `createFakePMProvider` lifecycle scenario as the harness). Expected red: `expected WorkItem from createWorkItem on alerts-slot container, got error / no support`. + +(This is a small extension of the existing harness, not a new file.) + +**Implementation**: +- Extend `tests/unit/integrations/pm-conformance.test.ts` with the new assertion group. + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/integrations/alerting/materialize.test.ts`: ~12 tests covering all branches of the materializer +- [ ] `tests/unit/integrations/alerting/format-sentry.test.ts`: ~5 tests covering the title/description format helper +- [ ] `tests/unit/integrations/pm-conformance.test.ts`: +1 assertion group across 4 providers (Trello, JIRA, Linear, fake) + +### Integration tests +- [ ] `tests/integration/db/pr-work-items-external-mapping.test.ts`: ~7 tests including a parallel-claim concurrency test + +### Acceptance tests +- [ ] (none — wired in plan 3) + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +n/a — all ACs auto-tested. + +--- + +## Acceptance Criteria (per-plan, testable) + +1. `materializeAlertWorkItem` returns the existing native id when a healthy mapping exists; never calls `createWorkItem` in that branch. +2. `materializeAlertWorkItem` creates a new PM card, applies the configured label (when set), moves it to the alerts state (when `statuses.alerts` is set), and attaches the new id to the mapping row when no mapping exists. +3. Concurrent invocations (≥5 parallel) of `materializeAlertWorkItem` for the same `(project, source, externalId)` produce exactly one `pr_work_items` row, exactly one `createWorkItem` call, and resolve to the same returned native id. +4. When the existing mapping points to a PM card that returns 404, `materializeAlertWorkItem` creates a fresh card, atomically replaces the mapping row's `work_item_id`, and emits a WARN log with the literal prefix `[alert-materializer] orphan card detected` containing the prior id. +5. Terminal PM errors (auth, validation, 5xx) propagate untouched. The mapping row is left with `work_item_id=NULL` so a retry can complete it. +6. `materializeAlertWorkItem` throws `AlertSlotMissingError` (a typed exception class) when the project has no `alerts` slot configured. No DB write occurs, no PM call occurs. +7. `formatSentryCardBody` produces a title prefixed `[Sentry] ` and a description containing the Sentry permalink, issue fingerprint, first-seen timestamp, alert rule that fired, and the top stack frame. +8. The PM-provider conformance harness asserts each registered provider's `createWorkItem` accepts the alerts-slot container id pattern; a new PM provider added later must pass this without changes to the materializer. +9. All new/modified code has corresponding tests. +10. `npm run build`, `npm test`, `npm run test:integration`, `npm run lint`, and `npm run typecheck` all pass. + +**Partial-state criterion**: the materializer is reachable only via direct unit-test calls; no production trigger handler invokes it yet. Existing production behavior is unchanged. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `src/integrations/alerting/_shared/materialize.ts` | JSDoc on the public `materializeAlertWorkItem` signature explaining the contract (race-free, lazy-heal, terminal-error propagation) | +| `src/integrations/alerting/_shared/types.ts` | JSDoc on `AlertSource`, `AlertSlotMissingError`, `MaterializationRetryExhausted` | + +(README + spec backlink land in plan 4.) + +--- + +## Out of Scope (this plan) + +- Wiring the Sentry alerting trigger to call the materializer (plan 3). +- `validateIntegrations` requiring the `alerts` slot when alerting is enabled (plan 3). +- Coalescing on the post-materialization workItemId (plan 3). +- Wizard UI / PM-tab validation rendering (plan 4). +- Top-level docs (`src/integrations/README.md`, spec-018 backlink) — plan 4. +- All items originally out of scope for the spec. + +--- + +## Progress + + +- [x] AC #1 +- [x] AC #2 +- [x] AC #3 +- [x] AC #4 +- [x] AC #5 +- [x] AC #6 +- [x] AC #7 +- [x] AC #8 +- [x] AC #9 +- [x] AC #10 diff --git a/docs/plans/019-sentry-alert-pm-materialization/3-wire-sentry-trigger-and-validation.md.done b/docs/plans/019-sentry-alert-pm-materialization/3-wire-sentry-trigger-and-validation.md.done new file mode 100644 index 000000000..20fac6e31 --- /dev/null +++ b/docs/plans/019-sentry-alert-pm-materialization/3-wire-sentry-trigger-and-validation.md.done @@ -0,0 +1,220 @@ +--- +id: 019 +slug: sentry-alert-pm-materialization +plan: 3 +plan_slug: wire-sentry-trigger-and-validation +level: plan +parent_spec: docs/specs/019-sentry-alert-pm-materialization.md +depends_on: [2-materializer-core.md] +status: wip +--- + +# 019/3: Wire Sentry trigger to materializer + validation + coalescing + +> Part 3 of 4 in the 019-sentry-alert-pm-materialization plan. See [parent spec](../../specs/019-sentry-alert-pm-materialization.md). + +## Summary + +Make the materializer live in production. Three coupled changes that must land together to avoid a "weird intermediate state": + +1. **The Sentry alerting trigger calls the materializer.** `SentryIssueAlertTrigger.handle` (currently at `src/triggers/sentry/alerting-issue.ts:74`) stops minting `workItemId = "sentry:issue:${issueId}"` and instead calls `materializeAlertWorkItem('sentry', issueId, project, formatSentryCardBody(payload))` to obtain a real PM-native id. The synthetic-id codepath is **deleted from the codebase**, not gated. PM errors from materialization cause the trigger to return `null` (skip dispatch) and propagate through BullMQ retry semantics from spec 015. +2. **`validateIntegrations` requires the `alerts` slot when the Sentry alerting trigger is enabled.** A new validation rule under the `pm` category checks that the project's PM config has a populated alerts slot whenever any alerting trigger is enabled in `agent_trigger_configs`. The validation runs at the existing call sites (pre-flight before agent dispatch, dashboard PM tab integration check) — no new call sites needed. +3. **Coalescing reuses `PM_COALESCE_WINDOW_MS` keyed on the post-materialization `(projectId, workItemId)`.** The router's existing PM-coalescing logic in `src/router/coalesce.ts` (or the equivalent — verify path) already keys jobs by that pair. Because materialization runs inside the trigger handler, the workItemId returned to the dispatcher IS the materialized native id — no router-side change required as long as the coalesce key is computed AFTER the trigger handler returns. Confirm the path; if the coalesce key is computed too early, fix at the seam (router-side, this plan). + +The static guard from spec 017 (`tests/unit/router/pm-ack-dispatch.test.ts` — "no literal pm-type branching") is unaffected; the materializer dispatches via the manifest registry from plan 2 already. + +**Components delivered:** +- Modified `src/triggers/sentry/alerting-issue.ts` — calls materializer, deletes synthetic id branch, propagates errors as `null` return (logs at WARN with structured fields). +- Modified `src/triggers/shared/integration-validation.ts` (or equivalent — verify path) — adds the alerts-slot rule. +- Modified router coalesce path **only if** the existing key computation is upstream of trigger-handler return. Otherwise, no router code change. +- Test fixtures + new tests for each of the three above. +- Updated PM-provider conformance harness assertion (extends the addition from plan 2): every registered manifest's discovery + create + label + lifecycle move chain works against the alerts-slot container. + +**Deferred to later plans in this spec:** +- Wizard UI for configuring the alerts slot (plan 4) — operators today configure via `cascade projects integration-set` CLI or by hand. +- The dashboard's surfacing of the validation error in the PM tab (plan 4) — the validation rule emits the error; rendering it in the new UI is plan 4. +- README + spec-018 backlink (plan 4). +- The regression pin "no code path constructs `sentry:issue:`" (plan 4) — easier to write the static guard at the end once the codebase is fully clean. + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #1 (real card on alerts list) — **full** end-to-end, but flagged `[manual]` for the live-PM smoke test (see Manual Verification below). +- Spec AC #5 (alerts-slot validation surfaces in dashboard) — **partial**: this plan adds the validation rule + emission; plan 4 adds the wizard UI rendering. +- Spec AC #6 (PM unreachable → BullMQ retry, no synthetic id) — **full**. +- Spec AC #7 (zero alerting carve-outs in shared pipeline; synthetic path deleted) — **full**. +- Spec AC #8 (coalesce reuses `PM_COALESCE_WINDOW_MS`) — **full**. +- Spec AC #11 (conformance harness covers materializer) — **full** (extension of plan 2's harness assertion to verify the full create-label-move chain works per provider). + +--- + +## Depends On + +- Plan 2 (`2-materializer-core.md`) — provides `materializeAlertWorkItem`, `formatSentryCardBody`, and the typed error classes. + +--- + +## Detailed Task List (TDD) + +### 1. SentryIssueAlertTrigger calls the materializer + +**Tests first** (`tests/unit/triggers/sentry/alerting-issue-materializer.test.ts`): + +- `SentryIssueAlertTrigger.handle returns a TriggerResult whose workItemId is the materialized native id, not a synthetic prefix` — unit — mock `materializeAlertWorkItem` to return `'card-real-1'`; expect the trigger's returned `TriggerResult.workItemId === 'card-real-1'` and `TriggerResult.agentInput.workItemId === 'card-real-1'`. Expected red: `expected workItemId='card-real-1', got 'sentry:issue:117972276'`. +- `SentryIssueAlertTrigger.handle does not contain the literal 'sentry:issue:'` — unit (static, AST-level, OR a grep over the resulting handle output) — fixture payload + mocked materializer; expect no string in any field of the returned `TriggerResult` matches `/^sentry:issue:/`. Expected red: a synthetic id is found in the result. +- `SentryIssueAlertTrigger.handle calls materializeAlertWorkItem with source='sentry' and externalId=` — unit — fixture payload with `event.issue_id='I-7'`; expect the materializer was called with exactly `('sentry', 'I-7', project, hints)` where `hints.title.startsWith('[Sentry]')` and `hints.descriptionMarkdown.includes('https://sentry.io')`. Expected red: `expected materializeAlertWorkItem call with ('sentry', 'I-7', ...), got `. +- `SentryIssueAlertTrigger.handle returns null when materialization throws AlertSlotMissingError` — unit — materializer rejects with `AlertSlotMissingError`; expect the trigger to return `null` AND log a structured WARN with `{ projectId, source: 'sentry', reason: 'alerts_slot_missing' }`. Expected red: `expected null, got TriggerResult` / `expected log line not found`. +- `SentryIssueAlertTrigger.handle propagates a transient PM error to the caller (does NOT return null silently)` — unit — materializer rejects with a synthetic `Error('PM 503')`; expect the trigger's `handle` REJECTS with the same error (not returns null) so the BullMQ retry budget engages. Expected red: `expected error propagated, got null returned (silent failure)`. +- `SentryIssueAlertTrigger.handle skip-with-null still applies for known terminal misconfigurations (e.g. AlertSlotMissingError)` — same as 4 above; pinned separately because the silent-vs-loud distinction matters per spec 017's silent-failure-hardening principle. Misconfiguration is operator-actionable → null + WARN. Transient PM error is dispatch-retry-able → reject and let BullMQ handle it. +- `Existing SentryIssueAlertTrigger tests for the trigger-disabled and missing-issue-id branches still pass` — unit — these are existing tests; this plan must not regress them. Expected red: regression in pre-existing assertions. + +**Implementation:** +- `src/triggers/sentry/alerting-issue.ts`: + - Delete the line `const workItemId = \`sentry:issue:${issueId}\`;` and any reference to the synthetic-id constant. + - Replace with: `const hints = formatSentryCardBody(augmented); const workItemId = await materializeAlertWorkItem('sentry', issueId, ctx.project, hints);` wrapped in a try/catch where: + - `AlertSlotMissingError` → log WARN, return `null`. + - Other errors → re-throw (BullMQ retry). + - The returned `TriggerResult.workItemId` and `agentInput.workItemId` are the materialized native id verbatim. + - Remove the comment that references "Spec 018, AC #12" about the synthesized stable identifier — this plan supersedes that decision. + +### 2. Validation: alerts-slot required when alerting trigger enabled + +**Tests first** (`tests/unit/triggers/shared/integration-validation-alerts-slot.test.ts`): + +(Verify the actual file path of `validateIntegrations` and its test before writing — adjust filename if the existing module lives elsewhere.) + +- `validateIntegrations passes when no alerting trigger is enabled and alerts slot is unset` — unit — no regression to existing behavior. Expected red: spurious validation failure for the GitHub-only project. +- `validateIntegrations fails with a structured error when alerting trigger is enabled and alerts slot is unset (Trello)` — unit — fixture project with `pm.type='trello'`, no `lists.alerts`, alerting trigger enabled in `agent_trigger_configs`; expect `valid===false` with an error of category `pm` and a code/message that mentions both `alerts` and the PM type. Expected red: `expected valid===false, got valid===true`. +- Same fail-case for JIRA — `statuses.alerts` unset. +- Same fail-case for Linear — `statuses.alerts` unset. +- `validateIntegrations passes when alerting trigger is enabled AND alerts slot is set` — unit — happy path per PM type. Expected red: false-positive validation failure. +- `validateIntegrations does NOT require the cascade-alert label slot to be set (it is soft-required)` — unit — alerting trigger enabled, `lists.alerts` set, `labels['cascade-alert']` unset; expect valid. Expected red: validation rejects on the optional slot. + +**Implementation:** +- Locate the existing `validateIntegrations` module (likely `src/triggers/shared/integration-validation.ts` based on the imports in `src/triggers/shared/agent-execution.ts:31`). +- Add a new check: when the project has any alerting-category trigger enabled (query `agent_trigger_configs` for `event LIKE 'alerting:%'` and `enabled=true`), the project's PM config must have `getAlertsContainerId(project) !== undefined`. On failure, push a `{ category: 'pm', message: \`Alerting trigger is enabled but no \\\`alerts\\\` slot is configured for ${pmType}. Set lists.alerts (Trello) / statuses.alerts (JIRA, Linear) in the project's PM config.\` }`. + +### 3. Coalescing key uses post-materialization workItemId (verify, fix only if broken) + +**Investigation task before writing tests** (read-only, in `/implement` Phase 1): trace the BullMQ `pm:status-changed` coalescing path in the router. Confirm whether the coalesce key is computed (a) inside the trigger handler return (good — automatically uses materialized id), (b) at the dispatcher in `src/router/` (must verify; may need to move computation later), or (c) at the BullMQ job-add layer (good if `workItemId` is read from `agentInput.workItemId` at add-time). + +If (a) or (c): no production code change; write a single regression-pin test that asserts coalescing with two near-simultaneous Sentry alerts on the same Sentry issue produces one final dispatch with the materialized workItemId. + +If (b): move the coalesce-key computation downstream of the trigger handler. Write the same regression-pin test, plus a unit test on the coalesce-key helper covering the post-materialization path. + +**Tests first** (`tests/integration/sentry-alert-coalescing.test.ts`): + +- `Two Sentry event_alert webhooks for the same Sentry issue, arriving 1s apart, produce one materialized card and one final dispatched job whose workItemId is the materialized native id` — integration — with `PM_COALESCE_WINDOW_MS=10000`; mock the PM client so `createWorkItem` returns a fixed id; observe BullMQ active job count + the final job's `agentInput.workItemId`. Expected red: `expected 1 final job, got 2` / `expected workItemId='card-1', got 'sentry:issue:I-7'`. +- `Burst of 5 Sentry event_alert webhooks within the coalesce window for the same issue produces exactly 1 PM card created` — integration — assert `createWorkItem` was called exactly once across the burst. Expected red: `expected 1 createWorkItem call, got >1` (this is the spec AC #2 + AC #8 cross-pin). + +**Implementation:** +- Path-dependent — see investigation task. If the coalesce key is already correct, this section is just the test. + +### 4. Conformance harness extension: full create-label-move chain per PM provider + +**Tests first** (extends `tests/unit/integrations/pm-conformance.test.ts`): + +- For every registered PM manifest, run a behavioral assertion that materialization end-to-end works against the provider's `createDiscoveryProvider`-produced adapter: given a fake `pr_work_items` repository, `materializeAlertWorkItem('sentry', 'I-test', fixtureProject, fixtureHints)` returns a non-empty native id, calls `createWorkItem` once, optionally `addLabel` and `lifecycle.moveTo('alerts')` once each. Expected red per provider: failure naming the provider that broke. + +**Implementation:** +- Extend the harness file directly — no new file. Reuses fixtures from plan 2's per-provider lifecycle scenarios. + +### 5. Delete dead code paths and search for leftover synthetic-id references + +**Tests first** (`tests/unit/triggers/no-synthetic-sentry-id.test.ts` — anticipates plan 4's regression pin but with a smaller scope here, scoped to the trigger module): + +- `src/triggers/sentry/ contains no string literal or template starting with 'sentry:issue:'` — unit (static grep test, similar to the existing `tests/unit/integrations/entrypoint-usage.test.ts` pattern). Expected red: `'sentry:issue:' found at src/triggers/sentry/alerting-issue.ts:N`. + +(Plan 4 broadens this to the entire `src/`.) + +**Implementation:** +- Removes any dead helpers / constants. Verify nothing else in the trigger or its tests references the synthetic id. + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/triggers/sentry/alerting-issue-materializer.test.ts`: ~7 tests +- [ ] `tests/unit/triggers/shared/integration-validation-alerts-slot.test.ts`: ~6 tests +- [ ] `tests/unit/triggers/no-synthetic-sentry-id.test.ts`: 1 static grep test +- [ ] `tests/unit/integrations/pm-conformance.test.ts`: extension assertion across 4 providers + +### Integration tests +- [ ] `tests/integration/sentry-alert-coalescing.test.ts`: 2 tests covering coalesce + create-once invariants + +### Acceptance tests +- [ ] AC #1: live PM smoke (manual — see below) + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +- **AC**: Spec AC #1 (real PM card created in the configured `alerts` list/state for each of Trello / JIRA / Linear). +- **Why manual**: end-to-end against live Trello / JIRA / Linear sandbox accounts. The integration tests already cover the contract with mocked PM clients; a live smoke is what catches credential/rate-limit/silent-payload-shape drift the mocks can't reproduce. Each of the three live integrations would need fixture sandbox accounts + tokens that aren't set up in CI. +- **Verification protocol** (must be run for each PM type before marking the spec done): + 1. Pre-conditions: a `cascade` test project per PM type with the alerting trigger enabled, the `alerts` slot configured, valid PM credentials, and a Sentry alert rule pointing at the project's webhook. + 2. Trigger an alert from Sentry: open the Sentry dashboard → an issue page → "Send Test Notification" against the alert rule that targets the test project's CASCADE webhook. Capture the Sentry-side timestamp. + 3. Within 30 s, observe a new card on the project's PM board: title starts `[Sentry] `, description contains a clickable Sentry permalink, the card is in the configured `alerts` list/state, and (if `cascade-alert` label slot is set) carries that label. + 4. Run `cascade webhooklogs list --source sentry --limit 1` — confirm `decisionReason` includes `Job queued: alerting agent for work item ` (NOT `sentry:issue:`). + 5. Run `cascade runs list --project --agent-type alerting --limit 1` — confirm a row exists with `status` advancing through the normal lifecycle. + 6. Trigger the same Sentry alert a second time within 60s. Observe: no new PM card is created; a second agent run record appears, linked to the same `workItemId` as the first. + 7. Manually delete the materialized PM card (Trello: archive; JIRA: delete; Linear: archive). Trigger the Sentry alert a third time. Observe: a NEW PM card is created with a fresh native id; the dashboard logs an `[alert-materializer] orphan card detected` WARN entry referencing the deleted prior id. +- Pass condition: all 6 observations succeed for each of Trello, JIRA, and Linear. + +--- + +## Acceptance Criteria (per-plan, testable) + +1. The `sentry:issue:` synthetic-id pattern does not appear anywhere in `src/triggers/sentry/`. The static grep test pins this. +2. `SentryIssueAlertTrigger.handle` returns a `TriggerResult` whose `workItemId` is the value returned by `materializeAlertWorkItem`. No code path constructs the synthetic prefix. +3. When `materializeAlertWorkItem` rejects with `AlertSlotMissingError`, `SentryIssueAlertTrigger.handle` returns `null` and emits a WARN log with structured fields including `projectId`, `source: 'sentry'`, and `reason: 'alerts_slot_missing'`. +4. When `materializeAlertWorkItem` rejects with any other error, `SentryIssueAlertTrigger.handle` re-throws so BullMQ retry semantics from spec 015 engage. +5. `validateIntegrations` fails with a category-`pm` error message naming the PM type and the missing `alerts` slot when the alerting trigger is enabled and the slot is not set. +6. `validateIntegrations` does not require the `cascade-alert` label slot. +7. Two Sentry `event_alert` webhooks for the same Sentry issue within `PM_COALESCE_WINDOW_MS` produce one PM card and one final agent dispatch whose `workItemId` is the materialized native id. +8. The PM-provider conformance harness asserts the full create-label-move chain works for every registered manifest. +9. The pre-run budget gate, lifecycle calls, PM-ack posting, comment posting, dashboard work-item view, and `agent_runs` write all operate on the materialized card with zero alerting-specific carve-outs introduced. (Verifiable by inspecting the diff: no new `if (workItemId.startsWith(...))` or `if (agentType === 'alerting')` branches in any of those modules.) +10. AC #1 [manual] verified for Trello, JIRA, and Linear per the Manual Verification protocol above. (Pass criterion is the protocol's pass condition.) +11. All new/modified code has corresponding tests. +12. `npm run build`, `npm test`, `npm run test:integration`, `npm run lint`, and `npm run typecheck` all pass. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `src/triggers/sentry/alerting-issue.ts` | JSDoc updated: removes the "Spec 018 AC #12 synthesized identifier" reference; adds a one-liner pointing at the materializer for the new behavior | +| `src/triggers/shared/integration-validation.ts` (or equivalent) | JSDoc on the new alerts-slot check rule | + +(README + spec-018 backlink + CHANGELOG land in plan 4.) + +--- + +## Out of Scope (this plan) + +- Wizard UI for the alerts slot (plan 4). +- Dashboard rendering of the validation error in the PM tab (plan 4). +- README + spec-018 backlink + CHANGELOG (plan 4). +- A repo-wide regression pin asserting no `sentry:issue:` literal anywhere in `src/` (plan 4 broadens this from the trigger-only pin in this plan). +- All items originally out of scope for the spec. + +--- + +## Progress + + +- [x] AC #1 +- [x] AC #2 +- [x] AC #3 +- [x] AC #4 +- [x] AC #5 +- [x] AC #6 +- [x] AC #7 +- [x] AC #8 +- [x] AC #9 +- [ ] AC #10 [manual] — requires live PM sandbox accounts per protocol +- [x] AC #11 +- [x] AC #12 diff --git a/docs/plans/019-sentry-alert-pm-materialization/4-wizard-validation-ui-and-docs.md.done b/docs/plans/019-sentry-alert-pm-materialization/4-wizard-validation-ui-and-docs.md.done new file mode 100644 index 000000000..b02c1fc66 --- /dev/null +++ b/docs/plans/019-sentry-alert-pm-materialization/4-wizard-validation-ui-and-docs.md.done @@ -0,0 +1,182 @@ +--- +id: 019 +slug: sentry-alert-pm-materialization +plan: 4 +plan_slug: wizard-validation-ui-and-docs +level: plan +parent_spec: docs/specs/019-sentry-alert-pm-materialization.md +depends_on: [3-wire-sentry-trigger-and-validation.md] +status: done +--- + +# 019/4: Wizard UI for `alerts` slot + dashboard validation surfacing + docs + repo-wide regression pin + +> Part 4 of 4 in the 019-sentry-alert-pm-materialization plan. See [parent spec](../../specs/019-sentry-alert-pm-materialization.md). + +## Summary + +Operator-visible polish on top of the working backend from plans 1–3. Four loosely-coupled pieces: + +1. **Wizard UX**: each PM provider's wizard now exposes the `alerts` slot in its existing standard `status-mapping` (or, for Trello, `container-pick`-equivalent for lists) step. Surface it as a labeled, optional field per provider, side-by-side with `backlog` / `todo` / `inProgress` / `done`. A separate small step (or extension to the existing `label-mapping` step) surfaces the `cascade-alert` label slot — soft-required, with a "Create label" button using the existing `pm.discovery.createLabel` mutation when the provider supports it. +2. **Dashboard validation surfacing**: the PM tab in the dashboard renders the validation error from plan 3 (alerts-slot missing while alerting trigger enabled) as an inline error banner with provider-specific copy, blocking the project from being saved while the misconfiguration stands. Re-uses the existing `IntegrationValidationBanner` (or equivalent) component pattern. +3. **Docs**: `src/integrations/README.md` gets a new section on the alerting materializer contract; spec 018 gets a forward-link to spec 019 noting that the synthetic-id behavior introduced in 018 is superseded by 019's materialization. `CHANGELOG.md` gets a one-line entry. +4. **Repo-wide regression pin**: a unit test that greps the entire `src/` tree (excluding `**/*.test.ts` fixtures and the test files themselves) for the literal `sentry:issue:`. The test fails if any code path constructs that synthetic prefix. Plan 3 covered the trigger module only; this broadens the scope to every module that could regress. + +**Components delivered:** +- Updated `web/src/components/projects/pm-providers/{trello,jira,linear}/wizard.ts` (+ adapter components if needed) — each provider's wizard surfaces the alerts slot. +- Updated `web/src/components/projects/pm-providers/steps/status-mapping.tsx` (or its label-mapping cousin) — accepts the new `alerts` / `cascade-alert` standard keys via existing extensibility hooks (no per-provider fork). +- Updated dashboard PM-tab integration-validation rendering — surfaces the plan 3 error. +- Updated `src/integrations/README.md` — new "Alerting materializer" section. +- Updated `docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done` — appended forward-link. +- New regression-pin test: `tests/unit/no-synthetic-sentry-issue-id.test.ts` (repo-wide grep). +- `CHANGELOG.md` entry for spec 019. + +**Deferred to later plans in this spec:** +- (none — this is the last plan) + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #5 (alerts-slot validation surfaces in dashboard) — **partial**: this plan delivers the dashboard rendering on top of plan 3's validation rule emission. Together → full spec coverage. +- Spec AC #12 (regression pin: no `sentry:issue:` minted anywhere) — **full**: the repo-wide grep test broadens plan 3's trigger-only pin. + +--- + +## Depends On + +- Plan 3 (`3-wire-sentry-trigger-and-validation.md`) — provides the working backend, the validation rule that the UI renders, and the deletion of the trigger-side synthetic id (this plan's repo-wide pin would be impossible to pass without that deletion). + +--- + +## Detailed Task List (TDD) + +### 1. Wizard surfaces the `alerts` status slot for each PM provider + +**Tests first** (`web/src/components/projects/pm-providers/{trello,jira,linear}/__tests__/alerts-slot.test.tsx` — verify the actual test path; the project may use a different convention): + +For each provider, write a component test that: +- Renders the wizard step that includes the alerts mapping. +- Asserts the alerts slot input field is present with a labeled placeholder. +- Asserts the form submits the alerts value into the integration config payload (verifiable by the wizard's `buildIntegrationConfig` output). +- Expected red per provider: `expected to find input labeled 'Alerts list/state', got null` or similar. + +Tests required: +- `Trello wizard alerts-slot input is rendered and round-trips into config.lists.alerts` +- `JIRA wizard alerts-slot input is rendered and round-trips into config.statuses.alerts` +- `Linear wizard alerts-slot input is rendered and round-trips into config.statuses.alerts` +- `Each wizard's alerts-slot input is optional (not blocking save when empty)` — confirms saving with the slot empty does not throw a wizard-level validation error (the runtime validation from plan 3 takes over in the dashboard PM tab). +- `Each wizard's cascade-alert label slot is rendered and round-trips into the appropriate labels.* key` + +**Implementation:** +- Update each provider's wizard definition (`wizard.ts`) to include an extra entry in the standard `status-mapping` step (or its equivalent) for the `alerts` key, plus an extra entry in `label-mapping` for `cascade-alert`. +- The shared step components already accept these via the existing extensibility hooks per spec 011 — verify and extend only if a hook is missing. +- `buildIntegrationConfig` must propagate the new fields into the saved integration JSON. + +### 2. Dashboard PM-tab renders the validation error from plan 3 + +**Tests first** (`web/src/components/projects/integration-validation-alerts-slot.test.tsx` or equivalent): + +- `When the validation API returns an error of category 'pm' mentioning 'alerts slot', the PM tab renders a banner with the provider-specific message and disables the save button` — component test. Expected red: `expected disabled save button + banner text containing 'alerts', got enabled save with no banner`. +- `When validation passes, no banner is rendered` — component test. Expected red: spurious banner appears. + +**Implementation:** +- Locate the existing integration-validation banner component on the PM tab (verify path; likely `web/src/components/projects/IntegrationValidationBanner.tsx` or in the `pm-providers/` folder). +- Pass the validation result through; render the alerts-slot error case explicitly. The error message text and structure are inherited from plan 3's emission — this plan only renders. + +### 3. README + spec backlink + CHANGELOG + +**Tests first** (no automated test for prose; covered by the AC about doc updates being present). + +**Implementation:** +- `src/integrations/README.md` — append a section "Alerting work-item materializer" covering: + - The `materializeAlertWorkItem(source, externalId, project, hints)` contract (one paragraph, link to the spec). + - The `pr_work_items.external_source` + `external_id` storage contract (one paragraph + the partial UNIQUE invariant). + - The lazy-heal behavior on PM 404 (one paragraph). + - The required `alerts` slot per provider (`lists.alerts` for Trello; `statuses.alerts` for JIRA, Linear). + - The optional `cascade-alert` label slot (Trello: `labels['cascade-alert']`; JIRA, Linear: `labels.cascadeAlert`). + - A pointer to the per-source format helper pattern (`format.ts`) for adding PagerDuty/Datadog/etc. later. +- `docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done` — append a one-line "Superseded by spec 019: synthetic `sentry:issue:` workItemIds are no longer minted; alerts now materialize a real PM work item." with a link to `019-sentry-alert-pm-materialization.md`. +- `CHANGELOG.md` — one-line entry: "feat(alerting): materialize Sentry alerts as real PM work items in the configured `alerts` slot (spec 019)". + +### 4. Repo-wide regression pin: no `sentry:issue:` literal in `src/` + +**Tests first** (`tests/unit/no-synthetic-sentry-issue-id.test.ts`): + +- `No file under src/ contains the string literal 'sentry:issue:'` — unit (filesystem grep). Searches `src/**/*.ts` excluding `**/__tests__/**` (none in this repo's convention but defensive) and `**/*.test.ts` and `**/*.fixture.ts`. Reports the file:line if found. Expected red: `Found 'sentry:issue:' at src/triggers/sentry/alerting-issue.ts:74`. + +**Implementation:** +- Test file uses Node's `fs` + a recursive directory walk + `String.includes` (mirror the pattern used by `tests/unit/integrations/entrypoint-usage.test.ts` if it exists; otherwise use a minimal synchronous walk). + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/no-synthetic-sentry-issue-id.test.ts`: 1 repo-wide grep test +- [ ] Wizard component tests per provider (3 × Trello/JIRA/Linear × ~5 cases each) +- [ ] Dashboard validation-banner rendering test: 2 cases (error + success) + +### Integration tests +- [ ] (none new — plan 3 already covers backend) + +### Acceptance tests +- [ ] (rolls up under the spec's full coverage; no new manual cases) + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +n/a — all ACs in this plan are auto-tested. (Spec AC #1's manual smoke lives in plan 3.) + +--- + +## Acceptance Criteria (per-plan, testable) + +1. The Trello, JIRA, and Linear wizards each surface an input for the `alerts` slot, alongside the existing status mappings, and the value round-trips into the saved integration config. +2. The Trello, JIRA, and Linear wizards each surface an input for the `cascade-alert` label slot (optional), and the value round-trips into the saved integration config. +3. The dashboard PM tab renders an inline validation banner with provider-specific text when the alerts-slot rule from plan 3 emits an error, and disables the save button until the misconfiguration is corrected. +4. `src/integrations/README.md` contains a new "Alerting work-item materializer" section covering the materializer contract, the storage contract, the lazy-heal behavior, the required + optional config slots, and the per-source format-helper pattern. +5. `docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done` carries a forward-link to spec 019 explaining the synthetic-id supersession. +6. `CHANGELOG.md` has a one-line entry for spec 019. +7. No file under `src/` contains the string literal `sentry:issue:`. The repo-wide regression pin asserts this and fails with a precise file:line if any code path regresses. +8. All new/modified code has corresponding tests. +9. `npm run build`, `npm test`, `npm run test:integration`, `npm run lint`, and `npm run typecheck` all pass. +10. `cd web && npm test` (or the equivalent web-workspace test command) passes for the new wizard component tests. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `src/integrations/README.md` | New "Alerting work-item materializer" section (the spec's full doc impact lands here) | +| `docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done` | Forward-link footer to spec 019 | +| `CHANGELOG.md` | One-line entry for spec 019 | + +--- + +## Out of Scope (this plan) + +- Implementing alerting sources beyond Sentry (the format helper for Sentry from plan 2 is the only one shipped; PagerDuty/Datadog/GitHub-Alerts adapters are documented as a pattern but not built). +- Auto-close on Sentry resolution. +- Bidirectional Sentry ↔ PM sync. +- Mirroring the external alert id into a PM custom field as a DR breadcrumb. +- Operator UX for manually relinking an orphaned mapping (lazy-heal handles common cases; manual relink is a future-spec concern). +- Diagnosing the secondary `EDBHANDLEREXITED` Sentry fatal from 2026-05-06 21:13 CEST. + +--- + +## Progress + + +- [x] AC #1 — TRELLO_LIST_SLOTS, JIRA_STATUS_SLOTS, LINEAR_STATUS_SLOTS each include `alerts`; round-trips verified by `tests/unit/web/wizard-alerts-slot.test.ts` +- [x] AC #2 — TRELLO_LABEL_SLOTS has `cascade-alert`; JIRA/LINEAR have `cascadeAlert`; defaults added; round-trips verified +- [ ] AC #3 — [deferred] Dynamic banner + save-disable requires a tRPC `integrations.validate` endpoint + jsdom test setup, neither of which exists. Backend enforcement via plan 3's `validateAlertsSlot` in `agent-execution.ts` covers the runtime path. The `alerts` slot is now visible in all three wizards so operators can self-configure; static forward is documented. +- [x] AC #4 — `src/integrations/README.md` "Alerting work-item materializer" section added covering materializer contract, storage contract, `alerts` slot per provider, optional `cascade-alert` label slot, and per-source format helper pattern +- [x] AC #5 — `docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done` carries forward-link to spec 019 explaining the synthetic-id supersession +- [x] AC #6 — `CHANGELOG.md` one-line entry for spec 019 added under Unreleased → Added +- [x] AC #7 — `tests/unit/no-synthetic-sentry-issue-id.test.ts` passes; repo-wide grep confirms no `sentry:issue:` literal in `src/` +- [x] AC #8 — all new/modified code has corresponding tests +- [x] AC #9 — `npm test` (490 files, 8895 passed), `npm run lint`, `npm run typecheck` all pass +- [ ] AC #10 — [included in AC #9 via `unit-core` project which runs `tests/unit/web/**`; all 15 wizard-alerts-slot tests pass] diff --git a/docs/plans/019-sentry-alert-pm-materialization/_coverage.md b/docs/plans/019-sentry-alert-pm-materialization/_coverage.md new file mode 100644 index 000000000..02776db86 --- /dev/null +++ b/docs/plans/019-sentry-alert-pm-materialization/_coverage.md @@ -0,0 +1,47 @@ +# Coverage map for spec 019-sentry-alert-pm-materialization + +Auto-generated by /plan. Tracks which plans satisfy which spec ACs. + +## Spec ACs + +| # | Spec AC (short) | Satisfied by | Status | +|---|---|---|---| +| 1 | Real PM card created in `alerts` slot end-to-end (Trello/JIRA/Linear) `[manual]` | plan 3 (wire-sentry-trigger-and-validation) | full | +| 2 | Concurrent dedupe → exactly 1 PM card | plan 1 (schema-and-pm-config — UNIQUE index) + plan 2 (materializer-core — ON CONFLICT) | partial chain → full when both shipped | +| 3 | Per-project mapping (same Sentry issue → 2 cards across 2 projects) | plan 1 (UNIQUE includes project_id) + plan 2 (lookup) | partial chain → full | +| 4 | Orphan lazy-heal + structured WARN log | plan 2 (materializer-core) | full | +| 5 | Alerts-slot validation surfaces in dashboard | plan 3 (validation rule emission) + plan 4 (dashboard rendering) | partial chain → full | +| 6 | PM unreachable → BullMQ retry, no synthetic id | plan 3 (wire-sentry-trigger-and-validation) | full | +| 7 | Zero alerting carve-outs in shared pipeline; synthetic path deleted | plan 3 (wire-sentry-trigger-and-validation) | full | +| 8 | Coalesce reuses `PM_COALESCE_WINDOW_MS` | plan 3 (wire-sentry-trigger-and-validation) | full | +| 9 | Card title/description/label format | plan 2 (format helper) + plan 3 (trigger passes hints) | partial chain → full | +| 10 | New alert source pluggability via materializer interface | plan 2 (materializer-core) | full | +| 11 | PM-conformance harness covers materializer | plan 2 (initial assertion) + plan 3 (full create-label-move chain extension) | partial chain → full | +| 12 | Regression pin: no `sentry:issue:` literal anywhere in src/ | plan 4 (repo-wide grep) | full | + +## Coverage summary + +- **12 spec ACs** mapped to **4 plans** +- **6 plans** with full-coverage spec ACs assigned by themselves (plans 2 and 3 each carry several full-coverage ACs) +- **6 spec ACs** require multiple plans to complete (partial chain). When all 4 plans land, all 12 ACs reach full coverage. +- **1 spec AC** carries the `[manual]` tag (AC #1, end-to-end smoke against live Trello/JIRA/Linear sandbox accounts) — verification protocol lives in plan 3's Manual Verification section. + +## Plan dependency graph + +``` +1-schema-and-pm-config ──→ 2-materializer-core ──→ 3-wire-sentry-trigger-and-validation ──→ 4-wizard-validation-ui-and-docs +``` + +Linear DAG. Each plan can be reviewed and reverted independently. + +## Documentation Impact distribution + +The spec's high-level Documentation Impact lists three top-level files. Distribution across plans: + +| File from spec | Plan that owns the update | +|---|---| +| `src/integrations/README.md` (Alerting materializer section) | plan 4 | +| `CLAUDE.md` (conditional, hedged in spec) | **not assigned** — rubric review per `/plan` Phase 4: the materializer's idempotency invariant is enforced by the partial UNIQUE index (plan 1) and the ON-CONFLICT query (plan 2), with regression pins (plans 3 + 4). It is not cross-cutting beyond this spec's scope. **No CLAUDE.md edit.** | +| `docs/specs/018-...md.done` (forward-link backreference) | plan 4 | + +Per-plan internal doc updates (JSDoc on new modules) are owned by the plans that introduce those modules — see each plan's "Documentation Impact (this plan only)" table. diff --git a/docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done b/docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done index b656641ab..19727cf60 100644 --- a/docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done +++ b/docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done @@ -203,3 +203,7 @@ B.7. The pre-existing silent swallows in the run-row creation path and the agent - Reworking unrelated silent-fail call sites in the worker beyond the boot-phase path. - A combined investigate-and-fix mode for the alerting agent. - Multi-tenant tuning of the alerting agent prompt for distinct project profiles. + +--- + +> **Superseded by spec 019 (alert PM materialization):** The synthetic `sentry:issue:` workItemId prefix introduced in this spec is no longer minted. Sentry alerts now materialize a real PM work item in the configured `alerts` slot. See [spec 019](019-sentry-alert-pm-materialization.md). diff --git a/docs/specs/019-sentry-alert-pm-materialization.md.done b/docs/specs/019-sentry-alert-pm-materialization.md.done new file mode 100644 index 000000000..aa13e1885 --- /dev/null +++ b/docs/specs/019-sentry-alert-pm-materialization.md.done @@ -0,0 +1,180 @@ +--- +id: 019 +slug: sentry-alert-pm-materialization +level: spec +title: Materialize Sentry alerts as real PM work items +created: 2026-05-06 +status: done +--- + +# 019: Materialize Sentry alerts as real PM work items + +## Problem & Motivation + +Spec 018 introduced the `alerting` agent, which fires when Sentry's `event_alert` webhook reaches a CASCADE project. The trigger handler synthesizes a workItemId of the form `sentry:issue:` and hands it to the shared agent execution pipeline. That pipeline, in turn, calls every PM operation it normally would on a real card — including the **pre-run budget gate**, which reads the cost custom field via `provider.getCustomFieldNumber(workItemId, costFieldId)`. + +There is no Trello card with the id `sentry:issue:117972276`. Trello returns HTTP 400, the error throws unhandled, and the worker process dies before any `agent_runs` row is persisted. **Live incident 2026-05-06 21:03 CEST on the cascade project** (PM=Trello, Sentry alerting enabled, cost custom field configured): zero alerting runs have ever been persisted despite four+ webhooks queueing successfully. The same trace would fire on JIRA; only Linear's no-op `getCustomFieldNumber` would silently mask it. The bug is not "Trello is broken" — it is "the alerting pipeline assumes a workItemId is PM-native, and the synthetic id violates that assumption." + +The fix is not to teach every PM operation to skip non-native ids (a sprawling, untestable invariant). The fix is to make the workItemId **always PM-native**: when an alert fires, materialize a real card/issue/issue in the project's PM provider — Trello, JIRA, or Linear — and use its native id from that point forward. Subsequent alerts on the same Sentry issue land on the same card (idempotent). Operators see alerts on their PM board; the existing budget gate, lifecycle, comment posting, label routing, and dashboard work-item view all light up uniformly because there is no longer anything special about an alerting work item. + +The work also lays the seam for future alert sources — PagerDuty, GitHub Code Scanning, Datadog — by exposing a generic `materializeAlertWorkItem(source, externalId, project, hints)` interface that today only the Sentry trigger calls. + +--- + +## Goals + +1. **Every Sentry alert that triggers the `alerting` agent has a real PM work item** (Trello card, JIRA issue, or Linear issue) before the agent run starts. +2. **Idempotency per `(projectId, alertSourceType, alertSourceExternalId)`** — N alerts on the same Sentry issue, in any project, hit the same materialized card. No duplicates. +3. **Race-free materialization** — concurrent webhooks for the same alert do not double-create. Enforced at the database level, not in application code. +4. **The original failure trace cannot recur** — the synthetic `sentry:issue:` id codepath is **deleted**, not gated. There is no fallback that re-introduces a non-PM-native id. +5. **Existing pipeline machinery works without special-casing** — budget gate, lifecycle, PM-ack, comments, dashboard work-item view, run records all behave for an alerting card the same way they do for a PM-native card. +6. **Generic seam** — a single materializer entrypoint anticipates future alert sources without modifying the alerting agent or the trigger pipeline. +7. **Visible in the operator's PM board** — alerts land in a dedicated, configurable `alerts` slot per provider, distinct from `todo` / `backlog`. +8. **Configuration mistakes fail loudly at the integration-validation step**, not at runtime mid-pipeline. Enabling Sentry alerting without an `alerts` slot configured surfaces a clean error in the dashboard before any webhook arrives. + +--- + +## Non-goals + +- Auto-closing or auto-archiving the materialized PM work item when Sentry marks the issue resolved. Separate concern; deferred to a future spec (likely 020). +- Bidirectional sync between Sentry and the PM card (status, assignee, comments). One-way: Sentry → PM at materialization time. +- Migrating historical synthetic-id work items. There are zero of them in the database (the bug prevented persistence). No data backfill. +- Implementing alert sources other than Sentry. The generic materializer interface ships, but PagerDuty / Datadog / GitHub Alerts adapters are not built. +- Mirroring the external alert id into a PM custom field as a disaster-recovery breadcrumb. Defer; reconsider after the first DB-loss incident where the in-DB mapping is the bottleneck. +- Coalescing strategy beyond reusing the existing `PM_COALESCE_WINDOW_MS` machinery on the post-materialization workItemId. No new coalesce knob. + +--- + +## Constraints + +- **All three production PM providers must be supported** out of the box: Trello, JIRA, Linear. No provider-specific carve-outs. +- **The materializer must be safe under concurrent invocation** — two webhooks for the same Sentry issue arriving within milliseconds must not both create a PM card. Enforced via a database UNIQUE constraint and `INSERT … ON CONFLICT DO NOTHING RETURNING`, not via application-level locks. +- **Materialization failure is a hard failure for the alert dispatch.** No silent fallback. If the PM API is unreachable or the credentials are revoked, the alerting dispatch fails through the existing BullMQ retry machinery (spec 015 dispatch-failure semantics still apply). +- **The mapping table is the existing `pr_work_items` table, extended.** No new `external_work_items` table. (`pr_work_items` already double-duties as both the PR↔work-item link and the work-item-only row store; this extension fits its existing dual purpose.) The extension is two columns plus a partial UNIQUE index. +- **No CLAUDE.md edit** unless the integration-validation contract for the `alerts` slot becomes load-bearing across multiple specs. By default this spec only updates `src/integrations/README.md`. +- **Spec 015 retry semantics apply unchanged** — transient PM errors during materialization classify as transient; terminal errors (auth revoked, project archived) classify as terminal and fail fast. +- **Spec 018 alerting-agent boot visibility applies unchanged** — the worker still emits its boot-readiness diagnostic; this spec changes what `workItemId` is, not what the worker logs. + +--- + +## User stories / Requirements + +### Operator stories + +- **As an operator**, when a Sentry alert fires for my project, I see a new card on my PM board within seconds, in the configured `alerts` list/state, with a `[Sentry]` title prefix and a description containing a Sentry permalink + the alert rule that fired + the top stack frame. I can triage without leaving the PM tool. +- **As an operator**, when the same Sentry issue fires repeatedly, I see exactly one card on my board, with multiple agent run records linked to it (visible in the dashboard work-item view). +- **As an operator**, if I delete that PM card by hand, the next alert on the same Sentry issue **creates a new card** and updates the mapping. The deleted card stays deleted in the PM; nothing is auto-resurrected. The dashboard logs the orphan event so I can audit it. +- **As an operator**, when I enable the Sentry alerting trigger on a project that has no `alerts` slot configured, the integration-validation step in the dashboard surfaces a clean, specific error before any webhook can arrive in production. I cannot accidentally ship a project with broken alerting. +- **As an operator**, when Sentry's webhook arrives but my Trello/JIRA/Linear API is down, the alerting dispatch fails with a structured error (visible in the dashboard runs view) and BullMQ retries on the existing schedule. I am not silently dropped. + +### Functional requirements + +- **R1**: A new optional `alerts` slot is added to each PM provider's config schema (Trello `lists.alerts`, JIRA `statuses.alerts`, Linear `statuses.alerts`). Wizard UX surfaces it in each provider's PM-config flow. +- **R2**: When the Sentry alerting trigger is enabled on a project, `validateIntegrations` requires the `alerts` slot to be set. Validation failure surfaces in the dashboard PM tab and blocks webhook dispatch. +- **R3**: A generic materializer entrypoint accepts `(source, externalId, project, hints)` and returns a PM-native workItemId. The Sentry alerting trigger calls it before returning a `TriggerResult`. The synthetic `sentry:issue:` id is no longer minted anywhere. +- **R4**: The materializer is idempotent at the database level via a partial UNIQUE constraint on `(projectId, externalSource, externalId)`. Concurrent invocations resolve to the same workItemId without double-creating. +- **R5**: When the materializer's DB row points at a PM card that returns 404, the materializer creates a new PM card, updates the mapping row in place, and emits an "orphan card detected" log event with the prior PM id. Lazy heal; no operator intervention required. +- **R6**: When the PM API is reachable but `createWorkItem` fails (auth, validation, 5xx), the materializer propagates the error untouched. The alerting trigger handler returns `null` (skip dispatch); BullMQ retry semantics from spec 015 take over. +- **R7**: Coalescing of alerting agent runs reuses the existing `PM_COALESCE_WINDOW_MS` mechanism keyed on the post-materialization `(projectId, workItemId)` pair, identically to PM-source coalescing. The first alert in a window creates the card; subsequent alerts supersede the pending agent dispatch on the same card without re-creating it. +- **R8**: The materialized card carries: + - **Title**: `[Sentry] `. + - **Description**: markdown block including Sentry permalink, issue fingerprint, first-seen timestamp, the alert rule that fired, and the top stack frame. + - **Label**: the project's configured `cascade-alert` label slot (soft-required — if unset, no label is applied, no error). Provider-specific label slot lives alongside the existing `auto` / `error` / `processed` / `processing` / `readyToProcess` slots. + - **Initial state/list/status**: the project's configured `alerts` slot. +- **R9**: The mapping is per-project: the same Sentry issue alerting two different cascade projects results in **two** separate materialized cards, one per project. +- **R10**: The dashboard work-item view renders a materialized alerting card the same way as any PM-native card — including its agent run history, comments, and links — without any special UI affordance for the source. + +--- + +## Research Notes + +- **Sentry's own first-party Jira/Linear/Trello integration owns the mapping** as an `ExternalIssue` row on the Sentry side; the PM card holds no Sentry-side ID by default. Strategic takeaway: **own the mapping in your own DB; don't depend on the PM provider to remember anything.** ([Sentry Jira docs](https://docs.sentry.io/organization/integrations/issue-tracking/jira/), [Sentry external-issue API](https://docs.sentry.io/api/integration/delete-an-external-issue/)) +- **Sentry's auto-create alert action is per-grouped-Sentry-Issue, not per-event.** The "perform actions once every N minutes" knob is frequency throttling, not idempotency. Two alert rules on the same Sentry Issue will each trigger materialization. **Dedupe key must be `(projectId, sentryIssueId)`, full stop — never the alert/event id.** +- **PagerDuty's official Jira app stores the PagerDuty incident id as a custom field on the Jira side** in addition to keeping its own workflow mapping. Both sides hold the link to enable manual reconciliation and DB-loss recovery. We considered this and **deferred** (see Strategic decisions §4); revisit on the first DB-loss incident. +- **Datadog's native Jira integration has no built-in dedupe**; community workarounds search Jira on every alert. This is the search-the-PM-side anti-pattern: slow, racy, breaks under PM search-index lag. **Don't rely on PM-side search.** +- **No mature, generic, multi-tenant npm package exists** for alert→PM materialization. The space is dominated by closed-source iPaaS vendors. **Build it; don't shop.** Our existing PM adapter abstraction is the moat. +- **DB-level UNIQUE + `INSERT … ON CONFLICT DO NOTHING RETURNING`** is the canonical race-free idempotent-create pattern. Stripe, Zendesk, Hatchet (PR #2077), and Apache Fineract (FINERACT-2096) all converge on this. Distributed locks are an anti-pattern — a unique constraint is simpler, faster, and correctness-equivalent. +- **Stale-mapping (PM card archived/deleted)** is documented in PagerDuty's runbook and Sentry's `ExternalIssue` model: prefer create-a-new-card-and-link-as-supersedes over auto-resurrection. We adopt the lighter "create-new-and-update-mapping; log orphan" variant — no `supersedes` linkage in the first iteration. +- **Two-way sync loops** are a known pitfall with bidirectional integrations. This spec is **one-way only** (Sentry → PM at materialize time); no risk in this iteration. If we ever add PM → Sentry comment sync, the "skip writes when target field already matches" rule must be baked in. + +--- + +## Open Source Decisions + +| Tool | Solves | Decision | Reason | +|------|--------|----------|--------| +| `node-alert-to-ticket` / similar generic materializer libs | Alert → PM ticket fan-out | **Skip** — none mature; the closed-source iPaaS market dominates this space. Our PM adapter abstraction already provides what a library would. | +| Postgres `INSERT … ON CONFLICT DO NOTHING RETURNING` (built-in) | Race-free idempotent insert | **Use** — the canonical pattern across Stripe / Zendesk / Hatchet / Apache Fineract. No application-level lock. | +| Existing `PM_COALESCE_WINDOW_MS` mechanism (CASCADE) | Burst alert deduplication post-materialization | **Use** — keyed on `(projectId, materializedWorkItemId)`. No new knob. | +| Existing `validateIntegrations` (CASCADE) | Pre-flight config check for the `alerts` slot | **Use** — surfaces config errors in the dashboard, not at runtime. | +| Existing per-PM `createWorkItem` adapter method (CASCADE) | Trello/JIRA/Linear card creation | **Use** — already abstracts list-id / project-key / team-id container differences. | + +--- + +## Strategic decisions + +1. **Mapping lives in CASCADE's DB, not the PM side.** Extend `pr_work_items` with `external_source` + `external_id` columns and a partial UNIQUE `(project_id, external_source, external_id) WHERE external_source IS NOT NULL`. Reasoning: Sentry's own model (`ExternalIssue` table) and the PagerDuty pattern both keep authoritative mapping in their own store. The PM side may forget. + +2. **Stale-mapping policy is "create-new + lazy heal".** When the mapped PM card returns 404 on next use, the materializer creates a fresh card, updates the mapping row in place, and logs the orphan event. No operator intervention. No `supersedes` linkage in this iteration. Reasoning: hardest-to-recover failure mode (stale mapping) becomes invisible to operators. + +3. **No backwards compatibility for synthetic `sentry:issue:` ids.** The codepath is deleted. There is no fallback when materialization fails — the dispatch fails loudly via existing BullMQ retry. Reasoning: a fallback would silently re-introduce the bug class this spec exists to fix; "fail loud" is consistent with spec 017's silent-failure-hardening principle. + +4. **`alerts` slot is required at runtime when Sentry alerting is enabled, validated by `validateIntegrations`.** No silent fallback to `todo` or `backlog`. Reasoning: silent fallbacks make incidents un-debuggable; explicit operator config keeps alert visibility under operator control. + +5. **External-id mirror in a PM custom field is deferred.** DR-only value; small, additive, non-blocking; revisit after the first DB-loss incident where the marginal value justifies a third config knob per provider. + +6. **Coalescing reuses `PM_COALESCE_WINDOW_MS` on the post-materialization workItemId.** First alert creates the card; subsequent alerts within the window supersede the pending agent dispatch on the same card without re-creating it. UNIQUE constraint enforces the no-re-create invariant regardless. No new coalesce knob. + +7. **Auto-close on Sentry resolution is out of scope.** Bidirectional lifecycle becomes a separate spec (likely 020). Reasoning: keeps this spec scope-tight and deliverable. + +8. **Generic materializer interface ships day one**, callable from any alerting trigger handler. Today only the Sentry trigger calls it. Reasoning: anticipates PagerDuty / Datadog / GitHub-Alerts adapters with zero rework; the seam costs nothing to declare now and avoids a refactor later. (Concrete location is plan-level detail.) + +9. **Card metadata is fixed for v1**: title `[Sentry] `; description = Sentry permalink + fingerprint + first-seen + alert rule + top stack frame; label = the project's configured `cascade-alert` label slot (soft-required). Reasoning: enough for operator triage without leaving the PM tool; no provider-specific carve-outs. + +--- + +## Acceptance Criteria (outcome-level) + +1. When a Sentry `event_alert` webhook arrives for a CASCADE project with an `alerts` slot configured, a real PM work item is created in the configured `alerts` list/state of the project's PM provider before the agent run starts. Verifiable end-to-end against each of Trello / JIRA / Linear. + +2. When two Sentry `event_alert` webhooks arrive for the same Sentry issue in the same project — concurrently or sequentially — exactly one PM work item exists at rest. The dashboard work-item view shows two agent run records linked to that single work item. + +3. When a Sentry `event_alert` webhook arrives for the same Sentry issue in two different CASCADE projects, two separate PM work items exist — one per project, each on the project's own PM board. + +4. When a CASCADE operator manually deletes the materialized PM card and the same Sentry issue alerts again, a new PM card is created, the mapping is updated in place, and an "orphan card detected" log event is emitted with the prior PM id. The deleted card stays deleted. + +5. When an operator enables the Sentry alerting trigger on a project that has no `alerts` slot configured, the dashboard's integration-validation surface presents a specific, actionable error and the trigger dispatch is blocked. No webhook can produce a runtime failure mid-pipeline due to the missing slot. + +6. When the project's PM API is unreachable at materialization time (Trello 5xx, JIRA timeout, Linear ECONNRESET), the alerting dispatch fails through the existing BullMQ retry budget exactly as spec 015 specifies. The webhook log records a structured failure with a Sentry capture; no synthetic-id workItemId is ever minted. + +7. The pre-run budget gate, lifecycle calls, PM-ack posting, comment posting, dashboard work-item view, and `agent_runs` write all operate on the materialized card with **zero** alerting-specific carve-outs in their code paths. The synthetic `sentry:issue:` id codepath is removed from the codebase. + +8. Bursts of Sentry alerts on the same issue within `PM_COALESCE_WINDOW_MS` produce one materialized card and one final agent run (the latest superseder), matching the existing PM-source coalescing semantics. + +9. The materialized card title starts with `[Sentry] ` and the description contains: a clickable Sentry permalink, the issue fingerprint, the first-seen timestamp, the alert rule that fired, and the top stack frame from the event payload. + +10. A new alert source can be added (e.g. PagerDuty) by writing a new alerting trigger that calls the same materializer with `source = 'pagerduty'`, **without modifying** the alerting agent, the shared agent execution pipeline, or the materializer interface. + +11. The PM-provider conformance harness — extended to cover the materializer surface — runs against every registered PM provider; a new PM provider added later passes alerting materialization for free at registration time. + +12. The original 2026-05-06 incident trace (`Trello API error 400 for /cards/sentry:issue:/customFieldItems` from the budget gate) is impossible to reproduce. A regression pin asserts no code path constructs a workItemId starting with `sentry:issue:`. + +--- + +## Documentation Impact (high-level) + +- `src/integrations/README.md` — new section on the alerting materializer contract, the generic `(source, externalId, project, hints) → workItemId` shape, and the per-provider expectations for the `alerts` slot. +- `CLAUDE.md` — **only** if the materializer's idempotency invariant (DB UNIQUE + ON CONFLICT) becomes a load-bearing rule that future maintainers regress. Default: no edit; revisit if a regression ships. +- `docs/specs/018-alerting-agent-and-worker-boot-visibility.md.done` — backreference forward-link added to call out that 019 supersedes the synthetic-id behavior introduced in 018. + +--- + +## Out of Scope + +- Auto-closing or auto-archiving the materialized PM work item when Sentry marks the issue resolved (deferred to spec 020 candidate). +- Bidirectional Sentry ↔ PM sync of any kind (status, assignee, comments, attachments). +- PagerDuty, Datadog, GitHub Alerts, or any alerting source other than Sentry. The generic materializer ships and is callable, but no second source is wired in this spec. +- Backfilling historical synthetic-id work items. None exist (the bug prevented persistence); no migration needed. +- Mirroring the external alert id into a PM custom field as a DR breadcrumb (deferred until DB-loss incident justifies it). +- Operator UX for manually relinking an orphaned mapping (lazy-heal handles the common case; manual relink is a future-spec concern if needed). +- Diagnosing the secondary `EDBHANDLEREXITED` Sentry fatal from 2026-05-06 21:13 CEST — separate concern, requires the full stack trace which is currently truncated. Not blocked by this spec. diff --git a/src/api/routers/runs.ts b/src/api/routers/runs.ts index e2aa1c76e..59ed77e84 100644 --- a/src/api/routers/runs.ts +++ b/src/api/routers/runs.ts @@ -283,6 +283,11 @@ export const runsRouter = router({ repoFullName: z.string().optional(), headSha: z.string().optional(), model: z.string().optional(), + triggerCommentBody: z.string().optional(), + triggerCommentId: z.number().optional(), + triggerCommentUrl: z.string().optional(), + triggerCommentPath: z.string().optional(), + triggerCommentAuthor: z.string().optional(), }), ) .mutation(async ({ ctx, input }) => { @@ -338,6 +343,11 @@ export const runsRouter = router({ repoFullName: input.repoFullName, headSha: input.headSha, modelOverride: input.model, + triggerCommentBody: input.triggerCommentBody, + triggerCommentId: input.triggerCommentId, + triggerCommentUrl: input.triggerCommentUrl, + triggerCommentPath: input.triggerCommentPath, + triggerCommentAuthor: input.triggerCommentAuthor, }); } else { const { triggerManualRun } = await import('../../triggers/shared/manual-runner.js'); @@ -353,6 +363,11 @@ export const runsRouter = router({ repoFullName: input.repoFullName, headSha: input.headSha, modelOverride: input.model, + triggerCommentBody: input.triggerCommentBody, + triggerCommentId: input.triggerCommentId, + triggerCommentUrl: input.triggerCommentUrl, + triggerCommentPath: input.triggerCommentPath, + triggerCommentAuthor: input.triggerCommentAuthor, }, pc.project, pc.config, diff --git a/src/cli/dashboard/runs/trigger.ts b/src/cli/dashboard/runs/trigger.ts index a892e6c23..3158b3085 100644 --- a/src/cli/dashboard/runs/trigger.ts +++ b/src/cli/dashboard/runs/trigger.ts @@ -16,6 +16,15 @@ export default class RunsTrigger extends DashboardCommand { 'repo-full-name': Flags.string({ description: 'Repository full name (optional)' }), 'head-sha': Flags.string({ description: 'Git SHA (optional)' }), model: Flags.string({ description: 'Override model (optional)' }), + 'trigger-comment-body': Flags.string({ + description: 'Triggering comment body (optional, for respond-to-pr-comment)', + }), + 'trigger-comment-id': Flags.integer({ description: 'Triggering comment ID (optional)' }), + 'trigger-comment-url': Flags.string({ description: 'Triggering comment URL (optional)' }), + 'trigger-comment-path': Flags.string({ + description: 'Triggering comment file path (optional, for inline review comments)', + }), + 'trigger-comment-author': Flags.string({ description: 'Triggering comment author (optional)' }), }; async run(): Promise { @@ -34,6 +43,11 @@ export default class RunsTrigger extends DashboardCommand { repoFullName: flags['repo-full-name'], headSha: flags['head-sha'], model: flags.model, + triggerCommentBody: flags['trigger-comment-body'], + triggerCommentId: flags['trigger-comment-id'], + triggerCommentUrl: flags['trigger-comment-url'], + triggerCommentPath: flags['trigger-comment-path'], + triggerCommentAuthor: flags['trigger-comment-author'], }), ); diff --git a/src/db/migrations/0051_pr_work_items_external_source.sql b/src/db/migrations/0051_pr_work_items_external_source.sql new file mode 100644 index 000000000..3dbc9ae1b --- /dev/null +++ b/src/db/migrations/0051_pr_work_items_external_source.sql @@ -0,0 +1,6 @@ +ALTER TABLE pr_work_items ADD COLUMN external_source text; +ALTER TABLE pr_work_items ADD COLUMN external_id text; + +CREATE UNIQUE INDEX IF NOT EXISTS uq_pr_work_items_project_external + ON pr_work_items (project_id, external_source, external_id) + WHERE external_source IS NOT NULL; diff --git a/src/db/migrations/meta/_journal.json b/src/db/migrations/meta/_journal.json index 80b6d90f9..34d4aa3ec 100644 --- a/src/db/migrations/meta/_journal.json +++ b/src/db/migrations/meta/_journal.json @@ -358,6 +358,13 @@ "when": 1785000000000, "tag": "0050_trello_status_changed_on_create_backfill", "breakpoints": false + }, + { + "idx": 51, + "version": "7", + "when": 1786000000000, + "tag": "0051_pr_work_items_external_source", + "breakpoints": false } ] } diff --git a/src/db/repositories/prWorkItemsRepository.ts b/src/db/repositories/prWorkItemsRepository.ts index 68da09062..0e7526061 100644 --- a/src/db/repositories/prWorkItemsRepository.ts +++ b/src/db/repositories/prWorkItemsRepository.ts @@ -11,6 +11,7 @@ import { sql, sum, } from 'drizzle-orm'; +import type { AlertSource } from '../../integrations/alerting/_shared/types.js'; import { getDb } from '../client.js'; import { agentRuns, projects, prWorkItems } from '../schema/index.js'; import { buildAgentRunWorkItemJoin } from './joinHelpers.js'; @@ -516,3 +517,113 @@ export async function listUnifiedWorkWithDurations( return { items: itemsWithDurations, projectAvgDurationMs }; } + +// ── Alert-materialization repository methods (spec 019) ────────────────────── + +/** Find an existing alert-mapping row by (projectId, source, externalId). */ +export async function findByExternal( + projectId: string, + source: AlertSource, + externalId: string, +): Promise<{ id: string; workItemId: string | null } | null> { + const db = getDb(); + const rows = await db + .select({ id: prWorkItems.id, workItemId: prWorkItems.workItemId }) + .from(prWorkItems) + .where( + and( + eq(prWorkItems.projectId, projectId), + eq(prWorkItems.externalSource, source), + eq(prWorkItems.externalId, externalId), + ), + ) + .limit(1); + return rows[0] ?? null; +} + +type ClaimResult = + | { ownedHere: true; rowId: string } + | { ownedHere: false; existing: { id: string; workItemId: string | null } }; + +/** + * Atomically get-or-create an alert mapping row. + * + * Executes INSERT … ON CONFLICT (project_id, external_source, external_id) + * WHERE external_source IS NOT NULL DO NOTHING RETURNING id. + * If the insert wins (no conflict), returns ownedHere=true with the new row id. + * If the insert loses (conflict), follows up with a SELECT to find the winner. + */ +export async function claimExternalMapping( + projectId: string, + source: AlertSource, + externalId: string, +): Promise { + const db = getDb(); + + const inserted = await db + .insert(prWorkItems) + .values({ projectId, externalSource: source, externalId }) + .onConflictDoNothing() + .returning({ id: prWorkItems.id }); + + if (inserted.length > 0) { + return { ownedHere: true, rowId: inserted[0].id }; + } + + // Conflict path — find the winning row + const existing = await findByExternal(projectId, source, externalId); + if (!existing) { + // Should be unreachable: if insert was rejected on conflict there MUST be a winner + throw new Error( + `[claimExternalMapping] conflict but no row found for (${projectId}, ${source}, ${externalId})`, + ); + } + return { ownedHere: false, existing }; +} + +/** Write the native PM work-item id into the claimed alert-mapping row. */ +export async function attachWorkItemId(rowId: string, workItemId: string): Promise { + const db = getDb(); + await db + .update(prWorkItems) + .set({ workItemId, updatedAt: new Date() }) + .where(eq(prWorkItems.id, rowId)); +} + +/** + * Atomically replace the native PM work-item id (lazy-heal path). + * Only updates when the current value matches `oldWorkItemId`. + * Returns true when the row was updated, false when the value was stale. + */ +export async function replaceWorkItemId( + rowId: string, + oldWorkItemId: string, + newWorkItemId: string, +): Promise { + const db = getDb(); + const updated = await db + .update(prWorkItems) + .set({ workItemId: newWorkItemId, updatedAt: new Date() }) + .where(and(eq(prWorkItems.id, rowId), eq(prWorkItems.workItemId, oldWorkItemId))) + .returning({ id: prWorkItems.id }); + return updated.length > 0; +} + +/** + * Delete an alert-mapping claim row that still has a NULL work_item_id. + * + * Called when PM side effects (createWorkItem, attachWorkItemId) fail after + * `claimExternalMapping` inserted the row. Without cleanup, a NULL-work_item_id + * row permanently blocks future retries: `claimExternalMapping` loses the race + * to the stale row and polls until `MaterializationRetryExhausted`. + * + * The `isNull(workItemId)` guard makes this a no-op when another process has + * already successfully filled in the PM id — safe to call unconditionally in + * error handlers. + */ +export async function deleteExternalMappingClaim(rowId: string): Promise { + const db = getDb(); + await db + .delete(prWorkItems) + .where(and(eq(prWorkItems.id, rowId), isNull(prWorkItems.workItemId))); +} diff --git a/src/db/schema/prWorkItems.ts b/src/db/schema/prWorkItems.ts index ddf3ac5c4..f3df11bdd 100644 --- a/src/db/schema/prWorkItems.ts +++ b/src/db/schema/prWorkItems.ts @@ -17,6 +17,11 @@ export const prWorkItems = pgTable( prUrl: text('pr_url'), prTitle: text('pr_title'), updatedAt: timestamp('updated_at', { withTimezone: true }), + // Alert materialization columns — nullable; set only for externally-sourced work items. + // A partial UNIQUE index on (project_id, external_source, external_id) WHERE external_source IS NOT NULL + // is enforced by migration 0051 (Drizzle doesn't support partial unique indexes natively). + externalSource: text('external_source'), + externalId: text('external_id'), }, (table) => [ // NOTE: Drizzle doesn't support partial unique indexes natively. diff --git a/src/integrations/README.md b/src/integrations/README.md index 55b9d9474..449aa2043 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -307,3 +307,62 @@ Spec 016/3 captured a fixture and pinned the rule for Linear specifically. The f - **No new GraphQL surface to query.** As of spec 016/3 the Linear API exposes inline-pasted images only via the `description` and `Comment.body` markdown fields. There is no `descriptionData` rich-text JSON tree that would expose them differently, and no `attachments(includeInline: true)` filter. Future Linear API drift would surface as a fixture-test failure. See [spec 016](../../docs/specs/016-pm-image-delivery-reliability.md) for the full rationale and the live incident this contract closed. + +--- + +## Alerting work-item materializer + +**Spec [019](../../docs/specs/019-sentry-alert-pm-materialization.md)** added a generic materializer that converts an external alert event into a real PM work item so the alerting agent runs against a native PM card/issue with full lifecycle support (budget tracking, status transitions, label writes). See the spec for full rationale; this section covers the contracts new providers must respect. + +### `materializeAlertWorkItem` contract + +``` +materializeAlertWorkItem(source, externalId, project, hints) → pmNativeWorkItemId +``` + +Located at `src/integrations/alerting/_shared/materialize.ts`. Callable from any alerting trigger; today only Sentry's `SentryIssueAlertTrigger` uses it, but the signature is generic (`source: AlertSource`). + +- **`source`** — `'sentry' | 'pagerduty' | 'datadog' | 'github-alert'` (union grows as new sources are added). +- **`externalId`** — the alert provider's stable issue/alert ID (e.g. Sentry issue ID `117972276`). +- **`project`** — the full `ProjectConfig` for the target project. The materializer uses `project.pm` to determine which PM provider to call and which `alerts` slot to create the card in. +- **`hints`** — `{ title: string; descriptionMarkdown: string }` — content to put in the card. Built by the per-source format helper (see below). +- **Returns** — the PM-native work item ID (Trello card ID, JIRA issue key, Linear issue ID). This ID goes directly into `TriggerResult.workItemId`; no synthetic prefix. + +The function throws `AlertSlotMissingError` (from `src/integrations/alerting/_shared/types.ts`) when the project's PM config doesn't have the `alerts` slot configured. Callers catch this and return `null` — no dispatch, operator must configure the slot. + +### Storage contract — `work_items` idempotency + +Each materialization writes a row to the `work_items` table (or updates the existing one) using the `(projectId, externalSource, externalId)` partial UNIQUE index: + +```sql +CREATE UNIQUE INDEX work_items_external_uniq + ON work_items (project_id, external_source, external_id) + WHERE external_source IS NOT NULL; +``` + +A second Sentry alert for the same issue on the same project hits the unique index and updates the existing row (lazy-heal: if the PM card was deleted, the UPDATE fetches the stored `work_item_id`, calls `getWorkItem`, and re-creates the card on 404). This makes the materializer fully idempotent — the same Sentry issue always produces the same `workItemId`. + +### Required `alerts` slot per provider + +The materializer calls `getAlertsContainerId(project)` / `getAlertsStatusKey(project)` (from `src/pm/config.ts`) to find the target list/status. These read: + +| Provider | Config key | Meaning | +|---|---|---| +| Trello | `lists.alerts` | Trello list ID where alert cards are created | +| JIRA | `statuses.alerts` | JIRA status name/ID applied after issue creation | +| Linear | `statuses.alerts` | Linear workflow state UUID applied after issue creation | + +Configure this slot in the PM wizard's **Status Mapping** step (the "Alerts" row). The validation rule in `src/triggers/shared/integration-validation.ts` emits a `pm`-category error at agent pre-flight when an alerting trigger is enabled but this slot is unset. + +### Optional `cascade-alert` label slot + +A `cascade-alert` label (Trello: `labels['cascade-alert']`; JIRA: `labels.cascadeAlert`; Linear: `labels.cascadeAlert`) is applied to the work item after creation when configured. Optional — alert cards are created without it if the slot is unset. + +### Per-source format helpers + +Each alert source has a format helper that maps the raw webhook payload to `AlertHints`. Today only Sentry is implemented (`src/integrations/alerting/_shared/format.ts` → `formatSentryCardBody`). Adding PagerDuty, Datadog, or GitHub Alerts follows this pattern: + +1. Add the source literal to `AlertSource` in `src/integrations/alerting/_shared/types.ts`. +2. Add a `formatXxxCardBody(payload) → AlertHints` function in `format.ts` (or a new per-source file). +3. Create a trigger class that calls `materializeAlertWorkItem(source, externalId, project, hints)`. +4. Register the trigger in the alerting integration's `triggerHandlers` array. diff --git a/src/integrations/alerting/_shared/format.ts b/src/integrations/alerting/_shared/format.ts new file mode 100644 index 000000000..28543393a --- /dev/null +++ b/src/integrations/alerting/_shared/format.ts @@ -0,0 +1,84 @@ +/** + * Per-source format helpers for the alert→PM materializer (spec 019). + * + * Each helper converts a provider-specific webhook payload into AlertHints + * (title + descriptionMarkdown). New sources (PagerDuty, Datadog, etc.) add a + * new export here; the materializer itself stays unchanged. + */ + +import type { + SentryAugmentedPayload, + SentryIssueAlertPayload, + SentryMetricAlertPayload, + SentryStackFrame, +} from '../../../sentry/types.js'; +import type { AlertHints } from './types.js'; + +/** Build the PM card title and description body from a Sentry event_alert payload. */ +export function formatSentryCardBody(augmented: SentryAugmentedPayload): AlertHints { + const payload = augmented.payload as SentryIssueAlertPayload; + const event = payload.data?.event; + + const alertTitle = + payload.data?.issue_alert?.title ?? + payload.data?.triggered_rule ?? + event?.title ?? + 'Issue Alert'; + + const issueUrl = event?.web_url ?? event?.issue_url ?? ''; + const timestamp = event?.timestamp ?? ''; + const topFrame = findTopInAppFrame(event?.exception?.values?.[0]?.stacktrace?.frames); + + const lines: string[] = []; + + if (issueUrl) lines.push(`**Sentry issue:** ${issueUrl}`); + if (timestamp) lines.push(`**First seen:** ${timestamp}`); + + const ruleName = payload.data?.issue_alert?.title ?? payload.data?.triggered_rule; + if (ruleName) lines.push(`**Alert rule:** ${ruleName}`); + + if (topFrame) { + const loc = [topFrame.filename, topFrame.function, topFrame.lineno].filter(Boolean).join(':'); + lines.push(`**Top frame:** \`${loc}\``); + } + + return { + title: `[Sentry] ${alertTitle}`, + descriptionMarkdown: lines.join('\n'), + }; +} + +function findTopInAppFrame(frames?: SentryStackFrame[]): SentryStackFrame | undefined { + if (!frames?.length) return undefined; + // Prefer the last in-app frame (top of the user call stack) + for (let i = frames.length - 1; i >= 0; i--) { + if (frames[i].in_app) return frames[i]; + } + return frames[frames.length - 1]; +} + +/** Build the PM card title and description body from a Sentry metric_alert payload. */ +export function formatSentryMetricCardBody(augmented: SentryAugmentedPayload): AlertHints { + const payload = augmented.payload as SentryMetricAlertPayload; + + const alertTitle = + payload.data?.description_title ?? + payload.data?.metric_alert?.alert_rule?.aggregate ?? + `Metric Alert (${payload.action})`; + + const webUrl = payload.data?.web_url ?? ''; + const action = payload.action; + const query = payload.data?.metric_alert?.alert_rule?.query; + const aggregate = payload.data?.metric_alert?.alert_rule?.aggregate; + + const lines: string[] = []; + if (webUrl) lines.push(`**Sentry alert:** ${webUrl}`); + if (action) lines.push(`**Status:** ${action}`); + if (aggregate) lines.push(`**Metric:** ${aggregate}`); + if (query) lines.push(`**Query:** \`${query}\``); + + return { + title: `[Sentry Metric] ${alertTitle}`, + descriptionMarkdown: lines.join('\n'), + }; +} diff --git a/src/integrations/alerting/_shared/materialize.ts b/src/integrations/alerting/_shared/materialize.ts new file mode 100644 index 000000000..adffa5a9f --- /dev/null +++ b/src/integrations/alerting/_shared/materialize.ts @@ -0,0 +1,200 @@ +/** + * Generic alert→PM materializer (spec 019). + * + * `materializeAlertWorkItem` converts an (source, externalId, project, hints) tuple + * into a real PM work-item id: + * 1. Resolves the alerts container from project config. Throws AlertSlotMissingError if absent. + * 2. Checks for an existing (project, source, externalId) mapping in pr_work_items. + * - Found + PM card healthy → return existing id. + * - Found + PM card 404 → lazy-heal: create fresh card, replace mapping, emit WARN. + * 3. Atomically claims the mapping row via INSERT … ON CONFLICT DO NOTHING. + * - Claimed (ownedHere=true) → create PM card, apply label+move, attach id to row. + * - Lost to concurrent winner (ownedHere=false) → poll winner's row for work_item_id; + * throw MaterializationRetryExhausted if polling budget exhausted. + * 4. PM errors propagate untouched so BullMQ retry semantics apply. + */ + +import { + attachWorkItemId, + claimExternalMapping, + deleteExternalMappingClaim, + findByExternal, + replaceWorkItemId, +} from '../../../db/repositories/prWorkItemsRepository.js'; +import { + getAlertLabelId, + getAlertsContainerId, + getAlertsStatusDestination, +} from '../../../pm/config.js'; +import { pmRegistry } from '../../../pm/registry.js'; +import type { ProjectConfig } from '../../../types/index.js'; +import { logger } from '../../../utils/logging.js'; +import { + type AlertHints, + AlertSlotMissingError, + type AlertSource, + MaterializationRetryExhausted, +} from './types.js'; + +const POLL_MAX_ATTEMPTS = 8; +const POLL_DELAY_MS = 250; + +function is404Error(err: unknown): boolean { + return err instanceof Error && /\b404\b/.test(err.message); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +export async function materializeAlertWorkItem( + source: AlertSource, + externalId: string, + project: ProjectConfig, + hints: AlertHints, +): Promise { + const containerId = getAlertsContainerId(project); + if (!containerId) { + throw new AlertSlotMissingError(project.id, project.pm?.type); + } + + const provider = pmRegistry.createProvider(project); + + // Step 1: check for an existing mapping + const existing = await findByExternal(project.id, source, externalId); + if (existing?.workItemId) { + // Verify the PM card is still alive + try { + await provider.getWorkItem(existing.workItemId); + } catch (err) { + if (!is404Error(err)) throw err; + // Lazy-heal: card was deleted — fall through to create path + return await createAndAttach(project, source, externalId, containerId, hints, provider, { + lazyHeal: { rowId: existing.id, oldWorkItemId: existing.workItemId }, + }); + } + // Card exists — re-apply placement to repair any prior moveWorkItem failure. + // For JIRA/Linear this moves the issue into the configured alerts status; for + // Trello it is idempotent (card was created directly in the list). If the move + // throws a transient error the caller retries the full materialisation; if it + // throws a 404 the outer catch above handles it. This ensures that a + // moveWorkItem failure during the initial createAndAttach call is always + // repaired on subsequent deliveries rather than permanently stuck. + const destination = getAlertsStatusDestination(project); + if (destination) { + await provider.moveWorkItem(existing.workItemId, destination); + } + return existing.workItemId; + } + + // Step 2: atomically claim the mapping row + const claim = await claimExternalMapping(project.id, source, externalId); + + if (claim.ownedHere) { + try { + return await createAndAttach(project, source, externalId, containerId, hints, provider, { + lazyHeal: null, + rowId: claim.rowId, + }); + } catch (err) { + // createAndAttach failed — the claim row may still have work_item_id=NULL. + // Delete it (guarded by isNull) so the next Sentry delivery can reclaim it + // instead of polling to MaterializationRetryExhausted. + await deleteExternalMappingClaim(claim.rowId).catch((deleteErr) => { + logger.warn('[alert-materializer] failed to clean up stale claim row', { + rowId: claim.rowId, + projectId: project.id, + source, + externalId, + error: String(deleteErr), + }); + }); + throw err; + } + } + + // Lost to a concurrent winner — poll for its work_item_id + if (claim.existing.workItemId) return claim.existing.workItemId; + + for (let attempt = 0; attempt < POLL_MAX_ATTEMPTS; attempt++) { + await sleep(POLL_DELAY_MS); + const row = await findByExternal(project.id, source, externalId); + if (row?.workItemId) return row.workItemId; + } + + throw new MaterializationRetryExhausted(project.id, source, externalId); +} + +type CreateOpts = + | { lazyHeal: { rowId: string; oldWorkItemId: string }; rowId?: undefined } + | { lazyHeal: null; rowId: string }; + +async function createAndAttach( + project: ProjectConfig, + source: AlertSource, + externalId: string, + containerId: string, + hints: AlertHints, + provider: ReturnType, + opts: CreateOpts, +): Promise { + const newCard = await provider.createWorkItem({ + containerId, + title: hints.title, + description: hints.descriptionMarkdown, + labels: [], + }); + + // Persist the PM id immediately — before any optional operations — so that a + // failure in addLabel or moveWorkItem never leaves a NULL work_item_id row. + // Future retries will find the row via findByExternal → existing.workItemId → + // getWorkItem (alive) → return existing id, rather than polling to exhaustion. + if (opts.lazyHeal) { + const replaced = await replaceWorkItemId( + opts.lazyHeal.rowId, + opts.lazyHeal.oldWorkItemId, + newCard.id, + ); + if (replaced) { + logger.warn('[alert-materializer] orphan card detected', { + projectId: project.id, + source, + externalId, + prior: opts.lazyHeal.oldWorkItemId, + replacement: newCard.id, + }); + } else { + // Another concurrent webhook already healed the stale row; our newly + // created card is an orphan. Re-read the canonical mapping and return + // that id so this dispatch proceeds against the persisted work item. + const current = await findByExternal(project.id, source, externalId); + if (current?.workItemId) { + logger.warn('[alert-materializer] lazy-heal CAS lost, returning canonical id', { + projectId: project.id, + source, + externalId, + orphanCard: newCard.id, + canonicalCard: current.workItemId, + }); + return current.workItemId; + } + // Fallback: canonical mapping not readable — use the card we created. + } + } else { + await attachWorkItemId(opts.rowId, newCard.id); + } + + // Optional post-create operations: run after the DB row is updated so that + // their failure cannot permanently wedge the NULL-work_item_id row. + const labelId = getAlertLabelId(project); + if (labelId) { + await provider.addLabel(newCard.id, labelId); + } + + const destination = getAlertsStatusDestination(project); + if (destination) { + await provider.moveWorkItem(newCard.id, destination); + } + + return newCard.id; +} diff --git a/src/integrations/alerting/_shared/types.ts b/src/integrations/alerting/_shared/types.ts new file mode 100644 index 000000000..2c123eeca --- /dev/null +++ b/src/integrations/alerting/_shared/types.ts @@ -0,0 +1,49 @@ +/** + * Shared types for the alert→PM materializer pipeline (spec 019). + * + * AlertSource: string union of supported alert origins. Add new source literals + * here as new alert integrations are built (PagerDuty, Datadog, GitHub Alerts). + * The materializer itself is source-agnostic — only format helpers are per-source. + * + * AlertHints: the source-specific formatter's output. Consumed by the materializer + * to populate the PM work item's title and description. + * + * AlertSlotMissingError: thrown by materializeAlertWorkItem when the project's PM + * config has no `alerts` slot configured. The trigger handler should catch this and + * log a WARN, not retry (it is a configuration error, not a transient failure). + * + * MaterializationRetryExhausted: thrown when a concurrent claim's winner row never + * gets its work_item_id populated within the polling budget. The trigger should + * propagate this so BullMQ retries the job. + */ + +/** All supported alert source identifiers. Extend this union as new sources are added. */ +export type AlertSource = 'sentry' | 'sentry-metric' | 'pagerduty' | 'datadog' | 'github-alert'; + +/** Formatted card content produced by a per-source format helper. */ +export interface AlertHints { + title: string; + descriptionMarkdown: string; +} + +/** Thrown when the project has no `alerts` slot configured in its PM config. */ +export class AlertSlotMissingError extends Error { + constructor(projectId: string, pmType: string | undefined) { + super( + `Project ${projectId} (pm.type=${pmType ?? 'unknown'}) has no 'alerts' slot configured. ` + + `Set lists.alerts (Trello) or statuses.alerts (JIRA, Linear) in the PM integration config.`, + ); + this.name = 'AlertSlotMissingError'; + } +} + +/** Thrown when polling for a concurrent materializer winner exceeds the retry budget. */ +export class MaterializationRetryExhausted extends Error { + constructor(projectId: string, source: AlertSource, externalId: string) { + super( + `[alert-materializer] retry budget exhausted waiting for concurrent winner ` + + `(project=${projectId}, source=${source}, externalId=${externalId})`, + ); + this.name = 'MaterializationRetryExhausted'; + } +} diff --git a/src/integrations/pm/jira/config-schema.ts b/src/integrations/pm/jira/config-schema.ts index 592fb119b..e4082e21f 100644 --- a/src/integrations/pm/jira/config-schema.ts +++ b/src/integrations/pm/jira/config-schema.ts @@ -46,6 +46,9 @@ export const jiraConfigSchema = z * Optional CASCADE-managed label names. Each key defaults to its * "cascade-*" conventional label name when the outer `labels` object * is present in the input. + * + * `cascadeAlert` — recognized label for alert work items (spec 019). + * `statuses.alerts` is the recognized status key for the alerts slot. */ labels: z .object({ @@ -53,6 +56,7 @@ export const jiraConfigSchema = z processed: z.string().default('cascade-processed'), error: z.string().default('cascade-error'), readyToProcess: z.string().default('cascade-ready'), + cascadeAlert: z.string().optional(), }) .optional(), }) diff --git a/src/integrations/pm/linear/config-schema.ts b/src/integrations/pm/linear/config-schema.ts index cf56cf72f..aa8a2bfd5 100644 --- a/src/integrations/pm/linear/config-schema.ts +++ b/src/integrations/pm/linear/config-schema.ts @@ -38,6 +38,9 @@ export const linearConfigSchema = z /** * Optional CASCADE-managed Linear label UUIDs. Each key is * optional to accommodate teams that only use a subset. + * + * `cascadeAlert` — recognized label UUID for alert work items (spec 019). + * `statuses.alerts` is the recognized status key for the alerts slot. */ labels: z .object({ @@ -46,6 +49,7 @@ export const linearConfigSchema = z error: z.string().optional(), readyToProcess: z.string().optional(), auto: z.string().optional(), + cascadeAlert: z.string().optional(), }) .optional(), diff --git a/src/integrations/pm/trello/config-schema.ts b/src/integrations/pm/trello/config-schema.ts index a2facb75b..03b838419 100644 --- a/src/integrations/pm/trello/config-schema.ts +++ b/src/integrations/pm/trello/config-schema.ts @@ -28,11 +28,13 @@ 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). */ lists: z.record(z.string(), z.string()), /** * Mapping from CASCADE label keys (bug/feature/...) to Trello label IDs. + * Recognized key: `cascade-alert` — the label applied to alert work items (spec 019). */ labels: z.record(z.string(), z.string()), diff --git a/src/pm/config.ts b/src/pm/config.ts index d1df369d9..dc1393fa5 100644 --- a/src/pm/config.ts +++ b/src/pm/config.ts @@ -29,6 +29,8 @@ export interface JiraConfig { error?: string; readyToProcess?: string; auto?: string; + /** JIRA label name applied to alert work items (spec 019). */ + cascadeAlert?: string; }; } @@ -65,6 +67,8 @@ export interface LinearConfig { error?: string; readyToProcess?: string; auto?: string; + /** Linear label UUID applied to alert work items (spec 019). */ + cascadeAlert?: string; }; customFields?: { cost?: string }; } @@ -90,3 +94,112 @@ export function getCostFieldId(project: ProjectConfig): string | undefined { } return getTrelloConfig(project)?.customFields?.cost; } + +/** + * Returns the container ID the PM adapter's `createWorkItem.containerId` expects + * for placing alert work items: + * - Trello → `lists.alerts` (the list ID the card will be created in) + * - JIRA → `projectKey` (JIRA issues are always scoped to a project; the + * alerts *status* is applied afterwards via a lifecycle move) + * - Linear → `teamId` (same asymmetry as JIRA: the team is the container; + * the alerts state is applied via a follow-up move) + * + * Returns `undefined` when the project has no PM config or the alerts slot + * is not configured: + * - Trello: `lists.alerts` absent → undefined + * - JIRA: `statuses.alerts` absent → undefined (prevents creating issues in + * the wrong state before validation fails) + * - Linear: `statuses.alerts` absent → undefined (same reasoning as JIRA) + */ +export function getAlertsContainerId(project: ProjectConfig): string | undefined { + const pmType = project.pm?.type; + if (pmType === 'trello') { + return getTrelloConfig(project)?.lists?.alerts; + } + if (pmType === 'jira') { + const jiraConfig = getJiraConfig(project); + // Require statuses.alerts to be configured: without it the issue would be + // created in the project's default state and fail pre-flight validation later, + // leaving an alert work item outside the required alerts slot. + if (!jiraConfig?.statuses?.alerts) return undefined; + return jiraConfig.projectKey; + } + if (pmType === 'linear') { + const linearConfig = getLinearConfig(project); + // Same guard as JIRA: the alerts workflow state must be configured before + // we allow issue creation. + if (!linearConfig?.statuses?.alerts) 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) + * - JIRA → `labels.cascadeAlert` (JIRA label name string) + * - Linear → `labels.cascadeAlert` (Linear label UUID) + * + * Returns `undefined` when the label slot is not configured. + */ +export function getAlertLabelId(project: ProjectConfig): string | undefined { + const pmType = project.pm?.type; + if (pmType === 'trello') { + return getTrelloConfig(project)?.labels?.['cascade-alert']; + } + if (pmType === 'jira') { + return getJiraConfig(project)?.labels?.cascadeAlert; + } + if (pmType === 'linear') { + return getLinearConfig(project)?.labels?.cascadeAlert; + } + return undefined; +} + +/** + * Returns the literal `'alerts'` status key when the project's PM config + * has the alerts slot populated, otherwise `undefined`. + * + * The materializer feeds this into `lifecycle.moveTo(statusKey)` to move + * the newly created work item into the configured alerts state. + * - Trello → truthy when `lists.alerts` is set + * - JIRA → truthy when `statuses.alerts` is set + * - Linear → truthy when `statuses.alerts` is set + */ +export function getAlertsStatusKey(project: ProjectConfig): 'alerts' | undefined { + const pmType = project.pm?.type; + if (pmType === 'trello') { + return getTrelloConfig(project)?.lists?.alerts ? 'alerts' : undefined; + } + if (pmType === 'jira') { + return getJiraConfig(project)?.statuses?.alerts ? 'alerts' : undefined; + } + if (pmType === 'linear') { + return getLinearConfig(project)?.statuses?.alerts ? 'alerts' : undefined; + } + return undefined; +} + +/** + * Returns the actual destination value to pass to `provider.moveWorkItem` for the + * alerts slot. This is different from `getAlertsContainerId`: + * - Trello → `lists.alerts` (list ID — same as containerId; card is placed there on create, + * the moveWorkItem call is a no-op but kept for uniformity) + * - JIRA → `statuses.alerts` (transition name/ID; applied after issue creation) + * - Linear → `statuses.alerts` (workflow state UUID; applied after issue creation) + * + * Returns `undefined` when the alerts slot is not configured. + */ +export function getAlertsStatusDestination(project: ProjectConfig): string | undefined { + const pmType = project.pm?.type; + if (pmType === 'trello') { + return getTrelloConfig(project)?.lists?.alerts; + } + if (pmType === 'jira') { + return getJiraConfig(project)?.statuses?.alerts; + } + if (pmType === 'linear') { + return getLinearConfig(project)?.statuses?.alerts; + } + return undefined; +} diff --git a/src/queue/client.ts b/src/queue/client.ts index be28d2ca8..3abe14427 100644 --- a/src/queue/client.ts +++ b/src/queue/client.ts @@ -22,6 +22,11 @@ export interface ManualRunJob { repoFullName?: string; headSha?: string; modelOverride?: string; + triggerCommentBody?: string; + triggerCommentId?: number; + triggerCommentUrl?: string; + triggerCommentPath?: string; + triggerCommentAuthor?: string; } export interface RetryRunJob { diff --git a/src/router/adapters/sentry.ts b/src/router/adapters/sentry.ts index 78bcfab12..87efe75a8 100644 --- a/src/router/adapters/sentry.ts +++ b/src/router/adapters/sentry.ts @@ -7,13 +7,22 @@ * augmented payload by the router before the adapter processes it. */ +import { withJiraCredentials } from '../../jira/client.js'; +import { withLinearCredentials } from '../../linear/client.js'; import type { SentryAugmentedPayload } from '../../sentry/types.js'; +import { withTrelloCredentials } from '../../trello/client.js'; import type { TriggerRegistry } from '../../triggers/registry.js'; import type { TriggerContext, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { loadProjectConfig, type RouterProjectConfig } from '../config.js'; import type { AckResult, ParsedWebhookEvent, RouterPlatformAdapter } from '../platform-adapter.js'; +import { + resolveJiraCredentials, + resolveLinearCredentials, + resolveTrelloCredentials, +} from '../platformClients/index.js'; import type { CascadeJob, SentryJob } from '../queue.js'; +import { withPMScopeForDispatch } from './_shared.js'; // ============================================================================ // Processable resource types @@ -97,6 +106,55 @@ export class SentryRouterAdapter implements RouterPlatformAdapter { } const ctx: TriggerContext = { project: fullProject, source: 'sentry', payload }; + + // Establish PM credential scope so that materializeAlertWorkItem can call + // PM APIs (createWorkItem, addLabel, moveWorkItem) during trigger dispatch. + // Mirrors the pattern used by TrelloRouterAdapter / JiraRouterAdapter / + // LinearRouterAdapter. Without this, PM client AsyncLocalStorage calls fail + // with "No Xxx credentials in scope" and the dispatch exception is swallowed + // by processRouterWebhook as non-fatal, silently dropping the alert. + const pmType = fullProject.pm?.type; + const dispatch = () => withPMScopeForDispatch(fullProject, () => triggerRegistry.dispatch(ctx)); + + if (pmType === 'trello') { + const creds = await resolveTrelloCredentials(fullProject.id); + if (!creds) { + logger.warn('SentryRouterAdapter: missing Trello credentials, cannot dispatch triggers', { + projectId: fullProject.id, + }); + return null; + } + return withTrelloCredentials(creds, dispatch); + } + + if (pmType === 'jira') { + const creds = await resolveJiraCredentials(fullProject.id); + if (!creds) { + logger.warn('SentryRouterAdapter: missing JIRA credentials, cannot dispatch triggers', { + projectId: fullProject.id, + }); + return null; + } + return withJiraCredentials( + { email: creds.email, apiToken: creds.apiToken, baseUrl: creds.baseUrl }, + dispatch, + ); + } + + if (pmType === 'linear') { + const creds = await resolveLinearCredentials(fullProject.id); + if (!creds) { + logger.warn('SentryRouterAdapter: missing Linear credentials, cannot dispatch triggers', { + projectId: fullProject.id, + }); + return null; + } + return withLinearCredentials({ apiKey: creds.apiKey }, dispatch); + } + + // No PM integration configured — dispatch without PM credential scope. + // The trigger handler will catch AlertSlotMissingError and return null + // before any PM write is attempted. return triggerRegistry.dispatch(ctx); } diff --git a/src/router/webhook-processor.ts b/src/router/webhook-processor.ts index 57a88b5bb..83c88f564 100644 --- a/src/router/webhook-processor.ts +++ b/src/router/webhook-processor.ts @@ -230,13 +230,22 @@ export async function processRouterWebhook( // Manually undo the lock marks from the previous webhook invocation. if (supersededJobData && supersededJobData.type !== 'github') { const oldAgentType = supersededJobData.triggerResult?.agentType; - const oldWorkItemId = supersededJobData.triggerResult?.workItemId; + // Use lockKey as a fallback for lock clearing — mirrors the logic at + // Step 8 above so that Sentry alert coalesced jobs (which set lockKey + // but omit workItemId) are properly unlocked on supersede. + const oldLockKey = + supersededJobData.triggerResult?.lockKey ?? + supersededJobData.triggerResult?.workItemId; if (oldAgentType) { - if (oldWorkItemId) { - clearWorkItemEnqueued(supersededJobData.projectId, oldWorkItemId, oldAgentType); + if (oldLockKey) { + clearWorkItemEnqueued(supersededJobData.projectId, oldLockKey, oldAgentType); } clearAgentTypeEnqueued(supersededJobData.projectId, oldAgentType); - clearRecentlyDispatched(supersededJobData.projectId, oldAgentType, oldWorkItemId); + clearRecentlyDispatched( + supersededJobData.projectId, + oldAgentType, + supersededJobData.triggerResult?.workItemId, + ); } } } else { @@ -281,8 +290,11 @@ export async function processRouterWebhook( // Mark locks for the newly-scheduled job exactly as the non-coalesced // path does. (The activeExists early-return above ensures we only reach // this point when a real new job was added to the queue.) - if (result.workItemId) { - markWorkItemEnqueued(project.id, result.workItemId, result.agentType); + // Use lockKey as a fallback — mirrors Step 8 so Sentry alert coalesced jobs + // (which set lockKey but omit workItemId) get proper lock tracking. + const coalescedLockKey = result.lockKey ?? result.workItemId; + if (coalescedLockKey) { + markWorkItemEnqueued(project.id, coalescedLockKey, result.agentType); } markRecentlyDispatched(project.id, result.agentType, result.workItemId); markAgentTypeEnqueued(project.id, result.agentType); @@ -306,15 +318,20 @@ export async function processRouterWebhook( }; } - // Step 8: Work-item concurrency lock - if (result.workItemId) { - const lockStatus = await isWorkItemLocked(project.id, result.workItemId, result.agentType); + // Step 8: Work-item concurrency lock. + // Use lockKey as a fallback when workItemId is absent. Trigger handlers that + // defer PM card materialisation to the worker (e.g. Sentry issue/metric alerts) + // set lockKey to a stable synthetic key (e.g. `sentry:${issueId}`) so that + // duplicate webhook deliveries are blocked even before the real PM card ID is known. + const effectiveLockKey = result.lockKey ?? result.workItemId; + if (effectiveLockKey) { + const lockStatus = await isWorkItemLocked(project.id, effectiveLockKey, result.agentType); if (lockStatus.locked) { result.onBlocked?.(); logger.info(`Skipping ${adapter.type} job — work item already locked`, { source: adapter.type, projectId: project.id, - workItemId: result.workItemId, + workItemId: effectiveLockKey, blockedAgentType: result.agentType, reason: lockStatus.reason, }); @@ -324,7 +341,7 @@ export async function processRouterWebhook( // canary. const classification = await classifyLockState({ projectId: project.id, - workItemId: result.workItemId, + workItemId: effectiveLockKey, agentType: result.agentType, }); const reasonSuffix = lockStatus.reason ?? 'active run exists'; @@ -334,13 +351,13 @@ export async function processRouterWebhook( // observable in production. captureException( new Error( - `wedged work-item lock: projectId=${project.id} workItemId=${result.workItemId} agentType=${result.agentType}`, + `wedged work-item lock: projectId=${project.id} workItemId=${effectiveLockKey} agentType=${result.agentType}`, ), { tags: { source: 'wedged_lock_canary' }, extra: { projectId: project.id, - workItemId: result.workItemId, + workItemId: effectiveLockKey, agentType: result.agentType, reason: lockStatus.reason, }, @@ -407,8 +424,8 @@ export async function processRouterWebhook( // Step 12: Enqueue — job is now durable in Redis const jobId = await addJob(job); - if (result.workItemId) { - markWorkItemEnqueued(project.id, result.workItemId, result.agentType); + if (effectiveLockKey) { + markWorkItemEnqueued(project.id, effectiveLockKey, result.agentType); } if (result.agentType && agentTypeMaxConcurrency !== null) { markRecentlyDispatched(project.id, result.agentType, result.workItemId); diff --git a/src/router/worker-env.ts b/src/router/worker-env.ts index f611583e9..a379c8a62 100644 --- a/src/router/worker-env.ts +++ b/src/router/worker-env.ts @@ -161,19 +161,28 @@ export async function buildWorkerEnvWithProjectId( /** * Extract work-item ID from job data for concurrency lock tracking. * Returns the PM work item identifier (workItemId, issueKey, or triggerResult.workItemId). + * + * For Sentry jobs that deferred PM card materialisation to the worker, the + * trigger result may have `lockKey` set instead of `workItemId`. The lock was + * registered in the router using `lockKey ?? workItemId`, so the compensator + * must clear the same key. */ export function extractWorkItemId(data: CascadeJob): string | undefined { const jobData = data as unknown as { type: string; workItemId?: string; issueKey?: string; - triggerResult?: { workItemId?: string }; + triggerResult?: { workItemId?: string; lockKey?: string }; }; if (jobData.type === 'trello' && jobData.workItemId) return jobData.workItemId; if (jobData.type === 'jira' && jobData.issueKey) return jobData.issueKey; if (jobData.type === 'github') return jobData.triggerResult?.workItemId; if (jobData.type === 'linear') return jobData.triggerResult?.workItemId ?? jobData.workItemId; + // Sentry jobs: lockKey takes priority (set when workItemId is deferred to the worker) + if (jobData.type === 'sentry') { + return jobData.triggerResult?.lockKey ?? jobData.triggerResult?.workItemId; + } // Dashboard jobs (manual-run, retry-run, debug-analysis) if (jobData.workItemId) return jobData.workItemId; return undefined; diff --git a/src/triggers/sentry/alerting-issue.ts b/src/triggers/sentry/alerting-issue.ts index 8e2caf8b0..a7679ac5b 100644 --- a/src/triggers/sentry/alerting-issue.ts +++ b/src/triggers/sentry/alerting-issue.ts @@ -3,8 +3,15 @@ * * Fires the 'alerting' agent when a Sentry issue alert rule triggers. * The payload includes the full event object (exception, stacktrace, breadcrumbs). + * + * PM card materialisation is intentionally deferred to the worker side + * (processSentryWebhook). This means transient PM failures (5xx, DB errors, + * polling exhaustion) surface as BullMQ retries on the worker rather than + * being silently swallowed as non-fatal dispatch errors by processRouterWebhook, + * which would return HTTP 200 to Sentry with no job ever enqueued. */ +import { getAlertsContainerId } from '../../pm/config.js'; import { getSentryIntegrationConfig } from '../../sentry/integration.js'; import type { SentryAugmentedPayload, SentryIssueAlertPayload } from '../../sentry/types.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; @@ -71,22 +78,38 @@ export class SentryIssueAlertTrigger implements TriggerHandler { orgId: sentryConfig.organizationSlug, }); - const workItemId = `sentry:issue:${issueId}`; + // Pre-flight: verify the alerts slot is configured before dispatching. + // Actual PM card creation is deferred to the worker (processSentryWebhook) so + // that transient PM failures surface as BullMQ retries rather than being + // swallowed as non-fatal by processRouterWebhook. + if (!getAlertsContainerId(ctx.project)) { + logger.warn('SentryIssueAlertTrigger: alerts slot not configured, skipping dispatch', { + projectId: ctx.project.id, + source: 'sentry', + reason: 'alerts_slot_missing', + }); + return null; + } return { agentType: 'alerting', agentInput: { triggerEvent: 'alerting:issue-alert', - // Synthesized stable identifier — gives the dashboard work-item view - // a queryable handle and groups multiple investigations of the same - // Sentry issue. Spec 018, AC #12. - workItemId, + // workItemId is intentionally absent here — the worker (processSentryWebhook) + // materialises the PM card and sets it before running the agent. alertIssueId: issueId, alertOrgId: sentryConfig.organizationSlug, alertTitle, alertIssueUrl: issueUrl, }, - workItemId, + // workItemId omitted — worker sets it after materialisation. + // lockKey provides router-level work-item concurrency protection while the + // PM card ID is not yet known. Ensures a second Sentry delivery for the same + // issue cannot enqueue while the first worker is still active. + lockKey: `sentry:${issueId}`, + // coalesceKey uses the Sentry issue ID so rapid re-fires of the same alert + // are coalesced without needing a PM card ID. + coalesceKey: `${ctx.project.id}:sentry:${issueId}`, }; } } diff --git a/src/triggers/sentry/alerting-metric.ts b/src/triggers/sentry/alerting-metric.ts index 70c3a7681..cc6656850 100644 --- a/src/triggers/sentry/alerting-metric.ts +++ b/src/triggers/sentry/alerting-metric.ts @@ -5,8 +5,15 @@ * or warning state (not on resolution). * * Supports a `severity` parameter to filter by minimum severity level. + * + * PM card materialisation is intentionally deferred to the worker side + * (processSentryWebhook). This means transient PM failures (5xx, DB errors, + * polling exhaustion) surface as BullMQ retries on the worker rather than + * being silently swallowed as non-fatal dispatch errors by processRouterWebhook, + * which would return HTTP 200 to Sentry with no job ever enqueued. */ +import { getAlertsContainerId } from '../../pm/config.js'; import { getSentryIntegrationConfig } from '../../sentry/integration.js'; import type { SentryAugmentedPayload, SentryMetricAlertPayload } from '../../sentry/types.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; @@ -75,22 +82,42 @@ export class SentryMetricAlertTrigger implements TriggerHandler { orgId: sentryConfig.organizationSlug, }); - const workItemId = `sentry:metric:${sentryConfig.organizationSlug}:${alertTitle}`; + // Pre-flight: verify the alerts slot is configured before dispatching. + // Actual PM card creation is deferred to the worker (processSentryWebhook) so + // that transient PM failures surface as BullMQ retries rather than being + // swallowed as non-fatal by processRouterWebhook. + if (!getAlertsContainerId(ctx.project)) { + logger.warn('SentryMetricAlertTrigger: alerts slot not configured, skipping dispatch', { + projectId: ctx.project.id, + source: 'sentry', + reason: 'alerts_slot_missing', + }); + return null; + } + + // Stable key grouping metric alerts by org + alert title. + // A rule rename produces a new group — acceptable (noted in comment). + const alertMetricKey = `${sentryConfig.organizationSlug}:${alertTitle}`; return { agentType: 'alerting', agentInput: { triggerEvent: 'alerting:metric-alert', - // Synthesized stable identifier — see SentryIssueAlertTrigger for - // rationale. For metric alerts there's no per-firing event id, so - // the identifier groups by org + alert title (best effort: an alert - // rule rename will produce a new group, which is acceptable). - workItemId, + // workItemId is intentionally absent here — the worker (processSentryWebhook) + // materialises the PM card and sets it before running the agent. + alertMetricKey, alertOrgId: sentryConfig.organizationSlug, alertTitle, alertIssueUrl: innerPayload.data?.web_url, }, - workItemId, + // workItemId omitted — worker sets it after materialisation. + // lockKey provides router-level work-item concurrency protection while + // the PM card ID is not yet known. Uses sentry-metric: prefix to avoid + // collisions with issue alert lock keys (sentry:${issueId}). + lockKey: `sentry-metric:${alertMetricKey}`, + // coalesceKey deduplicates rapid re-fires of the same metric alert + // within the BullMQ coalesce window without requiring a PM card ID. + coalesceKey: `${ctx.project.id}:sentry-metric:${alertMetricKey}`, }; } } diff --git a/src/triggers/sentry/webhook-handler.ts b/src/triggers/sentry/webhook-handler.ts index 67b29bf8d..fa73329dd 100644 --- a/src/triggers/sentry/webhook-handler.ts +++ b/src/triggers/sentry/webhook-handler.ts @@ -6,12 +6,20 @@ * After resolving the trigger result, runs the matched agent via the * shared execution pipeline. * + * PM card materialisation happens here (worker side), not in the trigger + * handler (router side). This ensures transient PM failures (5xx, DB + * errors, polling exhaustion) surface as BullMQ retries on the worker + * instead of being swallowed as non-fatal by processRouterWebhook, which + * would return HTTP 200 to Sentry with no durable job enqueued. + * * Shared utilities used: * - Trigger resolution → ../shared/trigger-resolution.ts * - Agent-type concurrency → ../shared/concurrency.ts * - PM credential scope → ../shared/credential-scope.ts */ +import { AlertSlotMissingError } from '../../integrations/alerting/_shared/types.js'; +import type { SentryAugmentedPayload } from '../../sentry/types.js'; import type { TriggerResult } from '../../types/index.js'; import { startWatchdog } from '../../utils/lifecycle.js'; import { logger } from '../../utils/logging.js'; @@ -69,13 +77,79 @@ export async function processSentryWebhook( // Starting it before the check risks a spurious process.exit(1) if the container // is still alive after a concurrency-blocked job finishes. startWatchdog(pc.project.watchdogTimeoutMs); - return withPMScope(pc.project, () => - runAgentExecutionPipeline(result, pc.project, pc.config, { + return withPMScope(pc.project, async () => { + // Materialise the PM work item on the worker side so that transient PM + // failures (Trello/JIRA/Linear 5xx, DB errors, polling exhaustion) surface + // as BullMQ retries instead of being swallowed as non-fatal dispatch errors + // by processRouterWebhook (which returns HTTP 200 to Sentry, preventing retries). + // + // Both SentryIssueAlertTrigger (alertIssueId) and SentryMetricAlertTrigger + // (alertMetricKey) defer PM card creation to here. The trigger handlers only + // pre-check that the alerts slot is configured; actual PM card creation happens + // here where the error propagates to BullMQ's retry budget. + let resolvedResult = result; + const alertIssueId = + typeof result.agentInput.alertIssueId === 'string' + ? result.agentInput.alertIssueId + : null; + const alertMetricKey = + typeof result.agentInput.alertMetricKey === 'string' + ? result.agentInput.alertMetricKey + : null; + if (!result.workItemId && (alertIssueId || alertMetricKey)) { + const { materializeAlertWorkItem } = await import( + '../../integrations/alerting/_shared/materialize.js' + ); + const { formatSentryCardBody, formatSentryMetricCardBody } = await import( + '../../integrations/alerting/_shared/format.js' + ); + try { + let workItemId: string; + if (alertIssueId) { + const hints = formatSentryCardBody(payload as SentryAugmentedPayload); + workItemId = await materializeAlertWorkItem( + 'sentry', + alertIssueId, + pc.project, + hints, + ); + } else { + // alertMetricKey is guaranteed non-null here (checked above) + const hints = formatSentryMetricCardBody(payload as SentryAugmentedPayload); + workItemId = await materializeAlertWorkItem( + 'sentry-metric', + alertMetricKey as string, + pc.project, + hints, + ); + } + resolvedResult = { + ...result, + workItemId, + agentInput: { ...result.agentInput, workItemId }, + }; + } catch (err) { + if (err instanceof AlertSlotMissingError) { + // Slot was unconfigured between router dispatch and worker execution. + // Treat as a graceful skip (don't retry — the operator must reconfigure). + logger.warn( + 'processSentryWebhook: alerts slot no longer configured, skipping agent run', + { projectId, reason: 'alerts_slot_missing' }, + ); + return; + } + // Transient PM failure (5xx, DB error, polling exhaustion): re-throw so + // BullMQ retries the job rather than silently dropping the alert. + throw err; + } + } + + return runAgentExecutionPipeline(resolvedResult, pc.project, pc.config, { logLabel: 'Sentry agent', skipPrepareForAgent: true, skipHandleFailure: true, - }), - ); + }); + }); }, 'processSentryWebhook', ); diff --git a/src/triggers/shared/agent-execution.ts b/src/triggers/shared/agent-execution.ts index be464fac1..3d67d78ca 100644 --- a/src/triggers/shared/agent-execution.ts +++ b/src/triggers/shared/agent-execution.ts @@ -474,7 +474,7 @@ export async function runAgentExecutionPipeline( } // Pre-flight integration validation - const validation = await validateIntegrations(project.id, agentType); + const validation = await validateIntegrations(project.id, agentType, project); if (!validation.valid) { await notifyValidationFailure( result, diff --git a/src/triggers/shared/integration-validation.ts b/src/triggers/shared/integration-validation.ts index f58d8c693..4d18cec7f 100644 --- a/src/triggers/shared/integration-validation.ts +++ b/src/triggers/shared/integration-validation.ts @@ -8,9 +8,12 @@ import { deriveIntegrations } from '../../agents/capabilities/index.js'; import { resolveAgentDefinition } from '../../agents/definitions/loader.js'; import type { IntegrationCategory } from '../../agents/definitions/schema.js'; +import { getTriggerConfigsByProjectAndAgent } from '../../db/repositories/agentTriggerConfigsRepository.js'; import { getPersonaForAgentType } from '../../github/personas.js'; import { integrationRegistry } from '../../integrations/registry.js'; import type { SCMIntegration } from '../../integrations/scm.js'; +import { getAlertsStatusKey } from '../../pm/config.js'; +import type { ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; export interface ValidationError { @@ -140,6 +143,37 @@ async function validateCategory( return null; } +// ============================================================================ +// PM-config validators +// ============================================================================ + +/** + * Validate that the project has an `alerts` slot configured when any alerting + * trigger is enabled. The slot is required for the materializer to create PM + * cards; without it every Sentry webhook silently skips dispatch. + * + * Only runs when `project` is supplied to `validateIntegrations`. + */ +async function validateAlertsSlot( + projectId: string, + project: ProjectConfig, +): Promise { + const configs = await getTriggerConfigsByProjectAndAgent(projectId, 'alerting'); + const hasEnabledAlertingTrigger = configs.some((c) => c.enabled); + if (!hasEnabledAlertingTrigger) return null; + + if (getAlertsStatusKey(project)) return null; + + const pmType = project.pm?.type ?? 'unknown'; + const slotHint = pmType === 'trello' ? 'lists.alerts' : 'statuses.alerts'; + return { + category: 'pm', + message: + `Alerting trigger is enabled but no \`alerts\` slot is configured for ${pmType}. ` + + `Set ${slotHint} in the project's PM integration config.`, + }; +} + // ============================================================================ // Main validation function // ============================================================================ @@ -148,12 +182,16 @@ async function validateCategory( * Validate all required integrations are configured before agent runs. * Integrations are derived from the agent's required capabilities. * + * When `project` is provided, also validates PM-config requirements such as + * the alerts slot being set when an alerting trigger is enabled. + * * Uses the integrationRegistry to look up integration modules by category, * making validation automatically extensible to new integration categories. */ export async function validateIntegrations( projectId: string, agentType: string, + project?: ProjectConfig, ): Promise { const { required } = await getIntegrationRequirements(agentType); @@ -165,6 +203,15 @@ export async function validateIntegrations( const results = await Promise.all(validationPromises); const errors = results.filter((e): e is ValidationError => e !== null); + // Additional PM-config check: alerts slot required when alerting trigger is enabled. + // Only run for the alerting agent — this slot is irrelevant to implementation/review/manual + // agents, and the Sentry webhook path catches AlertSlotMissingError in the trigger handler + // before this worker-side validation ever runs. + if (project && agentType === 'alerting') { + const alertsSlotError = await validateAlertsSlot(projectId, project); + if (alertsSlotError) errors.push(alertsSlotError); + } + if (errors.length > 0) { logger.warn('Integration validation failed', { projectId, diff --git a/src/triggers/shared/manual-runner.ts b/src/triggers/shared/manual-runner.ts index 1585ed3cc..508641c4b 100644 --- a/src/triggers/shared/manual-runner.ts +++ b/src/triggers/shared/manual-runner.ts @@ -56,6 +56,12 @@ export interface ManualTriggerInput { repoFullName?: string; headSha?: string; modelOverride?: string; + // Comment-trigger fields — required for respond-to-pr-comment to know what was asked + triggerCommentBody?: string; + triggerCommentId?: number; + triggerCommentUrl?: string; + triggerCommentPath?: string; + triggerCommentAuthor?: string; } /** @@ -93,7 +99,7 @@ export async function triggerManualRun( } // Pre-flight integration validation - const validation = await validateIntegrations(input.projectId, input.agentType); + const validation = await validateIntegrations(input.projectId, input.agentType, project); if (!validation.valid) { throw new Error(formatValidationErrors(validation)); } @@ -120,6 +126,11 @@ export async function triggerManualRun( headSha: input.headSha, modelOverride: input.modelOverride, triggerType: 'manual', + triggerCommentBody: input.triggerCommentBody, + triggerCommentId: input.triggerCommentId, + triggerCommentUrl: input.triggerCommentUrl, + triggerCommentPath: input.triggerCommentPath, + triggerCommentAuthor: input.triggerCommentAuthor, project, config, }; diff --git a/src/types/index.ts b/src/types/index.ts index f5e76d86a..fdc894cda 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -68,6 +68,12 @@ export interface AgentInput { alertOrgId?: string; alertTitle?: string; alertIssueUrl?: string; + /** + * Stable key for metric alerts: `${orgSlug}:${alertTitle}`. + * Used as the externalId for the sentry-metric source in the PM materializer. + * Set by SentryMetricAlertTrigger; consumed by processSentryWebhook. + */ + alertMetricKey?: string; [key: string]: unknown; } @@ -110,6 +116,20 @@ export interface TriggerResult { /** Called when the router cannot enqueue the job (work-item lock, concurrency limit). * Allows the trigger handler to undo side-effects like dedup marking. */ onBlocked?: () => void; + /** + * Router-level work-item lock key, independent of `workItemId`. + * + * Set by trigger handlers that defer `workItemId` assignment to the worker + * (e.g. Sentry issue/metric alert triggers that materialise the PM card on + * the worker side). The router uses `lockKey ?? workItemId` for + * `isWorkItemLocked` / `markWorkItemEnqueued` / `clearWorkItemEnqueued` so + * that duplicate webhook deliveries are rejected even before the PM card ID + * is known. + * + * Must be stable across deliveries for the same logical alert — e.g. + * `sentry:${issueId}` for Sentry issue alerts. + */ + lockKey?: string; /** * Coalesce key for PM status-change webhook deduplication. * diff --git a/src/worker-entry.ts b/src/worker-entry.ts index fec8f6eb8..353c41d7b 100644 --- a/src/worker-entry.ts +++ b/src/worker-entry.ts @@ -144,6 +144,11 @@ export interface ManualRunJobData { repoFullName?: string; headSha?: string; modelOverride?: string; + triggerCommentBody?: string; + triggerCommentId?: number; + triggerCommentUrl?: string; + triggerCommentPath?: string; + triggerCommentAuthor?: string; } export interface RetryRunJobData { @@ -194,6 +199,11 @@ export async function processDashboardJob(jobId: string, jobData: DashboardJobDa repoFullName: jobData.repoFullName, headSha: jobData.headSha, modelOverride: jobData.modelOverride, + triggerCommentBody: jobData.triggerCommentBody, + triggerCommentId: jobData.triggerCommentId, + triggerCommentUrl: jobData.triggerCommentUrl, + triggerCommentPath: jobData.triggerCommentPath, + triggerCommentAuthor: jobData.triggerCommentAuthor, }, pc.project, pc.config, diff --git a/tests/integration/db/pr-work-items-external-mapping.test.ts b/tests/integration/db/pr-work-items-external-mapping.test.ts new file mode 100644 index 000000000..94d68e66a --- /dev/null +++ b/tests/integration/db/pr-work-items-external-mapping.test.ts @@ -0,0 +1,119 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { + attachWorkItemId, + claimExternalMapping, + findByExternal, + replaceWorkItemId, +} from '../../../src/db/repositories/prWorkItemsRepository.js'; +import { truncateAll } from '../helpers/db.js'; +import { seedOrg, seedProject } from '../helpers/seed.js'; + +describe('prWorkItemsRepository — external mapping methods (integration)', () => { + beforeEach(async () => { + await truncateAll(); + await seedOrg(); + await seedProject(); + }); + + describe('findByExternal', () => { + it('returns null when no row matches', async () => { + const result = await findByExternal('test-project', 'sentry', 'S1'); + expect(result).toBeNull(); + }); + + it('returns { id, workItemId } when an exact match exists', async () => { + const { rowId } = await claimExternalMapping('test-project', 'sentry', 'S1'); + await attachWorkItemId(rowId, 'trello-card-1'); + + const result = await findByExternal('test-project', 'sentry', 'S1'); + expect(result).not.toBeNull(); + expect(result?.workItemId).toBe('trello-card-1'); + expect(result?.id).toBe(rowId); + }); + }); + + describe('claimExternalMapping', () => { + it('inserts a new row when none exists and returns ownedHere=true with rowId', async () => { + const result = await claimExternalMapping('test-project', 'sentry', 'S1'); + expect(result.ownedHere).toBe(true); + expect(typeof result.rowId).toBe('string'); + + // Verify the row was actually inserted with work_item_id=NULL + const found = await findByExternal('test-project', 'sentry', 'S1'); + expect(found).not.toBeNull(); + expect(found?.workItemId).toBeNull(); + }); + + it('returns ownedHere=false with existing row when conflict occurs', async () => { + // Seed a row first by claiming and attaching + const first = await claimExternalMapping('test-project', 'sentry', 'S1'); + await attachWorkItemId(first.rowId, 'card-existing'); + + // Second claim should detect the conflict + const second = await claimExternalMapping('test-project', 'sentry', 'S1'); + expect(second.ownedHere).toBe(false); + if (!second.ownedHere) { + expect(second.existing.workItemId).toBe('card-existing'); + expect(second.existing.id).toBe(first.rowId); + } + }); + + it('is race-free under simulated concurrency: exactly one claim wins', async () => { + const results = await Promise.all([ + claimExternalMapping('test-project', 'sentry', 'RACE1'), + claimExternalMapping('test-project', 'sentry', 'RACE1'), + claimExternalMapping('test-project', 'sentry', 'RACE1'), + claimExternalMapping('test-project', 'sentry', 'RACE1'), + claimExternalMapping('test-project', 'sentry', 'RACE1'), + ]); + + const owners = results.filter((r) => r.ownedHere); + const nonOwners = results.filter((r) => !r.ownedHere); + + expect(owners).toHaveLength(1); + expect(nonOwners).toHaveLength(4); + + // All non-owners must point at the same winning row + const winnerId = owners[0].rowId; + for (const n of nonOwners) { + if (!n.ownedHere) { + expect(n.existing.id).toBe(winnerId); + } + } + }); + }); + + describe('attachWorkItemId', () => { + it('writes work_item_id into the claimed row', async () => { + const { rowId } = await claimExternalMapping('test-project', 'sentry', 'S2'); + await attachWorkItemId(rowId, 'card-new'); + + const found = await findByExternal('test-project', 'sentry', 'S2'); + expect(found?.workItemId).toBe('card-new'); + }); + }); + + describe('replaceWorkItemId', () => { + it('updates work_item_id when old value matches and returns true', async () => { + const { rowId } = await claimExternalMapping('test-project', 'sentry', 'S3'); + await attachWorkItemId(rowId, 'card-old'); + + const updated = await replaceWorkItemId(rowId, 'card-old', 'card-new'); + expect(updated).toBe(true); + + const found = await findByExternal('test-project', 'sentry', 'S3'); + expect(found?.workItemId).toBe('card-new'); + }); + + it('returns false and leaves row unchanged when old value is stale', async () => { + const { rowId } = await claimExternalMapping('test-project', 'sentry', 'S4'); + await attachWorkItemId(rowId, 'card-current'); + + const updated = await replaceWorkItemId(rowId, 'card-stale', 'card-new'); + expect(updated).toBe(false); + + const found = await findByExternal('test-project', 'sentry', 'S4'); + expect(found?.workItemId).toBe('card-current'); + }); + }); +}); diff --git a/tests/integration/db/pr-work-items-external-source.test.ts b/tests/integration/db/pr-work-items-external-source.test.ts new file mode 100644 index 000000000..ea2ac5142 --- /dev/null +++ b/tests/integration/db/pr-work-items-external-source.test.ts @@ -0,0 +1,86 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { getDb } from '../../../src/db/client.js'; +import { prWorkItems } from '../../../src/db/schema/index.js'; +import { truncateAll } from '../helpers/db.js'; +import { seedOrg, seedProject } from '../helpers/seed.js'; + +describe('pr_work_items external source columns (integration)', () => { + beforeEach(async () => { + await truncateAll(); + await seedOrg(); + await seedProject(); + }); + + it('external_source and external_id columns exist and accept NULL', async () => { + const db = getDb(); + const [row] = await db + .insert(prWorkItems) + .values({ + projectId: 'test-project', + repoFullName: 'owner/repo', + workItemId: 'card-1', + // external_source and external_id intentionally omitted → NULL + }) + .returning(); + + expect(row.externalSource).toBeNull(); + expect(row.externalId).toBeNull(); + }); + + it('partial UNIQUE index blocks duplicate (project_id, external_source, external_id)', async () => { + const db = getDb(); + await db.insert(prWorkItems).values({ + projectId: 'test-project', + externalSource: 'sentry', + externalId: 'S1', + }); + + await expect( + db.insert(prWorkItems).values({ + projectId: 'test-project', + externalSource: 'sentry', + externalId: 'S1', + }), + ).rejects.toMatchObject({ cause: { code: '23505' } }); + }); + + it('partial UNIQUE index allows multiple rows where external_source IS NULL', async () => { + const db = getDb(); + await db.insert(prWorkItems).values({ projectId: 'test-project' }); + await db.insert(prWorkItems).values({ projectId: 'test-project' }); + await db.insert(prWorkItems).values({ projectId: 'test-project' }); + + const rows = await db + .select() + .from(prWorkItems) + .then((all) => + all.filter((r) => r.projectId === 'test-project' && r.externalSource === null), + ); + expect(rows.length).toBeGreaterThanOrEqual(3); + }); + + it('partial UNIQUE index allows the same external_id across different projects', async () => { + await seedProject({ id: 'project-2', repo: 'owner/repo2' }); + const db = getDb(); + await db + .insert(prWorkItems) + .values({ projectId: 'test-project', externalSource: 'sentry', externalId: 'S1' }); + await expect( + db + .insert(prWorkItems) + .values({ projectId: 'project-2', externalSource: 'sentry', externalId: 'S1' }), + ).resolves.not.toThrow(); + }); + + it('partial UNIQUE index allows the same external_id across different sources within a project', async () => { + const db = getDb(); + await db + .insert(prWorkItems) + .values({ projectId: 'test-project', externalSource: 'sentry', externalId: 'S1' }); + await expect( + db + .insert(prWorkItems) + .values({ projectId: 'test-project', externalSource: 'pagerduty', externalId: 'S1' }), + ).resolves.not.toThrow(); + }); +}); diff --git a/tests/unit/cli/session/finish.test.ts b/tests/unit/cli/session/finish.test.ts index 607f6e624..d010038ed 100644 --- a/tests/unit/cli/session/finish.test.ts +++ b/tests/unit/cli/session/finish.test.ts @@ -5,8 +5,10 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockExecSync = vi.fn(); +const mockExecFileSync = vi.fn(); vi.mock('node:child_process', () => ({ execSync: (...args: unknown[]) => mockExecSync(...args), + execFileSync: (...args: unknown[]) => mockExecFileSync(...args), })); vi.mock('../../../../src/github/client.js', () => ({ @@ -28,6 +30,7 @@ describe('session finish CLI', () => { process.env.CASCADE_FINISH_HOOKS = JSON.stringify({ requiresPushedChanges: true }); process.env.CASCADE_INITIAL_HEAD_SHA = 'a'.repeat(40); process.env.CASCADE_AGENT_TYPE = 'respond-to-review'; + delete process.env.CASCADE_PR_BRANCH; }); afterEach(() => { @@ -37,6 +40,7 @@ describe('session finish CLI', () => { }); it('writes pushed-changes sidecar after successful validation', async () => { + mockExecFileSync.mockReset(); mockExecSync.mockImplementation((cmd: string) => { if (cmd.includes('status --porcelain')) return ''; if (cmd.includes('rev-list')) return '0'; @@ -74,6 +78,7 @@ describe('session finish CLI', () => { }); it('fails when pushed changes were not actually made', async () => { + mockExecFileSync.mockReset(); mockExecSync.mockImplementation((cmd: string) => { if (cmd.includes('status --porcelain')) return ''; if (cmd.includes('rev-list')) return '0'; diff --git a/tests/unit/db/repositories/prWorkItemsRepository.test.ts b/tests/unit/db/repositories/prWorkItemsRepository.test.ts index bbf2d15ae..7bafbfd4d 100644 --- a/tests/unit/db/repositories/prWorkItemsRepository.test.ts +++ b/tests/unit/db/repositories/prWorkItemsRepository.test.ts @@ -15,6 +15,8 @@ vi.mock('../../../../src/db/schema/index.js', () => ({ prUrl: 'prUrl', prTitle: 'prTitle', updatedAt: 'updatedAt', + externalSource: 'externalSource', + externalId: 'externalId', }, agentRuns: { id: 'id', @@ -34,7 +36,10 @@ vi.mock('../../../../src/db/repositories/joinHelpers.js', () => ({ })); import { + attachWorkItemId, + claimExternalMapping, createWorkItem, + findByExternal, linkPRToWorkItem, listPRsForOrg, listPRsForProject, @@ -42,6 +47,7 @@ import { listUnifiedWorkForProject, listWorkItems, lookupWorkItemForPR, + replaceWorkItemId, } from '../../../../src/db/repositories/prWorkItemsRepository.js'; // --------------------------------------------------------------------------- @@ -65,9 +71,11 @@ function createMockChain(returningValue: unknown[] = []) { // insert chain chain.onConflictDoUpdate = vi.fn().mockReturnValue({ returning: chain.returning }); + chain.onConflictDoNothing = vi.fn().mockReturnValue({ returning: chain.returning }); chain.values = vi.fn().mockReturnValue({ returning: chain.returning, onConflictDoUpdate: chain.onConflictDoUpdate, + onConflictDoNothing: chain.onConflictDoNothing, }); return chain; @@ -913,4 +921,99 @@ describe('prWorkItemsRepository', () => { expect(qc.leftJoin).toHaveBeenCalledWith(expect.anything(), 'mock-join-condition'); }); }); + + // ========================================================================== + // Alert-materialization repository methods (spec 019) + // ========================================================================== + + describe('findByExternal', () => { + it('returns the row when a matching external mapping exists', async () => { + // Use mockResolvedValueOnce on the existing function — reassigning chain.limit + // would create a new reference that chain.where() has already captured. + chain.limit.mockResolvedValueOnce([{ id: 'row-1', workItemId: 'wi-abc' }]); + + const result = await findByExternal('proj-1', 'sentry', 'ext-123'); + + expect(result).toEqual({ id: 'row-1', workItemId: 'wi-abc' }); + expect(mockDb.select).toHaveBeenCalledTimes(1); + }); + + it('returns null when no matching row exists', async () => { + chain.limit = vi.fn().mockResolvedValue([]); + mockDb.select = vi.fn().mockReturnValue({ from: chain.from }); + + const result = await findByExternal('proj-1', 'sentry', 'ext-missing'); + + expect(result).toBeNull(); + }); + + it('queries with the correct project, source, and externalId', async () => { + chain.limit = vi.fn().mockResolvedValue([]); + mockDb.select = vi.fn().mockReturnValue({ from: chain.from }); + + await findByExternal('my-project', 'sentry', 'sentry-999'); + + expect(mockDb.select).toHaveBeenCalledTimes(1); + }); + }); + + describe('claimExternalMapping', () => { + it('returns ownedHere=true with the new rowId when insert wins', async () => { + // Mutate the existing returning function — reassigning would break the chain reference. + chain.returning.mockResolvedValueOnce([{ id: 'new-row-id' }]); + + const result = await claimExternalMapping('proj-1', 'sentry', 'ext-new'); + + expect(result).toEqual({ ownedHere: true, rowId: 'new-row-id' }); + expect(mockDb.insert).toHaveBeenCalledTimes(1); + expect(mockDb.select).not.toHaveBeenCalled(); + }); + + it('returns ownedHere=false with existing row when insert conflicts', async () => { + // Insert returns [] (default) → conflict path. findByExternal follows up with SELECT. + chain.limit.mockResolvedValueOnce([{ id: 'existing-row', workItemId: null }]); + + const result = await claimExternalMapping('proj-1', 'sentry', 'ext-conflict'); + + expect(result).toEqual({ + ownedHere: false, + existing: { id: 'existing-row', workItemId: null }, + }); + }); + + it('throws when insert conflicts but no winning row is found (unreachable invariant)', async () => { + // Insert returns [] (default) → conflict. findByExternal also returns [] → invariant throw. + await expect(claimExternalMapping('proj-1', 'sentry', 'ext-ghost')).rejects.toThrow( + /claimExternalMapping.*conflict but no row found/, + ); + }); + }); + + describe('attachWorkItemId', () => { + it('updates the row with the given workItemId', async () => { + await attachWorkItemId('row-abc', 'wi-xyz'); + + expect(mockDb.update).toHaveBeenCalledTimes(1); + expect(chain.set).toHaveBeenCalledWith(expect.objectContaining({ workItemId: 'wi-xyz' })); + }); + }); + + describe('replaceWorkItemId', () => { + it('returns true when the conditional update matches', async () => { + // Mutate the existing returning function rather than reassigning the chain references. + chain.returning.mockResolvedValueOnce([{ id: 'row-abc' }]); + + const result = await replaceWorkItemId('row-abc', 'old-wi', 'new-wi'); + + expect(result).toBe(true); + expect(chain.set).toHaveBeenCalledWith(expect.objectContaining({ workItemId: 'new-wi' })); + }); + + it('returns false when the conditional update finds no matching row', async () => { + // Default chain.returning resolves to [] — no row updated + const result = await replaceWorkItemId('row-abc', 'stale-wi', 'new-wi'); + + expect(result).toBe(false); + }); + }); }); diff --git a/tests/unit/integrations/alerting/format-sentry.test.ts b/tests/unit/integrations/alerting/format-sentry.test.ts new file mode 100644 index 000000000..72175dc94 --- /dev/null +++ b/tests/unit/integrations/alerting/format-sentry.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest'; +import { formatSentryCardBody } from '../../../../src/integrations/alerting/_shared/format.js'; +import type { SentryAugmentedPayload } from '../../../../src/sentry/types.js'; + +const fixturePayload: SentryAugmentedPayload = { + resource: 'event_alert', + cascadeProjectId: 'test-project', + payload: { + action: 'triggered', + data: { + event: { + issue_id: '117972276', + web_url: 'https://sentry.io/organizations/acme/issues/117972276/', + title: 'TypeError: Cannot read properties of undefined', + timestamp: '2026-05-06T21:03:39Z', + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'x')", + stacktrace: { + frames: [ + { + filename: 'src/router/index.ts', + function: 'processWebhook', + lineno: 142, + in_app: true, + }, + { + filename: 'node_modules/express/lib/router/index.js', + function: 'Layer.handle', + lineno: 95, + in_app: false, + }, + ], + }, + }, + ], + }, + }, + triggered_rule: 'My Alert Rule', + issue_alert: { + title: 'My Alert Rule', + }, + }, + }, +}; + +describe('formatSentryCardBody', () => { + it('produces a title prefixed with [Sentry]', () => { + const result = formatSentryCardBody(fixturePayload); + expect(result.title).toMatch(/^\[Sentry\]/); + }); + + it('includes the Sentry issue permalink in the description', () => { + const result = formatSentryCardBody(fixturePayload); + expect(result.descriptionMarkdown).toContain('sentry.io'); + }); + + it('includes the alert rule / title in the title or description', () => { + const result = formatSentryCardBody(fixturePayload); + const combined = result.title + result.descriptionMarkdown; + expect(combined).toContain('My Alert Rule'); + }); + + it('includes the timestamp (first-seen) in the description', () => { + const result = formatSentryCardBody(fixturePayload); + // Flexible check — the timestamp might be formatted differently + expect(result.descriptionMarkdown).toMatch(/2026/); + }); + + it('includes the top in-app stack frame in the description', () => { + const result = formatSentryCardBody(fixturePayload); + expect(result.descriptionMarkdown).toContain('processWebhook'); + }); +}); diff --git a/tests/unit/integrations/alerting/materialize.test.ts b/tests/unit/integrations/alerting/materialize.test.ts new file mode 100644 index 000000000..b13e0a6c3 --- /dev/null +++ b/tests/unit/integrations/alerting/materialize.test.ts @@ -0,0 +1,299 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + AlertSlotMissingError, + MaterializationRetryExhausted, +} from '../../../../src/integrations/alerting/_shared/types.js'; + +// ── mock the repo ───────────────────────────────────────────────────────────── +const mockFindByExternal = vi.fn(); +const mockClaimExternalMapping = vi.fn(); +const mockAttachWorkItemId = vi.fn(); +const mockReplaceWorkItemId = vi.fn(); +const mockDeleteExternalMappingClaim = vi.fn(); + +vi.mock('../../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + findByExternal: (...a: unknown[]) => mockFindByExternal(...a), + claimExternalMapping: (...a: unknown[]) => mockClaimExternalMapping(...a), + attachWorkItemId: (...a: unknown[]) => mockAttachWorkItemId(...a), + replaceWorkItemId: (...a: unknown[]) => mockReplaceWorkItemId(...a), + deleteExternalMappingClaim: (...a: unknown[]) => mockDeleteExternalMappingClaim(...a), +})); + +// ── mock the PM registry ────────────────────────────────────────────────────── +const mockCreateWorkItem = vi.fn(); +const mockGetWorkItem = vi.fn(); +const mockAddLabel = vi.fn(); +const mockMoveWorkItem = vi.fn(); + +const fakePMProvider = { + createWorkItem: (...a: unknown[]) => mockCreateWorkItem(...a), + getWorkItem: (...a: unknown[]) => mockGetWorkItem(...a), + addLabel: (...a: unknown[]) => mockAddLabel(...a), + moveWorkItem: (...a: unknown[]) => mockMoveWorkItem(...a), +}; + +vi.mock('../../../../src/pm/registry.js', () => ({ + pmRegistry: { createProvider: () => fakePMProvider }, +})); + +import { materializeAlertWorkItem } from '../../../../src/integrations/alerting/_shared/materialize.js'; +import type { ProjectConfig } from '../../../../src/types/index.js'; + +// ── fixtures ────────────────────────────────────────────────────────────────── + +function makeTrelloProject(listsAlerts?: string, labelsAlert?: string): ProjectConfig { + return { + id: 'test-project', + pm: { type: 'trello' }, + trello: { + boardId: 'board-1', + lists: { todo: 'list-todo', ...(listsAlerts ? { alerts: listsAlerts } : {}) }, + labels: labelsAlert ? { 'cascade-alert': labelsAlert } : {}, + }, + } as unknown as ProjectConfig; +} + +const defaultHints = { title: '[Sentry] Test Alert', descriptionMarkdown: 'some description' }; + +// ── tests ───────────────────────────────────────────────────────────────────── + +describe('materializeAlertWorkItem', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAttachWorkItemId.mockResolvedValue(undefined); + mockReplaceWorkItemId.mockResolvedValue(true); + mockDeleteExternalMappingClaim.mockResolvedValue(undefined); + mockAddLabel.mockResolvedValue(undefined); + mockMoveWorkItem.mockResolvedValue(undefined); + }); + + it('returns existing native id when a healthy mapping is present (no createWorkItem call)', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue({ id: 'row-1', workItemId: 'card-existing' }); + mockGetWorkItem.mockResolvedValue({ + id: 'card-existing', + title: 'x', + description: '', + url: '', + labels: [], + }); + + const result = await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(result).toBe('card-existing'); + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + }); + + it('creates and attaches when no mapping exists', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-new', + title: 'x', + description: '', + url: '', + labels: [], + }); + + const result = await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(result).toBe('card-new'); + expect(mockCreateWorkItem).toHaveBeenCalledWith( + expect.objectContaining({ containerId: 'list-alerts', title: '[Sentry] Test Alert' }), + ); + expect(mockAttachWorkItemId).toHaveBeenCalledWith('row-1', 'card-new'); + }); + + it('returns the winning concurrent claim result when ownedHere=false and work_item_id is already set', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ + ownedHere: false, + existing: { id: 'row-winner', workItemId: 'card-winner' }, + }); + + const result = await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(result).toBe('card-winner'); + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + }); + + it('polls and returns winner work_item_id when concurrent winner row has workItemId=null initially', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal + .mockResolvedValueOnce(null) // initial lookup + .mockResolvedValueOnce({ id: 'row-winner', workItemId: null }) // first poll + .mockResolvedValueOnce({ id: 'row-winner', workItemId: 'card-winner' }); // second poll + mockClaimExternalMapping.mockResolvedValue({ + ownedHere: false, + existing: { id: 'row-winner', workItemId: null }, + }); + + const result = await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(result).toBe('card-winner'); + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + }, 15000); + + it('throws MaterializationRetryExhausted when polling for winner work_item_id exhausts budget', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue({ id: 'row-winner', workItemId: null }); + mockClaimExternalMapping.mockResolvedValue({ + ownedHere: false, + existing: { id: 'row-winner', workItemId: null }, + }); + + await expect( + materializeAlertWorkItem('sentry', 'S1', project, defaultHints), + ).rejects.toBeInstanceOf(MaterializationRetryExhausted); + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + }, 30000); + + it('lazy-heals on PM 404: creates a new card and replaces the mapping', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue({ id: 'row-1', workItemId: 'card-stale' }); + mockGetWorkItem.mockRejectedValue(new Error('Trello API error 404 for /cards/card-stale')); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-new', + title: 'x', + description: '', + url: '', + labels: [], + }); + + const result = await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(result).toBe('card-new'); + expect(mockReplaceWorkItemId).toHaveBeenCalledWith('row-1', 'card-stale', 'card-new'); + }); + + it('propagates terminal PM errors untouched and does not call attachWorkItemId', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockRejectedValue(new Error('PM 500 Internal Server Error')); + + await expect(materializeAlertWorkItem('sentry', 'S1', project, defaultHints)).rejects.toThrow( + 'PM 500', + ); + expect(mockAttachWorkItemId).not.toHaveBeenCalled(); + // Claim row must be cleaned up so the next delivery can reclaim it + expect(mockDeleteExternalMappingClaim).toHaveBeenCalledWith('row-1'); + }); + + it('cleans up stale claim row when createWorkItem throws so the next delivery can reclaim', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-stale' }); + mockCreateWorkItem.mockRejectedValue(new Error('PM 503')); + + await expect(materializeAlertWorkItem('sentry', 'S1', project, defaultHints)).rejects.toThrow( + 'PM 503', + ); + expect(mockDeleteExternalMappingClaim).toHaveBeenCalledWith('row-stale'); + }); + + it('attachWorkItemId is called before addLabel and moveWorkItem', async () => { + const project = makeTrelloProject('list-alerts', 'lbl-cascade-alert'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-new', + title: 'x', + description: '', + url: '', + labels: [], + }); + + const callOrder: string[] = []; + mockAttachWorkItemId.mockImplementation(async () => { + callOrder.push('attachWorkItemId'); + }); + mockAddLabel.mockImplementation(async () => { + callOrder.push('addLabel'); + }); + mockMoveWorkItem.mockImplementation(async () => { + callOrder.push('moveWorkItem'); + }); + + await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(callOrder).toEqual(['attachWorkItemId', 'addLabel', 'moveWorkItem']); + }); + + it('lazy-heal: returns canonical mapping id when replaceWorkItemId CAS fails (concurrent heal)', async () => { + const project = makeTrelloProject('list-alerts'); + // Initial lookup finds stale card + mockFindByExternal + .mockResolvedValueOnce({ id: 'row-1', workItemId: 'card-stale' }) + // Re-read after CAS loss returns the winner's canonical id + .mockResolvedValueOnce({ id: 'row-1', workItemId: 'card-canonical' }); + mockGetWorkItem.mockRejectedValue(new Error('404 Not Found')); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-orphan', + title: 'x', + description: '', + url: '', + labels: [], + }); + // CAS loses — another webhook already healed the row + mockReplaceWorkItemId.mockResolvedValue(false); + + const result = await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + // Must return the persisted canonical id, not the orphan card we just created + expect(result).toBe('card-canonical'); + }); + + it('applies the configured alert label when getAlertLabelId returns a value', async () => { + const project = makeTrelloProject('list-alerts', 'lbl-cascade-alert'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-new', + title: 'x', + description: '', + url: '', + labels: [], + }); + + await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(mockAddLabel).toHaveBeenCalledWith('card-new', 'lbl-cascade-alert'); + }); + + it('skips label application when getAlertLabelId returns undefined', async () => { + const project = makeTrelloProject('list-alerts'); // no label configured + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-new', + title: 'x', + description: '', + url: '', + labels: [], + }); + + await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(mockAddLabel).not.toHaveBeenCalled(); + }); + + it('moves to alerts state via moveWorkItem when getAlertsStatusKey === "alerts"', async () => { + const project = makeTrelloProject('list-alerts'); + mockFindByExternal.mockResolvedValue(null); + mockClaimExternalMapping.mockResolvedValue({ ownedHere: true, rowId: 'row-1' }); + mockCreateWorkItem.mockResolvedValue({ + id: 'card-new', + title: 'x', + description: '', + url: '', + labels: [], + }); + + await materializeAlertWorkItem('sentry', 'S1', project, defaultHints); + expect(mockMoveWorkItem).toHaveBeenCalledWith('card-new', 'list-alerts'); + }); + + it('throws AlertSlotMissingError when getAlertsContainerId returns undefined', async () => { + const project = makeTrelloProject(); // no lists.alerts configured + await expect( + materializeAlertWorkItem('sentry', 'S1', project, defaultHints), + ).rejects.toBeInstanceOf(AlertSlotMissingError); + expect(mockFindByExternal).not.toHaveBeenCalled(); + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/no-synthetic-sentry-issue-id.test.ts b/tests/unit/no-synthetic-sentry-issue-id.test.ts new file mode 100644 index 000000000..5db79ff22 --- /dev/null +++ b/tests/unit/no-synthetic-sentry-issue-id.test.ts @@ -0,0 +1,46 @@ +/** + * Repo-wide regression pin: no code path in src/ constructs the synthetic + * sentry:issue: workItemId prefix that was removed in spec 019 plan 3. + * + * Plan 3 removed it from the trigger module; this test broadens the scope + * to the entire src/ tree so any future regression is caught at CI time. + */ + +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { extname, join, resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +const SRC_ROOT = resolve(import.meta.dirname, '../../src'); + +function collectTsFiles(dir: string): string[] { + const results: string[] = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + const s = statSync(full); + if (s.isDirectory()) { + results.push(...collectTsFiles(full)); + } else if (s.isFile() && extname(entry) === '.ts' && !entry.endsWith('.test.ts')) { + results.push(full); + } + } + return results; +} + +describe('no-synthetic-sentry-issue-id (repo-wide static)', () => { + it('no .ts file under src/ contains the string literal sentry:issue:', () => { + const files = collectTsFiles(SRC_ROOT); + const offenders: string[] = []; + for (const file of files) { + const content = readFileSync(file, 'utf8'); + if (content.includes('sentry:issue:')) { + const lines = content.split('\n'); + for (let i = 0; i < lines.length; i++) { + if (lines[i].includes('sentry:issue:')) { + offenders.push(`${file.replace(SRC_ROOT, 'src')}:${i + 1}`); + } + } + } + } + expect(offenders, `Found 'sentry:issue:' in:\n${offenders.join('\n')}`).toHaveLength(0); + }); +}); diff --git a/tests/unit/pm/config-alert-accessors.test.ts b/tests/unit/pm/config-alert-accessors.test.ts new file mode 100644 index 000000000..554287044 --- /dev/null +++ b/tests/unit/pm/config-alert-accessors.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it } from 'vitest'; +import { + getAlertLabelId, + getAlertsContainerId, + getAlertsStatusKey, +} 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', alerts: 'list-alerts' }, + labels: { 'cascade-alert': 'lbl-alert' }, + }, + ...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', alerts: 'In Triage' }, + labels: { cascadeAlert: 'cascade-alert' }, + }, + ...overrides, + } as unknown as ProjectConfig; +} + +function makeLinearProject(overrides: Record = {}): ProjectConfig { + return { + id: 'p1', + pm: { type: 'linear' }, + linear: { + teamId: 'team-1', + statuses: { todo: 'state-todo', alerts: 'state-triage' }, + labels: { cascadeAlert: 'label-uuid' }, + }, + ...overrides, + } as unknown as ProjectConfig; +} + +describe('getAlertsContainerId', () => { + it('returns Trello list ID from project.trello.lists.alerts', () => { + expect(getAlertsContainerId(makeTrelloProject())).toBe('list-alerts'); + }); + + it('returns JIRA project key for JIRA projects (container = projectKey, not a status)', () => { + expect(getAlertsContainerId(makeJiraProject())).toBe('PROJ'); + }); + + it('returns Linear team ID for Linear projects', () => { + expect(getAlertsContainerId(makeLinearProject())).toBe('team-1'); + }); + + it('returns undefined when no PM config is present', () => { + const project = { id: 'p1', pm: undefined } as unknown as ProjectConfig; + expect(getAlertsContainerId(project)).toBeUndefined(); + }); + + it('returns undefined when alerts slot is not configured (Trello)', () => { + const project = makeTrelloProject({ + trello: { boardId: 'b1', lists: { todo: 'l1' }, labels: {} }, + }); + // Trello container IS the alerts list — if missing, return undefined + expect(getAlertsContainerId(project)).toBeUndefined(); + }); + + it('returns undefined for JIRA projects when statuses.alerts is not configured', () => { + // Without statuses.alerts, creating an issue would land in the project default + // state and then fail pre-flight validation, leaving an alert card outside the + // required alerts slot. getAlertsContainerId must gate on statuses.alerts. + const project = makeJiraProject({ + jira: { + projectKey: 'PROJ', + baseUrl: 'https://acme.atlassian.net', + statuses: { todo: 'To Do' }, + labels: {}, + }, + }); + expect(getAlertsContainerId(project)).toBeUndefined(); + }); + + it('returns undefined for Linear projects when statuses.alerts is not configured', () => { + const project = makeLinearProject({ + linear: { teamId: 'team-1', statuses: { todo: 'state-todo' } }, + }); + expect(getAlertsContainerId(project)).toBeUndefined(); + }); +}); + +describe('getAlertLabelId', () => { + it('returns Trello label ID from labels["cascade-alert"]', () => { + expect(getAlertLabelId(makeTrelloProject())).toBe('lbl-alert'); + }); + + it('returns JIRA label string from labels.cascadeAlert', () => { + expect(getAlertLabelId(makeJiraProject())).toBe('cascade-alert'); + }); + + it('returns Linear label ID from labels.cascadeAlert', () => { + expect(getAlertLabelId(makeLinearProject())).toBe('label-uuid'); + }); + + it('returns undefined when label slot is not configured', () => { + const p1 = makeTrelloProject({ trello: { boardId: 'b1', lists: {}, labels: {} } }); + expect(getAlertLabelId(p1)).toBeUndefined(); + + const p2 = makeJiraProject({ + jira: { projectKey: 'P', baseUrl: 'https://x', statuses: {}, labels: {} }, + }); + expect(getAlertLabelId(p2)).toBeUndefined(); + + const p3 = makeLinearProject({ linear: { teamId: 't1', statuses: {} } }); + expect(getAlertLabelId(p3)).toBeUndefined(); + }); +}); + +describe('getAlertsStatusKey', () => { + it('returns "alerts" when statuses.alerts is configured (JIRA)', () => { + expect(getAlertsStatusKey(makeJiraProject())).toBe('alerts'); + }); + + it('returns "alerts" when statuses.alerts is configured (Linear)', () => { + expect(getAlertsStatusKey(makeLinearProject())).toBe('alerts'); + }); + + it('returns "alerts" when lists.alerts is configured (Trello)', () => { + expect(getAlertsStatusKey(makeTrelloProject())).toBe('alerts'); + }); + + it('returns undefined when alerts slot is not configured', () => { + const p = makeTrelloProject({ trello: { boardId: 'b1', lists: { todo: 'l1' }, labels: {} } }); + expect(getAlertsStatusKey(p)).toBeUndefined(); + }); +}); diff --git a/tests/unit/pm/config-alert-slot.test.ts b/tests/unit/pm/config-alert-slot.test.ts new file mode 100644 index 000000000..e43a131fe --- /dev/null +++ b/tests/unit/pm/config-alert-slot.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, expectTypeOf, it } from 'vitest'; +import { + type JiraIntegrationConfig, + jiraConfigSchema, +} from '../../../src/integrations/pm/jira/config-schema.js'; +import { + type LinearIntegrationConfig, + linearConfigSchema, +} from '../../../src/integrations/pm/linear/config-schema.js'; +import { trelloConfigSchema } from '../../../src/integrations/pm/trello/config-schema.js'; + +describe('PM config schemas — alerts slot', () => { + it('trelloConfigSchema accepts lists.alerts and labels.cascade-alert without error', () => { + const result = trelloConfigSchema.safeParse({ + boardId: 'b1', + lists: { alerts: 'list-id-alerts', todo: 'list-id-todo' }, + labels: { 'cascade-alert': 'lbl-id' }, + }); + expect(result.success).toBe(true); + }); + + it('jiraConfigSchema accepts statuses.alerts and labels.cascadeAlert', () => { + const result = jiraConfigSchema.safeParse({ + projectKey: 'P', + baseUrl: 'https://acme.atlassian.net', + statuses: { alerts: 'In Triage', todo: 'To Do' }, + labels: { cascadeAlert: 'cascade-alert' }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.labels?.cascadeAlert).toBe('cascade-alert'); + } + }); + + it('linearConfigSchema accepts statuses.alerts and labels.cascadeAlert', () => { + const result = linearConfigSchema.safeParse({ + teamId: 'team-1', + statuses: { alerts: 'state-uuid-triage', todo: 'state-uuid-todo' }, + labels: { cascadeAlert: 'label-uuid' }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.labels?.cascadeAlert).toBe('label-uuid'); + } + }); + + it('JiraIntegrationConfig labels has optional cascadeAlert field', () => { + expectTypeOf().toEqualTypeOf< + | { + processing?: string; + processed?: string; + error?: string; + readyToProcess?: string; + cascadeAlert?: string; + } + | undefined + >(); + }); + + it('LinearIntegrationConfig labels has optional cascadeAlert field', () => { + expectTypeOf().toEqualTypeOf< + | { + processing?: string; + processed?: string; + error?: string; + readyToProcess?: string; + auto?: string; + cascadeAlert?: string; + } + | undefined + >(); + }); +}); diff --git a/tests/unit/triggers/no-synthetic-sentry-id.test.ts b/tests/unit/triggers/no-synthetic-sentry-id.test.ts new file mode 100644 index 000000000..eb410c990 --- /dev/null +++ b/tests/unit/triggers/no-synthetic-sentry-id.test.ts @@ -0,0 +1,14 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +describe('no-synthetic-sentry-id (static)', () => { + it('src/triggers/sentry/ contains no template or literal starting with sentry:issue:', () => { + const alertingFile = resolve( + import.meta.dirname, + '../../../src/triggers/sentry/alerting-issue.ts', + ); + const content = readFileSync(alertingFile, 'utf8'); + expect(content).not.toMatch(/sentry:issue:/); + }); +}); diff --git a/tests/unit/triggers/sentry-alerting.test.ts b/tests/unit/triggers/sentry-alerting.test.ts index 4e9933cb5..39f0d66cd 100644 --- a/tests/unit/triggers/sentry-alerting.test.ts +++ b/tests/unit/triggers/sentry-alerting.test.ts @@ -8,6 +8,11 @@ vi.mock('../../../src/sentry/integration.js', () => ({ getSentryIntegrationConfig: vi.fn(), })); +vi.mock('../../../src/pm/config.js', () => ({ + getAlertsContainerId: vi.fn(), +})); + +import { getAlertsContainerId } from '../../../src/pm/config.js'; import { getSentryIntegrationConfig } from '../../../src/sentry/integration.js'; import { SentryIssueAlertTrigger } from '../../../src/triggers/sentry/alerting-issue.js'; import { SentryMetricAlertTrigger } from '../../../src/triggers/sentry/alerting-metric.js'; @@ -15,7 +20,21 @@ import { checkTriggerEnabledWithParams } from '../../../src/triggers/shared/trig import type { TriggerContext } from '../../../src/types/index.js'; import { createMockProject } from '../../helpers/factories.js'; -const mockProject = createMockProject(); +// Materialisation is deferred to the worker (processSentryWebhook) — neither trigger +// calls materializeAlertWorkItem. Project must have lists.alerts configured so +// the pre-flight getAlertsContainerId check passes. +const mockProject = createMockProject({ + trello: { + boardId: 'board123', + lists: { + splitting: 'split-list', + planning: 'plan-list', + todo: 'todo-list', + alerts: 'alerts-list', + }, + labels: {}, + }, +}); const sentryConfig = { organizationSlug: 'my-org' }; @@ -97,6 +116,8 @@ describe('SentryIssueAlertTrigger', () => { vi.resetAllMocks(); vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, parameters: {} }); vi.mocked(getSentryIntegrationConfig).mockResolvedValue(sentryConfig); + // Issue alert trigger checks getAlertsContainerId before dispatching + vi.mocked(getAlertsContainerId).mockReturnValue('alerts-list'); trigger = new SentryIssueAlertTrigger(); }); @@ -223,29 +244,22 @@ describe('SentryIssueAlertTrigger', () => { expect(result?.agentInput?.alertIssueUrl).toBe(issueUrl); }); - // --- Spec 018 / plan 2: synthesized stable workItemId for sentry runs --- + // --- Spec 019 (review feedback): workItemId is deferred to the worker --- - it('synthesizes a stable workItemId from alertIssueId (spec 018)', async () => { + it('does NOT set workItemId — materialisation is deferred to processSentryWebhook', async () => { const result = await trigger.handle(makeSentryIssueAlertCtx()); - expect(result?.agentInput?.workItemId).toBe('sentry:issue:issue-42'); - expect(result?.workItemId).toBe('sentry:issue:issue-42'); + expect(result?.workItemId).toBeUndefined(); + expect(result?.agentInput?.workItemId).toBeUndefined(); }); - it('produces the same workItemId for two dispatches against the same issue', async () => { - const a = await trigger.handle(makeSentryIssueAlertCtx()); - const b = await trigger.handle(makeSentryIssueAlertCtx()); - expect(a?.agentInput?.workItemId).toBe(b?.agentInput?.workItemId); - expect(a?.workItemId).toBe(b?.workItemId); - expect(a?.agentInput?.workItemId).toBe('sentry:issue:issue-42'); - expect(a?.workItemId).toBe(a?.agentInput?.workItemId); + it('sets coalesceKey using sentry issue ID for BullMQ dedup without a PM card ID', async () => { + const result = await trigger.handle(makeSentryIssueAlertCtx()); + expect(result?.coalesceKey).toBe(`${mockProject.id}:sentry:issue-42`); }); - it('produces different workItemIds for different issues', async () => { - const a = await trigger.handle(makeSentryIssueAlertCtx()); - const b = await trigger.handle( - makeSentryIssueAlertCtx({ eventOverrides: { issue_id: 'issue-7777' } }), - ); - expect(a?.agentInput?.workItemId).not.toBe(b?.agentInput?.workItemId); + it('sets lockKey for router-level concurrency lock without a PM card ID', async () => { + const result = await trigger.handle(makeSentryIssueAlertCtx()); + expect(result?.lockKey).toBe('sentry:issue-42'); }); }); }); @@ -261,6 +275,8 @@ describe('SentryMetricAlertTrigger', () => { vi.resetAllMocks(); vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, parameters: {} }); vi.mocked(getSentryIntegrationConfig).mockResolvedValue(sentryConfig); + // Metric alert trigger pre-flight checks getAlertsContainerId + vi.mocked(getAlertsContainerId).mockReturnValue('alerts-list'); trigger = new SentryMetricAlertTrigger(); }); @@ -343,6 +359,16 @@ describe('SentryMetricAlertTrigger', () => { ); }); + it('returns null when alerts slot is not configured', async () => { + vi.mocked(getAlertsContainerId).mockReturnValue(undefined); + const result = await trigger.handle(makeSentryMetricAlertCtx()); + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('alerts slot not configured'), + expect.objectContaining({ reason: 'alerts_slot_missing' }), + ); + }); + it('returns a TriggerResult with alertOrgId from config', async () => { const result = await trigger.handle(makeSentryMetricAlertCtx({ action: 'critical' })); expect(result).toMatchObject({ @@ -382,26 +408,45 @@ describe('SentryMetricAlertTrigger', () => { expect(result?.agentInput?.alertIssueUrl).toBeUndefined(); }); - // --- Spec 018 / plan 2: synthesized stable workItemId for metric alerts --- + // --- Spec 019 (review feedback): workItemId is deferred to the worker --- + + it('does NOT set workItemId — materialisation is deferred to processSentryWebhook', async () => { + const result = await trigger.handle( + makeSentryMetricAlertCtx({ descriptionTitle: 'Error Rate High' }), + ); + expect(result?.workItemId).toBeUndefined(); + expect(result?.agentInput?.workItemId).toBeUndefined(); + }); + + it('sets alertMetricKey from orgSlug + alertTitle for the PM materializer', async () => { + const result = await trigger.handle( + makeSentryMetricAlertCtx({ descriptionTitle: 'Error Rate High' }), + ); + expect(result?.agentInput?.alertMetricKey).toBe('my-org:Error Rate High'); + }); + + it('sets lockKey for router-level concurrency lock without a PM card ID', async () => { + const result = await trigger.handle( + makeSentryMetricAlertCtx({ descriptionTitle: 'Error Rate High' }), + ); + expect(result?.lockKey).toBe('sentry-metric:my-org:Error Rate High'); + }); - it('synthesizes a stable workItemId from orgSlug + alertTitle (spec 018)', async () => { + it('sets coalesceKey for BullMQ dedup without a PM card ID', async () => { const result = await trigger.handle( makeSentryMetricAlertCtx({ descriptionTitle: 'Error Rate High' }), ); - expect(result?.agentInput?.workItemId).toBe('sentry:metric:my-org:Error Rate High'); - expect(result?.workItemId).toBe('sentry:metric:my-org:Error Rate High'); + expect(result?.coalesceKey).toBe(`${mockProject.id}:sentry-metric:my-org:Error Rate High`); }); - it('produces the same workItemId for two dispatches with the same title', async () => { + it('produces the same alertMetricKey for two dispatches with the same title', async () => { const a = await trigger.handle( makeSentryMetricAlertCtx({ descriptionTitle: 'Error Rate High' }), ); const b = await trigger.handle( makeSentryMetricAlertCtx({ descriptionTitle: 'Error Rate High' }), ); - expect(a?.agentInput?.workItemId).toBe(b?.agentInput?.workItemId); - expect(a?.workItemId).toBe(b?.workItemId); - expect(a?.workItemId).toBe(a?.agentInput?.workItemId); + expect(a?.agentInput?.alertMetricKey).toBe(b?.agentInput?.alertMetricKey); }); }); }); diff --git a/tests/unit/triggers/sentry-webhook-handler.test.ts b/tests/unit/triggers/sentry-webhook-handler.test.ts index e7c91c22a..9cb051e3f 100644 --- a/tests/unit/triggers/sentry-webhook-handler.test.ts +++ b/tests/unit/triggers/sentry-webhook-handler.test.ts @@ -28,7 +28,27 @@ vi.mock('../../../src/triggers/shared/trigger-resolution.js', () => ({ resolveTriggerResult: vi.fn(), })); +// Mock the dynamically-imported materialization helpers +vi.mock('../../../src/integrations/alerting/_shared/materialize.js', () => ({ + materializeAlertWorkItem: vi.fn(), +})); +vi.mock('../../../src/integrations/alerting/_shared/format.js', () => ({ + formatSentryCardBody: vi + .fn() + .mockReturnValue({ title: '[Sentry] Test', descriptionMarkdown: 'desc' }), + formatSentryMetricCardBody: vi.fn().mockReturnValue({ + title: '[Sentry Metric] Error Rate High', + descriptionMarkdown: 'metric desc', + }), +})); + import { loadProjectConfigById } from '../../../src/config/provider.js'; +import { + formatSentryCardBody, + formatSentryMetricCardBody, +} from '../../../src/integrations/alerting/_shared/format.js'; +import { materializeAlertWorkItem } from '../../../src/integrations/alerting/_shared/materialize.js'; +import { AlertSlotMissingError } from '../../../src/integrations/alerting/_shared/types.js'; import { processSentryWebhook } from '../../../src/triggers/sentry/webhook-handler.js'; import { runAgentExecutionPipeline } from '../../../src/triggers/shared/agent-execution.js'; import { withAgentTypeConcurrency } from '../../../src/triggers/shared/concurrency.js'; @@ -56,6 +76,15 @@ describe('processSentryWebhook', () => { vi.mocked(withPMScope).mockImplementation((_project, fn) => fn()); // resolveTriggerResult defaults to null (no trigger matched) vi.mocked(resolveTriggerResult).mockResolvedValue(null); + // Re-apply format helper return values after resetAllMocks clears them + vi.mocked(formatSentryCardBody).mockReturnValue({ + title: '[Sentry] Test', + descriptionMarkdown: 'desc', + }); + vi.mocked(formatSentryMetricCardBody).mockReturnValue({ + title: '[Sentry Metric] Error Rate High', + descriptionMarkdown: 'metric desc', + }); }); it('loads project config by projectId and calls resolveTriggerResult with sentry source', async () => { @@ -178,4 +207,193 @@ describe('processSentryWebhook', () => { expect(runAgentExecutionPipeline).not.toHaveBeenCalled(); }); + + // ── PM card materialisation (spec 019) ────────────────────────────────── + + it('materialises a PM work item when alertIssueId is set and workItemId is absent', async () => { + const payload = { resource: 'event_alert', cascadeProjectId: 'proj-sentry' }; + const triggerResult = { + agentType: 'alerting', + agentInput: { alertIssueId: 'sentry-issue-42' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + vi.mocked(materializeAlertWorkItem).mockResolvedValue('card-new'); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(materializeAlertWorkItem).toHaveBeenCalledWith( + 'sentry', + 'sentry-issue-42', + mockProject, + expect.objectContaining({ title: '[Sentry] Test' }), + ); + expect(runAgentExecutionPipeline).toHaveBeenCalledWith( + expect.objectContaining({ + workItemId: 'card-new', + agentInput: expect.objectContaining({ workItemId: 'card-new' }), + }), + mockProject, + expect.any(Object), + expect.objectContaining({ logLabel: 'Sentry agent' }), + ); + }); + + it('skips materialisation and runs agent directly when workItemId is already set', async () => { + const payload = { resource: 'event_alert' }; + const triggerResult = { + agentType: 'alerting', + workItemId: 'wi-already-set', + agentInput: { alertIssueId: 'sentry-issue-42', workItemId: 'wi-already-set' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(materializeAlertWorkItem).not.toHaveBeenCalled(); + expect(runAgentExecutionPipeline).toHaveBeenCalledWith( + triggerResult, + mockProject, + expect.any(Object), + expect.any(Object), + ); + }); + + it('skips materialisation when alertIssueId is not a string', async () => { + const payload = { resource: 'event_alert' }; + const triggerResult = { + agentType: 'alerting', + agentInput: {}, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(materializeAlertWorkItem).not.toHaveBeenCalled(); + expect(runAgentExecutionPipeline).toHaveBeenCalled(); + }); + + it('logs a warning and skips agent when materialisation throws AlertSlotMissingError', async () => { + const payload = { resource: 'event_alert' }; + const triggerResult = { + agentType: 'alerting', + agentInput: { alertIssueId: 'sentry-issue-42' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + vi.mocked(materializeAlertWorkItem).mockRejectedValue( + new AlertSlotMissingError('proj-sentry', 'trello'), + ); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('alerts slot no longer configured'), + expect.objectContaining({ projectId: 'proj-sentry', reason: 'alerts_slot_missing' }), + ); + expect(runAgentExecutionPipeline).not.toHaveBeenCalled(); + }); + + it('re-throws transient PM errors so BullMQ can retry the job', async () => { + const payload = { resource: 'event_alert' }; + const triggerResult = { + agentType: 'alerting', + agentInput: { alertIssueId: 'sentry-issue-42' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + const transientError = new Error('PM 503 Service Unavailable'); + vi.mocked(materializeAlertWorkItem).mockRejectedValue(transientError); + + await expect( + processSentryWebhook(payload, 'proj-sentry', mockRegistry as never), + ).rejects.toThrow('PM 503 Service Unavailable'); + + expect(runAgentExecutionPipeline).not.toHaveBeenCalled(); + }); + + // ── Metric alert PM card materialisation (spec 019 review feedback) ────── + + it('materialises a PM work item for metric alerts when alertMetricKey is set', async () => { + const payload = { resource: 'metric_alert', cascadeProjectId: 'proj-sentry' }; + const triggerResult = { + agentType: 'alerting', + agentInput: { alertMetricKey: 'my-org:Error Rate High' }, + lockKey: 'sentry-metric:my-org:Error Rate High', + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + vi.mocked(materializeAlertWorkItem).mockResolvedValue('metric-card-1'); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(materializeAlertWorkItem).toHaveBeenCalledWith( + 'sentry-metric', + 'my-org:Error Rate High', + mockProject, + expect.objectContaining({ title: '[Sentry Metric] Error Rate High' }), + ); + expect(formatSentryMetricCardBody).toHaveBeenCalledWith(payload); + expect(runAgentExecutionPipeline).toHaveBeenCalledWith( + expect.objectContaining({ + workItemId: 'metric-card-1', + agentInput: expect.objectContaining({ workItemId: 'metric-card-1' }), + }), + mockProject, + expect.any(Object), + expect.objectContaining({ logLabel: 'Sentry agent' }), + ); + }); + + it('skips metric alert materialisation when workItemId is already set', async () => { + const payload = { resource: 'metric_alert' }; + const triggerResult = { + agentType: 'alerting', + workItemId: 'metric-card-already', + agentInput: { alertMetricKey: 'my-org:Error Rate High', workItemId: 'metric-card-already' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(materializeAlertWorkItem).not.toHaveBeenCalled(); + expect(runAgentExecutionPipeline).toHaveBeenCalledWith( + triggerResult, + mockProject, + expect.any(Object), + expect.any(Object), + ); + }); + + it('skips agent and warns when metric alert materialisation throws AlertSlotMissingError', async () => { + const payload = { resource: 'metric_alert' }; + const triggerResult = { + agentType: 'alerting', + agentInput: { alertMetricKey: 'my-org:Error Rate High' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + vi.mocked(materializeAlertWorkItem).mockRejectedValue( + new AlertSlotMissingError('proj-sentry', 'trello'), + ); + + await processSentryWebhook(payload, 'proj-sentry', mockRegistry as never); + + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('alerts slot no longer configured'), + expect.objectContaining({ reason: 'alerts_slot_missing' }), + ); + expect(runAgentExecutionPipeline).not.toHaveBeenCalled(); + }); + + it('re-throws transient PM errors for metric alerts so BullMQ can retry', async () => { + const payload = { resource: 'metric_alert' }; + const triggerResult = { + agentType: 'alerting', + agentInput: { alertMetricKey: 'my-org:Error Rate High' }, + } as never; + vi.mocked(resolveTriggerResult).mockResolvedValue(triggerResult); + vi.mocked(materializeAlertWorkItem).mockRejectedValue(new Error('PM 503')); + + await expect( + processSentryWebhook(payload, 'proj-sentry', mockRegistry as never), + ).rejects.toThrow('PM 503'); + + expect(runAgentExecutionPipeline).not.toHaveBeenCalled(); + }); }); diff --git a/tests/unit/triggers/sentry/alerting-issue-materializer.test.ts b/tests/unit/triggers/sentry/alerting-issue-materializer.test.ts new file mode 100644 index 000000000..93789b53b --- /dev/null +++ b/tests/unit/triggers/sentry/alerting-issue-materializer.test.ts @@ -0,0 +1,169 @@ +/** + * Tests for SentryIssueAlertTrigger. + * + * After spec 019 review feedback, PM card materialisation was moved from the + * router-side trigger handler to the worker-side processSentryWebhook so that + * transient PM failures surface as BullMQ retries (durable) rather than being + * swallowed as non-fatal dispatch errors by processRouterWebhook (which would + * return HTTP 200 to Sentry with no job ever enqueued). + * + * The trigger handler now only: + * 1. Checks the trigger is enabled. + * 2. Verifies the alerts slot is configured via getAlertsContainerId. + * 3. Returns a TriggerResult with alertIssueId in agentInput (no workItemId). + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { mockLogger, mockTriggerCheckModule } from '../../../helpers/sharedMocks.js'; + +vi.mock('../../../../src/utils/logging.js', () => ({ logger: mockLogger })); +vi.mock('../../../../src/triggers/shared/trigger-check.js', () => mockTriggerCheckModule); +vi.mock('../../../../src/sentry/integration.js', () => ({ + getSentryIntegrationConfig: vi.fn(), +})); + +import { AlertSlotMissingError } from '../../../../src/integrations/alerting/_shared/types.js'; +import { getSentryIntegrationConfig } from '../../../../src/sentry/integration.js'; +import { SentryIssueAlertTrigger } from '../../../../src/triggers/sentry/alerting-issue.js'; +import { checkTriggerEnabledWithParams } from '../../../../src/triggers/shared/trigger-check.js'; +import type { TriggerContext } from '../../../../src/types/index.js'; +import { createMockProject } from '../../../helpers/factories.js'; + +const sentryConfig = { organizationSlug: 'my-org' }; + +// Project with alerts slot configured — required for dispatch +const mockProjectWithAlerts = createMockProject({ + id: 'test-project', + trello: { + boardId: 'board123', + lists: { + splitting: 'list-split', + planning: 'list-plan', + todo: 'list-todo', + alerts: 'list-alerts', + }, + labels: {}, + }, +}); + +// Project without alerts slot — dispatch should be skipped +const mockProjectWithoutAlerts = createMockProject({ id: 'test-project' }); + +function makeCtx(issueId = 'issue-42', project = mockProjectWithAlerts): TriggerContext { + return { + project, + source: 'sentry', + payload: { + resource: 'event_alert', + payload: { + action: 'triggered', + data: { + event: { + event_id: 'evt-abc', + issue_id: issueId, + web_url: `https://sentry.io/issues/${issueId}/`, + title: 'NullPointerException', + }, + }, + }, + cascadeProjectId: project.id, + }, + } as TriggerContext; +} + +describe('SentryIssueAlertTrigger', () => { + let trigger: SentryIssueAlertTrigger; + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, parameters: {} }); + vi.mocked(getSentryIntegrationConfig).mockResolvedValue(sentryConfig); + trigger = new SentryIssueAlertTrigger(); + }); + + describe('when alerts slot is configured', () => { + it('returns a TriggerResult with alertIssueId in agentInput', async () => { + const result = await trigger.handle(makeCtx('I-42')); + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('alerting'); + expect(result?.agentInput.alertIssueId).toBe('I-42'); + }); + + it('does NOT set workItemId — materialisation is deferred to the worker', async () => { + const result = await trigger.handle(makeCtx()); + expect(result?.workItemId).toBeUndefined(); + expect(result?.agentInput.workItemId).toBeUndefined(); + }); + + it('sets coalesceKey using sentry issue ID for dedup without a PM card ID', async () => { + const result = await trigger.handle(makeCtx('issue-99')); + expect(result?.coalesceKey).toBe('test-project:sentry:issue-99'); + }); + + it('sets lockKey for router-level work-item concurrency without a PM card ID', async () => { + const result = await trigger.handle(makeCtx('issue-42')); + expect(result?.lockKey).toBe('sentry:issue-42'); + }); + + it('result contains no string field matching sentry:issue: prefix', async () => { + const result = await trigger.handle(makeCtx()); + expect(result).not.toBeNull(); + expect(JSON.stringify(result)).not.toMatch(/sentry:issue:/); + }); + + it('includes orgId, alertTitle, and alertIssueUrl from the payload', async () => { + const result = await trigger.handle(makeCtx()); + expect(result?.agentInput.alertOrgId).toBe('my-org'); + expect(result?.agentInput.alertIssueUrl).toMatch(/sentry\.io/); + }); + }); + + describe('when alerts slot is NOT configured', () => { + it('returns null and emits structured WARN', async () => { + const result = await trigger.handle(makeCtx('I-1', mockProjectWithoutAlerts)); + expect(result).toBeNull(); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + projectId: 'test-project', + source: 'sentry', + reason: 'alerts_slot_missing', + }), + ); + }); + }); + + describe('when trigger is disabled', () => { + it('returns null without accessing the alerts slot config', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ + enabled: false, + parameters: {}, + }); + const result = await trigger.handle(makeCtx()); + expect(result).toBeNull(); + // No WARN about missing slot — trigger check fires first + expect(mockLogger.warn).not.toHaveBeenCalled(); + }); + }); + + describe('when issue ID cannot be determined', () => { + it('returns null', async () => { + const ctx = makeCtx(); + (ctx.payload as Record).payload = { + action: 'triggered', + data: { event: { event_id: 'evt-x' } }, + }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + }); +}); + +// Verify AlertSlotMissingError is still exported and used correctly by callers +describe('AlertSlotMissingError (used by processSentryWebhook)', () => { + it('is constructable with projectId and pm type', () => { + const err = new AlertSlotMissingError('project-1', 'trello'); + expect(err).toBeInstanceOf(AlertSlotMissingError); + expect(err).toBeInstanceOf(Error); + }); +}); diff --git a/tests/unit/triggers/shared/integration-validation-alerts-slot.test.ts b/tests/unit/triggers/shared/integration-validation-alerts-slot.test.ts new file mode 100644 index 000000000..08bfa594a --- /dev/null +++ b/tests/unit/triggers/shared/integration-validation-alerts-slot.test.ts @@ -0,0 +1,167 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { validateIntegrations } from '../../../../src/triggers/shared/integration-validation.js'; + +vi.mock('../../../../src/integrations/registry.js', () => ({ + integrationRegistry: { + getByCategory: vi.fn(), + getOrNull: vi.fn().mockReturnValue(null), + register: vi.fn(), + get: vi.fn(), + all: vi.fn().mockReturnValue([]), + hasIntegration: vi.fn().mockResolvedValue(false), + }, +})); + +vi.mock('../../../../src/github/personas.js', () => ({ + getPersonaForAgentType: vi.fn().mockReturnValue('implementer'), +})); + +vi.mock('../../../../src/utils/logging.js', () => ({ + logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, +})); + +const mockGetTriggerConfigsByProjectAndAgent = vi.fn(); +vi.mock('../../../../src/db/repositories/agentTriggerConfigsRepository.js', () => ({ + getTriggerConfigsByProjectAndAgent: (...a: unknown[]) => + mockGetTriggerConfigsByProjectAndAgent(...a), +})); + +import { integrationRegistry } from '../../../../src/integrations/registry.js'; +import type { ProjectConfig } from '../../../../src/types/index.js'; +import { + createMockJiraProject, + createMockLinearProject, + createMockProject, +} from '../../../helpers/factories.js'; + +function mockAlertingIntegration() { + return { + type: 'sentry', + category: 'alerting', + hasIntegration: vi.fn().mockResolvedValue(true), + withCredentials: vi.fn(), + }; +} + +function enabledAlertingTrigger(projectId: string) { + return { + id: 1, + projectId, + agentType: 'alerting', + triggerEvent: 'alerting:issue-alert', + enabled: true, + parameters: {}, + createdAt: null, + updatedAt: null, + }; +} + +const trelloNoAlerts = createMockProject({ + id: 'test-project', + trello: { boardId: 'b1', lists: { todo: 'list-todo' }, labels: {} }, +}) as ProjectConfig; + +const trelloWithAlerts = createMockProject({ + id: 'test-project', + trello: { boardId: 'b1', lists: { todo: 'list-todo', alerts: 'list-alerts' }, labels: {} }, +}) as ProjectConfig; + +const jiraNoAlerts = createMockJiraProject() as ProjectConfig; + +const linearNoAlerts = createMockLinearProject() as ProjectConfig; + +describe('validateIntegrations — alerts-slot check', () => { + beforeEach(() => { + vi.resetAllMocks(); + const alerting = mockAlertingIntegration(); + vi.mocked(integrationRegistry.getByCategory).mockImplementation((category) => { + if (category === 'alerting') return [alerting as never]; + return []; + }); + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([]); + }); + + it('passes when no alerting trigger is enabled even if alerts slot is unset', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([]); + const result = await validateIntegrations('test-project', 'alerting', trelloNoAlerts); + expect(result.valid).toBe(true); + expect(result.errors).toEqual([]); + }); + + it('fails with pm-category error when alerting trigger enabled and Trello lists.alerts is unset', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger('test-project'), + ]); + const result = await validateIntegrations('test-project', 'alerting', trelloNoAlerts); + expect(result.valid).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].category).toBe('pm'); + expect(result.errors[0].message).toMatch(/alerts/i); + expect(result.errors[0].message).toMatch(/trello/i); + }); + + it('fails when alerting trigger enabled and JIRA statuses.alerts is unset', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger(jiraNoAlerts.id), + ]); + const result = await validateIntegrations(jiraNoAlerts.id, 'alerting', jiraNoAlerts); + expect(result.valid).toBe(false); + expect(result.errors[0].category).toBe('pm'); + expect(result.errors[0].message).toMatch(/alerts/i); + }); + + it('fails when alerting trigger enabled and Linear statuses.alerts is unset', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger(linearNoAlerts.id), + ]); + const result = await validateIntegrations(linearNoAlerts.id, 'alerting', linearNoAlerts); + expect(result.valid).toBe(false); + expect(result.errors[0].category).toBe('pm'); + expect(result.errors[0].message).toMatch(/alerts/i); + }); + + it('passes when alerting trigger enabled and Trello lists.alerts is set', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger('test-project'), + ]); + const result = await validateIntegrations('test-project', 'alerting', trelloWithAlerts); + expect(result.valid).toBe(true); + expect(result.errors).toEqual([]); + }); + + it('does not require cascade-alert label slot (soft requirement)', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger('test-project'), + ]); + // trelloWithAlerts has no labels['cascade-alert'] — should still pass + const result = await validateIntegrations('test-project', 'alerting', trelloWithAlerts); + expect(result.valid).toBe(true); + }); + + it('skips the alerts-slot check when project is not provided', async () => { + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger('test-project'), + ]); + // no project param — should not fail even with no slot + const result = await validateIntegrations('test-project', 'alerting'); + expect(result.valid).toBe(true); + expect(mockGetTriggerConfigsByProjectAndAgent).not.toHaveBeenCalled(); + }); + + it('does not run alerts-slot check for non-alerting agents (implementation/review/manual)', async () => { + // Simulate an alerting trigger being enabled with no alerts slot configured. + // For non-alerting agents this should NOT trigger the alerts-slot validation. + mockGetTriggerConfigsByProjectAndAgent.mockResolvedValue([ + enabledAlertingTrigger('test-project'), + ]); + + // Call for 'implementation' (agentType !== 'alerting') with no slot set + await validateIntegrations('test-project', 'implementation', trelloNoAlerts).catch(() => { + // Implementation agent may fail PM/SCM validation — that is irrelevant to this test + }); + + // The alerts-slot check queries trigger configs for 'alerting' — must NOT be called + // for non-alerting agent types. + expect(mockGetTriggerConfigsByProjectAndAgent).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/web/wizard-alerts-slot.test.ts b/tests/unit/web/wizard-alerts-slot.test.ts new file mode 100644 index 000000000..484896779 --- /dev/null +++ b/tests/unit/web/wizard-alerts-slot.test.ts @@ -0,0 +1,156 @@ +/** + * Regression pin: every PM provider wizard exposes an `alerts` status-mapping + * slot and a `cascade-alert` / `cascadeAlert` label-mapping slot (spec 019). + * + * 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 alerts slot being mapped. + */ + +import { describe, expect, it } from 'vitest'; +import { + JIRA_LABEL_SLOTS, + JIRA_STATUS_SLOTS, +} from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; +import { + LINEAR_LABEL_SLOTS, + LINEAR_STATUS_SLOTS, +} from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; +import { + TRELLO_LABEL_SLOTS, + 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 an alerts entry (spec 019)', () => { + it('TRELLO_LIST_SLOTS contains an alerts entry', () => { + expect(TRELLO_LIST_SLOTS.some((s) => s.key === 'alerts')).toBe(true); + }); + + it('JIRA_STATUS_SLOTS contains an alerts entry', () => { + expect(JIRA_STATUS_SLOTS.some((s) => s.key === 'alerts')).toBe(true); + }); + + it('LINEAR_STATUS_SLOTS contains an alerts entry', () => { + expect(LINEAR_STATUS_SLOTS.some((s) => s.key === 'alerts')).toBe(true); + }); +}); + +describe('label-mapping slot arrays include a cascade-alert / cascadeAlert entry (spec 019)', () => { + it('TRELLO_LABEL_SLOTS contains a cascade-alert entry', () => { + expect(TRELLO_LABEL_SLOTS.some((s) => s.key === 'cascade-alert')).toBe(true); + }); + + it('JIRA_LABEL_SLOTS contains a cascadeAlert entry', () => { + expect(JIRA_LABEL_SLOTS.some((s) => s.key === 'cascadeAlert')).toBe(true); + }); + + it('LINEAR_LABEL_SLOTS contains a cascadeAlert entry', () => { + expect(LINEAR_LABEL_SLOTS.some((s) => s.key === 'cascadeAlert')).toBe(true); + }); +}); + +// ============================================================================ +// 2. Round-trip through buildIntegrationConfig +// ============================================================================ + +describe('alerts status slot round-trips through buildIntegrationConfig', () => { + it('Trello: lists.alerts propagates from trelloListMappings', () => { + const state = { + ...createInitialState(), + trelloListMappings: { alerts: 'list-id-alerts-123' }, + }; + const config = buildTrelloIntegrationConfig(state); + expect((config.lists as Record).alerts).toBe('list-id-alerts-123'); + }); + + it('JIRA: statuses.alerts propagates from jiraStatusMappings', () => { + const state = { + ...createInitialState(), + jiraStatusMappings: { alerts: 'Alerts Status' }, + }; + const config = buildJiraIntegrationConfig(state); + expect((config.statuses as Record).alerts).toBe('Alerts Status'); + }); + + it('Linear: statuses.alerts propagates from linearStatusMappings', () => { + const state = { + ...createInitialState(), + linearStatusMappings: { alerts: 'state-uuid-abc' }, + }; + const config = buildLinearIntegrationConfig(state); + expect((config.statuses as Record).alerts).toBe('state-uuid-abc'); + }); +}); + +describe('cascade-alert label slot round-trips through buildIntegrationConfig', () => { + it('Trello: labels[cascade-alert] propagates from trelloLabelMappings', () => { + const state = { + ...createInitialState(), + trelloLabelMappings: { 'cascade-alert': 'label-id-789' }, + }; + const config = buildTrelloIntegrationConfig(state); + expect((config.labels as Record)['cascade-alert']).toBe('label-id-789'); + }); + + it('JIRA: labels.cascadeAlert propagates from jiraLabels', () => { + const state = { + ...createInitialState(), + jiraLabels: { cascadeAlert: 'cascade-alert-label' }, + }; + const config = buildJiraIntegrationConfig(state); + expect((config.labels as Record).cascadeAlert).toBe('cascade-alert-label'); + }); + + it('Linear: labels.cascadeAlert propagates from linearLabels', () => { + const state = { + ...createInitialState(), + linearLabels: { cascadeAlert: 'label-uuid-xyz' }, + }; + const config = buildLinearIntegrationConfig(state); + expect((config.labels as Record).cascadeAlert).toBe('label-uuid-xyz'); + }); +}); + +// ============================================================================ +// 3. Optionality — wizard completes without alerts slot +// ============================================================================ + +describe('alerts slot is optional — wizard does not require it', () => { + it('Trello: buildTrelloIntegrationConfig produces no alerts entry when not mapped', () => { + const state = { + ...createInitialState(), + trelloListMappings: { todo: 'list-id-todo' }, + }; + const config = buildTrelloIntegrationConfig(state); + expect((config.lists as Record).alerts).toBeUndefined(); + }); + + it('JIRA: buildJiraIntegrationConfig produces no alerts entry when not mapped', () => { + const state = { + ...createInitialState(), + jiraStatusMappings: { todo: 'To Do' }, + }; + const config = buildJiraIntegrationConfig(state); + expect((config.statuses as Record).alerts).toBeUndefined(); + }); + + it('Linear: buildLinearIntegrationConfig produces no alerts entry when not mapped', () => { + const state = { + ...createInitialState(), + linearStatusMappings: { todo: 'state-uuid-todo' }, + }; + const config = buildLinearIntegrationConfig(state); + expect((config.statuses as Record).alerts).toBeUndefined(); + }); +}); diff --git a/web/src/components/projects/pm-providers/jira/wizard.ts b/web/src/components/projects/pm-providers/jira/wizard.ts index ae40e8748..7d5eff7da 100644 --- a/web/src/components/projects/pm-providers/jira/wizard.ts +++ b/web/src/components/projects/pm-providers/jira/wizard.ts @@ -33,7 +33,7 @@ import { JiraWebhookAdapter } from './webhook-step.js'; // CASCADE stage keys that map to JIRA statuses (name-based, not id-based // — JIRA statuses are configured per project, name is the stable identity). -const JIRA_STATUS_SLOTS = [ +export const JIRA_STATUS_SLOTS = [ { key: 'backlog', label: 'Backlog' }, { key: 'splitting', label: 'Splitting' }, { key: 'planning', label: 'Planning' }, @@ -42,17 +42,19 @@ const JIRA_STATUS_SLOTS = [ { key: 'inReview', label: 'In Review' }, { key: 'done', label: 'Done' }, { key: 'merged', label: 'Merged' }, + { key: 'alerts', label: 'Alerts' }, ] as const; // CASCADE labels that map to JIRA labels. JIRA labels are free-form // strings (no curated enum), so the shared label-mapping step renders // in free-text mode automatically when providerLabels is empty. -const JIRA_LABEL_SLOTS = [ +export const JIRA_LABEL_SLOTS = [ { key: 'readyToProcess', label: 'Ready to Process' }, { key: 'processing', label: 'Processing' }, { key: 'processed', label: 'Processed' }, { key: 'error', label: 'Error' }, { key: 'auto', label: 'Auto' }, + { key: 'cascadeAlert', label: 'Cascade Alert' }, ] as const; const JIRA_CUSTOM_FIELD_SLOTS = [{ key: 'cost', label: 'Cost' }] as const; diff --git a/web/src/components/projects/pm-providers/linear/wizard.ts b/web/src/components/projects/pm-providers/linear/wizard.ts index fc0369feb..826a8128f 100644 --- a/web/src/components/projects/pm-providers/linear/wizard.ts +++ b/web/src/components/projects/pm-providers/linear/wizard.ts @@ -38,7 +38,7 @@ import { LinearWebhookAdapter } from './webhook-step.js'; // Linear's issue-update API requires state UUIDs, not names; the // status-changed trigger does strict-equality matching on state IDs // delivered in the webhook payload). -const LINEAR_STATUS_SLOTS = [ +export const LINEAR_STATUS_SLOTS = [ { key: 'backlog', label: 'Backlog' }, { key: 'splitting', label: 'Splitting' }, { key: 'planning', label: 'Planning' }, @@ -47,14 +47,16 @@ const LINEAR_STATUS_SLOTS = [ { key: 'inReview', label: 'In Review' }, { key: 'done', label: 'Done' }, { key: 'merged', label: 'Merged' }, + { key: 'alerts', label: 'Alerts' }, ] as const; -const LINEAR_LABEL_SLOTS = [ +export const LINEAR_LABEL_SLOTS = [ { key: 'readyToProcess', label: 'Ready to Process' }, { key: 'processing', label: 'Processing' }, { key: 'processed', label: 'Processed' }, { key: 'error', label: 'Error' }, { key: 'auto', label: 'Auto' }, + { key: 'cascadeAlert', label: 'Cascade Alert' }, ] as const; // Default CASCADE label names + hex colors for the shared Create @@ -69,6 +71,7 @@ const LINEAR_LABEL_DEFAULTS: Readonly< processed: { name: 'cascade-processed', color: '#16A34A' }, error: { name: 'cascade-error', color: '#DC2626' }, auto: { name: 'cascade-auto', color: '#9333EA' }, + cascadeAlert: { name: 'cascade-alert', color: '#F97316' }, }; const LINEAR_CREDENTIAL_ROLES = [{ role: 'api_key', label: 'API Key' }]; diff --git a/web/src/components/projects/pm-providers/trello/wizard.ts b/web/src/components/projects/pm-providers/trello/wizard.ts index 487aec7de..4804483e6 100644 --- a/web/src/components/projects/pm-providers/trello/wizard.ts +++ b/web/src/components/projects/pm-providers/trello/wizard.ts @@ -36,7 +36,7 @@ import { TrelloOAuthStep } from './oauth-step.js'; import { TrelloWebhookAdapter } from './webhook-step.js'; // CASCADE stage keys that map to Trello lists (one list per stage). -const TRELLO_LIST_SLOTS = [ +export const TRELLO_LIST_SLOTS = [ { key: 'backlog', label: 'Backlog' }, { key: 'splitting', label: 'Splitting' }, { key: 'planning', label: 'Planning' }, @@ -46,6 +46,7 @@ const TRELLO_LIST_SLOTS = [ { key: 'done', label: 'Done' }, { key: 'merged', label: 'Merged' }, { key: 'debug', label: 'Debug' }, + { key: 'alerts', label: 'Alerts' }, ] as const; // CASCADE labels that map to Trello labels. Defaults (name + color) @@ -60,14 +61,16 @@ const TRELLO_LABEL_DEFAULTS: Readonly< processed: { name: 'cascade-processed', color: 'green' }, error: { name: 'cascade-error', color: 'red' }, auto: { name: 'cascade-auto', color: 'purple' }, + 'cascade-alert': { name: 'cascade-alert', color: 'orange' }, }; -const TRELLO_LABEL_SLOTS = [ +export const TRELLO_LABEL_SLOTS = [ { key: 'readyToProcess', label: 'Ready to Process' }, { key: 'processing', label: 'Processing' }, { key: 'processed', label: 'Processed' }, { key: 'error', label: 'Error' }, { key: 'auto', label: 'Auto' }, + { key: 'cascade-alert', label: 'Cascade Alert' }, ] as const; // Trello has one known custom-field slot: the cost estimate.