docs(09-03): complete notify per-user routing plan
- Type additions: RouteToUser, ResolvedRecipient, NotifyResolver, UserRouteFallback, UserRouteFallbackReason in lib/types/pipeline.ts - New resolver registry: notify-resolvers.ts with 3 v1 resolvers - executeNotify rewrite with backward-compat path + user-route decision tree - Behavioral vitest guarantee: muted user does not fall back to global channel
This commit is contained in:
parent
fd19a5d997
commit
486f27eb77
1 changed files with 186 additions and 0 deletions
|
|
@ -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<string, Fn> 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<ResolvedRecipient>`
|
||||
- **`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<string, NotifyResolver>` — 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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue