polarsource

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.

polarsource 10,240 783 Updated 2w ago
GitHub

Install

npx skillscat add polarsource/polar/ship-safety

Install via the SkillsCat registry.

SKILL.md

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.