mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-26 04:22:26 +03:00
fix(deploy): make bun dev actually start, and sign-in actually work (#326)
Follow-up to #325. Six defects, all found by running the thing rather than reading it — #325 was verified by bundling, by tests, and by the container images, and none of those start a Worker. | | | |---|---| | `bun dev` never started | one multi-config process does not connect a service binding between the workers it loads — the API reported `AUTH [not connected]`. Two processes now, which is what the dev registry connects | | Neither server could bind | wrangler resolves `localhost` and takes `::1` first; a host with no IPv6 on its loopback dies with a bind error naming neither app nor port. `dev.ip` pinned, and `inspector_port` made distinct — it is not derived from the port, so the second server died on an address already in use | | The API worker failed to evaluate | a specifier ending in `.sql` is claimed by the bundler as its own module, so the schema file was emitted verbatim beside the bundle and the runtime threw on a missing export | | Sign-in failed on the second DB request | a pooled socket created while handling one request may not be touched while handling another on a Worker. The *first* request always succeeded, which is why nobody saw it | | The images named a base podman will not resolve | a short name needs a registry; the database service alongside them already spelled one | | Compose pinned container names | not scoped to the project, so `down` in one checkout stops another's containers — it stopped a running development database while this was being tested | Two of these are worth a second look because they are not confined to local development. **The database pool one is a live bug on Workers**, and it predates #325 — it arrived with the pool cache in `6c1d407`. On a Worker an I/O object created during one request cannot be used during another, so the cached socket throws *"Cannot perform I/O on behalf of a different request"* on the second request that reuses it. The cache is now kept only where a process outlives its requests, which is the case it was added for; a Worker goes back to a pool per invocation, which is what it did before. **The `.sql` import was also a layering break.** A route was reaching past the domain module into the schema to spell a download status. It asks the domain module now, which is the rule everywhere else here and happens to be what removes the bundler hazard. ## Verified A real sign-in, end to end, against both dev servers: a code requested over HTTP, read out of the issuer's log the way a person reads it out of their mail, redeemed, exchanged for tokens, and presented to the API — which resolved it to the account the sign-in had just created, and refused the same request without it. ``` 1. asked for a code -> 200 2. code from the log -> 811548 3. redeemed the code -> 302 4. exchanged for tokens-> access eyJhbGciOiJFUzI1NiIsImtp… 5. GET /user with it -> 200 {"data":{"id":"usr_071b56380001F9L3Wf8OQj8ogS", …}} 6. GET /user without -> 401 ``` Also: 316 tests pass, both images build, and compose substitution resolves with the required-variable guards refusing correctly when `.env` is incomplete. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved game download status validation for more consistent API behavior. - Improved database connection handling in Cloudflare Workers by using request-local connections. - **Developer Experience** - Added explicit local development and debugger ports for the API and authentication services. - Improved development process handling when running services together. - **Chores** - Updated container configuration to support multiple project checkouts without naming conflicts. - Standardized container image references for more reliable builds. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR repairs local development startup and sign-in behavior while making containerized development safer across environments. - Runs the authentication and API Workers as separate, jointly supervised development processes. - Assigns explicit IPv4 listener and distinct inspector ports. - Prevents Cloudflare Workers from reusing database I/O objects across requests. - Exposes download statuses through the domain module rather than importing a schema module from the route. - Corrects the authentication app’s Hono JSX transform configuration. - Uses fully qualified Bun image names and Compose-managed container names. <h3>Confidence Score: 5/5</h3> The PR appears safe to merge, with the previous process-supervision issue resolved and no new actionable regressions identified. The current development scripts stop the sibling server when either process exits, and the changes since the previous review preserve server entrypoint behavior while correcting the Hono JSX runtime selection. The previous thread was manually resolved after the supervision fix. <h3>Important Files Changed</h3> | Filename | Overview | |----------|----------| | package.json | Starts and supervises the two development servers independently; the follow-up direct entrypoint invocation preserves their intended behavior. | | apps/auth/tsconfig.json | Aligns authentication-server JSX transformation with Hono and the repository’s existing TypeScript configuration. | | packages/core/src/db/index.ts | Limits database pool caching to long-lived process environments so Worker requests do not reuse request-bound I/O. | | packages/core/src/game/download.ts | Exposes valid download statuses through the domain namespace for API consumers. | | apps/api/app/routes/game.ts | Uses the domain-level download status export, avoiding a direct runtime import of the SQL schema module. | | docker-compose.yml | Removes globally fixed container names so Compose projects remain isolated between checkouts. | <h3>Flowchart</h3> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart LR Dev["bun dev"] --> Auth["Auth Worker<br/>127.0.0.1:1337<br/>Inspector 9229"] Dev --> API["API Worker<br/>127.0.0.1:3000<br/>Inspector 9230"] API -->|AUTH service binding| Auth Auth --> DB["Request-local DB pool<br/>on Cloudflare Workers"] API --> DB Supervisor["Process supervisor"] --> Dev Auth -->|Either process exits| Supervisor API -->|Either process exits| Supervisor Supervisor -->|Stops sibling process| Dev ``` <sub>Reviews (3): Last reviewed commit: ["fix(deploy): make \`bun run dev:server\` s..."](https://github.com/nestrilabs/nestri/commit/46c3a67d4dd24544288b4bfeed5c6685dd4e60e8) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=60753832)</sub> <!-- /greptile_comment -->
This commit is contained in:
Wanjohi
+5
-2
@@ -10,7 +10,10 @@
|
||||
# exclude the whole TypeScript half, because the guest rootfs build was the
|
||||
# only Dockerfile here — that part now lives in `build/Dockerfile.dockerignore`,
|
||||
# beside the build it belongs to.
|
||||
FROM oven/bun:1.3.11-alpine AS deps
|
||||
# The registry host is part of the name on purpose: podman refuses a
|
||||
# short name that resolves to nothing, and a self-hoster is as likely to
|
||||
# have podman as docker.
|
||||
FROM docker.io/oven/bun:1.3.11-alpine AS deps
|
||||
|
||||
WORKDIR /app
|
||||
|
||||
@@ -30,7 +33,7 @@ COPY packages/auth/package.json packages/auth/
|
||||
RUN bun install --frozen-lockfile --production
|
||||
|
||||
|
||||
FROM oven/bun:1.3.11-alpine AS runtime
|
||||
FROM docker.io/oven/bun:1.3.11-alpine AS runtime
|
||||
|
||||
WORKDIR /app
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@ import { ErrorCodes, VisibleError } from '@nestri/core/error';
|
||||
import { Examples } from '@nestri/core/examples';
|
||||
import { Depot } from '@nestri/core/game/depot';
|
||||
import { GameDownload } from '@nestri/core/game/download';
|
||||
import { GameDownloadStatus } from '@nestri/core/game/download.sql';
|
||||
import { Game } from '@nestri/core/game/index';
|
||||
import { Identifier } from '@nestri/core/id';
|
||||
import { Library } from '@nestri/core/user/library';
|
||||
@@ -448,7 +447,7 @@ export namespace GameApi {
|
||||
description: 'Steam application ID',
|
||||
example: Examples.Game.steamAppId
|
||||
}),
|
||||
status: z.enum(GameDownloadStatus.enumValues).meta({
|
||||
status: z.enum(GameDownload.Status).meta({
|
||||
description: 'New download status',
|
||||
example: Examples.GameDownload.status
|
||||
}),
|
||||
|
||||
+11
-1
@@ -12,8 +12,18 @@
|
||||
"compatibility_date": "2026-09-05",
|
||||
"compatibility_flags": ["nodejs_compat"],
|
||||
"workers_dev": false,
|
||||
// `ip` is pinned rather than left to default. Wrangler otherwise binds
|
||||
// whatever `localhost` resolves to, which is `::1` first on most systems
|
||||
// — and a host with no IPv6 address on its loopback interface fails to
|
||||
// start at all, with a bind error from deep inside the runtime rather
|
||||
// than anything naming a port.
|
||||
"dev": {
|
||||
"port": 3000
|
||||
"ip": "127.0.0.1",
|
||||
"port": 3000,
|
||||
// Distinct per app. Both dev servers run at once and the debugger
|
||||
// port is not derived from the one above, so leaving it default
|
||||
// meant the second to start died on an address already in use.
|
||||
"inspector_port": 9230
|
||||
},
|
||||
|
||||
// `AUTH` routes by binding rather than by hostname, so it is one hop
|
||||
|
||||
@@ -10,7 +10,10 @@
|
||||
# exclude the whole TypeScript half, because the guest rootfs build was the
|
||||
# only Dockerfile here — that part now lives in `build/Dockerfile.dockerignore`,
|
||||
# beside the build it belongs to.
|
||||
FROM oven/bun:1.3.11-alpine AS deps
|
||||
# The registry host is part of the name on purpose: podman refuses a
|
||||
# short name that resolves to nothing, and a self-hoster is as likely to
|
||||
# have podman as docker.
|
||||
FROM docker.io/oven/bun:1.3.11-alpine AS deps
|
||||
|
||||
WORKDIR /app
|
||||
|
||||
@@ -30,7 +33,7 @@ COPY packages/auth/package.json packages/auth/
|
||||
RUN bun install --frozen-lockfile --production
|
||||
|
||||
|
||||
FROM oven/bun:1.3.11-alpine AS runtime
|
||||
FROM docker.io/oven/bun:1.3.11-alpine AS runtime
|
||||
|
||||
WORKDIR /app
|
||||
|
||||
|
||||
@@ -4,8 +4,14 @@
|
||||
"compilerOptions": {
|
||||
"module": "ESNext",
|
||||
"moduleResolution": "bundler",
|
||||
"jsx": "preserve",
|
||||
"jsxImportSource": "react",
|
||||
// The sign-in screens are `hono/jsx`, and there is no React in this
|
||||
// repository at all. Naming React here was left over from an earlier
|
||||
// scaffold and only bit when something transpiled from this directory:
|
||||
// `bun run serve` resolved `react/jsx-dev-runtime` from a component in
|
||||
// `packages/auth` and stopped before the server bound a port. The
|
||||
// package holding the components already says this; so does the root.
|
||||
"jsx": "react-jsx",
|
||||
"jsxImportSource": "hono/jsx",
|
||||
"types": ["@cloudflare/workers-types", "node", "bun"]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,8 +18,18 @@
|
||||
// second issuer as far as a token's `iss` claim is concerned, and every
|
||||
// token minted through it is rejected by the API.
|
||||
"workers_dev": false,
|
||||
// `ip` is pinned rather than left to default. Wrangler otherwise binds
|
||||
// whatever `localhost` resolves to, which is `::1` first on most systems
|
||||
// — and a host with no IPv6 address on its loopback interface fails to
|
||||
// start at all, with a bind error from deep inside the runtime rather
|
||||
// than anything naming a port.
|
||||
"dev": {
|
||||
"port": 1337
|
||||
"ip": "127.0.0.1",
|
||||
"port": 1337,
|
||||
// Distinct per app. Both dev servers run at once and the debugger
|
||||
// port is not derived from the one above, so leaving it default
|
||||
// meant the second to start died on an address already in use.
|
||||
"inspector_port": 9229
|
||||
},
|
||||
|
||||
// Local-only settings live in `.dev.vars` beside this file rather than in
|
||||
|
||||
+7
-3
@@ -9,6 +9,13 @@
|
||||
# docker compose up --build everything, built from source
|
||||
# docker compose up postgres just the database, for `bun dev`
|
||||
#
|
||||
# No service sets `container_name`. It reads like a nicety and behaves like a
|
||||
# global: the name is not scoped to the project, so a second checkout of this
|
||||
# repo — a worktree, a colleague's clone — gets the same three names, and
|
||||
# `docker compose down` in one of them stops the containers belonging to the
|
||||
# other. Compose names them per project on its own, which is what makes two
|
||||
# checkouts independent.
|
||||
#
|
||||
# **There are no credentials in this file, and none of them have defaults.**
|
||||
# Every one is read from `.env`, and compose refuses to start naming the
|
||||
# variable it wanted rather than falling back to something. A default is worth
|
||||
@@ -26,7 +33,6 @@ x-postgres-url: &postgres-url
|
||||
services:
|
||||
postgres:
|
||||
image: docker.io/postgres:18-alpine
|
||||
container_name: nestri_postgres
|
||||
environment:
|
||||
POSTGRES_USER: ${POSTGRES_USER:?set POSTGRES_USER in .env}
|
||||
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in .env}
|
||||
@@ -52,7 +58,6 @@ services:
|
||||
# there. Same reason for the API below.
|
||||
context: .
|
||||
dockerfile: apps/auth/Dockerfile
|
||||
container_name: nestri_auth
|
||||
depends_on:
|
||||
postgres:
|
||||
condition: service_healthy
|
||||
@@ -80,7 +85,6 @@ services:
|
||||
build:
|
||||
context: .
|
||||
dockerfile: apps/api/Dockerfile
|
||||
container_name: nestri_api
|
||||
depends_on:
|
||||
postgres:
|
||||
condition: service_healthy
|
||||
|
||||
+2
-2
@@ -19,8 +19,8 @@
|
||||
},
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"dev": "wrangler dev -c apps/auth/wrangler.jsonc -c apps/api/wrangler.jsonc",
|
||||
"dev:server": "bun run --cwd apps/auth serve & bun run --cwd apps/api serve",
|
||||
"dev": "wrangler dev -c apps/auth/wrangler.jsonc & issuer=$!; wrangler dev -c apps/api/wrangler.jsonc & api=$!; trap 'kill $issuer $api 2>/dev/null' EXIT INT TERM; while kill -0 $issuer 2>/dev/null && kill -0 $api 2>/dev/null; do sleep 1; done",
|
||||
"dev:server": "bun run apps/auth/src/server.ts & issuer=$!; bun run apps/api/app/server.ts & api=$!; trap 'kill $issuer $api 2>/dev/null' EXIT INT TERM; while kill -0 $issuer 2>/dev/null && kill -0 $api 2>/dev/null; do sleep 1; done",
|
||||
"dev:docker": "docker compose up --build",
|
||||
"db:migrate": "bun run --cwd packages/core db:migrate",
|
||||
"db:push": "bun run --cwd packages/core db:push",
|
||||
|
||||
@@ -67,10 +67,31 @@ export namespace Database {
|
||||
|
||||
const clients = new Map<string, Client>();
|
||||
|
||||
/**
|
||||
* Whether a pool may outlive the request that opened it.
|
||||
*
|
||||
* On a Worker it may not. An I/O object created while handling one request
|
||||
* cannot be touched while handling another — *"Cannot perform I/O on behalf
|
||||
* of a different request"* — so a kept socket is not a saving there, it is
|
||||
* an error thrown on the second request that reuses it, and the first
|
||||
* request always succeeds. That shape is why it went unnoticed: a single
|
||||
* call works, and a sign-in is two.
|
||||
*
|
||||
* A long-lived process has the opposite problem, which is what the cache
|
||||
* exists for — so the answer is not one rule but this test.
|
||||
*/
|
||||
const poolsOutliveRequests = !(
|
||||
typeof navigator !== 'undefined' && navigator.userAgent === 'Cloudflare-Workers'
|
||||
);
|
||||
|
||||
export function client(): Client {
|
||||
const url = Env.get().DATABASE_URL || process.env.DATABASE_URL;
|
||||
const key = url ?? 'local:nestri';
|
||||
|
||||
if (!poolsOutliveRequests) {
|
||||
return connect(url);
|
||||
}
|
||||
|
||||
const cached = clients.get(key);
|
||||
if (cached) {
|
||||
return cached;
|
||||
|
||||
@@ -8,6 +8,18 @@ import { Identifier } from '../id.js';
|
||||
import { GameDownloadStatus, GameDownloadTable } from './download.sql.js';
|
||||
|
||||
export namespace GameDownload {
|
||||
/**
|
||||
* The statuses a download can be in, for callers that need the list.
|
||||
*
|
||||
* Re-exported from the schema so that nothing outside this module has to
|
||||
* import a `.sql` module to spell a status. That is the layering rule
|
||||
* everywhere here, and it also avoids a concrete hazard: a specifier
|
||||
* ending in `.sql` is claimed by the Workers bundler as a module of its
|
||||
* own, which emitted this file's source verbatim beside the bundle and
|
||||
* failed at startup with an export it could not find.
|
||||
*/
|
||||
export const Status = GameDownloadStatus.enumValues;
|
||||
|
||||
export const Info = z
|
||||
.object({
|
||||
id: z.string().meta({
|
||||
@@ -22,7 +34,7 @@ export namespace GameDownload {
|
||||
description: 'The game being downloaded',
|
||||
example: Examples.GameDownload.gameId
|
||||
}),
|
||||
status: z.enum(GameDownloadStatus.enumValues).meta({
|
||||
status: z.enum(Status).meta({
|
||||
description: 'Current download status',
|
||||
example: Examples.GameDownload.status
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user