Skip to content

Add migration-checks CI workflow#555

Merged
dblane-digicatapult merged 13 commits into
mainfrom
ENG-313-migration-checks
Jul 14, 2026
Merged

Add migration-checks CI workflow#555
dblane-digicatapult merged 13 commits into
mainfrom
ENG-313-migration-checks

Conversation

@dblane-digicatapult

@dblane-digicatapult dblane-digicatapult commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Checklist

  • Have you read Digital Catapult's Code of Conduct?
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

PR Type

  • Bug Fix
  • Chore
  • Feature

Linked tickets

ENG-313

High level description

Adopts the new migration-checks-npm shared workflow to catch bad knex migrations in CI.

Detailed description

Adds a pull_request-triggered migration-checks.yml that calls digicatapult/shared-workflows/.github/workflows/migration-checks-npm.yml, pointed at src/didweb/migrations. Postgres is provided by the existing docker-compose.yml postgres service (did-web-server), so the CI database version matches what the agent runs.

A dedicated workflow is used rather than a job in test.yml because the lint and seeded-upgrade jobs compare against the pull_request base ref, whereas test.yml here runs on push.

Branch pin: the uses: reference points at @ENG-313-migration-checks until shared-workflows#111 merges, then flips to @main.

Follow-up: there is no seed script in this repo today, so the seeded-upgrade job runs against an empty database for now. Adding didweb smoke seeds is tracked as follow-up in ENG-313's out-of-scope list.

Describe alternatives you've considered

Adding the job to the existing push-triggered test.yml — rejected because the lint and seeded-upgrade jobs would self-skip without a PR base ref.

Operational impact

CI-only. New pull_request job that brings up the existing postgres compose service. No runtime change.

Additional context

Part of the ENG-313 rollout alongside testbed-portal, spec-rag-api, and the hello-world canary.

Adds a pull_request-triggered workflow calling migration-checks-npm from
shared-workflows (branch-pinned until digicatapult/shared-workflows#111
merges). A separate workflow rather than a job in test.yml because the
lint and seeded-upgrade jobs need a pull_request base ref, whereas
test.yml runs on push.

Migrations live in src/didweb/migrations; Postgres is provided by the
existing docker-compose postgres service (did-web-server).

ENG-313
Copilot AI review requested due to automatic review settings July 11, 2026 12:45
@dblane-digicatapult
dblane-digicatapult requested a review from a team as a code owner July 11, 2026 12:45
@dblane-digicatapult dblane-digicatapult added the v:patch Change requires a semver patch version change label Jul 11, 2026

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.

Pull request overview

This PR introduces a dedicated GitHub Actions workflow to run database migration checks on pull requests by reusing Digital Catapult’s shared migration-checks-npm workflow, configured to validate the did:web Knex migrations. It also bumps the NPM package version to keep repo versioning in sync with existing version automation.

Changes:

  • Add PR-triggered .github/workflows/migration-checks.yml that runs migration immutability/order checks plus migrate up/down roundtrips against Postgres.
  • Point migration checks at src/didweb/migrations.
  • Bump package version from 0.20.170.20.18 in package.json and package-lock.json.

Executive summary (≤10 lines)

  • Adds CI coverage for “bad” Knex migrations (immutability, ordering, and roundtrip apply/rollback/apply).
  • Uses shared workflow defaults for Postgres via docker-compose.yml service postgres, matching local runtime config.
  • Risk is low (CI-only, read-only permissions in the new workflow).
  • Merge recommendation: approve.

Blockers (Must-Fix)

  • None identified.

Targeted suggestions

  • None.

Test gap analysis

  • No new runtime logic introduced; workflow-only change. No additional unit/integration tests expected beyond the new CI job itself.

Integration risks

  • CI now requires the runner to successfully start the postgres service from docker-compose.yml and have port 5432 available for the job’s migration steps.
  • The reusable workflow runs npm ci and npm run db:migrate/npm run db:rollback; this repo’s defaults in knexfile.js align with that (host localhost, DB did-web-server, user/password postgres).

Scores (rubric)

Category Score (/10) Reason
Correctness 9 Workflow inputs/defaults align with repo’s knexfile.js and migration directory.
Security 8 CI-only, minimal permissions (contents: read) on the new workflow; reuse of shared workflow is consistent with existing CI patterns.
Testing 8 Adds preventative CI coverage for migration correctness; no app code changes requiring tests.
Maintainability 9 Keeps migration checks isolated from test.yml while reusing a shared workflow.
Performance 10 No runtime performance impact (CI-only).
Integration/Infra 8 Adds a new PR workflow that depends on docker compose bringing up Postgres reliably.
Overall 9 Low-risk CI enhancement aligned with existing workflow reuse patterns.

Release notes draft

  • Add PR CI workflow to validate Knex migrations (immutability, ordering, and migrate/rollback roundtrip).
  • Configure migration checks to run against did:web migrations under src/didweb/migrations.
  • Bump package version to 0.20.18.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
package.json Bumps package version to 0.20.18.
package-lock.json Keeps lockfile version in sync with package.json (0.20.18).
.github/workflows/migration-checks.yml Adds PR-triggered reusable workflow invocation for Knex migration checks targeting src/didweb/migrations.

dhblane added 2 commits July 11, 2026 14:02
Bringing up just the postgres service from the full testnet docker-compose
still validates the whole file, which fails on cert bind mounts that
setup:certs normally provisions. Use the image fallback pinned to the same
postgres image the compose file uses.

ENG-313
dhblane and others added 5 commits July 11, 2026 14:11
Resolves package.json/package-lock.json conflicts from #554 by taking
main; the version-sync bot re-bumps on the next synchronize. Fixes the
merge conflict that was suppressing pull_request CI runs.

ENG-313
Runs the migration-checks shared workflow on push to main and gates the
docker build/release
on it, so a migration that only fails against existing data blocks the
release rather than surfacing at deploy time. Branch-pinned until
digicatapult/shared-workflows#111 merges.

ENG-313
Resolves package.json/package-lock.json version conflict with main.

ENG-313
@dblane-digicatapult
dblane-digicatapult merged commit 47bd6bc into main Jul 14, 2026
21 checks passed
@dblane-digicatapult
dblane-digicatapult deleted the ENG-313-migration-checks branch July 14, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v:patch Change requires a semver patch version change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants