From 4751cf31648f693e8d723d15168c1409e15b77f4 Mon Sep 17 00:00:00 2001 From: saqib mir Date: Wed, 2 Sep 2026 14:53:53 +0530 Subject: [PATCH] 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 --- .../contracts/ticket-lifecycle-contract.md | 58 +++++++ specs/003-ticketing/data-model.md | 100 +++++++++++ specs/003-ticketing/plan.md | 139 ++++++++++++++++ specs/003-ticketing/quickstart.md | 56 +++++++ specs/003-ticketing/research.md | 156 ++++++++++++++++++ 5 files changed, 509 insertions(+) create mode 100644 specs/003-ticketing/contracts/ticket-lifecycle-contract.md create mode 100644 specs/003-ticketing/data-model.md create mode 100644 specs/003-ticketing/plan.md create mode 100644 specs/003-ticketing/quickstart.md create mode 100644 specs/003-ticketing/research.md diff --git a/specs/003-ticketing/contracts/ticket-lifecycle-contract.md b/specs/003-ticketing/contracts/ticket-lifecycle-contract.md new file mode 100644 index 0000000..a8fe97b --- /dev/null +++ b/specs/003-ticketing/contracts/ticket-lifecycle-contract.md @@ -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: , expectedVersion: }`. + +| 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). diff --git a/specs/003-ticketing/data-model.md b/specs/003-ticketing/data-model.md new file mode 100644 index 0000000..84f9ffd --- /dev/null +++ b/specs/003-ticketing/data-model.md @@ -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 | `--` (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). diff --git a/specs/003-ticketing/plan.md b/specs/003-ticketing/plan.md new file mode 100644 index 0000000..dfe9bc1 --- /dev/null +++ b/specs/003-ticketing/plan.md @@ -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.* diff --git a/specs/003-ticketing/quickstart.md b/specs/003-ticketing/quickstart.md new file mode 100644 index 0000000..bb7bd17 --- /dev/null +++ b/specs/003-ticketing/quickstart.md @@ -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. diff --git a/specs/003-ticketing/research.md b/specs/003-ticketing/research.md new file mode 100644 index 0000000..cfcde58 --- /dev/null +++ b/specs/003-ticketing/research.md @@ -0,0 +1,156 @@ +# Phase 0 Research: Ticket Creation, Messages & Attachments + +## Decision: Ticket code format + +- **Decision**: `--`, 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.