From d2e04fe2a07a7a9239158bbdd52cf9b534024218 Mon Sep 17 00:00:00 2001 From: Matthew Raymer Date: Sun, 31 Aug 2025 03:48:46 +0000 Subject: [PATCH] feat(api)!: fix $getActiveIdentity return type for ActiveDid migration Update $getActiveIdentity() method to return { activeDid: string } instead of full ActiveIdentity object. Add validation to ensure activeDid exists in accounts table and clear corrupted values. Update migration plan to reflect completed first step of API layer implementation. - Change return type from Promise to Promise<{ activeDid: string }> - Add account validation with automatic corruption cleanup - Simplify query to only select activeDid field - Improve error handling to return empty string instead of throwing - Update migration plan documentation with current status --- doc/activeDid-migration-plan.md | 184 ++++++++++++------------------ src/utils/PlatformServiceMixin.ts | 40 ++++--- 2 files changed, 97 insertions(+), 127 deletions(-) diff --git a/doc/activeDid-migration-plan.md b/doc/activeDid-migration-plan.md index c6caf476..9dd76bd6 100644 --- a/doc/activeDid-migration-plan.md +++ b/doc/activeDid-migration-plan.md @@ -1,8 +1,8 @@ # ActiveDid Migration Plan - Implementation Guide **Author**: Matthew Raymer -**Date**: 2025-08-29T08:03Z -**Status**: 🎯 **IMPLEMENTATION** - Ready for development +**Date**: 2025-08-31T03:34Z +**Status**: 🎯 **IMPLEMENTATION** - API Layer Incomplete ## Objective @@ -26,30 +26,34 @@ Follow this implementation checklist step-by-step to complete the migration. ### Phase 1: Database Migration ✅ READY - [x] Add migration to MIGRATIONS array in `src/db-sql/migration.ts` - [x] Create active_identity table with constraints +- [x] Include data migration from settings to active_identity table -### Phase 2: API Layer Updates ✅ COMPLETE -- [ ] Implement `$getActiveIdentity()` with validation +### Phase 2: API Layer Updates ❌ INCOMPLETE +- [x] Implement `$getActiveIdentity()` method (exists but wrong return type) +- [x] Fix `$getActiveIdentity()` return type to match documented interface - [ ] Update `$accountSettings()` to use new method - [ ] Update `$updateActiveDid()` with dual-write pattern -**Status**: Successfully implemented dual-write pattern with validation. Ready for testing. +**Status**: Return type fixed. Method now returns `{ activeDid: string }` as documented. Need to update other API methods. -### Phase 3: Component Updates ❌ +### Phase 3: Component Updates ❌ BLOCKED - [ ] Update 35+ components to use `$getActiveIdentity()` - [ ] Replace `this.activeDid = settings.activeDid` pattern - [ ] Test each component individually -### Phase 4: Testing ❌ +**Status**: Blocked until API layer is complete. 35 components identified via grep search. + +### Phase 4: Testing ❌ NOT STARTED - [ ] Test all platforms (Web, Electron, iOS, Android) - [ ] Test migration rollback scenarios - [ ] Test data corruption recovery ## Required Code Changes -### 1. Database Migration +### 1. Database Migration ✅ COMPLETE ```typescript -// Add to MIGRATIONS array in src/db-sql/migration.ts +// Already added to MIGRATIONS array in src/db-sql/migration.ts { name: "003_active_did_separate_table", sql: ` @@ -67,14 +71,22 @@ Follow this implementation checklist step-by-step to complete the migration. -- 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 + UPDATE active_identity + SET activeDid = (SELECT activeDid FROM settings WHERE id = 1), + lastUpdated = datetime('now') + WHERE id = 1 + AND EXISTS (SELECT 1 FROM settings WHERE id = 1 AND activeDid IS NOT NULL AND activeDid != ''); `, }, ``` -### 2. Missing API Method Implementation +### 2. Fix $getActiveIdentity() Return Type ```typescript -// Add to PlatformServiceMixin.ts +// Update in PlatformServiceMixin.ts - Change return type to match documented interface async $getActiveIdentity(): Promise<{ activeDid: string }> { try { const result = await this.$dbQuery( @@ -111,7 +123,7 @@ async $getActiveIdentity(): Promise<{ activeDid: string }> { } ``` -### 3. Updated $accountSettings Method +### 3. Update $accountSettings Method ```typescript // Update in PlatformServiceMixin.ts @@ -136,7 +148,7 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise } ``` -### 4. Updated $updateActiveDid Method +### 4. Update $updateActiveDid Method ```typescript // Update in PlatformServiceMixin.ts @@ -287,105 +299,54 @@ private async initializeSettings() { } ``` -### 6. Data Migration Function - -```typescript -// Add to migration.ts -async function migrateActiveDidToSeparateTable(): Promise { - const result: MigrationResult = { - success: false, - errors: [], - warnings: [], - dataMigrated: 0 - }; - - try { - // 1. Get current activeDid from settings (legacy approach) - const currentSettings = await retrieveSettingsForDefaultAccount(); - const activeDid = currentSettings.activeDid; - - if (!activeDid) { - result.warnings.push("No activeDid found in current settings"); - return result; - } - - // 2. Validate activeDid exists in accounts table - const accountExists = await dbQuery( - "SELECT did FROM accounts WHERE did = ?", - [activeDid] - ); - - if (!accountExists?.values?.length) { - result.errors.push(`ActiveDid ${activeDid} not found in accounts table - data corruption detected`); - return result; - } - - // 3. Update active_identity table (new system) - await dbExec( - "UPDATE active_identity SET activeDid = ?, lastUpdated = datetime('now') WHERE id = 1", - [activeDid] - ); - - // 4. Ensure legacy settings.activeDid stays in sync (backward compatibility) - await dbExec( - "UPDATE settings SET activeDid = ? WHERE id = ?", - [activeDid, MASTER_SETTINGS_KEY] - ); - - result.dataMigrated = 1; - result.warnings.push(`Successfully migrated activeDid: ${activeDid}`); - - } catch (error) { - result.errors.push(`Migration failed: ${error}`); - logger.error("[ActiveDid Migration] Critical error during migration:", error); - } - - return result; -} -``` - ## What Works (Evidence) - ✅ **Migration code exists** in MIGRATIONS array - - **Time**: 2025-08-29T08:03Z + - **Time**: 2025-08-31T03:34Z - **Evidence**: `src/db-sql/migration.ts:125` - `003_active_did_separate_table` migration defined - - **Verify at**: Migration script contains proper table creation and constraints + - **Verify at**: Migration script contains proper table creation and data migration -- ✅ **Current activeDid storage** in settings table works - - **Time**: 2025-08-29T08:03Z - - **Evidence**: `src/db-sql/migration.ts:67` - activeDid field exists in initial migration - - **Verify at**: Current database schema and Settings type definition +- ✅ **$getActiveIdentity() method exists** in PlatformServiceMixin + - **Time**: 2025-08-31T03:34Z + - **Evidence**: `src/utils/PlatformServiceMixin.ts:555` - Method implemented with correct return type + - **Verify at**: Method returns `{ activeDid: string }` as documented -- ✅ **PlatformServiceMixin integration** with activeDid (reverted to working state) - - **Time**: 2025-08-29T08:03Z - - **Evidence**: `src/utils/PlatformServiceMixin.ts:108` - activeDid tracking restored after test failures - - **Verify at**: Component usage across all platforms works again after reversion - -- ✅ **Database migration infrastructure** exists and is mature - - **Time**: 2025-08-29T08:03Z +- ✅ **Database migration infrastructure** exists and mature + - **Time**: 2025-08-31T03:34Z - **Evidence**: `src/db-sql/migration.ts:31` - migration system in place - **Verify at**: Existing migration scripts and database versioning ## What Doesn't (Evidence & Hypotheses) -- ❌ **Database state unknown** - IndexedDB database not inspected - - **Time**: 2025-08-29T08:03Z - - **Evidence**: Tests failed before application could start and initialize IndexedDB database - - **Hypothesis**: Database may or may not exist in IndexedDB, migration may or may not have run - - **Next probe**: Start application in browser and inspect IndexedDB storage via DevTools +- ✅ **$getActiveIdentity() return type fixed** - now returns `{ activeDid: string }` as documented + - **Time**: 2025-08-31T03:34Z + - **Evidence**: `src/utils/PlatformServiceMixin.ts:555` - Method updated with correct return type + - **Status**: Ready for use in component updates + +- ❌ **$accountSettings() not updated** to use new $getActiveIdentity() method + - **Time**: 2025-08-31T03:34Z + - **Evidence**: `src/utils/PlatformServiceMixin.ts:817` - Method still uses legacy pattern + - **Hypothesis**: Components will continue using old activeDid from settings + - **Next probe**: Update $accountSettings to call $getActiveIdentity and combine results + +- ❌ **$updateActiveDid() not implemented** with dual-write pattern + - **Time**: 2025-08-31T03:34Z + - **Evidence**: `src/utils/PlatformServiceMixin.ts:200` - Method only updates internal tracking + - **Hypothesis**: Database updates not happening, only in-memory changes + - **Next probe**: Implement dual-write to both active_identity and settings tables + +- ❌ **35 components still use old pattern** `this.activeDid = settings.activeDid` + - **Time**: 2025-08-31T03:34Z + - **Evidence**: Grep search found 35 instances across views and components + - **Hypothesis**: Components need updates but are blocked until API layer is ready + - **Next probe**: Update components after API layer is implemented -- ❌ **active_identity table status unknown** in IndexedDB database - - **Time**: 2025-08-29T08:03Z +- ❌ **Database state unknown** - IndexedDB database not inspected + - **Time**: 2025-08-31T03:34Z - **Evidence**: Cannot check IndexedDB database without running application - **Hypothesis**: Migration exists in code but may not have run in IndexedDB - **Next probe**: Start application and check IndexedDB for active_identity table -- ❌ **35+ components still use old pattern** `this.activeDid = settings.activeDid` - - **Time**: 2025-08-29T08:03Z - - **Evidence**: Grep search found 35+ instances across views and components - - **Hypothesis**: Components need updates but are blocked until API layer is ready - - **Next probe**: Update components after API layer is implemented - ## Risks, Limits, Assumptions - **Data Loss Risk**: Migration failure could lose activeDid values @@ -431,18 +392,17 @@ async function rollbackActiveDidMigration(): Promise { ## Next Steps -| Owner | Task | Exit Criteria | Target Date (UTC) | Priority | -|-------|------|---------------|-------------------|----------| -| Development Team | **CRITICAL**: Start application in browser | Application loads and initializes IndexedDB | 2025-08-29 | 🔴 HIGH | -| Development Team | Inspect IndexedDB via DevTools | Verify database exists and check for active_identity table | 2025-08-29 | 🔴 HIGH | -| Development Team | Implement $getActiveIdentity() method | Method added to PlatformServiceMixin | 2025-08-30 | 🟡 MEDIUM | -| Development Team | Update $accountSettings method | Method updated and tested | 2025-08-30 | 🟡 MEDIUM | -| Development Team | Update $updateActiveDid method | Method updated with dual-write pattern | 2025-08-30 | 🟡 MEDIUM | -| Development Team | Update 35+ components | All components use new API | 2025-08-31 | 🟢 LOW | -| QA Team | Platform testing | All platforms tested and verified | 2025-09-01 | 🟢 LOW | -| Development Team | Deploy migration | Production deployment successful | 2025-09-02 | 🟢 LOW | +| Task | Exit Criteria | Priority | +|------|---------------|----------| +| **Fix $getActiveIdentity() return type** | Method returns `{ activeDid: string }` as documented | ✅ COMPLETE | +| **Update $accountSettings() method** | Method calls $getActiveIdentity and combines with settings | 🔴 HIGH | +| **Implement $updateActiveDid() dual-write** | Method updates both active_identity and settings tables | 🔴 HIGH | +| **Start application in browser** | Application loads and initializes IndexedDB database | 🟡 MEDIUM | +| **Inspect IndexedDB via DevTools** | Verify active_identity table exists and contains data | 🟡 MEDIUM | +| **Update first component** | One component successfully uses new API pattern | 🟢 LOW | +| **Systematic component updates** | All 35 components use new API pattern | 🟢 LOW | -**Critical Blocker**: Need to start application in browser to check if IndexedDB database exists and migration has run. +**Critical Blocker**: Need to update $accountSettings() and $updateActiveDid() methods before component updates can proceed. ## Future Improvement: MASTER_SETTINGS_KEY Elimination @@ -465,16 +425,18 @@ async function rollbackActiveDidMigration(): Promise { ## Competence Hooks - *Why this works*: Separates concerns between identity selection and user preferences, prevents data corruption with foreign key constraints -- *Common pitfalls*: Forgetting to update all 35+ components, not implementing $getActiveIdentity() method, missing data validation during migration -- *Next skill unlock*: Systematic component updates with grep search and testing -- *Teach-back*: Explain why all components need updates and how to systematically find and replace the pattern +- *Common pitfalls*: Method signature mismatches, forgetting dual-write pattern, not testing database state +- *Next skill unlock*: Systematic API updates with backward compatibility +- *Teach-back*: Explain why dual-write pattern is needed during migration transition ## Collaboration Hooks - **Reviewers**: Database team, PlatformServiceMixin maintainers, QA team - **Sign-off checklist**: - [ ] Migration script integrated with existing MIGRATIONS array - - [ ] $getActiveIdentity() method implemented and tested + - [x] $getActiveIdentity() method returns correct type + - [ ] $accountSettings() method updated to use new API + - [ ] $updateActiveDid() method implements dual-write pattern - [ ] All 35+ components updated to use new API - [ ] Rollback procedures validated - [ ] All platforms tested diff --git a/src/utils/PlatformServiceMixin.ts b/src/utils/PlatformServiceMixin.ts index 9d98c085..071eae35 100644 --- a/src/utils/PlatformServiceMixin.ts +++ b/src/utils/PlatformServiceMixin.ts @@ -553,33 +553,41 @@ export const PlatformServiceMixin = { * Get active identity from the new active_identity table * This replaces the activeDid field in settings for better architecture */ - async $getActiveIdentity(): Promise { + async $getActiveIdentity(): Promise<{ activeDid: string }> { try { const result = await this.$dbQuery( - "SELECT id, activeDid, lastUpdated FROM active_identity WHERE id = 1", + "SELECT activeDid FROM active_identity WHERE id = 1", ); if (result?.values?.length) { - const [id, activeDid, lastUpdated] = result.values[0]; - return { - id: id as number, - activeDid: activeDid as string, - lastUpdated: lastUpdated as string, - }; + const activeDid = result.values[0][0] as string; + + // Validate activeDid exists in accounts + if (activeDid) { + const accountExists = await this.$dbQuery( + "SELECT did FROM accounts WHERE did = ?", + [activeDid] + ); + + if (accountExists?.values?.length) { + return { activeDid }; + } else { + // Clear corrupted activeDid + await this.$dbExec( + "UPDATE active_identity SET activeDid = '', lastUpdated = datetime('now') WHERE id = 1" + ); + return { activeDid: "" }; + } + } } - - // Return default if no record exists - return { - id: 1, - activeDid: "", - lastUpdated: new Date().toISOString(), - }; + + return { activeDid: "" }; } catch (error) { logger.error( "[PlatformServiceMixin] Error getting active identity:", error, ); - throw error; + return { activeDid: "" }; } },