From bc94257ad7325a8ea3bff94cc234be0764d15526 Mon Sep 17 00:00:00 2001 From: anishalle Date: Fri, 19 Jun 2026 17:14:40 -0500 Subject: [PATCH 1/4] feat: add superadmin endpoint to rebalance scan_stats cache --- claude.md | 2 +- cmd/api/api.go | 5 +++ cmd/api/scans.go | 27 ++++++++++++++ cmd/api/scans_test.go | 72 ++++++++++++++++++++++++++++++++++++ docs/docs.go | 58 +++++++++++++++++++++++++++++ internal/store/mock_store.go | 8 ++++ internal/store/scans.go | 62 +++++++++++++++++++++++++++++++ internal/store/storage.go | 1 + 8 files changed, 234 insertions(+), 1 deletion(-) diff --git a/claude.md b/claude.md index 28be66c6..b4686305 100644 --- a/claude.md +++ b/claude.md @@ -163,5 +163,5 @@ Runs on every push/PR to `main` (`.github/workflows/audit.yaml`): **Auth:** `GET /v1/auth/check-email`, `GET /v1/auth/me` **Hacker:** `GET|PATCH /v1/applications/me`, `POST /v1/applications/me/submit` **Admin:** `GET /v1/admin/applications`, `GET /v1/admin/applications/stats`, `GET /v1/admin/applications/{id}`, `GET /v1/admin/applications/{id}/notes`, `GET /v1/admin/reviews/pending`, `GET /v1/admin/reviews/completed`, `GET /v1/admin/reviews/next`, `PUT /v1/admin/reviews/{id}`, `GET /v1/admin/scans/types`, `POST /v1/admin/scans`, `GET /v1/admin/scans/user/{userID}`, `GET /v1/admin/scans/stats` -**Super Admin:** `GET|PUT /v1/superadmin/settings/saquestions`, `GET|POST /v1/superadmin/settings/reviews-per-app`, `GET|POST /v1/superadmin/settings/review-assignment-toggle`, `GET|POST /v1/superadmin/settings/admin-schedule-edit-toggle`, `POST /v1/superadmin/applications/assign`, `PATCH /v1/superadmin/applications/{id}/status`, `GET /v1/superadmin/applications/emails`, `PUT /v1/superadmin/settings/scan-types`, `POST /v1/superadmin/emails/qr` +**Super Admin:** `GET|PUT /v1/superadmin/settings/saquestions`, `GET|POST /v1/superadmin/settings/reviews-per-app`, `GET|POST /v1/superadmin/settings/review-assignment-toggle`, `GET|POST /v1/superadmin/settings/admin-schedule-edit-toggle`, `POST /v1/superadmin/applications/assign`, `PATCH /v1/superadmin/applications/{id}/status`, `GET /v1/superadmin/applications/emails`, `PUT /v1/superadmin/settings/scan-types`, `POST /v1/superadmin/scans/rebalance-stats`, `POST /v1/superadmin/emails/qr` **Infra (Basic Auth):** `GET /v1/health`, `GET /v1/debug/vars`, `GET /v1/swagger/*` diff --git a/cmd/api/api.go b/cmd/api/api.go index 76296357..cf2e87fa 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -96,6 +96,7 @@ const swaggerTagsSorter = `(a, b) => { "admin/schedule", "admin/sponsors", "superadmin/applications", + "superadmin/scans", "superadmin/settings", "superadmin/users" ]; @@ -271,6 +272,10 @@ func (app *application) mount() http.Handler { r.Get("/", app.searchUsersHandler) r.Patch("/{userID}/role", app.updateUserRoleHandler) }) + + r.Route("/scans", func(r chi.Router) { + r.Post("/rebalance-stats", app.rebalanceScanStatsHandler) + }) }) }) }) diff --git a/cmd/api/scans.go b/cmd/api/scans.go index 62064c20..25b05507 100644 --- a/cmd/api/scans.go +++ b/cmd/api/scans.go @@ -209,6 +209,33 @@ func (app *application) getScanStatsHandler(w http.ResponseWriter, r *http.Reque } } +// rebalanceScanStatsHandler recomputes the scan_stats counter cache from the scans table +// +// @Summary Rebalance scan statistics (Super Admin) +// @Description Recomputes the scan_stats counter cache from the authoritative scans table and returns the recomputed stats +// @Tags superadmin/scans +// @Produce json +// @Success 200 {object} ScanStatsResponse +// @Failure 401 {object} object{error=string} +// @Failure 403 {object} object{error=string} +// @Failure 500 {object} object{error=string} +// @Security CookieAuth +// @Router /superadmin/scans/rebalance-stats [post] +func (app *application) rebalanceScanStatsHandler(w http.ResponseWriter, r *http.Request) { + stats, err := app.store.Scans.RebalanceStats(r.Context()) + if err != nil { + app.internalServerError(w, r, err) + return + } + + admin := getUserFromContext(r.Context()) + app.logger.Infow("scan stats rebalanced", "admin_id", admin.ID) + + if err := app.jsonResponse(w, http.StatusOK, ScanStatsResponse{Stats: stats}); err != nil { + app.internalServerError(w, r, err) + } +} + // updateScanTypesHandler replaces all scan types with the provided array // // @Summary Update scan types (Super Admin) diff --git a/cmd/api/scans_test.go b/cmd/api/scans_test.go index 61f7487e..c4c8b8cc 100644 --- a/cmd/api/scans_test.go +++ b/cmd/api/scans_test.go @@ -2,6 +2,7 @@ package main import ( "encoding/json" + "errors" "net/http" "strings" "testing" @@ -286,6 +287,77 @@ func TestGetScanStats(t *testing.T) { }) } +func TestRebalanceScanStats(t *testing.T) { + t.Run("super admin recomputes stats", func(t *testing.T) { + app := newTestApplication(t) + mockScans := app.store.Scans.(*store.MockScansStore) + + stats := []store.ScanStat{ + {ScanType: "check_in", Count: 42}, + {ScanType: "lunch", Count: 17}, + } + + mockScans.On("RebalanceStats").Return(stats, nil).Once() + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, newSuperAdminUser()) + + rr := executeRequest(req, http.HandlerFunc(app.rebalanceScanStatsHandler)) + checkResponseCode(t, http.StatusOK, rr.Code) + + var body struct { + Data ScanStatsResponse `json:"data"` + } + err = json.NewDecoder(rr.Body).Decode(&body) + require.NoError(t, err) + assert.Len(t, body.Data.Stats, 2) + assert.Equal(t, "check_in", body.Data.Stats[0].ScanType) + assert.Equal(t, 42, body.Data.Stats[0].Count) + + mockScans.AssertExpectations(t) + }) + + t.Run("500 on store error", func(t *testing.T) { + app := newTestApplication(t) + mockScans := app.store.Scans.(*store.MockScansStore) + + mockScans.On("RebalanceStats").Return(nil, errors.New("db error")).Once() + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, newSuperAdminUser()) + + rr := executeRequest(req, http.HandlerFunc(app.rebalanceScanStatsHandler)) + checkResponseCode(t, http.StatusInternalServerError, rr.Code) + + mockScans.AssertExpectations(t) + }) + + t.Run("403 when admin (non-super-admin)", func(t *testing.T) { + app := newTestApplication(t) + handler := app.RequireRoleMiddleware(store.RoleSuperAdmin)(http.HandlerFunc(app.rebalanceScanStatsHandler)) + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, newAdminUser()) + + rr := executeRequest(req, handler) + checkResponseCode(t, http.StatusForbidden, rr.Code) + }) + + t.Run("401 when unauthenticated", func(t *testing.T) { + app := newTestApplication(t) + handler := app.RequireRoleMiddleware(store.RoleSuperAdmin)(http.HandlerFunc(app.rebalanceScanStatsHandler)) + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + + rr := executeRequest(req, handler) + checkResponseCode(t, http.StatusUnauthorized, rr.Code) + }) +} + func TestUpdateScanTypes(t *testing.T) { t.Run("success", func(t *testing.T) { app := newTestApplication(t) diff --git a/docs/docs.go b/docs/docs.go index b6525316..246b1763 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -2819,6 +2819,64 @@ const docTemplate = `{ } } }, + "/superadmin/scans/rebalance-stats": { + "post": { + "security": [ + { + "CookieAuth": [] + } + ], + "description": "Recomputes the scan_stats counter cache from the authoritative scans table and returns the recomputed stats", + "produces": [ + "application/json" + ], + "tags": [ + "superadmin/scans" + ], + "summary": "Rebalance scan statistics (Super Admin)", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/main.ScanStatsResponse" + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + }, + "403": { + "description": "Forbidden", + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + }, "/superadmin/settings/admin-schedule-edit-toggle": { "get": { "security": [ diff --git a/internal/store/mock_store.go b/internal/store/mock_store.go index 0469a60e..b95265d4 100644 --- a/internal/store/mock_store.go +++ b/internal/store/mock_store.go @@ -347,6 +347,14 @@ func (m *MockScansStore) HasCheckIn(ctx context.Context, userID string, checkInT return args.Bool(0), args.Error(1) } +func (m *MockScansStore) RebalanceStats(ctx context.Context) ([]ScanStat, error) { + args := m.Called() + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]ScanStat), args.Error(1) +} + // MockScheduleStore is a mock implementation of the Schedule interface type MockScheduleStore struct { mock.Mock diff --git a/internal/store/scans.go b/internal/store/scans.go index 310944a6..efe5cc89 100644 --- a/internal/store/scans.go +++ b/internal/store/scans.go @@ -171,3 +171,65 @@ func (s *ScansStore) HasCheckIn(ctx context.Context, userID string, checkInTypes return exists, nil } + +// RebalanceStats recomputes the scan_stats counter cache from the authoritative +// scans table and returns the recomputed stats (sorted by scan_type, matching +// GetStats). The settings row is locked FOR UPDATE to serialize against +// concurrent incrementScanStat calls. +func (s *ScansStore) RebalanceStats(ctx context.Context) ([]ScanStat, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return nil, err + } + defer tx.Rollback() + + if _, err := tx.ExecContext(ctx, `SELECT value FROM settings WHERE key = $1 FOR UPDATE`, SettingsKeyScanStats); err != nil { + return nil, err + } + + rows, err := tx.QueryContext(ctx, `SELECT scan_type, COUNT(*) FROM scans GROUP BY scan_type`) + if err != nil { + return nil, err + } + defer rows.Close() + + statsMap := make(map[string]int) + for rows.Next() { + var scanType string + var count int + if err := rows.Scan(&scanType, &count); err != nil { + return nil, err + } + statsMap[scanType] = count + } + if err := rows.Err(); err != nil { + return nil, err + } + + value, err := json.Marshal(statsMap) + if err != nil { + return nil, err + } + + if _, err := tx.ExecContext(ctx, `UPDATE settings SET value = $1, updated_at = NOW() WHERE key = $2`, value, SettingsKeyScanStats); err != nil { + return nil, err + } + + if err := tx.Commit(); err != nil { + return nil, err + } + + stats := make([]ScanStat, 0, len(statsMap)) + for scanType, count := range statsMap { + stats = append(stats, ScanStat{ScanType: scanType, Count: count}) + } + + sort.Slice(stats, func(i, j int) bool { + return stats[i].ScanType < stats[j].ScanType + }) + + return stats, nil +} diff --git a/internal/store/storage.go b/internal/store/storage.go index 7b396d48..a09f2c53 100644 --- a/internal/store/storage.go +++ b/internal/store/storage.go @@ -64,6 +64,7 @@ type Storage struct { GetByUserID(ctx context.Context, userID string) ([]Scan, error) GetStats(ctx context.Context) ([]ScanStat, error) HasCheckIn(ctx context.Context, userID string, checkInTypes []string) (bool, error) + RebalanceStats(ctx context.Context) ([]ScanStat, error) } ApplicationReviews interface { SubmitVote(ctx context.Context, reviewID string, adminID string, vote ReviewVote, notes *string) (*ApplicationReview, error) From 4b0aa8a3a950eaa2d1ead597fe2c10f0a31d2c46 Mon Sep 17 00:00:00 2001 From: anishalle Date: Fri, 19 Jun 2026 17:23:13 -0500 Subject: [PATCH 2/4] docs: restore concurrency caveat comment on RebalanceStats --- internal/store/scans.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/store/scans.go b/internal/store/scans.go index efe5cc89..b51f4698 100644 --- a/internal/store/scans.go +++ b/internal/store/scans.go @@ -176,6 +176,10 @@ func (s *ScansStore) HasCheckIn(ctx context.Context, userID string, checkInTypes // scans table and returns the recomputed stats (sorted by scan_type, matching // GetStats). The settings row is locked FOR UPDATE to serialize against // concurrent incrementScanStat calls. +// +// Concurrency caveat: a scan insert that commits between the COUNT(*) query and +// the FOR UPDATE lock acquisition could be missed. This is acceptable for a +// manually triggered rebalance. func (s *ScansStore) RebalanceStats(ctx context.Context) ([]ScanStat, error) { ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) defer cancel() From 9512843642afba66e2a7467f6b9c74e1ab81b009 Mon Sep 17 00:00:00 2001 From: Caleb Bae Date: Sun, 21 Jun 2026 11:53:35 -0700 Subject: [PATCH 3/4] feat(FE): rebalance frontend --- .claude/commands/ci-audit.md | 27 + .claude/commands/create-task.md | 84 +++ .claude/skills/backend/SKILL.md | 70 +++ .../backend/references/bulk-operations.md | 254 ++++++++++ .../backend/references/crud-resource.md | 386 ++++++++++++++ .../skills/backend/references/file-storage.md | 309 +++++++++++ .../backend/references/list-pagination.md | 291 +++++++++++ .claude/skills/backend/references/overview.md | 220 ++++++++ .../skills/backend/references/public-api.md | 124 +++++ .../backend/references/self-resource.md | 289 +++++++++++ .../backend/references/settings-pattern.md | 153 ++++++ .../backend/references/status-update.md | 206 ++++++++ .claude/skills/backend/references/testing.md | 283 +++++++++++ .../backend/references/workflow-queue.md | 258 ++++++++++ .claude/skills/frontend-design/SKILL.md | 290 +++++++++++ .claude/skills/frontend/SKILL.md | 158 ++++++ .../skills/frontend/references/new-page.md | 359 +++++++++++++ .claude/skills/skill-creator/SKILL.md | 479 ++++++++++++++++++ .gitignore | 2 +- claude.md | 2 +- .../web/src/pages/admin/scans/ScansPage.tsx | 4 + client/web/src/pages/admin/scans/api.ts | 11 + .../admin/scans/components/ScanTypesTable.tsx | 54 +- client/web/src/pages/admin/scans/store.ts | 23 + cmd/api/api.go | 6 +- cmd/api/scans.go | 6 +- cmd/api/scans_test.go | 12 +- docs/docs.go | 116 ++--- 28 files changed, 4398 insertions(+), 78 deletions(-) create mode 100644 .claude/commands/ci-audit.md create mode 100644 .claude/commands/create-task.md create mode 100644 .claude/skills/backend/SKILL.md create mode 100644 .claude/skills/backend/references/bulk-operations.md create mode 100644 .claude/skills/backend/references/crud-resource.md create mode 100644 .claude/skills/backend/references/file-storage.md create mode 100644 .claude/skills/backend/references/list-pagination.md create mode 100644 .claude/skills/backend/references/overview.md create mode 100644 .claude/skills/backend/references/public-api.md create mode 100644 .claude/skills/backend/references/self-resource.md create mode 100644 .claude/skills/backend/references/settings-pattern.md create mode 100644 .claude/skills/backend/references/status-update.md create mode 100644 .claude/skills/backend/references/testing.md create mode 100644 .claude/skills/backend/references/workflow-queue.md create mode 100644 .claude/skills/frontend-design/SKILL.md create mode 100644 .claude/skills/frontend/SKILL.md create mode 100644 .claude/skills/frontend/references/new-page.md create mode 100644 .claude/skills/skill-creator/SKILL.md diff --git a/.claude/commands/ci-audit.md b/.claude/commands/ci-audit.md new file mode 100644 index 00000000..327db0af --- /dev/null +++ b/.claude/commands/ci-audit.md @@ -0,0 +1,27 @@ +6s +Run npm run build + +> web@0.0.0 build +> tsc -b && vite build + +Error: src/pages/admin/assigned/components/ApplicationDetailsPanel.tsx(57,7): error TS2722: Cannot invoke an object which is possibly 'undefined'. +Error: Process completed with exit code 2.Run all CI audit checks locally to catch issues before pushing. Go through each check sequentially and report results. Fix any issues you find automatically. + +## Backend checks (from repo root) + +1. **gofmt** — Run `gofmt -l .` and verify no files are unformatted. If any are, run `gofmt -w .` to fix them. +2. **go mod verify** — Run `go mod verify`. +3. **go build** — Run `go build -v ./...`. +4. **go vet** — Run `go vet ./...`. +5. **go test** — Run `go test -race ./...`. + +## Frontend checks (from `client/web/`) + +6. **Format check** — Run `npm run format:check`. If it fails, run `npm run format` to fix, then re-check. +7. **Lint** — Run `npm run lint`. If it fails, report the errors and attempt to fix them. +8. **Build** — Run `npm run build` (includes TypeScript type checking). +9. **Dependency audit** — Run `npm audit --audit-level=high --omit=dev`. + +## Output + +After all checks complete, print a summary table showing each check's pass/fail status. If any check failed and could not be auto-fixed, explain what needs to be done. diff --git a/.claude/commands/create-task.md b/.claude/commands/create-task.md new file mode 100644 index 00000000..496b1d7d --- /dev/null +++ b/.claude/commands/create-task.md @@ -0,0 +1,84 @@ +# Create Task + +Create a new task in both ClickUp (Tech list) and GitHub Issues, then link them together. + +## User Mapping + +Use these mappings to resolve assignees across platforms. **Edit the PLACEHOLDER values below with real usernames.** + +| Name | GitHub Username | ClickUp Name/Email | +| -------- | ---------------- | ----------------------------------- | +| Me | balebbae | caleb.bae@acmutd.co | +| Anish | anishalle | anish.alle@acmutd.co | +| Arnav | axv2655 | arnav.vedula@acmutd.co | +| Jagadeep | jagadeep298218 | jagadeep.kalluri@acmutd.co | +| Noel | NoelVarghese2006 | noel.varghese@acmutd.co | +| Sree | 5sansiva | sreevasan.sivasubramanian@acmutd.co | +| Tharun | tharunthunder | tharun.sevvel@acmutd.co | + +## Instructions + +You are creating a task. Follow these steps exactly: + +### Step 1: Gather task details + +Ask the user for: + +- **Task name** (required) +- **Description** (required) +- **Priority** — urgent, high, normal, or low (default: normal) +- **Labels** — e.g. enhancement, bug, good first issue (default: enhancement) + +### Step 2: Ask for assignee + +Use the AskUserQuestion tool to ask who this task should be assigned to. The options are: + +- Me +- Anish +- Arnav +- Jagadeep +- Noel +- Sree +- Tharun +- No one + +### Step 3: Resolve the assignee + +Using the user mapping table above, resolve the selected name to: + +- A **GitHub username** for the `--assignee` flag on `gh issue create` +- A **ClickUp name/email** for the `clickup_resolve_assignees` / `clickup_create_task` assignee field + +If "No one" is selected, skip assignment on both platforms. + +### Step 4: Create the GitHub issue + +Run: + +``` +gh issue create --title "" --body "" --label "" [--assignee ""] +``` + +Capture the resulting issue URL and number. + +### Step 5: Create the ClickUp task + +Use the `clickup_create_task` tool with: + +- **list_id:** `901710506578` (Tech list in HackUTD) +- **name:** the task name +- **description:** the task description, with the GitHub issue URL appended at the bottom +- **priority:** the selected priority +- **assignees:** the resolved ClickUp user ID (use `clickup_resolve_assignees` first), or omit if "No one" + +### Step 6: Link them together + +Use `clickup_create_task_comment` on the newly created ClickUp task to add a comment: + +``` +GitHub Issue: +``` + +### Step 7: Confirm + +Tell the user the task was created with links to both the ClickUp task and the GitHub issue. diff --git a/.claude/skills/backend/SKILL.md b/.claude/skills/backend/SKILL.md new file mode 100644 index 00000000..7feb67a6 --- /dev/null +++ b/.claude/skills/backend/SKILL.md @@ -0,0 +1,70 @@ +--- +name: backend +description: "HARP Go backend development guide. Generates handlers, store implementations, migrations, mock stores, routes, and tests following the established conventions. Use this skill whenever implementing new backend features, adding API endpoints, creating database tables, writing Go handler tests, or modifying the store layer. Also use when the user asks to add a new resource/entity, CRUD endpoints, or admin routes to the Go backend." +--- + +# HARP Go Backend Development Guide + +This skill orchestrates pattern selection for Go backend work. The codebase has distinct endpoint patterns (CRUD, self-resource, paginated list, workflow queue, file storage, public API, bulk operations, status update, settings) — each with its own conventions documented in a reference file. Before writing code, identify the pattern that matches the request and read the matching reference plus `overview.md`. + +## Step 1 — Always read `overview.md` first + +`references/overview.md` covers universal rules that apply to **every** pattern: file map, JSON envelope, handler flow, error helpers, status codes, store rules, mock conventions, route mounting, migrations, Swagger annotations, logging. **Read it once at the start of any backend task.** The pattern references assume you already know these. + +## Step 2 — Identify the pattern(s) and read the reference(s) + +Match the user's request to one or more patterns. Read every applicable reference. Most features fit one pattern; some combine two (e.g., a CRUD resource that needs cursor pagination → `crud-resource.md` + `list-pagination.md`). + +| User asks for... | Pattern | Reference | +|------------------|---------|-----------| +| "Add a new resource" with admin CRUD (list/create/update/delete), or "add an endpoint for managing X" | CRUD Resource | `references/crud-resource.md` | +| "Users manage their own X" (e.g., draft an application, edit profile) — `/me` style | Self-Resource | `references/self-resource.md` | +| List endpoint with cursor pagination, filters, search, or sort | Paginated List | `references/list-pagination.md` | +| "Admin claims work item, then submits result" — queue / pending / next / submit / completed | Workflow Queue | `references/workflow-queue.md` | +| File upload or download (signed URL for large files; base64 JSON for small) | File Storage | `references/file-storage.md` | +| Endpoint for external clients via API key (no SuperTokens session) | Public API | `references/public-api.md` | +| Batch operation, multi-table reset, transactional bulk insert | Bulk Operation | `references/bulk-operations.md` | +| PATCH a single field (status, role, etc.) with whitelisted enum values | Status Update | `references/status-update.md` | +| Toggle / configurable JSON value read by middleware or UI | Settings | `references/settings-pattern.md` | +| Aggregate counts / stats endpoint | Stats sub-pattern | `references/crud-resource.md` ("Stats Sub-Pattern" section) | +| Anything that needs handler tests | (always) | `references/testing.md` | + +### Common combinations + +- **Admin list with pagination** (e.g., applications list): `crud-resource.md` + `list-pagination.md`. +- **Self-resource with file upload** (e.g., resume on application): `self-resource.md` + `file-storage.md`. +- **CRUD with feature flag gate** (e.g., schedule editing toggle): `crud-resource.md` + `settings-pattern.md`. +- **Self-resource with state machine and submit** (e.g., draft → submitted): `self-resource.md` (covers the submit verb). +- **Anything with admin authorization**: just role-gate via `RequireRoleMiddleware` — see `overview.md` "Route Mounting". + +## Step 3 — Build the feature using the references as templates + +Each reference is structured to match the codebase's actual layering: migration → store + interface + mock → handlers → routes → tests. Work top-down so you can run/check each layer as you go: + +1. **Migration** — bump number, write up + down (see pattern reference for column conventions). +2. **Store model + methods** — copy the relevant skeleton from the reference, adjust columns/constraints. +3. **Storage interface + wiring** — add the interface to `internal/store/storage.go`'s `Storage` struct, wire in `NewStorage()`. +4. **Mock store** — implement the matching mock methods in `internal/store/mock_store.go`, register in `NewMockStore()`. +5. **Handlers** — payload + response structs colocated; follow the standard handler flow. +6. **Routes** — mount in the appropriate group in `cmd/api/api.go`. +7. **Tests** — use `testing.md` recipes; coverage checklist comes from the pattern reference. +8. **Swagger** — `task gen-docs` regenerates the spec (also runs as a pre-command for `air`). + +## Quick Critical Rules + +These appear in `overview.md` but are worth keeping front-of-mind. Breaking them creates cleanup work: + +- **JSON envelope**: `app.jsonResponse(w, status, ResponseStruct{...})` — always wrap data in a named response struct, never pass raw slices/models. +- **Store timeouts**: every method begins with `ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration); defer cancel()` (double the duration for bulk ops). +- **Error mapping**: `store.ErrNotFound` → `notFoundResponse`, `store.ErrConflict` → `conflictResponse`, anything unexpected → `internalServerError`. Don't add per-handler logging — error helpers already log. +- **Mocks omit ctx**: `m.Called(args...)` receives all params except `ctx`; nil-check `args.Get(0)` before type-asserting. +- **Settings**: only on the `Settings` interface; key strings come from `SettingsKey*` constants; setters use JSON body, never query params; tag is `superadmin/settings`. +- **No ORM**: raw SQL with `$1, $2` placeholders. No external HTTP calls inside store methods. +- **No `Co-Authored-By`** in commits. + +## When in Doubt + +- Reading existing code in the matching pattern is more reliable than guessing. Each reference points to its canonical example file; open it. +- If the request crosses pattern boundaries (e.g., "add an admin CRUD resource that users can also see their own copy of"), build it as two pattern instances rather than fusing them. +- If you can't decide between `crud-resource.md` and `self-resource.md`, ask: does the URL include `/me`? If yes → self-resource. If no → CRUD. +- For tests, the pattern reference's "Coverage Checklist" tells you which cases matter; `testing.md` tells you how to wire each test up. diff --git a/.claude/skills/backend/references/bulk-operations.md b/.claude/skills/backend/references/bulk-operations.md new file mode 100644 index 00000000..92f3bb82 --- /dev/null +++ b/.claude/skills/backend/references/bulk-operations.md @@ -0,0 +1,254 @@ +# Bulk Operations Pattern + +For endpoints that touch many rows in one call: batch assignments, multi-table resets, mass status updates. The handler is thin — almost all logic lives in a transactional store method. + +## When to Use + +The user wants: +- "Assign all pending X to Y" (work distribution). +- "Reset everything" / "wipe and seed" (multi-table operation). +- "Send/update for all matching rows" (bulk action). + +Anything that would be N round-trips if done one at a time, or that needs to commit-or-roll-back as a unit. + +Working examples: +- **Batch assign reviews** (`ApplicationReviewsStore.BatchAssign`, `batchAssignReviews`). +- **Hackathon reset** (`HackathonStore.Reset`, `resetHackathonHandler`). + +## Store: Transactional, Doubled Timeout + +Bulk methods double the standard query timeout because they touch more rows: + +```go +ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration*2) +defer cancel() + +tx, err := s.db.BeginTx(ctx, nil) +if err != nil { return nil, err } +defer tx.Rollback() // safe — Commit makes Rollback a no-op + +// ... do all the work in tx ... + +return tx.Commit() +``` + +`defer tx.Rollback()` is paired with explicit `tx.Commit()` at the end — Go's `database/sql` makes `Rollback` after `Commit` a no-op, so this is the right idiom. + +### Pre-flight reads with FOR UPDATE + +When reading state to compute the bulk action, lock it so concurrent bulk callers don't collide: + +```go +selectQuery := `SELECT value FROM settings WHERE key = $1 FOR UPDATE` +``` + +For row-locking with skip-on-contention (work-distribution scenarios): + +```sql +SELECT id FROM applications +WHERE ... +ORDER BY ... +LIMIT 1 +FOR UPDATE SKIP LOCKED -- different sessions claim different rows +``` + +### Bulk INSERT via `unnest` + +When you have N pairs to insert, build the slices in Go and use one `unnest` query — that's one round-trip instead of N: + +```go +var pairAppIDs []string +var pairAdminIDs []string +for _, app := range apps { + for ... { + pairAppIDs = append(pairAppIDs, app.ID) + pairAdminIDs = append(pairAdminIDs, adminID) + } +} + +if len(pairAppIDs) > 0 { + insertQuery := ` + INSERT INTO application_reviews (application_id, admin_id) + SELECT * FROM unnest($1::uuid[], $2::uuid[]) + ON CONFLICT (application_id, admin_id) DO NOTHING + ` + result, err := tx.ExecContext(ctx, insertQuery, pairAppIDs, pairAdminIDs) + if err != nil { return nil, err } + + rowsAffected, err := result.RowsAffected() + if err != nil { return nil, err } + reviewsCreated = int(rowsAffected) +} +``` + +`ON CONFLICT ... DO NOTHING` makes re-runs idempotent — important if a partial bulk completed and the user retries. + +### Multi-table reset + +Order matters when foreign keys are involved. `TRUNCATE ... CASCADE` skips writing N delete rows — but read out anything you need first (e.g., file paths to delete from object storage): + +```go +if resetApplications { + // Collect resume paths BEFORE truncation — we need them for GCS cleanup + rows, err := tx.QueryContext(ctx, "SELECT resume_path FROM applications WHERE resume_path IS NOT NULL") + if err != nil { return nil, err } + defer rows.Close() + for rows.Next() { + var path string + if err := rows.Scan(&path); err != nil { return nil, err } + resumePaths = append(resumePaths, path) + } + if err := rows.Err(); err != nil { return nil, err } + rows.Close() + + if _, err := tx.ExecContext(ctx, "TRUNCATE TABLE applications CASCADE"); err != nil { + return nil, err + } +} +// ... other tables ... +``` + +### Result struct + +Return a small struct of counters / what-was-touched. JSON-render directly: + +```go +type BatchAssignmentResult struct { + ReviewsCreated int `json:"reviews_created"` +} +``` + +## Handlers + +Thin — read inputs, call store, return result. + +### POST without body (defaults from settings) + +```go +// @Summary Batch assign reviews (SuperAdmin) +// @Tags superadmin/applications +// @Router /superadmin/applications/assign [post] +func (app *application) batchAssignReviews(w http.ResponseWriter, r *http.Request) { + reviewsPerApp, err := app.store.Settings.GetReviewsPerApplication(r.Context()) + if err != nil { app.internalServerError(w, r, err); return } + + result, err := app.store.ApplicationReviews.BatchAssign(r.Context(), reviewsPerApp) + if err != nil { app.internalServerError(w, r, err); return } + + app.jsonResponse(w, http.StatusOK, result) +} +``` + +### POST with options struct + +```go +type ResetHackathonPayload struct { + ResetApplications bool `json:"reset_applications"` + ResetScans bool `json:"reset_scans"` + ResetSchedule bool `json:"reset_schedule"` + ResetSettings bool `json:"reset_settings"` +} + +type ResetHackathonResponse struct { + ResetApplications bool `json:"reset_applications"` + ResetScans bool `json:"reset_scans"` + ResetSchedule bool `json:"reset_schedule"` + ResetSettings bool `json:"reset_settings"` + ResumesDeleted int `json:"resumes_deleted"` +} + +func (app *application) resetHackathonHandler(w http.ResponseWriter, r *http.Request) { + var req ResetHackathonPayload + if err := readJSON(w, r, &req); err != nil { app.badRequestResponse(w, r, err); return } + if err := Validate.Struct(req); err != nil { app.badRequestResponse(w, r, err); return } + + // Cross-field validation: at least one option + if !req.ResetApplications && !req.ResetScans && !req.ResetSchedule && !req.ResetSettings { + app.badRequestResponse(w, r, errors.New("at least one reset option must be selected")) + return + } + + resumePaths, err := app.store.Hackathon.Reset(r.Context(), + req.ResetApplications, req.ResetScans, req.ResetSchedule, req.ResetSettings) + if err != nil { app.internalServerError(w, r, err); return } + + // Async best-effort side-effect (don't block the response on slow GCS calls) + if len(resumePaths) > 0 && app.gcsClient != nil { + go func(paths []string) { + for _, path := range paths { + _ = app.gcsClient.DeleteObject(context.Background(), path) + } + }(resumePaths) + } + + app.jsonResponse(w, http.StatusOK, ResetHackathonResponse{ + ResetApplications: req.ResetApplications, + ResetScans: req.ResetScans, + ResetSchedule: req.ResetSchedule, + ResetSettings: req.ResetSettings, + ResumesDeleted: len(resumePaths), + }) +} +``` + +For best-effort post-commit work (object-storage cleanup, log shipping, fire-and-forget notifications): spawn a goroutine with `context.Background()`. Don't use `r.Context()` — the request is already done. + +## Routes + +Bulk operations are almost always super-admin gated: + +```go +r.Group(func(r chi.Router) { + r.Use(app.RequireRoleMiddleware(store.RoleSuperAdmin)) + r.Route("/superadmin", func(r chi.Router) { + r.Post("/reset-hackathon", app.resetHackathonHandler) + r.Route("/applications", func(r chi.Router) { + r.Post("/assign", app.batchAssignReviews) + // ... + }) + }) +}) +``` + +## Tests + +Coverage checklist: + +| Case | Expected | +|------|----------| +| Success | 200 with result counters; verify all bulk inputs were passed to store | +| Validation (e.g., no options) | 400 | +| Store error | 500 | +| Async side-effect | (mock the side-effect dependency; assert it was called the right number of times) | + +```go +func TestBatchAssignReviews(t *testing.T) { + app := newTestApplication(t) + mockReviews := app.store.ApplicationReviews.(*store.MockApplicationReviewsStore) + mockSettings := app.store.Settings.(*store.MockSettingsStore) + + result := &store.BatchAssignmentResult{ReviewsCreated: 15} + mockSettings.On("GetReviewsPerApplication").Return(3, nil).Once() + mockReviews.On("BatchAssign", 3).Return(result, nil).Once() + + req, _ := http.NewRequest(http.MethodPost, "/", nil) + req = setUserContext(req, newSuperAdminUser()) + + rr := executeRequest(req, http.HandlerFunc(app.batchAssignReviews)) + checkResponseCode(t, http.StatusOK, rr.Code) + + var body struct{ Data store.BatchAssignmentResult `json:"data"` } + require.NoError(t, json.NewDecoder(rr.Body).Decode(&body)) + assert.Equal(t, 15, body.Data.ReviewsCreated) +} +``` + +Async cleanup is hard to assert deterministically — keep the goroutine logic simple, and test the synchronous outputs (the response body, the store calls). + +## What NOT to Do + +- Don't run N inserts in a loop — build slices and `unnest` once. +- Don't skip `ON CONFLICT DO NOTHING` for idempotency — partial bulk completes plus user retries are normal. +- Don't truncate without first reading any "external" pointers (file paths, third-party IDs) you'll need for cleanup. +- Don't block the response on best-effort cleanup — fire a goroutine with `context.Background()`. +- Don't keep the standard 5s timeout — bulk operations need `QueryTimeoutDuration*2` (or longer if you've measured). diff --git a/.claude/skills/backend/references/crud-resource.md b/.claude/skills/backend/references/crud-resource.md new file mode 100644 index 00000000..04fd5c30 --- /dev/null +++ b/.claude/skills/backend/references/crud-resource.md @@ -0,0 +1,386 @@ +# CRUD Resource Pattern + +Foundational pattern for an admin-managed entity with list/create/update/delete. Use this for resources where admins are the only writers and any signed-in admin (or super admin) sees the same view. Examples: schedule, sponsors. + +If you also need cursor pagination/filtering, layer in `list-pagination.md`. If users own their own copy of the resource, use `self-resource.md` instead. + +## When to Use + +The user is asking for "manage X" where X has: +- A flat collection (no per-user scoping) +- Admin-only or super-admin-only writes +- Standard CRUD shape (list, create, update, delete) — possibly plus a get-by-id + +Working example: **Schedule** (`internal/store/schedule.go`, `cmd/api/schedule.go`). + +## 1. Migration + +`cmd/migrate/migrations/000NNN_add_.up.sql` (check the highest existing number and increment). + +```sql +CREATE TABLE IF NOT EXISTS ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + name TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', + -- ... domain columns ... + tags TEXT[] NOT NULL DEFAULT '{}', -- TEXT[] for arrays + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE TRIGGER trg__updated_at +BEFORE UPDATE ON +FOR EACH ROW EXECUTE FUNCTION set_updated_at(); + +CREATE INDEX idx__ ON (); -- on the column you'll order/filter by +``` + +Down migration drops index, trigger, table (in that order): + +```sql +DROP INDEX IF EXISTS idx__; +DROP TRIGGER IF EXISTS trg__updated_at ON ; +DROP TABLE IF EXISTS ; +``` + +Conventions: UUID PKs, `TIMESTAMPTZ`, `TEXT[]` for string arrays, `JSONB` for structured data, `NOT NULL DEFAULT` for non-required fields. + +## 2. Store (`internal/store/.go`) + +### Model + +```go +type Sponsor struct { + ID string `json:"id"` + Name string `json:"name"` + Tier string `json:"tier"` + WebsiteURL string `json:"website_url"` + Description string `json:"description"` + DisplayOrder int `json:"display_order"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +type SponsorsStore struct { + db *sql.DB +} +``` + +JSON tags: snake_case. Nullable fields: `*string` / `*int`. ID: always `string` (UUID rendered as text). For `TEXT[]` columns use `store.StringArray` (defined in `schedule.go`) — it implements `sql.Scanner` and `driver.Valuer`. + +### List + +`defer rows.Close()`, nil-safe slice, check `rows.Err()`. + +```go +func (s *SponsorsStore) List(ctx context.Context) ([]Sponsor, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + query := ` + SELECT id, name, tier, website_url, description, display_order, created_at, updated_at + FROM sponsors + ORDER BY display_order ASC + ` + + rows, err := s.db.QueryContext(ctx, query) + if err != nil { + return nil, err + } + defer rows.Close() + + var sponsors []Sponsor + for rows.Next() { + var sp Sponsor + if err := rows.Scan(/* fields in same order as SELECT */); err != nil { + return nil, err + } + sponsors = append(sponsors, sp) + } + if sponsors == nil { + sponsors = []Sponsor{} // avoid `null` in JSON when empty + } + return sponsors, rows.Err() +} +``` + +### GetByID + +```go +func (s *SponsorsStore) GetByID(ctx context.Context, id string) (*Sponsor, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + var sp Sponsor + err := s.db.QueryRowContext(ctx, `SELECT ... FROM sponsors WHERE id = $1`, id).Scan(/* ... */) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + return nil, err + } + return &sp, nil +} +``` + +### Create + +`INSERT ... RETURNING id, created_at, updated_at`. Mutate the input pointer: + +```go +func (s *SponsorsStore) Create(ctx context.Context, sp *Sponsor) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + return s.db.QueryRowContext(ctx, ` + INSERT INTO sponsors (name, tier, website_url, description, display_order) + VALUES ($1, $2, $3, $4, $5) + RETURNING id, created_at, updated_at + `, sp.Name, sp.Tier, sp.WebsiteURL, sp.Description, sp.DisplayOrder, + ).Scan(&sp.ID, &sp.CreatedAt, &sp.UpdatedAt) +} +``` + +If you have a unique constraint that may collide, detect it and return `ErrConflict` — either via Postgres error code `23505` (`*pgconn.PgError`, see `scans.go`) or `strings.Contains(err.Error(), "")` (see `applications.go`). + +### Update + +`UPDATE ... RETURNING updated_at`. `sql.ErrNoRows` → `ErrNotFound`: + +```go +func (s *SponsorsStore) Update(ctx context.Context, sp *Sponsor) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + err := s.db.QueryRowContext(ctx, ` + UPDATE sponsors + SET name = $1, tier = $2, website_url = $3, description = $4, display_order = $5 + WHERE id = $6 + RETURNING updated_at + `, sp.Name, sp.Tier, sp.WebsiteURL, sp.Description, sp.DisplayOrder, sp.ID, + ).Scan(&sp.UpdatedAt) + + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return ErrNotFound + } + return err + } + return nil +} +``` + +### Delete + +`ExecContext` + `RowsAffected() == 0` → `ErrNotFound`: + +```go +func (s *SponsorsStore) Delete(ctx context.Context, id string) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + result, err := s.db.ExecContext(ctx, `DELETE FROM sponsors WHERE id = $1`, id) + if err != nil { + return err + } + rows, err := result.RowsAffected() + if err != nil { + return err + } + if rows == 0 { + return ErrNotFound + } + return nil +} +``` + +## 3. Interface & Wiring (`internal/store/storage.go`) + +Add the interface to `Storage`, wire in `NewStorage()`: + +```go +Sponsors interface { + List(ctx context.Context) ([]Sponsor, error) + GetByID(ctx context.Context, id string) (*Sponsor, error) + Create(ctx context.Context, sp *Sponsor) error + Update(ctx context.Context, sp *Sponsor) error + Delete(ctx context.Context, id string) error +} +``` + +```go +return Storage{ + // ... + Sponsors: &SponsorsStore{db: db}, +} +``` + +## 4. Mock (`internal/store/mock_store.go`) + +```go +type MockSponsorsStore struct{ mock.Mock } + +func (m *MockSponsorsStore) List(ctx context.Context) ([]Sponsor, error) { + args := m.Called() + if args.Get(0) == nil { return nil, args.Error(1) } + return args.Get(0).([]Sponsor), args.Error(1) +} +// ... GetByID, Create, Update, Delete ... +``` + +`m.Called()` receives all params **except ctx**. Nil-check `args.Get(0)` before type-asserting pointers/slices. Add to `NewMockStore()`. + +## 5. Handlers (`cmd/api/.go`) + +### Payload + Response Structs + +```go +type SponsorPayload struct { + Name string `json:"name" validate:"required,min=1,max=100"` + Tier string `json:"tier" validate:"required,min=1,max=50"` + WebsiteURL string `json:"website_url" validate:"omitempty,url"` + Description string `json:"description"` + DisplayOrder int `json:"display_order" validate:"min=0"` +} + +type SponsorListResponse struct { + Sponsors []store.Sponsor `json:"sponsors"` +} +``` + +Naming: payloads `Payload` or shared `Payload` (re-use for create + update). Responses: `ListResponse`, `Response`. Use `validate` tags. + +Tip: when the response struct has the same shape as the payload, you can type-convert: `SponsorListResponse(req)` (see `scans.go:266`). + +### Handlers + +```go +// @Summary List sponsors (Admin) +// @Description Returns all sponsors ordered by display order +// @Tags admin/sponsors +// @Produce json +// @Success 200 {object} SponsorListResponse +// @Failure 401 {object} object{error=string} +// @Failure 403 {object} object{error=string} +// @Failure 500 {object} object{error=string} +// @Security CookieAuth +// @Router /admin/sponsors [get] +func (app *application) listSponsorsHandler(w http.ResponseWriter, r *http.Request) { + sponsors, err := app.store.Sponsors.List(r.Context()) + if err != nil { + app.internalServerError(w, r, err) + return + } + if err := app.jsonResponse(w, http.StatusOK, SponsorListResponse{Sponsors: sponsors}); err != nil { + app.internalServerError(w, r, err) + } +} +``` + +Create / Update / Delete follow the standard handler flow — validate URL param, parse body, validate struct, call store, map errors, respond. See `cmd/api/sponsors.go` and `cmd/api/schedule.go` for the complete pattern. + +For DELETE: `w.WriteHeader(http.StatusNoContent)` — no body. + +## 6. Routes (`cmd/api/api.go`) + +Add inside the matching role block in `mount()`: + +```go +r.Group(func(r chi.Router) { + r.Use(app.RequireRoleMiddleware(store.RoleAdmin)) // or RoleSuperAdmin + + r.Route("/admin", func(r chi.Router) { + r.Route("/sponsors", func(r chi.Router) { + r.Get("/", app.listSponsorsHandler) + r.Post("/", app.createSponsorHandler) + r.Get("/{sponsorID}", app.getSponsorHandler) + r.Put("/{sponsorID}", app.updateSponsorHandler) + r.Delete("/{sponsorID}", app.deleteSponsorHandler) + }) + }) +}) +``` + +If a sub-group needs feature-flag enforcement, wrap with a settings-gated middleware (see `settings-pattern.md`): + +```go +r.Group(func(r chi.Router) { + r.Use(app.AdminScheduleEditPermissionMiddleware) + r.Post("/", app.createScheduleHandler) + r.Put("/{scheduleID}", app.updateScheduleHandler) + r.Delete("/{scheduleID}", app.deleteScheduleHandler) +}) +``` + +## 7. Tests + +See `testing.md` for utilities. CRUD coverage checklist: + +| Handler | Cases | +|---------|-------| +| List | 200 with items, 200 empty, (optionally) 500 store error | +| GetByID | 200 success, 404 not found, 400 missing ID | +| Create | 201 success, 400 validation failure | +| Update | 200 success, 404 not found, 400 validation | +| Delete | 204 success, 404 not found | + +Standard form (no URL params): + +```go +rr := executeRequest(req, http.HandlerFunc(app.listSponsorsHandler)) +checkResponseCode(t, http.StatusOK, rr.Code) +var body struct { Data SponsorListResponse `json:"data"` } +require.NoError(t, json.NewDecoder(rr.Body).Decode(&body)) +``` + +For URL-param handlers: see `testing.md` "URL parameters" section (mini chi router OR inject `chi.NewRouteContext()`). + +## Stats Sub-Pattern (aggregation endpoint) + +For a read-only aggregate counts endpoint (e.g., `getApplicationStatsHandler`): + +### Store + +Single SELECT with `COUNT(*) FILTER (WHERE ...)`: + +```go +func (s *ApplicationsStore) GetStats(ctx context.Context) (*ApplicationStats, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + query := ` + SELECT + COUNT(*) AS total, + COUNT(*) FILTER (WHERE status = 'submitted') AS submitted, + COUNT(*) FILTER (WHERE status = 'accepted') AS accepted + FROM applications + ` + var s ApplicationStats + if err := s.db.QueryRowContext(ctx, query).Scan(&s.Total, &s.Submitted, &s.Accepted); err != nil { + return nil, err + } + // derived fields (e.g., acceptance rate) computed in Go + return &s, nil +} +``` + +### Handler + +GET only, no params, no validation: + +```go +func (app *application) getApplicationStatsHandler(w http.ResponseWriter, r *http.Request) { + stats, err := app.store.Application.GetStats(r.Context()) + if err != nil { + app.internalServerError(w, r, err) + return + } + if err := app.jsonResponse(w, http.StatusOK, stats); err != nil { + app.internalServerError(w, r, err) + } +} +``` + +`*store.ApplicationStats` already has snake_case JSON tags, so it can be passed directly to `jsonResponse` without an extra wrapper. (For derived stats with no existing struct, define one in the handler file.) + +If the counter would be expensive to compute on every request, cache in the `settings` table and increment in the same transaction as writes — see `incrementScanStat` / `GetScanStats` in `internal/store/{scans,settings}.go`. diff --git a/.claude/skills/backend/references/file-storage.md b/.claude/skills/backend/references/file-storage.md new file mode 100644 index 00000000..c1bcbd59 --- /dev/null +++ b/.claude/skills/backend/references/file-storage.md @@ -0,0 +1,309 @@ +# File Storage Pattern + +Two file-upload patterns in the codebase, picked by file size: + +| Pattern | Examples | Use when | +|---------|----------|----------| +| **Signed URL (GCS)** | Resume upload/download | Files >100KB, especially user-uploaded content (PDFs, images) where you want to offload bandwidth from the API | +| **Base64 JSON in DB** | Sponsor logo | Files <1MB, admin-uploaded, where you want a single round-trip and the convenience of having the bytes available with the row | + +## When to Use Which + +- The user is going to upload directly from a browser → Signed URL. +- The thing is small, served alongside the resource, and admin-only → Base64 JSON. +- It's cheap to serve from the DB and you don't want a separate cleanup pipeline → Base64 JSON. +- It's a multi-megabyte file → Signed URL. + +## Signed URL Pattern (GCS) + +The backend mints a short-lived signed URL; the client `PUT`s the bytes directly to GCS, then sends the resulting object path back to the API on the next mutation. + +Working example: **Resume** (`cmd/api/resume.go`). + +### `app.gcsClient` interface + +```go +// internal/gcs/client.go (already exists) +type Client interface { + GenerateUploadURL(ctx context.Context, objectPath string) (string, error) + GenerateDownloadURL(ctx context.Context, objectPath string) (string, error) + DeleteObject(ctx context.Context, objectPath string) error +} +``` + +### Generate Upload URL + +```go +const randomResumeObjectIDBytes = 16 + +type ResumeUploadURLResponse struct { + UploadURL string `json:"upload_url"` + ResumePath string `json:"resume_path"` +} + +func (app *application) generateResumeUploadURLHandler(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + if user == nil { app.unauthorizedErrorResponse(w, r, errors.New("missing user")); return } + + // 1) Pre-flight ownership/state checks + application, err := app.store.Application.GetByUserID(r.Context(), user.ID) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("application not found")); return + } + app.internalServerError(w, r, err); return + } + if application.Status != store.StatusDraft { + app.conflictResponse(w, r, errors.New("cannot update submitted application")); return + } + + // 2) GCS configured? + if app.gcsClient == nil { + app.logger.Warnw("resume upload url requested but gcs is not configured", "user_id", user.ID) + writeJSONError(w, http.StatusServiceUnavailable, "resume uploads are not configured") + return + } + + // 3) Random object path scoped under the user + randomID, err := randomHex(randomResumeObjectIDBytes) + if err != nil { app.internalServerError(w, r, err); return } + objectPath := fmt.Sprintf("resumes/%s/%s.pdf", user.ID, randomID) + + // 4) Mint signed URL + uploadURL, err := app.gcsClient.GenerateUploadURL(r.Context(), objectPath) + if err != nil { app.internalServerError(w, r, err); return } + + app.jsonResponse(w, http.StatusOK, ResumeUploadURLResponse{ + UploadURL: uploadURL, + ResumePath: objectPath, + }) +} + +func randomHex(size int) (string, error) { + buf := make([]byte, size) + if _, err := rand.Read(buf); err != nil { return "", err } + return hex.EncodeToString(buf), nil +} +``` + +Key details: +- **Object path** is `//.` — owner-scoped to keep ACLs simple. +- **Random ID** uses `crypto/rand` (`randomHex`) — no enumeration via guessable paths. +- **GCS not configured** → 503 `Service Unavailable`. This is a deployment state, not a logic error; don't 500. +- **Frontend stores the resume path** on the resource via the regular update endpoint (e.g., PATCH `/applications/me` with `{"resume_path": "..."}`). The signed-URL handler does not write the path itself. + +### Generate Download URL + +For admin viewing — same shape, but loads the resource by ID and uses its `ResumePath`: + +```go +type ResumeDownloadURLResponse struct { + DownloadURL string `json:"download_url"` +} + +func (app *application) getResumeDownloadURLHandler(w http.ResponseWriter, r *http.Request) { + applicationID := chi.URLParam(r, "applicationID") + if applicationID == "" { + app.badRequestResponse(w, r, errors.New("application ID is required")); return + } + + application, err := app.store.Application.GetByID(r.Context(), applicationID) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("application not found")); return + } + app.internalServerError(w, r, err); return + } + if application.ResumePath == nil { + app.notFoundResponse(w, r, errors.New("resume not found")); return + } + if app.gcsClient == nil { + writeJSONError(w, http.StatusServiceUnavailable, "resume downloads are not configured") + return + } + + downloadURL, err := app.gcsClient.GenerateDownloadURL(r.Context(), *application.ResumePath) + if err != nil { app.internalServerError(w, r, err); return } + + app.jsonResponse(w, http.StatusOK, ResumeDownloadURLResponse{DownloadURL: downloadURL}) +} +``` + +### Delete + +Two-step: best-effort GCS delete (logged on failure, doesn't block) + clear the path on the row. + +```go +func (app *application) deleteResumeHandler(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + if user == nil { app.unauthorizedErrorResponse(w, r, nil); return } + + application, err := app.store.Application.GetByUserID(r.Context(), user.ID) + if err != nil { /* ... 404 / 500 ... */ } + if application.Status != store.StatusDraft { /* ... 409 ... */ } + if application.ResumePath == nil { /* ... 404 ... */ } + + if app.gcsClient != nil { + if err := app.gcsClient.DeleteObject(r.Context(), *application.ResumePath); err != nil { + // Best-effort — orphaned object cleanup is acceptable + app.logger.Warnw("failed to delete resume from gcs", + "application_id", application.ID, + "resume_path", *application.ResumePath, + "error", err) + } + } + + application.ResumePath = nil + if err := app.store.Application.Update(r.Context(), application); err != nil { + app.internalServerError(w, r, err); return + } + app.jsonResponse(w, http.StatusOK, application) +} +``` + +Treat GCS delete as best-effort — orphaned objects are cheap; a failed transactional cleanup blocks the user. + +### Routes + +Upload/download go in the appropriate auth group: + +```go +// Hacker self-resource (under AuthRequiredMiddleware + ApplicationsEnabledMiddleware) +r.Post("/me/resume-upload-url", app.generateResumeUploadURLHandler) +r.Delete("/me/resume", app.deleteResumeHandler) + +// Admin (under RequireRoleMiddleware(RoleAdmin)) +r.Get("/{applicationID}/resume-url", app.getResumeDownloadURLHandler) +``` + +## Base64 JSON Pattern + +For small files where the bytes live with the row. Working example: **Sponsor logo** (`cmd/api/sponsors.go:198-276`). + +### Schema + +Two columns on the parent row — `_data TEXT` and `_content_type TEXT`: + +```sql +logo_data TEXT NOT NULL DEFAULT '', +logo_content_type TEXT NOT NULL DEFAULT '', +``` + +### Handler + +```go +var allowedLogoContentTypes = map[string]bool{ + "image/png": true, + "image/jpeg": true, + "image/webp": true, + "image/gif": true, +} + +const maxLogoBytes = 1 * 1024 * 1024 // 1MB decoded + +type LogoUploadPayload struct { + LogoData string `json:"logo_data" validate:"required"` + ContentType string `json:"content_type" validate:"required"` +} + +func (app *application) uploadLogoHandler(w http.ResponseWriter, r *http.Request) { + id := chi.URLParam(r, "sponsorID") + if id == "" { app.badRequestResponse(w, r, errors.New("missing sponsor ID")); return } + + // 1) Confirm the parent exists (so we can return 404 cleanly) + if _, err := app.store.Sponsors.GetByID(r.Context(), id); err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("sponsor not found")); return + } + app.internalServerError(w, r, err); return + } + + // 2) Parse + validate + var p LogoUploadPayload + if err := readJSON(w, r, &p); err != nil { app.badRequestResponse(w, r, err); return } + if err := Validate.Struct(p); err != nil { app.badRequestResponse(w, r, err); return } + if !allowedLogoContentTypes[p.ContentType] { + app.badRequestResponse(w, r, fmt.Errorf("unsupported content type: %s", p.ContentType)) + return + } + + // 3) Decode + size check + decoded, err := base64.StdEncoding.DecodeString(p.LogoData) + if err != nil { app.badRequestResponse(w, r, errors.New("invalid base64 data")); return } + if len(decoded) > maxLogoBytes { + app.badRequestResponse(w, r, fmt.Errorf("logo exceeds maximum size of %d bytes", maxLogoBytes)) + return + } + + // 4) Persist (we keep the base64 string, not raw bytes — easier for direct JSON serving) + if err := app.store.Sponsors.UpdateLogo(r.Context(), id, p.LogoData, p.ContentType); err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("sponsor not found")); return + } + app.internalServerError(w, r, err); return + } + + sponsor, err := app.store.Sponsors.GetByID(r.Context(), id) + if err != nil { app.internalServerError(w, r, err); return } + app.jsonResponse(w, http.StatusOK, sponsor) +} +``` + +Defense layers in order: payload required → content type allowlist → decode validation → size cap. + +The validator's `MaxBytesReader` in `readJSON` already enforces a 1MB body cap; this is a second, semantic check that makes the error message specific. + +### Store + +```go +func (s *SponsorsStore) UpdateLogo(ctx context.Context, id string, data string, contentType string) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + result, err := s.db.ExecContext(ctx, + `UPDATE sponsors SET logo_data = $1, logo_content_type = $2 WHERE id = $3`, + data, contentType, id) + if err != nil { return err } + rows, err := result.RowsAffected() + if err != nil { return err } + if rows == 0 { return ErrNotFound } + return nil +} +``` + +## Tests + +Coverage checklist for signed-URL upload: + +| Case | Expected | +|------|----------| +| Success | 200 with `upload_url` + `resume_path` (verify path prefix and extension) | +| Resource not found | 404 | +| Wrong state (e.g., submitted) | 409 | +| GCS not configured | 503 | +| GCS signing fails | 500 | + +Mock GCS setup: + +```go +mockGCS := app.gcsClient.(*gcs.MockClient) +mockGCS.On( + "GenerateUploadURL", + mock.Anything, + mock.MatchedBy(func(p string) bool { + return strings.HasPrefix(p, "resumes/"+user.ID+"/") && strings.HasSuffix(p, ".pdf") + }), +).Return("https://upload.example.com", nil).Once() +``` + +To test the "not configured" case, set `app.gcsClient = nil` for that subtest and restore it afterward (see `cmd/api/resume_test.go:86-101`). + +For base64 upload: same shape plus `400` cases for invalid content type, invalid base64, oversized payload. + +## What NOT to Do + +- Don't proxy the file bytes through the backend if you can sign and let the client upload directly. +- Don't trust the resource path coming back from the client without scoping it under the owner — always re-derive on read (e.g., look up `application.ResumePath` server-side). +- Don't 500 when GCS isn't configured — that's a 503, and log a warning so ops sees it. +- Don't fail the user's mutation when GCS delete fails — log it best-effort and continue. +- Don't decode-and-store raw bytes for the base64 pattern; keep the base64 string so the row can be JSON-serialized as-is. diff --git a/.claude/skills/backend/references/list-pagination.md b/.claude/skills/backend/references/list-pagination.md new file mode 100644 index 00000000..48e60204 --- /dev/null +++ b/.claude/skills/backend/references/list-pagination.md @@ -0,0 +1,291 @@ +# List with Pagination, Filters, Search, Sort + +For list endpoints that return many rows and need cursor pagination, status filtering, search, or sorting. Layer this over `crud-resource.md` — the model and CRUD methods are unchanged; only `List` becomes more complex. + +Two pagination styles are in use: +- **Cursor-based** (preferred) — used for applications and the role-mode of users. Forward + backward, base64-encoded JSON cursor. +- **Offset-based** — used for the trigram-style user search (limit/offset). Simpler when the query already filters aggressively. + +## When to Use + +The list will grow large enough that pagination matters, OR the user wants any of: +- Status / category filters +- Sort by multiple columns +- Free-text search (substring match) + +Working examples: **Applications list** (`ApplicationsStore.List`, `listApplicationsHandler`) and **Users role-listing** (`UsersStore.ListUsers`, `listUsersByRole`). + +## Cursor Pagination + +### Cursor type + +Define a struct with the sort key + tiebreaker (always include `id` as a tiebreaker for stable ordering). Use short JSON tags to keep cursors short: + +```go +type ApplicationCursor struct { + CreatedAt time.Time `json:"c"` + ID string `json:"i"` + SortVal *int `json:"v,omitempty"` // populated only when sorting by a non-time column +} + +func EncodeCursor(createdAt time.Time, id string) string { + cursor := ApplicationCursor{CreatedAt: createdAt, ID: id} + data, _ := json.Marshal(cursor) + return base64.URLEncoding.EncodeToString(data) +} + +func EncodeSortCursor(sortVal int, id string) string { + cursor := ApplicationCursor{ID: id, SortVal: &sortVal} + data, _ := json.Marshal(cursor) + return base64.URLEncoding.EncodeToString(data) +} + +func DecodeCursor(encoded string) (*ApplicationCursor, error) { + data, err := base64.URLEncoding.DecodeString(encoded) + if err != nil { return nil, fmt.Errorf("invalid cursor encoding") } + var c ApplicationCursor + if err := json.Unmarshal(data, &c); err != nil { return nil, fmt.Errorf("invalid cursor format") } + if c.ID == "" { return nil, fmt.Errorf("invalid cursor: missing id") } + if c.CreatedAt.IsZero() && c.SortVal == nil { + return nil, fmt.Errorf("invalid cursor: missing sort value") + } + return &c, nil +} +``` + +### Direction enum + +```go +type PaginationDirection string + +const ( + DirectionForward PaginationDirection = "forward" + DirectionBackward PaginationDirection = "backward" +) +``` + +### Filters & sort enums + +Whitelist sort columns to **prevent SQL injection** when interpolating into the query string: + +```go +type ApplicationSortBy string + +const ( + SortByCreatedAt ApplicationSortBy = "created_at" + SortByAcceptVotes ApplicationSortBy = "accept_votes" + SortByRejectVotes ApplicationSortBy = "reject_votes" + SortByWaitlistVotes ApplicationSortBy = "waitlist_votes" +) + +func sortColumnName(sortBy ApplicationSortBy) string { + switch sortBy { + case SortByAcceptVotes: return "a.accept_votes" + case SortByRejectVotes: return "a.reject_votes" + case SortByWaitlistVotes: return "a.waitlist_votes" + default: return "a.created_at" + } +} + +type ApplicationListFilters struct { + Status *ApplicationStatus + Search *string + SortBy ApplicationSortBy +} +``` + +### List signature + +```go +func (s *ApplicationsStore) List( + ctx context.Context, + filters ApplicationListFilters, + cursor *ApplicationCursor, + direction PaginationDirection, + limit int, +) (*ApplicationListResult, error) +``` + +Inside: +1. Cap `limit` (default 50, max 100). +2. Build `WHERE` clause: status filter, search clause (`ILIKE`), cursor predicate. +3. Cursor predicate compares `(sort_key, id)` tuple — `<` for forward (DESC), `>` for backward (ASC). +4. Order DESC for forward, ASC for backward. +5. Fetch `limit + 1` rows to detect `HasMore`. +6. If `direction == DirectionBackward`, reverse the slice before returning. +7. Generate `NextCursor` / `PrevCursor` for the slice's first/last items. + +Schema: + +```go +type ApplicationListResult struct { + Applications []ApplicationListItem `json:"applications"` + NextCursor *string `json:"next_cursor,omitempty"` + PrevCursor *string `json:"prev_cursor,omitempty"` + HasMore bool `json:"has_more"` +} +``` + +See `internal/store/applications.go:325-527` for the canonical implementation, including the dual code path for vote-column vs. created_at sorts. **Don't reinvent it for new resources — copy and adapt.** + +### Search clause + +Substring match via `ILIKE '%' || $N || '%'`. For multi-field search (e.g., email + first/last name from JSONB), `OR` them and parameterize once: + +```sql +AND ($5::text IS NULL OR ( + u.email ILIKE '%' || $5 || '%' + OR a.responses->>'first_name' ILIKE '%' || $5 || '%' + OR a.responses->>'last_name' ILIKE '%' || $5 || '%' +)) +``` + +Reject too-short search input at the handler layer (typically `< 2` chars). + +## Handler — Query-Param Parsing + +`r.URL.Query()` for each param. Validate with whitelisted enum switch — return 400 on anything else: + +```go +func (app *application) listApplicationsHandler(w http.ResponseWriter, r *http.Request) { + query := r.URL.Query() + + // cursor + var cursor *store.ApplicationCursor + if cursorStr := query.Get("cursor"); cursorStr != "" { + var err error + cursor, err = store.DecodeCursor(cursorStr) + if err != nil { app.badRequestResponse(w, r, errors.New("invalid cursor")); return } + } + + // status filter — whitelist + var filters store.ApplicationListFilters + if statusStr := query.Get("status"); statusStr != "" { + status := store.ApplicationStatus(statusStr) + switch status { + case store.StatusDraft, store.StatusSubmitted, store.StatusAccepted, + store.StatusRejected, store.StatusWaitlisted: + filters.Status = &status + default: + app.badRequestResponse(w, r, errors.New("invalid status value")) + return + } + } + + // search — length bounds + if searchStr := query.Get("search"); searchStr != "" { + if len(searchStr) < 2 || len(searchStr) > 100 { + app.badRequestResponse(w, r, errors.New("search must be 2-100 characters")) + return + } + filters.Search = &searchStr + } + + // limit — bounded + limit := 50 + if limitStr := query.Get("limit"); limitStr != "" { + n, err := strconv.Atoi(limitStr) + if err != nil || n < 1 || n > 100 { + app.badRequestResponse(w, r, errors.New("limit must be between 1 and 100")) + return + } + limit = n + } + + // direction — enum + direction := store.DirectionForward + if dirStr := query.Get("direction"); dirStr != "" { + switch store.PaginationDirection(dirStr) { + case store.DirectionForward, store.DirectionBackward: + direction = store.PaginationDirection(dirStr) + default: + app.badRequestResponse(w, r, errors.New("direction must be 'forward' or 'backward'")) + return + } + } + + // sort_by — whitelist + if sortStr := query.Get("sort_by"); sortStr != "" { + switch store.ApplicationSortBy(sortStr) { + case store.SortByCreatedAt, store.SortByAcceptVotes, + store.SortByRejectVotes, store.SortByWaitlistVotes: + filters.SortBy = store.ApplicationSortBy(sortStr) + default: + app.badRequestResponse(w, r, errors.New("invalid sort_by value")) + return + } + } + + result, err := app.store.Application.List(r.Context(), filters, cursor, direction, limit) + if err != nil { app.internalServerError(w, r, err); return } + + app.jsonResponse(w, http.StatusOK, result) +} +``` + +The store's `*ApplicationListResult` already has snake_case JSON tags so it can pass directly to `jsonResponse` without an extra wrapper. + +### Swagger params + +```go +// @Param cursor query string false "Pagination cursor" +// @Param status query string false "Filter by status (draft, submitted, ...)" +// @Param limit query int false "Page size (default 50, max 100)" +// @Param direction query string false "forward or backward" +// @Param sort_by query string false "Sort column" +// @Success 200 {object} store.ApplicationListResult +``` + +## Offset Pagination (Search) + +Simpler — appropriate for trigram/ILIKE search where users page through a small result set. Used for `searchUsersHandler`: + +```go +func (s *UsersStore) Search(ctx context.Context, query string, limit int, offset int) (*UserSearchResult, error) { + // ... clamp limit, offset >= 0 ... + // first query: COUNT(*) for total + // second query: SELECT ... LIMIT $2 OFFSET $3 +} + +type UserSearchResult struct { + Users []UserListItem `json:"users"` + TotalCount int `json:"total_count"` +} +``` + +Handler validates `search` is 2-100 chars and clamps `limit`/`offset`. Use this only when the search clause is selective enough that paging far isn't a concern. + +## Tests + +Coverage checklist for cursor-paginated list: + +| Case | Expected | +|------|----------| +| No filters | 200 with default cursor/direction passed to store | +| Invalid status | 400 | +| Invalid limit (< 1, > 100, NaN) | 400 | +| Invalid direction | 400 | +| Invalid sort_by | 400 | +| Search too short / too long | 400 | +| Valid combination | 200, store called with parsed filters | + +Mock pattern — assert exact filters passed: + +```go +search := "john" +mockApps.On("List", + store.ApplicationListFilters{Search: &search}, + (*store.ApplicationCursor)(nil), + store.DirectionForward, + 50, +).Return(result, nil).Once() +``` + +See `cmd/api/applications_test.go:380-545`. + +## What NOT to Do + +- Don't interpolate user-supplied strings into the SQL query directly. Whitelist sort columns and use placeholders for everything else. +- Don't return `null` for an empty list — the cursor pagination layer at the store already initializes the slice; for nested fields, do `if items == nil { items = []T{} }`. +- Don't forget the `id` tiebreaker — without it, identical sort values produce non-deterministic page boundaries. +- Don't roll a new cursor format — copy the `(time, id)` or `(int, id)` shape. diff --git a/.claude/skills/backend/references/overview.md b/.claude/skills/backend/references/overview.md new file mode 100644 index 00000000..0a27e87e --- /dev/null +++ b/.claude/skills/backend/references/overview.md @@ -0,0 +1,220 @@ +# Backend Architecture Overview + +Universal conventions and a decision tree for picking the right endpoint pattern. Read this first, then read the pattern reference that matches the feature you're building. + +## File Map + +| Component | Location | Naming | +|-----------|----------|--------| +| Handlers | `cmd/api/.go` | One file per resource domain | +| Handler tests | `cmd/api/_test.go` | Same package (`package main`) | +| Store model + impl | `internal/store/.go` | One file per resource | +| Store interface | `internal/store/storage.go` | Add to `Storage` struct, wire in `NewStorage()` | +| Mock store | `internal/store/mock_store.go` | Add mock + update `NewMockStore()` | +| Migrations | `cmd/migrate/migrations/` | `000NNN__.{up,down}.sql` | +| Routes | `cmd/api/api.go` | `mount()`, in the matching role group | +| Test utilities | `cmd/api/test_utils_test.go` | Shared helpers (do not duplicate) | + +## Endpoint Pattern Decision Tree + +Match the user's request to a pattern, then read the corresponding reference. + +| User asks for... | Pattern | Reference | +|------------------|---------|-----------| +| "add a new resource" with admin CRUD (list/create/update/delete) | CRUD Resource | `crud-resource.md` | +| "users manage their own X" (e.g., draft/submit, applications) | Self-Resource | `self-resource.md` | +| List endpoint with cursor pagination, filters, sort, or search | Paginated List | `list-pagination.md` | +| Admin claims work item, processes it, submits result (queue) | Workflow Queue | `workflow-queue.md` | +| File upload/download — signed URLs (large files) or base64 JSON (small files) | File Storage | `file-storage.md` | +| Endpoint exposed via API key for external consumers | Public API | `public-api.md` | +| Batch action across many rows, multi-table reset, transactional bulk | Bulk Operation | `bulk-operations.md` | +| PATCH a single field (status, role) with whitelisted values | Status Update | `status-update.md` | +| Toggle/setting (boolean, JSON value) read by middleware or UI | Settings | `settings-pattern.md` | +| Aggregate counts/stats endpoint | Stats sub-pattern | `crud-resource.md` (Stats section) | +| Anything that needs handler tests | (always) | `testing.md` | + +For features that combine patterns (common): read each relevant reference. Example: "admin list of applications with cursor pagination" → `crud-resource.md` + `list-pagination.md`. + +## Universal Rules (apply to ALL patterns) + +These hold regardless of pattern. Don't break them. + +### JSON Envelope + +`app.jsonResponse(w, status, data)` wraps in `{"data": ...}`. Pass a **named response struct**, never a raw slice or model: + +```go +// Right +app.jsonResponse(w, http.StatusOK, ScheduleListResponse{Schedule: items}) +// Wrong +app.jsonResponse(w, http.StatusOK, items) +``` + +| Scenario | Response struct | JSON shape | +|----------|----------------|------------| +| Single item | `Response` | `{"data": {"sponsor": {...}}}` (or just the resource if no wrapper needed) | +| List | `Response` or `ListResponse` | `{"data": {"sponsors": [...]}}` | +| Composite | Descriptive struct | `{"data": {"applicants": [...], "count": 12}}` | + +JSON field names are snake_case. Response structs and their payloads live in the same handler file. + +### Handler Method Pattern + +Every handler is a method on `*application` and follows this flow: + +1. **URL params**: `chi.URLParam(r, "fooID")` — validate non-empty before use. +2. **Body parse**: `readJSON(w, r, &payload)` — only for POST/PUT/PATCH. +3. **Validate**: `Validate.Struct(payload)` — uses `go-playground/validator/v10`. +4. **User from context** (when needed): `getUserFromContext(r.Context())`. +5. **Call store**. +6. **Map errors**: `errors.Is(err, store.ErrNotFound)` → `notFoundResponse`, `errors.Is(err, store.ErrConflict)` → `conflictResponse`, anything else → `internalServerError`. +7. **Respond**: `app.jsonResponse(w, statusCode, ResponseStruct{...})`. + +### Error Helpers + +Use these (`cmd/api/errors.go`) — they log with `method`/`path` already, so don't duplicate: + +| Helper | Status | Use when... | +|--------|--------|-------------| +| `app.badRequestResponse(w, r, err)` | 400 | Bad input, validation failure | +| `app.unauthorizedErrorResponse(w, r, err)` | 401 | Missing/invalid auth | +| `app.forbiddenResponse(w, r, err)` | 403 | Auth ok, role/feature gate fails | +| `app.notFoundResponse(w, r, err)` | 404 | Resource missing | +| `app.conflictResponse(w, r, err)` | 409 | State conflict, duplicate | +| `app.internalServerError(w, r, err)` | 500 | Unexpected error | +| `writeJSONError(w, status, msg)` | any | Custom code (e.g., 503 for service unavailable) | + +### Status Codes + +| Operation | Code | +|-----------|------| +| GET (read) | 200 | +| POST (create) | 201 | +| PUT (full update) | 200 | +| PATCH (partial update) | 200 | +| DELETE | 204 (no body — `w.WriteHeader(http.StatusNoContent)`) | + +### Store Method Pattern + +Every store method: + +1. `ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration); defer cancel()` — `QueryTimeoutDuration` is 5s; double it for bulk operations. +2. Raw SQL with `$1, $2` placeholders (no ORM). +3. `sql.ErrNoRows` → `ErrNotFound`. +4. `RowsAffected() == 0` on UPDATE/DELETE → `ErrNotFound`. +5. INSERT/UPDATE uses `RETURNING` clause to populate generated fields (`id`, `created_at`, `updated_at`) on the input pointer. +6. Unique-constraint violations → `ErrConflict` (check via `pgconn.PgError` code `23505`, or `strings.Contains(err.Error(), "")`). + +### Mock Store Pattern + +For each store method, add a mock method: + +```go +func (m *MockFooStore) GetByID(ctx context.Context, id string) (*Foo, error) { + args := m.Called(id) // pass all params EXCEPT ctx + if args.Get(0) == nil { // nil-check before type assertion + return nil, args.Error(1) + } + return args.Get(0).(*Foo), args.Error(1) +} +``` + +For methods returning only error: just `return args.Error(0)`. For booleans: `args.Bool(0)`. For ints: `args.Int(0)`. Wire up in `NewMockStore()`. + +### Route Mounting (api.go) + +Routes nest by auth/role. Add inside the matching group: + +```go +// /v1/public/* — APIKeyMiddleware (no SuperTokens session) +// /v1/auth/check-email — unauthenticated +// /v1/auth/me — AuthRequiredMiddleware (any logged-in user) +// /v1/health — BasicAuthMiddleware +// inside r.Use(app.AuthRequiredMiddleware): +// /v1/applications/me — hacker self-resource +// inside r.Use(app.RequireRoleMiddleware(store.RoleAdmin)): +// /v1/admin/* — admin or super_admin +// inside r.Use(app.RequireRoleMiddleware(store.RoleSuperAdmin)): +// /v1/superadmin/* — super_admin only +``` + +Settings-gated middleware (`ApplicationsEnabledMiddleware`, `AdminScheduleEditPermissionMiddleware`) wraps subgroups via `r.Group` + `r.Use`. + +### Migration Naming + +Sequential 6-digit prefix: check the highest in `cmd/migrate/migrations/` and increment. Verbs: `create` (foundational), `add` (new feature/table/column/trigger), `alter` (modifying existing), `seed` (initial data). One concern per migration — table + its trigger + indexes go together; enum types separate. + +Tables with `updated_at` reuse the trigger from migration 000002: + +```sql +CREATE TRIGGER trg__updated_at + BEFORE UPDATE ON
FOR EACH ROW + EXECUTE FUNCTION set_updated_at(); +``` + +Conventions: UUID PKs (`gen_random_uuid()`), `TIMESTAMPTZ`, `TEXT[]` (use `store.StringArray`) for arrays, `JSONB` for structured data, columns `NOT NULL DEFAULT ...` whenever sensible. + +### Swagger Annotations + +Every handler has doc comments: + +```go +// @Summary Short summary (Role) +// @Description Longer description +// @Tags +// @Accept json // POST/PUT/PATCH only +// @Produce json +// @Param name in type required "description" +// @Success 200 {object} ResponseStructName +// @Failure 400 {object} object{error=string} +// @Security CookieAuth // omit for public (api-key) endpoints +// @Router /path [method] +``` + +Tags follow the route group: + +| Group | Tag | +|-------|-----| +| `/applications/me`, `/applications/enabled` | `hackers` | +| `/auth/*` | `auth` | +| `/public/*` | `public` | +| `/admin/applications/*` | `admin/applications` | +| `/admin/reviews/*` | `admin/reviews` | +| `/admin/scans/*` | `admin/scans` | +| `/admin/schedule/*` | `admin/schedule` | +| `/admin/sponsors/*` | `admin/sponsors` | +| `/superadmin/applications/*` | `superadmin/applications` | +| `/superadmin/settings/*` | `superadmin/settings` | +| `/superadmin/users/*` | `superadmin/users` | +| `/superadmin/reset-hackathon` | `superadmin` | + +After editing handlers, run `task gen-docs` to regenerate Swagger output (this is also a pre-command for `air`). + +### Logging + +Zap `SugaredLogger` configured for Google Cloud Logging. **Always** use the `w`-suffix structured methods (`Infow`, `Warnw`, `Errorw`). Never `Infof`/`Warnf`/`Errorf`. + +Error helpers (`cmd/api/errors.go`) already log with `method`/`path` — do not add explicit handler-level logs for normal request flows. Add explicit logs only for side-effects outside the request flow (e.g., user creation in middleware, GCS deletion warnings). + +When you do log: include `"method", r.Method, "path", r.URL.Path` and any context (`"user_id"`, `"email"`, `"error"`). + +### Imports & Tooling + +- Module path: `github.com/hackutd/portal/...` +- Routing: `github.com/go-chi/chi` +- Validation: `github.com/go-playground/validator/v10` +- Tests: `github.com/stretchr/testify/{mock,assert,require}` +- DB: `database/sql` (stdlib) + `github.com/jackc/pgx/v5/stdlib` driver +- Postgres errors: `github.com/jackc/pgx/v5/pgconn` (for `*pgconn.PgError`) +- Logging: `go.uber.org/zap` + +### Things to Avoid + +- An ORM. Raw SQL only. +- Calling external HTTP services from store methods. +- Passing raw slices/models to `app.jsonResponse` — wrap in named struct. +- Skipping `context.WithTimeout` in store methods. +- Hardcoding settings keys — define `SettingsKey` constants. +- Query params for setter handlers — use `readJSON` with a payload struct. +- Putting settings-table queries in resource-specific stores — they belong on `Settings`. +- Reaching for new abstractions when the existing pattern fits. diff --git a/.claude/skills/backend/references/public-api.md b/.claude/skills/backend/references/public-api.md new file mode 100644 index 00000000..f263c092 --- /dev/null +++ b/.claude/skills/backend/references/public-api.md @@ -0,0 +1,124 @@ +# Public API Pattern (X-API-Key Auth) + +For endpoints exposed to non-portal clients (e.g., the public hackathon website pulling sponsor data). Auth is by static API key in `X-API-Key` header — not SuperTokens session. + +## When to Use + +External, headless clients need read-only data the portal already has. Examples: public schedule, public sponsor list. Don't use this for write endpoints — anything mutating state belongs behind real auth. + +Working example: **Public schedule, public sponsors** (`cmd/api/public.go`). + +## The Auth Layer + +`APIKeyMiddleware` lives in `cmd/api/middlewares.go`: + +```go +func (app *application) APIKeyMiddleware(next http.Handler) http.Handler { + expectedKey := []byte(app.config.auth.publicAPIKey) + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := r.Header.Get("X-API-Key") + if key == "" || subtle.ConstantTimeCompare([]byte(key), expectedKey) != 1 { + app.unauthorizedErrorResponse(w, r, fmt.Errorf("invalid or missing API key")) + return + } + next.ServeHTTP(w, r) + }) +} +``` + +Constant-time compare via `crypto/subtle` — prevents timing attacks. + +## Routes + +Public endpoints sit on `/v1/public/*`, separate from `/auth/*` (which SuperTokens owns) and the role-gated `/admin/*` / `/superadmin/*` trees. + +```go +r.Route("/v1", func(r chi.Router) { + r.Route("/public", func(r chi.Router) { + r.Use(app.APIKeyMiddleware) + r.Get("/schedule", app.getPublicScheduleHandler) + r.Get("/sponsors", app.getPublicSponsorsHandler) + }) + // ... rest of routes ... +}) +``` + +CORS is configured separately in `mount()` — `publicCORSOrigin` is appended to allowed origins so the public website can hit these endpoints. + +## Handlers + +Often the public endpoint returns the same data as an internal one. Just delegate: + +```go +// @Summary Get schedule (Public) +// @Description Returns the full event schedule, ordered by start time ascending +// @Tags public +// @Produce json +// @Param X-API-Key header string true "API Key" +// @Success 200 {object} ScheduleListResponse +// @Failure 401 {object} object{error=string} +// @Failure 500 {object} object{error=string} +// @Router /public/schedule [get] +func (app *application) getPublicScheduleHandler(w http.ResponseWriter, r *http.Request) { + app.listScheduleHandler(w, r) +} +``` + +Important Swagger differences from authenticated endpoints: +- **No `@Security`** — public endpoints aren't cookie-authenticated. Document the API key as a `header` param instead. +- **`@Tags public`** — the spec orders these as one of the first tag groups. + +If the public version has different shape from the admin version (e.g., redacted fields), don't delegate — write a separate handler that calls a tailored store method. + +## Tests + +Public endpoints are easiest to test through the full router (`app.mount()`) so the API key middleware runs: + +```go +func TestGetPublicSchedule(t *testing.T) { + t.Run("returns 401 without API key", func(t *testing.T) { + app := newTestApplication(t) + mux := app.mount() + + req, _ := http.NewRequest(http.MethodGet, "/v1/public/schedule", nil) + rr := executeRequest(req, mux) + checkResponseCode(t, http.StatusUnauthorized, rr.Code) + }) + + t.Run("returns 200 with schedule items", func(t *testing.T) { + app := newTestApplication(t) + mockSchedule := app.store.Schedule.(*store.MockScheduleStore) + mux := app.mount() + + mockSchedule.On("List").Return([]store.ScheduleItem{ /* ... */ }, nil).Once() + + req, _ := http.NewRequest(http.MethodGet, "/v1/public/schedule", nil) + req.Header.Set("X-API-Key", "test-api-key") // matches newTestApplication config + + rr := executeRequest(req, mux) + checkResponseCode(t, http.StatusOK, rr.Code) + // ... decode body and assert ... + }) +} +``` + +`newTestApplication(t)` sets `auth.publicAPIKey: "test-api-key"`. Tests must use that exact value. + +Coverage checklist: + +| Case | Expected | +|------|----------| +| No header | 401 | +| Wrong key | 401 | +| Correct key | 200 with body | +| Empty result | 200 with empty array | +| Store error | 500 | + +See `cmd/api/public_test.go`. + +## What NOT to Do + +- Don't expose write endpoints through the public API key. Mutations belong behind authenticated routes. +- Don't compare keys with `==` on raw strings — use `subtle.ConstantTimeCompare`. +- Don't add `@Security CookieAuth` to public Swagger comments. Document the key as a header param instead. +- Don't reuse the SuperTokens cookie origin for public endpoints — `publicCORSOrigin` is its own config. diff --git a/.claude/skills/backend/references/self-resource.md b/.claude/skills/backend/references/self-resource.md new file mode 100644 index 00000000..d4ce5c4b --- /dev/null +++ b/.claude/skills/backend/references/self-resource.md @@ -0,0 +1,289 @@ +# Self-Resource Pattern (`/me` endpoints) + +For resources where each user has their own copy and manages it themselves. Examples: hacker application (`/applications/me`). + +Distinguishing features vs. CRUD: +- Resource is scoped by `user.ID` from the auth context, never by URL param. +- "Get-or-create" is the typical entry point. +- A state machine governs writability (e.g., `draft` mutable, `submitted` frozen). +- `/submit` (or similar transition) is a domain-specific verb, not a generic update. + +## When to Use + +User says "users should be able to manage their own X" / "applicants edit their application" / "members configure their profile". The handler reads the user from context and never trusts an ID from the URL or body. + +Working example: **Applications** (`internal/store/applications.go`, `cmd/api/applications.go`). + +## 1. Migration + +The table has a unique index on `user_id` (one row per user) and may include a status column with an enum type. + +```sql +-- 000NNN_add_application_types.up.sql (separate migration for the enum) +CREATE TYPE application_status AS ENUM ('draft', 'submitted', 'accepted', 'rejected', 'waitlisted'); + +-- 000NNN+1_add_applications.up.sql +CREATE TABLE applications ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + user_id UUID NOT NULL UNIQUE REFERENCES users(id) ON DELETE CASCADE, + status application_status NOT NULL DEFAULT 'draft', + responses JSONB NOT NULL DEFAULT '{}', + submitted_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE TRIGGER trg_applications_updated_at +BEFORE UPDATE ON applications FOR EACH ROW EXECUTE FUNCTION set_updated_at(); +``` + +Enum types live in their own migration (separate from the table that uses them). The unique constraint on `user_id` enables the get-or-create race recovery below. + +## 2. Store + +### Model with status enum + +```go +type ApplicationStatus string + +const ( + StatusDraft ApplicationStatus = "draft" + StatusSubmitted ApplicationStatus = "submitted" + StatusAccepted ApplicationStatus = "accepted" + StatusRejected ApplicationStatus = "rejected" + StatusWaitlisted ApplicationStatus = "waitlisted" +) + +type Application struct { + ID string `json:"id"` + UserID string `json:"user_id"` + Status ApplicationStatus `json:"status"` + Responses json.RawMessage `json:"responses"` + SubmittedAt *time.Time `json:"submitted_at"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} +``` + +### GetByUserID, Create, Update — standard + +`GetByUserID` returns `ErrNotFound` on `sql.ErrNoRows`. `Create` only requires `user_id` (other fields default in SQL). Translate the unique-constraint violation to `ErrConflict`: + +```go +func (s *ApplicationsStore) Create(ctx context.Context, app *Application) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + err := s.db.QueryRowContext(ctx, ` + INSERT INTO applications (user_id) VALUES ($1) + RETURNING id, status, responses, created_at, updated_at + `, app.UserID).Scan(&app.ID, &app.Status, &app.Responses, &app.CreatedAt, &app.UpdatedAt) + if err != nil { + if strings.Contains(err.Error(), "applications_user_id_key") { + return ErrConflict // race: another request created it + } + return err + } + return nil +} +``` + +### State-transition method (Submit) + +Constrain via SQL `WHERE status = 'draft'` so concurrent submits become idempotent. `sql.ErrNoRows` here means *not in draft* — return `ErrConflict`: + +```go +func (s *ApplicationsStore) Submit(ctx context.Context, app *Application) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + err := s.db.QueryRowContext(ctx, ` + UPDATE applications + SET status = 'submitted', submitted_at = NOW() + WHERE id = $1 AND status = 'draft' + RETURNING status, submitted_at, updated_at + `, app.ID).Scan(&app.Status, &app.SubmittedAt, &app.UpdatedAt) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return ErrConflict + } + return err + } + return nil +} +``` + +## 3. Handlers + +### Get-or-Create + +```go +func (app *application) getOrCreateApplicationHandler(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + if user == nil { + app.unauthorizedErrorResponse(w, r, nil) + return + } + + application, err := app.store.Application.GetByUserID(r.Context(), user.ID) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + // Create a new draft + application = &store.Application{UserID: user.ID} + if err := app.store.Application.Create(r.Context(), application); err != nil { + if errors.Is(err, store.ErrConflict) { + // Race: another request created it. Refetch. + application, err = app.store.Application.GetByUserID(r.Context(), user.ID) + if err != nil { + app.internalServerError(w, r, err) + return + } + } else { + app.internalServerError(w, r, err) + return + } + } + } else { + app.internalServerError(w, r, err) + return + } + } + // ... build response, possibly embedding related data ... + app.jsonResponse(w, http.StatusOK, application) +} +``` + +The race recovery is non-obvious but important — without it, two concurrent first-time loads can fail. **Always include it for `/me` resources with a unique constraint on `user_id`.** + +### Update with state guard + +Patch-shape body so partial updates work; reject if the resource has left the editable state: + +```go +type UpdateApplicationPayload struct { + Responses json.RawMessage `json:"responses"` // pointer-like via RawMessage + ResumePath *string `json:"resume_path"` // *string for nullable field +} + +func (app *application) updateApplicationHandler(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + if user == nil { app.unauthorizedErrorResponse(w, r, nil); return } + + application, err := app.store.Application.GetByUserID(r.Context(), user.ID) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("application not found")) + return + } + app.internalServerError(w, r, err) + return + } + if application.Status != store.StatusDraft { + app.conflictResponse(w, r, errors.New("cannot update submitted application")) + return + } + + var req UpdateApplicationPayload + if err := readJSON(w, r, &req); err != nil { + app.badRequestResponse(w, r, err) + return + } + + // Apply only fields present in the request + if req.Responses != nil { application.Responses = req.Responses } + if req.ResumePath != nil { application.ResumePath = req.ResumePath } + + if err := app.store.Application.Update(r.Context(), application); err != nil { + app.internalServerError(w, r, err) + return + } + app.jsonResponse(w, http.StatusOK, application) +} +``` + +For PATCH-style endpoints, use `json.RawMessage` (for opaque blobs) and `*T` (for nullable scalars) — checking `!= nil` lets you distinguish "field absent" from "field set to zero value". Don't use validate tags meant for required POST bodies. + +### Submit (state-transition verb) + +Run domain validation before flipping state; the store call itself enforces "must be in draft": + +```go +func (app *application) submitApplicationHandler(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + if user == nil { app.unauthorizedErrorResponse(w, r, nil); return } + + application, err := app.store.Application.GetByUserID(r.Context(), user.ID) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("application not found")) + return + } + app.internalServerError(w, r, err) + return + } + if application.Status != store.StatusDraft { + app.conflictResponse(w, r, errors.New("application already submitted")) + return + } + + // Domain validation (e.g., schema-driven, business-rule checks) + if errs := validateAgainstSchema(/* ... */); len(errs) > 0 { + app.badRequestResponse(w, r, fmt.Errorf("validation errors: %v", errs)) + return + } + + if err := app.store.Application.Submit(r.Context(), application); err != nil { + app.internalServerError(w, r, err) + return + } + app.jsonResponse(w, http.StatusOK, application) +} +``` + +## 4. Routes + +Mount under `AuthRequiredMiddleware`. Often gated by a settings middleware (e.g., `ApplicationsEnabledMiddleware`): + +```go +r.Group(func(r chi.Router) { + r.Use(app.AuthRequiredMiddleware) + + r.Route("/applications", func(r chi.Router) { + r.Get("/me", app.getOrCreateApplicationHandler) + r.Get("/enabled", app.getApplicationsEnabled) + + r.Group(func(r chi.Router) { + r.Use(app.ApplicationsEnabledMiddleware) + r.Patch("/me", app.updateApplicationHandler) + r.Post("/me/submit", app.submitApplicationHandler) + // file upload endpoints go here too — see file-storage.md + }) + }) +}) +``` + +## 5. Tests + +Coverage checklist: + +| Handler | Cases | +|---------|-------| +| Get-or-Create | returns existing, creates draft when none, handles race (Create returns Conflict → re-fetches) | +| Update | success, 409 when not in editable state, 404 when no resource, 400 on bad JSON | +| Submit (or other state transition) | success, 409 when not in editable state, 404 when no resource, 400 on validation failure | + +The race recovery test uses three calls in order: + +```go +mockApps.On("GetByUserID", user.ID).Return(nil, store.ErrNotFound).Once() +mockApps.On("Create", mock.AnythingOfType("*store.Application")).Return(store.ErrConflict).Once() +mockApps.On("GetByUserID", user.ID).Return(existing, nil).Once() +``` + +See `cmd/api/applications_test.go` for the full set. Inject the user with `setUserContext(req, user)` — see `testing.md`. + +## What NOT to Do + +- Don't accept the resource ID via URL — read it via `user.ID` from context, then look up the resource. +- Don't allow direct `Update` from a `submitted`/`accepted` state — gate at the handler AND in SQL. +- Don't replace `responses` blindly — for PATCH-style updates, only apply fields that are present in the request body. +- Don't skip the conflict-recovery branch in get-or-create — silent failures under load. diff --git a/.claude/skills/backend/references/settings-pattern.md b/.claude/skills/backend/references/settings-pattern.md new file mode 100644 index 00000000..254f9091 --- /dev/null +++ b/.claude/skills/backend/references/settings-pattern.md @@ -0,0 +1,153 @@ +# Settings Pattern Reference + +Settings are key-value pairs in the `settings` table with JSONB values. All settings code lives in `SettingsStore` / `Settings` interface — never in resource-specific stores. + +## Migration + +```sql +-- Up +INSERT INTO settings (key, value) VALUES ('my_setting', 'true'::jsonb) +ON CONFLICT (key) DO NOTHING; + +-- Down +DELETE FROM settings WHERE key = 'my_setting'; +``` + +## Store (`internal/store/settings.go`) + +Define a constant — never hardcode key strings: + +```go +const SettingsKeyMySetting = "my_setting" +``` + +### Getter + +Scan into `[]byte` + `json.Unmarshal`. Handle `sql.ErrNoRows` with a sensible default. + +```go +func (s *SettingsStore) GetMySettingEnabled(ctx context.Context) (bool, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + var value []byte + err := s.db.QueryRowContext(ctx, `SELECT value FROM settings WHERE key = $1`, SettingsKeyMySetting).Scan(&value) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return true, nil + } + return false, err + } + + var enabled bool + if err := json.Unmarshal(value, &enabled); err != nil { + return false, err + } + return enabled, nil +} +``` + +### Setter + +Return only `error`. Use `json.Marshal` + upsert. + +```go +func (s *SettingsStore) SetMySettingEnabled(ctx context.Context, enabled bool) error { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + jsonValue, err := json.Marshal(enabled) + if err != nil { + return err + } + + query := ` + INSERT INTO settings (key, value) + VALUES ($1, $2) + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = NOW() + ` + _, err = s.db.ExecContext(ctx, query, SettingsKeyMySetting, string(jsonValue)) + return err +} +``` + +## Interface (`internal/store/storage.go`) + +```go +Settings interface { + GetMySettingEnabled(ctx context.Context) (bool, error) + SetMySettingEnabled(ctx context.Context, enabled bool) error +} +``` + +## Mock (`internal/store/mock_store.go`) + +```go +func (m *MockSettingsStore) GetMySettingEnabled(ctx context.Context) (bool, error) { + args := m.Called() + return args.Bool(0), args.Error(1) +} + +func (m *MockSettingsStore) SetMySettingEnabled(ctx context.Context, enabled bool) error { + args := m.Called(enabled) + return args.Error(0) +} +``` + +Use `args.Bool(0)` for booleans. Pass params (except ctx) to `m.Called()`. + +## Handlers (`cmd/api/settings.go`) + +Payload, response structs, and handlers all live in `settings.go`. + +```go +type SetMySettingPayload struct { + Enabled bool `json:"enabled"` +} + +type MySettingResponse struct { + Enabled bool `json:"enabled"` +} +``` + +Setter handlers use `readJSON` with a payload struct — never query params. Swagger `@Tags` is `superadmin/settings`. Response can type-convert from payload: `MySettingResponse(req)`. + +## Settings-Gated Middleware (`cmd/api/middlewares.go`) + +```go +func (app *application) MySettingMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + if user == nil { + app.unauthorizedErrorResponse(w, r, fmt.Errorf("user not in context")) + return + } + if user.Role == store.RoleSuperAdmin { + next.ServeHTTP(w, r) + return + } + enabled, err := app.store.Settings.GetMySettingEnabled(r.Context()) + if err != nil { + app.internalServerError(w, r, err) + return + } + if !enabled { + app.forbiddenResponse(w, r, fmt.Errorf("feature is currently disabled")) + return + } + next.ServeHTTP(w, r) + }) +} +``` + +Wire with `r.Group` + `r.Use` in `api.go`. + +## Middleware Test Coverage + +| Case | Expected | +|------|----------| +| No user in context | 401 | +| Disabled + non-super-admin | 403 | +| Enabled + non-super-admin | 200 | +| Super admin + disabled | 200 (bypass) | +| Store error | 500 | diff --git a/.claude/skills/backend/references/status-update.md b/.claude/skills/backend/references/status-update.md new file mode 100644 index 00000000..07b3a929 --- /dev/null +++ b/.claude/skills/backend/references/status-update.md @@ -0,0 +1,206 @@ +# Status / Field Update Pattern + +For PATCH endpoints that change a single whitelisted field on a resource — typically a status, role, or other enum-bounded value. Thin handler, thin store method, strict `oneof=` validation. + +## When to Use + +The user is asking for "approve / reject / waitlist", "change role", "promote to admin", "set status to X". The endpoint touches one field, the value is from a fixed set, and only privileged users can call it. + +Don't use this for general-purpose updates — those belong in `crud-resource.md` or `self-resource.md`. + +Working examples: +- **Application status** (`PATCH /superadmin/applications/{id}/status`). +- **User role** (`PATCH /superadmin/users/{userID}/role`). +- **AI percent on a review** (`PUT /admin/applications/{id}/ai-percent`) — same shape but with a numeric `min/max` validation. + +## Payload + Response Shape + +Single-field payload + response wrapping the updated entity: + +```go +type SetStatusPayload struct { + Status store.ApplicationStatus `json:"status" validate:"required,oneof=accepted rejected waitlisted"` +} + +type ApplicationResponse struct { + Application *store.Application `json:"application"` +} +``` + +`oneof=...` is the critical validation — anything outside the allowed values returns 400. + +For role updates use `oneof=hacker admin super_admin`. For numeric scores use `min=0,max=100`. + +## Store + +Returns the **updated entity** so the client doesn't need a follow-up GET: + +```go +func (s *ApplicationsStore) SetStatus(ctx context.Context, id string, status ApplicationStatus) (*Application, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + query := ` + UPDATE applications + SET status = $2, updated_at = NOW() + WHERE id = $1 + RETURNING ` + applicationSelectCols // share the column list with GetByID/etc. + + var app Application + if err := scanApplication(s.db.QueryRowContext(ctx, query, id, status), &app); err != nil { + if errors.Is(err, sql.ErrNoRows) { return nil, ErrNotFound } + return nil, err + } + return &app, nil +} +``` + +Add to the interface in `internal/store/storage.go`: + +```go +Application interface { + // ... + SetStatus(ctx context.Context, id string, status ApplicationStatus) (*Application, error) +} +``` + +Mock: + +```go +func (m *MockApplicationStore) SetStatus(ctx context.Context, id string, status ApplicationStatus) (*Application, error) { + args := m.Called(id, status) + if args.Get(0) == nil { return nil, args.Error(1) } + return args.Get(0).(*Application), args.Error(1) +} +``` + +### Conditional update (when ownership / state matters) + +If the change should only succeed under certain conditions, encode them in the SQL `WHERE` clause and treat 0 rows affected (or `sql.ErrNoRows` on `RETURNING`) as `ErrNotFound`: + +```go +// SetAIPercent: succeeds only if the admin is assigned and the value isn't already set +query := ` + UPDATE applications + SET ai_percent = $3 + WHERE id = $1 + AND ai_percent IS NULL + AND EXISTS ( + SELECT 1 FROM application_reviews + WHERE application_id = $1 AND admin_id = $2 + ) +` +result, err := s.db.ExecContext(ctx, query, applicationID, adminID, percent) +if err != nil { return err } +rows, err := result.RowsAffected() +if err != nil { return err } +if rows == 0 { return ErrNotFound } // bundle "not found" + "not assigned" + "already set" +return nil +``` + +The handler can return a generic 404 with a message that covers all three cases. Don't add a separate "AlreadySet" error type unless the UI needs to differentiate. + +## Handler + +Standard handler flow: + +```go +// @Summary Set application status (Super Admin) +// @Tags superadmin/applications +// @Accept json +// @Produce json +// @Param applicationID path string true "Application ID" +// @Param status body SetStatusPayload true "New status" +// @Success 200 {object} ApplicationResponse +// @Failure 400 {object} object{error=string} +// @Failure 404 {object} object{error=string} +// @Security CookieAuth +// @Router /superadmin/applications/{applicationID}/status [patch] +func (app *application) setApplicationStatus(w http.ResponseWriter, r *http.Request) { + applicationID := chi.URLParam(r, "applicationID") + if applicationID == "" { + app.badRequestResponse(w, r, errors.New("application ID is required")) + return + } + + var payload SetStatusPayload + if err := readJSON(w, r, &payload); err != nil { app.badRequestResponse(w, r, err); return } + if err := Validate.Struct(payload); err != nil { app.badRequestResponse(w, r, err); return } + + application, err := app.store.Application.SetStatus(r.Context(), applicationID, payload.Status) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + app.notFoundResponse(w, r, errors.New("application not found")); return + } + app.internalServerError(w, r, err) + return + } + + app.jsonResponse(w, http.StatusOK, ApplicationResponse{Application: application}) +} +``` + +For routes pulling user from context (e.g., `setAIPercent` scoping by admin), grab it after URL param validation: + +```go +user := getUserFromContext(r.Context()) +// ... call store.SetAIPercent(ctx, applicationID, user.ID, req.AIPercent) ... +``` + +## Method Choice + +| Style | Use when | +|-------|----------| +| `PATCH /resource/{id}/status` | Updating a single sub-field of a resource (most common) | +| `PUT /resource/{id}/` | Updating one field, but the convention or naming reads better as PUT (e.g., `ai-percent`) | + +Both return 200 (not 204) — the response body carries the updated entity. + +## Routes + +Mount with the matching role gate: + +```go +r.Group(func(r chi.Router) { + r.Use(app.RequireRoleMiddleware(store.RoleSuperAdmin)) + r.Route("/superadmin", func(r chi.Router) { + r.Route("/applications", func(r chi.Router) { + r.Patch("/{applicationID}/status", app.setApplicationStatus) + }) + r.Route("/users", func(r chi.Router) { + r.Patch("/{userID}/role", app.updateUserRoleHandler) + }) + }) +}) +``` + +Sub-resource paths like `/{id}/status` keep the URL self-describing — easier than overloading PATCH on the parent route. + +## Tests + +Coverage checklist: + +| Case | Expected | +|------|----------| +| Valid value | 200; store called with parsed enum | +| Missing URL param | 400 | +| Invalid value (not in `oneof`) | 400 | +| Resource not found | 404 | +| Store error | 500 | + +Inject the URL param via chi route context: + +```go +rctx := chi.NewRouteContext() +rctx.URLParams.Add("applicationID", "app-1") +req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) +``` + +See `cmd/api/applications_test.go:548-603` (`TestSetApplicationStatus`) and `cmd/api/superadmin_users_test.go:93-144`. + +## What NOT to Do + +- Don't accept a free-form string and validate manually — use `validate:"oneof=..."` and fail at the boundary. +- Don't return 204 — clients want the updated resource. PATCH/PUT return 200 with the new state. +- Don't update multiple fields here. If the user wants that, use the resource's main `Update` method (see `crud-resource.md`). +- Don't differentiate "wrong owner" from "not found" with separate error types if you don't want to leak existence info — collapse them into one 404. diff --git a/.claude/skills/backend/references/testing.md b/.claude/skills/backend/references/testing.md new file mode 100644 index 00000000..adcd6720 --- /dev/null +++ b/.claude/skills/backend/references/testing.md @@ -0,0 +1,283 @@ +# Testing Patterns + +Shared utilities and recipes for handler tests in `cmd/api/`. All tests live in the same package as handlers (`package main`), use testify (`mock`, `assert`, `require`), and the mock store from `internal/store/mock_store.go`. + +Read this alongside the pattern reference for the endpoint you're testing — each pattern reference has a coverage checklist tailored to its shape. + +## Test Utilities + +All in `cmd/api/test_utils_test.go`. Use these instead of duplicating setup. + +| Helper | What it does | +|--------|-------------| +| `newTestApplication(t)` | Build `*application` with mock store, no-op zap logger, mock GCS, mock mailer, real fixed-window rate limiter (20/5s), basic auth `testuser:testpass`, public API key `test-api-key`. | +| `executeRequest(req, mux)` | Run req through handler/mux, return `*httptest.ResponseRecorder`. | +| `checkResponseCode(t, expected, actual)` | Assert status code with descriptive failure. | +| `addBasicAuth(req)` | Set `Authorization: Basic ` header. | +| `setUserContext(req, user)` | Inject `*store.User` into request context (bypasses real auth middleware). | +| `newTestUser()` | `RoleHacker`, ID `user-1`. | +| `newAdminUser()` | `RoleAdmin`, ID `admin-1`. | +| `newSuperAdminUser()` | `RoleSuperAdmin`, ID `superadmin-1`. | + +SuperTokens is initialized once via `initTestSuperTokens(t)` so `app.mount()` doesn't panic — handled inside `newTestApplication`. + +## Three Ways to Wire a Handler + +### 1) Direct handler — no URL params, no middleware + +The simplest setup. Wrap with `http.HandlerFunc` so the signature matches `http.Handler`: + +```go +rr := executeRequest(req, http.HandlerFunc(app.listScheduleHandler)) +``` + +### 2) Mini chi router — URL params + +When the handler reads `chi.URLParam`, build a tiny router so chi populates the context: + +```go +func scheduleRouter(app *application) chi.Router { + r := chi.NewRouter() + r.Put("/{scheduleID}", app.updateScheduleHandler) + r.Delete("/{scheduleID}", app.deleteScheduleHandler) + return r +} + +// In test: +r := scheduleRouter(app) +req, _ := http.NewRequest(http.MethodDelete, "/item-1", nil) +rr := executeRequest(req, r) +``` + +For a one-off, inject the route context directly without a router: + +```go +rctx := chi.NewRouteContext() +rctx.URLParams.Add("applicationID", "app-1") +req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) +``` + +A small helper in the test file is fine when reused: + +```go +withSponsorRouteParam := func(req *http.Request, sponsorID string) *http.Request { + rctx := chi.NewRouteContext() + rctx.URLParams.Add("sponsorID", sponsorID) + return req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) +} +``` + +### 3) Full mux — middleware, integration + +When you need the actual middleware chain (CORS, auth, rate-limiting, role gates): + +```go +mux := app.mount() +req, _ := http.NewRequest(http.MethodGet, "/v1/public/schedule", nil) +req.Header.Set("X-API-Key", "test-api-key") +rr := executeRequest(req, mux) +``` + +Use this for: public-API auth tests, role-gate enforcement, settings-gated middleware tests, end-to-end "the route is mounted" smoke tests. + +For settings-gated middleware tests over a slimmer subset of routes, build a minimal router: + +```go +func protectedScheduleMutationRouter(app *application) chi.Router { + r := chi.NewRouter() + r.With(app.AdminScheduleEditPermissionMiddleware).Post("/", app.createScheduleHandler) + r.With(app.AdminScheduleEditPermissionMiddleware).Put("/{scheduleID}", app.updateScheduleHandler) + r.With(app.AdminScheduleEditPermissionMiddleware).Delete("/{scheduleID}", app.deleteScheduleHandler) + return r +} +``` + +## Working with the Mock Store + +Each store interface has a `MockStore` (testify/mock). Assert on it through a typed cast: + +```go +app := newTestApplication(t) +mockApps := app.store.Application.(*store.MockApplicationStore) +mockApps.On("GetByUserID", user.ID).Return(existing, nil).Once() +``` + +`m.Called(args...)` receives all params **except `ctx`** — match those exactly. Use `mock.Anything` (only when the value is non-deterministic), `mock.AnythingOfType("*store.Application")` (typed wildcard), or `mock.MatchedBy(func(x T) bool { ... })` (predicate match): + +```go +mockSchedule.On("Create", mock.AnythingOfType("*store.ScheduleItem")).Return(nil).Once() + +mockGCS.On("GenerateUploadURL", mock.Anything, mock.MatchedBy(func(p string) bool { + return strings.HasPrefix(p, "resumes/"+user.ID+"/") && strings.HasSuffix(p, ".pdf") +})).Return("https://upload.example.com", nil).Once() +``` + +Use `.Once()` on every expectation, then `mockApps.AssertExpectations(t)` at the end of the subtest. This catches missing or extra calls. + +To inspect/mutate args at call time (e.g., to populate generated fields like ID): + +```go +mockSponsors.On("Create", mock.AnythingOfType("*store.Sponsor")).Run(func(args mock.Arguments) { + sponsor := args.Get(0).(*store.Sponsor) + sponsor.ID = "new-sponsor" +}).Return(nil).Once() +``` + +## Decoding Responses + +The JSON envelope is `{"data": ...}`. Decode into a wrapper struct in the test: + +```go +var body struct { + Data ScheduleListResponse `json:"data"` +} +require.NoError(t, json.NewDecoder(rr.Body).Decode(&body)) +assert.Len(t, body.Data.Schedule, 2) +``` + +For error responses, decode into: + +```go +var body struct{ Error string `json:"error"` } +require.NoError(t, json.NewDecoder(rr.Body).Decode(&body)) +assert.Contains(t, body.Error, "first_name is required") +``` + +## Mock GCS / Mailer + +`app.gcsClient` is `*gcs.MockClient`, `app.mailer` is `*mailer.MockClient`. Same testify/mock pattern: + +```go +mockGCS := app.gcsClient.(*gcs.MockClient) +mockGCS.On("DeleteObject", mock.Anything, resumePath).Return(nil).Once() +``` + +To test the "GCS not configured" branch, set `app.gcsClient = nil` for the duration of the subtest and restore it afterward: + +```go +app.gcsClient = nil +// ... run subtest ... +app.gcsClient = mockGCS +``` + +## Testify Choices + +| Need | Use | +|------|-----| +| Assert and continue | `assert.Equal`, `assert.Len`, `assert.True`, `assert.Contains` | +| Assert and stop on failure | `require.NoError`, `require.NotNil` | +| Mock expectations | `mock.On(...).Return(...).Once()` then `mock.AssertExpectations(t)` | + +`require` aborts the test on failure (use it for setup/decoding where downstream assertions would panic otherwise). `assert` continues (use it for verifying the actual behavior you care about, where you want to see all failures). + +## Subtests + +Use `t.Run("description", func(t *testing.T) { ... })` for each scenario. Phrase the description as the expected outcome: + +```go +func TestUpdateSchedule(t *testing.T) { + t.Run("returns 200 on success", func(t *testing.T) { /* ... */ }) + t.Run("returns 404 when not found", func(t *testing.T) { /* ... */ }) +} +``` + +The `app := newTestApplication(t)` and mock setup typically live inside each subtest so state doesn't leak. + +## Recipes + +### Test a settings-gated middleware + +```go +func TestApplicationsEnabledMiddleware(t *testing.T) { + app := newTestApplication(t) + ok := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + t.Run("403 when applications are disabled", func(t *testing.T) { + app.store.Settings.(*store.MockSettingsStore). + On("GetApplicationsEnabled", mock.Anything).Return(false, nil).Once() + + handler := app.ApplicationsEnabledMiddleware(ok) + req, _ := http.NewRequest(http.MethodGet, "/", nil) + req = setUserContext(req, newTestUser()) + + rr := executeRequest(req, handler) + checkResponseCode(t, http.StatusForbidden, rr.Code) + }) + + t.Run("super admin bypasses gate", func(t *testing.T) { + handler := app.ApplicationsEnabledMiddleware(ok) + req, _ := http.NewRequest(http.MethodGet, "/", nil) + req = setUserContext(req, newSuperAdminUser()) + + rr := executeRequest(req, handler) + checkResponseCode(t, http.StatusOK, rr.Code) + // No mock setup needed — super admin path doesn't read the setting + }) +} +``` + +### Test a public (API key) endpoint via the full mux + +```go +mux := app.mount() +req, _ := http.NewRequest(http.MethodGet, "/v1/public/schedule", nil) +req.Header.Set("X-API-Key", "test-api-key") +rr := executeRequest(req, mux) +``` + +### Inject user + URL param + body + +```go +body := strings.NewReader(`{"vote":"accept"}`) +req, _ := http.NewRequest(http.MethodPut, "/", body) +req.Header.Set("Content-Type", "application/json") +req = setUserContext(req, newAdminUser()) + +rctx := chi.NewRouteContext() +rctx.URLParams.Add("reviewID", "rev-1") +req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + +rr := executeRequest(req, http.HandlerFunc(app.submitVote)) +``` + +### Test a paginated list handler + +Construct the exact filters/cursor/direction/limit expected by the store: + +```go +status := store.StatusSubmitted +result := &store.ApplicationListResult{Applications: []store.ApplicationListItem{}, HasMore: false} + +mockApps.On("List", + store.ApplicationListFilters{Status: &status, SortBy: store.SortByRejectVotes}, + (*store.ApplicationCursor)(nil), + store.DirectionForward, + 50, +).Return(result, nil).Once() + +req, _ := http.NewRequest(http.MethodGet, "/?status=submitted&sort_by=reject_votes", nil) +req = setUserContext(req, newAdminUser()) +rr := executeRequest(req, http.HandlerFunc(app.listApplicationsHandler)) +checkResponseCode(t, http.StatusOK, rr.Code) +``` + +For the validation cases, no store call is expected — leave the mock empty and assert only the status code: + +```go +t.Run("400 for invalid status", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "/?status=invalid", nil) + req = setUserContext(req, newAdminUser()) + rr := executeRequest(req, http.HandlerFunc(app.listApplicationsHandler)) + checkResponseCode(t, http.StatusBadRequest, rr.Code) +}) +``` + +## What NOT to Do + +- Don't write a test that creates its own `*application` from scratch — use `newTestApplication(t)`. It already wires SuperTokens, mocks, logger, rate-limiter. +- Don't test through the real DB. Mock the store. +- Don't forget `mock.AssertExpectations(t)` — without it, missing calls don't fail the test. +- Don't reuse a single `app` across subtests when subtest A sets up a mock that subtest B doesn't expect — pull `app := newTestApplication(t)` inside the subtest. +- Don't mock `ctx` in `m.Called` — the mock pattern omits it. diff --git a/.claude/skills/backend/references/workflow-queue.md b/.claude/skills/backend/references/workflow-queue.md new file mode 100644 index 00000000..1d208d10 --- /dev/null +++ b/.claude/skills/backend/references/workflow-queue.md @@ -0,0 +1,258 @@ +# Workflow Queue Pattern + +For features where work items are claimed by an admin, processed, and submitted. Examples: application reviews (`pending` → `next` → `submit` → `completed`). + +## When to Use + +You have a stream of work items distributed across many admins, and: +- An admin wants to **see what's assigned to them** (pending list). +- An admin wants the **next item to work on** (atomic claim from a queue). +- An admin **submits a result** that closes the item. +- An admin wants to **see what they've already done** (completed list). + +There's also a separate "batch assign" pattern (super-admin pre-distributes work) — that lives in `bulk-operations.md`. + +Working example: **Application Reviews** (`internal/store/reviews.go`, `cmd/api/reviews.go`). + +## Data Model + +A join table between the work-target and the assigned admin, with a nullable result column: + +```sql +CREATE TABLE application_reviews ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + application_id UUID NOT NULL REFERENCES applications(id) ON DELETE CASCADE, + admin_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, + vote review_vote, -- nullable: NULL = pending, set = completed + notes TEXT, + assigned_at TIMESTAMPTZ NOT NULL DEFAULT now(), + reviewed_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + UNIQUE (application_id, admin_id) -- prevent double-assign +); +``` + +The unique pair `(target_id, admin_id)` is critical — it makes assignments idempotent and lets `ON CONFLICT DO NOTHING` work in the bulk-assign pattern. + +## Store Methods + +### Pending list (per admin) + +```go +func (s *ApplicationReviewsStore) GetPendingByAdminID(ctx context.Context, adminID string) ([]ApplicationReviewWithDetails, error) { + // SELECT ar.*, joined application + user fields ... + // WHERE ar.admin_id = $1 AND ar.vote IS NULL + // ORDER BY ar.assigned_at ASC +} +``` + +Pending = `vote IS NULL`. Order oldest-first. Hydrate with relevant target details (joined applicant info) so the admin can review without a second round-trip. + +### Completed list (per admin) + +```go +// WHERE ar.admin_id = $1 AND ar.vote IS NOT NULL +// ORDER BY ar.reviewed_at DESC +``` + +Completed = `vote IS NOT NULL`. Order most-recent-first. + +### Atomic claim (`AssignNextForAdmin`) + +The critical operation. Wrap in a transaction so concurrent admins don't claim the same item. + +```go +func (s *ApplicationReviewsStore) AssignNextForAdmin(ctx context.Context, adminID string, reviewsPerApp int) (*ApplicationReview, error) { + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { return nil, err } + defer tx.Rollback() + + // 1) Find next eligible target — locked, skipped if another tx holds it + findQuery := ` + SELECT id FROM applications + WHERE status = 'submitted' + AND reviews_assigned < $1 + AND user_id != $2 -- no self-review + AND NOT EXISTS ( + SELECT 1 FROM application_reviews ar + WHERE ar.application_id = applications.id AND ar.admin_id = $2 + ) + ORDER BY reviews_assigned ASC, submitted_at ASC + LIMIT 1 + FOR UPDATE SKIP LOCKED -- key clause: concurrent claims don't collide + ` + var applicationID string + if err := tx.QueryRowContext(ctx, findQuery, reviewsPerApp, adminID).Scan(&applicationID); err != nil { + if errors.Is(err, sql.ErrNoRows) { return nil, ErrNotFound } + return nil, err + } + + // 2) Insert assignment, idempotent on uniqueness collision + insertQuery := ` + INSERT INTO application_reviews (application_id, admin_id) + VALUES ($1, $2) + ON CONFLICT (application_id, admin_id) DO NOTHING + RETURNING id, application_id, admin_id, vote, notes, assigned_at, reviewed_at, created_at, updated_at + ` + var review ApplicationReview + if err := tx.QueryRowContext(ctx, insertQuery, applicationID, adminID).Scan(/* ... */); err != nil { + if errors.Is(err, sql.ErrNoRows) { return nil, ErrNotFound } + return nil, err + } + + if err := tx.Commit(); err != nil { return nil, err } + return &review, nil +} +``` + +Key clauses: +- `FOR UPDATE SKIP LOCKED` — locks the row for this transaction; if another transaction has it, skip and try the next. +- `ORDER BY reviews_assigned ASC, submitted_at ASC` — workload balancing across the queue. +- `user_id != $2` — admins don't review their own submissions. +- `NOT EXISTS (... AND admin_id = $2)` — same admin can't be assigned the same item twice. +- `ON CONFLICT ... DO NOTHING` — safety net; the unique constraint enforces it. + +### Submit result + +```go +func (s *ApplicationReviewsStore) SubmitVote(ctx context.Context, reviewID string, adminID string, vote ReviewVote, notes *string) (*ApplicationReview, error) { + // UPDATE application_reviews + // SET vote = $3, notes = $4, reviewed_at = NOW(), updated_at = NOW() + // WHERE id = $1 AND admin_id = $2 + // RETURNING ... +} +``` + +Scope by `(reviewID, adminID)` so admins can only submit on their own assignments — `sql.ErrNoRows` covers both "not found" and "wrong owner" with one error path. + +A trigger on the table updates the parent's `accept_votes`/`reject_votes`/`reviews_completed` columns automatically (see migration 000011) — handler doesn't need to maintain those. + +## Handlers + +### Pending / Completed lists + +```go +// @Summary Get pending reviews (Admin) +// @Tags admin/reviews +// @Router /admin/reviews/pending [get] +func (app *application) getPendingReviews(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + reviews, err := app.store.ApplicationReviews.GetPendingByAdminID(r.Context(), user.ID) + if err != nil { app.internalServerError(w, r, err); return } + app.jsonResponse(w, http.StatusOK, PendingReviewsListResponse{Reviews: reviews}) +} +``` + +`user.ID` from context — never trust an admin ID from query/body. + +### Claim next + +May call into a settings dependency (e.g., reviews-per-app) before claiming: + +```go +// @Router /admin/reviews/next [get] +func (app *application) getNextReview(w http.ResponseWriter, r *http.Request) { + user := getUserFromContext(r.Context()) + + reviewsPerApp, err := app.store.Settings.GetReviewsPerApplication(r.Context()) + if err != nil { app.internalServerError(w, r, err); return } + + review, err := app.store.ApplicationReviews.AssignNextForAdmin(r.Context(), user.ID, reviewsPerApp) + if err != nil { + switch { + case errors.Is(err, store.ErrNotFound): + app.notFoundResponse(w, r, errors.New("no applications need review")) + default: + app.internalServerError(w, r, err) + } + return + } + app.jsonResponse(w, http.StatusOK, ReviewResponse{Review: *review}) +} +``` + +`ErrNotFound` here means "queue is empty, try again later" — surface that as 404 with a specific message. + +### Submit + +Standard payload validation, then delegate to the scoped store method: + +```go +type SubmitVotePayload struct { + Vote store.ReviewVote `json:"vote" validate:"required,oneof=accept reject waitlist"` + Notes *string `json:"notes" validate:"omitempty,max=1000"` +} + +func (app *application) submitVote(w http.ResponseWriter, r *http.Request) { + reviewID := chi.URLParam(r, "reviewID") + if reviewID == "" { + app.badRequestResponse(w, r, errors.New("review ID is required")) + return + } + user := getUserFromContext(r.Context()) + + var req SubmitVotePayload + if err := readJSON(w, r, &req); err != nil { app.badRequestResponse(w, r, err); return } + if err := Validate.Struct(req); err != nil { app.badRequestResponse(w, r, err); return } + + review, err := app.store.ApplicationReviews.SubmitVote(r.Context(), reviewID, user.ID, req.Vote, req.Notes) + if err != nil { + if errors.Is(err, store.ErrNotFound) { app.notFoundResponse(w, r, err); return } + app.internalServerError(w, r, err) + return + } + app.jsonResponse(w, http.StatusOK, ReviewResponse{Review: *review}) +} +``` + +Use `oneof=` validation for the result enum so anything outside the allowed set is rejected at the boundary. + +## Routes + +Mount under the admin role group: + +```go +r.Group(func(r chi.Router) { + r.Use(app.RequireRoleMiddleware(store.RoleAdmin)) + + r.Route("/admin", func(r chi.Router) { + r.Route("/reviews", func(r chi.Router) { + r.Get("/pending", app.getPendingReviews) + r.Get("/next", app.getNextReview) + r.Put("/{reviewID}", app.submitVote) + r.Get("/completed", app.getCompletedReviews) + }) + }) +}) +``` + +## Tests + +Coverage checklist: + +| Endpoint | Cases | +|----------|-------| +| Pending list | returns items, returns empty list | +| Completed list | returns items | +| Claim next | success returns review, 404 when queue empty, 500 on store error | +| Submit | success (with and without notes), 400 invalid vote, 404 not found / not yours | + +For `submitVote`, mock the user-scoped variant — assert the admin ID is passed: + +```go +mockReviews.On("SubmitVote", "rev-1", admin.ID, store.ReviewVoteAccept, (*string)(nil)).Return(review, nil).Once() +``` + +See `cmd/api/reviews_test.go`. + +## What NOT to Do + +- Don't claim work without `FOR UPDATE SKIP LOCKED` — concurrent admins WILL collide. +- Don't drop the unique `(target_id, admin_id)` constraint — bulk re-assign and idempotent claim both rely on it. +- Don't trust an admin ID from URL/body — pull it from `getUserFromContext`. +- Don't update parent counter columns from the handler — the trigger handles that. +- Don't return all reviews to all admins — every retrieval method is scoped by `admin_id`. diff --git a/.claude/skills/frontend-design/SKILL.md b/.claude/skills/frontend-design/SKILL.md new file mode 100644 index 00000000..15e27ba9 --- /dev/null +++ b/.claude/skills/frontend-design/SKILL.md @@ -0,0 +1,290 @@ +--- +name: frontend-design +description: Create distinctive, production-grade hacker-facing UI for HARP following the Nike mobile app design language — bold black/white contrast, heavy athletic typography, and clean mobile-first layouts. Use this skill when building or redesigning any hacker-side page or component (application flow, dashboard, profile, status pages) and when visual/aesthetic quality matters. Complements the frontend skill (which handles architecture), this skill handles aesthetics. +--- + +# HARP Hacker-Side Design Guide + +All hacker-facing pages live under `src/pages/hacker/`. This skill governs **how they look and feel** — the frontend skill governs how they're wired up. Both skills apply together when building hacker pages. + +## Design Language: Nike Athletic Minimal + +The hacker-side of HARP follows the Nike mobile app's design language: stark black-and-white contrast, bold geometric typography, generous white space, and purposeful motion. Every screen should feel premium, intentional, and athletic — like putting on a product that performs. + +### Core Principles + +1. **Black and white as the primary palette** — color is for status only (success green, error red, warning orange). Never use decorative color. +2. **Typography carries weight** — headlines are bold and large; body is readable and restrained. Size contrast does the visual lifting. +3. **Generous spacing** — breathe. Nike screens use padding aggressively; content never fights the edges. +4. **Full-width, edge-to-edge sections** — cards and content blocks fill the container; avoid floating island layouts. +5. **Mobile-first always** — design at 390px first, then scale up. Every interactive element is thumb-reachable. + +--- + +## Typography + +Use **[Bebas Neue](https://fonts.google.com/specimen/Bebas+Neue)** (display/hero headings) paired with **[Inter](https://fonts.google.com/specimen/Inter)** (body and UI text). This mirrors Nike's pairing of a bold condensed display face with a clean, functional sans-serif. + +```css +/* In your Tailwind v4 CSS */ +@import url("https://fonts.googleapis.com/css2?family=Bebas+Neue&family=Inter:wght@400;500;600;700&display=swap"); + +@theme { + --font-display: "Bebas Neue", sans-serif; + --font-body: "Inter", sans-serif; +} +``` + +| Role | Font | Weight | Size (mobile) | Tracking | Example use | +| ----------------- | ---------- | ------ | ------------- | -------- | ------------------------------- | +| Hero / Page title | Bebas Neue | 400 | 48–64px | -0.03em | "YOUR APPLICATION", "WELCOME" | +| Section heading | Inter | 700 | 20–24px | -0.02em | "Personal Info", "Team Details" | +| Card label | Inter | 600 | 13–14px | 0.08em | Field labels, status badges | +| Body | Inter | 400 | 15–16px | 0 | Paragraph text, descriptions | +| Caption / helper | Inter | 400 | 12px | 0 | Help text, timestamps | +| CTA button | Inter | 700 | 14–15px | 0.05em | "SUBMIT APPLICATION", "SAVE" | + +**Rules:** + +- All-caps for Bebas Neue hero text always +- All-caps + wide tracking for button labels and small UI labels (Nike pattern) +- No more than 3 type sizes on a single screen + +--- + +## Color Palette + +```css +@theme { + /* Core — always available */ + --color-ink: #000000; /* primary text, filled buttons */ + --color-paper: #ffffff; /* page background, inverse text */ + --color-ink-muted: #6b6b6b; /* secondary text, placeholders */ + --color-surface: #f5f5f5; /* card backgrounds, input fills */ + --color-border: #e5e5e5; /* dividers, input borders */ + --color-border-strong: #111111; /* focused inputs, active states */ + + /* Status — use only for meaning, never decoration */ + --color-success: #1a8a1a; + --color-error: #d32f2f; + --color-warning: #e65100; + --color-info: #1565c0; +} +``` + +**Application status colors** (map to hacker application states): + +- `draft` → `--color-ink-muted` (gray) +- `submitted` → `--color-info` (blue) +- `under_review` → `--color-warning` (orange) +- `accepted` → `--color-success` (green) +- `rejected` / `waitlisted` → `--color-error` / `--color-ink-muted` + +--- + +## Component Patterns + +### Page Layout + +Every hacker page follows a consistent scaffold: + +```tsx +// Standard hacker page scaffold +export default function HackerPage() { + return ( +
+ {/* Full-bleed hero bar */} +
+

+ HackUTD +

+

+ Page Title +

+
+ + {/* Content — 24px horizontal padding, 32px section gap */} +
{/* sections */}
+
+ ); +} +``` + +### Buttons + +Two variants only — solid (primary actions) and outline (secondary). Both full-width on mobile. + +```tsx +// Primary CTA — solid black + + +// Secondary — outline + + +// Disabled state — always same structure, muted + +``` + +No rounded corners on primary buttons (Nike uses 0px or 2px radius max). Use `rounded-sm` at most. + +### Form Inputs + +Clean, borderless-bottom Nike-style inputs: + +```tsx +// Field group pattern +
+ + +
+``` + +For multi-line / complex form sections, use a card-style container: + +```tsx +
{/* fields */}
+``` + +### Cards + +Flat, no shadow, edge-to-edge or full-width within container: + +```tsx +// Info card — used for application status, summaries +
+

