diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8f4e54b..2215f2bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,26 @@ on: push: branches: [main] +# Cancel a still-running CI run for this branch/PR when a new commit lands +# on it, instead of letting both run to completion. Without this, rapid +# successive pushes spawn overlapping e2e runs that all target the SAME +# live E2E Supabase project (not per-run isolated) — two runs' global-setup/ +# global-teardown can race and delete [E2E]-prefixed seed rows out from +# under each other's still-in-progress tests, surfacing as unexplained +# "did not find some options" / element-not-found flakes that have nothing +# to do with the actual code under test. +# +# Not scoped to github.ref — github.ref differs between a pull_request event +# (refs/pull//merge) and a push event for the same branch/commit +# (refs/heads/), so a per-ref group lets a push run and a +# pull_request run for the same change execute concurrently, which is +# exactly the race this block exists to prevent. A single group for the +# whole workflow serializes every run (across all branches and PRs) that +# touches the shared E2E Supabase project. +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: true + jobs: checks: runs-on: ubuntu-latest @@ -96,3 +116,28 @@ jobs: name: playwright-report path: playwright-report/ retention-days: 14 + + # ── Database invariants (structural enforcement Tier 3) ────────────────── + # Checks the live schema for what no code-side check can see: RLS enabled + # on every public table, no unexpected policy-less (deny-all) tables, a + # covering index on every FK column, zero anon table grants. Runs against + # the DEDICATED E2E project (same secrets as the e2e job — CI never holds + # production credentials); both projects receive every migration, so + # schema invariants verified there hold for production by construction. + # Self-disarming like the e2e job: secrets absent → warning annotation, + # job passes. See scripts/check-db-invariants.mjs for the check details. + db-invariants: + runs-on: ubuntu-latest + env: + NEXT_PUBLIC_SUPABASE_URL: ${{ secrets.NEXT_PUBLIC_SUPABASE_URL }} + SUPABASE_SERVICE_ROLE_KEY: ${{ secrets.SUPABASE_SERVICE_ROLE_KEY }} + steps: + - uses: actions/checkout@v4 + with: + # This job only reads the repo and runs one script — no reason to + # leave the checkout token on disk. + persist-credentials: false + - uses: actions/setup-node@v4 + with: + node-version: 22 + - run: node scripts/check-db-invariants.mjs diff --git a/CLAUDE.md b/CLAUDE.md index ae3ccdf2..22729937 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1057,8 +1057,8 @@ item below" as part of the definition of done for any non-trivial change. ## Structural Enforcement — Guardrails Conventions in this file are enforced in code wherever they can be, so -following them stops being a memory test. Three layers, checked in CI via -`npm run lint` and `vitest run`: +following them stops being a memory test. Four layers, checked in CI via +`npm run lint` and `vitest run` (plus the `db-invariants` CI job for layer 4): 1. **ESLint rules** (`eslint.config.mjs`, the "Structural enforcement" config block) — AST-level bans scoped to `app/`, `lib/`, `components/`: @@ -1087,6 +1087,16 @@ following them stops being a memory test. Three layers, checked in CI via 3. **`check:ui-classes`** — the raw `btn-*`/`badge-*`/`card` class grep. +4. **DB invariant gate** (`scripts/check-db-invariants.mjs`, CI + `db-invariants` job) — the live-schema invariants no code-side check can + see, via `public.db_invariant_report()` against the dedicated E2E + project: RLS enabled on every public table, no policy-less (deny-all) + tables outside the script's shrink-only `SERVICE_ROLE_ONLY_TABLES` + allowlist, a covering index on every FK column, and zero `anon` table + grants (all revoked 2026-07-24 — no client reads tables + unauthenticated). Self-disarms with a warning when the E2E secrets are + absent, same as the e2e job. + **The meta-rule: a new convention ships WITH its guardrail.** If a rule is worth adding to this file, add its ESLint rule or `unit/guardrails/` test in the same PR — and the CLAUDE.md prose for mechanically-checkable rules diff --git a/app/(dashboard)/maintenance/CreateWorkOrderModal.tsx b/app/(dashboard)/maintenance/CreateWorkOrderModal.tsx index e21143e5..c0fe793c 100644 --- a/app/(dashboard)/maintenance/CreateWorkOrderModal.tsx +++ b/app/(dashboard)/maintenance/CreateWorkOrderModal.tsx @@ -30,6 +30,7 @@ export function CreateWorkOrderModal({ vendorCompliance = [], orgId = '', onClose, + onSuccess, onWarning, }: Readonly<{ properties: PropertyOptionWithCoords[] @@ -39,6 +40,7 @@ export function CreateWorkOrderModal({ vendorCompliance?: VendorComplianceRow[] orgId?: string onClose: () => void + onSuccess?: () => void onWarning?: (msg: string) => void }>) { const [state, action, pending] = useActionState(createWorkOrder, null) @@ -77,6 +79,13 @@ export function CreateWorkOrderModal({ useEffect(() => { if (!state?.success || !state.workOrderId) return + // revalidatePath() in the createWorkOrder Server Action refreshes the + // Server Component's data on the NEXT navigation, but this modal closes + // without one — the parent board's props otherwise go stale until some + // other navigation happens to trigger a refetch. router.refresh() (via + // onSuccess, wired in maintenance-board.tsx) forces that refetch now. + onSuccess?.() + if (state.warning) onWarning?.(state.warning) // No photos attached — close immediately diff --git a/app/(dashboard)/maintenance/maintenance-board.tsx b/app/(dashboard)/maintenance/maintenance-board.tsx index ff4bb144..b846efd3 100644 --- a/app/(dashboard)/maintenance/maintenance-board.tsx +++ b/app/(dashboard)/maintenance/maintenance-board.tsx @@ -1,7 +1,7 @@ 'use client' import { useState, useTransition, useEffect } from 'react' -import { useSearchParams } from 'next/navigation' +import { useRouter, useSearchParams } from 'next/navigation' import { Plus, ChevronDown, X, Wrench, Calendar, DollarSign, User, ChevronRight, AlertTriangle, CheckCircle2, Clock, @@ -855,6 +855,7 @@ export function MaintenanceBoard({ orgId?: string role: string }) { + const router = useRouter() const searchParams = useSearchParams() const urlFilter = searchParams.get('filter') @@ -1240,6 +1241,7 @@ export function MaintenanceBoard({ vendorCompliance={vendorCompliance} orgId={orgId} onClose={() => setShowCreate(false)} + onSuccess={() => router.refresh()} onWarning={setWarning} /> )} diff --git a/app/(dashboard)/maintenance/page.tsx b/app/(dashboard)/maintenance/page.tsx index 386d2fae..c7caec0c 100644 --- a/app/(dashboard)/maintenance/page.tsx +++ b/app/(dashboard)/maintenance/page.tsx @@ -8,13 +8,13 @@ export default async function MaintenancePage() { const { supabase, membership } = await requireOrgMember() const [ - { data: workOrders }, - { data: properties }, - { data: vendors }, - { data: schedules }, - { data: crewMembers }, - { data: propertyAssets }, - { data: vendorCompliance }, + workOrdersResult, + propertiesResult, + vendorsResult, + schedulesResult, + crewMembersResult, + propertyAssetsResult, + vendorComplianceResult, ] = await Promise.all([ supabase .from('work_orders') @@ -89,15 +89,28 @@ export default async function MaintenancePage() { .eq('org_id', membership.org_id), ]) + // A query erroring (bad filter value, RLS misconfiguration, etc.) and a + // query legitimately returning zero rows both leave `data` empty — `?? []` + // below can't tell them apart, so without this the board just silently + // renders as if nothing exists instead of surfacing a real outage. + const results = [ + ['work_orders', workOrdersResult], ['properties', propertiesResult], ['vendors', vendorsResult], + ['maintenance_schedules', schedulesResult], ['crew_members', crewMembersResult], + ['property_assets', propertyAssetsResult], ['vendor_compliance_status', vendorComplianceResult], + ] as const + for (const [name, result] of results) { + if (result.error) console.error(`[MaintenancePage] ${name} query failed:`, result.error) + } + return ( diff --git a/app/(dashboard)/turnovers/turnover-board.tsx b/app/(dashboard)/turnovers/turnover-board.tsx index 9b473391..8c7afd9b 100644 --- a/app/(dashboard)/turnovers/turnover-board.tsx +++ b/app/(dashboard)/turnovers/turnover-board.tsx @@ -262,7 +262,7 @@ function CrewAssignment({ onClick={() => handleAdd(c.id)} className="w-full text-left px-3 py-2 text-sm hover:bg-canvas-themed transition-colors flex items-center gap-2" > - + {c.name} @@ -353,6 +353,7 @@ function TurnoverCard({ return (
' }`. No row contents, no PII, no phone numbers, no + org ids. The client treats any broadcast as "go pull". +5. **Triggers must never break the underlying write.** Every + `realtime.send()` call is wrapped in its own exception handler. +6. **Crew client components never read Supabase directly** — Dexie only. + Writes only via `enqueueMutation()`. +7. `SMS_ENABLED` stays untouched. Nothing in this work touches SMS. + +### Supabase projects + +| | Project ref | Use | +|---|---|---| +| Production | `vpmznjktllhmmbfnxuvk` | Apply every migration here | +| E2E/CI | `syhthijeqlnltufdawyb` (fieldstay-e2e) | Apply every migration here too (schema parity for CI) | + +The Supabase account has **unrelated projects (Rootstock-vercel, +trade-suite-pro)** — never touch them. Apply migrations with the Supabase +MCP `apply_migration` tool (name = the migration filename's version + +description), and commit the identical SQL file to +`supabase/migrations/` in the same PR. + +### Repo working rules (summary — CLAUDE.md is authoritative) + +- Verification pass before every commit: + `npx tsc --noEmit && npm run lint && npx vitest run && npm run check:ui-classes` +- Migration filenames: `YYYYMMDDHHMMSS_description.sql`, version prefix must + be unique (the `migration-hygiene` guardrail test fails the build + otherwise). All DDL idempotent (`CREATE OR REPLACE`, `DROP ... IF EXISTS`). +- A migration that adds/changes columns updates `types/database.ts` in the + same commit (Phases 2–5 add no columns, so this shouldn't trigger). +- `Math.random()` is ESLint-banned; the two legitimate uses in this work + (reconnect jitter, backoff jitter) each need + `// eslint-disable-next-line no-restricted-properties` with a one-line + justification, matching the existing sampling/jitter sites. +- New conventions ship WITH a guardrail (ESLint rule or `unit/guardrails/` + test) in the same PR — see Phase 5. + +--- + +## 1. Entity → signal mapping (shared vocabulary for Phases 2 and 3) + +The broadcast payload's `entity` value must match what the client switches +on. Exactly three values exist: + +| `entity` value | Fired by changes to | Client reaction (Phase 3) | +|---|---|---| +| `turnovers` | `turnover_assignments`, `turnovers` | Full turnover scope pull (assignments reconciliation + turnover rows + checklists for fresh turnovers) | +| `checklists` | `checklist_instances`, `checklist_instance_items` | Checklist delta pull across the current assigned-turnover set | +| `work_orders` | `work_orders` | Work-order snapshot reconciliation + delta | + +`property_assets` deliberately has **no trigger**: crew-facing asset data is +low-churn, the property→crew fan-out join is wide, and the Phase 3 safety +poll (≤5 min staleness) plus the turnover-signal refresh covers it. If that +freshness ever becomes insufficient, add a sixth trigger later — don't do it +now. + +--- + +## 2. Phase 2 — Broadcast migration (deploys dark) + +**Goal:** all database-side broadcast infrastructure, live in production but +invisible — no client subscribes to these topics until Phase 3, so this +phase has zero user-facing risk and does not need to wait for any soak +window. + +**Deliverable:** one migration file +`supabase/migrations/_crew_sync_broadcast_triggers.sql` +(pick the current UTC timestamp; verify the version prefix is unique in the +directory), applied to **both** Supabase projects, plus verification +evidence. + +### 2a. Design constraints (why the SQL looks the way it does) + +- **Statement-level AFTER triggers with transition tables** — one trigger + invocation per statement regardless of row count (a bulk checklist + instantiation of 60 items = one broadcast, not 60). PostgreSQL does not + allow transition tables on multi-event triggers, so each table gets one + trigger **per event** (INSERT/UPDATE/DELETE as needed), all sharing one + trigger function that branches on `TG_OP`. +- **`SECURITY DEFINER` + `SET search_path = ''`** on every function: the + functions must read join tables (`crew_members`, `turnover_assignments`, + `checklist_instances`) without being filtered by the calling role's RLS, + and a pinned empty search_path (with fully schema-qualified references) + is the Supabase-advisor-clean way to write definer functions. +- **Per-user exception-safe send loop**: a `realtime.send()` failure must + log a warning and continue — it must never abort the transaction that + performed the actual write. +- **No DELETE trigger on `turnovers` or checklist tables**: deleting a + turnover cascade-deletes its `turnover_assignments` rows, and cascaded + deletes fire the child table's own statement trigger — so the + `turnover_assignments` DELETE trigger already signals the affected crew. + Checklist deletes likewise cascade from turnovers. A *standalone* + checklist deletion (a PM deleting a checklist row without its turnover — + permitted by the PM RLS policies) intentionally gets no trigger either: + deletion correctness is reconciliation's job by invariant #1, so the only + cost is signal latency, bounded by the next `turnovers` broadcast or the + 5-minute safety poll. That freshness tradeoff is accepted — do not add + checklist DELETE triggers for it. +- **UPDATE triggers notify old AND new parties** where a row can be + re-pointed (`turnover_assignments.crew_member_id`, + `work_orders.assigned_crew_member_id`) — a reassigned crew member must be + told the item left their scope, not just the new assignee told it + arrived. +- **`crew_members.user_id` is nullable** (some crew have no auth account) — + always filter `user_id IS NOT NULL`. + +### 2b. The migration SQL + +Use this SQL as written (re-verify column names against +`types/database.ts` / the live schema before applying — key joins: +`turnover_assignments(turnover_id, crew_member_id)`, +`crew_members(id, user_id)`, `checklist_instances(id, turnover_id)`, +`checklist_instance_items(id, instance_id)`, +`work_orders(id, assigned_crew_member_id)`): + +```sql +-- Crew Sync v2 Phase 2: broadcast wake-up signals for the crew PWA. +-- Statement-level triggers call realtime.send() on topic 'crew:{user_id}' +-- with a minimal {entity} payload. Signal-only: no row data, no PII. +-- Deploys dark — no client subscribes until the Phase 3 cutover. + +-- ── Shared send helper ────────────────────────────────────────────────── +CREATE OR REPLACE FUNCTION public.notify_crew_sync(p_user_ids uuid[], p_entity text) +RETURNS void +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_id uuid; +BEGIN + IF p_user_ids IS NULL THEN + RETURN; + END IF; + FOR v_user_id IN SELECT DISTINCT u FROM unnest(p_user_ids) AS u WHERE u IS NOT NULL + LOOP + BEGIN + PERFORM realtime.send( + jsonb_build_object('entity', p_entity), -- payload: signal only, never row data + 'sync', -- event + 'crew:' || v_user_id::text, -- topic + true -- private channel + ); + EXCEPTION WHEN OTHERS THEN + -- A broadcast failure must never break the write that triggered it. + RAISE WARNING 'notify_crew_sync: send failed for user % (%): %', + v_user_id, p_entity, SQLERRM; + END; + END LOOP; +END; +$$; + +-- Not callable by clients — trigger-context only. An authenticated user +-- must not be able to spam arbitrary crew topics through this definer fn. +REVOKE EXECUTE ON FUNCTION public.notify_crew_sync(uuid[], text) FROM PUBLIC, anon, authenticated; + +-- ── turnover_assignments → 'turnovers' ───────────────────────────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_turnover_assignments() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + IF TG_OP = 'INSERT' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.crew_members cm ON cm.id = r.crew_member_id + WHERE cm.user_id IS NOT NULL; + ELSIF TG_OP = 'DELETE' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM old_rows r + JOIN public.crew_members cm ON cm.id = r.crew_member_id + WHERE cm.user_id IS NOT NULL; + ELSE -- UPDATE: notify both the previous and the new crew member + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM ( + SELECT crew_member_id FROM new_rows + UNION + SELECT crew_member_id FROM old_rows + ) r + JOIN public.crew_members cm ON cm.id = r.crew_member_id + WHERE cm.user_id IS NOT NULL; + END IF; + + PERFORM public.notify_crew_sync(v_user_ids, 'turnovers'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_turnover_assignments_ins ON public.turnover_assignments; +CREATE TRIGGER crew_sync_turnover_assignments_ins + AFTER INSERT ON public.turnover_assignments + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnover_assignments(); + +DROP TRIGGER IF EXISTS crew_sync_turnover_assignments_upd ON public.turnover_assignments; +CREATE TRIGGER crew_sync_turnover_assignments_upd + AFTER UPDATE ON public.turnover_assignments + REFERENCING OLD TABLE AS old_rows NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnover_assignments(); + +DROP TRIGGER IF EXISTS crew_sync_turnover_assignments_del ON public.turnover_assignments; +CREATE TRIGGER crew_sync_turnover_assignments_del + AFTER DELETE ON public.turnover_assignments + REFERENCING OLD TABLE AS old_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnover_assignments(); + +-- ── turnovers (UPDATE only) → 'turnovers' ────────────────────────────── +-- INSERT is pointless (a brand-new turnover has no assignments yet — the +-- assignment INSERT is the signal). DELETE is covered by the FK cascade +-- firing crew_sync_turnover_assignments_del. +CREATE OR REPLACE FUNCTION public.crew_sync_on_turnovers() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.turnover_assignments ta ON ta.turnover_id = r.id + JOIN public.crew_members cm ON cm.id = ta.crew_member_id + WHERE cm.user_id IS NOT NULL; + + PERFORM public.notify_crew_sync(v_user_ids, 'turnovers'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_turnovers_upd ON public.turnovers; +CREATE TRIGGER crew_sync_turnovers_upd + AFTER UPDATE ON public.turnovers + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnovers(); + +-- ── checklist_instances (INSERT, UPDATE) → 'checklists' ──────────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_checklist_instances() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.turnover_assignments ta ON ta.turnover_id = r.turnover_id + JOIN public.crew_members cm ON cm.id = ta.crew_member_id + WHERE cm.user_id IS NOT NULL; + + PERFORM public.notify_crew_sync(v_user_ids, 'checklists'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_checklist_instances_ins ON public.checklist_instances; +CREATE TRIGGER crew_sync_checklist_instances_ins + AFTER INSERT ON public.checklist_instances + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_instances(); + +DROP TRIGGER IF EXISTS crew_sync_checklist_instances_upd ON public.checklist_instances; +CREATE TRIGGER crew_sync_checklist_instances_upd + AFTER UPDATE ON public.checklist_instances + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_instances(); + +-- ── checklist_instance_items (INSERT, UPDATE) → 'checklists' ─────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_checklist_items() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.checklist_instances ci ON ci.id = r.instance_id + JOIN public.turnover_assignments ta ON ta.turnover_id = ci.turnover_id + JOIN public.crew_members cm ON cm.id = ta.crew_member_id + WHERE cm.user_id IS NOT NULL; + + PERFORM public.notify_crew_sync(v_user_ids, 'checklists'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_checklist_items_ins ON public.checklist_instance_items; +CREATE TRIGGER crew_sync_checklist_items_ins + AFTER INSERT ON public.checklist_instance_items + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_items(); + +DROP TRIGGER IF EXISTS crew_sync_checklist_items_upd ON public.checklist_instance_items; +CREATE TRIGGER crew_sync_checklist_items_upd + AFTER UPDATE ON public.checklist_instance_items + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_items(); + +-- ── work_orders (INSERT, UPDATE, DELETE) → 'work_orders' ─────────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_work_orders() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + IF TG_OP = 'INSERT' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.crew_members cm ON cm.id = r.assigned_crew_member_id + WHERE r.assigned_crew_member_id IS NOT NULL AND cm.user_id IS NOT NULL; + ELSIF TG_OP = 'DELETE' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM old_rows r + JOIN public.crew_members cm ON cm.id = r.assigned_crew_member_id + WHERE r.assigned_crew_member_id IS NOT NULL AND cm.user_id IS NOT NULL; + ELSE -- UPDATE: notify previous and new assignee (covers reassignment) + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM ( + SELECT assigned_crew_member_id FROM new_rows + UNION + SELECT assigned_crew_member_id FROM old_rows + ) r + JOIN public.crew_members cm ON cm.id = r.assigned_crew_member_id + WHERE r.assigned_crew_member_id IS NOT NULL AND cm.user_id IS NOT NULL; + END IF; + + PERFORM public.notify_crew_sync(v_user_ids, 'work_orders'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_work_orders_ins ON public.work_orders; +CREATE TRIGGER crew_sync_work_orders_ins + AFTER INSERT ON public.work_orders + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_work_orders(); + +DROP TRIGGER IF EXISTS crew_sync_work_orders_upd ON public.work_orders; +CREATE TRIGGER crew_sync_work_orders_upd + AFTER UPDATE ON public.work_orders + REFERENCING OLD TABLE AS old_rows NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_work_orders(); + +DROP TRIGGER IF EXISTS crew_sync_work_orders_del ON public.work_orders; +CREATE TRIGGER crew_sync_work_orders_del + AFTER DELETE ON public.work_orders + REFERENCING OLD TABLE AS old_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_work_orders(); + +-- ── Authorize crew clients to receive their own private-topic broadcasts ─ +-- Private Realtime channels authorize against RLS on realtime.messages. +-- A crew user may join exactly one topic: crew:{their own auth.uid()}. +DROP POLICY IF EXISTS "crew_receive_own_sync_broadcasts" ON realtime.messages; +CREATE POLICY "crew_receive_own_sync_broadcasts" + ON realtime.messages + FOR SELECT + TO authenticated + USING ( + realtime.messages.extension = 'broadcast' + AND realtime.topic() = 'crew:' || (SELECT auth.uid())::text + ); +``` + +### 2c. Applying it + +1. Write the file into `supabase/migrations/` with a current-UTC timestamp + name; run the guardrail suite (`npx vitest run unit/guardrails`) to + confirm migration-hygiene passes (no version collision; this file + creates no tables so the RLS-in-same-file rule doesn't bite). +2. Apply to production (`vpmznjktllhmmbfnxuvk`) via Supabase MCP + `apply_migration`, name matching the filename (without `.sql`). +3. Apply the identical SQL to the e2e project (`syhthijeqlnltufdawyb`). +4. Run Supabase MCP `get_advisors` (security) on production afterward — + the definer functions should NOT be flagged (search_path is pinned); if + anything new appears, fix it before merging. + +### 2d. Verification — REQUIRED before Phase 3 starts + +**SQL-level (production):** + +```sql +-- 1. Find a turnover with an assignment whose crew member has a user_id: +SELECT t.id AS turnover_id, cm.user_id +FROM turnovers t +JOIN turnover_assignments ta ON ta.turnover_id = t.id +JOIN crew_members cm ON cm.id = ta.crew_member_id +WHERE cm.user_id IS NOT NULL +LIMIT 1; + +-- 2. Touch it (harmless: bumps updated_at, which the delta layer absorbs): +UPDATE turnovers SET updated_at = now() WHERE id = ''; + +-- 3. Confirm the broadcast row landed: +SELECT topic, event, payload, extension, inserted_at +FROM realtime.messages +ORDER BY inserted_at DESC +LIMIT 5; +-- Expect: topic = 'crew:', event = 'sync', +-- payload = {"entity": "turnovers"}, extension = 'broadcast' +``` + +Repeat the touch test for one `checklist_instance_items` row (expect +`entity = 'checklists'`) and one assigned `work_orders` row (expect +`entity = 'work_orders'`). Also verify the failure-isolation property: +the `UPDATE` statements themselves must succeed regardless of broadcast +outcome. Note `realtime.messages` is partitioned with short retention — +query soon after the touch. + +**Scratch-client (run against the e2e project, not prod):** a throwaway +Node script using `@supabase/supabase-js`: + +1. Sign in as a seeded crew user (e2e project credentials). +2. `await supabase.realtime.setAuth()` then + `supabase.channel('crew:' + user.id, { config: { private: true } })` + `.on('broadcast', { event: 'sync' }, cb).subscribe()` — must reach + `SUBSCRIBED`. +3. Touch an assigned turnover row via SQL → `cb` must fire with + `{ entity: 'turnovers' }` within a couple of seconds. +4. Negative test: subscribe to `crew:` — must FAIL to + subscribe (`CHANNEL_ERROR` / unauthorized). This proves the RLS policy + actually scopes topics per-user. **Do not skip this test.** + +**Definition of done (Phase 2):** migration file merged; applied to both +projects; all three SQL touch tests produce correct `realtime.messages` +rows; scratch client receives own-topic broadcast and is rejected from a +foreign topic; advisors clean; full local verification pass green. + +--- + +## 3. Phase 3 — Client cutover (behind a flag, ships dormant) + +**Goal:** the crew PWA subscribes to its single private broadcast topic and +converts signals into debounced delta pulls — gated behind +`NEXT_PUBLIC_CREW_SYNC_V2` so it ships dormant and the old path remains the +default until Phase 5 flips the flag. + +**Precondition:** Phase 2 verified (section 2d), and Phase 1 has soaked in +production for at least a few real field-days with no sync regressions +(check Sentry for `[DexieProvider]`/sync errors and any crew bug reports +via `crew_feedback`). + +### 3a. Files touched + +- `lib/dexie/context.tsx` — the only substantial change site. +- `.env.example` (if present) — document `NEXT_PUBLIC_CREW_SYNC_V2`. +- `unit/dexie/` — tests for the debouncer and signal→action mapping + (extract both as pure functions so they're testable without a DOM). + +### 3b. Behavior spec + +All inside `DexieProvider`, keyed off +`process.env.NEXT_PUBLIC_CREW_SYNC_V2 === 'true'`: + +**Flag OFF (default):** current behavior, untouched — three +`postgres_changes` channels + generation-token refresh machinery. Do not +refactor it "while you're in there"; it gets deleted wholesale in Phase 5. + +**Flag ON:** + +1. **No `postgres_changes` channels at all.** One channel: + + ```typescript + await supabase.realtime.setAuth() // required before joining private channels + const channel = supabase + .channel(`crew:${userId}`, { config: { private: true } }) + .on('broadcast', { event: 'sync' }, ({ payload }) => { + handleSyncSignal(payload?.entity) + }) + .subscribe(handleChannelStatus) + ``` + + Re-run `setAuth()` on Supabase auth `TOKEN_REFRESHED` events (there is + already an `onAuthStateChange` listener in the provider — extend it). + Check the installed `@supabase/supabase-js` version's private-channel + semantics; newer versions refresh realtime auth automatically, but the + explicit call is harmless and version-proof. + +2. **Signal → action map with a 1 s trailing debounce per entity.** + `handleSyncSignal(entity)` validates entity is one of + `'turnovers' | 'checklists' | 'work_orders'` (ignore anything else) and + schedules the matching refresh through a per-entity debouncer: burst of + N broadcasts inside 1 s → one pull. Actions: + - `turnovers` → the full turnover-scope sync from + `lib/dexie/sync/turnovers.ts` (assignment reconciliation + turnover + delta + checklists for fresh turnovers). This is a full-scope pull, so + cursor advancement is allowed. + - `checklists` → checklist pull across the full current assigned- + turnover id set with `advanceCursors: true` (full scope). Never a + partial-scope pull with cursor advancement. + - `work_orders` → the WO reconciliation+delta sync from + `lib/dexie/sync/work-orders.ts`. + Serialize refreshes per entity (if a pull is in flight when the debounce + fires again, queue exactly one follow-up run — don't stack). + +3. **Safety poll:** every 5 minutes, run the full `resync()` (all entities + + reconciliation). This is the correctness backstop for missed + broadcasts AND the freshness path for `property_assets` (which has no + trigger — see section 1). Also run a full `resync()`: + - on mount (already exists), + - on `online` (already exists), + - on `visibilitychange` → visible (add it — PWA returning from + background has likely missed broadcasts). + +4. **Reconnect with jitter:** on channel status `CHANNEL_ERROR`, + `TIMED_OUT`, or `CLOSED` (when not deliberately unmounting), tear down + and resubscribe after `base + jitter` where jitter is uniform in + [0, 30 s] (e.g. base 5 s → total delay uniform in 5–35 s) — prevents a + thundering herd of rejoins when a Realtime node restarts. `Math.random()` here needs the + eslint-disable + justification line. After every successful + (re)subscribe, run one full `resync()` — the gap while disconnected may + have swallowed signals. + +5. **Keep** the outbox `online` flush and everything in + `lib/dexie/syncService.ts` untouched (Phase 4's concern). + +6. **Do not delete** the generation-token machinery or the old channels in + this phase — the flag must be able to toggle back. Deletion is + Phase 5. + +### 3c. Tests + +Extract and unit-test as pure/injectable pieces (fake timers): +- the per-entity debouncer (burst coalescing; serialized in-flight + + queued-follow-up behavior), +- the entity→action mapping (unknown entity ignored, each known entity + invokes the right sync fn), +- reconnect jitter bounds (delay always within [base−0, base+30 s] window + chosen). + +**Definition of done (Phase 3):** merged with flag defaulting off; full +verification pass green; a manual smoke with the flag on locally +(`NEXT_PUBLIC_CREW_SYNC_V2=true npm run dev` against the e2e project) +showing subscribe + signal → pull in the network tab; old path verified +untouched with flag off. + +--- + +## 4. Phase 4 — Outbox retry backoff + +**Goal:** failed outbox mutations retry on exponential backoff with jitter +instead of hammering on every drain. Independent of Phases 2–3; can be its +own small PR. + +### 4a. Spec + +- `lib/dexie/schema.ts`: bump to `this.version(8)` (current max is 7 — + re-check at implementation time). Add `nextAttemptAt?: number` + (epoch ms) to `MutationRow`. Keep the existing store index string + unchanged unless an index is genuinely needed (it isn't — the drain + scans in insertion order anyway). Follow the existing pattern of + repeating the full `stores({...})` map in the new version block. +- `lib/dexie/syncService.ts` `processOutbox()`: + - Before pushing a mutation: if `mutation.nextAttemptAt` is set and + `> Date.now()`, **stop the drain entirely** (do not skip-and-continue — + later mutations against the same record must never jump ahead; the + existing stop-on-first-error semantics already encode this, backoff + just adds "not due yet" as a stop reason). + - On push failure: + `retryCount += 1`, and + `nextAttemptAt = Date.now() + Math.min(2 ** (retryCount - 1) * 5_000, 300_000) * (0.5 + jitter)` + (the `- 1` because `retryCount` was already incremented — without it + the first retry would wait 10 s, not 5 s) + where `jitter` is uniform in [0, 1) (`Math.random()` with the + eslint-disable justification — spreads retry storms after an outage). + So delays grow 5 s → 10 s → 20 s … capped at 5 min, each scaled by + 0.5–1.5×. + - Preserve the existing `failed: true` permanent-failure classification + exactly as-is. + - When the drain stops on a not-yet-due mutation, schedule a one-shot + `setTimeout` to re-run `processOutbox()` at `nextAttemptAt` (clear any + previously scheduled one first — keep a single timer handle on the + SyncEngine instance). The existing `online` listener and + `enqueueMutation()` fire-and-forget drains remain additional entry + points. + +### 4b. Tests + +In `unit/dexie/` with the existing fake + fake timers: backoff delay math +(growth, cap, jitter bounds), drain stops at a not-yet-due head mutation +and touches nothing behind it, due mutation retries and clears +`nextAttemptAt` on success, permanent-failure path unchanged. + +**Definition of done (Phase 4):** merged; verification pass green; unit +tests cover the four behaviors above. + +--- + +## 5. Phase 5 — Rollout, acceptance, deletion, convention + +**Preconditions:** Phases 2–4 merged; Phase 3 deployed dark to production. + +### 5a. Owner (human) tasks — surface these, don't do them + +- Supabase dashboard → Realtime settings: confirm the concurrent-clients + quota comfortably covers the crew fleet (~1,500 was the discussed + target). +- Set `NEXT_PUBLIC_CREW_SYNC_V2=true` in Vercel — **Preview environment + first**, production only after the acceptance test passes. + +### 5b. Two-device acceptance test (run on Preview with the flag on) + +With one PM session and one crew device (or two crew devices where noted): + +1. PM assigns a turnover to the crew member → appears on the crew device + within ~2 s without a manual refresh. +2. PM unassigns it → disappears from the crew device (broadcast → full + scope pull → reconciliation removes it). +3. Crew device A completes a checklist item → crew device B (same + turnover) shows it within ~2 s. +4. PM assigns a work order → appears; PM reassigns it to another crew + member → vanishes from the first device, appears on the second. +5. Put the crew device offline (airplane mode), make PM-side changes, + reconnect → device catches up via the reconnect resync (within + seconds, not the 5-min poll). +6. Leave a device idle 10+ minutes → confirm the safety poll fires (network + tab) and nothing accumulates errors in the console/Sentry. + +### 5c. Production flip and soak + +Flip the flag in production. Watch for one week: Sentry (crew PWA errors, +channel-subscribe failures), Supabase Realtime dashboard (concurrent +connections, message counts), `crew_feedback` table, and DB CPU (trigger +overhead should be negligible; confirm). + +### 5d. Old-code deletion (only after a green soak week) + +In one PR: remove the flag conditionals (v2 becomes the only path), delete +the three `postgres_changes` channel setups, delete +`refreshChecklistSubscription` / `refreshAssetsSubscription` and the entire +generation-token machinery from `lib/dexie/context.tsx`, delete any tests +that exist solely to cover the deleted machinery, and remove the env var +from Vercel/docs. Keep the safety poll and reconnect-resync forever — they +are load-bearing correctness backstops, not scaffolding. + +### 5e. Convention + guardrail (the CLAUDE.md meta-rule applies) + +Add to `CLAUDE.md` (Dexie/crew section): **"Every Supabase-backed table the +crew PWA caches in Dexie is covered by the safety poll (the full `resync()` +covers all of them); every such table must ALSO either have a broadcast +trigger in the crew-sync trigger migration (low-latency entities) or be +explicitly listed in the `SAFETY_POLL_ONLY` allowlist — new cached tables +must be placed in one of those two sets in the same PR."** (Broadcast- +triggered tables are deliberately covered by both mechanisms — the poll is +the correctness backstop, so this is a union check, not exclusive-or.) + +Per the repo's meta-rule, ship the guardrail with it: a new +`unit/guardrails/crew-sync-coverage.test.ts` that derives the list of +synced Supabase-backed tables from `lib/dexie/schema.ts` (maintain an +explicit exported const if parsing is brittle — e.g. +`CREW_SYNCED_TABLES`), keeps purely-local Dexie tables (`mutations`, +`sync_meta`, and anything else with no Supabase counterpart) in a separate +`LOCAL_ONLY` list, and asserts every remaining table appears in the union +of (a) the trigger migration SQL (grep the +`supabase/migrations/*crew_sync_broadcast*` file for `ON public.`) +and (b) an explicit `SAFETY_POLL_ONLY` allowlist (initially: +`property_assets`, plus any other cached remote table without a trigger — +enumerate them from the schema when writing the test, don't assume this +list is complete). A new cached table then fails CI until the developer +consciously places it. + +**Definition of done (Phase 5 / the whole program):** flag removed, old +path deleted, acceptance test recorded as passed, soak week clean, +CLAUDE.md + guardrail merged. + +--- + +## Appendix A — Enforcement Tier 3 (separate work, unscheduled) + +Not part of crew sync; listed so it isn't lost. DB-level invariant checks +in CI, run against the **e2e project** (never hold prod credentials in CI): + +1. Every `public` table has `rowsecurity = true` and at least one policy + (`pg_tables` / `pg_policies`). +2. Every FK column has a covering index. +3. No unexpected `anon`/`authenticated` grants (diff against a committed + allowlist). +4. `types/database.ts` drift check: generate types from the e2e project + (Supabase MCP `generate_typescript_types` or CLI) and diff the table/ + column shape against the committed file. + +Each check is a script under `scripts/` wired into `.github/workflows/ci.yml`, +self-disarming when the e2e secrets are absent (same pattern the e2e job +already uses). + +## Appendix B — Quick reference + +- Verification pass: `npx tsc --noEmit && npm run lint && npx vitest run && npm run check:ui-classes` +- Prod Supabase: `vpmznjktllhmmbfnxuvk` · E2E: `syhthijeqlnltufdawyb` — never any other project. +- Migrations: apply via Supabase MCP `apply_migration` to BOTH projects + commit the file, same PR. +- Broadcast topic: `crew:{auth user id}` · event: `sync` · payload: `{ entity }` only. +- Entities: `turnovers` | `checklists` | `work_orders`. +- Cursor rules: forward-only, full-scope pulls only, fresh ids pulled cursorless, deletion via reconciliation only. +- Flag: `NEXT_PUBLIC_CREW_SYNC_V2` (default off until Phase 5). diff --git a/docs/SCALABILITY_TIERS_REMAINING.md b/docs/SCALABILITY_TIERS_REMAINING.md new file mode 100644 index 00000000..b4ea2be0 --- /dev/null +++ b/docs/SCALABILITY_TIERS_REMAINING.md @@ -0,0 +1,158 @@ +# Structural Scalability — Remaining Tier Items: Implementation Instructions + +Companion to `docs/CREW_SYNC_V2_PHASES.md`. That document covers the crew +PWA realtime redesign (originally Tier 1 item 3 of the scalability +assessment); this one captures everything else still open from that +assessment, written so an agent with no prior context can execute each +item. Read `CLAUDE.md` at the repo root in full first — every rule there +applies, and the guardrail suite will fail your build if you skip it. + +## 0. Where the assessment stands + +The original assessment ranked findings into three tiers. Verified current +status (checked against the live codebase, not assumed): + +| Item | Tier | Status | +|---|---|---| +| 1. Serial per-org crons → event fan-out | 1 | ✅ Done — `daily-wrapup.ts`, SMS morning/evening crons, `ownerrez/incremental-sync`, `turnover-priority-decay` all converted to `step.sendEvent` fan-out | +| 2. OwnerRez/Hospitable shared-IP budget fair-share | 1 | ✅ Done | +| 3. Crew PWA Realtime footprint | 1 | 🔶 In progress — redesigned as Crew Sync v2; Phases 0–1 live, Phases 2–5 remain (see `CREW_SYNC_V2_PHASES.md`) | +| 4. Drop redundant per-row `is_org_member()` from SELECT policies | 1 | ✅ Done — migration applied | +| 5. SMS spend/throughput guard before `SMS_ENABLED=true` | 1 | ✅ Done (flag itself stays false until 10DLC clears) | +| 6. Memoize `requireOrgMember()` + fix layout waterfall | 2 | ✅ Done — `lib/auth.ts` wraps auth context in React `cache()` | +| 7. Dexie delta sync + outbox backoff | 2 | 🔶 Half done — delta sync shipped as Crew Sync v2 Phase 1; **outbox backoff = Phase 4, still open** | +| 8. Bound the unbounded queries | 2 | ✅ Done — `checklist-signals` has a 180-day rolling window, reviews/owners pages are `.limit()`-bounded | +| 9. Enforcement Tiers 1–3 (ESLint/guardrails → typed ServiceRoleContext → DB invariant CI gate) | — | ✅ Done — Tier 3 is PR #505 | +| 10. Tier 3 hygiene list | 3 | ⬜ **All open — sections below** | + +So the actual remaining work is: **Crew Sync v2 Phases 2–5** (the other +document), plus the four Tier 3 hygiene items and one enforcement leftover +below. Each section here is independent — they can be separate small PRs +in any order. + +--- + +## 1. `notifications` retention cron + +**Problem:** the `notifications` table (in-app bell events, added +2026-07-15) has no retention job — it grows forever. Every other +append-heavy table already has one (`audit-retention.ts`, +`comms-retention.ts`, `guest-pii-retention.ts` in +`lib/inngest/functions/cron/`). + +**Instructions:** + +1. Read `lib/inngest/functions/cron/audit-retention.ts` and mirror its + shape exactly (batch-deleting cron, service client with + `{ system: 'inngest:...' }` context, logger calls). +2. Policy: delete `notifications` rows where `read_at IS NOT NULL AND + created_at < now() - interval '90 days'`, and unread rows older than + 180 days. Delete in bounded batches (the existing retention crons show + the pattern) — never one unbounded `DELETE`. +3. Register the event in `lib/inngest/events.ts` (before the closing brace + of `FieldStayEvents`) and the function in `app/api/inngest/route.ts` + (inside the ONE existing `serve()` call). +4. No migration needed; no `types/database.ts` change. +5. Verification pass, commit. + +## 2. Hostaway incremental sync + +**Problem:** Hostaway sync is initial-import only / full-refetch — no +incremental cursor, unlike OwnerRez which has +`ownerrez/incremental-sync.ts`. + +**Instructions:** + +1. Read `lib/integrations/providers/` for the Hostaway provider and the + OwnerRez incremental sync function as the reference implementation. +2. Mirror the OwnerRez pattern: a cron that fans out one event per + Hostaway connection (`step.sendEvent`), a per-connection handler with + a `latestActivity`/modified-since cursor stored on + `integration_connections` (check what cursor fields already exist + before adding columns — if a migration is needed, update + `types/database.ts` in the same commit). +3. Respect the shared API budget pattern used for OwnerRez (per-org + fair-share, resume-where-left-off — item 2 of the assessment, already + built; reuse its helpers rather than reimplementing). +4. Register event + function per the standard Inngest rules; idempotency + per CLAUDE.md (bookings dedup on external id, `ON CONFLICT DO NOTHING`). + +## 3. Kroger API rate limiter + +**Problem:** `lib/integrations/providers/kroger.ts` calls the Kroger API +with no rate limiting or 429 handling — cart automation fanning out across +orgs shares one IP/token budget, same class of problem as OwnerRez was. + +**Instructions:** + +1. Check Kroger's published limits (public docs: 10,000/day per endpoint + class is the commonly cited figure — verify at implementation time). +2. Add a limiter in the same style as `lib/rate-limit.ts`'s Upstash + sliding-window limiters (one shared limiter keyed per endpoint class, + not per org), consulted inside the Kroger provider before each call. +3. Handle 429 responses: honor `Retry-After` when present; inside Inngest + steps, throw a retriable error so Inngest's backoff does the waiting + (never `sleep()` inside a step). +4. Fail open if Redis is unavailable **for reads of the limiter**, but see + section 4 — outbound-spend budgets are the one place we fail closed. + +## 4. Fail-closed outbound budgets on Redis outage + +**Problem:** budget/spend limiters (SMS budget chokepoint in +`lib/sms/telnyx.ts`, retailer/cart spend) currently follow the same +fail-open-on-Redis-error convention as the abuse rate limiters in +`proxy.ts`. For token-enumeration throttles fail-open is correct (an +outage shouldn't take down public pages); for **money-spending** paths it +is backwards — a Redis outage would remove all spend ceilings exactly when +nothing is watching. + +**Instructions:** + +1. Identify every limiter whose purpose is bounding *spend* rather than + *abuse* (grep `lib/rate-limit.ts` consumers; the SMS budget check in + `lib/sms/telnyx.ts` is the canonical one). +2. For those call sites only: on limiter error, **skip the send and log + loudly** (`console.error` + `reportError`) instead of proceeding. + Message the failure into the PM notification stream if the existing + notification helpers make that cheap. +3. Do NOT change the fail-open behavior of `proxy.ts`'s public-route + limiters — that direction is deliberate and documented there. +4. Per the CLAUDE.md meta-rule, add a one-line note to the SMS section of + CLAUDE.md and, if practical, a guardrail test asserting the SMS send + path contains the fail-closed branch (grep-style, like + `forbidden-patterns`). + +## 5. Enforcement leftover: `types/database.ts` drift check + +**Problem:** PR #505's `db-invariants` CI job implemented checks 1–3 of +the Tier 3 outline (RLS everywhere, FK covering indexes, zero anon +grants). Check 4 — generating types from the e2e project and diffing the +table/column shape against the committed `types/database.ts` — was +deferred. + +**Instructions:** + +1. Extend `scripts/check-db-invariants.mjs` (or add a sibling script) to + fetch generated types for the e2e project (`syhthijeqlnltufdawyb`) via + the Supabase CLI or REST, reduce both the generated output and the + committed `types/database.ts` to a comparable table→column→nullability + shape, and diff. +2. Report drift as a CI failure listing the specific tables/columns — + this is exactly the class of bug that cost half a day this session + (`wo_status` missing `quote_requested` on e2e; see migration + `20260725043000`). The check exists to make that impossible to miss + again. +3. Same self-disarming behavior as the rest of the job when e2e secrets + are absent. +4. Expect an initial reconciliation pass: the first run will surface + existing drift; fix it with migrations (both projects) rather than + loosening the check. + +--- + +## Quick reference + +- Verification pass: `npx tsc --noEmit && npm run lint && npx vitest run && npm run check:ui-classes` +- Prod Supabase: `vpmznjktllhmmbfnxuvk` · E2E: `syhthijeqlnltufdawyb` — never any other project; migrations applied to BOTH + file committed, same PR. +- Inngest: functions in `lib/inngest/functions/`, events registered in `lib/inngest/events.ts`, ONE `serve()` in `app/api/inngest/route.ts`. +- New convention ⇒ ships with its guardrail, same PR. diff --git a/e2e/global-setup.ts b/e2e/global-setup.ts index 748abd1f..da0339c1 100644 --- a/e2e/global-setup.ts +++ b/e2e/global-setup.ts @@ -2,6 +2,7 @@ import { chromium, type FullConfig } from '@playwright/test' import { createClient, type SupabaseClient } from '@supabase/supabase-js' import * as fs from 'fs' import * as path from 'path' +import type { Database } from '../types/database.generated' export default async function globalSetup(_config: FullConfig) { const baseUrl = process.env.E2E_BASE_URL ?? 'http://localhost:3000' @@ -46,7 +47,16 @@ export default async function globalSetup(_config: FullConfig) { `trial_ends_at for the E2E PM org in the database.` ) } - throw new Error(`Login failed — current URL: ${url}`) + // signInWithPassword() runs entirely client-side (login-form.tsx) — it + // never round-trips through the Next.js server, so a failed sign-in + // produces no server-side log even with webServer stdout/stderr now + // piped. The only place the actual reason (bad credentials, Supabase + // rate-limiting, an outage) is visible at all is this on-page error + // banner — grab it so a login failure doesn't come with `current URL: + // .../login` as its only clue. + const bannerText = await page.locator('.bg-red-50').first().textContent().catch(() => null) + const bannerSuffix = bannerText ? ` — page error: "${bannerText.trim()}"` : ' (no error banner found on page)' + throw new Error(`Login failed — current URL: ${url}${bannerSuffix}`) } await page.context().storageState({ path: 'e2e/.auth/pm.json' }) @@ -56,15 +66,14 @@ export default async function globalSetup(_config: FullConfig) { // Tear down any stale [E2E] data first, then re-seed. // This ensures a clean starting state even if a previous run aborted. - const supabase = createClient( + const supabase = createClient( process.env.NEXT_PUBLIC_SUPABASE_URL!, process.env.SUPABASE_SERVICE_ROLE_KEY!, { auth: { persistSession: false } } ) // Find the test org from the PM account's membership - const { data: authUser } = await supabase.auth.admin.listUsers() - const pmUser = authUser.users.find((u) => u.email === email) + const pmUser = await findUserByEmail(supabase, email) if (!pmUser) { throw new Error(`Could not find Supabase user for ${email}`) @@ -120,25 +129,43 @@ export default async function globalSetup(_config: FullConfig) { throw new Error('Failed to create seed property [E2E] The Lakehouse') } - // Seed one crew member - await supabase.from('crew_members').insert({ - org_id: orgId, - name: '[E2E] Alex Cleaner', - phone: '+15550001234', - email: null, - role: 'cleaner', - status: 'active', + // Seed one crew member. role must be a real crew_role enum value + // ('cleaning', not 'cleaner') and the active flag is is_active — the + // original insert used 'cleaner' + a nonexistent status column and, + // with no error check, failed silently on every run, which is why the + // crew specs never found '[E2E] Alex Cleaner'. + const { error: crewSeedErr } = await supabase.from('crew_members').insert({ + org_id: orgId, + name: '[E2E] Alex Cleaner', + phone: '+15550001234', + email: null, + role: 'cleaning', + specialty: 'cleaning', + is_active: true, }) + if (crewSeedErr) { + throw new Error(`Failed to seed crew member [E2E] Alex Cleaner: ${crewSeedErr.message}`) + } // Seed one vendor - await supabase.from('vendors').insert({ + const { error: vendorSeedErr } = await supabase.from('vendors').insert({ org_id: orgId, name: '[E2E] Reliable Plumbing Co.', email: 'plumber@e2e-test.invalid', specialty: 'plumbing', portal_enabled: true, is_active: true, + // stripe_connect_charges_enabled defaults to false — without this, + // VendorPortal (app/work-orders/[token]/vendor-portal.tsx) renders its + // "set up payouts before submitting" Connect gate instead of the + // invoice/line-items form, so the form's inputs (e.g. + // input[placeholder="Description"], used by + // 21-work-order-offline.spec.ts) never mount. + stripe_connect_charges_enabled: true, }) + if (vendorSeedErr) { + throw new Error(`Failed to seed vendor [E2E] Reliable Plumbing Co.: ${vendorSeedErr.message}`) + } // ── 3. Seed a crew login + an assigned turnover/checklist item ─────────── // Used by e2e/specs/22-crew-logout-guard.spec.ts to exercise the crew PWA @@ -169,8 +196,7 @@ async function seedCrewLoginAndAssignment( // Reuse the auth user across runs rather than erroring on "already // registered" — this account is test-only and never has other state // attached to it beyond what this function seeds fresh each run. - const { data: existingUsers } = await supabase.auth.admin.listUsers() - let crewAuthUser = existingUsers.users.find((u) => u.email === crewEmail) + let crewAuthUser = await findUserByEmail(supabase, crewEmail) if (!crewAuthUser) { const { data: created, error: createErr } = await supabase.auth.admin.createUser({ @@ -271,12 +297,94 @@ async function seedCrewLoginAndAssignment( await browser.close() } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -async function cleanE2EData(supabase: SupabaseClient, orgId: string): Promise { +async function cleanE2EData(supabase: SupabaseClient, orgId: string): Promise { // Delete in FK-safe order. Properties cascade to bookings and turnovers. + // communication_logs was missing here entirely — 16-comms-log.spec.ts's + // hardcoded '[E2E] Confirmed service window' entry persisted across a CI + // retry (or a prior run) and collided with itself: the empty-state test + // found a stale row instead of nothing, and the create-entry test hit a + // strict-mode violation with two identical rows. Deleted before vendors + // since it FKs to vendors.id. + await supabase.from('communication_logs').delete().eq('org_id', orgId).like('subject', '[E2E]%') await supabase.from('work_orders') .delete().eq('org_id', orgId).like('title', '[E2E]%') await supabase.from('bookings') .delete().eq('org_id', orgId).like('guest_name', '[E2E]%') await supabase.from('crew_members') .delete().eq('org_id', orgId).like('name', '[E2E]%') await supabase.from('vendors') .delete().eq('org_id', orgId).like('name', '[E2E]%') await supabase.from('properties') .delete().eq('org_id', orgId).like('name', '[E2E]%') + + await cleanOrphanedDisposableAuthUsers(supabase) +} + +// Specs that create a disposable crew Supabase Auth user per test +// (21-work-order-offline, 22-crew-logout-guard, 27-crew-feedback) delete it +// in their own `finally`/cleanup block — but a CI run that gets killed or +// cancelled mid-test (e.g. superseded by a newer push under this repo's +// concurrency: cancel-in-progress workflow setting) never reaches that +// block, orphaning the auth user permanently. These accumulate silently +// until supabase.auth.admin.listUsers()'s pagination (newest-first) pushes +// the long-lived seeded PM/crew accounts off the first page entirely, +// which is exactly what broke every run in this session once enough +// orphans had built up — findUserByEmail() below is the durable fix for +// that symptom, but the orphans themselves are still waste worth sweeping. +// +// Only sweeps users older than an hour — anything younger could belong to +// a still-in-progress concurrent run (this repo's CI concurrency group now +// serializes runs of THIS workflow, but doesn't protect against a manually +// triggered local run against the same shared E2E project overlapping with +// CI). Deleting a live run's own disposable user out from under it would +// fail that run with a confusing "user not found" error instead of the +// clean, expected orphan sweep this is meant to be. +const DISPOSABLE_AUTH_USER_PREFIXES = ['e2e-crew-wo-', 'e2e-crew-logout-', 'e2e-crew-feedback-'] + +function isStaleDisposableUser( + user: { email?: string | null; created_at?: string | null }, + staleBeforeMs: number, +): boolean { + if (!user.email || !user.created_at) return false + if (new Date(user.created_at).getTime() > staleBeforeMs) return false + return DISPOSABLE_AUTH_USER_PREFIXES.some((prefix) => user.email!.startsWith(prefix)) +} + +async function cleanOrphanedDisposableAuthUsers(supabase: SupabaseClient): Promise { + const staleBeforeMs = Date.now() - 60 * 60 * 1000 + // listUsers() is offset-based pagination — deleting mid-page shifts later + // pages' offsets and can skip a user who shifts into an already-visited + // slot. Collect every ID across all pages first, then delete once + // pagination is done so no delete can affect an in-flight page fetch. + const userIdsToDelete: string[] = [] + + let page = 1 + for (;;) { + const { data, error } = await supabase.auth.admin.listUsers({ page, perPage: 200 }) + if (error) throw new Error(`listUsers() failed while sweeping orphaned disposable users (page ${page}): ${error.message}`) + if (!data || data.users.length === 0) break + + userIdsToDelete.push(...data.users.filter((u) => isStaleDisposableUser(u, staleBeforeMs)).map((u) => u.id)) + + if (data.users.length < 200) break + page += 1 + } + + for (const userId of userIdsToDelete) { + await supabase.auth.admin.deleteUser(userId) + } +} + +// supabase.auth.admin.listUsers() paginates (newest-first, ~50/page by +// default) — with enough disposable test users in play, a plain +// .find() over the first page alone can silently miss a long-lived +// account like the seeded PM/crew logins. Page through until found. +async function findUserByEmail(supabase: SupabaseClient, email: string) { + let page = 1 + for (;;) { + const { data, error } = await supabase.auth.admin.listUsers({ page, perPage: 200 }) + if (error) throw new Error(`listUsers() failed while looking up ${email} (page ${page}): ${error.message}`) + if (!data || data.users.length === 0) return undefined + + const match = data.users.find((u) => u.email === email) + if (match) return match + + if (data.users.length < 200) return undefined + page += 1 + } } diff --git a/e2e/helpers/cookies.ts b/e2e/helpers/cookies.ts index 7f4119ca..b7928c1a 100644 --- a/e2e/helpers/cookies.ts +++ b/e2e/helpers/cookies.ts @@ -5,13 +5,18 @@ export async function dismissCookieBanner(page: Page): Promise { const isVisible = await banner.isVisible().catch(() => false) if (!isVisible) return - const dismissBtn = page.getByRole('button', { + // Scoped to the banner region: with force:true below, a page-wide locator + // could force-click an open dialog's Close/OK button instead. + const dismissBtn = banner.getByRole('button', { name: /accept|got it|ok|dismiss|close|agree|allow/i, }).first() const btnVisible = await dismissBtn.isVisible().catch(() => false) if (btnVisible) { - await dismissBtn.click() + // force: the fixed-position banner can sit under a Dialog overlay + // (fixed inset-0) that intercepts pointer events — specs legitimately + // dismiss the banner while a dialog is open, so bypass the hit-test. + await dismissBtn.click({ force: true }) await banner.waitFor({ state: 'hidden', timeout: 3_000 }).catch(() => {}) } } diff --git a/e2e/helpers/forms.ts b/e2e/helpers/forms.ts new file mode 100644 index 00000000..de9d610a --- /dev/null +++ b/e2e/helpers/forms.ts @@ -0,0 +1,30 @@ +import type { Locator } from '@playwright/test' + +// selectOption() by label can race a option which is hidden. // Assert the select element itself is visible instead. await expect(page.locator('select').first()).toBeVisible() @@ -13,7 +13,12 @@ test.describe('Bookings', () => { test('[E2E] add manual booking creates booking and success banner', async ({ page }) => { await page.goto('/bookings') - await page.getByRole('button', { name: /Add Booking/i }).click() + // Dismiss once, before any dialog opens — the banner and the Dialog + // backdrop share z-50, and since the Dialog portal paints later in DOM + // order it sits on top; dismissing later (while a dialog is open) can + // land the click on the backdrop instead and close the dialog. + await dismissCookieBanner(page) + await page.getByRole('button', { name: /Add Booking/i }).first().click() await expect(page.getByRole('heading', { name: /Log Non-Synced Booking/i })).toBeVisible() @@ -26,12 +31,17 @@ test.describe('Bookings', () => { await page.fill('[name="checkout_date"]', checkout) await page.fill('[name="guest_name"]', '[E2E] Jane Playwright') - // Cookie banner can intercept the submit click — dismiss it first - await dismissCookieBanner(page) - await page.click('button[type="submit"]') - await expect(page.getByText(/Booking added/i)).toBeVisible({ timeout: 8_000 }) + // createBooking's critical path (property lookup, insert, + // logAuditEvent, detectAndFlagOverlaps, inngest.send, two + // revalidatePath calls) is fully awaited before the client sees + // success — under sustained E2E-project DB load this occasionally + // pushes past a tight timeout even though the insert itself always + // completes (confirmed: a "failed" attempt's booking still exists on + // the next attempt). 20s gives real headroom without masking an + // actual hang. + await expect(page.getByText(/Booking added/i)).toBeVisible({ timeout: 20_000 }) await expect(page.getByText('[E2E] Jane Playwright')).toBeVisible() }) diff --git a/e2e/specs/05-work-orders.spec.ts b/e2e/specs/05-work-orders.spec.ts index cf144151..02290da5 100644 --- a/e2e/specs/05-work-orders.spec.ts +++ b/e2e/specs/05-work-orders.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from '../fixtures' import { dismissCookieBanner } from '../helpers/cookies' +import { getServiceClient } from '../helpers/teardown' test.describe('Work Orders / Maintenance', () => { @@ -11,8 +12,11 @@ test.describe('Work Orders / Maintenance', () => { ).toBeVisible() }) - test('[E2E] create work order appears on board', async ({ page }) => { + test('[E2E] create work order appears on board', async ({ page, ctx }) => { await page.goto('/maintenance') + // Dismiss before opening any dialog — see 03-bookings.spec.ts for why + // dismissing while a dialog is open can close the dialog instead. + await dismissCookieBanner(page) const newBtn = page.getByRole('button', { name: /New Work Order|Add Work Order|Create|New WO/i, @@ -27,11 +31,40 @@ test.describe('Work Orders / Maintenance', () => { await prioritySelect.selectOption('medium') } - await dismissCookieBanner(page) await page.click('button[type="submit"]') - await page.waitForURL(/\/maintenance/, { timeout: 10_000 }) - await expect(page.getByText('[E2E] Fix Leaking Faucet')).toBeVisible({ timeout: 8_000 }) + // Not waitForURL — createWorkOrder (Server Action) never redirects, it + // just revalidates and the modal closes itself client-side once + // useActionState resolves state.success (CreateWorkOrderModal.tsx), so + // waitForURL(/\/maintenance/) was a same-URL no-op that didn't actually + // wait for the create to complete. Wait for the dialog to close instead + // — that's the real signal the mutation (including its await'd + // inngest.send() call) has finished, not just that the click dispatched. + await expect(page.getByRole('dialog')).not.toBeVisible({ timeout: 10_000 }) + + try { + await expect(page.getByText('[E2E] Fix Leaking Faucet')).toBeVisible({ timeout: 8_000 }) + } catch (uiErr) { + // The dialog closing proves createWorkOrder returned success — but + // this assertion has failed repeatedly in CI with no server-side + // error (confirmed via playwright.config.ts's webServer stdout/ + // stderr piping). Query the DB directly on failure so the CI log + // says definitively whether the row was ever persisted (a real + // create/RLS/visibility bug) or exists but isn't rendering (a + // client refresh/query-filter bug) — static code review alone + // couldn't distinguish these. + const supabase = getServiceClient() + const { data: rows, error: dbErr } = await supabase + .from('work_orders') + .select('id, title, status, org_id, property_id, created_at') + .eq('org_id', ctx.orgId) + .like('title', '[E2E] Fix Leaking Faucet%') + console.error( + '[05-work-orders diagnostic] DB rows for this org/title after UI assertion failed:', + JSON.stringify({ rows, dbErr }), + ) + throw uiErr + } }) test('[E2E] work order detail page opens', async ({ page }) => { diff --git a/e2e/specs/06-crew.spec.ts b/e2e/specs/06-crew.spec.ts index c9993308..078a7d0e 100644 --- a/e2e/specs/06-crew.spec.ts +++ b/e2e/specs/06-crew.spec.ts @@ -17,23 +17,19 @@ test.describe('Crew Management', () => { await page.goto('/crew-manage') await page.waitForLoadState('networkidle') - // Broaden regex to cover: Add Crew Member, New Member, + Add, Invite, etc. - const addBtn = page.getByRole('button', { - name: /add|new|invite|crew|member|\+/i, - }).first() - - await addBtn.waitFor({ state: 'visible', timeout: 8_000 }) + // The original broad regex (/add|new|invite|crew|member|\+/i) matched + // page-level nav/header elements ahead of the real trigger in DOM order — + // the actual toggle button (crew-manage-client.tsx) reads exactly + // "+ Add Member". + const addBtn = page.getByRole('button', { name: '+ Add Member' }) await addBtn.click() - // Fill whichever input is visible — name and phone are standard fields - const nameInput = page.locator('input[name="name"], input[placeholder*="name" i]').first() - await nameInput.waitFor({ state: 'visible', timeout: 5_000 }) - await nameInput.fill('[E2E] Sam Housekeeper') - - const phoneInput = page.locator('input[name="phone"], input[type="tel"]').first() - if (await phoneInput.isVisible()) { - await phoneInput.fill('+15550009999') - } + // AddCrewForm (crew-manage-client.tsx) — name and email are both + // `required`; email must be filled or native HTML5 validation blocks + // the submit. + await page.fill('input[name="name"]', '[E2E] Sam Housekeeper') + await page.fill('input[name="email"]', 'sam-housekeeper@e2e-test.invalid') + await page.fill('input[name="phone"]', '+15550009999') await page.click('button[type="submit"]') diff --git a/e2e/specs/10-vendors.spec.ts b/e2e/specs/10-vendors.spec.ts index 430c5b30..1cba96a9 100644 --- a/e2e/specs/10-vendors.spec.ts +++ b/e2e/specs/10-vendors.spec.ts @@ -4,17 +4,23 @@ test.describe('Vendors', () => { test('vendors page loads with seeded vendor', async ({ page }) => { await page.goto('/vendors') - await expect(page.getByText('[E2E] Reliable Plumbing Co.')).toBeVisible() + // vendors-client.tsx renders both a mobile card list (`md:hidden`) and a + // desktop table (`hidden md:block`) unconditionally — .first() picks the + // mobile copy since it comes first in DOM order, and it's CSS-hidden at + // this project's Desktop Chrome viewport. Filter to the visible one. + await expect( + page.getByText('[E2E] Reliable Plumbing Co.').filter({ visible: true }).first() + ).toBeVisible() }) test('can open vendor detail', async ({ page }) => { await page.goto('/vendors') - await page.getByText('[E2E] Reliable Plumbing Co.').click() + await page.getByText('[E2E] Reliable Plumbing Co.').filter({ visible: true }).first().click() // Vendors open a detail panel — assert the vendor name remains visible // in the panel (or on the detail page if navigation occurs). await expect( - page.getByText('[E2E] Reliable Plumbing Co.').first() + page.getByText('[E2E] Reliable Plumbing Co.').filter({ visible: true }).first() ).toBeVisible({ timeout: 8_000 }) }) diff --git a/e2e/specs/13-maintenance-schedules.spec.ts b/e2e/specs/13-maintenance-schedules.spec.ts index 2dfcd2d1..4b79416c 100644 --- a/e2e/specs/13-maintenance-schedules.spec.ts +++ b/e2e/specs/13-maintenance-schedules.spec.ts @@ -13,6 +13,11 @@ test.describe('Maintenance Schedules', () => { test('[E2E] add maintenance schedule to seeded property', async ({ page }) => { await page.goto('/maintenance') + // Dismiss before opening any dialog — the banner and the Dialog backdrop + // share z-50, and since the Dialog portal paints later in DOM order it + // sits on top; dismissing later (while a dialog is open) can land the + // click on the backdrop instead and close the dialog. + await dismissCookieBanner(page) await page.getByRole('button', { name: /Maintenance Schedules/i }).click() // The section's own "Add Schedule" trigger is the only one in the DOM @@ -25,7 +30,6 @@ test.describe('Maintenance Schedules', () => { await dialog.locator('[name="name"]').fill('[E2E] HVAC Filter Change') await dialog.locator('[name="property_id"]').selectOption({ label: '[E2E] The Lakehouse' }) - await dismissCookieBanner(page) // Trigger button and modal submit button share the same accessible name // once the dialog is open — scope to the dialog to disambiguate. await dialog.getByRole('button', { name: 'Add Schedule', exact: true }).click() diff --git a/e2e/specs/16-comms-log.spec.ts b/e2e/specs/16-comms-log.spec.ts index f6d53caf..512c22ef 100644 --- a/e2e/specs/16-comms-log.spec.ts +++ b/e2e/specs/16-comms-log.spec.ts @@ -11,6 +11,9 @@ test.describe('Comms Log', () => { test('[E2E] log a communication entry to seeded vendor', async ({ page }) => { await page.goto('/comms-log') + // Dismiss before opening any dialog — see 03-bookings.spec.ts for why + // dismissing while a dialog is open can close the dialog instead. + await dismissCookieBanner(page) await page.getByRole('button', { name: 'Log Communication' }).click() @@ -27,13 +30,16 @@ test.describe('Comms Log', () => { await vendorSelect.selectOption({ label: '[E2E] Reliable Plumbing Co.' }) } - await dialog.locator('[name="subject"]').fill('[E2E] Confirmed service window') + // Unique per attempt so a same-run Playwright retry (CI sets retries: 2) + // can't collide with a row its own prior attempt already created — + // global-setup.ts's cleanE2EData() only guards against cross-run staleness. + const subject = `[E2E] Confirmed service window ${Date.now()}` + await dialog.locator('[name="subject"]').fill(subject) await dialog.locator('[name="body"]').fill('[E2E] Called to confirm Tuesday appointment.') - await dismissCookieBanner(page) await dialog.getByRole('button', { name: 'Save Entry' }).click() - await expect(page.getByText('[E2E] Confirmed service window')).toBeVisible({ timeout: 8_000 }) + await expect(page.getByText(subject)).toBeVisible({ timeout: 8_000 }) }) }) diff --git a/e2e/specs/18-messages.spec.ts b/e2e/specs/18-messages.spec.ts index 3c4be2a8..bacebffd 100644 --- a/e2e/specs/18-messages.spec.ts +++ b/e2e/specs/18-messages.spec.ts @@ -2,14 +2,15 @@ import { test, expect } from '../fixtures' test.describe('Messages', () => { - test('messages page loads (no linked crew accounts yet)', async ({ page }) => { + test('messages page loads with linked crew account in thread list', async ({ page }) => { await page.goto('/messages') // The thread list only includes crew members with a linked auth user - // (user_id IS NOT NULL). The seeded crew member has no linked account, - // so this asserts the empty state renders correctly rather than the - // page erroring — a full compose/send test needs a crew member seeded - // with an accepted invite, which global-setup.ts doesn't create. - await expect(page.getByText(/No crew members found/i)).toBeVisible({ timeout: 8_000 }) + // (messages/page.tsx: .not('user_id', 'is', null)). "[E2E] Alex + // Cleaner" (global-setup.ts) has no linked account, but + // "[E2E] Logout Guard Crew" does (seeded via seedCrewLoginAndAssignment + // for 22-crew-logout-guard.spec.ts) — so the thread list is never + // actually empty in this suite; assert that crew member appears. + await expect(page.getByText('[E2E] Logout Guard Crew')).toBeVisible({ timeout: 8_000 }) }) }) diff --git a/e2e/specs/20-help.spec.ts b/e2e/specs/20-help.spec.ts index 09fb2aea..fb8e3cfb 100644 --- a/e2e/specs/20-help.spec.ts +++ b/e2e/specs/20-help.spec.ts @@ -16,11 +16,23 @@ test.describe('Help & Support', () => { test('can expand an FAQ item', async ({ page }) => { await page.goto('/help') - const firstQuestion = page.getByRole('button', { expanded: false }).first() - if (await firstQuestion.isVisible()) { - await firstQuestion.click() - await expect(firstQuestion).toHaveAttribute('aria-expanded', 'true') - } + // dashboard-shell.tsx (the layout wrapping every dashboard page) has its + // own aria-expanded toggle (a sidebar section) that renders before the + // page content in DOM order — an unscoped page-wide locator's .first() + // picks that instead of the actual first FAQ button. Scope to
. + // + // getByRole('button', { expanded: false }) is a LIVE filter: Playwright + // re-evaluates it (including the expanded condition) on every + // interaction, not just once at creation. After the click flips this + // item's aria-expanded to "true", the same locator no longer matches it + // and silently re-resolves .first() to the NEXT still-closed item — + // so the toHaveAttribute assertion below was checking the wrong + // element. Select by attribute presence (matches regardless of its + // current value) so the locator's identity doesn't shift mid-test. + const firstQuestion = page.getByRole('main').locator('button[aria-expanded]').first() + await expect(firstQuestion).toBeVisible() + await firstQuestion.click() + await expect(firstQuestion).toHaveAttribute('aria-expanded', 'true') }) }) diff --git a/e2e/specs/21-work-order-offline.spec.ts b/e2e/specs/21-work-order-offline.spec.ts index 749d0c7d..21dc1c2b 100644 --- a/e2e/specs/21-work-order-offline.spec.ts +++ b/e2e/specs/21-work-order-offline.spec.ts @@ -48,7 +48,17 @@ test.describe('Work order offline support', () => { // Fresh, unauthenticated context — the default `page` fixture carries // the PM's storageState, which would put the crew layout's PM-guard // redirect in the way of a crew login. - const context = await browser.newContext() + // + // storageState: undefined is required, not optional — Playwright Test + // instruments every browser.newContext() created during a running + // test (not just the fixture-provided `context`/`page`) and silently + // re-applies the project's configured use.storageState + // ('e2e/.auth/pm.json') to it. A bare browser.newContext() here is + // therefore secretly PM-authenticated: page.goto('/login?next=/crew') + // 307s straight past the login form to /crew, which the crew layout's + // PM-guard then 307s again to /ops, so the next line's page.fill + // times out waiting for an #email that was never on that page. + const context = await browser.newContext({ storageState: undefined }) const page = await context.newPage() await page.goto('/login?next=/crew') @@ -119,6 +129,12 @@ test.describe('Work order offline support', () => { await page.locator('input[placeholder="Description"]').first().fill('[E2E] Replaced valve') await page.locator('input[placeholder="0.00"]').first().fill('125') + // #technician-name is a required native (vendor-portal.tsx) — + // added after this spec was written, so the browser's own constraint + // validation was silently blocking the form submit before + // handleSubmit ever ran, and "Saved" never appeared for a reason that + // had nothing to do with the offline queueing being tested. + await page.locator('#technician-name').fill('[E2E] Tech') await page.route('**/api/work-orders/*/complete', (route) => route.abort()) @@ -174,6 +190,12 @@ test.describe('Work order offline support', () => { await page.locator('input[placeholder="Description"]').first().fill('[E2E] Replaced valve') await page.locator('input[placeholder="0.00"]').first().fill('125') + // #technician-name is a required native (vendor-portal.tsx) — + // added after this spec was written, so the browser's own constraint + // validation was silently blocking the form submit before + // handleSubmit ever ran, and "Saved" never appeared for a reason that + // had nothing to do with the offline queueing being tested. + await page.locator('#technician-name').fill('[E2E] Tech') await page.route('**/api/work-orders/*/complete', (route) => route.abort()) await page.getByRole('button', { name: /submit invoice/i }).click() diff --git a/e2e/specs/22-crew-logout-guard.spec.ts b/e2e/specs/22-crew-logout-guard.spec.ts index 3b0c6ef7..6fa738e0 100644 --- a/e2e/specs/22-crew-logout-guard.spec.ts +++ b/e2e/specs/22-crew-logout-guard.spec.ts @@ -1,86 +1,288 @@ import { test, expect } from '../fixtures' +import { getServiceClient } from '../helpers/teardown' -// This spec authenticates as the seeded crew login (e2e/.auth/crew.json, -// captured in global-setup.ts) rather than the default PM storageState — -// the crew PWA's CrewLayout guard rejects any user without an active -// crew_members record, which the PM account doesn't have. -test.use({ storageState: 'e2e/.auth/crew.json' }) - +// This spec creates its OWN disposable crew login + turnover per test +// (mirroring 27-crew-feedback.spec.ts) rather than reusing the shared +// e2e/.auth/crew.json. Two of the three tests below end in a real logout +// (supabase.auth.signOut(), which revokes the session server-side) — since +// crew.json is a static storageState snapshot loaded fresh per test but the +// underlying session is a single shared server-side record, whichever test +// logs out first permanently kills the session for every test that runs +// after it in this same file, regardless of declaration order (only one of +// the three tests preserves the session by clicking "Stay Logged In"). A +// throwaway per-test account with its own turnover has no such shared-state +// hazard. test.describe('Crew logout guard', () => { + // loginAsFreshCrewWithTurnover() below does 5 sequential Supabase Admin + // API round trips (createUser, crew_members, turnovers, + // turnover_assignments, checklist_instances, checklist_instance_items) + // plus a full page navigation/login before a test's own assertions even + // start — under CI load that alone can eat most of the default 30s + // per-test budget, so these tests were reaching the correct destination + // (login genuinely succeeded) and still failing on "Test timeout of + // 30000ms exceeded." + test.describe.configure({ timeout: 60_000 }) - test('logout with no unsynced work redirects immediately, no warning dialog', async ({ page }) => { - await page.goto('/crew') - await page.waitForLoadState('networkidle') - - await page.getByRole('button', { name: 'Log out' }).click() + test('logout with no unsynced work redirects immediately, no warning dialog', async ({ ctx, browser }) => { + const { page, cleanup } = await loginAsFreshCrewWithTurnover(ctx.orgId, browser) + try { + await page.getByRole('button', { name: 'Log out' }).click() - await page.waitForURL('**/login**', { timeout: 10_000 }) - await expect(page.getByText('Unsynced work on this device')).not.toBeVisible() + await page.waitForURL('**/login**', { timeout: 10_000 }) + await expect(page.getByText('Unsynced work on this device')).not.toBeVisible() + } finally { + await cleanup() + } }) - test('offline checklist tick blocks logout with a warning, "Stay Logged In" cancels', async ({ page }) => { - await page.goto('/crew') - await page.waitForLoadState('networkidle') + test('offline checklist tick blocks logout with a warning, "Stay Logged In" cancels', async ({ ctx, browser }) => { + const { page, cleanup } = await loginAsFreshCrewWithTurnover(ctx.orgId, browser) + try { + // Open the seeded turnover, then its checklist — the counters item lives there. + const turnoverLink = page.locator('a[href^="/crew/turnovers/"]').first() + await turnoverLink.waitFor({ timeout: 15_000 }) + await turnoverLink.click() + await page.getByText('Turnover Checklist').click() + await page.getByText('[E2E] Wipe kitchen counters').waitFor({ timeout: 10_000 }) + + // Go offline before ticking the item, so the mutation queues locally + // and never reaches the outbox handler. + await page.context().setOffline(true) + + await page.getByLabel(/Mark (complete|incomplete)/).first().click() + // Optimistic local write — no network round trip to wait for. + await page.waitForTimeout(300) + + await page.getByRole('button', { name: 'Log out' }).click() + + const dialog = page.getByText('Unsynced work on this device') + await expect(dialog).toBeVisible({ timeout: 10_000 }) + await expect(page.getByText(/1 item.*haven.t reached FieldStay yet/)).toBeVisible() + + await page.getByRole('button', { name: 'Stay Logged In' }).click() + await expect(dialog).not.toBeVisible() + + // Session must still be active — no redirect happened. + await expect(page).toHaveURL(/\/crew/) + + await page.context().setOffline(false) + } finally { + await cleanup() + } + }) - // Open the seeded turnover, then its checklist — the counters item lives there. - await page.locator('a[href^="/crew/turnovers/"]').first().click() - await page.getByText('Turnover Checklist').click() - await page.getByText('[E2E] Wipe kitchen counters').waitFor({ timeout: 10_000 }) + test('offline checklist tick + "Log Out Anyway" clears local data and redirects', async ({ ctx, browser }) => { + const { page, cleanup } = await loginAsFreshCrewWithTurnover(ctx.orgId, browser) + try { + const turnoverLink = page.locator('a[href^="/crew/turnovers/"]').first() + await turnoverLink.waitFor({ timeout: 15_000 }) + await turnoverLink.click() + await page.getByText('Turnover Checklist').click() + await page.getByText('[E2E] Wipe kitchen counters').waitFor({ timeout: 10_000 }) - // Go offline before ticking the item, so the mutation queues locally - // and never reaches the outbox handler. - await page.context().setOffline(true) + await page.context().setOffline(true) - await page.getByLabel(/Mark (complete|incomplete)/).first().click() - // Optimistic local write — no network round trip to wait for. - await page.waitForTimeout(300) + // Toggling the same item again is fine — the guard counts queued + // mutation rows, not completion direction. + await page.getByLabel(/Mark (complete|incomplete)/).first().click() + await page.waitForTimeout(300) - await page.getByRole('button', { name: 'Log out' }).click() + await page.getByRole('button', { name: 'Log out' }).click() + await expect(page.getByText('Unsynced work on this device')).toBeVisible({ timeout: 10_000 }) - const dialog = page.getByText('Unsynced work on this device') - await expect(dialog).toBeVisible({ timeout: 10_000 }) - await expect(page.getByText(/1 item.*haven.t reached FieldStay yet/)).toBeVisible() + // Restore connectivity before confirming — performLogout() + // (app/crew/crew-shell.tsx) calls supabase.auth.signOut() and then a + // client-side router.push('/login'), both of which need a network + // round trip (the router.push fetches the destination route's RSC + // payload). The offline simulation above only needs to hold long + // enough to queue the local mutation and trigger this warning — it + // isn't testing that the logout navigation itself works while + // offline, and previously ran the confirm-and-redirect step still + // offline, which failed on page.waitForURL with net::ERR_FAILED. + await page.context().setOffline(false) - await page.getByRole('button', { name: 'Stay Logged In' }).click() - await expect(dialog).not.toBeVisible() + await page.getByRole('button', { name: 'Log Out Anyway' }).click() - // Session must still be active — no redirect happened. - await expect(page).toHaveURL(/\/crew/) + await page.waitForURL('**/login**', { timeout: 10_000 }) - await page.context().setOffline(false) + // performLogout() deletes the per-user Dexie database before the + // redirect — confirm it's actually gone, not just that the dialog closed. + const dbNames = await page.evaluate(async () => { + const dbs = await indexedDB.databases() + return dbs.map((d) => d.name) + }) + expect(dbNames.some((name) => name?.startsWith('fieldstay-crew-'))).toBe(false) + } finally { + await cleanup() + } }) - test('offline checklist tick + "Log Out Anyway" clears local data and redirects', async ({ page }) => { - await page.goto('/crew') - await page.waitForLoadState('networkidle') +}) - await page.locator('a[href^="/crew/turnovers/"]').first().click() - await page.getByText('Turnover Checklist').click() - await page.getByText('[E2E] Wipe kitchen counters').waitFor({ timeout: 10_000 }) +async function loginAsFreshCrewWithTurnover(orgId: string, browser: import('@playwright/test').Browser) { + const supabase = getServiceClient() - await page.context().setOffline(true) + const { data: property, error: propertyErr } = await supabase + .from('properties') + .select('id') + .eq('org_id', orgId) + .eq('name', '[E2E] The Lakehouse') + .single() + if (propertyErr || !property) throw new Error(`Seed property [E2E] The Lakehouse not found: ${propertyErr?.message}`) - // Toggling the same item again is fine — the guard counts queued - // mutation rows, not completion direction. - await page.getByLabel(/Mark (complete|incomplete)/).first().click() - await page.waitForTimeout(300) + // crypto.randomUUID() rather than Date.now() — this file's three tests + // could in principle run alongside another disposable-crew spec + // (27-crew-feedback.spec.ts) and a millisecond collision would fail + // createUser() on a duplicate email. + const crewEmail = `e2e-crew-logout-${crypto.randomUUID()}@e2e-test.invalid` + const crewPassword = 'E2E-Crew-Logout-Test-1!' + const { data: created, error: createErr } = await supabase.auth.admin.createUser({ + email: crewEmail, password: crewPassword, email_confirm: true, + }) + if (createErr || !created.user) throw new Error(`Failed to create crew test user: ${createErr?.message}`) + const userId = created.user.id - await page.getByRole('button', { name: 'Log out' }).click() - await expect(page.getByText('Unsynced work on this device')).toBeVisible({ timeout: 10_000 }) + // Everything past this point can throw — without this catch, a failure + // here would skip the `cleanup` this function never got to return, + // orphaning the just-created auth user and any rows already inserted in + // the E2E project. Track each row's id as soon as it's created so the + // catch block can roll all of them back, not just the auth user. + let context: import('@playwright/test').BrowserContext | undefined + let crewMemberId: string | undefined + let turnoverId: string | undefined + try { + const { data: crewMember, error: cmErr } = await supabase + .from('crew_members') + .insert({ + org_id: orgId, + user_id: userId, + // Distinct from the static "[E2E] Logout Guard Crew" name + // global-setup.ts's seedCrewLoginAndAssignment() seeds for + // e2e/.auth/crew.json — 18-messages.spec.ts asserts on that exact + // seeded row, and this helper creates a fresh one per test here, + // so sharing the name would leave ambiguous duplicates in the DB + // for the rest of the run. + name: '[E2E] Fresh Logout Guard Crew', + role: 'cleaning', + specialty: 'cleaning', + is_active: true, + invite_accepted_at: new Date().toISOString(), + }) + .select('id') + .single() + if (cmErr || !crewMember) throw new Error(`Failed to create crew_members row: ${cmErr?.message}`) + crewMemberId = crewMember.id - await page.getByRole('button', { name: 'Log Out Anyway' }).click() + const checkout = new Date(Date.now() + 2 * 60 * 60 * 1000) // 2h from now + const checkin = new Date(Date.now() + 26 * 60 * 60 * 1000) // next day - await page.waitForURL('**/login**', { timeout: 10_000 }) + const { data: turnover, error: turnoverErr } = await supabase + .from('turnovers') + .insert({ + org_id: orgId, + property_id: property.id, + checkout_datetime: checkout.toISOString(), + checkin_datetime: checkin.toISOString(), + status: 'assigned', + priority: 'medium', + auto_generated: false, + }) + .select('id') + .single() + if (turnoverErr || !turnover) throw new Error(`Failed to create turnover: ${turnoverErr?.message}`) + turnoverId = turnover.id - // performLogout() deletes the per-user Dexie database before the - // redirect — confirm it's actually gone, not just that the dialog closed. - const dbNames = await page.evaluate(async () => { - const dbs = await indexedDB.databases() - return dbs.map((d) => d.name) + const { error: assignErr } = await supabase.from('turnover_assignments').insert({ + turnover_id: turnover.id, + crew_member_id: crewMember.id, + org_id: orgId, + property_id: property.id, }) - expect(dbNames.some((name) => name?.startsWith('fieldstay-crew-'))).toBe(false) + if (assignErr) throw new Error(`Failed to create turnover_assignments row: ${assignErr.message}`) - await page.context().setOffline(false) - }) + const { data: instance, error: instanceErr } = await supabase + .from('checklist_instances') + .insert({ + turnover_id: turnover.id, + org_id: orgId, + template_snapshot: {}, + status: 'not_started', + }) + .select('id') + .single() + if (instanceErr || !instance) throw new Error(`Failed to create checklist instance: ${instanceErr?.message}`) -}) + const { error: itemErr } = await supabase.from('checklist_instance_items').insert({ + instance_id: instance.id, + turnover_id: turnover.id, + section_name: '[E2E] Kitchen', + task: '[E2E] Wipe kitchen counters', + requires_photo: false, + is_completed: false, + sort_order: 0, + }) + if (itemErr) throw new Error(`Failed to create checklist instance item: ${itemErr.message}`) + + // Fresh, unauthenticated context — the default `page` fixture carries the + // PM's storageState, which would put the crew layout's PM-guard redirect + // in the way of a crew login. + // + // storageState: undefined is NOT redundant with the bare call below it — + // Playwright Test instruments every browser.newContext() made during a + // running test (not just the fixture-provided `context`/`page`), and + // silently re-applies the project's configured `use.storageState` + // ('e2e/.auth/pm.json' — see playwright.config.ts) to it. A bare + // `browser.newContext()` here is therefore secretly PM-authenticated, + // not fresh: page.goto('/login?next=/crew') 307s straight past the + // login form (proxy.ts sees an authenticated user hitting a public + // route) to /crew, which the crew layout's PM-guard then 307s again to + // /ops — and #email never existed on that page, so the next line's + // page.fill() times out. Confirmed by a standalone repro against the + // installed @playwright/test package: a bare browser.newContext() came + // back carrying a cookie seeded only via the project's storageState + // config. Explicitly overriding it to undefined is the only way to get + // a genuinely blank context. + context = await browser.newContext({ storageState: undefined }) + const page = await context.newPage() + + await page.goto('/login?next=/crew') + await page.fill('#email', crewEmail) + await page.fill('#password', crewPassword) + await page.click('button[type="submit"]') + await page.waitForURL((url) => url.pathname === '/crew', { timeout: 15_000 }) + // Not waitForLoadState('networkidle') — the crew PWA's Dexie sync layer + // (lib/dexie/context.tsx) polls/syncs continuously in the background, + // so the page never actually reaches a genuinely idle network state and + // this hung for the test's entire remaining time budget every run. + // "Log out" is in the crew shell's header, present as soon as the + // authenticated layout has rendered — a real signal the page is ready. + await page.getByRole('button', { name: 'Log out' }).waitFor({ timeout: 15_000 }) + + return { + page, + cleanup: async () => { + // context.close() throwing must not skip the deletes below — same + // orphaned-data hazard this try/catch exists to close. + try { + await context!.close() + } finally { + // turnovers first — turnover_assignments/checklist_instances/ + // checklist_instance_items cascade from it (ON DELETE CASCADE). + await supabase.from('turnovers').delete().eq('id', turnover.id) + // crew_members.user_id is ON DELETE SET NULL, not CASCADE — the + // auth user delete alone would leave this row behind orphaned. + await supabase.from('crew_members').delete().eq('id', crewMember.id) + await supabase.auth.admin.deleteUser(userId) + } + }, + } + } catch (err) { + await context?.close().catch(() => {}) + try { + if (turnoverId) await supabase.from('turnovers').delete().eq('id', turnoverId) + if (crewMemberId) await supabase.from('crew_members').delete().eq('id', crewMemberId) + } catch { /* best-effort rollback — the outer error is what matters */ } + await supabase.auth.admin.deleteUser(userId).catch(() => {}) + throw err + } +} diff --git a/e2e/specs/23-booking-validation.spec.ts b/e2e/specs/23-booking-validation.spec.ts index 369f5efb..595cf1a2 100644 --- a/e2e/specs/23-booking-validation.spec.ts +++ b/e2e/specs/23-booking-validation.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from '../fixtures' import { dismissCookieBanner } from '../helpers/cookies' +import { selectOptionWhenReady } from '../helpers/forms' // Covers validation/boundary-condition gaps left by 03-bookings.spec.ts, which // only exercises the happy path. createBooking() (app/(dashboard)/bookings/actions.ts) @@ -19,17 +20,21 @@ test.describe('Booking validation', () => { test('[E2E] checkout date equal to checkin date is rejected', async ({ page }) => { await page.goto('/bookings') - await page.getByRole('button', { name: /Add Booking/i }).click() + // Dismiss before opening any dialog — the banner and the Dialog backdrop + // share z-50, and since the Dialog portal paints later in DOM order it + // sits on top; dismissing later (while a dialog is open) can land the + // click on the backdrop instead and close the dialog. + await dismissCookieBanner(page) + await page.getByRole('button', { name: /Add Booking/i }).first().click() await expect(page.getByRole('heading', { name: /Log Non-Synced Booking/i })).toBeVisible() - await page.selectOption('[name="property_id"]', { label: '[E2E] The Lakehouse' }) + await selectOptionWhenReady(page.locator('[name="property_id"]'), '[E2E] The Lakehouse') const sameDate = getFutureDate(30) await page.fill('[name="checkin_date"]', sameDate) await page.fill('[name="checkout_date"]', sameDate) await page.fill('[name="guest_name"]', '[E2E] Same Day Guest') - await dismissCookieBanner(page) await page.click('button[type="submit"]') await expect(page.getByText(/Check-out must be after check-in/i)).toBeVisible({ timeout: 8_000 }) @@ -43,24 +48,36 @@ test.describe('Booking validation', () => { // First booking — should succeed await page.goto('/bookings') - await page.getByRole('button', { name: /Add Booking/i }).click() - await page.selectOption('[name="property_id"]', { label: '[E2E] The Lakehouse' }) + // Dismiss before opening any dialog — see the earlier test in this file + // for why dismissing while a dialog is open can close the dialog instead. + await dismissCookieBanner(page) + await page.getByRole('button', { name: /Add Booking/i }).first().click() + await selectOptionWhenReady(page.locator('[name="property_id"]'), '[E2E] The Lakehouse') + // bookings_manual_dates_unique only applies WHERE source = 'manual' — + // the form's Source dropdown. + // toBeAttached() (present in the DOM) is the correct check here. + await expect(option).toBeAttached() await expect(option).toHaveText(new RegExp(`${escapeRegex(vendorName)}.*Blocked`)) // Disabled options can't be chosen through the real UI — assert the // underlying disabled attribute rather than attempting selectOption(), @@ -41,35 +51,65 @@ test.describe('Vendor compliance hard-block', () => { // must independently reject a hard-blocked vendor_id. Force-enable the // option (simulating a modified/bypassed client) and submit the real // Server Action to prove the server itself blocks it. - const vendorName = '[E2E] Hard Blocked Direct Submit' - await addVendor(page, vendorName, 'hardblocked-direct@e2e-test.invalid') - await addComplianceDocument(page, vendorName, daysAgo(35)) + const vendorName = `[E2E] Hard Blocked Direct Submit ${Date.now()}` + await addVendor(page, vendorName, `hardblocked-direct-${Date.now()}@e2e-test.invalid`) + // 46+ days past expiry is hard_blocked (supabase/migrations/ + // 20260720170645_widen_vendor_compliance_grace_period_to_45_days.sql); + // 1-45 days is only grace_period, which the server does NOT reject — + // daysAgo(35) here previously meant this test's own vendor was never + // actually hard-blocked, so the "compliance hard-blocked" assertion + // below was failing for the right reason with the wrong root cause. + await addComplianceDocument(page, vendorName, daysAgo(50)) await page.goto('/maintenance') await page.getByRole('button', { name: /New Work Order|Add Work Order|Create|New WO/i }).first().click() - await page.selectOption('[name="property_id"]', { label: '[E2E] The Lakehouse' }) - - const option = page.locator('#wo-vendor option', { hasText: vendorName }) - await option.evaluate((el: HTMLOptionElement) => { el.disabled = false }) - await page.selectOption('#wo-vendor', { label: vendorName }) + await selectOptionWhenReady(page.locator('[name="property_id"]'), '[E2E] The Lakehouse') + + // Force-enable and select in a single atomic evaluate() on the
+ // and a mobile card (VendorRow/VendorCard) for the same vendor, so scope + // to whichever one is actually visible at the current viewport. + const vendorRow = page.locator('[role="button"]').filter({ hasText: vendorName }).filter({ visible: true }).first() + await vendorRow.getByRole('link', { name: 'Details' }).click() + await page.waitForURL(/\/vendors\/[0-9a-f-]+$/, { timeout: 10_000 }) await page.getByRole('button', { name: 'Add Document' }).click() const dialog = page.getByRole('dialog') @@ -115,7 +197,6 @@ async function addComplianceDocument( await dialog.locator('#document-name').fill('[E2E] General Liability COI') await dialog.locator('#expiry-date').fill(expiryDate) - await dismissCookieBanner(page) await dialog.getByRole('button', { name: 'Add Document' }).click() await expect(dialog).not.toBeVisible({ timeout: 8_000 }) } diff --git a/e2e/specs/25-owner-portal.spec.ts b/e2e/specs/25-owner-portal.spec.ts index b0767688..1bc0c66f 100644 --- a/e2e/specs/25-owner-portal.spec.ts +++ b/e2e/specs/25-owner-portal.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from '../fixtures' import { dismissCookieBanner } from '../helpers/cookies' +import { selectOptionWhenReady } from '../helpers/forms' // Covers the public, token-only owner portal (app/owner/[token]/page.tsx) — // a money-sensitive surface (owner_transactions P&L) reachable without any @@ -22,25 +23,36 @@ import { dismissCookieBanner } from '../helpers/cookies' test.describe('Owner portal token lifecycle', () => { test('[E2E] generate link, view public portal, hide a transaction, then revoke', async ({ page, browser }) => { + // Unique per attempt — CI sets retries: 2, and a Playwright retry + // re-runs this whole test from scratch without cleaning up the owner + // the previous failed attempt already created (global teardown only + // runs once at the end of the whole suite). A static name meant a + // retry's ownerCard locator matched BOTH the stale and fresh cards, + // hitting a strict-mode violation on the first scoped click. + const ownerName = `[E2E] Portal Test Owner ${Date.now()}` + await page.goto('/owners') + // Dismiss before opening any dialog — the banner and the Dialog backdrop + // share z-50, and since the Dialog portal paints later in DOM order it + // sits on top; dismissing later (while a dialog is open) can land the + // click on the backdrop instead and close the dialog. + await dismissCookieBanner(page) const addBtn = page.getByRole('button', { name: /Add Owner|Add First Owner/i }).first() await addBtn.click() const addDialog = page.getByRole('dialog') await expect(addDialog.getByRole('heading', { name: 'Add Property Owner' })).toBeVisible() - await addDialog.locator('[name="property_id"]').selectOption({ label: '[E2E] The Lakehouse' }) - await addDialog.locator('[name="name"]').fill('[E2E] Portal Test Owner') - await dismissCookieBanner(page) + await selectOptionWhenReady(addDialog.locator('[name="property_id"]'), '[E2E] The Lakehouse') + await addDialog.locator('[name="name"]').fill(ownerName) await addDialog.getByRole('button', { name: 'Add Owner', exact: true }).click() - await expect(page.getByText('[E2E] Portal Test Owner')).toBeVisible({ timeout: 8_000 }) + await expect(page.getByText(ownerName)).toBeVisible({ timeout: 8_000 }) - const ownerCard = page.locator('.card').filter({ hasText: '[E2E] Portal Test Owner' }) + const ownerCard = page.locator('.card').filter({ hasText: ownerName }) // Record $1,500 of monthly revenue through the quick-entry field — // this always writes visible_to_owner: true (addOwnerTransaction in // app/(dashboard)/owners/actions.ts hardcodes it for manual entries). await ownerCard.locator('#monthly-revenue-amount').fill('1500') - await dismissCookieBanner(page) await ownerCard.getByRole('button', { name: 'Save', exact: true }).click() await expect(ownerCard.getByText(/\$1500\.00 recorded/)).toBeVisible({ timeout: 8_000 }) @@ -52,17 +64,32 @@ test.describe('Owner portal token lifecycle', () => { expect(portalUrl).toBeTruthy() // Visit the portal as an unauthenticated visitor — a fresh context - // without the PM's storageState. - const publicContext = await browser.newContext() + // without the PM's storageState. storageState: undefined is required, + // not optional — Playwright Test instruments every browser.newContext() + // created during a running test and silently re-applies the project's + // configured use.storageState ('e2e/.auth/pm.json') to it, so a bare + // browser.newContext() here would secretly carry the PM's session (see + // 21/22/27's identical fix — this route happens not to break today only + // because /owner/ is a proxy.ts TOKEN_ROUTE that bypasses the + // session-redirect logic entirely, not because the context is genuinely + // unauthenticated). + const publicContext = await browser.newContext({ storageState: undefined }) const publicPage = await publicContext.newPage() await publicPage.goto(portalUrl!) + // Not getByRole('heading', ...) — app/owner/[token]/page.tsx only + // renders the owner's name as an

in the multi-property branch + // (isMulti). This owner has exactly one linked property, so the

+ // is the PROPERTY name and the owner's name renders as a plain

in + // the header's top-right corner instead — confirmed against the + // actual rendered markup, not a timing issue. + await expect(publicPage.getByText(ownerName).first()).toBeVisible({ timeout: 10_000 }) + // Scope to the "Total Revenue" summary card specifically — with only // one revenue transaction and no expenses, Net Income equals the same // $1,500.00 amount, so an unscoped page-wide getByText('$1,500.00') // would match both cards (and the "+$1,500.00" line-item row) and hit // Playwright's strict-mode violation. - await expect(publicPage.getByRole('heading', { name: '[E2E] Portal Test Owner' })).toBeVisible({ timeout: 10_000 }) const revenueCard = publicPage.locator('div').filter({ hasText: 'Total Revenue' }).last() await expect(revenueCard.getByText('$1,500.00')).toBeVisible() @@ -99,11 +126,26 @@ test.describe('Owner portal token lifecycle', () => { }) test('nonexistent portal token shows a 404, not owner data', async ({ browser }) => { - const publicContext = await browser.newContext() + // See the storageState: undefined comment above — same latent + // PM-storageState leak applies here. + const publicContext = await browser.newContext({ storageState: undefined }) const publicPage = await publicContext.newPage() - const response = await publicPage.goto('/owner/00000000-0000-0000-0000-000000000000') + await publicPage.goto('/owner/00000000-0000-0000-0000-000000000000') + + // Not response?.status() — app/owner/[token]/loading.tsx wraps the page + // in an implicit Suspense boundary, so Next.js streams the route: the + // initial shell flushes with a 200 status before the async + // load-owner-portal-data.ts lookup resolves and notFound() actually + // fires deep in the tree. The HTTP status is already sent by then, so + // it stays 200 even though the correct not-found UI renders — a known + // Next.js App Router limitation with streamed notFound(), not a gap in + // this route's own auth/lookup logic (verified correct on inspection: + // it does call notFound() when no token row matches). Assert on the + // rendered content instead, since that's what actually guards against + // leaking owner data to a bogus token. + await expect(publicPage.getByRole('heading', { name: '404' })).toBeVisible({ timeout: 10_000 }) + await expect(publicPage.getByRole('heading', { name: 'This page could not be found.' })).toBeVisible() - expect(response?.status()).toBe(404) await publicContext.close() }) diff --git a/e2e/specs/26-turnover-crew-assignment.spec.ts b/e2e/specs/26-turnover-crew-assignment.spec.ts index 4963eef3..1b8f05be 100644 --- a/e2e/specs/26-turnover-crew-assignment.spec.ts +++ b/e2e/specs/26-turnover-crew-assignment.spec.ts @@ -1,58 +1,102 @@ import { test, expect } from '../fixtures' import { dismissCookieBanner } from '../helpers/cookies' +import { selectOptionWhenReady } from '../helpers/forms' +import { getServiceClient } from '../helpers/teardown' // Covers the core turnover -> crew assignment workflow, which 04-turnovers.spec.ts // never exercises (it only checks the board/calendar/filter render). Creates // its own turnover through the real "Add Turnover" UI (createManualTurnover -// in app/(dashboard)/turnovers/actions.ts) with a checkout ~200 days out so +// in app/(dashboard)/turnovers/actions.ts) with a checkout ~30 days out so // it lands in the board's "Upcoming" section (anything beyond 7 days, // per groupTurnovers() in turnover-board.tsx) — a section no other seeded // or spec-created turnover reaches, so it can be located unambiguously // without needing service-role seeding or fragile card-ordering assumptions. +// Must stay under 60 days out — app/(dashboard)/turnovers/page.tsx's Server +// Component query only fetches turnovers with checkout_datetime within +// [-7, +60] days of now, so a turnover created further out than that (this +// spec originally used +200/+201) is silently invisible to every +// subsequent page load: not just its own card missing, but the entire +// "Upcoming" section unmounting (BoardSection returns null when its +// group is empty), which is exactly the symptom this spec was hitting. // // addCrewToTurnover() flips turnover_status from pending_assignment to // assigned as soon as the first crew member is added — the assertion below // is on that exact transition (CLAUDE.md's turnover_status enum). test.describe('Turnover crew assignment', () => { - test('[E2E] assigning crew moves a turnover from pending to assigned', async ({ page }) => { - const checkoutDate = getFutureDate(200) - const checkinDate = getFutureDate(201) + test('[E2E] assigning crew moves a turnover from pending to assigned', async ({ page, ctx }) => { + const checkoutDate = getFutureDate(30) + const checkinDate = getFutureDate(31) + const marker = `[E2E] Crew Assignment Test ${Date.now()}` await page.goto('/turnovers') + // Dismiss before opening any dialog — the banner and the Dialog backdrop + // share z-50, and since the Dialog portal paints later in DOM order it + // sits on top; dismissing later (while a dialog is open) can land the + // click on the backdrop instead and close the dialog. + await dismissCookieBanner(page) await page.getByRole('button', { name: 'Add Turnover' }).click() const dialog = page.getByRole('dialog') await expect(dialog.getByRole('heading', { name: 'Add Turnover' })).toBeVisible() - await dialog.locator('[name="property_id"]').selectOption({ label: '[E2E] The Lakehouse' }) + await selectOptionWhenReady(dialog.locator('[name="property_id"]'), '[E2E] The Lakehouse') await dialog.locator('[name="checkout_date"]').fill(checkoutDate) await dialog.locator('[name="checkin_date"]').fill(checkinDate) - await dismissCookieBanner(page) + await dialog.locator('[name="notes"]').fill(marker) await dialog.getByRole('button', { name: 'Create Turnover' }).click() await expect(dialog).not.toBeVisible({ timeout: 8_000 }) + // Scope to this exact turnover by ID (TurnoverCard's root + // data-testid="turnover-card-") rather than by text/position — + // a "only card in Upcoming" assumption hit a strict-mode violation + // (2 elements) under CI load with no duplicate-insert or duplicate- + // render path found on investigation, and a notes-text filter doesn't + // work either since notes only render once the card is expanded + // (turnover-board.tsx's `{expanded && ... turnover.notes}`). The id + // is always present and unique regardless of what's rendered. + // Poll rather than a single query — this read goes over a separate + // service-role connection from the browser's own session that just + // wrote the row, and a single immediate .single() call intermittently + // found zero rows even though the insert had already succeeded + // (dialog closing proves createManualTurnover returned success). + const supabase = getServiceClient() + let turnoverId: string | undefined + await expect(async () => { + const { data } = await supabase + .from('turnovers') + .select('id') + .eq('org_id', ctx.orgId) + .eq('notes', marker) + .maybeSingle() + turnoverId = data?.id + expect(turnoverId).toBeTruthy() + }).toPass({ timeout: 8_000 }) + const card = page.getByTestId(`turnover-card-${turnoverId}`) + // "Upcoming" (groups.upcoming, defaultOpen) is the only section a - // 200-day-out turnover can land in — scope everything to it so this - // can't collide with the near-term seeded turnover from global-setup.ts - // (checkout ~2h out, lands in "Today") or any other spec's turnovers. - // BoardSection renders its heading button and its cards as siblings - // inside one wrapping div — walk from the "Upcoming" button up to that - // wrapper, then down to the single card by its root classes - // (turnover-board.tsx's TurnoverCard root: bg-card-themed rounded-xl). - const upcomingHeading = page.getByRole('button', { name: /^Upcoming/ }) - await expect(upcomingHeading).toBeVisible({ timeout: 8_000 }) - const upcomingSection = upcomingHeading.locator('xpath=..') - const card = upcomingSection.locator('.bg-card-themed.rounded-xl') + // 30-day-out turnover can land in per groupTurnovers() in + // turnover-board.tsx — confirm the section itself renders before + // asserting on the card within it. + await expect(page.getByRole('button', { name: /^Upcoming/ })).toBeVisible({ timeout: 8_000 }) + await expect(card).toBeVisible({ timeout: 8_000 }) // Status badge text comes from TURNOVER_STATUS_LABELS (lib/utils.ts): // pending_assignment -> "Needs Crew", assigned -> "Crew Assigned". await expect(card.getByText('Needs Crew')).toBeVisible({ timeout: 8_000 }) - await card.getByRole('button', { name: 'Assign' }).click() - await card.getByRole('button', { name: '[E2E] Alex Cleaner' }).click() + // exact: true — the card header's own role="button" wrapper (used for + // expand/collapse) has no explicit aria-label of its own, so its + // computed accessible name absorbs all nested text including the + // literal word "Assign" from this button, and a substring match + // resolves to both elements. + await card.getByRole('button', { name: 'Assign', exact: true }).click() + // Same reasoning as above — the header wrapper's computed name/text + // also absorbs the crew-chip text once assigned (the crew-assignment + // UI, dropdown included, is nested inside that same role="button" div). + await card.getByRole('button', { name: '[E2E] Alex Cleaner', exact: true }).click() // Crew chip appears and the status badge flips off "Needs Crew". - await expect(card.getByText('[E2E] Alex Cleaner')).toBeVisible({ timeout: 8_000 }) + await expect(card.getByText('[E2E] Alex Cleaner', { exact: true })).toBeVisible({ timeout: 8_000 }) await expect(card.getByText('Needs Crew')).not.toBeVisible() await expect(card.getByText('Crew Assigned')).toBeVisible() }) diff --git a/e2e/specs/27-crew-feedback.spec.ts b/e2e/specs/27-crew-feedback.spec.ts index 7fc9a904..7c7a5445 100644 --- a/e2e/specs/27-crew-feedback.spec.ts +++ b/e2e/specs/27-crew-feedback.spec.ts @@ -1,49 +1,140 @@ import { test, expect } from '../fixtures' +import { getServiceClient } from '../helpers/teardown' // Covers app/api/crew/feedback/route.ts — the "Send feedback" entry point on // the crew PWA home (app/crew/page.tsx), which is untested by every existing // crew-facing spec (21-work-order-offline.spec.ts and // 22-crew-logout-guard.spec.ts only cover work-order completion and the -// logout guard). This is the "Crew API routes — no helper exists" auth -// pattern documented in CLAUDE.md: getUser() -> crew_members lookup by -// user_id -> 401/403 on failure, insert via service client on success. +// logout guard). // -// Reuses the shared seeded crew login (e2e/.auth/crew.json, established in -// global-setup.ts) rather than the offline-WO spec's per-test throwaway -// crew user — this flow has no offline/Dexie interaction, so the ordinary -// crew session is sufficient and cheaper to reuse. -test.use({ storageState: 'e2e/.auth/crew.json' }) - +// This spec creates its OWN disposable crew login per test (mirroring +// 21-work-order-offline.spec.ts) rather than reusing the shared +// e2e/.auth/crew.json — 22-crew-logout-guard.spec.ts's tests all click +// "Log out"/"Log Out Anyway", which call supabase.auth.signOut() and +// revoke that session server-side. crew.json is a static snapshot never +// rewritten after global-setup captures it, so once any earlier-run spec +// file signs that shared account out, every later file reusing the +// snapshot gets a dead session — exactly what made both tests here fail +// deterministically. A throwaway per-test account has no such shared-state +// hazard. test.describe('Crew feedback', () => { + // loginAsFreshCrew() below does a createUser + crew_members Admin API + // round trip plus a full page navigation/login before a test's own + // assertions even start — under CI load that alone can eat most of the + // default 30s per-test budget, so these tests were reaching the correct + // destination (login genuinely succeeded) and still failing on "Test + // timeout of 30000ms exceeded." + test.describe.configure({ timeout: 60_000 }) + + test('[E2E] crew can submit feedback from the crew home screen', async ({ ctx, browser }) => { + const { page, cleanup } = await loginAsFreshCrew(ctx.orgId, browser) + try { + await page.getByRole('button', { name: 'Send feedback' }).click() - test('[E2E] crew can submit feedback from the crew home screen', async ({ page }) => { - await page.goto('/crew') - await page.waitForLoadState('networkidle') + const dialog = page.getByRole('dialog') + await expect(dialog.getByText('Send feedback')).toBeVisible() - await page.getByRole('button', { name: 'Send feedback' }).click() + await dialog.locator('textarea').fill('[E2E] A checklist item description was hard to read on my phone.') + await dialog.getByRole('button', { name: 'Submit', exact: true }).click() - const dialog = page.getByRole('dialog') - await expect(dialog.getByText('Send feedback')).toBeVisible() + await expect(dialog.getByText('Thank you!')).toBeVisible({ timeout: 8_000 }) - await dialog.locator('textarea').fill('[E2E] A checklist item description was hard to read on my phone.') - await dialog.getByRole('button', { name: 'Submit', exact: true }).click() + await dialog.getByRole('button', { name: 'Done' }).click() + await expect(dialog).not.toBeVisible() + } finally { + await cleanup() + } + }) - await expect(dialog.getByText('Thank you!')).toBeVisible({ timeout: 8_000 }) + test('[E2E] submitting empty feedback is a no-op — send stays disabled', async ({ ctx, browser }) => { + const { page, cleanup } = await loginAsFreshCrew(ctx.orgId, browser) + try { + await page.getByRole('button', { name: 'Send feedback' }).click() + const dialog = page.getByRole('dialog') + await expect(dialog.getByText('Send feedback')).toBeVisible() - await dialog.getByRole('button', { name: 'Done' }).click() - await expect(dialog).not.toBeVisible() + const submitBtn = dialog.getByRole('button', { name: 'Submit', exact: true }) + await expect(submitBtn).toBeDisabled() + } finally { + await cleanup() + } }) - test('[E2E] submitting empty feedback is a no-op — send stays disabled', async ({ page }) => { - await page.goto('/crew') - await page.waitForLoadState('networkidle') +}) - await page.getByRole('button', { name: 'Send feedback' }).click() - const dialog = page.getByRole('dialog') - await expect(dialog.getByText('Send feedback')).toBeVisible() +async function loginAsFreshCrew(orgId: string, browser: import('@playwright/test').Browser) { + const supabase = getServiceClient() - const submitBtn = dialog.getByRole('button', { name: 'Submit', exact: true }) - await expect(submitBtn).toBeDisabled() + const crewEmail = `e2e-crew-feedback-${Date.now()}@e2e-test.invalid` + const crewPassword = 'E2E-Crew-Feedback-Test-1!' + const { data: created, error: createErr } = await supabase.auth.admin.createUser({ + email: crewEmail, password: crewPassword, email_confirm: true, }) + if (createErr || !created.user) throw new Error(`Failed to create crew test user: ${createErr?.message}`) + const userId = created.user.id -}) + // Everything past this point can throw (crew_members insert, context + // creation, the login flow itself) — without this catch, a failure here + // would propagate and skip the `cleanup` this function never got to + // return, orphaning the just-created auth user in the E2E project. + let context: import('@playwright/test').BrowserContext | undefined + try { + const { error: cmErr } = await supabase.from('crew_members').insert({ + org_id: orgId, + user_id: userId, + name: '[E2E] Crew Feedback Tester', + role: 'general', + is_active: true, + invite_accepted_at: new Date().toISOString(), + }) + if (cmErr) throw new Error(`Failed to create crew_members row: ${cmErr.message}`) + + // Fresh, unauthenticated context — the default `page` fixture carries the + // PM's storageState, which would put the crew layout's PM-guard redirect + // in the way of a crew login (see 21-work-order-offline.spec.ts). + // + // storageState: undefined is required, not optional — Playwright Test + // instruments every browser.newContext() created during a running test + // (not just the fixture-provided `context`/`page`) and silently + // re-applies the project's configured use.storageState + // ('e2e/.auth/pm.json') to it. A bare browser.newContext() here is + // therefore secretly PM-authenticated: page.goto('/login?next=/crew') + // 307s straight past the login form to /crew, which the crew layout's + // PM-guard then 307s again to /ops, so the next line's page.fill times + // out waiting for an #email that was never on that page. + context = await browser.newContext({ storageState: undefined }) + const page = await context.newPage() + + await page.goto('/login?next=/crew') + await page.fill('#email', crewEmail) + await page.fill('#password', crewPassword) + await page.click('button[type="submit"]') + await page.waitForURL((url) => url.pathname === '/crew', { timeout: 15_000 }) + // Not waitForLoadState('networkidle') — the crew PWA's Dexie sync layer + // (lib/dexie/context.tsx) polls/syncs continuously in the background, + // so the page never actually reaches a genuinely idle network state and + // this hung for the test's entire remaining time budget every run. + // "Send feedback" is what every test in this file interacts with next + // (app/crew/page.tsx renders it unconditionally once mounted), so + // waiting for it directly is both a real readiness signal and exactly + // what's needed. + await page.getByRole('button', { name: 'Send feedback' }).waitFor({ timeout: 15_000 }) + + return { + page, + cleanup: async () => { + // context.close() throwing must not skip deleteUser — that's the + // same orphaned-user hazard this try/catch exists to close. + try { + await context!.close() + } finally { + await supabase.auth.admin.deleteUser(userId) + } + }, + } + } catch (err) { + await context?.close().catch(() => {}) + await supabase.auth.admin.deleteUser(userId).catch(() => {}) + throw err + } +} diff --git a/lib/vendors/compliance.ts b/lib/vendors/compliance.ts index 2fbc7105..ec0455cb 100644 --- a/lib/vendors/compliance.ts +++ b/lib/vendors/compliance.ts @@ -1,8 +1,9 @@ import type { SupabaseClient } from '@supabase/supabase-js' -// vendor_compliance_status (migration 20260606051120) computes compliance_status -// live off vendor_compliance_documents.expiry_date — 'hard_blocked' means the -// vendor's oldest expired document has been expired 31+ days. Per CLAUDE.md this +// vendor_compliance_status (migration 20260606051120, grace period widened to +// 45 days by 20260720170645) computes compliance_status live off +// vendor_compliance_documents.expiry_date — 'hard_blocked' means the vendor's +// oldest expired document has been expired 46+ days. Per CLAUDE.md this // means "no WO assignment": every path that assigns a vendor to a work order // (manual create/edit, bulk assign, suggestion accept, maintenance-schedule // auto-assign) must check this server-side — the disabled option in the New/Edit @@ -23,4 +24,4 @@ export async function isVendorHardBlocked( } export const VENDOR_HARD_BLOCKED_ERROR = - 'This vendor is compliance hard-blocked (a required document has been expired 31+ days) and cannot be assigned to a work order. Update their compliance documents first.' + 'This vendor is compliance hard-blocked (a required document has been expired 46+ days) and cannot be assigned to a work order. Update their compliance documents first.' diff --git a/playwright.config.ts b/playwright.config.ts index 70aa0836..dfd08b25 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -45,14 +45,30 @@ export default defineConfig({ }, ], - // Start local dev server automatically when not using a remote BASE_URL + // Start the app server automatically unless E2E_BASE_URL points at a + // remote deployment. Unset E2E_BASE_URL means the localhost default — + // the previous `!process.env.E2E_BASE_URL?.startsWith('http://localhost')` + // check made `undefined` fall into the no-webServer branch, so the first + // armed CI run (which doesn't set E2E_BASE_URL) had no server at all and + // died at global-setup with ERR_CONNECTION_REFUSED. ...( - !process.env.E2E_BASE_URL?.startsWith('http://localhost') ? {} : { + process.env.E2E_BASE_URL && !process.env.E2E_BASE_URL.startsWith('http://localhost') ? {} : { webServer: { - command: 'pnpm run dev', + // CI has no dev server to reuse — build once and serve the + // production build (dev-mode compile-on-navigate is also slow + // enough to blow per-test timeouts in CI). + command: process.env.CI ? 'pnpm run build && pnpm run start' : 'pnpm run dev', url: 'http://localhost:3000', reuseExistingServer: !process.env.CI, - timeout: 120_000, + timeout: process.env.CI ? 600_000 : 120_000, + // Playwright's default is to discard the web server's own + // stdout/stderr entirely, which meant every Server Action's + // console.error (createWorkOrder, etc.) was invisible in CI job + // logs — every "why did this mutation silently fail" investigation + // this session had to work around that blind spot instead of just + // reading the actual server error. Pipe it into the CI step output. + stdout: 'pipe', + stderr: 'pipe', }, } ), diff --git a/proxy.ts b/proxy.ts index bc42ff5c..2daa8988 100644 --- a/proxy.ts +++ b/proxy.ts @@ -205,7 +205,18 @@ export async function proxy(request: NextRequest) { // entry are unaffected (rateLimiterForPathname returns null for them). const tokenRouteLimiter = rateLimiterForPathname(pathname) - if (tokenRouteLimiter) { + // Skip the network round trip entirely when Upstash isn't configured + // (e.g. CI, or a local dev env without the KV addon) instead of letting + // the underlying @upstash/redis client attempt — and internally retry — + // a fetch against an undefined URL before the catch below fails open. + // That retry/backoff was measured adding ~4.3s to EVERY request on a + // rate-limited route, which was tight enough to blow several e2e tests' + // short (5-10s) post-mutation assertion timeouts on /work-orders/[token] + // and /owner/[token]. + const upstashConfigured = + !!process.env.upstash_fieldstay_KV_REST_API_URL && !!process.env.upstash_fieldstay_KV_REST_API_TOKEN + + if (tokenRouteLimiter && upstashConfigured) { const ip = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ?? request.headers.get('x-real-ip') ?? diff --git a/scripts/check-db-invariants.mjs b/scripts/check-db-invariants.mjs new file mode 100644 index 00000000..63377fba --- /dev/null +++ b/scripts/check-db-invariants.mjs @@ -0,0 +1,150 @@ +#!/usr/bin/env node +/** + * FieldStay — DB invariant check (structural enforcement, Tier 3). + * + * The ESLint rules and unit/guardrails/ suite police the CODE; this script + * polices the DATABASE — the invariants CLAUDE.md states but no code-side + * check can see. It calls public.db_invariant_report() (see + * supabase/migrations/20260724131000_db_invariant_report.sql) and fails on: + * + * 1. any public table without RLS enabled + * 2. any RLS-enabled table with ZERO policies that is not in the + * SERVICE_ROLE_ONLY_TABLES allowlist below (deny-all is a valid stance + * only when it's deliberate) — the allowlist is shrink-only: a stale + * entry is itself a failure, same ratchet rule as the Tailwind baseline + * 3. any FK column without a covering index + * 4. any anon grant on a public table (all were revoked by + * 20260724130000_revoke_stale_anon_table_grants.sql; new ones are drift) + * + * Runs in the CI `db-invariants` job against the DEDICATED E2E PROJECT + * (docs/E2E_SETUP.md) — never production; CI must not hold prod credentials. + * Both projects receive every migration, so schema-level invariants verified + * on the E2E project hold for production by construction. Grant state is the + * one exception (it isn't purely migration-driven — Supabase default + * privileges differ per project), which is why check 4 demands ZERO rather + * than diffing a baseline. + * + * Self-disarms with a CI warning annotation when the E2E secrets are absent, + * mirroring the e2e job's gate. + */ + +const url = process.env.NEXT_PUBLIC_SUPABASE_URL +const key = process.env.SUPABASE_SERVICE_ROLE_KEY + +if (!url || !key) { + console.log( + '::warning title=DB invariant gate UNARMED::NEXT_PUBLIC_SUPABASE_URL / ' + + 'SUPABASE_SERVICE_ROLE_KEY are not configured, so database invariants ' + + '(RLS on every table, FK indexes, anon-grant hygiene) were NOT checked. ' + + 'Follow docs/E2E_SETUP.md to arm the gate.' + ) + process.exit(0) +} + +const PROD_PROJECT_REF = 'vpmznjktllhmmbfnxuvk' +if (url.includes(PROD_PROJECT_REF)) { + console.error( + 'Refusing to run: NEXT_PUBLIC_SUPABASE_URL points at the PRODUCTION ' + + 'Supabase project. CI must use the dedicated E2E project — see ' + + 'docs/E2E_SETUP.md.' + ) + process.exit(1) +} + +// Tables that are deliberately service-role-only: RLS enabled with zero +// policies = clients fully locked out, all access via createServiceClient(). +// Shrink-only — if a table here gains policies (or is dropped), remove it. +const SERVICE_ROLE_ONLY_TABLES = new Set([ + 'pending_integration_links', + 'pending_oauth_authorizations', + 'processed_webhooks', +]) + +const res = await fetch(new URL('/rest/v1/rpc/db_invariant_report', url), { + method: 'POST', + headers: { + apikey: key, + authorization: `Bearer ${key}`, + 'content-type': 'application/json', + }, + body: '{}', +}) + +if (!res.ok) { + // Status code only — the response body is network-controlled data and + // doesn't belong in CI logs (Sonar S5145 log-injection rule). + console.error(`db_invariant_report RPC failed: HTTP ${res.status}`) + console.error( + 'Has supabase/migrations/20260724131000_db_invariant_report.sql been applied to the E2E project?' + ) + process.exit(1) +} + +const report = await res.json() +const failures = [] + +// ── 1. RLS on every table ───────────────────────────────────────────────── +if (report.tables_without_rls.length > 0) { + failures.push( + `Tables WITHOUT row level security: ${report.tables_without_rls.join(', ')}\n` + + ' Every table gets ALTER TABLE ... ENABLE ROW LEVEL SECURITY in the same ' + + 'migration that creates it (CLAUDE.md, Critical Security Rules #2).' + ) +} + +// ── 2. Policy-less tables vs the service-role-only allowlist ────────────── +const noPolicies = new Set(report.tables_without_policies) +const unlisted = [...noPolicies].filter((t) => !SERVICE_ROLE_ONLY_TABLES.has(t)) +const staleAllowlist = [...SERVICE_ROLE_ONLY_TABLES].filter((t) => !noPolicies.has(t)) + +if (unlisted.length > 0) { + failures.push( + `RLS-enabled tables with ZERO policies (deny-all): ${unlisted.join(', ')}\n` + + ' Either write real SELECT/INSERT/UPDATE/DELETE policies, or — if the ' + + 'table is genuinely service-role-only — add it to SERVICE_ROLE_ONLY_TABLES ' + + 'in scripts/check-db-invariants.mjs with that justification.' + ) +} +if (staleAllowlist.length > 0) { + failures.push( + `Stale SERVICE_ROLE_ONLY_TABLES entries (table now has policies, or was dropped): ${staleAllowlist.join(', ')}\n` + + ' Remove them from scripts/check-db-invariants.mjs — the allowlist only shrinks.' + ) +} + +// ── 3. Unindexed FK columns ─────────────────────────────────────────────── +if (report.unindexed_fk_columns.length > 0) { + const rows = report.unindexed_fk_columns + .map((f) => ` ${f.table}(${f.columns}) — ${f.constraint}`) + .join('\n') + failures.push( + `Foreign-key columns with no covering index:\n${rows}\n` + + ' Add CREATE INDEX IF NOT EXISTS in the same migration as the FK — an ' + + 'unindexed FK sequential-scans the referencing table on every parent ' + + 'DELETE/UPDATE.' + ) +} + +// ── 4. anon grants ──────────────────────────────────────────────────────── +if (report.anon_grant_tables.length > 0) { + failures.push( + `Tables with anon grants: ${report.anon_grant_tables.join(', ')}\n` + + ' All anon table grants were revoked by ' + + '20260724130000_revoke_stale_anon_table_grants.sql — no client reads ' + + 'tables unauthenticated (public surfaces go through the service client ' + + 'server-side). Revoke the grant; if a genuinely anon-readable table is ' + + 'ever introduced, that is a security-review conversation, not an allowlist edit.' + ) +} + +// ── Verdict ─────────────────────────────────────────────────────────────── +if (failures.length > 0) { + console.error(`DB invariant check FAILED (${failures.length} finding${failures.length === 1 ? '' : 's'}):\n`) + for (const f of failures) console.error(`✗ ${f}\n`) + process.exit(1) +} + +console.log( + 'DB invariants OK — RLS on every table, no unexpected deny-all tables, ' + + 'all FK columns indexed, zero anon grants.' +) diff --git a/supabase/migrations/20260724130000_revoke_stale_anon_table_grants.sql b/supabase/migrations/20260724130000_revoke_stale_anon_table_grants.sql new file mode 100644 index 00000000..af4b7c8d --- /dev/null +++ b/supabase/migrations/20260724130000_revoke_stale_anon_table_grants.sql @@ -0,0 +1,26 @@ +-- Revoke stale anon grants on public tables (defense-in-depth). +-- +-- Audit findings (2026-07-24): 20 production tables carried anon grants +-- (audit_events, organization_members, owner_transactions, work_orders, ...) +-- left over from how they were originally created — none are needed. Every +-- unauthenticated surface in the app (guidebook, owner portal, media kit, +-- vendor-connect) reads server-side through the service client, the auth +-- pages call supabase.auth.* only, and every browser-side table read runs +-- with an authenticated session. RLS was already blocking anon on org-scoped +-- tables (auth.uid() IS NULL fails every policy), but tables with open read +-- policies (e.g. integration_providers' "Anyone can read active providers") +-- were world-readable through the REST API with just the public anon key. +-- +-- Grants to `authenticated` are untouched — RLS depends on them +-- (see 20260710200000_grant_authenticated_missing_tables.sql). +-- +-- Enforced going forward by scripts/check-db-invariants.mjs (CI +-- db-invariants job): any future table that picks up an anon grant fails CI. + +REVOKE ALL ON ALL TABLES IN SCHEMA public FROM anon; +REVOKE ALL ON ALL SEQUENCES IN SCHEMA public FROM anon; + +-- Stop future tables/sequences created by this role from getting anon +-- grants via default privileges (no-op if the defaults never included anon). +ALTER DEFAULT PRIVILEGES IN SCHEMA public REVOKE ALL ON TABLES FROM anon; +ALTER DEFAULT PRIVILEGES IN SCHEMA public REVOKE ALL ON SEQUENCES FROM anon; diff --git a/supabase/migrations/20260724130500_index_unindexed_fk_columns.sql b/supabase/migrations/20260724130500_index_unindexed_fk_columns.sql new file mode 100644 index 00000000..3fe576a8 --- /dev/null +++ b/supabase/migrations/20260724130500_index_unindexed_fk_columns.sql @@ -0,0 +1,25 @@ +-- Add covering indexes for the six FK columns that had none +-- (2026-07-24 audit). An unindexed FK makes every DELETE/UPDATE on the +-- referenced table sequential-scan the referencing table to enforce the +-- constraint (and ON DELETE SET NULL/CASCADE actions pay it too). +-- +-- Enforced going forward by scripts/check-db-invariants.mjs (CI +-- db-invariants job): any new FK column without a covering index fails CI. + +CREATE INDEX IF NOT EXISTS idx_org_inventory_catalog_platform_item + ON public.org_inventory_catalog (platform_catalog_item_id); + +CREATE INDEX IF NOT EXISTS idx_org_maintenance_catalog_items_platform_item + ON public.org_maintenance_catalog_items (platform_catalog_item_id); + +CREATE INDEX IF NOT EXISTS idx_organizations_bedroom_room_template + ON public.organizations (bedroom_room_template_id); + +CREATE INDEX IF NOT EXISTS idx_organizations_bathroom_room_template + ON public.organizations (bathroom_room_template_id); + +CREATE INDEX IF NOT EXISTS idx_pending_oauth_authorizations_provider + ON public.pending_oauth_authorizations (provider_id); + +CREATE INDEX IF NOT EXISTS idx_vendor_assignment_outcomes_property + ON public.vendor_assignment_outcomes (property_id); diff --git a/supabase/migrations/20260724131000_db_invariant_report.sql b/supabase/migrations/20260724131000_db_invariant_report.sql new file mode 100644 index 00000000..e01338d3 --- /dev/null +++ b/supabase/migrations/20260724131000_db_invariant_report.sql @@ -0,0 +1,82 @@ +-- db_invariant_report(): structural-enforcement Tier 3 backstop. +-- +-- Returns a jsonb report of schema-level invariants that no code-side check +-- (ESLint, guardrail tests) can see. Called by scripts/check-db-invariants.mjs +-- from the CI db-invariants job against the E2E project — the checks needs +-- pg_catalog, which the REST API can't reach directly, so the query lives +-- here as a SECURITY DEFINER function callable only by service_role. +-- +-- Sections: +-- tables_without_rls — public tables with RLS disabled (must be empty) +-- tables_without_policies — RLS on but zero policies (deny-all; allowed +-- only for the deliberately service-role-only +-- tables allowlisted in the CI script) +-- unindexed_fk_columns — FK columns with no covering index (leading +-- prefix of a valid index). Partial indexes +-- count: this codebase deliberately indexes +-- nullable FKs as (col) WHERE col IS NOT NULL, +-- and FK-enforcement probes are always col = $1, +-- which implies that predicate. +-- anon_grant_tables — tables with any anon grant (must be empty; +-- see 20260724130000_revoke_stale_anon_table_grants.sql) + +CREATE OR REPLACE FUNCTION public.db_invariant_report() +RETURNS jsonb +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $$ + SELECT jsonb_build_object( + 'tables_without_rls', ( + SELECT coalesce(jsonb_agg(t.tablename ORDER BY t.tablename), '[]'::jsonb) + FROM pg_catalog.pg_tables t + WHERE t.schemaname = 'public' AND NOT t.rowsecurity + ), + 'tables_without_policies', ( + SELECT coalesce(jsonb_agg(t.tablename ORDER BY t.tablename), '[]'::jsonb) + FROM pg_catalog.pg_tables t + WHERE t.schemaname = 'public' + AND NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_policies p + WHERE p.schemaname = 'public' AND p.tablename = t.tablename + ) + ), + 'unindexed_fk_columns', ( + SELECT coalesce( + jsonb_agg( + jsonb_build_object('table', f.tbl, 'constraint', f.conname, 'columns', f.cols) + ORDER BY f.tbl, f.conname + ), + '[]'::jsonb + ) + FROM ( + SELECT + (SELECT cl.relname FROM pg_catalog.pg_class cl WHERE cl.oid = c.conrelid) AS tbl, + c.conname, + (SELECT string_agg(a.attname, ',' ORDER BY k.ord) + FROM unnest(c.conkey) WITH ORDINALITY k(attnum, ord) + JOIN pg_catalog.pg_attribute a + ON a.attrelid = c.conrelid AND a.attnum = k.attnum) AS cols + FROM pg_catalog.pg_constraint c + WHERE c.contype = 'f' + AND c.connamespace = 'public'::regnamespace + AND NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_index i + WHERE i.indrelid = c.conrelid + AND i.indisvalid + AND (i.indkey::int2[])[0:cardinality(c.conkey)-1] @> c.conkey + ) + ) f + ), + 'anon_grant_tables', ( + SELECT coalesce(jsonb_agg(DISTINCT g.table_name::text ORDER BY g.table_name::text), '[]'::jsonb) + FROM information_schema.role_table_grants g + WHERE g.table_schema = 'public' AND g.grantee = 'anon' + ) + ); +$$; + +-- Introspection-only, but there's no reason clients should ever call it. +REVOKE EXECUTE ON FUNCTION public.db_invariant_report() FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.db_invariant_report() TO service_role; diff --git a/supabase/migrations/20260724150000_fix_handle_new_user_search_path.sql b/supabase/migrations/20260724150000_fix_handle_new_user_search_path.sql new file mode 100644 index 00000000..4872ef0b --- /dev/null +++ b/supabase/migrations/20260724150000_fix_handle_new_user_search_path.sql @@ -0,0 +1,28 @@ +-- Fix handle_new_user's missing search_path pin. +-- +-- The original migration created this trigger function with no SET +-- search_path and an unqualified `INSERT INTO profiles`. The auth admin +-- connection that fires it (GoTrue, on POST /auth/v1/admin/users and +-- signups) does not have `public` on its search path on newer Supabase +-- projects, so every user creation failed with +-- `relation "profiles" does not exist` — rolling back the whole signup. +-- +-- Production was hotfixed directly at some point (its live definition +-- already pins search_path and qualifies public.profiles) but the fix +-- never landed as a migration, so the E2E project — built purely from +-- migration files — reproduced the original bug on its first dashboard +-- "add user" attempt (2026-07-24, see auth logs). This migration is the +-- hotfix as a file: a no-op on production, the fix everywhere else. + +CREATE OR REPLACE FUNCTION public.handle_new_user() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path TO 'public' +AS $function$ +BEGIN + INSERT INTO public.profiles (id, full_name) + VALUES (NEW.id, NEW.raw_user_meta_data->>'full_name'); + RETURN NEW; + END; + $function$; diff --git a/supabase/migrations/20260724160000_capture_prod_drift_functions_columns_seed.sql b/supabase/migrations/20260724160000_capture_prod_drift_functions_columns_seed.sql new file mode 100644 index 00000000..969ba328 --- /dev/null +++ b/supabase/migrations/20260724160000_capture_prod_drift_functions_columns_seed.sql @@ -0,0 +1,506 @@ +-- Capture schema drift from direct-SQL changes made on production. +-- +-- A full prod-vs-E2E diff (2026-07-24) found that a number of production +-- objects had been created or edited directly in the dashboard without a +-- matching migration file, so any environment built purely from +-- supabase/migrations/ (the E2E project) diverged: +-- +-- * 12 functions differed and 1 was missing — including the RLS-critical +-- helpers get_user_org_ids / is_org_member / get_crew_member_id +-- * crew_feedback's timestamp column was renamed created_at → submitted_at +-- on prod only +-- * inventory_template_items.par_level was widened integer → numeric on +-- prod only +-- * the 23-row maintenance_catalog_items platform seed existed on prod only +-- +-- (Policy drift found in the same diff is already covered by the existing +-- 20260723090000/20260723120000 migrations; the handle_new_user drift was +-- captured separately in 20260724150000.) +-- +-- This migration is the prod-authoritative state as a file: a no-op where +-- the state already matches (both live projects at time of writing), the +-- fix everywhere else. Everything here is idempotent. + +-- ── Column drift ───────────────────────────────────────────────────────── + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_schema='public' AND table_name='crew_feedback' AND column_name='created_at') + AND NOT EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_schema='public' AND table_name='crew_feedback' AND column_name='submitted_at') + THEN + ALTER TABLE public.crew_feedback RENAME COLUMN created_at TO submitted_at; + END IF; + + IF EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_schema='public' AND table_name='inventory_template_items' + AND column_name='par_level' AND data_type <> 'numeric') + THEN + ALTER TABLE public.inventory_template_items ALTER COLUMN par_level TYPE numeric; + END IF; +END $$; + +-- ── Function drift (prod definitions, verbatim) ────────────────────────── + +CREATE OR REPLACE FUNCTION public.get_user_org_ids() + RETURNS SETOF uuid + LANGUAGE sql + STABLE SECURITY DEFINER + SET search_path TO 'public' +AS $function$ + SELECT org_id FROM organization_members + WHERE user_id = auth.uid() + AND invite_accepted_at IS NOT NULL +$function$; + +CREATE OR REPLACE FUNCTION public.is_org_member(p_org_id uuid, p_roles member_role[] DEFAULT NULL::member_role[]) + RETURNS boolean + LANGUAGE sql + STABLE SECURITY DEFINER + SET search_path TO 'public' +AS $function$ + SELECT EXISTS ( + SELECT 1 + FROM organization_members + WHERE org_id = p_org_id + AND user_id = auth.uid() + AND invite_accepted_at IS NOT NULL + AND ( + p_roles IS NULL -- no role restriction: any member passes + OR role = ANY(p_roles) -- explicit role match + OR role = 'owner'::member_role -- org owner always has full access + ) + ) +$function$; + +CREATE OR REPLACE FUNCTION public.get_crew_member_id() + RETURNS uuid + LANGUAGE sql + STABLE SECURITY DEFINER + SET search_path TO 'public' +AS $function$ + SELECT id FROM crew_members WHERE user_id = auth.uid() LIMIT 1 +$function$; + +CREATE OR REPLACE FUNCTION public.assign_wo_number() + RETURNS trigger + LANGUAGE plpgsql + SET search_path TO 'public' +AS $function$ +BEGIN + IF NEW.wo_number IS NULL THEN + NEW.wo_number := next_wo_number(NEW.org_id); + END IF; + RETURN NEW; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.next_wo_number(p_org_id uuid) + RETURNS text + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_year smallint := EXTRACT(YEAR FROM NOW())::smallint; + v_number integer; +BEGIN + INSERT INTO wo_number_counters (org_id, last_number, current_year) + VALUES (p_org_id, 1, v_year) + ON CONFLICT (org_id) DO UPDATE + SET last_number = CASE + WHEN wo_number_counters.current_year = v_year + THEN wo_number_counters.last_number + 1 + ELSE 1 + END, + current_year = v_year + RETURNING last_number INTO v_number; + RETURN 'WO-' || v_year || '-' || LPAD(v_number::text, 4, '0'); +END; +$function$; + +CREATE OR REPLACE FUNCTION public.cleanup_expired_oauth_states() + RETURNS void + LANGUAGE sql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ + DELETE FROM public.oauth_states WHERE expires_at < now(); +$function$; + +CREATE OR REPLACE FUNCTION public.set_comm_log_updated_at() + RETURNS trigger + LANGUAGE plpgsql + SET search_path TO 'public' +AS $function$ +BEGIN + -- communication_logs has no updated_at; this is a no-op placeholder + -- included for schema consistency + RETURN NEW; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.sync_wo_actual_cost() + RETURNS trigger + LANGUAGE plpgsql + SET search_path TO 'public' +AS $function$ +DECLARE + v_wo_id uuid; + v_item_count integer; + v_total numeric(10,2); +BEGIN + v_wo_id := COALESCE(NEW.work_order_id, OLD.work_order_id); + SELECT COUNT(*), COALESCE(SUM(line_total), 0) + INTO v_item_count, v_total + FROM work_order_line_items + WHERE work_order_id = v_wo_id; + IF v_item_count > 0 THEN + UPDATE work_orders SET actual_cost = v_total WHERE id = v_wo_id; + END IF; + RETURN COALESCE(NEW, OLD); +END; +$function$; + +CREATE OR REPLACE FUNCTION public.protect_checklist_instances_crew_columns() + RETURNS trigger + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + is_pm boolean; +BEGIN + SELECT EXISTS ( + SELECT 1 FROM organization_members + WHERE user_id = auth.uid() + AND org_id = NEW.org_id + AND role IN ('admin'::member_role, 'manager'::member_role, 'owner'::member_role) + ) INTO is_pm; + + IF is_pm THEN + RETURN NEW; + END IF; + + -- Not a PM on this org — a legitimate crew write only ever changes + -- completed_at/completed_by_crew_id. Reject anything else outright + -- rather than silently reverting it, so a client-side bug surfaces + -- immediately instead of masking a write that silently didn't apply. + IF NEW.org_id IS DISTINCT FROM OLD.org_id + OR NEW.turnover_id IS DISTINCT FROM OLD.turnover_id + OR NEW.template_id IS DISTINCT FROM OLD.template_id + OR NEW.template_snapshot IS DISTINCT FROM OLD.template_snapshot + OR NEW.status IS DISTINCT FROM OLD.status + OR NEW.started_at IS DISTINCT FROM OLD.started_at + OR NEW.section_photo_path IS DISTINCT FROM OLD.section_photo_path + THEN + RAISE EXCEPTION 'crew members may only update completed_at and completed_by_crew_id on checklist_instances'; + END IF; + + RETURN NEW; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.purge_expired_audit_events() + RETURNS jsonb + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_financial_cutoff timestamptz := NOW() - INTERVAL '7 years'; + v_operational_cutoff timestamptz := NOW() - INTERVAL '3 years'; + v_financial_deleted integer; + v_operational_deleted integer; +BEGIN + -- Financial records: billing and owner transaction audit events (7-year IRS/GAAP retention) + DELETE FROM audit_events + WHERE created_at < v_financial_cutoff + AND (action LIKE 'billing.%' OR action LIKE 'owner.transaction.%'); + GET DIAGNOSTICS v_financial_deleted = ROW_COUNT; + + -- Operational records: all other audit events (3-year SOC2/GDPR retention) + DELETE FROM audit_events + WHERE created_at < v_operational_cutoff + AND action NOT LIKE 'billing.%' + AND action NOT LIKE 'owner.transaction.%'; + GET DIAGNOSTICS v_operational_deleted = ROW_COUNT; + + RETURN jsonb_build_object( + 'financial_deleted', v_financial_deleted, + 'operational_deleted', v_operational_deleted, + 'run_at', NOW() + ); +END; +$function$; + +CREATE OR REPLACE FUNCTION public.apply_crew_score_recompute() + RETURNS jsonb + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_scored_count integer := 0; + v_crew_count integer := 0; + v_capacity_count integer := 0; +BEGIN + -- Atomically claim + score outcomes in one statement: candidates are + -- computed, claimed (scored_at set), and folded into a per-crew delta, all + -- within a single UPDATE ... FROM chain. A retry after any failure here + -- rolls back entirely (functions run in the caller's transaction) and sees + -- the exact same unscored candidates again — no partial-apply/double-count + -- window like the two-phase JS loop this replaces. + WITH candidates AS ( + SELECT + ao.id, + ao.crew_member_id, + ao.was_missed, + ( + NOT ao.was_missed + AND ao.completed_at IS NOT NULL + AND t.checkin_datetime IS NOT NULL + AND ao.completed_at > t.checkin_datetime + ) AS was_late, + ao.pm_rating + FROM assignment_outcomes ao + LEFT JOIN turnovers t ON t.id = ao.turnover_id + WHERE ao.scored_at IS NULL + AND (ao.completed_at IS NOT NULL OR ao.was_missed = true) + ), + scored AS ( + UPDATE assignment_outcomes ao + SET scored_at = now(), + was_late = candidates.was_late + FROM candidates + WHERE ao.id = candidates.id + RETURNING ao.id, candidates.crew_member_id, candidates.was_missed, candidates.was_late, candidates.pm_rating + ), + deltas AS ( + SELECT + crew_member_id, + SUM( + CASE + WHEN was_missed THEN -0.15 + ELSE + (CASE WHEN was_late THEN -0.05 ELSE 0.02 END) + + COALESCE((pm_rating - 3) * 0.03, 0) + END + ) AS delta + FROM scored + GROUP BY crew_member_id + ), + updated_crew AS ( + UPDATE crew_members cm + SET reliability_score = GREATEST(0, LEAST(1, COALESCE(cm.reliability_score, 1.0) + deltas.delta)), + updated_at = now() + FROM deltas + WHERE cm.id = deltas.crew_member_id + RETURNING cm.id + ) + SELECT + (SELECT count(*) FROM scored), + (SELECT count(*) FROM updated_crew) + INTO v_scored_count, v_crew_count; + + -- Capacity score: pure recompute-from-scratch every run (not a delta), so + -- naturally idempotent/retry-safe on its own — no claim step needed. + WITH capacity AS ( + SELECT + crew_member_id, + count(*) FILTER (WHERE property_bedrooms >= 4) AS large_count, + count(*) AS total_count + FROM assignment_outcomes + WHERE property_bedrooms IS NOT NULL + AND completed_at IS NOT NULL + GROUP BY crew_member_id + HAVING count(*) >= 3 + ), + updated_capacity AS ( + UPDATE crew_members cm + SET capacity_score = ROUND((capacity.large_count::numeric / capacity.total_count), 3), + updated_at = now() + FROM capacity + WHERE cm.id = capacity.crew_member_id + RETURNING cm.id + ) + SELECT count(*) FROM updated_capacity INTO v_capacity_count; + + RETURN jsonb_build_object( + 'scored', v_scored_count, + 'crewUpdated', v_crew_count, + 'capacityUpdated', v_capacity_count + ); +END; +$function$; + +CREATE OR REPLACE FUNCTION public.claim_pending_integration_link(p_pending_link_token text, p_user_id uuid) + RETURNS TABLE(provider_id text, external_user_id text, org_id uuid) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public', 'vault' +AS $function$ +DECLARE + v_pending public.pending_integration_links%ROWTYPE; + v_org_id uuid; + v_old_secret_id uuid; + v_old_refresh_secret_id uuid; +BEGIN + SELECT * INTO v_pending + FROM public.pending_integration_links + WHERE pending_link_token = p_pending_link_token + AND expires_at > now() + FOR UPDATE; + + IF NOT FOUND THEN + RETURN; + END IF; + + PERFORM pg_advisory_xact_lock(hashtextextended('integration_connection:' || p_user_id::text || ':' || v_pending.provider_id, 0)); + + SELECT om.org_id INTO v_org_id + FROM public.organization_members om + WHERE om.user_id = p_user_id + AND om.invite_accepted_at IS NOT NULL + ORDER BY om.created_at ASC + LIMIT 1; + + -- Capture whatever secrets an existing connection row currently points at, + -- before the upsert below overwrites those columns with the pending link's. + SELECT vault_secret_id, refresh_token_vault_secret_id + INTO v_old_secret_id, v_old_refresh_secret_id + FROM public.integration_connections + WHERE user_id = p_user_id AND provider_id = v_pending.provider_id; + + INSERT INTO public.integration_connections + (user_id, org_id, provider_id, external_user_id, vault_secret_id, refresh_token_vault_secret_id, scope, metadata, status) + VALUES + (p_user_id, v_org_id, v_pending.provider_id, v_pending.external_user_id, v_pending.vault_secret_id, + v_pending.refresh_token_vault_secret_id, v_pending.scope, v_pending.metadata, 'active') + ON CONFLICT (user_id, provider_id) DO UPDATE + SET vault_secret_id = EXCLUDED.vault_secret_id, + refresh_token_vault_secret_id = EXCLUDED.refresh_token_vault_secret_id, + external_user_id = EXCLUDED.external_user_id, + scope = EXCLUDED.scope, + metadata = EXCLUDED.metadata, + status = 'active', + org_id = COALESCE(public.integration_connections.org_id, EXCLUDED.org_id), + reconnect_email_sent_at = NULL, + updated_at = now(); + + -- Now safe to delete the superseded secrets — the row no longer references them. + IF v_old_secret_id IS NOT NULL AND v_old_secret_id IS DISTINCT FROM v_pending.vault_secret_id THEN + DELETE FROM vault.secrets WHERE id = v_old_secret_id; + END IF; + IF v_old_refresh_secret_id IS NOT NULL AND v_old_refresh_secret_id IS DISTINCT FROM v_pending.refresh_token_vault_secret_id THEN + DELETE FROM vault.secrets WHERE id = v_old_refresh_secret_id; + END IF; + + DELETE FROM public.pending_integration_links WHERE id = v_pending.id; + + RETURN QUERY SELECT v_pending.provider_id, v_pending.external_user_id, v_org_id; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.store_integration_token(p_user_id uuid, p_provider_id text, p_access_token text, p_external_user_id text, p_scope text DEFAULT NULL::text, p_metadata jsonb DEFAULT '{}'::jsonb) + RETURNS uuid + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public', 'vault' +AS $function$ +DECLARE + v_secret_id uuid; + v_existing_secret_id uuid; + v_connection_exists boolean := false; + v_org_id uuid; + v_secret_name text := p_provider_id || '_token_' || p_user_id::text; +BEGIN + PERFORM pg_advisory_xact_lock(hashtextextended('integration_connection:' || p_user_id::text || ':' || p_provider_id, 0)); + + SELECT org_id INTO v_org_id + FROM public.organization_members + WHERE user_id = p_user_id + AND invite_accepted_at IS NOT NULL + ORDER BY created_at ASC + LIMIT 1; + + SELECT vault_secret_id, true + INTO v_existing_secret_id, v_connection_exists + FROM public.integration_connections + WHERE user_id = p_user_id + AND provider_id = p_provider_id; + + -- No connection row pointing at a secret — there may still be an orphaned + -- one sitting in vault.secrets under this deterministic name (row deleted + -- without going through revoke_integration_token). Reuse it if so. + IF v_existing_secret_id IS NULL THEN + SELECT id INTO v_existing_secret_id FROM vault.secrets WHERE name = v_secret_name; + END IF; + + IF v_connection_exists THEN + IF v_existing_secret_id IS NOT NULL THEN + PERFORM vault.update_secret(v_existing_secret_id, p_access_token); + v_secret_id := v_existing_secret_id; + ELSE + v_secret_id := vault.create_secret(p_access_token, v_secret_name, 'OAuth access token for ' || p_provider_id); + END IF; + + UPDATE public.integration_connections + SET vault_secret_id = v_secret_id, + external_user_id = p_external_user_id, + scope = p_scope, + metadata = COALESCE(metadata, '{}'::jsonb) || p_metadata, + status = 'active', + org_id = COALESCE(org_id, v_org_id), + reconnect_email_sent_at = NULL, + updated_at = now() + WHERE user_id = p_user_id + AND provider_id = p_provider_id; + ELSE + IF v_existing_secret_id IS NOT NULL THEN + PERFORM vault.update_secret(v_existing_secret_id, p_access_token); + v_secret_id := v_existing_secret_id; + ELSE + v_secret_id := vault.create_secret(p_access_token, v_secret_name, 'OAuth access token for ' || p_provider_id); + END IF; + + INSERT INTO public.integration_connections + (user_id, org_id, provider_id, external_user_id, vault_secret_id, scope, metadata) + VALUES + (p_user_id, v_org_id, p_provider_id, p_external_user_id, v_secret_id, p_scope, p_metadata); + END IF; + + RETURN v_secret_id; +END; +$function$; + +-- ── Platform seed data drift: maintenance_catalog_items (23 rows) ──────── +-- Seeded on prod outside migrations; captured here so a from-migrations +-- rebuild has the catalog. ON CONFLICT keeps this a no-op on prod. + +INSERT INTO public.maintenance_catalog_items (id, name, category, is_active, sort_order, description, asset_category, suggested_recurrence) VALUES +('030c9c10-375a-4805-9e3e-ff259502cee8', 'Pool service', 'water_features', true, 1, 'Water chemistry check, skimming, filter backwash, and equipment inspection. Seasonal frequency.', 'pool', 'weekly'), +('d3fe917e-6560-40a3-bc5c-c5f5cefb28d6', 'Hot tub / jacuzzi service', 'water_features', true, 2, 'Water chemistry balance, filter clean, inspect jets, heater, and cover.', 'hot_tub', 'monthly'), +('ef8ab673-2d7b-4df1-84e8-afb26e017dbb', 'Fountain service', 'water_features', true, 3, 'Clean basin, check pump, inspect for algae or debris buildup.', 'fountain', 'monthly'), +('36e22017-94e7-4708-9051-4dba05396c06', 'Dock / boat slip maintenance', 'water_features', true, 4, 'Inspect boards, cleats, lines, and lighting. Check for rot or structural issues.', 'dock', 'semi_annual'), +('7e808aeb-2304-4b28-9b60-7edac622ca9d', 'Chimney sweep & inspection', 'heating_fuel', true, 5, 'Clean flue, inspect cap, damper, firebox, and smoke chamber. Required for wood-burning fireplaces.', 'chimney', 'annual'), +('5a2c1dfb-ab5f-41a3-9fd3-995cb2481abf', 'Gas fireplace cleaning & inspection', 'heating_fuel', true, 6, 'Clean burner assembly, inspect igniter, thermocouple, and glass seal.', 'fireplace', 'annual'), +('1ec4a608-a863-4207-8e6c-46f6e119596e', 'Propane tank inspection', 'heating_fuel', true, 7, 'Check tank level, inspect regulator, supply lines, and connections for leaks.', 'propane', 'annual'), +('d5ddafa9-f17f-4a92-b216-c15ef12bdf02', 'Generator service & load test', 'heating_fuel', true, 8, 'Change oil and filter, test under load, inspect fuel system and battery connections.', 'generator', 'annual'), +('606942b8-150f-447d-8976-54dde8c2b2b7', 'Snow removal', 'outdoor_grounds', true, 9, 'Seasonal. Driveway, walkways, and all entry areas. Adjust frequency to snowfall.', 'grounds', 'weekly'), +('dff16c6c-86dc-41b5-aec0-4b7a6c89ea1a', 'Irrigation / sprinkler system service & winterization', 'outdoor_grounds', true, 10, 'Spring startup and fall winterization. Check heads, valves, timer, and inspect for leaks.', 'irrigation', 'semi_annual'), +('e3d83308-b353-4de5-937f-924b003c9f8d', 'Fence repair & staining', 'outdoor_grounds', true, 11, 'Inspect for loose boards, damaged posts, or rot. Restain or seal wood fencing as needed.', 'exterior', 'annual'), +('103902b9-840e-4bed-be13-7d35196c2d67', 'Driveway sealing', 'outdoor_grounds', true, 12, 'Clean surface and apply sealant to asphalt or concrete driveway. Fills cracks and extends life.', 'exterior', 'annual'), +('b9b66785-6ac4-4e80-84c5-979104d9525c', 'Outdoor kitchen cleaning & service', 'outdoor_grounds', true, 13, 'Deep clean grill grates, burners, and all surfaces. Inspect and test gas connections.', 'outdoor_kitchen', 'semi_annual'), +('28a903cd-220f-4715-a2d7-099c83e1a154', 'Water softener service', 'systems', true, 14, 'Refill salt, clean resin tank, verify settings and regeneration cycle are correct.', 'plumbing', 'semi_annual'), +('835fc06a-a99e-48f2-b7b8-59caa413befc', 'Sump pump inspection', 'systems', true, 15, 'Test pump operation by pouring water into pit, check float switch, inspect discharge line.', 'plumbing', 'semi_annual'), +('d8c3e08a-7904-462f-a0a6-c038ed8d2c1c', 'Well pump inspection', 'systems', true, 16, 'Inspect pressure tank, test pump output, check water quality, and inspect electrical connections.', 'plumbing', 'annual'), +('05ef5891-d49c-43d4-ae6e-677608fa0b40', 'Septic tank pumping', 'systems', true, 17, 'Pump every 3–5 years typical; inspect annually. Frequency depends on occupancy and tank size.', 'septic', 'annual'), +('0d19e11c-99ad-4f0d-adee-4c354bc99b44', 'Solar panel cleaning & inspection', 'systems', true, 18, 'Clean panels with soft brush and water, inspect mounting hardware and wiring, check inverter output.', 'solar', 'semi_annual'), +('384e6653-a856-4f5b-9c35-194a13d74d9c', 'EV charging station inspection', 'systems', true, 19, 'Test charge output at full load, inspect cable and connector for wear, check for firmware updates.', 'electrical', 'annual'), +('ee281f68-293c-47ff-8121-36d82deab9bf', 'Garage door service & lubrication', 'systems', true, 20, 'Lubricate rollers, hinges, and springs with lithium grease. Test auto-reverse safety function.', 'garage', 'annual'), +('97faad59-cdb1-4341-94b0-5dccfa013b9a', 'Elevator / stair lift service', 'systems', true, 21, 'Certified technician required. Inspect all safety systems, lubricate drive mechanism.', 'elevator', 'annual'), +('00be4f2d-f499-4597-b106-c0985e007dc7', 'Security camera system check', 'systems', true, 22, 'Verify all cameras are operational and recording. Clean lenses, check storage capacity, test motion alerts.', 'security', 'quarterly'), +('7fd52aaf-1711-4df1-a64d-2c59f6cc49f9', 'Sauna / steam room service', 'amenities', true, 23, 'Clean interior surfaces and benches, inspect heater and stones, check door seal and thermometer accuracy.', 'sauna', 'monthly') +ON CONFLICT (id) DO NOTHING; diff --git a/supabase/migrations/20260725043000_add_quote_requested_to_wo_status.sql b/supabase/migrations/20260725043000_add_quote_requested_to_wo_status.sql new file mode 100644 index 00000000..7831aaf3 --- /dev/null +++ b/supabase/migrations/20260725043000_add_quote_requested_to_wo_status.sql @@ -0,0 +1,10 @@ +-- wo_status.quote_requested exists on the production project but was never +-- captured in a tracked migration (added out-of-band at some point after +-- the original 20260524165615_fieldstay_v1_extensions_enums.sql, which only +-- defines pending/assigned/in_progress/completed/cancelled). The E2E project +-- (created fresh from migrations alone) never got it, so every query +-- filtering work_orders.status with 'quote_requested' in the list — e.g. +-- app/(dashboard)/maintenance/page.tsx's board query — throws "invalid +-- input value for enum wo_status" there, silently (the query's `error` is +-- never checked), making every work order vanish from the board. +ALTER TYPE wo_status ADD VALUE IF NOT EXISTS 'quote_requested' AFTER 'pending';