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<ActiveIdentity> 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
2 weeks ago · d2e04fe2a0
2 changed files with 97 additions and 127 deletions
--- 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<Settings>
 }
 ```

-### 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<MigrationResult> {
-  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<boolean> {

 ## 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<boolean> {
 ## 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
--- 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<ActiveIdentity> {
+    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: "" };
      }
    },