more fixes

This commit is contained in:
Andras Schmelczer 2026-04-25 20:42:34 +01:00
commit 7a8c497462
6 changed files with 94 additions and 38 deletions

View file

@ -2,6 +2,7 @@ import type { TestDefinition, TestResult, TestStep } from "./test-definition";
import { DeterministicAgent } from "./deterministic-agent"; import { DeterministicAgent } from "./deterministic-agent";
import type { ServerControl } from "./server-control"; import type { ServerControl } from "./server-control";
import type { SyncSettings, Logger } from "sync-client"; import type { SyncSettings, Logger } from "sync-client";
import { CONFLICT_PATH_REGEX } from "sync-client";
import { assert } from "./utils/assert"; import { assert } from "./utils/assert";
import { AssertableState } from "./utils/assertable-state"; import { AssertableState } from "./utils/assertable-state";
import { sleep } from "./utils/sleep"; import { sleep } from "./utils/sleep";
@ -326,6 +327,15 @@ export class TestRunner {
this.logger.info("✓ All clients are consistent"); this.logger.info("✓ All clients are consistent");
const conflictFiles = referenceFiles.filter((path) =>
CONFLICT_PATH_REGEX.test(path)
);
if (conflictFiles.length > 0) {
throw new Error(
`Found ${conflictFiles.length} conflict file(s) — local displacements indicate a reconciliation regression: [${conflictFiles.join(", ")}]`
);
}
if (verify) { if (verify) {
this.logger.info("Running custom verification..."); this.logger.info("Running custom verification...");
try { try {

View file

@ -37,6 +37,7 @@ export type { AuthenticationError } from "./errors/authentication-error";
export type { MaybeOutdatedClientCursors } from "./types/maybe-outdated-client-cursors"; export type { MaybeOutdatedClientCursors } from "./types/maybe-outdated-client-cursors";
export { DocumentSyncStatus } from "./types/document-sync-status"; export { DocumentSyncStatus } from "./types/document-sync-status";
export { SyncClient } from "./sync-client"; export { SyncClient } from "./sync-client";
export { CONFLICT_PATH_REGEX } from "./sync-operations/conflict-path";
export type { TextWithCursors, CursorPosition } from "reconcile-text"; export type { TextWithCursors, CursorPosition } from "reconcile-text";
export const debugging = { export const debugging = {

View file

@ -14,15 +14,14 @@ export class ServerConfig {
private response: Promise<PingResponse> | undefined; private response: Promise<PingResponse> | undefined;
private config: ServerConfigData | undefined; private config: ServerConfigData | undefined;
public constructor(private readonly syncService: SyncService) {} public constructor(private readonly syncService: SyncService) { }
private static validateConfig(config: ServerConfigData): void { private static validateConfig(config: ServerConfigData): void {
if (config.supportedApiVersion !== SUPPORTED_API_VERSION) { if (config.supportedApiVersion !== SUPPORTED_API_VERSION) {
const shouldUpgradeClient = const shouldUpgradeClient =
config.supportedApiVersion > SUPPORTED_API_VERSION; config.supportedApiVersion > SUPPORTED_API_VERSION;
throw new ServerVersionMismatchError( throw new ServerVersionMismatchError(
`Unsupported API version: ${config.supportedApiVersion}. Consider upgrading the ${ `Unsupported API version: ${config.supportedApiVersion}. Consider upgrading the ${shouldUpgradeClient ? "client" : "sync-server"
shouldUpgradeClient ? "client" : "sync-server"
} to ensure compatibility` } to ensure compatibility`
); );
} }
@ -41,7 +40,7 @@ export class ServerConfig {
try { try {
let { response } = this; let { response } = this;
if (!response || forceUpdate) { if (!response || forceUpdate) {
response = this.response = this.syncService.ping(); response = this.startPing();
} }
const result: PingResponse = await response; // it must be defined, otherwise we would have thrown above const result: PingResponse = await response; // it must be defined, otherwise we would have thrown above
@ -68,7 +67,7 @@ export class ServerConfig {
public async getConfig(): Promise<ServerConfigData> { public async getConfig(): Promise<ServerConfigData> {
if (!this.config) { if (!this.config) {
this.response ??= this.syncService.ping(); this.response ??= this.startPing();
this.config = await this.response; this.config = await this.response;
} }
@ -77,6 +76,17 @@ export class ServerConfig {
return this.config; return this.config;
} }
private startPing(): Promise<PingResponse> {
const pending = this.syncService.ping().catch((e: unknown) => {
if (this.response === pending) {
this.response = undefined;
}
throw e;
});
this.response = pending;
return pending;
}
public reset(): void { public reset(): void {
this.response = undefined; this.response = undefined;
this.config = undefined; this.config = undefined;

View file

@ -399,7 +399,7 @@ export class SyncClient {
return DocumentSyncStatus.SYNCING_IS_DISABLED; return DocumentSyncStatus.SYNCING_IS_DISABLED;
} }
if (!this.syncer.isFirstSyncStarted || !this.hasFinishedOfflineSync) { if (!this.hasFinishedOfflineSync) {
return DocumentSyncStatus.SYNCING; return DocumentSyncStatus.SYNCING;
} }
@ -441,20 +441,25 @@ export class SyncClient {
} }
this.isDestroying = true; this.isDestroying = true;
await this.pause(); // Run cleanup in `finally` so a thrown pause() — or anything else
// mid-shutdown — still leaves the client in the disposed state
// instead of bricked with subscribers/telemetry hanging on.
try {
await this.pause();
} finally {
this.hasBeenDestroyed = true;
this.hasBeenDestroyed = true; this.resetInMemoryState();
this.resetInMemoryState(); this.eventUnsubscribers.forEach((unsubscribe) => {
unsubscribe();
});
this.eventUnsubscribers.length = 0;
this.eventUnsubscribers.forEach((unsubscribe) => { this.logger.info("SyncClient has been successfully disposed");
unsubscribe();
});
this.eventUnsubscribers.length = 0;
this.logger.info("SyncClient has been successfully disposed"); this.unloadTelemetry?.();
}
this.unloadTelemetry?.();
} }
private async startSyncing(): Promise<void> { private async startSyncing(): Promise<void> {

View file

@ -38,8 +38,16 @@ export class SyncEventQueue {
// It maps pending changes onto the local filesystem. // It maps pending changes onto the local filesystem.
private readonly events: SyncEvent[] = []; private readonly events: SyncEvent[] = [];
// file creations for paths matching any of these patterns will be ignored // file creations for paths matching any of these patterns are ignored
private ignorePatterns: RegExp[]; // because the user explicitly told us to ignore them.
private userIgnorePatterns: RegExp[];
// Whether `CONFLICT_PATH_REGEX` is applied at enqueue time. Conflict files
// exist because the syncer set them aside; ignoring them at runtime
// prevents resync churn. During an offline scan we DO want to surface them
// so a stranded conflict file (e.g. one this client previously displaced
// and was unable to re-sync) gets picked up as a normal new file.
private ignoreConflictPaths = true;
public constructor( public constructor(
private readonly settings: Settings, private readonly settings: Settings,
@ -47,19 +55,16 @@ export class SyncEventQueue {
initialState: Partial<StoredSyncState> | undefined, initialState: Partial<StoredSyncState> | undefined,
private readonly saveData: (data: StoredSyncState) => Promise<void> private readonly saveData: (data: StoredSyncState) => Promise<void>
) { ) {
this.ignorePatterns = [ this.userIgnorePatterns = globsToRegexes(
CONFLICT_PATH_REGEX, // conflict paths need to be resolved before they can be synced again this.settings.getSettings().ignorePatterns,
...globsToRegexes( this.logger
this.settings.getSettings().ignorePatterns, );
this.logger
)
];
this.settings.onSettingsChanged.add((newSettings) => { this.settings.onSettingsChanged.add((newSettings) => {
this.ignorePatterns = [ this.userIgnorePatterns = globsToRegexes(
CONFLICT_PATH_REGEX, newSettings.ignorePatterns,
...globsToRegexes(newSettings.ignorePatterns, this.logger) this.logger
]; );
}); });
initialState ??= {}; initialState ??= {};
@ -94,19 +99,35 @@ export class SyncEventQueue {
this._lastSeenUpdateId.add(id); this._lastSeenUpdateId.add(id);
} }
/**
* Toggle whether `CONFLICT_PATH_REGEX` filters incoming events. The
* offline scan flips this off so a stranded conflict file gets surfaced
* as a regular create; everywhere else conflict files stay ignored.
*/
public setIgnoreConflictPaths(ignore: boolean): void {
this.ignoreConflictPaths = ignore;
}
public async enqueue(input: FileSyncEvent): Promise<void> { public async enqueue(input: FileSyncEvent): Promise<void> {
const path = const path =
input.type === SyncEventType.RemoteChange input.type === SyncEventType.RemoteChange
? input.remoteVersion.relativePath ? input.remoteVersion.relativePath
: input.path; : input.path;
if (this.ignorePatterns.some((pattern) => pattern.test(path))) { if (this.userIgnorePatterns.some((pattern) => pattern.test(path))) {
this.logger.info( this.logger.info(
`Ignoring ${input.type} for ${path} as it matches ignore patterns` `Ignoring ${input.type} for ${path} as it matches ignore patterns`
); );
return; return;
} }
if (this.ignoreConflictPaths && CONFLICT_PATH_REGEX.test(path)) {
this.logger.info(
`Ignoring ${input.type} for ${path} as it is a conflict path`
);
return;
}
if (input.type === SyncEventType.RemoteChange) { if (input.type === SyncEventType.RemoteChange) {
this.events.push(input); this.events.push(input);
return; return;
@ -198,11 +219,23 @@ export class SyncEventQueue {
/** /**
* Update the settled document map and persist the new document version. * Update the settled document map and persist the new document version.
*
* If the document is already tracked under a different path (e.g. after a
* rename) the old entry is removed so the map stays keyed by the latest
* disk path and `getDocumentByDocumentId` can't return a stale match.
*/ */
public async setDocument( public async setDocument(
path: RelativePath, path: RelativePath,
record: DocumentRecord record: DocumentRecord
): Promise<void> { ): Promise<void> {
for (const [existingPath, existingRecord] of this.documents) {
if (
existingPath !== path &&
existingRecord.documentId === record.documentId
) {
this.documents.delete(existingPath);
}
}
this.documents.set(path, record); this.documents.set(path, record);
return this.save(); return this.save();
} }

View file

@ -45,7 +45,6 @@ export class Syncer {
private readonly queue: SyncEventQueue; private readonly queue: SyncEventQueue;
private _isFirstSyncStarted = false;
private runningScheduleSyncForOfflineChanges: Promise<void> | undefined; private runningScheduleSyncForOfflineChanges: Promise<void> | undefined;
private drainPromise: Promise<void> | undefined; private drainPromise: Promise<void> | undefined;
private isScanning = false; private isScanning = false;
@ -75,10 +74,6 @@ export class Syncer {
); );
} }
public get isFirstSyncStarted(): boolean {
return this._isFirstSyncStarted;
}
public syncLocallyCreatedFile(relativePath: RelativePath): void { public syncLocallyCreatedFile(relativePath: RelativePath): void {
void this.queue.enqueue({ void this.queue.enqueue({
type: SyncEventType.LocalCreate, type: SyncEventType.LocalCreate,
@ -121,8 +116,6 @@ export class Syncer {
}); });
this.ensureDraining(); this.ensureDraining();
this._isFirstSyncStarted = true;
} }
public async scheduleSyncForOfflineChanges(): Promise<void> { public async scheduleSyncForOfflineChanges(): Promise<void> {
@ -160,7 +153,6 @@ export class Syncer {
} }
public reset(): void { public reset(): void {
this._isFirstSyncStarted = false;
this.queue.clearPending(); this.queue.clearPending();
const current = this.runningScheduleSyncForOfflineChanges; const current = this.runningScheduleSyncForOfflineChanges;
if (current !== undefined) { if (current !== undefined) {
@ -184,6 +176,10 @@ export class Syncer {
private async internalScheduleSyncForOfflineChanges(): Promise<void> { private async internalScheduleSyncForOfflineChanges(): Promise<void> {
this.isScanning = true; this.isScanning = true;
// Surface stranded conflict files (e.g. ones we displaced in a prior
// session and never resynced) as regular creates during the scan; the
// queue re-enables conflict filtering when we're done.
this.queue.setIgnoreConflictPaths(false);
try { try {
while (this.drainPromise !== undefined) { while (this.drainPromise !== undefined) {
await this.drainPromise; await this.drainPromise;
@ -203,6 +199,7 @@ export class Syncer {
} }
); );
} finally { } finally {
this.queue.setIgnoreConflictPaths(true);
this.isScanning = false; this.isScanning = false;
} }