claude claims it woorks
This commit is contained in:
parent
8e87537e49
commit
9151e0b2d6
9 changed files with 582 additions and 63 deletions
|
|
@ -0,0 +1,185 @@
|
|||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert";
|
||||
import { Logger } from "../tracing/logger";
|
||||
import { Settings } from "../persistence/settings";
|
||||
import { STORED_STATE_SCHEMA_VERSION, SyncEventQueue } from "./sync-event-queue";
|
||||
import { scheduleOfflineChanges } from "./offline-change-detector";
|
||||
import type { FileOperations } from "../file-operations/file-operations";
|
||||
import type { RelativePath } from "./types";
|
||||
|
||||
const makeQueue = async (): Promise<SyncEventQueue> => {
|
||||
const logger = new Logger();
|
||||
const settings = new Settings(logger, {}, async () => {
|
||||
/* no-op */
|
||||
});
|
||||
return new SyncEventQueue(
|
||||
settings,
|
||||
logger,
|
||||
{ schemaVersion: STORED_STATE_SCHEMA_VERSION },
|
||||
async () => {
|
||||
/* no-op */
|
||||
}
|
||||
);
|
||||
};
|
||||
|
||||
const makeOperations = (
|
||||
files: Record<string, Uint8Array>
|
||||
): FileOperations => {
|
||||
return {
|
||||
listFilesRecursively: async () => Object.keys(files),
|
||||
read: async (path: RelativePath) => {
|
||||
const data = files[path];
|
||||
if (data === undefined) {
|
||||
throw new Error(`File not found: ${path}`);
|
||||
}
|
||||
return data;
|
||||
}
|
||||
} as unknown as FileOperations;
|
||||
};
|
||||
|
||||
describe("scheduleOfflineChanges", () => {
|
||||
it("does not bind a local file to a placement-pending record whose remoteRelativePath was persisted before the doc moved on the server", async () => {
|
||||
// The bug: persisted byDocId can carry a placement-pending record
|
||||
// whose `remoteRelativePath` was saved before the doc was moved
|
||||
// server-side. After restart, offline-scan running before WS
|
||||
// catch-up would bind an unrelated local file at that stale path
|
||||
// to the moved doc and push the user's content as an update —
|
||||
// silently corrupting the moved doc and stranding the local file.
|
||||
const queue = await makeQueue();
|
||||
|
||||
// Stale placement-pending record: server has moved this doc
|
||||
// away from "stale-X.md" since this snapshot was saved.
|
||||
await queue.upsertRecord({
|
||||
documentId: "MOVED-DOC",
|
||||
parentVersionId: 5,
|
||||
remoteRelativePath: "stale-X.md" as RelativePath,
|
||||
remoteHash: "hash-from-old-state",
|
||||
localPath: undefined
|
||||
});
|
||||
|
||||
// User has an unrelated local file at the stale path.
|
||||
const operations = makeOperations({
|
||||
"stale-X.md": new TextEncoder().encode(
|
||||
"user's unrelated local content"
|
||||
)
|
||||
});
|
||||
|
||||
const enqueued: { kind: string; path: string }[] = [];
|
||||
await scheduleOfflineChanges(
|
||||
new Logger(),
|
||||
operations,
|
||||
queue,
|
||||
(path) => enqueued.push({ kind: "create", path }),
|
||||
(args) => enqueued.push({ kind: "update", path: args.relativePath }),
|
||||
(path) => enqueued.push({ kind: "delete", path })
|
||||
);
|
||||
|
||||
// The local file must become a fresh CREATE — never a hostile
|
||||
// UPDATE on the moved doc.
|
||||
assert.deepStrictEqual(enqueued, [
|
||||
{ kind: "create", path: "stale-X.md" }
|
||||
]);
|
||||
|
||||
// The placement-pending record must remain placement-pending —
|
||||
// its localPath must not have been bound to the unrelated user
|
||||
// file. The reconciler will place it correctly once WS catch-up
|
||||
// updates `remoteRelativePath` to the doc's current location.
|
||||
const record = queue.getDocumentByDocumentId("MOVED-DOC");
|
||||
assert.notStrictEqual(record, undefined);
|
||||
assert.strictEqual(record?.localPath, undefined);
|
||||
});
|
||||
|
||||
it("schedules an update for a local file that matches a settled record's localPath", async () => {
|
||||
const queue = await makeQueue();
|
||||
await queue.upsertRecord({
|
||||
documentId: "SETTLED-DOC",
|
||||
parentVersionId: 2,
|
||||
remoteRelativePath: "doc.md" as RelativePath,
|
||||
remoteHash: "hash",
|
||||
localPath: "doc.md" as RelativePath
|
||||
});
|
||||
|
||||
const operations = makeOperations({
|
||||
"doc.md": new TextEncoder().encode("content")
|
||||
});
|
||||
|
||||
const enqueued: { kind: string; path: string }[] = [];
|
||||
await scheduleOfflineChanges(
|
||||
new Logger(),
|
||||
operations,
|
||||
queue,
|
||||
(path) => enqueued.push({ kind: "create", path }),
|
||||
(args) => enqueued.push({ kind: "update", path: args.relativePath }),
|
||||
(path) => enqueued.push({ kind: "delete", path })
|
||||
);
|
||||
|
||||
assert.deepStrictEqual(enqueued, [
|
||||
{ kind: "update", path: "doc.md" }
|
||||
]);
|
||||
});
|
||||
|
||||
it("schedules a delete for a settled record whose local file is missing", async () => {
|
||||
const queue = await makeQueue();
|
||||
await queue.upsertRecord({
|
||||
documentId: "VANISHED-DOC",
|
||||
parentVersionId: 4,
|
||||
remoteRelativePath: "gone.md" as RelativePath,
|
||||
remoteHash: "hash",
|
||||
localPath: "gone.md" as RelativePath
|
||||
});
|
||||
|
||||
const operations = makeOperations({});
|
||||
|
||||
const enqueued: { kind: string; path: string }[] = [];
|
||||
await scheduleOfflineChanges(
|
||||
new Logger(),
|
||||
operations,
|
||||
queue,
|
||||
(path) => enqueued.push({ kind: "create", path }),
|
||||
(args) => enqueued.push({ kind: "update", path: args.relativePath }),
|
||||
(path) => enqueued.push({ kind: "delete", path })
|
||||
);
|
||||
|
||||
assert.deepStrictEqual(enqueued, [
|
||||
{ kind: "delete", path: "gone.md" }
|
||||
]);
|
||||
});
|
||||
|
||||
it("detects an offline rename when an untracked file matches a deleted record's content hash", async () => {
|
||||
const queue = await makeQueue();
|
||||
const content = new TextEncoder().encode("body");
|
||||
const contentHash = await (await import("../utils/hash")).hash(content);
|
||||
|
||||
await queue.upsertRecord({
|
||||
documentId: "DOC-1",
|
||||
parentVersionId: 5,
|
||||
remoteRelativePath: "old.md" as RelativePath,
|
||||
remoteHash: contentHash,
|
||||
localPath: "old.md" as RelativePath
|
||||
});
|
||||
const operations = makeOperations({ "new.md": content });
|
||||
|
||||
const enqueued: {
|
||||
kind: string;
|
||||
path: string;
|
||||
oldPath?: string;
|
||||
}[] = [];
|
||||
await scheduleOfflineChanges(
|
||||
new Logger(),
|
||||
operations,
|
||||
queue,
|
||||
(path) => enqueued.push({ kind: "create", path }),
|
||||
(args) =>
|
||||
enqueued.push({
|
||||
kind: "update",
|
||||
path: args.relativePath,
|
||||
oldPath: args.oldPath
|
||||
}),
|
||||
(path) => enqueued.push({ kind: "delete", path })
|
||||
);
|
||||
|
||||
assert.deepStrictEqual(enqueued, [
|
||||
{ kind: "update", path: "new.md", oldPath: "old.md" }
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -10,6 +10,17 @@ import { removeFromArray } from "../utils/remove-from-array";
|
|||
* Scans the local filesystem and the document database to determine
|
||||
* which files were created, updated, moved, or deleted while the
|
||||
* client was offline, then enqueues the appropriate sync events.
|
||||
*
|
||||
* Placement-pending records (`localPath === undefined`) are deliberately
|
||||
* NOT bound to local files at the same `remoteRelativePath` here. The
|
||||
* persisted byDocId snapshot can be stale — a doc's server-side path
|
||||
* may have changed since the last save, so binding by stored path would
|
||||
* fold an unrelated user file into a moved doc and silently corrupt it.
|
||||
* Local files at those paths fall through to the LocalCreate flow below;
|
||||
* the server's create_document handler dedupes by path+freshness when
|
||||
* the doc really is at that path, and otherwise creates a new doc that
|
||||
* the reconciler places correctly once catch-up updates the stale
|
||||
* record's `remoteRelativePath`.
|
||||
*/
|
||||
export async function scheduleOfflineChanges(
|
||||
logger: Logger,
|
||||
|
|
@ -30,28 +41,6 @@ export async function scheduleOfflineChanges(
|
|||
// next pass.
|
||||
const allDocuments = queue.allSettledDocuments();
|
||||
|
||||
// Placement-pending records (`localPath === undefined`) name a server
|
||||
// path that the reconciler will eventually place. If the user already
|
||||
// has a local file at that path — common after a sync-disable or
|
||||
// reset that discarded a successful create's response, leaving the
|
||||
// server-known doc as a placement-pending record once catch-up
|
||||
// re-delivered it — treating it as an untracked file would
|
||||
// re-create a duplicate doc at the server's deconflicted path. Bind
|
||||
// each placement-pending record to its on-disk file: a same-hash
|
||||
// file just inherits the record's localPath; a different-hash file
|
||||
// is folded into the sync-up update flow below (an UPDATE on the
|
||||
// existing doc rather than a fresh CREATE).
|
||||
for (const record of queue.allRecords()) {
|
||||
if (record.localPath !== undefined) {
|
||||
continue;
|
||||
}
|
||||
if (!allLocalFiles.has(record.remoteRelativePath)) {
|
||||
continue;
|
||||
}
|
||||
await queue.setLocalPath(record.documentId, record.remoteRelativePath);
|
||||
allDocuments.set(record.remoteRelativePath, record);
|
||||
}
|
||||
|
||||
// A doc is "possibly deleted" only if it has no local file. Including
|
||||
// docs that still exist locally would queue a spurious delete alongside
|
||||
// the update below.
|
||||
|
|
@ -75,6 +64,13 @@ export async function scheduleOfflineChanges(
|
|||
for (const localFile of allLocalFiles) {
|
||||
if (allDocuments.has(localFile)) {
|
||||
syncedLocalFiles.push(localFile);
|
||||
} else if (queue.hasPendingCreateForPath(localFile)) {
|
||||
// A LocalCreate for this path is still in flight (no
|
||||
// record yet — its docId is a Promise). Re-enqueueing
|
||||
// would fire a second HTTP create that the server then
|
||||
// deconflicts to a sibling path, leaving the same bytes
|
||||
// in two docs. Skip; the in-flight create owns this slot.
|
||||
continue;
|
||||
} else {
|
||||
locallyPossibleCreatedFiles.push(localFile);
|
||||
}
|
||||
|
|
@ -131,6 +127,40 @@ export async function scheduleOfflineChanges(
|
|||
}
|
||||
|
||||
for (const path of syncedLocalFiles) {
|
||||
const record = allDocuments.get(path);
|
||||
if (
|
||||
record !== undefined &&
|
||||
record.localPath !== undefined &&
|
||||
record.localPath !== record.remoteRelativePath &&
|
||||
!allLocalFiles.has(record.remoteRelativePath) &&
|
||||
queue.byLocalPath.get(record.remoteRelativePath) === undefined
|
||||
) {
|
||||
// Lost local-rename recovery. The record's `localPath`
|
||||
// (where the user has the file now) and
|
||||
// `remoteRelativePath` (where the server still thinks it
|
||||
// lives) disagree, which means a queued user-rename's
|
||||
// LocalUpdate never reached the server before the queue
|
||||
// was wiped (typically a sync reset). Without this
|
||||
// branch the next `enqueueUpdate({ relativePath: path })`
|
||||
// is a content-only update — server keeps the doc at the
|
||||
// old path, the user's file at the new path orphans, and
|
||||
// other clients never see the rename. Replay the rename
|
||||
// by restoring the OLD localPath so the queue's enqueue
|
||||
// can find the record by `oldPath`, then enqueueUpdate
|
||||
// moves it back to the new path with `isUserRename`.
|
||||
// Only fires when the old slot is genuinely empty
|
||||
// (neither on disk nor claimed by another tracked
|
||||
// record) — otherwise the rename target is occupied and
|
||||
// we'd be confusing the byLocalPath index.
|
||||
const oldPath = record.remoteRelativePath;
|
||||
const newPath = record.localPath;
|
||||
logger.info(
|
||||
`Lost local rename detected: doc ${record.documentId} at ${oldPath} (server) vs ${newPath} (local); replaying rename to server`
|
||||
);
|
||||
await queue.setLocalPath(record.documentId, oldPath);
|
||||
enqueueUpdate({ oldPath, relativePath: newPath });
|
||||
continue;
|
||||
}
|
||||
logger.info(
|
||||
`File ${path} may have been updated while offline, scheduling sync to update it`
|
||||
);
|
||||
|
|
|
|||
|
|
@ -94,6 +94,7 @@ export class Reconciler {
|
|||
const allRecords = this.collectAllRecords();
|
||||
|
||||
const movesNeeded: PlannedMove[] = [];
|
||||
const deferredPlacements: DocumentRecord[] = [];
|
||||
|
||||
for (const record of allRecords) {
|
||||
if (record.localPath === record.remoteRelativePath) {
|
||||
|
|
@ -129,7 +130,7 @@ export class Reconciler {
|
|||
}
|
||||
|
||||
if (record.localPath === undefined) {
|
||||
await this.tryInitialPlacement(record);
|
||||
deferredPlacements.push(record);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -160,11 +161,35 @@ export class Reconciler {
|
|||
});
|
||||
}
|
||||
|
||||
if (movesNeeded.length === 0) {
|
||||
return;
|
||||
if (movesNeeded.length > 0) {
|
||||
await this.executeMoves(movesNeeded);
|
||||
}
|
||||
|
||||
await this.executeMoves(movesNeeded);
|
||||
// Run placements *after* moves so a placement whose target slot
|
||||
// was occupied by a tracked record at the start of the pass can
|
||||
// still succeed once that record's move frees the slot. Without
|
||||
// this ordering, a placement-pending record stalls until the
|
||||
// next reconciler tick — which only fires when new events
|
||||
// arrive, leaving the doc absent on disk if the queue happens
|
||||
// to be quiescent at that moment.
|
||||
for (const record of deferredPlacements) {
|
||||
// Re-check the gating conditions: a pending event may have
|
||||
// been enqueued for this doc while we were processing
|
||||
// moves above, and an interleaved placement would race
|
||||
// it.
|
||||
if (
|
||||
this.queue.hasPendingLocalEventsForDocumentId(record.documentId)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
if (this.queue.hasPendingServerDelete(record.documentId)) {
|
||||
continue;
|
||||
}
|
||||
if (record.localPath !== undefined) {
|
||||
continue;
|
||||
}
|
||||
await this.tryInitialPlacement(record);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -26,6 +26,22 @@ export class SyncEventQueue {
|
|||
(count: number) => unknown
|
||||
>();
|
||||
|
||||
// Fires whenever a record's `localPath` transitions to a different
|
||||
// value. Subscribers see every disk-side path change — watcher-
|
||||
// driven user renames, post-create deconflicts placed by the
|
||||
// reconciler, lost-rename replays in offline-scan, displacements
|
||||
// when another record claims a slot. Useful for callers that
|
||||
// mirror disk-side state (e.g. test harnesses that maintain a
|
||||
// "do-not-touch" list keyed by current path). Both `oldPath` and
|
||||
// `newPath` may be `undefined` (placement-pending state).
|
||||
public readonly onDocumentPathChanged = new EventListeners<
|
||||
(
|
||||
documentId: DocumentId,
|
||||
oldPath: RelativePath | undefined,
|
||||
newPath: RelativePath | undefined
|
||||
) => unknown
|
||||
>();
|
||||
|
||||
private readonly _lastSeenUpdateId: MinCovered;
|
||||
|
||||
// Primary index of every settled document, keyed by docId. The wire loop
|
||||
|
|
@ -837,13 +853,16 @@ export class SyncEventQueue {
|
|||
record: DocumentRecord,
|
||||
newLocalPath: RelativePath | undefined
|
||||
): void {
|
||||
const previousLocalPath = record.localPath;
|
||||
if (
|
||||
record.localPath !== undefined &&
|
||||
this._byLocalPath.get(record.localPath) === record
|
||||
previousLocalPath !== undefined &&
|
||||
this._byLocalPath.get(previousLocalPath) === record
|
||||
) {
|
||||
this._byLocalPath.delete(record.localPath);
|
||||
this._byLocalPath.delete(previousLocalPath);
|
||||
}
|
||||
record.localPath = newLocalPath;
|
||||
let displacedRecord: DocumentRecord | undefined;
|
||||
let displacedOldPath: RelativePath | undefined;
|
||||
if (newLocalPath !== undefined) {
|
||||
const displaced = this._byLocalPath.get(newLocalPath);
|
||||
if (displaced !== undefined && displaced !== record) {
|
||||
|
|
@ -851,10 +870,26 @@ export class SyncEventQueue {
|
|||
// We're about to overwrite that slot, so clear the
|
||||
// displaced record's localPath; the reconciler will
|
||||
// re-place it via tryInitialPlacement on the next pass.
|
||||
displacedOldPath = displaced.localPath;
|
||||
displaced.localPath = undefined;
|
||||
displacedRecord = displaced;
|
||||
}
|
||||
this._byLocalPath.set(newLocalPath, record);
|
||||
}
|
||||
if (previousLocalPath !== newLocalPath) {
|
||||
this.onDocumentPathChanged.trigger(
|
||||
record.documentId,
|
||||
previousLocalPath,
|
||||
newLocalPath
|
||||
);
|
||||
}
|
||||
if (displacedRecord !== undefined) {
|
||||
this.onDocumentPathChanged.trigger(
|
||||
displacedRecord.documentId,
|
||||
displacedOldPath,
|
||||
undefined
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private notifyPendingUpdateCountChanged(): void {
|
||||
|
|
|
|||
|
|
@ -506,12 +506,6 @@ export class Syncer {
|
|||
contentBytes
|
||||
});
|
||||
|
||||
// If the user renamed the file while the create request was in flight,
|
||||
// event.path now points at the renamed disk slot. Apply response bytes
|
||||
// and install the local record there; the queued LocalUpdate carries
|
||||
// the server-side rename intent.
|
||||
const localPath = event.path;
|
||||
|
||||
// Same-docId collapse. While our LocalCreate sat in the queue, a
|
||||
// RemoteCreate may have arrived for this same path. The wire-loop's
|
||||
// `processRemoteCreateForNewDocument` would have built a record with
|
||||
|
|
@ -523,8 +517,14 @@ export class Syncer {
|
|||
let remoteHash = contentHash;
|
||||
if (response.type === "MergingUpdate") {
|
||||
const responseBytes = base64ToBytes(response.contentBase64);
|
||||
// Read `event.path` live for both the write target and the
|
||||
// cache key. A user rename arriving between HTTP-send and
|
||||
// HTTP-response rewrites `event.path` via
|
||||
// `updatePendingCreatePath`; the merge write must land on
|
||||
// the current slot so the queued LocalUpdate that follows
|
||||
// sees the merged bytes.
|
||||
await this.operations.write(
|
||||
localPath,
|
||||
event.path,
|
||||
contentBytes,
|
||||
responseBytes
|
||||
);
|
||||
|
|
@ -532,13 +532,13 @@ export class Syncer {
|
|||
await this.updateCache(
|
||||
response.vaultUpdateId,
|
||||
responseBytes,
|
||||
localPath
|
||||
event.path
|
||||
);
|
||||
} else {
|
||||
await this.updateCache(
|
||||
response.vaultUpdateId,
|
||||
contentBytes,
|
||||
localPath
|
||||
event.path
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -548,6 +548,17 @@ export class Syncer {
|
|||
// path placement, if needed.)
|
||||
this.pendingPlacementContent.delete(response.documentId);
|
||||
|
||||
// Snapshot `event.path` only after the write has settled. The
|
||||
// write itself can drive synchronous watcher callbacks (e.g.
|
||||
// an atomic-update fileSystemOperations that fires a "file
|
||||
// changed" event back into the queue), and the test harness's
|
||||
// user-facing renames also race here. Either path mutates
|
||||
// `event.path` via `updatePendingCreatePath`; reading it once
|
||||
// up front would lock in a stale slot and leave
|
||||
// `record.localPath` pointing at a vacated path with no
|
||||
// LocalRename ever materializing.
|
||||
const localPath = event.path;
|
||||
|
||||
await this.queue.resolveCreate(event, {
|
||||
documentId: response.documentId,
|
||||
parentVersionId: response.vaultUpdateId,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue