mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(sync): discard example-task ops on first adoption of populated remote (#7985)
The deferred "Fix C" from #7980: a never-synced client that adopts a populated, non-encrypted SuperSync account (seeded by normal upload, so no SYNC_IMPORT fires the incoming-import gate's discard) would upload its 4 onboarding example-task ops onto that remote, propagating throwaway scaffolding to every device. In OperationLogSyncService.uploadPendingOps, when the pre-download-captured isNeverSynced is true AND hasSyncedOps() is now true (this cycle's download just adopted remote ops), reject the pending example-create ops so the upload never sends them. The pre-download capture is what makes this reliable — a live hasSyncedOps() read is useless here because the download flips it mid-cycle (#7980 §10). Stranding safety: discard only from a PRISTINE post-boot batch — example creates plus default GLOBAL_CONFIG writes, nothing else. hasMeaningfulPendingOps() is not sufficient: it ignores Move/Batch (and planner/board/section) ops, which can reference an example task id without being "meaningful". Unlike the incoming- SYNC_IMPORT discard — where SyncImportFilterService drops any concurrent dependent op against the import — this adoption path has no import to filter them, so a surviving reorder Move would strand a dangling reference once its create is rejected. Requiring an all-examples+config batch also preserves "an edited example syncs as real data": any user action (edit, reorder, real task) adds a non-config op and skips the cleanup, so everything uploads together. Scope: stops cross-device propagation (the documented harm). On SuperSync the example tasks remain in this device's local NgRx state (op-based adoption merges rather than replaces); removing them from local state is a separate increment. The file-based path already clears them via snapshot hydration.
This commit is contained in:
Johannes Millan
parent
58e354162b
commit
81b0c9f891
2 changed files with 237 additions and 0 deletions
|
|
@ -260,6 +260,202 @@ describe('OperationLogSyncService', () => {
|
|||
.and.returnValue(Promise.resolve(1)); // Not fresh (has seq)
|
||||
});
|
||||
|
||||
describe('example-task cleanup on first adoption of a populated remote (#7985)', () => {
|
||||
const exampleCreate = (taskId: string): OperationLogEntry => ({
|
||||
seq: 1,
|
||||
op: {
|
||||
id: `ex-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: {
|
||||
actionPayload: { task: { id: taskId }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const realTaskCreate = (taskId: string): OperationLogEntry => ({
|
||||
seq: 2,
|
||||
op: {
|
||||
id: `real-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: '[Task] Add Task' as ActionType,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: { actionPayload: { task: { id: taskId } }, entityChanges: [] },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const configOp = (): OperationLogEntry => ({
|
||||
seq: 3,
|
||||
op: {
|
||||
id: 'config-op-1',
|
||||
clientId: 'client-A',
|
||||
actionType: '[Global Config] Update Global Config Section' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'sync',
|
||||
payload: { sectionKey: 'sync' },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
// A reorder of an example task: a Move op (entityType TASK, entityId = the example id)
|
||||
// that hasMeaningfulPendingOps() does NOT flag — would strand a dangling reference if
|
||||
// the cleanup rejected the create but let this Move upload (cross-validated by review).
|
||||
const exampleMove = (taskId: string): OperationLogEntry => ({
|
||||
seq: 4,
|
||||
op: {
|
||||
id: `mov-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: '[Project] Move Task In Backlog/Today' as ActionType,
|
||||
opType: OpType.Move,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: {},
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const exampleUpdate = (taskId: string): OperationLogEntry => ({
|
||||
seq: 5,
|
||||
op: {
|
||||
id: `upd-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: '[Task] Update Task' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: { actionPayload: { task: { id: taskId } }, entityChanges: [] },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const mockProvider = { isReady: () => Promise.resolve(true) } as any;
|
||||
|
||||
beforeEach(() => {
|
||||
uploadServiceSpy.uploadPendingOps.and.resolveTo({
|
||||
uploadedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
});
|
||||
opLogStoreSpy.markRejected.and.resolveTo();
|
||||
});
|
||||
|
||||
it('discards untouched example-task ops when a never-synced client adopted a populated remote', async () => {
|
||||
// isNeverSynced captured pre-download = true; hasSyncedOps now true = adopted this cycle.
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
exampleCreate('ex-2'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([
|
||||
'ex-op-ex-1',
|
||||
'ex-op-ex-2',
|
||||
]);
|
||||
});
|
||||
|
||||
it('does NOT discard when the client had already synced before this cycle (isNeverSynced false)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([exampleCreate('ex-1')]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: false });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT discard when nothing was adopted this cycle (empty-server seeding, hasSyncedOps false)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(false);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([exampleCreate('ex-1')]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT discard when meaningful user work is also pending (a real task) — keeps everything', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
realTaskCreate('real-1'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still discards when only default config ops coexist with the example tasks', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
configOp(),
|
||||
exampleCreate('ex-2'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([
|
||||
'ex-op-ex-1',
|
||||
'ex-op-ex-2',
|
||||
]);
|
||||
});
|
||||
|
||||
it('does NOT discard when a reorder Move of an example task is pending (would otherwise strand a dangling ref)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
exampleMove('ex-1'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT discard when an example task was edited (its Update keeps the whole batch)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
exampleUpdate('ex-1'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('uploadPendingOps', () => {
|
||||
it('should return localWinOpsCreated: 0 when no piggybacked ops', async () => {
|
||||
opLogStoreSpy.getUnsynced.and.returnValue(Promise.resolve([]));
|
||||
|
|
|
|||
|
|
@ -198,6 +198,47 @@ export class OperationLogSyncService {
|
|||
return { kind: 'blocked_fresh_client' };
|
||||
}
|
||||
|
||||
// #7985: identity-based example-task cleanup (the deferred "Fix C" of #7980).
|
||||
// `isNeverSyncedAtSyncStart` is captured PRE-download; if hasSyncedOps() is now true,
|
||||
// this cycle's download just ADOPTED a populated remote. A genuinely-fresh client's
|
||||
// onboarding example tasks are throwaway scaffolding every install regenerates;
|
||||
// uploading them onto the adopted remote pollutes it and propagates to every device
|
||||
// (the residual #7980 documented for non-encrypted accounts seeded by normal upload,
|
||||
// where no SYNC_IMPORT fires the incoming-import gate's discard). Reject the example
|
||||
// create ops so the upload never sends them.
|
||||
//
|
||||
// A status read alone is unsafe (download flips hasSyncedOps mid-cycle — #7980 §10);
|
||||
// the PRE-download `isNeverSynced` is what makes this reliable.
|
||||
//
|
||||
// Stranding safety: we discard only from a PRISTINE post-boot batch — example creates
|
||||
// plus the default GLOBAL_CONFIG writes, nothing else. hasMeaningfulPendingOps() is NOT
|
||||
// enough here: it ignores Move/Batch (and planner/board/section) ops, which can reference
|
||||
// an example task id without being "meaningful". Unlike the incoming-SYNC_IMPORT discard
|
||||
// — where SyncImportFilterService drops any concurrent dependent op against the import —
|
||||
// this adoption path has no import to filter them, so a surviving Move would strand a
|
||||
// dangling reference once its create is rejected. Requiring an all-examples+config batch
|
||||
// also preserves the "edited example syncs as real data" property: any user action (edit,
|
||||
// reorder, real task) adds a non-config op and skips the cleanup, so everything uploads.
|
||||
if (isNeverSyncedAtSyncStart && (await this.opLogStore.hasSyncedOps())) {
|
||||
const pendingOps = await this.opLogStore.getUnsynced();
|
||||
const isPristinePostBootBatch = pendingOps.every(
|
||||
(entry) =>
|
||||
isExampleTaskCreateOp(entry) || entry.op.entityType === 'GLOBAL_CONFIG',
|
||||
);
|
||||
if (isPristinePostBootBatch) {
|
||||
const exampleOpIds = pendingOps
|
||||
.filter(isExampleTaskCreateOp)
|
||||
.map((entry) => entry.op.id);
|
||||
if (exampleOpIds.length > 0) {
|
||||
await this._discardExampleTaskOps(exampleOpIds);
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Discarded ${exampleOpIds.length} untouched example-task ` +
|
||||
'op(s) — never-synced client adopted a populated remote this sync (#7985).',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// SERVER MIGRATION CHECK: Passed as callback to execute INSIDE the upload lock.
|
||||
// This prevents race conditions where multiple tabs could both detect migration
|
||||
// and create duplicate SYNC_IMPORT operations.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue