> ## Documentation Index
> Fetch the complete documentation index at: https://docs.encoreos.io/llms.txt
> Use this file to discover all available pages before exploring further.

# ADR-021: One-Time Exception to "Never Edit Existing Migrations"

> Documents a one-time exception to the never-edit-migrations rule for the 20260513225500 timestamp collision, and codifies criteria future exceptions must meet.

**Status:** Accepted
**Date:** 2026-05-23
**Participants:** Jeremy Bloom, Platform Architecture (via superpowers:brainstorming)

***

## Context

Project rule `.claude/rules/database.md` states:

> Never edit existing migrations — create new ones

This ADR documents a one-time exception to that rule for a specific migration-collision bug, and codifies the criteria future exceptions must meet.

### The collision

Two migrations share timestamp prefix `20260513225500`:

* `20260513225500_rls_helper_signature_standardization.sql` (file 1)
* `20260513225500_rls_helper_standardization_policy_checks.sql` (file 2)

File 1 installs 22 RLS helper functions, 9 of them with legacy / mixed / reversed parameter names. File 2 attempts to `CREATE OR REPLACE` all 22 with canonical `(p_org_id, p_user_id)` signatures. Postgres rejects parameter renames via `CREATE OR REPLACE` (SQLSTATE 42P13), so file 2 fails on a fresh database (`supabase db start` in CI).

In production and on the dev project, file 2 was marked applied in `supabase_migrations.schema_migrations` but never actually ran successfully — the functions are in their legacy file 1 state.

### Investigation findings (2026-05-23)

| Question                                                                         | Result                                                       |
| -------------------------------------------------------------------------------- | ------------------------------------------------------------ |
| Anything in the codebase uses named-arg syntax (`func(p_org_id := ...)`)?        | **No.** Zero hits.                                           |
| All 565 dependent RLS policies use positional args                               | Yes                                                          |
| Does `gr_has_org_access` produce wrong results despite reversed parameter names? | No — body uses positional pass-through; reversal cancels out |
| Has production been operating correctly with legacy sigs since 2026-05-13?       | Yes                                                          |

**Nothing depends on the canonical signatures.** File 2's signature-normalization portion is purely cosmetic.

***

## Options Considered

### Option A: New migration with dynamic policy save-and-restore

A new migration after the collision that snapshots `pg_policies` for the 9 functions, drops them CASCADE, recreates with canonical sigs, restores the 565 dependent policies via reconstructed `CREATE POLICY` DDL.

* **Pros:** Adheres to "never edit migrations" rule.
* **Cons:** Must run BETWEEN file 1 and file 2 (filename sort trickery). DO block reconstructing 565 `CREATE POLICY` DDLs from `pg_policies` columns is brittle (PERMISSIVE/RESTRICTIVE, roles arrays, schemas). High blast-radius for a purely cosmetic outcome.
* **Why not chosen:** Investigation showed no caller needs canonical signatures.

### Option B: Edit file 1 to use canonical signatures

Change file 1's 9 conflicting function declarations to match file 2's canonical signatures.

* **Pros:** Single small diff.
* **Cons:** Production already has LEGACY signatures (file 2 never ran successfully). Editing file 1 to canonical would create a file-vs-DB-state divergence on prod/dev.
* **Why not chosen:** Creates divergence rather than resolving it.

### Option C: Function overloading

Define 9 NEW canonical-named functions that delegate to the legacy-named ones; migrate policies to v2 over time.

* **Pros:** No edits, no drops.
* **Cons:** 9 redundant functions in the schema indefinitely. "Eventually drop" rarely happens.
* **Why not chosen:** Permanent schema cruft for purely cosmetic gain.

### Option D: Edit file 2 to keep only its policy changes ✓ **Chosen**

Remove file 2's 22 `CREATE OR REPLACE FUNCTION` blocks. Keep its 5 policy DROP/CREATE blocks. File 1 and file 2 then both reflect the legacy reality that production already runs with.

