docs: plan and design artifacts for ticketing feature
/speckit-plan output for 003-ticketing: technical context and constitution gate check (all PASS), Phase 0 research (9 decisions: ticket code format, explicit 12-state lifecycle transition table, optimistic concurrency via version column, idempotency-key upsert reusing 002's CustomerReference pattern, explicit-reference-only recurring-problem linking, config-driven message visibility mapping, presigned-PUT attachment pipeline, a fail-closed placeholder malware scanner since none exists in this stack, and adding MinIO to Docker Compose for local/test S3-compatible storage), Phase 1 data model (Ticket/Problem/TicketMessage/TicketAttachment plus the inbound request -> ticket creation behavior), the lifecycle/messages/ attachments contract, and a 6-scenario quickstart. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
be2eb9907a
commit
4751cf3164
@@ -0,0 +1,58 @@
|
|||||||
|
# Contract: Ticket Lifecycle, Messages & Attachments
|
||||||
|
|
||||||
|
## Ticket creation (via the inbound trust boundary)
|
||||||
|
|
||||||
|
`POST /v1/support/requests` (002-saas-integration) now, after successful auth:
|
||||||
|
|
||||||
|
1. Resolve/create `Problem` (research.md's explicit-reference-only rule).
|
||||||
|
2. Atomic create-or-fetch `Ticket` on `(productId, idempotencyKey)` — a retried request returns
|
||||||
|
the same ticket, never a second one (FR-004/SC-002).
|
||||||
|
3. Write a `SYSTEM_EVENT` message.
|
||||||
|
4. Respond `202` with `{ ticketId, code, status, problemId }`.
|
||||||
|
|
||||||
|
**Guarantee**: no request that passes the trust boundary ever completes without a ticket existing
|
||||||
|
(FR-001/SC-001) — ticket creation is synchronous within the same request, not queued.
|
||||||
|
|
||||||
|
## Ticket status transitions
|
||||||
|
|
||||||
|
`PATCH /tickets/:ticketId/status` — body `{ status: <new status>, expectedVersion: <int> }`.
|
||||||
|
|
||||||
|
| Step | Failure |
|
||||||
|
|---|---|
|
||||||
|
| Ticket exists and caller is tenant-authorized | `404` / `403` |
|
||||||
|
| `expectedVersion` matches the ticket's current `version` | `409 CONFLICT` (FR-007/SC-007) — caller must re-read and retry |
|
||||||
|
| Requested transition is a valid edge from the current status (research.md's table) | `400 INVALID_TRANSITION` |
|
||||||
|
|
||||||
|
On success: `status` and `version` (+1) update atomically; a `SYSTEM_EVENT` message records the
|
||||||
|
transition.
|
||||||
|
|
||||||
|
## Messages
|
||||||
|
|
||||||
|
- `POST /tickets/:ticketId/messages` — body `{ type, body }` (`authorRef`/`visibleToCustomer`
|
||||||
|
derived server-side, never accepted as input — FR-008).
|
||||||
|
- `GET /tickets/:ticketId/messages` — the caller's scope (customer vs. agent/admin) determines
|
||||||
|
which types are queried; a customer-scoped caller's query never includes
|
||||||
|
`visibleToCustomer: false` rows (FR-009) — enforced in the repository's `WHERE` clause, not by
|
||||||
|
filtering an already-fetched list.
|
||||||
|
|
||||||
|
## Attachments
|
||||||
|
|
||||||
|
1. `POST /tickets/:ticketId/attachments/upload-url` — body `{ fileName, mimeType, sizeBytes }`,
|
||||||
|
validated against configured limits (FR-012) before a presigned PUT URL is returned. No
|
||||||
|
`TicketAttachment` row exists yet at this point.
|
||||||
|
2. Caller PUTs the file directly to the returned URL (file bytes never transit this API).
|
||||||
|
3. `POST /tickets/:ticketId/attachments/:attachmentId/confirm` — creates the `TicketAttachment`
|
||||||
|
row (`scanStatus: pending`) and enqueues the scan job on `attachments-queue`.
|
||||||
|
4. `GET /tickets/:ticketId/attachments/:attachmentId/download-url` — returns a presigned GET URL
|
||||||
|
only if `scanStatus == 'clean'`; otherwise `409` with the current scan status (FR-013/FR-014).
|
||||||
|
|
||||||
|
## Guarantees (callable contract)
|
||||||
|
|
||||||
|
1. **Ticket existence is synchronous with trust-boundary success** — never eventually-consistent.
|
||||||
|
2. **Idempotency key reuse never creates a second ticket**, regardless of retry count (SC-002).
|
||||||
|
3. **No internal-only message type is ever returned to a customer-scoped read**, verified per
|
||||||
|
type (SC-003).
|
||||||
|
4. **No attachment file byte ever reaches PostgreSQL** — only `storageKey` metadata (SC-004).
|
||||||
|
5. **No attachment is downloadable before `scanStatus: clean`**, every time it's attempted
|
||||||
|
(SC-005).
|
||||||
|
6. **A concurrent, stale-version status update is rejected, never silently overwritten** (SC-007).
|
||||||
@@ -0,0 +1,100 @@
|
|||||||
|
# Phase 1 Data Model: Ticket Creation, Messages & Attachments
|
||||||
|
|
||||||
|
All new models use `cuid()` ids, matching the convention established in 002-saas-integration.
|
||||||
|
Relations to not-yet-existing models (AI sessions, assignments, SLA runs, escalation events,
|
||||||
|
investigations, root causes, solutions — all later phases) are deliberately omitted for now and
|
||||||
|
added when those phases introduce the models they'd point to; Prisma can't reference a model that
|
||||||
|
doesn't exist.
|
||||||
|
|
||||||
|
## Ticket
|
||||||
|
|
||||||
|
| Field | Type | Notes |
|
||||||
|
|---|---|---|
|
||||||
|
| id | String @id @default(cuid()) | |
|
||||||
|
| code | String @unique | `<PRODUCT_CODE>-<YEAR>-<SEQUENCE>` (research.md) |
|
||||||
|
| productId | String | FK → `Product` |
|
||||||
|
| problemId | String | FK → `Problem` |
|
||||||
|
| customerId | String | FK → `CustomerReference` (from 002-saas-integration) — the normalized reference |
|
||||||
|
| externalUserId | String | Denormalized copy, matches doc 06's literal shape for query convenience without a join |
|
||||||
|
| externalTenantId | String | Denormalized copy, same rationale |
|
||||||
|
| status | String | One of the 12 states in research.md's state machine; `@default("NEW")` |
|
||||||
|
| priority | String | Free-text for now — `PriorityPolicy`-driven derivation is Phase 6/orchestration, not this feature |
|
||||||
|
| severity | String | |
|
||||||
|
| categoryId | String? | FK → existing `Category` model (already in schema from the original scaffold) |
|
||||||
|
| idempotencyKey | String? | Research.md's idempotency mechanism |
|
||||||
|
| version | Int @default(1) | Optimistic concurrency (research.md) |
|
||||||
|
| createdAt / updatedAt | DateTime | |
|
||||||
|
|
||||||
|
**Constraints**: `@@unique([productId, idempotencyKey])` (nullable-excluded — two tickets with
|
||||||
|
`idempotencyKey: null` don't conflict). Index on `(productId, status)` and
|
||||||
|
`(externalTenantId, externalUserId)` for the query patterns FR-015 requires (tenant/user-scoped
|
||||||
|
lookups).
|
||||||
|
|
||||||
|
**Status transition rule**: enforced entirely in the service layer against the explicit adjacency
|
||||||
|
table in research.md — the column itself has no DB-level CHECK constraint beyond "is a known
|
||||||
|
string," since Prisma doesn't model state machines natively and a CHECK constraint would need to
|
||||||
|
be duplicated in code anyway for the "attempted from X" half of transition validation.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
| Field | Type | Notes |
|
||||||
|
|---|---|---|
|
||||||
|
| id | String @id @default(cuid()) | |
|
||||||
|
| statement | String | |
|
||||||
|
| symptoms | String | |
|
||||||
|
| impact | String? | |
|
||||||
|
| productId | String | FK → `Product` |
|
||||||
|
| categoryId | String? | FK → existing `Category` model |
|
||||||
|
| severity | String | |
|
||||||
|
| customerImpact | String? | |
|
||||||
|
| businessImpact | String? | |
|
||||||
|
| environment | String? | |
|
||||||
|
| createdAt | DateTime @default(now()) | |
|
||||||
|
|
||||||
|
**Relations added by this feature**: `tickets Ticket[]` (1 problem : many tickets, Constitution
|
||||||
|
Principle VIII).
|
||||||
|
|
||||||
|
## TicketMessage
|
||||||
|
|
||||||
|
| Field | Type | Notes |
|
||||||
|
|---|---|---|
|
||||||
|
| id | String @id @default(cuid()) | |
|
||||||
|
| ticketId | String | FK → `Ticket` |
|
||||||
|
| type | String | `CUSTOMER_MESSAGE` \| `AI_MESSAGE` \| `AGENT_MESSAGE` \| `INTERNAL_NOTE` \| `SYSTEM_EVENT` \| `INVESTIGATION_NOTE` \| `SOLUTION_NOTE` |
|
||||||
|
| authorRef | String | `agentId`, `"ai"`, `"system"`, or `externalUserId` — never a local FK (Constitution Principle I) |
|
||||||
|
| body | String | |
|
||||||
|
| visibleToCustomer | Boolean | Set from the type→visibility map at write time (research.md) — **never** accepted as request input |
|
||||||
|
| createdAt | DateTime @default(now()) | |
|
||||||
|
|
||||||
|
**Index**: `(ticketId, visibleToCustomer, createdAt)` — the exact shape customer-scoped reads
|
||||||
|
query on (FR-009).
|
||||||
|
|
||||||
|
## TicketAttachment
|
||||||
|
|
||||||
|
| Field | Type | Notes |
|
||||||
|
|---|---|---|
|
||||||
|
| id | String @id @default(cuid()) | |
|
||||||
|
| ticketId | String | FK → `Ticket` |
|
||||||
|
| storageKey | String | S3/MinIO object key — never the file itself (FR-011) |
|
||||||
|
| fileName | String | Original filename, for display only |
|
||||||
|
| mimeType | String | Validated against an allow-list at upload-confirm time (FR-012) |
|
||||||
|
| sizeBytes | Int | Validated against a configured max at upload-confirm time |
|
||||||
|
| scanStatus | String | `pending` \| `clean` \| `infected` \| `rejected` (research.md's `MalwareScanner`) |
|
||||||
|
| uploadedBy | String | `agentId` or `externalUserId` — same non-FK convention as `TicketMessage.authorRef` |
|
||||||
|
| createdAt | DateTime @default(now()) | |
|
||||||
|
|
||||||
|
**Download rule**: a presigned GET URL is generated only when `scanStatus == 'clean'` — enforced
|
||||||
|
in the service layer before calling `storageService.getPresignedUrl`, never left to the caller to
|
||||||
|
check first (FR-013/FR-014).
|
||||||
|
|
||||||
|
## Inbound Request → Ticket Creation (behavior, not a new table)
|
||||||
|
|
||||||
|
Extends 002-saas-integration's inbound flow. After `authenticateProductIntegration` succeeds and
|
||||||
|
populates `request.reqContext`, the route handler (previously a stub echoing context back) now:
|
||||||
|
|
||||||
|
1. Resolves or creates a `Problem` (research.md's explicit-reference-only linking).
|
||||||
|
2. Atomically creates (or, on idempotency-key conflict, fetches) the `Ticket` in `NEW` status.
|
||||||
|
3. Writes a `SYSTEM_EVENT` `TicketMessage` recording the creation (Constitution Principle VI —
|
||||||
|
durable audit trail via the message timeline itself).
|
||||||
|
4. Returns the ticket's `id`/`code`/`status` to the caller (still `202`, now backed by a real
|
||||||
|
record instead of an echo).
|
||||||
@@ -0,0 +1,139 @@
|
|||||||
|
# Implementation Plan: Ticket Creation, Messages & Attachments
|
||||||
|
|
||||||
|
**Branch**: `003-ticketing` | **Date**: 2026-09-02 | **Spec**: [spec.md](./spec.md)
|
||||||
|
|
||||||
|
**Input**: Feature specification from `specs/003-ticketing/spec.md`
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Add the `Ticket`/`Problem`/`TicketMessage`/`TicketAttachment` domain: creating a ticket (and its
|
||||||
|
problem) immediately from a validated inbound request (wiring into 002-saas-integration's
|
||||||
|
`POST /v1/support/requests`, which today only echoes trusted context back), a typed message
|
||||||
|
timeline with enforced internal-note privacy, and an attachment pipeline built on the
|
||||||
|
already-scaffolded S3-compatible `storageService` plus a new async malware-scan job on the
|
||||||
|
already-scaffolded `attachments-queue`. Idempotency-key enforcement (deferred from
|
||||||
|
002-saas-integration's FR-012) is implemented here since `Ticket` now exists.
|
||||||
|
|
||||||
|
## Technical Context
|
||||||
|
|
||||||
|
**Language/Version**: TypeScript 5.4 / Node.js 20+.
|
||||||
|
|
||||||
|
**Primary Dependencies**: Prisma (new models), BullMQ (new attachment-scan job, using the
|
||||||
|
existing `attachments-queue` and `QueueManager`), `@aws-sdk/client-s3` +
|
||||||
|
`@aws-sdk/s3-request-presigner` (already wired via `storageService` — no new dependency), Zod.
|
||||||
|
|
||||||
|
**Storage**: PostgreSQL via Prisma (new `Ticket`, `Problem`, `TicketMessage`, `TicketAttachment`
|
||||||
|
models per `docs/06-database-schema.md`) + S3-compatible object storage (AWS S3 in
|
||||||
|
test/production, MinIO locally — per the user's confirmed choice, matching doc 04's explicit
|
||||||
|
guidance and the existing `storageConfig`/`storageService` scaffold).
|
||||||
|
|
||||||
|
**Testing**: Vitest — unit tests for the state-machine transition table and message-visibility
|
||||||
|
mapping; integration tests for ticket creation (incl. idempotency and recurring-problem linking),
|
||||||
|
message read-scoping, and the attachment upload → scan → download flow, against a real
|
||||||
|
Postgres/Redis/S3-compatible target (MinIO via `docker-compose.test.yml`, extended by this
|
||||||
|
feature — see research.md).
|
||||||
|
|
||||||
|
**Target Platform**: Same Fastify modular monolith. Extends the already-scaffolded
|
||||||
|
`src/modules/ticketing/{tickets,messages,attachments}` modules (currently: `tickets` has a bare
|
||||||
|
`GET /tickets` stub; `messages`/`attachments` are unimplemented skeletons).
|
||||||
|
|
||||||
|
**Project Type**: Backend service — single project.
|
||||||
|
|
||||||
|
**Performance Goals**: Ticket creation (FR-001) must complete synchronously within the inbound
|
||||||
|
request's own response — no async/eventual-consistency gap between "request accepted" and
|
||||||
|
"ticket exists" (this is the whole point of Constitution Principle "ticket created immediately").
|
||||||
|
Malware scanning is explicitly asynchronous (SC-005 only requires it's enforced *before
|
||||||
|
download*, not before upload completes).
|
||||||
|
|
||||||
|
**Constraints**: MUST NOT store attachment file bytes in PostgreSQL (FR-011); MUST NOT make an
|
||||||
|
attachment downloadable before its scan clears (FR-013); ticket status updates MUST use
|
||||||
|
optimistic concurrency (FR-007); every ticket/message/attachment query MUST be tenant-scoped
|
||||||
|
(FR-015).
|
||||||
|
|
||||||
|
**Scale/Scope**: One inbound-flow change (002-saas-integration's stub handler becomes real ticket
|
||||||
|
creation), full CRUD-ish surface for messages or a customer/agent to read, and an attachment
|
||||||
|
upload/download surface. Explicitly excludes investigation/root-cause/solution/resolution
|
||||||
|
(Phase 9) and AI diagnosis (Phase 4) per 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 | Ticket/message/attachment queries are scoped by the already-validated `reqContext` (productId/customerId/tenantId) from 002-saas-integration's trust boundary — never a caller-supplied id alone (FR-015). | PASS |
|
||||||
|
| II. Configuration Over Hardcoding | Message-type visibility mapping, attachment size/type limits, and scan-status gate are all defined as data/config, not scattered conditionals — see data-model.md. | PASS |
|
||||||
|
| III. Layered Architecture With Enforced Module Boundaries | Extends the existing `tickets`/`messages`/`attachments` modules through their own controller/service/repository layers and public `index.ts`; the scan job lives in `src/jobs/attachments/` per the existing scaffold, calling the `attachments` module's repository through its public API only. | PASS |
|
||||||
|
| IV. AI Recommends, Deterministic Policy Decides | Not applicable — no AI in this feature (explicitly deferred to Phase 4). | PASS — N/A |
|
||||||
|
| V. Evidence-Based Verification | Not applicable — resolution/verification is Phase 9. | PASS — N/A |
|
||||||
|
| VI. Durable Audit & History | Ticket status transitions and attachment scan-status changes are written as `SYSTEM_EVENT` ticket messages (visible per FR-008's type-driven visibility), giving a durable, queryable history without a separate audit mechanism for this feature's own state changes. | PASS |
|
||||||
|
| VII. Concurrency-Safe, Durable Job Handling | Ticket status updates use optimistic concurrency (a `version` column, FR-007); the malware-scan job is idempotent (re-running it for an already-scanned attachment is a no-op); idempotency-key enforcement (FR-004) reuses the same atomic-upsert pattern as 002-saas-integration's `CustomerReference.findOrCreate`. | PASS |
|
||||||
|
| VIII. Problem and Ticket Are Separate, Related Entities | Directly implements this principle (FR-003) — this is the feature that first creates both entities. | PASS |
|
||||||
|
| Technology & Platform Constraints | Uses only already-present dependencies (Prisma, BullMQ, AWS SDK, Zod) — no new runtime dependency. | PASS |
|
||||||
|
|
||||||
|
No violations requiring Complexity Tracking justification.
|
||||||
|
|
||||||
|
## Post-Design Constitution Re-check
|
||||||
|
|
||||||
|
All gates above remain PASS after Phase 1 design. One design detail worth calling out: the
|
||||||
|
malware scanner (research.md) fails closed (defaults to `infected`, never `clean`) precisely
|
||||||
|
because Principle V's spirit ("evidence-based, not assumed") applies here even though this
|
||||||
|
feature's own scope is pre-Phase-9 — an attachment pipeline that silently marked everything
|
||||||
|
"clean" would be asserting a safety property with no evidence behind it.
|
||||||
|
|
||||||
|
## Project Structure
|
||||||
|
|
||||||
|
### Documentation (this feature)
|
||||||
|
|
||||||
|
```text
|
||||||
|
specs/003-ticketing/
|
||||||
|
├── plan.md # This file
|
||||||
|
├── research.md # Phase 0 output
|
||||||
|
├── data-model.md # Phase 1 output
|
||||||
|
├── quickstart.md # Phase 1 output
|
||||||
|
├── contracts/ # Phase 1 output
|
||||||
|
└── tasks.md # Phase 2 output (/speckit-tasks — not created here)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Source Code (repository root)
|
||||||
|
|
||||||
|
```text
|
||||||
|
supporthub-api/
|
||||||
|
├── prisma/
|
||||||
|
│ └── schema.prisma # MODIFIED — add Ticket, Problem, TicketMessage,
|
||||||
|
│ TicketAttachment models per docs/06
|
||||||
|
├── src/
|
||||||
|
│ ├── modules/
|
||||||
|
│ │ ├── catalog/products/
|
||||||
|
│ │ │ └── routes/inbound-request.routes.ts # MODIFIED — actually create a ticket instead
|
||||||
|
│ │ │ of echoing context back
|
||||||
|
│ │ └── ticketing/
|
||||||
|
│ │ ├── tickets/ # EXTENDED (existing scaffold) — create/get/
|
||||||
|
│ │ │ ├── controller/ list/updateStatus, state machine, idempotency
|
||||||
|
│ │ │ ├── service/
|
||||||
|
│ │ │ ├── repository/
|
||||||
|
│ │ │ ├── routes/
|
||||||
|
│ │ │ ├── schema/
|
||||||
|
│ │ │ ├── mapper/
|
||||||
|
│ │ │ └── types/
|
||||||
|
│ │ ├── messages/ # EXTENDED (existing scaffold) — post/list,
|
||||||
|
│ │ │ └── ... visibility-scoped reads
|
||||||
|
│ │ └── attachments/ # EXTENDED (existing scaffold) — upload
|
||||||
|
│ │ └── ... (presign + confirm), scan-gated download
|
||||||
|
│ └── jobs/
|
||||||
|
│ └── attachments/ # EXTENDED (existing scaffold) — real malware
|
||||||
|
│ └── index.ts scan worker (pluggable scanner, see research.md)
|
||||||
|
└── tests/
|
||||||
|
├── unit/ticketing/ # state machine, visibility mapping
|
||||||
|
└── integration/ # creation/idempotency, messages, attachments
|
||||||
|
```
|
||||||
|
|
||||||
|
**Structure Decision**: Single project, extending the three already-scaffolded `ticketing`
|
||||||
|
submodules rather than restructuring them — `docs/07-backend-architecture.md`'s module layout
|
||||||
|
already anticipated exactly this shape. The inbound-request handler from 002-saas-integration is
|
||||||
|
modified in place (it's the one integration point between "request trusted" and "ticket exists"),
|
||||||
|
not duplicated.
|
||||||
|
|
||||||
|
## Complexity Tracking
|
||||||
|
|
||||||
|
*No constitution violations — table intentionally omitted.*
|
||||||
@@ -0,0 +1,56 @@
|
|||||||
|
# Quickstart: Validating Ticket Creation, Messages & Attachments
|
||||||
|
|
||||||
|
Prerequisites: 002-saas-integration's inbound trust boundary working (a seeded/registered
|
||||||
|
`ProductIntegration`), MinIO running locally (research.md), migrations applied.
|
||||||
|
|
||||||
|
## Scenario 1 — a trusted request creates a ticket immediately (User Story 1)
|
||||||
|
|
||||||
|
1. Send a valid inbound request through `POST /v1/support/requests`.
|
||||||
|
2. **Expected**: `202` with a `ticketId`/`code`/`status: NEW`; the `Ticket` and its `Problem`
|
||||||
|
exist in the database immediately — no polling needed.
|
||||||
|
|
||||||
|
## Scenario 2 — idempotency key prevents duplicate tickets (User Story 1)
|
||||||
|
|
||||||
|
1. Send the same request twice with the same `idempotencyKey`.
|
||||||
|
2. **Expected**: both responses reference the *same* `ticketId`; only one `Ticket` row exists.
|
||||||
|
|
||||||
|
## Scenario 3 — recurring problem links to the existing Problem (User Story 1)
|
||||||
|
|
||||||
|
1. Create a ticket, note its `problemId`.
|
||||||
|
2. Send a second, distinct request whose `referenceIds` includes that same problem's reference.
|
||||||
|
3. **Expected**: the new ticket has the *same* `problemId` as the first — no second `Problem`
|
||||||
|
created.
|
||||||
|
|
||||||
|
## Scenario 4 — internal notes never leak to a customer-scoped read (User Story 2)
|
||||||
|
|
||||||
|
1. Post one message of each type (`CUSTOMER_MESSAGE`, `AI_MESSAGE`, `AGENT_MESSAGE`,
|
||||||
|
`INTERNAL_NOTE`, `SYSTEM_EVENT`, `INVESTIGATION_NOTE`, `SOLUTION_NOTE`) to a ticket.
|
||||||
|
2. Read the ticket's messages through a customer-scoped call.
|
||||||
|
3. **Expected**: only `CUSTOMER_MESSAGE`, `AI_MESSAGE`, `AGENT_MESSAGE`, `SYSTEM_EVENT` appear —
|
||||||
|
the other three are absent entirely, not present-but-flagged.
|
||||||
|
4. Read the same ticket's messages through an agent-scoped call.
|
||||||
|
5. **Expected**: all seven messages appear.
|
||||||
|
|
||||||
|
## Scenario 5 — an attachment is unusable until it clears scanning (User Story 3)
|
||||||
|
|
||||||
|
1. Request an upload URL, PUT a file, confirm the upload.
|
||||||
|
2. Immediately request a download URL.
|
||||||
|
3. **Expected**: `409` — `scanStatus` is `pending`.
|
||||||
|
4. Wait for the scan job to run (with the placeholder scanner, research.md — it fails closed to
|
||||||
|
`infected`).
|
||||||
|
5. Request a download URL again.
|
||||||
|
6. **Expected**: still refused — `scanStatus: infected` — confirming the pipeline correctly gates
|
||||||
|
on a real (even if placeholder) scan result rather than defaulting to available.
|
||||||
|
|
||||||
|
## Scenario 6 — a stale ticket-status update is rejected, not overwritten (Edge Cases)
|
||||||
|
|
||||||
|
1. Read a ticket's current `status`/`version`.
|
||||||
|
2. In two separate calls, attempt two different valid status transitions using the *same*
|
||||||
|
`expectedVersion`.
|
||||||
|
3. **Expected**: exactly one succeeds; the other receives `409 CONFLICT` and must re-read the
|
||||||
|
ticket to retry.
|
||||||
|
|
||||||
|
## What "done" looks like
|
||||||
|
|
||||||
|
All six scenarios pass, and together they demonstrate every functional requirement and success
|
||||||
|
criterion in `spec.md` without needing to read the implementation to know what "correct" means.
|
||||||
@@ -0,0 +1,156 @@
|
|||||||
|
# Phase 0 Research: Ticket Creation, Messages & Attachments
|
||||||
|
|
||||||
|
## Decision: Ticket code format
|
||||||
|
|
||||||
|
- **Decision**: `<PRODUCT_CODE>-<YEAR>-<SEQUENCE>`, e.g. `DQB-2026-00567` (matches doc 01/04's own
|
||||||
|
example exactly). `PRODUCT_CODE` is derived from the `Product.externalProductId` (first
|
||||||
|
alphabetic segment, uppercased, max 4 chars, falling back to a generic prefix if the external
|
||||||
|
id doesn't yield one cleanly) and `SEQUENCE` is a per-product-per-year monotonic counter.
|
||||||
|
- **Rationale**: Doc 01/04 use this exact shape as the running example throughout the guide;
|
||||||
|
matching it keeps generated codes recognizable against the spec's own illustrations. A
|
||||||
|
per-product-per-year counter (not a global one) keeps codes short and stable even as ticket
|
||||||
|
volume grows across many products.
|
||||||
|
- **Alternatives considered**: A UUID-derived short code — rejected, not human-referenceable the
|
||||||
|
way doc 01's own example implies support agents need ("DQB-2026-00567" is meant to be readable
|
||||||
|
and speakable, not just unique).
|
||||||
|
|
||||||
|
## Decision: Ticket lifecycle state machine
|
||||||
|
|
||||||
|
- **Decision**: States, exactly as doc 04 §1 and doc 06's `Ticket.status` comment list them:
|
||||||
|
`NEW, AI_ANALYZING, AI_TROUBLESHOOTING, AI_VERIFYING, AI_RESOLVED, HUMAN_ESCALATION,
|
||||||
|
IN_PROGRESS, WAITING_FOR_CUSTOMER, RESOLUTION_PENDING_CUSTOMER, RESOLVED, CLOSED, REOPENED`.
|
||||||
|
Valid transitions are defined as an explicit adjacency table (not "anything can go anywhere"):
|
||||||
|
- `NEW` → `AI_ANALYZING`, `HUMAN_ESCALATION`
|
||||||
|
- `AI_ANALYZING` → `AI_TROUBLESHOOTING`, `HUMAN_ESCALATION`
|
||||||
|
- `AI_TROUBLESHOOTING` → `AI_VERIFYING`, `HUMAN_ESCALATION`
|
||||||
|
- `AI_VERIFYING` → `AI_RESOLVED`, `HUMAN_ESCALATION`
|
||||||
|
- `AI_RESOLVED` → `RESOLUTION_PENDING_CUSTOMER`, `RESOLVED`, `HUMAN_ESCALATION` (verification
|
||||||
|
failure re-escalating, per doc 04 §7)
|
||||||
|
- `HUMAN_ESCALATION` → `IN_PROGRESS`
|
||||||
|
- `IN_PROGRESS` → `WAITING_FOR_CUSTOMER`, `RESOLUTION_PENDING_CUSTOMER`, `HUMAN_ESCALATION`
|
||||||
|
(re-escalation)
|
||||||
|
- `WAITING_FOR_CUSTOMER` → `IN_PROGRESS`
|
||||||
|
- `RESOLUTION_PENDING_CUSTOMER` → `RESOLVED`, `IN_PROGRESS` (customer disputes, reopens
|
||||||
|
investigation per doc 04 §7)
|
||||||
|
- `RESOLVED` → `CLOSED`, `REOPENED`
|
||||||
|
- `CLOSED` → `REOPENED`
|
||||||
|
- `REOPENED` → `IN_PROGRESS`, `AI_ANALYZING`
|
||||||
|
- **Rationale**: An explicit table is what makes FR-006 ("only valid transitions") actually
|
||||||
|
enforceable and testable, rather than a status field anyone can set to anything. The specific
|
||||||
|
edges are read directly off doc 04's described flows (§1 happy path, §3 human flow, §7
|
||||||
|
verification-failure branches, §9 resolve/close/reopen).
|
||||||
|
- **Alternatives considered**: No enforced table (any transition allowed) — rejected, directly
|
||||||
|
contradicts FR-006; a generic workflow-engine library — rejected as disproportionate for a
|
||||||
|
fixed, spec-defined state set with no per-tenant customization need at this phase.
|
||||||
|
|
||||||
|
## Decision: Ticket status optimistic concurrency
|
||||||
|
|
||||||
|
- **Decision**: `Ticket` gains an `Int` `version` column (default `1`, matching doc 06's general
|
||||||
|
guidance to add this to mutable shared-state tables). A status update runs
|
||||||
|
`UPDATE tickets SET status = ?, version = version + 1 WHERE id = ? AND version = ?`; zero rows
|
||||||
|
affected means the caller's view was stale, and the update is rejected (`409 CONFLICT`, caller
|
||||||
|
must re-read and retry).
|
||||||
|
- **Rationale**: This is the standard optimistic-locking pattern and satisfies FR-007/SC-007
|
||||||
|
without needing row-level locks or a distributed lock service — Postgres's own atomic
|
||||||
|
`UPDATE ... WHERE` already guarantees exactly one concurrent writer wins.
|
||||||
|
- **Alternatives considered**: `SELECT ... FOR UPDATE` pessimistic locking — rejected, holds a
|
||||||
|
transaction open across what could be a slow caller round-trip; optimistic locking only holds
|
||||||
|
the lock for the single atomic statement.
|
||||||
|
|
||||||
|
## Decision: Idempotency-key enforcement
|
||||||
|
|
||||||
|
- **Decision**: `Ticket` gains a nullable, unique-per-product `idempotencyKey` column
|
||||||
|
(`@@unique([productId, idempotencyKey])`, nullable values excluded from the constraint per
|
||||||
|
Postgres's standard NULL-handling). Ticket creation is an atomic
|
||||||
|
`INSERT ... ON CONFLICT (productId, idempotencyKey) DO NOTHING RETURNING *`-style upsert; a
|
||||||
|
conflict means the key was already used, and the existing ticket is looked up and returned
|
||||||
|
instead.
|
||||||
|
- **Rationale**: Same atomic-upsert pattern already used for `CustomerReference.findOrCreate` in
|
||||||
|
002-saas-integration — proven, race-safe under concurrent retries, no separate idempotency-key
|
||||||
|
cache/table needed. Scoped per-product (not globally unique) because two different products'
|
||||||
|
clients could coincidentally generate the same key value.
|
||||||
|
- **Alternatives considered**: A separate `IdempotencyKey` tracking table — rejected as
|
||||||
|
unnecessary indirection when the key can live directly on the row it's deduplicating.
|
||||||
|
|
||||||
|
## Decision: Recurring-problem linking
|
||||||
|
|
||||||
|
- **Decision**: Per spec.md's Assumptions, this feature links to an existing `Problem` only when
|
||||||
|
the inbound request's `referenceIds` (docs/02 §3) or the ticket-creation call explicitly
|
||||||
|
supplies a known prior ticket/problem id; otherwise a new `Problem` is always created. No
|
||||||
|
fuzzy/semantic matching is attempted here.
|
||||||
|
- **Rationale**: Real recurring-problem detection needs product-aware understanding of symptoms —
|
||||||
|
that's the AI-support feature's job (Phase 4), not this one's. Building a heuristic here would
|
||||||
|
either be too naive to be useful or scope-creep into Phase 4's actual responsibility.
|
||||||
|
- **Alternatives considered**: Simple text-similarity matching on `Problem.statement` — rejected,
|
||||||
|
would produce false-positive links (different problems, similar wording) with no way to correct
|
||||||
|
them until Phase 4 exists to do it properly.
|
||||||
|
|
||||||
|
## Decision: Message type → visibility mapping
|
||||||
|
|
||||||
|
- **Decision**: A single source-of-truth constant map (not per-message logic):
|
||||||
|
```
|
||||||
|
CUSTOMER_MESSAGE: true, AI_MESSAGE: true, AGENT_MESSAGE: true, SYSTEM_EVENT: true,
|
||||||
|
INTERNAL_NOTE: false, INVESTIGATION_NOTE: false, SOLUTION_NOTE: false
|
||||||
|
```
|
||||||
|
`TicketMessage.visibleToCustomer` (doc 06's own field) is *set from this map at write time*,
|
||||||
|
never accepted as caller input — and customer-scoped reads filter `WHERE visibleToCustomer =
|
||||||
|
true` at the repository/query layer, not by trimming the response after fetching everything.
|
||||||
|
- **Rationale**: Directly satisfies FR-008 (visibility derived from type, not independently
|
||||||
|
settable) and FR-009 (enforced at the query layer, so a serialization bug can't leak a note that
|
||||||
|
was never fetched in the first place).
|
||||||
|
- **Alternatives considered**: Accepting `visibleToCustomer` as an API input — rejected outright,
|
||||||
|
this is exactly the "trust the client" mistake FR-008 exists to prevent.
|
||||||
|
|
||||||
|
## Decision: Attachment pipeline shape
|
||||||
|
|
||||||
|
- **Decision**: Two-phase upload — (1) caller requests a presigned PUT URL for a given
|
||||||
|
filename/content-type (`storageService` already has the S3 client wired, needs a presigned-PUT
|
||||||
|
method added alongside its existing presigned-GET `getPresignedUrl`); (2) caller PUTs the file
|
||||||
|
directly to object storage, then confirms the upload, which creates the `TicketAttachment` row
|
||||||
|
(`scanStatus: pending`) and enqueues a scan job on the existing `attachments-queue`
|
||||||
|
(`src/jobs/attachments/index.ts`, currently a log-only stub). Downloads use the existing
|
||||||
|
`storageService.getPresignedUrl` (GET), but only after the repository confirms
|
||||||
|
`scanStatus: clean` — the presigned URL is never generated for a `pending`/`infected`/`rejected`
|
||||||
|
attachment.
|
||||||
|
- **Rationale**: A presigned-PUT upload means file bytes never transit the Fastify process at all
|
||||||
|
(satisfies FR-011 more strongly than a server-side proxy-upload would, and avoids adding
|
||||||
|
multipart-body handling to this feature). The existing `attachments-queue` scaffold is exactly
|
||||||
|
where the scan step belongs per doc 07's own module layout.
|
||||||
|
- **Alternatives considered**: Server-side proxy upload (client → Fastify → S3) — rejected as
|
||||||
|
unnecessary complexity/latency when presigned PUT achieves the same security properties with
|
||||||
|
less code.
|
||||||
|
|
||||||
|
## Decision: Malware scanning — no scanner exists in this stack yet
|
||||||
|
|
||||||
|
- **Decision**: Define a small `MalwareScanner` interface (`scan(objectKey): Promise<'clean' |
|
||||||
|
'infected'>`) and ship one implementation now: a clearly-named
|
||||||
|
`UnimplementedPlaceholderScanner` that always returns `'infected'` (fails closed, never
|
||||||
|
`'clean'`) and logs a loud warning — so attachments are correctly gated as never-downloadable
|
||||||
|
until a real scanner (e.g. ClamAV via a sidecar, or a cloud provider's scanning API) is wired
|
||||||
|
in as a follow-up. The job worker calls whichever implementation is bound at startup.
|
||||||
|
- **Rationale**: No antivirus/scanning service exists anywhere in this codebase or its
|
||||||
|
dependencies, and standing one up (e.g. deploying ClamAV) is real infrastructure work outside
|
||||||
|
this feature's scope. The alternative — a scanner that always returns `'clean'` — would satisfy
|
||||||
|
the code path but silently defeat FR-013's actual security purpose; failing closed means the
|
||||||
|
pipeline is honest about "attachments aren't actually safe to download yet" rather than
|
||||||
|
pretending they are. This mirrors the same honesty principle as the credential-storage
|
||||||
|
placeholder decision in 002-saas-integration's research.md.
|
||||||
|
- **Alternatives considered**: Always-`'clean'` stub — rejected, defeats the feature's own
|
||||||
|
purpose and would be easy to forget to replace since nothing would ever surface the gap.
|
||||||
|
Skipping the scan step's implementation entirely (leave the job as its current log-only stub)
|
||||||
|
— rejected, FR-013 requires attachments be gated on scan status, and a permanently-`pending`
|
||||||
|
attachment (nothing ever calls the job) makes attachments unusable rather than correctly gated.
|
||||||
|
|
||||||
|
## Decision: Local/test object storage — add MinIO to Docker Compose
|
||||||
|
|
||||||
|
- **Decision**: Add a `minio` service to `docker-compose.test.yml` and
|
||||||
|
`docker-compose.development.yml`, and set `AWS_S3_ENDPOINT` in `.env.test`/`.env.development`
|
||||||
|
to point at it. `storageClient.ts` already branches on `storageConfig.endpoint` being set
|
||||||
|
(`forcePathStyle: true` for MinIO compatibility) — no client code changes needed, only compose
|
||||||
|
wiring and env values.
|
||||||
|
- **Rationale**: Doc 04 explicitly calls for "MinIO for local dev," and the client already
|
||||||
|
anticipated this (the `endpoint`/`forcePathStyle` branch exists in code that predates this
|
||||||
|
feature) — this decision just finishes wiring what was already half-built.
|
||||||
|
- **Alternatives considered**: Mocking S3 calls in tests instead of running real MinIO — rejected,
|
||||||
|
this feature's whole point includes verifying presigned URLs actually work and expire, which a
|
||||||
|
mock can't meaningfully verify.
|
||||||
Reference in New Issue
Block a user