Skip to content

NEO-61: apply migrations on deploy and gate traffic on health - #100

Merged
lucasvienna merged 3 commits into
mainfrom
lucas/neo-61-migrations-are-never-applied-on-railway
Aug 25, 2026
Merged

NEO-61: apply migrations on deploy and gate traffic on health#100
lucasvienna merged 3 commits into
mainfrom
lucas/neo-61-migrations-are-never-applied-on-railway

Conversation

@lucasvienna

Copy link
Copy Markdown
Contributor

Already applied to production. Railway does not read the repo, so these settings only took effect when I ran railway config apply. That happened during the work, and the deploys below are the verification. Merging this makes the repo agree with what Railway already has, rather than changing anything.

Description

No Railway service set preDeployCommand, so pending migrations reached production only when someone remembered to run them by hand, and a deploy carrying one came up against the old schema. uppity-server now runs ./entrypoint.sh migrate between build and deploy. A non-zero exit aborts the deploy and leaves the running version serving, which is what entrypoint.sh and migrate.ts already exit for.

Deployment 84c30a66 shows it working:

07:46:14  migrations applied      <- pre-deploy container
07:46:19  Listening on :8080      <- app container
07:46:21  SUCCESS

The journal was already current, so no new DDL ran. The path itself is proven: the pre-deploy container connects, runs the migrator, exits 0, and only then does the app container start.

Why only the web tier

drizzle-orm's PgDialect.migrate reads the newest journal row, then applies every later file in one transaction. It takes no lock, so a second copy racing it would apply the same DDL twice. Running the migration on all three services would need an advisory lock added first.

The comment above the migrator's connection claimed the library takes an advisory lock. It does not, and that claim is what this decision turns on, so it is corrected here.

The cost is that the workers deploy alongside and run against the old schema for the length of the migration. That window is not free — worker-monitor calls initializeMaintenanceJobs() before its poll loop and outside any try, so a migration touching maintenance_job kills it, and restartPolicyMaxRetries is 3. Filed as NEO-63 rather than fixed here. It is still strictly better than the previous state, where the schema was never migrated at all and new worker code met the old schema indefinitely.

Health check

Found while applying. Railway does not read the Dockerfile's HEALTHCHECK — that instruction serves docker run and Compose. Railway uses deploy.healthcheckPath, and uppity-server set none, so a deploy counted as healthy the moment the process started. Railway then switched traffic over and tore down the previous version, whether or not the new container could serve.

/api/health runs SELECT 1 and answers 503 while Postgres is unreachable, so it fails for the reason worth catching rather than answering 200 from a process that has started and nothing more. This matters more now that deploys carry migrations. The workers keep no health check; they serve no HTTP and the probe would fail for them by design.

Related Issues

Closes NEO-61. Files NEO-63 as follow-up. Depends on nothing; #99 is already merged.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • CI/Build changes

Checklist

  • I have read the Contributing Guide
  • I have added a changeset (aubr changeset, or aubr changeset --empty if no release is needed)
  • My code follows the project's code style
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass (aubr test:unit run) — 308/308
  • Linting and formatting pass (aubr lint:ci)
  • Type checking passes (aubr check)
  • I have updated documentation if needed

On tests: no new ones. The change is three declarative lines of deploy config; the behaviour they invoke is already covered. scripts/entrypoint.spec.ts has 24 tests over the migrate target, including its permission set and its network allowlist. A test asserting railway.ts contains the literal it contains would restate the config, not verify it. The real verification is the deploy log above.

On docs: docs/runtime-permissions.md describes what each process may access, not when each runs, so nothing there goes stale.

Additional Notes

While applying this I found that the workers were never actually running. #99 fixed their startCommand in the repo and merged, but nobody ran railway config apply, so it never reached Railway. worker-monitor was logging Listening on http://0.0.0.0:8080/ — running the web server, not the scheduler. Monitor checks and notification delivery had not been running on the hosted deployment.

That is fixed now, by the same apply. Checks against Uppity, DSC, MBOS, wforacle and Apple are landing, and the maintenance jobs ran for the first time, including a cleanup that deleted 3421 old check rows. No code change in this PR does that — it was #99's fix, finally applied.