+ Application Status +

+

Under Review

+
+ +// Dark card — used for highlighted information +
+

+ Decision +

+

Accepted

+
+``` + +### Progress / Steps + +Multi-step application progress (Nike progress bar pattern): + +```tsx +// Step indicator — horizontal dash progress +
+ {steps.map((_, i) => ( +
+ ))} +
+``` + +### Status Badges + +```tsx +// Inline badge — all-caps, no border-radius + + Submitted + + +// Or outline variant + + Draft + +``` + +### Navigation / Tab Bar + +Bottom tab bar (mobile), top nav bar (desktop): + +```` + +--- + +## Motion + +Use CSS transitions and `transition-*` Tailwind classes. For page-level reveals, use Tailwind's `animate-` utilities or minimal CSS keyframes. No complex animation libraries needed — restraint is the Nike way. + +```css +/* Staggered reveal for list items — add to global CSS */ +@keyframes fade-up { + from { opacity: 0; transform: translateY(12px); } + to { opacity: 1; transform: translateY(0); } +} + +.animate-fade-up { + animation: fade-up 0.3s ease forwards; +} +```` + +```tsx +// Apply stagger via inline style +{ + items.map((item, i) => ( +
+ {/* content */} +
+ )); +} +``` + +**Rules:** + +- Duration: 200–350ms for UI interactions, 300–500ms for page reveals +- Easing: `ease-out` for reveals, `ease-in-out` for state changes +- `active:scale-[0.98]` on all tappable elements (Nike's press feedback) +- No bouncy/springy animations — everything is precise and athletic + +--- + +## Layout & Spacing + +| Token | Value | Use | +| ------- | ------- | -------------------------------- | +| page-x | 24px | Horizontal page padding | +| section | 32px | Vertical gap between sections | +| card | 20px | Internal card padding | +| stack | 12–16px | Vertical gap within a section | +| tight | 4–8px | Label-to-input, icon-to-text gap | + +Use `safe-area-inset` padding for iOS: `pt-safe`, `pb-safe` (configure in Tailwind v4 or use `env(safe-area-inset-*)` directly). + +--- + +## What to Avoid + +- No rounded-full pills on buttons (use `rounded-sm` at most) +- No box shadows (flat design only; borders for separation) +- No gradients or decorative color +- No hero images that are purely decorative — if an image exists, it's content +- No border-radius on cards (Nike cards are sharp-cornered) +- No competing font families — Bebas Neue + Inter only +- No purple, teal, or gradient color schemes +- No dense information layouts — if it feels cluttered, add space +- No small touch targets — minimum 44×44px for all interactive elements diff --git a/.claude/skills/frontend/SKILL.md b/.claude/skills/frontend/SKILL.md new file mode 100644 index 00000000..9997d88c --- /dev/null +++ b/.claude/skills/frontend/SKILL.md @@ -0,0 +1,158 @@ +--- +name: frontend +description: "HARP React frontend development guide (React 19 + TypeScript + Vite + Tailwind v4). Generates pages, components, Zustand stores, API modules, form validations, and route definitions following established conventions. Use this skill whenever building new frontend pages, adding UI components, creating Zustand stores, writing API client code, adding form validation, modifying routes, or working in client/web/. Also use when the user asks to add a new admin page, hacker-facing feature, or super admin view to the React frontend." +--- + +# HARP Frontend Development Guide + +All frontend code lives under `client/web/`. For adding a new page end-to-end, read `references/new-page.md`. + +## File Locations + +| Component | Location | Naming | +| ------------------------ | -------------------------------------------------- | --------------------------------- | +| Page entry | `src/pages///FeaturePage.tsx` | PascalCase, default export | +| Page API / store / types | `src/pages///` | `api.ts`, `store.ts`, `types.ts` | +| Page hooks / components | `src/pages///hooks/`, `components/` | `use[Name].ts`, PascalCase `.tsx` | +| Global types | `src/types.ts` | Shared across pages | +| Shared API client | `src/shared/lib/api.ts` | `getRequest`, `postRequest`, etc. | +| UI components | `src/components/ui/` | shadcn/ui — managed by CLI | +| Admin shared | `src/pages/admin/_shared/` | Barrel via `index.ts` | +| Auth guards | `src/shared/auth/` | Barrel export only | +| Routes | `src/routes.tsx` | `createBrowserRouter` | + +## Import Boundaries (ESLint enforced — breaks CI) + +| Layer | Can import from | +| ------------- | ------------------------------------------------ | +| `shared/` | `shared/` only | +| `components/` | `shared/`, `components/` | +| `layouts/` | `shared/`, `components/`, `pages/admin/_shared/` | +| `pages/` | anything | + +Blocked: `@/lib/*`, `@/hooks/*`, `@/stores/*`, `@/features/*` — use `@/shared/*`. No deep imports into `@/shared/auth/guards/*` — use the `@/shared/auth` barrel. + +## API Layer + +All HTTP through `shared/lib/api.ts` — never call `fetch()` directly. Returns `ApiResponse` with `{ status, data?, error? }`. Page-level `api.ts` wraps the shared client: + +```typescript +import { getRequest } from "@/shared/lib/api"; +import type { ApiResponse } from "@/types"; +import type { FeatureListResult, FetchParams } from "./types"; + +export async function fetchItems( + params?: FetchParams, + signal?: AbortSignal, +): Promise> { + const qp = new URLSearchParams(); + if (params?.status) qp.set("status", params.status); + const qs = qp.toString(); + return getRequest( + `/admin/feature${qs ? `?${qs}` : ""}`, + "feature items", + signal, + ); +} +``` + +Errors: `errorAlert(res)` from `@/shared/lib/api` shows a toast. Success: `toast.success("message")` via sonner. + +## State Management (Zustand) + +Global stores in `shared/stores/` (only `useUserStore`). Page-level stores co-located in page dirs. Use **selectors**: `useStore((s) => s.field)`. + +```typescript +// store.ts +import { create } from "zustand"; +import { fetchItems as apiFetch } from "./api"; + +interface FeatureState { + items: Item[]; + loading: boolean; + fetchItems: (params?: FetchParams, signal?: AbortSignal) => Promise; +} + +export const useFeatureStore = create((set, get) => ({ + items: [], + loading: false, + fetchItems: async (params, signal) => { + set({ loading: true }); + const res = await apiFetch(params, signal); + if (signal?.aborted) return; + if (res.status === 200 && res.data) { + set({ items: res.data.items, loading: false }); + } else { + set({ items: [], loading: false }); + } + }, +})); +``` + +For reusable stores, use a factory in `createStore.ts` and a configured instance in `store.ts`. + +## Components + +- `React.memo()` with named functions for list/table components: `export const Table = memo(function Table({...}) {...})` +- Props: `[Component]Props` suffix +- Classes: `cn()` from `@/shared/lib/utils` for conditional merging +- All UI from `@/components/ui/` (shadcn/ui) — no competing libraries + +## Forms (React Hook Form + Zod) + +Schemas in page dir (`validations.ts`). Multi-step: `STEP_FIELDS` mapping + `form.trigger(STEP_FIELDS[step])`. Form components use `useFormContext()` + shadcn `FormField`/`FormItem`/`FormControl`/`FormMessage`. + +## Data Fetching + +Always use `AbortController` cleanup: + +```typescript +useEffect(() => { + const controller = new AbortController(); + fetchItems(undefined, controller.signal); + return () => controller.abort(); +}, [fetchItems]); +``` + +Debounced search: `useState` + `setTimeout(500ms)` + `useRef` to skip first render. + +## Routing + +Lazy-load with `React.lazy()` + `}>`. Guards: `` (hacker), `` (admin — applied at `/admin` parent), `` (super admin, path under `sa/`). + +## Naming Conventions + +| Category | Convention | Example | +| --------------- | ----------------------- | ------------------------- | +| Component files | PascalCase | `ApplicationsTable.tsx` | +| Hooks | camelCase, `use` prefix | `useApplicationDetail.ts` | +| Directories | kebab-case | `all-applicants/` | +| Constants | UPPER_SNAKE_CASE | `STEP_FIELDS` | +| Booleans | `is`/`has`/`can` prefix | `isLoading`, `hasMore` | +| API functions | `fetch[Resource]` | `fetchApplications` | +| Handlers | `handle[Action]` | `handleStatusFilter` | +| Stores | `use[Feature]Store` | `useApplicationsStore` | + +## Types + +- Global in `src/types.ts`, page-level in `types.ts` within page dir +- Keep backend snake_case field names as-is +- Union types for enums: `type Status = "draft" | "submitted"` +- Store interfaces: `[Feature]State` suffix +- Import types with `import type` + +## Styling + +Tailwind v4 with semantic tokens (`bg-primary`, `text-muted-foreground`) — no raw color values. `cn()` for conditional classes. Mobile-first responsive (`md:`/`lg:` breakpoints). + +## What NOT to Do + +- Don't call `fetch()` directly — use `@/shared/lib/api` +- Don't add competing UI libraries — shadcn/ui only +- Don't import across page boundaries (use `_shared/` for shared admin components) +- Don't deep-import `@/shared/auth/guards/*` — use barrel +- Don't use `@/lib/*`, `@/hooks/*`, `@/stores/*` — use `@/shared/*` +- Don't convert backend snake_case to camelCase +- Don't skip `AbortController` cleanup in `useEffect` +- Don't skip `` on lazy-loaded routes +- Don't add `Co-Authored-By` to commits diff --git a/.claude/skills/frontend/references/new-page.md b/.claude/skills/frontend/references/new-page.md new file mode 100644 index 00000000..ab359b29 --- /dev/null +++ b/.claude/skills/frontend/references/new-page.md @@ -0,0 +1,359 @@ +# Adding a New Page End-to-End + +This guide walks through creating a complete new page feature in the HARP frontend. Use the admin "all-applicants" page as the canonical reference implementation. + +## Step 1: Create the Page Directory + +Choose the right role directory based on who accesses this page: + +``` +src/pages/hacker// # Hacker-facing pages +src/pages/admin// # Admin pages (admin + super_admin) +src/pages/superadmin// # Super admin only pages +``` + +Directory names use **kebab-case**. Create this structure: + +``` +src/pages/// +├── FeaturePage.tsx # Page entry component (PascalCase, default export) +├── api.ts # API endpoint wrappers +├── types.ts # Feature-specific TypeScript types +├── store.ts # Zustand store instance +├── components/ # Feature-specific components +└── hooks/ # Feature-specific hooks (optional) +``` + +Only create files you actually need — a simple page might only need `FeaturePage.tsx` and `api.ts`. + +## Step 2: Define Types + +Create `types.ts` with the feature's data shapes. Match the backend's JSON field names exactly (snake_case): + +```typescript +// types.ts +export interface FeatureItem { + id: string; + name: string; + status: FeatureStatus; + created_at: string; + updated_at: string; +} + +export type FeatureStatus = "active" | "inactive" | "pending"; + +// Paginated response (matches backend envelope) +export interface FeatureListResult { + items: FeatureItem[]; + next_cursor: string | null; + has_more: boolean; +} + +// Query parameters for the list endpoint +export interface FetchParams { + cursor?: string; + status?: FeatureStatus | null; + search?: string; +} +``` + +If a type is shared across multiple pages (like `Application`, `User`, `ApiResponse`), add it to `src/types.ts` instead. + +## Step 3: Create the API Module + +Create `api.ts` that wraps the shared HTTP client for this feature's endpoints: + +```typescript +// api.ts +import { getRequest, postRequest } from "@/shared/lib/api"; +import type { ApiResponse } from "@/types"; + +import type { FeatureItem, FeatureListResult, FetchParams } from "./types"; + +export async function fetchFeatureItems( + params?: FetchParams, + signal?: AbortSignal, +): Promise> { + const queryParams = new URLSearchParams(); + if (params?.status) queryParams.set("status", params.status); + if (params?.cursor) queryParams.set("cursor", params.cursor); + if (params?.search) queryParams.set("search", params.search); + + const qs = queryParams.toString(); + return getRequest( + `/admin/feature${qs ? `?${qs}` : ""}`, + "feature items", + signal, + ); +} + +export async function fetchFeatureById( + id: string, + signal?: AbortSignal, +): Promise> { + return getRequest( + `/admin/feature/${id}`, + "feature item", + signal, + ); +} + +export async function createFeatureItem( + body: Partial, +): Promise> { + return postRequest("/admin/feature", body, "feature item"); +} +``` + +Key points: +- Always accept `signal?: AbortSignal` on GET requests for cleanup +- Build query strings with `URLSearchParams` +- Return `Promise>` with the specific response type +- The second arg to `getRequest`/`postRequest` is the error context string for toast messages + +## Step 4: Create the Store + +For a simple page, create `store.ts` directly: + +```typescript +// store.ts +import { create } from "zustand"; + +import { errorAlert } from "@/shared/lib/api"; + +import { fetchFeatureItems as apiFetch } from "./api"; +import type { FeatureItem, FeatureStatus, FetchParams } from "./types"; + +interface FeatureState { + items: FeatureItem[]; + loading: boolean; + currentStatus: FeatureStatus | null; + fetchItems: (params?: FetchParams, signal?: AbortSignal) => Promise; +} + +export const useFeatureStore = create((set, get) => ({ + items: [], + loading: false, + currentStatus: null, + + fetchItems: async (params, signal) => { + set({ loading: true }); + + const status = params?.status !== undefined ? params.status : get().currentStatus; + + const res = await apiFetch({ ...params, status }, signal); + + if (signal?.aborted) return; + + if (res.status === 200 && res.data) { + set({ + items: res.data.items, + loading: false, + currentStatus: status, + }); + } else { + errorAlert(res); + set({ items: [], loading: false }); + } + }, +})); +``` + +If the store needs to be reusable with different configurations (e.g., same list page used in admin and super admin with different defaults), use the factory pattern instead — put the factory in `createStore.ts` and the configured instance in `store.ts`. + +## Step 5: Build the Page Component + +Create `FeaturePage.tsx` with a default export: + +```typescript +// FeaturePage.tsx +import { useEffect, useRef, useState } from "react"; + +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Input } from "@/components/ui/input"; + +import { FeatureTable } from "./components/FeatureTable"; +import { useFeatureStore } from "./store"; + +export default function FeaturePage() { + // Store selectors + const items = useFeatureStore((s) => s.items); + const loading = useFeatureStore((s) => s.loading); + const fetchItems = useFeatureStore((s) => s.fetchItems); + const currentSearch = useFeatureStore((s) => s.currentSearch); + + // Local UI state + const [searchInput, setSearchInput] = useState(currentSearch ?? ""); + const isFirstRender = useRef(true); + + // Fetch on mount + useEffect(() => { + const controller = new AbortController(); + fetchItems(undefined, controller.signal); + return () => controller.abort(); + }, [fetchItems]); + + // Debounced search + useEffect(() => { + if (isFirstRender.current) { + isFirstRender.current = false; + return; + } + const timer = setTimeout(() => { + fetchItems({ search: searchInput.length >= 2 ? searchInput : "" }); + }, 500); + return () => clearTimeout(timer); + }, [searchInput, fetchItems]); + + return ( +
+
+

