Dashboard had a defense-in-depth Navigate to /login when sessionId is
null. This fires on initial render before the session is set, causing
E2E tests to fail (they wait for the impersonation banner which never
renders because Dashboard redirected away).
Revert to main-branch behavior: show "Please sign in" message instead
of redirecting. The CustomerPortal-level redirect is sufficient.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes E2E race condition where setSession and setInitComplete are batched
by React concurrent rendering, causing redirect to fire before session
is set. The sessionAttempted flag tracks "did we try" so redirect only
fires when there was NO attempt, not when the state update is pending.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The portal/me endpoint is only called in the client dev user flow
(devUser.type === 'client'), NOT in the impersonation flow which uses
the sessionId param. Removing this mock eliminates potential interference.
The POST /api/portal/dev-session mock is dead code in impersonation tests
since the fixture seeds devUser.type=staff, which skips that code path.
Removed it to eliminate potential interference.
Also changed portal/me mock pattern from 'GET **/api/portal/me' to
'**/api/portal/me**' to ensure it matches correctly regardless of
how Playwright interprets the URL pattern syntax.
The API returns a flat ImpersonationSession object. CustomerPortal.tsx
reads s.id directly from the response. My previous fix incorrectly
wrapped the mock in { session: {...} }, causing s.id to be undefined
and setSession() to never fire.
This reverts the mock structure to be flat, matching the actual API
response format from portal.ts line 516.
The mock returned { id, client } but CustomerPortal.tsx expects
{ session: { id, client } }. This caused setSession to never be called,
leading to redirect to /login and test timeouts.
Also seed dev user in localStorage for impersonation tests to
ensure getDevUser() returns a known state.
QA identified that impersonation.spec.ts mocks impersonation
session endpoints but not portal session endpoints. When
CustomerPortal.tsx validates the session it calls GET /api/portal/me
which fails without a mock, causing the redirect to fire and tests
to fail.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- CustomerPortal.tsx: add initComplete state to track async session
initialization. After init completes with no valid session, redirect:
staff dev users → /admin, all others → /login
- Dashboard.tsx: change !sessionId fallback from dead-end UI to
<Navigate to="/login" replace /> (defense-in-depth)
- All 85 web unit tests pass
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The hardcoded DEV_STAFF_ID (all zeros) did not exist in the staff
table, causing a foreign-key violation and 500 error. Now falls back
to the demo-manager (KNOWN_STAFF_ID from seed) or any active staff
record instead.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Adds pwa-192x192.png, pwa-512x512.png, and favicon.svg to the web
public directory. These are referenced by the VitePWA plugin manifest
and were causing 404 errors on every page load.
cc @cpfarhood
Co-Authored-By: Paperclip <noreply@paperclip.ing>
When a client user selects their account from the dev login selector,
the portal previously had no way to establish an authenticated session —
it only checked for a ?sessionId= URL param (used by the real staff
impersonation flow). This caused the portal to always show "Hi, Guest".
Changes:
- POST /api/portal/dev-session: new endpoint (auth-disabled only) that
creates an impersonation session for a given clientId, using a fixed
dev staff ID to avoid conflicts with the one-active-session-per-staff
rule in the real impersonation flow. Sessions are long-lived (24h).
- CustomerPortal: on mount, after checking for ?sessionId=, also check
for a dev client user in localStorage and call /api/portal/dev-session
to obtain a session. This mirrors the real impersonation flow so all
existing portal API calls (which require X-Impersonation-Session-Id)
work without modification.
cc @cpfarhood
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Add active=true filter to all 3 superUserCount queries in staff.ts
(revoke, deactivate, delete) so inactive super users aren't counted,
preventing false positives when checking the last-super-user guardrail.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Fixes seed script crash — both onConflictDoUpdate calls on clients table now use schema.clients.id (PK) as conflict target instead of non-unique schema.clients.email. Email added to set clause for both call sites.
Resolves GRO-298. Unblocks GRO-290, GRO-295, GRO-297.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Before: ~5% probabilistic pending invoices meant UAT couldn't reliably
find billing test data. Shedward was blocked from testing Pay Now flows.
After: deterministic 5 UAT clients (uat-alpha through uat-echo) each get
a completed appointment + pending invoice on every seed run. Client
names and emails documented in Shedward AGENTS.md for direct access.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Backend:
- PATCH /api/staff/:id now accepts optional isSuperUser field
- Only super users can change isSuperUser (403 otherwise)
- Revoke (isSuperUser=false) blocked if target is last super user (400)
- Deactivate (active=false) blocked if target is last super user (400)
- DELETE /:id blocked if target is last super user (400)
- New GET /api/staff/me returns current authenticated staff record
Frontend (Staff.tsx):
- Super User column in staff table with badge indicator
- Grant/Revoke SU button visible only to super users
- Last-super-user guardrail disables revoke button with tooltip
- API errors shown inline below table header
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Each CI build now produces an immutable tag (pr-N-sha7 or
YYYY.MM.DD-sha7) so that docker/build-push-action cache-from
type=gha cannot cross-contaminate between commits.
Previously the shared pr-N tag caused GHA layer cache to reuse
stale JS bundles from earlier builds of the same PR.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GitHub App token pushes do not trigger pull_request workflow events,
blocking CI on bot-authored PRs. Add workflow_dispatch to allow manual
CI runs via: gh workflow run ci.yml --ref <branch>
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit also includes GRO-287 fixes:
- PasswordChange: add stateful form with password-match validation
- ReportCards: replace window.location.reload() with refetch via useRef
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Remove readOnly guard from Pay Now button and PaymentModal in BillingPayments.
The readOnly guard was too broad — it hid the Pay Now button during staff
impersonation sessions, making it impossible for staff to collect payments.
Other readOnly guards (Remove payment method, Autopay toggle) remain intact.
Co-authored-by: groombook-engineer[bot] <269742240+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Fixes GRO-261 — billing portal session header mismatch and response format bug.
- x-session-id → X-Impersonation-Session-Id in BillingPayments.tsx and Dashboard.tsx
- Handle bare array response from /api/portal/invoices
Co-Authored-By: Paperclip <noreply@paperclip.ing>
cc @cpfarhood
Unauthenticated users saw a blank screen because the needsSetup null-guard
fired before the LoginPage render check. needsSetup stays null for
unauthenticated users since the setup-check effect early-returns when
!session. Now the login check runs first so users see the login page.
Co-authored-by: Flea Flicker <flea-flicker@groombook.io>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Scrubs McBarkley (CEO) <ceo-bot@groombook.farh.net>
Closes GRO-261 — Pay Now button on Billing page now opens a payment modal with invoice selection and simulated payment flow.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(portal): wire Rebook Now button to navigate to booking wizard (GRO-265)
The "Rebook Now" button on the Report Card detail view had no click
handler. Now navigates to /admin/book with pet info pre-filled via URL
params (petName, serviceName). Button text changed from "Book Now" to
"Rebook Now" per the bug report.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(book): pre-fill form from URL params to ensure React state is set
Add useSearchParams to read URL parameters (e.g., ?clientName=Jane)
and sync them to the BookingBody state on mount via useEffect.
This ensures validation checks React state, not empty initial state.
Fixes GRO-255
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(book): add inline validation for date input format (GRO-266)
Date picker now shows a clear error when the value doesn't match
YYYY-MM-DD, instead of silently failing with a browser console warning.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(portal): wire Rebook Now button + clean .js artifacts (GRO-265)
Cherry-picked from contaminated PR #160:
- ReportCards.tsx: Rebook Now button navigates to /admin/book with pet info
- Book.tsx: pre-fill form from URL params (GRO-255)
- Book.tsx: inline date validation (GRO-266)
Also removes compiled .js artifacts (Book.js, ReportCards.js)
that were incorrectly committed.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
---------
Co-authored-by: groombook-ci[bot] <ci@groombook.bot>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Merges PR #158 — fixes critical production login bug.
- Adds apps/web/.env.production with VITE_API_URL= (empty)
- Prevents localhost:3000 from being baked into the prod bundle
- Auth client now uses relative URLs through the gateway
GRO-258 | QA: Lint Roller ✓ | CTO: The Dogfather ✓
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Manual workflow_dispatch trigger to promote a tested image tag
to production by creating an infra PR. No auto-merge — UAT sign-off
required before prod deploy.
Co-authored-by: groombook-ci[bot] <ci@groombook.bot>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-ceo[bot] <269735724+groombook-ceo[bot]@users.noreply.github.com>
Prevents localhost:3000 from being baked into the production bundle.
Vite automatically loads .env.production for prod builds, which
with VITE_API_URL= explicitly sets the var to empty string so
auth-client.ts uses relative URLs (?? "" fallback).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Convert raw inserts to upserts (ON CONFLICT DO UPDATE) for:
- staff: upsert on email (unique constraint)
- services: upsert on id (deterministic UUID)
- clients: upsert on email (unique constraint)
- pets: upsert on id (deterministic UUID)
This fixes the duplicate key violation when re-running the seed
script against an existing database (e.g., after schema migrations
or test restarts).
Note: appointments, invoices, visit logs still use raw inserts
and would need DELETE-before-insert for full idempotency. Those
tables use deterministic UUIDs so a second seed run without
prior DELETE would still fail. This is scoped to the immediate
staff email constraint violation reported.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat: add cd job to update groombook/infra image tags on main merge (GRO-178)
- Adds `cd` job that runs after `docker` on main branch pushes only
- Uses tibdex/github-app-token to get infra repo push token
- Updates image tags in apps/groombook/base/{api,web,migrate-job,seed-job}.yaml
- Opens auto-merge PR on groombook/infra
Trade-off: deploy-dev continues using kubectl set image directly for PR
previews (speed over full GitOps auditability for short-lived previews).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix: correct --head branch format and use --enable-auto-merge (GRO-178)
CTO review fixes:
- Remove bogus "groombook-engineer[bot]:" prefix from --head — gh pr
create does not use owner:branch syntax when pushing from a cloned
repo; just the branch name is needed
- Replace invalid --auto-merges-branch=main flag with
--enable-auto-merge (valid gh flag that activates repo auto-merge)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix: broaden annotation sed pattern, fix PR body link, remove error swallowing (GRO-178)
CTO review remaining fixes:
- Annotation sed pattern: broaden [a-f0-9]* to [a-zA-Z0-9-]* since
migrate-job and seed-job use "groXXX" suffixes (e.g. "2026.03.28-gro177")
which contain non-hex letters
- PR body link: fix /d50d9792/issues/GRO-178 → /GRO/issues/GRO-178
- Remove error swallowing: "|| echo" was hiding PR creation failures;
let the step fail naturally so CI catches it
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(cd): split --enable-auto-merge into separate merge command
CTO review fix: gh pr create does not support --enable-auto-merge flag.
Split into two commands: create PR, then gh pr merge with --auto --merge.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
---------
Co-authored-by: groombook-engineer[bot] <3141748+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Flea Flicker <flea-flicker@groombook.io>
Co-authored-by: groombook-engineer[bot] <269742240+groombook-engineer[bot]@users.noreply.github.com>
## Changes
- Replace toNodeHandler with auth.handler(c.req.raw) sub-app mount for Hono compatibility
- Add /api/auth/ path skip in authMiddleware and resolveStaffMiddleware
- Add OIDC_INTERNAL_BASE env var for split-horizon (hairpin NAT) URL resolution
- Replace render-time signIn.social() with LoginPage component (fixes redirect loop)
- Change auth-client baseURL to relative (empty string) for deployed environments
- Add POST /api/portal/appointments/:id/reschedule endpoint with session auth
- Add RescheduleFlow modal, PetForm component, and wire Dashboard/Appointments UI
## CTO Note
Auth fix is P0-critical. Portal mock data (UAT blocker) predates this PR and is tracked separately in GRO-218.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Add boolean is_super_user column (default false) to staff table.
Update Staff interface in shared types.
Mark first manager as super user in both seed modes.
Update test fixtures to include isSuperUser field.
Co-authored-by: groombook-ci[bot] <ci@groombook.bot>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
All CI checks pass. Verified on groombook.dev.farh.net. Second approval from groombook-ceo[bot] per GRO-171.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(db): add Better-Auth schema tables (GRO-118)
Add user, session, account, and verification tables required by
Better-Auth's Drizzle adapter. Add nullable userId FK on staff to
link business identity to auth identity. Fix test fixtures and
factory to include the new column.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(api): mount Better-Auth handler at /api/auth/** (GRO-118)
- Import toNodeHandler from better-auth/node and auth from ./lib/auth.js
- Mount Better-Auth HTTP handler before auth middleware block
- Handles OAuth callbacks, sign-in/sign-out, session management
- Supports GET/POST/PUT/PATCH/DELETE/OPTIONS methods
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(api): replace JWT auth with Better-Auth session validation (GRO-118)
- Replace jose/jwtVerify with auth.api.getSession()
- Session token validated via cookie/header, DB-backed
- jwtPayload.sub now = Better-Auth user ID (not OIDC sub)
- Dev mode bypass preserved; production guard against AUTH_DISABLED preserved
- rbac.ts and tests updated in subsequent tasks
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(api): update resolveStaffMiddleware for Better-Auth userId (GRO-118)
- Remove JwtPayload import; use inline type in AppEnv
- Production and dev mode lookups now use staff.userId (not oidcSub)
- Backward compat: jwtPayload.sub now = Better-Auth user ID
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* chore(api): remove jose and openid-client deps (GRO-118)
- Remove unused jose and openid-client packages
- Regenerate pnpm lockfile
- Pre-existing Zod type errors resolved (1 remaining: JwtPayload in test)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(api): remove stale JwtPayload import from impersonation test (GRO-118)
auth.ts no longer exports JwtPayload — replace with inline type.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* test(api): update RBAC tests for Better-Auth userId (GRO-128)
- Add userId field to mock staff records (MANAGER, RECEPTIONIST, GROOMER)
- Update jwtPayload.sub to use userId instead of oidcSub in test helpers
- Update dev mode X-Dev-User-Id header to use userId
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* chore(api): upgrade zod to v4 with v3 compat layer (GRO-131)
- Bump zod from ^3.24.1 to ^4.3.6
- Bump @hono/zod-validator from ^0.4.3 to ^0.7.6
- Update all 12 route files to import from "zod/v3" compat layer
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(api): add Better-Auth configuration (GRO-118)
Exports the better-auth() instance configured with:
- Drizzle PG adapter
- genericOAuth plugin for Authentik OIDC
- 7-day session with 5-min cookie cache
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(web): install Better-Auth client and create config (GRO-118)
- Add better-auth to apps/web/package.json dependencies
- Create apps/web/src/lib/auth-client.ts with createAuthClient config
- Export signIn, signOut, useSession from the client
- Add vite-env.d.ts for Vite client types
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat(web): use Better-Auth session state in App.tsx (GRO-126)
Add useSession hook to check Better-Auth session for production auth.
Redirect to Authentik sign-in when no session in production mode.
Dev mode flow (DevLoginSelector) preserved.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(web): scope devFetch interceptor to dev mode only (GRO-127)
* fix(api): validate BETTER_AUTH_SECRET and fix lockfile specifier (GRO-118)
- Add startup validation for BETTER_AUTH_SECRET when auth is enabled
- Fix pnpm-lock.yaml typescript specifier mismatch (^5.9.3 → ^5.7.3)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(web): mock authDisabled=true in App.test.tsx to fix CI failures
App.test.tsx "App navigation" tests were failing because the beforeEach
set authDisabled=false (production mode), which triggers the Better Auth
useSession() path. Since useSession() was not mocked in tests, the
component rendered null instead of the admin nav.
Now uses authDisabled=true + dev user in localStorage for those tests,
bypassing the Better Auth dependency while still testing the nav render.
Also removes duplicate App.test.js (compiled artifact).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(e2e): set authDisabled=true in fixtures to bypass Better Auth
The App.tsx production auth path calls signIn.social() when
authDisabled=false, causing E2E tests to render blank. The fixtures
must mock authDisabled=true so the dev login selector is used instead.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(e2e): add dev/config, dev/users, and branding mocks to navigation.spec.ts
Playwright matches routes in last-registered-first-served order, so the
catch-all /api/** handler was overwriting the authDisabled: true fixture.
Added specific handlers before the catch-all to ensure auth config,
user list, and branding responses are properly shaped.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(web): gate DevLoginSelector on API authDisabled, not import.meta.env.DEV
Move the DevLoginSelector rendering check from import.meta.env.DEV to the
API-driven authDisabled state, after the loading guard. Simplify the redirect
condition to remove the now-redundant pathname exception.
Fixes E2E login tests that were failing because DevLoginSelector was never
rendered in Docker production builds where import.meta.env.DEV is false.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(db): add missing migration journal entries 0012-0017
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(web): import App.tsx (not App.js) in App.test.tsx (#137)
* fix(web): mock /api/auth/get-session in Dev login selector test
The "redirects to /login when auth is disabled and no user selected" test
fails because useSession() from better-auth/react calls /api/auth/get-session
which wasn't mocked, causing sessionLoading to stay true indefinitely.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(web): import App.tsx (not App.js) in test to get authDisabled bypass
The Dev login selector test was importing the compiled App.js instead of
the source App.tsx. App.js has different logic (uses import.meta.env.DEV
instead of API-based authDisabled) and doesn't implement the
sessionLoading bypass needed for tests to pass.
Also applied the rawSession/rawSessionLoading refactor in App.tsx that
bypasses useSession result when authDisabled=true.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(web): use extensionless import for App in test
The `.tsx` extension in the import path is not allowed without
`allowImportingTsExtensions` (TS5097). Use extensionless `../App`
which resolves correctly via moduleResolution: "bundler".
Co-Authored-By: Paperclip <noreply@paperclip.ing>
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
* fix(auth): dev login resolve staff by id, not userId
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(rbac): fallback lookup for staff records predating Better-Auth userId (#140)
GRO-153: /api/staff returned 403 for all staff because resolveStaffMiddleware
looked up by staff.userId (Better-Auth ID) but dev login sent staff.id (PK),
and existing staff records had userId=NULL.
Changes:
- resolveStaffMiddleware: try userId first, fall back to staff.id (dev mode)
- resolveStaffMiddleware: try userId first, fall back to oidcSub (production)
- GET /api/dev/users: include userId field for DevLoginSelector
- DevLoginSelector: send userId (not staff.id) as X-Dev-User-Id
- Migration 0018: backfill userId for known demo staff
Co-authored-by: groombook-engineer[bot] <groombook-engineer@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Barkley Trimsworth <barkley@groombook.farh.net>
* fix(rbac): allow all staff roles to READ /api/staff
GRO-156 follow-up: RBAC middleware was blocking groomer/receptionist
from GET /api/staff. The QA review found 403 with "role groomer is not
permitted" after PR #140 deployment.
Fix: split the /staff/* guard — GET requests allow all roles
(groomer, receptionist, manager); write operations remain manager-only.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-engineer[bot] <269742240+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Flea Flicker <flea-flicker@paperclip.ing>
Co-authored-by: groombook-engineer[bot] <groombook-engineer@users.noreply.github.com>
Co-authored-by: Barkley Trimsworth <barkley@groombook.farh.net>
groombook-engineer[bot]