* **Pros:** Smallest possible diff (\~-400 lines). File content matches DB state. Fresh DB succeeds. No policy churn. No new schema objects.
* **Cons:** Edits an existing migration file. Requires ADR-021 (this document) to justify.
* **Why chosen:** Lowest risk path with zero functional regression. Restores fresh-DB validation. The "canonical signatures" goal was unsupported by any caller.

***

## Decision

Edit `supabase/migrations/20260513225500_rls_helper_standardization_policy_checks.sql` to remove the 22 `CREATE OR REPLACE FUNCTION` blocks (lines 8–415 of the original 494-line file). Retain its 5 policy `DROP POLICY IF EXISTS` + `CREATE POLICY` blocks. Update the header comment to point readers to this ADR.

***

## Consequences

### Positive

* `supabase-ci / validate-migrations` returns to green on fresh-DB rebuilds.
* File 2's content matches the actual schema state on prod and dev.
* No policy or table changes; no DB-side migration required.
* The `validate-migrations` check becomes eligible to add to required-status-checks (separate follow-up).

### Negative

* One-time exception to the "never edit migrations" rule. The rule itself remains in force.
* Any developer who had a local DB where file 2's function-redefinition portion DID succeed (unlikely; would require manually applying file 2 first, then file 1) will see drift between their local schema and the file on next `supabase db reset`. Mitigation: announce the change in the team channel and document `supabase db reset` as the canonical refresh action.

### Precedent — criteria for future migration-edit exceptions

Any future request to edit an existing migration must independently demonstrate ALL of:

1. **No behavior change for the migrated DBs.** The edit makes file content match existing DB state, not the other way around.
2. **A new-migration approach is impractical.** Either (a) ordering constraints prevent insertion at a different timestamp, or (b) the new migration would require dynamic DDL reconstruction that's brittler than the edit.
3. **Investigation has confirmed no downstream code depends on the migrated-away state.** Includes grep across `src/`, `supabase/migrations/`, `supabase/functions/`, and named-arg syntax.
4. **Its own ADR** documents (a)–(c) and explicitly states the dev-side migration repair instructions.

Failure to meet ALL four criteria means a new migration is required.

***

## Addendum (2026-05-24): version-dedup + repo-wide collision guard

ADR-021 removed file 2's failing function blocks but left both files sharing the
`20260513225500` version. On a fresh `supabase db reset`, two files with the same
version still collide on `supabase_migrations.schema_migrations` (PK = version),
and two further pre-existing pairs were found on `development`:

* `20260521200000` — `hr_aca_fte_count_numeric` + `hr_grievance_assignees`
* `20260521210000` — `hr_grievances_platform_admin_insert` + `rh_episode_type`

On the dev project only one file of each pair was recorded; the other never
applied (e.g. `hr_aca_compliance.fte_count` was still `integer`, not `NUMERIC`).
Root cause of recurrence: the within-PR check (`check-prod-collision.ts`) only
inspects migrations added in the current diff, so two PRs — or the two repos in
the Lovable↔canonical sync — can each add a file at the same timestamp and the
clash only surfaces once both land.

**Resolution:** the dependency-later file of each pair was renamed to a unique
`+1s` version (all three migrations are idempotent, so they re-apply cleanly on
remotes that recorded the other file):

* `20260513225500_rls_helper_standardization_policy_checks.sql` → `…225501_…`
* `20260521200000_hr_aca_fte_count_numeric.sql` → `…200001_…`
* `20260521210000_rh_episode_type.sql` → `…210001_…`

This complements (does not reverse) the Option D content fix above. A new
repo-wide guard, `scripts/database/check-migration-version-collisions.mjs`
(npm `db:check:migration-collisions`), now scans the **entire** migrations
directory and runs in the `migration-guard` job of `supabase-ci.yml`, so cross-PR
/ cross-repo version collisions can no longer reach `development`.

***

## Addendum (2026-06-09): blessed exception — automated version-prefix rename

