diff --git a/.windsurf/skills/error-logging-flow/SKILL.md b/.windsurf/skills/error-logging-flow/SKILL.md new file mode 100644 index 0000000..738b2e1 --- /dev/null +++ b/.windsurf/skills/error-logging-flow/SKILL.md @@ -0,0 +1,104 @@ +--- +name: error-logging-flow +description: Use when writing or reviewing error handling anywhere in this app-starter — API route handlers, services, frontend data calls, or anything that catches an exception. Explains the first-class error-logging flow (5xx persist to the error log and surface on the /errors page) and the hard rule against swallowing real defects with try/catch + console.error. Use it to wire errors correctly and to spot swallowed errors during review. +--- + +# Error-logging flow + +Error reporting is a first-class feature here, not an afterthought: real defects +(5xx) are persisted to a DB-backed error log and shown on the `/errors` page. The +cardinal sin is a `try { … } catch { console.error(e) }` that eats a real error — +it makes production failures invisible. This skill is how the flow works end to +end and how to stay inside it. + +> **Scope:** IIS flavor. The error log lives in `_meta_errors` (T-SQL), written by +> `usp_error_log`, read by `fn_error_list`/`fn_error_get` via `ErrorsRepo`. + +## The one rule + +**Never swallow a real defect.** In an API route, you do *not* catch-and-log — +you call `next(err)` and let the central error handler decide. The only code that +legitimately swallows is the logger itself (so a logging failure can't take down +the request) — and it still writes to the console as a last resort. + +## API side — how it flows + +1. **Route handlers `next(err)`.** Every handler is `async (req, res, next) => { + try { … } catch (err) { next(err); } }`. You never `res.status(500)` yourself + and never `console.error` and move on. +2. **Throw typed errors for expected 4xx.** Use `throw new HttpError(404, 'Not + found')` (or the `notFound`/`badRequest`/… response helpers) for client errors. +3. **The central handler** (`packages/api/src/middleware/error-handler.ts`, + mounted last) decides: + - `ZodError` → 422 with a field map (validation — not logged). + - `HttpError` → its status + message (4xx — **not** logged; 4xx is user input + and would drown out real bugs). + - anything else → **500, and persisted** to `_meta_errors` via + `ErrorsRepo.log({ level, source:'api', message, stack, context, user_id })`, + fire-and-forget. +4. **The 500 response body** is generic in production (`serverError` in + `response.ts`) — real `error.message` strings leak table/column names. The + server-side log is the source of truth. + +```ts +// CORRECT — route handler +router.post('/', validate(ProjectCreateSchema), async (req, res, next) => { + try { + const id = await projects.create({ ...req.body, owner_id: req.user!.id }); + const row = await projects.getById(id); + if (!row) throw new HttpError(500, 'Project disappeared after create'); + created(res, row); + } catch (err) { + next(err); // ← logging happens centrally; do NOT console.error here + } +}); +``` + +```ts +// WRONG — swallows a real defect; nothing reaches /errors +try { + await projects.create(input); +} catch (err) { + console.error(err); // ❌ invisible in prod + res.status(500).json({ error: 'oops' }); // ❌ bypasses the handler + log +} +``` + +## Frontend side — automatic + +You almost never log manually on the frontend. The plumbing already covers it: + +- **`apiFetch`** (`lib/api.ts`) routes any **5xx** response through `reportError` + → `POST /errors`. 4xx is left for the form layer to display (it throws + `ApiFetchError`, which `` maps to field errors / a banner). The + `/errors` path itself is excluded to avoid an infinite loop. +- **`installGlobalErrorHandlers`** (`lib/errorLog.ts`) catches uncaught errors and + unhandled promise rejections and reports them. +- The **`ErrorBoundary`** reports render-time crashes. + +So: build features with `apiFetch` and you get error logging for free. Don't add a +parallel `try/catch … console.error` around API calls — it would hide the very +5xx the flow is meant to capture. + +## Reviewing for swallowed errors + +Grep for the anti-patterns and confirm each is legitimate: + +```bash +rg -n "catch" packages/api/src | rg -i "console\.(error|log)" # suspicious in API +rg -n "catch\s*\(.*\)\s*\{\s*\}" packages/ # empty catch blocks +``` + +A catch block is acceptable only when it **re-throws**, calls **`next(err)`**, or +**is** the logger (`ErrorsRepo.log` / `reportError`, which intentionally swallow +and console-fallback). Anything else that eats an error and returns a success-ish +result is a defect. (CLAUDE.md critical rule: do not bypass the error-logging flow +for real defects.) + +## Verify it actually logs + +Force a 500 (e.g. a route hitting the DB while it's unreachable, or a deliberate +throw) while running the app, then open **`/errors`** — the entry should be there +with level `error`, source `api` (or `frontend`), the message, and the request +context. If it isn't, the error was swallowed somewhere upstream — find and fix +that, don't paper over it. diff --git a/.windsurf/skills/feature-surface-map/SKILL.md b/.windsurf/skills/feature-surface-map/SKILL.md index f2801f9..7899e1b 100644 --- a/.windsurf/skills/feature-surface-map/SKILL.md +++ b/.windsurf/skills/feature-surface-map/SKILL.md @@ -1,6 +1,6 @@ --- name: feature-surface-map -description: Use when adding, removing, reviewing, or auditing a feature / domain entity (e.g. tasks, issues, projects, invoices) in this app-starter monorepo. Lists every layer and exact file a feature touches — shared types, the SQL Server data layer (tables, functions, repo, migration, schema snapshot, tests), the API (route + the two wiring files), and the frontend (hooks, pages, and the three registration files). Walk it in build order to implement a feature without missing a layer, or in reverse to remove one without leaving orphans behind. +description: Use when adding, removing, reviewing, or auditing a feature / domain entity (e.g. tasks, issues, projects, invoices) in this app-starter monorepo. Lists every layer and exact file a feature touches — shared types, the SQL Server data layer (tables, stored procedures, repo, migration, schema snapshot, tests), the API (route + the two wiring files), and the frontend (hooks, pages, and the three registration files). Walk it in build order to implement a feature without missing a layer, or in reverse to remove one without leaving orphans behind. --- # Feature surface map (IIS + SQL Server flavor) @@ -16,6 +16,19 @@ feature lives so nothing is missed in either direction. > If you are in the un-ejected template instead, the SQL Server package is at > `packages/db-mssql/` and `packages/db/` is the Postgres copy — adjust paths. +> **⚠ The DB layer below is described for THIS project, not the app-starter template.** +> This project keeps data access in **stored procedures** (very few functions), +> under a path like **`db/schema/stored-procedures/`**, with **schema-qualified, +> prefix-less** names such as `crd.adjustment_dtl_delete` or `dbo._` +> across schemas (`dbo`, `crd`, …). Their exact naming and read/write **conventions & +> best practices are TBD — determine them by analyzing the existing procs**, then +> capture them in a `stored-proc-authoring` skill. (The shipped app-starter template +> instead uses `usp_`/`fn_` objects under `dbo`; the `mssql-function-authoring` skill +> covers template T-SQL mechanics — GO batches, positional binding — that transfer +> regardless of naming.) The *shape* of the surface map (types → DB → API → frontend, +> plus the "register" files) holds either way — only the DB paths and object names +> change. + Throughout, `` is the singular domain noun (`task`, `issue`, `project`). The running real example is **`issues`**, which exists in the repo end to end and even has a child entity (`issue_comments`) — pattern-match against it. @@ -30,9 +43,9 @@ Build top-to-bottom (each layer depends on the ones above). Remove bottom-to-top |---|-------|--------------------------------|------| | 1 | **Shared types** | `packages/types/src/.ts` | define | | 1r | **Types barrel** | `packages/types/src/index.ts` → `export * from './.js'` | **register** | -| 2 | **DB migration** | `packages/db/src/migrations/NNN_.sql` (+ optional `.down.sql`) | define | -| 3 | **Table snapshot** | `packages/db/schema/tables/.sql` | define | -| 4 | **Function snapshots** | `packages/db/schema/functions/{usp,fn}__*.sql` (one file each) | define | +| 2 | **DB migration / DDL** | `packages/db/src/migrations/NNN_.sql` (+ optional `.down.sql`) — creates the table **and its stored procedures** | define | +| 3 | **Table snapshot** | `schema/tables/
.sql` | define | +| 4 | **Stored procedures** (reads **and** writes; one `.sql` per proc) | `db/schema/stored-procedures/._.sql` — e.g. `crd.adjustment_dtl_delete.sql` (schemas `dbo`/`crd`; **no** `usp_`/`fn_` prefix). Exact naming + read/write conventions & best practices **TBD — determine them by analyzing this project's existing stored procedures** (then capture them in a `stored-proc-authoring` skill). | define | | 5 | **Row interface** | `packages/db/src/database.ts` → `interface Table` + entry on `Database` | define | | 6 | **Repository** | `packages/db/src/repositories/.repo.ts` | define | | 6r | **DB barrel** | `packages/db/src/index.ts` → export the repo + row type | **register** | @@ -77,8 +90,8 @@ rg -i "issue" packages/ --glob '!**/node_modules/**' # Files only (a quick map of the blast radius): rg -il "issue" packages/ --glob '!**/node_modules/**' -# The SQL function names specifically: -rg -l "fn_issue|usp_issue" packages/ +# The DB object names specifically (stored procs / functions for the entity): +rg -l "issue_(create|update|delete|get|list|select)|fn_issue|usp_issue" packages/ ``` Run this twice — once with the singular (`issue`) and once with the plural/table @@ -97,21 +110,24 @@ Go top-to-bottom. Each step has a real file to copy. model `z.infer`-ed from it, plus `…CreateSchema` / `…UpdateSchema`. Then add the `export * from './.js'` line to `index.ts`. **Never** redefine a schema in the frontend — import from `@app/types`. -2. **Migration** (`packages/db/src/migrations/NNN_.sql`): create the - table, `CHECK` constraints matching the enum tuples from step 1, and the - functions — `usp__create/update/delete` (writes) and - `fn__get/list` (reads). **No direct table access anywhere else** — - functions are the only door. -3. **Snapshots** (`schema/tables/` + `schema/functions/`): one file per table - and per function, **in the same commit** as the migration. The contract test - fails otherwise. +2. **Migration + DB objects**: create the table, `CHECK` constraints matching the + enum tuples from step 1, and the **stored procedures** for every read and write + (`_create/update/delete`, `_get/list/select` — use your repo's + actual action vocabulary and schema, e.g. `crd._`). **No direct + table access anywhere else** — a proc (or function) is the only door. *(Exact + naming + read/write conventions & best practices are TBD — derive them from this + project's existing stored procedures; see the `mssql-function-authoring` skill.)* +3. **Snapshots**: one `.sql` per table and **per stored proc / function**, in the + same commit as the migration (your project: `db/schema/stored-procedures/`; + template: `schema/{tables,functions}/`). The contract test fails otherwise. 4. **`database.ts`**: add the `Table` interface and the `Database` entry. 5. **Repo** (`repositories/.repo.ts`, mirror `issues.repo.ts`): a thin wrapper where every method calls a function via the `callFunction*` helpers. Export it from `src/index.ts`. -6. **Tests** (`test/.test.ts`): seed via `usp_`, read via `fn_`, validate - the raw row with `Schema.strict()`; CRUD round-trip. This is the - highest-value coverage in the repo (the SQL↔TS seam `tsc` can't see). +6. **Tests** (`test/.test.ts`): seed via the write proc, read via the read + proc/function, validate the raw row with `Schema.strict()`; CRUD + round-trip. This is the highest-value coverage in the repo (the SQL↔TS seam + `tsc` can't see). 7. **Route** (`routes/.ts`, mirror `routes/tasks.ts`): `validate(schema)` at the boundary, `auth.requireAuth`, always `next(err)` for 5xx (the error handler logs to the error table — never `console.error` and swallow). @@ -151,19 +167,21 @@ transient broken states). Grep first (see above), then walk the map in reverse: both files for a now-unused repo/type. 4. **DB** — delete the repo, its `src/index.ts` exports, the `Table` interface + `Database` entry in `database.ts`, the `test/.test.ts`, the - migration file(s), and **the snapshot files** (`schema/tables/` + - `schema/functions/`). Removing snapshots without removing functions (or vice - versa) fails the contract test. If a deployed DB already has the objects, write - a `DROP`-ing migration rather than just deleting files. + migration file(s), and **every snapshot file** for the entity (the table + snapshot **and each stored-proc/function `.sql`** — your project: + `db/schema/stored-procedures/`; template: `schema/{tables,functions}/`). + Removing snapshots without removing the DB objects (or vice versa) fails the + contract test. If a deployed DB already has the objects, write a `DROP`-ing + migration rather than just deleting files. 5. **Types** — delete `packages/types/src/.ts` and its line in `index.ts`. 6. **Sweep** — re-run the grep. Zero hits for the entity name (outside git history) means clean. Then run the full verify stack — a dangling import or a stale schema snapshot will surface there. > **Watch for shared/child references.** `issues` has a child `issue_comments` -> (its own table, `fn_issue_comments_list`, `usp_issue_comment_create`, and a -> detail page) — removing `issues` means removing those too. Conversely, don't -> delete something a *surviving* feature still imports (e.g. a shared formatter). +> (its own table, its own `issue_comments_*` procs, and a detail page) — removing +> `issues` means removing those too. Conversely, don't delete something a +> *surviving* feature still imports (e.g. a shared formatter). --- @@ -172,12 +190,14 @@ transient broken states). Grep first (see above), then walk the map in reverse: For each layer in the map, confirm **present + consistent**: - **Schema is the single source of truth.** The `Table` interface - (`database.ts`), the SQL function output, and the frontend form all trace back - to the one `@app/types` schema. No re-spelled "form schema." -- **Functions-only.** No `selectFrom` / raw table access — reads go through - `fn_*`, writes through `usp_*`. -- **Snapshots match the DB.** Every `fn_`/`usp_` has a `schema/functions/` file; - the contract test enforces it. + (`database.ts`), the read proc/function's output, and the frontend form all + trace back to the one `@app/types` schema. No re-spelled "form schema." +- **Procedures/functions-only.** No `selectFrom` / raw table access — every read + and write goes through a stored procedure (or function). *(Follow this project's + existing proc naming/conventions — they don't use `usp_`/`fn_` prefixes.)* +- **Snapshots match the DB.** Every stored proc / function has a matching `.sql` + snapshot (your project: `db/schema/stored-procedures/`; template: + `schema/functions/`); the contract test enforces it. - **Errors flow to the log.** Routes `next(err)` rather than swallowing — 5xx must land in the error log / `/errors` page. - **No `
`, no bare forms.** Lists use ``; forms use ``. @@ -214,11 +234,13 @@ test for UI the suite can't cover. ## SQL Server specifics (this flavor's data layer) -When writing migrations/functions, T-SQL conventions differ from Postgres: +When writing migrations / stored procs, T-SQL conventions differ from Postgres: -- Writes are **stored procedures** called via `EXEC`, returning the new id as a - `value` column; reads are **inline table-valued functions** - (`SELECT * FROM dbo.fn__get(@id)`). +- Reads **and** writes go through **stored procedures** (the occasional function + aside). How they're invoked, how ids/rows come back, and how they're named are + **TBD — determine them from this project's existing procs** (e.g. + `crd._`, no `usp_`/`fn_` prefix); see the `mssql-function-authoring` + skill. - UUID PKs: `uniqueidentifier DEFAULT NEWID()`. Timestamps: `datetimeoffset(7)` defaulting to `SYSUTCDATETIME()` (UTC). JSON columns: `nvarchar(max)` with an `ISJSON` check. diff --git a/.windsurf/skills/frontend-crud-page/SKILL.md b/.windsurf/skills/frontend-crud-page/SKILL.md new file mode 100644 index 0000000..0975cbe --- /dev/null +++ b/.windsurf/skills/frontend-crud-page/SKILL.md @@ -0,0 +1,157 @@ +--- +name: frontend-crud-page +description: Use when building or reviewing a frontend list/create/edit page in this app-starter's React/Vite SPA. Covers the required building blocks — DataTable for lists (never a raw table element), AppForm + typed field wrappers for forms (schema imported from @app/types, never re-declared), the per-domain hooks file, the three-page route layout, and registering routes in router.ts / AppSidebar / lib/routes. Enforces the UI-completeness rule (no half-wired columns or fields). +--- + +# Frontend CRUD page + +The SPA has a fixed shape for domain pages. Following it keeps lists, forms, +validation, and error handling consistent and wired to the shared contract. The +two rules that matter most: **lists use `` (never a raw `
`)**, +and **forms use `` with the zod schema from `@app/types` (never a +re-declared "form schema")**. + +> **Scope:** both flavors share this frontend. `` is the singular domain +> noun; `tasks` is the canonical example to copy (under `routes/starters/tasks/`). +> Real domain entities live at `routes//`. + +## File layout per domain + +``` +src/hooks/.ts # one file per domain: queries + mutations +src/routes// + page.tsx # list → / + new/page.tsx # create → //new + $Id/edit/page.tsx # edit → //:id/edit + _components/ # local badges, formatters, cell renderers +``` + +Then register the routes in **three** files (easy to forget — all three or the +page won't appear / won't link): + +- `src/router.ts` — `import { Route as … }` for each page + add to `addChildren([…])` +- `src/components/AppSidebar.tsx` — a `` in `` +- `src/lib/routes.ts` — entries in `ROUTE_LABELS` (breadcrumb text) and + `NAVIGABLE_ROUTES` (clickable segments) + +## Hooks — one file per domain + +Don't append to a generic `queries.ts`. Each query/mutation uses `apiFetch` and +invalidates the domain's query key on success (so 5xx auto-log via `apiFetch`): + +```ts +// src/hooks/projects.ts +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import type { Project, ProjectCreateInput, ProjectUpdateInput } from '@app/types'; +import { apiFetch } from '@/lib/api'; + +const PROJECTS_KEY = ['projects'] as const; + +export function useProjects(params?: { sort?: string }) { + return useQuery({ + queryKey: [...PROJECTS_KEY, params ?? {}], + queryFn: () => apiFetch('/projects', { query: { sort: params?.sort } }), + }); +} +export function useProject(id: string | undefined) { + return useQuery({ queryKey: [...PROJECTS_KEY, id], queryFn: () => apiFetch(`/projects/${id}`), enabled: !!id }); +} +export function useCreateProject() { + const qc = useQueryClient(); + return useMutation({ + mutationFn: (input) => apiFetch('/projects', { method: 'POST', body: input }), + onSuccess: () => qc.invalidateQueries({ queryKey: PROJECTS_KEY }), + }); +} +// useUpdateProject(id), useDeleteProject() follow the same shape +``` + +## List page — ``, never `
` + +Define `ColumnDef[]`, render ``. **A column must be fully +wired** — header + accessor + cell renderer. No placeholder columns (UI +completeness rule). + +```tsx +const columns: ColumnDef[] = [ + { accessorKey: 'name', header: 'Name', meta: { width: '2fr' } }, + { accessorKey: 'status', header: 'Status', meta: { width: '120px' }, + cell: (ctx) => ()} /> }, + { accessorKey: '_created_at', header: 'Created', meta: { width: '160px' }, + cell: (ctx) => formatDateTime(ctx.getValue()) }, +]; + +function ProjectsListPage() { + const navigate = useNavigate(); + const projects = useProjects(); + return ( +
+ {/* header + "New" link */} + + columns={columns} + data={projects.data ?? []} + rowKey={(p) => p.project_id} + loading={projects.isLoading} + heightMode="fill" + emptyMessage="No projects yet." + onRowClick={(p) => navigate({ to: '/projects/$projectId/edit', params: { projectId: p.project_id } })} + /> +
+ ); +} +export const Route = createRoute({ getParentRoute: () => RootRoute, path: '/projects', component: ProjectsListPage }); +``` + +## Create / edit pages — `` + +The form's schema is the **`@app/types` schema** — `…CreateSchema` for new, +`…UpdateSchema` for edit. Never re-spell the validation in the component (that's +how API and UI drift). Use the typed field wrappers (`TextField`, `TextareaField`, +`SelectField`, `DateField`, `SubmitButton`, `FormErrorBanner`) — no bare +`react-hook-form` calls. + +```tsx +// src/routes/projects/new/page.tsx + { + setTopError(null); + await createProject.mutateAsync(values); + await navigate({ to: '/projects' }); + }} + onTopLevelError={setTopError} + className="space-y-4" +> + + + + {/* SubmitButton + Cancel */} + +``` + +The edit page (`$Id/edit/page.tsx`) adds `Route.useParams()` for the id, +`useProject(id)` to load (handle `isLoading` / `error`), `useUpdateProject(id)` for +the patch, and a destructive Delete button guarded by `confirm()`. Select option +lists (e.g. `STATUS_OPTIONS`) come from the same `as const` vocabulary tuple that +backs the DB `CHECK` and the zod enum — one source. + +## Validation & errors — already handled + +- 422s: `apiFetch` throws `ApiFetchError`; `` maps `isValidationError()` + to field errors and routes anything else to `onTopLevelError` (the banner). +- 5xx: `apiFetch` auto-reports to `/errors`. Don't wrap calls in your own + `try/catch … console.error` — see the `error-logging-flow` skill. + +## Don't + +- ❌ a raw `
` / hand-rolled grid — use ``. +- ❌ a zod schema defined in the page — import from `@app/types`. +- ❌ a column/field/button that isn't fully wired — no dead placeholders. +- ❌ appending to a shared hooks file — one file per domain. + +## Verify + +`pnpm typecheck`, then run `pnpm dev:api` + `pnpm dev:frontend` and walk +list → new → edit → delete plus a validation failure (see the `verify-before-done` +skill). diff --git a/.windsurf/skills/mssql-function-authoring/SKILL.md b/.windsurf/skills/mssql-function-authoring/SKILL.md new file mode 100644 index 0000000..4fd7309 --- /dev/null +++ b/.windsurf/skills/mssql-function-authoring/SKILL.md @@ -0,0 +1,265 @@ +--- +name: mssql-function-authoring +description: Use when adding or changing the SQL Server data layer in this IIS-flavor app-starter — a migration, a stored procedure (usp_), a table-valued function (fn_), a repository method, the schema snapshot, or a data-layer test. Encodes the functions-only contract, the T-SQL conventions (GO batches, uniqueidentifier/datetimeoffset, EXEC procs returning a `value` column, inline TVFs), the positional @p0 parameter binding, the schema-snapshot-in-the-same-commit rule, and how to validate the SQL↔TypeScript seam with the zod schema. +--- + +# Authoring the SQL Server data layer (T-SQL) + +This is the riskiest layer in the repo: it's the seam between T-SQL and the +TypeScript/zod types, and `tsc` cannot see into it. A wrong column name, type, or +parameter order compiles fine and fails at runtime. Lean on the read test (below) +to catch drift. + +**Read this as template T-SQL *mechanics*, not naming gospel.** The worked example +uses the app-starter template's `usp_`/`fn_` names — **your project doesn't use +those prefixes** (it uses schema-qualified, prefix-less procs like +`crd.adjustment_dtl_delete`; see the callout). What genuinely transfers regardless +of naming: GO batching, `NEWID()`/`datetimeoffset` defaults, `COALESCE` +partial-update logic, positional `@p0` binding, the snapshot-in-the-same-commit +rule, and validating the SQL↔zod seam with the read test. The exact proc naming, +schema split, and return convention are **TBD — take them from this project's +existing procs.** + +> **Scope:** IIS flavor. The data layer is `@app/db` at `packages/db/` (T-SQL, +> `mssql` driver). In the un-ejected template it's `packages/db-mssql/` — same +> code. All examples use a new `projects` entity; mirror the real `tasks` files +> next to them. + +> **⚠ Template conventions vs. YOUR project — the repo you're in wins.** +> This skill documents the *app-starter template's* data-layer style: reads as +> `fn_*` inline table-valued functions, writes as `usp_*` stored procedures, all +> under the `dbo` schema, snapshots in `schema/functions/`. A real project forked +> from (or migrated into) this template often follows **different** conventions — +> commonly: +> - **mostly stored procedures, very few functions** (CRUD is all procs); +> - procs living under a path like **`db/schema/stored-procedures/`**, one file +> per proc; +> - **schema-qualified, prefix-less names** such as `crd.adjustment_dtl_delete` or +> `dbo._` — multiple schemas (`dbo`, `crd`, …), and **no** +> `usp_`/`fn_` prefix. +> +> When the existing code disagrees with this skill, **follow the existing code.** +> Match the real naming, schema split, parameter style, and return convention you +> see in the project's procs — don't impose the template's `usp_`/`fn_` scheme on a +> repo that doesn't use it. The unbreakable rules below (functions/procs-only +> access, snapshot-in-the-same-commit, GO batches, positional binding) still apply; +> only the *names and layout* differ. +> +> **TODO — author a project-specific skill.** Once you can read several real stored +> procedures in this project, write a dedicated skill (e.g. +> `.windsurf/skills/stored-proc-authoring/SKILL.md`) capturing *that* project's +> actual conventions: the exact name pattern, the schemas in use (`dbo`/`crd`/…), +> how parameters are declared, how ids/rows/result sets are returned, error +> handling, and where the `.sql` files live. Use those real procs as the source of +> truth — this template skill is only a starting reference until then. + +## The two unbreakable rules + +1. **Functions-only.** Application code never touches a table directly. Reads go + through `fn_*` inline table-valued functions; writes through `usp_*` stored + procedures. There is no query builder, no `selectFrom`. The only door into the + DB is a function call via the `callFunction*` helpers. +2. **Snapshot in the same commit.** Every migration that adds/changes a `usp_`/`fn_` + must add/update the matching file under `schema/functions/`, and every table + under `schema/tables/`. The `contract.test.ts` test diffs the live DB against + these files in both directions and fails the build otherwise. + +## File checklist for one entity + +``` +packages/db/src/migrations/NNN_.sql # forward (+ optional NNN_.down.sql) +packages/db/schema/tables/
.sql # table snapshot (NEW) +packages/db/schema/functions/usp__create.sql # one file PER function +packages/db/schema/functions/usp__update.sql +packages/db/schema/functions/usp__delete.sql +packages/db/schema/functions/fn__get.sql +packages/db/schema/functions/fn__list.sql +packages/db/src/database.ts # add Table interface + Database entry +packages/db/src/repositories/.repo.ts # thin wrapper over callFunction* +packages/db/src/index.ts # export the repo +packages/db/test/.test.ts # read-validation + CRUD round-trip +``` + +The migration file **contains** every function; the `schema/functions/*.sql` +files are byte-for-byte the same `CREATE` statements, one per file. Author the +migration, then copy each object into its snapshot file. + +## T-SQL conventions (non-negotiable, the driver depends on them) + +- **GO-separated batches.** `CREATE PROCEDURE` / `CREATE FUNCTION` must be the + first statement in its batch. The runner splits the file on `GO` lines and runs + each batch separately. Put `GO` after the table, after each index, after each + proc/function. +- **PK:** `uniqueidentifier NOT NULL ... PRIMARY KEY DEFAULT NEWID()`. +- **Timestamps:** `datetimeoffset(7)` defaulting to `SYSUTCDATETIME()` (UTC). + Metadata columns on every table: `_owner_id varchar(64) NOT NULL`, + `_created_at`, `_updated_at`. +- **Enums:** a `varchar` column + `CHECK (col IN (...))` whose values match the + `as const` vocabulary tuple in `@app/types` exactly. Mismatch = validation + passes, INSERT explodes. +- **`date` columns:** in `fn_*` reads, return them as strings via + `CONVERT(varchar(10), col, 23)` (→ `YYYY-MM-DD`). A bare `date` comes back as a + JS Date and would normalize to a full ISO timestamp, breaking the contract. +- **JSON columns:** `nvarchar(max)` with an `ISJSON(col) = 1` CHECK. **Also** add + the column name to `JSON_COLUMNS` in `connection.ts` so the driver parses it + back to an object (it currently knows `context`, `session_context`). Forgetting + this means the API returns a JSON *string*, not an object. +- **Writes return the id:** create procs end with `SELECT @v_id AS value;` + (the `callFunctionValue` helper reads the `value` column). Update/delete procs + return nothing. +- **Partial updates:** use `COALESCE(@param, col)` so `NULL` means "leave alone," + plus a `@p_clear_* bit = 0` flag per nullable column to distinguish "leave + alone" from "set to NULL" (`CASE WHEN @clear = 1 THEN NULL ELSE COALESCE(...)`). +- **Sorting in `fn_*_list`:** no dynamic SQL. Use a conditional `ORDER BY` with a + trailing stable tiebreaker, paired with `OFFSET/FETCH`. The repo whitelists the + sort column before calling. + +## Worked example — a `projects` migration (TEMPLATE mechanics) + +> The `usp_`/`fn_` names and the `dbo` schema below are the **template's**. In your +> project the names are prefix-less and schema-qualified (`crd._`) +> and reads may be procs too — **match the existing procs.** Study this for the +> *mechanics* (batching, binding, `COALESCE`, returning the id), not the names. + +```sql +-- packages/db/src/migrations/005_projects.sql +CREATE TABLE dbo.projects ( + project_id uniqueidentifier NOT NULL CONSTRAINT pk_projects PRIMARY KEY DEFAULT NEWID(), + name nvarchar(500) NOT NULL, + status varchar(12) NOT NULL, + _created_at datetimeoffset(7) NOT NULL CONSTRAINT df_projects_created DEFAULT SYSUTCDATETIME(), + _updated_at datetimeoffset(7) NOT NULL CONSTRAINT df_projects_updated DEFAULT SYSUTCDATETIME(), + _owner_id varchar(64) NOT NULL, + CONSTRAINT ck_project_status CHECK (status IN ('active', 'archived')) -- matches @app/types tuple +); +GO +CREATE INDEX ix_projects_owner ON dbo.projects (_owner_id); +GO + +CREATE PROCEDURE dbo.usp_project_create + @p0 nvarchar(500), -- name + @p1 varchar(12), -- status + @p2 varchar(64) -- owner_id +AS +BEGIN + SET NOCOUNT ON; + DECLARE @v_id uniqueidentifier = NEWID(); + INSERT INTO dbo.projects (project_id, name, status, _owner_id) VALUES (@v_id, @p0, @p1, @p2); + SELECT @v_id AS value; -- callFunctionValue reads this +END +GO + +CREATE PROCEDURE dbo.usp_project_update + @p0 uniqueidentifier, -- project_id + @p1 nvarchar(500) = NULL, -- name + @p2 varchar(12) = NULL -- status +AS +BEGIN + SET NOCOUNT ON; + UPDATE dbo.projects SET + name = COALESCE(@p1, name), + status = COALESCE(@p2, status), + _updated_at = SYSUTCDATETIME() + WHERE project_id = @p0; +END +GO + +CREATE PROCEDURE dbo.usp_project_delete @p0 uniqueidentifier +AS BEGIN SET NOCOUNT ON; DELETE FROM dbo.projects WHERE project_id = @p0; END +GO + +CREATE FUNCTION dbo.fn_project_get (@p0 uniqueidentifier) +RETURNS TABLE AS RETURN ( + SELECT project_id, name, status, _created_at, _updated_at, _owner_id + FROM dbo.projects WHERE project_id = @p0 +); +GO + +CREATE FUNCTION dbo.fn_project_list (@p0 varchar(64) = NULL, @p1 int = NULL, @p2 int = NULL) +RETURNS TABLE AS RETURN ( + SELECT project_id, name, status, _created_at, _updated_at, _owner_id + FROM dbo.projects + WHERE (@p0 IS NULL OR _owner_id = @p0) + ORDER BY _created_at DESC + OFFSET COALESCE(@p2, 0) ROWS FETCH NEXT COALESCE(@p1, 2147483647) ROWS ONLY +); +GO +``` + +## The repository — positional params, order matters + +`@p0…@pn` are bound positionally. **The repo's `values` array must be in the same +order as the proc's parameters.** This is the #1 silent bug. + +```ts +// packages/db/src/repositories/projects.repo.ts +import { callFunctionRows, callFunctionValue } from '../client.js'; +import type { DbExecutor } from '../connection.js'; +import type { Project } from '@app/types'; + +export class ProjectsRepo { + constructor(private readonly db: DbExecutor) {} + + async create(input: { name: string; status: ProjectStatus; owner_id: string }): Promise { + const id = await callFunctionValue(this.db, 'usp_project_create', [ + input.name, input.status, input.owner_id, // order === @p0, @p1, @p2 + ]); + if (!id) throw new Error('usp_project_create returned no id'); + return id; + } + + async getById(projectId: string): Promise { + const rows = await callFunctionRows(this.db, 'fn_project_get', [projectId]); + return rows[0] ?? null; + } + + async list(opts: { owner_id?: string; limit?: number; offset?: number } = {}): Promise { + return callFunctionRows(this.db, 'fn_project_list', [ + opts.owner_id ?? null, opts.limit ?? null, opts.offset ?? null, + ]); + } +} +``` + +Then export it from `src/index.ts` and add the row interface to `database.ts`. + +## Validate the seam — the read test is the point + +Add `test/.test.ts`. Seed via the `usp_` writer, read via each `fn_`, and +parse the **raw** row with the `@app/types` schema using `.strict()`. A renamed or +mistyped column fails here loudly — this is the coverage `tsc` can't give you: + +```ts +import { ProjectSchema } from '@app/types'; +test('fn_project_get returns a row matching ProjectSchema', async ({ db }) => { + const repo = new ProjectsRepo(db); + const id = await repo.create({ name: 'X', status: 'active', owner_id: 'dev' }); + const rows = await db.query(`SELECT * FROM dbo.fn_project_get(@p0)`, [id]); + expect(() => ProjectSchema.strict().parse(rows.rows[0])).not.toThrow(); +}); +``` + +Tests run inside a rolled-back transaction (`test/support/db.ts`) against a real +SQL Server, so nothing commits. The `contract.test.ts` needs **no** per-entity +edits — it auto-checks your new functions have snapshot files. + +## Verify + +```bash +pnpm --filter @app/db typecheck +pnpm migrate && pnpm migrate:status # apply + confirm +# point DATABASE_URL at a THROWAWAY db, e.g. sqlserver://sa:Your_password123@localhost:1433/app_test +pnpm --filter @app/db test +``` + +## Common mistakes + +| Symptom | Cause | +|---|---| +| Runtime error, compiles fine | Repo `values` array order doesn't match the proc's `@p0…@pn`. | +| `contract.test.ts` fails | A `usp_`/`fn_` has no `schema/functions/*.sql` file (or a file has no DB object). | +| Read test fails on `.strict()` | A column was renamed/added/typed differently than the `@app/types` schema. | +| `date` field comes back as a full timestamp | Forgot `CONVERT(varchar(10), col, 23)` in the `fn_`. | +| JSON column is a string, not an object | Forgot to add the column to `JSON_COLUMNS` in `connection.ts`. | +| Migration fails on `CREATE PROCEDURE` | Missing `GO` — the proc isn't the first statement in its batch. | +| INSERT explodes but validation passed | `CHECK` constraint values drifted from the `@app/types` `as const` tuple. | diff --git a/.windsurf/skills/mssql-troubleshoot/SKILL.md b/.windsurf/skills/mssql-troubleshoot/SKILL.md new file mode 100644 index 0000000..09ea914 --- /dev/null +++ b/.windsurf/skills/mssql-troubleshoot/SKILL.md @@ -0,0 +1,151 @@ +--- +name: mssql-troubleshoot +description: "Troubleshoots SQL Server issues in this IIS-flavor app-starter by running diagnostic T-SQL. Use when investigating database problems — migration state, missing/renamed functions, the error log, slow or blocked queries, row counts, or schema drift. Triggers on: debug mssql, diagnose sql server, check db, mssql troubleshoot." +--- + +# SQL Server troubleshooting + +Diagnose data-layer issues by running queries directly against SQL Server. This is +the IIS-flavor counterpart to the Postgres `pg-troubleshoot` skill. + +> **Scope:** IIS flavor — `@app/db` on SQL Server (`mssql`). The connection comes +> from `DATABASE_URL` in the repo-root `.env`, e.g. +> `sqlserver://sa:Your_password123@localhost:1433/app`. + +> **⚠ Naming differs by project.** The template names objects `usp_*`/`fn_*` under +> the `dbo` schema. A real project may instead use **schema-qualified, prefix-less** +> procs across several schemas — e.g. `crd.adjustment_dtl_delete`, +> `dbo._`. The diagnostic queries below filter on `dbo` and the +> `usp_`/`fn_` prefixes; **drop those filters when your project doesn't use them** +> (see the "objects present" query for a prefix-less, all-schema variant). + +## Connecting with sqlcmd + +`DATABASE_URL` is a `sqlserver://user:pass@host:port/db` URL — `sqlcmd` needs the +parts split out. `-C` trusts a self-signed dev cert; `-d` selects the database: + +```bash +# From sqlserver://sa:Your_password123@localhost:1433/app_test +sqlcmd -C -S localhost,1433 -U sa -P 'Your_password123' -d app_test -Q "SELECT 1" +``` + +Multi-line queries via a heredoc: + +```bash +sqlcmd -C -S localhost,1433 -U sa -P 'Your_password123' -d app_test <<'SQL' +SELECT TOP 5 * FROM dbo.tasks ORDER BY _created_at DESC; +SQL +``` + +> Point at the right database. Use a dev / test DB; never run diagnostics against +> a production `DATABASE_URL` unless that is explicitly intended. + +## Permission model + +**Autonomous (read-only):** `SELECT`, `SET SHOWPLAN_*` / execution-plan inspection, +and `fn_*` read functions (`SELECT * FROM dbo.fn_error_list(...)`). + +**Ask the user first:** `INSERT`/`UPDATE`/`DELETE` (including most `usp_*` writes), +`CREATE`/`ALTER`/`DROP`, `TRUNCATE`, `GRANT`/`REVOKE`. When asking, show (1) the +exact SQL, (2) what it changes, (3) why. **Schema changes belong in a migration** +(`packages/db/src/migrations/NNN_*.sql` + a matching `schema/` snapshot), not an +ad-hoc `ALTER` — use this skill to *diagnose*, then fix via the migration flow. + +## Diagnostic queries + +### Migration ledger +```sql +SELECT name, executed_at FROM dbo.app_migrations ORDER BY name; +``` +Compare against files in `packages/db/src/migrations/` to spot a migration applied +on one environment but not another. (`pnpm migrate:status` prints the same.) + +### Functions / procedures present (the functions-only access layer) +```sql +SELECT o.name, o.type_desc +FROM sys.objects o +WHERE o.schema_id = SCHEMA_ID('dbo') + AND o.type IN ('P', 'FN', 'IF', 'TF') -- P=proc, IF=inline TVF, FN/TF=other fns + AND (o.name LIKE 'fn[_]%' OR o.name LIKE 'usp[_]%') +ORDER BY o.name; +``` +This is exactly what `contract.test.ts` checks against `schema/functions/`. A +mismatch here = stale snapshot or missing/renamed migration object. + +If your project uses **prefix-less, multi-schema** names (`crd._`, +`dbo._`), list everything across schemas instead and read the +schema name explicitly: +```sql +SELECT s.name AS [schema], o.name, o.type_desc +FROM sys.objects o +JOIN sys.schemas s ON s.schema_id = o.schema_id +WHERE o.type IN ('P', 'FN', 'IF', 'TF') -- procs + functions, any schema +ORDER BY s.name, o.name; +``` + +### Recent application errors (the first-class error log) +```sql +SELECT * FROM dbo.fn_error_list(NULL, NULL, NULL, 50, NULL); -- level, source, since, limit, offset +-- or straight from the table if the function shape is in doubt: +SELECT TOP 50 * FROM dbo._meta_errors ORDER BY occurred_at DESC; +``` + +### A function's parameters / columns (debugging a contract mismatch) +```sql +-- parameters of a proc/function +SELECT p.name, TYPE_NAME(p.user_type_id) AS type, p.max_length +FROM sys.parameters p WHERE p.object_id = OBJECT_ID('dbo.usp_task_update') ORDER BY p.parameter_id; +-- columns a TVF returns +SELECT c.name, TYPE_NAME(c.user_type_id) AS type +FROM sys.columns c WHERE c.object_id = OBJECT_ID('dbo.fn_task_get'); +``` + +### Active / long-running queries +```sql +SELECT r.session_id, r.status, r.command, + DATEDIFF(SECOND, r.start_time, SYSUTCDATETIME()) AS running_secs, + t.text AS sql_text +FROM sys.dm_exec_requests r +CROSS APPLY sys.dm_exec_sql_text(r.sql_handle) t +WHERE r.session_id <> @@SPID +ORDER BY running_secs DESC; +``` + +### Blocking / locks +```sql +SELECT w.session_id AS blocked, w.blocking_session_id AS blocker, + w.wait_type, w.wait_duration_ms, t.text AS blocked_sql +FROM sys.dm_os_waiting_tasks w +CROSS APPLY sys.dm_exec_sql_text( + (SELECT sql_handle FROM sys.dm_exec_requests r WHERE r.session_id = w.session_id) +) t +WHERE w.blocking_session_id IS NOT NULL; +``` + +### Table sizes / row counts +```sql +-- row counts for the core tables +SELECT 'users' AS t, COUNT(*) c FROM dbo.users +UNION ALL SELECT 'tasks', COUNT(*) FROM dbo.tasks +UNION ALL SELECT '_meta_errors', COUNT(*) FROM dbo._meta_errors; + +-- size by table +SELECT t.name, SUM(p.rows) AS rows, + SUM(a.total_pages) * 8 / 1024 AS total_mb +FROM sys.tables t +JOIN sys.partitions p ON p.object_id = t.object_id AND p.index_id IN (0,1) +JOIN sys.allocation_units a ON a.container_id = p.partition_id +GROUP BY t.name ORDER BY total_mb DESC; +``` + +## Workflow + +1. **Identify the symptom** — slow? broken? inconsistent? missing object? +2. **Run diagnostic SELECTs** — find the affected state (ledger, functions, error + log, locks). +3. **Root cause** — why did it happen? +4. **Propose the fix** — schema/data changes get drafted as a **migration** (+ a + `schema/` snapshot update) and presented for approval; never ad-hoc `ALTER`. +5. **Apply** — schema via `pnpm migrate`; data fixes only after user approval. +6. **Verify** — re-run the diagnostics; for schema, `pnpm migrate:status` and + `pnpm --filter @app/db test` (contract + read tests). diff --git a/.windsurf/skills/pr-workflow/SKILL.md b/.windsurf/skills/pr-workflow/SKILL.md new file mode 100644 index 0000000..9027a00 --- /dev/null +++ b/.windsurf/skills/pr-workflow/SKILL.md @@ -0,0 +1,100 @@ +--- +name: pr-workflow +description: Use when committing, branching, or opening a pull request in this app-starter repo. Covers the branching model (feature → dev → main), conventional-commit messages, the PR template checklist mapped to the project's critical rules, and what CI enforces so a PR passes on the first try. Use it before creating a branch, writing a commit, or opening a PR. +--- + +# Commit & PR workflow + +How to land a change cleanly in this repo. The goal is a PR that passes CI and +review on the first pass: right branch, conventional commit, checklist honored. + +## Branching model: feature → dev → main + +``` +feature/ → dev → main +``` + +- Cut your working branch from the latest `dev` (or `main` if `dev` doesn't exist + yet): `feat/...`, `fix/...`, `chore/...`, `docs/...`. +- **Never commit directly to `main`.** Open a PR. +- A PR **to `main` must come from `dev`** — the `pr-policy` workflow enforces this + once a `dev` branch exists (it skips itself until then, so early feature→main + PRs aren't blocked). Day-to-day feature work targets `dev`. +- Don't force-push shared branches; don't skip hooks (`--no-verify`) or signing. + +## Commits — conventional, present tense + +Format: `type: summary` (`feat`, `fix`, `chore`, `docs`, `refactor`, `test`). +Imperative mood, no trailing period in the subject. Reference issues in the body +(`Refs #NN` / `Closes #NN`). Prefer a new commit over amending a pushed one. + +``` +feat: add projects CRUD across types, db, api, and frontend + +Mirrors the tasks slice. Closes #57. +``` + +Match the surrounding history (`git log --oneline`) for scope conventions. + +## Before you push — run the local checks + +CI runs typecheck → lint → build → DB tests. Run the same locally so you don't +round-trip a red PR (see the `verify-before-done` skill): + +```bash +pnpm typecheck && pnpm lint && pnpm build +# DB tests need a throwaway SQL Server in DATABASE_URL: +pnpm test +``` + +## The PR — fill the template, honor the checklist + +The PR body uses `.github/pull_request_template.md`. Its checklist mirrors the +project's critical rules — actually verify each before ticking: + +- [ ] DB access stays **functions-only** (`fn_*` reads, `usp_*` writes) — no + ORM / query-builder table access. +- [ ] Any migration change updates the **schema snapshot** in + `packages/db/schema/{tables,functions}/` in the **same commit**. +- [ ] Shared request/response shapes live in `@app/types` and validate both API + and frontend (no re-declared schemas). +- [ ] If `FEATURE_WORKFLOW.md` changed, `FEATURE_WORKFLOW.html` changed too + (CI enforces the pair). +- [ ] No incomplete UI (no half-wired columns, fields, or buttons). +- [ ] Real defects go through the **error-logging flow** — not swallowed. +- [ ] `pnpm typecheck && pnpm lint && pnpm build && pnpm test` pass locally. + +Write a one or two sentence **What & why**, link the issue, and note anything +non-obvious (trade-offs, manual test steps) under **Notes for reviewers**. + +## What CI checks (IIS flavor — `ci.yml`) + +On every PR and push to `main`, against a throwaway SQL Server service container: + +1. `pnpm install --frozen-lockfile` +2. `pnpm typecheck` +3. `pnpm lint` +4. `pnpm build` +5. create the `app_test` database, run `pnpm --filter @app/db migrate:latest` +6. `pnpm --filter @app/db test` (read-validation + CRUD + the schema-snapshot + contract test) + +If the snapshot contract test fails, you changed a `usp_`/`fn_` without updating +its `schema/functions/*.sql` file (or vice versa) — fix that in the same commit. + +## Creating the PR (gh CLI) + +```bash +git push -u origin feat/ +gh pr create --base dev --title "feat: ..." --body-file - <<'EOF' +## What & why +... +EOF +``` + +(Use `--base main` only for the `dev → main` promotion PR.) + +> **CODEOWNERS** is shipped fully commented-out on purpose — an active rule with a +> non-existent owner hard-blocks every PR once "require code owner review" is on. +> If you enable owners, replace the placeholder handles with a real team/user +> first, then uncomment. diff --git a/.windsurf/skills/types-contract/SKILL.md b/.windsurf/skills/types-contract/SKILL.md new file mode 100644 index 0000000..7d70eea --- /dev/null +++ b/.windsurf/skills/types-contract/SKILL.md @@ -0,0 +1,109 @@ +--- +name: types-contract +description: Use when adding or changing a shared model, zod schema, enum/vocabulary, or request/response shape in @app/types — or when you're tempted to declare a validation schema in the frontend or API. Enforces the single-source-of-truth contract: the zod schema is authored once in @app/types, the TypeScript type is z.infer-ed from it, enum tuples are `as const`, and both the API and frontend import the same schema so they cannot drift. +--- + +# The @app/types contract + +`@app/types` is the one place request/response shapes and models are defined. The +DB row shape, the API validation, and the frontend forms all import from here, so +they cannot disagree. The failure this prevents: a schema re-spelled in two places +that silently drift until a 422 looks correct in Postman but breaks the form. + +> **Scope:** both flavors share `@app/types`. One file per domain +> (`tasks.ts`, `issues.ts`, …) re-exported from `index.ts`. + +## The three rules + +1. **Schema first, type inferred.** Author the zod schema; derive the TS type with + `z.infer`. Never write a hand-maintained `interface` alongside a schema for the + same shape — they will drift. The schema is the source of truth. +2. **One schema, imported everywhere.** The API route's `validate(...)` and the + frontend's `` use the *same* exported schema. Never + redeclare a "form schema" in a component or a "body schema" in a route. +3. **Vocabulary tuples are `as const`.** Enum-shaped values are one + `export const X = [...] as const` tuple that backs the DB `CHECK` constraint, + the zod enum, and the frontend `` options — one source for all + three. + +## The shape of a domain file + +Mirror `packages/types/src/tasks.ts`: + +```ts +import { z } from 'zod'; + +// 1) Vocabulary — one tuple, reused by DB CHECK + zod + UI options. +export const PROJECT_STATUSES = ['active', 'archived'] as const; +export type ProjectStatus = (typeof PROJECT_STATUSES)[number]; +export const ProjectStatusSchema = z.enum(PROJECT_STATUSES); + +// 2) Row schema — what fn_*_get / fn_*_list return. Type is INFERRED from it. +// Timestamps/dates are z.string() (the DB layer delivers them as strings, +// not Date — see the connection-layer normalization). +export const ProjectSchema = z.object({ + project_id: z.string().uuid(), + name: z.string(), + status: ProjectStatusSchema, + _created_at: z.string(), + _updated_at: z.string(), + _owner_id: z.string(), +}); +export type Project = z.infer; + +// 3) Create / Update input schemas + inferred types. +export const ProjectCreateSchema = z.object({ + name: z.string().min(1, 'Required').max(500, 'Too long'), + status: ProjectStatusSchema, +}); +export type ProjectCreateInput = z.infer; + +// Update = all optional, but keep each field's constraints (don't .partial() +// away the enum), and refine to reject an empty patch. +export const ProjectUpdateSchema = z + .object({ + name: z.string().min(1).max(500).optional(), + status: ProjectStatusSchema.optional(), + }) + .refine((v) => Object.keys(v).length > 0, { message: 'No fields to update' }); +export type ProjectUpdateInput = z.infer; +``` + +Then re-export from `index.ts`: + +```ts +export * from './projects.js'; +``` + +## How the contract is consumed (don't bypass) + +- **DB:** the row schema is parsed against real `fn_*` output in the data-layer + read test with `.strict()` — a column drift fails the test. The `CHECK` + constraint values must equal the `as const` tuple. +- **API:** `validate(ProjectCreateSchema, 'body')` middleware enforces the input + at the boundary; bad bodies → 422 before the handler runs. +- **Frontend:** `` validates the same shape; + `` is fed from `PROJECT_STATUSES`. + +## Notes & gotchas + +- **Timestamps/dates are strings, not `.datetime()`.** The DB layer hands back ISO + strings (and `YYYY-MM-DD` for `date` columns); validate as `z.string()`. Using + `.datetime()` would reject Postgres' non-`T` timestamp text in the other flavor. +- **`_owner_id` is a free-form `z.string()`** (not a uuid/FK) — it holds the dev + sentinel, a local user id, or (aws) a Cognito sub. +- **Create input uses `z.coerce.date()`** for date fields so the form's `Date` + serializes correctly; the row schema still reads them back as strings. +- **Don't `.partial()` the create schema** to make the update schema — it drops + per-field constraints (e.g. the enum) when a value *is* provided. Spell the + update schema out. + +## Verify + +```bash +pnpm --filter @app/types typecheck +pnpm --filter @app/types build # @app/types is consumed as a built package +``` + +A drift between the schema and the DB surfaces in the data-layer read test +(`pnpm test`), not in `tsc` — that test is the contract's enforcement. diff --git a/.windsurf/skills/verify-before-done/SKILL.md b/.windsurf/skills/verify-before-done/SKILL.md new file mode 100644 index 0000000..43a68f0 --- /dev/null +++ b/.windsurf/skills/verify-before-done/SKILL.md @@ -0,0 +1,76 @@ +--- +name: verify-before-done +description: Use before claiming any change is done, before opening a PR, or when asked to verify/test a change in this IIS-flavor app-starter. Defines the exact local check sequence (typecheck, lint, build, DB integration tests against a throwaway SQL Server) and the manual browser walk for UI changes, and forbids declaring success without running them. Mirrors what CI enforces, so a green local run means a green PR. +--- + +# Verify before saying "done" + +Do not report a change as complete, working, or fixed until these checks pass. +"It should work" / "this looks right" is not done. CI runs the same sequence — a +clean local run is the cheapest way to a green PR. If a step fails, fix the cause; +don't skip it and don't claim partial success as success. + +> **Scope:** IIS flavor (SQL Server, no Docker, no AWS infra). Commands below are +> the post-`configure --flavor=iis` scripts. + +## The static stack — always run, in order + +```bash +pnpm typecheck # @app/types, @app/db, @app/api, @app/frontend — strict, no `any` +pnpm lint # eslint across the repo +pnpm build # bundler-level errors tsc misses +``` + +Run them top to bottom and stop at the first failure — later steps often cascade +from an earlier one. `typecheck` is where a dangling import or a missing barrel +export after add/remove work shows up. + +## The DB integration tests — the high-value coverage + +The data layer is the primary test surface (the SQL↔TS seam). These need a **real +SQL Server** pointed at a **throwaway database** — never dev or prod data: + +```bash +# DATABASE_URL must point at a disposable db (CI uses app_test): +# sqlserver://sa:Your_password123@localhost:1433/app_test +pnpm test +``` + +Each test runs in a transaction that is rolled back, so the DB stays clean. If +there is no SQL Server reachable, say so explicitly — do not silently skip the +tests and call the change verified. (This host has no Docker; the DB must be an +existing/remote SQL Server set in `DATABASE_URL`.) + +## Manual verification — required for UI / behavior changes + +The suite intentionally does **not** cover React glue. For any frontend or +end-to-end behavior change, exercising it IS the test: + +```bash +pnpm dev:api # one terminal — Express under :3001 (talks to DATABASE_URL) +pnpm dev:frontend # another — Vite SPA +``` + +Walk the **golden path** for the feature (list → new → edit → delete), then a +couple of edge cases: + +- a **validation failure** (submit a bad form) — expect a 422 surfaced inline, not + a crash; +- a **forced 500** (e.g. hit a route with the DB down, or trigger a server error) — + then open **`/errors`** and confirm it was logged. If a 5xx does *not* appear on + `/errors`, the error flow is broken (something swallowed it) — that's a defect, + not "done." + +## Done checklist + +Before you say done / open the PR, confirm every box: + +- [ ] `pnpm typecheck` clean +- [ ] `pnpm lint` clean +- [ ] `pnpm build` clean +- [ ] `pnpm test` clean against a throwaway SQL Server (or explicitly state it couldn't run, and why) +- [ ] For UI/behavior changes: walked the golden path + a validation error + a forced 500, and confirmed the 500 landed on `/errors` +- [ ] No incomplete UI left behind (no half-wired columns/fields/buttons) + +State the actual results — paste failing output if anything failed. If you skipped +a step, say which and why. Don't round "mostly passing" up to "done."