diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index a2d4b23..decb0e9 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -126,9 +126,16 @@ Plans: **Plans:** 4 plans Plans: +**Wave 1** - [ ] 05-01-PLAN.md — Wave 1: go get aws-sdk-go-v2 + migration 0005_files + sqlc queries + files.Store + FileStorer interface + RED test scaffold + MinIO in compose.yaml + +**Wave 2** *(blocked on Wave 1 completion)* - [ ] 05-02-PLAN.md — Wave 2: handlers_files.go (FilesDeps + FileUploadHandler + TabloFilesTabHandler + TabloTasksTabHandler) + tablos.templ 3-tab layout + files.templ (upload form + list) + router + main.go wiring (FILE-01, FILE-02, FILE-03, FILE-06) + +**Wave 3** *(blocked on Wave 2 completion)* - [ ] 05-03-PLAN.md — Wave 3: FileDownloadHandler (302 → presigned URL) + FileDeleteConfirmHandler + FileDeleteHandler (S3-first delete) + FileDeleteConfirmFragment + full TestFile* green (FILE-04, FILE-05, FILE-06) + +**Wave 4** *(blocked on Wave 3 completion)* - [ ] 05-04-PLAN.md — Wave 4: Human-verify checkpoint: tab navigation + upload/list/download/delete end-to-end browser walkthrough ### Phase 6: Background Worker diff --git a/.planning/STATE.md b/.planning/STATE.md index 6766f52..0a9d774 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,13 +3,13 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone status: ready_to_plan -last_updated: "2026-05-15T08:58:24.500Z" +last_updated: "2026-05-15T10:14:34.258Z" progress: total_phases: 7 completed_phases: 4 - total_plans: 18 + total_plans: 22 completed_plans: 18 - percent: 100 + percent: 82 --- # STATE diff --git a/.planning/phases/05-files/05-01-PLAN.md b/.planning/phases/05-files/05-01-PLAN.md index 84ab803..3a2ca7f 100644 --- a/.planning/phases/05-files/05-01-PLAN.md +++ b/.planning/phases/05-files/05-01-PLAN.md @@ -31,6 +31,8 @@ must_haves: - "files.Store has Upload, Delete, PresignDownload methods satisfying FileStorer interface" - "handlers_files_test.go contains RED test stubs for FILE-01..FILE-06 (compile but skip)" - "MinIO service runs in compose.yaml with mc init container creating xtablo-dev bucket" + - "D-03: compose.yaml contains minio and minio-init services; minio-init creates xtablo-dev bucket with restart: no" + - "D-05: files.Store.Upload calls http.DetectContentType on first 512 bytes to detect content-type server-side" artifacts: - path: "backend/migrations/0005_files.sql" provides: "tablo_files schema" diff --git a/.planning/phases/05-files/05-02-PLAN.md b/.planning/phases/05-files/05-02-PLAN.md index 5b98ef8..042648c 100644 --- a/.planning/phases/05-files/05-02-PLAN.md +++ b/.planning/phases/05-files/05-02-PLAN.md @@ -27,6 +27,14 @@ must_haves: - "Only the tablo owner can see or POST to the files tab (non-owner gets 404)" - "Tab bar on tablo detail page shows Overview / Tasks / Files with hx-push-url; URL updates on tab switch" - "Uploading a file >25MB returns a friendly error message above the upload form (not a 500)" + - "D-01: FileUploadHandler receives multipart POST and streams file body directly to S3 via files.Store.Upload (server-proxied, no temp file)" + - "D-02: main.go reads S3_ENDPOINT, S3_BUCKET, S3_ACCESS_KEY, S3_SECRET_KEY, S3_REGION, S3_USE_PATH_STYLE env vars and passes them to files.NewStore" + - "D-04: FileUploadHandler constructs S3 key as files/{tablo_id}/{uuid} before calling files.Store.Upload" + - "D-06: FileUploadHandler generates a new UUID per upload regardless of filename; no dedup check; duplicate filenames each get their own DB row" + - "D-07: TabloDetailPage signature includes activeTab string param; tab bar renders Overview, Tasks, Files links" + - "D-08: Tab bar links carry hx-push-url attribute matching their href so browser URL updates on HTMX tab switch" + - "D-09: FileUploadHandler sets r.Body = http.MaxBytesReader(w, r.Body, maxBytes) before ParseMultipartForm; max size read from MAX_UPLOAD_SIZE_MB env var (default 25)" + - "D-10: FileUploadHandler has no file-type allowlist check; all MIME types accepted after http.DetectContentType detection" artifacts: - path: "backend/internal/web/handlers_files.go" provides: "FilesDeps, TabloFilesTabHandler, FileUploadHandler, TabloTasksTabHandler" @@ -148,6 +156,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): loadOwnedTabloForFile helper — same shape as loadOwnedTabloForTask but for file_id URL param and GetTabloFileByID query. Signature: func loadOwnedTabloForFile(w, r, deps FilesDeps) (sqlc.Tablo, sqlc.TabloFile, *auth.User, bool). TabloFilesTabHandler (GET /tablos/{id}/files): + 0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement before any other logic 1. loadOwnedTablo → 404 on failure 2. deps.Queries.ListFilesByTablo → log error + empty slice on failure 3. Set Content-Type: text/html; charset=utf-8 @@ -159,12 +168,13 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): 2. deps.Queries.ListTasksByTablo → log error + empty slice on failure 3. If HX-Request == "true": render templates.TasksTabFragment(tablo, tasks, csrf.Token(r)) — this component is created in Task 2 of this plan 4. Else: render templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, nil, "tasks") - Note: TabloTasksTabHandler lives in handlers_files.go since it is part of the tab wiring introduced this phase. It takes FilesDeps (which has Queries). Alternatively put it in handlers_tasks.go — choose whichever file keeps imports clean. + File placement: TabloTasksTabHandler lives in handlers_files.go (it is part of the tab wiring introduced this phase). TasksTabFragment (the templ component wrapping KanbanBoard) goes in tablos.templ (it is a tablo-level concern). TabloTasksTabHandler does NOT go in handlers_tasks.go. FileUploadHandler (POST /tablos/{id}/files): + 0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement before reading the request body 1. loadOwnedTablo → 404 on failure 2. maxBytes := int64(deps.MaxUploadMB) * 1024 * 1024 - 3. r.Body = http.MaxBytesReader(w, r.Body, maxBytes) — MUST be first, before ParseMultipartForm (Pitfall 2 in RESEARCH) + 3. r.Body = http.MaxBytesReader(w, r.Body, maxBytes) — MUST be before ParseMultipartForm (Pitfall 2 in RESEARCH) 4. if err := r.ParseMultipartForm(2 << 20); err != nil: check errors.As(err, &mbErr) for *http.MaxBytesError; if yes, render upload error fragment with "File too large (max {MaxUploadMB} MB)." and return 422; else http.Error 400 5. file, header, err := r.FormFile("file") — 400 on error 6. defer file.Close() @@ -187,7 +197,9 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): - go build ./... exits 0 - backend/internal/web/handlers_files.go contains "type FilesDeps struct" and "func TabloFilesTabHandler(" and "func FileUploadHandler(" and "func TabloTasksTabHandler(" - - FileUploadHandler contains "http.MaxBytesReader" on the line before "ParseMultipartForm" (Pitfall 2 guard) + - FileUploadHandler first statement is "deps.Files == nil" nil guard returning 503 before any body read + - TabloFilesTabHandler first statement is "deps.Files == nil" nil guard returning 503 + - FileUploadHandler contains "http.MaxBytesReader" before "ParseMultipartForm" (Pitfall 2 guard) - FileUploadHandler contains "files/" string for S3 key construction (D-04) - FileUploadHandler contains "http.MaxBytesError" for size violation detection - go test ./internal/web/ -run "TestFileUpload|TestFilesTab" exits 0 (PASS or SKIP, no FAIL) @@ -234,7 +246,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): Also add stand-alone fragment components that can be returned by HTMX tab-switch responses: templ TabloOverviewTabFragment(tablo sqlc.Tablo, csrfToken string) — minimal overview content - templ TasksTabFragment(tablo sqlc.Tablo, tasks []sqlc.Task, csrfToken string) — wraps existing @KanbanBoard(tablo.ID, csrfToken, tasks) call + templ TasksTabFragment(tablo sqlc.Tablo, tasks []sqlc.Task, csrfToken string) — wraps existing @KanbanBoard(tablo.ID, csrfToken, tasks) call; lives in tablos.templ (tablo-level concern) These are called by TabloTasksTabHandler when HX-Request == "true" (Plan 02 Task 1). Step 2 — Update TWO call sites in backend/internal/web/handlers_tablos.go: @@ -274,7 +286,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): maxUploadMB int: parse MAX_UPLOAD_SIZE_MB env var with strconv.Atoi, default 25 on empty/error If s3Endpoint == "" || s3Bucket == "": log slog.Warn (not Error + Exit — allow server to start without S3 for non-file routes in dev) filesStore, err := files.NewStore(ctx, s3Endpoint, s3Bucket, s3Region, s3AccessKey, s3SecretKey, s3UsePathStyle) — if err != nil: log slog.Error + os.Exit(1) - Only construct filesStore if s3Endpoint != "" — else set filesStore = nil and fileDeps.Files = nil. File handlers must check for nil Files and return 503 "storage not configured". + Only construct filesStore if s3Endpoint != "" — else set filesStore = nil and fileDeps.Files = nil. File handlers check for nil Files as their FIRST statement and return 503 "storage not configured". fileDeps := web.FilesDeps{Queries: q, Files: filesStore, MaxUploadMB: maxUploadMB} Update NewRouter call to pass fileDeps before csrfKey: web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, fileDeps, csrfKey, env) @@ -290,6 +302,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): - go build ./... exits 0 - go test ./... exits 0 (no regressions in existing test suite; TestTask* and TestTablo* remain PASS) - backend/templates/tablos.templ contains "activeTab string" in TabloDetailPage signature and "hx-push-url" and "tab-content" + - backend/templates/tablos.templ contains "TasksTabFragment" component (wrapping KanbanBoard) - backend/templates/files.templ contains "FilesTabFragment" and "FileUploadForm" and "hx-encoding" (required for HTMX multipart) and "FileRowGone" - backend/internal/web/router.go contains "fileDeps FilesDeps" in NewRouter signature and "TabloFilesTabHandler" and "TabloTasksTabHandler" - backend/cmd/web/main.go contains "files.NewStore" and "fileDeps := web.FilesDeps" @@ -305,7 +318,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): | Boundary | Description | |----------|-------------| | browser → FileUploadHandler | Multipart form body; attacker controls filename, file content, MIME hints, and Content-Length | -| FilesDeps.Files (FileStorer) | Nil-checked in main.go; handlers must check for nil before calling Upload/Delete/PresignDownload | +| FilesDeps.Files (FileStorer) | Nil-checked as the first statement of every file handler; returns 503 before touching the request body | ## STRIDE Threat Register @@ -316,7 +329,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types): | T-05-02-03 | Spoofing | FileUploadHandler — content-type | mitigate | Browser Content-Type header ignored; server calls http.DetectContentType on first 512 bytes via files.Store.Upload (D-05) | | T-05-02-04 | Elevation of Privilege | TabloFilesTabHandler — IDOR | mitigate | loadOwnedTablo called as first step in every handler; GetTabloByID query includes UserID filter; non-owner gets 404 (FILE-06) | | T-05-02-05 | Tampering | File routes — CSRF | mitigate | gorilla/csrf middleware already in stack (Phase 2); all state-changing POSTs require valid CSRF token; @ui.CSRFField(csrfToken) present in upload form and future delete form | -| T-05-02-06 | Denial of Service | TabloFilesTabHandler — nil FileStorer | mitigate | main.go only constructs filesStore when S3_ENDPOINT set; FileUploadHandler checks deps.Files == nil and returns 503 "storage not configured" before reading body | +| T-05-02-06 | Denial of Service | TabloFilesTabHandler + FileUploadHandler — nil FileStorer | mitigate | deps.Files == nil guard is the FIRST statement in TabloFilesTabHandler and FileUploadHandler; returns 503 "storage not configured" before any body read or DB call; main.go sets fileDeps.Files = nil when S3_ENDPOINT is unset | @@ -326,6 +339,7 @@ After both tasks: - cd backend && go test ./internal/web/ -run TestFileUpload -v shows PASS or SKIP (not FAIL) - backend/templates/files.templ exists and contains FilesTabFragment, FileUploadForm, FileRowGone - backend/templates/tablos.templ TabloDetailPage signature has 6 args (including files []sqlc.TabloFile and activeTab string) +- backend/templates/tablos.templ contains TasksTabFragment component - router.go NewRouter has fileDeps FilesDeps parameter diff --git a/.planning/phases/05-files/05-03-PLAN.md b/.planning/phases/05-files/05-03-PLAN.md index 58d409c..17b353a 100644 --- a/.planning/phases/05-files/05-03-PLAN.md +++ b/.planning/phases/05-files/05-03-PLAN.md @@ -148,16 +148,19 @@ From backend/internal/web/handlers_tasks.go (delete handler pattern lines 285-30 Replace FileDownloadHandler stub (currently returns 501) with full implementation: + 0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement 1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure (FILE-06) 2. url, err := deps.Files.PresignDownload(r.Context(), file.S3Key) — 500 on error; log with slog.Default().Error("files download: PresignDownload failed", "file_id", file.ID, "err", err) 3. http.Redirect(w, r, url, http.StatusFound) — 302 redirect to signed URL (per CONTEXT.md discretion item: download redirects to signed URL) Replace FileDeleteConfirmHandler stub with implementation (inline confirm pattern from Phase 3): + 0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement 1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure 2. Set Content-Type: text/html; charset=utf-8 3. Render templates.FileDeleteConfirmFragment(tablo.ID, file, csrf.Token(r)) — new component created in Task 2 Replace FileDeleteHandler stub with implementation (S3-first, always-delete-DB per RESEARCH.md Pattern 6): + 0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement 1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure 2. Delete from S3: if err := deps.Files.Delete(r.Context(), file.S3Key); err != nil: slog.Default().Error("files delete: S3 Delete failed", "key", file.S3Key, "err", err) — log but do NOT return; continue to DB delete (orphan S3 objects are Phase 6 worker's problem per CONTEXT.md deferred items) 3. if err := deps.Queries.DeleteTabloFile(r.Context(), sqlc.DeleteTabloFileParams{ID: file.ID, TabloID: tablo.ID}); err != nil: log + http.Error 500 + return @@ -174,6 +177,9 @@ From backend/internal/web/handlers_tasks.go (delete handler pattern lines 285-30 - go test ./internal/web/ -run "TestFileDownload|TestFileDelete|TestFileOwnership" exits 0 with "--- PASS" for each test (no SKIP, no FAIL) + - backend/internal/web/handlers_files.go FileDownloadHandler first statement is "deps.Files == nil" nil guard returning 503 + - backend/internal/web/handlers_files.go FileDeleteConfirmHandler first statement is "deps.Files == nil" nil guard returning 503 + - backend/internal/web/handlers_files.go FileDeleteHandler first statement is "deps.Files == nil" nil guard returning 503 - backend/internal/web/handlers_files.go FileDownloadHandler contains "http.StatusFound" (302 redirect) - backend/internal/web/handlers_files.go FileDeleteHandler contains "deps.Files.Delete" on a line before "deps.Queries.DeleteTabloFile" (S3 first per RESEARCH Pattern 6) - backend/internal/web/handlers_files.go FileDeleteHandler: the slog.Error for S3 failure does NOT have an early return; DB delete is always attempted @@ -244,6 +250,7 @@ From backend/internal/web/handlers_tasks.go (delete handler pattern lines 285-30 | T-05-03-03 | Elevation of Privilege | FileDeleteHandler — IDOR | mitigate | Same loadOwnedTabloForFile preamble; DeleteTabloFile parameterized with both file ID AND tablo_id — non-owner cannot delete even with a valid file UUID | | T-05-03-04 | Denial of Service | FileDeleteHandler — partial S3 failure loop | accept | S3 delete failure is logged but does not abort; DB row is always deleted; orphan objects accumulate and are cleaned by Phase 6 worker (explicit design decision from CONTEXT.md deferred items) | | T-05-03-05 | Tampering | FileDeleteConfirmFragment — CSRF on delete form | mitigate | @ui.CSRFField(csrfToken) present in FileDeleteConfirmFragment form; gorilla/csrf middleware rejects POST /files/{id}/delete without valid token | +| T-05-03-06 | Denial of Service | FileDownloadHandler + FileDeleteConfirmHandler + FileDeleteHandler — nil FileStorer | mitigate | deps.Files == nil guard is the FIRST statement in all three handlers; returns 503 "storage not configured" before any S3 or DB call | @@ -251,6 +258,7 @@ After both tasks: - cd backend && go test ./... -count=1 -timeout 60s exits 0 (all TestFile* PASS, all TestTask* and TestTablo* still PASS) - cd backend && go test ./internal/web/ -run TestFile -v shows 6x "--- PASS" - backend/internal/web/handlers_files.go contains no "501" (all stubs replaced) +- backend/internal/web/handlers_files.go FileDownloadHandler, FileDeleteConfirmHandler, FileDeleteHandler each have "deps.Files == nil" as the first guard - backend/templates/files.templ contains FileDeleteConfirmFragment with ui.CSRFField diff --git a/.planning/phases/05-files/05-PATTERNS.md b/.planning/phases/05-files/05-PATTERNS.md new file mode 100644 index 0000000..0e2423c --- /dev/null +++ b/.planning/phases/05-files/05-PATTERNS.md @@ -0,0 +1,750 @@ +# Phase 5: Files - Pattern Map + +**Mapped:** 2026-05-15 +**Files analyzed:** 9 (new/modified) +**Analogs found:** 9 / 9 + +## File Classification + +| New/Modified File | Role | Data Flow | Closest Analog | Match Quality | +|-------------------|------|-----------|----------------|---------------| +| `backend/migrations/0005_files.sql` | migration | batch | `backend/migrations/0004_tasks.sql` | exact | +| `backend/internal/db/queries/files.sql` | query | CRUD | `backend/internal/db/queries/tasks.sql` | exact | +| `backend/internal/files/store.go` | service | file-I/O | `backend/internal/files/doc.go` (placeholder) | new (no analog body) | +| `backend/internal/web/handlers_files.go` | controller | request-response | `backend/internal/web/handlers_tasks.go` | exact | +| `backend/internal/web/router.go` | config | request-response | `backend/internal/web/router.go` (self, modify) | exact | +| `backend/templates/tablos/detail.templ` (= `backend/templates/tablos.templ`) | component | request-response | `backend/templates/tablos.templ` (self, modify) | exact | +| `backend/templates/files.templ` | component | request-response | `backend/templates/tasks.templ` | role-match | +| `backend/cmd/web/main.go` | config | request-response | `backend/cmd/web/main.go` (self, modify) | exact | +| `compose.yaml` | config | — | `backend/compose.yaml` (self, modify) | exact | + +--- + +## Pattern Assignments + +### `backend/migrations/0005_files.sql` (migration, batch) + +**Analog:** `backend/migrations/0004_tasks.sql` + +**Full migration structure** (lines 1–26): +```sql +-- migrations/0004_tasks.sql +-- Phase 4: Tasks (Kanban) + +-- +goose Up + +-- ENUM declaration order must match visual left-to-right column order (Pitfall 6). +CREATE TYPE task_status AS ENUM ('todo', 'in_progress', 'in_review', 'done'); + +CREATE TABLE tasks ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + tablo_id uuid NOT NULL REFERENCES tablos(id) ON DELETE CASCADE, + title text NOT NULL, + description text, + status task_status NOT NULL DEFAULT 'todo', + position integer NOT NULL DEFAULT 100, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() +); + +CREATE INDEX tasks_tablo_id_status_idx ON tasks(tablo_id, status, position); + +-- +goose Down +-- Table MUST be dropped before type (Pitfall 3 — type is still referenced by table column). +DROP TABLE IF EXISTS tasks; +DROP TYPE IF EXISTS task_status; +``` + +**Key conventions to copy:** +- Header comment: `-- migrations/0005_files.sql` + phase description line +- `-- +goose Up` / `-- +goose Down` section markers (required by goose) +- `id uuid PRIMARY KEY DEFAULT gen_random_uuid()` for all new tables +- `tablo_id uuid NOT NULL REFERENCES tablos(id) ON DELETE CASCADE` — CASCADE is the pattern; file rows are cleaned when tablo is deleted +- `timestamptz NOT NULL DEFAULT now()` — always timestamptz, never timestamp +- Index comment explains composite key rationale +- `DROP TABLE IF EXISTS` in Down block (no DROP TYPE needed since `tablo_files` has no custom ENUM) + +**Deviation for 0005:** No `updated_at` column (files are immutable — upload-once, delete-only). Use `bigint` for `size_bytes` (not `integer`). No custom ENUM type. + +--- + +### `backend/internal/db/queries/files.sql` (query, CRUD) + +**Analog:** `backend/internal/db/queries/tasks.sql` + +**Full query file** (lines 1–30): +```sql +-- name: ListTasksByTablo :many +SELECT id, tablo_id, title, description, status, position, created_at, updated_at +FROM tasks +WHERE tablo_id = $1 +ORDER BY status, position, created_at; + +-- name: InsertTask :one +INSERT INTO tasks (tablo_id, title, description, status, position) +VALUES ($1, $2, $3, $4, $5) +RETURNING id, tablo_id, title, description, status, position, created_at, updated_at; + +-- name: GetTaskByID :one +SELECT id, tablo_id, title, description, status, position, created_at, updated_at +FROM tasks +WHERE id = $1 AND tablo_id = $2; + +-- name: DeleteTask :exec +DELETE FROM tasks WHERE id = $1 AND tablo_id = $2; +``` + +**Conventions to copy:** +- `-- name: QueryName :return_type` annotation on every query (required by sqlc) +- `:one` for INSERT RETURNING + single SELECT; `:many` for list; `:exec` for DELETE +- Always include `tablo_id = $N` in WHERE clauses for ownership (IDOR prevention) +- RETURNING clause lists all columns explicitly (not `RETURNING *`) +- ORDER BY in list queries: `created_at DESC` (newest first) for files + +**Files query set needed:** `InsertTabloFile :one`, `ListFilesByTablo :many`, `GetTabloFileByID :one`, `DeleteTabloFile :exec` + +--- + +### `backend/internal/files/store.go` (service, file-I/O) + +**Analog:** No body analog in codebase (placeholder only at `backend/internal/files/doc.go`). Use RESEARCH.md Patterns 1–3 exclusively. + +**Package declaration** (`doc.go` line 1–2): +```go +// Package files is a Phase 1 placeholder; the upload/storage implementation lands in Phase 5. +package files +``` + +**Implementation pattern** from RESEARCH.md (Patterns 1–3 — verified against aws-sdk-go-v2 docs): + +Store struct and constructor: +```go +type Store struct { + client *s3.Client + bucket string +} + +func NewStore(ctx context.Context, endpoint, bucket, region, accessKey, secretKey string, usePathStyle bool) (*Store, error) { + cfg, err := config.LoadDefaultConfig(ctx, + config.WithRegion(region), + config.WithCredentialsProvider( + credentials.NewStaticCredentialsProvider(accessKey, secretKey, ""), + ), + ) + if err != nil { + return nil, err + } + client := s3.NewFromConfig(cfg, func(o *s3.Options) { + o.BaseEndpoint = aws.String(endpoint) + o.UsePathStyle = usePathStyle // true for MinIO, false or omit for R2 + }) + return &Store{client: client, bucket: bucket}, nil +} +``` + +Upload (sniff + stream pattern): +```go +func (s *Store) Upload(ctx context.Context, key string, file io.Reader) (contentType string, bytesWritten int64, err error) { + var sniffBuf [512]byte + n, readErr := io.ReadFull(file, sniffBuf[:]) + // Accept io.ErrUnexpectedEOF — normal for files < 512 bytes (Pitfall 3) + if readErr != nil && !errors.Is(readErr, io.ErrUnexpectedEOF) { + return "", 0, readErr + } + contentType = http.DetectContentType(sniffBuf[:n]) + body := io.MultiReader(bytes.NewReader(sniffBuf[:n]), file) + // Wrap body in a counting reader before passing to PutObject (Pitfall 8) + // ... +} +``` + +PresignDownload: +```go +func (s *Store) PresignDownload(ctx context.Context, key string) (string, error) { + presignClient := s3.NewPresignClient(s.client) + req, err := presignClient.PresignGetObject(ctx, &s3.GetObjectInput{ + Bucket: aws.String(s.bucket), + Key: aws.String(key), + }, func(o *s3.PresignOptions) { + o.Expires = 5 * time.Minute + }) + if err != nil { + return "", err + } + return req.URL, nil +} +``` + +Delete: +```go +func (s *Store) Delete(ctx context.Context, key string) error { + _, err := s.client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: aws.String(s.bucket), + Key: aws.String(key), + }) + return err +} +``` + +**Interface for test injection** (per RESEARCH.md validation architecture): +```go +type FileStorer interface { + Upload(ctx context.Context, key string, file io.Reader) (contentType string, bytesWritten int64, err error) + Delete(ctx context.Context, key string) error + PresignDownload(ctx context.Context, key string) (string, error) +} +``` + +--- + +### `backend/internal/web/handlers_files.go` (controller, request-response) + +**Analog:** `backend/internal/web/handlers_tasks.go` + +**Imports pattern** (lines 1–19 of handlers_tasks.go): +```go +package web + +import ( + "errors" + "log/slog" + "net/http" + + "backend/internal/auth" + "backend/internal/db/sqlc" + "backend/templates" + + "github.com/go-chi/chi/v5" + "github.com/google/uuid" + "github.com/gorilla/csrf" + "github.com/jackc/pgx/v5" +) +``` + +**FilesDeps struct pattern** (mirrors TasksDeps, lines 21–24 of handlers_tasks.go): +```go +// TasksDeps holds dependencies for all task handlers. +type TasksDeps struct { + Queries *sqlc.Queries +} +``` +Copy this exactly: +```go +// FilesDeps holds dependencies for all file handlers. +type FilesDeps struct { + Queries *sqlc.Queries + Files FileStorer // interface — allows stub injection in tests + MaxUploadMB int // parsed from MAX_UPLOAD_SIZE_MB env var (default 25) +} +``` + +**loadOwnedTabloForTask pattern** — copy for `loadOwnedTabloForFile` (lines 44–76): +```go +func loadOwnedTabloForTask(w http.ResponseWriter, r *http.Request, deps TasksDeps) (sqlc.Tablo, sqlc.Task, *auth.User, bool) { + tablo, user, ok := loadOwnedTablo(w, r, TablosDeps{Queries: deps.Queries}) + if !ok { + return sqlc.Tablo{}, sqlc.Task{}, nil, false + } + taskID, err := uuid.Parse(chi.URLParam(r, "task_id")) + if err != nil { + http.NotFound(w, r) + return sqlc.Tablo{}, sqlc.Task{}, nil, false + } + task, err := deps.Queries.GetTaskByID(r.Context(), sqlc.GetTaskByIDParams{ + ID: taskID, + TabloID: tablo.ID, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.NotFound(w, r) + return sqlc.Tablo{}, sqlc.Task{}, nil, false + } + slog.Default().Error("tasks: GetTaskByID failed", "id", taskID, "err", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return sqlc.Tablo{}, sqlc.Task{}, nil, false + } + return tablo, task, user, true +} +``` + +**Handler constructor pattern** — all handlers follow this shape (lines 80–98): +```go +func TaskNewFormHandler(deps TasksDeps) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + tablo, _, ok := loadOwnedTablo(w, r, TablosDeps{Queries: deps.Queries}) + if !ok { + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _ = templates.TaskCreateFormFragment(...).Render(r.Context(), w) + } +} +``` + +**HX-Request detection pattern** (lines 150–166 of handlers_tasks.go): +```go +if r.Header.Get("HX-Request") == "true" { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("HX-Retarget", "#add-task-slot-"+statusStr) + w.Header().Set("HX-Reswap", "innerHTML") + w.WriteHeader(http.StatusUnprocessableEntity) + _ = templates.TaskCreateFormFragment(...).Render(ctx, w) + return +} +http.Redirect(w, r, "/tablos/"+tablo.ID.String(), http.StatusSeeOther) +``` + +**Delete handler pattern** (lines 285–309 of handlers_tasks.go): +```go +func TaskDeleteHandler(deps TasksDeps) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + tablo, task, _, ok := loadOwnedTabloForTask(w, r, deps) + if !ok { + return + } + if err := deps.Queries.DeleteTask(r.Context(), sqlc.DeleteTaskParams{ + ID: task.ID, + TabloID: tablo.ID, + }); err != nil { + slog.Default().Error("tasks delete: DeleteTask failed", "id", task.ID, "err", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if r.Header.Get("HX-Request") == "true" { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _ = templates.TaskCardGone(task.ID).Render(r.Context(), w) + return + } + http.Redirect(w, r, "/tablos/"+tablo.ID.String(), http.StatusSeeOther) + } +} +``` + +**FileDeleteHandler deviation:** Delete S3 first (log error, do not abort), always delete DB row. The file-specific ordering is: `deps.Files.Delete(ctx, file.S3Key)` → log if error → `deps.Queries.DeleteTabloFile(...)` → 500 if DB fails. See RESEARCH.md Pattern 6. + +**Upload handler additions** not present in tasks.go: +- `r.Body = http.MaxBytesReader(w, r.Body, maxBytes)` — MUST be first line (before ParseMultipartForm) +- `r.ParseMultipartForm(2 << 20)` + `errors.As(err, &(*http.MaxBytesError)(nil))` for size error detection +- `r.FormFile("file")` + `io.ReadFull` sniff + `io.MultiReader` reconstruct +- `uuid.New()` for S3 key: `"files/" + tablo.ID.String() + "/" + fileUUID.String()` + +--- + +### `backend/internal/web/router.go` (config, request-response — modify) + +**Analog:** `backend/internal/web/router.go` (self) + +**Current NewRouter signature** (line 47): +```go +func NewRouter(pinger Pinger, staticDir string, deps AuthDeps, tabloDeps TablosDeps, taskDeps TasksDeps, csrfKey []byte, env string, trustedOrigins ...string) http.Handler { +``` + +**Updated signature** (add `fileDeps FilesDeps` before `csrfKey`): +```go +func NewRouter(pinger Pinger, staticDir string, deps AuthDeps, tabloDeps TablosDeps, taskDeps TasksDeps, fileDeps FilesDeps, csrfKey []byte, env string, trustedOrigins ...string) http.Handler { +``` + +**Route registration pattern** (lines 93–106 of router.go — task routes as model): +```go +// Task routes — static segments BEFORE parametric (Pitfall 1). +r.Get("/tablos/{id}/tasks/new", TaskNewFormHandler(taskDeps)) +r.Get("/tablos/{id}/tasks/cancel-new", TaskCancelNewHandler(taskDeps)) +r.Post("/tablos/{id}/tasks", TaskCreateHandler(taskDeps)) +r.Post("/tablos/{id}/tasks/reorder", TaskReorderHandler(taskDeps)) +// Parametric task routes — must come after static task segments. +r.Get("/tablos/{id}/tasks/{task_id}/show", TaskShowHandler(taskDeps)) +r.Get("/tablos/{id}/tasks/{task_id}/edit", TaskEditHandler(taskDeps)) +r.Post("/tablos/{id}/tasks/{task_id}", TaskUpdateHandler(taskDeps)) +r.Get("/tablos/{id}/tasks/{task_id}/delete-confirm", TaskDeleteConfirmHandler(taskDeps)) +r.Post("/tablos/{id}/tasks/{task_id}/delete", TaskDeleteHandler(taskDeps)) +``` + +**New routes to add** (insert after existing task routes, following same static-before-parametric rule): +```go +// Tab entry points — GET /tablos/{id}/tasks (Tasks tab) must come BEFORE +// /tablos/{id}/tasks/new and other static task sub-routes (chi Pitfall 1). +r.Get("/tablos/{id}/tasks", TabloTasksTabHandler(taskDeps)) // NEW — Tasks tab entry + +// File routes — static segments BEFORE parametric (Pitfall 6 in RESEARCH). +r.Get("/tablos/{id}/files", TabloFilesTabHandler(fileDeps)) +r.Post("/tablos/{id}/files", FileUploadHandler(fileDeps)) +// Parametric file routes — AFTER static file segment. +r.Get("/tablos/{id}/files/{file_id}/download", FileDownloadHandler(fileDeps)) +r.Get("/tablos/{id}/files/{file_id}/delete-confirm", FileDeleteConfirmHandler(fileDeps)) +r.Post("/tablos/{id}/files/{file_id}/delete", FileDeleteHandler(fileDeps)) +``` + +**IMPORTANT:** `r.Get("/tablos/{id}/tasks", ...)` must be registered BEFORE `r.Get("/tablos/{id}/tasks/new", ...)` — insert it at line 96 (before the existing task block). + +--- + +### `backend/templates/tablos.templ` (component, request-response — modify) + +**Analog:** `backend/templates/tablos.templ` (self) + +**Current TabloDetailPage signature** (line 173): +```go +templ TabloDetailPage(user *auth.User, csrfToken string, tablo sqlc.Tablo, tasks []sqlc.Task) { +``` + +**Updated signature** (add files + activeTab trailing params — Pitfall 4 in RESEARCH): +```go +templ TabloDetailPage(user *auth.User, csrfToken string, tablo sqlc.Tablo, tasks []sqlc.Task, files []sqlc.TabloFile, activeTab string) { +``` + +**All call sites that must be updated:** +- `handlers_tablos.go` line 205: `templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks)` → `templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, nil, "overview")` +- `handlers_tablos.go` line 311: same update +- Any test files calling `TabloDetailPage` + +**Current detail page body** (lines 174–191) for reference — the tab bar wraps the existing zones: +```go +templ TabloDetailPage(user *auth.User, csrfToken string, tablo sqlc.Tablo, tasks []sqlc.Task) { + @Layout("Tablos — Xtablo", user, csrfToken) { +
+ ← Back to tablos +
+
+ @TabloTitleDisplay(tablo, csrfToken) +
+
+ @TabloDescDisplay(tablo, csrfToken) +
+
+ @TabloDeleteButtonFragment(tablo, csrfToken) +
+
+ @KanbanBoard(tablo.ID, csrfToken, tasks) +
+ } +} +``` + +**Tab bar pattern** — insert between delete-zone and content area. Tab links use `hx-get` + `hx-target="#tab-content"` + `hx-push-url`: +```html + +
+ +
+``` + +**Fragment-only components needed:** `TabloOverviewTabFragment`, `TabloTasksTabFragment`, `TabloFilesTabFragment` — each is what the `#tab-content` div contains when that tab is active. These are rendered either as the full-page content area (initial load) or as standalone fragments (HTMX request). + +--- + +### `backend/templates/files.templ` (component, request-response — new) + +**Analog:** `backend/templates/tasks.templ` + +**Package/import pattern** (lines 1–8 of tasks.templ): +```go +package templates + +import ( + "backend/internal/db/sqlc" + "backend/internal/web/ui" + "github.com/google/uuid" +) +``` + +**Delete confirmation pattern** (lines 285–326 of tasks.templ — copy structure for file delete confirm): +```go +templ TaskDeleteConfirmFragment(tabloID uuid.UUID, task sqlc.Task, csrfToken string) { +
+
+

