Repository navigation
fix(docker): make compose stack boot from a fresh .env - #1674
LucasSantana-Dev merged 4 commits into
Conversation
📝 WalkthroughWalkthroughAdds a ChangesEnvironment configuration
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 1
🤖 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 @.env.example:
- Around line 57-60: The comment on POSTGRES_PASSWORD overstates how
DATABASE_URL and DIRECT_URL are derived; update the surrounding explanatory text
in .env.example so it only says the password is required by docker-compose and
is used for the in-container setup, without implying the placeholder
DATABASE_URL/DIRECT_URL values are built from it. Keep the guidance aligned with
the POSTGRES_PASSWORD entry and the DATABASE_URL/DIRECT_URL placeholders so
users aren’t misled about manual configuration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 742a397b-d750-4862-9fd4-fce54159449b
📒 Files selected for processing (2)
.env.exampledocker-compose.yml
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: Docker config fixes: adds POSTGRES_PASSWORD default and DIRECT_URL override for Prisma in docker-compose. Low-risk configuration-only changes.
Re-trigger cubic
|
👋 Heads-up: I'm draining the open-PR backlog. main just landed a CI fix (#1809: npm@12 strict- |
…rations (#1830) ## Summary Adds missing `DIRECT_URL` environment variable to `docker-compose.staging.yml` and `docker-compose.dev.yml`. **Why:** Prisma `migrate deploy` requires a non-pooled `DIRECT_URL` connection for schema migrations. PR #1674 added this to `docker-compose.yml` (production), but the staging and dev compose files were missing it, causing P1001 errors on fresh Docker stack boots. **What:** Mirrors the `DIRECT_URL` value to both environments, set identical to `DATABASE_URL` (no pgbouncer in these stacks). - `docker-compose.staging.yml`: Added to `x-common-app-env` anchor (line 47) - `docker-compose.dev.yml`: Added to `lucky-bot-dev` service environment (line 66) Closes #1793 <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Add `DIRECT_URL` to `docker-compose.staging.yml` and `docker-compose.dev.yml` so Prisma `migrate deploy` uses a direct (non-pooled) connection. Set it equal to `DATABASE_URL` to avoid P1001 errors on fresh boots (closes #1793). <sup>Written for commit 7714582. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1830?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated development and staging configurations with an additional direct database connection setting. * Services using the shared staging configuration now inherit the direct database connection automatically. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
d23c167 to
8e86111
Compare
|
@LucasSantana-Dev thanks for the heads-up. Rebased onto latest main (clean, no conflicts). The two commits are still just the .env.example POSTGRES_PASSWORD default and the DIRECT_URL override in docker-compose.yml. Fork CI workflows are sitting on |
8e86111 to
912cf66
Compare
Kimi review (
|
|
Addressed in dca1840. The example and dashboard docs now require a URL-safe hexadecimal POSTGRES_PASSWORD and include the openssl command used to generate one. docker compose config passes with the updated example. The branch is up to date with main. |
There was a problem hiding this comment.
Thanks for sticking with this one, it's been open a while and the diagnosis is right.
The DIRECT_URL fix is the valuable part. The Prisma CLI reading DIRECT_URL while the runtime client reads DATABASE_URL is genuinely non-obvious, and falling back to a localhost value that's unreachable from inside the container is a horrible first-run failure to debug. The comment explaining it is exactly what a future reader needs. Using ${POSTGRES_PASSWORD?POSTGRES_PASSWORD_required} instead of a :- default is also the right instinct: fail loudly rather than boot with a silently wrong credential.
The docs changes are good too. Calling out URL-safe hexadecimal and giving the exact openssl rand -hex 24 command matters here, because the value goes straight into a Postgres URL where a stray @, : or / breaks parsing in a way that looks like anything but a quoting bug. Both the Docs.tsx table entry and the rotation bullet now say the same thing.
The one thing I want to change: the placeholder cancels out the guard.
POSTGRES_PASSWORD=000000000000000000000000000000000000000000000000
The ? guard in docker-compose.yml only fires when the variable is unset or empty. This line sets it, so the guard can't trigger for anyone doing the documented thing of copying .env.example to .env — which is the exact flow this PR exists to make work. The old commented-out # POSTGRES_PASSWORD=change-me-in-production did trigger it.
So docker compose up now succeeds out of the box with a 48-zero database password, and the operator is never forced to look at it. "It booted, so it must be configured" is the failure mode, and this repo's compose stack is what runs on my homelab, so that's not purely hypothetical.
I recognise the tension: the whole point of the PR is a stack that boots from a fresh .env, and an empty required variable works against that. I'm working through the options now (empty value plus a one-line setup command, versus generating it in a setup script or Make target) and I'll follow up here with a direction so you're not guessing. Everything else in the PR is good to go.
On the red checks: danger / danger fails with Request failed [403]: .../issues/1674/comments, which is a token permission problem posting its comment on a fork PR, not anything in your diff. The npm ci errors are a stale lockfile on main, Security was an unpassable repo-wide gate, and kimi-review fails on every PR because an API key isn't set (#1877). Those three are fixed in #1876; rebase once it lands and I'll sort the danger permission separately.
|
Following up with the direction I promised, and I owe you a correction: my instinct about how to fix this was wrong, and testing it is what caught the problem. I'd assumed that shipping
So the guard you inherited only ever protected against a missing line. An empty placeholder under it would have booted Postgres with an empty password, which is worse than your 48 zeros. Good thing I checked before asking you to change it. I've fixed the guard itself in #1881: both interpolation sites move from What that means for this PR, once #1881 lands:
I've also added the generation step to the README setup block in #1881, so Full reasoning and the alternatives I rejected (including a Sorry for the extra round trip. Thanks for the patience on this one, it's been open longer than it should have been. |
|
Heads-up: the CI blockers I mentioned are fixed on What that clears:
Please rebase onto The review feedback above is separate and still stands. |
0526e44 to
aaededf
Compare
|
Addressed in 853a9bd (squashed, rebased onto main).
|
aaededf to
853a9bd
Compare
LucasSantana-Dev
left a comment
There was a problem hiding this comment.
Review: approve with nits
The fix is correct and matches the repo's Prisma/compose architecture. Only the "no manual edits" claim is overstated, plus consistency nits.
P2 — quickstart claim vs reality: the added .env.example:60 placeholder is POSTGRES_PASSWORD= (empty), and the compose change deliberately uses ${POSTGRES_PASSWORD:?...} which errors on set-but-empty. So cp .env.example .env && docker compose up -d still exits with POSTGRES_PASSWORD_required until the user generates and fills the password. The design itself is right (shipping a default DB password would be worse) and the failure is loud and actionable; please adjust the PR body / docs quickstart to list generating the password (openssl rand -hex 24) as a required step.
P3 (nits): docker-compose.dev.yml:66 and docker-compose.staging.yml:47 still use the lax ${POSTGRES_PASSWORD?...} form now hardened to :? in prod (follow-up, not this PR); the .env.example:56 section header still says "Optional" directly above the now-required-for-Docker password; no CI gate runs docker compose config, so interpolation regressions are only caught manually (optional hardening).
What's good: the ? → :? hardening is the non-obvious correct pairing for the empty placeholder — the old form accepts set-but-empty, which would have silently passed an empty password to postgres initdb. The DIRECT_URL override lands at exactly the right layer (prisma/prisma.config.ts:19 resolves DIRECT_URL ?? DATABASE_URL; the bot entrypoint runs prisma migrate deploy; compose environment: takes precedence over env_file), matching what dev/staging compose already did. Comments state the why in both files, and Docs.tsx is updated in the same PR.
Override DIRECT_URL inside compose so Prisma migrate reaches postgres from the container. Document URL-safe hex passwords, leave POSTGRES_PASSWORD empty in .env.example, and use :? guards so empty and unset both fail loudly.
853a9bd to
8ed8465
Compare
|
Addressed the approve-with-nits feedback.
Rebased onto main. Danger 403 / SonarCloud on the fork side look like token/permission noise rather than this diff. |
## Summary Two structural failures block every fork PR's required checks (seen on #1863, #1864, #1865, #1866, #1867, #1674 after their CI was approved): - **SonarCloud Scan (required) hard-fails on forks**: fork PRs get no secrets, so `SONAR_TOKEN` is never present and the token-policy step exits 1. Now the sonar job is skipped for fork PRs (a skipped required check counts as passing). Same pattern deploy-staging already uses. - **danger 403s on forks**: `review-tools.yml` ran on `pull_request`, where the fork token is forced read-only and the comment POST fails with 403. Switched to `pull_request_target`; the reusable workflow checks out and executes base-repo code only (documented in the file header, same safety rule as the other target workflows). ## Test plan - [x] actionlint clean on both files - [ ] Next push to an external contributor PR: SonarCloud Scan shows skipped, danger posts its comment After merge I will update the seven open contributor branches to main so they pick this up. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Unblocks fork PRs by fixing CI gates for SonarCloud and `danger`. Fork PRs now pass required checks without secrets and get review comments. - Bug Fixes - Skip SonarCloud Scan on fork PRs to avoid failing when `SONAR_TOKEN` is unavailable (skipped required check counts as passing). - Run review tools on `pull_request_target` so `danger` can comment on forks; workflow executes base-repo code only. <sup>Written for commit 316f567. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1898?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
## Summary PR #1674 (external contributor) fails danger with no possible resolution: the `.env*` protection rule fires on `.env.example`, and its own message demands "explicit confirmation per project policy" but the code offers no confirmation mechanism. Any PR touching the tracked template fails forever. Fix: exempt the standard template names (`.env.example`, `.env.sample`, `.env.template`) from the fail. Real `.env` files still fail. Danger runs on `pull_request_target` against base-repo code, so once this merges, re-running danger on #1674 picks it up without a branch update. ## Test plan - [ ] danger re-run on #1674 passes (only the changelog warning remains) <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Exempts `.env.example`, `.env.sample`, and `.env.template` from the Danger `.env*` protection so template updates don’t fail CI, while still blocking real `.env` files. This unblocks PRs that touch tracked env templates (e.g., #1674). - **Bug Fixes** - Updated `dangerfile.ts` to ignore standard env templates in the `.env*` check. - The rule still fails on actual `.env` files. - Re-running Danger on affected PRs will pass. <sup>Written for commit 0520fe5. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1899?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
|
A note from the maintainer side: sorry this PR waited as long as it did for a proper review, and sorry for the rounds of branch updates and re-running checks today. The churn was on our side, not yours. Your PRs exposed real gaps in how this repo handled external contributions: CI runs sat in a silent approval queue, some gates could never pass on fork PRs (SonarCloud, danger), and the team had no notification when external PRs arrived. Those are all fixed as of today:
Your branch is up to date and the full suite is green. Thanks for the patience and for the contribution. External contributors are very welcome here. |
…1881) ## What `docker-compose.yml` guarded the Postgres password with `${POSTGRES_PASSWORD?POSTGRES_PASSWORD_required}`. Docker Compose fires that form **only when the variable is absent**. A present-but-empty value passes straight through, so `POSTGRES_PASSWORD=` in `.env` starts Postgres with an empty password and no warning. Verified against `docker-compose` 5.1.4: | `.env` state | `${VAR?err}` | `${VAR:?err}` | |---|---|---| | unset | errors | errors | | `POSTGRES_PASSWORD=` (empty) | **passes** | errors | | real value | passes | passes | Both interpolation sites now use `:?`, so the guard means what it looks like it means. Confirmed against the real compose file: empty value now fails with `required variable POSTGRES_PASSWORD is missing a value: POSTGRES_PASSWORD_required`, a real value renders the URL normally. Also documents generating the value in the README setup block. It is interpolated straight into a `postgres://` URL, so it has to be URL-safe, hence `openssl rand -hex 24`. Duplicate keys in `.env` resolve last-wins (verified), so `>>` is safe, and it avoids the `sed -i` GNU/BSD portability trap in a public README. ## Why now Came out of reviewing #1674, which proposes an **active** 48-zero placeholder in `.env.example`. That would permanently disable this guard for anyone following the documented `cp .env.example .env` flow. The fix there is an empty placeholder, which only works once the guard actually rejects empty values, which is this PR. This is a pre-existing weakness, not something #1674 introduced. ## Decision record `decisions/2026-07-26-postgres-password-env-guard.md` covers the alternatives, including the `scripts/setup-env.sh` option and why it is the most likely thing to revisit. ## Not addressed here Changing `POSTGRES_PASSWORD` in `.env` after first boot does not change the password, because `PGDATA` is initialised once and the volume persists the original. Pre-existing; filed separately. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Treats an empty `POSTGRES_PASSWORD` as missing in Docker Compose so dev and staging stacks fail fast instead of starting Postgres with an empty password. Updates the README to generate a URL‑safe password and adds a decision record. - **Bug Fixes** - Switched guards to `${POSTGRES_PASSWORD:?POSTGRES_PASSWORD_required}` in `docker-compose.staging.yml` and `docker-compose.dev.yml` for `DATABASE_URL`, `DIRECT_URL`, and the Postgres service. - **Migration** - Set a non-empty, URL-safe `POSTGRES_PASSWORD` before `docker compose up`, e.g. `echo "POSTGRES_PASSWORD=$(openssl rand -hex 24)" >> .env` <sup>Written for commit eacd52b. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1881?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
🤖 I have created a release *beep* *boop* --- <details><summary>2.38.0</summary> ## [2.38.0](v2.37.3...v2.38.0) (2026-07-27) ### Features * **bot:** add /ticket-setup for support category and agent role ([#1863](#1863)) ([3f4af39](3f4af39)) * **frontend:** per-action loading and connection gating on music controls ([#1866](#1866)) ([2dda60f](2dda60f)) * **frontend:** show stale progress when music SSE lags ([#1867](#1867)) ([4952e73](4952e73)) * **music:** surface recommendationReason in nowplaying and queue ([#1864](#1864)) ([960fd62](960fd62)) * **ops:** blue/green zero-downtime deploys — Phase 1 web tier ([#1786](#1786)) ([f5f7597](f5f7597)) ### Bug Fixes * **docker:** make compose stack boot from a fresh .env ([#1674](#1674)) ([babe0ef](babe0ef)) * **docker:** treat an empty db password as missing in compose guards ([#1881](#1881)) ([718c0ad](718c0ad)) * **frontend:** make landing page usable at mobile widths ([#1865](#1865)) ([6190350](6190350)) * **frontend:** stop hero grid columns overflowing on narrow viewports ([#1874](#1874)) ([ce5cea0](ce5cea0)) * **invite:** add /invite where cloudflare pages reads it ([#1895](#1895)) ([0528f66](0528f66)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Following the documented Docker quickstart on a clean checkout fails before the bot starts:
cp .env.example .env # set DISCORD_TOKEN and CLIENT_ID docker compose up -dTwo separate issues block it.
1. Missing POSTGRES_PASSWORD
The compose file requires it (
${POSTGRES_PASSWORD?POSTGRES_PASSWORD_required}), but.env.exampleonly carries it inside a commented block further down. The copied.envhas no active value, so compose exits right away:2. DIRECT_URL resolves to localhost inside the container
The bot runs
prisma migrate deployon startup, and the Prisma CLI readsDIRECT_URL. The shared app env overridesDATABASE_URLto thepostgresservice but leavesDIRECT_URLon the.envlocalhost value, so the migration step fails:Fix
.env.example: add an activePOSTGRES_PASSWORDin the database section, with a command to generate one.docker-compose.yml: overrideDIRECT_URLnext toDATABASE_URLin the shared app env, so both point at thepostgresservice.Verification
With both changes,
docker compose configresolvesDIRECT_URLtopostgresql://discordbot:***@postgres:5432/discordbotfor the bot and backend, and the documented quickstart brings the bot up with migrations applied and no manual edits.Checklist
docker compose configvalidated)Destructive / irreversible interaction (Tier A)
Not applicable. No Discord actions are added or changed.
Feature-removal sweep
Not applicable. Nothing is removed.
Summary by cubic
Fixes the Docker quickstart so the compose stack boots from a fresh
.envwithout manual edits. Validates a URL-safe hexPOSTGRES_PASSWORDand overridesDIRECT_URLin compose so Prisma migrations reach thepostgresservice..env.example: add an active, emptyPOSTGRES_PASSWORDwithopenssl rand -hex 24guidance; require URL-safe hex; note compose overridesDATABASE_URL/DIRECT_URLinside containers.docker-compose.yml: use${POSTGRES_PASSWORD:?POSTGRES_PASSWORD_required}to fail on unset or empty; setDIRECT_URLto thepostgresservice to avoid PrismaP1001.POSTGRES_PASSWORDbefore the firstdocker compose up; env table and rotation steps require URL-safe hex and show how to generate it.Written for commit 9cd5d2f. Summary will update on new commits.
Summary by CodeRabbit
POSTGRES_PASSWORDplaceholder with clearer guidance for local vs Docker-based Postgres setups.DIRECT_URLto the Postgres service hostname, enabling reliable in-container migrations.