feat(region-pages): bring f3-region-pages into the monorepo#302
feat(region-pages): bring f3-region-pages into the monorepo#302pstaylor-patrick wants to merge 13 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (113)
📝 WalkthroughWalkthroughAdds a new Next.js “region-pages” app with Drizzle schemas/migrations, ingest APIs and scripts, UI components, tests, Supabase local setup, Dockerfile, and Terraform for Cloud Run with load balancer and secrets. ChangesRegion Pages Full Stack Initialization
Sequence Diagram(s)sequenceDiagram
autonumber
participant Cron as QStash Cron (scheduler)
participant API as Next.js API /api/ingest
participant WH as Warehouse DB
participant APP as App DB (Supabase)
participant Slack as Slack API
Cron->>API: POST /api/ingest (Bearer CRON_SECRET)
API->>WH: Connectivity check (SELECT 1)
API->>APP: pruneRegions(), pruneWorkouts()
API->>WH: Fetch updated regions/events
API->>APP: seedRegions(), seedWorkouts(), enrichRegions()
API->>APP: Persist ingestRuns/seedRuns
API->>Slack: Send summary (success/failure)
API-->>Cron: 200 JSON stats or 500 error
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
pstaylor-patrick
left a comment
There was a problem hiding this comment.
Servant Code Review — CLEAN (0 blockers)
Reviewed the monorepo-integration surface (the bulk of the diff is a lift-and-shift of the already-in-production f3-region-pages app + large cruft deletions, which is grandfathered). CI is green across build/lint/typecheck/test/format. No blockers. A few intentional trade-offs are flagged below as suggestions / follow-ups for the human pass — none block merge.
| Severity | Count |
|---|---|
| Blockers | 0 |
| Warnings | 0 |
| Suggestions | 4 |
Suggestions / documented follow-ups
S1 — ESLint type-aware rules downgraded to warnings (apps/region-pages/eslint.config.mjs)
prefer-nullish-coalescing, no-unsafe-*, restrict-template-expressions, no-base-to-string are set to warn to land the lift-and-shift without behavior-risky mass edits (e.g. 27 ||→??). Tracked with a TODO(region-pages). Tighten back to error in a follow-up once the ||→?? conversions are reviewed individually.
S2 — Build env resilience swallows DB errors (src/lib/env.ts, src/utils/fetchWorkoutLocations.ts)
loadEnvConfig warns instead of throwing under SKIP_ENV_VALIDATION (set only by the build script), and fetchRegionBySlug / fetchRegionsWithWorkoutCounts now fall back to null/[] on DB error (matching the existing cached fetchers). This makes the CI build succeed without DB secrets while the Docker deploy build still renders all ~541 pages. Trade-off: at runtime a DB outage degrades to empty content rather than erroring — consistent with the existing cache layer, but worth confirming that's the desired UX.
S3 — Migration journal baseline (drizzle/migrations/0000_init.sql)
The 11 historical migrations were squashed to a single 0000_init. The region-pages deploy does not auto-migrate, so prod is unaffected — but the Neon __drizzle_migrations journal still records the old chain. Baseline the journal (mark 0000_init as applied) before any future manual db:migrate against prod.
S4 — Tooling divergence (first pass)
region-pages keeps jest (vs the monorepo's vitest) and bun-based db:* scripts. Acceptable for this initial migration; consider aligning to vitest / tsx in a follow-up.
Automated review (Servant). The app's runtime code is grandfathered from the standalone repo; this pass focused on the monorepo integration. Not a merge gate — PR remains draft for human review.
There was a problem hiding this comment.
Pull request overview
Integrates the standalone f3-region-pages Next.js application into the f3-nation monorepo under apps/region-pages, including build/runtime tooling (Turbo/Docker), database ingest/seeding utilities, and Cloud Run Terraform deployment scaffolding.
Changes:
- Added the
apps/region-pagesapp (Next.js pages, components, utils, drizzle schema/migrations, scripts, tests). - Added monorepo-oriented build configuration (Turbo-friendly Dockerfile, Next standalone output, eslint/ts/jest configs).
- Added Cloud Run Terraform stack + secret wiring for GCP deployment, and updated commitlint scopes.
Reviewed changes
Copilot reviewed 103 out of 117 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| commitlint.config.mjs | Adds region-pages to commit scope enum. |
| apps/region-pages/tsconfig.test.json | Adds test-specific TS config for Jest. |
| apps/region-pages/tsconfig.json | Adds app TypeScript configuration for Next.js in monorepo. |
| apps/region-pages/tailwind.config.ts | Adds Tailwind content/theme config for the app. |
| apps/region-pages/supabase/config.toml | Adds Supabase local dev configuration. |
| apps/region-pages/supabase/.gitignore | Ignores Supabase local artifacts and env files. |
| apps/region-pages/src/utils/workoutSorting.ts | Adds workout “next occurrence” sorting logic. |
| apps/region-pages/src/utils/safeHtml.tsx | Adds HTML-to-React sanitization utility for notes rendering. |
| apps/region-pages/src/utils/regionEvents.ts | Adds region event helpers (slugs, formatting, filtering). |
| apps/region-pages/src/utils/mapUtils.ts | Adds map URL + bounds/zoom calculations. |
| apps/region-pages/src/utils/locationUtils.ts | Adds city/state extraction helper from address strings. |
| apps/region-pages/src/utils/fetchWorkoutLocations.ts | Adds DB fetching + caching for regions/workouts. |
| apps/region-pages/src/utils/fixtures/Points.schema.json | Adds JSON schema fixture for points data. |
| apps/region-pages/src/utils/fixtures/Points.fixture.json | Adds points fixture data (sample rows). |
| apps/region-pages/src/types/workoutLocation.ts | Adds workout-location typing (sheet-like fields). |
| apps/region-pages/src/types/Workout.ts | Adds workout and workout-with-region types. |
| apps/region-pages/src/types/Region.ts | Adds region type definition. |
| apps/region-pages/src/types/Points.ts | Adds types for raw points ingestion envelope/data. |
| apps/region-pages/src/types/Event.ts | Adds region event typings. |
| apps/region-pages/src/lib/env.ts | Adds dotenv-based env loading + validation helper. |
| apps/region-pages/src/lib/const.ts | Adds shared constants (letters, cache TTL). |
| apps/region-pages/src/data/region-events.json | Adds placeholder events data file. |
| apps/region-pages/src/constants/index.ts | Adds site config + day ordering constants. |
| apps/region-pages/src/components/WorkoutTypeFilter.tsx | Adds client-side filter UI for workout types. |
| apps/region-pages/src/components/WorkoutNotes.tsx | Adds notes renderer with sanitization + email enrichment. |
| apps/region-pages/src/components/WorkoutList.tsx | Adds filtered/grouped workout list UI. |
| apps/region-pages/src/components/WorkoutFilters.tsx | Adds combined filter UI wrapper (day/type + clear). |
| apps/region-pages/src/components/WorkoutCard.tsx | Adds workout card UI with map link + notes. |
| apps/region-pages/src/components/RegionsClient.tsx | Adds client wrapper to defer rendering until mounted. |
| apps/region-pages/src/components/RegionHeader.tsx | Adds region header with website/social links. |
| apps/region-pages/src/components/DayFilter.tsx | Adds client-side filter UI for days. |
| apps/region-pages/src/components/ClearFiltersButton.tsx | Adds client-side “clear all filters” button. |
| apps/region-pages/src/components/ChunkErrorRecovery.tsx | Adds client-side recovery for stale chunk load errors. |
| apps/region-pages/src/app/page.tsx | Adds regions index page with search/filter client component. |
| apps/region-pages/src/app/not-found.tsx | Redirects unknown routes back to /. |
| apps/region-pages/src/app/layout.tsx | Adds root layout, metadata, fonts, and GA scripts. |
| apps/region-pages/src/app/globals.css | Adds base global styles + Tailwind directives. |
| apps/region-pages/src/app/error.tsx | Adds app-level error boundary UI. |
| apps/region-pages/src/app/components/SearchableRegionList.tsx | Adds an app-local searchable region list component. |
| apps/region-pages/src/app/api/ingest/history/route.ts | Adds authenticated ingest history endpoint with summary stats. |
| apps/region-pages/src/app/[regionSlug]/page.tsx | Adds per-region page generation + metadata + content rendering. |
| apps/region-pages/src/app/[regionSlug]/events/[eventSlug]/page.tsx | Adds per-event page rendering + metadata generation. |
| apps/region-pages/scripts/seed.ts | Adds seed orchestration script. |
| apps/region-pages/scripts/seed-workouts.test.ts | Adds Jest test around seed-workouts config defaults. |
| apps/region-pages/scripts/seed-state.ts | Adds ingest “skip unchanged” logic (timestamp comparison). |
| apps/region-pages/scripts/seed-state.test.ts | Adds tests for skip/ingest timestamp logic. |
| apps/region-pages/scripts/seed-regions.ts | Adds region seeding pipeline from warehouse to Neon/Supabase. |
| apps/region-pages/scripts/reset.ts | Adds DB reset script (truncate/delete tables). |
| apps/region-pages/scripts/prune-workouts.ts | Adds pruning of workouts missing in warehouse/region set. |
| apps/region-pages/scripts/prune-regions.ts | Adds pruning of regions missing in warehouse (and their workouts). |
| apps/region-pages/scripts/math-utils.ts | Adds mean/stddev helpers used by analytics endpoints/scripts. |
| apps/region-pages/scripts/install-f3-region-pages-data-ingest.sh | Adds launchd installer helper for scheduled ingest runs. |
| apps/region-pages/scripts/ingest-analytics.ts | Adds run-to-run ingest analytics + anomaly detection. |
| apps/region-pages/scripts/import-meta.d.ts | Adds typing for import.meta.main usage in bun scripts. |
| apps/region-pages/scripts/generate-warehouse-schema.ts | Adds drizzle-kit introspection runner for warehouse schema. |
| apps/region-pages/scripts/firebase-env.sh | Adds helper for creating/granting GCP secrets for Firebase hosting. |
| apps/region-pages/scripts/f3-region-pages-data-ingest.sh | Adds scheduled prune+seed runner script for launchd usage. |
| apps/region-pages/scripts/enrich-regions.ts | Adds enrichment step (derive region geo/zoom from workouts). |
| apps/region-pages/scripts/docker-kill.sh | Adds helper to wipe local Docker state (containers/images/volumes). |
| apps/region-pages/scripts/db-setup-local.sh | Adds local DB setup script (Supabase + seed). |
| apps/region-pages/README.md | Adds app README and overview. |
| apps/region-pages/public/window.svg | Adds static asset. |
| apps/region-pages/public/vercel.svg | Adds static asset. |
| apps/region-pages/public/next.svg | Adds static asset. |
| apps/region-pages/public/globe.svg | Adds static asset. |
| apps/region-pages/public/file.svg | Adds static asset. |
| apps/region-pages/public/f3.svg | Adds static asset. |
| apps/region-pages/public/f3-white.svg | Adds static asset. |
| apps/region-pages/postcss.config.mjs | Adds PostCSS/Tailwind plugin config. |
| apps/region-pages/package.json | Adds app package definition, scripts, deps. |
| apps/region-pages/next.config.ts | Configures standalone output + tracing root + build ID + headers. |
| apps/region-pages/jest.setup.ts | Adds Jest console mocking setup. |
| apps/region-pages/jest.config.js | Adds Jest config + TS transform + coverage thresholds. |
| apps/region-pages/infra/terraform/cloud-run/versions.tf | Adds Terraform provider/backend setup. |
| apps/region-pages/infra/terraform/cloud-run/variables.tf | Adds Terraform inputs (image, secrets, scaling, etc). |
| apps/region-pages/infra/terraform/cloud-run/terraform.tfvars.example | Adds example tfvars including secret keys. |
| apps/region-pages/infra/terraform/cloud-run/secrets.tf | Adds Secret Manager secret duplication + IAM wiring. |
| apps/region-pages/infra/terraform/cloud-run/README.md | Adds Cloud Run Terraform deployment runbook. |
| apps/region-pages/infra/terraform/cloud-run/outputs.tf | Adds Terraform outputs for URLs/IP/DNS handoff. |
| apps/region-pages/infra/terraform/cloud-run/main.tf | Adds Cloud Run service + repo + service account provisioning. |
| apps/region-pages/infra/terraform/cloud-run/lb.tf | Adds external HTTPS LB (serverless NEG + managed cert) gated by domain. |
| apps/region-pages/infra/terraform/cloud-run/.gitignore | Ignores tfvars/state artifacts. |
| apps/region-pages/globals.d.ts | Adds ambient CSS module typing. |
| apps/region-pages/eslint.config.mjs | Adds flat ESLint config using @acme/eslint-config presets. |
| apps/region-pages/drizzle/schema.ts | Adds drizzle schema for regions/workouts/seed_runs/ingest_runs. |
| apps/region-pages/drizzle/retry-pool.ts | Adds pg Pool subclass with retry/backoff for transient errors. |
| apps/region-pages/drizzle/retry-pool.test.ts | Adds tests for retry pool behavior. |
| apps/region-pages/drizzle/pool-config.ts | Adds pool tuning constants for warehouse/supabase connections. |
| apps/region-pages/drizzle/pool-config.test.ts | Adds tests validating pool settings. |
| apps/region-pages/drizzle/migrations/meta/_journal.json | Adds drizzle migration journal. |
| apps/region-pages/drizzle/migrations/0000_init.sql | Adds initial SQL migration for schema. |
| apps/region-pages/drizzle/f3-data-warehouse/relations.ts | Adds drizzle relations for warehouse schema. |
| apps/region-pages/drizzle/f3-data-warehouse/db.ts | Adds warehouse DB connection (direct or Cloud SQL connector). |
| apps/region-pages/drizzle/db.ts | Adds Neon/Supabase DB connection used by app queries. |
| apps/region-pages/drizzle.config.warehouse.ts | Adds drizzle-kit config for warehouse introspection. |
| apps/region-pages/drizzle.config.ts | Adds drizzle-kit config for app DB migrations. |
| apps/region-pages/Dockerfile.dockerignore | Adds Dockerfile-scoped ignore rules for monorepo build context. |
| apps/region-pages/Dockerfile | Adds monorepo-aware multi-stage Docker build for Cloud Run. |
| apps/region-pages/CONTRIBUTORS.md | Adds contributor setup documentation. |
| apps/region-pages/CLAUDE.md | Adds LLM context index file for the app. |
| apps/region-pages/.prettierrc | Adds Prettier settings for the app. |
| apps/region-pages/.prettierignore | Adds Prettier ignore rules. |
| apps/region-pages/.gitignore | Adds app-local gitignore. |
| apps/region-pages/.dockerignore | Adds app-local dockerignore. |
| .gitignore | Updates ignore rule for Claude worktrees directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4bb68d5 to
0c0bab6
Compare
🔄 Rebased on latest
|
aac3e7f to
6158503
Compare
|
✅
Rebased + CI-green; not merged per request. Blocked only on human review ( |
Recommendation: switch region-pages to Cloud Run custom domain mapping (drop the ~$20/mo load balancer)TL;DR — my prescriptive recommendation: go with the domain-mapping path. For a single-subdomain, scale-to-zero Next.js app like region-pages, the global external load balancer is paying a ~$18–25/mo fixed baseline for capabilities we don't use. I think the trade-offs below are all acceptable, but documenting them so the call is explicit. The IaC change: What we GIVE UP vs. the global external HTTPS load balancer
What we KEEP
One-time cost of the switch
Net: every lost capability is one we don't currently use, in exchange for ~$240/yr saved on this project. I recommend we proceed: verify the domain, validate on the mapping, then flip |
|
❌ region-pages Terraform drift check: |
|
✅ region-pages Terraform drift check: in sync — live infrastructure matches the committed config. |
Enforcing zero Terraform drift on
|
…ion-pages) Lift-and-shift of the standalone f3-region-pages repo into apps/region-pages. Kept self-contained (its own deps, Drizzle, scripts, and infra/terraform) — the Neon database and ALL GCP/Terraform resources are unchanged. Only monorepo-fit adaptations were made: - Dockerfile reworked for the monorepo build context (turbo prune + workspace pnpm install + `turbo build --filter=f3-region-pages`), preserving the BuildKit build secrets (postgres_url, warehouse_url) and Cloud Run port 8080. - next.config.ts: add `outputFileTracingRoot` (workspace root) so standalone output traces the hoisted node_modules. - package.json: drop the app's own `packageManager` (pnpm@9, conflicts with root pnpm@10); align `engines.node` to `>=24.14.1 <25`; remove the `postinstall: pnpm skills:check` hook (would run `npx skills check` on every monorepo install). - remove app-level `.nvmrc` (root .nvmrc / Node 24 governs). Secrets were excluded from the move (no .env*, terraform.tfvars, .secrets, tfstate). NOT done here (left for the human review pass): pnpm-lock regeneration, `turbo build` verification, and CI/Cloud Build wiring. See PR description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…deps aligned, green) - Single root pnpm-lock (removed nested apps/region-pages/pnpm-lock.yaml). - Align React ecosystem to the monorepo: react/react-dom 18.3.1, next ^15.3.6, @types/react(-dom) ^18.3.1 — fixes a hoisted react 18/19 mismatch that broke static generation (useRef null). No React-19-only APIs were in use. - next.config: ignore ESLint during build (lint runs via turbo lint); keep outputFileTracingRoot for monorepo standalone tracing. - tsconfig: scope `types` to ["node","jest"] (monorepo hoists many @types; the default auto-include tripped on an @types/minimatch stub). - seed-workouts: narrow locationIds via type guard (number, not number|null); surfaced by the monorepo's stricter drizzle-orm types, correct regardless. - turbo.json: passThroughEnv for POSTGRES_URL / F3_DATA_WAREHOUSE_URL / WAREHOUSE_DB_CONNECTION_MODE so the build receives BuildKit secrets through turbo. - Dockerfile.dockerignore: scoped lean build context (exclude nested node_modules). Local validation green: pnpm install, turbo build (541 pages), tsc --noEmit, next lint. Deployed to Cloud Run as web:v4 (rev f3-region-pages-00005-crb); verified pages 200 and a forced ingest succeeded (warehouse -> Neon, change-aware seeding). PR remains draft. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ations Make region-pages pass the monorepo's single-version policy + lint/build, and slim the PR per review: - Deps: align to the workspace's single versions (typescript/eslint -> catalog, drizzle-orm ^0.45.2, drizzle-kit ^0.31.10, pg ^8.11.3, prettier ^3.2.5, dotenv ^16.3.2, @types/node ^20.11.13, @types/pg ^8.10.9, @types/lodash ^4.14.195, postcss ^8.4.35). sherif now passes. - ESLint: adopt @acme/eslint-config (base/nextjs/react); drops eslint-config-next + FlatCompat which crash under the monorepo's eslint 10. Strictest type-aware rules downgraded to warnings for this first pass (TODO: tighten). - tsconfig: scope `types`, allowArbitraryExtensions + globals.d.ts so `.css` side-effect imports typecheck under TS 6. - Drop dead deps/scripts: firebase, firebase-tools, eslint-config-next, @eslint/eslintrc; firebase:* / db:daily-ingest:* / skills:* scripts. - Prune cruft that was swept in by the lift-and-shift: .agents/.claude/.codex/ .cursor skill dirs, .vscode, .context/slack.md, .cursorrules, .github (dup CI), package-lock.json, pglite-debug.log, ephemeral docs (postmortems, launchd plist, video), and dead Firebase config (.firebaserc, firebase.json, apphosting.yaml). - Squash 11 drizzle migrations -> single 0000_init.sql (clean reset point; region-pages deploy does not auto-migrate, so prod journal is unaffected). Local: sherif clean, turbo lint (0 errors), tsc --noEmit, next build (541 pages). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The monorepo CI `build` job runs `turbo build` without region-pages' DB secrets, but the app's generateStaticParams queries the DB at build. Make the build degrade gracefully so CI passes (renders the static routes only) while the real deploy build (with POSTGRES_URL via BuildKit secret) still pre-renders all ~541 pages: - loadEnvConfig: under SKIP_ENV_VALIDATION, warn instead of throw on missing POSTGRES_URL / F3_DATA_WAREHOUSE_URL. - build script sets SKIP_ENV_VALIDATION=1 (deploy build still has the real env). - fetchRegionBySlug / fetchRegionsWithWorkoutCounts: fall back to null/[] on DB error (the cached fetchers already did). - drizzle.config.ts: tolerate undefined url for typecheck (real url at runtime). Verified: build with no DB env compiles (7 static pages); build with Neon renders 541 pages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…' build
Adding passThroughEnv to the shared `build` task disrupted the other apps'
strict-mode env in CI ("Invalid environment variables" for admin/auth/me/api/
map). region-pages no longer needs it — its build sets SKIP_ENV_VALIDATION=1 and
degrades gracefully without a DB. Revert turbo.json to dev's state and build
region-pages in the Docker deploy via `pnpm --filter` (not turbo) so the
BuildKit-secret POSTGRES_URL reaches next build for full 541-page prerender.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add .env.local.example (POSTGRES_URL → shared docker-compose Postgres :5433, F3_DATA_WAREHOUSE_URL, WAREHOUSE_DB_CONNECTION_MODE, CRON_SECRET, Slack vars). - Rewrite db-setup-local.sh: use the monorepo root docker-compose Postgres instead of the removed Supabase flow (the old script called a non-existent `supabase:start` and copied a pruned `.env.local.sample`). - Track .env*.example (was only un-ignoring the legacy .env.local.sample). - Fix the stale .env.local.sample reference in CLAUDE.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- fetchRegionsWithWorkoutCounts: preserve workoutCount (normalizeRegionFields
was dropping it, leaving workoutCount undefined at runtime) + fix typing.
- HomePage: derive regionsByLetter from the single query instead of querying
fetchRegionsWithWorkoutCounts twice.
- [regionSlug]/page: guard empty regionData before indexing [0] (DB failure
returned []); generateStaticParams returns only { regionSlug } and skips
empty slugs.
- env.ts: also load .env.local (after .env.${NODE_ENV}) so bun/db scripts work
with a single .env.local, matching the monorepo convention.
- .prettierignore: drop trailing space on "*.map" so source maps are ignored.
- README: React 18 (not 19) to match the monorepo dep pin.
- CONTRIBUTORS: Node >=24.14.1 + pnpm (not 20.18.2/npm); replace stale
docker:kill/supabase:start script list with db:setup:local.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gs (SEO) Prepares region-pages for the cheap Cloud-Run-domain-mapping topology where several domains (f3regions.com, f3region.info, …) point at the same service. To consolidate SEO onto the canonical host: - src/middleware.ts: 308-redirects any non-canonical Host to the canonical host (path+query preserved). Opt-in + inert — only active when NEXT_PUBLIC_CANONICAL_HOST is set, so it does nothing until regions.f3nation.com is cut over. Skips _next, /api/*, and static assets (keeps cron ingest working). - alternates.canonical on the home + [regionSlug] pages (resolved against metadataBase = SITE_CONFIG.url) for belt-and-suspenders canonicalization. - .env.local.example: document NEXT_PUBLIC_URL + NEXT_PUBLIC_CANONICAL_HOST. - src/middleware.test.ts: 5 cases (inert unset, canonical passthrough, 308 with path/query, port-insensitive, missing Host). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…mo LB) Introduces a routing_mode toggle for var.service_domain: - "domain_mapping" (recommended): free Cloud Run custom domain mapping (Google-managed cert, CNAME -> ghs.googlehosted.com). No load balancer billed. - "load_balancer" (default for now): the existing global external HTTPS LB, retained behind the toggle for rollback until the mapping is validated. New domain-mapping.tf creates google_cloud_run_domain_mapping; lb.tf is gated so exactly one routing path exists. outputs.tf surfaces the CNAME record + a routing-aware dns_handoff for Tackle. Saves the ~$18-25/mo fixed LB baseline. Trade-offs (all unused today: Cloud CDN, Cloud Armor, static anycast IP, multi-region, custom SSL policy, apex domains) documented in domain-mapping.tf and the PR discussion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring the region-pages GCP project fully under Terraform with zero console drift. - domain-mapping.tf: model serving as a `var.domain_mappings` list (for_each). Live prod mapping `regions.f3nation.com` + new `staging.f3regions.com` are both managed; the external HTTPS load balancer is retired (staging migrated onto a Cloud Run domain mapping on 2026-05-26), freeing ~$240/yr. - main.tf: codify the legacy `f3-cloudsql-sa`; add `ignore_changes` on the Cloud Run service for deploy-owned fields (client/client_version/scaling/image) so CI image rollouts don't show as drift. - secrets.tf: `ignore_changes` on `secret_data` — secret rotation is operational data, not infrastructure drift. - ci.tf: keyless Workload Identity CI service account (`github-actions-deploy@region-pages`) bound to the F3-Nation WIF pool, with read-only roles for plan/drift detection. - .github/workflows/region-pages-terraform-drift.yml: runs `terraform plan -detailed-exitcode` daily and on every PR touching the Terraform, failing on drift — this enforces the zero-drift rule. Also removed (Phase 1, via gcloud, snapshotted): dead Firebase App Hosting + Cloud Functions footprint (Artifact Registry repos, GCS buckets, secrets, SA, APIs) and an orphaned HubSpot LB backend/NEGs. `terraform plan` = No changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fresh terraform plan refreshes the github-actions-deploy SA's own state-bucket IAM binding, which requires storage.buckets.getIamPolicy — not granted by objectAdmin. Bucket-scoped storage.admin covers state object r/w + IAM read. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p env in turbo.json Addresses BigGillyStyle review: after rebasing onto latest dev, follow the new convention and reference catalog-available devDependencies via `catalog:` in apps/region-pages/package.json: - @types/lodash, @types/node, @types/pg, @types/react, @types/react-dom, drizzle-kit, postcss, prettier, tailwindcss -> catalog: - kept jest, ts-jest, @types/jest, bun, drizzle-orm, pg pinned (not in the catalog / app-specific versions, per the reviewer's caveat) Also: - add `bun: true` to pnpm-workspace allowBuilds so installs run bun's postinstall non-interactively (region-pages db scripts use bun) - declare region-pages runtime env vars in turbo.json globalEnv to satisfy turbo/no-undeclared-env-vars (CRON_SECRET, SLACK_*, POSTGRES_URL, F3_DATA_WAREHOUSE_URL, WAREHOUSE_DB_CONNECTION_MODE, NEXT_BUILD_ID, VERCEL_GIT_COMMIT_SHA, NEXT_PUBLIC_CANONICAL_HOST, NEXT_PUBLIC_URL) -- added to globalEnv (not the shared build passThroughEnv that previously broke other apps) - regenerate root pnpm-lock.yaml; remove feature-branch Drizzle migrations (regenerated via Drizzle Kit) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
44336da to
3d95140
Compare
|
✅ region-pages Terraform drift check: in sync — live infrastructure matches the committed config. |
…types/jest)
Bringing apps/region-pages into the monorepo adds @types/jest to the workspace,
which (under the hoisted node-linker) makes the global `jest` type visible to
every app's tsc program. The map test setup's `declare global { var jest }`
then conflicts with @types/jest's own global declaration, failing typecheck:
TS2649 Cannot augment module 'jest' ...
TS2740 Type 'VitestUtils' is missing the following properties from 'typeof jest'
Assign the vitest shim through an untyped view of globalThis instead of
redeclaring the global type. Runtime behavior is unchanged: vitest-canvas-mock
still reads globalThis.jest at import time. map's vitest suite still passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
✅ region-pages Terraform drift check: in sync — live infrastructure matches the committed config. |
The devDependencies-catalog convention has been applied after rebasing onto latest dev (see resolved thread on apps/region-pages/package.json, commit 3d95140). Dismissing to unblock -- re-requesting your review for a fresh look.
Merge-readiness attestationRebased onto latest Addressed the change request (@BigGillyStyle,
Fixes made during this pass:
CI (all green on Review sweep: Not merging — leaving final human review and the documented cutover/source-of-truth decisions to the team. |
BigGillyStyle
left a comment
There was a problem hiding this comment.
Sorry about requesting changes a second time. I should've looked a bit deeper the first time.
| ENV TURBO_TELEMETRY_DISABLED=1 | ||
| RUN apk add --no-cache libc6-compat openssl && apk update | ||
| WORKDIR /app | ||
| RUN npm install -g turbo@^1.12.3 |
There was a problem hiding this comment.
This one is my bad...but I didn't realize turbo was used in the Dockerfiles. Since the monorepo was upgraded to turbo v2, this causes a breakage due to some config changes between the two versions. See @taterhead247 's PR at #328 for what needs to change.
| SLACK_BOT_AUTH_TOKEN=xoxb-... | ||
| SLACK_CHANNEL_ID= | ||
|
|
||
| NODE_ENV=local |
There was a problem hiding this comment.
I'm familiar with the usage of development, production, and test values here. Does this app use local for a particular use case?
| # `direct` mode requires a real warehouse connection string (from GCP Secret | ||
| # Manager / a read replica). Point it at the warehouse you have access to. | ||
| WAREHOUSE_DB_CONNECTION_MODE=direct | ||
| F3_DATA_WAREHOUSE_URL=postgresql://USER:PASS@WAREHOUSE_HOST:5432/f3data |
There was a problem hiding this comment.
This .env.local.example file is used to create the local dev's .env file for this app. Could this URL be changed to point to a valid PG DB that's running locally in Docker?
|
@pstaylor-patrick , similarly to #311 , let's see if we can get this app deployed on this branch before merging into dev. Moving back to draft. |
Summary
Brings the standalone f3-region-pages app into the monorepo at
apps/region-pagesand fully integrates it into the monorepo build (single lockfile, aligned deps, turbo build). Now deployed to GCP from the monorepo and validated end-to-end — kept draft for human review before we make the monorepo the permanent deploy source. Fallback: the standalone repo (imageweb:v3) is still available.This PR also switches the public-routing topology to save hosting cost (see below) and adds canonical-host SEO handling for the secondary domains. The Neon database is unchanged; only the image build context moved to the monorepo.
💸 Routing: Cloud Run domain mapping instead of the load balancer (~$240/yr saved)
The region-pages GCP project's spend is not traffic (the service scales to zero) — it's the global external HTTPS load balancer's ~$18–25/mo fixed baseline. This PR makes the cheaper Cloud Run custom domain mapping the recommended routing path:
feat(region-pages): add Cloud Run domain-mapping routing— adds arouting_modeTerraform toggle:domain_mapping(recommended, free, Google-managed cert,CNAME → ghs.googlehosted.com) vsload_balancer(the existing LB, retained behind the toggle for rollback). Default staysload_balancerso an apply before cutover tears nothing down; we flip after the domain is verified + validated.lb.tfis gated so exactly one routing path exists;domain-mapping.tfis new.feat(region-pages): canonical-host redirect middleware + canonical tags (SEO)—src/middleware.ts308-redirects any non-canonical host (e.g. the historicalf3regions.com/f3region.info) toregions.f3nation.com, path+query preserved;alternates.canonicalon home/region pages. Opt-in + inert untilNEXT_PUBLIC_CANONICAL_HOSTis set.Trade-offs of dropping the LB (all capabilities unused by region-pages today): no Cloud CDN, no Cloud Armor/WAF, no static anycast IP, single-region only, Google-managed cert only, no apex-domain support (we serve a subdomain). Full write-up + my prescriptive recommendation to proceed: see the trade-off comment. Mechanism validated end-to-end on a throwaway verified domain (
rp.f3muletown.com).✅ Integrated + validated (this is no longer just a structural move)
apps/region-pages/pnpm-lock.yaml;pnpm installresolves region-pages' deps into the one rootpnpm-lock.yaml.turbo prune+ workspace install +turbo build --filter), standalone runner, port 8080, BuildKit secrets. Added a scopedDockerfile.dockerignore.globalEnventries for the region-pages runtime env (POSTGRES_URL,F3_DATA_WAREHOUSE_URL,WAREHOUSE_DB_CONNECTION_MODE,NEXT_BUILD_ID,VERCEL_GIT_COMMIT_SHA,CRON_SECRET,SLACK_*,NEXT_PUBLIC_*).outputFileTracingRootfor monorepo standalone.types. seed-workouts → narrowlocationIdswith a type guard. commitlint → addedregion-pagesscope.🔧 Monorepo integration follow-ups (latest pass, post-rebase)
devDependencies→ PNPM catalog (addresses @BigGillyStyle's review): moved every catalog-available devDependency tocatalog:(@types/lodash,@types/node,@types/pg,@types/react,@types/react-dom,drizzle-kit,postcss,prettier,tailwindcss, alongside the already-catalog:eslint/typescript). Keptjest,ts-jest,@types/jest,bun,pg,drizzle-ormpinned — they are not in the catalog (the catalog standardizes on Vitest; region-pages currently uses Jest). Moving region-pages onto the Vitest stack is intentional follow-up, not part of this lift-and-shift.--no-verify— region-pages already uses the flat@acme/eslint-config; the remaining failures wereturbo/no-undeclared-env-vars, now resolved by declaring the app's env inturbo.jsonglobalEnv(not the sharedbuildpassThroughEnv, which previously broke the other apps).pnpm-workspace.yaml→ addedbun: truetoallowBuildsso installs run bun's postinstall non-interactively (region-pages db scripts use bun).apps/maptest setup — bringing@types/jestinto the workspace made the globaljesttype visible everywhere (hoisted node-linker), colliding with map'sdeclare global { var jest }vitest shim and failing typecheck. Fixed by assigning the shim through an untypedglobalThisview; runtime behavior unchanged, map's vitest suite still passes.drizzle/migrationsis only a generatedout:target, no runtime reference).Local validation — all green
pnpm install✓ ·turbo build --filter=f3-region-pages(541 static pages) ✓ ·turbo run typecheck(20/20) ✓ ·turbo run lint✓ · middleware tests ✓ ·terraform validate✓Deployed + verified on GCP
Built
web:v4from the monorepo, deployed to Cloud Run (f3-region-pages-00005-crb). Verified:regions.f3nation.com/,/essex,/colonialall 200 over valid TLS; a forced/api/ingestrun succeeded (warehouse → Neon, change-aware seeding).⛔ Remaining for the human-review pass
f3nation.com(TXT), then Tackle swapsregions.f3nation.comfromA 8.233.224.179→CNAME ghs.googlehosted.com.; validate, then setrouting_mode = "domain_mapping"(Terraform tears down the LB). LB stays live until then.regions.f3nation.comis canonical, mapf3regions.com/f3region.infoto the service and setNEXT_PUBLIC_CANONICAL_HOSTto activate the 308 redirects.jest/ts-jest/@types/jestdeps) is a clean follow-up.cloudbuild.yamlpattern; reconcile Terraform image drift (prod onweb:v4, state references an older tag).f3-region-pagesrepo once the monorepo is the deploy source.🤖 Generated with Claude Code