Skip to content

Prepare HiveLore preview deployment - #32

Open
KareemN1936 wants to merge 1 commit into
mainfrom
codex/deployment-audit-20260918
Open

KareemN1936 wants to merge 1 commit into
mainfrom
codex/deployment-audit-20260918

Conversation

@KareemN1936

Copy link
Copy Markdown
Collaborator

Summary

  • align the web client, examples, CI, and docs on NEXT_PUBLIC_API_BASE_URL
  • remove the production localhost fallback and validate the public API URL
  • update dependencies within the current framework majors, including the critical Next.js fix
  • add harmless public Supabase placeholders to CI builds

Local verification

  • Prisma format, validation, and client generation
  • ESLint and TypeScript checks
  • production builds for API and web
  • API /health returned 200
  • high-confidence secret scan found no tracked env/key files or token patterns

Known follow-up

  • the repository-wide Prettier check already fails on pre-existing files
  • npm audit retains Prisma 7 and Next 15 findings whose published fixes require major-version changes

@github-actions

Copy link
Copy Markdown

Vercel preview is ready:

https://hivelore-hiden7025-kareemn1936s-projects.vercel.app

Commit: e26b8d5

@mcfarhat mcfarhat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes — but not on the code, which is clean. The problem is the branch this is aimed at.

The change itself builds and passes. I cloned it, installed, and ran the full set at e6b3c5c: typecheck pass, lint pass, format:check pass, build pass for both web and API. Consolidating client.ts onto env.ts is the right call — main genuinely had two different variable names in two files, and you fixed that. The CI placeholders and the URL validation are both improvements.


Blocking

This targets main, which is 89 commits behind develop

Your branch points at main and forks from 17ef470 (29 July). Every previous PR of yours went to develop, which is where the work actually is.

The consequence isn't stylistic. On this branch nothing imports the modules you changed:

$ git grep -n "lib/env\|lib/api/client" -- 'apps/web/src/**'   # excluding the two files themselves
(no results)

Both lib/env.ts and lib/api/client.ts are orphaned here. So the rename and the fallback removal land on code nothing calls.

On develop those modules are very much alive — app/page.tsx imports env, and six modules under lib/api/ import apiClient. And develop still carries eleven NEXT_PUBLIC_API_URL references:

.env.example                    .github/workflows/ci.yml
README.md (x2)                  apps/web/.env.example
apps/web/README.md (x3)         apps/web/src/lib/env.ts (x3)

So if this merges, the application people actually run is unchanged, and the next develop → main merge reintroduces the old name on top of it.

Three things that follow from the stale base

develop's env.ts exports a field this version doesn't. Develop has { apiBaseUrl, googleAuthEnabled }; yours has { apiBaseUrl }. If that merge is ever resolved in main's favour, env.googleAuthEnabled disappears silently.

The green tick on this PR is weaker than it looks. main's CI runs format, prisma, lint, typecheck, build. develop's CI additionally runs db:migrate:check:fresh, the migration-upgrade check, and npm test. No tests ran here — the root package.json on this branch has no test script at all:

$ npm test
npm ERR! Missing script: "test"

Dependency versions now diverge from develop in 15 packages — Prisma 7.10 vs 7.8, Fastify 5.12 vs 5.4, Next 15.5.25 vs 15.4.2, zod 4.6.5 vs 4.0.5, and others. Bumping them only on main means a 1,800-line lockfile conflict at the next merge, and leaves develop without the Next.js fix you describe as critical — which is the branch that needs it.


One thing to carry across when you re-target

Removing the localhost fallback does not fail the build. I checked:

$ env -u NEXT_PUBLIC_API_BASE_URL npm run build
rc=0

It passes here only because nothing imports env on this branch. On develop, app/page.tsx imports it at module scope, so a missing variable throws at runtime — the deploy succeeds and the page breaks for users. Dropping the fallback is still the right decision, but it needs the Vercel variable set beforehand, and it's worth considering an explicit build-time assertion so a missing value fails the deploy instead of the page.


What I'd like

Rebase onto develop and re-target the PR there. Everything you've done is wanted — the rename, the validation, the CI placeholders, the bumps. It just needs to be on the branch where the code lives.

For what it's worth, this is an easy trap rather than carelessness: main is the repository's default branch, so anything branching from the default lands on a July snapshot without any signal that it's stale. That's a repo problem and I'll deal with it separately — main being the default while develop is the real trunk is what made this possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants