Repository navigation
ci: run the @cipherstash/migrate database suites in CI - #1295
Conversation
The migrate integration suites (forward and reverse backfill: resume, the force obligation, the concurrent dual-write guard) skip unless PG_TEST_URL is set, and nothing set it, so they ran nowhere and a skipped suite read as a passing one. The run-tests job now creates a separate migrate_test database on its existing Postgres service and passes PG_TEST_URL through turbo. A separate database because each suite drops the cipherstash schema, which would race the CLI live suites running in parallel in the shared one. A guard test fails if the variable, its database or the step that creates it goes missing. Refs #1294 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TacM4u97tr5yPAFynxYXW
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation — source: Claude Code /code-review medium plus cross-check-review-findings (company-skills), against head a6891cc.
Result: no critical, blocking, or other findings.
Checked:
PG_TEST_URLis read only by the three@cipherstash/migrateintegration suites, so no other package starts running.migrate_testis created beforeRun tests, and the CI image'scipherstashuser owns the container, soCREATE DATABASEis permitted.- The suites'
DROP SCHEMA cipherstash CASCADEstays off the CLI's live database. - The migrate vitest config sets
fileParallelism: false, so the three suites do not race each other on that schema. turbo.jsonpasses the variable through, and the new guard test covers the URL, the separate database, and the create step's order.- No changeset is needed for a CI-only change, and no skills are affected.
Existing review feedback holds no findings (changeset-bot and bot status only), so nothing was deduplicated.
Generated by Claude Code
Requested by Toby · project thread
Summary
Before: the
@cipherstash/migratedatabase tests (the package behindstash encrypt backfillandstash decrypt backfill) skipped themselves in CI, because no workflow setPG_TEST_URL. A broken resume,--forceor concurrent-write guard could merge on green CI.After: they run on every PR, in the existing
Run Testsjob, against its Postgres 16 and 17 containers. A guard test fails if they are switched off again.Changes
tests.yml: a step creates amigrate_testdatabase on the job's Postgres service, andRun testssetsPG_TEST_URLto it. It is a separate database because each migrate suite starts withDROP SCHEMA cipherstash CASCADE, and turbo runs packages in parallel, so in the shared database that drop would race the CLI's live suites.turbo.json: thetesttask passesPG_TEST_URLthrough (turbo withholds unlisted variables).scripts/__tests__/cli-live-postgres-ci.test.mjs: fails ifPG_TEST_URL, its separate database, or the step that creates that database goes missing.Verification
pnpm run test:scripts: 79 files, 1417 tests pass (1416 before, plus the new guard).PG_TEST_URLset,@cipherstash/migrateruns 132 tests and all pass (the suites use a stub decryption client, so they need no CipherStash credentials and no EQL install).psqlstep on the runner.Related
Closes #1294. Follow-up to #1193, whose reverse-backfill suite (
decrypt.integration.test.ts) these settings will also pick up once it merges.Generated by Claude Code
Summary by CodeRabbit