feat: Slice 1 (Module 02) — Document upload & list foundation

[mGasiorek998]

Closes #9

Summary

• Added documents and document_shares tables to the Drizzle schema
• Added DOCUMENT_STORAGE_PATH env var (defaults to apps/api/storage/documents/) with auto-create on startup
• Implemented document storage service (write/read/delete with UUID-named files)
• Implemented uploadDocument and listDocuments use cases with MIME allowlist enforcement
• Added POST /documents (10 MiB bodyLimit, 413/415/422) and GET /documents routes with requireAuth + requireRole(['patient'])
• Filled in packages/contracts/src/documents.ts with Zod schemas for both endpoints
• Added /patient/documents web page with file upload and document list
• 12 integration tests + 4 storage unit tests covering all acceptance criteria

Test plan

• pnpm verify exits 0 (68 tests pass, typecheck and lint clean)
• Upload a valid PDF ≤ 10 MiB → 201 with metadata, UUID file on disk
• Upload file > 10 MiB → 413 FILE_TOO_LARGE
• Upload disallowed MIME → 415 UNSUPPORTED_MEDIA_TYPE
• Upload with no file → 422 MISSING_FILE
• GET /documents returns list ordered by uploadedAt desc, empty array when none
• Both endpoints 401 without JWT, 403 with doctor JWT
• Storage directory auto-created at startup (no manual mkdir needed)

🤖 Generated with Claude Code

[claude]

Claude finished @mGasiorek998's task in 3m 48s —— View job

---

Review — Slice 1 (Module 02): Document upload & list

Verdict: APPROVE — would merge. The slice delivers a clean end-to-end vertical (schema → storage → use case → route → contract → UI → tests) and all acceptance criteria from #9 are functionally covered. Notes below are mostly polish; one is worth checking before the next slice.

---

Spec compliance (vs. issue #9 acceptance criteria)

• ✅ AC: POST valid PDF ≤10 MiB → 201 + UUID file — covered by documents.test.ts:84-131; UUID v4 regex assertion is solid.
• ✅ AC: POST > 10 MiB → 413 FILE_TOO_LARGE — documents.test.ts:133-154. Body limit set to 10 MiB + 1 per the issue's literal wording.
• ✅ AC: disallowed MIME → 415 UNSUPPORTED_MEDIA_TYPE — covered for text/plain and image/gif.
• ✅ AC: no file part → 422 MISSING_FILE — covered.
• ✅ AC: GET /documents desc-ordered, [] when empty — covered (uses real 10 ms gap to disambiguate timestamps).
• ✅ AC: 401 without JWT, 403 with doctor JWT — both endpoints, both verbs.
• ✅ AC: storage dir auto-created at startup — apps/api/src/index.ts:29 mkdirSync(..., { recursive: true }).
• ✅ Storage unit tests against real temp dir — document-storage.test.ts mkdtemp + rm in afterAll.
• ⚠️ AC: "Repository integration tests pass against real Postgres" — there is no dedicated repository test file. The repo is exercised end-to-end by documents.test.ts, so functionally covered, but if a separate document-repository.test.ts is part of the slice's TDD ladder it's missing. Non-blocking.

Architecture

• ✅ Layering respected: route → use case → infra. upload-document.ts and list-documents.ts know nothing about Hono; storage adapter knows nothing about DB. Matches docs/ARCHITECTURE_RULES.md.
• ✅ Factory-function DI: createDocumentsRouter(deps), createDocumentStorage(path) — consistent with createAuthRouter.
• ✅ Co-located tests, no barrel files, async/await, no any.
• ⚠️ Schema deviates from module-02 spec (docs/modules/02-patient-documents-and-sharing.md:34): spec has size_bytes (bigint), schema in db/schema.ts:48 has size: integer. Integer caps at ~2 GB which is fine for a 10 MiB cap, but the column name and type drift away from the canonical glossary/spec. Consider renaming to sizeBytes / bigint now while no rows exist, otherwise Module 02's later slices and Module 04 will have to accommodate the drift.
• 🛈 Routes mounted at /documents, not /api/documents as module-02 spec describes. The issue body itself uses /documents (no /api), so this is per-issue, but it's worth deciding once and aligning the spec/glossary to whatever you pick before more endpoints land. The _probe router uses /api/_probe, so the project is currently mixed.

Security

• ✅ UUID v4 filenames on disk — no path enumeration, no original-name traversal. Original filename only ever lives in the DB row.
• ✅ MIME allowlist enforced server-side; route is behind requireAuth + requireRole(['patient']).
• ✅ Auth checked before any file bytes are touched.
• 🛈 MIME validation trusts the client-declared file.type (per spec VR-001 — "file part mime type"). A determined attacker can still upload a renamed binary as application/pdf. That's per-spec for the prototype; flagging in case you later want magic-byte sniffing.
• 🛈 No virus scanning / no quarantine — out of scope for this slice, but worth an ADR before exposing downloads to doctors in a later slice.

Test quality

• Tests target external behavior (HTTP status + body shape, on-disk effects) rather than implementation internals. ✓
• Real Postgres + real temp filesystem — no DB mocking, matches the testing rule.
• Cleanup in afterAll for both DB rows and tmp dir. ✓
• One small gap: boundary case EC-001 (file of exactly 10 485 760 bytes must succeed) is not exercised. Because Hono bodyLimit measures the whole request body, multipart boundaries (~150–200 bytes of overhead) plus a 10 MiB file will exceed the configured 10 MiB + 1 limit and 413 instead of 201. If you intend EC-001 to hold, set MAX_BODY_SIZE to something like 10 MiB + 8 KiB (file + multipart slack) and add a test for the exact-10-MiB case. Non-blocking for this slice but will bite the next time someone reads the spec.

Suggestions (non-blocking)

• Dead branch: documents-router.ts:47-49 checks result.reason === 'missing_file', but the route already returned 422 before calling uploadDocument when file is not a File instance (line 40). Pick one home for the missing-file check — I'd keep the use-case as the source of truth and drop the early router check, so the use case is callable in isolation.
• Generic 500 body: the catch in upload-document.ts:58-62 swallows the DB error and returns INTERNAL_SERVER_ERROR. Spec EH-003 distinguishes STORAGE_ERROR (disk failure) from DB failure; right now both collapse to one error code. Consider logging result.reason === 'db_error' via deps.logger and using a specific code so operators can tell the two apart in production.
• Disk-error path is uncovered: if storage.write itself throws (mid-upload disk failure), the exception propagates out of the route handler and Hono returns a default 500 with no JSON body. Per EH-003 the response should be { error: "STORAGE_ERROR", ... } and any partial file should be deleted. Worth wrapping the storage.write in a try/catch with cleanup before this module ships its delete + share slices.
• Schema column ordering: documents has size while module-02 spec has size_bytes. Aligning now is one pnpm db:push away; once any production-shaped data exists this becomes a migration.
• uploaded_at default: defaultNow() is fine, but the patient could in principle override it in a future slice if you ever expose the column to the DTO. Consider .notNull().defaultNow() plus an explicit Drizzle insert that doesn't accept a client-supplied value — already true today, just future-proofing.
• Frontend documents.tsx: the upload errors are surfaced as error.message, which for a 413 will read FILE_TOO_LARGE. That's fine for a dev build but a future slice should map error codes → human strings (and ideally show the 10 MiB limit in the input label).

Fix the EC-001 boundary + body-limit issue →

Fix the dead missing-file branch →

Align schema with module-02 spec →

---

Checklist

• Read linked issue Slice 1 (Module 02): Document upload & list — foundation #9 acceptance criteria
• Read ARCHITECTURE_RULES.md, GLOSSARY.md, module-02 spec
• Review backend changes (schema, storage, repo, use-cases, router, config)
• Review frontend changes (route, hooks, list component, api-client)
• Review test coverage (unit + integration)
• Post review verdict
  · branch afk/issue-9