[devinder-dev]

What

Step 2.6 of Phase 2: the API connects to Postgres, and the tests run against a real one.

Stacked on #27 (base feat/schema-migrations; #27 replaces #25): the integration tests
need migration 001. Before #27 merges, this PR's base is switched to main; then it gets one rebase.

Why it works this way

• The API waits for the database before it listens (at most 60 s, by the clock). Readiness
  never checks the database: one database blip would take every pod out at once (the point from
  the review of infra: add the API manifests (2 pods, probes, zero-downtime rollout) #15). A pod with a wrong DATABASE_URL never listens, so its startupProbe fails,
  the rollout fails and relay-deploy rolls back.
• Waiting can't fix a wrong login, so 28P01 (wrong password), 28000 (login not allowed)
  and 3D000 (no such database) stop it at once. Everything else (refused, timeout, 57P03
  "starting up") is retried every 2 s. Each try is limited to the time that is left (at most
  5 s). A try that runs over is abandoned, not cancelled: postgres.js 3.4.9's cancel() drops
  the promise of its own cancel request, so a failed cancel would go uncaught.
• A database connection error answers 503 service_unavailable with Retry-After: 5, in
  our usual error shape. Every other database error stays a 500, including 57014 (a query
  stopped by statement_timeout): slow SQL is our bug, not an outage, and must not look like one.
• postgres.js 3.4.9, pinned (4.0.0 came out on 29 Oct). Pool 5 per pod, idle 30 s,
  connect_timeout 5 s, statement_timeout 5 s, application_name relay-api.
• Signal handlers are set up before the wait, not after listen: bun is process 1 in the
  container, and Linux ignores SIGTERM for process 1 unless it has a handler. Shutdown is
  app.close(), then sql.end({ timeout: 5 }).

Changes

• API: config.ts (DATABASE_URL required, DB_POOL_MAX 1–20, default 5), db.ts (new),
  server.ts, error-handler.ts, the comment in routes/health.ts; unit tests with a fake
  database and a fake clock.
• Tests against a real Postgres: api/scripts/test-db.sh with db:up, test:db,
  db:down and test:unit. A throwaway container on 127.0.0.1:55432, with production's image
  (the digest is read from k8s/data/postgres.yaml) and the same first-start script, so the
  tests log in as relay. db:down removes only the container the script made (it carries a
  label). It takes production's image from api/scripts/postgres-image.sh (feat: schema v1 and migrations before every rollout #27), the same
  script the CI smoke test uses. api/test/integration/: the connection tests and the schema tests.
• CI: api-test runs db:up, then test:db. shellcheck also checks api/scripts/.
• Kubernetes: k8s/app/deployment.yaml: DATABASE_URL from Secret relay-db (key url);
  a startupProbe (45 × 2 s = 90 s, more than the API's own 60 s wait); liveness without the
  15 s delay; terminationGracePeriodSeconds: 30 written out, with what has to fit inside it.
• Docs: docs/api.md (readiness never checks the database; the new 503),
  docs/CONTEXT.md, the error table in ADR 0003.

One change from the reviewed plan

CI starts the test Postgres with docker run, the same way as the existing image smoke test,
instead of a services: container. Services start before checkout, so they can't mount
k8s/data/initdb/, and the first-start script would not run the way production runs it.

How it was checked

• 59 tests pass against the real Postgres, logged in as relay. They include:
  • relay isn't a superuser;
  • our settings arrive on every connection;
  • a wrong password stops at the first answer with 28P01 (no retry is logged);
  • pg_sleep(6) is cancelled at 5 s with 57014;
  • a connection ended with pg_terminate_backend comes back (a pool of exactly one connection);
  • one valid row per table, two NULL idempotency keys don't collide, the limits themselves
    (20 event types, a 255-character key) are accepted, and deleting an owner cascades;
  • 14 rule-breaking rows, each refused with the right SQLSTATE (23514, 23505, 23503,
    23502).
• Negative checks:
  • without DATABASE_URL, the integration tests fail (exit 1) and never skip;
  • without DATABASE_URL, the API stops at once (exit 1, → at DATABASE_URL).
• On the cluster, a server-side dry run of the Deployment is accepted with no Pod Security
  warnings. The same file with a misspelled probe field is refused (--validate=strict).
• Other checks:
  • tsc --strict is clean, and so is shellcheck;
  • Snyk Code: 0 findings in our code;
  • bun audit: 0 vulnerabilities.

Changed after the review

• No cancel on a timed-out try (above). A unit test checks that a late error after the
  timeout is still handled.

• 503 codes listed by name:

  • added 57P02 (Postgres crashed), EPIPE, EHOSTUNREACH and ENETUNREACH;
  • "every 08…" was replaced by the five connection codes by name;
  • 08P01 (protocol violation, a driver bug) and 08007 ("did the commit happen? unknown")
    now stay a 500: "try again" could save a write twice.

• The clock: waitForDatabase uses performance.now(), which never jumps when the system
  clock is set.

• db:up always starts fresh. It removes its own old container first, so an older image,
  first-start script or port binding can't be reused.

• No stopwatch checks in the integration tests. They would be flaky on a slow runner; the
  SQLSTATE and "no retry was logged" prove the same things.

• The terminationGracePeriodSeconds comment is corrected: the 5 s preStop runs inside the
  30 s, before SIGTERM.

• From the review of infra: Postgres 18.6 on a disk that survives deletion #22: the comment on RELAY_DB_PASSWORD in k8s/data/postgres.yaml
  now says it stays in the pod's env on purpose, because runbook 6b reads it again. It's only a
  comment, so Postgres doesn't need to be applied again.

Before it can go live

It needs #22 applied (done on 1 Nov: the Secret relay-db and Postgres) and #27's migration 001. Without the
Secret, the new pods can't start: the rollout fails and rolls back (safe, but red).

The order:

1. infra: Postgres 18.6 on a disk that survives deletion #22 merged and applied (done);
2. feat: schema v1 and migrations before every rollout #27 merged (its deploy runs migration 001);
3. this PR rebased and merged.

Not in this PR:

• docs/CONTEXT.md's "Where we are": updated at the end of Phase 2.
• The API doesn't read any table yet: Phase 3 gives the routes the pool with the first repository.

[AbdikhaniAbdirashid]

Read db.ts, server.ts, error-handler.ts, the integration tests, test-db.sh and the deployment changes, and checked the pin against npm. Approving. One forward-looking note at the end — it isn't a defect here, but it becomes one in step 3.2 if nobody writes it down.

Four things I checked rather than read past

The pin, against the registry:

postgres 3.4.9   published 2026-04-05
postgres 4.0.0   published 2026-10-29   ← three days old

"postgres": "3.4.9", exact, no caret. That is the same two-weeks-old rule I used to pick Node 24 over 26 in ADR 0005, applied to a different dependency on the same day. Worth saying out loud because it means the rule is actually a rule and not a line in a document.

pingDatabase abandons rather than cancels, and the reasoning holds. You say Promise.race still handles the abandoned query's late answer or error — it does, and it is worth being precise about why: race attaches then(resolve, reject) to every input promise, so the slow query already has a rejection handler on it even after the race has settled on the timeout. A late rejection is absorbed instead of becoming an unhandled rejection. If you had instead written await Promise.any or raced a wrapper that drops the reference, that would not be true.

Signal handlers before the wait. bun is process 1 in the container, and Linux ignores SIGTERM for process 1 unless there is a handler — so a pod stopped during the 60 s wait would sit there until SIGKILL. Setting them up first costs nothing and removes a 30 s stall nobody would ever reproduce locally, where the process isn't PID 1.

startupProbe 45 × 2 s = 90 s against the app's own 60 s deadline. The ordering is the right way round: the app gives up first, with ❌ database not reachable, stopping in the log, instead of being killed by the probe halfway through a retry. If the probe were the shorter of the two, every one of these failures would look identical in kubectl describe and tell you nothing about why.

The line I liked most

//   08007  "did the commit happen? unknown". 503 means "try again", which could save it twice

That is the sharpest distinction in the PR, and it is the kind of thing that is invisible until it has already happened twice in production. Same for keeping 57014 a 500: a query killed by statement_timeout is our slow SQL, and dressing it as an outage would mean we never fix it. The comment block listing what is deliberately not in the set is worth more than the set itself.

One note for step 3.2

isDatabaseUnavailable tests the shape of an error code, not its origin. Most of the set is unambiguous — the SQLSTATE codes and the CONNECTION_* ones can only come from postgres.js. But these are generic Node socket and DNS codes:

ECONNRESET  ETIMEDOUT  EPIPE  EHOSTUNREACH  ENETUNREACH  ENOTFOUND  EAI_AGAIN

Today that is fine: nothing else in the API does network I/O, so any of those can only have come from the pool.

Step 3.2 changes that. The server-side SSRF check has to resolve the customer's host name — that is the whole point of doing it server-side, since the form can only see the name. A customer endpoint whose DNS doesn't resolve raises ENOTFOUND or EAI_AGAIN, and if that reaches the error handler unconverted, the API answers

503 service_unavailable, Retry-After: 5
"The service is briefly unavailable, try again shortly"

— telling the customer our database is down when the truth is their URL doesn't resolve. The contract says that case is 422 invalid_endpoint_url. And it is the worst shape of wrong: the customer retries after 5 s, gets the same answer, and concludes Relay is broken.

Two ways out, and I prefer the first:

1. The SSRF check catches its own DNS errors and converts them to invalid_endpoint_url before they ever leave the service. That is where the knowledge belongs anyway: only that code knows the lookup was for a customer's host and not for ours.
2. Make isDatabaseUnavailable require the error to have come from a query, so the generic codes only count when the pool raised them.

Nothing to change in this PR — the set is correct for the code that exists. But a line in db.ts next to the generic codes, something like "generic Node codes: only unambiguous while the pool is the only thing doing network I/O — 3.2 resolves customer host names and must convert its own DNS errors", would put the warning where whoever writes 3.2 will actually be looking.

The test setup

test-db.sh reading the Postgres digest out of k8s/data/postgres.yaml is the right instinct — one place for the version, and the tests run against production's image rather than something close to it. Logging in as relay instead of as the superuser matters for the same reason: a test that runs as superuser can't catch a missing GRANT.

Which brings up the one inconsistency across the stack: #25's migration smoke test hardcodes that digest in pipeline.yml instead of reading it the way test-db.sh does. Bump the version in postgres.yaml and the smoke test quietly keeps testing the old image. It is one PR down the stack so I will leave it there rather than block this — I have added it as a note on #25.

Approving. Please post the integration-test output when you run it against the real cluster, and I would like to see ❌ database not reachable, stopping once with a deliberately wrong DATABASE_URL, since the rollback path depends on that pod failing to listen rather than starting unhealthy.

The base branch was changed.

[AbdikhaniAbdirashid]

Re-approving after the rebase. My approval of 998e9f2 was dismissed by the force-push, so I
compared file contents between that commit and 81e7b63 rather than the patches — at review
time the branch sat on an older schema branch, which makes the patches useless for this.

37 files differ, 34 of them only because main caught up (#9's dashboard, #27's migrations).
Three are yours:

File	Change
.github/workflows/pipeline.yml	the smoke test reads the digest through postgres-image.sh
api/scripts/test-db.sh	its own prod_image() is gone, same script instead
k8s/data/postgres.yaml	the RELAY_DB_PASSWORD comment from #22

No behaviour changed, and the one inconsistency I found is closed: there is now a single place
that knows which Postgres image we run.

Three things I ran rather than read:

• postgres-image.sh prints the pinned image, and with the digest taken out of the manifest it
  exits 1 with its own message on stderr.
• set -e aborts the caller on PG_IMAGE=$(…). So the "on its own line" comment is exactly
  right, and worth keeping: inline as docker run … "$(…)" the shell would have carried on with
  an empty image name.
• REPO_DIR didn't become a dead variable when prod_image() left — INITDB_DIR still uses it.

Prerequisites, from outside: Secret relay-db has url and password, schema_migrations
holds 20261031140445, and postgres-0 plus both API pods are Running. The rollout should be
green.

Still open, and still not a blocker: the generic Node codes in db.ts have no line about step
3.2. I'd rather it sat next to ENOTFOUND than in this thread, where whoever writes the SSRF
check won't be looking — but it's a comment, so it can ride along with 3.2 instead of holding
this up. Nit: two blank lines left where prod_image() was.