# Ship Safety

> Check whether a diff is safe to deploy — schema changes that break the currently running code, blocking DDL, renaming or moving background task actors while jobs are in flight, queue priority, cron catch-up, locking, batch size, and whether the change should be split into more than one PR. Use before merging a PR that touches migrations, tasks.py, models, or removes an endpoint.

- Skill: `polarsource/ship-safety` (Agent Skill)
- Install (CLI): `npx skillmds@latest add polarsource/ship-safety`
- Raw SKILL.md: https://api.skillmd.com/api/skills/polarsource/ship-safety/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: DevOps & Infra
- License: MIT
- Author: polarsource (https://skillmd.com/u/polarsource)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/polarsource/ship-safety

---


# Ship Safety (Polar)

The handle is the **diff**. The evidence is **what breaks between the moment this merges and
the moment it is fully deployed**.

Every check here is about that gap. Polar does not deploy atomically: migrations run first,
the API deploys before the workers, the old frontend can be live against the new backend, and
the queues already hold jobs enqueued under the old names. Code that is correct in its final
state can still take production down on the way there.

## Scope

Diffs touching `server/migrations/versions/`, `**/tasks.py`, `polar/models/`,
`server/scripts/`; or that remove or rename an endpoint; or that span `server/` and `clients/`
with a dependency between them.

**Owned elsewhere.** `ADR-0006` covers the migration rules — lock timeout, nullable → batched
`run_batched_update` script → NOT NULL across separate PRs, the unconditional `UPDATE` in the
enforce migration, and keeping migration PRs isolated from code. CI enforces it with the
Migration Isolation Check. `adr-check` reports violations; do not restate ADR-0006 here.
Reinvented helpers → `reuse-check`. Billing-specific lock and cycle rules → `billing-review`.

What is left for you is everything ADR-0006 does not say.

## Checks

### 1. Schema ahead of code

ADR-0006 stops code shipping ahead of its schema. The reverse still bites: between the
migration and the new code, the **old code runs against the new schema**.

- **Dropping a column or table** breaks the running app immediately — SQLAlchemy maps every
  model to a table at import, so a dropped table can fail before any query runs. Stop using
  it, deploy, drop in a later PR.
- **Renaming** is always two steps.

Ask of every schema change: *is the currently-deployed code still correct against this?*

### 2. Blocking DDL

- `postgresql_concurrently=True` for an index on a large table, with the migration outside a
  transaction.
- For NOT NULL on a large table, prefer `CHECK ... NOT VALID` then `VALIDATE CONSTRAINT`, so
  Postgres skips the full-table lock. On a small table this is ceremony — say which you think
  applies.
- New foreign keys on money tables get `ondelete="restrict"`.

### 3. Tasks already in flight

When the PR merges, the queues hold jobs enqueued by the old code.

- **Renaming an actor strands every queued job** — the worker looks for the old `actor_name`
  and finds nothing. Sequence: add `order.invoice.v2`, start enqueueing it, deploy, wait for
  the old queue to drain, then remove the old actor and swap the name back. Moving an actor to
  a different queue is the same problem.
- **Changing a signature** breaks jobs enqueued with the old arguments. New parameters need
  defaults.
- **Queue priority.** `TaskPriority.HIGH` is checkout-path only. Analytics, exports and
  backfills go `LOW`. Slow work on `HIGH` starves checkout.
- **Cron actors.** A new `cron_trigger` needs an answer for a missed run. A catch-up loop that
  walks forward through skipped periods usually computes state wrong; prefer an invariant
  alert that the run did not happen.
- **Retries.** Polar relies on automatic retries. A new failure path must raise, not swallow,
  and `max_retries=0` must be deliberate.

### 4. Locks and volume

- A new `with_for_update` belongs in the task or service that owns the unit of work, not
  inside a processor-specific helper. Every lock needs an answer to *what releases this if the
  process dies?*
- Release on one path only. A lock released on two paths is a bug waiting to happen.
- Loading every matching row into memory does not survive production volume.
- A scheduled sweep that scans an entire busy table needs an index or a bounded window.

### 5. Split this PR

Flag for splitting when the diff:

- removes a backend endpoint **and** updates its frontend caller — the old frontend can be
  live against the new backend;
- renames an actor and removes the old one together;
- changes native mobile code alongside TypeScript — native blocks an over-the-air release, so
  ship the TS-only change first;
- was already flagged in review as "should move to another module". A follow-up PR is a fine
  answer, but say so rather than silently deferring.

## Output

```
## Ship Safety

### 🔴 Blocking
- `file:line` — <what breaks, in which window>. Fix: <fix>

### 🟠 Should fix
- `file:line` — <what happens under load>. Fix: <fix>

### 🟡 Question
- `file:line` — <question, including "split this PR?">

### Notes
- run before merge: <script path, or none>
- manual step after deploy: <e.g. "remove order.invoice v1 after 4h", or none>

### Verdict
✅ Safe to ship  |  ❌ n blocking, n should-fix
```

Fill in Notes even when the verdict is green. A required script run that is not written down
is a required script run that does not happen.