diff --git a/doc/active-identity-upgrade-plan.md b/doc/active-identity-upgrade-plan.md new file mode 100644 index 00000000..1a5d9fab --- /dev/null +++ b/doc/active-identity-upgrade-plan.md @@ -0,0 +1,510 @@ +# Active Identity Migration & Upgrade (Merged) + +## Part 1: Database Migration Impact Analysis + +# Database Migration Impact Analysis + +## Current State Analysis + +### Existing Migration Structure +The current migration system has: +- **Migration 003**: `003_active_identity_and_seed_backup` - Creates `active_identity` table with basic structure +- **Foreign Key**: `ON DELETE SET NULL` constraint +- **Basic Indexing**: Single unique index on `id` +- **Bootstrapping**: Auto-selects first account if `activeDid` is null/empty + +### Current Schema (Migration 003) +```sql +CREATE TABLE IF NOT EXISTS active_identity ( + id INTEGER PRIMARY KEY CHECK (id = 1), + activeDid TEXT DEFAULT NULL, + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')), + FOREIGN KEY (activeDid) REFERENCES accounts(did) ON DELETE SET NULL +); +``` + +## Proposed Changes Impact + +### 1. Foreign Key Constraint Change +**Current**: `ON DELETE SET NULL` +**Proposed**: `ON DELETE RESTRICT` + +**Impact**: +- **Data Safety**: Prevents accidental deletion of active account +- **Migration Rewrite**: Update existing migration 003 + +### 2. Automatic Timestamp Updates +**Current**: Manual `lastUpdated` updates in code +**Proposed**: Database trigger for automatic updates + +**Impact**: +- **Code Simplification**: Remove manual timestamp updates from `PlatformServiceMixin` +- **Consistency**: Ensures `lastUpdated` is always current + +### 3. Enhanced Indexing +**Current**: Single unique index on `id` +**Proposed**: Additional index on `accounts(dateCreated, did)` + +**Impact**: +- **Performance Improvement**: Faster account selection queries +- **Minimal Risk**: Additive change only + +## Implementation Strategy + +### Rewrite Migration 003 +Since the `active_identity` table is new and we're in active development, we can simply rewrite the existing migration: + +```sql +{ + name: "003_active_identity_and_seed_backup", + sql: ` + -- Enable foreign key constraints for data integrity + PRAGMA foreign_keys = ON; + + -- Add UNIQUE constraint to accounts.did for foreign key support + CREATE UNIQUE INDEX IF NOT EXISTS idx_accounts_did_unique ON accounts(did); + + -- Create active_identity table with enhanced constraints + CREATE TABLE IF NOT EXISTS active_identity ( + id INTEGER PRIMARY KEY CHECK (id = 1), + activeDid TEXT REFERENCES accounts(did) ON DELETE RESTRICT, + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')) + ); + + -- Add performance indexes + CREATE UNIQUE INDEX IF NOT EXISTS idx_active_identity_single_record ON active_identity(id); + CREATE INDEX IF NOT EXISTS idx_accounts_pick ON accounts(dateCreated, did); + + -- Seed singleton row (only if not already exists) + INSERT INTO active_identity (id, activeDid, lastUpdated) + SELECT 1, NULL, datetime('now') + WHERE NOT EXISTS (SELECT 1 FROM active_identity WHERE id = 1); + + -- Add hasBackedUpSeed field to settings + ALTER TABLE settings ADD COLUMN hasBackedUpSeed BOOLEAN DEFAULT FALSE; + + -- Create automatic timestamp trigger + CREATE TRIGGER IF NOT EXISTS trg_active_identity_touch + AFTER UPDATE ON active_identity + FOR EACH ROW + BEGIN + UPDATE active_identity + SET lastUpdated = datetime('now') + WHERE id = 1; + END; + ` +} +``` + + +## Migration Service Updates Required + +### Enhanced Validation Logic +**File**: `src/services/migrationService.ts` + +The existing validation for migration 003 needs to be enhanced to check for: +- **Trigger existence**: `trg_active_identity_touch` +- **Performance index**: `idx_accounts_pick` +- **Foreign key constraint**: `ON DELETE RESTRICT` + +### Enhanced Schema Detection +**File**: `src/services/migrationService.ts` + +The existing schema detection for migration 003 needs to be enhanced to verify: +- **Table structure**: Enhanced constraints +- **Trigger presence**: Automatic timestamp updates +- **Index presence**: Performance optimization + +## Risk Assessment + +### Low Risk Changes +- **Performance Index**: Additive only, no data changes +- **Trigger Creation**: Additive only, improves consistency +- **Migration Rewrite**: Clean implementation, no additional migrations + +### Medium Risk Changes +- **Foreign Key Change**: `ON DELETE RESTRICT` is more restrictive +- **Validation Updates**: Need to test enhanced validation logic + +### Mitigation Strategies +1. **Comprehensive Testing**: Test migration on various database states +2. **Development Phase**: Since we're in active development, risk is minimal +3. **Clean Implementation**: Single migration with all enhancements +4. **Validation Coverage**: Enhanced validation ensures correctness + +## Implementation Timeline + +### Phase 1: Migration Rewrite +- [ ] Update migration 003 with enhanced constraints +- [ ] Add enhanced validation logic +- [ ] Add enhanced schema detection logic +- [ ] Test migration on clean database + +### Phase 2: Testing +- [ ] Test migration on existing databases +- [ ] Validate foreign key constraints work correctly +- [ ] Test trigger functionality +- [ ] Test performance improvements + +### Phase 3: Deployment +- [ ] Deploy enhanced migration to development +- [ ] Monitor migration success rates +- [ ] Deploy to production +- [ ] Monitor for any issues + +## Code Changes Required + +### Files to Modify +1. **`src/db-sql/migration.ts`** - Update migration 003 with enhanced constraints +2. **`src/services/migrationService.ts`** - Add enhanced validation and detection logic +3. **`src/utils/PlatformServiceMixin.ts`** - Remove manual timestamp updates + +### Estimated Impact +- **Migration File**: ~20 lines modified (enhanced constraints) +- **Migration Service**: ~50 lines added (enhanced validation) +- **PlatformServiceMixin**: ~20 lines removed (manual timestamps) +- **Total**: ~50 lines changed + +## Conclusion + +The proposed database changes are **feasible and beneficial** with a much simpler implementation: + +**Benefits**: +- Improved data integrity with `ON DELETE RESTRICT` +- Automatic timestamp consistency +- Better query performance +- Cleaner code (no manual timestamp updates) +- **Simpler implementation** - just rewrite migration 003 + +**Risks**: +- More restrictive foreign key constraints +- Enhanced validation logic needs testing + +**Recommendation**: Proceed with **rewriting migration 003** as it provides the cleanest path forward with minimal complexity and maximum benefit. + + +--- + +## Part 2: Active Identity System Upgrade Plan + +# Active Identity System Upgrade Plan + +## Overview + +This document outlines a comprehensive upgrade to the `$getActiveIdentity` system +to address current ambiguities, race conditions, and improve overall robustness. + +## Current Issues Identified + +### 1. **Ambiguous Return States** + +- Current: Returns `{ activeDid: string }` where empty string conflates multiple states +- Problem: Cannot distinguish between "no accounts", "migration gap", "corruption cleared" +- Impact: UI cannot provide appropriate user guidance + +### 2. **Race Conditions (TOCTOU)** + +- Current: Read → Write → Re-read across multiple statements +- Problem: No transactional guard, potential for inconsistent state +- Impact: Rare but possible data corruption scenarios + +### 3. **No Single-Row Enforcement** + +- Current: App logic assumes `id=1` singleton, but DB doesn't enforce +- Problem: Multiple rows could theoretically exist +- Impact: Unpredictable behavior + +### 4. **Deletion Hazards** + +- Current: No protection against deleting active account +- Problem: App "discovers" corruption after the fact +- Impact: Poor user experience, potential data loss + +### 5. **Inconsistent Updates** + +- Current: `lastUpdated` only set on some code paths +- Problem: Audit trail incomplete +- Impact: Debugging difficulties + +## Proposed Solution Architecture + +### Database Schema Changes + +```sql +-- Enhanced active_identity table with constraints +CREATE TABLE IF NOT EXISTS active_identity ( + id INTEGER PRIMARY KEY CHECK (id = 1), + activeDid TEXT REFERENCES accounts(did) ON DELETE RESTRICT, + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')) +); + +-- Automatic timestamp updates +CREATE TRIGGER IF NOT EXISTS trg_active_identity_touch +AFTER UPDATE ON active_identity +FOR EACH ROW +BEGIN + UPDATE active_identity + SET lastUpdated = datetime('now') + WHERE id = 1; +END; + +-- Deterministic account selection index +CREATE INDEX IF NOT EXISTS idx_accounts_pick + ON accounts(dateCreated, did); + +-- Seed singleton row +INSERT INTO active_identity (id, activeDid) +SELECT 1, NULL +WHERE NOT EXISTS (SELECT 1 FROM active_identity WHERE id = 1); +``` + +### Type System Enhancement + +```typescript +type ActiveIdentity = + | { state: 'ok'; activeDid: string } + | { state: 'unset'; activeDid: '' } // row exists, but no active set yet + | { state: 'auto-selected'; activeDid: string } // we selected & saved first account + | { state: 'empty-accounts'; activeDid: '' } // there are no accounts to select + | { state: 'cleared-corrupt'; activeDid: '' } // DB had a non-existent DID; cleared + | { state: 'error'; activeDid: '' }; +``` + +## Migration Strategy + +### Phase 1: Database Schema Migration + +**Files to modify:** + +- `src/db-sql/migration.ts` - Add new migration +- `src/services/migrationService.ts` - Update validation logic + +**Migration Steps:** + +1. Add foreign key constraint to `active_identity.activeDid` +2. Add `ON DELETE RESTRICT` constraint +3. Create timestamp trigger +4. Add deterministic index +5. Ensure singleton row exists + +**Risk Assessment:** LOW +- Additive changes only +- Existing data preserved +- Rollback possible + +### Phase 2: Core Method Refactor + +**Files to modify:** + +- `src/utils/PlatformServiceMixin.ts` - Replace `$getActiveIdentity` +- `src/interfaces/` - Add new type definitions + +**Changes:** + +1. Implement transactional `$getActiveIdentity` with explicit states +2. Add `$setActiveIdentity` method +3. Add `$deleteAccountSafely` method +4. Update return type to `ActiveIdentity` + +**Risk Assessment:** MEDIUM + +- Breaking change to return type +- Requires coordinated updates across codebase +- Extensive testing needed + +### Phase 3: Consumer Updates + +**Files to modify:** + +- All Vue components using `$getActiveIdentity` +- Router guards +- Service layer components + +**Changes:** + +1. Update all callers to handle new return type +2. Implement state-specific UI logic +3. Add appropriate user messaging + +**Risk Assessment:** HIGH + +- Wide impact across codebase +- UI/UX changes required +- Extensive testing needed + +## Implementation Phases + +### Phase 1: Database Foundation + +- [ ] Create migration for schema changes +- [ ] Test migration on development databases +- [ ] Validate foreign key constraints work correctly +- [ ] Test rollback scenarios + +### Phase 2: Core Implementation + +- [ ] Implement new `$getActiveIdentity` method +- [ ] Add `$setActiveIdentity` method +- [ ] Add `$deleteAccountSafely` method +- [ ] Create comprehensive unit tests +- [ ] Test transactional behavior + +### Phase 3: Consumer Updates + +- [ ] Audit all current usage of `$getActiveIdentity` +- [ ] Create migration guide for components +- [ ] Update critical path components first +- [ ] Implement state-specific UI logic + +### Phase 4: Testing & Rollout + +- [ ] End-to-end testing +- [ ] Performance testing +- [ ] User acceptance testing +- [ ] Gradual rollout with feature flags + +## Backward Compatibility Strategy + +### Gradual Migration Approach + +The gradual migration strategy allows us to implement the new active identity system without breaking existing functionality. This approach minimizes risk and provides a clear path for incremental updates. + +#### Phase 1: Dual Method Implementation + +- **Keep existing method**: Leave current `$getActiveIdentity` unchanged (no renaming) +- **Add new method**: Implement `$getActiveIdentityV2` with new return type and logic +- **Feature flag control**: Use `FEATURE_FLAG_ACTIVE_IDENTITY_V2` to control which method is used +- **Default to existing**: Start with existing method enabled to ensure no breaking changes + +#### Phase 2: Component Migration + +- **Audit usage**: Identify all components using `$getActiveIdentity` +- **Create migration guide**: Document the changes needed for each component type +- **Update incrementally**: Migrate components to use `$getActiveIdentityV2` one +by one, starting with non-critical paths +- **Test each migration**: Validate each component works with the new method +before proceeding + +#### Phase 3: Feature Flag Rollout + +- **Enable for migrated components**: Switch feature flag for components that have been updated +- **Monitor performance**: Track any performance or behavior changes +- **Gradual enablement**: Enable the new method for more components as they're migrated +- **Fallback capability**: Keep legacy method available for rollback if issues arise + +#### Phase 4: Legacy Cleanup + +- **Remove feature flag**: Once all components are migrated, remove the feature flag +- **Replace existing method**: Replace the implementation of `$getActiveIdentity` with the V2 logic +- **Remove V2 method**: Delete `$getActiveIdentityV2` and related code +- **Update documentation**: Remove references to the V2 method +- **Final validation**: Ensure no remaining references to V2 method + +#### Benefits of This Approach + +- **Zero downtime**: No breaking changes during migration +- **Incremental testing**: Each component can be tested individually +- **Easy rollback**: Can revert individual components or entire system if needed +- **Clear progress tracking**: Easy to see which components still need migration +- **Reduced risk**: Issues are isolated to individual components rather than system-wide + +## Risk Mitigation + +### Database Risks + +- **Foreign Key Violations**: Test with existing data +- **Migration Failures**: Comprehensive rollback plan +- **Performance Impact**: Monitor query performance + +### Code Risks + +- **Breaking Changes**: Extensive testing, gradual rollout +- **Race Conditions**: Comprehensive unit tests +- **State Management**: Clear documentation, examples + +### User Experience Risks + +- **Confusing States**: Clear UI messaging +- **Data Loss**: Backup strategies, validation +- **Performance**: Monitor response times + +## Success Metrics + +### Technical Metrics + +- Zero race condition incidents +- 100% test coverage for new methods +- <50ms average response time +- Zero data corruption incidents + +### User Experience Metrics + +- Clear state-specific messaging +- Reduced user confusion +- Improved onboarding flow +- Faster account switching + +## Rollback Plan + +### Database Rollback + +1. Remove foreign key constraints +2. Remove triggers +3. Remove indexes +4. Restore previous migration state + +### Code Rollback + +1. Revert to previous `$getActiveIdentity` implementation +2. Remove new type definitions +3. Restore previous component logic +4. Remove new methods + +## Testing Strategy + +### Unit Tests + +- Test all state transitions +- Test transactional behavior +- Test error conditions +- Test edge cases + +### Integration Tests + +- Test with real database +- Test migration scenarios +- Test concurrent access +- Test foreign key constraints + +### End-to-End Tests + +- Test complete user flows +- Test account creation/deletion +- Test active identity switching +- Test error recovery + +## Documentation Requirements + +### Developer Documentation + +- Migration guide for components +- API documentation for new methods +- Type definitions and examples +- Best practices guide + +### User Documentation + +- State-specific UI messaging +- Error message guidelines +- Onboarding flow updates +- Troubleshooting guide + +## Conclusion + +This upgrade addresses critical architectural issues while providing a clear path +forward. The phased approach minimizes risk while ensuring thorough testing and +validation at each step. + +**Recommendation:** Proceed with Phase 1 (Database Schema) immediately, as it +provides immediate benefits with minimal risk. The gradual migration approach allows +for incremental updates without breaking existing functionality.