Repository navigation
Conversation
Hono has no suffix matching, so the '/*.svg' and '/*.ico' static routes never matched and every root-level asset vite copies out of public/ returned 404. Mount them by path instead, and lift the duplicated client root into a const.
Closes devarshishimpi#108. apps/node/Dockerfile builds the dashboard and the server bundle in a multi-stage build and ships a runner carrying only the bundle, dist/client, and the node-server workspace's production dependencies (191MB image, 27MB of node_modules). It calls 'npx vite build' rather than the root build script, which chains cf-typegen and would drag wrangler into the image. docker-compose.yml brings up Postgres, Redis, and the app. Migrations are not part of the server bundle, so a one-shot migrate service runs them from the same image and the app gates on it completing; Postgres and Redis are gated on healthchecks rather than bare depends_on, and both are published on 127.0.0.1 so a VPS deploy does not expose them. .env.docker.example documents every variable the container needs to boot.
- Mark the runner's bundle as ESM. tsup emits ESM as index.js, which ran only because Node's module-syntax auto-detection covers it on current 20.19+ and 22.7+ images; a pinned older digest would fail to start. - Derive DATABASE_URL from POSTGRES_USER/PASSWORD/DB instead of hardcoding the credentials. Changing POSTGRES_PASSWORD alone previously left migrate and the app authenticating with the old password, and the app never started because it gates on migrate completing. - Stop documenting Redis as session storage: sessions use InMemorySessionStore, so a restart signs dashboard users out. Recorded as a known limitation. - Drop the telemetry block from the env example. Telemetry lives in apps/worker and is not in this image, so the opt-out variable did nothing. - Note that the bundled credentials are development defaults and that the published ports can be removed.
There was a problem hiding this comment.
Codra Review
✅ Nothing to flag. Reviewed 7 files (364 changed lines) and found no issues worth raising.
Note
4 files could not be reviewed, so this pass is incomplete.
Reviewed commit: d706f79142
ℹ️ About Codra in GitHub
Your team has set up Codra to review pull requests in this repo. Reviews are triggered when you:
- Open a pull request for review
- Mark a draft as ready
Every review posts a summary here. A clean pass also gets a 👍 on the pull request itself.
Owner
|
Closing this as well, as part of #123 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #108.
Adds a Dockerfile for
@codraoss/node-serverand a compose stack that brings up Postgres, Redis, and the app.What's here
apps/node/Dockerfile— multi-stage. Builds the dashboard and the server bundle, then ships a runner with only the bundle,dist/client, and the node-server workspace's production deps. 191MB image, 27MB ofnode_modules.docker-compose.yml—postgres,redis, a one-shotmigrate, andcodra-app..env.docker.example— every variable the container needs to boot.apps/node/README.md— setup and configuration..dockerignore, plus a.gitignoreexception so.env.docker.exampleis tracked.Notes on the implementation
Dashboard build. The image runs
npx vite buildrather than the rootbuildscript, which chainscf-typegenand would pull wrangler into the image.Migrations. The migration runner is a plain script and isn't part of the tsup bundle, so the runner stage carries
packages/db/scriptsandpackages/db/migrations, and a one-shotmigrateservice applies them. The app gates onservice_completed_successfully, so a first boot lands on a ready schema. Re-running is a no-op.Readiness. Postgres and Redis are gated on healthchecks rather than bare
depends_on, which only waits for container start.DATABASE_URLis derived fromPOSTGRES_USER/PASSWORD/DB. Hardcoding it meant changingPOSTGRES_PASSWORDalone leftmigrateand the app on the old credentials, and the app then never started.Deviations from the issue
node:22-alpineandpostgres:16-alpineinstead of 20 and 15, to match CI.127.0.0.1rather than all interfaces, so deploying this to a VPS doesn't put the database on the internet. They ship with development credentials; the README says to changePOSTGRES_PASSWORDand to drop theports:entries if host access isn't needed.Happy to change either.
Also included
9ce0fdcfixes a pre-existing bug inapps/node/src/index.ts: the'/*.svg'and'/*.ico'static routes never matched, since Hono has no suffix matching, so the favicon and the severity icons 404'd. Separate commit, easy to drop if you'd rather it went on its own.Not in scope
START_WORKER/START_APIbelong to #107, and the full self-hosting guide to #109. Queued reviews still aren't processed — the Node review runtime is #106/#107 — so jobs land in Redis and wait. The README says so.Verification
Local, on a clean
docker compose up -d --buildwith volumes removed:/healthz,/,/assets/*,/favicon.svg,/favicon.ico,/icons/*→ 200POST /webhook→ 400; unauthenticatedGET /api/jobs→ 401docker compose down && up→ migrations idempotent, app healthyPOSTGRES_PASSWORDchanged, to check the derived URLnpm run lint,npm run typecheck, and the node-server typecheck all pass