Delete task?

+

This cannot be undone.

+
+
+ @ui.CSRFField(csrfToken) + @ui.Button(ui.ButtonProps{ + Label: "Yes, delete", + Variant: ui.ButtonVariantDanger, + Tone: ui.ButtonToneSolid, + ... + }) +
+ @ui.Button(ui.ButtonProps{ + Label: "Keep task", + Variant: ui.ButtonVariantNeutral, + Tone: ui.ButtonToneSoft, + ... + }) +
+
+
+} +``` + +**ui.Card pattern** for file list rows (from tablos.templ lines 70–93): +```go +@ui.Card(templ.Attributes{"id": "tablo-" + tablo.ID.String()}) { +
+ ... +
+} +``` + +**OOB removal pattern** (from TaskCardGone, lines 343–345 of tasks.templ): +```go +templ TaskCardGone(taskID uuid.UUID) { +
+} +``` +File analog: `FileRowGone(fileID uuid.UUID)` — same empty-div-with-id pattern. + +**Upload form** uses `method="POST"` + `enctype="multipart/form-data"` + `hx-post` + `hx-encoding="multipart/form-data"` (HTMX multipart requires this attribute). Input: ``. Add `@ui.CSRFField(csrfToken)`. + +--- + +### `backend/cmd/web/main.go` (config — modify) + +**Analog:** `backend/cmd/web/main.go` (self) + +**Current deps wiring** (lines 78–82): +```go +deps := web.AuthDeps{Queries: q, Store: store, Secure: secure, Limiter: rl} +tabloDeps := web.TablosDeps{Queries: q} +taskDeps := web.TasksDeps{Queries: q} + +router := web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, csrfKey, env) +``` + +**Pattern for reading env vars** (lines 27–35 — copy pattern for S3 env vars): +```go +env := os.Getenv("ENV") +if env == "" { + env = "development" +} +port := os.Getenv("PORT") +if port == "" { + port = "8080" +} +dsn := os.Getenv("DATABASE_URL") +``` + +**Fast-fail pattern for required vars** (lines 41–44): +```go +if dsn == "" { + slog.Error("DATABASE_URL is required but unset") + os.Exit(1) +} +``` + +**S3 wiring to add** (insert before `router := ...`): +```go +// S3 / files store +s3Endpoint := os.Getenv("S3_ENDPOINT") +s3Bucket := os.Getenv("S3_BUCKET") +// ... read remaining vars ... +maxUploadMB := 25 +if v := os.Getenv("MAX_UPLOAD_SIZE_MB"); v != "" { + // strconv.Atoi + fallback +} +filesStore, err := files.NewStore(ctx, s3Endpoint, s3Bucket, ...) +if err != nil { + slog.Error("s3 client init failed", "err", err) + os.Exit(1) +} +fileDeps := web.FilesDeps{Queries: q, Files: filesStore, MaxUploadMB: maxUploadMB} + +router := web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, fileDeps, csrfKey, env) +``` + +--- + +### `compose.yaml` (config — modify) + +**Analog:** `backend/compose.yaml` (self) + +**Existing postgres service** (lines 1–20 — copy structure exactly): +```yaml +services: + postgres: + image: postgres:16-alpine + container_name: xtablo-backend-postgres + restart: unless-stopped + environment: + POSTGRES_DB: xtablo + POSTGRES_USER: xtablo + POSTGRES_PASSWORD: xtablo + ports: + - "5432:5432" + volumes: + - postgres_data:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U xtablo -d xtablo"] + interval: 5s + timeout: 5s + retries: 10 + +volumes: + postgres_data: +``` + +**MinIO services to add** (per RESEARCH.md Pattern 7 — verified against MinIO Docker Hub): +```yaml + minio: + image: minio/minio:latest + container_name: xtablo-backend-minio + restart: unless-stopped + environment: + MINIO_ROOT_USER: minioadmin + MINIO_ROOT_PASSWORD: minioadmin + ports: + - "9000:9000" # S3 API + - "9001:9001" # Console UI + command: server /data --console-address ":9001" + volumes: + - minio_data:/data + healthcheck: + test: ["CMD", "mc", "ready", "local"] + interval: 5s + timeout: 5s + retries: 10 + + minio-init: + image: minio/mc:latest + depends_on: + minio: + condition: service_healthy + entrypoint: > + /bin/sh -c " + mc alias set local http://minio:9000 minioadmin minioadmin; + mc mb --ignore-existing local/xtablo-dev; + echo 'bucket ready'; + " + restart: "no" # init container — must NOT be unless-stopped (Pitfall 7) +``` + +**Add `minio_data:` under `volumes:` block alongside `postgres_data:`.** + +--- + +## Shared Patterns + +### Auth / Ownership (loadOwnedTablo) +**Source:** `backend/internal/web/handlers_tablos.go` lines 150–182 +**Apply to:** All file handlers (TabloFilesTabHandler, FileUploadHandler, FileDownloadHandler, FileDeleteHandler, FileDeleteConfirmHandler) + +```go +func loadOwnedTablo(w http.ResponseWriter, r *http.Request, deps TablosDeps) (sqlc.Tablo, *auth.User, bool) { + _, user, _ := auth.Authed(r.Context()) + tabloID, err := uuid.Parse(chi.URLParam(r, "id")) + if err != nil { + http.NotFound(w, r) + return sqlc.Tablo{}, nil, false + } + tablo, err := deps.Queries.GetTabloByID(r.Context(), sqlc.GetTabloByIDParams{ID: tabloID, UserID: user.ID}) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.NotFound(w, r) + return sqlc.Tablo{}, nil, false + } + slog.Default().Error("tablos: GetTabloByID failed", "id", tabloID, "err", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return sqlc.Tablo{}, nil, false + } + return tablo, user, true +} +``` + +### Error Logging +**Source:** `backend/internal/web/handlers_tasks.go` (throughout) +**Apply to:** All handler error paths + +```go +slog.Default().Error("files upload: PutObject failed", "tablo_id", tablo.ID, "err", err) +http.Error(w, "internal server error", http.StatusInternalServerError) +``` +Convention: log key is `" : failed"`. Always include `"tablo_id"` and `"err"` keys. + +### HTMX Fragment vs Full-Page +**Source:** `backend/internal/web/handlers_tasks.go` (throughout), `backend/internal/web/handlers_tablos.go` +**Apply to:** TabloFilesTabHandler, TabloTasksTabHandler (tab entry points) + +```go +w.Header().Set("Content-Type", "text/html; charset=utf-8") +if r.Header.Get("HX-Request") == "true" { + _ = templates.FilesTabFragment(tablo, files, csrf.Token(r)).Render(r.Context(), w) + return +} +// Full page for direct navigation / browser refresh +_ = templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, files, "files").Render(r.Context(), w) +``` + +### CSRF Field in All Forms +**Source:** `backend/templates/tablos.templ` line 108, `backend/templates/tasks.templ` lines 35, 183, 244 +**Apply to:** All POST forms in files.templ (upload form, delete confirm form) + +```go +@ui.CSRFField(csrfToken) +``` + +### Post/Redirect/Get (non-HTMX fallback) +**Source:** `backend/internal/web/handlers_tasks.go` line 248, 307 +**Apply to:** FileUploadHandler, FileDeleteHandler non-HTMX paths + +```go +http.Redirect(w, r, "/tablos/"+tablo.ID.String()+"/files", http.StatusSeeOther) +// Always 303 SeeOther on POST success — NOT 302 (Pitfall 9) +``` + +### ui.Button component usage +**Source:** `backend/templates/tablos.templ` lines 339–353, `backend/templates/tasks.templ` lines 258–278 +**Apply to:** Upload form submit button, delete confirm buttons in files.templ + +```go +@ui.Button(ui.ButtonProps{ + Label: "Upload", + Variant: ui.ButtonVariantDefault, + Tone: ui.ButtonToneSolid, + Size: ui.SizeMD, + Type: "submit", +}) +@ui.Button(ui.ButtonProps{ + Label: "Yes, delete", + Variant: ui.ButtonVariantDanger, + Tone: ui.ButtonToneSolid, + Size: ui.SizeMD, + Type: "submit", +}) +``` + +### templ.SafeURL for action attributes +**Source:** `backend/templates/tablos.templ` lines 217, 367; `backend/templates/tasks.templ` line 177 +**Apply to:** All `action=` attributes in files.templ forms + +```go +action={ templ.SafeURL("/tablos/" + tablo.ID.String() + "/files") } +``` + +--- + +## No Analog Found + +No files in this phase lack a codebase analog. `backend/internal/files/store.go` has no body analog, but RESEARCH.md Patterns 1–3 provide verified, concrete implementation patterns with line-level specificity. + +--- + +## Metadata + +**Analog search scope:** `backend/` directory — migrations, queries, handlers, templates, cmd, compose.yaml +**Files scanned:** 12 source files read directly +**Pattern extraction date:** 2026-05-15 diff --git a/.planning/phases/05-files/05-RESEARCH.md b/.planning/phases/05-files/05-RESEARCH.md index a351f12..b554d0d 100644 --- a/.planning/phases/05-files/05-RESEARCH.md +++ b/.planning/phases/05-files/05-RESEARCH.md @@ -679,17 +679,19 @@ MAX_UPLOAD_SIZE_MB=25 | A3 | `multipart.FileHeader.Size` is unreliable (some clients send 0) | Pitfall 8, Code Examples | If reliable, a counting wrapper is unnecessary; but safest to always count bytes written | | A4 | MinIO `latest` image includes `mc` binary (for healthcheck `mc ready local`) | Pattern 7 compose.yaml | `mc` may not be bundled in `minio/minio` image; separate `minio/mc` init container is safer (used in Pattern 7) | -## Open Questions +## Open Questions (RESOLVED) 1. **Tab route for `/tablos/{id}/tasks` conflicts with existing `/tablos/{id}/tasks/*` child routes** - What we know: `GET /tablos/{id}/tasks` is NOT currently registered. All task routes are `GET/POST /tablos/{id}/tasks/...` (with a trailing segment). - What's unclear: chi v5's behavior when registering `GET /tablos/{id}/tasks` alongside `GET /tablos/{id}/tasks/new` and `POST /tablos/{id}/tasks/reorder`. Static + trailing segments should be fine, but the exact chi precedence should be verified. - Recommendation: Add `r.Get("/tablos/{id}/tasks", TabloTasksTabHandler(...))` BEFORE the existing task sub-routes. Write a test that `GET /tablos/{id}/tasks` returns 200 with the kanban board, and `GET /tablos/{id}/tasks/new` still returns the new-task form. + - **RESOLVED:** 05-02 Task 2 step 5 registers `GET /tablos/{id}/tasks` (via `TabloTasksTabHandler`) before existing task sub-routes in router.go, following chi's static-before-parametric discipline. Route ordering is enforced in the plan action. 2. **Byte-counting for `size_bytes` column** - What we know: `s3.PutObject` does not return the number of bytes written. `multipart.FileHeader.Size` may be 0. - What's unclear: The cleanest counting approach (wrap body in a custom `io.Reader` that increments a counter, vs. use `header.Size` as-is and accept that it may be browser-dependent). - Recommendation: Wrap the body with a simple `byteCountReader` before passing to `PutObject`. After upload, use the counter value for `InsertTabloFileParams.SizeBytes`. + - **RESOLVED:** 05-01 Task 1 step 5 implements `byteCountReader` in `store.go`, wrapping the multipart body before the `PutObject` call. Counter value is passed as `SizeBytes` in `InsertTabloFileParams`. ## Environment Availability diff --git a/.planning/phases/05-files/05-VALIDATION.md b/.planning/phases/05-files/05-VALIDATION.md new file mode 100644 index 0000000..445af01 --- /dev/null +++ b/.planning/phases/05-files/05-VALIDATION.md @@ -0,0 +1,84 @@ +--- +phase: 5 +slug: files +status: draft +nyquist_compliant: false +wave_0_complete: false +created: 2026-05-15 +--- + +# Phase 5 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | go test | +| **Config file** | none — built into Go toolchain | +| **Quick run command** | `cd backend && go test ./internal/... -run TestFile -timeout 30s` | +| **Full suite command** | `cd backend && go test ./... -timeout 60s` | +| **Estimated runtime** | ~15 seconds | + +--- + +## Sampling Rate + +- **After every task commit:** Run `cd backend && go test ./internal/... -run TestFile -timeout 30s` +- **After every plan wave:** Run `cd backend && go test ./... -timeout 60s` +- **Before `/gsd-verify-work`:** Full suite must be green +- **Max feedback latency:** 60 seconds + +--- + +## Per-Task Verification Map + +| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| +| 05-migration | — | 1 | FILE-01 | — | tablo_files schema correct | unit | `cd backend && go test ./internal/db/...` | ❌ W0 | ⬜ pending | +| 05-s3-client | — | 1 | FILE-02 | — | S3 client connects to MinIO | unit | `cd backend && go test ./internal/files/...` | ❌ W0 | ⬜ pending | +| 05-upload | — | 1 | FILE-02 | — | MaxBytesReader enforced, bytes stored | unit | `cd backend && go test ./internal/files/... -run TestUpload` | ❌ W0 | ⬜ pending | +| 05-list | — | 1 | FILE-03 | — | Files listed newest-first | unit | `cd backend && go test ./internal/web/... -run TestFilesList` | ❌ W0 | ⬜ pending | +| 05-download | — | 1 | FILE-04 | — | Signed URL generated, 302 redirect | unit | `cd backend && go test ./internal/web/... -run TestFilesDownload` | ❌ W0 | ⬜ pending | +| 05-delete | — | 1 | FILE-05 | — | DB row and S3 object both removed | unit | `cd backend && go test ./internal/web/... -run TestFilesDelete` | ❌ W0 | ⬜ pending | +| 05-authz | — | 2 | FILE-06 | — | Non-owner gets 403 | unit | `cd backend && go test ./internal/web/... -run TestFilesOwnership` | ❌ W0 | ⬜ pending | +| 05-tabs | — | 2 | FILE-01 | — | Tab routes render correct fragment | unit | `cd backend && go test ./internal/web/... -run TestTabloDetailTabs` | ❌ W0 | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +--- + +## Wave 0 Requirements + +- [ ] `backend/internal/files/store_test.go` — stubs for FILE-02, FILE-04, FILE-05 +- [ ] `backend/internal/web/handlers_files_test.go` — stubs for FILE-01, FILE-03, FILE-06 +- [ ] MinIO running in compose.yaml — integration tests need the local S3 endpoint + +*If none: "Existing infrastructure covers all phase requirements."* + +--- + +## Manual-Only Verifications + +| Behavior | Requirement | Why Manual | Test Instructions | +|----------|-------------|------------|-------------------| +| File upload form renders correctly in browser | FILE-01 | Visual layout cannot be asserted in go test | Open `/tablos/{id}/files`, attach a file, verify form submits and file appears in list | +| Tab navigation updates URL via hx-push-url | FILE-01 | Browser URL state not testable in unit tests | Click Tasks tab, verify URL changes to `/tablos/{id}/tasks`; hit Back, verify Overview tab reactivates | +| 25MB upload size enforcement shows friendly error | FILE-02 | Browser file size display is visual | Attempt to upload a >25MB file; verify error message appears above the form | +| Download opens file correctly via 302 redirect | FILE-04 | Actual browser navigation/download | Click download link; verify browser downloads the file within signed URL TTL | + +--- + +## Validation Sign-Off + +- [ ] All tasks have `` verify or Wave 0 dependencies +- [ ] Sampling continuity: no 3 consecutive tasks without automated verify +- [ ] Wave 0 covers all MISSING references +- [ ] No watch-mode flags +- [ ] Feedback latency < 60s +- [ ] `nyquist_compliant: true` set in frontmatter + +**Approval:** pending