The lesson generalises: merged IaC is not applied IaC. Worth checking railway config plan reads clean after any PR touching .railway/railway.ts merges.

One wart for whoever applies next: ~ Update uppity-server networking reappears in every plan and never converges. privateNetworkEndpoint: "uppity-server" matches the name Railway derives by default, so Railway appears not to record it and the diff persists. Harmless and graded safe, but it means the plan never reads clean — which is exactly the signal that would have caught #99 sitting unapplied.

https://claude.ai/code/session_01AqHppaB4Px72dTuAvpAEBD

No service set preDeployCommand, so pending migrations reached production
only when someone ran them by hand, and a deploy carrying one came up
against the old schema.

uppity-server now runs ./entrypoint.sh migrate between build and deploy.
A non-zero exit aborts the deploy and leaves the running version serving,
which is what both entrypoint.sh and migrate.ts already exit for.

Only the web tier runs it. drizzle-orm's migrator takes no lock -- its
PgDialect.migrate reads the newest journal row, then applies every later
file in one transaction -- so a second copy racing it would apply the
same DDL twice. The workers deploy alongside and run against the old
schema for the length of the migration.

That window is not free: worker-monitor calls initializeMaintenanceJobs()
before its poll loop and outside any try, so a migration touching
maintenance_job kills it, and restartPolicyMaxRetries is 3. It is still
strictly better than the status quo, where the schema was never migrated
at all and new worker code met the old schema indefinitely. Tracked in
NEO-63.

Also corrects the comment above the migrator's connection, which claimed
an advisory lock that drizzle-orm does not take. That claim is what the
server-only decision turns on.

Refs NEO-61

Claude-Session: https://claude.ai/code/session_01AqHppaB4Px72dTuAvpAEBD
Railway does not read the Dockerfile's HEALTHCHECK -- that instruction
serves docker run and Compose. Railway uses deploy.healthcheckPath, and
uppity-server set none, so a deploy counted as healthy the moment the
process started. Railway then switched traffic over and tore down the
previous version, whether or not the new container could serve.

/api/health runs SELECT 1 and answers 503 while Postgres is unreachable,
so it fails for the reason worth catching rather than answering 200 from
a process that has started and nothing more.

This matters more now that deploys carry migrations: the pre-deploy step
proves the schema applied, and the healthcheck proves the container that
meets that schema can actually serve before it takes traffic.

The workers keep no healthcheck. They serve no HTTP, and the probe would
fail for them by design.

Refs NEO-61

Claude-Session: https://claude.ai/code/session_01AqHppaB4Px72dTuAvpAEBD
Copilot AI lite review requested due to automatic review settings August 25, 2026 07:57
@linear-code

linear-code Bot commented Aug 25, 2026

Copy link
Copy Markdown

NEO-61

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@changeset-bot

changeset-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 867b3fb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
uppity Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

The healthcheck comment claimed /api/health answering 503 was "the state
worth catching here." Railway does not catch 503. It queries the endpoint
until it gets a 200 and fails the deploy at timeout, so every non-200 and
the no-answer case behave alike. The sentence described a specificity the
mechanism does not have, and narrated a route in another directory that
it could not keep up with. What remains is the part that earns its place:
Railway ignores the Dockerfile's HEALTHCHECK, which is a trap this repo
sets for itself by declaring one on the same path.

The max: 1 comment stated its own stakes and they were nothing -- "a pool
would add nothing here but idle sockets." A one-shot script opening one
connection surprises no one, and "so max: 1" above { max: 1 } is the
config reading itself aloud.

The lock sentence stays. drizzle-orm 0.45.2 has no advisory lock anywhere
and a transaction is not mutual exclusion, so two migrators both read the
last applied row before either commits. That is a foreign dependency
behaving surprisingly on a live path, and the railway.ts comment cites it.

Refs NEO-61

Claude-Session: https://claude.ai/code/session_01AqHppaB4Px72dTuAvpAEBD
@lucasvienna
lucasvienna merged commit c01e545 into main Aug 25, 2026
14 checks passed
@lucasvienna
lucasvienna deleted the lucas/neo-61-migrations-are-never-applied-on-railway branch August 25, 2026 08:22
@github-actions github-actions Bot mentioned this pull request Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants