fix(sync): exempt fresh-client startup ops from conflict gate

Fresh clients generate startup config and genesis operations before their first sync. Treat those operations as setup state until a prior sync exists, while continuing to protect later config changes and all ordinary user work from authoritative snapshot replacement.
This commit is contained in:
Johannes Millan 2026-07-10 19:08:38 +02:00
parent be0d6e7dd0
commit ab9e1d38c9
3 changed files with 78 additions and 36 deletions

View file

@ -582,9 +582,13 @@ export class OperationLogSyncService {
.filter((id): id is string => id !== undefined),
);
const exampleTaskOpIds = exampleTaskEntries.map((entry) => entry.op.id);
// Nothing from this sync is persisted yet, so this live read reflects
// whether the client completed a prior sync cycle.
const hasCompletedInitialSync = await this.opLogStore.hasSyncedOps();
const hasMeaningfulUserData =
this.syncImportConflictGateService.hasMeaningfulPendingOps(unsyncedOps) ||
this.syncLocalStateService.hasMeaningfulStoreData(exampleTaskIds);
this.syncImportConflictGateService.hasMeaningfulPendingOps(unsyncedOps, {
hasCompletedInitialSync,
}) || this.syncLocalStateService.hasMeaningfulStoreData(exampleTaskIds);
if (hasMeaningfulUserData) {
// SPAP-9: before surfacing the binary USE_LOCAL/USE_REMOTE dialog, use

View file

@ -94,9 +94,8 @@ describe('SyncImportConflictGateService', () => {
});
});
it('should produce dialog data when pending ops contain user config changes', async () => {
const incomingSyncImport = createOperation();
const pendingConfigEntry = createEntry(
const createPendingConfigEntry = (): OperationLogEntry =>
createEntry(
createOperation({
id: 'local-config-update',
actionType: '[Global Config] Update Global Config Section' as ActionType,
@ -108,15 +107,27 @@ describe('SyncImportConflictGateService', () => {
vectorClock: { clientA: 1 },
}),
);
opLogStoreSpy.getUnsynced.and.resolveTo([pendingConfigEntry]);
const result = await service.checkIncomingFullStateConflict([incomingSyncImport]);
it('should produce dialog data for a synced client with a pending config change', async () => {
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
opLogStoreSpy.getUnsynced.and.resolveTo([createPendingConfigEntry()]);
const result = await service.checkIncomingFullStateConflict([createOperation()]);
expect(result.fullStateOp).toBe(incomingSyncImport);
expect(result.hasMeaningfulPending).toBeTrue();
expect(result.dialogData).toBeDefined();
});
it('should NOT flag a pending config change on a never-synced client', async () => {
opLogStoreSpy.hasSyncedOps.and.resolveTo(false);
opLogStoreSpy.getUnsynced.and.resolveTo([createPendingConfigEntry()]);
const result = await service.checkIncomingFullStateConflict([createOperation()]);
expect(result.hasMeaningfulPending).toBeFalse();
expect(result.dialogData).toBeUndefined();
});
it('should not produce dialog data when pending task creates are startup example tasks', async () => {
const incomingSyncImport = createOperation();
const pendingExampleTaskEntry = createEntry(
@ -391,7 +402,7 @@ describe('SyncImportConflictGateService', () => {
expect(service.hasMeaningfulPendingOps([pendingCounter])).toBeTrue();
});
it('should treat MIGRATION/RECOVERY genesis batches as meaningful recovered user data', async () => {
it('should treat MIGRATION/RECOVERY genesis batches as meaningful only once the client has synced', async () => {
const pendingMigration = createEntry(
createOperation({
id: 'local-genesis',
@ -419,8 +430,15 @@ describe('SyncImportConflictGateService', () => {
{ seq: 2 },
);
expect(service.hasMeaningfulPendingOps([pendingMigration])).toBeTrue();
expect(service.hasMeaningfulPendingOps([pendingRecovery])).toBeTrue();
const synced = { hasCompletedInitialSync: true };
expect(service.hasMeaningfulPendingOps([pendingMigration], synced)).toBeTrue();
expect(service.hasMeaningfulPendingOps([pendingRecovery], synced)).toBeTrue();
const neverSynced = { hasCompletedInitialSync: false };
expect(
service.hasMeaningfulPendingOps([pendingMigration], neverSynced),
).toBeFalse();
expect(service.hasMeaningfulPendingOps([pendingRecovery], neverSynced)).toBeFalse();
});
});

View file

@ -10,6 +10,13 @@ import { OperationWriteFlushService } from './operation-write-flush.service';
import { SyncImportConflictData } from './dialog-sync-import-conflict/dialog-sync-import-conflict.component';
import { isExampleTaskCreateOp } from '../validation/is-example-task-op.util';
/**
* Startup/system entities are written automatically during first-time setup.
* Before the client has completed a sync they are setup state, not divergence;
* after that, later writes are genuine local changes.
*/
const STARTUP_ENTITY_TYPES = new Set(['GLOBAL_CONFIG', 'MIGRATION', 'RECOVERY']);
export interface IncomingFullStateConflictGateResult {
fullStateOp?: Operation;
pendingOps: OperationLogEntry[];
@ -34,13 +41,20 @@ export class SyncImportConflictGateService {
private writeFlushService = inject(OperationWriteFlushService);
/**
* Every pending op is user work unless it is an onboarding example-task create.
* Entity-wide exemptions are unsafe: GLOBAL_CONFIG contains synced preferences,
* while MIGRATION and RECOVERY genesis operations contain the user's full recovered
* database. Full-state ops are always meaningful because applying a newer full-state
* op can invalidate their local import/repair semantics.
* Every pending op is user work unless it is an onboarding example-task create,
* or a startup/system write on a client that has not yet completed a sync.
* Full-state ops are always meaningful because applying a newer full-state op
* can invalidate their local import/repair semantics.
*
* The lifecycle default is deliberately conservative: a caller that does not
* know whether initial sync completed must protect startup-entity changes.
*/
hasMeaningfulPendingOps(ops: OperationLogEntry[]): boolean {
hasMeaningfulPendingOps(
ops: OperationLogEntry[],
options: { hasCompletedInitialSync: boolean } = {
hasCompletedInitialSync: true,
},
): boolean {
return ops.some((entry) => {
if (FULL_STATE_OP_TYPES.has(entry.op.opType as OpType)) {
return true;
@ -48,6 +62,9 @@ export class SyncImportConflictGateService {
if (isExampleTaskCreateOp(entry)) {
return false;
}
if (STARTUP_ENTITY_TYPES.has(entry.op.entityType)) {
return options.hasCompletedInitialSync;
}
return true;
});
}
@ -87,12 +104,8 @@ export class SyncImportConflictGateService {
const pendingOps = options.preCapturedPendingOps
? this._mergePendingOps(options.preCapturedPendingOps, livePendingOps)
: livePendingOps;
const hasMeaningfulPending = this.hasMeaningfulPendingOps(pendingOps);
// Example-task ops that the caller may reject when it accepts the import silently.
// When `hasMeaningfulPending` is true (real work pending alongside example tasks),
// the conflict dialog is shown instead and these are intentionally left untouched:
// if the user keeps local state, their example tasks ride along with the rest.
//
// These must come from a LIVE read: with a pre-captured snapshot, example ops
// accepted earlier in the same upload round are already marked synced and must
// not be re-marked rejected by the caller.
@ -100,6 +113,27 @@ export class SyncImportConflictGateService {
.filter(isExampleTaskCreateOp)
.map((entry) => entry.op.id);
// Preserve the cheap example-task-only path. If nothing could be meaningful
// even for a synced client, there is no reason to read sync history.
const canContainMeaningfulPending = this.hasMeaningfulPendingOps(pendingOps);
if (!canContainMeaningfulPending) {
return {
fullStateOp,
pendingOps,
hasMeaningfulPending: false,
discardablePendingOpIds,
};
}
// Capture lifecycle state before deciding whether startup writes are setup
// noise or post-sync divergence. Piggyback callers pass their pre-sync snapshot
// because a live read there already includes this sync cycle's writes.
const isNeverSynced =
options.isNeverSynced ?? !(await this.opLogStore.hasSyncedOps());
const hasMeaningfulPending = this.hasMeaningfulPendingOps(pendingOps, {
hasCompletedInitialSync: !isNeverSynced,
});
const result = {
fullStateOp,
pendingOps,
@ -111,20 +145,6 @@ export class SyncImportConflictGateService {
return result;
}
// A client that has never completed a sync cannot have diverged from remote — its
// "meaningful" pending ops are pre-first-sync startup state (e.g. example tasks).
// Flag this so the dialog guards the destructive USE_LOCAL choice with an extra
// confirmation (it would overwrite the populated remote with throwaway data).
//
// Callers SHOULD pass `isNeverSynced` captured at sync-cycle start (pre-download).
// A live `hasSyncedOps()` here is unreliable on the piggyback-upload path: by the
// time that gate runs, this same sync has already persisted downloaded ops
// (syncedAt set) and marked accepted uploads synced, so reading it now would see
// post-sync state and wrongly clear the guard. Fall back to a live read only for
// standalone callers (e.g. the download path, where nothing is persisted yet).
const isNeverSynced =
options.isNeverSynced ?? !(await this.opLogStore.hasSyncedOps());
return {
...result,
dialogData: {