Unify local env loading#84
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR consolidates environment configuration to a single root Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/README.md (1)
8-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRunning command contradicts the root-env script note.
The docs say package scripts load root
.env, butbun dev --port 3500bypasses package scripts. Recommend documentingbun run dev(ornpm run dev) here to match the env-loading behavior.Also applies to: 12-14
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/README.md` around lines 8 - 10, The README currently instructs readers to run "bun dev --port 3500", which bypasses package scripts and contradicts the note that package scripts load the root .env; update the instructions to recommend using the package script invocation (e.g., "bun run dev" or "npm run dev") instead of direct "bun dev --port 3500" so the root .env is loaded; update the two occurrences (lines showing "bun dev --port 3500") and add a short parenthetical note that running via the package script ensures the root .env is sourced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.dev.yml`:
- Around line 74-75: The frontend service currently injects the full root .env
via the env_file: .env entry which exposes server-only secrets; edit the
docker-compose service named frontend to remove the env_file: .env line and
replace it with an explicit environment: block listing only the public frontend
variables required at runtime (e.g., NEXT_PUBLIC_API_URL,
NEXT_PUBLIC_ANALYTICS_KEY or other NEXT_PUBLIC_* keys used by the Next.js app),
ensuring no SERVER_ or secret keys are included; locate the frontend service
block in the compose diff and make this substitution so only scoped public env
vars are passed into the container.
In `@makefiles/Makefile`:
- Line 26: The Makefile recipe uses non-POSIX conditional syntax ([[ ... ]])
(e.g., the test involving $$value and $$placeholder in the recipe and similar
checks at lines referencing the same pattern), which breaks under /bin/sh;
either change those checks to POSIX test syntax ([ ... ] with proper quoting and
||/&& adjustments) across all occurrences (the checks at the shown diff and the
similar ones at the other noted locations) or pin the Makefile shell by adding a
top-level SHELL := /bin/bash so targets like dev and convex-* run under bash;
update all occurrences of [[ ... ]] accordingly and ensure quoting of $$value
and $$placeholder remains correct.
---
Outside diff comments:
In `@frontend/README.md`:
- Around line 8-10: The README currently instructs readers to run "bun dev
--port 3500", which bypasses package scripts and contradicts the note that
package scripts load the root .env; update the instructions to recommend using
the package script invocation (e.g., "bun run dev" or "npm run dev") instead of
direct "bun dev --port 3500" so the root .env is loaded; update the two
occurrences (lines showing "bun dev --port 3500") and add a short parenthetical
note that running via the package script ensures the root .env is sourced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 008baf79-430c-4e0b-9cd5-85a535d25cc2
📒 Files selected for processing (24)
.env.example.gitignoreCLAUDE.mdREADME.mdbackend/.env.examplebackend/README.mdbackend/package.jsonbackend/src/convex.tsbackend/src/env.tsbackend/src/index.tsbackend/src/mastra/tools/investigate-tool.tsbackend/src/pipeline/schema-inference.tsdocker-compose.dev.ymlfrontend/.env.examplefrontend/.gitignorefrontend/README.mdfrontend/components/ThemeToggle.tsxfrontend/components/table/ColumnHeader.tsxfrontend/components/table/DatasetTable.tsxfrontend/components/table/TableHeader.tsxfrontend/lib/analytics.tsfrontend/package.jsonmakefiles/Makefilescripts/with-root-env.mjs
💤 Files with no reviewable changes (2)
- backend/.env.example
- frontend/.env.example
| env_file: | ||
| - .env |
There was a problem hiding this comment.
Remove full root .env injection from the frontend service.
Line 74 introduces env_file: .env for frontend, which makes server-only keys available inside the frontend container runtime. Keep frontend env scoped to explicit required vars to preserve least privilege.
Suggested fix
frontend:
build:
context: ./frontend
dockerfile: Dockerfile.dev
- env_file:
- - .env
ports:
- "3500:3500"As per coding guidelines, frontend/**: "Frontend uses Next.js 16, React 19, Tailwind 4 for pure UI — no server-side auth logic".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env_file: | |
| - .env |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.dev.yml` around lines 74 - 75, The frontend service currently
injects the full root .env via the env_file: .env entry which exposes
server-only secrets; edit the docker-compose service named frontend to remove
the env_file: .env line and replace it with an explicit environment: block
listing only the public frontend variables required at runtime (e.g.,
NEXT_PUBLIC_API_URL, NEXT_PUBLIC_ANALYTICS_KEY or other NEXT_PUBLIC_* keys used
by the Next.js app), ensuring no SERVER_ or secret keys are included; locate the
frontend service block in the compose diff and make this substitution so only
scoped public env vars are passed into the container.
| @check_env() { \ | ||
| key="$$1"; placeholder="$$2"; \ | ||
| value="$$(grep "^$$key=" .env | cut -d= -f2-)"; \ | ||
| if [[ -z "$$value" || "$$value" == "$$placeholder" || "$$value" == *"..."* ]]; then \ |
There was a problem hiding this comment.
Use POSIX test syntax or pin Make shell to bash.
These recipes use [[ ... ]], which breaks when make runs under /bin/sh (common on Debian/Ubuntu). make dev/convex-* can fail before startup.
Suggested fix
+SHELL := /bin/bash
+
.PHONY: all dev validate-dev-env down clean convex-push convex-env seed-public-datasetsOr replace [[ ... ]] with POSIX [ ... ] in all recipe checks.
Also applies to: 40-40, 44-44, 58-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@makefiles/Makefile` at line 26, The Makefile recipe uses non-POSIX
conditional syntax ([[ ... ]]) (e.g., the test involving $$value and
$$placeholder in the recipe and similar checks at lines referencing the same
pattern), which breaks under /bin/sh; either change those checks to POSIX test
syntax ([ ... ] with proper quoting and ||/&& adjustments) across all
occurrences (the checks at the shown diff and the similar ones at the other
noted locations) or pin the Makefile shell by adding a top-level SHELL :=
/bin/bash so targets like dev and convex-* run under bash; update all
occurrences of [[ ... ]] accordingly and ensure quoting of $$value and
$$placeholder remains correct.
Summary
.env.examplethe canonical local env template and remove child env templates.envfor backend/frontend package scripts, Docker services, and Convex make targetsImportant after pulling main
.env.frontend/.env.local,frontend/.env,backend/.env, or any other nested env file in the repo.frontend/.env.localand can override the root-env flow, which makes Convex commands look like the rootCONVEX_SELF_HOSTED_ADMIN_KEYis missing or wrong..env, then move/delete the nested env files outside the repo..envasCONVEX_SELF_HOSTED_ADMIN_KEY.Verification
npm run buildinbackendnpm run buildinfrontendnpm run lint -- --quietinfrontendgit diff --checkmake validate-dev-envfails fast when root.envis missingNo real env files were inspected or committed.