plan: design for SLA and escalation feature (008)
Phase 0 research resolves the business-calendar working-hours algorithm (day-by-day walk via luxon, the first date/timezone dependency in this codebase), the workingHours JSON shape, most-specific SLA-policy match (reusing 005/006's resolution pattern), durable pause/resume (absolute due-date shift, no in-memory state), and a repeatable-job breach-detection design over per-run delayed jobs. Phase 1 adds data-model.md (one additive refinement beyond doc06: SLARun.firstResponseBreachedAt), the admin/read contract, and six quickstart scenarios including a genuine process-restart boundary test for Constitution Principle VII. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
1ad9007e79
commit
199bd4eb4e
Generated
+9
@@ -24,12 +24,14 @@
|
||||
"fastify": "^4.26.2",
|
||||
"fastify-plugin": "^4.5.1",
|
||||
"ioredis": "^5.3.2",
|
||||
"luxon": "^3.7.2",
|
||||
"pino": "^8.20.0",
|
||||
"pino-pretty": "^11.0.0",
|
||||
"prom-client": "^15.1.1",
|
||||
"zod": "^3.22.4"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/luxon": "^3.7.5",
|
||||
"@types/node": "^20.12.7",
|
||||
"@typescript-eslint/eslint-plugin": "^7.6.0",
|
||||
"@typescript-eslint/parser": "^7.6.0",
|
||||
@@ -1973,6 +1975,13 @@
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@types/luxon": {
|
||||
"version": "3.7.5",
|
||||
"resolved": "https://registry.npmjs.org/@types/luxon/-/luxon-3.7.5.tgz",
|
||||
"integrity": "sha512-jJ41Q4z6ZVO260MNDdHfW7+7a5iMiX8Mr6ZJHcmgrvhZha6dz5704o/lF2kKl6URjH6ivEL97w9xS/MgpJEphg==",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@types/node": {
|
||||
"version": "20.19.43",
|
||||
"resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.43.tgz",
|
||||
|
||||
@@ -61,12 +61,14 @@
|
||||
"fastify": "^4.26.2",
|
||||
"fastify-plugin": "^4.5.1",
|
||||
"ioredis": "^5.3.2",
|
||||
"luxon": "^3.7.2",
|
||||
"pino": "^8.20.0",
|
||||
"pino-pretty": "^11.0.0",
|
||||
"prom-client": "^15.1.1",
|
||||
"zod": "^3.22.4"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/luxon": "^3.7.5",
|
||||
"@types/node": "^20.12.7",
|
||||
"@typescript-eslint/eslint-plugin": "^7.6.0",
|
||||
"@typescript-eslint/parser": "^7.6.0",
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
# Contract: SLA and Escalation
|
||||
|
||||
Every admin CRUD/manual-escalation route below is gated by `fastify.authenticate` (research.md —
|
||||
known limitation inherited from 002/003/004/005/006/007). SLA-run creation, pause/resume, and
|
||||
breach detection have no public trigger endpoint — they run automatically off the domain event
|
||||
bus and the breach-detection BullMQ job (research.md), matching 007's "orchestration has no
|
||||
manual trigger endpoint" precedent.
|
||||
|
||||
## SLA Policy admin
|
||||
|
||||
- `POST /admin/sla-policies` — body `{ name, productId?, categoryId?, problemTypeId?, priority?,
|
||||
firstResponseMinutes, investigationMinutes?, resolutionMinutes, customerResponseMinutes?,
|
||||
businessCalendarId? }`. `404` if `productId`/`categoryId`/`businessCalendarId` is given but
|
||||
doesn't exist.
|
||||
- `GET /admin/sla-policies` — list, optionally filtered by `productId`.
|
||||
- `GET /admin/sla-policies/:id` — `404` if not found.
|
||||
- `PATCH /admin/sla-policies/:id` — partial update, same existence checks as create.
|
||||
- `DELETE /admin/sla-policies/:id` — soft delete (`active: false`), never a hard delete (matches
|
||||
005/006 precedent for policy-shaped config the system may still reference).
|
||||
|
||||
## Business Calendar admin
|
||||
|
||||
- `POST /admin/business-calendars` — body `{ name, timezone, workingHours }`. `400` if
|
||||
`timezone` isn't a valid IANA zone name, or if any `workingHours` entry's `start`/`end` isn't a
|
||||
valid `HH:mm` pair with `start < end`.
|
||||
- `GET /admin/business-calendars` / `GET /admin/business-calendars/:id` — `404` if not found.
|
||||
- `PATCH /admin/business-calendars/:id` — same validation as create.
|
||||
- `POST /admin/business-calendars/:id/holidays` — body `{ date, description? }`.
|
||||
- `DELETE /admin/business-calendars/:id/holidays/:holidayId`.
|
||||
|
||||
## Escalation Policy / Rule admin
|
||||
|
||||
- `POST /admin/escalation-policies` — body `{ name, productId? }`. `404` if `productId` given
|
||||
but doesn't exist.
|
||||
- `GET /admin/escalation-policies` / `GET /admin/escalation-policies/:id`.
|
||||
- `POST /admin/escalation-policies/:id/rules` — body `{ triggerType, condition, targetNodeId,
|
||||
notify, active? }`. `triggerType` validated against doc 05 §6's full 10-value set (research.md
|
||||
— only 2 are ever evaluated, all 10 are valid config). `404` if `targetNodeId` doesn't
|
||||
reference an existing `HierarchyNode` (FR-012).
|
||||
- `PATCH /admin/escalation-policies/:id/rules/:ruleId` — same validation as create.
|
||||
- `DELETE /admin/escalation-policies/:id/rules/:ruleId` — soft delete (`active: false`).
|
||||
|
||||
## SLA run reads
|
||||
|
||||
- `GET /tickets/:ticketId/sla-run` — the current `SLARun` for the ticket, or `404` if none was
|
||||
ever created (e.g. the ticket was never assigned, or no policy matched at assignment time).
|
||||
|
||||
## Manual escalation
|
||||
|
||||
- `POST /tickets/:ticketId/escalate` — body `{ targetNodeId, reason }`. `404` if `ticketId` or
|
||||
`targetNodeId` doesn't exist (FR-017). Records an `EscalationEvent` with `triggeredBy` set to
|
||||
the calling actor and re-assigns via the same scoped-assignment path a rule-fired escalation
|
||||
uses (research.md).
|
||||
|
||||
## Guarantees (callable contract)
|
||||
|
||||
1. **An `SLARun` is created the moment a ticket receives its first successful assignment (007),
|
||||
if and only if an active `SLAPolicy` matches the ticket's context** — never for an unassigned
|
||||
ticket, never inventing a default policy when none matches (FR-005, US2).
|
||||
2. **`firstResponseDueAt`/`resolutionDueAt` are always computed by walking the resolved policy's
|
||||
business calendar**, excluding non-working hours, weekends, and holidays — never a naive
|
||||
`createdAt + N hours` addition (FR-004, SC-001).
|
||||
3. **A ticket entering `WAITING_FOR_CUSTOMER` pauses its running `SLARun`; leaving it resumes
|
||||
with the remaining time preserved exactly** — the paused duration is neither double-counted
|
||||
nor dropped, and this holds even if the process restarts while paused (FR-007/FR-008, SC-002).
|
||||
4. **A breach is detected within one breach-detection job cycle of its due date passing**, even
|
||||
if the process wasn't running at the exact due instant — never silently missed (FR-009,
|
||||
SC-003).
|
||||
5. **A run that completes before its due date is never marked breached; a paused run is never
|
||||
marked breached** (FR-010/FR-011).
|
||||
6. **Every `resolution_breach` or `first_response_breach` detection evaluates every active
|
||||
`EscalationRule` matching that trigger type under the ticket's resolved `EscalationPolicy`,
|
||||
firing one `EscalationEvent` (and one scoped re-assignment) per matching rule** — a breach
|
||||
with no matching rule is still recorded as breached, with no `EscalationEvent` (FR-013/FR-014/
|
||||
FR-015, SC-004).
|
||||
7. **A manual escalation to a nonexistent `targetNodeId` always returns `404` and creates neither
|
||||
an `EscalationEvent` nor a reassignment** (FR-017, SC-005).
|
||||
@@ -0,0 +1,127 @@
|
||||
# Data Model: SLA and Escalation
|
||||
|
||||
Field shapes below match `docs/06-database-schema.md` "Domain: SLA" / "Domain: Escalation"
|
||||
exactly, with two additive refinements called out explicitly (both purely additive — nothing in
|
||||
doc 06's shape is removed or narrowed).
|
||||
|
||||
## SLAPolicy
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `name` | `String` | |
|
||||
| `productId` | `String?` | wildcard when `null` — FK to `Product.id` |
|
||||
| `categoryId` | `String?` | wildcard when `null` — FK to `Category.id` |
|
||||
| `problemTypeId` | `String?` | wildcard when `null` — free-text reference, no `ProblemType` table exists in this codebase (problem taxonomy lives on `Problem` directly, per 004/005); stored and matched as opaque text |
|
||||
| `priority` | `String?` | wildcard when `null` — free-text, matches `Ticket.priority` |
|
||||
| `firstResponseMinutes` | `Int` | required — every policy must define a first-response target |
|
||||
| `investigationMinutes` | `Int?` | stored per doc 06; not read by any calculation in this feature (spec.md Assumptions) |
|
||||
| `resolutionMinutes` | `Int` | required |
|
||||
| `customerResponseMinutes` | `Int?` | stored per doc 06; not read by any calculation in this feature (spec.md Assumptions) |
|
||||
| `businessCalendarId` | `String?` | FK to `BusinessCalendar.id`; `null` means "24/7, no exclusions" (an explicit policy choice, not a missing-calendar error) |
|
||||
| `active` | `Boolean @default(true)` | inactive policies are excluded from resolution |
|
||||
| `createdAt` / `updatedAt` | `DateTime` | `updatedAt` used as the resolution tie-break (research.md) |
|
||||
|
||||
**Validation** (Zod, at the schema layer): `firstResponseMinutes > 0`, `resolutionMinutes > 0`,
|
||||
`investigationMinutes`/`customerResponseMinutes` positive when present; `productId`/`categoryId`/
|
||||
`businessCalendarId` must reference an existing row when provided (repository-level existence
|
||||
check, same convention as every prior feature's FK-shaped free-form input).
|
||||
|
||||
**Resolution** (`findApplicablePolicy(ticket)`): among active policies where each set scope field
|
||||
equals the ticket's corresponding value and each unset field is a wildcard, return the one with
|
||||
the fewest wildcards; tie-break by latest `updatedAt`. No match → no `SLARun` is created (FR-005).
|
||||
|
||||
## SLARun
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `ticketId` | `String @unique` | one run per ticket — no reopen-cycle support (spec.md Assumptions) |
|
||||
| `policyId` | `String` | FK to `SLAPolicy.id`, the policy resolved at creation time |
|
||||
| `firstResponseDueAt` | `DateTime?` | computed via the calendar walk from `assignedAt`; `null` when the policy has no `firstResponseMinutes`... (always present per policy validation, so effectively always set) |
|
||||
| `resolutionDueAt` | `DateTime?` | computed the same way from `resolutionMinutes` |
|
||||
| `status` | `String` | `running \| paused \| warning \| breached \| completed` — matches doc 06 exactly |
|
||||
| `pausedAt` | `DateTime?` | set when `status` transitions to `paused`; cleared on resume |
|
||||
| `resumedAt` | `DateTime?` | last resume timestamp, informational (audit convenience, mirrors `AssignmentHistory`'s always-append style) |
|
||||
| `breachedAt` | `DateTime?` | set once, the first time `resolutionDueAt` is detected passed while `running` |
|
||||
| `completedAt` | `DateTime?` | set when the ticket reaches a resolved/closed status; a completed run is never later marked breached (FR-011) |
|
||||
| **`firstResponseBreachedAt`** | `DateTime?` | **additive refinement, not in doc 06's literal listing** — records the first-response breach separately from `status`/`breachedAt`, which this feature reserves for the resolution timer; doubles as the idempotency guard for the breach-detection job (research.md) |
|
||||
|
||||
**Status transitions** (enforced in the service layer, not a DB constraint — same convention as
|
||||
`Ticket.status`'s 12-state machine in 003): `running → paused` (on ticket entering
|
||||
`WAITING_FOR_CUSTOMER`) → `running` (on leaving it, due dates shifted forward by the pause
|
||||
duration) → `breached` (resolution due date passed while running) → `completed` (ticket resolved/
|
||||
closed, from any of `running`/`paused`/`breached`). `warning` is reserved by doc 06's enum for a
|
||||
future near-breach signal; no code path in this feature sets it (documented, not implemented —
|
||||
same discipline as the 8 inert `EscalationRule.triggerType` values).
|
||||
|
||||
## BusinessCalendar
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `name` | `String` | |
|
||||
| `timezone` | `String` | IANA zone name (e.g. `"America/New_York"`), validated against `Intl.supportedValuesOf('timeZone')` at the schema layer |
|
||||
| `workingHours` | `Json` | shape: `{ mon?: {start: "HH:mm", end: "HH:mm"}, tue?: ..., wed?: ..., thu?: ..., fri?: ..., sat?: ..., sun?: ... }` — a missing key means zero working hours that weekday (research.md) |
|
||||
| `holidays` | `Holiday[]` | |
|
||||
|
||||
## Holiday
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `calendarId` | `String` | FK to `BusinessCalendar.id` |
|
||||
| `date` | `DateTime` | compared by calendar date only (year/month/day in the calendar's own timezone), not by exact instant |
|
||||
| `description` | `String?` | |
|
||||
|
||||
## EscalationPolicy
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `name` | `String` | |
|
||||
| `productId` | `String?` | wildcard (global) when `null` |
|
||||
| `active` | `Boolean @default(true)` | |
|
||||
| `rules` | `EscalationRule[]` | |
|
||||
|
||||
## EscalationRule
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `policyId` | `String` | FK to `EscalationPolicy.id` |
|
||||
| `triggerType` | `String` | one of doc 05 §6's 10 values; schema accepts all 10, only `resolution_breach`/`first_response_breach` are ever evaluated (research.md) |
|
||||
| `condition` | `Json` | stored, not evaluated, by this feature (research.md) |
|
||||
| `targetNodeId` | `String` | FK to `HierarchyNode.id`, validated to exist at creation time (FR-017's rejection rule applies identically here) |
|
||||
| `notify` | `Json` | who/how to notify — stored and returned only; no delivery mechanism exists (spec.md Assumptions, `platform/notifications` untouched) |
|
||||
| `active` | `Boolean @default(true)` | |
|
||||
|
||||
## EscalationEvent
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `id` | `String @id @default(cuid())` | |
|
||||
| `ticketId` | `String` | FK to `Ticket.id` |
|
||||
| `ruleId` | `String?` | `null` for a manual escalation or a breach with no matching rule |
|
||||
| `fromNodeId` | `String?` | the node the ticket was assigned to immediately before this event, if any |
|
||||
| `toNodeId` | `String?` | the rule's `targetNodeId` (or the manually-specified node); `null` when no rule matched |
|
||||
| `reason` | `String` | free text — for a rule firing, a generated description (e.g. `"resolution SLA breached"`); for manual escalation, the caller-supplied reason |
|
||||
| `triggeredBy` | `String` | `system \| <agentId> \| <adminId>` — never a bare `"customer"` literal in this feature's own write paths (doc 06 lists it as a valid value for a future customer-initiated trigger type, not one this feature fires) |
|
||||
| `createdAt` | `DateTime @default(now())` | |
|
||||
|
||||
## Relations added to existing models
|
||||
|
||||
- `Ticket.slaRun SLARun?` (inverse of `SLARun.ticketId @unique`)
|
||||
- `Ticket.escalationEvents EscalationEvent[]`
|
||||
- `Product.slaPolicies SLAPolicy[]`, `Product.escalationPolicies EscalationPolicy[]`
|
||||
- `Category.slaPolicies SLAPolicy[]`
|
||||
- `HierarchyNode.escalationRules EscalationRule[]` (inverse of `targetNodeId`)
|
||||
|
||||
## Out of scope for this data model (per spec.md Assumptions)
|
||||
|
||||
- No `investigationDueAt`/`customerResponseDueAt` fields — doc 06's `SLARun` doesn't define them,
|
||||
and nothing in spec.md's acceptance scenarios exercises them; `investigationMinutes`/
|
||||
`customerResponseMinutes` remain stored-but-unused on `SLAPolicy`, same as doc 06 itself defines.
|
||||
- No FK tightening of `HierarchyNode.slaPolicyId`/`escalationPolicyId` (still free-text, per 006) —
|
||||
SLA policy resolution in this feature is scope-based (product/category/problemType/priority),
|
||||
not looked up through those two fields; they remain unvalidated free text, unchanged from 006.
|
||||
@@ -0,0 +1,151 @@
|
||||
# Implementation Plan: SLA and Escalation
|
||||
|
||||
**Branch**: `008-sla-escalation` | **Date**: 2026-09-03 | **Spec**: [spec.md](./spec.md)
|
||||
|
||||
**Input**: Feature specification from `specs/008-sla-escalation/spec.md`
|
||||
|
||||
## Summary
|
||||
|
||||
Populate the existing `platform/business-calendars`, `orchestration/sla`, and
|
||||
`orchestration/escalation` stub directories (today: `isWorkingHour` hardcoded `true`, a
|
||||
`SlaDueDateCalculator` doing naive `createdAt + hours` addition, an `EscalationEngine` that
|
||||
always returns `{ escalated: false }`) with the real engine: `business-calendars` walks a
|
||||
`BusinessCalendar`'s `workingHours`/`Holiday` records via `luxon` to compute calendar-aware
|
||||
durations; `sla` resolves the most-specific matching `SLAPolicy` on a ticket's first successful
|
||||
007 assignment, computes `firstResponseDueAt`/`resolutionDueAt` through the calendar walk,
|
||||
durably pauses/resumes on `WAITING_FOR_CUSTOMER` transitions (a `TICKET_UPDATED` domain-event
|
||||
subscriber), and detects breaches via a repeatable BullMQ job; `escalation` resolves the
|
||||
applicable `EscalationPolicy`, fires an `EscalationEvent` per matching active `EscalationRule` on
|
||||
a breach (or on a manual request), and re-assigns through a new, specifically-scoped entry point
|
||||
added to 007's `AssignmentEngine`.
|
||||
|
||||
## Technical Context
|
||||
|
||||
**Language/Version**: TypeScript 5.4 / Node.js 20+.
|
||||
|
||||
**Primary Dependencies**: Prisma (new models), Zod, BullMQ (already a dependency — new
|
||||
repeatable job, same queue infrastructure as 003's attachment scan and 005's AI session queues),
|
||||
`luxon` (**new** — the first date/timezone library in this codebase; research.md).
|
||||
|
||||
**Storage**: PostgreSQL via Prisma (new `SLAPolicy`, `SLARun`, `BusinessCalendar`, `Holiday`,
|
||||
`EscalationPolicy`, `EscalationRule`, `EscalationEvent` models). No new infrastructure — reuses
|
||||
`src/infrastructure/queue` for the breach-detection job, same as every prior BullMQ consumer.
|
||||
|
||||
**Testing**: Vitest — unit tests for the calendar-walk algorithm (weekend/holiday exclusion,
|
||||
partial-day clipping, timezone correctness), the most-specific SLA-policy match, and pause/resume
|
||||
arithmetic; integration tests for the full assignment→SLA-run→pause/resume→breach→escalation
|
||||
flow against real Postgres/Redis, including one test that rebuilds `buildApp()` mid-test to
|
||||
verify pause/resume state survives a genuine process-restart boundary (Constitution Principle
|
||||
VII, quickstart Scenario 3) — the first feature in this codebase whose correctness depends on
|
||||
that guarantee specifically, not just within-process concurrency safety.
|
||||
|
||||
**Target Platform**: Same Fastify modular monolith. Populates existing module directories:
|
||||
`src/modules/platform/business-calendars/`, `src/modules/orchestration/{sla,escalation}/`. Adds
|
||||
one new BullMQ worker registration alongside the existing ones in `src/infrastructure/queue`.
|
||||
|
||||
**Project Type**: Backend service — single project.
|
||||
|
||||
**Performance Goals**: The breach-detection job must complete a full scan-and-mark pass in
|
||||
well under its own tick interval even as `SLARun` rows accumulate — indexed on
|
||||
`(status, resolutionDueAt)` so the query stays a targeted range scan, not a table scan. Not
|
||||
otherwise performance-sensitive.
|
||||
|
||||
**Constraints**: MUST NOT compute due dates naively (FR-004); MUST create an `SLARun` only on a
|
||||
successful assignment with a matching policy (FR-005); MUST survive a process restart for
|
||||
pause/resume and breach detection (FR-007/FR-008/FR-009, Constitution Principle VII); MUST never
|
||||
mark a completed-in-time or paused run breached (FR-010/FR-011); MUST re-assign scoped to the
|
||||
rule's exact `targetNodeId`, not a fresh unscoped resolution (FR-014).
|
||||
|
||||
**Scale/Scope**: Three populated modules, one new dependency, three admin CRUD surfaces (SLA
|
||||
policies, business calendars, escalation policies/rules), one manual-escalation endpoint, one
|
||||
new BullMQ repeatable job, one new method on 007's `AssignmentEngine`. Explicitly excludes:
|
||||
notification delivery, 8 of doc 05's 10 escalation trigger types, investigation/customer-response
|
||||
timers, SLA restart on ticket reopen (see spec.md Assumptions).
|
||||
|
||||
## Constitution Check
|
||||
|
||||
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
|
||||
|
||||
| Principle / Section | Check | Result |
|
||||
|---|---|---|
|
||||
| I. SaaS Is the Sole Identity & Access Authority | SLA/escalation reference `Ticket`/`HierarchyNode`/`Agent` — all SupportHub's own domain. No SaaS identity touched. | PASS |
|
||||
| II. Configuration Over Hardcoding | Every SLA target, calendar, and escalation rule is admin-configured data, not a hardcoded constant — replacing the literal hardcoded-`true`/naive-arithmetic stubs is the point of this feature. | PASS |
|
||||
| III. Layered Architecture With Enforced Module Boundaries | Three modules follow the standard shape; `orchestration/sla`→`platform/business-calendars` and `orchestration/escalation`→`orchestration/sla` (for breach signals) and →`orchestration/assignments` (007, for the new scoped-assignment method) are all one-directional — no cycle, since 007 doesn't import anything from 008. | PASS |
|
||||
| IV. AI Recommends, Deterministic Policy Decides | No AI involvement in this feature at all — every decision (policy match, breach, escalation) is deterministic. | PASS — N/A |
|
||||
| V. Evidence-Based Verification | Not applicable — no resolution/verification concept in this feature. | PASS — N/A |
|
||||
| VI. Durable Audit & History | `EscalationEvent` is the durable, append-only record doc 06 defines for every escalation, automatic or manual — mirrors `AssignmentHistory`'s established shape. | PASS |
|
||||
| VII. Concurrency-Safe, Durable Job Handling | This principle's "state must survive a process restart" clause is directly load-bearing here for the first time as the primary correctness requirement (not just a concurrent-request race) — pause/resume and breach detection are both pure-DB-state-plus-polling-job, no in-memory timer anywhere (research.md, quickstart Scenario 3). | PASS |
|
||||
| VIII. Problem and Ticket Are Separate, Related Entities | SLA/escalation reference `Ticket`, not `Problem` — doesn't touch the distinction. | PASS — N/A |
|
||||
| Technology & Platform Constraints | Prisma + Zod + existing BullMQ infrastructure, plus the one new `luxon` dependency (justified in research.md — no timezone-correct alternative already exists in this codebase). | PASS |
|
||||
|
||||
No violations requiring Complexity Tracking justification.
|
||||
|
||||
## Post-Design Constitution Re-check
|
||||
|
||||
All gates above remain PASS after Phase 1 design. Worth calling out against Principle VII
|
||||
explicitly: pause/resume shifts a single absolute `DateTime` column and breach detection is a
|
||||
plain polling query — by design there is no code path in this feature that could even *appear*
|
||||
to depend on in-memory state surviving a restart, which is what makes the restart-boundary
|
||||
integration test (quickstart Scenario 3) a meaningful verification rather than a formality.
|
||||
|
||||
## Project Structure
|
||||
|
||||
### Documentation (this feature)
|
||||
|
||||
```text
|
||||
specs/008-sla-escalation/
|
||||
├── plan.md # This file
|
||||
├── research.md # Phase 0 output
|
||||
├── data-model.md # Phase 1 output
|
||||
├── quickstart.md # Phase 1 output
|
||||
├── contracts/ # Phase 1 output
|
||||
└── tasks.md # Phase 2 output (/speckit-tasks — not created here)
|
||||
```
|
||||
|
||||
### Source Code (repository root)
|
||||
|
||||
```text
|
||||
supporthub-api/
|
||||
├── package.json # MODIFIED — add luxon, @types/luxon
|
||||
├── prisma/
|
||||
│ └── schema.prisma # MODIFIED — add SLAPolicy, SLARun,
|
||||
│ BusinessCalendar, Holiday, EscalationPolicy,
|
||||
│ EscalationRule, EscalationEvent
|
||||
├── src/
|
||||
│ ├── infrastructure/
|
||||
│ │ └── queue/ # MODIFIED — register the breach-detection
|
||||
│ │ repeatable job alongside existing workers
|
||||
│ └── modules/
|
||||
│ ├── platform/
|
||||
│ │ └── business-calendars/ # REPLACED stub — full standard shape +
|
||||
│ │ ├── controller/ routes/ schema/ calculators/ for the day-walk algorithm
|
||||
│ │ │ repository/ service/ types/
|
||||
│ │ │ mapper/ constants/ index.ts
|
||||
│ │ └── calculators/
|
||||
│ └── orchestration/
|
||||
│ ├── sla/ # REPLACED stub — full standard shape, keeps
|
||||
│ │ ├── controller/ routes/ schema/ its existing calculators/ dir (due-date
|
||||
│ │ │ repository/ service/ types/ calculator replaced, not removed) and adds
|
||||
│ │ │ mapper/ constants/ index.ts engine/ for breach evaluation
|
||||
│ │ ├── engine/ (policy resolution + breach detection)
|
||||
│ │ └── calculators/ (due-date calculator, replaced)
|
||||
│ └── escalation/ # REPLACED stub — full standard shape, keeps
|
||||
│ ├── controller/ routes/ schema/ engine/ for rule matching + firing
|
||||
│ │ repository/ service/ types/
|
||||
│ │ mapper/ constants/ index.ts
|
||||
│ └── engine/
|
||||
└── tests/
|
||||
├── unit/
|
||||
│ ├── platform/business-calendars/ # calendar-walk algorithm
|
||||
│ └── orchestration/{sla,escalation}/ # policy match, breach logic, rule match
|
||||
└── integration/ # full flow incl. restart-boundary test
|
||||
```
|
||||
|
||||
**Structure Decision**: Single project. `business-calendars` gets a full standard shape (not
|
||||
internal-only) since it needs its own CRUD surface for calendars/holidays, unlike 007's
|
||||
internal-only `routing` module. `sla` and `escalation` each keep the `engine/` extension doc 07
|
||||
§8 reserves for modules with real decision logic, matching 005/007 precedent.
|
||||
|
||||
## Complexity Tracking
|
||||
|
||||
*No constitution violations — table intentionally omitted.*
|
||||
@@ -0,0 +1,77 @@
|
||||
# Quickstart: Validating SLA and Escalation
|
||||
|
||||
Prerequisites: migrations applied; at least one hierarchy node/agent/product set up per
|
||||
006-support-organization's and 007-orchestration-assignment's own quickstarts, since this feature
|
||||
starts an `SLARun` on a successful 007 assignment and escalation re-assigns through 007's engine.
|
||||
|
||||
## Scenario 1 — policy definition and most-specific match (User Story 1)
|
||||
|
||||
1. Create a global `SLAPolicy` (`productId: null`, ...) and a second, product-scoped policy for
|
||||
the same product with tighter minutes.
|
||||
2. Assign a ticket for that product (triggers Scenario 2's creation path).
|
||||
3. **Expected**: the `SLARun` resolves the product-scoped policy, not the global one.
|
||||
4. Delete the product-scoped policy's applicability (set `active: false`). Assign a new ticket for
|
||||
the same product. **Expected**: falls back to the global policy.
|
||||
|
||||
## Scenario 2 — calendar-aware due dates on assignment (User Story 2)
|
||||
|
||||
1. Create a `BusinessCalendar` with `workingHours` only Mon-Fri 09:00-17:00, `timezone`
|
||||
`"America/New_York"`, and one `Holiday` next Monday. Attach it to an `SLAPolicy` with
|
||||
`resolutionMinutes: 480` (one working day).
|
||||
2. Assign a ticket late on a Friday afternoon so that a naive `createdAt + 480min` would land on
|
||||
Saturday.
|
||||
3. **Expected**: `resolutionDueAt` lands the following Tuesday (Monday excluded as a holiday),
|
||||
never on the weekend.
|
||||
4. Assign a ticket for a product/category/priority combination matching no active policy.
|
||||
**Expected**: no `SLARun` is created; `GET /tickets/:ticketId/sla-run` returns `404`.
|
||||
|
||||
## Scenario 3 — durable pause/resume across a process restart (User Story 3)
|
||||
|
||||
1. Assign a ticket (Scenario 2), note `resolutionDueAt`.
|
||||
2. Transition the ticket to `WAITING_FOR_CUSTOMER`. **Expected**: `SLARun.status` becomes
|
||||
`paused`, `pausedAt` set.
|
||||
3. Restart the application process (rebuild `buildApp()` fresh, simulating the restart the
|
||||
constitution's Principle VII requires surviving).
|
||||
4. Wait a real interval, then transition the ticket out of `WAITING_FOR_CUSTOMER`.
|
||||
**Expected**: `SLARun.status` becomes `running`; the new `resolutionDueAt` equals the original
|
||||
plus exactly the paused wall-clock duration — never reset to a fresh full duration.
|
||||
|
||||
## Scenario 4 — durable breach detection (User Story 4)
|
||||
|
||||
1. Assign a ticket against a policy with a very short `resolutionMinutes` (e.g. `1`) and a 24/7
|
||||
calendar (`businessCalendarId: null`).
|
||||
2. Wait past `resolutionDueAt` without resolving the ticket.
|
||||
3. **Expected**: within one breach-detection job tick, `SLARun.status` becomes `breached`,
|
||||
`breachedAt` set.
|
||||
4. Repeat, but resolve the ticket before `resolutionDueAt` passes. **Expected**: `status` reaches
|
||||
`completed` and is never later flipped to `breached` by a subsequent job tick.
|
||||
5. Repeat, but pause the run before `resolutionDueAt` passes. **Expected**: the run is never
|
||||
marked `breached` while paused, even after the due instant passes.
|
||||
|
||||
## Scenario 5 — breach-triggered escalation and scoped re-assignment (User Story 5)
|
||||
|
||||
1. Create an `EscalationPolicy` scoped to the ticket's product with an active `EscalationRule`
|
||||
(`triggerType: "resolution_breach"`, `targetNodeId` set to a second hierarchy node with a
|
||||
different eligible agent).
|
||||
2. Reach a `breached` run (Scenario 4). **Expected**: exactly one `EscalationEvent` is created
|
||||
(`ruleId` set, `toNodeId` the rule's `targetNodeId`), and the ticket is reassigned to an agent
|
||||
eligible under that specific node — not re-resolved from the ticket's original context.
|
||||
3. Repeat with no matching `EscalationRule` for the resolved policy. **Expected**: the run is
|
||||
still marked `breached`; no `EscalationEvent` is created.
|
||||
|
||||
## Scenario 6 — manual escalation (User Story 6)
|
||||
|
||||
1. `POST /tickets/:ticketId/escalate` with a valid `targetNodeId` and a reason.
|
||||
2. **Expected**: an `EscalationEvent` is created (`ruleId: null`, `triggeredBy` the calling
|
||||
actor), and the ticket is reassigned through the same scoped path as Scenario 5.
|
||||
3. Repeat with a nonexistent `targetNodeId`. **Expected**: `404`, no `EscalationEvent` created.
|
||||
4. Trigger a manual escalation on a ticket whose run is concurrently being auto-escalated by
|
||||
Scenario 5's breach path. **Expected**: both `EscalationEvent` rows are recorded; the final
|
||||
assignment reflects 007's already-tested concurrency handling, not a corrupted double-write.
|
||||
|
||||
## What "done" looks like
|
||||
|
||||
All six scenarios pass, and together they demonstrate every functional requirement and success
|
||||
criterion in `spec.md` — including SC-002's explicit restart-survival requirement, which must be
|
||||
verified by an actual fresh `buildApp()` in the middle of the test, not merely by asserting on
|
||||
stored field values without ever exercising a real process boundary.
|
||||
@@ -0,0 +1,175 @@
|
||||
# Phase 0 Research: SLA and Escalation
|
||||
|
||||
## Decision: Module placement — three existing stubs, mapped directly
|
||||
|
||||
- **Decision**: `platform/business-calendars` (currently a one-file stub,
|
||||
`isWorkingHour` hardcoded `true`), `orchestration/sla` (stub `SlaEngine.evaluateSlaTargets`
|
||||
always returns `NORMAL`; stub `SlaDueDateCalculator` does naive `createdAt + hours`), and
|
||||
`orchestration/escalation` (stub `EscalationEngine.triggerEscalation` always returns
|
||||
`{ escalated: false }`) are populated directly, matching doc 07's placement exactly — no new
|
||||
module locations invented.
|
||||
- **Rationale**: Documented layout, not an open choice; every stub's current behavior is exactly
|
||||
what doc 05 §5 explicitly warns against (`SlaDueDateCalculator`'s naive addition is the literal
|
||||
anti-pattern FR-004 forbids) — replacing it is the point of this feature.
|
||||
- **Alternatives considered**: None.
|
||||
|
||||
## Decision: A real timezone-aware date library — `luxon` — is a genuinely new dependency
|
||||
|
||||
- **Decision**: Add `luxon` (a single package, no companion timezone package needed, IANA
|
||||
timezone support built in) for every calendar-aware date computation in this feature.
|
||||
- **Rationale**: No date/timezone library exists anywhere in this codebase yet — every prior
|
||||
feature's `DateTime`/`Json`-typed "schedule" fields (e.g. 006's `AgentAvailability.
|
||||
workingHours`) were stored but never actually walked by any code. This feature is the first to
|
||||
need to *compute* against calendar time correctly (FR-004's explicit "MUST NOT... ignore the
|
||||
calendar"), and hand-rolling DST-correct, IANA-timezone-aware business-hour arithmetic without
|
||||
a library is exactly the kind of mistake this system's own constitution warns against elsewhere
|
||||
("don't reinvent what a library already solves correctly" is this codebase's working norm, even
|
||||
if not literally in the constitution's text) — matching the same "one new dependency for the
|
||||
one new genuinely-needed capability" precedent 005 set for `@anthropic-ai/sdk`.
|
||||
- **Alternatives considered**: `date-fns` + `date-fns-tz` (two packages for the same
|
||||
capability) — rejected in favor of the single-package option. Hand-rolled arithmetic —
|
||||
rejected; timezone/DST correctness is precisely the kind of subtly-wrong-most-of-the-time code
|
||||
a library exists to prevent.
|
||||
|
||||
## Decision: `BusinessCalendar.workingHours` shape — one window per weekday
|
||||
|
||||
- **Decision**: `{ mon?: { start: "09:00", end: "17:00" }, tue?: ..., ..., sun?: ... }` — three-
|
||||
letter weekday keys, `HH:mm` 24-hour strings interpreted in the calendar's own `timezone`, a
|
||||
missing key meaning "not a working day" (FR's "unconfigured day contributes zero time").
|
||||
- **Rationale**: Doc 05 §5's stated need ("business hours, weekends... per-team schedules") is
|
||||
satisfied by one contiguous window per day — doc 06 doesn't specify a richer shape (split
|
||||
shifts), and nothing in spec.md asks for one; a single window per day is the simplest structure
|
||||
that satisfies every acceptance scenario without speculative complexity.
|
||||
- **Alternatives considered**: An array of windows per day (split-shift support) — rejected as
|
||||
unrequested scope; the shape can be extended later (an array is a strict superset) without a
|
||||
breaking change to a single-window calendar's own data.
|
||||
|
||||
## Decision: Calendar-aware due-date arithmetic — a day-by-day walk
|
||||
|
||||
- **Decision**: `addBusinessMinutes(start, minutes, calendar, holidays)` walks forward from
|
||||
`start` one calendar day at a time (in the calendar's timezone): a holiday date or a weekday
|
||||
with no configured window contributes zero available minutes; otherwise the day's working
|
||||
window (clipped by `start`'s own time on the first day) contributes up to its own duration,
|
||||
consumed from the running `minutes` total; the walk ends the moment `minutes` reaches zero,
|
||||
returning that exact timestamp.
|
||||
- **Rationale**: This directly implements FR-004 — every acceptance scenario (weekend/holiday
|
||||
exclusion) is a direct consequence of this algorithm, not a special case bolted on. A
|
||||
day-granularity loop is bounded (even a multi-week SLA window is, at most, a few dozen
|
||||
iterations) and easy to unit-test exhaustively.
|
||||
- **Alternatives considered**: Minute-by-minute simulation — rejected as needlessly slow and
|
||||
harder to reason about for the same result; day-granularity with within-day clipping is exactly
|
||||
as correct and far simpler.
|
||||
|
||||
## Decision: SLA policy resolution — most-specific match, same shape as 005's confidence policy
|
||||
|
||||
- **Decision**: Given a ticket's `productId`/`categoryId`/`problemTypeId`/`priority`, an active
|
||||
`SLAPolicy` matches when each of its own scope fields is either `null` (wildcard) or equal to
|
||||
the ticket's corresponding value. Among matches, the one with the fewest `null` scope fields
|
||||
(most specific) wins; a tie is broken by most-recently-`updatedAt`.
|
||||
- **Rationale**: FR-002 requires most-specific-match, not first-found — this is the same
|
||||
resolution shape 005's `AIConfidencePolicy` and 006's hierarchy scope matching already
|
||||
established in this codebase, reused rather than reinvented a third time.
|
||||
- **Alternatives considered**: A single global default policy with per-scope overrides (005's
|
||||
`(productId, categoryId)` two-level shape) — rejected; SLA policy has four independent scope
|
||||
dimensions doc 06 itself defines, so a strict specificity count (not a fixed lookup order) is
|
||||
the correct generalization.
|
||||
|
||||
## Decision: Pause/resume — shift the absolute due date by the paused wall-clock duration
|
||||
|
||||
- **Decision**: Pausing records `pausedAt = now()` (status → `paused`); resuming shifts
|
||||
`resolutionDueAt` (and `firstResponseDueAt`, if still pending) forward by `now() - pausedAt`
|
||||
and clears `pausedAt` (status → `running`). No separate "remaining minutes" bookkeeping field
|
||||
is needed — the absolute due-date field itself, shifted, *is* the remaining-time record.
|
||||
- **Rationale**: FR-007/FR-008/SC-002 require the paused duration to be excluded, durably, across
|
||||
a restart — shifting an absolute timestamp already stored in Postgres satisfies both with the
|
||||
simplest possible mechanism; no in-memory state exists at any point.
|
||||
- **Alternatives considered**: Storing remaining minutes and recomputing the due date via the
|
||||
calendar walk on every resume — rejected as unnecessary; the pause window itself doesn't need
|
||||
calendar-awareness (a paused SLA isn't "elapsing" business time by definition, so shifting by
|
||||
real wall-clock pause duration is exactly correct, not an approximation).
|
||||
|
||||
## Decision: Breach detection — one repeatable BullMQ job, not one delayed job per run
|
||||
|
||||
- **Decision**: A single repeatable job (e.g. every 60 seconds) queries every `SLARun` with
|
||||
`status: 'running'` whose `resolutionDueAt <= now()`, marking each `breached` — and separately,
|
||||
every running run with `firstResponseDueAt <= now()` and no `firstResponseBreachedAt` yet
|
||||
(data-model.md refinement) and no `AGENT_MESSAGE` recorded for the ticket, marking
|
||||
`firstResponseBreachedAt`. Each newly-detected breach triggers escalation-rule evaluation
|
||||
(research.md below).
|
||||
- **Rationale**: Constitution Principle VII requires durability, not sub-second precision — a
|
||||
short-interval polling job is trivially durable (BullMQ's repeatable jobs are themselves
|
||||
persisted, and a missed tick is caught by the next one) and avoids the bookkeeping a
|
||||
per-run delayed-job approach would need on every pause/resume (canceling and rescheduling a
|
||||
delayed job each time, versus just updating a timestamp a polling query already reads).
|
||||
- **Alternatives considered**: One delayed BullMQ job scheduled per `SLARun`, rescheduled on every
|
||||
pause/resume — rejected; every pause/resume would need to cancel and re-add a job, doubling the
|
||||
operations pause/resume already does, for a precision (sub-minute breach detection) nothing in
|
||||
spec.md actually requires.
|
||||
|
||||
## Decision: Escalation firing reuses 007's `AssignmentEngine`, scoped to a specific node
|
||||
|
||||
- **Decision**: `AssignmentEngine` (007) gains a new method, `assignToSpecificNode(ticketId,
|
||||
hierarchyNodeId, strategyOverride?, actor, reason?)` — resolves the eligible-agent set the same
|
||||
way `RoutingService` already does, but scoped to exactly the given node (its own `skills`
|
||||
unioned with the ticket's derived required skills, per 007's existing composition rule) rather
|
||||
than 007's general "find whichever node matches the ticket's context" resolution. Runs the
|
||||
node's own configured strategy (or `strategyOverride`) and persists through the same
|
||||
`Assignment`/`AssignmentHistory` mechanism 007 already built and tested for concurrent writes.
|
||||
- **Rationale**: FR-014 requires escalation to land the ticket specifically at the rule's
|
||||
`targetNodeId` — 007's existing `evaluateAndAssign` always re-derives the applicable node from
|
||||
ticket context, which could resolve to a *different* node than the one the rule targeted (the
|
||||
ticket's context hasn't changed, only its status has). A new, explicit "assign to this node"
|
||||
entry point is the correct extension, not a workaround.
|
||||
- **Alternatives considered**: Having 008 duplicate 007's eligible-agent-resolution and
|
||||
`Assignment`-persistence logic — rejected; directly against this codebase's repeated "extend an
|
||||
existing module's public surface for a later feature" precedent (004's `productsRepository`,
|
||||
005's `problemsRepository`, 007's own reuse of 006's `capabilityLookupService`).
|
||||
|
||||
## Decision: `EscalationRule.triggerType` is stored broadly; only two types are ever evaluated
|
||||
|
||||
- **Decision**: The Zod schema for creating a rule accepts any of doc 06's ten `triggerType`
|
||||
values — an admin can configure a rule for `inactivity` or `critical_incident` today, and it
|
||||
will simply never fire (no code path evaluates those triggers yet), rather than being rejected
|
||||
at creation time.
|
||||
- **Rationale**: spec.md's Assumptions state this explicitly — storing configuration ahead of the
|
||||
event source that will eventually feed it is this codebase's established pattern (006's
|
||||
`slaPolicyId` stored before this feature existed to validate it); rejecting valid doc-06-shaped
|
||||
configuration at the schema layer would be a regression from that pattern, not a safety
|
||||
improvement (nothing unsafe happens from an inert rule sitting unfired).
|
||||
- **Alternatives considered**: Restricting the schema to only the two implemented trigger types —
|
||||
rejected; would force a breaking schema change on every future phase that wires up one more
|
||||
trigger type, for no correctness benefit today.
|
||||
|
||||
## Decision: Escalation policy resolution — product match or global, most-specific first
|
||||
|
||||
- **Decision**: `EscalationPolicy.productId` is the only scope dimension doc 06 gives it (unlike
|
||||
`SLAPolicy`'s four). Resolution: prefer an active policy whose `productId` equals the ticket's
|
||||
product; fall back to an active policy with `productId: null` (a global policy) if no
|
||||
product-specific one exists. A breach with neither is recorded breached with no rule evaluated
|
||||
(spec.md Edge Cases: "a breach with no matching rule is still recorded as breached").
|
||||
- **Rationale**: Same most-specific-first shape as `SLAPolicy`, degenerately simple because doc 06
|
||||
only gives `EscalationPolicy` one scope field — no new resolution mechanism invented.
|
||||
- **Alternatives considered**: None; doc 06's shape leaves no other reasonable reading.
|
||||
|
||||
## Decision: `EscalationRule.condition` is stored, not evaluated, by this feature
|
||||
|
||||
- **Decision**: Every active `EscalationRule` under the resolved policy whose `triggerType`
|
||||
matches the firing breach type (`resolution_breach` or `first_response_breach`) fires — the
|
||||
`condition` Json field is persisted as given at creation but not parsed or evaluated as a
|
||||
filter.
|
||||
- **Rationale**: spec.md's FR-013 says "every matching active EscalationRule fires" scoped by
|
||||
trigger type alone; nothing in spec.md defines a `condition` grammar to evaluate, and inventing
|
||||
one now would be exactly the kind of unrequested scope this codebase's established discipline
|
||||
(005's inert trigger types, 006's unvalidated `slaPolicyId`) consistently avoids. `condition` is
|
||||
accepted and returned by the CRUD schema so a future feature can give it real meaning without a
|
||||
breaking change.
|
||||
- **Alternatives considered**: A minimal condition-matching evaluator (e.g. `{ minPriority }`) —
|
||||
rejected as speculative; spec.md never asked for conditional rule filtering beyond trigger type.
|
||||
|
||||
## Decision: SLA/Escalation admin endpoints reuse the existing auth stub
|
||||
|
||||
- **Decision**: Every admin CRUD endpoint (policies, calendars, escalation rules) and the manual-
|
||||
escalation endpoint are gated by `fastify.authenticate`, same known-limitation stub as every
|
||||
prior feature.
|
||||
- **Rationale**: Consistency with established precedent.
|
||||
- **Alternatives considered**: None.
|
||||
Reference in New Issue
Block a user