Feature Name

+ setSearchInput(e.target.value)} + className="max-w-sm" + /> +
+ + + + + +
+ ); +} +``` + +## Step 6: Create Components + +Put feature-specific components in `components/`. Use `React.memo()` for table/list components: + +```typescript +// components/FeatureTable.tsx +import { memo } from "react"; + +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "@/components/ui/table"; +import { LoadingSpinner } from "@/components"; + +import type { FeatureItem } from "../types"; + +interface FeatureTableProps { + items: FeatureItem[]; + loading: boolean; + onSelect?: (id: string) => void; +} + +export const FeatureTable = memo(function FeatureTable({ + items, + loading, + onSelect, +}: FeatureTableProps) { + if (loading) return ; + + if (items.length === 0) { + return

No items found.

; + } + + return ( +
+ + + Name + Status + Created + + + + {items.map((item) => ( + onSelect?.(item.id)} + > + {item.name} + {item.status} + {new Date(item.created_at).toLocaleDateString()} + + ))} + +
+ ); +}); +``` + +## Step 7: Add the Route + +In `src/routes.tsx`: + +1. Add the lazy import at the top: + +```typescript +const FeaturePage = lazy( + () => import("@/pages/admin/feature-name/FeaturePage"), +); +``` + +2. Add the route to the appropriate children array: + +```typescript +// For admin pages — inside the /admin children array: +{ + path: "feature-name", + element: ( + }> + + + ), +} + +// For super admin pages — wrap with RequireSuperAdmin: +{ + path: "sa/feature-name", + element: ( + + }> + + + + ), +} + +// For hacker pages — add as a top-level route with RequireAuth: +{ + path: "/app/feature-name", + element: ( + + }> + + + + ), +} +``` + +## Step 8: Add Sidebar Navigation (Admin Pages) + +For admin pages, add a link in the sidebar. The sidebar is defined in `src/pages/admin/_shared/`. Add the new page's path and icon to the nav items array. + +## Checklist + +Before considering the page complete: + +- [ ] Types match backend response shape (snake_case fields) +- [ ] API module uses shared client, accepts `AbortSignal` +- [ ] Store checks `signal?.aborted` before calling `set()` +- [ ] Page component cleans up with `AbortController` in `useEffect` +- [ ] Route is lazy-loaded with `}>` +- [ ] Route has the correct auth guard (`RequireAuth`, `RequireAdmin`, or `RequireSuperAdmin`) +- [ ] Imports follow boundary rules (no cross-page imports, no deep `@/shared/auth/*` imports) +- [ ] `npm run lint` passes +- [ ] `npm run build` passes (TypeScript + Vite) diff --git a/.claude/skills/skill-creator/SKILL.md b/.claude/skills/skill-creator/SKILL.md new file mode 100644 index 00000000..e23e3e81 --- /dev/null +++ b/.claude/skills/skill-creator/SKILL.md @@ -0,0 +1,479 @@ +--- +name: skill-creator +description: Create new skills, modify and improve existing skills, and measure skill performance. Use when users want to create a skill from scratch, update or optimize an existing skill, run evals to test a skill, benchmark skill performance with variance analysis, or optimize a skill's description for better triggering accuracy. +--- + +# Skill Creator + +A skill for creating new skills and iteratively improving them. + +At a high level, the process of creating a skill goes like this: + +- Decide what you want the skill to do and roughly how it should do it +- Write a draft of the skill +- Create a few test prompts and run claude-with-access-to-the-skill on them +- Help the user evaluate the results both qualitatively and quantitatively + - While the runs happen in the background, draft some quantitative evals if there aren't any (if there are some, you can either use as is or modify if you feel something needs to change about them). Then explain them to the user (or if they already existed, explain the ones that already exist) + - Use the `eval-viewer/generate_review.py` script to show the user the results for them to look at, and also let them look at the quantitative metrics +- Rewrite the skill based on feedback from the user's evaluation of the results (and also if there are any glaring flaws that become apparent from the quantitative benchmarks) +- Repeat until you're satisfied +- Expand the test set and try again at larger scale + +Your job when using this skill is to figure out where the user is in this process and then jump in and help them progress through these stages. So for instance, maybe they're like "I want to make a skill for X". You can help narrow down what they mean, write a draft, write the test cases, figure out how they want to evaluate, run all the prompts, and repeat. + +On the other hand, maybe they already have a draft of the skill. In this case you can go straight to the eval/iterate part of the loop. + +Of course, you should always be flexible and if the user is like "I don't need to run a bunch of evaluations, just vibe with me", you can do that instead. + +Then after the skill is done (but again, the order is flexible), you can also run the skill description improver, which we have a whole separate script for, to optimize the triggering of the skill. + +Cool? Cool. + +## Communicating with the user + +The skill creator is liable to be used by people across a wide range of familiarity with coding jargon. If you haven't heard (and how could you, it's only very recently that it started), there's a trend now where the power of Claude is inspiring plumbers to open up their terminals, parents and grandparents to google "how to install npm". On the other hand, the bulk of users are probably fairly computer-literate. + +So please pay attention to context cues to understand how to phrase your communication! In the default case, just to give you some idea: + +- "evaluation" and "benchmark" are borderline, but OK +- for "JSON" and "assertion" you want to see serious cues from the user that they know what those things are before using them without explaining them + +It's OK to briefly explain terms if you're in doubt, and feel free to clarify terms with a short definition if you're unsure if the user will get it. + +--- + +## Creating a skill + +### Capture Intent + +Start by understanding the user's intent. The current conversation might already contain a workflow the user wants to capture (e.g., they say "turn this into a skill"). If so, extract answers from the conversation history first — the tools used, the sequence of steps, corrections the user made, input/output formats observed. The user may need to fill the gaps, and should confirm before proceeding to the next step. + +1. What should this skill enable Claude to do? +2. When should this skill trigger? (what user phrases/contexts) +3. What's the expected output format? +4. Should we set up test cases to verify the skill works? Skills with objectively verifiable outputs (file transforms, data extraction, code generation, fixed workflow steps) benefit from test cases. Skills with subjective outputs (writing style, art) often don't need them. Suggest the appropriate default based on the skill type, but let the user decide. + +### Interview and Research + +Proactively ask questions about edge cases, input/output formats, example files, success criteria, and dependencies. Wait to write test prompts until you've got this part ironed out. + +Check available MCPs - if useful for research (searching docs, finding similar skills, looking up best practices), research in parallel via subagents if available, otherwise inline. Come prepared with context to reduce burden on the user. + +### Write the SKILL.md + +Based on the user interview, fill in these components: + +- **name**: Skill identifier +- **description**: When to trigger, what it does. This is the primary triggering mechanism - include both what the skill does AND specific contexts for when to use it. All "when to use" info goes here, not in the body. Note: currently Claude has a tendency to "undertrigger" skills -- to not use them when they'd be useful. To combat this, please make the skill descriptions a little bit "pushy". So for instance, instead of "How to build a simple fast dashboard to display internal Anthropic data.", you might write "How to build a simple fast dashboard to display internal Anthropic data. Make sure to use this skill whenever the user mentions dashboards, data visualization, internal metrics, or wants to display any kind of company data, even if they don't explicitly ask for a 'dashboard.'" +- **compatibility**: Required tools, dependencies (optional, rarely needed) +- **the rest of the skill :)** + +### Skill Writing Guide + +#### Anatomy of a Skill + +``` +skill-name/ +├── SKILL.md (required) +│ ├── YAML frontmatter (name, description required) +│ └── Markdown instructions +└── Bundled Resources (optional) + ├── scripts/ - Executable code for deterministic/repetitive tasks + ├── references/ - Docs loaded into context as needed + └── assets/ - Files used in output (templates, icons, fonts) +``` + +#### Progressive Disclosure + +Skills use a three-level loading system: +1. **Metadata** (name + description) - Always in context (~100 words) +2. **SKILL.md body** - In context whenever skill triggers (<500 lines ideal) +3. **Bundled resources** - As needed (unlimited, scripts can execute without loading) + +These word counts are approximate and you can feel free to go longer if needed. + +**Key patterns:** +- Keep SKILL.md under 500 lines; if you're approaching this limit, add an additional layer of hierarchy along with clear pointers about where the model using the skill should go next to follow up. +- Reference files clearly from SKILL.md with guidance on when to read them +- For large reference files (>300 lines), include a table of contents + +**Domain organization**: When a skill supports multiple domains/frameworks, organize by variant: +``` +cloud-deploy/ +├── SKILL.md (workflow + selection) +└── references/ + ├── aws.md + ├── gcp.md + └── azure.md +``` +Claude reads only the relevant reference file. + +#### Principle of Lack of Surprise + +This goes without saying, but skills must not contain malware, exploit code, or any content that could compromise system security. A skill's contents should not surprise the user in their intent if described. Don't go along with requests to create misleading skills or skills designed to facilitate unauthorized access, data exfiltration, or other malicious activities. Things like a "roleplay as an XYZ" are OK though. + +#### Writing Patterns + +Prefer using the imperative form in instructions. + +**Defining output formats** - You can do it like this: +```markdown +## Report structure +ALWAYS use this exact template: +# [Title] +## Executive summary +## Key findings +## Recommendations +``` + +**Examples pattern** - It's useful to include examples. You can format them like this (but if "Input" and "Output" are in the examples you might want to deviate a little): +```markdown +## Commit message format +**Example 1:** +Input: Added user authentication with JWT tokens +Output: feat(auth): implement JWT-based authentication +``` + +### Writing Style + +Try to explain to the model why things are important in lieu of heavy-handed musty MUSTs. Use theory of mind and try to make the skill general and not super-narrow to specific examples. Start by writing a draft and then look at it with fresh eyes and improve it. + +### Test Cases + +After writing the skill draft, come up with 2-3 realistic test prompts — the kind of thing a real user would actually say. Share them with the user: [you don't have to use this exact language] "Here are a few test cases I'd like to try. Do these look right, or do you want to add more?" Then run them. + +Save test cases to `evals/evals.json`. Don't write assertions yet — just the prompts. You'll draft assertions in the next step while the runs are in progress. + +```json +{ + "skill_name": "example-skill", + "evals": [ + { + "id": 1, + "prompt": "User's task prompt", + "expected_output": "Description of expected result", + "files": [] + } + ] +} +``` + +See `references/schemas.md` for the full schema (including the `assertions` field, which you'll add later). + +## Running and evaluating test cases + +This section is one continuous sequence — don't stop partway through. Do NOT use `/skill-test` or any other testing skill. + +Put results in `-workspace/` as a sibling to the skill directory. Within the workspace, organize results by iteration (`iteration-1/`, `iteration-2/`, etc.) and within that, each test case gets a directory (`eval-0/`, `eval-1/`, etc.). Don't create all of this upfront — just create directories as you go. + +### Step 1: Spawn all runs (with-skill AND baseline) in the same turn + +For each test case, spawn two subagents in the same turn — one with the skill, one without. This is important: don't spawn the with-skill runs first and then come back for baselines later. Launch everything at once so it all finishes around the same time. + +**With-skill run:** + +``` +Execute this task: +- Skill path: +- Task: +- Input files: +- Save outputs to: /iteration-/eval-/with_skill/outputs/ +- Outputs to save: +``` + +**Baseline run** (same prompt, but the baseline depends on context): +- **Creating a new skill**: no skill at all. Same prompt, no skill path, save to `without_skill/outputs/`. +- **Improving an existing skill**: the old version. Before editing, snapshot the skill (`cp -r /skill-snapshot/`), then point the baseline subagent at the snapshot. Save to `old_skill/outputs/`. + +Write an `eval_metadata.json` for each test case (assertions can be empty for now). Give each eval a descriptive name based on what it's testing — not just "eval-0". Use this name for the directory too. If this iteration uses new or modified eval prompts, create these files for each new eval directory — don't assume they carry over from previous iterations. + +```json +{ + "eval_id": 0, + "eval_name": "descriptive-name-here", + "prompt": "The user's task prompt", + "assertions": [] +} +``` + +### Step 2: While runs are in progress, draft assertions + +Don't just wait for the runs to finish — you can use this time productively. Draft quantitative assertions for each test case and explain them to the user. If assertions already exist in `evals/evals.json`, review them and explain what they check. + +Good assertions are objectively verifiable and have descriptive names — they should read clearly in the benchmark viewer so someone glancing at the results immediately understands what each one checks. Subjective skills (writing style, design quality) are better evaluated qualitatively — don't force assertions onto things that need human judgment. + +Update the `eval_metadata.json` files and `evals/evals.json` with the assertions once drafted. Also explain to the user what they'll see in the viewer — both the qualitative outputs and the quantitative benchmark. + +### Step 3: As runs complete, capture timing data + +When each subagent task completes, you receive a notification containing `total_tokens` and `duration_ms`. Save this data immediately to `timing.json` in the run directory: + +```json +{ + "total_tokens": 84852, + "duration_ms": 23332, + "total_duration_seconds": 23.3 +} +``` + +This is the only opportunity to capture this data — it comes through the task notification and isn't persisted elsewhere. Process each notification as it arrives rather than trying to batch them. + +### Step 4: Grade, aggregate, and launch the viewer + +Once all runs are done: + +1. **Grade each run** — spawn a grader subagent (or grade inline) that reads `agents/grader.md` and evaluates each assertion against the outputs. Save results to `grading.json` in each run directory. The grading.json expectations array must use the fields `text`, `passed`, and `evidence` (not `name`/`met`/`details` or other variants) — the viewer depends on these exact field names. For assertions that can be checked programmatically, write and run a script rather than eyeballing it — scripts are faster, more reliable, and can be reused across iterations. + +2. **Aggregate into benchmark** — run the aggregation script from the skill-creator directory: + ```bash + python -m scripts.aggregate_benchmark /iteration-N --skill-name + ``` + This produces `benchmark.json` and `benchmark.md` with pass_rate, time, and tokens for each configuration, with mean ± stddev and the delta. If generating benchmark.json manually, see `references/schemas.md` for the exact schema the viewer expects. +Put each with_skill version before its baseline counterpart. + +3. **Do an analyst pass** — read the benchmark data and surface patterns the aggregate stats might hide. See `agents/analyzer.md` (the "Analyzing Benchmark Results" section) for what to look for — things like assertions that always pass regardless of skill (non-discriminating), high-variance evals (possibly flaky), and time/token tradeoffs. + +4. **Launch the viewer** with both qualitative outputs and quantitative data: + ```bash + nohup python /eval-viewer/generate_review.py \ + /iteration-N \ + --skill-name "my-skill" \ + --benchmark /iteration-N/benchmark.json \ + > /dev/null 2>&1 & + VIEWER_PID=$! + ``` + For iteration 2+, also pass `--previous-workspace /iteration-`. + + **Cowork / headless environments:** If `webbrowser.open()` is not available or the environment has no display, use `--static ` to write a standalone HTML file instead of starting a server. Feedback will be downloaded as a `feedback.json` file when the user clicks "Submit All Reviews". After download, copy `feedback.json` into the workspace directory for the next iteration to pick up. + +Note: please use generate_review.py to create the viewer; there's no need to write custom HTML. + +5. **Tell the user** something like: "I've opened the results in your browser. There are two tabs — 'Outputs' lets you click through each test case and leave feedback, 'Benchmark' shows the quantitative comparison. When you're done, come back here and let me know." + +### What the user sees in the viewer + +The "Outputs" tab shows one test case at a time: +- **Prompt**: the task that was given +- **Output**: the files the skill produced, rendered inline where possible +- **Previous Output** (iteration 2+): collapsed section showing last iteration's output +- **Formal Grades** (if grading was run): collapsed section showing assertion pass/fail +- **Feedback**: a textbox that auto-saves as they type +- **Previous Feedback** (iteration 2+): their comments from last time, shown below the textbox + +The "Benchmark" tab shows the stats summary: pass rates, timing, and token usage for each configuration, with per-eval breakdowns and analyst observations. + +Navigation is via prev/next buttons or arrow keys. When done, they click "Submit All Reviews" which saves all feedback to `feedback.json`. + +### Step 5: Read the feedback + +When the user tells you they're done, read `feedback.json`: + +```json +{ + "reviews": [ + {"run_id": "eval-0-with_skill", "feedback": "the chart is missing axis labels", "timestamp": "..."}, + {"run_id": "eval-1-with_skill", "feedback": "", "timestamp": "..."}, + {"run_id": "eval-2-with_skill", "feedback": "perfect, love this", "timestamp": "..."} + ], + "status": "complete" +} +``` + +Empty feedback means the user thought it was fine. Focus your improvements on the test cases where the user had specific complaints. + +Kill the viewer server when you're done with it: + +```bash +kill $VIEWER_PID 2>/dev/null +``` + +--- + +## Improving the skill + +This is the heart of the loop. You've run the test cases, the user has reviewed the results, and now you need to make the skill better based on their feedback. + +### How to think about improvements + +1. **Generalize from the feedback.** The big picture thing that's happening here is that we're trying to create skills that can be used a million times (maybe literally, maybe even more who knows) across many different prompts. Here you and the user are iterating on only a few examples over and over again because it helps move faster. The user knows these examples in and out and it's quick for them to assess new outputs. But if the skill you and the user are codeveloping works only for those examples, it's useless. Rather than put in fiddly overfitty changes, or oppressively constrictive MUSTs, if there's some stubborn issue, you might try branching out and using different metaphors, or recommending different patterns of working. It's relatively cheap to try and maybe you'll land on something great. + +2. **Keep the prompt lean.** Remove things that aren't pulling their weight. Make sure to read the transcripts, not just the final outputs — if it looks like the skill is making the model waste a bunch of time doing things that are unproductive, you can try getting rid of the parts of the skill that are making it do that and seeing what happens. + +3. **Explain the why.** Try hard to explain the **why** behind everything you're asking the model to do. Today's LLMs are *smart*. They have good theory of mind and when given a good harness can go beyond rote instructions and really make things happen. Even if the feedback from the user is terse or frustrated, try to actually understand the task and why the user is writing what they wrote, and what they actually wrote, and then transmit this understanding into the instructions. If you find yourself writing ALWAYS or NEVER in all caps, or using super rigid structures, that's a yellow flag — if possible, reframe and explain the reasoning so that the model understands why the thing you're asking for is important. That's a more humane, powerful, and effective approach. + +4. **Look for repeated work across test cases.** Read the transcripts from the test runs and notice if the subagents all independently wrote similar helper scripts or took the same multi-step approach to something. If all 3 test cases resulted in the subagent writing a `create_docx.py` or a `build_chart.py`, that's a strong signal the skill should bundle that script. Write it once, put it in `scripts/`, and tell the skill to use it. This saves every future invocation from reinventing the wheel. + +This task is pretty important (we are trying to create billions a year in economic value here!) and your thinking time is not the blocker; take your time and really mull things over. I'd suggest writing a draft revision and then looking at it anew and making improvements. Really do your best to get into the head of the user and understand what they want and need. + +### The iteration loop + +After improving the skill: + +1. Apply your improvements to the skill +2. Rerun all test cases into a new `iteration-/` directory, including baseline runs. If you're creating a new skill, the baseline is always `without_skill` (no skill) — that stays the same across iterations. If you're improving an existing skill, use your judgment on what makes sense as the baseline: the original version the user came in with, or the previous iteration. +3. Launch the reviewer with `--previous-workspace` pointing at the previous iteration +4. Wait for the user to review and tell you they're done +5. Read the new feedback, improve again, repeat + +Keep going until: +- The user says they're happy +- The feedback is all empty (everything looks good) +- You're not making meaningful progress + +--- + +## Advanced: Blind comparison + +For situations where you want a more rigorous comparison between two versions of a skill (e.g., the user asks "is the new version actually better?"), there's a blind comparison system. Read `agents/comparator.md` and `agents/analyzer.md` for the details. The basic idea is: give two outputs to an independent agent without telling it which is which, and let it judge quality. Then analyze why the winner won. + +This is optional, requires subagents, and most users won't need it. The human review loop is usually sufficient. + +--- + +## Description Optimization + +The description field in SKILL.md frontmatter is the primary mechanism that determines whether Claude invokes a skill. After creating or improving a skill, offer to optimize the description for better triggering accuracy. + +### Step 1: Generate trigger eval queries + +Create 20 eval queries — a mix of should-trigger and should-not-trigger. Save as JSON: + +```json +[ + {"query": "the user prompt", "should_trigger": true}, + {"query": "another prompt", "should_trigger": false} +] +``` + +The queries must be realistic and something a Claude Code or Claude.ai user would actually type. Not abstract requests, but requests that are concrete and specific and have a good amount of detail. For instance, file paths, personal context about the user's job or situation, column names and values, company names, URLs. A little bit of backstory. Some might be in lowercase or contain abbreviations or typos or casual speech. Use a mix of different lengths, and focus on edge cases rather than making them clear-cut (the user will get a chance to sign off on them). + +Bad: `"Format this data"`, `"Extract text from PDF"`, `"Create a chart"` + +Good: `"ok so my boss just sent me this xlsx file (its in my downloads, called something like 'Q4 sales final FINAL v2.xlsx') and she wants me to add a column that shows the profit margin as a percentage. The revenue is in column C and costs are in column D i think"` + +For the **should-trigger** queries (8-10), think about coverage. You want different phrasings of the same intent — some formal, some casual. Include cases where the user doesn't explicitly name the skill or file type but clearly needs it. Throw in some uncommon use cases and cases where this skill competes with another but should win. + +For the **should-not-trigger** queries (8-10), the most valuable ones are the near-misses — queries that share keywords or concepts with the skill but actually need something different. Think adjacent domains, ambiguous phrasing where a naive keyword match would trigger but shouldn't, and cases where the query touches on something the skill does but in a context where another tool is more appropriate. + +The key thing to avoid: don't make should-not-trigger queries obviously irrelevant. "Write a fibonacci function" as a negative test for a PDF skill is too easy — it doesn't test anything. The negative cases should be genuinely tricky. + +### Step 2: Review with user + +Present the eval set to the user for review using the HTML template: + +1. Read the template from `assets/eval_review.html` +2. Replace the placeholders: + - `__EVAL_DATA_PLACEHOLDER__` → the JSON array of eval items (no quotes around it — it's a JS variable assignment) + - `__SKILL_NAME_PLACEHOLDER__` → the skill's name + - `__SKILL_DESCRIPTION_PLACEHOLDER__` → the skill's current description +3. Write to a temp file (e.g., `/tmp/eval_review_.html`) and open it: `open /tmp/eval_review_.html` +4. The user can edit queries, toggle should-trigger, add/remove entries, then click "Export Eval Set" +5. The file downloads to `~/Downloads/eval_set.json` — check the Downloads folder for the most recent version in case there are multiple (e.g., `eval_set (1).json`) + +This step matters — bad eval queries lead to bad descriptions. + +### Step 3: Run the optimization loop + +Tell the user: "This will take some time — I'll run the optimization loop in the background and check on it periodically." + +Save the eval set to the workspace, then run in the background: + +```bash +python -m scripts.run_loop \ + --eval-set \ + --skill-path \ + --model \ + --max-iterations 5 \ + --verbose +``` + +Use the model ID from your system prompt (the one powering the current session) so the triggering test matches what the user actually experiences. + +While it runs, periodically tail the output to give the user updates on which iteration it's on and what the scores look like. + +This handles the full optimization loop automatically. It splits the eval set into 60% train and 40% held-out test, evaluates the current description (running each query 3 times to get a reliable trigger rate), then calls Claude with extended thinking to propose improvements based on what failed. It re-evaluates each new description on both train and test, iterating up to 5 times. When it's done, it opens an HTML report in the browser showing the results per iteration and returns JSON with `best_description` — selected by test score rather than train score to avoid overfitting. + +### How skill triggering works + +Understanding the triggering mechanism helps design better eval queries. Skills appear in Claude's `available_skills` list with their name + description, and Claude decides whether to consult a skill based on that description. The important thing to know is that Claude only consults skills for tasks it can't easily handle on its own — simple, one-step queries like "read this PDF" may not trigger a skill even if the description matches perfectly, because Claude can handle them directly with basic tools. Complex, multi-step, or specialized queries reliably trigger skills when the description matches. + +This means your eval queries should be substantive enough that Claude would actually benefit from consulting a skill. Simple queries like "read file X" are poor test cases — they won't trigger skills regardless of description quality. + +### Step 4: Apply the result + +Take `best_description` from the JSON output and update the skill's SKILL.md frontmatter. Show the user before/after and report the scores. + +--- + +### Package and Present (only if `present_files` tool is available) + +Check whether you have access to the `present_files` tool. If you don't, skip this step. If you do, package the skill and present the .skill file to the user: + +```bash +python -m scripts.package_skill +``` + +After packaging, direct the user to the resulting `.skill` file path so they can install it. + +--- + +## Claude.ai-specific instructions + +In Claude.ai, the core workflow is the same (draft → test → review → improve → repeat), but because Claude.ai doesn't have subagents, some mechanics change. Here's what to adapt: + +**Running test cases**: No subagents means no parallel execution. For each test case, read the skill's SKILL.md, then follow its instructions to accomplish the test prompt yourself. Do them one at a time. This is less rigorous than independent subagents (you wrote the skill and you're also running it, so you have full context), but it's a useful sanity check — and the human review step compensates. Skip the baseline runs — just use the skill to complete the task as requested. + +**Reviewing results**: If you can't open a browser (e.g., Claude.ai's VM has no display, or you're on a remote server), skip the browser reviewer entirely. Instead, present results directly in the conversation. For each test case, show the prompt and the output. If the output is a file the user needs to see (like a .docx or .xlsx), save it to the filesystem and tell them where it is so they can download and inspect it. Ask for feedback inline: "How does this look? Anything you'd change?" + +**Benchmarking**: Skip the quantitative benchmarking — it relies on baseline comparisons which aren't meaningful without subagents. Focus on qualitative feedback from the user. + +**The iteration loop**: Same as before — improve the skill, rerun the test cases, ask for feedback — just without the browser reviewer in the middle. You can still organize results into iteration directories on the filesystem if you have one. + +**Description optimization**: This section requires the `claude` CLI tool (specifically `claude -p`) which is only available in Claude Code. Skip it if you're on Claude.ai. + +**Blind comparison**: Requires subagents. Skip it. + +**Packaging**: The `package_skill.py` script works anywhere with Python and a filesystem. On Claude.ai, you can run it and the user can download the resulting `.skill` file. + +--- + +## Cowork-Specific Instructions + +If you're in Cowork, the main things to know are: + +- You have subagents, so the main workflow (spawn test cases in parallel, run baselines, grade, etc.) all works. (However, if you run into severe problems with timeouts, it's OK to run the test prompts in series rather than parallel.) +- You don't have a browser or display, so when generating the eval viewer, use `--static ` to write a standalone HTML file instead of starting a server. Then proffer a link that the user can click to open the HTML in their browser. +- For whatever reason, the Cowork setup seems to disincline Claude from generating the eval viewer after running the tests, so just to reiterate: whether you're in Cowork or in Claude Code, after running tests, you should always generate the eval viewer for the human to look at examples before revising the skill yourself and trying to make corrections, using `generate_review.py` (not writing your own boutique html code). Sorry in advance but I'm gonna go all caps here: GENERATE THE EVAL VIEWER *BEFORE* evaluating inputs yourself. You want to get them in front of the human ASAP! +- Feedback works differently: since there's no running server, the viewer's "Submit All Reviews" button will download `feedback.json` as a file. You can then read it from there (you may have to request access first). +- Packaging works — `package_skill.py` just needs Python and a filesystem. +- Description optimization (`run_loop.py` / `run_eval.py`) should work in Cowork just fine since it uses `claude -p` via subprocess, not a browser, but please save it until you've fully finished making the skill and the user agrees it's in good shape. + +--- + +## Reference files + +The agents/ directory contains instructions for specialized subagents. Read them when you need to spawn the relevant subagent. + +- `agents/grader.md` — How to evaluate assertions against outputs +- `agents/comparator.md` — How to do blind A/B comparison between two outputs +- `agents/analyzer.md` — How to analyze why one version beat another + +The references/ directory has additional documentation: +- `references/schemas.md` — JSON structures for evals.json, grading.json, etc. + +--- + +Repeating one more time the core loop here for emphasis: + +- Figure out what the skill is about +- Draft or edit the skill +- Run claude-with-access-to-the-skill on test prompts +- With the user, evaluate the outputs: + - Create benchmark.json and run `eval-viewer/generate_review.py` to help the user review them + - Run quantitative evals +- Repeat until you and the user are satisfied +- Package the final skill and return it to the user. + +Please add steps to your TodoList, if you have such a thing, to make sure you don't forget. If you're in Cowork, please specifically put "Create evals JSON and run `eval-viewer/generate_review.py` so human can review test cases" in your TodoList to make sure it happens. + +Good luck! \ No newline at end of file diff --git a/.gitignore b/.gitignore index b5d155fc..69f85a7f 100644 --- a/.gitignore +++ b/.gitignore @@ -48,7 +48,7 @@ pnpm-debug.log* lerna-debug.log* # Claude -.claude/ +.claude/settings.local.json # Generated docs docs/swagger.json diff --git a/claude.md b/claude.md index b4686305..ac0cf54f 100644 --- a/claude.md +++ b/claude.md @@ -162,6 +162,6 @@ Runs on every push/PR to `main` (`.github/workflows/audit.yaml`): **Auth:** `GET /v1/auth/check-email`, `GET /v1/auth/me` **Hacker:** `GET|PATCH /v1/applications/me`, `POST /v1/applications/me/submit` -**Admin:** `GET /v1/admin/applications`, `GET /v1/admin/applications/stats`, `GET /v1/admin/applications/{id}`, `GET /v1/admin/applications/{id}/notes`, `GET /v1/admin/reviews/pending`, `GET /v1/admin/reviews/completed`, `GET /v1/admin/reviews/next`, `PUT /v1/admin/reviews/{id}`, `GET /v1/admin/scans/types`, `POST /v1/admin/scans`, `GET /v1/admin/scans/user/{userID}`, `GET /v1/admin/scans/stats` +**Admin:** `GET /v1/admin/applications`, `GET /v1/admin/applications/stats`, `GET /v1/admin/applications/{id}`, `GET /v1/admin/applications/{id}/notes`, `GET /v1/admin/reviews/pending`, `GET /v1/admin/reviews/completed`, `GET /v1/admin/reviews/next`, `PUT /v1/admin/reviews/{id}`, `GET /v1/admin/scans/types`, `POST /v1/admin/scans`, `GET /v1/admin/scans/user/{userID}`, `GET /v1/admin/scans/stats`, `POST /v1/admin/scans/rebalance-stats` **Super Admin:** `GET|PUT /v1/superadmin/settings/saquestions`, `GET|POST /v1/superadmin/settings/reviews-per-app`, `GET|POST /v1/superadmin/settings/review-assignment-toggle`, `GET|POST /v1/superadmin/settings/admin-schedule-edit-toggle`, `POST /v1/superadmin/applications/assign`, `PATCH /v1/superadmin/applications/{id}/status`, `GET /v1/superadmin/applications/emails`, `PUT /v1/superadmin/settings/scan-types`, `POST /v1/superadmin/scans/rebalance-stats`, `POST /v1/superadmin/emails/qr` **Infra (Basic Auth):** `GET /v1/health`, `GET /v1/debug/vars`, `GET /v1/swagger/*` diff --git a/client/web/src/pages/admin/scans/ScansPage.tsx b/client/web/src/pages/admin/scans/ScansPage.tsx index c2961d50..b7db5e03 100644 --- a/client/web/src/pages/admin/scans/ScansPage.tsx +++ b/client/web/src/pages/admin/scans/ScansPage.tsx @@ -17,9 +17,11 @@ export default function ScansPage() { typesLoading, statsLoading, saving, + rebalancing, fetchTypes, fetchStats, saveScanTypes, + rebalanceStats, setActiveScanType, } = useScansStore(); @@ -77,8 +79,10 @@ export default function ScansPage() { stats={stats} isSuperAdmin={isSuperAdmin} saving={saving} + rebalancing={rebalancing} onSelect={setActiveScanType} onSave={saveScanTypes} + onRebalance={rebalanceStats} /> diff --git a/client/web/src/pages/admin/scans/api.ts b/client/web/src/pages/admin/scans/api.ts index 3336a15b..b8fd7a8f 100644 --- a/client/web/src/pages/admin/scans/api.ts +++ b/client/web/src/pages/admin/scans/api.ts @@ -41,6 +41,17 @@ export async function fetchScanStats( ); } +export async function rebalanceScanStats( + signal?: AbortSignal, +): Promise> { + return postRequest( + "/admin/scans/rebalance-stats", + undefined, + "scan stats", + signal, + ); +} + export async function saveScanTypes( scanTypes: ScanType[], ): Promise> { diff --git a/client/web/src/pages/admin/scans/components/ScanTypesTable.tsx b/client/web/src/pages/admin/scans/components/ScanTypesTable.tsx index 011ba287..9ac11bba 100644 --- a/client/web/src/pages/admin/scans/components/ScanTypesTable.tsx +++ b/client/web/src/pages/admin/scans/components/ScanTypesTable.tsx @@ -2,6 +2,7 @@ import { Loader2, Pencil, Plus, + RefreshCw, ScanLine, Trash2, UserCheck, @@ -51,10 +52,12 @@ interface ScanTypesTableProps { stats: ScanStat[]; isSuperAdmin: boolean; saving: boolean; + rebalancing: boolean; onSelect: (scanType: ScanType) => void; onSave: ( scanTypes: ScanType[], ) => Promise<{ success: boolean; error?: string }>; + onRebalance: () => void; } export function ScanTypesTable({ @@ -62,13 +65,16 @@ export function ScanTypesTable({ stats, isSuperAdmin, saving, + rebalancing, onSelect, onSave, + onRebalance, }: ScanTypesTableProps) { const [editingIndex, setEditingIndex] = useState(null); const [editDisplayName, setEditDisplayName] = useState(""); const [deleteIndex, setDeleteIndex] = useState(null); const [pendingNew, setPendingNew] = useState(null); + const [rebalanceOpen, setRebalanceOpen] = useState(false); const editRowRef = useRef(null); // When there's a pending new row, append it so it renders in the table @@ -256,14 +262,30 @@ export function ScanTypesTable({ return ( - + {displayTypes.length} scan type(s){" "} {isSuperAdmin ? "configured" : "available"} - {saving && ( - - )} +
+ {saving && ( + + )} + +
@@ -509,6 +531,30 @@ export function ScanTypesTable({ + + + + + Rebalance scan counts? + + This recomputes every scan count from scratch against the + database. It's an expensive operation and should only be used + when you need to verify the actual counts are accurate. + + + + + Cancel + + onRebalance()} + > + Rebalance + + + + ); } diff --git a/client/web/src/pages/admin/scans/store.ts b/client/web/src/pages/admin/scans/store.ts index 0c2cce14..d3db4f01 100644 --- a/client/web/src/pages/admin/scans/store.ts +++ b/client/web/src/pages/admin/scans/store.ts @@ -1,9 +1,13 @@ +import { toast } from "sonner"; import { create } from "zustand"; +import { errorAlert } from "@/shared/lib/api"; + import { createScan as apiCreateScan, fetchScanStats, fetchScanTypes, + rebalanceScanStats as apiRebalanceScanStats, saveScanTypes as apiSaveScanTypes, } from "./api"; import type { Scan, ScanStat, ScanType } from "./types"; @@ -21,12 +25,14 @@ export interface ScansState { statsLoading: boolean; scanning: boolean; saving: boolean; + rebalancing: boolean; activeScanType: ScanType | null; lastScanResult: ScanResult | null; fetchTypes: (signal?: AbortSignal) => Promise; fetchStats: (signal?: AbortSignal) => Promise; performScan: (userId: string) => Promise; + rebalanceStats: () => Promise; saveScanTypes: ( scanTypes: ScanType[], ) => Promise<{ success: boolean; error?: string }>; @@ -41,6 +47,7 @@ export const useScansStore = create((set, get) => ({ statsLoading: false, scanning: false, saving: false, + rebalancing: false, activeScanType: null, lastScanResult: null, @@ -108,6 +115,22 @@ export const useScansStore = create((set, get) => ({ } }, + rebalanceStats: async () => { + if (get().rebalancing) return; + + set({ rebalancing: true }); + + const res = await apiRebalanceScanStats(); + + if (res.status === 200 && res.data) { + set({ stats: res.data.stats, rebalancing: false }); + toast.success("Scan counts rebalanced"); + } else { + set({ rebalancing: false }); + errorAlert(res, "Failed to rebalance scan counts"); + } + }, + saveScanTypes: async (scanTypes: ScanType[]) => { set({ saving: true }); const res = await apiSaveScanTypes(scanTypes); diff --git a/cmd/api/api.go b/cmd/api/api.go index b7d306b0..cd0f5fc7 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -104,7 +104,6 @@ const swaggerTagsSorter = `(a, b) => { "admin/schedule", "admin/sponsors", "superadmin/applications", - "superadmin/scans", "superadmin/settings", "superadmin/users" ]; @@ -227,6 +226,7 @@ func (app *application) mount() http.Handler { r.Get("/types", app.getScanTypesHandler) r.Get("/user/{userID}", app.getUserScansHandler) r.Get("/stats", app.getScanStatsHandler) + r.Post("/rebalance-stats", app.rebalanceScanStatsHandler) }) // Schedule @@ -295,10 +295,6 @@ func (app *application) mount() http.Handler { r.Patch("/{userID}/role", app.updateUserRoleHandler) }) - r.Route("/scans", func(r chi.Router) { - r.Post("/rebalance-stats", app.rebalanceScanStatsHandler) - }) - // Scheduled push notifications r.Route("/notifications", func(r chi.Router) { r.Get("/", app.listScheduledNotificationsHandler) diff --git a/cmd/api/scans.go b/cmd/api/scans.go index 82daeab0..c8766d5d 100644 --- a/cmd/api/scans.go +++ b/cmd/api/scans.go @@ -268,16 +268,16 @@ func (app *application) getScanStatsHandler(w http.ResponseWriter, r *http.Reque // rebalanceScanStatsHandler recomputes the scan_stats counter cache from the scans table // -// @Summary Rebalance scan statistics (Super Admin) +// @Summary Rebalance scan statistics (Admin) // @Description Recomputes the scan_stats counter cache from the authoritative scans table and returns the recomputed stats -// @Tags superadmin/scans +// @Tags admin/scans // @Produce json // @Success 200 {object} ScanStatsResponse // @Failure 401 {object} object{error=string} // @Failure 403 {object} object{error=string} // @Failure 500 {object} object{error=string} // @Security CookieAuth -// @Router /superadmin/scans/rebalance-stats [post] +// @Router /admin/scans/rebalance-stats [post] func (app *application) rebalanceScanStatsHandler(w http.ResponseWriter, r *http.Request) { stats, err := app.store.Scans.RebalanceStats(r.Context()) if err != nil { diff --git a/cmd/api/scans_test.go b/cmd/api/scans_test.go index 5aa7c884..2e0104b8 100644 --- a/cmd/api/scans_test.go +++ b/cmd/api/scans_test.go @@ -380,7 +380,7 @@ func TestGetScanStats(t *testing.T) { } func TestRebalanceScanStats(t *testing.T) { - t.Run("super admin recomputes stats", func(t *testing.T) { + t.Run("admin recomputes stats", func(t *testing.T) { app := newTestApplication(t) mockScans := app.store.Scans.(*store.MockScansStore) @@ -393,7 +393,7 @@ func TestRebalanceScanStats(t *testing.T) { req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err) - req = setUserContext(req, newSuperAdminUser()) + req = setUserContext(req, newAdminUser()) rr := executeRequest(req, http.HandlerFunc(app.rebalanceScanStatsHandler)) checkResponseCode(t, http.StatusOK, rr.Code) @@ -426,13 +426,13 @@ func TestRebalanceScanStats(t *testing.T) { mockScans.AssertExpectations(t) }) - t.Run("403 when admin (non-super-admin)", func(t *testing.T) { + t.Run("403 when hacker (non-admin)", func(t *testing.T) { app := newTestApplication(t) - handler := app.RequireRoleMiddleware(store.RoleSuperAdmin)(http.HandlerFunc(app.rebalanceScanStatsHandler)) + handler := app.RequireRoleMiddleware(store.RoleAdmin)(http.HandlerFunc(app.rebalanceScanStatsHandler)) req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err) - req = setUserContext(req, newAdminUser()) + req = setUserContext(req, newTestUser()) rr := executeRequest(req, handler) checkResponseCode(t, http.StatusForbidden, rr.Code) @@ -440,7 +440,7 @@ func TestRebalanceScanStats(t *testing.T) { t.Run("401 when unauthenticated", func(t *testing.T) { app := newTestApplication(t) - handler := app.RequireRoleMiddleware(store.RoleSuperAdmin)(http.HandlerFunc(app.rebalanceScanStatsHandler)) + handler := app.RequireRoleMiddleware(store.RoleAdmin)(http.HandlerFunc(app.rebalanceScanStatsHandler)) req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err) diff --git a/docs/docs.go b/docs/docs.go index fa056b41..39612254 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -920,6 +920,64 @@ const docTemplate = `{ } } }, + "/admin/scans/rebalance-stats": { + "post": { + "security": [ + { + "CookieAuth": [] + } + ], + "description": "Recomputes the scan_stats counter cache from the authoritative scans table and returns the recomputed stats", + "produces": [ + "application/json" + ], + "tags": [ + "admin/scans" + ], + "summary": "Rebalance scan statistics (Admin)", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/main.ScanStatsResponse" + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + }, + "403": { + "description": "Forbidden", + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + }, "/admin/scans/stats": { "get": { "security": [ @@ -3414,64 +3472,6 @@ const docTemplate = `{ } } }, - "/superadmin/scans/rebalance-stats": { - "post": { - "security": [ - { - "CookieAuth": [] - } - ], - "description": "Recomputes the scan_stats counter cache from the authoritative scans table and returns the recomputed stats", - "produces": [ - "application/json" - ], - "tags": [ - "superadmin/scans" - ], - "summary": "Rebalance scan statistics (Super Admin)", - "responses": { - "200": { - "description": "OK", - "schema": { - "$ref": "#/definitions/main.ScanStatsResponse" - } - }, - "401": { - "description": "Unauthorized", - "schema": { - "type": "object", - "properties": { - "error": { - "type": "string" - } - } - } - }, - "403": { - "description": "Forbidden", - "schema": { - "type": "object", - "properties": { - "error": { - "type": "string" - } - } - } - }, - "500": { - "description": "Internal Server Error", - "schema": { - "type": "object", - "properties": { - "error": { - "type": "string" - } - } - } - } - } - } - }, "/superadmin/settings/admin-schedule-edit-toggle": { "get": { "security": [ From 6b15be7cd5ad396aee3f683f55198f501d8fea58 Mon Sep 17 00:00:00 2001 From: Caleb Bae Date: Sun, 21 Jun 2026 12:00:40 -0700 Subject: [PATCH 4/4] add: ci pipeline skill --- .claude/skills/ci-pipeline/SKILL.md | 206 ++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 .claude/skills/ci-pipeline/SKILL.md diff --git a/.claude/skills/ci-pipeline/SKILL.md b/.claude/skills/ci-pipeline/SKILL.md new file mode 100644 index 00000000..f96b16d5 --- /dev/null +++ b/.claude/skills/ci-pipeline/SKILL.md @@ -0,0 +1,206 @@ +--- +name: ci-pipeline +description: >- + Authoritative reference for HARP's CI/CD pipeline — what GitHub Actions run and + when, the release-please release flow, automated version bumping, the + Cloud Build → Cloud Run deploy, the Docker build, and the local git hooks + (pre-commit gofmt + commit-msg Conventional Commits). Use this skill whenever + someone asks how CI works, what checks run on a PR or push, why a workflow + exists, how releases or versioning happen, how deploys are triggered, what the + git hooks do, or how to set them up — even if they don't say "CI" explicitly + (e.g. "what runs when I open a PR?", "how does a new version ship?", "why is my + commit being rejected?", "what does push to main do?"). Prefer this over + guessing from memory; it reflects the actual workflow files in this repo. +--- + +# HARP CI/CD Pipeline + +This skill explains how HARP's continuous integration, release, and deployment +machinery actually works. It is a **reference** — answer questions accurately and +cite the concrete file/step the behavior comes from, so the user can verify. + +The source of truth is three workflow files in `.github/workflows/`, the git +hooks in `.github/hooks/`, the `Dockerfile`, and `Taskfile.yml`. If a question +goes beyond what's described here, read those files directly rather than +inventing an answer — pipelines drift, and a wrong answer about CI wastes a push +cycle. + +## The big picture + +There are three pipelines, triggered by different events: + +| Pipeline | Trigger | What it does | +| ------------------- | ----------------------------- | -------------------------------------------------------- | +| **CI / audit** | push **or** PR to `main` | Lint, build, and test the Go backend and React frontend | +| **Release** | push to `main` | release-please PR → tag/release → bump `version` in code | +| **Deploy (CD)** | merge to `main` | Google Cloud Build → Cloud Run (auto-deploy) | + +And two **local** git hooks (they run on your machine, not in CI) keep commits +clean before they ever reach GitHub. + +A normal change flows: branch → commit (hooks run locally) → open PR (CI runs) → +merge to `main` (CI runs again + release + deploy fire). + +## CI — `.github/workflows/audit.yaml` + +Workflow name: **CI**. Runs on `push` to `main` and on `pull_request` targeting +`main`. Two independent jobs run in parallel on `ubuntu-latest`. If either job +fails, the check is red and the PR can't merge cleanly. + +### `backend-audit` job (Go) + +Go version **1.24.x**. Steps, in order — each is a gate: + +1. **Check gofmt** — `gofmt -l .`; fails if any file is unformatted. Fix with + `gofmt -w .`. +2. **Verify Dependencies** — `go mod verify`. +3. **Build** — `go build -v ./...`. +4. **go vet** — `go vet ./...`. +5. **staticcheck** — installs `honnef.co/go/tools/cmd/staticcheck@v0.6.1`, then + `staticcheck ./...`. +6. **Tests** — `go test -race ./...` (race detector on). + +### `frontend-audit` job (React, in `client/web`) + +Node **22**, npm cache keyed on `client/web/package-lock.json`. Runs with working +directory `client/web`. Steps, in order: + +1. **Install** — `npm ci` (clean install from lockfile). +2. **Format Check** — `npm run format:check` (Prettier `--check`). +3. **Lint** — `npm run lint` (ESLint). +4. **Type Check & Build** — `npm run build` (which is `tsc -b && vite build`, so + this is *both* the TypeScript type check and the production build). +5. **Dependency Audit** — `npm audit --audit-level=high --omit=dev` (prod deps + only; high+ severity fails). +6. **Tests** — runs `npx vitest run` **only if** test files exist (a `__tests__` + dir or any `*.test.*` / `*.spec.*` under `src`); otherwise it prints "No test + files found, skipping" and passes. + +### Reproducing CI locally + +The fastest way to predict a green/red CI run is to run the same commands before +pushing. The CLAUDE.md command tables list them; the key mirrors are +`gofmt -l .`, `go vet ./...`, `staticcheck ./...`, `go test -race ./...` for the +backend and `npm run format:check && npm run lint && npm run build` in +`client/web` for the frontend. + +## Release — `release-please.yaml` + `update-api-version.yaml` + +Releasing is automated and driven by Conventional Commit messages. Two workflows +cooperate, both triggered on **push to `main`**. + +### 1. release-please — `release-please.yaml` + +Uses `googleapis/release-please-action@v4` with **`release-type: simple`**, +authenticated by the `MY_RELEASE_PLEASE_TOKEN` secret (a PAT, so its pushes can +re-trigger other workflows). + +How it works: on every push to `main`, release-please scans Conventional Commits +since the last release and maintains an open **"release PR"**. That PR +accumulates the next version bump and the generated `CHANGELOG.md` entries. + +- `feat:` commits → **minor** bump, `fix:` → **patch** bump, `feat!:`/`fix!:` or + a `BREAKING CHANGE:` footer → **major** bump. `chore:`, `docs:`, `refactor:`, + etc. show up in the changelog but don't drive the version on their own. +- **Nothing is released until you merge the release PR.** Merging it makes + release-please create the git tag and the GitHub Release and finalize the + `CHANGELOG.md` for that version. + +This is why commit message format matters (and why the `commit-msg` hook exists). + +### 2. Version bump in code — `update-api-version.yaml` + +Workflow name: **Update Version and Release**. Also on push to `main`. It keeps +the Go binary's reported version in sync with the changelog: + +1. Extract the latest version from `CHANGELOG.md` (first `[X.Y.Z]` it finds). +2. `sed` it into `const version = "..."` in `cmd/api/main.go`. +3. If that changed anything, commit as `chore: update api version to X.Y.Z` and + push (as `GitHub Action `, using `MY_RELEASE_PLEASE_TOKEN`). + +So `const version` in `cmd/api/main.go` (currently `1.2.0`) is **machine-managed** +— don't hand-edit it; it follows the changelog after a release PR merges. + +## Deploy (CD) — Cloud Build → Cloud Run + +Merges to `main` trigger **Google Cloud Build**, which builds the container and +deploys to **Google Cloud Run** (auto-deploy). The build config lives in Google +Cloud (not a YAML in this repo), so there's nothing here to read for the trigger +itself — the contract is the `Dockerfile`. + +### The Docker build — `Dockerfile` + +Multi-stage, producing a tiny `scratch` image: + +1. **Stage `frontend`** (`node:22-alpine`): `npm ci` then `npm run build` in + `client/web`. Takes a build arg `VITE_GOOGLE_AUTH_ENABLED` (default `true`). +2. **Stage `builder`** (`golang:1.24`): `go mod download`, then a static build + `CGO_ENABLED=0 GOOS=linux go build -trimpath -ldflags="-s -w" -o /app/api ./cmd/api`. +3. **Stage final** (`scratch`): copies CA certs, the `api` binary, and the built + frontend into `./static`. `EXPOSE 8080`, `CMD ["./api"]`. + +Key consequence: **the frontend is compiled at image-build time and served as +static files by the Go binary** — there is no separate frontend server in prod. +The single container listens on **8080**. + +Managed dependencies in prod: **Neon** (PostgreSQL), **Google Cloud Storage** +(file storage), **SuperTokens** (auth), **SendGrid** (email). + +## Local git hooks — `.github/hooks/` + +These run on the developer's machine, gating commits **before** code reaches +GitHub. They are not GitHub Actions. They live in `.github/hooks/` and are +activated by pointing git at that directory. + +### Enabling them — `task setup-hooks` + +Hooks are **opt-in per clone**. Run once after cloning: + +``` +task setup-hooks +``` + +which runs `git config core.hooksPath .github/hooks`. Verify with +`git config core.hooksPath` — it should print `.github/hooks`. If it prints +something else (e.g. a leftover `.husky/_` from another tool), the project hooks +are **not active** and you should re-run `task setup-hooks`. + +### `pre-commit` — auto-format Go + +On commit, runs `gofmt` against staged `.go` files (`--diff-filter=ACM`). Any +unformatted file is reformatted with `gofmt -w` **and re-staged automatically**, +so your commit ends up gofmt-clean. It never blocks the commit; it fixes and +continues. This is what keeps the CI `gofmt` gate green. No-ops if no `.go` files +are staged. + +### `commit-msg` — enforce Conventional Commits + +Validates the commit subject against: + +``` +^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\(.+\))?!?: .+ +``` + +i.e. `type(optional-scope)!: description`. Merge commits (`^Merge `) are exempt. +A non-conforming message is **rejected** with a help text listing valid types and +examples. This matters because release-please parses these messages to compute +the next version and changelog — bad messages mean bad releases. + +Valid: `feat(auth): add Google OAuth login`, `fix: resolve pagination bug`, +`chore!: drop Node 16 support`. Invalid: `updated stuff`, `WIP`. + +## Quick answers to common questions + +- **"What runs when I open a PR?"** → the **CI** workflow only (backend-audit + + frontend-audit). Release and deploy are `main`-only. +- **"What happens when something merges to `main`?"** → CI runs again, *and* + release-please updates/creates the release PR, *and* the version-bump workflow + runs, *and* Cloud Build deploys to Cloud Run. +- **"How do I cut a release?"** → merge the open release-please PR. You don't tag + manually. +- **"Why was my commit rejected?"** → the `commit-msg` hook; your subject isn't a + Conventional Commit. (See the regex above.) +- **"Why is `const version` changing in commits I didn't make?"** → the + `update-api-version.yaml` workflow syncs it from `CHANGELOG.md`. +- **"My hooks aren't running."** → `core.hooksPath` isn't set to `.github/hooks`; + run `task setup-hooks`.