chore(release): make staging smoke migration-owned and dependency-aware - #22
Merged
Conversation
A deployment provisioned by `migration:run` and started with
DB_SYNCHRONIZE=false connects, reports healthy, and then fails every insert:
QueryFailedError: null value in column "id" of relation "assets"
violates not-null constraint
The migrations declare every uuid primary key as `"id" uuid PRIMARY KEY` with
no default, but the entities use @PrimaryGeneratedColumn('uuid'), which the
TypeORM Postgres driver implements through the column default rather than
generating the value in the application. All fifteen uuid primary keys were
affected. AssetsService.create() never sets an id, so POST /api/assets 500s on
any migrated database.
The same class of drift left asset_type_enum with four of the six values
AssetType declares, so MODEL and ORACLE could not be persisted, and left
purchases."transactionHash" two characters short of the entity's varchar(64) —
which a later synchronize would reconcile by dropping and re-adding the column,
discarding the hashes the replay guard depends on.
None of this was visible because docker-compose.yml and the ci workflow both ran
with DB_SYNCHRONIZE=true, and auto-synchronize adds the defaults.
data-source.ts also imported the three delivery entities without listing them,
so the next `migration:generate` would have planned to drop delivery_commands,
delivery_outbox and delivery_results.
Refs #17
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
validateConfig() warns that token operations are unavailable when
STELLAR_ADMIN_SECRET_KEY is absent, and the next line called
Keypair.fromSecret('') anyway, which throws and kills the bootstrap. The
warning promised graceful degradation the code did not deliver, and requiring
a long-lived Stellar secret merely to start the process works against keeping
one out of CI.
The keypair is now derived only when a secret is configured, and the two call
sites that sign go through requireAdminKeypair(), so a missing secret surfaces
as a clear error on the operation that needs it rather than as a boot failure.
Refs #17
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GET /api/health caught the database error, set db: 'error', and returned status: 'ok' with HTTP 200 regardless, so the Docker HEALTHCHECK only ever proved the process was listening. Its own spec asserted that a failed query still yields 'ok'. It now checks the database, the applied migration, Soroban RPC, the marketplace contract and the delivery worker, and answers 503 when a required dependency is down. Reporting the applied migration also catches the case this issue is about: a deployment started with DB_SYNCHRONIZE=false against a schema no migration has built connects fine and fails on first query. Soroban RPC counts as required only once a real marketplace contract is configured; a value containing PLACEHOLDER is treated as absent, matching how TokensService.validateConfig() and the purchase mock gate already read it. The RPC probe is cached for 15s and times out after 2s so an unauthenticated, unthrottled endpoint cannot be used to amplify traffic at the RPC. /api/health/live is new and answers process liveness only, for restart probes that should not cycle a container over a transient dependency outage. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things stood between a clean checkout and a deployment that starts. Nothing applied migrations. entrypoint.sh went straight from waiting on PostgreSQL to `exec node dist/main`, so with DB_SYNCHRONIZE=false the container served traffic against whatever schema happened to exist. It now runs them and, under `set -e`, aborts the boot if any fails. RUN_MIGRATIONS_ON_START=false hands that to a separate release job, which multi-replica rollouts need because concurrent migration runners race. `npm run build && npm run start:prod` could not start. The root-level demo-prompt.ts and test-kms.ts widened the TypeScript root directory, so `nest build` emitted dist/src/main.js while start:prod and entrypoint.sh both run `node dist/main`. The Docker image escaped this only because its builder stage copies just src/ (Dockerfile:7); tsconfig.build.json now makes that restriction explicit, so dist/main.js and dist/database/data-source.js are emitted the same way everywhere. migration:run:prod runs migrations from dist/, which the production image needs since it prunes ts-node. Compose built its schema by auto-synchronize while declaring NODE_ENV=production, which is what hid the schema drift. It now defaults DB_SYNCHRONIZE to false. The api service also no longer waits on MinIO: no file under src/ imports @aws-sdk/client-s3, so that gate could only ever delay startup for a bucket nothing reads. The service stays defined for the blob-storage work. AWS_KMS_KEY_ID is required in production but was missing from .env.example entirely, so filling that file in still produced a container that aborted at bootstrap. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Runs the market journey against a real PostgreSQL whose schema was built by migrations, with DB_SYNCHRONIZE=false — the configuration a deployed environment uses, and the one under which auto-sync can no longer hide drift. Eighteen assertions covering the deploy shape (migrations applied, every uuid primary key defaulted, every AssetType storable, no drift that changes what the database can hold), the wallet handshake against real Ed25519 signatures including wrong-signer and replayed-challenge rejection, a write path that does not supply its own primary key, and the purchase guards this issue asks about: unauthorized confirmation, duplicate confirmation, replayed transaction hash, and cross-wallet access to a settled purchase and to a delivery result. It refuses to run without DB_HOST and DB_NAME rather than passing vacuously, and uses its own jest config so `npm run test:e2e` — whose specs mock their data layer and need no database — is unaffected. What it does not prove is stated in the file header rather than implied: with no marketplace contract configured the confirmation preceding each guard is mock-verified, and no encrypted delivery result can exist because confirmation does not enqueue a delivery command. The real-settlement case is left as a todo naming its blockers instead of asserting a substitute for it. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e suite Builds, verifies the deploy entrypoints were emitted, applies migrations with the compiled data source, boots the compiled application under production env validation with DB_SYNCHRONIZE=false, probes /api/health for the expected applied migration, then runs the smoke suite against that database. Deliberately a separate workflow from `ci`. That one owns the lint, test and build quality gates and is red on main for reasons tracked in #14; gating release evidence on it would mean this never runs. It touches none of those steps. No SOROBAN_MARKETPLACE_CONTRACT_ID and no STELLAR_ADMIN_SECRET_KEY are set anywhere in it. The admin key that production validation demands is generated inside a single step, never written to a file or to $GITHUB_ENV, and never used to sign; a final step fails the run if a Stellar secret seed appears in a tracked file or in the captured application log. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The runbook carries the sixteen-variable production environment contract taken from env.validation.ts rather than from the README, the deploy and verify procedure, rollback and reconciliation notes, and secret ownership. Values only a maintainer with deploy access can produce — URLs, contract ids, image digests, the smoke run — are empty TODO(maintainer) fields next to the command that produces each. An invented URL here would read as evidence, so there are none. The known gaps that block criteria 3 and 4 are listed as blockers with the issue that owns each, not omitted. ADR 004 records why migrations, not synchronize, own the deployed schema, and which drift the smoke suite tolerates: identifier-only differences, because the migrations name indexes and constraints explicitly while TypeORM derives hashed names, and neither affects a deployment running with synchronize false. README gains the health endpoints, the smoke suite, the new workflow, and a pointer to the runbook. Its environment table also had eight rows stranded below the marketplace section by an earlier edit; they are back inside the table. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The migration narrowed the column from varchar(128) to the entity's varchar(64) on the stated grounds that "a Stellar transaction hash is 64 hex characters, so no stored value can exceed the narrower width". Nothing enforces that: ConfirmPurchaseDto accepts 32-128 characters with no hex or exact-length check, and confirm() writes the client-supplied string straight to the column on both the failure and the success branch. So the narrowing converted a malformed-but-accepted hash from a stored value into a 22001 error inside confirm(), which nothing catches — a 500 where a 400 belongs, and it would take the PurchaseStatus.FAILED write down with it. It also made the migration unsafe to apply to any database already holding such a row, which under `set -e` in the entrypoint means a boot abort rather than a degraded start. Narrowing the column and tightening the DTO have to happen together, and the DTO belongs to the purchase flow, not to release provisioning. The column keeps the width the migrations gave it; the smoke suite's drift guard names this one difference as accepted and says why, rather than being quietly relaxed. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in the readiness endpoint this branch added. TypeORM creates the `migrations` table only through the migration runner, and DatabaseModule configures neither `migrations` nor `migrationsRun`. So on any deployment where auto-synchronize builds the schema the table simply does not exist, and the schema check reported a required failure — a permanent 503 on a working application, including for the README's own local-setup flow and for a container run with RUN_MIGRATIONS_ON_START=false, which this branch documents as legitimate. The check now reports `skipped` when DB_SYNCHRONIZE is on, and stays required only where migrations genuinely own the schema. The probes were also unbounded and serialized. A partitioned database accepts the TCP connect and never answers, so `SELECT 1` would block until the OS timeout and the schema query would block again after it — the endpoint that exists to tell a load balancer "stop sending traffic" would hang instead of answering 503. Both queries now carry a 3s deadline, and the five checks run concurrently so one slow dependency does not add its latency to the rest. Refs #17 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`docker-compose.yml` defaulted `DB_SYNCHRONIZE` to `false`, and both ADR 004 and
the README said so — but the api service also declares `env_file: .env`, and
Compose interpolates `${DB_SYNCHRONIZE}` from that same file. `.env.example`
still shipped `DB_SYNCHRONIZE=true`, and `cp .env.example .env` is the flow the
README documents, so the only compose path anyone follows resolved the default
to `true`. The container applied all five migrations and then let
auto-synchronize rewrite the result — precisely what ADR 004 says must never
happen, and invisible to the health check, because the migrations table is there
and populated either way.
`.env.example` now sets `false`, so the file the operator copies agrees with the
compose default instead of overriding it, and the README's local setup runs
migrations like every other environment does.
Also corrected in the docs this branch added:
- The runbook told maintainers to run the smoke suite against staging. With a
real marketplace contract configured, building a purchase intent goes to
Soroban RPC and needs a funded buyer — criterion 4's blocker. It now says
plainly that the suite runs against a migrated, unconfigured database.
- The evidence table's image-digest row named a `RepoDigests` command, and
rollback step 1 depended on it. No workflow here builds or pushes an image.
- The environment table said a `PLACEHOLDER` marketplace id is treated as
unconfigured "by token config"; TokensService never inspects that variable.
- The workflow's env block claimed no admin key or contract id is set anywhere
in it, sixty lines above the step that sets both. Reworded to what it actually
does, and the generated keypair is now masked so it cannot surface in the log.
- ADR 004 claimed the smoke suite covers drift generally; it covers the entities
registered in DatabaseModule, which is why unmigrated `token_transactions` is
invisible to it.
Refs #17
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 tasks
Collaborator
Author
|
Staging-smoke passed on the hosted runner, including migrations, production-like boot, health/readiness, schema verification, and secret-scan. CI test, e2e, and build also passed; the remaining CI failure is the pre-existing repository-wide lint debt tracked in Backend #14 (312 errors/61 warnings). This PR intentionally does not claim the real deployed UI/Freighter/Testnet purchase and delivery criteria. |
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
2 participants
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.
Refs #17
Summary
Test Plan
Notes
This is the migration-owned staging/release slice. The full deployed market journey remains blocked by the dependencies documented in the issue and runbook; this PR must not auto-close #17.