diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bfbc6845c7..3a0b89eb6b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -444,3 +444,29 @@ jobs: - name: Knip (unused code gate) run: bun run knip:check + + migration-safety: + name: Destructive Migration Check + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + fetch-depth: 0 + + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 + + # Newly added packages/db/drizzle/*.sql migrations that contain a destructive + # statement (DROP TABLE/COLUMN, TRUNCATE, a type change, an enum-swap rename, + # or a NOT NULL column with no DEFAULT) must carry a leading + # `-- destructive-migration-ack: ` comment. See + # scripts/check-destructive-migrations.ts for the full pattern list and scope + # notes (per-file only, not retroactive on existing migrations). + - name: Check for unacknowledged destructive migrations + run: | + BASE_REF="${{ github.event.pull_request.base.sha || github.event.before }}" + if [ -z "$BASE_REF" ] || [ "$BASE_REF" = "0000000000000000000000000000000000000000" ]; then + BASE_REF="HEAD~1" + fi + bun scripts/check-destructive-migrations.ts "$BASE_REF" diff --git a/.github/workflows/docker-images.yml b/.github/workflows/docker-images.yml index 8d5164f50d..9f6927a487 100644 --- a/.github/workflows/docker-images.yml +++ b/.github/workflows/docker-images.yml @@ -33,13 +33,17 @@ on: - 'scripts/**' - 'bun.lock' - '.github/workflows/docker-images.yml' + # deploy-staging/smoke-test/deploy-fly run these directly from the checked-out + # commit (not baked into any image) — without this, a fix here sits inert until + # an unrelated apps/packages change happens to also touch master. + - 'scripts/deploy/**' env: REGISTRY: ghcr.io IMAGE_PREFIX: ghcr.io/2witstudios/pagespace -# Serialize deploys: back-to-back merges to master queue instead of racing on the -# mutable :latest tag (the slower run would otherwise win and land prod on an older +# Serialize deploys: back-to-back merges to master queue instead of racing each other +# (the slower run would otherwise finish last and clobber a newer deploy with an older # commit). Queue rather than cancel so an in-flight migration/deploy is never interrupted. concurrency: group: deploy-fly-${{ github.ref }} @@ -158,14 +162,18 @@ jobs: ${{ matrix.service == 'admin' && format('SENTRY_ORG={0}', secrets.SENTRY_ORG) || '' }} ${{ matrix.service == 'admin' && format('SENTRY_PROJECT={0}', secrets.SENTRY_PROJECT_ADMIN) || '' }} - deploy-fly: - name: Deploy to Fly.io + deploy-staging: + name: Deploy to Staging needs: [build-and-push, ci] runs-on: ubuntu-latest - environment: fly-production + environment: fly-staging if: github.ref == 'refs/heads/master' steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - name: Setup flyctl uses: superfly/flyctl-actions/setup-flyctl@master @@ -181,77 +189,69 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - - name: Run migrations + # Same sha-pinned tag that smoke-test verifies and deploy-fly (prod) later + # promotes — staging and prod always run the exact same artifact. + - name: Run staging migrations run: | - docker pull ghcr.io/2witstudios/pagespace-migrate:latest - docker tag ghcr.io/2witstudios/pagespace-migrate:latest registry.fly.io/pagespace-web:migrate - docker push registry.fly.io/pagespace-web:migrate - - # Migrations exit before reaching "started" state, so flyctl errors with - # "failed to reach desired start state" after 60s. This is expected. - # Use || true so bash -e doesn't exit; we verify success via machine state below. - flyctl machine run registry.fly.io/pagespace-web:migrate \ - --app pagespace-web \ - --region iad \ - --restart no \ - 2>&1 | tee /tmp/machine_output.txt || true - - MACHINE_ID=$(grep -oP 'Machine ID: \K[0-9a-f]+' /tmp/machine_output.txt | head -1) - if [ -z "$MACHINE_ID" ]; then - echo "ERROR: could not parse machine ID from flyctl output" - cat /tmp/machine_output.txt - exit 1 - fi - echo "Migration machine: $MACHINE_ID" + scripts/deploy/run-fly-migration.sh pagespace-web-staging \ + ghcr.io/2witstudios/pagespace-migrate "sha-${GITHUB_SHA::7}" + env: + FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} + FLY_NO_UPDATE_CHECK: "true" - # Fail on any unexpected error (not the expected "desired start state" one) - if grep -q "Error:" /tmp/machine_output.txt && ! grep -q "desired start state" /tmp/machine_output.txt; then - echo "ERROR: unexpected flyctl error:" - grep "Error:" /tmp/machine_output.txt - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force 2>/dev/null || true - exit 1 - fi + - name: Deploy web to staging + run: | + scripts/deploy/deploy-fly-service.sh pagespace-web-staging \ + ghcr.io/2witstudios/pagespace-web "sha-${GITHUB_SHA::7}" 180 + env: + FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - # Poll for terminal state. A machine that exits 0 → "stopped"; non-zero → "failed". - # flyctl machine wait --state stopped hangs the full timeout when migrations fail - # because the machine is already in "failed" state, not "stopped". Poll both. - TIMEOUT=900 - ELAPSED=0 - while [ $ELAPSED -lt $TIMEOUT ]; do - # flyctl machine status has no --json flag (outputs a formatted table regardless). - # Use the Fly.io Machines REST API directly for reliable JSON state polling. - HTTP_CODE=$(curl -s -o /tmp/fly_machine.json -w "%{http_code}" \ - -H "Authorization: Bearer $FLY_API_TOKEN" \ - "https://api.machines.dev/v1/apps/pagespace-web/machines/$MACHINE_ID") - if [ "$HTTP_CODE" = "200" ]; then - STATE=$(jq -r '.state // "unknown"' /tmp/fly_machine.json 2>/dev/null || echo "unknown") - elif [ "$HTTP_CODE" = "404" ]; then - echo "ERROR: migration machine $MACHINE_ID not found (unexpectedly destroyed)" - exit 1 - else - STATE="unknown" - fi - echo " [${ELAPSED}s] state: $STATE" - if [ "$STATE" = "stopped" ]; then - break - elif [ "$STATE" = "failed" ] || [ "$STATE" = "destroyed" ]; then - echo "ERROR: migration machine reached $STATE state (migrations failed)" - flyctl machine logs "$MACHINE_ID" --app pagespace-web 2>/dev/null || true - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force 2>/dev/null || true - exit 1 - fi - sleep 10 - ELAPSED=$((ELAPSED + 10)) - done + smoke-test: + name: Smoke Test Staging + needs: deploy-staging + runs-on: ubuntu-latest - if [ $ELAPSED -ge $TIMEOUT ]; then - echo "ERROR: timed out after ${TIMEOUT}s waiting for migration machine" - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force 2>/dev/null || true - exit 1 - fi + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Poll /api/health until healthy + run: scripts/deploy/smoke-test.sh https://pagespace-web-staging.fly.dev + + deploy-fly: + name: Deploy to Fly.io (Production) + needs: [build-and-push, ci, smoke-test] + runs-on: ubuntu-latest + environment: fly-production + if: github.ref == 'refs/heads/master' + + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Setup flyctl + uses: superfly/flyctl-actions/setup-flyctl@master - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force - echo "Migrations complete" + - name: Log in to Fly registry + run: flyctl auth docker + env: + FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} + + - name: Log in to GHCR + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + # Promotes the EXACT tag that smoke-test just verified against staging — + # never :latest, which could have moved since the build. + - name: Run migrations + run: | + scripts/deploy/run-fly-migration.sh pagespace-web \ + ghcr.io/2witstudios/pagespace-migrate "sha-${GITHUB_SHA::7}" env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} FLY_NO_UPDATE_CHECK: "true" @@ -281,72 +281,9 @@ jobs: exit 1 fi - docker pull ghcr.io/2witstudios/pagespace-migrate:latest - docker tag ghcr.io/2witstudios/pagespace-migrate:latest registry.fly.io/pagespace-web:migrate-admin - docker push registry.fly.io/pagespace-web:migrate-admin - - # Same expected "desired start state" error as the main migration machine - # (one-shot machines exit before reaching "started"); verify via state below. - flyctl machine run registry.fly.io/pagespace-web:migrate-admin \ - --app pagespace-web \ - --region iad \ - --restart no \ - --env ADMIN_DATABASE_URL_MIGRATE="$ADMIN_DATABASE_URL_MIGRATE" \ - bun run db:migrate:admin \ - 2>&1 | tee /tmp/admin_machine_output.txt || true - - MACHINE_ID=$(grep -oP 'Machine ID: \K[0-9a-f]+' /tmp/admin_machine_output.txt | head -1) - if [ -z "$MACHINE_ID" ]; then - echo "ERROR: could not parse machine ID from flyctl output" - cat /tmp/admin_machine_output.txt - exit 1 - fi - echo "Admin migration machine: $MACHINE_ID" - - if grep -q "Error:" /tmp/admin_machine_output.txt && ! grep -q "desired start state" /tmp/admin_machine_output.txt; then - echo "ERROR: unexpected flyctl error:" - grep "Error:" /tmp/admin_machine_output.txt - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force 2>/dev/null || true - exit 1 - fi - - # Poll for terminal state (stopped = success, failed/destroyed = failure) — - # same REST-API polling as the main migration step. - TIMEOUT=900 - ELAPSED=0 - while [ $ELAPSED -lt $TIMEOUT ]; do - HTTP_CODE=$(curl -s -o /tmp/fly_admin_machine.json -w "%{http_code}" \ - -H "Authorization: Bearer $FLY_API_TOKEN" \ - "https://api.machines.dev/v1/apps/pagespace-web/machines/$MACHINE_ID") - if [ "$HTTP_CODE" = "200" ]; then - STATE=$(jq -r '.state // "unknown"' /tmp/fly_admin_machine.json 2>/dev/null || echo "unknown") - elif [ "$HTTP_CODE" = "404" ]; then - echo "ERROR: admin migration machine $MACHINE_ID not found (unexpectedly destroyed)" - exit 1 - else - STATE="unknown" - fi - echo " [${ELAPSED}s] state: $STATE" - if [ "$STATE" = "stopped" ]; then - break - elif [ "$STATE" = "failed" ] || [ "$STATE" = "destroyed" ]; then - echo "ERROR: admin migration machine reached $STATE state (migrations failed)" - flyctl machine logs "$MACHINE_ID" --app pagespace-web 2>/dev/null || true - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force 2>/dev/null || true - exit 1 - fi - sleep 10 - ELAPSED=$((ELAPSED + 10)) - done - - if [ $ELAPSED -ge $TIMEOUT ]; then - echo "ERROR: timed out after ${TIMEOUT}s waiting for admin migration machine" - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force 2>/dev/null || true - exit 1 - fi - - flyctl machine destroy "$MACHINE_ID" --app pagespace-web --force - echo "Admin migrations complete" + scripts/deploy/run-fly-migration.sh pagespace-web \ + ghcr.io/2witstudios/pagespace-migrate "sha-${GITHUB_SHA::7}" \ + -- --env ADMIN_DATABASE_URL_MIGRATE="$ADMIN_DATABASE_URL_MIGRATE" bun run db:migrate:admin env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} FLY_NO_UPDATE_CHECK: "true" @@ -370,51 +307,41 @@ jobs: # Pinned by apps/realtime/src/__tests__/deploy-order.guard.test.ts. - name: Deploy realtime run: | - docker pull ghcr.io/2witstudios/pagespace-realtime:latest - docker tag ghcr.io/2witstudios/pagespace-realtime:latest registry.fly.io/pagespace-realtime:latest - docker push registry.fly.io/pagespace-realtime:latest - flyctl deploy --app pagespace-realtime --image registry.fly.io/pagespace-realtime:latest --wait-timeout 300 + scripts/deploy/deploy-fly-service.sh pagespace-realtime \ + ghcr.io/2witstudios/pagespace-realtime "sha-${GITHUB_SHA::7}" 300 env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - name: Deploy web run: | - docker pull ghcr.io/2witstudios/pagespace-web:latest - docker tag ghcr.io/2witstudios/pagespace-web:latest registry.fly.io/pagespace-web:latest - docker push registry.fly.io/pagespace-web:latest - flyctl deploy --app pagespace-web --image registry.fly.io/pagespace-web:latest --wait-timeout 300 + scripts/deploy/deploy-fly-service.sh pagespace-web \ + ghcr.io/2witstudios/pagespace-web "sha-${GITHUB_SHA::7}" 300 env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - name: Deploy processor + # strategy=immediate (fly.processor.toml): restarts in-place, no two-phase rolling swap. + # flyctl deploy with immediate strategy completes in ~3s (updates config + restart) and + # returns "good state" only after the machine is up. No post-deploy machine wait needed — + # flyctl machine wait against an immediate-strategy machine hits a transient "replacing" + # state and produces false-negative CI failures. run: | - docker pull ghcr.io/2witstudios/pagespace-processor:latest - docker tag ghcr.io/2witstudios/pagespace-processor:latest registry.fly.io/pagespace-processor:latest - docker push registry.fly.io/pagespace-processor:latest - # strategy=immediate (fly.processor.toml): restarts in-place, no two-phase rolling swap. - # flyctl deploy with immediate strategy completes in ~3s (updates config + restart) and - # returns "good state" only after the machine is up. No post-deploy machine wait needed — - # flyctl machine wait against an immediate-strategy machine hits a transient "replacing" - # state and produces false-negative CI failures. - flyctl deploy --app pagespace-processor --image registry.fly.io/pagespace-processor:latest --wait-timeout 120 + scripts/deploy/deploy-fly-service.sh pagespace-processor \ + ghcr.io/2witstudios/pagespace-processor "sha-${GITHUB_SHA::7}" 120 env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - name: Deploy admin run: | - docker pull ghcr.io/2witstudios/pagespace-admin:latest - docker tag ghcr.io/2witstudios/pagespace-admin:latest registry.fly.io/pagespace-admin:latest - docker push registry.fly.io/pagespace-admin:latest - flyctl deploy --app pagespace-admin --image registry.fly.io/pagespace-admin:latest --wait-timeout 300 + scripts/deploy/deploy-fly-service.sh pagespace-admin \ + ghcr.io/2witstudios/pagespace-admin "sha-${GITHUB_SHA::7}" 300 env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - name: Deploy cron run: | - docker pull ghcr.io/2witstudios/pagespace-cron:latest - docker tag ghcr.io/2witstudios/pagespace-cron:latest registry.fly.io/pagespace-cron:latest - docker push registry.fly.io/pagespace-cron:latest - flyctl deploy --app pagespace-cron --image registry.fly.io/pagespace-cron:latest --wait-timeout 300 + scripts/deploy/deploy-fly-service.sh pagespace-cron \ + ghcr.io/2witstudios/pagespace-cron "sha-${GITHUB_SHA::7}" 300 # Cron is a SINGLETON scheduler: exactly one machine must run, or scheduled jobs # (memory/pulse crons) double-fire. Reconcile to one machine, ensure it's started, @@ -438,9 +365,7 @@ jobs: - name: Deploy marketing run: | - docker pull ghcr.io/2witstudios/pagespace-marketing:latest - docker tag ghcr.io/2witstudios/pagespace-marketing:latest registry.fly.io/pagespace-marketing:latest - docker push registry.fly.io/pagespace-marketing:latest - flyctl deploy --app pagespace-marketing --image registry.fly.io/pagespace-marketing:latest --wait-timeout 300 + scripts/deploy/deploy-fly-service.sh pagespace-marketing \ + ghcr.io/2witstudios/pagespace-marketing "sha-${GITHUB_SHA::7}" 300 env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} diff --git a/apps/web/src/app/api/health/__tests__/route.test.ts b/apps/web/src/app/api/health/__tests__/route.test.ts index ca20c0b3ce..2e0561e04f 100644 --- a/apps/web/src/app/api/health/__tests__/route.test.ts +++ b/apps/web/src/app/api/health/__tests__/route.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { GET } from '../route'; +// GET is re-imported fresh per test (see beforeEach) because route.ts tracks +// consecutive DB failures in module-level state — a static import would leak +// that counter across tests. +let GET: typeof import('../route').GET; const mockExecute = vi.hoisted(() => vi.fn()); const mockGetPoolStats = vi.hoisted(() => vi.fn()); const mockGetMonitoringIngestStatus = vi.hoisted(() => vi.fn()); @@ -31,8 +34,10 @@ vi.mock('@/middleware/monitoring', () => ({ })); describe('GET /api/health', () => { - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks(); + vi.resetModules(); + ({ GET } = await import('../route')); mockGetMonitoringIngestStatus.mockReturnValue('active'); mockGetPoolStats.mockReturnValue({ total: 0, idle: 0, waiting: 0 }); }); @@ -112,7 +117,7 @@ describe('GET /api/health', () => { }); describe('database failures', () => { - it('given database connection fails, should return degraded status', async () => { + it('given a single database failure, should report degraded but still return 200 (debounced)', async () => { mockExecute.mockRejectedValue(new Error('Connection refused')); const request = new Request('https://example.com/api/health', { @@ -125,21 +130,45 @@ describe('GET /api/health', () => { expect(response.status).toBe(200); expect(body.status).toBe('degraded'); expect(body.checks.database).toBe('disconnected'); + expect(body.error).toContain('Database'); }); - it('given database timeout, should return degraded with error details', async () => { - mockExecute.mockRejectedValue(new Error('Query timeout')); + it('given two consecutive database failures, should still return 200', async () => { + mockExecute.mockRejectedValue(new Error('Connection refused')); + const request = new Request('https://example.com/api/health', { method: 'GET' }); - const request = new Request('https://example.com/api/health', { - method: 'GET', - }); + await GET(request); + const response = await GET(request); + + expect(response.status).toBe(200); + }); + + it('given three consecutive database failures, should return 503', async () => { + mockExecute.mockRejectedValue(new Error('Connection refused')); + const request = new Request('https://example.com/api/health', { method: 'GET' }); + await GET(request); + await GET(request); const response = await GET(request); const body = await response.json(); - expect(response.status).toBe(200); + expect(response.status).toBe(503); expect(body.status).toBe('degraded'); - expect(body.error).toContain('Database'); + }); + + it('given a recovery after sustained failures, should return to 200', async () => { + mockExecute.mockRejectedValue(new Error('Connection refused')); + const request = new Request('https://example.com/api/health', { method: 'GET' }); + await GET(request); + await GET(request); + await GET(request); + + mockExecute.mockResolvedValue([{ '1': 1 }]); + const response = await GET(request); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(body.status).toBe('healthy'); }); }); diff --git a/apps/web/src/app/api/health/route.ts b/apps/web/src/app/api/health/route.ts index 89579fc805..1984174925 100644 --- a/apps/web/src/app/api/health/route.ts +++ b/apps/web/src/app/api/health/route.ts @@ -26,11 +26,20 @@ interface HealthResponse { error?: string; } +// Fly's health check has no failure-count threshold of its own — a single failed +// probe right after the grace period elapses is enough to fail a rolling deploy. +// Require this many CONSECUTIVE failures before reporting unhealthy over HTTP, so +// one transient blip (e.g. a slow DB reconnect) doesn't flip the check. +const CONSECUTIVE_FAILURE_THRESHOLD = 3; +let consecutiveDbFailures = 0; + const checkDatabase = async (): Promise => { try { await db.execute(sql`SELECT 1`); + consecutiveDbFailures = 0; return true; } catch { + consecutiveDbFailures += 1; return false; } }; @@ -87,14 +96,20 @@ export async function GET(_request: Request): Promise { }); } + // Only fail the HTTP status on a sustained DB outage, not monitoring + // misconfiguration — the latter is a config warning, not a traffic-serving + // failure, and shouldn't cause Fly to cycle machines over it. + const dbSustainedFailure = consecutiveDbFailures >= CONSECUTIVE_FAILURE_THRESHOLD; + return Response.json(response, { - status: 200, + status: dbSustainedFailure ? 503 : 200, headers: { 'Cache-Control': 'no-store, no-cache, must-revalidate', }, }); } catch (error) { loggers.api.error('Health check failed', error as Error); + consecutiveDbFailures += 1; const response: HealthResponse = { status: 'degraded', @@ -115,7 +130,7 @@ export async function GET(_request: Request): Promise { }; return Response.json(response, { - status: 200, + status: consecutiveDbFailures >= CONSECUTIVE_FAILURE_THRESHOLD ? 503 : 200, headers: { 'Cache-Control': 'no-store, no-cache, must-revalidate', }, diff --git a/infrastructure/__tests__/ci-admin-migrate.test.ts b/infrastructure/__tests__/ci-admin-migrate.test.ts index d9ece53694..6f371c3033 100644 --- a/infrastructure/__tests__/ci-admin-migrate.test.ts +++ b/infrastructure/__tests__/ci-admin-migrate.test.ts @@ -42,18 +42,44 @@ describe('docker-images.yml admin-DB migrate step', () => { expect(adminStep.if).toContain('ADMIN_DB_MIGRATIONS_ENABLED'); }); - it('given the admin migrations step, should run db:migrate:admin on a one-shot machine', () => { - expect(adminStep.run).toContain('flyctl machine run'); + // The one-shot-machine mechanics (flyctl machine run, polling for + // failed/destroyed states) live in scripts/deploy/run-fly-migration.sh — + // shared by both the main and admin migration steps, so it's tested once + // there (scripts/deploy is not a vitest project; verified by reading the + // script directly below) instead of duplicated inline in each step. + const RUN_FLY_MIGRATION_SCRIPT = readFileSync( + resolve(__dirname, '../../scripts/deploy/run-fly-migration.sh'), + 'utf-8' + ); + + it('given the admin migrations step, should run db:migrate:admin via the shared one-shot-machine script', () => { + expect(adminStep.run).toContain('scripts/deploy/run-fly-migration.sh'); expect(adminStep.run).toContain('db:migrate:admin'); }); + it('given the shared one-shot-machine script, should run flyctl machine run', () => { + expect(RUN_FLY_MIGRATION_SCRIPT).toContain('flyctl machine run'); + }); + it('given the admin migrations step, should use the same migrate image as the main step', () => { - expect(adminStep.run).toContain('pagespace-migrate:latest'); + const imageOf = (run?: string) => run?.match(/ghcr\.io\/2witstudios\/pagespace-migrate\b/)?.[0]; + expect(imageOf(adminStep.run)).toBeDefined(); + expect(imageOf(adminStep.run)).toBe(imageOf(steps[mainIdx].run)); + }); + + it('given the admin and main migrations steps, should never pin the mutable :latest tag', () => { + expect(adminStep.run).not.toContain('pagespace-migrate:latest'); + expect(steps[mainIdx].run).not.toContain('pagespace-migrate:latest'); + }); + + it('given the admin migrations step, should delegate to the same script as the main step (so it fails on failed/destroyed machine states identically)', () => { + expect(adminStep.run).toContain('scripts/deploy/run-fly-migration.sh'); + expect(steps[mainIdx].run).toContain('scripts/deploy/run-fly-migration.sh'); }); - it('given the admin migrations step, should fail on failed/destroyed machine states like the main step', () => { - expect(adminStep.run).toContain('failed'); - expect(adminStep.run).toContain('destroyed'); + it('given the shared one-shot-machine script, should fail on failed/destroyed machine states', () => { + expect(RUN_FLY_MIGRATION_SCRIPT).toContain('failed'); + expect(RUN_FLY_MIGRATION_SCRIPT).toContain('destroyed'); }); it('given the main migrations step, should NOT be conditional (main DB always migrates)', () => { diff --git a/scripts/__tests__/check-destructive-migrations.test.ts b/scripts/__tests__/check-destructive-migrations.test.ts new file mode 100644 index 0000000000..5c02f5f617 --- /dev/null +++ b/scripts/__tests__/check-destructive-migrations.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect } from 'vitest'; +import { ACK_PATTERN, findDestructiveReasons, statementsOf, stripSqlComments } from '../check-destructive-migrations'; + +describe('statementsOf', () => { + it('given a Drizzle-style file, should split on statement-breakpoint markers', () => { + const sql = 'DROP TABLE "a";--> statement-breakpoint\nDROP TABLE "b";'; + expect(statementsOf(sql)).toEqual(['DROP TABLE "a";', 'DROP TABLE "b";']); + }); + + it('given a single-statement file, should return one statement', () => { + expect(statementsOf('TRUNCATE "x";')).toEqual(['TRUNCATE "x";']); + }); +}); + +describe('findDestructiveReasons', () => { + it('given DROP TABLE, should flag it', () => { + expect(findDestructiveReasons('DROP TABLE "alert_history";')).toEqual(['DROP TABLE']); + }); + + it('given DROP COLUMN, should flag it', () => { + expect(findDestructiveReasons('ALTER TABLE "users" DROP COLUMN IF EXISTS "password";')).toEqual([ + 'DROP COLUMN', + ]); + }); + + it('given TRUNCATE, should flag it', () => { + expect(findDestructiveReasons('TRUNCATE security_audit_log;')).toEqual(['TRUNCATE']); + }); + + it('given DROP TYPE, should flag it', () => { + expect(findDestructiveReasons('DROP TYPE "WorkflowRunStatus";')).toEqual(['DROP TYPE']); + }); + + it('given an enum-swap rename (RENAME TO ..._old), should flag it', () => { + expect( + findDestructiveReasons('ALTER TYPE "WorkflowRunStatus" RENAME TO "WorkflowRunStatus_old";') + ).toEqual(['enum-swap rename (RENAME TO ..._old)']); + }); + + it('given ALTER COLUMN ... TYPE, should flag it', () => { + expect( + findDestructiveReasons( + 'ALTER TABLE "activity_logs" ALTER COLUMN "contentFormat" SET DATA TYPE content_format USING "contentFormat"::content_format;' + ) + ).toEqual(['ALTER COLUMN ... TYPE (data type change)']); + }); + + it('given ADD COLUMN ... NOT NULL with no DEFAULT, should flag it', () => { + expect( + findDestructiveReasons('ALTER TABLE "calendar_triggers" ADD COLUMN "workflowId" text NOT NULL;') + ).toEqual(['ADD COLUMN ... NOT NULL without a DEFAULT']); + }); + + it('given ADD COLUMN ... NOT NULL with a DEFAULT, should not flag it', () => { + expect( + findDestructiveReasons('ALTER TABLE "t" ADD COLUMN "x" boolean DEFAULT false NOT NULL;') + ).toEqual([]); + }); + + it('given ADD COLUMN ... bigserial NOT NULL, should not flag it (self-populating)', () => { + expect( + findDestructiveReasons('ALTER TABLE "activity_logs" ADD COLUMN "chainSeq" bigserial NOT NULL;') + ).toEqual([]); + }); + + it('given a purely additive statement, should not flag anything', () => { + expect(findDestructiveReasons('ALTER TABLE "t" ADD COLUMN "x" text;')).toEqual([]); + }); + + it('given multiple destructive statements in one file, should flag each distinct reason once', () => { + const sql = [ + 'TRUNCATE TABLE "calendar_triggers";--> statement-breakpoint', + 'ALTER TABLE "calendar_triggers" DROP COLUMN IF EXISTS "status";--> statement-breakpoint', + 'DROP TYPE "CalendarTriggerStatus";', + ].join('\n'); + const reasons = findDestructiveReasons(sql); + expect(reasons).toContain('TRUNCATE'); + expect(reasons).toContain('DROP COLUMN'); + expect(reasons).toContain('DROP TYPE'); + expect(reasons).toHaveLength(3); + }); + + it('given ADD COLUMN NOT NULL with a comment mentioning DEFAULT (not a real one), should still flag it', () => { + expect( + findDestructiveReasons( + '-- table is empty, so no DEFAULT is needed\nALTER TABLE "t" ADD COLUMN "x" text NOT NULL;' + ) + ).toEqual(['ADD COLUMN ... NOT NULL without a DEFAULT']); + }); + + it('given ADD COLUMN NOT NULL with a comment mentioning SERIAL (not a real one), should still flag it', () => { + expect( + findDestructiveReasons( + '-- not a serial column, just named that way\nALTER TABLE "t" ADD COLUMN "x" text NOT NULL;' + ) + ).toEqual(['ADD COLUMN ... NOT NULL without a DEFAULT']); + }); + + it('given a real DROP TABLE only inside a comment, should not flag it', () => { + expect(findDestructiveReasons('-- old code used to DROP TABLE "x" here, no longer true\nSELECT 1;')).toEqual( + [] + ); + }); +}); + +describe('stripSqlComments', () => { + it('given a line comment, should remove it', () => { + expect(stripSqlComments('-- a comment\nDROP TABLE "x";')).toBe('\nDROP TABLE "x";'); + }); + + it('given a block comment, should remove it', () => { + expect(stripSqlComments('/* a block comment */ DROP TABLE "x";')).toBe(' DROP TABLE "x";'); + }); + + it('given a multi-line block comment, should remove it', () => { + expect(stripSqlComments('/* line one\nline two */\nDROP TABLE "x";')).toBe('\nDROP TABLE "x";'); + }); + + it('given no comments, should return the input unchanged', () => { + expect(stripSqlComments('DROP TABLE "x";')).toBe('DROP TABLE "x";'); + }); +}); + +describe('ACK_PATTERN', () => { + it('given a destructive-migration-ack comment, should match', () => { + expect(ACK_PATTERN.test('-- destructive-migration-ack: no old code reads this table\nDROP TABLE "x";')).toBe( + true + ); + }); + + it('given an ack comment with no reason text, should not match', () => { + expect(ACK_PATTERN.test('-- destructive-migration-ack:\nDROP TABLE "x";')).toBe(false); + }); + + it('given no ack comment, should not match', () => { + expect(ACK_PATTERN.test('DROP TABLE "x";')).toBe(false); + }); + + it('given an unrelated comment, should not match', () => { + expect(ACK_PATTERN.test('-- this table is old\nDROP TABLE "x";')).toBe(false); + }); +}); diff --git a/scripts/__tests__/run-fly-migration.test.ts b/scripts/__tests__/run-fly-migration.test.ts new file mode 100644 index 0000000000..37d339dab5 --- /dev/null +++ b/scripts/__tests__/run-fly-migration.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, chmodSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; + +// Exercises the REAL script (not a reimplementation) with fake flyctl/docker/curl +// executables shadowing PATH, so the test proves actual exit-code behavior rather +// than source-text presence (the assertion style CodeRabbit flagged in the +// docker-images.yml guard test). + +const SCRIPT = resolve(__dirname, '../deploy/run-fly-migration.sh'); + +let binDir: string; +let stateResponsePath: string; + +function writeFakeExecutable(name: string, body: string) { + const path = join(binDir, name); + writeFileSync(path, `#!/bin/sh\n${body}\n`); + chmodSync(path, 0o755); +} + +beforeEach(() => { + binDir = mkdtempSync(join(tmpdir(), 'run-fly-migration-fakebin-')); + stateResponsePath = join(binDir, 'state-response.json'); + + writeFakeExecutable('docker', 'exit 0'); + + writeFakeExecutable( + 'flyctl', + ` +case "$*" in + "machine run"*) echo "Machine ID: fake0001234"; exit 0 ;; + "machine destroy"*) exit 0 ;; + "machine logs"*) exit 0 ;; + *) exit 0 ;; +esac +` + ); + + // Mimics `curl -s -o -w "%{http_code}" `: writes the canned Machines + // API response to the -o file and prints "200" to stdout (the polled HTTP code). + writeFakeExecutable( + 'curl', + ` +outfile="" +prev="" +for arg in "$@"; do + if [ "$prev" = "-o" ]; then outfile="$arg"; fi + prev="$arg" +done +cp "${stateResponsePath}" "$outfile" +printf '200' +` + ); +}); + +afterEach(() => { + rmSync(binDir, { recursive: true, force: true }); +}); + +function runScript() { + return spawnSync(SCRIPT, ['pagespace-web', 'ghcr.io/2witstudios/pagespace-migrate', 'sha-abc1234'], { + env: { + ...process.env, + PATH: `${binDir}:${process.env.PATH}`, + FLY_API_TOKEN: 'fake-token', + }, + encoding: 'utf-8', + }); +} + +describe('run-fly-migration.sh — machine exit-code handling', () => { + it('given the machine reaches stopped with exit code 0, should report success', () => { + writeFileSync( + stateResponsePath, + JSON.stringify({ + id: 'fake0001234', + state: 'stopped', + events: [{ type: 'exit', request: { exit_event: { exit_code: 0, exit_signal: 0 } } }], + }) + ); + + const result = runScript(); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('Migrations complete'); + }); + + it('given the machine reaches stopped with a non-zero exit code, should treat it as a failure', () => { + writeFileSync( + stateResponsePath, + JSON.stringify({ + id: 'fake0001234', + state: 'stopped', + events: [{ type: 'exit', request: { exit_event: { exit_code: 1, exit_signal: 0 } } }], + }) + ); + + const result = runScript(); + + expect(result.status).not.toBe(0); + expect(result.stdout).not.toContain('Migrations complete'); + }); + + it('given the machine reaches stopped with a missing exit code, should treat it as a failure (fail closed)', () => { + writeFileSync( + stateResponsePath, + JSON.stringify({ + id: 'fake0001234', + state: 'stopped', + events: [], + }) + ); + + const result = runScript(); + + expect(result.status).not.toBe(0); + expect(result.stdout).not.toContain('Migrations complete'); + }); +}); diff --git a/scripts/check-destructive-migrations.ts b/scripts/check-destructive-migrations.ts new file mode 100644 index 0000000000..b756c06b2b --- /dev/null +++ b/scripts/check-destructive-migrations.ts @@ -0,0 +1,138 @@ +#!/usr/bin/env bun +/** + * Destructive-migration CI gate. + * + * Fails if a migration file ADDED in this diff (compared to a base ref, across both + * packages/db/drizzle and the drizzle-admin trust-plane migrations) contains a + * destructive SQL statement — DROP TABLE/COLUMN/TYPE, TRUNCATE, a column type + * change, an enum-swap rename, or a NOT NULL column added without a DEFAULT — + * without a `-- destructive-migration-ack: ` comment anywhere in the file. + * + * Scope: per-file only, and only for newly added files — existing migrations are + * never retroactively flagged. A destructive operation split across two separate + * migration files (e.g. a TRUNCATE in migration N enabling a NOT-NULL-without- + * default in migration N+1) is out of scope for this check. + * + * Usage: bun scripts/check-destructive-migrations.ts + */ + +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; + +// drizzle-admin holds the trust-plane admin DB migrations, run via `db:migrate:admin` +// when ADMIN_DB_MIGRATIONS_ENABLED=true (see docker-images.yml) — a destructive +// migration there is just as dangerous as one in the main drizzle dir. +const MIGRATIONS_DIRS = ['packages/db/drizzle', 'packages/db/drizzle-admin']; +// The reason must be on the same line as the marker — `\s` would also match a +// newline, letting an empty `ack:` line "borrow" non-whitespace from whatever +// text happens to follow it (e.g. the next line's SQL). +export const ACK_PATTERN = /--\s*destructive-migration-ack:[ \t]*\S/i; + +export const DESTRUCTIVE_CHECKS: { name: string; test: (stmt: string) => boolean }[] = [ + { name: 'DROP TABLE', test: (s) => /\bDROP\s+TABLE\b/i.test(s) }, + { name: 'DROP COLUMN', test: (s) => /\bDROP\s+COLUMN\b/i.test(s) }, + { name: 'TRUNCATE', test: (s) => /\bTRUNCATE\b/i.test(s) }, + { name: 'DROP TYPE', test: (s) => /\bDROP\s+TYPE\b/i.test(s) }, + { name: 'enum-swap rename (RENAME TO ..._old)', test: (s) => /RENAME\s+TO\s+"?\w+_old"?/i.test(s) }, + { name: 'ALTER COLUMN ... TYPE (data type change)', test: (s) => /\bALTER\s+COLUMN\b.*\bTYPE\b/i.test(s) }, + { + name: 'ADD COLUMN ... NOT NULL without a DEFAULT', + test: (s) => + /\bADD\s+COLUMN\b/i.test(s) && + /\bNOT\s+NULL\b/i.test(s) && + !/\bDEFAULT\b/i.test(s) && + !/\b(BIG)?SERIAL\b/i.test(s), // serial/bigserial self-populate via an implicit sequence default + }, +]; + +function addedMigrationFiles(baseRef: string): string[] { + const output = execFileSync( + 'git', + [ + 'diff', + '--name-only', + '--diff-filter=A', + `${baseRef}...HEAD`, + '--', + ...MIGRATIONS_DIRS.map((dir) => `${dir}/*.sql`), + ], + { encoding: 'utf-8' } + ); + return output.split('\n').map((line) => line.trim()).filter(Boolean); +} + +export function statementsOf(sql: string): string[] { + return sql + .split(/-->\s*statement-breakpoint/i) + .map((s) => s.trim()) + .filter(Boolean); +} + +// Strip `--` line comments and `/* */` block comments before testing a statement +// against DESTRUCTIVE_CHECKS. Without this, an explanatory comment that happens to +// contain a keyword — e.g. `-- table is empty, no DEFAULT needed` above an +// `ADD COLUMN ... NOT NULL` — satisfies the DEFAULT/SERIAL regex and lets a genuinely +// destructive statement through unacknowledged. +export function stripSqlComments(sql: string): string { + return sql.replace(/\/\*[\s\S]*?\*\//g, '').replace(/--[^\n]*/g, ''); +} + +export function findDestructiveReasons(sql: string): string[] { + const reasons = new Set(); + for (const stmt of statementsOf(sql)) { + const stripped = stripSqlComments(stmt); + for (const check of DESTRUCTIVE_CHECKS) { + if (check.test(stripped)) reasons.add(check.name); + } + } + return [...reasons]; +} + +export function main() { + const baseRef = process.argv[2]; + if (!baseRef) { + console.error('Usage: bun scripts/check-destructive-migrations.ts '); + process.exit(2); + } + + const files = addedMigrationFiles(baseRef); + if (files.length === 0) { + console.log('No new migration files in this diff.'); + return; + } + + let failed = false; + for (const file of files) { + const sql = readFileSync(file, 'utf-8'); + const reasons = findDestructiveReasons(sql); + if (reasons.length === 0) { + console.log(`OK ${file} (no destructive patterns)`); + continue; + } + if (ACK_PATTERN.test(sql)) { + console.log(`OK ${file} (destructive: ${reasons.join(', ')} — acknowledged)`); + continue; + } + failed = true; + console.error(`FAIL ${file}`); + console.error(` Destructive pattern(s) found: ${reasons.join(', ')}`); + console.error( + ' Add a leading comment acknowledging expand/contract review, e.g.:' + ); + console.error( + ' -- destructive-migration-ack: ' + ); + } + + if (failed) { + console.error( + '\nDestructive migration(s) added without an ack comment. See errors above.' + ); + process.exit(1); + } +} + +// Only run if executed directly (not imported by tests) +if (typeof process !== 'undefined' && process.argv[1]?.endsWith('check-destructive-migrations.ts')) { + main(); +} diff --git a/scripts/deploy/deploy-fly-service.sh b/scripts/deploy/deploy-fly-service.sh new file mode 100755 index 0000000000..1ef5c05606 --- /dev/null +++ b/scripts/deploy/deploy-fly-service.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# Pull a GHCR image, retag it into the Fly registry, and deploy it to a Fly app. +# +# Usage: +# deploy-fly-service.sh [wait-timeout-seconds] +# +# Example: +# deploy-fly-service.sh pagespace-web-staging \ +# ghcr.io/2witstudios/pagespace-web sha-abc1234 180 +set -euo pipefail + +if [ "$#" -lt 3 ]; then + echo "Usage: deploy-fly-service.sh [wait-timeout-seconds]" >&2 + exit 2 +fi + +FLY_APP="$1" +GHCR_IMAGE="$2" +TAG="$3" +WAIT_TIMEOUT="${4:-300}" + +FULL_GHCR_IMAGE="${GHCR_IMAGE}:${TAG}" +FLY_IMAGE="registry.fly.io/${FLY_APP}:${TAG}" + +echo "--- Deploying $FLY_APP from $FULL_GHCR_IMAGE ---" +docker pull "$FULL_GHCR_IMAGE" +docker tag "$FULL_GHCR_IMAGE" "$FLY_IMAGE" +docker push "$FLY_IMAGE" +flyctl deploy --app "$FLY_APP" --image "$FLY_IMAGE" --wait-timeout "$WAIT_TIMEOUT" diff --git a/scripts/deploy/run-fly-migration.sh b/scripts/deploy/run-fly-migration.sh new file mode 100755 index 0000000000..32f15bbdfe --- /dev/null +++ b/scripts/deploy/run-fly-migration.sh @@ -0,0 +1,119 @@ +#!/usr/bin/env bash +# Push the migrate image into a Fly app's registry namespace, run it as a one-off +# machine, and wait for it to finish. +# +# The migration image exits before flyctl considers the machine "started", so +# `flyctl machine run` reports "failed to reach desired start state" even on a +# successful migration. That's expected — this script verifies success by +# polling the Machines REST API for the machine's terminal state instead of +# trusting flyctl's own exit code. +# +# Usage: +# run-fly-migration.sh [-- extra flyctl machine run args...] +# +# Example: +# run-fly-migration.sh pagespace-web-staging \ +# ghcr.io/2witstudios/pagespace-migrate sha-abc1234 +# +# Required env: FLY_API_TOKEN +set -euo pipefail + +if [ "$#" -lt 3 ]; then + echo "Usage: run-fly-migration.sh [-- extra flyctl machine run args...]" >&2 + exit 2 +fi + +FLY_APP="$1" +GHCR_IMAGE="$2" +TAG="$3" +shift 3 +if [ "${1:-}" = "--" ]; then + shift +fi +EXTRA_ARGS=("$@") + +: "${FLY_API_TOKEN:?FLY_API_TOKEN must be set}" + +FULL_GHCR_IMAGE="${GHCR_IMAGE}:${TAG}" +FLY_IMAGE="registry.fly.io/${FLY_APP}:migrate-${TAG}" +MACHINE_NAME="migrations-$(date +%s)" +OUTPUT_FILE="$(mktemp)" +STATE_FILE="$(mktemp)" +trap 'rm -f "$OUTPUT_FILE" "$STATE_FILE"' EXIT + +echo "--- Running migrations on $FLY_APP ($FULL_GHCR_IMAGE) ---" +docker pull "$FULL_GHCR_IMAGE" +docker tag "$FULL_GHCR_IMAGE" "$FLY_IMAGE" +docker push "$FLY_IMAGE" + +flyctl machine run "$FLY_IMAGE" \ + --app "$FLY_APP" \ + --name "$MACHINE_NAME" \ + --region iad \ + --restart no \ + "${EXTRA_ARGS[@]+"${EXTRA_ARGS[@]}"}" \ + 2>&1 | tee "$OUTPUT_FILE" || true + +MACHINE_ID=$(grep -o 'Machine ID: [0-9a-f]*' "$OUTPUT_FILE" | head -1 | sed 's/Machine ID: //') +if [ -z "$MACHINE_ID" ]; then + echo "ERROR: could not parse machine ID from flyctl output" >&2 + cat "$OUTPUT_FILE" >&2 + exit 1 +fi +echo "Migration machine: $MACHINE_ID" + +if grep -q "Error:" "$OUTPUT_FILE" && ! grep -q "desired start state" "$OUTPUT_FILE"; then + echo "ERROR: unexpected flyctl error:" >&2 + grep "Error:" "$OUTPUT_FILE" >&2 + flyctl machine destroy "$MACHINE_ID" --app "$FLY_APP" --force 2>/dev/null || true + exit 1 +fi + +TIMEOUT=900 +ELAPSED=0 +while [ "$ELAPSED" -lt "$TIMEOUT" ]; do + HTTP_CODE=$(curl -s -o "$STATE_FILE" -w "%{http_code}" \ + -H "Authorization: Bearer $FLY_API_TOKEN" \ + "https://api.machines.dev/v1/apps/$FLY_APP/machines/$MACHINE_ID") || HTTP_CODE="000" + if [ "$HTTP_CODE" = "200" ]; then + STATE=$(jq -r '.state // "unknown"' "$STATE_FILE" 2>/dev/null || echo "unknown") + elif [ "$HTTP_CODE" = "404" ]; then + echo "ERROR: migration machine $MACHINE_ID not found (unexpectedly destroyed)" >&2 + exit 1 + else + STATE="unknown" + fi + echo " [${ELAPSED}s] state: $STATE" + if [ "$STATE" = "stopped" ]; then + break + elif [ "$STATE" = "failed" ] || [ "$STATE" = "destroyed" ]; then + echo "ERROR: migration machine reached $STATE state (migrations failed)" >&2 + flyctl machine logs "$MACHINE_ID" --app "$FLY_APP" 2>/dev/null || true + flyctl machine destroy "$MACHINE_ID" --app "$FLY_APP" --force 2>/dev/null || true + exit 1 + fi + sleep 10 + ELAPSED=$((ELAPSED + 10)) +done + +if [ "$ELAPSED" -ge "$TIMEOUT" ]; then + echo "ERROR: timed out after ${TIMEOUT}s waiting for migration machine" >&2 + flyctl machine destroy "$MACHINE_ID" --app "$FLY_APP" --force 2>/dev/null || true + exit 1 +fi + +# With --restart no, the machine reaches "stopped" on ANY process exit — including a +# non-zero one. "stopped" alone is not success; the migration command itself may have +# errored. $STATE_FILE already holds the last-polled response body (the one that broke +# the loop above), so read its exit code from there instead of polling again. Missing +# exit_code (e.g. no exit event recorded) fails closed rather than assuming success. +EXIT_CODE=$(jq -r '[.events[]? | select(.type == "exit")] | last | .request.exit_event.exit_code // empty' "$STATE_FILE" 2>/dev/null) +if [ -z "$EXIT_CODE" ] || [ "$EXIT_CODE" != "0" ]; then + echo "ERROR: migration machine stopped with exit code '${EXIT_CODE:-unknown}' (migrations failed)" >&2 + flyctl machine logs "$MACHINE_ID" --app "$FLY_APP" 2>/dev/null || true + flyctl machine destroy "$MACHINE_ID" --app "$FLY_APP" --force 2>/dev/null || true + exit 1 +fi + +flyctl machine destroy "$MACHINE_ID" --app "$FLY_APP" --force +echo "Migrations complete on $FLY_APP" diff --git a/scripts/deploy/smoke-test.sh b/scripts/deploy/smoke-test.sh new file mode 100755 index 0000000000..edd2ecc236 --- /dev/null +++ b/scripts/deploy/smoke-test.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# Post-deploy smoke test: poll /api/health until it reports a genuinely healthy, +# DB-connected instance (not just "the process answers HTTP"). +# +# Usage: +# smoke-test.sh [retries] [delay-seconds] +set -euo pipefail + +if [ "$#" -lt 1 ]; then + echo "Usage: smoke-test.sh [retries] [delay-seconds]" >&2 + exit 2 +fi + +BASE_URL="${1%/}" +RETRIES="${2:-6}" +DELAY="${3:-10}" +HEALTH_URL="$BASE_URL/api/health" +RESPONSE_FILE="$(mktemp)" +trap 'rm -f "$RESPONSE_FILE"' EXIT + +echo "--- Smoke testing $HEALTH_URL ---" + +for i in $(seq 1 "$RETRIES"); do + # curl already prints "000" via -w on total connection failure; falling back to a + # literal "000" (not appending one) avoids HTTP_CODE becoming "000000" when both fire. + HTTP_CODE=$(curl -s -o "$RESPONSE_FILE" -w "%{http_code}" --max-time 10 "$HEALTH_URL") || HTTP_CODE="000" + + if [ "$HTTP_CODE" = "200" ]; then + STATUS=$(jq -r '.status // ""' "$RESPONSE_FILE" 2>/dev/null || echo "") + DATABASE=$(jq -r '.checks.database // ""' "$RESPONSE_FILE" 2>/dev/null || echo "") + + if [ "$STATUS" = "healthy" ] && [ "$DATABASE" = "connected" ]; then + echo "Smoke test PASSED: $HEALTH_URL -> 200, status=healthy, database=connected" + exit 0 + fi + + echo " [$i/$RETRIES] HTTP 200 but status=$STATUS database=$DATABASE — retrying in ${DELAY}s" + else + echo " [$i/$RETRIES] HTTP $HTTP_CODE — retrying in ${DELAY}s" + fi + + sleep "$DELAY" +done + +echo "Smoke test FAILED after $RETRIES attempts against $HEALTH_URL" >&2 +echo "Last response:" >&2 +cat "$RESPONSE_FILE" >&2 2>/dev/null || true +exit 1 diff --git a/tasks/deploy-safety-review-fixes-epic.md b/tasks/deploy-safety-review-fixes-epic.md new file mode 100644 index 0000000000..58cf32dc27 --- /dev/null +++ b/tasks/deploy-safety-review-fixes-epic.md @@ -0,0 +1,12 @@ +# Deploy Safety — PR #2125 Review-Fix Follow-ups + +Companion to the deploy-safety audit (`tasks/solo-tells-audit-2026-07-17.md` section 6) and +PR #2125 (`pu/staging-deploy`). Tracks fixes for CodeRabbit/Codex review findings that +survived the initial merge, triaged via `/aidd-pr`. + +## Requirements + +- Given a migration one-shot machine reaches Fly's `stopped` state with a non-zero + `exit_event.exit_code` (e.g. the migration script itself errored), `run-fly-migration.sh` + should treat it as a failure — destroy the machine, print the failure, and exit 1 — not + print "Migrations complete" and exit 0.