--- title: "Permission SSoT cutover — how three authoring sites accumulated and one class of write path got missed" date: 2026-04-23 scope: refactor / SSoT migration / staged delivery primary-prs: ["#768", "#790", "#1831"] primary-issues: ["#766", "#769", "#794", "#1806", "#1808"] status: post-mortem (reference) --- # Lessons Learned — Permission SSoT cutover (2026-04-11 → 2026-04-23) ## What happened Over 12 days, the system-permission model went through a staged refactor that ended with a P1 bug in production data and a set of open tails in the code. The refactor direction was correct; the execution dropped stitches in three ways that are generalizable far beyond this subsystem. ### Timeline | Date | PR / Issue | Actor | Event | |---|---|---|---| | 2026-01-27 | `22ca489a` | Qi Liu | 70 `Perm` Go constants introduced as permission keys. No `_System_Permission` table yet; this was the reasonable starting point. | | 2026-04-11 12:54 | **#768** | Ian | "Unify system permission storage to SSoT" — junction table becomes authoritative for reads, with 26 `Permissions*` bitmap columns kept as a write-through cache. Backfill covers `_System_ProfilePermission → junction`. | | 2026-04-11 18:35 | **#790** | Ian | "Expose full 63-permission catalog" — adds `AllSystemPermissions()` Go catalog function + `PermissionDefinition` struct as a **third authoring site** for label/category/prereq, 6 hours after #768. | | 2026-04-11 19:07 | #766 closed | Ian | Closing note: "Remaining legacy cleanup, if we still want it, is narrow enough to track separately." | | 2026-04-11 19:08 | **#794 opened** | Ian | Self-opened follow-up: remove `_System_ProfilePermission` remnants. **Still OPEN 12 days later.** | | 2026-04-15 | #835 | Ian | Metadata control plane with JSON pack model lands. Now technically possible to move the permission catalog out of Go. | | 2026-04-23 02:54 | **#1806 opened** | — | Staging data shows 47 of 48 profile-owned PermissionSets under-seeded. `System Administrator (Salesforce)` PS has 200 `Permissions*=true` bitmap fields but only 24 junction rows. Setup → Security shows 3 of 9 tiles. Bug had been live for 12 days. | | 2026-04-23 | **#1831** | Ian | "Permission metadata refactor" — collapses three authoring sites into `standard_permissions.json`. Data spec step 1 quietly fixes #1806, but PR does not link #1806. | ### What the user saw From #1806, staging evidence (2026-04-23): | Profile | Junction rows | `Permissions*=true` columns | |---|---|---| | System Administrator (native bootstrap) | 70 | 70 | | System Administrator (Salesforce, Fivetran-imported) | **24** | **200** | | 47 / 48 profile-owned PSes under-seeded; 4 at 0 | | | Visible symptoms: Setup → Security shows 3 of 9 tiles (hidden tiles gated by `ManageProfilesPermSets` / `ManageRoles` / `ManageSharing`, all missing from junction but present as `true` in bitmap). Create User modal body empty (FieldPermissions has the same under-seed pattern). ### The concrete failures **F1 — The SSoT cutover missed a write path.** #768's backfill covered exactly the write paths the engineer knew about from reading the codebase: `SeedSystemPermissions` (bootstrap seeder), the legacy `_System_ProfilePermission` rows, and all writes funneling through the new `SetSystemPermission()` gate. It did not cover writes that enter the database from outside the codebase — specifically, Salesforce data imports written directly into `PermissionSet.Permissions*` bitmap columns via Fivetran / bulk loaders / admin SQL. Once the read path flipped to the junction, those grants were silently invisible. **F2 — The same misconception was carried forward into #1831.** The seeder comment at `controlplane/packs/permissions.go:444-447` reads "Any historical grant data in the bitmap columns that predated the junction SSoT was backfilled by Batch 3's ProfilePermission migration (above) on prior boots." This is factually wrong — Batch 3 migrates from `_System_ProfilePermission`, never from bitmap columns. The comment reproduces #768's blind spot and will mislead the next engineer. **F3 — The intermediate refactor was marketed as a finished refactor.** #768's PR body used final-state language ("single source of truth"). The follow-up issue #794 was self-opened one minute after closing #766 and ignored for 12 days. #790 introduced a new authoring site (`AllSystemPermissions()`) with no tracking issue for its eventual migration to the pack model. #1831 fixes all of this, but its PR body does not link #1806 / #1808 / #794. **F4 — Exporting a "simple" primitive preserved a bypass route.** #1831 P12 makes `ApplyPermissionChanges` the "sole write gate" and P16 deletes four bypass helpers. But `PermissionService.SetSystemPermission` stays exported with a docstring instructing callers to do prereq validation themselves — the same pattern the refactor set out to eliminate. Zero external callers today; the method exists only so a future engineer can skip the gate "just this once". **F5 — F1 recurred, one layer down, inside #1831's own remediation specs.** The three-spec chain written to fix #1806 enumerated 26 `Permissions*` columns by hand in stage 1 (backfill), stage 2 (audit), and stage 3 (drop) — the exact 26 that the pre-refactor `system_tables.json` had declared. But the #1806 incident itself was proof that production clusters hold more than those 26: the Fivetran-imported admin PermissionSet had 200 `Permissions*=true` bitmap fields, and the user-visible symptom list named `ManageRoles`, `ManageProfilesPermSets`, and `ManageSharing` — none of which are in the 26. The same engineer-codebase-view blind spot that caused F1 in #768 reproduced in the specs that were supposed to fix F1. Stage 1 would have backfilled 26 columns and left the rest; stage 2 would have vacuously passed with only 26 invariants; stage 3 would have dropped 26 and left Fivetran-created columns as orphans. Fix (post-review): the three specs were restructured so the column set is discovered per-cluster from `INFORMATION_SCHEMA.COLUMNS` via a preflight query that is byte-identical across all three stages. No enumeration, no 1054 exemption, greenfield clusters are a natural no-op (preflight returns zero rows → no statement referencing a missing column is ever emitted). L11 formalizes the rule. **F6 — F1 recurred a second time, inside the runtime seeder itself.** Post-review of #1831: `controlplane/packs/permissions.go` still ran a "Batch 3" migration on every boot that read `_System_ProfilePermission` and wrote `_System_PermissionSetPermission` via `ON DUPLICATE KEY UPDATE IsEnabled = VALUES(IsEnabled)`. Because `_System_ProfilePermission` has had no writer since #768 (pre-cutover frozen data), the rows it held were guaranteed to be stale after any admin edit via the new write gate — and the `ON DUPLICATE` clause faithfully overwrote the admin's revocation on the next seed. Mechanically this was F1 a third time: a legacy source the cutover had "retired" kept doubling as a runtime writer to the SSoT, bypassing admin intent, silently, every boot. Fix: remove Batch 3 from the runtime entirely; move the one-time upgrade transfer into `data-specs/2026-04-23d-migrate-profilepermission-to-junction.md` with `INSERT IGNORE` (no `UPDATE` clause); keep the legacy table and its generated constants as schema-level compat debt only. L12 formalizes the rule. **F7 — The destructive cleanup spec claimed protocol compatibility that the protocol explicitly rejects.** The stage 3 drop file contained `ALTER TABLE ... DROP COLUMN` statements while still declaring `apiVersion: data-spec/v1` and claiming per-manifest execution "preserves the data-spec-apply protocol." That was a contract bug: `data-spec-apply` requires `apiVersion: data-spec/v1` and rejects `DROP COLUMN` in its forbidden-pattern scan. Prose cannot make an operation compatible with a protocol that explicitly forbids it. Fix (post-review): stage 3 now declares `apiVersion: destructive-runbook/v1`, is manual-operator-only, requires two CODEOWNER approvers, and says ordinary `data-spec-apply` must reject it before running SQL. L13 formalizes the boundary. --- ## Lessons — generic, extractable ### L1. Before flipping a read path to a new SSoT, enumerate every writer A cutover that backfills from "every place in my codebase that writes this data" is incomplete by default. Production databases are written by things outside your codebase: ETL replication, bulk data loaders, admin SQL, legacy binaries during rolling deploy. The question is not "what does my code write" but "what ends up in this table, from any source". Minimum: for every flip of a read path, produce a list of all writers (code paths + external data sources) with evidence that each one either flows into the new SSoT or is captured by an explicit backfill. If the list has a "possibly" / "should" / "usually" in it, the cutover is not ready. *Reference: F1, #1806.* ### L2. "SSoT" is three unifications, not one A legitimate SSoT claim requires: 1. **Single storage** — one row for each fact. 2. **Single write path** — one function / gate through which all writes pass. 3. **Single authoring site** — one file / table where humans edit the values. Claiming SSoT with any of the three still plural is technical debt with a marketing label. Dual-write with a "cache" is not SSoT. An exported helper with a "caller beware" docstring is not a single write gate. *Reference: F4, #768's "unified SSoT" while keeping bitmap cache, #1831 keeping `SetSystemPermission` exported.* ### L3. Bug fixes should not expand the number of authoring sites without justification When a user-visible bug has root cause "reads consume the wrong source", the default response is one of: - Change what the reads consume (point them at the SSoT), or - Change what the source contains (backfill / migrate data into the SSoT). Adding a new parallel source whose shape happens to match what the reads want is a legitimate option only when the default responses are infeasible — and in that case the PR body must say why, explicitly. The failure mode this rule targets is a PR that ships the quick fix without noting it is a quick fix, leaving the extra authoring site as permanent debt with no tracking. If the root cause is already stated in the PR body and the fix still adds a parallel source, the author owes a written reason for not taking the default response. *Reference: F3, #790 added `AllSystemPermissions()` as a third authoring site while its own PR body stated the root cause was "reads read the wrong source" — no written reason for not fixing the reads instead.* ### L4. Intermediate refactor PRs must declare they are intermediate PRs that ship a staged refactor in parts must say so, in the title and the body, and must link the tracking issue for the deferred tail. Final-state language ("single source of truth", "unifies X", "removes Y") guarantees the tail is forgotten — including by the author, weeks later, on the same subsystem. Useful prefixes: `bridge:`, `stage N of M:`, `refactor (interim):`. The body must include a section titled "What this PR does NOT do" listing every deliberate deferral with a linked tracking issue. *Reference: F3, #768 marketed as done, #794 ignored for 12 days by its own author.* ### L5. A self-opened tracking issue is not sufficient on its own An issue opened by the author for the author has no external enforcement. It can exist, but it must be paired with a mechanical backstop so that ignoring it has a cost. Treating the tracking issue itself as evidence the work will happen is not reliable — the failure mode this rule targets is a PR that defers work with nothing but a GitHub issue between the deferral and oblivion. When a PR defers work, at least one of the following backstops must be present: - PR-body checklist that links the tracking issue and commits to blocking the next related merge (or a specific future PR) until the issue closes - Code `// TODO(#NNNN, expires YYYY-MM-DD)` that a linter / CI check fails on after the expiry date - Milestone or release-blocker label on the tracking issue with a named owner and a concrete date - Interim code in a form that is uncomfortable to leave in place (e.g. a deprecation shim that panics after a date, a metric that alerts while the workaround is active) so the tail has a cost to ignore The rule is not "self-opened issues are bad" — it is "self-opened issues plus nothing else is not a plan". *Reference: F3, #794 opened 2026-04-11 19:08, unresolved 2026-04-23 — same author, overlapping subsystem, 12 days elapsed. No backstop beyond the issue itself.* ### L6. "Belt-and-suspenders" migrations require evidence, or they should be reclassified Any migration described as "double-check", "just in case", or "belt-and-suspenders" rests on a claim that a primary fix already did the work. That claim requires evidence from the real system — row counts before / after, log excerpts from a real boot, or equivalent observational data. If the author cannot cite evidence that the primary migration ran, the "belt-and-suspenders" migration is the primary fix under a cautious label, and should be reclassified as such: full validation, audit invariants, linked bug issues, and the PR body should name it for what it is. *Reference: F1/F2, #1831's data-spec called its backfill a "belt-and-suspenders sweep" on the assumption a direction-1 reconcile had already run. #1806's staging numbers (24 of 200 grants populated) proved the reconcile had not run for externally-imported data. The "belt-and-suspenders" backfill was in fact the primary fix for a P1 bug.* ### L7. External compatibility is a projection problem, not a storage problem When external clients depend on a specific wire-shape (Salesforce API, database column names, JSON field names), the first instinct is to store data in the shape the external caller expects. This couples storage layout to external contract and blocks internal improvements. Almost always, the wire-shape can be produced at the boundary: describe endpoints synthesize the shape from a catalog row; query paths project junction rows back into the expected shape; write paths intercept the expected shape and route to the normalized store. Storage is then free to follow whatever model is actually correct. Before storing anything in a shape dictated by an external consumer, ask: can this shape be synthesized on the boundary? *Reference: For 12 days, everyone assumed `describe(PermissionSet)` returning `Permissions*` boolean fields meant 26 physical columns had to exist. #1831 showed the describe response can be synthesized from catalog rows on demand. The 26 columns were never a storage requirement — they were a projection, misfiled as storage.* ### L8. Write-through caches drift as soon as one writer bypasses the gate Any "junction table + bitmap column cache, kept in sync by the write gate" pattern holds only if every writer in the ecosystem goes through the gate. Salesforce data imports, Fivetran replication, CSV bulk loaders, admin SQL, and old pods during rolling deploy are all writers. If any of them bypasses the gate even once, the cache drifts silently and the first reader who trusts the cache over the junction (or the junction over the cache — direction doesn't matter) gets wrong answers. Unless every writer is demonstrably going through the gate, a write-through cache is a liability, not an optimization. Prefer derived-on-read (compute the cache shape from the SSoT at query time) over stored-cache unless you have benchmarks showing the projection cost is prohibitive. *Reference: F1, bitmap cache drifted from day one of #768 because Fivetran-imported profiles wrote straight to bitmap columns without ever touching `SetSystemPermission`.* ### L9. Destructive migrations require a mandatory audit window When a migration combines a backfill step with a destructive step (DROP COLUMN, DELETE, TRUNCATE) in a single spec, the validation window between them is zero. If the backfill misses a row, the destructive step runs anyway and the data is lost permanently (the source column is already gone). Split destructive migrations into two specs with a blocking audit between them. The audit must assert a concrete invariant — e.g. `count(new_table rows) == count(old_source rows)` — on real data, not just a dry-run. The second spec cannot be applied until the audit passes. *Reference: F1, #1831's `drop-permissionset-bitmap-columns` spec combines "backfill junction from bitmap" (step 1) with "DROP COLUMN bitmap" (step 2) in one file. If step 1 misses any row in any of the 26 columns, step 2 destroys the data permanently.* ### L10. PRs must link the bugs they actually fix, even incidentally Before requesting review, grep open issues in the affected subsystem for any bug the diff might fix. Any issue resolved — even as a side-effect of another change — must be linked with `Closes #` in the PR body. Labeler bots and human triagers do not make this connection reliably. Without the link, the issue stays OPEN, the symptom stays assumed-live in dashboards, and a future engineer re-discovers the same bug. *Reference: F3, #1831 fixes #1806 and #1808 via its data-spec step 1 but does not link either; tacitly defers #794 without declaration.* ### L11. A migration's source-of-truth for "the set of things to migrate" must be live, not enumerated When the set of rows, columns, or tables a migration operates on depends on production reality that is not fully knowable from the codebase — ETL-created columns, Fivetran-replicated tables, tenant-variable schemas, user-authored artifacts, any surface that external writers can expand — the migration's SQL must derive that set at apply time from a live source (typically `INFORMATION_SCHEMA`), not from a list the author typed out. The SSoT for "which columns exist on PermissionSet today" is the database itself, not `system_tables.json` and not the engineer's grep. Practical form: - A **preflight** query at the top of the spec, textually identical across every stage of a multi-spec chain so the reviewer can confirm one set is shared. - An explicit **empty-result branch** (no-op, mark applied, stop) so greenfield clusters never emit SQL referencing a missing artifact — the data-spec-apply protocol's strict missing-column-STOP gate then stays in force end-to-end with no exemption. - An explicit **non-empty branch** that operates once per discovered row, substituting column / table names into a static per-row template statement — preserving per-statement dry-run and row-count verification in the applier protocol. - When a chain has multiple stages, each stage re-runs its own preflight (do not reuse an earlier stage's result) and a destructive stage cross-checks its preflight against the manifest the preceding gate recorded, halting if a new artifact appeared in between. This rule is the antidote to F1/F5: enumerate writers and the artifacts they produce from production reality, in SQL, at apply time — not from codebase review. *Reference: F5, #1831's three-spec chain originally enumerated 26 `Permissions*` columns by hand and was restructured (post-review) to discover them from `INFORMATION_SCHEMA` per stage.* ### L12. All legacy-to-SSoT writers must be enumerated; stale-after-cutover sources must leave the runtime > All legacy-to-SSoT writers must be enumerated. If the legacy source > can be stale after cutover, it must be removed from runtime and, if > still needed for upgrade, moved to a one-time additive-only data > spec. The rule is stronger than "change `ON DUPLICATE KEY UPDATE` to `INSERT IGNORE`". Switching the SQL form fixes the immediate drift but leaves a legacy read path in the runtime, which is an invitation to reintroduce the drift next time someone "tidies up". The actual root fix is that the legacy table ceases to be a runtime source of truth at all — its only remaining role is schema-level compat debt, and any upgrade transfer is a one-shot additive-only data-spec. Operationally, every time the fix "this legacy writer was still overwriting the SSoT" is made, grep the whole repo for the same pattern before declaring the work done. The permission subsystem produced this bug class in three distinct places (the #768 bitmap cache, the #1831 data-spec Stage 1 `ON DUPLICATE KEY UPDATE`, and the `controlplane/packs/permissions.go` Batch 3) — fixing one at a time is how the refactor stayed silently wrong for 12 days. Practical form: - **Runtime**: zero reads from any legacy source into the SSoT. Enforced by a grep-style invariant test on the seeder file (permissions package has `TestPermissionsRuntime_NoLegacyProfilePermissionWriter` — copy that pattern for the next subsystem that makes this cut). - **Upgrade transfer**: one-shot data-spec, additive-only (`INSERT IGNORE` with no `UPDATE` clause). Preflight-gated so greenfield / post-migration clusters are a natural no-op. - **Legacy table**: may remain in `system_tables.json` and generated constants as compat debt for rollback value; must not gain new writers or readers. Drop can be tracked as a separate follow-up issue. *Reference: F6, `_System_ProfilePermission` Batch 3 was deleted from `controlplane/packs/permissions.go` in #1831; the one-time transfer moved to `data-specs/2026-04-23d-migrate-profilepermission-to-junction.md`; `TestPermissionsRuntime_NoLegacyProfilePermissionWriter` locks the invariant against regression.* ### L13. Data-spec prose cannot override the apply protocol When a migration file's prose says it is compatible with an apply protocol, that claim must be checked against the protocol text. If the protocol rejects an operation — for example `data-spec-apply` rejecting `DROP COLUMN` — the file has only two honest choices: - Extend the protocol deliberately in a dedicated PR with a general gate and full review. - Declare itself outside the protocol via a non-`data-spec/v1` `apiVersion` and document explicit manual-operator execution gates. A compatibility claim that contradicts the protocol is a contract bug, not a wording issue. Do not create a one-off escape hatch inside an unrelated refactor. *Reference: F7, #1831's stage 3 destructive cleanup originally declared `data-spec/v1` while containing `DROP COLUMN` statements that the protocol forbids.* --- ## What good looks like (tl;dr for future refactors) The spirit of these rules is **"ship evidence, not assertions"**. Most are enforced by "show me the numbers / show me the mechanism", not by absolute prohibition. 1. Before flipping a read path, produce a written list of every writer to the source table — code and non-code — with backfill evidence for each. Evidence, not "I grepped my repo". 2. When claiming "SSoT", verify three unifications (storage + write gate + authoring site). State the three claims in the PR body and cite a grep-verifiable fact for each. 3. When fixing a user-visible bug whose root cause is mis-routed reads, the default response is to fix the reads or the source. If instead a new parallel source is added, the PR body must say why the default response was infeasible. 4. When shipping part of a larger refactor, say so in the title, link the tracking issue in the body, and include a "What this PR does NOT do" section. 5. A self-opened tracking issue alone is not a plan. Pair it with a mechanical backstop (blocking checklist, expiring TODO, milestone with owner+date, uncomfortable interim shim). 6. "Belt-and-suspenders" is a claim about a primary step. Back it with numbers, or reclassify the migration as the primary fix. 7. Treat wire-shape compatibility as a projection problem. Synthesize on the boundary; do not contort storage. 8. Avoid write-through caches when any writer in the ecosystem can bypass the gate. Prefer derived-on-read unless benchmarks say otherwise — and if you keep the cache, name every writer. 9. Never combine backfill and destructive operations in one migration. Separate specs, blocking audit, real-data invariant. (This one is absolute — the failure mode is irreversible data loss.) 10. Link every issue your PR fixes, even incidentally. `Closes #` is not optional. 11. When a migration operates on a set that depends on production reality (ETL-created columns, tenant-variable tables, any surface external writers can expand), derive that set at apply time from `INFORMATION_SCHEMA` — not from an author-typed list. Share the preflight query byte-identically across stages; let greenfield be a natural no-op via the empty-result branch, with no missing-column exemption. 12. Enumerate every legacy-to-SSoT writer in the runtime, not just the one the current bug points at. Any legacy source that can be stale after cutover must leave the runtime — its rows belong in a one-time additive-only data-spec, not in a boot-time migration. Lock the absence with a grep-style invariant test on the seeder file so the writer cannot come back under a different name. 13. A migration file's prose cannot override the apply protocol. If the operation is forbidden by the protocol, either extend the protocol in a dedicated PR with a real gate, or mark the file outside that protocol with a non-`data-spec/v1` `apiVersion` and explicit manual-operator gates.