diff --git a/packages/shared-schema/src/migrations/lww-replacement-barrier-v2-to-v3.ts b/packages/shared-schema/src/migrations/lww-replacement-barrier-v2-to-v3.ts index 6a4fe6c388..b179dfffd5 100644 --- a/packages/shared-schema/src/migrations/lww-replacement-barrier-v2-to-v3.ts +++ b/packages/shared-schema/src/migrations/lww-replacement-barrier-v2-to-v3.ts @@ -1,7 +1,11 @@ import type { SchemaMigration } from '../migration.types'; +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + /** - * Compatibility barrier for synthetic LWW replacement payloads. + * Compatibility barrier for synthetic LWW replacement payloads, plus an idle + * config backfill. * * Schema v3 introduces replacement-mode LWW envelopes whose omitted fields * are intentionally cleared. NOTE: this stamp does NOT stop released clients. @@ -13,11 +17,45 @@ import type { SchemaMigration } from '../migration.types'; * post-v18.14.0 receivers only, which block any newer-schema op outright. See the * bump policy in schema-version.ts before relying on a version fence. * Historical v2 operations retain their patch semantics when migrated. + * + * State transform: v2 snapshots (v18.14 and earlier) can lack + * `globalConfig.idle.isSuppressIdleDuringFocusMode`, which #8965 added as a + * required field WITHOUT its own schema bump. The v4 state validator requires + * it, and the migration path validates the raw snapshot BEFORE the loadAllData + * reducer can fill its default — so on upgrade the whole snapshot fails + * validation and hydration aborts. Backfill the opt-in default (false) here to + * match DEFAULT_GLOBAL_CONFIG.idle, but only when the field is not already a + * boolean so an existing user choice is never clobbered. A missing `idle` + * section is left to the reducer's own default merge. + * + * The hardcoded `false` is intentionally decoupled from DEFAULT_GLOBAL_CONFIG: + * a migration must freeze the historical default even if the live default later + * changes (and shared-schema cannot import app code). */ export const LwwReplacementBarrierMigration_v2v3: SchemaMigration = { fromVersion: 2, toVersion: 3, - description: 'Gate replacement-mode LWW operations from older clients.', - migrateState: (state: unknown): unknown => state, + description: + 'Gate replacement-mode LWW operations; backfill idle.isSuppressIdleDuringFocusMode default.', + migrateState: (state: unknown): unknown => { + if (!isRecord(state)) { + return state; + } + const globalConfig = state.globalConfig; + if (!isRecord(globalConfig)) { + return state; + } + const idle = globalConfig.idle; + if (!isRecord(idle) || typeof idle.isSuppressIdleDuringFocusMode === 'boolean') { + return state; + } + return { + ...state, + globalConfig: { + ...globalConfig, + idle: { ...idle, isSuppressIdleDuringFocusMode: false }, + }, + }; + }, requiresOperationMigration: false, }; diff --git a/packages/shared-schema/tests/migrations/lww-replacement-barrier-v2-to-v3.spec.ts b/packages/shared-schema/tests/migrations/lww-replacement-barrier-v2-to-v3.spec.ts index 85ec181921..f5b7fc4470 100644 --- a/packages/shared-schema/tests/migrations/lww-replacement-barrier-v2-to-v3.spec.ts +++ b/packages/shared-schema/tests/migrations/lww-replacement-barrier-v2-to-v3.spec.ts @@ -11,7 +11,7 @@ describe('LWW replacement compatibility barrier v2 -> v3', () => { expect(LwwReplacementBarrierMigration_v2v3.toVersion).toBe(3); }); - it('leaves v2 state data unchanged', () => { + it('leaves state without a globalConfig unchanged', () => { const state = { task: { ids: ['task-1'] } }; const result = migrateState(state, 2, 3); @@ -20,6 +20,81 @@ describe('LWW replacement compatibility barrier v2 -> v3', () => { expect(result.data).toBe(state); }); + it('backfills the missing isSuppressIdleDuringFocusMode default (#8965)', () => { + const state = { + task: { ids: [] }, + globalConfig: { + idle: { + isEnableIdleTimeTracking: true, + isOnlyOpenIdleWhenCurrentTask: false, + minIdleTime: 5 * 60 * 1000, + // isSuppressIdleDuringFocusMode absent (v18.14 snapshot shape) + }, + }, + }; + + const result = migrateState(state, 2, 3); + + expect(result.success).toBe(true); + const idle = (result.data as { globalConfig: { idle: Record } }) + .globalConfig.idle; + expect(idle.isSuppressIdleDuringFocusMode).toBe(false); + // Existing fields preserved + expect(idle.isEnableIdleTimeTracking).toBe(true); + expect(idle.minIdleTime).toBe(5 * 60 * 1000); + }); + + it('never clobbers an existing boolean isSuppressIdleDuringFocusMode value', () => { + const state = { + globalConfig: { + idle: { + isEnableIdleTimeTracking: true, + isOnlyOpenIdleWhenCurrentTask: false, + minIdleTime: 5 * 60 * 1000, + isSuppressIdleDuringFocusMode: true, + }, + }, + }; + + const result = migrateState(state, 2, 3); + + expect(result.success).toBe(true); + // Identity when the field is already a boolean — same reference, user choice kept + expect(result.data).toBe(state); + }); + + it('backfills when the field is present but not a boolean (null/undefined)', () => { + // A `null`/`undefined` value would otherwise survive the reducer's + // `{ ...DEFAULT, ...idle }` merge and keep failing validation, so the guard + // must treat non-boolean the same as absent. + const state = { + globalConfig: { + idle: { + isEnableIdleTimeTracking: true, + isOnlyOpenIdleWhenCurrentTask: false, + minIdleTime: 5 * 60 * 1000, + isSuppressIdleDuringFocusMode: null, + }, + }, + }; + + const result = migrateState(state, 2, 3); + + expect(result.success).toBe(true); + const idle = (result.data as { globalConfig: { idle: Record } }) + .globalConfig.idle; + expect(idle.isSuppressIdleDuringFocusMode).toBe(false); + }); + + it('leaves state whose globalConfig has no idle section unchanged', () => { + const state = { globalConfig: { misc: {} } }; + + const result = migrateState(state, 2, 3); + + expect(result.success).toBe(true); + expect(result.data).toBe(state); + }); + it('preserves historical v2 operation semantics while stamping schema v3', () => { const operation: OperationLike = { id: 'legacy-lww', diff --git a/src/app/op-log/persistence/operation-log-snapshot.service.spec.ts b/src/app/op-log/persistence/operation-log-snapshot.service.spec.ts index de40323c99..fb3b5d8d65 100644 --- a/src/app/op-log/persistence/operation-log-snapshot.service.spec.ts +++ b/src/app/op-log/persistence/operation-log-snapshot.service.spec.ts @@ -703,7 +703,12 @@ describe('OperationLogSnapshotService', () => { ); }); - it('should restore backup and not save when migrated state fails validation', async () => { + it('should restore backup, not save, but still return the migrated snapshot when state fails validation', async () => { + // Non-fatal path: a validation failure here is often reducer-healable (e.g. + // an older snapshot missing a newly-required config default). Bricking to + // recovery would empty the store on upgrade, so we roll the on-disk cache + // back to the backup, never persist the unvalidated snapshot, yet return it + // for reducer-healed hydration (Checkpoint C re-checks and persists). const snapshot = createSnapshot(); const migratedSnapshot = { ...snapshot, schemaVersion: CURRENT_SCHEMA_VERSION }; mockOpLogStore.saveStateCacheBackup.and.resolveTo(undefined); @@ -714,10 +719,9 @@ describe('OperationLogSnapshotService', () => { }); mockOpLogStore.restoreStateCacheFromBackup.and.resolveTo(undefined); - await expectAsync( - service.migrateSnapshotWithBackup(snapshot), - ).toBeRejectedWithError(/Migrated snapshot validation failed/); + const result = await service.migrateSnapshotWithBackup(snapshot); + expect(result).toBe(migratedSnapshot); expect(mockOpLogStore.saveStateCache).not.toHaveBeenCalled(); expect(mockOpLogStore.restoreStateCacheFromBackup).toHaveBeenCalled(); expect(mockOpLogStore.clearStateCacheBackup).not.toHaveBeenCalled(); diff --git a/src/app/op-log/persistence/operation-log-snapshot.service.ts b/src/app/op-log/persistence/operation-log-snapshot.service.ts index 4e847f5767..345ce55ce1 100644 --- a/src/app/op-log/persistence/operation-log-snapshot.service.ts +++ b/src/app/op-log/persistence/operation-log-snapshot.service.ts @@ -175,9 +175,14 @@ export class OperationLogSnapshotService { * Migrates a snapshot with backup safety (A.7.12). * Creates a backup before migration and restores it if migration fails. * + * State-validation failures are non-fatal: the migrated snapshot is returned + * for hydration (unpersisted) rather than throwing. See the step-4 note. + * * @param snapshot - The snapshot to migrate * @returns The migrated snapshot - * @throws If migration fails and rollback also fails + * @throws On migration failure or metadata-validation failure. The backup is + * rolled back first; if that rollback ALSO fails, a combined error is + * thrown instead. State-validation failure does NOT throw (step 4). */ async migrateSnapshotWithBackup(snapshot: StateCache): Promise { OpLog.normal( @@ -197,16 +202,47 @@ export class OperationLogSnapshotService { throw new Error('Migrated snapshot metadata validation failed'); } - // 4. Validate migrated snapshot state before persisting or clearing the backup. - // Otherwise an invalid current-schema cache could be trusted on next startup. + // 4. Validate migrated snapshot state before PERSISTING it — an invalid + // current-schema cache must not be trusted on next startup (Checkpoint B + // skips validation for a matching schema version). But a validation + // failure here is NOT fatal to hydration: many "invalid" shapes are + // reducer-healable — e.g. a snapshot from an older release missing a + // newly-required config field the loadAllData reducer backfills by + // default. Bailing to disaster recovery in that case bricks the app to an + // empty store on upgrade, even though the data is fine. So on failure we + // roll the on-disk cache back to the pre-migration backup (never persist + // the unvalidated migrated snapshot) yet still RETURN it, letting the + // hydrator load it through the reducer (which heals defaults) and boot + // with correct data. + // + // Caveat: this non-fatal path does NOT itself persist a clean snapshot. + // Re-persist only happens if the hydrator's post-replay Checkpoint C runs + // and its save gate is met (tail ops present, not a full-state-op load, + // >10 replayed ops) — otherwise the on-disk cache stays at the old version + // and the migration re-runs every boot (correct, but a startup tax). For + // fields with a migration backfill (below) validation passes and step 5 + // persists a clean snapshot immediately, so that path is the real fix; + // this branch is the safety net for the not-yet-backfilled case. + // TODO(followup): persist a fresh snapshot after a migration ran whenever + // Checkpoint C validates, regardless of tail-op count, to converge in one + // boot instead of every boot. const validationResult = await this.validateStateService.validateState( migratedSnapshot.state as Record, ); if (!validationResult.isValid) { - throw new Error( - `Migrated snapshot validation failed (${validationResult.typiaErrors.length} typia errors` + - `${validationResult.crossModelError ? `, cross-model: ${validationResult.crossModelError}` : ''})`, + OpLog.warn( + 'OperationLogSnapshotService: Migrated snapshot failed validation — ' + + 'restoring backup and hydrating unpersisted so the reducer can heal it.', + { + typiaErrorCount: validationResult.typiaErrors.length, + crossModelError: validationResult.crossModelError, + }, ); + await this.opLogStore.restoreStateCacheFromBackup(); + OpLog.normal( + 'OperationLogSnapshotService: Backup restored; returning migrated snapshot for reducer-healed hydration.', + ); + return migratedSnapshot; } // 5. Save migrated snapshot