The repo-wide guard added on 2026-05-24 only *detects* collisions. The
integration-time guard (`scripts/database/ensure-unique-migration-versions.ts`,
spec `docs/superpowers/specs/2026-06-09-migration-collision-proofing-and-declarative-pilot-design.md`)
now *fixes* them at the deploy boundary. A **version-prefix rename with no
SQL-body change** is hereby a blessed exception class to "Never edit existing
migrations":

* **What is allowed:** changing only the 14-digit filename prefix of an
  **unapplied** migration (the lexically-later file of a colliding pair), with
  the SQL contents byte-identical.
* **Hard safety boundary:** never rename a file whose **current version is
  already recorded** on the target remote. Renames target the unrecorded twin
  only. A shadowed rename (the twin's version is recorded) is allowed solely when
  the unrecorded file passes the `isReapplySafe` static scan; otherwise the tool
  REFUSES and a human handles it.
* **Why it does not need a fresh ADR each time:** unlike the four-criteria
  content-edit exception above, a prefix rename changes no behavior on any DB —
  it only assigns a unique, monotonically-greater version. This addendum is the
  standing authorization.

***

## Addendum (2026-06-09b): live-verification fixes — ledger acquisition + exit-code semantics

The first real dev/prod deploys after the guard landed (PR #1058) **failed at the
guard step itself** — a regression, since the guard ran before drift-check and push.
Two defects, both fixed:

* **Remote-ledger acquisition.** The deploy step read the ledger with
  `supabase migration list --linked --output-format json`. On the pinned CLI (2.101)
  `--output-format json` is a **no-op for the `migration list` subcommand** — it still
  prints the three-column `LOCAL │ REMOTE │ TIME` text table — so `JSON.parse` threw
  (`Unexpected token 'L'`). This is exactly what design §5.3 warned against ("do not
  parse `migration list` text… the human-readable table is not a stable contract"),
  but the SQL-via-`json_agg` source it preferred needs a direct DB connection string
  the deploy job does not have. Resolution: a pure, unit-tested parser
  `parseRemoteVersionsFromMigrationListText` reads **only the Remote column** of that
  table (a local-only row = not recorded; a remote-only orphan row, e.g. issue #851,
  \= recorded), fed via the CLI's new `--remote-text` flag. `--remote-json` is retained
  for the SQL-source upgrade path. **Do not "fix" the workflow back to `--output-format
  json`** — it is silently broken on this CLI.
* **Exit-code semantics.** The CLI exits **2** on an IO/parse error and **1** on a real
  collision/refusal, but the workflow bash conflated them — a ledger-read failure was
  mislabeled "Unsafe (shadowed) collision — refused." The guard now branches on the
  exact code: 0 → continue, 2 → fail loud as an IO error (not a collision), 1 → apply
  (dev) or fail loud (prod). The no-collision common case is a clean no-op.

**Persist-tier decision (Test-Plan item 2).** Dev stays at **Tier 2 = auto-PR + human
merge**. `development` branch protection requires 1 approving review and 5 status
checks, and the Actions `GITHUB_TOKEN` runs as `github-actions[bot]` — not a repo admin
— so `gh pr merge --admin` cannot work for it (`enforce_admins` only exempts real
admins). Auto-merging the rename PR would need a net-new admin App/PAT and would be an
auto-write to a protected branch (the design's principal risk, §9). The bot therefore
opens `bot/migration-version-bump` + a PR and aborts the deploy; a human merges to
unblock. Prod never auto-commits (Tier 1, fail-loud).

***

## Addendum (2026-06-13): dedicated auto-PR token + sanctioned resolution for a shadowed-collision REFUSAL

**Status: Proposed — pending owner sign-off.** The rest of ADR-021 remains Accepted; this
addendum governs a resolution path not yet exercised. Follow-on from the Phase-2 migration
work (`docs/superpowers/specs/2026-06-13-supabase-stack-migration-resolution-design.md`).

### 1. The auto-PR needed a dedicated token (not new authorization — an enabler)

Addendum 2026-06-09b chose Tier-2 (auto-PR + human merge) and noted auto-*merge* "would need a
net-new admin App/PAT." In practice even **opening** the PR didn't fire: the
`github-actions[bot]` `GITHUB_TOKEN` is blocked by GitHub from creating pull requests and — when
it can — the resulting PR does **not** trigger `pull_request`/`push` workflows (anti-recursion),
so required checks never run and it can't be merged. That is why colliding versions kept being
fixed by hand. Phase 2 wires a dedicated **`MIGRATION_AUTOBUMP_TOKEN`** (recommended: fine-grained
PAT scoped to this repo, **Contents: R/W + Pull requests: R/W**) used **only** to push the bot
branch and open the PR; it falls back to `GITHUB_TOKEN` with a loud `::warning::` when unset, and
the bot branch is now deterministic per fix (`bot/migration-version-bump-<sig>`) so distinct
collisions get distinct PRs. This is **still not auto-merge** — a human reviews and merges. It
authorizes nothing beyond the rename already blessed in addendum 2026-06-09; it just makes the
already-blessed mechanism actually function.

### 2. Scope clarification: `isReapplySafe` gates ONLY shadowed renames

`isReapplySafe` is **not** a global "every migration must be idempotent" mandate. Most migrations
create objects exactly once and are *intentionally* not re-apply-safe. It is consulted **only** when
deciding whether the unrecorded twin of a **shadowed** collision may be auto-renamed (a rename makes
that file apply fresh, so unguarded `CREATE`/`ADD COLUMN` would error if the object already exists).
A tree-wide scan (2026-06-13) found 16/138 files (12%) not re-apply-safe in isolation but **0**
actual candidates — there are **0** version collisions. **There is no idempotency backlog to
retrofit;** this addendum defines what to do *if/when* a shadowed-collision REFUSAL occurs, so nobody
silently edits an applied migration under deadline pressure (the precise failure ADR-021's "Option B"
was rejected to prevent).

### 3. Sanctioned resolution path for a REFUSED shadowed collision

When `ensure-unique-migration-versions.ts` REFUSES (exit 1, "not re-apply-safe"), resolve in this
priority order. **Never edit a migration already recorded on any remote ledger.**

1. **Identify the recorded twin.** Check `supabase migration list --linked` on **both** dev and
   prod. The recorded twin is immutable; only the **unrecorded** twin is a candidate.
2. **Unrecorded everywhere → guarding it is permitted.** If the unrecorded twin is absent from both
   ledgers, adding `IF NOT EXISTS` / `DROP … IF EXISTS` guards to **that file** is allowed: it meets
   ADR-021's four content-edit criteria (no applied DB has run it, the guards change no effective
   behavior, it is reviewed in a PR, and it unblocks a real deploy). Re-run the guard; the rename
   then proceeds.
3. **Object already exists on the target → supersede, don't edit.** Do **not** edit either historical
   file. Author a **new** forward migration that applies cleanly (guarded DDL, or a no-op when the
   object exists), and leave both colliding files byte-unchanged.
4. **Record it.** Note the chosen path in the PR, citing this addendum, so the exception stays
   auditable.

### 4. Criteria a future *automated* handler must meet (if we ever auto-resolve case 2)

ADR-021's four content-edit criteria, **plus**: the edit touches only a file the tool has just
re-verified absent from **every** remote ledger immediately before applying. Until that
ledger-absence re-verification exists, **case 2 stays human-only** — the tool continues to REFUSE.

***

## References

* Spec: `docs/superpowers/specs/2026-05-23-migration-collision-42p13-design.md`
* Plan: `docs/archive/superpowers-plans-2026-05/2026-05-23-migration-collision-42p13.md`
* Project rule: `.claude/rules/database.md` (Migration Files)
* Related: `docs/superpowers/specs/2026-05-23-github-actions-consolidation-design.md` (parent CI cleanup)
* Repo-wide guard: `scripts/database/check-migration-version-collisions.mjs`