diff --git a/.planning/phases/09-user-profile-preferences-new/09-03-SUMMARY.md b/.planning/phases/09-user-profile-preferences-new/09-03-SUMMARY.md new file mode 100644 index 0000000..8c8fbb6 --- /dev/null +++ b/.planning/phases/09-user-profile-preferences-new/09-03-SUMMARY.md @@ -0,0 +1,186 @@ +--- +phase: 09-user-profile-preferences-new +plan: "03" +subsystem: pipeline-notify-routing +tags: [pipeline, notify, routing, types, vitest, phase-9] +dependency_graph: + requires: + - "09-01: user_event_subscriptions table, notification_channels.owner_user_id column" + provides: + - "RouteToUser, ResolvedRecipient, NotifyResolver, UserRouteFallback, UserRouteFallbackReason types" + - "notify-resolvers.ts: three v1 resolvers + registerResolver + resolveRecipient" + - "executeNotify: backward-compat global path + per-user route path with mute/fallback semantics" + - "_INTERNALS test seam on notify.ts" + - "vitest behavioral guarantee for D-12/ROUTE-05 (mute must not fall back)" + affects: + - "lib/services/pipeline-steps/notify.ts — downstream pipelines using route_to_user config" + - "pipeline_execution_steps.output_data — gains user_route/user_route_fallback/skipped_reason keys" +tech_stack: + added: [] + patterns: + - "_INTERNALS test seam pattern from lib/services/analyzer/link-discovery.ts" + - "vi.mock hoisting before dynamic import for named exports" + - "Map resolver registry for single-file extension" +key_files: + created: + - lib/services/pipeline-steps/notify-resolvers.ts + - lib/services/pipeline-steps/notify.test.ts + modified: + - lib/types/pipeline.ts + - lib/services/pipeline-steps/notify.ts +decisions: + - "Extracted dispatchToGlobalChannel as a named helper so fallbackToGlobal can reuse it without code duplication" + - "_INTERNALS export follows link-discovery.ts precedent — avoids re-exporting private functions as top-level named exports" + - "vi.mock('../pipeline-engine') needed to suppress registerStepExecutor side effect during test import" + - "resolveRecipient wraps resolver in try/catch — throws become user_not_found fallback, not unhandled rejections" +metrics: + duration_minutes: 3 + completed_date: "2026-05-10" + tasks_completed: 3 + files_created: 2 + files_modified: 2 +--- + +# Phase 9 Plan 03: Notify Per-User Routing Summary + +One-liner: notify.ts gains an optional route_to_user block implementing resolver dispatch, mute-check, personal-channel lookup, and global fallback — with a behavioral vitest guarantee that muting suppresses delivery without triggering the fallback path. + +## Tasks Completed + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | Add route_to_user types and resolver registry | d27462f | lib/types/pipeline.ts, lib/services/pipeline-steps/notify-resolvers.ts | +| 2 | Rewrite executeNotify with route_to_user branch and fallback semantics | 86acc06 | lib/services/pipeline-steps/notify.ts | +| 3 | Vitest unit test — muted user must NOT fall back to global channel | fd19a5d | lib/services/pipeline-steps/notify.test.ts | + +## What Was Built + +### lib/types/pipeline.ts — type additions (ROUTE-01) + +Five new exported types appended after `UserEventSubscription` (no existing types modified): + +- **`ResolverName`** — union `'autotask_resource_email' | 'direct_email' | 'pulse_user_id' | string` +- **`RouteToUser`** — optional config block on notify steps. Fields: `source`, `field` (single-level, v1 limitation documented in JSDoc), `resolve`, `event_key`, `channel_type?` +- **`ResolvedRecipient`** — discriminated union `{ user_id: string } | { email: string } | null` +- **`NotifyResolver`** — `(fieldValue: unknown) => Promise` +- **`UserRouteFallbackReason`** — union of five string literals: `'no_channel' | 'send_failed' | 'user_not_found' | 'no_field_value' | 'resolver_unknown'` +- **`UserRouteFallback`** — `{ reason, user_id?, channel_type, error? }` + +### lib/services/pipeline-steps/notify-resolvers.ts — resolver registry (ROUTE-02) + +New file. Side-effect-free (no worker startup, no module-scope timers). + +**Three v1 resolver implementations:** + +| Resolver key | Input | SQL | Output | +|---|---|---|---| +| `direct_email` | string matching email regex | none | `{ email }` or null | +| `pulse_user_id` | non-empty string | `SELECT id FROM "user" WHERE id = $1` | `{ user_id }` or null | +| `autotask_resource_email` | integer or numeric string | `SELECT email FROM resources WHERE id = $1` | `{ email }` or null | + +**Registry exports:** +- `export const RESOLVERS: Map` — initialized with all three +- `export function registerResolver(name, fn)` — `RESOLVERS.set(name, fn)` +- `export async function resolveRecipient(name, fieldValue)` — looks up resolver, wraps call in try/catch; returns `{ recipient, resolverFound }` + +**Adding a fourth resolver:** call `registerResolver('my_resolver', async (v) => ...)` — no other files change. + +### lib/services/pipeline-steps/notify.ts — executeNotify rewrite (ROUTE-03..06) + +Full rewrite preserving all four send helpers (`sendTeams`, `sendTelegram`, `sendNtfy`, `sendWebhook`) verbatim and keeping `registerStepExecutor('notify', executeNotify)` at the bottom. + +**Decision tree:** + +``` +executeNotify(step, context, _executionId) + ├─ no route_to_user → dispatchToGlobalChannel (BACKWARD COMPAT — byte-identical) + └─ route_to_user present → dispatchUserRoute + ├─ no field value → fallbackToGlobal(no_field_value) + ├─ resolver unknown → fallbackToGlobal(resolver_unknown) + ├─ recipient null → fallbackToGlobal(user_not_found) + ├─ email recipient → SELECT user by email → not found → fallbackToGlobal(user_not_found) + └─ for each channelType in [channel_type] or ['ntfy','teams']: + ├─ user_event_subscriptions.enabled = false → return {notified:false, skipped_reason:'user_muted'} (NO FALLBACK) + ├─ no personal channel + more types remain → continue + ├─ no personal channel + last type → fallbackToGlobal(no_channel) + ├─ send success → return {notified:true, user_route:{...}} + ├─ send failed + more types remain → continue + └─ send failed + last type → fallbackToGlobal(send_failed) +``` + +**Output shapes:** + +| Scenario | `output` keys | +|---|---| +| No route_to_user | existing shape (notified, channel) | +| User-route success | `notified, channel, user_route: { user_id, channel_type, event_key }` | +| Mute (D-12) | `notified: false, skipped_reason: 'user_muted', user_id, event_key, channel_type` | +| Fallback (D-11) | existing global output + `user_route_fallback: { reason, user_id?, channel_type, error? }` | + +**Test seam:** `export const _INTERNALS = { dispatchToGlobalChannel, dispatchUserRoute, fallbackToGlobal }` — follows `link-discovery.ts` precedent. + +**Backward compatibility verified:** Steps with no `route_to_user` key call only `dispatchToGlobalChannel`, which contains the exact pre-Phase-9 SQL (`SELECT * FROM notification_channels WHERE id = $1 AND is_active = true`) and switch statement. No resolver queries, no subscription queries. + +### lib/services/pipeline-steps/notify.test.ts — behavioral vitest guarantee (ROUTE-05 / D-12) + +Single test file, one test: + +**`'muted user must not fall back to global channel'`** + +Mocks: +- `../postgres-client` — `queryMock` returns `[{ enabled: false }]` for `user_event_subscriptions` and throws for any other SQL +- `./notify-resolvers` — `resolveRecipient` returns `{ recipient: { user_id: 'test-user-id' }, resolverFound: true }` +- `../pipeline-engine` — `registerStepExecutor` is a no-op (suppresses side effect) +- `global.fetch` — `fetchMock` records all outbound HTTP calls + +Assertions: +1. `result.success === true` +2. `result.output.notified === false` +3. `result.output.skipped_reason === 'user_muted'` +4. `result.output.user_route_fallback === undefined` +5. No SQL containing `'notification_channels'` was issued +6. `fetchMock` was not called (no HTTP send) + +`npx vitest run lib/services/pipeline-steps/notify.test.ts` exits 0. + +### v1 single-level field-path limitation (deferred-idea) + +The resolver receives `context[route.source]?.[route.field]` — a single-level lookup. Dotted paths like `"company.id"` are NOT walked. Admins must surface nested values at the top level of the pipeline context via an upstream transform step. Documented in `RouteToUser.field` JSDoc and as a candidate deferred-idea for a later phase. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 2 - Missing critical mock] Added pipeline-engine mock to test file** +- **Found during:** Task 3 +- **Issue:** `notify.ts` calls `registerStepExecutor('notify', executeNotify)` at module scope. Without mocking `../pipeline-engine`, the test import would trigger the real `registerStepExecutor` which registers the executor globally — a harmless but potentially confusing side effect in test isolation. +- **Fix:** Added `vi.mock('../pipeline-engine', () => ({ registerStepExecutor: vi.fn() }))` to the test file. +- **Files modified:** lib/services/pipeline-steps/notify.test.ts + +### Pre-existing Test Failures (Out of Scope) + +`lib/services/analyzer/itglue-search.test.ts` has 2 pre-existing failures related to `getFlexibleAssetsForOrganization` not being a function in the mock. These failures existed before Plan 03 began and are not caused by any change in this plan. Logged to deferred items per deviation rule scope boundary. + +## Known Stubs + +None — all routing logic is fully implemented and connected to real database queries. + +## Threat Flags + +No new network endpoints, auth paths, or schema changes beyond what the plan's threat model declared. The resolver registry (`notify-resolvers.ts`) is side-effect-free at module scope (no timers, no worker startup). The `LIMIT 1` on the personal channel SELECT enforces single-recipient semantics (T-09-03-01). + +## Self-Check: PASSED + +Files exist: +- lib/types/pipeline.ts: FOUND +- lib/services/pipeline-steps/notify-resolvers.ts: FOUND +- lib/services/pipeline-steps/notify.ts: FOUND +- lib/services/pipeline-steps/notify.test.ts: FOUND + +Commits exist: +- d27462f: FOUND (Task 1) +- 86acc06: FOUND (Task 2) +- fd19a5d: FOUND (Task 3) + +TypeScript: `npx tsc --noEmit --pretty` exit 0 — no errors. +Vitest: `npx vitest run lib/services/pipeline-steps/notify.test.ts` exit 0 — 1/1 tests pass.