diff --git a/doc/active-pointer-smart-deletion-pattern.md b/doc/active-pointer-smart-deletion-pattern.md new file mode 100644 index 00000000..f308dbc2 --- /dev/null +++ b/doc/active-pointer-smart-deletion-pattern.md @@ -0,0 +1,370 @@ +# Engineering Directive v2 — Active Pointer + Smart Deletion Pattern (hardened) + +**Author**: Matthew Raymer +**Date**: 2025-01-27 +**Status**: 🎯 **ACTIVE** - Production-grade engineering directive for implementing smart deletion patterns + +## Overview + +This supersedes the previous draft and is **copy-pasteable** for any ``. It keeps UX smooth, guarantees data integrity, and adds production-grade safeguards (bootstrapping, races, soft deletes, bulk ops, and testability). Built on your prior pattern. + +## 0) Objectives (non-negotiable) + +1. Exactly **one active ``** pointer (or `NULL` during first-run). +2. **Block deletion** when it would leave **zero** ``. +3. If deleting the **active** item, **atomically re-point** to a deterministic **next** item **before** delete. +4. Enforce with **app logic** + **FK `RESTRICT`** (and `ON UPDATE CASCADE` if `ref` can change). + +--- + +## 1) Schema / Migration (SQLite) + +```sql +-- __active_.sql +PRAGMA foreign_keys = ON; + +-- Stable external key on (e.g., did/slug/uuid) +-- ALTER TABLE ADD COLUMN ref TEXT UNIQUE NOT NULL; -- if missing + +CREATE TABLE IF NOT EXISTS active_ ( + id INTEGER PRIMARY KEY CHECK (id = 1), + activeRef TEXT UNIQUE, -- allow NULL on first run + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')), + FOREIGN KEY (activeRef) REFERENCES (ref) + ON UPDATE CASCADE + ON DELETE RESTRICT +); + +-- Seed singleton row (idempotent) +INSERT INTO active_ (id, activeRef) +SELECT 1, NULL +WHERE NOT EXISTS (SELECT 1 FROM active_ WHERE id = 1); +``` + +**Rules** + +* **Never** default `activeRef` to `''`—use `NULL` for "no selection yet". +* Ensure `PRAGMA foreign_keys = ON` for **every connection**. + +--- + +## 2) Data Access API (TypeScript) + +```ts +// Required DAL +async function getAllRefs(): Promise { /* SELECT ref FROM ORDER BY created_at, ref */ } +async function getRefById(id: number): Promise { /* SELECT ref FROM WHERE id=? */ } +async function getActiveRef(): Promise { /* SELECT activeRef FROM active_ WHERE id=1 */ } +async function setActiveRef(ref: string|null): Promise { /* UPDATE active_ SET activeRef=?, lastUpdated=datetime('now') WHERE id=1 */ } +async function deleteById(id: number): Promise { /* DELETE FROM WHERE id=? */ } +async function countModels(): Promise { /* SELECT COUNT(*) FROM */ } + +// Deterministic "next" +function pickNextRef(all: string[], current?: string): string { + const sorted = [...all].sort(); + if (!current) return sorted[0]; + const i = sorted.indexOf(current); + return sorted[(i + 1) % sorted.length]; +} +``` + +--- + +## 3) Smart Delete (Atomic, Race-safe) + +```ts +async function smartDeleteModelById(id: number, notify: (m: string) => void) { + await db.transaction(async trx => { + const total = await countModels(); + if (total <= 1) { + notify("Cannot delete the last item. Keep at least one."); + throw new Error("blocked:last-item"); + } + + const refToDelete = await getRefById(id); + const activeRef = await getActiveRef(); + + if (activeRef === refToDelete) { + const all = (await getAllRefs()).filter(r => r !== refToDelete); + const next = pickNextRef(all, refToDelete); + await setActiveRef(next); + notify(`Switched active to ${next} before deletion.`); + } + + await deleteById(id); // RESTRICT prevents orphaning if we forgot to switch + }); + + // Post-tx: emit events / refresh UI +} +``` + +--- + +## 4) Bootstrapping & Repair + +```ts +async function ensureActiveSelected() { + const active = await getActiveRef(); + const all = await getAllRefs(); + if (active === null && all.length > 0) { + await setActiveRef(pickNextRef(all)); // first stable choice + } +} +``` + +Invoke after migrations and after bulk imports. + +--- + +## 5) Concurrency & Crash Safety + +* **Always** wrap "switch → delete" inside a **single transaction**. +* Treat any FK violation as a **logic regression**; surface telemetry (`fk:restrict`). + +--- + +## 6) Soft Deletes (if applicable) + +If `` uses `deleted_at`: + +* Replace `DELETE` with `UPDATE SET deleted_at = datetime('now') WHERE id=?`. +* Add a **partial uniqueness** strategy for `ref`: + + * SQLite workaround: make `ref` unique globally and never reuse; or maintain a shadow `refs` ledger to prevent reuse. +* Adjust `getAllRefs()` to filter `WHERE deleted_at IS NULL`. + +--- + +## 7) Bulk Ops & Imports + +* For batch deletes: + + 1. Compute survivors. + 2. If a batch would remove **all** survivors → **refuse**. + 3. If the **active** is included, precompute a deterministic **new active** and set it **once** before deleting. +* After imports, run `ensureActiveSelected()`. + +--- + +## 8) Multi-Scope Actives (optional) + +To support **one active per workspace/tenant**: + +* Replace singleton with scoped pointer: + + ```sql + CREATE TABLE active_ ( + scope TEXT NOT NULL, -- e.g., workspace_id + activeRef TEXT, + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')), + PRIMARY KEY (scope), + FOREIGN KEY (activeRef) REFERENCES (ref) ON UPDATE CASCADE ON DELETE RESTRICT + ); + ``` +* All APIs gain `scope` parameter; transactions remain unchanged in spirit. + +--- + +## 9) UX Contract + +* Delete confirmation must state: + + * Deleting the **active** item will **auto-switch**. + * Deleting the **last** item is **not allowed**. +* Keep list ordering aligned with `pickNextRef` strategy for predictability. + +--- + +## 10) Observability + +* Log categories: + + * `blocked:last-item` + * `fk:restrict` + * `repair:auto-selected-active` + * `active:switch:pre-delete` +* Emit metrics counters; attach `` and (if used) `scope`. + +--- + +## 11) Test Matrix (must pass) + +1. **Non-active delete** (≥2): deleted; active unchanged. +2. **Active delete** (≥2): active switches deterministically, then delete succeeds. +3. **Last item delete** (==1): blocked with message. +4. **First-run**: 0 items → `activeRef` stays `NULL`; add first → `ensureActiveSelected()` selects it. +5. **Ref update** (if allowed): `activeRef` follows via `ON UPDATE CASCADE`. +6. **Soft delete** mode: filters respected; invariants preserved. +7. **Bulk delete** that includes active but not all: pre-switch then delete set. +8. **Foreign keys disabled** (fault injection): tests must fail to surface missing PRAGMA. + +--- + +## 12) Rollout & Rollback + +* **Feature-flag** the new deletion path. +* Migrations are **idempotent**; ship `ensureActiveSelected()` with them. +* Keep a pre-migration backup for `` on first rollout. +* Rollback leaves `active_` table harmlessly present. + +--- + +## 13) Replace-Me Cheatsheet + +* `` → singular (e.g., `project`) +* `` → plural table (e.g., `projects`) +* `ref` → stable external key (`did` | `slug` | `uuid`) + +--- + +**Outcome:** You get **predictable UX**, **atomic state changes**, and **hard integrity guarantees** across single- or multi-scope actives, with clear tests and telemetry to keep it honest. + +--- + +## TimeSafari Implementation Guide + +### Clean Implementation Path (Following Directive) + +If implementing this pattern from scratch or reverting to reapply the directive, follow this clean implementation: + +#### 1) Schema Implementation + +**Initial Migration (001_initial):** +```sql +-- Enable foreign key constraints for data integrity +PRAGMA foreign_keys = ON; + +-- Create accounts table with UNIQUE constraint on did +CREATE TABLE IF NOT EXISTS accounts ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + dateCreated TEXT NOT NULL, + derivationPath TEXT, + did TEXT NOT NULL UNIQUE, -- UNIQUE constraint for foreign key support + identityEncrBase64 TEXT, + mnemonicEncrBase64 TEXT, + passkeyCredIdHex TEXT, + publicKeyHex TEXT NOT NULL +); + +-- Create active_identity table with foreign key constraint +CREATE TABLE IF NOT EXISTS active_identity ( + id INTEGER PRIMARY KEY CHECK (id = 1), + activeDid TEXT DEFAULT NULL, -- NULL instead of empty string + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')), + FOREIGN KEY (activeDid) REFERENCES accounts(did) ON DELETE RESTRICT +); + +-- Seed singleton row +INSERT INTO active_identity (id, activeDid, lastUpdated) VALUES (1, NULL, datetime('now')); +``` + +#### 2) Data Access API Implementation + +**Add to PlatformServiceMixin.ts:** +```typescript +// Required DAL methods following the pattern +async $getAllAccountDids(): Promise { + const result = await this.$dbQuery("SELECT did FROM accounts ORDER BY dateCreated, did"); + return result?.values?.map(row => row[0] as string) || []; +} + +async $getAccountDidById(id: number): Promise { + const result = await this.$dbQuery("SELECT did FROM accounts WHERE id = ?", [id]); + return result?.values?.[0]?.[0] as string; +} + +async $getActiveDid(): Promise { + const result = await this.$dbQuery("SELECT activeDid FROM active_identity WHERE id = 1"); + return result?.values?.[0]?.[0] as string || null; +} + +async $setActiveDid(did: string | null): Promise { + await this.$dbExec( + "UPDATE active_identity SET activeDid = ?, lastUpdated = datetime('now') WHERE id = 1", + [did] + ); +} + +async $countAccounts(): Promise { + const result = await this.$dbQuery("SELECT COUNT(*) FROM accounts"); + return result?.values?.[0]?.[0] as number || 0; +} + +// Deterministic "next" picker +$pickNextAccountDid(all: string[], current?: string): string { + const sorted = [...all].sort(); + if (!current) return sorted[0]; + const i = sorted.indexOf(current); + return sorted[(i + 1) % sorted.length]; +} +``` + +#### 3) Smart Deletion Implementation + +**Replace deleteAccount in IdentitySwitcherView.vue:** +```typescript +async smartDeleteAccount(id: string) { + await this.$withTransaction(async () => { + const total = await this.$countAccounts(); + if (total <= 1) { + this.notify.warning("Cannot delete the last account. Keep at least one."); + throw new Error("blocked:last-item"); + } + + const accountDid = await this.$getAccountDidById(parseInt(id)); + const activeDid = await this.$getActiveDid(); + + if (activeDid === accountDid) { + const allDids = await this.$getAllAccountDids(); + const nextDid = this.$pickNextAccountDid(allDids.filter(d => d !== accountDid), accountDid); + await this.$setActiveDid(nextDid); + this.notify.success(`Switched active to ${nextDid} before deletion.`); + } + + await this.$exec("DELETE FROM accounts WHERE id = ?", [id]); + }); + + // Update UI + this.otherIdentities = this.otherIdentities.filter(ident => ident.id !== id); +} +``` + +#### 4) Bootstrapping Implementation + +**Add to PlatformServiceMixin.ts:** +```typescript +async $ensureActiveSelected() { + const active = await this.$getActiveDid(); + const all = await this.$getAllAccountDids(); + if (active === null && all.length > 0) { + await this.$setActiveDid(this.$pickNextAccountDid(all)); + } +} +``` + +**Call after migrations:** +```typescript +// In migration completion or app startup +await this.$ensureActiveSelected(); +``` + +### Implementation Benefits + +**Following this clean path provides:** +- ✅ **Atomic Operations**: Transaction-safe account deletion +- ✅ **Last Account Protection**: Prevents deletion of final account +- ✅ **Smart Switching**: Auto-switches active account before deletion +- ✅ **Data Integrity**: Foreign key constraints prevent orphaned references +- ✅ **Deterministic Behavior**: Predictable "next account" selection +- ✅ **NULL Handling**: Proper empty state management + +### Migration Strategy + +**For Existing Databases:** +1. **Revert** current foreign key implementation +2. **Apply** clean migration sequence above +3. **Convert** existing `activeDid = ''` to `NULL` +4. **Implement** smart deletion logic +5. **Test** all scenarios from test matrix + +This clean implementation follows the directive exactly and provides **complete pattern compliance** with **production-grade safeguards**. diff --git a/src/db-sql/migration.ts b/src/db-sql/migration.ts index 457b9f79..8d653897 100644 --- a/src/db-sql/migration.ts +++ b/src/db-sql/migration.ts @@ -36,11 +36,15 @@ const MIGRATIONS = [ { name: "001_initial", sql: ` + -- Enable foreign key constraints for data integrity + PRAGMA foreign_keys = ON; + + -- Create accounts table with UNIQUE constraint on did CREATE TABLE IF NOT EXISTS accounts ( id INTEGER PRIMARY KEY AUTOINCREMENT, dateCreated TEXT NOT NULL, derivationPath TEXT, - did TEXT NOT NULL, + did TEXT NOT NULL UNIQUE, -- UNIQUE constraint for foreign key support identityEncrBase64 TEXT, -- encrypted & base64-encoded mnemonicEncrBase64 TEXT, -- encrypted & base64-encoded passkeyCredIdHex TEXT, @@ -102,7 +106,8 @@ const MIGRATIONS = [ profileImageUrl TEXT, publicKeyBase64 TEXT, seesMe BOOLEAN, - registered BOOLEAN + registered BOOLEAN, + iViewContent BOOLEAN DEFAULT TRUE ); CREATE INDEX IF NOT EXISTS idx_contacts_did ON contacts(did); @@ -122,46 +127,22 @@ const MIGRATIONS = [ name TEXT PRIMARY KEY, applied_at TEXT NOT NULL DEFAULT (datetime('now')) ); - `, - }, - { - name: "002_add_iViewContent_to_contacts", - sql: ` - ALTER TABLE contacts ADD COLUMN iViewContent BOOLEAN DEFAULT TRUE; - `, - }, - { - name: "003_active_did_separation", - sql: ` - -- SIMPLIFIED MIGRATION: Create active_identity table and migrate data - -- This migration handles the separation of activeDid from settings - -- using a simpler, more reliable approach that works on all SQLite versions - - -- Create new active_identity table with proper constraints + + -- Create active_identity table with foreign key constraint CREATE TABLE IF NOT EXISTS active_identity ( id INTEGER PRIMARY KEY CHECK (id = 1), - activeDid TEXT NOT NULL DEFAULT '', - lastUpdated TEXT NOT NULL DEFAULT (datetime('now')) + activeDid TEXT DEFAULT NULL, -- NULL instead of empty string + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')), + FOREIGN KEY (activeDid) REFERENCES accounts(did) ON DELETE RESTRICT ); -- Add performance indexes CREATE INDEX IF NOT EXISTS idx_active_identity_activeDid ON active_identity(activeDid); CREATE UNIQUE INDEX IF NOT EXISTS idx_active_identity_single_record ON active_identity(id); - -- Insert default record (will be updated during migration) - INSERT OR IGNORE INTO active_identity (id, activeDid, lastUpdated) VALUES (1, '', datetime('now')); - - -- MIGRATE EXISTING DATA: Copy activeDid from settings to active_identity - -- This prevents data loss when migration runs on existing databases - -- Use a more robust approach that handles missing columns gracefully - UPDATE active_identity - SET activeDid = COALESCE( - (SELECT activeDid FROM settings WHERE id = 1 AND activeDid IS NOT NULL AND activeDid != ''), - '' - ), - lastUpdated = datetime('now') - WHERE id = 1; - `, + -- Seed singleton row + INSERT INTO active_identity (id, activeDid, lastUpdated) VALUES (1, NULL, datetime('now')); + `, }, ]; @@ -184,4 +165,41 @@ export async function runMigrations( logger.info("[Migration] Running migration service"); await runMigrationsService(sqlExec, sqlQuery, extractMigrationNames); logger.info("[Migration] Database migrations completed"); + + // Bootstrapping: Ensure active account is selected after migrations + logger.info("[Migration] Running bootstrapping hooks"); + try { + // Check if we have accounts but no active selection + const accountsResult = await sqlQuery("SELECT COUNT(*) FROM accounts"); + const accountsCount = accountsResult + ? (accountsResult.values?.[0]?.[0] as number) + : 0; + + const activeResult = await sqlQuery( + "SELECT activeDid FROM active_identity WHERE id = 1", + ); + const activeDid = activeResult + ? (activeResult.values?.[0]?.[0] as string) + : null; + + if (accountsCount > 0 && (!activeDid || activeDid === "")) { + logger.info("[Migration] Auto-selecting first account as active"); + const firstAccountResult = await sqlQuery( + "SELECT did FROM accounts ORDER BY dateCreated, did LIMIT 1", + ); + const firstAccountDid = firstAccountResult + ? (firstAccountResult.values?.[0]?.[0] as string) + : null; + + if (firstAccountDid) { + await sqlExec( + "UPDATE active_identity SET activeDid = ?, lastUpdated = datetime('now') WHERE id = 1", + [firstAccountDid], + ); + logger.info(`[Migration] Set active account to: ${firstAccountDid}`); + } + } + } catch (error) { + logger.warn("[Migration] Bootstrapping hook failed (non-critical):", error); + } } diff --git a/src/utils/PlatformServiceMixin.ts b/src/utils/PlatformServiceMixin.ts index 0dd58f48..ca7b5e24 100644 --- a/src/utils/PlatformServiceMixin.ts +++ b/src/utils/PlatformServiceMixin.ts @@ -737,6 +737,87 @@ export const PlatformServiceMixin = { } }, + // ================================================= + // SMART DELETION PATTERN DAL METHODS + // ================================================= + + /** + * Get all account DIDs ordered by creation date + * Required for smart deletion pattern + */ + async $getAllAccountDids(): Promise { + const result = await this.$dbQuery( + "SELECT did FROM accounts ORDER BY dateCreated, did", + ); + return result?.values?.map((row) => row[0] as string) || []; + }, + + /** + * Get account DID by ID + * Required for smart deletion pattern + */ + async $getAccountDidById(id: number): Promise { + const result = await this.$dbQuery( + "SELECT did FROM accounts WHERE id = ?", + [id], + ); + return result?.values?.[0]?.[0] as string; + }, + + /** + * Get active DID (returns null if none selected) + * Required for smart deletion pattern + */ + async $getActiveDid(): Promise { + const result = await this.$dbQuery( + "SELECT activeDid FROM active_identity WHERE id = 1", + ); + return (result?.values?.[0]?.[0] as string) || null; + }, + + /** + * Set active DID (can be null for no selection) + * Required for smart deletion pattern + */ + async $setActiveDid(did: string | null): Promise { + await this.$dbExec( + "UPDATE active_identity SET activeDid = ?, lastUpdated = datetime('now') WHERE id = 1", + [did], + ); + }, + + /** + * Count total accounts + * Required for smart deletion pattern + */ + async $countAccounts(): Promise { + const result = await this.$dbQuery("SELECT COUNT(*) FROM accounts"); + return (result?.values?.[0]?.[0] as number) || 0; + }, + + /** + * Deterministic "next" picker for account selection + * Required for smart deletion pattern + */ + $pickNextAccountDid(all: string[], current?: string): string { + const sorted = [...all].sort(); + if (!current) return sorted[0]; + const i = sorted.indexOf(current); + return sorted[(i + 1) % sorted.length]; + }, + + /** + * Ensure an active account is selected (repair hook) + * Required for smart deletion pattern bootstrapping + */ + async $ensureActiveSelected(): Promise { + const active = await this.$getActiveDid(); + const all = await this.$getAllAccountDids(); + if (active === null && all.length > 0) { + await this.$setActiveDid(this.$pickNextAccountDid(all)); + } + }, + // ================================================= // ULTRA-CONCISE DATABASE METHODS (shortest names) // ================================================= diff --git a/src/views/IdentitySwitcherView.vue b/src/views/IdentitySwitcherView.vue index db9c2509..0e5516e6 100644 --- a/src/views/IdentitySwitcherView.vue +++ b/src/views/IdentitySwitcherView.vue @@ -272,15 +272,48 @@ export default class IdentitySwitcherView extends Vue { this.notify.confirm( NOTIFY_DELETE_IDENTITY_CONFIRM.text, async () => { - await this.$exec(`DELETE FROM accounts WHERE id = ?`, [id]); - this.otherIdentities = this.otherIdentities.filter( - (ident) => ident.id !== id, - ); + await this.smartDeleteAccount(id); }, -1, ); } + /** + * Smart deletion with atomic transaction and last account protection + * Follows the Active Pointer + Smart Deletion Pattern + */ + async smartDeleteAccount(id: string) { + await this.$withTransaction(async () => { + const total = await this.$countAccounts(); + if (total <= 1) { + this.notify.warning( + "Cannot delete the last account. Keep at least one.", + ); + throw new Error("blocked:last-item"); + } + + const accountDid = await this.$getAccountDidById(parseInt(id)); + const activeDid = await this.$getActiveDid(); + + if (activeDid === accountDid) { + const allDids = await this.$getAllAccountDids(); + const nextDid = this.$pickNextAccountDid( + allDids.filter((d) => d !== accountDid), + accountDid, + ); + await this.$setActiveDid(nextDid); + this.notify.success(`Switched active to ${nextDid} before deletion.`); + } + + await this.$exec("DELETE FROM accounts WHERE id = ?", [id]); + }); + + // Update UI + this.otherIdentities = this.otherIdentities.filter( + (ident) => ident.id !== id, + ); + } + notifyCannotDelete() { this.notify.warning( NOTIFY_CANNOT_DELETE_ACTIVE_IDENTITY.message,