From bfdc63e0c65e8562c101a00050b282390fedbddd Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 10 Jul 2026 22:50:33 +0200 Subject: [PATCH] fix(sync): preserve actionable recovery state --- .../imex/sync/sync-wrapper.service.spec.ts | 13 +++ src/app/imex/sync/sync-wrapper.service.ts | 12 ++- .../operation-log-store.service.spec.ts | 91 +++++++++++++++---- .../operation-log-store.service.ts | 6 ++ .../sync/immediate-upload.service.spec.ts | 21 ++++- .../op-log/sync/immediate-upload.service.ts | 12 ++- .../sync/operation-log-sync.service.spec.ts | 44 +++++++++ .../ws-triggered-download.service.spec.ts | 21 ++++- .../sync/ws-triggered-download.service.ts | 12 ++- 9 files changed, 199 insertions(+), 33 deletions(-) diff --git a/src/app/imex/sync/sync-wrapper.service.spec.ts b/src/app/imex/sync/sync-wrapper.service.spec.ts index 4e3d3e380a..c487876b9d 100644 --- a/src/app/imex/sync/sync-wrapper.service.spec.ts +++ b/src/app/imex/sync/sync-wrapper.service.spec.ts @@ -1066,6 +1066,19 @@ describe('SyncWrapperService', () => { }); }); + it('should preserve an existing persistent recovery action for incomplete remote work', async () => { + mockSnackService.hasPendingPersistentAction.and.returnValue(true); + mockSyncService.downloadRemoteOps.and.rejectWith( + new IncompleteRemoteOperationsError(new Error('archive failed')), + ); + + const result = await service.sync(); + + expect(result).toBe('HANDLED_ERROR'); + expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR'); + expect(mockSnackService.open).not.toHaveBeenCalled(); + }); + it('should handle PotentialCorsError with snack message', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.reject(new PotentialCorsError('https://example.com')), diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts index 17c8870b1d..2cd42204f2 100644 --- a/src/app/imex/sync/sync-wrapper.service.ts +++ b/src/app/imex/sync/sync-wrapper.service.ts @@ -710,11 +710,13 @@ export class SyncWrapperService { return 'HANDLED_ERROR'; } else if (error instanceof IncompleteRemoteOperationsError) { this._providerManager.setSyncStatus('ERROR'); - this._snackService.open({ - msg: T.F.SYNC.S.INCOMPLETE_REMOTE_OPERATIONS, - type: 'ERROR', - config: { duration: 0 }, - }); + if (!this._snackService.hasPendingPersistentAction()) { + this._snackService.open({ + msg: T.F.SYNC.S.INCOMPLETE_REMOTE_OPERATIONS, + type: 'ERROR', + config: { duration: 0 }, + }); + } return 'HANDLED_ERROR'; } else if ( error instanceof AuthFailSPError || diff --git a/src/app/op-log/persistence/operation-log-store.service.spec.ts b/src/app/op-log/persistence/operation-log-store.service.spec.ts index c0ef012b90..70280af355 100644 --- a/src/app/op-log/persistence/operation-log-store.service.spec.ts +++ b/src/app/op-log/persistence/operation-log-store.service.spec.ts @@ -6,6 +6,7 @@ import { VectorClockService } from '../sync/vector-clock.service'; import { ActionType, Operation, + OperationLogEntry, OpType, EntityType, VectorClock, @@ -29,7 +30,12 @@ import { MAX_VECTOR_CLOCK_SIZE, } from '../core/operation-log.const'; import { IndexedDBOpenError } from '../core/errors/indexed-db-open.error'; -import { FULL_STATE_OPS_META_KEY, SINGLETON_KEY, STORE_NAMES } from './db-keys.const'; +import { + FULL_STATE_OPS_META_KEY, + OPS_INDEXES, + SINGLETON_KEY, + STORE_NAMES, +} from './db-keys.const'; import { ArchiveModel } from '../../features/time-tracking/time-tracking.model'; describe('OperationLogStoreService', () => { @@ -1555,6 +1561,32 @@ describe('OperationLogStoreService', () => { }); describe('markFailed', () => { + const seedLegacyTerminalRemoteFailure = async (op: Operation): Promise => { + await service.append(op, 'remote', { pendingApply: true }); + for (let retry = 0; retry < 4; retry++) { + await service.markFailed([op.id]); + } + await service.markRejected([op.id]); + + const adapter = ( + service as unknown as { + _adapter: OpLogDbAdapter; + } + )._adapter; + await adapter.transaction([STORE_NAMES.OPS], 'readwrite', async (tx) => { + const entry = await tx.getFromIndex( + STORE_NAMES.OPS, + OPS_INDEXES.BY_ID, + op.id, + ); + if (!entry) { + throw new Error('Expected seeded legacy operation'); + } + entry.applicationStatus = undefined; + await tx.put(STORE_NAMES.OPS, entry); + }); + }; + it('should increment retry count', async () => { const op = createTestOperation(); await service.append(op, 'remote', { pendingApply: true }); @@ -1594,11 +1626,7 @@ describe('OperationLogStoreService', () => { it('should re-quarantine legacy terminal remote failures', async () => { const op = createTestOperation(); - await service.append(op, 'remote', { pendingApply: true }); - for (let retry = 0; retry < 4; retry++) { - await service.markFailed([op.id]); - } - await service.markRejected([op.id]); + await seedLegacyTerminalRemoteFailure(op); expect(await service.recoverLegacyTerminalRemoteFailures()).toBe(1); @@ -1610,25 +1638,32 @@ describe('OperationLogStoreService', () => { it('should run the legacy terminal remote failure repair only once', async () => { const firstLegacyOp = createTestOperation({ entityId: 'first-legacy' }); - await service.append(firstLegacyOp, 'remote', { pendingApply: true }); - for (let retry = 0; retry < 4; retry++) { - await service.markFailed([firstLegacyOp.id]); - } - await service.markRejected([firstLegacyOp.id]); + await seedLegacyTerminalRemoteFailure(firstLegacyOp); expect(await service.recoverLegacyTerminalRemoteFailures()).toBe(1); const laterLegacyOp = createTestOperation({ entityId: 'later-legacy' }); - await service.append(laterLegacyOp, 'remote', { pendingApply: true }); - for (let retry = 0; retry < 4; retry++) { - await service.markFailed([laterLegacyOp.id]); - } - await service.markRejected([laterLegacyOp.id]); + await seedLegacyTerminalRemoteFailure(laterLegacyOp); expect(await service.recoverLegacyTerminalRemoteFailures()).toBe(0); expect((await service.getOpById(laterLegacyOp.id))?.rejectedAt).toBeDefined(); }); + it('should leave legitimately rejected failed remote work untouched', async () => { + const op = createTestOperation({ entityId: 'legitimately-rejected' }); + await service.append(op, 'remote', { pendingApply: true }); + for (let retry = 0; retry < 4; retry++) { + await service.markFailed([op.id]); + } + await service.markRejected([op.id]); + + expect(await service.recoverLegacyTerminalRemoteFailures()).toBe(0); + + const stored = await service.getOpById(op.id); + expect(stored?.rejectedAt).toBeDefined(); + expect(stored?.applicationStatus).toBe('failed'); + }); + it('should handle empty array', async () => { await service.markFailed([]); // Should not throw @@ -2139,6 +2174,30 @@ describe('OperationLogStoreService', () => { lastTimeTrackingFlush: 0, }); + it('should atomically clear an interrupted raw-rebuild marker', async () => { + await service.runRemoteStateReplacement({ + baselineState: { task: { ids: [], entities: {} } }, + vectorClock: { remote: 1 }, + schemaVersion: 4, + snapshotEntityKeys: [], + archiveYoung: createArchive('remote-young'), + archiveOld: createArchive('remote-old'), + }); + expect(await service.isRawRebuildIncomplete()).toBe(true); + + await service.runDestructiveStateReplacement({ + syncImportOp: createTestOperation({ + opType: OpType.BackupImport, + entityType: 'ALL' as EntityType, + entityId: 'restored-backup', + payload: { task: { ids: [], entities: {} } }, + }), + snapshotEntityKeys: [], + }); + + expect(await service.isRawRebuildIncomplete()).toBe(false); + }); + it('should write archives in the same destructive replacement', async () => { const archiveYoung = createArchive('young-task'); const archiveOld = createArchive('old-task'); diff --git a/src/app/op-log/persistence/operation-log-store.service.ts b/src/app/op-log/persistence/operation-log-store.service.ts index cc02a797a8..a94ed39154 100644 --- a/src/app/op-log/persistence/operation-log-store.service.ts +++ b/src/app/op-log/persistence/operation-log-store.service.ts @@ -1152,6 +1152,7 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort= 4 ) { entry.rejectedAt = undefined; @@ -2061,6 +2062,11 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort { mockSyncWrapperService = { isEncryptionOperationInProgress: false, }; - mockSnackService = jasmine.createSpyObj('SnackService', ['open']); + mockSnackService = jasmine.createSpyObj('SnackService', [ + 'open', + 'hasPendingPersistentAction', + ]); + mockSnackService.hasPendingPersistentAction.and.returnValue(false); TestBed.configureTestingModule({ providers: [ @@ -181,6 +185,21 @@ describe('ImmediateUploadService', () => { }); })); + it('should preserve an existing persistent recovery action for incomplete remote work', fakeAsync(() => { + mockSnackService.hasPendingPersistentAction.and.returnValue(true); + mockSyncService.uploadPendingOps.and.rejectWith( + new IncompleteRemoteOperationsError(new Error('archive failed')), + ); + + service.initialize(); + service.trigger(); + tick(2000); + flush(); + + expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR'); + expect(mockSnackService.open).not.toHaveBeenCalled(); + })); + it('should report UNKNOWN_OR_CHANGED when a local-win follow-up lacks a mandatory encryption key', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValues( Promise.resolve(completedResult({ uploadedCount: 1, localWinOpsCreated: 1 })), diff --git a/src/app/op-log/sync/immediate-upload.service.ts b/src/app/op-log/sync/immediate-upload.service.ts index 6ebed036b1..d2440aa2da 100644 --- a/src/app/op-log/sync/immediate-upload.service.ts +++ b/src/app/op-log/sync/immediate-upload.service.ts @@ -330,11 +330,13 @@ export class ImmediateUploadService implements OnDestroy { } catch (e) { if (e instanceof IncompleteRemoteOperationsError) { this._providerManager.setSyncStatus('ERROR'); - this._snackService.open({ - msg: T.F.SYNC.S.INCOMPLETE_REMOTE_OPERATIONS, - type: 'ERROR', - config: { duration: 0 }, - }); + if (!this._snackService.hasPendingPersistentAction()) { + this._snackService.open({ + msg: T.F.SYNC.S.INCOMPLETE_REMOTE_OPERATIONS, + type: 'ERROR', + config: { duration: 0 }, + }); + } return; } diff --git a/src/app/op-log/sync/operation-log-sync.service.spec.ts b/src/app/op-log/sync/operation-log-sync.service.spec.ts index 48508f8ab0..d7bfd73e6b 100644 --- a/src/app/op-log/sync/operation-log-sync.service.spec.ts +++ b/src/app/op-log/sync/operation-log-sync.service.spec.ts @@ -1064,6 +1064,50 @@ describe('OperationLogSyncService', () => { ); }); + it('should allow uploads after stranded-rebuild Undo clears the marker', async () => { + let isRawRebuildIncomplete = true; + opLogStoreSpy.isRawRebuildIncomplete.and.callFake( + async () => isRawRebuildIncomplete, + ); + spyOn(service, 'forceDownloadRemoteState').and.rejectWith( + new Error('USE_REMOTE aborted: remote returned no data to rebuild from.'), + ); + opLogStoreSpy.loadImportBackup.and.resolveTo({ savedAt: 4242 } as any); + backupServiceSpy.restoreImportBackup.and.callFake(async () => { + isRawRebuildIncomplete = false; + return true; + }); + uploadServiceSpy.uploadPendingOps.and.resolveTo({ + uploadedCount: 0, + piggybackedOps: [], + rejectedCount: 0, + rejectedOps: [], + }); + const mockProvider = { isReady: () => Promise.resolve(true) } as any; + + await expectAsync(service.downloadRemoteOps(mockProvider)).toBeRejected(); + + const undoSnack = snackServiceSpy.open.calls + .allArgs() + .map(([params]) => params) + .find( + (params) => + typeof params === 'object' && + params !== null && + params.msg === T.F.SYNC.S.LOCAL_DATA_REPLACE_UNDO, + ); + expect(undoSnack).toBeDefined(); + if (typeof undoSnack !== 'object' || undoSnack === null || !undoSnack.actionFn) { + throw new Error('Expected the stranded-rebuild Undo action'); + } + await undoSnack.actionFn(); + + await service.uploadPendingOps(mockProvider); + + expect(backupServiceSpy.restoreImportBackup).toHaveBeenCalledWith(4242); + expect(uploadServiceSpy.uploadPendingOps).toHaveBeenCalled(); + }); + it('should not respawn the recovery snack while one is already showing', async () => { opLogStoreSpy.isRawRebuildIncomplete.and.resolveTo(true); spyOn(service, 'forceDownloadRemoteState').and.rejectWith( diff --git a/src/app/op-log/sync/ws-triggered-download.service.spec.ts b/src/app/op-log/sync/ws-triggered-download.service.spec.ts index 7014500853..1efd679ea9 100644 --- a/src/app/op-log/sync/ws-triggered-download.service.spec.ts +++ b/src/app/op-log/sync/ws-triggered-download.service.spec.ts @@ -61,7 +61,11 @@ describe('WsTriggeredDownloadService', () => { // off it lazily (via Injector, to avoid a DI cycle). Provide a minimal mock so // the real (heavily-dependent) service is never constructed in the unit test. mockSyncWrapper = { isEncryptionOperationInProgress: false }; - mockSnackService = jasmine.createSpyObj('SnackService', ['open']); + mockSnackService = jasmine.createSpyObj('SnackService', [ + 'open', + 'hasPendingPersistentAction', + ]); + mockSnackService.hasPendingPersistentAction.and.returnValue(false); TestBed.configureTestingModule({ providers: [ @@ -251,6 +255,21 @@ describe('WsTriggeredDownloadService', () => { }); })); + it('should preserve an existing persistent recovery action for incomplete remote work', fakeAsync(() => { + mockSnackService.hasPendingPersistentAction.and.returnValue(true); + mockSyncService.downloadRemoteOps.and.rejectWith( + new IncompleteRemoteOperationsError(new Error('archive failed')), + ); + + service.start(); + notification$.next({ latestSeq: 1 }); + tick(500); + flushMicrotasks(); + + expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR'); + expect(mockSnackService.open).not.toHaveBeenCalled(); + })); + it('should be idempotent when start is called twice', fakeAsync(() => { service.start(); service.start(); diff --git a/src/app/op-log/sync/ws-triggered-download.service.ts b/src/app/op-log/sync/ws-triggered-download.service.ts index 762cb7fd22..3135c7e7ea 100644 --- a/src/app/op-log/sync/ws-triggered-download.service.ts +++ b/src/app/op-log/sync/ws-triggered-download.service.ts @@ -176,11 +176,13 @@ export class WsTriggeredDownloadService implements OnDestroy { err, ); this._providerManager.setSyncStatus('ERROR'); - this._snackService.open({ - msg: T.F.SYNC.S.INCOMPLETE_REMOTE_OPERATIONS, - type: 'ERROR', - config: { duration: 0 }, - }); + if (!this._snackService.hasPendingPersistentAction()) { + this._snackService.open({ + msg: T.F.SYNC.S.INCOMPLETE_REMOTE_OPERATIONS, + type: 'ERROR', + config: { duration: 0 }, + }); + } return; } if (err instanceof AuthFailSPError || err instanceof MissingCredentialsSPError) {