Compare commits
12
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 saqib mirandClaude Sonnet 5
|
ea50e3596a | ||
|
saqib mirandClaude Sonnet 5
|
de5915a8c1 | ||
|
saqib mirandClaude Sonnet 5
|
f75589d9ce | ||
|
saqib mirandClaude Sonnet 5
|
acd3843aaf | ||
|
saqib mirandClaude Sonnet 5
|
0135e4ca05 | ||
|
saqib mirandClaude Sonnet 5
|
5a0fe9f847 | ||
|
saqib mirandClaude Sonnet 5
|
09ba56d3af | ||
|
saqib mirandClaude Sonnet 5
|
79bc2ef25b | ||
|
saqib mirandClaude Sonnet 5
|
3bdccc901f | ||
|
saqib mirandClaude Sonnet 5
|
65175a85b5 | ||
|
saqib mirandClaude Sonnet 5
|
52f1fa3db0 | ||
|
saqib mirandClaude Sonnet 5
|
b016e77b70 |
Generated
+157
-25
@@ -17,7 +17,10 @@
|
||||
"@fastify/swagger": "^8.14.0",
|
||||
"@fastify/swagger-ui": "^3.0.0",
|
||||
"@opentelemetry/api": "^1.8.0",
|
||||
"@opentelemetry/sdk-trace-base": "^1.22.0",
|
||||
"@opentelemetry/context-async-hooks": "^2.11.0",
|
||||
"@opentelemetry/exporter-trace-otlp-http": "^0.222.0",
|
||||
"@opentelemetry/resources": "^2.11.0",
|
||||
"@opentelemetry/sdk-trace-base": "^2.11.0",
|
||||
"@prisma/client": "^5.12.1",
|
||||
"bcryptjs": "^3.0.3",
|
||||
"bullmq": "^5.7.1",
|
||||
@@ -1387,58 +1390,187 @@
|
||||
"node": ">=8.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/core": {
|
||||
"version": "1.30.1",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-1.30.1.tgz",
|
||||
"integrity": "sha512-OOCM2C/QIURhJMuKaekP3TRBxBKxG/TWWA0TL2J6nXUtDnuCtccy49LUJF8xPFXMX+0LMcxFpCo8M9cGY1W6rQ==",
|
||||
"node_modules/@opentelemetry/api-logs": {
|
||||
"version": "0.222.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/api-logs/-/api-logs-0.222.0.tgz",
|
||||
"integrity": "sha512-9mb1If+IF6u0ZVXkHQ6ogEae5HwA6ajIVUgpSDQyRASxft6BSXHvBvPooRle3yFN/fKnCdSOnuu0OC3PLcF6+g==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/semantic-conventions": "1.28.0"
|
||||
"@opentelemetry/api": "^1.3.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=14"
|
||||
"node": ">=8.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/context-async-hooks": {
|
||||
"version": "2.11.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/context-async-hooks/-/context-async-hooks-2.11.0.tgz",
|
||||
"integrity": "sha512-Tr79DyWI8itsBdg+jH+opjfrwLzX+erk1/ExkIwhWoAVjVrJIn2y5+cGjTC0Vy8fyNIA/y8wuJPZwr1T3xCZeQ==",
|
||||
"license": "Apache-2.0",
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.0.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/core": {
|
||||
"version": "2.11.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.11.0.tgz",
|
||||
"integrity": "sha512-7YP44XH0tV6+Mb54x2YGf84i7yi+31MBZlE8JwvozkxyTvXbSp10X7cI7YE49ChJ3shMJoBmCJF3+1QFBJctGA==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/semantic-conventions": "^1.29.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.0.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/exporter-trace-otlp-http": {
|
||||
"version": "0.222.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/exporter-trace-otlp-http/-/exporter-trace-otlp-http-0.222.0.tgz",
|
||||
"integrity": "sha512-RCnPWcHppwiquQ+cV3nWvNwdf0MG1w26e5jewW2T83nTZOlXgcg88sY9ulCVgagGHcq1mj0L0GP6YHgzk2v8oA==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/otlp-exporter-base": "0.222.0",
|
||||
"@opentelemetry/otlp-transformer": "0.222.0",
|
||||
"@opentelemetry/sdk-trace": "2.11.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": "^1.3.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/otlp-exporter-base": {
|
||||
"version": "0.222.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/otlp-exporter-base/-/otlp-exporter-base-0.222.0.tgz",
|
||||
"integrity": "sha512-YbywG3veEm2Fb6TbdxRkuquWob6eVWXuA8/Ba1tXz9jHfUqpdE3keilOHEtPboC4CvS1bjeeVfNkWGOOrLj+lw==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/otlp-transformer": "0.222.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": "^1.3.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/otlp-transformer": {
|
||||
"version": "0.222.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/otlp-transformer/-/otlp-transformer-0.222.0.tgz",
|
||||
"integrity": "sha512-/F3BZ89+CJQnZkMh2tCrtcdB+XT2Dxhj4FFE+WPQ//413hmFL0/RfEX6vgOIWGhiSzrkHWTK3+6SiT7K5/g/jQ==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/api-logs": "0.222.0",
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/resources": "2.11.0",
|
||||
"@opentelemetry/sdk-logs": "0.222.0",
|
||||
"@opentelemetry/sdk-metrics": "2.11.0",
|
||||
"@opentelemetry/sdk-trace": "2.11.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": "^1.3.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/resources": {
|
||||
"version": "1.30.1",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-1.30.1.tgz",
|
||||
"integrity": "sha512-5UxZqiAgLYGFjS4s9qm5mBVo433u+dSPUFWVWXmLAD4wB65oMCoXaJP1KJa9DIYYMeHu3z4BZcStG3LC593cWA==",
|
||||
"version": "2.11.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.11.0.tgz",
|
||||
"integrity": "sha512-Ie7+8q8MDF4FAEQCKVMTx3ReUvxiIAgIiiW3c9JdmP8+HMcDy20puT+AHjexnExgnbvBxjQ9fjkFDWrikJ2jQA==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/core": "1.30.1",
|
||||
"@opentelemetry/semantic-conventions": "1.28.0"
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/semantic-conventions": "^1.29.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=14"
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.0.0 <1.10.0"
|
||||
"@opentelemetry/api": ">=1.3.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/sdk-logs": {
|
||||
"version": "0.222.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-logs/-/sdk-logs-0.222.0.tgz",
|
||||
"integrity": "sha512-+19YHODIjaUCArxleaJtuufFZVpz/xvvK+VllQqE+W8hHolxdoRwHfK/s667zezwh1hkx6FFF+oYzetYgqK+Bg==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/api-logs": "0.222.0",
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/resources": "2.11.0",
|
||||
"@opentelemetry/semantic-conventions": "^1.29.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.4.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/sdk-metrics": {
|
||||
"version": "2.11.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.11.0.tgz",
|
||||
"integrity": "sha512-7GXXcObyHyDUUSG+L+kJoquty01bzm7ivE7+SSgXXJcHuPzGviptxwARmI2c+bnnxjexGQbJnyNlN8HxBP/Y7A==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/resources": "2.11.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.9.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/sdk-trace": {
|
||||
"version": "2.11.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace/-/sdk-trace-2.11.0.tgz",
|
||||
"integrity": "sha512-fFnTqGm8/G73GQVnxYi7LXa1ZVYEUvgL6XI1LpvV0bPC7WQ/ZGgKxCSl8FnlZBKto9JHHEFTO6s6CUpvvtwFrA==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/resources": "2.11.0",
|
||||
"@opentelemetry/semantic-conventions": "^1.29.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.3.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/sdk-trace-base": {
|
||||
"version": "1.30.1",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-1.30.1.tgz",
|
||||
"integrity": "sha512-jVPgBbH1gCy2Lb7X0AVQ8XAfgg0pJ4nvl8/IiQA6nxOsPvS+0zMJaFSs2ltXe0J6C8dqjcnpyqINDJmU30+uOg==",
|
||||
"version": "2.11.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.11.0.tgz",
|
||||
"integrity": "sha512-H19x/TX/LZdqiYOjM7fqtSxwlplC5pgelavqbQdHbhdq0q/AI/TGkM2dfGuuynTXmJPeF2HoZVoPDu+TGoW78A==",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@opentelemetry/core": "1.30.1",
|
||||
"@opentelemetry/resources": "1.30.1",
|
||||
"@opentelemetry/semantic-conventions": "1.28.0"
|
||||
"@opentelemetry/core": "2.11.0",
|
||||
"@opentelemetry/resources": "2.11.0",
|
||||
"@opentelemetry/sdk-trace": "2.11.0",
|
||||
"@opentelemetry/semantic-conventions": "^1.29.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=14"
|
||||
"node": "^18.19.0 || >=20.6.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@opentelemetry/api": ">=1.0.0 <1.10.0"
|
||||
"@opentelemetry/api": ">=1.3.0 <1.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@opentelemetry/semantic-conventions": {
|
||||
"version": "1.28.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.28.0.tgz",
|
||||
"integrity": "sha512-lp4qAiMTD4sNWW4DbKLBkfiMZ4jbAboJIGOQr5DvciMRI494OapieI9qiODpOt0XBr1LjIDy1xAGAnVs5supTA==",
|
||||
"version": "1.43.0",
|
||||
"resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.43.0.tgz",
|
||||
"integrity": "sha512-eSYWTm620tTk45EKSedaUL8MFYI8hW164hIXsgIHyxu3VobUB3fFCu5t0hQby6OoWRPsG1KkKUG2M5UadiLiVg==",
|
||||
"license": "Apache-2.0",
|
||||
"engines": {
|
||||
"node": ">=14"
|
||||
|
||||
+4
-1
@@ -54,7 +54,10 @@
|
||||
"@fastify/swagger": "^8.14.0",
|
||||
"@fastify/swagger-ui": "^3.0.0",
|
||||
"@opentelemetry/api": "^1.8.0",
|
||||
"@opentelemetry/sdk-trace-base": "^1.22.0",
|
||||
"@opentelemetry/context-async-hooks": "^2.11.0",
|
||||
"@opentelemetry/exporter-trace-otlp-http": "^0.222.0",
|
||||
"@opentelemetry/resources": "^2.11.0",
|
||||
"@opentelemetry/sdk-trace-base": "^2.11.0",
|
||||
"@prisma/client": "^5.12.1",
|
||||
"bcryptjs": "^3.0.3",
|
||||
"bullmq": "^5.7.1",
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
# Specification Quality Checklist: Authentication Hardening
|
||||
|
||||
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
||||
**Created**: 2026-09-07
|
||||
**Feature**: [spec.md](../spec.md)
|
||||
|
||||
## Content Quality
|
||||
|
||||
- [x] No implementation details (languages, frameworks, APIs)
|
||||
- [x] Focused on user value and business needs
|
||||
- [x] Written for non-technical stakeholders
|
||||
- [x] All mandatory sections completed
|
||||
|
||||
## Requirement Completeness
|
||||
|
||||
- [x] No [NEEDS CLARIFICATION] markers remain
|
||||
- [x] Requirements are testable and unambiguous
|
||||
- [x] Success criteria are measurable
|
||||
- [x] Success criteria are technology-agnostic (no implementation details)
|
||||
- [x] All acceptance scenarios are defined
|
||||
- [x] Edge cases are identified
|
||||
- [x] Scope is clearly bounded
|
||||
- [x] Dependencies and assumptions identified
|
||||
|
||||
## Feature Readiness
|
||||
|
||||
- [x] All functional requirements have clear acceptance criteria
|
||||
- [x] User scenarios cover primary flows
|
||||
- [x] Feature meets measurable outcomes defined in Success Criteria
|
||||
- [x] No implementation details leak into specification
|
||||
|
||||
## Notes
|
||||
|
||||
- This is `docs/10-implementation-roadmap.md`'s own Phase 11 ("security hardening pass"), first
|
||||
slice, per explicit user direction — the two concrete gaps 010-identity-auth's own Assumptions
|
||||
named as deliberately out of its scope: password-reset and login rate-limiting. MFA, the third
|
||||
item 010 named, is intentionally excluded here as its own larger follow-up.
|
||||
- Password-reset's email-delivery step is explicitly stubbed (server-side log, not a real send)
|
||||
per explicit user decision — this codebase has no email-sending infrastructure at all today
|
||||
(no library, no configured provider), discovered while scoping this feature, and introducing
|
||||
one is a separate decision the user chose to defer rather than bundle into this pass.
|
||||
- Password-strength policy (User Story 2) was added beyond the two named gaps because it's a
|
||||
direct, unavoidable dependency of User Story 1 — a password-reset flow that accepts any
|
||||
password would be hardening one gap while leaving the other wide open at the same door.
|
||||
- All items pass; no revision iterations were needed.
|
||||
|
||||
## Implementation Notes (post-build)
|
||||
|
||||
- `tests/helpers/auth.ts`'s shared `loginAs()` helper previously reused two fixed accounts
|
||||
(`test-admin@supporthub.test` / `test-agent@supporthub.test`) across every integration test
|
||||
file via `upsert`. Once login became rate-limited per email (User Story 3), the ~30 files that
|
||||
each call it once in their own `beforeAll` collectively exceeded the attempt budget for those
|
||||
two shared addresses well before most files' own tests ran, turning their legitimate logins
|
||||
into `429`s. Fixed by giving each `loginAs()` call its own unique, randomly-suffixed email —
|
||||
nothing in the suite depended on the literal fixed addresses, so no call sites needed to
|
||||
change, only the helper itself.
|
||||
- While re-running the full suite for regression, `tests/integration/orchestration-strategies.test.ts`'s
|
||||
"SKILL_BASED prefers the eligible agent with the higher proficiency level" test was found
|
||||
failing (picks the lower-proficiency agent). Verified via `git stash` that this reproduces
|
||||
identically on the clean pre-013 `HEAD` with none of this feature's changes present — it is a
|
||||
pre-existing bug in 007-orchestration-assignment's `SKILL_BASED` strategy, unrelated to and out
|
||||
of scope for this feature. Left unfixed here; worth its own follow-up.
|
||||
- `tests/integration/ticket-attachments.test.ts`'s 2 known MinIO-dependent failures (accepted
|
||||
baseline, this project doesn't run MinIO) remain unchanged by this feature.
|
||||
- All other integration and unit tests pass, including 010-identity-auth's own login/admin-account
|
||||
tests, confirming no regression from `AuthService.login`'s new rate-limit check or the shared
|
||||
`validatePasswordStrength` call added to `UsersService.create`.
|
||||
@@ -0,0 +1,48 @@
|
||||
# Contract: Authentication Hardening
|
||||
|
||||
## `POST /auth/password-reset/request`
|
||||
|
||||
**Auth**: None (like login itself — the caller has no session yet).
|
||||
|
||||
**Request body**: `{ "email": "string" }`
|
||||
|
||||
**Response `200`** (always, regardless of whether the account exists):
|
||||
|
||||
```json
|
||||
{ "success": true, "data": { "message": "If that account exists, a reset link has been sent." }, "meta": null }
|
||||
```
|
||||
|
||||
No token, ever, appears in this response — it's only visible via the stub's own server-side log
|
||||
line (`{ "event": "password_reset_requested", "userId": "...", "resetUrl": "..." }`).
|
||||
|
||||
## `POST /auth/password-reset/consume`
|
||||
|
||||
**Auth**: None (the token itself is the credential).
|
||||
|
||||
**Request body**: `{ "token": "string", "newPassword": "string" }`
|
||||
|
||||
**Responses**:
|
||||
- `200` — `{ "success": true, "data": { "message": "Password updated." }, "meta": null }`
|
||||
- `400 VALIDATION_ERROR` — `newPassword` doesn't meet `validatePasswordStrength`.
|
||||
- `400 INVALID_RESET_TOKEN` (or equivalent) — token missing, expired, or already used. The
|
||||
response never distinguishes which of the three — matching data-model.md's own note that a
|
||||
consumer can't otherwise tell "expired" from "already used" from "never existed."
|
||||
|
||||
## `PATCH /admin/users` — unchanged route, tightened validation
|
||||
|
||||
`POST /admin/users` (010-identity-auth) now also rejects a `password` shorter than
|
||||
`PASSWORD_MIN_LENGTH` with the same `validatePasswordStrength` message the reset-consume
|
||||
endpoint uses — no new route, no schema field change, just a stricter check on the existing
|
||||
`password` field.
|
||||
|
||||
## `POST /auth/login` — unchanged route, new pre-check
|
||||
|
||||
Before this feature: any number of attempts, any speed. After: attempts for the same submitted
|
||||
`email` beyond `LOGIN_RATE_LIMIT_MAX_ATTEMPTS` within `LOGIN_RATE_LIMIT_WINDOW_SECONDS` receive:
|
||||
|
||||
```json
|
||||
{ "success": false, "error": { "code": "RATE_LIMIT_EXCEEDED", "message": "Too many login attempts. Try again later." } }
|
||||
```
|
||||
|
||||
with HTTP `429`, distinct from the existing `401` identical-failure-response 010 already
|
||||
returns for wrong credentials.
|
||||
@@ -0,0 +1,50 @@
|
||||
# Data Model: Authentication Hardening
|
||||
|
||||
No Postgres schema changes. `User.passwordHash` (010-identity-auth) is updated in place by a
|
||||
successful reset; no other model changes.
|
||||
|
||||
## Redis-only: Password Reset Token
|
||||
|
||||
Not a Prisma model — exists only as two paired Redis keys, both expiring together.
|
||||
|
||||
| Key | Value | TTL |
|
||||
|---|---|---|
|
||||
| `password-reset:token:<sha256(token)>` | `userId` | `PASSWORD_RESET_TOKEN_LIFETIME_MINUTES` |
|
||||
| `password-reset:user:<userId>` | `sha256(token)` | same |
|
||||
|
||||
**Issuing** (`requestPasswordReset`): if `password-reset:user:<userId>` already has a value,
|
||||
delete `password-reset:token:<that value>` first (invalidating the prior token — FR-002), then
|
||||
set both new keys.
|
||||
|
||||
**Consuming** (`resetPassword`): `GET password-reset:token:<sha256(presented token)>` → if
|
||||
absent, reject (FR-004: invalid/expired/already-used, indistinguishably — the key not existing
|
||||
covers all three cases identically, which is itself desirable: a consumer can't tell "expired"
|
||||
from "already used" from "never existed," matching the same non-leaking spirit as 010's own
|
||||
login-failure parity). If present, resolve `userId`, delete both keys (single-use), update the
|
||||
password.
|
||||
|
||||
## Configuration (new)
|
||||
|
||||
| Env var | Purpose | Default |
|
||||
|---|---|---|
|
||||
| `PASSWORD_MIN_LENGTH` | Minimum password length, enforced everywhere a password is set | `10` |
|
||||
| `PASSWORD_RESET_TOKEN_LIFETIME_MINUTES` | How long a reset token stays valid | `30` |
|
||||
| `LOGIN_RATE_LIMIT_MAX_ATTEMPTS` | Max login attempts per email per window | `5` |
|
||||
| `LOGIN_RATE_LIMIT_WINDOW_SECONDS` | The window `LOGIN_RATE_LIMIT_MAX_ATTEMPTS` applies over | `300` |
|
||||
|
||||
## Validation / Business Rules
|
||||
|
||||
- `requestPasswordReset(email)`: always returns the same shape regardless of whether `email`
|
||||
resolves to a real, active account (FR-001) — internally, only issues a real token when it
|
||||
does; the caller-visible response is identical either way.
|
||||
- `resetPassword(token, newPassword)`: `validatePasswordStrength` runs first (fail fast on the
|
||||
cheap, stateless check), then the token is looked up. Unlike login/reset-request,
|
||||
account-existence secrecy doesn't apply here — FR-004 and User Story 2 both call for their
|
||||
*own*, specific rejection reasons ("password too short" vs. "invalid or expired token"); only
|
||||
FR-001's account-existence question needs the identical-response treatment, not this
|
||||
endpoint's two legitimately-different failure modes.
|
||||
- `login(email, password)`: the rate-limit check (`login:<email>`) runs first, before
|
||||
`repo.findByEmail`/`verifyPassword` (FR-007) — a rate-limited request never reaches the
|
||||
identical-failure-response logic 010 already built; it gets its own distinct rate-limit
|
||||
rejection instead (Acceptance Scenario 1's own point: a rate limit is an honestly-different
|
||||
condition from a credentials failure, not disguised as one).
|
||||
@@ -0,0 +1,126 @@
|
||||
# Implementation Plan: Authentication Hardening
|
||||
|
||||
**Branch**: `013-auth-hardening` | **Date**: 2026-09-07 | **Spec**: [spec.md](./spec.md)
|
||||
|
||||
**Input**: Feature specification from `specs/013-auth-hardening/spec.md`
|
||||
|
||||
## Summary
|
||||
|
||||
Adds `POST /auth/password-reset/request` and `POST /auth/password-reset/consume` to
|
||||
`identity/auth` (the module that already owns login/logout/self-identity mechanics), backed by
|
||||
a Redis-stored, single-use reset token — the "delivery" step logs the token server-side rather
|
||||
than emailing it. Adds a shared password-strength validator used by both the reset-consume
|
||||
endpoint and 010's own `POST /admin/users`. Adds a pre-credential-check rate limit to
|
||||
`POST /auth/login`, reusing the existing `checkRateLimit` helper 002's own inbound trust
|
||||
boundary already established.
|
||||
|
||||
## Technical Context
|
||||
|
||||
**Language/Version**: TypeScript 5.4 / Node.js 20+ (unchanged).
|
||||
|
||||
**Primary Dependencies**: None new — reuses `crypto` (Node built-in, for token generation and
|
||||
hashing), the existing `ioredis` client, and `zod`.
|
||||
|
||||
**Storage**: No schema change. Reset tokens live entirely in Redis (never in Postgres) — two
|
||||
keys per active token, mirroring the existing revocation-denylist's own Redis-key-with-TTL shape:
|
||||
`password-reset:token:<sha256(token)>` → `userId`, and `password-reset:user:<userId>` →
|
||||
`sha256(token)`, both with the same TTL (the reset token's own lifetime). The second key is what
|
||||
lets issuing a new token invalidate the previous one (FR-002) without a database table.
|
||||
|
||||
**Testing**: Vitest — unit tests for the password-strength validator and the rate-limit's own
|
||||
pre-credential-check ordering; integration tests against real Postgres/Redis for the full
|
||||
request → (read the token from the stub's log output) → consume → login-with-new-password flow,
|
||||
the identical-response-regardless-of-existing-account behavior, and the login rate limit
|
||||
actually rejecting the N+1th attempt while a different account's login proceeds normally.
|
||||
|
||||
**Target Platform**: Same Fastify modular monolith. Modifies `identity/auth` (new routes,
|
||||
service methods, the shared password-strength validator) and `identity/agents` (existing
|
||||
`POST /admin/users` now calls the shared validator instead of accepting any password
|
||||
unchecked).
|
||||
|
||||
**Project Type**: Backend service — single project.
|
||||
|
||||
**Performance Goals**: The login rate-limit check is one Redis `INCR` (already how
|
||||
`checkRateLimit` works) — no added database round trip on the login hot path, consistent with
|
||||
010's own performance goal for `fastify.authenticate`.
|
||||
|
||||
**Constraints**: FR-001/SC-001 — reset-request must respond identically regardless of account
|
||||
existence, including timing-shape (the same pattern 010's login already established: do the
|
||||
same amount of work either way). FR-007 — the rate-limit check MUST run before
|
||||
`bcrypt.compare`, not after i.e. before any password-verification cost is paid, both for
|
||||
FR-007's own ordering requirement and so a rate-limited attacker gains no timing signal from a
|
||||
skipped bcrypt call.
|
||||
|
||||
**Scale/Scope**: Two new routes, one new shared validator, one new env-configured rate-limit
|
||||
policy, one modified existing endpoint (`POST /admin/users`). No new module, no schema
|
||||
migration, no new module dependencies. Explicitly excludes: MFA, real email delivery, IP-based
|
||||
rate limiting, password complexity rules beyond minimum length (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 | Same carve-out as 010 — this hardens SupportHub's own staff authentication, never touching SaaS-delegated customer identity. | PASS |
|
||||
| II. Configuration Over Hardcoding | Password minimum length and the login rate-limit's max-attempts/window are both new env-configured values (`PASSWORD_MIN_LENGTH`, `LOGIN_RATE_LIMIT_MAX_ATTEMPTS`, `LOGIN_RATE_LIMIT_WINDOW_SECONDS`), never hardcoded magic numbers — matches spec.md's own Assumptions and the roadmap's "never hardcode a placeholder value and ship it as final." | PASS |
|
||||
| III. Layered Architecture With Enforced Module Boundaries | Reset endpoints live in `identity/auth` (owns auth mechanics); the shared password-strength validator is exported from `identity/auth`'s own public `index.ts` for `identity/agents` to consume, the same precedent `hashPassword`/`verifyPassword` themselves already set. | PASS |
|
||||
| IV. AI Recommends, Deterministic Policy Decides | Not applicable. | PASS — N/A |
|
||||
| V. Evidence-Based Verification | Not applicable. | PASS — N/A |
|
||||
| VI. Durable Audit & History | Not applicable — no new audit-relevant mutable domain state (a password hash change isn't itself an audited business event in this codebase's existing model). | PASS — N/A |
|
||||
| VII. Concurrency-Safe, Durable Job Handling | Reset-token issuance/consumption is a single Redis operation per step, no shared in-memory state; two concurrent consume attempts for the same token race safely (Redis `GET`+`DEL` — the loser sees the key already gone and is rejected, not a partial/double-apply). | PASS |
|
||||
| VIII. Problem and Ticket Are Separate, Related Entities | Not applicable. | PASS — N/A |
|
||||
| Technology & Platform Constraints | No new dependencies or infrastructure — email delivery is explicitly stubbed (spec.md Assumptions, user decision), not a real provider integration. | PASS |
|
||||
|
||||
No violations requiring Complexity Tracking justification.
|
||||
|
||||
## Project Structure
|
||||
|
||||
### Documentation (this feature)
|
||||
|
||||
```text
|
||||
specs/013-auth-hardening/
|
||||
├── plan.md
|
||||
├── research.md
|
||||
├── data-model.md
|
||||
├── quickstart.md
|
||||
├── contracts/
|
||||
└── tasks.md
|
||||
```
|
||||
|
||||
### Source Code (repository root)
|
||||
|
||||
```text
|
||||
supporthub-api/
|
||||
├── src/
|
||||
│ ├── config/
|
||||
│ │ └── auth.ts # MODIFIED — passwordMinLength, loginRateLimit config
|
||||
│ └── modules/
|
||||
│ └── identity/
|
||||
│ ├── auth/ # MODIFIED
|
||||
│ │ ├── mapper/
|
||||
│ │ │ └── password-policy.ts # NEW — shared validatePasswordStrength
|
||||
│ │ ├── mapper/
|
||||
│ │ │ └── reset-token.ts # NEW — generate/hash reset tokens
|
||||
│ │ ├── repository/
|
||||
│ │ │ └── reset-token.repository.ts # NEW — the two-Redis-key shape
|
||||
│ │ ├── service/ # MODIFIED — requestPasswordReset, resetPassword,
|
||||
│ │ │ login's new pre-check rate-limit call
|
||||
│ │ ├── controller/ routes/ # MODIFIED — the two new routes
|
||||
│ │ └── schema/ # MODIFIED — request/consume body schemas
|
||||
│ └── agents/
|
||||
│ └── service/
|
||||
│ └── users.service.ts # MODIFIED — calls the shared validator
|
||||
└── tests/
|
||||
├── unit/identity/ # password-policy validator, rate-limit ordering
|
||||
└── integration/ # full reset flow, identical-response check,
|
||||
login rate-limit behavior
|
||||
```
|
||||
|
||||
**Structure Decision**: Single project, no new module. Everything lives in `identity/auth`
|
||||
(already owns login/logout/self-identity) except the one-line call site change in
|
||||
`identity/agents/service/users.service.ts`.
|
||||
|
||||
## Complexity Tracking
|
||||
|
||||
*No constitution violations — table intentionally omitted.*
|
||||
@@ -0,0 +1,37 @@
|
||||
# Quickstart: Validating Authentication Hardening
|
||||
|
||||
## Scenario 1 — password reset, end to end
|
||||
|
||||
1. `POST /auth/password-reset/request` with a real seeded account's email. **Expected**: `200`,
|
||||
generic message; the server log shows a `password_reset_requested` line with a `resetUrl`
|
||||
containing the real token.
|
||||
2. Repeat with an email that doesn't exist. **Expected**: identical `200` response body to
|
||||
step 1 — diff them to confirm.
|
||||
3. `POST /auth/password-reset/consume` with the token from step 1's log and a policy-meeting new
|
||||
password. **Expected**: `200`.
|
||||
4. Repeat step 3 with the same token. **Expected**: rejected — the token is single-use.
|
||||
5. `POST /auth/login` with the account's email and the new password from step 3. **Expected**:
|
||||
`200`. Repeat with the account's old password. **Expected**: `401`.
|
||||
|
||||
## Scenario 2 — password strength enforced everywhere
|
||||
|
||||
1. `POST /admin/users` (as admin) with a password shorter than `PASSWORD_MIN_LENGTH`.
|
||||
**Expected**: `400`, naming the actual minimum length.
|
||||
2. `POST /auth/password-reset/consume` with a valid token and a too-short new password.
|
||||
**Expected**: the same `400` rejection reason as step 1.
|
||||
|
||||
## Scenario 3 — login rate limiting
|
||||
|
||||
1. Submit `LOGIN_RATE_LIMIT_MAX_ATTEMPTS` failed login attempts for the same email within
|
||||
`LOGIN_RATE_LIMIT_WINDOW_SECONDS`. **Expected**: each returns `401` (the existing
|
||||
identical-failure-response).
|
||||
2. Submit one more attempt for that same email, still within the window — this time with the
|
||||
*correct* password. **Expected**: `429`, not `200` — the rate limit is checked before
|
||||
credentials (FR-007).
|
||||
3. Submit an attempt for a *different* email within the same window. **Expected**: proceeds
|
||||
normally (evaluated on its own credentials, not rate-limited).
|
||||
|
||||
## What "done" looks like
|
||||
|
||||
All three scenarios pass against a real Postgres/Redis, and `POST /admin/users`'s own existing
|
||||
tests (010-identity-auth) still pass with the added password-strength check in place.
|
||||
@@ -0,0 +1,83 @@
|
||||
# Research: Authentication Hardening
|
||||
|
||||
## Decision: reset tokens live only in Redis, as a paired key shape, never in Postgres
|
||||
|
||||
- **Decision**: A random 32-byte token (`crypto.randomBytes(32).toString('hex')`) is generated
|
||||
per request; only its SHA-256 hash is ever stored (the raw token is returned to the caller of
|
||||
`requestPasswordReset` for the stub-delivery step to log, then discarded). Two Redis keys per
|
||||
active token, both with the same TTL (the reset lifetime):
|
||||
- `password-reset:token:<hash>` → `userId` (resolves a presented token at consume time)
|
||||
- `password-reset:user:<userId>` → `hash` (lets issuing a new token find and delete the prior
|
||||
one's `token:` key, invalidating it — FR-002)
|
||||
- **Rationale**: Storing only the hash (never the raw token) mirrors this codebase's own
|
||||
password-hashing discipline (010's `hashPassword`) and 002's encrypted-credential-at-rest
|
||||
precedent — a Redis compromise alone shouldn't hand over usable reset tokens. The paired-key
|
||||
shape gets "only one active token per account" (FR-002) without a database table or a list
|
||||
scan; it's the same Redis-key-with-TTL pattern 010's own revocation denylist and 002's jti
|
||||
replay-guard already established, not a new pattern for this codebase.
|
||||
- **Alternatives considered**: A signed JWT with a `purpose: 'password-reset'` claim — rejected;
|
||||
a JWT can't be "invalidated by issuing a new one" without also tracking issued tokens
|
||||
somewhere (defeating the point of using a stateless token), so it would need the same Redis
|
||||
bookkeeping anyway while adding JWT-parsing overhead for no benefit. A Postgres table — works,
|
||||
but adds a migration and a cleanup/expiry job for data Redis's own TTL already expires for
|
||||
free; rejected as unnecessary durability for a short-lived, non-audit-relevant credential.
|
||||
|
||||
## Decision: the "delivery" stub is a structured log line, not a fake email object
|
||||
|
||||
- **Decision**: `requestPasswordReset` logs `{ event: 'password_reset_requested', userId,
|
||||
resetUrl }` at `info` level via the existing Pino logger — no new "mock email" abstraction,
|
||||
no `EmailService` interface to later swap out.
|
||||
- **Rationale**: Per the user's own explicit choice (stub delivery, not real email), the
|
||||
simplest honest stub is exactly what a developer needs during this phase: the token, visible
|
||||
in the same place every other structured log already goes. Building a fake `EmailService`
|
||||
interface now, before any real provider is chosen, would be speculative abstraction for a
|
||||
contract nobody has decided yet (which provider, which template).
|
||||
- **Alternatives considered**: A dedicated `EmailService`/`NotificationService` interface with a
|
||||
console/log implementation, swapped for a real one later — rejected as premature
|
||||
infrastructure for a single call site; revisit when a real provider is actually chosen (a
|
||||
separate, later decision per spec.md Assumptions).
|
||||
|
||||
## Decision: one shared `validatePasswordStrength`, minimum length only, `PASSWORD_MIN_LENGTH`-configured
|
||||
|
||||
- **Decision**: `identity/auth/mapper/password-policy.ts` exports
|
||||
`validatePasswordStrength(password: string): void`, throwing `ValidationError` naming the
|
||||
actual requirement (e.g. "Password must be at least N characters.") if `password.length <
|
||||
env.PASSWORD_MIN_LENGTH`. Called from both `AuthService`'s new `resetPassword` and
|
||||
`identity/agents`'s existing `UsersService.create`.
|
||||
- **Rationale**: FR-005 requires one policy enforced identically everywhere a password is set —
|
||||
a shared function is the only way to guarantee that rather than trusting two call sites to
|
||||
stay in sync by convention. Minimum length only (no character-class rules) matches current
|
||||
NIST guidance (length matters far more than forced complexity) and spec.md's own explicit
|
||||
scope boundary.
|
||||
- **Alternatives considered**: A zod `.refine()` embedded separately in each schema — rejected;
|
||||
duplicates the rule text and the minimum-length constant at two call sites, exactly the drift
|
||||
FR-005 exists to prevent.
|
||||
|
||||
## Decision: login rate-limit reuses the existing `checkRateLimit` helper, keyed by email
|
||||
|
||||
- **Decision**: `AuthService.login` calls
|
||||
`checkRateLimit(`login:${email}`, env.LOGIN_RATE_LIMIT_MAX_ATTEMPTS,
|
||||
env.LOGIN_RATE_LIMIT_WINDOW_SECONDS)` as its very first step, before `repo.findByEmail` or
|
||||
`verifyPassword` — throwing `RateLimitError` (already a distinct error/status from
|
||||
`AuthenticationError`, per the existing `common/errors`) if exceeded.
|
||||
- **Rationale**: `checkRateLimit` (`src/infrastructure/cache/rate-limiter.ts`) already exists,
|
||||
already used by 002's own inbound-request rate limiting, and is exactly the fixed-window
|
||||
Redis-`INCR` shape this feature needs — reusing it is the literal instruction 010's own
|
||||
Assumptions gave ("beyond what 002's existing generic rate-limit infrastructure might already
|
||||
cover"). Keying by the *submitted* email (not a resolved user id) means the limiter runs
|
||||
identically whether or not the account exists, so it can't itself become a second
|
||||
account-existence oracle.
|
||||
- **Alternatives considered**: `@fastify/rate-limit`'s own global plugin (already registered,
|
||||
1000 req/min) — insufficient on its own; that's a blunt per-IP-or-global HTTP-level limit, not
|
||||
a per-account brute-force defense, and 010's own Assumptions already anticipated needing
|
||||
something more targeted for login specifically.
|
||||
|
||||
## Decision: `POST /admin/users` gets the shared validator via a one-line call-site change
|
||||
|
||||
- **Decision**: `UsersService.create` calls `validatePasswordStrength(body.password)` before
|
||||
hashing, right alongside its existing duplicate-email check — no schema change, no new route.
|
||||
- **Rationale**: FR-005's "identically everywhere" requirement includes this pre-existing
|
||||
010 endpoint, which today accepts any non-empty string as a password. Minimal, surgical fix
|
||||
at the one call site that needed it.
|
||||
- **Alternatives considered**: None — this is the only other password-setting call site in the
|
||||
codebase (confirmed by searching for every `hashPassword(` call).
|
||||
@@ -0,0 +1,194 @@
|
||||
# Feature Specification: Authentication Hardening
|
||||
|
||||
**Feature Branch**: `013-auth-hardening`
|
||||
|
||||
**Created**: 2026-09-07
|
||||
|
||||
**Status**: Draft
|
||||
|
||||
**Input**: User description: "Phase 11 security hardening pass, first slice: password-reset
|
||||
(self-service, with a stubbed email-delivery step logging the reset link instead of actually
|
||||
emailing it), a password-strength policy applied wherever a password is set, and login
|
||||
rate-limiting to slow down credential-stuffing/brute-force attempts against POST /auth/login.
|
||||
MFA is a separate, larger follow-up feature, not this one's scope."
|
||||
|
||||
## User Scenarios & Testing *(mandatory)*
|
||||
|
||||
### User Story 1 - A user resets a forgotten password (Priority: P1)
|
||||
|
||||
A user who has forgotten their password requests a reset; the system issues a single-use,
|
||||
short-lived reset token and "delivers" it (this feature stubs delivery — see Assumptions — a
|
||||
later feature wires up real email). The user submits the token with a new password and can log
|
||||
in with it immediately afterward.
|
||||
|
||||
**Why this priority**: 010-identity-auth explicitly deferred this ("the smallest viable fix
|
||||
today is an admin recreating the account") — this is the first real self-service fix for a
|
||||
locked-out user, and the whole reason this feature exists.
|
||||
|
||||
**Independent Test**: Request a reset for a known account; retrieve the issued token (via the
|
||||
stub's own log output, since there's no real inbox to check); consume it with a new password;
|
||||
confirm login succeeds with the new password and fails with the old one.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** an existing account, **When** its email requests a password reset, **Then** a
|
||||
single-use reset token is issued and "delivered" via the stub — the response itself never
|
||||
includes the token (it's not a client-visible value, matching a real email-delivery
|
||||
contract).
|
||||
2. **Given** an email that doesn't correspond to any account, **When** it requests a password
|
||||
reset, **Then** the response is identical to Scenario 1's own success response — never
|
||||
revealing whether the account exists (mirrors 010's own FR-002 philosophy).
|
||||
3. **Given** a valid, unexpired reset token, **When** it's submitted with a new password meeting
|
||||
the password-strength policy (User Story 2), **Then** the account's password is updated and
|
||||
the token becomes unusable — a second consume attempt with the same token is rejected.
|
||||
4. **Given** an expired or already-used reset token, **When** it's submitted, **Then** the
|
||||
request is rejected with a clear, specific reason — never silently accepted.
|
||||
5. **Given** a freshly-reset password, **When** the user logs in with it, **Then** login
|
||||
succeeds; the old password no longer works.
|
||||
|
||||
---
|
||||
|
||||
### User Story 2 - Password strength is enforced wherever a password is set (Priority: P1)
|
||||
|
||||
Whenever a password is set — an admin creating a new staff account, or a user resetting their
|
||||
own — the system enforces a minimum strength policy and rejects a weak password with a specific,
|
||||
actionable reason.
|
||||
|
||||
**Why this priority**: 010-identity-auth's own admin-account-creation (`POST /admin/users`) and
|
||||
this feature's own password-reset both accept a plaintext password with no strength check today
|
||||
— the most basic hardening gap a "security hardening pass" exists to close first.
|
||||
|
||||
**Independent Test**: Attempt to create an account (or reset a password) with a password that
|
||||
fails the policy (too short); confirm a clear rejection naming what's wrong. Repeat with a
|
||||
policy-meeting password; confirm it succeeds.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the admin account-creation endpoint, **When** a password shorter than the
|
||||
configured minimum length is submitted, **Then** the request is rejected with a message
|
||||
naming the actual requirement, not a generic validation error.
|
||||
2. **Given** the password-reset consume endpoint, **When** a policy-violating password is
|
||||
submitted, **Then** it's rejected the same way — one policy, enforced identically everywhere
|
||||
a password is ever set.
|
||||
3. **Given** a password meeting the policy, **When** it's submitted to either endpoint,
|
||||
**Then** it's accepted.
|
||||
|
||||
---
|
||||
|
||||
### User Story 3 - Login attempts are rate-limited (Priority: P1)
|
||||
|
||||
Repeated login attempts against the same account within a short window are throttled, slowing
|
||||
down credential-stuffing and brute-force attacks without permanently locking out a legitimate
|
||||
user who mistypes their password a few times.
|
||||
|
||||
**Why this priority**: `POST /auth/login` has no attempt limit today — an attacker can try
|
||||
passwords against a known email address as fast as the network allows. This is the other
|
||||
baseline hardening gap named explicitly in 010-identity-auth's own Assumptions.
|
||||
|
||||
**Independent Test**: Submit repeated failed login attempts for the same email within the
|
||||
configured window; confirm attempts beyond the configured maximum are rejected with a
|
||||
rate-limit response, distinct from an authentication failure; confirm a successful login for a
|
||||
*different* account is unaffected.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the configured maximum login attempts per window has been reached for one email,
|
||||
**When** another attempt is made for that same email within the window, **Then** it's
|
||||
rejected with a clear rate-limit response (not the identical-failure-response body User
|
||||
Story 1/010 uses for wrong credentials — a rate limit is a different, honestly-reported
|
||||
condition).
|
||||
2. **Given** the same exhausted window, **When** a login attempt is made for a *different*
|
||||
email, **Then** it proceeds normally — the limit is per-account, not global.
|
||||
3. **Given** the rate-limit window has elapsed, **When** a new attempt is made for the
|
||||
previously-limited email, **Then** it's evaluated normally again.
|
||||
|
||||
---
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- What happens if a user requests a password reset for the same account multiple times before
|
||||
consuming the first token? Each request issues its own new token; consuming any valid,
|
||||
unexpired one succeeds, and consuming one invalidates all of that account's other outstanding
|
||||
reset tokens (never allowing two guesses to both later succeed independently).
|
||||
- What happens if a reset token is consumed for an account that was deactivated after the token
|
||||
was issued but before it was used? The reset is rejected — reactivating a deactivated account
|
||||
is an admin action (010's own domain), not something a password-reset flow performs
|
||||
incidentally.
|
||||
- What happens to a rate-limited login attempt that would have actually succeeded (correct
|
||||
password, but the account is rate-limited from prior failed attempts)? It's still rejected —
|
||||
the rate limit is evaluated before credentials, exactly like a real brute-force defense must
|
||||
be, not skipped for a lucky correct guess.
|
||||
|
||||
## Requirements *(mandatory)*
|
||||
|
||||
### Functional Requirements
|
||||
|
||||
- **FR-001**: The system MUST let a user request a password reset by email, always returning an
|
||||
identical response regardless of whether the email corresponds to an existing account
|
||||
(mirrors 010's FR-002).
|
||||
- **FR-002**: The system MUST issue a single-use, time-limited reset token per request, and MUST
|
||||
invalidate a token immediately upon use or upon a newer token being issued for the same
|
||||
account.
|
||||
- **FR-003**: The system MUST "deliver" the reset token via a clearly-labeled stub (server-side
|
||||
log output) rather than a real email — this feature does not add email-sending infrastructure
|
||||
(Assumptions).
|
||||
- **FR-004**: The system MUST let a user consume a valid reset token with a new password,
|
||||
updating the account's password hash and rejecting an invalid, expired, or already-used token
|
||||
with a specific, distinguishable reason.
|
||||
- **FR-005**: The system MUST enforce one configured password-strength policy (at minimum, a
|
||||
minimum length) identically at every point a password is ever set — admin account creation
|
||||
and password-reset consumption alike — never two different or duplicated policies.
|
||||
- **FR-006**: The system MUST rate-limit `POST /auth/login` attempts per submitted email within
|
||||
a configured window, rejecting attempts beyond the configured maximum with a response distinct
|
||||
from a credentials failure.
|
||||
- **FR-007**: The login rate limit MUST be evaluated before password verification, so a
|
||||
rate-limited attempt is rejected regardless of whether the submitted password is actually
|
||||
correct.
|
||||
- **FR-008**: The system MUST NOT lock an account indefinitely — the rate limit is a rolling/
|
||||
fixed window that clears on its own, not a manual-unlock-required lockout.
|
||||
|
||||
### Key Entities
|
||||
|
||||
- **Password Reset Token**: A single-use, time-limited credential tying one request to one
|
||||
account, consumed exactly once to authorize a password change.
|
||||
- **Password Policy**: The configured minimum-strength rule(s) applied identically at every
|
||||
password-setting point in the system.
|
||||
- **Login Attempt Counter**: A rolling/fixed-window count of failed login attempts per
|
||||
submitted email, backing the rate limit.
|
||||
|
||||
## Success Criteria *(mandatory)*
|
||||
|
||||
### Measurable Outcomes
|
||||
|
||||
- **SC-001**: 100% of password-reset requests (existing or nonexistent account) receive an
|
||||
identical response — 0% reveal account existence.
|
||||
- **SC-002**: 100% of password-reset tokens are usable exactly once; a second consume attempt
|
||||
with the same token fails 100% of the time.
|
||||
- **SC-003**: 100% of passwords accepted by any password-setting endpoint meet the configured
|
||||
policy; 0% of policy-violating passwords are ever stored.
|
||||
- **SC-004**: An account subjected to more login attempts than the configured maximum within
|
||||
the configured window is rejected on 100% of the excess attempts, regardless of whether the
|
||||
submitted password was correct.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- **Email delivery is stubbed, not real** — the reset token is logged server-side rather than
|
||||
emailed, per explicit user decision; wiring up a real email provider is a separate, later
|
||||
concern once that infrastructure choice is made.
|
||||
- **MFA is out of scope** — a separate, larger follow-up feature; this pass only closes the two
|
||||
gaps 010-identity-auth's own Assumptions named as "not this feature's job."
|
||||
- **No account self-registration** — unchanged from 010; password reset only ever applies to an
|
||||
existing account, never creates one.
|
||||
- **The password-strength policy is a minimum-length rule, configurable, not a fixed hardcoded
|
||||
value** (`docs/10-implementation-roadmap.md`'s own "never hardcode a placeholder value and
|
||||
ship it as final" instruction) — the exact minimum is a `CONFIGURABLE` value with a reasonable
|
||||
default, not a business-confirmed final number; additional complexity rules (character
|
||||
classes, breached-password checks) are a possible future enhancement, not required here.
|
||||
- **Rate limiting is per submitted email, not per IP** — the most direct defense against
|
||||
credential-stuffing a specific known account; IP-based limiting is a possible future
|
||||
enhancement layered on top, not required here.
|
||||
- **Existing sessions are not force-revoked on password reset** — a reset invalidates the
|
||||
password (and all other outstanding reset tokens for that account), but any already-issued,
|
||||
unexpired login session remains valid until its own natural expiry (010's own 4-hour token
|
||||
lifetime bounds this) rather than requiring a database check on every authenticated request
|
||||
(010's own performance goal of a single Redis round trip per request, no DB read).
|
||||
@@ -0,0 +1,151 @@
|
||||
---
|
||||
description: "Task list for 013-auth-hardening"
|
||||
---
|
||||
|
||||
# Tasks: Authentication Hardening
|
||||
|
||||
**Input**: Design documents from `specs/013-auth-hardening/`
|
||||
|
||||
**Prerequisites**: [plan.md](./plan.md), [spec.md](./spec.md), [research.md](./research.md),
|
||||
[data-model.md](./data-model.md),
|
||||
[contracts/auth-hardening-contract.md](./contracts/auth-hardening-contract.md),
|
||||
[quickstart.md](./quickstart.md)
|
||||
|
||||
**Organization**: Tasks are grouped by user story (US1 = P1 password reset, US2 = P1 password
|
||||
policy, US3 = P1 login rate-limiting). US2 is a dependency US1's own consume endpoint needs, so
|
||||
build it first despite the nominal priority tie; US3 is independent of both.
|
||||
|
||||
## Format: `[ID] [P?] [Story] Description`
|
||||
|
||||
All file paths are relative to `supporthub-api/` (repo root).
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Foundational (Blocking Prerequisites)
|
||||
|
||||
- [x] T001 Add `PASSWORD_MIN_LENGTH` (default `10`),
|
||||
`PASSWORD_RESET_TOKEN_LIFETIME_MINUTES` (default `30`),
|
||||
`LOGIN_RATE_LIMIT_MAX_ATTEMPTS` (default `5`), and `LOGIN_RATE_LIMIT_WINDOW_SECONDS`
|
||||
(default `300`) to `src/config/env.ts`, exposed via `src/config/auth.ts`'s existing
|
||||
`authConfig` object
|
||||
|
||||
**Checkpoint**: Config in place. Both user stories can now be built.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: User Story 2 - Password strength is enforced wherever a password is set (Priority: P1)
|
||||
|
||||
**Goal**: One shared validator, called from both the (not-yet-built) reset-consume endpoint and
|
||||
the existing admin account-creation endpoint.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 2.
|
||||
|
||||
### Tests for User Story 2
|
||||
|
||||
- [x] T002 [P] [US2] Unit test for `validatePasswordStrength` (too-short rejected with the
|
||||
actual minimum named; policy-meeting password passes) in
|
||||
`tests/unit/identity/password-policy.test.ts`
|
||||
|
||||
### Implementation for User Story 2
|
||||
|
||||
- [x] T003 [US2] Add `identity/auth/mapper/password-policy.ts`'s
|
||||
`validatePasswordStrength(password): void`, throwing `ValidationError` (depends on T001)
|
||||
- [x] T004 [US2] Export it from `identity/auth`'s public `index.ts` (depends on T003)
|
||||
- [x] T005 [US2] Call it from `identity/agents/service/users.service.ts`'s `UsersService.create`,
|
||||
before hashing (depends on T004)
|
||||
- [x] T006 [US2] Run Quickstart Scenario 2 step 1 locally and confirm it passes; re-run
|
||||
010-identity-auth's own existing `POST /admin/users` tests to confirm no regression
|
||||
|
||||
**Checkpoint**: No password shorter than the policy can ever be set via the admin endpoint.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: User Story 1 - A user resets a forgotten password (Priority: P1)
|
||||
|
||||
**Goal**: The full request → stub-delivery → consume → login-with-new-password flow.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 1.
|
||||
|
||||
### Tests for User Story 1
|
||||
|
||||
- [x] T007 [US1] Integration test covering Quickstart Scenario 1 (request issues a token via
|
||||
the log stub; a nonexistent email gets an identical response; consume succeeds once and
|
||||
fails the second time; login works with the new password and fails with the old) in
|
||||
`tests/integration/password-reset-flow.test.ts` (depends on T006)
|
||||
|
||||
### Implementation for User Story 1
|
||||
|
||||
- [x] T008 [US1] Add `identity/auth/mapper/reset-token.ts` — `generateResetToken()` (raw token +
|
||||
its SHA-256 hash) (depends on T001)
|
||||
- [x] T009 [US1] Add `identity/auth/repository/reset-token.repository.ts` — `issue(userId,
|
||||
tokenHash, ttlSeconds)` (deletes any prior token for this user first, per data-model.md's
|
||||
paired-key shape), `resolve(tokenHash)` (returns `userId` or null), `consume(tokenHash,
|
||||
userId)` (deletes both keys) (depends on T008)
|
||||
- [x] T010 [US1] Add `AuthService.requestPasswordReset(email)`: always returns the same public
|
||||
result; internally, if the email resolves to an active account, issues a token and logs
|
||||
the stub delivery event (structured log, research.md) (depends on T009)
|
||||
- [x] T011 [US1] Add `AuthService.resetPassword(token, newPassword)`: validates password
|
||||
strength first (depends on T004), then resolves/consumes the token, 400s with a specific
|
||||
reason if the token is missing/expired/used, hashes and stores the new password (depends
|
||||
on T009, T004)
|
||||
- [x] T012 [US1] Add `POST /auth/password-reset/request` and `POST /auth/password-reset/consume`
|
||||
(both ungated — no session exists yet) in `identity/auth/controller/` + `routes/` +
|
||||
`schema/`, registered from `src/api/routes.ts` (already registers `authRoutes` as a
|
||||
whole, so no new registration call needed — depends on T010, T011)
|
||||
- [x] T013 [US1] Run Quickstart Scenario 1 locally and confirm all 5 steps pass
|
||||
|
||||
**Checkpoint**: A locked-out user has a real, working self-service fix.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: User Story 3 - Login attempts are rate-limited (Priority: P1)
|
||||
|
||||
**Goal**: `POST /auth/login` throttles repeated attempts per submitted email, checked before any
|
||||
credential verification.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 3.
|
||||
|
||||
### Tests for User Story 3
|
||||
|
||||
- [x] T014 [P] [US3] Unit test confirming the rate-limit check is invoked before
|
||||
`repo.findByEmail`/`verifyPassword` in `AuthService.login` (a fake repo/mapper that would
|
||||
throw if called after an already-exceeded limit) in
|
||||
`tests/unit/identity/login-rate-limit-ordering.test.ts`
|
||||
- [x] T015 [US3] Integration test covering Quickstart Scenario 3 (N attempts get 401, the N+1th
|
||||
— even with the correct password — gets 429, a different email is unaffected) in
|
||||
`tests/integration/login-rate-limit.test.ts` (depends on T001)
|
||||
|
||||
### Implementation for User Story 3
|
||||
|
||||
- [x] T016 [US3] In `AuthService.login`, call the existing
|
||||
`checkRateLimit(`login:${email}`, authConfig.loginRateLimitMaxAttempts,
|
||||
authConfig.loginRateLimitWindowSeconds)` (from `@/infrastructure/cache`) as the very first
|
||||
step, throwing `RateLimitError` if exceeded (depends on T001)
|
||||
- [x] T017 [US3] Run Quickstart Scenario 3 locally and confirm all 3 steps pass
|
||||
|
||||
**Checkpoint**: All three user stories work independently and together — this feature's full
|
||||
scope.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: Polish & Cross-Cutting Concerns
|
||||
|
||||
- [x] T018 [P] Update `specs/013-auth-hardening/checklists/requirements.md` Notes with any
|
||||
implementation-time findings
|
||||
- [x] T019 Run `npx tsx scripts/check-architecture.ts` and `npm run lint`/`npm run typecheck`
|
||||
- [x] T020 Full regression: `npm run test:unit` then the full integration suite against real
|
||||
Docker-provisioned Postgres/Redis, confirming nothing outside this feature regressed
|
||||
(particularly 010-identity-auth's own login/admin-account tests, now touched by this
|
||||
feature's changes)
|
||||
|
||||
---
|
||||
|
||||
## Dependencies & Execution Order
|
||||
|
||||
- **Foundational (Phase 1)**: No dependencies — BLOCKS everything
|
||||
- **User Story 2 (Phase 2)**: Depends on Foundational — BLOCKS User Story 1 (its consume
|
||||
endpoint needs the shared validator)
|
||||
- **User Story 1 (Phase 3)**: Depends on User Story 2
|
||||
- **User Story 3 (Phase 4)**: Depends only on Foundational — independent of US1/US2, could be
|
||||
built in parallel with either
|
||||
- **Polish (Phase 5)**: Depends on all three
|
||||
@@ -0,0 +1,100 @@
|
||||
# Specification Quality Checklist: Full Observability
|
||||
|
||||
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
||||
**Created**: 2026-09-07
|
||||
**Feature**: [spec.md](../spec.md)
|
||||
|
||||
## Content Quality
|
||||
|
||||
- [x] No implementation details (languages, frameworks, APIs)
|
||||
- [x] Focused on user value and business needs
|
||||
- [x] Written for non-technical stakeholders
|
||||
- [x] All mandatory sections completed
|
||||
|
||||
## Requirement Completeness
|
||||
|
||||
- [x] No [NEEDS CLARIFICATION] markers remain
|
||||
- [x] Requirements are testable and unambiguous
|
||||
- [x] Success criteria are measurable
|
||||
- [x] Success criteria are technology-agnostic (no implementation details)
|
||||
- [x] All acceptance scenarios are defined
|
||||
- [x] Edge cases are identified
|
||||
- [x] Scope is clearly bounded
|
||||
- [x] Dependencies and assumptions identified
|
||||
|
||||
## Feature Readiness
|
||||
|
||||
- [x] All functional requirements have clear acceptance criteria
|
||||
- [x] User scenarios cover primary flows
|
||||
- [x] Feature meets measurable outcomes defined in Success Criteria
|
||||
- [x] No implementation details leak into specification
|
||||
|
||||
## Notes
|
||||
|
||||
- This is `docs/10-implementation-roadmap.md`'s own Phase 11, second sub-area, per explicit user
|
||||
direction (the first was 013-auth-hardening's security pass). The user explicitly chose "Full
|
||||
observability" over "Reporting/analytics dashboards" as a distinct, separately-scoped sub-area
|
||||
— FR-009 and several Assumptions exist specifically to keep this feature from drifting into
|
||||
that adjacent, not-yet-started work.
|
||||
- The three named infrastructure gaps (no per-request access log, a dead request-duration
|
||||
histogram, a never-initialized tracer) and all eleven "key metrics to track" being completely
|
||||
untracked today were confirmed by direct code inspection before writing this spec, not assumed.
|
||||
- All items pass; no revision iterations were needed. No [NEEDS CLARIFICATION] markers were
|
||||
required — every open question had a reasonable, documented default (see Assumptions).
|
||||
|
||||
## Implementation Notes (post-build)
|
||||
|
||||
- Registering a real `TracerProvider` alone was not sufficient to make span nesting work across
|
||||
this feature's own async event-bus subscribers: without also registering an
|
||||
`AsyncLocalStorageContextManager` (`@opentelemetry/context-async-hooks`, a third new
|
||||
dependency beyond the two research.md originally named), the OpenTelemetry API's
|
||||
`context.active()` is a no-op that does not propagate across `await` boundaries at all —
|
||||
`orchestration.assignment` came out as its own unrelated root span/trace instead of nesting
|
||||
under `ai.escalation`. Caught by the tracing integration test's own parent/child assertions
|
||||
actually failing on the first implementation, not assumed correct from reading the SDK's docs.
|
||||
- Installing `@opentelemetry/exporter-trace-otlp-http` alongside the already-pinned
|
||||
`@opentelemetry/sdk-trace-base@^1.22.0` pulled two incompatible OpenTelemetry core/resources
|
||||
major versions (1.x and 2.x) side by side. Resolved by bumping `sdk-trace-base` to `^2.11.0` to
|
||||
match — this also happened to close a moderate DoS advisory in `@opentelemetry/core <2.8.0`
|
||||
that the 1.x line was pinned to.
|
||||
- T016 (graceful degradation under an unreachable OTLP endpoint) ended up as its own unit test
|
||||
(`tests/unit/observability/tracing-graceful-degradation.test.ts`) rather than living in
|
||||
`tracing.test.ts` as tasks.md originally described. Reason: `tracing.ts` always uses the
|
||||
in-memory test exporter when `NODE_ENV=test`, so the integration suite's own running app can't
|
||||
be pointed at a bad OTLP endpoint to exercise this. The unit test instead constructs a real
|
||||
`BasicTracerProvider`/`BatchSpanProcessor`/`OTLPTraceExporter` pointed at a genuinely
|
||||
unreachable address directly, and — importantly — verifies the SDK's _background_ export path
|
||||
(what production actually exercises) never produces an unhandled rejection, rather than calling
|
||||
`forceFlush()` directly, which is documented OpenTelemetry behavior that _does_ reject on a
|
||||
failed export by design (the first version of this test asserted the wrong thing and failed
|
||||
against real, correct SDK behavior — not a bug in this feature's own code).
|
||||
- `sla.service.ts`'s pre-existing status-overwrite gap (a `'breached'` run's status silently
|
||||
becomes `'completed'` if the ticket later resolves — see research.md §5) was worked around for
|
||||
the metric's own correctness (read `run.status` before the overwrite) but left unfixed in the
|
||||
underlying data, consistent with how 013-auth-hardening documented a pre-existing bug it found
|
||||
without fixing it.
|
||||
- Found and fixed one genuine cross-file test-isolation bug this feature's own new test caused:
|
||||
`business-metrics.test.ts`'s "human resolution" case originally drove a ticket through a real
|
||||
`HUMAN_ESCALATION` transition via `ticketsService.updateStatus`, which — same as any other
|
||||
escalation in this codebase — triggers the real orchestration subscriber's default
|
||||
`ROUND_ROBIN` auto-assignment against every agent in the shared throwaway database, including
|
||||
other concurrently-running test files' own dedicated agents (reproduced deterministically
|
||||
against `agent-ticket-queue.test.ts`). Fixed by driving the intermediate state-machine
|
||||
transitions directly through `ticketsRepository.updateStatus` (no domain-event publish)
|
||||
instead, reserving the real, event-publishing `ticketsService.updateStatus` call for only the
|
||||
final `RESOLVED` transition the metric subscriber actually needs to observe.
|
||||
- Separately, found (not caused by this feature — confirmed via `git checkout` to the clean
|
||||
pre-014 commit and reproducing the identical failure) a pre-existing systemic collision risk in
|
||||
ticket-code generation: `ticket-code.ts`'s `deriveProductCode` keeps only the first 4
|
||||
alphabetic characters of `externalProductId`, so essentially every integration test file in
|
||||
this codebase (nearly all of which name their test products `TEST_<SOMETHING>`) collapses to
|
||||
the identical `"TEST"` code prefix. Running enough `TEST_*`-prefixed files concurrently (as
|
||||
vitest does by default across worker threads/processes) makes independent files race for the
|
||||
same `TEST-<year>-<sequence>` numbering space, occasionally exceeding
|
||||
`tickets.service.ts`'s fixed `MAX_CODE_RETRIES = 5` and surfacing as a real `500`
|
||||
(`Unique constraint failed on the fields: (code)`) instead of the retry silently absorbing it.
|
||||
Confirmed independent of this feature (reproduces on `79bc2ef`, 013-auth-hardening's tip, with
|
||||
none of this feature's code present) and left unfixed here — a ticket-code-generation
|
||||
concurrency fix belongs to 003-ticketing's own module, out of scope for an observability
|
||||
feature. Worth a dedicated future fix (e.g. a longer/hash-based product code, or a
|
||||
database-level sequence rather than a `COUNT`-then-retry scheme).
|
||||
@@ -0,0 +1,54 @@
|
||||
# Contract: `/metrics` output
|
||||
|
||||
This feature adds no new HTTP endpoints — `GET /metrics` already exists and its response shape
|
||||
(Prometheus text exposition format) is unchanged. This document is the contract for its
|
||||
**content**: which metric series a consumer (Prometheus, or any scraper) can rely on after this
|
||||
feature ships, replacing the usual per-endpoint request/response contract for a feature with no
|
||||
new routes.
|
||||
|
||||
## Guarantees
|
||||
|
||||
1. Every metric already exposed today (the default `prom-client` process metrics, and
|
||||
`supporthub_http_request_duration_seconds`) continues to appear, with the same name and label
|
||||
set — FR-010. `supporthub_http_request_duration_seconds` gains real observations where today
|
||||
it has none; its metric name/labels/type do not change.
|
||||
2. Each of the eleven new series in [data-model.md](../data-model.md#metrics-prometheus-via-prom-client)
|
||||
appears on `/metrics` from process start (a `Counter`/`Histogram` with zero observations
|
||||
still exports its metadata — `# HELP`/`# TYPE` lines — even before its first increment; a
|
||||
consumer's dashboard/alert config can reference it immediately without waiting for the first
|
||||
event).
|
||||
3. No metric name or label value is derived from unbounded, request-supplied input — every
|
||||
label is one of: a fixed small enum (`outcome`, `resolved_by`, `matched`), a route pattern
|
||||
(bounded by the number of registered routes), a tool name (bounded by the tool registry), an
|
||||
error code or category ID (bounded by admin-configured product data, not raw user text).
|
||||
This is a deliberate constraint, not an incidental one — unbounded label cardinality is a
|
||||
well-known way to make a Prometheus deployment fall over, and every label chosen in
|
||||
data-model.md was checked against this before being finalized.
|
||||
4. `/health`, `/health/live`, `/health/ready` response shapes are unchanged (FR-010) — this
|
||||
feature does not touch `health.service.ts` or `health.routes.ts`.
|
||||
|
||||
## Example (illustrative, not exhaustive)
|
||||
|
||||
```text
|
||||
# HELP supporthub_http_request_duration_seconds Duration of HTTP requests in seconds
|
||||
# TYPE supporthub_http_request_duration_seconds histogram
|
||||
supporthub_http_request_duration_seconds_bucket{method="POST",route="/tickets",status_code="201",le="0.1"} 3
|
||||
supporthub_http_request_duration_seconds_count{method="POST",route="/tickets",status_code="201"} 3
|
||||
|
||||
# HELP supporthub_ai_session_outcomes_total Count of AI support sessions by terminal outcome
|
||||
# TYPE supporthub_ai_session_outcomes_total counter
|
||||
supporthub_ai_session_outcomes_total{outcome="resolved"} 12
|
||||
supporthub_ai_session_outcomes_total{outcome="escalated"} 4
|
||||
|
||||
# HELP supporthub_sla_run_outcomes_total Count of SLA runs by outcome
|
||||
# TYPE supporthub_sla_run_outcomes_total counter
|
||||
supporthub_sla_run_outcomes_total{outcome="met"} 9
|
||||
supporthub_sla_run_outcomes_total{outcome="breached"} 1
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
Integration tests assert against this contract by scraping `GET /metrics` (a real
|
||||
`app.inject` call, real registry) before and after driving each metric's real underlying event
|
||||
through the real API, parsing the specific series' value out of the text response and asserting
|
||||
it moved by exactly the expected amount — never by mocking `prom-client` or the registry itself.
|
||||
@@ -0,0 +1,72 @@
|
||||
# Data Model: Full Observability
|
||||
|
||||
No Prisma schema changes — every entity here is in-process or exported to an external
|
||||
observability sink, never persisted to Postgres.
|
||||
|
||||
## Request Context Store
|
||||
|
||||
`AsyncLocalStorage<RequestContextSnapshot>`, populated once per request in
|
||||
`request-context.plugin.ts`'s existing `onRequest` hook (the same hook that already builds
|
||||
`request.reqContext`), read by `logger.ts`'s Pino `mixin` function on every subsequent log call
|
||||
made anywhere during that request's handling.
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `requestId` | `string` | Same value already assigned to `request.reqContext.requestId` |
|
||||
| `correlationId` | `string` | Same value already assigned to `request.reqContext.correlationId` |
|
||||
|
||||
## Access Log Line (shape, not a stored entity)
|
||||
|
||||
Emitted once per completed request via the existing `logger` singleton from the new
|
||||
`onResponse` hook.
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `method` | `string` | HTTP method |
|
||||
| `route` | `string` | Parameterized route pattern (`request.routeOptions.url`), not the raw URL |
|
||||
| `statusCode` | `number` | Response status |
|
||||
| `durationMs` | `number` | `reply.elapsedTime` |
|
||||
| `requestId` / `correlationId` | `string` | Via the mixin, same as every other line for this request |
|
||||
| `event` | `string` | Fixed value `"http_request_completed"` — lets log queries filter to access-log lines specifically |
|
||||
|
||||
Log level: `info` for 2xx/3xx, `warn` for 4xx, `error` for 5xx — mirrors the existing
|
||||
error-handler's own level choices (`app.ts`) so severity is consistent across both sources of
|
||||
request-outcome logging.
|
||||
|
||||
## Metrics (Prometheus, via `prom-client`)
|
||||
|
||||
All registered in `infrastructure/observability/metrics.ts` on the existing default registry
|
||||
(`metricsRegistry`, already exposed at `GET /metrics`), all prefixed `supporthub_` to match the
|
||||
existing histogram and default-metrics prefix.
|
||||
|
||||
| Metric name | Type | Labels | Incremented/observed when |
|
||||
|---|---|---|---|
|
||||
| `supporthub_http_request_duration_seconds` | Histogram *(existing, now actually observed)* | `method`, `route`, `status_code` | Every completed HTTP request |
|
||||
| `supporthub_ai_session_outcomes_total` | Counter | `outcome` (`resolved` \| `escalated`) | An AI support session reaches a terminal `resolved`/`escalated` status |
|
||||
| `supporthub_ticket_resolutions_total` | Counter | `resolved_by` (`ai` \| `human`) | A ticket reaches `RESOLVED`, labeled from the ticket's `Resolution.resolvedBy` |
|
||||
| `supporthub_ticket_resolution_duration_seconds` | Histogram | — | A ticket reaches `RESOLVED` — observes `resolvedAt - ticket.createdAt` |
|
||||
| `supporthub_ticket_first_response_duration_seconds` | Histogram | — | The first `AGENT_MESSAGE` is posted on a ticket — observes `firstResponseAt - ticket.createdAt` |
|
||||
| `supporthub_sla_run_outcomes_total` | Counter | `outcome` (`met` \| `breached`) | An SLA run completes on time (`met`) or is flagged by the breach sweep (`breached`) |
|
||||
| `supporthub_escalations_total` | Counter | `reason` | An `ESCALATION_TRIGGERED` domain event fires (already published unconditionally today) |
|
||||
| `supporthub_problems_created_total` | Counter | `category_id` (or `uncategorized`) | A `Problem` row is created (at ticket-intake time) |
|
||||
| `supporthub_known_error_lookups_total` | Counter | `code` | A valid error code's known issues are looked up |
|
||||
| `supporthub_knowledge_retrieval_outcomes_total` | Counter | `matched` (`true` \| `false`) | The AI's `searchProductKnowledge` tool call returns zero vs. one-or-more results |
|
||||
| `supporthub_tool_invocations_total` | Counter | `tool`, `outcome` (`success` \| `failed`) | Every AI tool-call result, any tool |
|
||||
|
||||
Deliberately **not** separate metrics (per spec.md's Assumptions): "recurring problems" and
|
||||
"most common errors" are read directly off `supporthub_problems_created_total` and
|
||||
`supporthub_known_error_lookups_total` respectively via a monitoring stack's own `topk`/`rate`
|
||||
query — no additional "top N" metric or logic is computed by this application.
|
||||
|
||||
## Traces / Spans (exported, not persisted)
|
||||
|
||||
| Span | Parent | Attributes | Created in |
|
||||
|---|---|---|---|
|
||||
| `ticket.create` | (root) | `ticket.id`, `product.externalProductId` | `ticketing/tickets/service/tickets.service.ts` |
|
||||
| `ai.escalation` | `ticket.create` (if within the same request) or its own root (async paths) | `ticket.id`, `session.id` | `ai-support/sessions/service/session.service.ts`, around the escalation branch |
|
||||
| `orchestration.assignment` | `ai.escalation` (via the `TICKET_UPDATED`/`HUMAN_ESCALATION` subscriber) | `ticket.id`, `strategy` | `orchestration/orchestration` + `orchestration/assignments`, wrapping the existing `handleHumanEscalation` call |
|
||||
|
||||
Span context propagation across the domain-event bus relies on the OpenTelemetry Context API's
|
||||
own async-local propagation — since `eventBus.publish(...)` is `await`ed synchronously within
|
||||
the same call chain (confirmed in `tickets.service.ts`/`escalation.service.ts`), no manual
|
||||
context-carrying payload field is needed.
|
||||
@@ -0,0 +1,152 @@
|
||||
# Implementation Plan: Full Observability
|
||||
|
||||
**Branch**: `014-full-observability` | **Date**: 2026-09-07 | **Spec**: [spec.md](./spec.md)
|
||||
|
||||
**Input**: Feature specification from `specs/014-full-observability/spec.md`
|
||||
|
||||
## Summary
|
||||
|
||||
Wires three already-scaffolded-but-inert observability primitives into something real: a
|
||||
per-request structured access log (none exists today — Fastify's own request logging is fully
|
||||
disabled), the existing-but-never-observed request-duration histogram, and a real OpenTelemetry
|
||||
tracer provider behind the existing-but-never-called `getTracer()` helper. Adds eleven live
|
||||
Prometheus counters/histograms for the business-health metrics `docs/09-testing-observability-
|
||||
cicd.md` names, each wired at one existing choke point per metric (an event-bus subscriber where
|
||||
one already exists for the transition, a single already-existing method otherwise) rather than
|
||||
scattered across every call site. No new endpoints, no schema changes, no `supporthub-web` work
|
||||
— see research.md for the exact hook point chosen for each of the fourteen instrumentation
|
||||
targets (3 infra + 11 named metrics) and why.
|
||||
|
||||
## Technical Context
|
||||
|
||||
**Language/Version**: TypeScript 5.4 / Node.js 20+ (unchanged).
|
||||
|
||||
**Primary Dependencies**: New — `@opentelemetry/exporter-trace-otlp-http` (OTLP/HTTP span
|
||||
export), `@opentelemetry/resources` (service-name resource attribute). Reused, already
|
||||
installed — `@opentelemetry/api`, `@opentelemetry/sdk-trace-base` (provider, processors, and
|
||||
both the console and in-memory exporters used here all come from this one package), `prom-client`,
|
||||
`pino`. Reused Node built-in — `async_hooks`' `AsyncLocalStorage`.
|
||||
|
||||
**Storage**: No schema change. All new state is either in-process (Prometheus metric registry,
|
||||
the ALS request-context store, the tracer provider) or exported to wherever tracing is
|
||||
configured to send it — no new Postgres/Redis reads or writes beyond a handful of existing-table
|
||||
lookups already needed to label a metric correctly (e.g. `resolutionRepository.findByTicketId`
|
||||
to distinguish AI vs. human resolution).
|
||||
|
||||
**Testing**: Vitest — unit tests for the ALS-based logger mixin (a log call inside a request
|
||||
context carries requestId/correlationId; one outside carries neither) and for the
|
||||
SLA-compliance metric's "don't double-count an already-breached run as met" guard. Integration
|
||||
tests against real Postgres/Redis for: the access-log line's presence/shape (captured via a
|
||||
`logger.info` spy, same technique as 013's password-reset test), `/metrics` scraped before/after
|
||||
real traffic showing the duration histogram and each of the eleven business counters/histograms
|
||||
change by the expected amount when their real underlying event is driven through the real API,
|
||||
and a real multi-span trace (read back from the test-environment `InMemorySpanExporter`) for the
|
||||
two named cross-module paths.
|
||||
|
||||
**Target Platform**: Same Fastify modular monolith. Modifies
|
||||
`infrastructure/observability/*` (logger, metrics, tracing, a new request-context store) and
|
||||
`plugins/request-context.plugin.ts` (the new `onResponse` hook); adds small, single-call-site
|
||||
instrumentation lines inside `ai-support/sessions`, `ai-support/knowledge`, `ai-support/tools`,
|
||||
`ticketing/tickets`, `ticketing/messages`, `orchestration/sla`, and a handful of new subscribers
|
||||
in `src/events/handlers/index.ts`. No module gains a new public export surface beyond what
|
||||
`getTracer()` already exposed.
|
||||
|
||||
**Project Type**: Backend service — single project.
|
||||
|
||||
**Performance Goals**: The `onResponse` hook adds one Pino log call and one histogram `.observe`
|
||||
per request — both already-paid-for infrastructure (the logger and the metric object already
|
||||
exist), no new I/O on the request hot path. Trace export runs via `BatchSpanProcessor` (out of
|
||||
the request's own async chain) so span export latency never adds to response time. Metric
|
||||
increments at the eleven business hook points are in-memory counter operations, not database
|
||||
writes — the handful of read lookups needed for correct labeling (e.g. the resolution lookup for
|
||||
#3/#4) are single-row, already-indexed reads on tables these modules already query routinely.
|
||||
|
||||
**Constraints**: FR-007 — tracing must degrade gracefully; the API must start and serve traffic
|
||||
normally with no collector configured or reachable. FR-009 — no new human-facing endpoint,
|
||||
dashboard, or aggregation logic; every FR-008 metric is a raw counter/histogram for an external
|
||||
scraper, full stop. FR-010 — `/health*` and the existing histogram's shape on `/metrics` must
|
||||
not change for any existing consumer (only new metrics are added, nothing existing is renamed or
|
||||
removed).
|
||||
|
||||
**Scale/Scope**: Zero new routes. Three modified observability infrastructure files plus one new
|
||||
request-context store. Eleven new metric definitions plus their one-choke-point instrumentation
|
||||
call each. Two new dependencies. No schema migration, no new module.
|
||||
|
||||
## 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 | Not applicable — no identity/access surface touched. | PASS — N/A |
|
||||
| II. Configuration Over Hardcoding | The tracing exporter destination (`OTEL_EXPORTER_OTLP_ENDPOINT`) is env-driven, not hardcoded per environment; no business policy value is introduced by this feature (no SLA/routing/threshold numbers). | PASS |
|
||||
| III. Layered Architecture With Enforced Module Boundaries | No new module; existing module boundaries unchanged (each metric's instrumentation call lives inside the module that already owns the event, per research.md's per-metric table). The two repository-layer instrumentation calls (#1/#2, AI session status) are a deliberate, disclosed exception — see research.md §5's justification: observability calls are already a cross-cutting concern used from any layer in this codebase (e.g. `logger.error` inside `tool-executor.ts`), not the kind of business-logic leakage this principle targets. | PASS |
|
||||
| IV. AI Recommends, Deterministic Policy Decides | Not applicable — no AI decision logic changed, only observation of its outcomes. | PASS — N/A |
|
||||
| V. Evidence-Based Verification | Not applicable. | PASS — N/A |
|
||||
| VI. Durable Audit & History | Directly implements this principle's own stated requirement — "every log line MUST carry a request ID/correlation ID" is written in the constitution today but not actually true until this feature (FR-001/FR-002). | PASS — this feature closes a pre-existing constitutional gap |
|
||||
| VII. Concurrency-Safe, Durable Job Handling | The first-response-time metric (#5) has a benign, disclosed race (two concurrent first `AGENT_MESSAGE`s could both read "zero prior messages" and both observe) — acceptable because it is a best-effort observability metric, not the assignment/SLA correctness this principle is protecting; no persisted state or business decision depends on it. | PASS |
|
||||
| VIII. Problem and Ticket Are Separate, Related Entities | Not applicable — no model change. | PASS — N/A |
|
||||
| Technology & Platform Constraints | Two new dependencies (both OpenTelemetry, both already in the stack's declared technology list — "OpenAPI" aside, tracing itself was always part of the stated stack via the pre-existing `@opentelemetry/api`/`sdk-trace-base` dependencies) — no new infrastructure category introduced. | PASS |
|
||||
|
||||
No violations requiring Complexity Tracking justification.
|
||||
|
||||
## Project Structure
|
||||
|
||||
### Documentation (this feature)
|
||||
|
||||
```text
|
||||
specs/014-full-observability/
|
||||
├── plan.md
|
||||
├── research.md
|
||||
├── data-model.md
|
||||
├── quickstart.md
|
||||
├── contracts/
|
||||
│ └── metrics-contract.md
|
||||
└── tasks.md
|
||||
```
|
||||
|
||||
### Source Code (repository root)
|
||||
|
||||
```text
|
||||
supporthub-api/
|
||||
├── src/
|
||||
│ ├── infrastructure/
|
||||
│ │ └── observability/
|
||||
│ │ ├── logger.ts # MODIFIED — mixin reads the new ALS store
|
||||
│ │ ├── metrics.ts # MODIFIED — 11 new Counter/Histogram definitions
|
||||
│ │ ├── tracing.ts # MODIFIED — real provider init, exporter selection
|
||||
│ │ └── request-context.store.ts # NEW — AsyncLocalStorage<RequestContext>
|
||||
│ ├── plugins/
|
||||
│ │ └── request-context.plugin.ts # MODIFIED — onResponse access-log + histogram hook,
|
||||
│ │ onRequest now runs the rest of the request
|
||||
│ │ inside the ALS store
|
||||
│ ├── events/
|
||||
│ │ └── handlers/index.ts # MODIFIED — 3 new subscribers (human-resolution +
|
||||
│ │ resolution-time on TICKET_UPDATED/RESOLVED,
|
||||
│ │ escalation-rate on ESCALATION_TRIGGERED)
|
||||
│ └── modules/
|
||||
│ ├── ai-support/
|
||||
│ │ ├── sessions/repository/session.repository.ts # MODIFIED — AI resolution/escalation
|
||||
│ │ ├── knowledge/service/error-codes.service.ts # MODIFIED — most-common-errors
|
||||
│ │ └── tools/service/tools.service.ts # MODIFIED — tool-failure + knowledge-
|
||||
│ │ effectiveness
|
||||
│ ├── ticketing/
|
||||
│ │ ├── tickets/service/tickets.service.ts # MODIFIED — recurring-problems, plus
|
||||
│ │ │ the two named trace spans
|
||||
│ │ └── messages/service/messages.service.ts # MODIFIED — first-response-time
|
||||
│ └── orchestration/
|
||||
│ └── sla/service/sla.service.ts # MODIFIED — SLA-compliance
|
||||
└── tests/
|
||||
├── unit/observability/ # ALS mixin, SLA-compliance double-count guard
|
||||
└── integration/observability/ # access log, /metrics scrape assertions (11 metrics
|
||||
+ duration histogram), cross-module trace
|
||||
```
|
||||
|
||||
**Structure Decision**: Single project, no new module. All changes are surgical additions inside
|
||||
`infrastructure/observability` (the module that already owns this concern) plus one small,
|
||||
justified instrumentation line inside each of six existing business modules, following the
|
||||
per-metric hook points research.md already identified against the real, current code.
|
||||
|
||||
## Complexity Tracking
|
||||
|
||||
*No constitution violations — table intentionally omitted.*
|
||||
@@ -0,0 +1,70 @@
|
||||
# Quickstart: Full Observability
|
||||
|
||||
Manual verification steps for each user story, against a running instance backed by real
|
||||
Postgres/Redis (the throwaway Docker containers already used throughout this project's test
|
||||
suite work equally well for a manual run).
|
||||
|
||||
## Scenario 1 — Per-request access log (User Story 1)
|
||||
|
||||
1. Start the API. Send any request (e.g. `GET /health`).
|
||||
2. **Expected**: exactly one log line appears with `event: "http_request_completed"`, the
|
||||
request's method, route, status code, and a `requestId`.
|
||||
3. Send a request to a route that triggers additional internal logging (e.g. a login attempt).
|
||||
4. **Expected**: every log line produced while handling that request — the access-log line and
|
||||
any domain log lines — carries the same `requestId`/`correlationId`.
|
||||
5. Send a request to a route that doesn't exist.
|
||||
6. **Expected**: a 404 access-log line is still emitted (not silently dropped).
|
||||
|
||||
## Scenario 2 — Live request-health metrics (User Story 2)
|
||||
|
||||
1. Send a mix of successful and failing requests (e.g. a valid login, then three wrong-password
|
||||
logins).
|
||||
2. Scrape `GET /metrics`.
|
||||
3. **Expected**: `supporthub_http_request_duration_seconds_count` has observations labeled
|
||||
`route="/auth/login"` with both `status_code="200"` and `status_code="401"` present, letting
|
||||
an operator compute the error rate for that route from these two series alone.
|
||||
|
||||
## Scenario 3 — Cross-module trace (User Story 3)
|
||||
|
||||
1. With the API running in a mode where tracing exports to the console (no
|
||||
`OTEL_EXPORTER_OTLP_ENDPOINT` configured), drive a request that escalates a ticket to a human
|
||||
and triggers automatic orchestration/assignment.
|
||||
2. **Expected**: console output shows a `ticket.create`-or-`ai.escalation` root span and an
|
||||
`orchestration.assignment` child span sharing the same trace ID, with the child's start time
|
||||
at or after the parent's.
|
||||
3. Stop the (nonexistent) collector / leave `OTEL_EXPORTER_OTLP_ENDPOINT` pointed at an
|
||||
unreachable address.
|
||||
4. **Expected**: the API still starts and serves requests normally; only a logged export-failure
|
||||
warning appears, nothing surfaces to any HTTP response.
|
||||
|
||||
## Scenario 4 — Business-health metrics (User Story 4)
|
||||
|
||||
For each metric, scrape `/metrics`, note the current value, drive the real event, scrape again,
|
||||
and confirm the expected series moved by exactly one (or by the expected duration observation):
|
||||
|
||||
1. Complete an AI session without escalating → `supporthub_ai_session_outcomes_total{outcome="resolved"}` +1.
|
||||
2. Complete an AI session that escalates, then have a human agent resolve the ticket →
|
||||
`supporthub_ai_session_outcomes_total{outcome="escalated"}` +1, and once resolved,
|
||||
`supporthub_ticket_resolutions_total{resolved_by="human"}` +1.
|
||||
3. Resolve any ticket → `supporthub_ticket_resolution_duration_seconds` gains one new observation.
|
||||
4. Post the first agent reply on a ticket → `supporthub_ticket_first_response_duration_seconds`
|
||||
gains one new observation.
|
||||
5. Let an SLA run complete on time, and separately let one breach (via the existing breach-sweep
|
||||
test helper) → `supporthub_sla_run_outcomes_total{outcome="met"}` and
|
||||
`{outcome="breached"}` each +1 respectively.
|
||||
6. Trigger an escalation → `supporthub_escalations_total{reason="<the actual reason>"}` +1.
|
||||
7. Create a ticket for a categorized problem →
|
||||
`supporthub_problems_created_total{category_id="<id>"}` +1.
|
||||
8. Look up a valid error code's known issues →
|
||||
`supporthub_known_error_lookups_total{code="<code>"}` +1.
|
||||
9. Have the AI's `searchProductKnowledge` tool return zero results, then results →
|
||||
`supporthub_knowledge_retrieval_outcomes_total{matched="false"}` then `{matched="true"}`,
|
||||
each +1 in turn.
|
||||
10. Have any AI tool invocation fail → `supporthub_tool_invocations_total{tool="<name>",
|
||||
outcome="failed"}` +1.
|
||||
|
||||
## What "done" looks like
|
||||
|
||||
All four scenarios pass against a real Postgres/Redis, `/health*` and the existing
|
||||
`supporthub_http_request_duration_seconds` metric's shape are unchanged for any existing
|
||||
consumer, and the API starts and serves traffic normally with no tracing collector configured.
|
||||
@@ -0,0 +1,172 @@
|
||||
# Research: Full Observability
|
||||
|
||||
All decisions below were made against the actual current code (grep/read), not assumption —
|
||||
several existing pieces (the histogram, `getTracer()`) are dead scaffolding that looked complete
|
||||
from their exports alone but do nothing today.
|
||||
|
||||
## 1. Per-request access log
|
||||
|
||||
**Decision**: Add an `onResponse` hook (Fastify fires this for every completed response,
|
||||
including 404s and early replies from other hooks like the rate limiter, satisfying the FR-001
|
||||
edge case) that logs one line via the existing `logger` singleton: `{method, route, statusCode,
|
||||
durationMs, requestId, correlationId}`. `route` uses `request.routeOptions.url` (the
|
||||
parameterized pattern, e.g. `/tickets/:id`) rather than `request.url`, to keep label/log
|
||||
cardinality bounded — the raw URL contains IDs. `reply.elapsedTime` (Fastify's own built-in
|
||||
per-request timer) supplies duration with no manual `Date.now()` bookkeeping.
|
||||
|
||||
**Why not Fastify's built-in request logger**: `app.ts` deliberately sets `logger: false` and
|
||||
routes all logging through the shared Pino `logger` singleton (see its own comment: "Managed
|
||||
centrally via Pino logger instance"). Re-enabling Fastify's built-in logger would mean two
|
||||
independent logging paths with two different configurations; a hook that calls the existing
|
||||
singleton keeps one path.
|
||||
|
||||
**Where**: `request-context.plugin.ts` already owns the per-request lifecycle (it's the one
|
||||
place with an `onRequest` hook establishing `reqContext`) — its `onResponse` counterpart is
|
||||
added in the same file, not a new plugin, so request-lifecycle logging concerns stay together.
|
||||
|
||||
## 2. Attaching request ID/correlation ID to every log line (FR-002)
|
||||
|
||||
**Decision**: `AsyncLocalStorage<RequestContext>`, populated in the same `onRequest` hook that
|
||||
already builds `reqContext`, combined with Pino's `mixin` option (a function called for every
|
||||
log line, merging its return value into that line) reading from the store. This makes every
|
||||
call through the existing shared `logger` singleton automatically carry `requestId`/
|
||||
`correlationId` with **zero changes to any existing call site** — dozens of `logger.info/warn/
|
||||
error(...)` calls across every module already pass ad hoc fields but not always `requestId`
|
||||
consistently.
|
||||
|
||||
**Why not `request.log`**: Fastify's per-request child logger (`request.log`) is the standard
|
||||
Fastify idiom for this, but it would require passing `request` (or `request.log`) into every
|
||||
service/repository/mapper that currently imports the plain `logger` singleton directly — a
|
||||
sweeping, high-risk refactor across nearly every module for a feature whose whole point is
|
||||
*reducing* risk. The ALS+mixin approach reaches the same outcome (every log line correlated)
|
||||
without touching a single existing call site.
|
||||
|
||||
**Merge order**: Pino applies `mixin()`'s fields before merging the call's own object, so an
|
||||
explicit `requestId` passed at a call site (several already do this manually, e.g.
|
||||
`app.ts`'s error handler) still wins — no behavior change for those call sites, just now
|
||||
redundant (harmless).
|
||||
|
||||
## 3. Request-duration histogram + request-count
|
||||
|
||||
**Decision**: `httpRequestDurationHistogram.observe({method, route, status_code},
|
||||
reply.elapsedTime / 1000)` in the same `onResponse` hook. Prometheus histograms automatically
|
||||
expose a `<name>_count` and `<name>_sum` per label combination — FR-004's "compute error rate
|
||||
per route/status" is satisfied by that built-in output; no separate counter metric is added, to
|
||||
avoid two metrics tracking overlapping information.
|
||||
|
||||
## 4. Distributed tracing
|
||||
|
||||
**Decision**: Initialize a real `BasicTracerProvider` (from the already-installed
|
||||
`@opentelemetry/sdk-trace-base` — no new dependency for the SDK itself) at process start, with
|
||||
`trace.setGlobalTracerProvider(...)` so the existing, previously-inert `getTracer()` helper
|
||||
starts returning a working tracer with zero change to its own signature. Exporter selection is
|
||||
config-driven (`OTEL_EXPORTER_OTLP_ENDPOINT`, following the OpenTelemetry project's own standard
|
||||
env var name rather than inventing a new one):
|
||||
|
||||
- Set → `OTLPTraceExporter` (new dependency: `@opentelemetry/exporter-trace-otlp-http`, the
|
||||
lighter HTTP/JSON variant, avoiding the gRPC exporter's heavier dependency footprint), wrapped
|
||||
in a `BatchSpanProcessor`.
|
||||
- Unset (local dev, and any environment that hasn't configured a collector) →
|
||||
`ConsoleSpanExporter` (part of `sdk-trace-base`, zero extra dependency) wrapped in a
|
||||
`SimpleSpanProcessor`, so spans are visible immediately without standing up a collector.
|
||||
- Test environment → `InMemorySpanExporter` (also part of `sdk-trace-base`, built specifically
|
||||
for tests) wrapped in a `SimpleSpanProcessor` — this lets integration tests assert on real,
|
||||
actually-exported span data (names, parent/child nesting, attributes) with a real
|
||||
`TracerProvider` doing real work, the only substitution is *where the spans end up*, the same
|
||||
"real infrastructure, substitute only the destination" pattern already used for Pino's
|
||||
transport (`pino-pretty` in development, plain JSON otherwise).
|
||||
|
||||
**New dependencies**: `@opentelemetry/exporter-trace-otlp-http`, `@opentelemetry/resources` (for
|
||||
a `service.name: supporthub-api` resource attribute — without it, every span is anonymous in
|
||||
whatever backend receives them).
|
||||
|
||||
**Graceful degradation (FR-007)**: `BatchSpanProcessor`'s own export failures are caught and
|
||||
logged by the OpenTelemetry SDK internally (it never throws into application code); nothing in
|
||||
this feature needs to add its own try/catch around span creation for this to hold, but the SDK's
|
||||
internal diagnostic logger is wired to `logger.warn` (via `diag.setLogger`) so export failures
|
||||
are visible in this project's own log stream rather than swallowed silently.
|
||||
|
||||
**Where spans are added (FR-006)**: two entry points, wrapping already-existing method calls
|
||||
rather than restructuring them:
|
||||
- `ai-support/sessions/service/session.service.ts`'s escalation path — a span around the call
|
||||
that ultimately triggers `orchestrationService.handleHumanEscalation` (via the
|
||||
`TICKET_UPDATED` → `HUMAN_ESCALATION` domain-event subscriber in
|
||||
`src/events/handlers/index.ts`), and a child span inside
|
||||
`orchestration/orchestration`'s and `orchestration/assignments`'s own handling — showing the
|
||||
AI-diagnosis → escalation → assignment path as one connected trace.
|
||||
- `ticketing/tickets/service/tickets.service.ts`'s ticket-creation method — a root span for
|
||||
ticket intake, with the domain-event-driven downstream reactions (SLA-run creation, etc.)
|
||||
as child spans, per FR-006's second named path.
|
||||
|
||||
Trace context propagates across the event-bus's synchronous `await eventBus.publish(...)` calls
|
||||
for free (both publisher and subscriber run within the same Node async-context chain the OTel
|
||||
context API rides on — no manual context passing needed, since nothing here crosses a process/
|
||||
queue boundary; BullMQ jobs are explicitly out of scope for this feature's two named paths).
|
||||
|
||||
## 5. The eleven named business-health metrics (FR-008) — instrumentation points
|
||||
|
||||
Each is a `prom-client` `Counter` or `Histogram`, registered once in
|
||||
`infrastructure/observability/metrics.ts` alongside the existing histogram, and incremented/
|
||||
observed at one single already-existing choke point per metric — chosen specifically to avoid
|
||||
scattering an instrumentation call across every one of a metric's several call sites.
|
||||
|
||||
| # | Metric | Type | Hook point (file : method) | Label(s) |
|
||||
|---|---|---|---|---|
|
||||
| 1 | AI resolution rate | Counter | `ai-support/sessions/repository/session.repository.ts` : `updateStatus`, when `status === 'resolved'` | — |
|
||||
| 2 | AI escalation rate | Counter | same method, when `status === 'escalated'` | — |
|
||||
| 3 | Human resolution rate | Counter | new `TICKET_UPDATED` subscriber (`events/handlers/index.ts`) on `newStatus === 'RESOLVED'`, looking up `resolutionRepository.findByTicketId` for `resolvedBy` | `resolvedBy !== 'ai'` only |
|
||||
| 4 | Average resolution time | Histogram | same subscriber — observes `resolvedAt - ticket.createdAt` | — |
|
||||
| 5 | First response time | Histogram | `ticketing/messages/service/messages.service.ts` : `post`, when `type === 'AGENT_MESSAGE'` and no prior `AGENT_MESSAGE` exists for the ticket | — |
|
||||
| 6 | SLA compliance | Counter | `orchestration/sla/service/sla.service.ts` : `complete` (outcome `met`, only if the run wasn't already `breached`) and `runBreachDetectionSweep` (outcome `breached`) | `outcome` |
|
||||
| 7 | Escalation rate | Counter | new `ESCALATION_TRIGGERED` subscriber (`events/handlers/index.ts`) — this event is already published unconditionally on every escalation (`escalation.service.ts`) but "for audit, not for logic" (its own comment) and has zero subscribers today | `reason` |
|
||||
| 8 | Recurring problems | Counter | `ticketing/tickets/service/tickets.service.ts` — the ticket-creation method's existing `problemsRepo.create(...)` call | `categoryId` (or `uncategorized`) |
|
||||
| 9 | Most common errors | Counter | `ai-support/knowledge/service/error-codes.service.ts` : `findKnownIssuesByErrorCode`, after a valid code is confirmed to exist | `code` |
|
||||
| 10 | Knowledge effectiveness | Counter | `ai-support/tools/service/tools.service.ts`'s single `executeTool(...)` call site, when `block.name === 'searchProductKnowledge'` | `matched` (results non-empty vs empty) |
|
||||
| 11 | Tool failure rate | Counter | same call site, every tool invocation | `tool`, `outcome` |
|
||||
|
||||
**Why the event bus for #3, #4, #7 instead of editing `resolutions.service.ts`/
|
||||
`escalation.service.ts` directly**: those two modules' domain events (`TICKET_UPDATED` with
|
||||
`newStatus`, and `ESCALATION_TRIGGERED`) are already published unconditionally for every
|
||||
relevant transition (confirmed by reading `tickets.service.ts` and `escalation.service.ts`
|
||||
directly) specifically so that a new concern reacting to "a ticket resolved" or "an escalation
|
||||
happened" never needs to modify the module that owns the transition — the exact precedent
|
||||
`src/events/handlers/index.ts`'s existing four subscribers already establish for 005/007/008.
|
||||
Metrics is exactly this kind of concern.
|
||||
|
||||
**Why the repository layer for #1/#2 instead of the event bus**: AI-session resolved/escalated
|
||||
is not currently published as a domain event at all (only ticket-level and escalation-level
|
||||
events exist) and `session.service.ts` calls `this.sessions.updateStatus(...)` from ten
|
||||
different branches — adding a domain-event publish there to reuse the event-bus pattern would
|
||||
mean either introducing a new event type used by exactly one subscriber (this feature) or
|
||||
touching all ten call sites to route through a new shared wrapper. Instrumenting the one
|
||||
repository method both approaches would have to fire through instead is the minimal, lowest-risk
|
||||
option. This mirrors how `logger` calls already appear directly inside repository/service code
|
||||
throughout this codebase (e.g. `tool-executor.ts`'s `logger.error`) — observability calls are
|
||||
already treated as a cross-cutting concern usable from any layer, not something Constitution
|
||||
Principle III's "repository is Prisma-only" rule was written to police (that rule targets
|
||||
business-logic leakage and direct Prisma access from the wrong layer, not a metrics increment
|
||||
alongside an existing Prisma call).
|
||||
|
||||
**A pre-existing correctness note surfaced while researching #6**: `sla.service.ts`'s
|
||||
`complete()` only skips its update when the run is *already* `'completed'` — not when it is
|
||||
`'breached'` — so a run that breached and then later resolved would have its `status`
|
||||
overwritten from `'breached'` back to `'completed'` in the database, silently losing the breach
|
||||
record. This is a pre-existing 008/012 behavior, not something this feature changes (the SLA
|
||||
run's persisted status is out of scope for an observability feature) — the metric itself reads
|
||||
`run.status` *before* calling `complete()`'s own update, so the metric is accurate (correctly
|
||||
counted as `breached`, never double-counted as `met`) regardless of this separate, pre-existing
|
||||
data-quality gap. Documented in this feature's own checklist Notes as a discovered issue for a
|
||||
future fix, the same way 013-auth-hardening documented the `orchestration-strategies.test.ts`
|
||||
bug it found without fixing it.
|
||||
|
||||
## 6. Test strategy for the eleven metrics and tracing
|
||||
|
||||
**Decision**: Integration tests scrape the real `/metrics` endpoint's text output (a real
|
||||
`app.inject({method: 'GET', url: '/metrics'})` call, no mocking) before and after driving the
|
||||
real underlying event through the real API (create a ticket, resolve an AI session, trigger an
|
||||
escalation, etc. — exactly as every prior feature's integration suite already does against real
|
||||
Postgres/Redis), asserting the specific metric line's value increased by the expected amount.
|
||||
Tracing is verified by reading back spans from the `InMemorySpanExporter` (test-environment
|
||||
exporter, per §4) after a real cross-module request, asserting span names and parent/child
|
||||
`spanId`/`parentSpanId` relationships — a real trace, produced by a real `TracerProvider`, just
|
||||
captured in memory instead of shipped to a collector.
|
||||
@@ -0,0 +1,131 @@
|
||||
# Feature Specification: Full Observability
|
||||
|
||||
**Feature Branch**: `014-full-observability`
|
||||
|
||||
**Created**: 2026-09-07
|
||||
|
||||
**Status**: Draft
|
||||
|
||||
**Input**: User description: "Full observability: wire the already-scaffolded logging, metrics, and tracing infrastructure into an actually working end-to-end observability layer — structured per-request access logs, a working request-duration histogram, real OpenTelemetry tracing with exported spans across critical request paths, and live Prometheus counters for the key operational metrics named in docs/09-testing-observability-cicd.md."
|
||||
|
||||
## User Scenarios & Testing *(mandatory)*
|
||||
|
||||
### User Story 1 - Trace one request end to end from its logs (Priority: P1)
|
||||
|
||||
An engineer investigating a production incident (a customer's ticket got stuck, an API call failed) needs to reconstruct exactly what the system did for that one request: which route was hit, how long it took, what it returned, and — because a support case touches many internal calls (AI session → tool calls → escalation → assignment → SLA events) — which of those internal log lines belong to the same originating request.
|
||||
|
||||
**Why this priority**: Without a per-request access log, there is currently no record that a given request even happened unless it errored. This is the minimum viable observability floor everything else builds on.
|
||||
|
||||
**Independent Test**: Can be fully tested by sending a request to any route and confirming exactly one structured access-log line is emitted for it, carrying the same request ID as any other log line produced while handling that request.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the API is running, **When** any HTTP request completes (success or failure), **Then** exactly one structured log line is emitted recording its method, route, status code, and duration.
|
||||
2. **Given** a request carries an inbound correlation ID header (or one is generated for it), **When** that request triggers further log lines anywhere in the codebase during its handling, **Then** every one of those log lines carries the same request ID and correlation ID as the access-log line for that request.
|
||||
3. **Given** a request fails with an unhandled error, **When** the access log line is emitted, **Then** it is distinguishable (by log level) from a successful request without needing to duplicate the existing error-handler logging.
|
||||
|
||||
---
|
||||
|
||||
### User Story 2 - See live request-health metrics (Priority: P1)
|
||||
|
||||
An engineer wants to know, right now, whether the API is healthy under current traffic — request volume, latency distribution, and error rate by route — without needing to grep logs.
|
||||
|
||||
**Why this priority**: A request-duration metric already exists in code but is never recorded, so `/metrics` currently reports nothing useful about request health. This is the second half of the observability floor (logs tell you what happened to one request; metrics tell you the shape of all of them).
|
||||
|
||||
**Independent Test**: Can be fully tested by sending a mix of successful and failing requests, then scraping `/metrics` and confirming the request-duration histogram and a request-count-by-status metric both reflect that traffic.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the API has served requests since it started, **When** `/metrics` is scraped, **Then** the request-duration histogram has observations labeled by method, route, and status code matching that traffic.
|
||||
2. **Given** some requests succeeded and others returned 4xx/5xx, **When** `/metrics` is scraped, **Then** a request-count metric lets an operator compute error rate by route and status class.
|
||||
|
||||
---
|
||||
|
||||
### User Story 3 - Trace a single incident's cross-module path (Priority: P2)
|
||||
|
||||
An engineer debugging why a specific ticket took an unexpectedly long or unexpected path (e.g., AI failed to resolve it, escalation didn't fire when expected) wants to see the causal chain of operations across modules for that one ticket — not just isolated log lines, but a connected trace showing how long each step took relative to the others.
|
||||
|
||||
**Why this priority**: Distributed tracing infrastructure already exists in the dependency list and a `getTracer()` helper is exported, but no tracer provider is ever initialized and no code ever calls it — today it silently does nothing. This is more valuable than plain logs for understanding *why* a multi-step flow behaved the way it did, but the system is usable without it (User Stories 1-2 already restore basic visibility), so it is P2.
|
||||
|
||||
**Independent Test**: Can be fully tested by triggering a request that flows through at least two instrumented modules (e.g., an AI escalation that results in orchestration/assignment) and confirming a trace is produced whose spans are parented correctly and whose combined duration accounts for the modules involved.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** tracing is enabled, **When** the API starts, **Then** a real tracer provider is active (not the OpenTelemetry no-op default) and spans created via the existing `getTracer()` helper are actually exported somewhere inspectable.
|
||||
2. **Given** a request flows through multiple instrumented operations (e.g., AI diagnosis triggers an escalation which triggers orchestration/assignment), **When** that request completes, **Then** the resulting trace shows each operation as a distinct, correctly-nested span under one root.
|
||||
3. **Given** tracing is not configured with an external collector in a given environment, **When** the API starts, **Then** it still starts successfully (tracing degrades gracefully, it never blocks startup or request handling).
|
||||
|
||||
---
|
||||
|
||||
### User Story 4 - See the business-health metrics this project committed to tracking (Priority: P2)
|
||||
|
||||
An engineer or team lead wants live visibility (via the same `/metrics` endpoint, for consumption by whatever monitoring stack is deployed) into the operational health metrics this project's own design doc names as important: AI resolution rate, AI escalation rate, human resolution rate, average resolution time, first response time, SLA compliance, escalation rate, recurring problems, most common errors, knowledge effectiveness, and tool failure rate.
|
||||
|
||||
**Why this priority**: These are real, currently-invisible gaps — none of them are tracked anywhere today, live or otherwise. They are P2 (not P1) because they instrument business outcomes that already have a durable system of record (the ticket/problem/SLA/escalation tables) — a missing counter is a visibility gap, not a data-loss risk, unlike User Stories 1-2's request-level blind spot.
|
||||
|
||||
**Why this scope boundary**: This story is about each metric *existing and being live-updated correctly* at the point the underlying event occurs, exposed as raw counters/histograms on `/metrics` for an external monitoring stack to graph and alert on. It explicitly does NOT include building any dashboard, chart, or human-facing report — that is a separate, not-yet-started project phase (reporting/analytics dashboards).
|
||||
|
||||
**Independent Test**: Can be fully tested, metric by metric, by driving the real underlying event (resolve a ticket via AI, resolve one via a human agent, breach an SLA, trigger an escalation, log a known error, etc.) against a running instance and confirming the corresponding value on `/metrics` changed by exactly the expected amount.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** an AI session resolves a ticket without escalating, **When** `/metrics` is scraped, **Then** the AI-resolution counter has incremented and the AI-escalation counter has not.
|
||||
2. **Given** an AI session escalates to a human and that human later resolves the ticket, **When** `/metrics` is scraped, **Then** the AI-escalation counter and the human-resolution counter have both incremented.
|
||||
3. **Given** a ticket is resolved, **When** `/metrics` is scraped, **Then** the resolution-time histogram has a new observation reflecting that ticket's actual open-to-resolved duration.
|
||||
4. **Given** an agent sends the first reply on a ticket, **When** `/metrics` is scraped, **Then** the first-response-time histogram has a new observation.
|
||||
5. **Given** an SLA run resolves as either met or breached, **When** `/metrics` is scraped, **Then** the SLA-compliance counter reflects that outcome.
|
||||
6. **Given** an escalation event fires, **When** `/metrics` is scraped, **Then** the escalation-rate counter increments, labeled by trigger reason.
|
||||
7. **Given** an AI tool invocation succeeds or fails, **When** `/metrics` is scraped, **Then** the tool-failure-rate counter reflects the outcome, labeled by tool name.
|
||||
8. **Given** a known error code is surfaced to a customer, **When** `/metrics` is scraped, **Then** a counter labeled by that error code has incremented (supports both "most common errors" and, via repeated occurrence on the same product/category, "recurring problems").
|
||||
9. **Given** the AI's knowledge retrieval step either does or does not find a usable match for the customer's problem, **When** `/metrics` is scraped, **Then** a knowledge-effectiveness counter reflects that outcome.
|
||||
|
||||
---
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- What happens when the configured tracing exporter/collector is unreachable? The API must still start and continue serving requests; span export failures must be logged but never surface to the request/response cycle.
|
||||
- What happens to in-flight metrics/traces if the process crashes before a scrape/export completes? Acceptable data loss for that window — this feature does not need to guarantee zero metric loss across a crash, only correctness of what is recorded and exported during normal operation.
|
||||
- What happens when a request has no matching route (404) or is rejected before reaching a handler (e.g., by a global rate limiter)? It must still produce exactly one access-log line and one metrics observation, so operators can see rejected traffic, not just successfully-routed traffic.
|
||||
- What happens when two requests share the same client-supplied correlation ID (e.g., a retried request)? Each still gets its own request ID and its own access-log line; only the correlation ID is shared, by design (that is what lets an operator group retries together).
|
||||
- How does the system behave for a route that legitimately never touches any of the business-event counters (e.g., a health check)? No business-metric line is expected for it — only the generic request-count/duration metrics from User Story 2 apply.
|
||||
|
||||
## Requirements *(mandatory)*
|
||||
|
||||
### Functional Requirements
|
||||
|
||||
- **FR-001**: System MUST emit exactly one structured access-log line per completed HTTP request (including requests that error, 404, or are rejected by a global hook before reaching a route handler), containing at minimum: HTTP method, route/path, response status code, duration, request ID, and correlation ID.
|
||||
- **FR-002**: System MUST attach the request ID and correlation ID already established by the existing request-context mechanism to every log line produced while handling that request, not only the access-log line.
|
||||
- **FR-003**: System MUST record every completed HTTP request's duration into the existing request-duration metric, labeled at minimum by method, route, and status code.
|
||||
- **FR-004**: System MUST expose a request-count metric (or equivalent derivable from FR-003's histogram) sufficient to compute error rate per route and status class.
|
||||
- **FR-005**: System MUST initialize a real distributed-tracing pipeline at startup so that spans created via the existing `getTracer()` helper are captured and exported to an inspectable destination, rather than discarded by the OpenTelemetry no-op default.
|
||||
- **FR-006**: System MUST create spans for the AI diagnosis → escalation → orchestration/assignment path and for the ticket-creation → orchestration path, correctly nested under one root span per originating request, so a single incident's cross-module timing is visible in one trace.
|
||||
- **FR-007**: System MUST continue to start up and serve requests normally if the configured tracing export destination is unreachable; export failures MUST be logged, never raised to the request/response cycle.
|
||||
- **FR-008**: System MUST expose live counters/histograms on the existing `/metrics` endpoint for each of: AI resolution rate, AI escalation rate, human resolution rate, average resolution time, first response time, SLA compliance, escalation rate, recurring problems, most common errors, knowledge effectiveness, and tool failure rate — each updated at the moment its underlying real event occurs (not computed by a batch job or exposed through any new endpoint).
|
||||
- **FR-009**: System MUST NOT introduce any new human-facing dashboard, chart, or reporting API as part of this feature — every metric from FR-008 is a raw, unaggregated-by-this-system counter/histogram intended for an external monitoring stack to graph, in keeping with the explicit scope boundary against the separate reporting/analytics dashboards work.
|
||||
- **FR-010**: Existing `/health`, `/health/live`, `/health/ready`, and `/metrics` endpoints MUST continue to function unchanged in shape for any existing consumer.
|
||||
|
||||
### Key Entities
|
||||
|
||||
- **Access log line**: A structured log record emitted once per completed HTTP request; not a persisted database entity — it exists only in the log stream.
|
||||
- **Request-duration metric**: A histogram, keyed by method/route/status, recording how long each request took.
|
||||
- **Trace / span**: A record of one operation's start/end time and its parent-child relationship to other operations within the same originating request, exported to wherever tracing is configured to send it.
|
||||
- **Business-event counter**: One of the eleven named live metrics in FR-008/User Story 4, each incremented (or observed, for the two duration-based ones) at the exact point its real-world event already occurs elsewhere in the system (ticket resolution, SLA run completion, escalation firing, tool invocation, etc.) — this feature adds the instrumentation call at each of those existing points, it does not change what those points do.
|
||||
|
||||
## Success Criteria *(mandatory)*
|
||||
|
||||
### Measurable Outcomes
|
||||
|
||||
- **SC-001**: Given any request made to the running API, an operator can identify, from logs alone, its method, route, outcome, duration, and every other log line produced while handling it, within seconds of it happening.
|
||||
- **SC-002**: An operator watching `/metrics` can determine current request error rate and latency distribution per route without needing to read application logs.
|
||||
- **SC-003**: An operator can find and inspect the complete cross-module trace for a specific incident that touched at least two instrumented modules, showing correctly-attributed timing per module.
|
||||
- **SC-004**: All eleven named business-health metrics are visible on `/metrics` and each one's value changes correctly and immediately in response to its real underlying event, verified against real (non-mocked) system behavior.
|
||||
- **SC-005**: Enabling this feature's tracing pipeline introduces no observable request-handling failure, and the API starts and serves traffic normally even when the tracing destination is unreachable.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- "Exported to an inspectable destination" (FR-005) means a destination this project's own test/dev environment can actually verify against — an OTLP-compatible collector endpoint in production-like environments, and an in-process/console exporter for local development and automated tests, both driven by configuration rather than hardcoded per environment. No specific commercial tracing backend (e.g., Jaeger, Honeycomb, Datadog) is mandated by this feature; wiring a specific backend in a given deployment is an operations concern outside this spec.
|
||||
- The existing Prometheus (`prom-client`) and Pino stack are the metrics/logging technology already chosen for this project (confirmed by existing code) and are reused rather than replaced.
|
||||
- "Knowledge effectiveness" is scoped to whether the AI's knowledge-retrieval step found and used a matching entry for a given diagnosis attempt (a binary outcome per attempt), not a more elaborate relevance-scoring scheme — no such scoring exists elsewhere in the system to build on.
|
||||
- "Recurring problems" and "most common errors" (FR-008) are satisfied by labeled counters an operator's monitoring stack can rank/aggregate over any time window (e.g., `topk` in PromQL) — this feature does not need to compute or store a "top N" itself, consistent with FR-009's boundary against building reporting logic.
|
||||
- This feature is backend-only (`supporthub-api`); no `supporthub-web` changes are in scope, since nothing here is presented to any human through a UI.
|
||||
- Existing `RequestContext` (`requestId`/`correlationId`), already populated by both the customer and staff auth paths (010-identity-auth), is reused as the identifier scheme for FR-001/FR-002 rather than introducing a second identifier scheme.
|
||||
@@ -0,0 +1,222 @@
|
||||
---
|
||||
description: 'Task list for 014-full-observability'
|
||||
---
|
||||
|
||||
# Tasks: Full Observability
|
||||
|
||||
**Input**: Design documents from `specs/014-full-observability/`
|
||||
|
||||
**Prerequisites**: [plan.md](./plan.md), [spec.md](./spec.md), [research.md](./research.md),
|
||||
[data-model.md](./data-model.md),
|
||||
[contracts/metrics-contract.md](./contracts/metrics-contract.md), [quickstart.md](./quickstart.md)
|
||||
|
||||
**Organization**: Tasks are grouped by user story (US1 = P1 access log, US2 = P1 request-health
|
||||
metrics, US3 = P2 tracing, US4 = P2 business-health metrics). US2 shares its hook point with
|
||||
US1 (both live in the same `onResponse` hook) so US2 depends on US1's hook existing, not on its
|
||||
own separate one. US3 and US4 are each independent of US1/US2 and of each other.
|
||||
|
||||
## Format: `[ID] [P?] [Story] Description`
|
||||
|
||||
All file paths are relative to `supporthub-api/` (repo root).
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Foundational (Blocking Prerequisites)
|
||||
|
||||
- [x] T001 Add `@opentelemetry/exporter-trace-otlp-http` and `@opentelemetry/resources` to
|
||||
`package.json` (`npm install`)
|
||||
- [x] T002 Add `src/infrastructure/observability/request-context.store.ts` — a module-level
|
||||
`AsyncLocalStorage<{requestId: string; correlationId: string}>` with a `run()` passthrough
|
||||
and a `getStore()` re-export
|
||||
- [x] T003 [P] Wire `logger.ts`'s Pino options with a `mixin` function reading from T002's store
|
||||
(returns `{}` when no store is active — a log call outside any request, e.g. at startup,
|
||||
must not throw) (depends on T002)
|
||||
|
||||
**Checkpoint**: Every subsequent log call through the shared `logger` singleton is
|
||||
request-correlated automatically, once a request actually runs inside the store (US1 wires that
|
||||
part next).
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: User Story 1 - Trace one request end to end from its logs (Priority: P1)
|
||||
|
||||
**Goal**: One structured access-log line per request; every other log line produced during that
|
||||
request's handling shares its request ID.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 1.
|
||||
|
||||
### Tests for User Story 1
|
||||
|
||||
- [x] T004 [P] [US1] Unit test: a `logger.info(...)` call made inside T002's `store.run(...)`
|
||||
carries `requestId`/`correlationId` in its output; one made outside carries neither, in
|
||||
`tests/unit/observability/request-context-mixin.test.ts` (depends on T003)
|
||||
|
||||
### Implementation for User Story 1
|
||||
|
||||
- [x] T005 [US1] In `plugins/request-context.plugin.ts`'s existing `onRequest` hook, after
|
||||
building `request.reqContext`, call the T002 store's `run()` wrapping the remainder of the
|
||||
request's handling (Fastify's `onRequest` hooks accept a `done` callback / return a
|
||||
promise — the run wraps whichever style this hook currently uses) so every subsequent
|
||||
hook/handler for this request executes inside the ALS context (depends on T002)
|
||||
- [x] T006 [US1] Add an `onResponse` hook (same plugin) that logs one line via the shared
|
||||
`logger`: `{event: "http_request_completed", method, route: request.routeOptions.url,
|
||||
statusCode: reply.statusCode, durationMs: reply.elapsedTime}`, at `info`/`warn`/`error`
|
||||
level by status class (depends on T005)
|
||||
- [x] T007 [US1] Integration test covering Quickstart Scenario 1 (one access-log line per
|
||||
request incl. 404; shared requestId across the access-log line and an internal log line
|
||||
from the same request) in `tests/integration/observability/access-log.test.ts`, using a
|
||||
`logger.info`/`logger.warn` spy the same way `password-reset-flow.test.ts` (013) already
|
||||
does (depends on T006)
|
||||
|
||||
**Checkpoint**: Quickstart Scenario 1 passes. Every request is now visible in logs even when it
|
||||
never errors.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: User Story 2 - See live request-health metrics (Priority: P1)
|
||||
|
||||
**Goal**: The existing (previously dead) request-duration histogram actually has observations;
|
||||
error rate per route/status is computable from `/metrics` alone.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 2.
|
||||
|
||||
### Implementation for User Story 2
|
||||
|
||||
- [x] T008 [US2] In the same `onResponse` hook added by T006, call
|
||||
`httpRequestDurationHistogram.observe({method, route: request.routeOptions.url, status_code:
|
||||
String(reply.statusCode)}, reply.elapsedTime / 1000)` (depends on T006)
|
||||
- [x] T009 [US2] Integration test covering Quickstart Scenario 2 (send a mix of successful/
|
||||
failing requests to the same route, scrape `/metrics`, assert both status-code label
|
||||
values are present with the expected counts) in
|
||||
`tests/integration/observability/request-metrics.test.ts` (depends on T008)
|
||||
|
||||
**Checkpoint**: Quickstart Scenario 2 passes. `/metrics` now reflects real request traffic.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: User Story 3 - Trace a single incident's cross-module path (Priority: P2)
|
||||
|
||||
**Goal**: A real `TracerProvider` is active; `getTracer()` produces spans that are actually
|
||||
exported; two named cross-module paths are instrumented.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 3.
|
||||
|
||||
### Implementation for User Story 3
|
||||
|
||||
- [x] T010 [P] [US3] Rewrite `infrastructure/observability/tracing.ts` to initialize a
|
||||
`BasicTracerProvider` at module load with a `Resource` (`service.name: "supporthub-api"`)
|
||||
and register it via `trace.setGlobalTracerProvider(...)`; exporter/processor chosen by
|
||||
`NODE_ENV`/`OTEL_EXPORTER_OTLP_ENDPOINT` per research.md §4 (`InMemorySpanExporter` +
|
||||
`SimpleSpanProcessor` in test, `OTLPTraceExporter` + `BatchSpanProcessor` when the env var
|
||||
is set, `ConsoleSpanExporter` + `SimpleSpanProcessor` otherwise); export a
|
||||
`getTestSpanExporter()` accessor (test env only) for T015 to read exported spans back;
|
||||
`getTracer()`'s own exported signature is unchanged (depends on T001)
|
||||
- [x] T011 [US3] Wire the OpenTelemetry SDK's internal diagnostic logger
|
||||
(`diag.setLogger(...)`) to the shared `logger.warn`, so span-export failures land in this
|
||||
project's own log stream instead of stderr or nowhere (depends on T010)
|
||||
- [x] T012 [P] [US3] Add a `ticket.create` span (`getTracer().startActiveSpan(...)`) around
|
||||
`tickets/service/tickets.service.ts`'s ticket-creation method, with `ticket.id` and
|
||||
`product.externalProductId` attributes, ended in a `finally` (depends on T010)
|
||||
- [x] T013 [P] [US3] Add an `ai.escalation` span around `ai-support/sessions/service/
|
||||
session.service.ts`'s escalation branch(es), with `ticket.id`/`session.id` attributes
|
||||
(depends on T010)
|
||||
- [x] T014 [US3] Add an `orchestration.assignment` span wrapping the existing
|
||||
`orchestrationService.handleHumanEscalation` call in the `TICKET_UPDATED`/
|
||||
`HUMAN_ESCALATION` subscriber (`src/events/handlers/index.ts`), with `ticket.id`/
|
||||
`strategy` attributes, so it nests under T013's span when both occur in the same request
|
||||
(depends on T010, T013)
|
||||
- [x] T015 [US3] Integration test covering Quickstart Scenario 3 steps 1-2: drive a real
|
||||
ticket-creation → escalation → orchestration/assignment flow, read spans back via T010's
|
||||
`getTestSpanExporter()`, assert `ticket.create`/`ai.escalation`/`orchestration.assignment`
|
||||
all share one trace ID with correct parent/child `spanId` relationships, in
|
||||
`tests/integration/observability/tracing.test.ts` (depends on T012, T013, T014)
|
||||
- [x] T016 [US3] Integration test covering Quickstart Scenario 3 steps 3-4: point
|
||||
`OTEL_EXPORTER_OTLP_ENDPOINT` at an unreachable address, confirm `buildApp()` still
|
||||
resolves and a request still completes successfully, in the same test file (depends on
|
||||
T010)
|
||||
|
||||
**Checkpoint**: Quickstart Scenario 3 passes. A real, inspectable trace exists for the first
|
||||
time; tracing failure never blocks the app.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: User Story 4 - See the business-health metrics this project committed to tracking (Priority: P2)
|
||||
|
||||
**Goal**: All eleven named metrics (data-model.md) are live on `/metrics`, each updated at the
|
||||
exact real event research.md identified.
|
||||
|
||||
**Independent Test**: Quickstart Scenario 4.
|
||||
|
||||
### Implementation for User Story 4
|
||||
|
||||
- [x] T017 [US4] Define all eleven new `Counter`/`Histogram` objects in
|
||||
`infrastructure/observability/metrics.ts` per data-model.md's table, exported individually
|
||||
(depends on T001)
|
||||
- [x] T018 [P] [US4] Increment `supporthub_ai_session_outcomes_total` in `ai-support/sessions/
|
||||
repository/session.repository.ts`'s `updateStatus`, labeled `outcome` when `status` is
|
||||
`'resolved'`/`'escalated'` (depends on T017)
|
||||
- [x] T019 [P] [US4] Add a `TICKET_UPDATED`/`newStatus === 'RESOLVED'` subscriber in
|
||||
`src/events/handlers/index.ts` that looks up `resolutionRepository.findByTicketId`,
|
||||
increments `supporthub_ticket_resolutions_total{resolved_by}` (`ai` vs. any other value),
|
||||
fetches the ticket for `createdAt`, and observes
|
||||
`supporthub_ticket_resolution_duration_seconds` (depends on T017)
|
||||
- [x] T020 [P] [US4] In `ticketing/messages/service/messages.service.ts`'s `post`, when
|
||||
`type === 'AGENT_MESSAGE'`, check for a prior `AGENT_MESSAGE` on the ticket and — only for
|
||||
the first one — observe `supporthub_ticket_first_response_duration_seconds` against the
|
||||
ticket's `createdAt` (depends on T017)
|
||||
- [x] T021 [P] [US4] In `orchestration/sla/service/sla.service.ts`: in `complete()`, read
|
||||
`run.status` before updating and increment `supporthub_sla_run_outcomes_total{outcome:
|
||||
"met"}` only if it was not already `'breached'`; in `runBreachDetectionSweep()`, increment
|
||||
`{outcome: "breached"}` for each newly-flagged run (depends on T017)
|
||||
- [x] T022 [P] [US4] Add an `ESCALATION_TRIGGERED` subscriber in `src/events/handlers/index.ts`
|
||||
that increments `supporthub_escalations_total{reason}` from the event payload's `reason`
|
||||
(depends on T017)
|
||||
- [x] T023 [P] [US4] In `ticketing/tickets/service/tickets.service.ts`'s ticket-creation method,
|
||||
increment `supporthub_problems_created_total{category_id}` right after `problemsRepo.create`
|
||||
succeeds (`categoryId ?? 'uncategorized'`) (depends on T017)
|
||||
- [x] T024 [P] [US4] In `ai-support/knowledge/service/error-codes.service.ts`'s
|
||||
`findKnownIssuesByErrorCode`, increment `supporthub_known_error_lookups_total{code}` once
|
||||
the error code is confirmed to exist (after the `NotFoundError` branch, not before)
|
||||
(depends on T017)
|
||||
- [x] T025 [P] [US4] In `ai-support/tools/service/tools.service.ts`'s single `executeTool(...)`
|
||||
call site: increment `supporthub_tool_invocations_total{tool, outcome}` for every call, and
|
||||
— only when `block.name === 'searchProductKnowledge'` — increment
|
||||
`supporthub_knowledge_retrieval_outcomes_total{matched}` from whether `result.output` is a
|
||||
non-empty array (depends on T017)
|
||||
- [x] T026 [US4] Unit test: `sla.service.ts`'s `complete()` does not increment the `met` outcome
|
||||
for a run already `'breached'` (a fake repo returning `status: 'breached'`) in
|
||||
`tests/unit/observability/sla-compliance-metric.test.ts` (depends on T021)
|
||||
- [x] T027 [US4] Integration test covering Quickstart Scenario 4 (all ten sub-scenarios — the
|
||||
eleventh, tool-failure, is covered by the same test file's tool-invocation case) —
|
||||
scrape `/metrics` before/after driving each real event through the real API, in
|
||||
`tests/integration/observability/business-metrics.test.ts` (depends on T018, T019, T020,
|
||||
T021, T022, T023, T024, T025)
|
||||
|
||||
**Checkpoint**: Quickstart Scenario 4 passes. All eleven named metrics are live and correct
|
||||
against real infrastructure.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6: Polish & Cross-Cutting Concerns
|
||||
|
||||
- [x] T028 [P] Update `specs/014-full-observability/checklists/requirements.md` Notes with any
|
||||
implementation-time findings (including the pre-existing SLA-run status data-quality gap
|
||||
research.md §5 already surfaced)
|
||||
- [x] T029 Run `npx tsx scripts/check-architecture.ts` and `npm run lint`/`npm run typecheck`
|
||||
- [x] T030 Full regression: `npm run test:unit` then the full integration suite against real
|
||||
Docker-provisioned Postgres/Redis, confirming nothing outside this feature regressed
|
||||
(particularly every module touched by a single-line instrumentation addition: ai-support
|
||||
sessions/knowledge/tools, ticketing tickets/messages, orchestration/sla, and the event-bus
|
||||
handlers)
|
||||
|
||||
---
|
||||
|
||||
## Dependencies & Execution Order
|
||||
|
||||
- **Foundational (Phase 1)**: No dependencies — BLOCKS User Story 1 (and transitively 2)
|
||||
- **User Story 1 (Phase 2)**: Depends on Foundational — BLOCKS User Story 2 (shares its hook)
|
||||
- **User Story 2 (Phase 3)**: Depends on User Story 1
|
||||
- **User Story 3 (Phase 4)**: Depends only on Foundational (T001) — independent of US1/US2/US4
|
||||
- **User Story 4 (Phase 5)**: Depends only on Foundational (T001/T017) — independent of
|
||||
US1/US2/US3
|
||||
- **Polish (Phase 5)**: Depends on all four user stories
|
||||
@@ -3,4 +3,8 @@ import { env } from './env';
|
||||
export const authConfig = {
|
||||
jwtSecret: env.JWT_SECRET,
|
||||
tokenLifetimeHours: env.AUTH_TOKEN_LIFETIME_HOURS,
|
||||
passwordMinLength: env.PASSWORD_MIN_LENGTH,
|
||||
passwordResetTokenLifetimeMinutes: env.PASSWORD_RESET_TOKEN_LIFETIME_MINUTES,
|
||||
loginRateLimitMaxAttempts: env.LOGIN_RATE_LIMIT_MAX_ATTEMPTS,
|
||||
loginRateLimitWindowSeconds: env.LOGIN_RATE_LIMIT_WINDOW_SECONDS,
|
||||
};
|
||||
|
||||
@@ -65,6 +65,21 @@ const envSchema = z.object({
|
||||
// already-required JWT_SECRET above (defined since the original scaffold, never consumed
|
||||
// until now) — see specs/010-identity-auth/research.md.
|
||||
AUTH_TOKEN_LIFETIME_HOURS: z.coerce.number().default(4),
|
||||
|
||||
// Authentication Hardening (013) — password-strength policy, reset-token lifetime, and
|
||||
// login rate-limiting, all CONFIGURABLE per docs/10-implementation-roadmap.md's own
|
||||
// "never hardcode a placeholder value and ship it as final" instruction — see
|
||||
// specs/013-auth-hardening/research.md.
|
||||
PASSWORD_MIN_LENGTH: z.coerce.number().default(10),
|
||||
PASSWORD_RESET_TOKEN_LIFETIME_MINUTES: z.coerce.number().default(30),
|
||||
LOGIN_RATE_LIMIT_MAX_ATTEMPTS: z.coerce.number().default(5),
|
||||
LOGIN_RATE_LIMIT_WINDOW_SECONDS: z.coerce.number().default(300),
|
||||
|
||||
// Full Observability (014) — the OpenTelemetry project's own standard env var name (not
|
||||
// invented here) for the collector endpoint spans are exported to. Unset means "no collector
|
||||
// configured" — tracing still runs, just exports to the console instead (never a startup
|
||||
// requirement) — see specs/014-full-observability/research.md "Distributed tracing".
|
||||
OTEL_EXPORTER_OTLP_ENDPOINT: z.string().optional(),
|
||||
});
|
||||
|
||||
export type EnvConfig = z.infer<typeof envSchema>;
|
||||
|
||||
@@ -1,9 +1,18 @@
|
||||
import { SpanStatusCode } from '@opentelemetry/api';
|
||||
import { eventBus } from '../event-bus';
|
||||
import { DomainEventName } from '../domain-events';
|
||||
import { BaseDomainEvent } from '../event-types';
|
||||
import { sessionsService } from '@/modules/ai-support/sessions';
|
||||
import { orchestrationService } from '@/modules/orchestration/orchestration';
|
||||
import { slaService } from '@/modules/orchestration/sla';
|
||||
import { ticketsService } from '@/modules/ticketing/tickets';
|
||||
import { resolutionRepository } from '@/modules/problem-management/resolutions';
|
||||
import {
|
||||
getTracer,
|
||||
ticketResolutionsCounter,
|
||||
ticketResolutionDurationHistogram,
|
||||
escalationsCounter,
|
||||
} from '@/infrastructure/observability';
|
||||
|
||||
interface TicketUpdatedPayload {
|
||||
ticketId: string;
|
||||
@@ -19,6 +28,14 @@ interface TicketAssignedPayload {
|
||||
actor: string;
|
||||
}
|
||||
|
||||
interface EscalationTriggeredPayload {
|
||||
ticketId: string;
|
||||
ruleId: string;
|
||||
targetNodeId: string;
|
||||
actor: string;
|
||||
reason: string;
|
||||
}
|
||||
|
||||
let registered = false;
|
||||
|
||||
/**
|
||||
@@ -50,7 +67,44 @@ export function registerDomainEventHandlers(): void {
|
||||
DomainEventName.TICKET_UPDATED,
|
||||
async (event: BaseDomainEvent<TicketUpdatedPayload>) => {
|
||||
if (event.payload.newStatus !== 'HUMAN_ESCALATION') return;
|
||||
await orchestrationService.handleHumanEscalation(event.payload.ticketId);
|
||||
// 014-full-observability data-model.md: nests under session.service.ts's `ai.escalation`
|
||||
// span when this fired from that same await chain (an escalation triggered some other way
|
||||
// — e.g. a direct admin action — still gets its own root span here, never left untraced).
|
||||
await getTracer().startActiveSpan(
|
||||
'orchestration.assignment',
|
||||
{ attributes: { 'ticket.id': event.payload.ticketId } },
|
||||
async (span) => {
|
||||
try {
|
||||
await orchestrationService.handleHumanEscalation(event.payload.ticketId);
|
||||
} catch (error) {
|
||||
span.recordException(error as Error);
|
||||
span.setStatus({ code: SpanStatusCode.ERROR });
|
||||
throw error;
|
||||
} finally {
|
||||
span.end();
|
||||
}
|
||||
},
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
// 014-full-observability data-model.md #3/#4: human-vs-AI resolution and resolution-time,
|
||||
// read off the Resolution row's own resolvedBy ("ai" | agentId — see prisma/schema.prisma)
|
||||
// rather than duplicating that distinction here.
|
||||
eventBus.subscribe(
|
||||
DomainEventName.TICKET_UPDATED,
|
||||
async (event: BaseDomainEvent<TicketUpdatedPayload>) => {
|
||||
if (event.payload.newStatus !== 'RESOLVED') return;
|
||||
const [ticket, resolution] = await Promise.all([
|
||||
ticketsService.getById(event.payload.ticketId),
|
||||
resolutionRepository.findByTicketId(event.payload.ticketId),
|
||||
]);
|
||||
if (!resolution) return;
|
||||
|
||||
ticketResolutionsCounter.inc({
|
||||
resolved_by: resolution.resolvedBy === 'ai' ? 'ai' : 'human',
|
||||
});
|
||||
ticketResolutionDurationHistogram.observe((Date.now() - ticket.createdAt.getTime()) / 1000);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -87,4 +141,14 @@ export function registerDomainEventHandlers(): void {
|
||||
await slaService.complete(event.payload.ticketId);
|
||||
},
|
||||
);
|
||||
|
||||
// 014-full-observability data-model.md #7: ESCALATION_TRIGGERED has been published
|
||||
// unconditionally on every escalation since 008-sla-escalation ("for audit, not for logic" —
|
||||
// escalation.service.ts's own comment) but had zero subscribers until now.
|
||||
eventBus.subscribe(
|
||||
DomainEventName.ESCALATION_TRIGGERED,
|
||||
async (event: BaseDomainEvent<EscalationTriggeredPayload>) => {
|
||||
escalationsCounter.inc({ reason: event.payload.reason });
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2,3 +2,4 @@ export * from './logger';
|
||||
export * from './metrics';
|
||||
export * from './tracing';
|
||||
export * from './health.service';
|
||||
export * from './request-context.store';
|
||||
|
||||
@@ -1,11 +1,18 @@
|
||||
import pino from 'pino';
|
||||
import { env } from '@/config';
|
||||
import { getRequestContextSnapshot } from './request-context.store';
|
||||
|
||||
const pinoOptions: pino.LoggerOptions = {
|
||||
level: env.LOG_LEVEL,
|
||||
base: {
|
||||
env: env.NODE_ENV,
|
||||
},
|
||||
// 014-full-observability FR-002: merges the current request's requestId/correlationId (if
|
||||
// any — a log call outside any request, e.g. at startup, gets neither) into every log line
|
||||
// made through this logger, anywhere in the codebase, with no change to any existing call
|
||||
// site. Pino applies these fields before the call's own object, so an explicit requestId a
|
||||
// call site already passes manually still wins.
|
||||
mixin: () => getRequestContextSnapshot() ?? {},
|
||||
};
|
||||
|
||||
if (env.NODE_ENV === 'development') {
|
||||
@@ -20,3 +27,7 @@ if (env.NODE_ENV === 'development') {
|
||||
}
|
||||
|
||||
export const logger = pino(pinoOptions);
|
||||
|
||||
// Exported so tests can build a real pino instance (same mixin, a different destination) rather
|
||||
// than mocking the logger itself — see tests/unit/observability/request-context-mixin.test.ts.
|
||||
export const loggerOptions = pinoOptions;
|
||||
|
||||
@@ -9,4 +9,70 @@ export const httpRequestDurationHistogram = new client.Histogram({
|
||||
buckets: [0.01, 0.05, 0.1, 0.3, 0.5, 1, 3, 5],
|
||||
});
|
||||
|
||||
// 014-full-observability data-model.md "Metrics (Prometheus, via prom-client)" — the eleven
|
||||
// named business-health metrics docs/09-testing-observability-cicd.md calls for, each a raw
|
||||
// counter/histogram for an external monitoring stack (FR-009 — no aggregation/dashboard logic
|
||||
// here). "Recurring problems" and "most common errors" are deliberately read directly off
|
||||
// problemsCreatedCounter/knownErrorLookupsCounter via a topk/rate query, not a separate metric.
|
||||
|
||||
export const aiSessionOutcomesCounter = new client.Counter({
|
||||
name: 'supporthub_ai_session_outcomes_total',
|
||||
help: 'Count of AI support sessions by terminal outcome',
|
||||
labelNames: ['outcome'],
|
||||
});
|
||||
|
||||
export const ticketResolutionsCounter = new client.Counter({
|
||||
name: 'supporthub_ticket_resolutions_total',
|
||||
help: 'Count of ticket resolutions by who resolved them',
|
||||
labelNames: ['resolved_by'],
|
||||
});
|
||||
|
||||
export const ticketResolutionDurationHistogram = new client.Histogram({
|
||||
name: 'supporthub_ticket_resolution_duration_seconds',
|
||||
help: 'Duration from ticket creation to resolution, in seconds',
|
||||
buckets: [60, 300, 900, 3600, 14400, 86400, 259200, 604800],
|
||||
});
|
||||
|
||||
export const ticketFirstResponseDurationHistogram = new client.Histogram({
|
||||
name: 'supporthub_ticket_first_response_duration_seconds',
|
||||
help: 'Duration from ticket creation to the first agent response, in seconds',
|
||||
buckets: [60, 300, 900, 3600, 14400, 86400],
|
||||
});
|
||||
|
||||
export const slaRunOutcomesCounter = new client.Counter({
|
||||
name: 'supporthub_sla_run_outcomes_total',
|
||||
help: 'Count of SLA runs by outcome',
|
||||
labelNames: ['outcome'],
|
||||
});
|
||||
|
||||
export const escalationsCounter = new client.Counter({
|
||||
name: 'supporthub_escalations_total',
|
||||
help: 'Count of escalation events by trigger reason',
|
||||
labelNames: ['reason'],
|
||||
});
|
||||
|
||||
export const problemsCreatedCounter = new client.Counter({
|
||||
name: 'supporthub_problems_created_total',
|
||||
help: 'Count of problems created, by category',
|
||||
labelNames: ['category_id'],
|
||||
});
|
||||
|
||||
export const knownErrorLookupsCounter = new client.Counter({
|
||||
name: 'supporthub_known_error_lookups_total',
|
||||
help: 'Count of known-issue lookups by error code',
|
||||
labelNames: ['code'],
|
||||
});
|
||||
|
||||
export const knowledgeRetrievalOutcomesCounter = new client.Counter({
|
||||
name: 'supporthub_knowledge_retrieval_outcomes_total',
|
||||
help: 'Count of AI knowledge-retrieval attempts by whether a match was found',
|
||||
labelNames: ['matched'],
|
||||
});
|
||||
|
||||
export const toolInvocationsCounter = new client.Counter({
|
||||
name: 'supporthub_tool_invocations_total',
|
||||
help: 'Count of AI tool invocations by tool and outcome',
|
||||
labelNames: ['tool', 'outcome'],
|
||||
});
|
||||
|
||||
export const metricsRegistry = client.register;
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
import { AsyncLocalStorage } from 'async_hooks';
|
||||
|
||||
export interface RequestContextSnapshot {
|
||||
requestId: string;
|
||||
correlationId: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* 014-full-observability research.md §2: lets every log line produced through the shared
|
||||
* `logger` singleton — anywhere, any layer, no matter how deep the call stack — automatically
|
||||
* carry the current request's requestId/correlationId (via logger.ts's Pino `mixin`), without
|
||||
* threading `request`/`request.log` through every service and repository.
|
||||
*/
|
||||
export const requestContextStore = new AsyncLocalStorage<RequestContextSnapshot>();
|
||||
|
||||
export function getRequestContextSnapshot(): RequestContextSnapshot | undefined {
|
||||
return requestContextStore.getStore();
|
||||
}
|
||||
@@ -1,5 +1,83 @@
|
||||
import { trace, Tracer } from '@opentelemetry/api';
|
||||
import { trace, context, diag, DiagLogLevel, Tracer } from '@opentelemetry/api';
|
||||
import {
|
||||
BasicTracerProvider,
|
||||
BatchSpanProcessor,
|
||||
SimpleSpanProcessor,
|
||||
ConsoleSpanExporter,
|
||||
InMemorySpanExporter,
|
||||
} from '@opentelemetry/sdk-trace-base';
|
||||
import type { SpanProcessor } from '@opentelemetry/sdk-trace';
|
||||
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
|
||||
import { resourceFromAttributes } from '@opentelemetry/resources';
|
||||
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
|
||||
import { env } from '@/config';
|
||||
import { logger } from './logger';
|
||||
|
||||
/**
|
||||
* Without a registered ContextManager, the OpenTelemetry API's `context.active()` is a no-op
|
||||
* that does not propagate across async boundaries at all — `startActiveSpan` would only make a
|
||||
* span "active" for the literal synchronous extent of its callback, so a child span created
|
||||
* after an `await` (e.g. across this codebase's own event-bus `await eventBus.publish(...)`
|
||||
* chain, data-model.md's whole reason FR-006's two paths work) would silently come out as its
|
||||
* own unrelated root span instead of nesting. This is the tracing equivalent of the ALS-backed
|
||||
* request-context store — same mechanism, different consumer.
|
||||
*/
|
||||
context.setGlobalContextManager(new AsyncLocalStorageContextManager().enable());
|
||||
|
||||
/**
|
||||
* 014-full-observability research.md §4: routes the OpenTelemetry SDK's own internal
|
||||
* diagnostics (span export failures included — FR-007) through this project's own log stream
|
||||
* instead of stderr/nowhere, at WARN so routine SDK chatter isn't logged at every span.
|
||||
*/
|
||||
diag.setLogger(
|
||||
{
|
||||
error: (msg, ...args) => logger.error({ otel: args }, msg),
|
||||
warn: (msg, ...args) => logger.warn({ otel: args }, msg),
|
||||
info: (msg, ...args) => logger.info({ otel: args }, msg),
|
||||
debug: (msg, ...args) => logger.debug({ otel: args }, msg),
|
||||
verbose: (msg, ...args) => logger.trace({ otel: args }, msg),
|
||||
},
|
||||
DiagLogLevel.WARN,
|
||||
);
|
||||
|
||||
let testSpanExporter: InMemorySpanExporter | undefined;
|
||||
|
||||
/**
|
||||
* Real infra, substituted destination only (same pattern as pino-pretty in development, or the
|
||||
* password-reset token's stub delivery) — never a mock of the tracer/provider itself:
|
||||
* - test: `InMemorySpanExporter`, so integration tests can read back real exported spans.
|
||||
* - `OTEL_EXPORTER_OTLP_ENDPOINT` set: real OTLP/HTTP export via `BatchSpanProcessor` (the
|
||||
* exporter reads the same env var itself for the actual collector URL — no need to hand-build
|
||||
* the `/v1/traces` path here).
|
||||
* - otherwise (local dev, or any environment with no collector configured): `ConsoleSpanExporter`
|
||||
* so spans are visible without standing one up.
|
||||
*/
|
||||
function buildSpanProcessor(): SpanProcessor {
|
||||
if (env.NODE_ENV === 'test') {
|
||||
testSpanExporter = new InMemorySpanExporter();
|
||||
return new SimpleSpanProcessor(testSpanExporter);
|
||||
}
|
||||
if (env.OTEL_EXPORTER_OTLP_ENDPOINT) {
|
||||
return new BatchSpanProcessor(new OTLPTraceExporter());
|
||||
}
|
||||
return new SimpleSpanProcessor(new ConsoleSpanExporter());
|
||||
}
|
||||
|
||||
const tracerProvider = new BasicTracerProvider({
|
||||
resource: resourceFromAttributes({ 'service.name': 'supporthub-api' }),
|
||||
spanProcessors: [buildSpanProcessor()],
|
||||
});
|
||||
|
||||
trace.setGlobalTracerProvider(tracerProvider);
|
||||
|
||||
export function getTracer(name = 'supporthub-api'): Tracer {
|
||||
return trace.getTracer(name);
|
||||
}
|
||||
|
||||
/** Test environment only — throws otherwise. See tests/integration/observability/tracing.test.ts. */
|
||||
export function getTestSpanExporter(): InMemorySpanExporter {
|
||||
if (!testSpanExporter) {
|
||||
throw new Error('getTestSpanExporter() is only available when NODE_ENV=test.');
|
||||
}
|
||||
return testSpanExporter;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { ErrorCode, KnownIssue } from '@prisma/client';
|
||||
import { NotFoundError } from '@/common/errors';
|
||||
import { knownErrorLookupsCounter } from '@/infrastructure/observability';
|
||||
import {
|
||||
errorCodesRepository,
|
||||
ErrorCodesRepository,
|
||||
@@ -26,6 +27,11 @@ export class ErrorCodesService {
|
||||
async findKnownIssuesByErrorCode(productId: string, code: string): Promise<KnownIssue[]> {
|
||||
const errorCode = await this.errorCodesRepo.findByCode(productId, code);
|
||||
if (!errorCode) throw new NotFoundError('Error code not found.');
|
||||
|
||||
// 014-full-observability data-model.md #9: "most common errors" — a raw counter, ranked by
|
||||
// an external monitoring stack (FR-009), counted only once the code is confirmed real.
|
||||
knownErrorLookupsCounter.inc({ code });
|
||||
|
||||
return this.knownIssuesRepo.findByErrorCodeId(errorCode.id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { AISupportSession } from '@prisma/client';
|
||||
import { prismaClient } from '@/infrastructure/database';
|
||||
import { aiSessionOutcomesCounter } from '@/infrastructure/observability';
|
||||
import { ACTIVE_SESSION_STATUSES } from '../mapper';
|
||||
|
||||
export class SessionRepository {
|
||||
@@ -38,10 +39,19 @@ export class SessionRepository {
|
||||
async updateStatus(sessionId: string, status: string): Promise<AISupportSession> {
|
||||
const isTerminal =
|
||||
status === 'resolved' || status === 'escalated' || status === 'ended_by_agent';
|
||||
return this.prisma.aISupportSession.update({
|
||||
const updated = await this.prisma.aISupportSession.update({
|
||||
where: { id: sessionId },
|
||||
data: { status, ...(isTerminal ? { endedAt: new Date() } : {}) },
|
||||
});
|
||||
|
||||
// 014-full-observability data-model.md: the single choke point every escalation/resolution
|
||||
// branch in session.service.ts funnels through (research.md §5's "why the repository layer"
|
||||
// — observability calls are already a cross-cutting concern used from any layer here).
|
||||
if (status === 'resolved' || status === 'escalated') {
|
||||
aiSessionOutcomesCounter.inc({ outcome: status });
|
||||
}
|
||||
|
||||
return updated;
|
||||
}
|
||||
|
||||
async setActiveRunbook(sessionId: string, runbookKey: string, stepIndex: number): Promise<void> {
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { SpanStatusCode } from '@opentelemetry/api';
|
||||
import { AISupportSession, KnowledgeEntry, AIInteraction, Ticket, Problem } from '@prisma/client';
|
||||
import { AppError, NotFoundError } from '@/common/errors';
|
||||
import { getTracer } from '@/infrastructure/observability';
|
||||
import { ticketsService, problemsRepository } from '@/modules/ticketing/tickets';
|
||||
import { messagesService } from '@/modules/ticketing/messages';
|
||||
import { knowledgeService } from '@/modules/ai-support/knowledge';
|
||||
@@ -125,17 +127,35 @@ export class SessionsService {
|
||||
reason: string,
|
||||
stepsAttempted: string[] = [],
|
||||
) {
|
||||
const diagnosis = await this.diagnoses.findLatestBySession(session.id);
|
||||
const result = this.escalation.buildSummary(diagnosis, reason, stepsAttempted);
|
||||
// 014-full-observability data-model.md — root span for the AI-escalation -> orchestration/
|
||||
// assignment path (FR-006): syncTicketStatus below publishes TICKET_UPDATED synchronously,
|
||||
// and the orchestration subscriber's own span (src/events/handlers/index.ts) nests under
|
||||
// this one automatically via OTel's active-context propagation through that same await chain.
|
||||
return getTracer().startActiveSpan(
|
||||
'ai.escalation',
|
||||
{ attributes: { 'ticket.id': ticketId, 'session.id': session.id } },
|
||||
async (span) => {
|
||||
try {
|
||||
const diagnosis = await this.diagnoses.findLatestBySession(session.id);
|
||||
const result = this.escalation.buildSummary(diagnosis, reason, stepsAttempted);
|
||||
|
||||
if (!(ACTIVE_SESSION_STATUSES as readonly string[]).includes(session.status)) {
|
||||
return result;
|
||||
}
|
||||
if (!(ACTIVE_SESSION_STATUSES as readonly string[]).includes(session.status)) {
|
||||
return result;
|
||||
}
|
||||
|
||||
await this.sessions.updateStatus(session.id, 'escalated');
|
||||
await syncTicketStatus(ticketId, 'escalated');
|
||||
await this.sessions.updateStatus(session.id, 'escalated');
|
||||
await syncTicketStatus(ticketId, 'escalated');
|
||||
|
||||
return result;
|
||||
return result;
|
||||
} catch (error) {
|
||||
span.recordException(error as Error);
|
||||
span.setStatus({ code: SpanStatusCode.ERROR });
|
||||
throw error;
|
||||
} finally {
|
||||
span.end();
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
private async runDiagnosisTurn(
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import Anthropic from '@anthropic-ai/sdk';
|
||||
import {
|
||||
toolInvocationsCounter,
|
||||
knowledgeRetrievalOutcomesCounter,
|
||||
} from '@/infrastructure/observability';
|
||||
import { actionRepository, ActionRepository } from '../repository';
|
||||
import { evaluateToolProposal } from './policy-gate';
|
||||
import { executeTool, ToolExecutionContext } from './tool-executor';
|
||||
@@ -58,6 +62,15 @@ export class ToolsService {
|
||||
const result = await executeTool(block.name, block.input, context);
|
||||
await this.actions.createResult(action.id, result.output, result.status);
|
||||
|
||||
// 014-full-observability data-model.md #10/#11: the single choke point every tool
|
||||
// invocation passes through — labeled by outcome, and (for the knowledge-search tool
|
||||
// specifically) by whether it found anything.
|
||||
toolInvocationsCounter.inc({ tool: block.name, outcome: result.status });
|
||||
if (block.name === 'searchProductKnowledge') {
|
||||
const matched = Array.isArray(result.output) && result.output.length > 0;
|
||||
knowledgeRetrievalOutcomesCounter.inc({ matched: String(matched) });
|
||||
}
|
||||
|
||||
if (result.status === 'failed') anyFailed = true;
|
||||
if (block.name === 'escalateToHuman' && result.status === 'success') {
|
||||
const output = result.output as { reason?: string };
|
||||
|
||||
@@ -1,16 +1,19 @@
|
||||
import { User } from '@prisma/client';
|
||||
import { ConflictError } from '@/common/errors';
|
||||
import { hashPassword } from '@/modules/identity/auth';
|
||||
import { hashPassword, validatePasswordStrength } from '@/modules/identity/auth';
|
||||
import { usersRepository, UsersRepository } from '../repository';
|
||||
import { CreateUserBody } from '../schema';
|
||||
|
||||
export class UsersService {
|
||||
constructor(private readonly repo: UsersRepository = usersRepository) {}
|
||||
|
||||
/** FR-008: rejects a duplicate email — never a second account silently sharing one. */
|
||||
/** FR-008: rejects a duplicate email — never a second account silently sharing one.
|
||||
* 013-auth-hardening FR-005: the same password-strength policy every password-setting call
|
||||
* site enforces. */
|
||||
async create(body: CreateUserBody): Promise<Omit<User, 'passwordHash'>> {
|
||||
const existing = await this.repo.findByEmail(body.email);
|
||||
if (existing) throw new ConflictError('An account with this email already exists.');
|
||||
validatePasswordStrength(body.password);
|
||||
|
||||
const passwordHash = await hashPassword(body.password);
|
||||
const user = await this.repo.create({
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { FastifyReply, FastifyRequest } from 'fastify';
|
||||
import { AuthenticationError } from '@/common/errors';
|
||||
import { authService, AuthService } from '../service';
|
||||
import { loginSchema } from '../schema';
|
||||
import { loginSchema, requestPasswordResetSchema, resetPasswordSchema } from '../schema';
|
||||
|
||||
function bearerToken(request: FastifyRequest): string {
|
||||
const header = request.headers.authorization;
|
||||
@@ -29,6 +29,26 @@ export class AuthController {
|
||||
await this.service.logout(bearerToken(request));
|
||||
return reply.status(200).send({ success: true, data: { loggedOut: true }, meta: null });
|
||||
}
|
||||
|
||||
/** 013-auth-hardening FR-001/SC-001: identical response regardless of account existence —
|
||||
* the service itself is what decides whether a real token gets issued. */
|
||||
async requestPasswordReset(request: FastifyRequest, reply: FastifyReply) {
|
||||
const { email } = requestPasswordResetSchema.parse(request.body);
|
||||
await this.service.requestPasswordReset(email);
|
||||
return reply.status(200).send({
|
||||
success: true,
|
||||
data: { message: 'If that account exists, a reset link has been sent.' },
|
||||
meta: null,
|
||||
});
|
||||
}
|
||||
|
||||
async resetPassword(request: FastifyRequest, reply: FastifyReply) {
|
||||
const { token, newPassword } = resetPasswordSchema.parse(request.body);
|
||||
await this.service.resetPassword(token, newPassword);
|
||||
return reply
|
||||
.status(200)
|
||||
.send({ success: true, data: { message: 'Password updated.' }, meta: null });
|
||||
}
|
||||
}
|
||||
|
||||
export const authController = new AuthController();
|
||||
|
||||
@@ -4,4 +4,5 @@ export { requireRole } from './service';
|
||||
export type { LoginBody } from './schema';
|
||||
export type { LoginResult } from './service';
|
||||
export { hashPassword, verifyPassword, signToken, verifyToken, toAuthUser } from './mapper';
|
||||
export { validatePasswordStrength } from './mapper';
|
||||
export { AUTH_CONSTANTS } from './constants';
|
||||
|
||||
@@ -1 +1,3 @@
|
||||
export * from './auth.mapper';
|
||||
export * from './password-policy';
|
||||
export * from './reset-token';
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
import { ValidationError } from '@/common/errors';
|
||||
import { authConfig } from '@/config';
|
||||
|
||||
/** 013-auth-hardening FR-005: the one password-strength rule, enforced identically everywhere
|
||||
* a password is ever set (010's own POST /admin/users and this feature's own password-reset
|
||||
* consume endpoint) — never duplicated or allowed to drift between call sites. */
|
||||
export function validatePasswordStrength(password: string): void {
|
||||
if (password.length < authConfig.passwordMinLength) {
|
||||
throw new ValidationError(
|
||||
`Password must be at least ${authConfig.passwordMinLength} characters.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,18 @@
|
||||
import { randomBytes, createHash } from 'crypto';
|
||||
|
||||
export interface GeneratedResetToken {
|
||||
token: string;
|
||||
tokenHash: string;
|
||||
}
|
||||
|
||||
/** 013-auth-hardening: the raw token is what gets "delivered" (logged, per the stub decision,
|
||||
* research.md); only its SHA-256 hash is ever persisted (data-model.md) — mirrors this
|
||||
* codebase's own password-hashing discipline, never storing a usable secret at rest. */
|
||||
export function generateResetToken(): GeneratedResetToken {
|
||||
const token = randomBytes(32).toString('hex');
|
||||
return { token, tokenHash: hashResetToken(token) };
|
||||
}
|
||||
|
||||
export function hashResetToken(token: string): string {
|
||||
return createHash('sha256').update(token).digest('hex');
|
||||
}
|
||||
@@ -14,6 +14,11 @@ export class AuthRepository {
|
||||
if (!user || !user.active) return null;
|
||||
return user;
|
||||
}
|
||||
|
||||
/** 013-auth-hardening: applies a password-reset's new hash. */
|
||||
async updatePassword(id: string, passwordHash: string): Promise<void> {
|
||||
await this.prisma.user.update({ where: { id }, data: { passwordHash } });
|
||||
}
|
||||
}
|
||||
|
||||
export const authRepository = new AuthRepository();
|
||||
|
||||
@@ -1 +1,2 @@
|
||||
export * from './auth.repository';
|
||||
export * from './reset-token.repository';
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
import { cacheService } from '@/infrastructure/cache';
|
||||
|
||||
const TOKEN_KEY_PREFIX = 'password-reset:token:';
|
||||
const USER_KEY_PREFIX = 'password-reset:user:';
|
||||
|
||||
/** 013-auth-hardening data-model.md: two paired Redis keys per active reset token — the same
|
||||
* Redis-key-with-TTL shape as 010's own revocation denylist. Only one active token exists per
|
||||
* user at any time (FR-002): issuing a new one deletes the prior token's own key. */
|
||||
export class ResetTokenRepository {
|
||||
async issue(userId: string, tokenHash: string, ttlSeconds: number): Promise<void> {
|
||||
const priorHash = await cacheService.get<string>(`${USER_KEY_PREFIX}${userId}`);
|
||||
if (priorHash) {
|
||||
await cacheService.del(`${TOKEN_KEY_PREFIX}${priorHash}`);
|
||||
}
|
||||
await cacheService.set(`${TOKEN_KEY_PREFIX}${tokenHash}`, userId, ttlSeconds);
|
||||
await cacheService.set(`${USER_KEY_PREFIX}${userId}`, tokenHash, ttlSeconds);
|
||||
}
|
||||
|
||||
async resolve(tokenHash: string): Promise<string | null> {
|
||||
return cacheService.get<string>(`${TOKEN_KEY_PREFIX}${tokenHash}`);
|
||||
}
|
||||
|
||||
/** Single-use (FR-002/SC-002): deletes both keys for this token/user pair. */
|
||||
async consume(tokenHash: string, userId: string): Promise<void> {
|
||||
await cacheService.del(`${TOKEN_KEY_PREFIX}${tokenHash}`);
|
||||
await cacheService.del(`${USER_KEY_PREFIX}${userId}`);
|
||||
}
|
||||
}
|
||||
|
||||
export const resetTokenRepository = new ResetTokenRepository();
|
||||
@@ -11,4 +11,12 @@ export async function authRoutes(fastify: FastifyInstance): Promise<void> {
|
||||
fastify.post('/auth/logout', { preHandler: fastify.authenticate }, (req, reply) =>
|
||||
authController.handleLogout(req, reply),
|
||||
);
|
||||
|
||||
// 013-auth-hardening: ungated, like login itself — the caller has no session yet.
|
||||
fastify.post('/auth/password-reset/request', (req, reply) =>
|
||||
authController.requestPasswordReset(req, reply),
|
||||
);
|
||||
fastify.post('/auth/password-reset/consume', (req, reply) =>
|
||||
authController.resetPassword(req, reply),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -8,3 +8,21 @@ export const loginSchema = z
|
||||
.strict();
|
||||
|
||||
export type LoginBody = z.infer<typeof loginSchema>;
|
||||
|
||||
/** 013-auth-hardening */
|
||||
export const requestPasswordResetSchema = z
|
||||
.object({
|
||||
email: z.string().email(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export type RequestPasswordResetBody = z.infer<typeof requestPasswordResetSchema>;
|
||||
|
||||
export const resetPasswordSchema = z
|
||||
.object({
|
||||
token: z.string().min(1),
|
||||
newPassword: z.string().min(1),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export type ResetPasswordBody = z.infer<typeof resetPasswordSchema>;
|
||||
|
||||
@@ -1,8 +1,23 @@
|
||||
import { User } from '@prisma/client';
|
||||
import { AuthenticationError } from '@/common/errors';
|
||||
import { revokeToken } from '@/infrastructure/cache';
|
||||
import { authRepository, AuthRepository } from '../repository';
|
||||
import { verifyPassword, signToken, verifyToken } from '../mapper';
|
||||
import { AppError, AuthenticationError, RateLimitError } from '@/common/errors';
|
||||
import { checkRateLimit, revokeToken } from '@/infrastructure/cache';
|
||||
import { logger } from '@/infrastructure/observability';
|
||||
import { authConfig } from '@/config';
|
||||
import {
|
||||
authRepository,
|
||||
AuthRepository,
|
||||
resetTokenRepository,
|
||||
ResetTokenRepository,
|
||||
} from '../repository';
|
||||
import {
|
||||
verifyPassword,
|
||||
signToken,
|
||||
verifyToken,
|
||||
hashPassword,
|
||||
generateResetToken,
|
||||
hashResetToken,
|
||||
validatePasswordStrength,
|
||||
} from '../mapper';
|
||||
import { LoginBody } from '../schema';
|
||||
|
||||
export interface LoginResult {
|
||||
@@ -15,14 +30,29 @@ function toPublicUser(user: User): LoginResult['user'] {
|
||||
}
|
||||
|
||||
export class AuthService {
|
||||
constructor(private readonly repo: AuthRepository = authRepository) {}
|
||||
constructor(
|
||||
private readonly repo: AuthRepository = authRepository,
|
||||
private readonly resetTokens: ResetTokenRepository = resetTokenRepository,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* FR-002/SC-003: every failure branch (no such email, inactive account, wrong password)
|
||||
* throws the identical AuthenticationError — bcrypt.compare always runs exactly once,
|
||||
* against a fixed dummy hash when no user is found, so timing never leaks which branch fired.
|
||||
* 013-auth-hardening FR-006/FR-007: the rate-limit check runs first, before any credential
|
||||
* work — a rate-limited attempt never reaches (and can't distinguish itself via timing from)
|
||||
* the identical-failure-response path below.
|
||||
*/
|
||||
async login(body: LoginBody): Promise<LoginResult> {
|
||||
const rateLimit = await checkRateLimit(
|
||||
`login:${body.email}`,
|
||||
authConfig.loginRateLimitMaxAttempts,
|
||||
authConfig.loginRateLimitWindowSeconds,
|
||||
);
|
||||
if (!rateLimit.allowed) {
|
||||
throw new RateLimitError('Too many login attempts. Try again later.');
|
||||
}
|
||||
|
||||
const user = await this.repo.findByEmail(body.email);
|
||||
const passwordMatches = await verifyPassword(body.password, user?.passwordHash ?? null);
|
||||
|
||||
@@ -46,6 +76,59 @@ export class AuthService {
|
||||
const remainingSeconds = Math.max(1, (payload.exp ?? 0) - Math.floor(Date.now() / 1000));
|
||||
await revokeToken(payload.jti, remainingSeconds);
|
||||
}
|
||||
|
||||
/**
|
||||
* 013-auth-hardening FR-001/SC-001: always resolves the same way regardless of whether the
|
||||
* email corresponds to a real, active account — only issues a real token when it does. The
|
||||
* "delivery" step is a stubbed structured log line (research.md), not a real email.
|
||||
*/
|
||||
async requestPasswordReset(email: string): Promise<void> {
|
||||
const user = await this.repo.findByEmail(email);
|
||||
if (user && user.active) {
|
||||
const { token, tokenHash } = generateResetToken();
|
||||
await this.resetTokens.issue(
|
||||
user.id,
|
||||
tokenHash,
|
||||
authConfig.passwordResetTokenLifetimeMinutes * 60,
|
||||
);
|
||||
logger.info(
|
||||
{
|
||||
event: 'password_reset_requested',
|
||||
userId: user.id,
|
||||
resetUrl: `/reset-password?token=${token}`,
|
||||
},
|
||||
'Password reset requested — stubbed delivery (013-auth-hardening research.md): no real ' +
|
||||
'email is sent yet, this log line is the only place the token is visible.',
|
||||
);
|
||||
}
|
||||
// Same outcome either way (FR-001) — no branch here reveals which case fired.
|
||||
}
|
||||
|
||||
/**
|
||||
* 013-auth-hardening FR-004/FR-005: password strength is checked before the token is even
|
||||
* looked up (data-model.md); the token itself is single-use (SC-002) — resolving and
|
||||
* consuming it happen together so a second attempt with the same token always fails.
|
||||
* Edge Cases: a token issued for an account later deactivated is rejected — reactivation is
|
||||
* 010's own admin domain, not something this flow performs incidentally.
|
||||
*/
|
||||
async resetPassword(token: string, newPassword: string): Promise<void> {
|
||||
validatePasswordStrength(newPassword);
|
||||
|
||||
const tokenHash = hashResetToken(token);
|
||||
const userId = await this.resetTokens.resolve(tokenHash);
|
||||
if (!userId) {
|
||||
throw new AppError('Invalid or expired reset token.', 'INVALID_RESET_TOKEN', 400);
|
||||
}
|
||||
await this.resetTokens.consume(tokenHash, userId);
|
||||
|
||||
const user = await this.repo.findActiveById(userId);
|
||||
if (!user) {
|
||||
throw new AppError('Invalid or expired reset token.', 'INVALID_RESET_TOKEN', 400);
|
||||
}
|
||||
|
||||
const passwordHash = await hashPassword(newPassword);
|
||||
await this.repo.updatePassword(userId, passwordHash);
|
||||
}
|
||||
}
|
||||
|
||||
export const authService = new AuthService();
|
||||
|
||||
@@ -3,6 +3,7 @@ import { NotFoundError, ValidationError } from '@/common/errors';
|
||||
import { ticketsService } from '@/modules/ticketing/tickets';
|
||||
import { messagesService } from '@/modules/ticketing/messages';
|
||||
import { escalationService, EscalationService } from '@/modules/orchestration/escalation';
|
||||
import { slaRunOutcomesCounter } from '@/infrastructure/observability';
|
||||
import {
|
||||
slaPolicyRepository,
|
||||
SlaPolicyRepository,
|
||||
@@ -135,6 +136,14 @@ export class SlaService {
|
||||
const run = await this.runs.findByTicketId(ticketId);
|
||||
if (!run || run.status === 'completed') return;
|
||||
|
||||
// 014-full-observability data-model.md #6: read BEFORE the update below — a run already
|
||||
// 'breached' by the time it resolves was already counted breached by the sweep and must
|
||||
// never also be counted 'met' here, even though this update still (pre-existing behavior,
|
||||
// unrelated to this feature — see research.md §5) overwrites its status to 'completed'.
|
||||
if (run.status !== 'breached') {
|
||||
slaRunOutcomesCounter.inc({ outcome: 'met' });
|
||||
}
|
||||
|
||||
await this.runs.update(run.id, { status: 'completed', completedAt: new Date() });
|
||||
}
|
||||
|
||||
@@ -151,6 +160,7 @@ export class SlaService {
|
||||
const resolutionBreaches = await this.runs.findRunningPastResolutionDueAt(now);
|
||||
for (const run of resolutionBreaches) {
|
||||
await this.runs.update(run.id, { status: 'breached', breachedAt: now });
|
||||
slaRunOutcomesCounter.inc({ outcome: 'breached' });
|
||||
await this.escalation.handleBreach(run.ticketId, 'resolution_breach');
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { TicketMessage } from '@prisma/client';
|
||||
import { ticketsRepository } from '@/modules/ticketing/tickets';
|
||||
import { ticketFirstResponseDurationHistogram } from '@/infrastructure/observability';
|
||||
import { messagesRepository, MessagesRepository } from '../repository';
|
||||
import { MessageType, isVisibleToCustomer } from '../mapper';
|
||||
|
||||
@@ -13,13 +15,32 @@ export class MessagesService {
|
||||
type: MessageType,
|
||||
body: string,
|
||||
): Promise<TicketMessage> {
|
||||
return this.repo.create({
|
||||
// 014-full-observability data-model.md #5: checked BEFORE creating the new message, so it
|
||||
// reflects "is there already an agent response" at the moment this one is being posted.
|
||||
// Benign race (research.md §5/plan.md Constraint) — two concurrent first responses could
|
||||
// both observe once — acceptable for a best-effort metric, not a business-correctness path.
|
||||
const isFirstAgentMessage =
|
||||
type === 'AGENT_MESSAGE' &&
|
||||
!(await this.repo.findAll(ticketId)).some((m) => m.type === 'AGENT_MESSAGE');
|
||||
|
||||
const message = await this.repo.create({
|
||||
ticketId,
|
||||
authorRef,
|
||||
type,
|
||||
body,
|
||||
visibleToCustomer: isVisibleToCustomer(type),
|
||||
});
|
||||
|
||||
if (isFirstAgentMessage) {
|
||||
const ticket = await ticketsRepository.findById(ticketId);
|
||||
if (ticket) {
|
||||
ticketFirstResponseDurationHistogram.observe(
|
||||
(message.createdAt.getTime() - ticket.createdAt.getTime()) / 1000,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return message;
|
||||
}
|
||||
|
||||
async listForCustomer(ticketId: string): Promise<TicketMessage[]> {
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { randomUUID } from 'crypto';
|
||||
import { SpanStatusCode } from '@opentelemetry/api';
|
||||
import { Ticket } from '@prisma/client';
|
||||
import { AppError } from '@/common/errors';
|
||||
import { getTracer, problemsCreatedCounter } from '@/infrastructure/observability';
|
||||
import {
|
||||
ticketsRepository,
|
||||
TicketsRepository,
|
||||
@@ -48,10 +50,31 @@ export class TicketsService {
|
||||
async createFromInboundRequest(
|
||||
input: InboundTicketRequest,
|
||||
): Promise<{ ticket: Ticket; wasExisting: boolean }> {
|
||||
const span = getTracer().startSpan('ticket.create', {
|
||||
attributes: { 'product.externalProductId': input.externalProductId },
|
||||
});
|
||||
try {
|
||||
const result = await this.doCreateFromInboundRequest(input);
|
||||
span.setAttribute('ticket.id', result.ticket.id);
|
||||
return result;
|
||||
} catch (error) {
|
||||
span.recordException(error as Error);
|
||||
span.setStatus({ code: SpanStatusCode.ERROR });
|
||||
throw error;
|
||||
} finally {
|
||||
span.end();
|
||||
}
|
||||
}
|
||||
|
||||
private async doCreateFromInboundRequest(
|
||||
input: InboundTicketRequest,
|
||||
): Promise<{ ticket: Ticket; wasExisting: boolean }> {
|
||||
const existingProblem = input.referenceIds?.length
|
||||
? await this.problemsRepo.findByReference(input.referenceIds)
|
||||
: null;
|
||||
|
||||
const problem =
|
||||
(input.referenceIds?.length
|
||||
? await this.problemsRepo.findByReference(input.referenceIds)
|
||||
: null) ??
|
||||
existingProblem ??
|
||||
(await this.problemsRepo.create({
|
||||
statement: input.problem,
|
||||
symptoms: input.problem,
|
||||
@@ -59,6 +82,14 @@ export class TicketsService {
|
||||
severity: 'medium',
|
||||
}));
|
||||
|
||||
// 014-full-observability: "recurring problems" — a raw counter, ranked/aggregated by an
|
||||
// external monitoring stack (spec.md FR-009/Assumptions), not computed here.
|
||||
if (!existingProblem) {
|
||||
problemsCreatedCounter.inc({
|
||||
category_id: problem.categoryId ?? 'uncategorized',
|
||||
});
|
||||
}
|
||||
|
||||
const year = new Date().getFullYear();
|
||||
const codePrefix = `${deriveProductCode(input.externalProductId)}-${year}-`;
|
||||
let attempt = 0;
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
import { FastifyPluginAsync, FastifyRequest } from 'fastify';
|
||||
import { FastifyPluginAsync, FastifyReply, FastifyRequest } from 'fastify';
|
||||
import fp from 'fastify-plugin';
|
||||
import { generateUuid } from '@/common/utils';
|
||||
import { RequestContext } from '@/common/types';
|
||||
import { APP_CONSTANTS } from '@/common/constants';
|
||||
import {
|
||||
requestContextStore,
|
||||
logger,
|
||||
httpRequestDurationHistogram,
|
||||
} from '@/infrastructure/observability';
|
||||
|
||||
declare module 'fastify' {
|
||||
interface FastifyRequest {
|
||||
@@ -10,8 +15,17 @@ declare module 'fastify' {
|
||||
}
|
||||
}
|
||||
|
||||
function routeLabel(request: FastifyRequest): string {
|
||||
return request.routeOptions?.url ?? 'unmatched';
|
||||
}
|
||||
|
||||
const requestContextPluginCallback: FastifyPluginAsync = async (fastify) => {
|
||||
fastify.addHook('onRequest', async (request: FastifyRequest, reply) => {
|
||||
// Callback-style (not async) so `done` is available to hand to requestContextStore.run —
|
||||
// everything Fastify does next for this request (remaining hooks, the route handler, and this
|
||||
// plugin's own onResponse hook below) runs as a continuation of this call, so it all inherits
|
||||
// the ALS context (research.md §2/§1 — Node's AsyncLocalStorage propagates through a
|
||||
// continuation chain, not just the literal synchronous call).
|
||||
fastify.addHook('onRequest', (request: FastifyRequest, reply: FastifyReply, done) => {
|
||||
const rawReqId = request.headers[APP_CONSTANTS.REQUEST_ID_HEADER];
|
||||
const rawCorrId = request.headers[APP_CONSTANTS.CORRELATION_HEADER];
|
||||
|
||||
@@ -25,6 +39,42 @@ const requestContextPluginCallback: FastifyPluginAsync = async (fastify) => {
|
||||
|
||||
reply.header(APP_CONSTANTS.REQUEST_ID_HEADER, requestId);
|
||||
reply.header(APP_CONSTANTS.CORRELATION_HEADER, correlationId);
|
||||
|
||||
requestContextStore.run({ requestId, correlationId }, done);
|
||||
});
|
||||
|
||||
// 014-full-observability FR-001/FR-003: fires for every completed response, including 404s
|
||||
// and replies sent early by another hook (e.g. rate limiting) — nothing is silently unlogged.
|
||||
fastify.addHook('onResponse', async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
const method = request.method;
|
||||
const route = routeLabel(request);
|
||||
const statusCode = reply.statusCode;
|
||||
const durationSeconds = reply.elapsedTime / 1000;
|
||||
|
||||
httpRequestDurationHistogram.observe(
|
||||
{ method, route, status_code: String(statusCode) },
|
||||
durationSeconds,
|
||||
);
|
||||
|
||||
const logPayload = {
|
||||
event: 'http_request_completed',
|
||||
method,
|
||||
route,
|
||||
statusCode,
|
||||
durationMs: reply.elapsedTime,
|
||||
// Explicit here (not left to the mixin alone), matching this codebase's existing
|
||||
// convention (app.ts's error handler already does the same) — the access log is the one
|
||||
// line an operator most needs to grep by requestId without knowing about mixin internals.
|
||||
requestId: request.reqContext?.requestId,
|
||||
correlationId: request.reqContext?.correlationId,
|
||||
};
|
||||
if (statusCode >= 500) {
|
||||
logger.error(logPayload, `${method} ${route} ${statusCode}`);
|
||||
} else if (statusCode >= 400) {
|
||||
logger.warn(logPayload, `${method} ${route} ${statusCode}`);
|
||||
} else {
|
||||
logger.info(logPayload, `${method} ${route} ${statusCode}`);
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
|
||||
+12
-8
@@ -1,3 +1,4 @@
|
||||
import { randomUUID } from 'crypto';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
import bcrypt from 'bcryptjs';
|
||||
import { prismaClient } from '@/infrastructure/database';
|
||||
@@ -6,20 +7,23 @@ const TEST_PASSWORD = 'Test-Password-123!';
|
||||
|
||||
/**
|
||||
* 010-identity-auth made fastify.authenticate real — every test file calling a route already
|
||||
* gated by it (across 002-009's own suites) needs a real session now. Rather than depend on
|
||||
* gated by it (across 002-009's own suites) needs a real session now. This creates its own
|
||||
* throwaway admin/agent account directly and logs in as it, so callers don't depend on
|
||||
* prisma/seed/roles.seed.ts having already been run against whatever database the suite
|
||||
* connects to, this upserts its own throwaway admin/agent account directly (idempotent — safe
|
||||
* to call from many test files' own beforeAll against the same database) and logs in as it.
|
||||
* connects to.
|
||||
*
|
||||
* 013-auth-hardening: the email is unique per call (not a fixed `test-admin@...` shared across
|
||||
* every integration test file) because login is now rate-limited per email — dozens of files
|
||||
* each calling this once in their own beforeAll would otherwise share one rate-limit bucket and
|
||||
* trip it well before any file's own tests get to run.
|
||||
*/
|
||||
export async function loginAs(
|
||||
app: FastifyInstance,
|
||||
role: 'ADMIN' | 'AGENT' = 'ADMIN',
|
||||
): Promise<string> {
|
||||
const email = `test-${role.toLowerCase()}@supporthub.test`;
|
||||
await prismaClient.user.upsert({
|
||||
where: { email },
|
||||
update: {},
|
||||
create: {
|
||||
const email = `test-${role.toLowerCase()}-${randomUUID()}@supporthub.test`;
|
||||
await prismaClient.user.create({
|
||||
data: {
|
||||
email,
|
||||
name: `Test ${role}`,
|
||||
role,
|
||||
|
||||
@@ -166,6 +166,10 @@ describe('Agent ticket queue (User Stories 1-2)', () => {
|
||||
await prismaClient.hierarchyNode.deleteMany({ where: { name: 'ATQ Node' } });
|
||||
await prismaClient.agentSkill.deleteMany({ where: { agentId: { in: [agentXId, agentYId] } } });
|
||||
await prismaClient.ticketMessage.deleteMany({ where: ticketFilter });
|
||||
// A wildcard (non-product-scoped) SLA policy from another concurrently-running suite (e.g.
|
||||
// sla-escalation-flow.test.ts's own "Global policy") can match these tickets too, leaving a
|
||||
// real sla_run row that would otherwise RESTRICT this delete.
|
||||
await prismaClient.sLARun.deleteMany({ where: ticketFilter });
|
||||
await prismaClient.ticket.deleteMany({ where: { id: { in: createdTicketIds } } });
|
||||
await prismaClient.problem.deleteMany({ where: { productId } });
|
||||
await prismaClient.agent.deleteMany({ where: { teamId } });
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { buildApp } from '@/app';
|
||||
import { prismaClient } from '@/infrastructure/database';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
import bcrypt from 'bcryptjs';
|
||||
import { authConfig } from '@/config';
|
||||
|
||||
/**
|
||||
* Covers specs/013-auth-hardening/quickstart.md Scenario 3 against a real Postgres/Redis.
|
||||
*/
|
||||
describe('Login rate limiting (User Story 3)', () => {
|
||||
let app: FastifyInstance;
|
||||
const suffix = Date.now();
|
||||
const email = `rate-limit-test-${suffix}@supporthub.test`;
|
||||
const otherEmail = `rate-limit-other-${suffix}@supporthub.test`;
|
||||
const correctPassword = 'Correct-Password-1!';
|
||||
let userId: string;
|
||||
let otherUserId: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
app = await buildApp();
|
||||
const user = await prismaClient.user.create({
|
||||
data: {
|
||||
email,
|
||||
name: 'Rate Limit Test User',
|
||||
role: 'AGENT',
|
||||
passwordHash: await bcrypt.hash(correctPassword, 10),
|
||||
},
|
||||
});
|
||||
userId = user.id;
|
||||
|
||||
const otherUser = await prismaClient.user.create({
|
||||
data: {
|
||||
email: otherEmail,
|
||||
name: 'Rate Limit Other User',
|
||||
role: 'AGENT',
|
||||
passwordHash: await bcrypt.hash(correctPassword, 10),
|
||||
},
|
||||
});
|
||||
otherUserId = otherUser.id;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await prismaClient.user.deleteMany({ where: { id: { in: [userId, otherUserId] } } });
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('blocks the same email after its attempt budget is exhausted, without affecting other emails', async () => {
|
||||
for (let i = 0; i < authConfig.loginRateLimitMaxAttempts; i++) {
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email, password: 'definitely-wrong' },
|
||||
});
|
||||
expect(res.statusCode).toBe(401);
|
||||
}
|
||||
|
||||
// One more attempt for the same email, this time with the CORRECT password — still 429.
|
||||
const blockedRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email, password: correctPassword },
|
||||
});
|
||||
expect(blockedRes.statusCode).toBe(429);
|
||||
|
||||
// A different email in the same window is unaffected.
|
||||
const otherRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email: otherEmail, password: correctPassword },
|
||||
});
|
||||
expect(otherRes.statusCode).toBe(200);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,75 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, vi, MockInstance } from 'vitest';
|
||||
import { buildApp } from '@/app';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
import { logger } from '@/infrastructure/observability';
|
||||
|
||||
/** Covers specs/014-full-observability/quickstart.md Scenario 1 against a real running app. */
|
||||
describe('Per-request access log (User Story 1)', () => {
|
||||
let app: FastifyInstance;
|
||||
|
||||
beforeAll(async () => {
|
||||
app = await buildApp();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await app.close();
|
||||
});
|
||||
|
||||
function callsData(spy: MockInstance): Record<string, unknown>[] {
|
||||
return spy.mock.calls.map(([data]: unknown[]) => data as Record<string, unknown>);
|
||||
}
|
||||
|
||||
function accessLogCalls(spy: MockInstance): Record<string, unknown>[] {
|
||||
return callsData(spy).filter((data) => data?.event === 'http_request_completed');
|
||||
}
|
||||
|
||||
it('emits exactly one access-log line for a successful request, carrying a requestId', async () => {
|
||||
const infoSpy = vi.spyOn(logger, 'info');
|
||||
|
||||
const res = await app.inject({ method: 'GET', url: '/health/live' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
|
||||
const lines = accessLogCalls(infoSpy);
|
||||
expect(lines).toHaveLength(1);
|
||||
expect(lines[0]).toMatchObject({ method: 'GET', route: '/health/live', statusCode: 200 });
|
||||
expect(lines[0]?.requestId).toBeTruthy();
|
||||
expect(lines[0]?.requestId).toBe(res.headers['x-request-id']);
|
||||
|
||||
infoSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('correlates the access-log line with other log lines produced for the same request', async () => {
|
||||
const warnSpy = vi.spyOn(logger, 'warn');
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email: `nobody-${Date.now()}@supporthub.test`, password: 'wrong' },
|
||||
});
|
||||
expect(res.statusCode).toBe(401);
|
||||
|
||||
const accessLine = accessLogCalls(warnSpy)[0];
|
||||
expect(accessLine).toBeDefined();
|
||||
|
||||
const authFailureLine = callsData(warnSpy).find((data) => data?.code === 'UNAUTHORIZED');
|
||||
expect(authFailureLine).toBeDefined();
|
||||
|
||||
expect(authFailureLine?.requestId).toBe(accessLine?.requestId);
|
||||
expect(accessLine?.requestId).toBe(res.headers['x-request-id']);
|
||||
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('still emits an access-log line for a 404', async () => {
|
||||
const warnSpy = vi.spyOn(logger, 'warn');
|
||||
|
||||
const res = await app.inject({ method: 'GET', url: '/this-route-does-not-exist' });
|
||||
expect(res.statusCode).toBe(404);
|
||||
|
||||
const lines = accessLogCalls(warnSpy);
|
||||
expect(lines).toHaveLength(1);
|
||||
expect(lines[0]).toMatchObject({ statusCode: 404 });
|
||||
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,346 @@
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { buildApp } from '@/app';
|
||||
import { prismaClient } from '@/infrastructure/database';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
import { loginAs, authHeader } from '../../helpers/auth';
|
||||
import {
|
||||
encryptCredential,
|
||||
generateCredentialSecret,
|
||||
issueIntegrationToken,
|
||||
} from '@/modules/catalog/products';
|
||||
import { sessionsService, sessionRepository } from '@/modules/ai-support/sessions';
|
||||
import { ticketsService, ticketsRepository } from '@/modules/ticketing/tickets';
|
||||
import { messagesService } from '@/modules/ticketing/messages';
|
||||
import { resolutionRepository } from '@/modules/problem-management/resolutions';
|
||||
import { slaService, slaRunRepository } from '@/modules/orchestration/sla';
|
||||
import { escalationService } from '@/modules/orchestration/escalation';
|
||||
import { errorCodesService } from '@/modules/ai-support/knowledge';
|
||||
import { toolsService } from '@/modules/ai-support/tools';
|
||||
|
||||
/**
|
||||
* Covers specs/014-full-observability/quickstart.md Scenario 4 against a real Postgres/Redis —
|
||||
* every named business-health metric, scraped from the real /metrics endpoint before and after
|
||||
* driving its real underlying event through the real service layer (not mocked). Several flows
|
||||
* (human resolution, SLA runs) create rows directly against the repositories that already own
|
||||
* the relevant validation elsewhere in this codebase's own test suite — this file's job is only
|
||||
* to prove the metric increments at the correct point, not to re-verify those modules' own
|
||||
* business rules (already covered by problem-resolution-flow.test.ts / sla-escalation-flow.test.ts).
|
||||
*/
|
||||
describe('Business-health metrics (User Story 4)', () => {
|
||||
let app: FastifyInstance;
|
||||
let authToken: string;
|
||||
const externalProductId = `TEST_BIZ_METRICS_PROD_${Date.now()}`;
|
||||
let productId: string;
|
||||
let secret: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
app = await buildApp();
|
||||
authToken = await loginAs(app, 'ADMIN');
|
||||
|
||||
const product = await prismaClient.product.create({
|
||||
data: { externalProductId, name: 'Business Metrics Test Product', status: 'active' },
|
||||
});
|
||||
productId = product.id;
|
||||
secret = generateCredentialSecret();
|
||||
await prismaClient.productIntegration.create({
|
||||
data: {
|
||||
productId,
|
||||
credentialRef: encryptCredential(secret),
|
||||
authMechanism: 'signed_token',
|
||||
allowedScope: { tenantIds: ['tenant-1'] },
|
||||
status: 'active',
|
||||
rateLimitPerMinute: 1000,
|
||||
rateLimitPerUserPerMinute: 1000,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await app.close();
|
||||
});
|
||||
|
||||
async function createTicket(): Promise<string> {
|
||||
const token = issueIntegrationToken(secret, {
|
||||
externalProductId,
|
||||
tenantId: 'tenant-1',
|
||||
userId: 'user-1',
|
||||
});
|
||||
const created = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/v1/support/requests',
|
||||
headers: { authorization: `Bearer ${token}` },
|
||||
payload: {
|
||||
productId: externalProductId,
|
||||
tenantId: 'tenant-1',
|
||||
userId: 'user-1',
|
||||
source: 'test',
|
||||
problem: `Business metrics test ${Date.now()}-${Math.random()}`,
|
||||
},
|
||||
});
|
||||
expect(created.statusCode).toBe(202);
|
||||
return created.json().data.ticketId as string;
|
||||
}
|
||||
|
||||
async function scrape(): Promise<string> {
|
||||
const res = await app.inject({ method: 'GET', url: '/metrics' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
return res.body;
|
||||
}
|
||||
|
||||
function metricValue(body: string, name: string, labels?: Record<string, string>): number {
|
||||
const labelPart = labels
|
||||
? `\\{${Object.entries(labels)
|
||||
.map(([k, v]) => `${k}="${v}"`)
|
||||
.join(',')}\\}`
|
||||
: '(?:\\{\\})?';
|
||||
const match = body.match(new RegExp(`${name}${labelPart}\\s+([0-9.]+)`));
|
||||
return match?.[1] ? parseFloat(match[1]) : 0;
|
||||
}
|
||||
|
||||
it('counts an AI session resolving without escalating', async () => {
|
||||
const ticketId = await createTicket();
|
||||
const session = await sessionRepository.create(ticketId);
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_ai_session_outcomes_total', {
|
||||
outcome: 'resolved',
|
||||
});
|
||||
await sessionRepository.updateStatus(session.id, 'resolved');
|
||||
const after = metricValue(await scrape(), 'supporthub_ai_session_outcomes_total', {
|
||||
outcome: 'resolved',
|
||||
});
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts an AI session escalating, and nothing for resolved', async () => {
|
||||
const ticketId = await createTicket();
|
||||
const session = await sessionRepository.create(ticketId);
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_ai_session_outcomes_total', {
|
||||
outcome: 'escalated',
|
||||
});
|
||||
await sessionsService.escalate(session, ticketId, 'Escalating for metrics test.');
|
||||
const after = metricValue(await scrape(), 'supporthub_ai_session_outcomes_total', {
|
||||
outcome: 'escalated',
|
||||
});
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts the first agent message on a ticket, observing first-response duration', async () => {
|
||||
const ticketId = await createTicket();
|
||||
|
||||
const bodyBefore = await scrape();
|
||||
const before = metricValue(
|
||||
bodyBefore,
|
||||
'supporthub_ticket_first_response_duration_seconds_count',
|
||||
);
|
||||
|
||||
await messagesService.post(ticketId, 'agent-1', 'AGENT_MESSAGE', 'Hi, looking into this.');
|
||||
// A second agent message must NOT observe again.
|
||||
await messagesService.post(ticketId, 'agent-1', 'AGENT_MESSAGE', 'Following up.');
|
||||
|
||||
const after = metricValue(
|
||||
await scrape(),
|
||||
'supporthub_ticket_first_response_duration_seconds_count',
|
||||
);
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts a human resolution and observes resolution duration when a ticket reaches RESOLVED', async () => {
|
||||
const ticketId = await createTicket();
|
||||
|
||||
// Reach RESOLUTION_PENDING_CUSTOMER via the repository directly (bypassing
|
||||
// ticketsService.updateStatus's domain-event publish) — this test only cares about the
|
||||
// final RESOLVED transition and the Resolution row's own resolvedBy, not the intermediate
|
||||
// states, and going through the real event bus here would trigger a REAL, unscoped
|
||||
// HUMAN_ESCALATION auto-assignment against the default strategy — which can land on some
|
||||
// other concurrently-running test file's own dedicated agent (a real cross-file
|
||||
// contamination this test caused once, fixed here by not publishing those events at all).
|
||||
let ticket = await prismaClient.ticket.findUniqueOrThrow({ where: { id: ticketId } });
|
||||
for (const status of ['HUMAN_ESCALATION', 'IN_PROGRESS', 'RESOLUTION_PENDING_CUSTOMER']) {
|
||||
const updated = await ticketsRepository.updateStatus(ticketId, status, ticket.version);
|
||||
if (!updated) throw new Error(`Failed to transition ticket to ${status} in test setup.`);
|
||||
ticket = updated;
|
||||
}
|
||||
await resolutionRepository.create({ ticketId, outcome: 'fixed', resolvedBy: 'agent-1' });
|
||||
|
||||
const bodyBefore = await scrape();
|
||||
const resolvedBefore = metricValue(bodyBefore, 'supporthub_ticket_resolutions_total', {
|
||||
resolved_by: 'human',
|
||||
});
|
||||
const durationBefore = metricValue(
|
||||
bodyBefore,
|
||||
'supporthub_ticket_resolution_duration_seconds_count',
|
||||
);
|
||||
|
||||
await ticketsService.updateStatus(ticketId, 'RESOLVED', ticket.version, 'agent-1');
|
||||
|
||||
const bodyAfter = await scrape();
|
||||
expect(
|
||||
metricValue(bodyAfter, 'supporthub_ticket_resolutions_total', { resolved_by: 'human' }),
|
||||
).toBe(resolvedBefore + 1);
|
||||
expect(metricValue(bodyAfter, 'supporthub_ticket_resolution_duration_seconds_count')).toBe(
|
||||
durationBefore + 1,
|
||||
);
|
||||
});
|
||||
|
||||
it('counts an SLA run completing on time as met', async () => {
|
||||
const policy = await prismaClient.sLAPolicy.create({
|
||||
data: {
|
||||
name: `Metrics Policy ${Date.now()}`,
|
||||
productId,
|
||||
firstResponseMinutes: 30,
|
||||
resolutionMinutes: 240,
|
||||
},
|
||||
});
|
||||
const ticketId = await createTicket();
|
||||
await slaRunRepository.create({
|
||||
ticketId,
|
||||
policyId: policy.id,
|
||||
firstResponseDueAt: new Date(Date.now() + 30 * 60_000),
|
||||
resolutionDueAt: new Date(Date.now() + 240 * 60_000),
|
||||
});
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_sla_run_outcomes_total', {
|
||||
outcome: 'met',
|
||||
});
|
||||
await slaService.complete(ticketId);
|
||||
const after = metricValue(await scrape(), 'supporthub_sla_run_outcomes_total', {
|
||||
outcome: 'met',
|
||||
});
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts an overdue SLA run as breached via the sweep (at least once — a shared sweep may also catch unrelated overdue runs)', async () => {
|
||||
const policy = await prismaClient.sLAPolicy.create({
|
||||
data: {
|
||||
name: `Metrics Breach Policy ${Date.now()}`,
|
||||
productId,
|
||||
firstResponseMinutes: 30,
|
||||
resolutionMinutes: 1,
|
||||
},
|
||||
});
|
||||
const ticketId = await createTicket();
|
||||
await slaRunRepository.create({
|
||||
ticketId,
|
||||
policyId: policy.id,
|
||||
firstResponseDueAt: new Date(Date.now() + 30 * 60_000),
|
||||
resolutionDueAt: new Date(Date.now() - 60_000),
|
||||
});
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_sla_run_outcomes_total', {
|
||||
outcome: 'breached',
|
||||
});
|
||||
await slaService.runBreachDetectionSweep();
|
||||
const after = metricValue(await scrape(), 'supporthub_sla_run_outcomes_total', {
|
||||
outcome: 'breached',
|
||||
});
|
||||
|
||||
expect(after).toBeGreaterThanOrEqual(before + 1);
|
||||
});
|
||||
|
||||
it('counts a manual escalation event by reason', async () => {
|
||||
const node = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/admin/hierarchy-nodes',
|
||||
headers: authHeader(authToken),
|
||||
payload: {
|
||||
name: `Metrics Node ${Date.now()}`,
|
||||
order: 0,
|
||||
productScope: [externalProductId],
|
||||
skills: [],
|
||||
assignmentStrategy: 'ROUND_ROBIN',
|
||||
},
|
||||
});
|
||||
const nodeId = node.json().data.id as string;
|
||||
const ticketId = await createTicket();
|
||||
const reason = `metrics-test-reason-${Date.now()}`;
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_escalations_total', { reason });
|
||||
await escalationService.escalateManually(ticketId, nodeId, 'admin-test', reason);
|
||||
const after = metricValue(await scrape(), 'supporthub_escalations_total', { reason });
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts a problem created, labeled by category (uncategorized here)', async () => {
|
||||
const before = metricValue(await scrape(), 'supporthub_problems_created_total', {
|
||||
category_id: 'uncategorized',
|
||||
});
|
||||
await createTicket();
|
||||
const after = metricValue(await scrape(), 'supporthub_problems_created_total', {
|
||||
category_id: 'uncategorized',
|
||||
});
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts a knowledge-search tool call that finds nothing as unmatched', async () => {
|
||||
const ticketId = await createTicket();
|
||||
const session = await sessionRepository.create(ticketId);
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_knowledge_retrieval_outcomes_total', {
|
||||
matched: 'false',
|
||||
});
|
||||
await toolsService.proposeAndEvaluate(
|
||||
session.id,
|
||||
[
|
||||
{
|
||||
type: 'tool_use',
|
||||
caller: { type: 'direct' },
|
||||
id: `toolu_${Date.now()}`,
|
||||
name: 'searchProductKnowledge',
|
||||
input: { feature: `nonexistent-feature-${Date.now()}` },
|
||||
},
|
||||
],
|
||||
{ ticketId, productId },
|
||||
);
|
||||
const after = metricValue(await scrape(), 'supporthub_knowledge_retrieval_outcomes_total', {
|
||||
matched: 'false',
|
||||
});
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts a tool invocation that fails at execution time', async () => {
|
||||
const ticketId = await createTicket();
|
||||
const session = await sessionRepository.create(ticketId);
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_tool_invocations_total', {
|
||||
tool: 'getTicketSnapshot',
|
||||
outcome: 'failed',
|
||||
});
|
||||
await toolsService.proposeAndEvaluate(
|
||||
session.id,
|
||||
[
|
||||
{
|
||||
type: 'tool_use',
|
||||
caller: { type: 'direct' },
|
||||
id: `toolu_${Date.now()}`,
|
||||
name: 'getTicketSnapshot',
|
||||
input: {},
|
||||
},
|
||||
],
|
||||
{ ticketId: 'nonexistent-ticket-id', productId },
|
||||
);
|
||||
const after = metricValue(await scrape(), 'supporthub_tool_invocations_total', {
|
||||
tool: 'getTicketSnapshot',
|
||||
outcome: 'failed',
|
||||
});
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
|
||||
it('counts a valid known-error-code lookup', async () => {
|
||||
const code = `METRICS-ERR-${Date.now()}`;
|
||||
await errorCodesService.createErrorCode(productId, code, 'A test error for metrics.');
|
||||
|
||||
const before = metricValue(await scrape(), 'supporthub_known_error_lookups_total', { code });
|
||||
await errorCodesService.findKnownIssuesByErrorCode(productId, code);
|
||||
const after = metricValue(await scrape(), 'supporthub_known_error_lookups_total', { code });
|
||||
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,40 @@
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { buildApp } from '@/app';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
|
||||
/** Covers specs/014-full-observability/quickstart.md Scenario 2 against a real running app. */
|
||||
describe('Live request-health metrics (User Story 2)', () => {
|
||||
let app: FastifyInstance;
|
||||
|
||||
beforeAll(async () => {
|
||||
app = await buildApp();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('records request-duration observations labeled by method/route/status for both success and failure', async () => {
|
||||
const email = `metrics-test-${Date.now()}@supporthub.test`;
|
||||
await app.inject({ method: 'POST', url: '/auth/login', payload: { email, password: 'wrong' } });
|
||||
await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email, password: 'wrong2' },
|
||||
});
|
||||
|
||||
const metricsRes = await app.inject({ method: 'GET', url: '/metrics' });
|
||||
expect(metricsRes.statusCode).toBe(200);
|
||||
|
||||
expect(metricsRes.body).toMatch(
|
||||
/supporthub_http_request_duration_seconds_count\{method="POST",route="\/auth\/login",status_code="401"\}\s+\d+/,
|
||||
);
|
||||
|
||||
// A second scrape's body reflects the first scrape's own request too — proves 2xx routes
|
||||
// are observed just as 4xx ones are, not only error paths.
|
||||
const secondScrape = await app.inject({ method: 'GET', url: '/metrics' });
|
||||
expect(secondScrape.body).toMatch(
|
||||
/supporthub_http_request_duration_seconds_count\{method="GET",route="\/metrics",status_code="200"\}\s+\d+/,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,101 @@
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { buildApp } from '@/app';
|
||||
import { prismaClient } from '@/infrastructure/database';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
import {
|
||||
encryptCredential,
|
||||
generateCredentialSecret,
|
||||
issueIntegrationToken,
|
||||
} from '@/modules/catalog/products';
|
||||
import { sessionsService, sessionRepository } from '@/modules/ai-support/sessions';
|
||||
import { getTestSpanExporter } from '@/infrastructure/observability';
|
||||
|
||||
/**
|
||||
* Covers specs/014-full-observability/quickstart.md Scenario 3, against a real running app.
|
||||
* Reads spans back from getTestSpanExporter() (a real TracerProvider, real spans — only the
|
||||
* export *destination* is swapped for an in-memory one, per research.md §4) rather than through
|
||||
* an external collector.
|
||||
*
|
||||
* Drives the escalation path directly via sessionsService.escalate(...) instead of through a
|
||||
* real AI reasoning turn — the tracing behavior under test (span creation/nesting) is identical
|
||||
* either way, and this avoids requiring a paid ANTHROPIC_API_KEY for every test run (see
|
||||
* ai-verification-and-escalation.test.ts's own `describe.skipIf(!hasRealApiKey)` for the
|
||||
* alternative this project already uses when a real reasoning call is actually required).
|
||||
*/
|
||||
describe('Cross-module trace (User Story 3)', () => {
|
||||
let app: FastifyInstance;
|
||||
|
||||
beforeAll(async () => {
|
||||
app = await buildApp();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await app.close();
|
||||
});
|
||||
|
||||
async function createTicketViaInboundRequest(): Promise<string> {
|
||||
const externalProductId = `TEST_TRACE_PROD_${Date.now()}_${Math.random().toString(36).slice(2)}`;
|
||||
const product = await prismaClient.product.create({
|
||||
data: { externalProductId, name: 'Tracing Test Product', status: 'active' },
|
||||
});
|
||||
const secret = generateCredentialSecret();
|
||||
await prismaClient.productIntegration.create({
|
||||
data: {
|
||||
productId: product.id,
|
||||
credentialRef: encryptCredential(secret),
|
||||
authMechanism: 'signed_token',
|
||||
allowedScope: { tenantIds: ['tenant-1'] },
|
||||
status: 'active',
|
||||
rateLimitPerMinute: 1000,
|
||||
rateLimitPerUserPerMinute: 1000,
|
||||
},
|
||||
});
|
||||
const token = issueIntegrationToken(secret, {
|
||||
externalProductId,
|
||||
tenantId: 'tenant-1',
|
||||
userId: 'user-1',
|
||||
});
|
||||
const created = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/v1/support/requests',
|
||||
headers: { authorization: `Bearer ${token}` },
|
||||
payload: {
|
||||
productId: externalProductId,
|
||||
tenantId: 'tenant-1',
|
||||
userId: 'user-1',
|
||||
source: 'test',
|
||||
problem: 'Needs a human, for tracing.',
|
||||
},
|
||||
});
|
||||
expect(created.statusCode).toBe(202);
|
||||
return created.json().data.ticketId as string;
|
||||
}
|
||||
|
||||
it('produces a ticket.create span for ticket intake', async () => {
|
||||
getTestSpanExporter().reset();
|
||||
|
||||
await createTicketViaInboundRequest();
|
||||
|
||||
const spans = getTestSpanExporter().getFinishedSpans();
|
||||
const createSpan = spans.find((s) => s.name === 'ticket.create');
|
||||
expect(createSpan).toBeDefined();
|
||||
expect(createSpan?.attributes['ticket.id']).toBeTruthy();
|
||||
});
|
||||
|
||||
it('nests orchestration.assignment under ai.escalation, sharing one trace', async () => {
|
||||
getTestSpanExporter().reset();
|
||||
|
||||
const ticketId = await createTicketViaInboundRequest();
|
||||
const session = await sessionRepository.create(ticketId);
|
||||
await sessionsService.escalate(session, ticketId, 'Escalating for tracing test.');
|
||||
|
||||
const spans = getTestSpanExporter().getFinishedSpans();
|
||||
const escalationSpan = spans.find((s) => s.name === 'ai.escalation');
|
||||
const assignmentSpan = spans.find((s) => s.name === 'orchestration.assignment');
|
||||
|
||||
expect(escalationSpan).toBeDefined();
|
||||
expect(assignmentSpan).toBeDefined();
|
||||
expect(assignmentSpan?.spanContext().traceId).toBe(escalationSpan?.spanContext().traceId);
|
||||
expect(assignmentSpan?.parentSpanContext?.spanId).toBe(escalationSpan?.spanContext().spanId);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,113 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
|
||||
import { buildApp } from '@/app';
|
||||
import { prismaClient } from '@/infrastructure/database';
|
||||
import { FastifyInstance } from 'fastify';
|
||||
import bcrypt from 'bcryptjs';
|
||||
import { logger } from '@/infrastructure/observability';
|
||||
|
||||
/**
|
||||
* Covers specs/013-auth-hardening/quickstart.md Scenario 1 against a real Postgres/Redis — the
|
||||
* full request -> (read the token from the stub's own log line) -> consume -> login-with-new-
|
||||
* password flow, and the identical-response-regardless-of-existing-account behavior.
|
||||
*/
|
||||
describe('Password reset flow (User Story 1)', () => {
|
||||
let app: FastifyInstance;
|
||||
const suffix = Date.now();
|
||||
const email = `reset-test-${suffix}@supporthub.test`;
|
||||
const originalPassword = 'Original-Password-1!';
|
||||
const newPassword = 'Brand-New-Password-2!';
|
||||
let userId: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
app = await buildApp();
|
||||
const user = await prismaClient.user.create({
|
||||
data: {
|
||||
email,
|
||||
name: 'Reset Test User',
|
||||
role: 'AGENT',
|
||||
passwordHash: await bcrypt.hash(originalPassword, 10),
|
||||
},
|
||||
});
|
||||
userId = user.id;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await prismaClient.user.deleteMany({ where: { id: userId } });
|
||||
await app.close();
|
||||
});
|
||||
|
||||
function extractLoggedToken(): string {
|
||||
const infoSpy = vi.mocked(logger.info);
|
||||
const call = infoSpy.mock.calls.find(
|
||||
([data]) => (data as { event?: string }).event === 'password_reset_requested',
|
||||
);
|
||||
if (!call) throw new Error('Expected a password_reset_requested log line, but none was found.');
|
||||
|
||||
const resetUrl = (call[0] as unknown as { resetUrl: string }).resetUrl;
|
||||
const token = new URL(resetUrl, 'http://localhost').searchParams.get('token');
|
||||
if (!token) throw new Error('Expected the logged resetUrl to carry a token query param.');
|
||||
return token;
|
||||
}
|
||||
|
||||
it('Scenario 1: request -> stub-logged token -> consume -> login with the new password', async () => {
|
||||
const infoSpy = vi.spyOn(logger, 'info');
|
||||
|
||||
const requestRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/password-reset/request',
|
||||
payload: { email },
|
||||
});
|
||||
expect(requestRes.statusCode).toBe(200);
|
||||
expect(requestRes.json().data.message).not.toMatch(/token|[a-f0-9]{64}/i);
|
||||
|
||||
const nonexistentRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/password-reset/request',
|
||||
payload: { email: `nobody-${suffix}@supporthub.test` },
|
||||
});
|
||||
expect(nonexistentRes.statusCode).toBe(200);
|
||||
expect(nonexistentRes.json()).toEqual(requestRes.json());
|
||||
|
||||
const token = extractLoggedToken();
|
||||
|
||||
const consumeRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/password-reset/consume',
|
||||
payload: { token, newPassword },
|
||||
});
|
||||
expect(consumeRes.statusCode).toBe(200);
|
||||
|
||||
// Single-use — the same token fails a second time.
|
||||
const secondConsumeRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/password-reset/consume',
|
||||
payload: { token, newPassword: 'Another-Password-3!' },
|
||||
});
|
||||
expect(secondConsumeRes.statusCode).toBe(400);
|
||||
|
||||
const loginWithNew = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email, password: newPassword },
|
||||
});
|
||||
expect(loginWithNew.statusCode).toBe(200);
|
||||
|
||||
const loginWithOld = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/login',
|
||||
payload: { email, password: originalPassword },
|
||||
});
|
||||
expect(loginWithOld.statusCode).toBe(401);
|
||||
|
||||
infoSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('rejects an invalid token outright', async () => {
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/auth/password-reset/consume',
|
||||
payload: { token: 'not-a-real-token', newPassword: 'Whatever-Password-1!' },
|
||||
});
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import bcrypt from 'bcryptjs';
|
||||
import { AuthService } from '@/modules/identity/auth/service/auth.service';
|
||||
import * as cache from '@/infrastructure/cache';
|
||||
|
||||
const REAL_PASSWORD_HASH = bcrypt.hashSync('the-real-password', 10);
|
||||
|
||||
@@ -19,6 +20,10 @@ function fakeUser(overrides: Partial<Record<string, unknown>> = {}) {
|
||||
}
|
||||
|
||||
describe('AuthService.login failure parity', () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(cache, 'checkRateLimit').mockResolvedValue({ allowed: true, count: 1 });
|
||||
});
|
||||
|
||||
it('throws the identical error for a nonexistent email and a wrong password', async () => {
|
||||
const repoFoundUser = { findByEmail: vi.fn().mockResolvedValue(fakeUser()) } as never;
|
||||
const repoNoUser = { findByEmail: vi.fn().mockResolvedValue(null) } as never;
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { AuthService } from '@/modules/identity/auth/service/auth.service';
|
||||
import * as cache from '@/infrastructure/cache';
|
||||
|
||||
describe('AuthService.login rate-limit ordering (User Story 3)', () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('checks the rate limit before ever looking up the account', async () => {
|
||||
const findByEmail = vi.fn().mockResolvedValue(null);
|
||||
const repo = { findByEmail } as never;
|
||||
const service = new AuthService(repo);
|
||||
|
||||
vi.spyOn(cache, 'checkRateLimit').mockResolvedValue({ allowed: false, count: 6 });
|
||||
|
||||
await expect(
|
||||
service.login({ email: 'agent@example.com', password: 'anything' }),
|
||||
).rejects.toMatchObject({ statusCode: 429 });
|
||||
|
||||
expect(findByEmail).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('proceeds to credential checks once the rate limit allows the attempt', async () => {
|
||||
const findByEmail = vi.fn().mockResolvedValue(null);
|
||||
const repo = { findByEmail } as never;
|
||||
const service = new AuthService(repo);
|
||||
|
||||
vi.spyOn(cache, 'checkRateLimit').mockResolvedValue({ allowed: true, count: 1 });
|
||||
|
||||
await expect(
|
||||
service.login({ email: 'agent@example.com', password: 'anything' }),
|
||||
).rejects.toMatchObject({ statusCode: 401 });
|
||||
|
||||
expect(findByEmail).toHaveBeenCalledWith('agent@example.com');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,17 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { validatePasswordStrength } from '@/modules/identity/auth/mapper/password-policy';
|
||||
import { authConfig } from '@/config';
|
||||
|
||||
describe('validatePasswordStrength', () => {
|
||||
it('rejects a password shorter than the configured minimum, naming the actual requirement', () => {
|
||||
const tooShort = 'a'.repeat(authConfig.passwordMinLength - 1);
|
||||
expect(() => validatePasswordStrength(tooShort)).toThrowError(
|
||||
`Password must be at least ${authConfig.passwordMinLength} characters.`,
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts a password meeting the configured minimum', () => {
|
||||
const meetsPolicy = 'a'.repeat(authConfig.passwordMinLength);
|
||||
expect(() => validatePasswordStrength(meetsPolicy)).not.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,55 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { Writable } from 'stream';
|
||||
import pino from 'pino';
|
||||
import { loggerOptions } from '@/infrastructure/observability/logger';
|
||||
import { requestContextStore } from '@/infrastructure/observability/request-context.store';
|
||||
|
||||
function buildCapturingLogger() {
|
||||
const lines: Record<string, unknown>[] = [];
|
||||
const sink = new Writable({
|
||||
write(chunk, _enc, callback) {
|
||||
lines.push(JSON.parse(chunk.toString()));
|
||||
callback();
|
||||
},
|
||||
});
|
||||
// Same options (same mixin) the real singleton uses — only the destination differs, per
|
||||
// logger.ts's own comment on why loggerOptions is exported. `transport` is never set outside
|
||||
// NODE_ENV=development (see logger.ts), so it's always undefined under `npm test`.
|
||||
const testLogger = pino(loggerOptions, sink);
|
||||
return { testLogger, lines };
|
||||
}
|
||||
|
||||
describe('logger mixin (014-full-observability FR-002)', () => {
|
||||
it('attaches requestId/correlationId to a log line made inside the request context store', () => {
|
||||
const { testLogger, lines } = buildCapturingLogger();
|
||||
|
||||
requestContextStore.run({ requestId: 'req-1', correlationId: 'corr-1' }, () => {
|
||||
testLogger.info('inside request');
|
||||
});
|
||||
|
||||
expect(lines).toHaveLength(1);
|
||||
expect(lines[0]).toMatchObject({ requestId: 'req-1', correlationId: 'corr-1' });
|
||||
});
|
||||
|
||||
it('attaches neither field to a log line made outside any request context', () => {
|
||||
const { testLogger, lines } = buildCapturingLogger();
|
||||
|
||||
testLogger.info('outside any request');
|
||||
|
||||
expect(lines).toHaveLength(1);
|
||||
expect(lines[0]?.requestId).toBeUndefined();
|
||||
expect(lines[0]?.correlationId).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not leak one request context into a log line logged after that context ends', () => {
|
||||
const { testLogger, lines } = buildCapturingLogger();
|
||||
|
||||
requestContextStore.run({ requestId: 'req-2', correlationId: 'corr-2' }, () => {
|
||||
testLogger.info('inside');
|
||||
});
|
||||
testLogger.info('after');
|
||||
|
||||
expect(lines[0]).toMatchObject({ requestId: 'req-2' });
|
||||
expect(lines[1]?.requestId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,58 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { SlaService } from '@/modules/orchestration/sla/service/sla.service';
|
||||
import * as observability from '@/infrastructure/observability';
|
||||
|
||||
function fakeRun(overrides: Partial<Record<string, unknown>> = {}) {
|
||||
return {
|
||||
id: 'run-1',
|
||||
ticketId: 'ticket-1',
|
||||
status: 'running',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('SLA compliance metric (014-full-observability data-model.md #6)', () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('counts a run that completes while still running as met', async () => {
|
||||
const incSpy = vi.spyOn(observability.slaRunOutcomesCounter, 'inc');
|
||||
const runs = {
|
||||
findByTicketId: vi.fn().mockResolvedValue(fakeRun({ status: 'running' })),
|
||||
update: vi.fn().mockResolvedValue(undefined),
|
||||
} as never;
|
||||
const service = new SlaService(undefined, runs);
|
||||
|
||||
await service.complete('ticket-1');
|
||||
|
||||
expect(incSpy).toHaveBeenCalledWith({ outcome: 'met' });
|
||||
});
|
||||
|
||||
it('does not double-count a run that was already breached before it resolved', async () => {
|
||||
const incSpy = vi.spyOn(observability.slaRunOutcomesCounter, 'inc');
|
||||
const runs = {
|
||||
findByTicketId: vi.fn().mockResolvedValue(fakeRun({ status: 'breached' })),
|
||||
update: vi.fn().mockResolvedValue(undefined),
|
||||
} as never;
|
||||
const service = new SlaService(undefined, runs);
|
||||
|
||||
await service.complete('ticket-1');
|
||||
|
||||
expect(incSpy).not.toHaveBeenCalledWith({ outcome: 'met' });
|
||||
});
|
||||
|
||||
it('does not count anything for a run already completed', async () => {
|
||||
const incSpy = vi.spyOn(observability.slaRunOutcomesCounter, 'inc');
|
||||
const runs = {
|
||||
findByTicketId: vi.fn().mockResolvedValue(fakeRun({ status: 'completed' })),
|
||||
update: vi.fn().mockResolvedValue(undefined),
|
||||
} as never;
|
||||
const service = new SlaService(undefined, runs);
|
||||
|
||||
await service.complete('ticket-1');
|
||||
|
||||
expect(incSpy).not.toHaveBeenCalled();
|
||||
expect((runs as unknown as { update: ReturnType<typeof vi.fn> }).update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,54 @@
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import { BasicTracerProvider, BatchSpanProcessor } from '@opentelemetry/sdk-trace-base';
|
||||
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
|
||||
|
||||
/**
|
||||
* 014-full-observability FR-007/Quickstart Scenario 3 steps 3-4: an unreachable tracing
|
||||
* collector must never surface as an application-level failure. This exercises the real
|
||||
* OpenTelemetry SDK's actual background export path (BasicTracerProvider + BatchSpanProcessor +
|
||||
* a real OTLPTraceExporter, against a real, deliberately-unreachable address — not a mock) the
|
||||
* same way it runs in production: a span ends, the processor's own internal timer schedules the
|
||||
* export, and a failed export is caught by the SDK's own error handler — never left as an
|
||||
* unhandled rejection that could crash the process.
|
||||
*
|
||||
* (Deliberately does NOT call provider.forceFlush() to prove this — forceFlush() is documented
|
||||
* OpenTelemetry SDK behavior that *does* reject on a failed export, by design, so a caller that
|
||||
* explicitly asks "did my flush succeed?" can find out. This feature's own code never calls
|
||||
* forceFlush() on the request-handling path, only the SDK's own background timer does, which is
|
||||
* what this test exercises instead.)
|
||||
*/
|
||||
describe('Tracing graceful degradation', () => {
|
||||
let unhandledRejection: unknown;
|
||||
const onUnhandledRejection = (reason: unknown) => {
|
||||
unhandledRejection = reason;
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
process.removeListener('unhandledRejection', onUnhandledRejection);
|
||||
});
|
||||
|
||||
it('does not produce an unhandled rejection when the background export to an unreachable endpoint fails', async () => {
|
||||
unhandledRejection = undefined;
|
||||
process.on('unhandledRejection', onUnhandledRejection);
|
||||
|
||||
const exporter = new OTLPTraceExporter({
|
||||
url: 'http://127.0.0.1:1/v1/traces', // port 1 — nothing listens there
|
||||
timeoutMillis: 500,
|
||||
});
|
||||
const provider = new BasicTracerProvider({
|
||||
spanProcessors: [
|
||||
new BatchSpanProcessor(exporter, { scheduledDelayMillis: 10, exportTimeoutMillis: 500 }),
|
||||
],
|
||||
});
|
||||
|
||||
const span = provider.getTracer('test').startSpan('unreachable-export-test');
|
||||
span.end(); // triggers the processor's own internal timer, not forceFlush()
|
||||
|
||||
// Long enough for the internal timer (10ms) + the failed connection attempt to resolve.
|
||||
await new Promise((resolve) => setTimeout(resolve, 1500));
|
||||
|
||||
expect(unhandledRejection).toBeUndefined();
|
||||
|
||||
await provider.shutdown();
|
||||
}, 10000);
|
||||
});
|
||||
Reference in New Issue
Block a user