diff --git a/CLAUDE.md b/CLAUDE.md index 39161e39..2caab8dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,16 +7,15 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co VaultLink is a self-hosted Obsidian file-sync system. Two halves of one repo: - `sync-server/` — Rust (axum + sqlx/SQLite). Source of truth for vault state, broadcasts changes via WebSocket. -- `frontend/` — npm workspaces. The sync engine (`sync-client`) is consumed by an Obsidian plugin, a standalone CLI, a fuzz E2E harness, a scripted determinism harness, and a history UI. +- `frontend/` — npm workspaces. The sync engine (`sync-client`) is consumed by an Obsidian plugin, a standalone CLI, a fuzz E2E harness, and a scripted determinism harness. -The HTTP/WS API types are generated from Rust (`ts-rs`) and mirrored into the TS workspaces. **Never hand-edit files in `frontend/sync-client/src/services/types/` or `frontend/history-ui/src/lib/types/`** — run `scripts/update-api-types.sh` after changing anything Serde-derived in the server. +The HTTP/WS API types are generated from Rust (`ts-rs`) and mirrored into the TS workspaces. **Never hand-edit files in `frontend/sync-client/src/services/types/`** — run `scripts/update-api-types.sh` after changing anything Serde-derived in the server. ### Frontend workspaces - `sync-client` — the sync engine; published to consumers via `dist/`. All other TS workspaces depend on it via `file:../sync-client`. - `obsidian-plugin` — Obsidian plugin built from `sync-client`. - `local-client-cli` — same engine wrapped as a standalone CLI. -- `history-ui` — vault-history web UI. - `test-client` — fuzz E2E harness (random ops across N processes). - `deterministic-tests` — scripted multi-client tests with an in-memory FS, run against a real server. @@ -67,7 +66,7 @@ Frontend dev (sync-client + obsidian-plugin watch in parallel): cd frontend && npm install && npm run dev ``` -Regenerate TS bindings from Rust types (touches `frontend/{sync-client,history-ui}/src/.../types/`): +Regenerate TS bindings from Rust types (touches `frontend/sync-client/src/services/types/`): ```sh scripts/update-api-types.sh @@ -119,7 +118,7 @@ Local FS events from the watcher update `localPath` synchronously at enqueue tim **Watermark.** `lastSeenUpdateId` uses a `MinCovered` (a contiguous-prefix tracker over a stream of integers): we only advance the published min when the next consecutive id has been processed, so out-of-order RemoteChange ids don't fool the WebSocket handshake into requesting a too-recent catch-up. -**Server catch-up.** The server's WS handshake replays events newer than the client's `last_seen_vault_update_id` from the `latest_document_versions` view (one row per doc, the latest). On those replayed rows `is_new_file` means _new to this client_ (`creation_vault_update_id > last_seen_vault_update_id`), not "this row is the doc's first version" — necessary because the catch-up only carries the latest version; if a doc was created and updated past the watermark, the client never sees its create otherwise. +**Server catch-up.** The server's WS handshake replays events newer than the client's `last_seen_vault_update_id`, computed as the latest version per document as of the cursor. The catch-up only carries each doc's *latest* version, not its full history. The client treats any RemoteChange whose `documentId` it has no record of as a fresh create and downloads the bytes. ## Edge-case patterns the sync engine has to survive @@ -135,8 +134,6 @@ The two-loop split defuses most of the old race catalogue (slot-collision stashe **Watermark advancement is load-bearing both ways.** Branches that _skip_ a remote event without advancing `lastSeenUpdateId` create permanent gaps that re-deliver forever. Branches that _advance_ without applying the content lose data: the server has no further event to re-deliver, the catch-up only carries the latest version, and any state in between is gone. Don't advance unless the event was actually applied (or deliberately discarded after weighing both halves). -**`isNewFile` semantics differ between catch-up and real-time.** On WS handshake replay it means _new to this client_ (`creation_vault_update_id > last_seen_vault_update_id`); on real-time broadcasts it means _this version is the create_ (`creation_vault_update_id == vault_update_id`). A handler that decides based on one interpretation will be wrong on the other channel; reasoning about fetch-and-treat-as-new vs. ignore needs to know which channel delivered the event. - **Pause / disable-sync mid-flight** is the one race the new model doesn't structurally fix. An HTTP that committed server-side but whose response was discarded leaves the server holding a doc the client has no record of. Resume → offline scan → server-side dedupe handles it (the server merges the duplicate create into the existing doc), but if the merge produces a deconflict, the client picks up an extra file. Out of scope for the two-loop split. **Cycle reconciliation uses in-memory content swap.** When the move graph contains a cycle, the reconciler reads every file in the cycle into memory and writes each back to its new slot, with no tmp files. A write-ahead marker at `.vaultlink/swap-.json` lists each leg; on startup the reconciler reads the marker, hashes each `from` to determine which legs ran, and replays the rest. The `.vaultlink/**` glob is hard-coded as an internal ignore pattern so swap markers don't get sync'd. diff --git a/frontend/deterministic-tests/README.md b/frontend/deterministic-tests/README.md index 487c7e1c..a420c1c0 100644 --- a/frontend/deterministic-tests/README.md +++ b/frontend/deterministic-tests/README.md @@ -89,18 +89,19 @@ export const myScenarioTest: TestDefinition = { The `verify` callback receives an `AssertableState` object with chainable assertion methods: ```typescript -s.assertFileCount(n); // exact file count -s.assertFileExists("path"); // file must exist -s.assertFileNotExists("path"); // file must not exist -s.assertContent("path", "expected"); // exact content match -s.assertContains("path", "a", "b"); // all substrings present in file -s.assertContainsAny("path", "a", "b"); // at least one substring present -s.assertAnyFileContains("text"); // substring present in some file -s.assertNoFileContains("text"); // substring absent from every file -s.assertSubstringCount("path", "x", 3); // substring appears exactly N times -s.assertContentInAtMostOneFile("text"); // no duplicate content -s.ifFileExists("path", (s) => { /* … */ }); // conditional block -s.getContent("path"); // raw content (or "" if missing) +s.assertFileCount(n); // exact file count +s.assertFileExists("path"); // file must exist +s.assertFileNotExists("path"); // file must not exist +s.assertContent("path", "expected"); // exact content match +s.assertContains("path", "a", "b"); // all substrings present in file +s.assertContainsAny("path", "a", "b"); // at least one substring present +s.assertAnyFileContains("text"); // substring present in some file +s.assertNoFileContains("text"); // substring absent from every file +s.assertContentInAtMostOneFile("text"); // no duplicate content +s.ifFileExists("path", (s) => { + /* … */ +}); // conditional block +s.getContent("path"); // raw content (or "" if missing) ``` 2. Register it in `src/test-registry.ts`: diff --git a/frontend/deterministic-tests/src/cli.ts b/frontend/deterministic-tests/src/cli.ts index 6e15cac0..0beaca03 100644 --- a/frontend/deterministic-tests/src/cli.ts +++ b/frontend/deterministic-tests/src/cli.ts @@ -42,7 +42,7 @@ function testUsesPauseServer(test: TestDefinition): boolean { */ function findProjectRoot(): string { let dir = path.dirname(__filename); - const root = path.parse(dir).root; + const { root } = path.parse(dir); while (dir !== root) { if ( fs.existsSync(path.join(dir, "sync-server")) && diff --git a/frontend/deterministic-tests/src/deterministic-agent.ts b/frontend/deterministic-tests/src/deterministic-agent.ts index b32b01c2..08baef59 100644 --- a/frontend/deterministic-tests/src/deterministic-agent.ts +++ b/frontend/deterministic-tests/src/deterministic-agent.ts @@ -37,15 +37,15 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { private readonly wsFactory = new ManagedWebSocketFactory(); private nextWriteRename: | { - oldPath: RelativePath; - newPath: RelativePath; - } + oldPath: RelativePath; + newPath: RelativePath; + } | undefined; private nextCreateResponseDrop: | { - dropped: Promise; - resolveDropped: () => void; - } + dropped: Promise; + resolveDropped: () => void; + } | undefined; public constructor( @@ -59,6 +59,26 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { this.data.settings = { ...initialSettings }; } + private static isCreateDocumentRequest( + input: RequestInfo | URL, + init: RequestInit | undefined + ): boolean { + const method = + init?.method ?? + (typeof Request !== "undefined" && input instanceof Request + ? input.method + : "GET"); + if (method.toUpperCase() !== "POST") { + return false; + } + + const url = + input instanceof URL + ? input + : new URL(typeof input === "string" ? input : input.url); + return /\/documents\/?$/.test(url.pathname); + } + public async init( fetchImplementation: typeof globalThis.fetch ): Promise { @@ -118,13 +138,12 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { this.nextCreateResponseDrop === undefined, `Client ${this.clientId} already has a create response drop armed` ); - let resolveDropped!: () => void; - const dropped = new Promise((resolve) => { - resolveDropped = resolve; - }); + const resolvers = Promise.withResolvers(); this.nextCreateResponseDrop = { - dropped, - resolveDropped + dropped: resolvers.promise as Promise, + resolveDropped: (): void => { + resolvers.resolve(undefined); + } }; this.log("Armed next create response drop"); } @@ -155,9 +174,7 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { await withTimeout( new Promise((resolve) => { const unsubscribe = this.client.onSyncHistoryUpdated.add(() => { - const entry = this.client - .getHistoryEntries() - .find(matches); + const entry = this.client.getHistoryEntries().find(matches); if (entry === undefined) { return; } @@ -304,11 +321,8 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { }); } - const nextWriteRename = this.nextWriteRename; - if ( - nextWriteRename !== undefined && - nextWriteRename.oldPath === path - ) { + const { nextWriteRename } = this; + if (nextWriteRename?.oldPath === path) { this.nextWriteRename = undefined; await super.rename( nextWriteRename.oldPath, @@ -460,24 +474,4 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { return response; }; } - - private static isCreateDocumentRequest( - input: RequestInfo | URL, - init: RequestInit | undefined - ): boolean { - const method = - init?.method ?? - (typeof Request !== "undefined" && input instanceof Request - ? input.method - : "GET"); - if (method.toUpperCase() !== "POST") { - return false; - } - - const url = - input instanceof URL - ? input - : new URL(typeof input === "string" ? input : input.url); - return /\/documents\/?$/.test(url.pathname); - } } diff --git a/frontend/deterministic-tests/src/server-control.ts b/frontend/deterministic-tests/src/server-control.ts index 9cb4cde0..62475779 100644 --- a/frontend/deterministic-tests/src/server-control.ts +++ b/frontend/deterministic-tests/src/server-control.ts @@ -46,7 +46,7 @@ export class ServerControl { // Retry on bind failure: findFreePort closes its probe before we // spawn, so under heavy parallelism another process can grab the // same port. Each attempt picks a fresh port. - let lastError: unknown; + let lastError: unknown = undefined; for (let attempt = 1; attempt <= SERVER_START_MAX_ATTEMPTS; attempt++) { try { await this.startOnce(); @@ -65,69 +65,6 @@ export class ServerControl { ); } - private async startOnce(): Promise { - const reservation = await findFreePort(); - this._port = reservation.port; - const tmpBase = os.tmpdir(); - this.tempDir = fs.mkdtempSync(path.join(tmpBase, "vault-link-test-")); - const tempConfigPath = path.join(this.tempDir, "config.yml"); - const dbDir = path.join(this.tempDir, "databases"); - - this.writeConfigFile(tempConfigPath, dbDir); - - this.logger.info( - `Starting server: ${this.serverPath} (port ${this._port})` - ); - - // Release the port reservation right before spawning to minimize - // the TOCTOU window between port discovery and server binding. - reservation.release(); - - this.process = spawn(this.serverPath, [tempConfigPath], { - stdio: ["ignore", "pipe", "pipe"], - detached: false - }); - - this.process.stdout?.on("data", (data: Buffer) => { - this.logger.info(`[SERVER] ${data.toString().trim()}`); - }); - - this.process.stderr?.on("data", (data: Buffer) => { - this.logger.info(`[SERVER] ${data.toString().trim()}`); - }); - - this.process.on("error", (err) => { - this.logger.error(`[SERVER] Process error: ${err.message}`); - }); - - const currentProcess = this.process; - currentProcess.on("exit", (code, signal) => { - this.logger.info( - `Server exited with code ${code}, signal ${signal}` - ); - // Only clear state if this handler is for the current process. - // A fast stop→start cycle could create a new process before this - // handler fires — clearing state here would corrupt the new one. - if (this.process === currentProcess) { - this.process = null; - this._isPaused = false; - } - }); - - try { - await this.waitForReady(); - } catch (error) { - // Kill the spawned process if it failed to become ready, - // preventing a zombie process from lingering. - try { - await this.stop(); - } catch { - // Best-effort cleanup - } - throw error; - } - } - public async waitForReady( maxAttempts: number = SERVER_READY_MAX_ATTEMPTS ): Promise { @@ -239,8 +176,7 @@ export class ServerControl { public isRunning(): boolean { const proc = this.process; return ( - proc !== null && - proc.pid !== undefined && + proc?.pid !== undefined && proc.exitCode === null && proc.signalCode === null ); @@ -269,6 +205,69 @@ export class ServerControl { } } + private async startOnce(): Promise { + const reservation = await findFreePort(); + this._port = reservation.port; + const tmpBase = os.tmpdir(); + this.tempDir = fs.mkdtempSync(path.join(tmpBase, "vault-link-test-")); + const tempConfigPath = path.join(this.tempDir, "config.yml"); + const dbDir = path.join(this.tempDir, "databases"); + + this.writeConfigFile(tempConfigPath, dbDir); + + this.logger.info( + `Starting server: ${this.serverPath} (port ${this._port})` + ); + + // Release the port reservation right before spawning to minimize + // the TOCTOU window between port discovery and server binding. + reservation.release(); + + this.process = spawn(this.serverPath, [tempConfigPath], { + stdio: ["ignore", "pipe", "pipe"], + detached: false + }); + + this.process.stdout?.on("data", (data: Buffer) => { + this.logger.info(`[SERVER] ${data.toString().trim()}`); + }); + + this.process.stderr?.on("data", (data: Buffer) => { + this.logger.info(`[SERVER] ${data.toString().trim()}`); + }); + + this.process.on("error", (err) => { + this.logger.error(`[SERVER] Process error: ${err.message}`); + }); + + const currentProcess = this.process; + currentProcess.on("exit", (code, signal) => { + this.logger.info( + `Server exited with code ${code}, signal ${signal}` + ); + // Only clear state if this handler is for the current process. + // A fast stop→start cycle could create a new process before this + // handler fires — clearing state here would corrupt the new one. + if (this.process === currentProcess) { + this.process = null; + this._isPaused = false; + } + }); + + try { + await this.waitForReady(); + } catch (error) { + // Kill the spawned process if it failed to become ready, + // preventing a zombie process from lingering. + try { + await this.stop(); + } catch { + // Best-effort cleanup + } + throw error; + } + } + private writeConfigFile(destPath: string, dbDir: string): void { // Assumes config-e2e.yml has exactly one 2-space-indented `port:` and // one `databases_directory_path:` (under `server:` and `database:` diff --git a/frontend/deterministic-tests/src/test-registry.ts b/frontend/deterministic-tests/src/test-registry.ts index 2ecd7d37..dfe267d2 100644 --- a/frontend/deterministic-tests/src/test-registry.ts +++ b/frontend/deterministic-tests/src/test-registry.ts @@ -103,6 +103,7 @@ import { renamedPendingCreateReusedPathThenDeleteTest } from "./tests/renamed-pe import { renamePendingCreateOntoPendingDeletePathTest } from "./tests/rename-pending-create-onto-pending-delete-path.test"; import { remoteQuickWriteRenameBeforeRecordTest } from "./tests/remote-quick-write-rename-before-record.test"; import { selfMergePendingRenameAliasesSecondCreateTest } from "./tests/self-merge-pending-rename-aliases-second-create.test"; +import { disableMidCreateThenDeleteTest } from "./tests/disable-mid-create-then-delete.test"; export const TESTS: Partial> = { "rename-create-conflict": renameCreateConflictTest, @@ -239,5 +240,6 @@ export const TESTS: Partial> = { "remote-quick-write-rename-before-record": remoteQuickWriteRenameBeforeRecordTest, "self-merge-pending-rename-aliases-second-create": - selfMergePendingRenameAliasesSecondCreateTest + selfMergePendingRenameAliasesSecondCreateTest, + "disable-mid-create-then-delete": disableMidCreateThenDeleteTest }; diff --git a/frontend/deterministic-tests/src/test-runner.ts b/frontend/deterministic-tests/src/test-runner.ts index 411e9b08..2bb29704 100644 --- a/frontend/deterministic-tests/src/test-runner.ts +++ b/frontend/deterministic-tests/src/test-runner.ts @@ -1,7 +1,7 @@ import type { TestDefinition, TestResult, TestStep } from "./test-definition"; import { DeterministicAgent } from "./deterministic-agent"; import type { ServerControl } from "./server-control"; -import type { SyncSettings, Logger } from "sync-client"; +import { SyncType, type SyncSettings, type Logger } from "sync-client"; import { assert } from "./utils/assert"; import { AssertableState } from "./utils/assertable-state"; import { sleep } from "./utils/sleep"; @@ -188,9 +188,11 @@ export class TestRunner { const agent = this.getAgent(step.client); const historySeen = agent.waitForHistoryEntry( (entry) => - entry.details.type === step.syncType && + entry.details.type === SyncType[step.syncType] && entry.details.relativePath === step.path, - () => this.serverControl.pause() + () => { + this.serverControl.pause(); + } ); this.serverControl.resume(); await historySeen; diff --git a/frontend/deterministic-tests/src/tests/catchup-create-and-update-not-skipped.test.ts b/frontend/deterministic-tests/src/tests/catchup-create-and-update-not-skipped.test.ts index 2d40228f..675deaeb 100644 --- a/frontend/deterministic-tests/src/tests/catchup-create-and-update-not-skipped.test.ts +++ b/frontend/deterministic-tests/src/tests/catchup-create-and-update-not-skipped.test.ts @@ -5,14 +5,12 @@ export const catchupCreateAndUpdateNotSkippedTest: TestDefinition = { description: "Client 1 disconnects (sync disabled). Client 0 creates a doc and " + "then updates it. When Client 1 reconnects, the server's catch-up " + - "stream sends only the doc's *latest* version (the update), not the " + - "full history. Pre-fix the wire's `is_new_file` was set to " + - "`creation == latest_version`, so the catch-up flagged the doc as " + - "non-new even though Client 1 had never seen its creation. Client " + - "1's `processRemoteChange` then dropped it as a 'stale RemoteChange " + - "for untracked, non-new document' and the doc was silently lost. " + - "Post-fix `is_new_file` in the catch-up stream means 'new relative " + - "to the recipient's watermark' (`creation > last_seen_vault_update_id`).", + "stream sends only the doc's *latest* version (the update), not " + + "the full history. Client 1 must still pick up the doc — any handler " + + "that gates the create-on-untracked path on a server-supplied " + + "'is this the first version' flag would drop it (the latest version " + + "is not the create), silently leaking the doc. The client treats " + + "every untracked-doc RemoteChange as a fresh create.", clients: 2, steps: [ { type: "enable-sync", client: 0 }, @@ -36,7 +34,7 @@ export const catchupCreateAndUpdateNotSkippedTest: TestDefinition = { // Client 0 updates the doc (vault_update_id v_X > v_C). The // server's `latest_document_versions` view now returns the - // *update* row — its `creation_vault_update_id != vault_update_id`. + // *update* row — the create row is no longer the latest. { type: "update", client: 0, @@ -46,10 +44,9 @@ export const catchupCreateAndUpdateNotSkippedTest: TestDefinition = { { type: "sync", client: 0 }, // Client 1 reconnects. Server's catch-up replays docs with - // `vault_update_id > last_seen`. For doc.md it sends v_X with - // `is_new_file` derived from `creation_vault_update_id > - // last_seen_vault_update_id` (post-fix) — so Client 1 treats it - // as a fresh create and downloads the latest content. + // `vault_update_id > last_seen`. For doc.md it sends v_X; Client + // 1 has no record of the doc, so it treats the RemoteChange as a + // fresh create and downloads the latest content. { type: "enable-sync", client: 1 }, { type: "barrier" }, diff --git a/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-create-first.test.ts b/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-create-first.test.ts index cd8046ce..719cde4d 100644 --- a/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-create-first.test.ts +++ b/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-create-first.test.ts @@ -1,49 +1,50 @@ import type { AssertableState } from "../utils/assertable-state"; import type { TestDefinition } from "../test-definition"; -export const concurrentRenameAndCreateAtTargetCreateFirstTest: TestDefinition = { - description: - "One client renames X to Y while another creates a new file at Y, " + - "both offline. After syncing, Y should contain merged content from " + - "both the renamed file and the newly created file.", - clients: 2, - steps: [ - { - type: "create", - client: 0, - path: "X.md", - content: "original file X" - }, - { type: "enable-sync", client: 0 }, - { type: "enable-sync", client: 1 }, - { type: "barrier" }, +export const concurrentRenameAndCreateAtTargetCreateFirstTest: TestDefinition = + { + description: + "One client renames X to Y while another creates a new file at Y, " + + "both offline. After syncing, Y should contain merged content from " + + "both the renamed file and the newly created file.", + clients: 2, + steps: [ + { + type: "create", + client: 0, + path: "X.md", + content: "original file X" + }, + { type: "enable-sync", client: 0 }, + { type: "enable-sync", client: 1 }, + { type: "barrier" }, - { type: "disable-sync", client: 0 }, - { type: "disable-sync", client: 1 }, + { type: "disable-sync", client: 0 }, + { type: "disable-sync", client: 1 }, - { type: "rename", client: 0, oldPath: "X.md", newPath: "Y.md" }, + { type: "rename", client: 0, oldPath: "X.md", newPath: "Y.md" }, - { - type: "create", - client: 1, - path: "Y.md", - content: "brand new Y content" - }, + { + type: "create", + client: 1, + path: "Y.md", + content: "brand new Y content" + }, - { type: "enable-sync", client: 1 }, - { type: "sync", client: 1 }, + { type: "enable-sync", client: 1 }, + { type: "sync", client: 1 }, - { type: "enable-sync", client: 0 }, - { type: "barrier" }, + { type: "enable-sync", client: 0 }, + { type: "barrier" }, - { - type: "assert-consistent", - verify: (state: AssertableState): void => { - state - .assertFileCount(2) - .assertContains("Y (1).md", "original file X") - .assertContains("Y.md", "brand new Y content"); + { + type: "assert-consistent", + verify: (state: AssertableState): void => { + state + .assertFileCount(2) + .assertContains("Y (1).md", "original file X") + .assertContains("Y.md", "brand new Y content"); + } } - } - ] -}; + ] + }; diff --git a/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-rename-first.test.ts b/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-rename-first.test.ts index 0ac0b721..a1ba9c2c 100644 --- a/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-rename-first.test.ts +++ b/frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-rename-first.test.ts @@ -1,52 +1,53 @@ import type { AssertableState } from "../utils/assertable-state"; import type { TestDefinition } from "../test-definition"; -export const concurrentRenameAndCreateAtTargetRenameFirstTest: TestDefinition = { - description: - "One client renames X to Y while another creates a new file at Y, " + - "both offline. We can't merge the create because it would result in a cycle", - clients: 2, - steps: [ - { - type: "create", - client: 0, - path: "X.md", - content: "original file X" - }, - { type: "enable-sync", client: 0 }, - { type: "enable-sync", client: 1 }, - { type: "barrier" }, +export const concurrentRenameAndCreateAtTargetRenameFirstTest: TestDefinition = + { + description: + "One client renames X to Y while another creates a new file at Y, " + + "both offline. We can't merge the create because it would result in a cycle", + clients: 2, + steps: [ + { + type: "create", + client: 0, + path: "X.md", + content: "original file X" + }, + { type: "enable-sync", client: 0 }, + { type: "enable-sync", client: 1 }, + { type: "barrier" }, - { type: "disable-sync", client: 0 }, - { type: "disable-sync", client: 1 }, + { type: "disable-sync", client: 0 }, + { type: "disable-sync", client: 1 }, - { type: "rename", client: 0, oldPath: "X.md", newPath: "Y.md" }, + { type: "rename", client: 0, oldPath: "X.md", newPath: "Y.md" }, - { - type: "create", - client: 1, - path: "Y.md", - content: "brand new Y content" - }, + { + type: "create", + client: 1, + path: "Y.md", + content: "brand new Y content" + }, - { type: "enable-sync", client: 0 }, - { type: "sync", client: 0 }, + { type: "enable-sync", client: 0 }, + { type: "sync", client: 0 }, - { type: "enable-sync", client: 1 }, - { type: "barrier" }, + { type: "enable-sync", client: 1 }, + { type: "barrier" }, - { - type: "assert-consistent", - verify: (state: AssertableState): void => { - state - .assertFileNotExists("X.md") - .assertFileExists("Y.md") - .assertFileExists("Y (1).md") - .assertAnyFileContains( - "original file X", - "brand new Y content" - ); + { + type: "assert-consistent", + verify: (state: AssertableState): void => { + state + .assertFileNotExists("X.md") + .assertFileExists("Y.md") + .assertFileExists("Y (1).md") + .assertAnyFileContains( + "original file X", + "brand new Y content" + ); + } } - } - ] -}; + ] + }; diff --git a/frontend/deterministic-tests/src/tests/disable-mid-create-then-delete.test.ts b/frontend/deterministic-tests/src/tests/disable-mid-create-then-delete.test.ts new file mode 100644 index 00000000..5ff1d529 --- /dev/null +++ b/frontend/deterministic-tests/src/tests/disable-mid-create-then-delete.test.ts @@ -0,0 +1,56 @@ +import type { AssertableState } from "../utils/assertable-state"; +import type { TestDefinition } from "../test-definition"; + +export const disableMidCreateThenDeleteTest: TestDefinition = { + description: + "Reproduces a fuzz failure where one client's create-then-delete-then-disable-sync " + + "sequence loses the file: the create commits server-side, the response is " + + "lost (sync reset), the local file is deleted, then sync is re-enabled. The " + + "catch-up replay should redeliver the create so both clients converge to " + + "having the file (the delete never reached the server because its docId " + + "Promise was rejected when the queue cleared).", + clients: 2, + steps: [ + // Client 0 is online (the witness); client 1 starts disabled. + { type: "enable-sync", client: 0 }, + + // Client 1 creates the file while offline so the LocalCreate is queued. + { type: "create", client: 1, path: "file-32.md", content: "hello" }, + + // Arm the drop so client 1's create POST commits server-side but the + // response is replaced with SyncResetError (matches the fuzz scenario + // where sync was disabled mid-flight). + { type: "drop-next-create-response", client: 1 }, + + // Enable sync on client 1: offline scan picks up file-32, drain fires + // POST /documents, server commits, broadcast goes out, response is + // dropped on the client. SyncResetError exits the drain leaving the + // create event still in the queue. + { type: "enable-sync", client: 1 }, + { type: "wait-for-dropped-create-response", client: 1 }, + + // The user then deletes the file locally and toggles sync off/on + // (the same flow the fuzz harness used). The disable's pause() + // does not clear the queue, but the re-enable runs an offline + // scan that calls clearPending() — wiping the dangling LocalCreate + // and any LocalDelete behind it. The local disk is empty, so + // nothing is enqueued. + { type: "delete", client: 1, path: "file-32.md" }, + { type: "disable-sync", client: 1 }, + { type: "enable-sync", client: 1 }, + + // Catch-up on the new WS connection should deliver file-32 (vault + // update id 1) since client 1's lastSeenUpdateId is still 0. + { type: "barrier" }, + + { + type: "assert-consistent", + verify: (s: AssertableState): void => { + s.assertFileExists("file-32.md").assertContent( + "file-32.md", + "hello" + ); + } + } + ] +}; diff --git a/frontend/deterministic-tests/src/utils/assertable-state.ts b/frontend/deterministic-tests/src/utils/assertable-state.ts index 7c6f192c..67a300af 100644 --- a/frontend/deterministic-tests/src/utils/assertable-state.ts +++ b/frontend/deterministic-tests/src/utils/assertable-state.ts @@ -106,22 +106,6 @@ export class AssertableState { return this; } - public assertSubstringCount( - path: string, - substring: string, - expected: number - ): this { - this.assertFileExists(path); - const content = this.files.get(path) ?? ""; - const actual = content.split(substring).length - 1; - if (actual !== expected) { - throw new Error( - `Expected "${substring}" to appear ${expected} time(s) in "${path}", found ${actual}. Content: "${content}"` - ); - } - return this; - } - public assertContentInAtMostOneFile(substring: string): this { const matches = Array.from(this.files.entries()).filter(([, content]) => content.includes(substring) @@ -143,8 +127,4 @@ export class AssertableState { } return this; } - - public getContent(path: string): string { - return this.files.get(path) ?? ""; - } } diff --git a/frontend/local-client-cli/src/args.test.ts b/frontend/local-client-cli/src/args.test.ts index fdf0b6c8..075f9446 100644 --- a/frontend/local-client-cli/src/args.test.ts +++ b/frontend/local-client-cli/src/args.test.ts @@ -169,7 +169,6 @@ test("parseArgs - parse ERROR log level", () => { assert.equal(args.logLevel, LogLevel.ERROR); }); - test("parseArgs - reads required options from environment variables", () => { process.env.VAULTLINK_LOCAL_PATH = "/env/path"; process.env.VAULTLINK_REMOTE_URI = "https://env.example.com"; diff --git a/frontend/local-client-cli/src/cli.ts b/frontend/local-client-cli/src/cli.ts index 31c81d5c..39c3eb38 100644 --- a/frontend/local-client-cli/src/cli.ts +++ b/frontend/local-client-cli/src/cli.ts @@ -1,11 +1,10 @@ import * as path from "path"; import * as fs from "fs/promises"; import * as fsSync from "fs"; -import type { NetworkConnectionStatus } from "sync-client"; +import type { NetworkConnectionStatus, Logger } from "sync-client"; import { SyncClient, DEFAULT_SETTINGS, - Logger, LogLevel, LogLine, type SyncSettings, diff --git a/frontend/local-client-cli/src/node-filesystem.ts b/frontend/local-client-cli/src/node-filesystem.ts index ba95ab6a..794072bd 100644 --- a/frontend/local-client-cli/src/node-filesystem.ts +++ b/frontend/local-client-cli/src/node-filesystem.ts @@ -15,7 +15,7 @@ import { toUnixPath } from "./path-utils"; export const VAULTLINK_DIR = ".vaultlink"; export class NodeFileSystemOperations implements FileSystemOperations { - public constructor(private readonly basePath: string) { } + public constructor(private readonly basePath: string) {} public async listFilesRecursively( directory: RelativePath | undefined diff --git a/frontend/obsidian-plugin/src/obsidian-file-system.ts b/frontend/obsidian-plugin/src/obsidian-file-system.ts index ceb8bc2a..f1a43518 100644 --- a/frontend/obsidian-plugin/src/obsidian-file-system.ts +++ b/frontend/obsidian-plugin/src/obsidian-file-system.ts @@ -139,10 +139,6 @@ export class ObsidianFileSystemOperations implements FileSystemOperations { return (await this.statFile(path)).size; } - public async getModificationTime(path: RelativePath): Promise { - return new Date((await this.statFile(path)).mtime); - } - public async exists(path: RelativePath): Promise { return this.vault.adapter.exists(normalizePath(path)); } diff --git a/frontend/package-lock.json b/frontend/package-lock.json index d1057be3..b0c7d1e0 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -10,8 +10,7 @@ "obsidian-plugin", "test-client", "deterministic-tests", - "local-client-cli", - "history-ui" + "local-client-cli" ], "devDependencies": { "concurrently": "^9.2.1", @@ -40,6 +39,7 @@ }, "history-ui": { "version": "0.14.0", + "extraneous": true, "devDependencies": { "@sveltejs/vite-plugin-svelte": "^5.0.0", "svelte": "^5.0.0", @@ -83,278 +83,6 @@ "node": ">=14.17.0" } }, - "node_modules/@esbuild/aix-ppc64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.25.12.tgz", - "integrity": "sha512-Hhmwd6CInZ3dwpuGTF8fJG6yoWmsToE+vYgD4nytZVxcu1ulHpUQRAB1UJ8+N1Am3Mz4+xOByoQoSZf4D+CpkA==", - "cpu": [ - "ppc64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "aix" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/android-arm": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/android-arm/-/android-arm-0.25.12.tgz", - "integrity": "sha512-VJ+sKvNA/GE7Ccacc9Cha7bpS8nyzVv0jdVgwNDaR4gDMC/2TTRc33Ip8qrNYUcpkOHUT5OZ0bUcNNVZQ9RLlg==", - "cpu": [ - "arm" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "android" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/android-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/android-arm64/-/android-arm64-0.25.12.tgz", - "integrity": "sha512-6AAmLG7zwD1Z159jCKPvAxZd4y/VTO0VkprYy+3N2FtJ8+BQWFXU+OxARIwA46c5tdD9SsKGZ/1ocqBS/gAKHg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "android" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/android-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/android-x64/-/android-x64-0.25.12.tgz", - "integrity": "sha512-5jbb+2hhDHx5phYR2By8GTWEzn6I9UqR11Kwf22iKbNpYrsmRB18aX/9ivc5cabcUiAT/wM+YIZ6SG9QO6a8kg==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "android" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/darwin-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/darwin-arm64/-/darwin-arm64-0.25.12.tgz", - "integrity": "sha512-N3zl+lxHCifgIlcMUP5016ESkeQjLj/959RxxNYIthIg+CQHInujFuXeWbWMgnTo4cp5XVHqFPmpyu9J65C1Yg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "darwin" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/darwin-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/darwin-x64/-/darwin-x64-0.25.12.tgz", - "integrity": "sha512-HQ9ka4Kx21qHXwtlTUVbKJOAnmG1ipXhdWTmNXiPzPfWKpXqASVcWdnf2bnL73wgjNrFXAa3yYvBSd9pzfEIpA==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "darwin" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/freebsd-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/freebsd-arm64/-/freebsd-arm64-0.25.12.tgz", - "integrity": "sha512-gA0Bx759+7Jve03K1S0vkOu5Lg/85dou3EseOGUes8flVOGxbhDDh/iZaoek11Y8mtyKPGF3vP8XhnkDEAmzeg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "freebsd" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/freebsd-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/freebsd-x64/-/freebsd-x64-0.25.12.tgz", - "integrity": "sha512-TGbO26Yw2xsHzxtbVFGEXBFH0FRAP7gtcPE7P5yP7wGy7cXK2oO7RyOhL5NLiqTlBh47XhmIUXuGciXEqYFfBQ==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "freebsd" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-arm": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-arm/-/linux-arm-0.25.12.tgz", - "integrity": "sha512-lPDGyC1JPDou8kGcywY0YILzWlhhnRjdof3UlcoqYmS9El818LLfJJc3PXXgZHrHCAKs/Z2SeZtDJr5MrkxtOw==", - "cpu": [ - "arm" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-arm64/-/linux-arm64-0.25.12.tgz", - "integrity": "sha512-8bwX7a8FghIgrupcxb4aUmYDLp8pX06rGh5HqDT7bB+8Rdells6mHvrFHHW2JAOPZUbnjUpKTLg6ECyzvas2AQ==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-ia32": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-ia32/-/linux-ia32-0.25.12.tgz", - "integrity": "sha512-0y9KrdVnbMM2/vG8KfU0byhUN+EFCny9+8g202gYqSSVMonbsCfLjUO+rCci7pM0WBEtz+oK/PIwHkzxkyharA==", - "cpu": [ - "ia32" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-loong64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-loong64/-/linux-loong64-0.25.12.tgz", - "integrity": "sha512-h///Lr5a9rib/v1GGqXVGzjL4TMvVTv+s1DPoxQdz7l/AYv6LDSxdIwzxkrPW438oUXiDtwM10o9PmwS/6Z0Ng==", - "cpu": [ - "loong64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-mips64el": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-mips64el/-/linux-mips64el-0.25.12.tgz", - "integrity": "sha512-iyRrM1Pzy9GFMDLsXn1iHUm18nhKnNMWscjmp4+hpafcZjrr2WbT//d20xaGljXDBYHqRcl8HnxbX6uaA/eGVw==", - "cpu": [ - "mips64el" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-ppc64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-ppc64/-/linux-ppc64-0.25.12.tgz", - "integrity": "sha512-9meM/lRXxMi5PSUqEXRCtVjEZBGwB7P/D4yT8UG/mwIdze2aV4Vo6U5gD3+RsoHXKkHCfSxZKzmDssVlRj1QQA==", - "cpu": [ - "ppc64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-riscv64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-riscv64/-/linux-riscv64-0.25.12.tgz", - "integrity": "sha512-Zr7KR4hgKUpWAwb1f3o5ygT04MzqVrGEGXGLnj15YQDJErYu/BGg+wmFlIDOdJp0PmB0lLvxFIOXZgFRrdjR0w==", - "cpu": [ - "riscv64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/linux-s390x": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-s390x/-/linux-s390x-0.25.12.tgz", - "integrity": "sha512-MsKncOcgTNvdtiISc/jZs/Zf8d0cl/t3gYWX8J9ubBnVOwlk65UIEEvgBORTiljloIWnBzLs4qhzPkJcitIzIg==", - "cpu": [ - "s390x" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, "node_modules/@esbuild/linux-x64": { "version": "0.27.1", "resolved": "https://registry.npmjs.org/@esbuild/linux-x64/-/linux-x64-0.27.1.tgz", @@ -371,159 +99,6 @@ "node": ">=18" } }, - "node_modules/@esbuild/netbsd-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/netbsd-arm64/-/netbsd-arm64-0.25.12.tgz", - "integrity": "sha512-xXwcTq4GhRM7J9A8Gv5boanHhRa/Q9KLVmcyXHCTaM4wKfIpWkdXiMog/KsnxzJ0A1+nD+zoecuzqPmCRyBGjg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "netbsd" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/netbsd-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/netbsd-x64/-/netbsd-x64-0.25.12.tgz", - "integrity": "sha512-Ld5pTlzPy3YwGec4OuHh1aCVCRvOXdH8DgRjfDy/oumVovmuSzWfnSJg+VtakB9Cm0gxNO9BzWkj6mtO1FMXkQ==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "netbsd" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/openbsd-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/openbsd-arm64/-/openbsd-arm64-0.25.12.tgz", - "integrity": "sha512-fF96T6KsBo/pkQI950FARU9apGNTSlZGsv1jZBAlcLL1MLjLNIWPBkj5NlSz8aAzYKg+eNqknrUJ24QBybeR5A==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "openbsd" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/openbsd-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/openbsd-x64/-/openbsd-x64-0.25.12.tgz", - "integrity": "sha512-MZyXUkZHjQxUvzK7rN8DJ3SRmrVrke8ZyRusHlP+kuwqTcfWLyqMOE3sScPPyeIXN/mDJIfGXvcMqCgYKekoQw==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "openbsd" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/openharmony-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/openharmony-arm64/-/openharmony-arm64-0.25.12.tgz", - "integrity": "sha512-rm0YWsqUSRrjncSXGA7Zv78Nbnw4XL6/dzr20cyrQf7ZmRcsovpcRBdhD43Nuk3y7XIoW2OxMVvwuRvk9XdASg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "openharmony" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/sunos-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/sunos-x64/-/sunos-x64-0.25.12.tgz", - "integrity": "sha512-3wGSCDyuTHQUzt0nV7bocDy72r2lI33QL3gkDNGkod22EsYl04sMf0qLb8luNKTOmgF/eDEDP5BFNwoBKH441w==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "sunos" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/win32-arm64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/win32-arm64/-/win32-arm64-0.25.12.tgz", - "integrity": "sha512-rMmLrur64A7+DKlnSuwqUdRKyd3UE7oPJZmnljqEptesKM8wx9J8gx5u0+9Pq0fQQW8vqeKebwNXdfOyP+8Bsg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/win32-ia32": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/win32-ia32/-/win32-ia32-0.25.12.tgz", - "integrity": "sha512-HkqnmmBoCbCwxUKKNPBixiWDGCpQGVsrQfJoVGYLPT41XWF8lHuE5N6WhVia2n4o5QK5M4tYr21827fNhi4byQ==", - "cpu": [ - "ia32" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/@esbuild/win32-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/win32-x64/-/win32-x64-0.25.12.tgz", - "integrity": "sha512-alJC0uCZpTFrSL0CCDjcgleBXPnCrEAhTBILpeAp7M/OFgoqtAetfBzX0xM00MUsVVPpVjlPuMbREqnZCXaTnA==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ], - "engines": { - "node": ">=18" - } - }, "node_modules/@eslint-community/eslint-utils": { "version": "4.9.0", "resolved": "https://registry.npmjs.org/@eslint-community/eslint-utils/-/eslint-utils-4.9.0.tgz", @@ -746,17 +321,6 @@ "@jridgewell/trace-mapping": "^0.3.24" } }, - "node_modules/@jridgewell/remapping": { - "version": "2.3.5", - "resolved": "https://registry.npmjs.org/@jridgewell/remapping/-/remapping-2.3.5.tgz", - "integrity": "sha512-LI9u/+laYG4Ds1TDKSJW2YPrIlcVYOwi2fUC6xB43lueCjgxV4lffOCZCtYFiH6TNOX+tQKXx97T4IKHbhyHEQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@jridgewell/gen-mapping": "^0.3.5", - "@jridgewell/trace-mapping": "^0.3.24" - } - }, "node_modules/@jridgewell/resolve-uri": { "version": "3.1.2", "dev": true, @@ -873,395 +437,6 @@ "url": "https://opencollective.com/parcel" } }, - "node_modules/@rollup/rollup-android-arm-eabi": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-android-arm-eabi/-/rollup-android-arm-eabi-4.59.0.tgz", - "integrity": "sha512-upnNBkA6ZH2VKGcBj9Fyl9IGNPULcjXRlg0LLeaioQWueH30p6IXtJEbKAgvyv+mJaMxSm1l6xwDXYjpEMiLMg==", - "cpu": [ - "arm" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "android" - ] - }, - "node_modules/@rollup/rollup-android-arm64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-android-arm64/-/rollup-android-arm64-4.59.0.tgz", - "integrity": "sha512-hZ+Zxj3SySm4A/DylsDKZAeVg0mvi++0PYVceVyX7hemkw7OreKdCvW2oQ3T1FMZvCaQXqOTHb8qmBShoqk69Q==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "android" - ] - }, - "node_modules/@rollup/rollup-darwin-arm64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-darwin-arm64/-/rollup-darwin-arm64-4.59.0.tgz", - "integrity": "sha512-W2Psnbh1J8ZJw0xKAd8zdNgF9HRLkdWwwdWqubSVk0pUuQkoHnv7rx4GiF9rT4t5DIZGAsConRE3AxCdJ4m8rg==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "darwin" - ] - }, - "node_modules/@rollup/rollup-darwin-x64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-darwin-x64/-/rollup-darwin-x64-4.59.0.tgz", - "integrity": "sha512-ZW2KkwlS4lwTv7ZVsYDiARfFCnSGhzYPdiOU4IM2fDbL+QGlyAbjgSFuqNRbSthybLbIJ915UtZBtmuLrQAT/w==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "darwin" - ] - }, - "node_modules/@rollup/rollup-freebsd-arm64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-freebsd-arm64/-/rollup-freebsd-arm64-4.59.0.tgz", - "integrity": "sha512-EsKaJ5ytAu9jI3lonzn3BgG8iRBjV4LxZexygcQbpiU0wU0ATxhNVEpXKfUa0pS05gTcSDMKpn3Sx+QB9RlTTA==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "freebsd" - ] - }, - "node_modules/@rollup/rollup-freebsd-x64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-freebsd-x64/-/rollup-freebsd-x64-4.59.0.tgz", - "integrity": "sha512-d3DuZi2KzTMjImrxoHIAODUZYoUUMsuUiY4SRRcJy6NJoZ6iIqWnJu9IScV9jXysyGMVuW+KNzZvBLOcpdl3Vg==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "freebsd" - ] - }, - "node_modules/@rollup/rollup-linux-arm-gnueabihf": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-arm-gnueabihf/-/rollup-linux-arm-gnueabihf-4.59.0.tgz", - "integrity": "sha512-t4ONHboXi/3E0rT6OZl1pKbl2Vgxf9vJfWgmUoCEVQVxhW6Cw/c8I6hbbu7DAvgp82RKiH7TpLwxnJeKv2pbsw==", - "cpu": [ - "arm" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-arm-musleabihf": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-arm-musleabihf/-/rollup-linux-arm-musleabihf-4.59.0.tgz", - "integrity": "sha512-CikFT7aYPA2ufMD086cVORBYGHffBo4K8MQ4uPS/ZnY54GKj36i196u8U+aDVT2LX4eSMbyHtyOh7D7Zvk2VvA==", - "cpu": [ - "arm" - ], - "dev": true, - "libc": [ - "musl" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-arm64-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-arm64-gnu/-/rollup-linux-arm64-gnu-4.59.0.tgz", - "integrity": "sha512-jYgUGk5aLd1nUb1CtQ8E+t5JhLc9x5WdBKew9ZgAXg7DBk0ZHErLHdXM24rfX+bKrFe+Xp5YuJo54I5HFjGDAA==", - "cpu": [ - "arm64" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-arm64-musl": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-arm64-musl/-/rollup-linux-arm64-musl-4.59.0.tgz", - "integrity": "sha512-peZRVEdnFWZ5Bh2KeumKG9ty7aCXzzEsHShOZEFiCQlDEepP1dpUl/SrUNXNg13UmZl+gzVDPsiCwnV1uI0RUA==", - "cpu": [ - "arm64" - ], - "dev": true, - "libc": [ - "musl" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-loong64-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-loong64-gnu/-/rollup-linux-loong64-gnu-4.59.0.tgz", - "integrity": "sha512-gbUSW/97f7+r4gHy3Jlup8zDG190AuodsWnNiXErp9mT90iCy9NKKU0Xwx5k8VlRAIV2uU9CsMnEFg/xXaOfXg==", - "cpu": [ - "loong64" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-loong64-musl": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-loong64-musl/-/rollup-linux-loong64-musl-4.59.0.tgz", - "integrity": "sha512-yTRONe79E+o0FWFijasoTjtzG9EBedFXJMl888NBEDCDV9I2wGbFFfJQQe63OijbFCUZqxpHz1GzpbtSFikJ4Q==", - "cpu": [ - "loong64" - ], - "dev": true, - "libc": [ - "musl" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-ppc64-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-ppc64-gnu/-/rollup-linux-ppc64-gnu-4.59.0.tgz", - "integrity": "sha512-sw1o3tfyk12k3OEpRddF68a1unZ5VCN7zoTNtSn2KndUE+ea3m3ROOKRCZxEpmT9nsGnogpFP9x6mnLTCaoLkA==", - "cpu": [ - "ppc64" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-ppc64-musl": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-ppc64-musl/-/rollup-linux-ppc64-musl-4.59.0.tgz", - "integrity": "sha512-+2kLtQ4xT3AiIxkzFVFXfsmlZiG5FXYW7ZyIIvGA7Bdeuh9Z0aN4hVyXS/G1E9bTP/vqszNIN/pUKCk/BTHsKA==", - "cpu": [ - "ppc64" - ], - "dev": true, - "libc": [ - "musl" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-riscv64-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-riscv64-gnu/-/rollup-linux-riscv64-gnu-4.59.0.tgz", - "integrity": "sha512-NDYMpsXYJJaj+I7UdwIuHHNxXZ/b/N2hR15NyH3m2qAtb/hHPA4g4SuuvrdxetTdndfj9b1WOmy73kcPRoERUg==", - "cpu": [ - "riscv64" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-riscv64-musl": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-riscv64-musl/-/rollup-linux-riscv64-musl-4.59.0.tgz", - "integrity": "sha512-nLckB8WOqHIf1bhymk+oHxvM9D3tyPndZH8i8+35p/1YiVoVswPid2yLzgX7ZJP0KQvnkhM4H6QZ5m0LzbyIAg==", - "cpu": [ - "riscv64" - ], - "dev": true, - "libc": [ - "musl" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-s390x-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-s390x-gnu/-/rollup-linux-s390x-gnu-4.59.0.tgz", - "integrity": "sha512-oF87Ie3uAIvORFBpwnCvUzdeYUqi2wY6jRFWJAy1qus/udHFYIkplYRW+wo+GRUP4sKzYdmE1Y3+rY5Gc4ZO+w==", - "cpu": [ - "s390x" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-x64-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-x64-gnu/-/rollup-linux-x64-gnu-4.59.0.tgz", - "integrity": "sha512-3AHmtQq/ppNuUspKAlvA8HtLybkDflkMuLK4DPo77DfthRb71V84/c4MlWJXixZz4uruIH4uaa07IqoAkG64fg==", - "cpu": [ - "x64" - ], - "dev": true, - "libc": [ - "glibc" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-linux-x64-musl": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-linux-x64-musl/-/rollup-linux-x64-musl-4.59.0.tgz", - "integrity": "sha512-2UdiwS/9cTAx7qIUZB/fWtToJwvt0Vbo0zmnYt7ED35KPg13Q0ym1g442THLC7VyI6JfYTP4PiSOWyoMdV2/xg==", - "cpu": [ - "x64" - ], - "dev": true, - "libc": [ - "musl" - ], - "license": "MIT", - "optional": true, - "os": [ - "linux" - ] - }, - "node_modules/@rollup/rollup-openbsd-x64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-openbsd-x64/-/rollup-openbsd-x64-4.59.0.tgz", - "integrity": "sha512-M3bLRAVk6GOwFlPTIxVBSYKUaqfLrn8l0psKinkCFxl4lQvOSz8ZrKDz2gxcBwHFpci0B6rttydI4IpS4IS/jQ==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "openbsd" - ] - }, - "node_modules/@rollup/rollup-openharmony-arm64": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-openharmony-arm64/-/rollup-openharmony-arm64-4.59.0.tgz", - "integrity": "sha512-tt9KBJqaqp5i5HUZzoafHZX8b5Q2Fe7UjYERADll83O4fGqJ49O1FsL6LpdzVFQcpwvnyd0i+K/VSwu/o/nWlA==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "openharmony" - ] - }, - "node_modules/@rollup/rollup-win32-arm64-msvc": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-arm64-msvc/-/rollup-win32-arm64-msvc-4.59.0.tgz", - "integrity": "sha512-V5B6mG7OrGTwnxaNUzZTDTjDS7F75PO1ae6MJYdiMu60sq0CqN5CVeVsbhPxalupvTX8gXVSU9gq+Rx1/hvu6A==", - "cpu": [ - "arm64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ] - }, - "node_modules/@rollup/rollup-win32-ia32-msvc": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-ia32-msvc/-/rollup-win32-ia32-msvc-4.59.0.tgz", - "integrity": "sha512-UKFMHPuM9R0iBegwzKF4y0C4J9u8C6MEJgFuXTBerMk7EJ92GFVFYBfOZaSGLu6COf7FxpQNqhNS4c4icUPqxA==", - "cpu": [ - "ia32" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ] - }, - "node_modules/@rollup/rollup-win32-x64-gnu": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-x64-gnu/-/rollup-win32-x64-gnu-4.59.0.tgz", - "integrity": "sha512-laBkYlSS1n2L8fSo1thDNGrCTQMmxjYY5G0WFWjFFYZkKPjsMBsgJfGf4TLxXrF6RyhI60L8TMOjBMvXiTcxeA==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ] - }, - "node_modules/@rollup/rollup-win32-x64-msvc": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-x64-msvc/-/rollup-win32-x64-msvc-4.59.0.tgz", - "integrity": "sha512-2HRCml6OztYXyJXAvdDXPKcawukWY2GpR5/nxKp4iBgiO3wcoEGkAaqctIbZcNB6KlUQBIqt8VYkNSj2397EfA==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "win32" - ] - }, "node_modules/@sentry-internal/browser-utils": { "version": "10.30.0", "resolved": "https://registry.npmjs.org/@sentry-internal/browser-utils/-/browser-utils-10.30.0.tgz", @@ -1337,56 +512,6 @@ "node": ">=18" } }, - "node_modules/@sveltejs/acorn-typescript": { - "version": "1.0.9", - "resolved": "https://registry.npmjs.org/@sveltejs/acorn-typescript/-/acorn-typescript-1.0.9.tgz", - "integrity": "sha512-lVJX6qEgs/4DOcRTpo56tmKzVPtoWAaVbL4hfO7t7NVwl9AAXzQR6cihesW1BmNMPl+bK6dreu2sOKBP2Q9CIA==", - "dev": true, - "license": "MIT", - "peerDependencies": { - "acorn": "^8.9.0" - } - }, - "node_modules/@sveltejs/vite-plugin-svelte": { - "version": "5.1.1", - "resolved": "https://registry.npmjs.org/@sveltejs/vite-plugin-svelte/-/vite-plugin-svelte-5.1.1.tgz", - "integrity": "sha512-Y1Cs7hhTc+a5E9Va/xwKlAJoariQyHY+5zBgCZg4PFWNYQ1nMN9sjK1zhw1gK69DuqVP++sht/1GZg1aRwmAXQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@sveltejs/vite-plugin-svelte-inspector": "^4.0.1", - "debug": "^4.4.1", - "deepmerge": "^4.3.1", - "kleur": "^4.1.5", - "magic-string": "^0.30.17", - "vitefu": "^1.0.6" - }, - "engines": { - "node": "^18.0.0 || ^20.0.0 || >=22" - }, - "peerDependencies": { - "svelte": "^5.0.0", - "vite": "^6.0.0" - } - }, - "node_modules/@sveltejs/vite-plugin-svelte-inspector": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/@sveltejs/vite-plugin-svelte-inspector/-/vite-plugin-svelte-inspector-4.0.1.tgz", - "integrity": "sha512-J/Nmb2Q2y7mck2hyCX4ckVHcR5tu2J+MtBEQqpDrrgELZ2uvraQcK/ioCV61AqkdXFgriksOKIceDcQmqnGhVw==", - "dev": true, - "license": "MIT", - "dependencies": { - "debug": "^4.3.7" - }, - "engines": { - "node": "^18.0.0 || ^20.0.0 || >=22" - }, - "peerDependencies": { - "@sveltejs/vite-plugin-svelte": "^5.0.0", - "svelte": "^5.0.0", - "vite": "^6.0.0" - } - }, "node_modules/@types/codemirror": { "version": "5.60.8", "dev": true, @@ -1441,13 +566,6 @@ "@types/estree": "*" } }, - "node_modules/@types/trusted-types": { - "version": "2.0.7", - "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", - "integrity": "sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==", - "dev": true, - "license": "MIT" - }, "node_modules/@typescript-eslint/eslint-plugin": { "version": "8.49.0", "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.49.0.tgz", @@ -2005,26 +1123,6 @@ "dev": true, "license": "Python-2.0" }, - "node_modules/aria-query": { - "version": "5.3.1", - "resolved": "https://registry.npmjs.org/aria-query/-/aria-query-5.3.1.tgz", - "integrity": "sha512-Z/ZeOgVl7bcSYZ/u/rh0fOpvEpq//LZmdbkXyc7syVzjPAhfOa9ebsdTSjEBDU4vs5nC98Kfduj1uFo0qyET3g==", - "dev": true, - "license": "Apache-2.0", - "engines": { - "node": ">= 0.4" - } - }, - "node_modules/axobject-query": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/axobject-query/-/axobject-query-4.1.0.tgz", - "integrity": "sha512-qIj0G9wZbMGNLjLmg1PT6v2mE9AH2zlnADJD/2tC6E00hgmhUOfEB6greHPAfLRSufHqROIUTkw6E+M3lH0PTQ==", - "dev": true, - "license": "Apache-2.0", - "engines": { - "node": ">= 0.4" - } - }, "node_modules/balanced-match": { "version": "1.0.2", "dev": true, @@ -2242,16 +1340,6 @@ "node": ">=6" } }, - "node_modules/clsx": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/clsx/-/clsx-2.1.1.tgz", - "integrity": "sha512-eYm0QWBtUrBWZWG0d386OGAw16Z995PiOVo2B7bjWSbHedGl5e0ZWaq65kOGgUSNesEIDkB9ISbTg/JK9dhCZA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6" - } - }, "node_modules/color-convert": { "version": "2.0.1", "dev": true, @@ -2413,16 +1501,6 @@ "dev": true, "license": "MIT" }, - "node_modules/deepmerge": { - "version": "4.3.1", - "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-4.3.1.tgz", - "integrity": "sha512-3sUqbMEc77XqpdNO7FRyRog+eW3ph+GYCbj+rK+uYyRMuwsVy0rMiVtPn+QJlKFvWP/1PYpapqYn0Me2knFn+A==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/detect-libc": { "version": "1.0.3", "dev": true, @@ -2446,13 +1524,6 @@ "dev": true, "license": "MIT" }, - "node_modules/devalue": { - "version": "5.6.4", - "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.6.4.tgz", - "integrity": "sha512-Gp6rDldRsFh/7XuouDbxMH3Mx8GMCcgzIb1pDTvNyn8pZGQ22u+Wa+lGV9dQCltFQ7uVw0MhRyb8XDskNFOReA==", - "dev": true, - "license": "MIT" - }, "node_modules/dunder-proto": { "version": "1.0.1", "dev": true, @@ -3130,13 +2201,6 @@ "url": "https://opencollective.com/eslint" } }, - "node_modules/esm-env": { - "version": "1.2.2", - "resolved": "https://registry.npmjs.org/esm-env/-/esm-env-1.2.2.tgz", - "integrity": "sha512-Epxrv+Nr/CaL4ZcFGPJIYLWFom+YeV1DqMLHJoEd9SYRxNbaFruBwfEX/kkHUJf55j2+TUbmDcmuilbP1TmXHA==", - "dev": true, - "license": "MIT" - }, "node_modules/espree": { "version": "10.4.0", "resolved": "https://registry.npmjs.org/espree/-/espree-10.4.0.tgz", @@ -3166,17 +2230,6 @@ "node": ">=0.10" } }, - "node_modules/esrap": { - "version": "2.2.4", - "resolved": "https://registry.npmjs.org/esrap/-/esrap-2.2.4.tgz", - "integrity": "sha512-suICpxAmZ9A8bzJjEl/+rLJiDKC0X4gYWUxT6URAWBLvlXmtbZd5ySMu/N2ZGEtMCAmflUDPSehrP9BQcsGcSg==", - "dev": true, - "license": "MIT", - "dependencies": { - "@jridgewell/sourcemap-codec": "^1.4.15", - "@typescript-eslint/types": "^8.2.0" - } - }, "node_modules/esrecurse": { "version": "4.3.0", "dev": true, @@ -3503,10 +2556,6 @@ "node": ">= 0.4" } }, - "node_modules/history-ui": { - "resolved": "history-ui", - "link": true - }, "node_modules/icss-utils": { "version": "5.1.0", "dev": true, @@ -3640,16 +2689,6 @@ "node": ">=0.10.0" } }, - "node_modules/is-reference": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/is-reference/-/is-reference-3.0.3.tgz", - "integrity": "sha512-ixkJoqQvAP88E6wLydLGGqCJsrFUnqoH6HnaczB8XmDH1oaWU+xxdptvikTgaEhtZ53Ky6YXiBuUI2WXLMCwjw==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/estree": "^1.0.6" - } - }, "node_modules/isexe": { "version": "2.0.0", "dev": true, @@ -3734,16 +2773,6 @@ "node": ">=0.10.0" } }, - "node_modules/kleur": { - "version": "4.1.5", - "resolved": "https://registry.npmjs.org/kleur/-/kleur-4.1.5.tgz", - "integrity": "sha512-o+NO+8WrRiQEE4/7nwRJhN1HWpVmJm511pBHUxPLtp0BUISzlBplORYSmTclCnJvQq2tKu/sgl3xVpkc7ZWuQQ==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6" - } - }, "node_modules/levn": { "version": "0.4.1", "dev": true, @@ -3786,13 +2815,6 @@ "resolved": "local-client-cli", "link": true }, - "node_modules/locate-character": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/locate-character/-/locate-character-3.0.0.tgz", - "integrity": "sha512-SW13ws7BjaeJ6p7Q6CO2nchbYEc3X3J6WrmTTDto7yMPqVSZTUyY5Tjbid+Ab8gLnATtygYtiDIJGQRRn2ZOiA==", - "dev": true, - "license": "MIT" - }, "node_modules/locate-path": { "version": "6.0.0", "dev": true, @@ -3812,16 +2834,6 @@ "dev": true, "license": "MIT" }, - "node_modules/magic-string": { - "version": "0.30.21", - "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.30.21.tgz", - "integrity": "sha512-vd2F4YUyEXKGcLHoq+TEyCjxueSeHnFxyyjNp80yg0XV4vUhnDer/lvvlqM/arB5bXQN5K2/3oinyCRyx8T2CQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@jridgewell/sourcemap-codec": "^1.5.5" - } - }, "node_modules/math-intrinsics": { "version": "1.1.0", "dev": true, @@ -4503,51 +3515,6 @@ "node": ">=12" } }, - "node_modules/rollup": { - "version": "4.59.0", - "resolved": "https://registry.npmjs.org/rollup/-/rollup-4.59.0.tgz", - "integrity": "sha512-2oMpl67a3zCH9H79LeMcbDhXW/UmWG/y2zuqnF2jQq5uq9TbM9TVyXvA4+t+ne2IIkBdrLpAaRQAvo7YI/Yyeg==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/estree": "1.0.8" - }, - "bin": { - "rollup": "dist/bin/rollup" - }, - "engines": { - "node": ">=18.0.0", - "npm": ">=8.0.0" - }, - "optionalDependencies": { - "@rollup/rollup-android-arm-eabi": "4.59.0", - "@rollup/rollup-android-arm64": "4.59.0", - "@rollup/rollup-darwin-arm64": "4.59.0", - "@rollup/rollup-darwin-x64": "4.59.0", - "@rollup/rollup-freebsd-arm64": "4.59.0", - "@rollup/rollup-freebsd-x64": "4.59.0", - "@rollup/rollup-linux-arm-gnueabihf": "4.59.0", - "@rollup/rollup-linux-arm-musleabihf": "4.59.0", - "@rollup/rollup-linux-arm64-gnu": "4.59.0", - "@rollup/rollup-linux-arm64-musl": "4.59.0", - "@rollup/rollup-linux-loong64-gnu": "4.59.0", - "@rollup/rollup-linux-loong64-musl": "4.59.0", - "@rollup/rollup-linux-ppc64-gnu": "4.59.0", - "@rollup/rollup-linux-ppc64-musl": "4.59.0", - "@rollup/rollup-linux-riscv64-gnu": "4.59.0", - "@rollup/rollup-linux-riscv64-musl": "4.59.0", - "@rollup/rollup-linux-s390x-gnu": "4.59.0", - "@rollup/rollup-linux-x64-gnu": "4.59.0", - "@rollup/rollup-linux-x64-musl": "4.59.0", - "@rollup/rollup-openbsd-x64": "4.59.0", - "@rollup/rollup-openharmony-arm64": "4.59.0", - "@rollup/rollup-win32-arm64-msvc": "4.59.0", - "@rollup/rollup-win32-ia32-msvc": "4.59.0", - "@rollup/rollup-win32-x64-gnu": "4.59.0", - "@rollup/rollup-win32-x64-msvc": "4.59.0", - "fsevents": "~2.3.2" - } - }, "node_modules/rxjs": { "version": "7.8.2", "dev": true, @@ -4874,34 +3841,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/svelte": { - "version": "5.53.12", - "resolved": "https://registry.npmjs.org/svelte/-/svelte-5.53.12.tgz", - "integrity": "sha512-4x/uk4rQe/d7RhfvS8wemTfNjQ0bJbKvamIzRBfTe2eHHjzBZ7PZicUQrC2ryj83xxEacfA1zHKd1ephD1tAxA==", - "dev": true, - "license": "MIT", - "dependencies": { - "@jridgewell/remapping": "^2.3.4", - "@jridgewell/sourcemap-codec": "^1.5.0", - "@sveltejs/acorn-typescript": "^1.0.5", - "@types/estree": "^1.0.5", - "@types/trusted-types": "^2.0.7", - "acorn": "^8.12.1", - "aria-query": "5.3.1", - "axobject-query": "^4.1.0", - "clsx": "^2.1.1", - "devalue": "^5.6.4", - "esm-env": "^1.2.1", - "esrap": "^2.2.2", - "is-reference": "^3.0.3", - "locate-character": "^3.0.0", - "magic-string": "^0.30.11", - "zimmerframe": "^1.1.2" - }, - "engines": { - "node": ">=18" - } - }, "node_modules/sync-client": { "resolved": "sync-client", "link": true @@ -5330,191 +4269,6 @@ "resolved": "obsidian-plugin", "link": true }, - "node_modules/vite": { - "version": "6.4.1", - "resolved": "https://registry.npmjs.org/vite/-/vite-6.4.1.tgz", - "integrity": "sha512-+Oxm7q9hDoLMyJOYfUYBuHQo+dkAloi33apOPP56pzj+vsdJDzr+j1NISE5pyaAuKL4A3UD34qd0lx5+kfKp2g==", - "dev": true, - "license": "MIT", - "dependencies": { - "esbuild": "^0.25.0", - "fdir": "^6.4.4", - "picomatch": "^4.0.2", - "postcss": "^8.5.3", - "rollup": "^4.34.9", - "tinyglobby": "^0.2.13" - }, - "bin": { - "vite": "bin/vite.js" - }, - "engines": { - "node": "^18.0.0 || ^20.0.0 || >=22.0.0" - }, - "funding": { - "url": "https://github.com/vitejs/vite?sponsor=1" - }, - "optionalDependencies": { - "fsevents": "~2.3.3" - }, - "peerDependencies": { - "@types/node": "^18.0.0 || ^20.0.0 || >=22.0.0", - "jiti": ">=1.21.0", - "less": "*", - "lightningcss": "^1.21.0", - "sass": "*", - "sass-embedded": "*", - "stylus": "*", - "sugarss": "*", - "terser": "^5.16.0", - "tsx": "^4.8.1", - "yaml": "^2.4.2" - }, - "peerDependenciesMeta": { - "@types/node": { - "optional": true - }, - "jiti": { - "optional": true - }, - "less": { - "optional": true - }, - "lightningcss": { - "optional": true - }, - "sass": { - "optional": true - }, - "sass-embedded": { - "optional": true - }, - "stylus": { - "optional": true - }, - "sugarss": { - "optional": true - }, - "terser": { - "optional": true - }, - "tsx": { - "optional": true - }, - "yaml": { - "optional": true - } - } - }, - "node_modules/vite/node_modules/@esbuild/linux-x64": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/@esbuild/linux-x64/-/linux-x64-0.25.12.tgz", - "integrity": "sha512-uqZMTLr/zR/ed4jIGnwSLkaHmPjOjJvnm6TVVitAa08SLS9Z0VM8wIRx7gWbJB5/J54YuIMInDquWyYvQLZkgw==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "MIT", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">=18" - } - }, - "node_modules/vite/node_modules/esbuild": { - "version": "0.25.12", - "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.25.12.tgz", - "integrity": "sha512-bbPBYYrtZbkt6Os6FiTLCTFxvq4tt3JKall1vRwshA3fdVztsLAatFaZobhkBC8/BrPetoa0oksYoKXoG4ryJg==", - "dev": true, - "hasInstallScript": true, - "license": "MIT", - "bin": { - "esbuild": "bin/esbuild" - }, - "engines": { - "node": ">=18" - }, - "optionalDependencies": { - "@esbuild/aix-ppc64": "0.25.12", - "@esbuild/android-arm": "0.25.12", - "@esbuild/android-arm64": "0.25.12", - "@esbuild/android-x64": "0.25.12", - "@esbuild/darwin-arm64": "0.25.12", - "@esbuild/darwin-x64": "0.25.12", - "@esbuild/freebsd-arm64": "0.25.12", - "@esbuild/freebsd-x64": "0.25.12", - "@esbuild/linux-arm": "0.25.12", - "@esbuild/linux-arm64": "0.25.12", - "@esbuild/linux-ia32": "0.25.12", - "@esbuild/linux-loong64": "0.25.12", - "@esbuild/linux-mips64el": "0.25.12", - "@esbuild/linux-ppc64": "0.25.12", - "@esbuild/linux-riscv64": "0.25.12", - "@esbuild/linux-s390x": "0.25.12", - "@esbuild/linux-x64": "0.25.12", - "@esbuild/netbsd-arm64": "0.25.12", - "@esbuild/netbsd-x64": "0.25.12", - "@esbuild/openbsd-arm64": "0.25.12", - "@esbuild/openbsd-x64": "0.25.12", - "@esbuild/openharmony-arm64": "0.25.12", - "@esbuild/sunos-x64": "0.25.12", - "@esbuild/win32-arm64": "0.25.12", - "@esbuild/win32-ia32": "0.25.12", - "@esbuild/win32-x64": "0.25.12" - } - }, - "node_modules/vite/node_modules/fdir": { - "version": "6.5.0", - "resolved": "https://registry.npmjs.org/fdir/-/fdir-6.5.0.tgz", - "integrity": "sha512-tIbYtZbucOs0BRGqPJkshJUYdL+SDH7dVM8gjy+ERp3WAUjLEFJE+02kanyHtwjWOnwrKYBiwAmM0p4kLJAnXg==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=12.0.0" - }, - "peerDependencies": { - "picomatch": "^3 || ^4" - }, - "peerDependenciesMeta": { - "picomatch": { - "optional": true - } - } - }, - "node_modules/vite/node_modules/picomatch": { - "version": "4.0.3", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", - "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=12" - }, - "funding": { - "url": "https://github.com/sponsors/jonschlinkert" - } - }, - "node_modules/vitefu": { - "version": "1.1.2", - "resolved": "https://registry.npmjs.org/vitefu/-/vitefu-1.1.2.tgz", - "integrity": "sha512-zpKATdUbzbsycPFBN71nS2uzBUQiVnFoOrr2rvqv34S1lcAgMKKkjWleLGeiJlZ8lwCXvtWaRn7R3ZC16SYRuw==", - "dev": true, - "license": "MIT", - "workspaces": [ - "tests/deps/*", - "tests/projects/*", - "tests/projects/workspace/packages/*" - ], - "peerDependencies": { - "vite": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-beta.0" - }, - "peerDependenciesMeta": { - "vite": { - "optional": true - } - } - }, "node_modules/w3c-keyname": { "version": "2.2.8", "resolved": "https://registry.npmjs.org/w3c-keyname/-/w3c-keyname-2.2.8.tgz", @@ -5833,13 +4587,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/zimmerframe": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/zimmerframe/-/zimmerframe-1.1.4.tgz", - "integrity": "sha512-B58NGBEoc8Y9MWWCQGl/gq9xBCe4IiKM0a2x7GZdQKOW5Exr8S1W24J6OgM1njK8xCRGvAJIL/MxXHf6SkmQKQ==", - "dev": true, - "license": "MIT" - }, "obsidian-plugin": { "name": "vault-link-obsidian-plugin", "version": "0.14.0", diff --git a/frontend/sync-client/src/file-operations/file-operations.test.ts b/frontend/sync-client/src/file-operations/file-operations.test.ts index 7916ab57..0597ca8b 100644 --- a/frontend/sync-client/src/file-operations/file-operations.test.ts +++ b/frontend/sync-client/src/file-operations/file-operations.test.ts @@ -7,7 +7,6 @@ import { assertSetContainsExactly } from "../utils/assert-set-contains-exactly"; import type { FileSystemOperations } from "./filesystem-operations"; import type { TextWithCursors } from "reconcile-text"; import type { ServerConfig, ServerConfigData } from "../services/server-config"; -import { ExpectedFsEvents } from "../sync-operations/expected-fs-events"; import { FileAlreadyExistsError } from "../errors/file-already-exists-error"; class MockServerConfig implements Pick { @@ -72,8 +71,7 @@ function makeOps(): { const ops = new FileOperations( new Logger(), fs, - new MockServerConfig() as ServerConfig, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion - new ExpectedFsEvents() + new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion ); return { fs, ops }; } @@ -85,7 +83,7 @@ describe("File operations", () => { const result = await ops.create("a", new Uint8Array()); assertSetContainsExactly(fs.names, "a"); - assert.equal(result.actualPath, "a"); + assert.equal(result, "a"); }); it("create throws FileAlreadyExistsError when the path is occupied", async () => { @@ -109,7 +107,7 @@ describe("File operations", () => { const result = await ops.move("a", "b"); assertSetContainsExactly(fs.names, "b"); - assert.equal(result.actualPath, "b"); + assert.equal(result, "b"); }); it("move with same source and target is a no-op", async () => { @@ -119,7 +117,7 @@ describe("File operations", () => { const result = await ops.move("a", "a"); assertSetContainsExactly(fs.names, "a"); - assert.equal(result.actualPath, "a"); + assert.equal(result, "a"); }); it("move throws FileAlreadyExistsError when the target is occupied", async () => { diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index 17a2c655..efd1d5b2 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -9,17 +9,6 @@ import { isBinary } from "../utils/is-binary"; import type { ServerConfig } from "../services/server-config"; import { FileNotFoundError } from "../errors/file-not-found-error"; import { FileAlreadyExistsError } from "../errors/file-already-exists-error"; -import type { ExpectedFsEvents } from "../sync-operations/expected-fs-events"; - -/** - * Outcome of a `move`/`create`. `actualPath` is where the file ended up; - * with the conflict-path machinery removed it is always equal to the - * requested path. The shape is preserved so callers don't all need to - * change. - */ -export interface FileOpResult { - actualPath: RelativePath; -} export class FileOperations { private readonly fs: SafeFileSystemOperations; @@ -28,7 +17,6 @@ export class FileOperations { private readonly logger: Logger, fs: FileSystemOperations, private readonly serverConfig: ServerConfig, - private readonly expectedFsEvents: ExpectedFsEvents, private readonly nativeLineEndings = "\n" ) { this.fs = new SafeFileSystemOperations(fs, logger); @@ -68,7 +56,7 @@ export class FileOperations { public async create( path: RelativePath, newContent: Uint8Array - ): Promise { + ): Promise { if (await this.fs.exists(path)) { throw new FileAlreadyExistsError( `Refusing to create '${path}': file already exists`, @@ -77,14 +65,8 @@ export class FileOperations { } await this.createParentDirectories(path); - this.expectedFsEvents.expectCreate(path); - try { - await this.fs.write(path, this.toNativeLineEndings(newContent)); - } catch (e) { - this.expectedFsEvents.unexpectCreate(path); - throw e; - } - return { actualPath: path }; + await this.fs.write(path, this.toNativeLineEndings(newContent)); + return path; } /** @@ -105,12 +87,6 @@ export class FileOperations { return; } - // Single-source the expectation registration: register exactly once - // per call, and unexpect from the catch if the underlying fs op - // throws (FileNotFoundError or otherwise). The previous shape - // registered inside each branch and let the catch swallow - // FileNotFoundError, leaking the expectation into the map. - this.expectedFsEvents.expectUpdate(path); try { if ( !isFileTypeMergable( @@ -175,7 +151,6 @@ export class FileOperations { } ); } catch (e) { - this.expectedFsEvents.unexpectUpdate(path); if (e instanceof FileNotFoundError) { this.logger.debug( `File ${path} disappeared during write; not recreating` @@ -188,13 +163,7 @@ export class FileOperations { public async delete(path: RelativePath): Promise { if (await this.exists(path)) { - this.expectedFsEvents.expectDelete(path); - try { - await this.fs.delete(path); - } catch (e) { - this.expectedFsEvents.unexpectDelete(path); - throw e; - } + await this.fs.delete(path); await this.deletingEmptyParentDirectoriesOfDeletedFile(path); } else { this.logger.debug(`No need to delete '${path}', it doesn't exist`); @@ -220,9 +189,9 @@ export class FileOperations { public async move( oldPath: RelativePath, newPath: RelativePath - ): Promise { + ): Promise { if (oldPath === newPath) { - return { actualPath: oldPath }; + return oldPath; } if (await this.fs.exists(newPath)) { @@ -233,15 +202,9 @@ export class FileOperations { } await this.createParentDirectories(newPath); - this.expectedFsEvents.expectRename(oldPath, newPath); - try { - await this.fs.rename(oldPath, newPath); - } catch (e) { - this.expectedFsEvents.unexpectRename(oldPath, newPath); - throw e; - } + await this.fs.rename(oldPath, newPath); await this.deletingEmptyParentDirectoriesOfDeletedFile(oldPath); - return { actualPath: newPath }; + return newPath; } private async deletingEmptyParentDirectoriesOfDeletedFile( diff --git a/frontend/sync-client/src/services/sync-service.ts b/frontend/sync-client/src/services/sync-service.ts index 0a99fe84..56d28d3d 100644 --- a/frontend/sync-client/src/services/sync-service.ts +++ b/frontend/sync-client/src/services/sync-service.ts @@ -13,8 +13,6 @@ import { HttpClientError } from "../errors/http-client-error"; import type { SerializedError } from "./types/SerializedError"; import type { DocumentVersionWithoutContent } from "./types/DocumentVersionWithoutContent"; import type { DocumentUpdateResponse } from "./types/DocumentUpdateResponse"; -import type { DocumentVersion } from "./types/DocumentVersion"; -import type { FetchLatestDocumentsResponse } from "./types/FetchLatestDocumentsResponse"; import type { PingResponse } from "./types/PingResponse"; import type { UpdateTextDocumentVersion } from "./types/UpdateTextDocumentVersion"; import { buildVaultUrl } from "./build-vault-url"; @@ -272,32 +270,6 @@ export class SyncService { }); } - public async get({ - documentId - }: { - documentId: DocumentId; - }): Promise { - return this.retryForever(async () => { - this.logger.debug(`Getting document with id ${documentId}`); - - const response = await this.client( - this.getUrl(`/documents/${documentId}`), - { - headers: this.getDefaultHeaders() - } - ); - - await SyncService.throwIfNotOk(response, "get document"); - - const result: DocumentVersion = - (await response.json()) as DocumentVersion; // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion - - this.logger.debug(`Got document ${JSON.stringify(result)}`); - - return result; - }); - } - public async getDocumentVersionContent({ documentId, vaultUpdateId @@ -332,36 +304,6 @@ export class SyncService { }); } - public async getAll( - since?: VaultUpdateId - ): Promise { - return this.retryForever(async () => { - this.logger.debug( - "Getting all documents" + - (since != null ? ` since ${since}` : "") - ); - - const url = new URL(this.getUrl("/documents")); - if (since !== undefined) { - url.searchParams.append("since_update_id", since.toString()); - } - const response = await this.client(url.toString(), { - headers: this.getDefaultHeaders() - }); - - await SyncService.throwIfNotOk(response, "get documents"); - - const result: FetchLatestDocumentsResponse = - (await response.json()) as FetchLatestDocumentsResponse; // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion - - this.logger.debug( - `Got ${result.latestDocuments.length} document metadata` - ); - - return result; - }); - } - public async ping(): Promise { this.logger.debug("Pinging server"); const response = await this.pingClient(this.getUrl("/ping"), { diff --git a/frontend/sync-client/src/services/types/DocumentVersionWithoutContent.ts b/frontend/sync-client/src/services/types/DocumentVersionWithoutContent.ts index 662b41e5..4b24e7c5 100644 --- a/frontend/sync-client/src/services/types/DocumentVersionWithoutContent.ts +++ b/frontend/sync-client/src/services/types/DocumentVersionWithoutContent.ts @@ -9,8 +9,4 @@ export interface DocumentVersionWithoutContent { userId: string; deviceId: string; contentSize: number; - /** - * True iff this is the first version of the document - */ - isNewFile: boolean; } diff --git a/frontend/sync-client/src/services/types/FetchLatestDocumentsResponse.ts b/frontend/sync-client/src/services/types/FetchLatestDocumentsResponse.ts deleted file mode 100644 index 315d701a..00000000 --- a/frontend/sync-client/src/services/types/FetchLatestDocumentsResponse.ts +++ /dev/null @@ -1,13 +0,0 @@ -// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { DocumentVersionWithoutContent } from "./DocumentVersionWithoutContent"; - -/** - * Response to a fetch latest documents request. - */ -export interface FetchLatestDocumentsResponse { - latestDocuments: DocumentVersionWithoutContent[]; - /** - * The update ID of the latest document in the response. - */ - lastUpdateId: bigint; -} diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index dd537296..463ac081 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -30,7 +30,6 @@ import { setUpTelemetry } from "./utils/set-up-telemetry"; import { ServerConfig } from "./services/server-config"; import type { EventListeners } from "./utils/data-structures/event-listeners"; import { Lock } from "./utils/data-structures/locks"; -import { ExpectedFsEvents } from "./sync-operations/expected-fs-events"; export class SyncClient { private hasFinishedOfflineSync = false; @@ -55,14 +54,7 @@ export class SyncClient { private readonly fileChangeNotifier: FileChangeNotifier, private readonly contentCache: FixedSizeDocumentCache, private readonly serverConfig: ServerConfig, - private readonly syncService: SyncService, - private readonly expectedFsEvents: ExpectedFsEvents, - private readonly persistence: PersistenceProvider< - Partial<{ - settings: Partial; - database: Partial; - }> - > + private readonly syncService: SyncService ) {} public get syncedDocumentCount(): number { @@ -172,7 +164,7 @@ export class SyncClient { // new deviceId, the server-side query would miss, and the // pending-but-lost create would deconflict instead of // binding to the doc its content was already absorbed into. - let deviceId = state.deviceId; + let { deviceId } = state; if (deviceId === undefined) { deviceId = createClientId(); state = { ...state, deviceId }; @@ -215,13 +207,10 @@ export class SyncClient { const serverConfig = new ServerConfig(syncService, settings); - const expectedFsEvents = new ExpectedFsEvents(); - const fileOperations = new FileOperations( logger, fs, serverConfig, - expectedFsEvents, nativeLineEndings ); @@ -268,9 +257,7 @@ export class SyncClient { fileChangeNotifier, contentCache, serverConfig, - syncService, - expectedFsEvents, - persistence + syncService ); logger.info("SyncClient created successfully"); @@ -322,26 +309,6 @@ export class SyncClient { } } - /** - * Reload settings from disk overriding current in-memory settings. - * Missing values will be filled in from DEFAULT_SETTINGS rather than - * retaining current in-memory settings. - */ - public async reloadSettings(): Promise { - this.checkIfDestroyed("reloadSettings"); - - const state = (await this.persistence.load()) ?? { - settings: undefined - }; - - const settings = { - ...DEFAULT_SETTINGS, - ...(state.settings ?? {}) - }; - - await this.setSettings(settings); - } - public async checkConnection(): Promise { this.checkIfDestroyed("checkConnection"); @@ -405,10 +372,6 @@ export class SyncClient { this.checkIfDestroyed("syncLocallyCreatedFile"); this.fileChangeNotifier.notifyOfFileChange(relativePath); // this is for updating cursors - if (this.expectedFsEvents.matchCreate(relativePath)) { - return; - } - this.syncer.syncLocallyCreatedFile(relativePath); } @@ -422,10 +385,6 @@ export class SyncClient { this.checkIfDestroyed("syncLocallyUpdatedFile"); this.fileChangeNotifier.notifyOfFileChange(relativePath); // this is for updating cursors - if (this.expectedFsEvents.matchUpdate(relativePath, oldPath)) { - return; - } - this.syncer.syncLocallyUpdatedFile({ oldPath, relativePath @@ -436,10 +395,6 @@ export class SyncClient { this.checkIfDestroyed("syncLocallyDeletedFile"); this.fileChangeNotifier.notifyOfFileChange(relativePath); // this is for updating cursors - if (this.expectedFsEvents.matchDelete(relativePath)) { - return; - } - this.syncer.syncLocallyDeletedFile(relativePath); } @@ -567,10 +522,6 @@ export class SyncClient { // paused (offline edits, deletes, renames) wouldn't be detected, and // an incoming remote update would silently overwrite them. this.syncer.clearOfflineScanGate(); - // Drop any expected fs events that were registered but never matched - // (e.g. an op aborted by SyncResetError). Otherwise a real user edit - // at the same path after re-enable would be swallowed. - this.expectedFsEvents.clear(); } private resetInMemoryState(): void { diff --git a/frontend/sync-client/src/sync-operations/expected-fs-events.ts b/frontend/sync-client/src/sync-operations/expected-fs-events.ts deleted file mode 100644 index a2c4f52f..00000000 --- a/frontend/sync-client/src/sync-operations/expected-fs-events.ts +++ /dev/null @@ -1,138 +0,0 @@ -import type { RelativePath } from "./types"; - -/** - * Counter-based registry of filesystem events the syncer is about to - * cause. The syncer's own writes/renames/deletes go through - * `FileOperations`, which calls into the host filesystem; the host then - * fires watcher events that come back through `SyncClient.syncLocallyXxx`. - * Without filtering, those echo events would be re-uploaded to the server - * and broadcast back, producing an unbounded loop. - * - * The fix: every fs call in `FileOperations` registers the event it is - * about to provoke; the matching `syncLocallyXxx` handler consumes it. - * User-initiated edits never register, so they pass through unchanged. - * - * Counts are per (kind, path) so back-to-back syncer ops on the same path - * (e.g. apply remote update then re-apply during convergence) match - * one-for-one. If the watcher never fires for a registered op (e.g. the - * fs throws before notifying), the entry is left behind; `clear()` is - * called on pause/destroy to drop those before they collide with a real - * user event later. - */ -export class ExpectedFsEvents { - private readonly creates = new Map(); - private readonly updates = new Map(); - private readonly deletes = new Map(); - // Renames are keyed by `JSON.stringify({oldPath, newPath})` so the - // delimiter cannot occur inside either path. - private readonly renames = new Map(); - - private static renameKey( - oldPath: RelativePath, - newPath: RelativePath - ): string { - return JSON.stringify({ oldPath, newPath }); - } - - public expectCreate(path: RelativePath): void { - this.bump(this.creates, path); - } - - public expectUpdate(path: RelativePath): void { - this.bump(this.updates, path); - } - - public expectDelete(path: RelativePath): void { - this.bump(this.deletes, path); - } - - public expectRename(oldPath: RelativePath, newPath: RelativePath): void { - this.bump(this.renames, ExpectedFsEvents.renameKey(oldPath, newPath)); - } - - /** - * Cancel a previously-registered expectation when the fs op that registered - * it failed before any watcher event could fire. Without this, a leaked - * expectation silently swallows the next genuine user event at the same - * path (or, for renames, the same `oldPath → newPath` pair). - * - * Floored at zero: if the watcher *did* fire (op partially completed) and - * already consumed the entry, the unexpect is a no-op. The fallback is - * acceptable — at worst we re-upload a real edit we'd otherwise filter. - */ - public unexpectCreate(path: RelativePath): void { - this.decrement(this.creates, path); - } - - public unexpectUpdate(path: RelativePath): void { - this.decrement(this.updates, path); - } - - public unexpectDelete(path: RelativePath): void { - this.decrement(this.deletes, path); - } - - public unexpectRename(oldPath: RelativePath, newPath: RelativePath): void { - this.decrement( - this.renames, - ExpectedFsEvents.renameKey(oldPath, newPath) - ); - } - - public matchCreate(path: RelativePath): boolean { - return this.consume(this.creates, path); - } - - public matchUpdate( - path: RelativePath, - oldPath: RelativePath | undefined - ): boolean { - if (oldPath !== undefined) { - return this.consume( - this.renames, - ExpectedFsEvents.renameKey(oldPath, path) - ); - } - return this.consume(this.updates, path); - } - - public matchDelete(path: RelativePath): boolean { - return this.consume(this.deletes, path); - } - - public clear(): void { - this.creates.clear(); - this.updates.clear(); - this.deletes.clear(); - this.renames.clear(); - } - - private bump(map: Map, key: RelativePath): void { - map.set(key, (map.get(key) ?? 0) + 1); - } - - private consume( - map: Map, - key: RelativePath - ): boolean { - const count = map.get(key) ?? 0; - if (count === 0) { - return false; - } - if (count === 1) { - map.delete(key); - } else { - map.set(key, count - 1); - } - return true; - } - - private decrement(map: Map, key: RelativePath): void { - const count = map.get(key) ?? 0; - if (count <= 1) { - map.delete(key); - } else { - map.set(key, count - 1); - } - } -} diff --git a/frontend/sync-client/src/sync-operations/offline-change-detector.test.ts b/frontend/sync-client/src/sync-operations/offline-change-detector.test.ts index cc710e6a..3a7007ef 100644 --- a/frontend/sync-client/src/sync-operations/offline-change-detector.test.ts +++ b/frontend/sync-client/src/sync-operations/offline-change-detector.test.ts @@ -2,7 +2,10 @@ 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 { + 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"; @@ -22,19 +25,20 @@ const makeQueue = async (): Promise => { ); }; -const makeOperations = ( - files: Record -): FileOperations => { - return { - listFilesRecursively: async () => Object.keys(files), +const makeOperations = (files: Record): FileOperations => { + const map = new Map(Object.entries(files)); + const partial: Partial = { + listFilesRecursively: async () => [...map.keys()], read: async (path: RelativePath) => { - const data = files[path]; + const data = map.get(path); if (data === undefined) { throw new Error(`File not found: ${path}`); } return data; } - } as unknown as FileOperations; + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + return partial as FileOperations; }; describe("scheduleOfflineChanges", () => { @@ -70,7 +74,8 @@ describe("scheduleOfflineChanges", () => { operations, queue, (path) => enqueued.push({ kind: "create", path }), - (args) => enqueued.push({ kind: "update", path: args.relativePath }), + (args) => + enqueued.push({ kind: "update", path: args.relativePath }), (path) => enqueued.push({ kind: "delete", path }) ); @@ -109,13 +114,12 @@ describe("scheduleOfflineChanges", () => { operations, queue, (path) => enqueued.push({ kind: "create", path }), - (args) => enqueued.push({ kind: "update", path: args.relativePath }), + (args) => + enqueued.push({ kind: "update", path: args.relativePath }), (path) => enqueued.push({ kind: "delete", path }) ); - assert.deepStrictEqual(enqueued, [ - { kind: "update", path: "doc.md" } - ]); + assert.deepStrictEqual(enqueued, [{ kind: "update", path: "doc.md" }]); }); it("schedules a delete for a settled record whose local file is missing", async () => { @@ -136,13 +140,12 @@ describe("scheduleOfflineChanges", () => { operations, queue, (path) => enqueued.push({ kind: "create", path }), - (args) => enqueued.push({ kind: "update", path: args.relativePath }), + (args) => + enqueued.push({ kind: "update", path: args.relativePath }), (path) => enqueued.push({ kind: "delete", path }) ); - assert.deepStrictEqual(enqueued, [ - { kind: "delete", path: "gone.md" } - ]); + 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 () => { diff --git a/frontend/sync-client/src/sync-operations/offline-change-detector.ts b/frontend/sync-client/src/sync-operations/offline-change-detector.ts index 5b91e782..320ec92a 100644 --- a/frontend/sync-client/src/sync-operations/offline-change-detector.ts +++ b/frontend/sync-client/src/sync-operations/offline-change-detector.ts @@ -7,6 +7,24 @@ import type { SyncEventQueue } from "./sync-event-queue"; import { removeFromArray } from "../utils/remove-from-array"; import { FileNotFoundError } from "../errors/file-not-found-error"; +async function readOrUndefined( + operations: FileOperations, + path: RelativePath, + logger: Logger +): Promise { + try { + return await operations.read(path); + } catch (e) { + if (e instanceof FileNotFoundError) { + logger.debug( + `File ${path} disappeared before offline-scan could read it; skipping` + ); + return undefined; + } + throw e; + } +} + /** * Scans the local filesystem and the document database to determine * which files were created, updated, moved, or deleted while the @@ -85,18 +103,10 @@ export async function scheduleOfflineChanges( // the whole scan; nothing to sync for a file that's already gone. const disappearedPaths = new Set(); for (const path of locallyPossibleCreatedFiles) { - let content: Uint8Array; - try { - content = await operations.read(path); - } catch (e) { - if (e instanceof FileNotFoundError) { - logger.debug( - `File ${path} disappeared before offline-scan could read it; skipping` - ); - disappearedPaths.add(path); - continue; - } - throw e; + const content = await readOrUndefined(operations, path, logger); + if (content === undefined) { + disappearedPaths.add(path); + continue; } const contentHash = await hash(content); @@ -148,8 +158,7 @@ export async function scheduleOfflineChanges( for (const path of syncedLocalFiles) { const record = allDocuments.get(path); if ( - record !== undefined && - record.localPath !== undefined && + record?.localPath !== undefined && record.localPath !== record.remoteRelativePath && !allLocalFiles.has(record.remoteRelativePath) && queue.byLocalPath.get(record.remoteRelativePath) === undefined diff --git a/frontend/sync-client/src/sync-operations/reconciler.test.ts b/frontend/sync-client/src/sync-operations/reconciler.test.ts index 13a08363..9533b8e7 100644 --- a/frontend/sync-client/src/sync-operations/reconciler.test.ts +++ b/frontend/sync-client/src/sync-operations/reconciler.test.ts @@ -2,7 +2,10 @@ import { describe, it } from "node:test"; import assert from "node:assert"; import { Logger, LogLevel } from "../tracing/logger"; import { Settings } from "../persistence/settings"; -import { STORED_STATE_SCHEMA_VERSION, SyncEventQueue } from "./sync-event-queue"; +import { + STORED_STATE_SCHEMA_VERSION, + SyncEventQueue +} from "./sync-event-queue"; import { Reconciler } from "./reconciler"; import { SyncResetError } from "../errors/sync-reset-error"; import type { FileOperations } from "../file-operations/file-operations"; @@ -32,18 +35,22 @@ describe("Reconciler", () => { localPath: undefined }); - const operations = { + const operationsPartial: Partial = { exists: async () => false, create: async () => { assert.fail("reset-interrupted placement should not write"); } - } as unknown as FileOperations; + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const operations = operationsPartial as FileOperations; - const syncService = { + const syncServicePartial: Partial = { getDocumentVersionContent: async () => { throw new SyncResetError(); } - } as unknown as SyncService; + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const syncService = syncServicePartial as SyncService; const reconciler = new Reconciler( logger, diff --git a/frontend/sync-client/src/sync-operations/reconciler.ts b/frontend/sync-client/src/sync-operations/reconciler.ts index 93505a3c..f4d57762 100644 --- a/frontend/sync-client/src/sync-operations/reconciler.ts +++ b/frontend/sync-client/src/sync-operations/reconciler.ts @@ -306,9 +306,65 @@ export class Reconciler { } } + // Re-check ownership after the content fetch. A user rename or + // other interleaved op may have placed bytes / claimed the slot + // during the await. Without this, `upsertRecord` below would + // displace the new owner (clearing its `localPath`) and the + // following `operations.create` would then throw + // `FileAlreadyExistsError`, leaving the displaced record + // placement-pending with its bytes orphaned on disk. + try { + if (await this.operations.exists(target)) { + this.logger.debug( + `Reconciler: cannot place ${record.documentId} at ${target} ` + + `— slot newly occupied on disk after fetch; will retry next pass` + ); + return; + } + } catch (e) { + this.logger.error( + `Reconciler: existence check failed for ${target}: ${String(e)}` + ); + return; + } + if (this.queue.byLocalPath.get(target) !== undefined) { + this.logger.debug( + `Reconciler: cannot place ${record.documentId} at ${target} ` + + `— slot newly tracked by another record after fetch; will retry next pass` + ); + return; + } + + // Install the slot *before* the disk write so the watcher's + // create echo, when it arrives, sees `byLocalPath[target]` set + // and the queue's enqueue-time echo guard drops it. Also pre- + // populates `remoteHash` so any downstream operation that + // compares against it (e.g. `processLocalUpdate`'s hashChanged + // skip) sees the right value. Mirrors the ordering in + // `processRemoteCreateForNewDocument`'s quick-write branch. + const contentHash = await hash(content); + try { + await this.queue.upsertRecord({ + documentId: record.documentId, + parentVersionId: record.parentVersionId, + remoteRelativePath: record.remoteRelativePath, + remoteHash: contentHash, + localPath: target + }); + } catch (e) { + this.logger.error( + `Reconciler: upsertRecord before create failed for ${record.documentId}: ${String(e)}` + ); + return; + } + try { await this.operations.create(target, content); } catch (e) { + // Roll back the slot claim so a later pass can retry or + // re-resolve. Without this, the record looks placed but the + // bytes never made it to disk. + await this.queue.setLocalPath(record.documentId, undefined); if (e instanceof FileNotFoundError) { this.logger.debug( `Reconciler: create at ${target} hit FileNotFound (likely parent ` + @@ -329,14 +385,6 @@ export class Reconciler { return; } - try { - await this.queue.setLocalPath(record.documentId, target); - } catch (e) { - this.logger.error( - `Reconciler: setLocalPath after create failed for ${record.documentId}: ${String(e)}` - ); - return; - } this.pendingPlacementContent.delete(record.documentId); this.logger.debug( `Reconciler: placed ${record.documentId} at ${target}` @@ -663,8 +711,10 @@ export class Reconciler { // We pass the freshly-read pre-write content as // `expectedContent` so the 3-way merge inside `operations.write` // becomes a clean overwrite (no concurrent edits to merge with). - // `operations.write` registers `expectUpdate` itself, so the - // watcher swallows each leg's modify event. + // Each leg's echo modify event is harmless: the wire-loop's + // `processLocalUpdate` re-hashes the file and compares to + // `record.remoteHash`, which was set to match the just-written + // bytes — so the echo is dropped as a no-op. const writtenLegs: SwapLeg[] = []; for (const leg of legs) { const newBytes = contentByDocId.get(leg.documentId); diff --git a/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts index d2676011..f4aa6d22 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts @@ -66,7 +66,6 @@ function fakeRemoteVersion( userId: "user", deviceId: "device", contentSize: 100, - isNewFile: true, ...overrides }; } @@ -248,36 +247,33 @@ describe("SyncEventQueue", () => { assert.strictEqual(second.isUserRename, true); }); - it("settled record owns a path over a stale pending create", async () => { + it("drops LocalCreate echoes for paths already tracked", async () => { + // The syncer's own remote-create writes (quick-write + + // reconciler placements) upsert the record at `localPath` + // before calling `operations.create`. The watcher echo then + // re-enters as a LocalCreate at the same path — it must be + // dropped here, otherwise the wire-loop would POST a duplicate + // and the server would deconflict it into a phantom file. const queue = createQueue(); await queue.upsertRecord(fakeRecord("A", { localPath: "b.md" })); await queue.enqueue({ type: SyncEventType.LocalCreate, path: "b.md" }); - await queue.enqueue({ - type: SyncEventType.LocalUpdate, - path: "c.md", - oldPath: "b.md" - }); - const aRecord = queue.getDocumentByDocumentId("A"); - assert.strictEqual(aRecord?.localPath, "c.md"); - assert.strictEqual( - queue.getRecordByLocalPath("b.md" as RelativePath), - undefined - ); - assert.strictEqual( - queue.getRecordByLocalPath("c.md" as RelativePath)?.documentId, - "A" - ); + assert.strictEqual(await queue.next(), undefined); + }); + + it("admits LocalCreate when the prior owner is pending server delete", async () => { + // A user create at a path whose previous doc is in the + // HTTP-acked-but-WS-pending window is genuine — propagate it. + const queue = createQueue(); + await queue.upsertRecord(fakeRecord("A", { localPath: "b.md" })); + queue.markServerDeletePending("A"); + + await queue.enqueue({ type: SyncEventType.LocalCreate, path: "b.md" }); const create = await queue.next(); assert.strictEqual(create?.type, SyncEventType.LocalCreate); assert.strictEqual(create.path, "b.md"); - - const update = await queue.next(); - assert.strictEqual(update?.type, SyncEventType.LocalUpdate); - assert.strictEqual(update.documentId, "A"); - assert.strictEqual(update.path, "c.md"); }); it("byLocalPath stays consistent across upsertRecord, setLocalPath, and rename", async () => { @@ -307,7 +303,10 @@ describe("SyncEventQueue", () => { queue.byLocalPath.get("renamed.md" as RelativePath), undefined ); - assert.strictEqual(queue.getDocumentByDocumentId("A")?.localPath, "a.md"); + assert.strictEqual( + queue.getDocumentByDocumentId("A")?.localPath, + "a.md" + ); // setLocalPath does re-key — it's the explicit path-mutation API. await queue.setLocalPath("A", "later.md" as RelativePath); diff --git a/frontend/sync-client/src/sync-operations/sync-event-queue.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.ts index 75f675d0..f94f0d5b 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.ts @@ -94,6 +94,16 @@ export class SyncEventQueue { // `clearAllState` / schema-version-mismatch reset. private readonly _pendingServerDeletes = new Set(); + // DocIds we've seen deleted in this session. `removeDocumentById` + // adds here so that any stale `RemoteChange` for that doc that + // arrives later (e.g. an older vuid buffered in the network-chaos + // jitter pipeline, or a re-enqueue that landed after the delete's + // `purgeRemoteChangesForDocumentId`) is recognised in + // `processRemoteChange` and skipped instead of falling through to + // `processRemoteCreateForNewDocument` and resurrecting the doc + // with pre-delete bytes. Cleared on `clearAllState`. + private readonly _deletedDocumentIds = new Set(); + public constructor( private readonly settings: Settings, private readonly logger: Logger, @@ -145,9 +155,9 @@ export class SyncEventQueue { displaced.localPath = undefined; this.logger.warn( `Persisted state had two records sharing localPath ` + - `${record.localPath} (${displaced.documentId} and ` + - `${record.documentId}); clearing the prior holder's ` + - `localPath so the reconciler re-places it` + `${record.localPath} (${displaced.documentId} and ` + + `${record.documentId}); clearing the prior holder's ` + + `localPath so the reconciler re-places it` ); } this._byLocalPath.set(record.localPath, record); @@ -220,9 +230,7 @@ export class SyncEventQueue { * path) still fires when neither side holds a record for the * collision target. */ - public lastSeenUpdateIdForCreate( - requestPath: RelativePath - ): VaultUpdateId { + public lastSeenUpdateIdForCreate(requestPath: RelativePath): VaultUpdateId { let watermark = this._lastSeenUpdateId.min; for (const record of this.byDocId.values()) { if ( @@ -269,6 +277,16 @@ export class SyncEventQueue { } if (input.type === SyncEventType.LocalCreate) { + const owner = this._byLocalPath.get(path); + if ( + owner !== undefined && + !this.hasPendingServerDelete(owner.documentId) + ) { + this.logger.debug( + `Ignoring LocalCreate echo at ${path}: slot is already tracked by ${owner.documentId}` + ); + return; + } this.events.push({ type: SyncEventType.LocalCreate, path, @@ -281,7 +299,7 @@ export class SyncEventQueue { const lookupPath = input.type === SyncEventType.LocalUpdate && - input.oldPath !== undefined + input.oldPath !== undefined ? input.oldPath : path; const record = this._byLocalPath.get(lookupPath); @@ -324,7 +342,7 @@ export class SyncEventQueue { !pendingCreate.isProcessing ) { this.cancelPendingCreate(pendingCreate); - if (recordIsDeleting && record !== undefined) { + if (recordIsDeleting) { // A stale deleting record was still claiming this path. // The not-yet-started create/delete pair collapsed to // nothing, and the disk file is gone, so clear the stale @@ -343,11 +361,11 @@ export class SyncEventQueue { path: lookupPath }); this.notifyPendingUpdateCountChanged(); - if (recordOwnsLookupPath && record !== undefined) { + if (recordOwnsLookupPath) { // The file is gone from disk; clear the doc's localPath so the // Reconciler doesn't try to operate on a vacated slot. await this.setLocalPath(record.documentId, undefined); - } else if (recordIsDeleting && record !== undefined) { + } else if (recordIsDeleting) { // A stale deleting record was still claiming this path while a // newer pending create owned the actual disk file. Drop the // stale claim now that the file is gone. @@ -597,6 +615,15 @@ export class SyncEventQueue { } public async removeDocumentById(documentId: DocumentId): Promise { + // Record the tombstone unconditionally: `processRemoteChange` + // checks it to drop late RemoteChanges that would otherwise + // resurrect the doc via `processRemoteCreateForNewDocument`. + // Purging the queue (below) only catches events that are + // already enqueued; events that arrive after this point (e.g. + // a stale broadcast buffered in the network-chaos jitter + // pipeline, or a re-enqueue that lands after this purge) need + // the tombstone to be skipped. + this._deletedDocumentIds.add(documentId); const record = this.byDocId.get(documentId); if (record === undefined) { // Still clear any deletion-pending mark and purge stale @@ -620,13 +647,16 @@ export class SyncEventQueue { // in the queue ahead of it. Once those drain and the doc is // removed, a still-pending RemoteChange for an earlier version // would be processed by `processRemoteCreateForNewDocument` (the - // doc is now untracked, and catch-up's `isNewFile=true` semantics - // qualify it as a fresh create), resurrecting the doc on disk - // with stale bytes that disagree with every other agent. + // doc is now untracked), resurrecting the doc on disk with stale + // bytes that disagree with every other agent. this.purgeRemoteChangesForDocumentId(documentId); return this.save(); } + public hasBeenDeleted(documentId: DocumentId): boolean { + return this._deletedDocumentIds.has(documentId); + } + /** * Mark a doc as "HTTP DELETE has been acked by the server but the * WebSocket receipt that would call `removeDocumentById` hasn't arrived @@ -648,14 +678,6 @@ export class SyncEventQueue { return this.byDocId.get(target); } - public getDocumentByDocumentIdOrFail(target: DocumentId): DocumentRecord { - const result = this.getDocumentByDocumentId(target); - if (!result) { - throw new Error(`No document found with id ${target}`); - } - return result; - } - public getRecordByLocalPath( path: RelativePath ): DocumentRecord | undefined { @@ -740,6 +762,7 @@ export class SyncEventQueue { this.byDocId.clear(); this._byLocalPath.clear(); this._pendingServerDeletes.clear(); + this._deletedDocumentIds.clear(); this._lastSeenUpdateId.reset(); await this.save(); } @@ -807,13 +830,14 @@ export class SyncEventQueue { return; } - for (let i = createIndex + 1; i < this.events.length; ) { + for (let i = createIndex + 1; i < this.events.length;) { const event = this.events[i]; if ( event.type === SyncEventType.LocalDelete && event.path === path && event.documentId !== promise ) { + // eslint-disable-next-line no-restricted-syntax -- splice-by-index here is a reorder, not an item removal this.events.splice(i, 1); this.events.splice(createIndex, 0, event); createIndex++; @@ -859,13 +883,14 @@ export class SyncEventQueue { return; } - for (let i = createIndex + 1; i < this.events.length; ) { + for (let i = createIndex + 1; i < this.events.length;) { const event = this.events[i]; if ( event.type === SyncEventType.LocalUpdate && typeof event.documentId === "string" && blockingDocIds.has(event.documentId) ) { + // eslint-disable-next-line no-restricted-syntax -- splice-by-index here is a reorder, not an item removal this.events.splice(i, 1); this.events.splice(createIndex, 0, event); createIndex++; @@ -907,8 +932,8 @@ export class SyncEventQueue { this._byLocalPath.delete(previousLocalPath); } record.localPath = newLocalPath; - let displacedRecord: DocumentRecord | undefined; - let displacedOldPath: RelativePath | undefined; + let displacedRecord: DocumentRecord | undefined = undefined; + let displacedOldPath: RelativePath | undefined = undefined; if (newLocalPath !== undefined) { const displaced = this._byLocalPath.get(newLocalPath); if (displaced !== undefined && displaced !== record) { diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 4e908600..adc34217 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -592,10 +592,28 @@ export class Syncer { ): Promise { const documentId = await event.documentId; const record = this.queue.getDocumentByDocumentId(documentId); - if ( - record?.localPath !== undefined && - record.localPath !== event.path - ) { + if (record === undefined) { + // The doc is no longer tracked. Typical cause: a remote delete + // arrived first and `processRemoteDelete` already ran + // `removeDocumentById`, but `operations.delete` fired a + // watcher echo that landed in the queue as a stale LocalDelete. + // Without this skip we'd re-send DELETE to the server for a + // doc that's already gone. + this.logger.debug( + `Skipping local-delete for ${documentId} — doc no longer tracked` + ); + return; + } + if (this.queue.hasPendingServerDelete(documentId)) { + // We already initiated the server delete; nothing more to do. + // Reaches here when a LocalDelete echo lands behind the + // already-queued LocalDelete that drove the server delete. + this.logger.debug( + `Skipping local-delete for ${documentId} — server delete already pending` + ); + return; + } + if (record.localPath !== undefined && record.localPath !== event.path) { this.logger.debug( `Skipping local-delete for ${documentId} at ${event.path}: ` + `record now owns ${record.localPath}` @@ -703,8 +721,7 @@ export class Syncer { if (response.isDeleted) { await this.processRemoteDelete(record.localPath, { ...response, - contentSize: 0, - isNewFile: false + contentSize: 0 }); return; } @@ -859,10 +876,19 @@ export class Syncer { return this.processRemoteUpdate(trackedRecord, remoteVersion); } - if (!remoteVersion.isNewFile) { + // Tombstoned: we removed this doc in this session via + // `removeDocumentById` (either WS delete receipt or PUT response + // with `isDeleted=true`). A late RemoteChange for the same doc + // can still reach us — buffered in the network-chaos jitter + // pipeline, or re-enqueued after the delete's purge — and + // without this gate `processRemoteCreateForNewDocument` would + // happily fetch pre-delete bytes and resurrect the doc, blocking + // any other doc whose `remoteRelativePath` happens to be the + // same slot. + if (this.queue.hasBeenDeleted(remoteVersion.documentId)) { this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId; this.logger.debug( - `Ignoring stale RemoteChange for untracked, non-new document ${remoteVersion.documentId}` + `Discarding stale remote update for tombstoned ${remoteVersion.documentId} at ${remoteVersion.relativePath}` ); return; } @@ -1103,7 +1129,7 @@ export class Syncer { remoteHash, localPath: target }); - const result = await this.operations.create( + const createdPath = await this.operations.create( target, remoteContent ); @@ -1112,7 +1138,7 @@ export class Syncer { ); localPath = liveRecord === undefined - ? result.actualPath + ? createdPath : liveRecord.localPath; await this.updateCache( remoteVersion.vaultUpdateId, diff --git a/frontend/sync-client/src/tracing/logger.ts b/frontend/sync-client/src/tracing/logger.ts index 6d544fbc..1801a40f 100644 --- a/frontend/sync-client/src/tracing/logger.ts +++ b/frontend/sync-client/src/tracing/logger.ts @@ -54,11 +54,6 @@ export class Logger { ); } - public reset(): void { - this.messages.length = 0; - this.debug("Logger has been reset"); - } - private pushMessage(message: string, level: LogLevel): void { const logLine = new LogLine(level, message); this.messages.push(logLine); diff --git a/frontend/sync-client/src/utils/data-structures/locks.ts b/frontend/sync-client/src/utils/data-structures/locks.ts index 99c33075..452fa874 100644 --- a/frontend/sync-client/src/utils/data-structures/locks.ts +++ b/frontend/sync-client/src/utils/data-structures/locks.ts @@ -92,10 +92,6 @@ export class Locks { this.waiters.clear(); } - public isLocked(key: T): boolean { - return this.locked.has(key); - } - /** * Attempts to acquire a lock immediately without waiting. * Must call `unlock()` if successful. diff --git a/frontend/test-client/src/agent/mock-agent.ts b/frontend/test-client/src/agent/mock-agent.ts index d4fc8c82..53dd59f1 100644 --- a/frontend/test-client/src/agent/mock-agent.ts +++ b/frontend/test-client/src/agent/mock-agent.ts @@ -58,16 +58,18 @@ export class MockAgent extends MockClient { // (e.g. `initial-1.md → initial-1 (2).md` after a same-path // collision) lands at a path the touch-list never knew about, // and an offline rename against that path strands the file. - this.client.onDocumentPathChanged.add((_documentId, oldPath, newPath) => { - if (oldPath !== undefined && newPath !== undefined) { - if (this.doNotTouchWhileOffline.includes(oldPath)) { - this.doNotTouchWhileOffline.push(newPath); - } - if (this.doNotRenameWhileOffline.includes(oldPath)) { - this.doNotRenameWhileOffline.push(newPath); + this.client.onDocumentPathChanged.add( + (_documentId, oldPath, newPath) => { + if (oldPath !== undefined && newPath !== undefined) { + if (this.doNotTouchWhileOffline.includes(oldPath)) { + this.doNotTouchWhileOffline.push(newPath); + } + if (this.doNotRenameWhileOffline.includes(oldPath)) { + this.doNotRenameWhileOffline.push(newPath); + } } } - }); + ); this.client.logger.onLogEmitted.add((logLine: LogLine) => { const state = this.client.getSettings().isSyncEnabled diff --git a/frontend/test-client/src/cli.ts b/frontend/test-client/src/cli.ts index ece94cc3..11e776f5 100644 --- a/frontend/test-client/src/cli.ts +++ b/frontend/test-client/src/cli.ts @@ -7,7 +7,7 @@ import { randomCasing } from "./utils/random-casing"; import { TimeoutError } from "./utils/with-timeout"; import { TestErrorTracker } from "./utils/test-error-tracker"; -const TEST_ITERATIONS = 5; +const TEST_ITERATIONS = 50; const MAX_INITIAL_DOCS = 10; // Simulate async file access by injecting waiting time before returning from file operations. diff --git a/scripts/check.sh b/scripts/check.sh index 2ee0dd62..26776086 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -21,9 +21,10 @@ cargo test --verbose if [[ "$FIX_MODE" == true ]]; then cargo clippy --all-targets --all-features --fix --allow-dirty --allow-staged + cargo clippy --all-targets --all-features -- -D warnings cargo fmt --all else - cargo clippy --all-targets --all-features + cargo clippy --all-targets --all-features -- -D warnings cargo fmt --all -- --check fi diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 7ab8d90c..eee13507 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -91,25 +91,10 @@ print_failed_log() { return 1 } -E2E_TIMEOUT=${2:-3600} -start_time=$(date +%s) -echo "Monitoring $process_count processes (timeout: ${E2E_TIMEOUT}s)" +echo "Monitoring $process_count processes" # Monitor processes while true; do - # Script-level timeout to prevent indefinite hangs - current_time=$(date +%s) - elapsed=$((current_time - start_time)) - if [ $elapsed -ge $E2E_TIMEOUT ]; then - echo "E2E timeout reached (${E2E_TIMEOUT}s). Killing remaining processes." - for pid in "${pids[@]}"; do - if [ -n "$pid" ]; then - kill $pid 2>/dev/null || true - fi - done - exit 1 - fi - if print_failed_log; then # Kill remaining processes for pid in "${pids[@]}"; do diff --git a/scripts/update-api-types.sh b/scripts/update-api-types.sh index 3f4a9e2a..5c49f10d 100755 --- a/scripts/update-api-types.sh +++ b/scripts/update-api-types.sh @@ -8,11 +8,9 @@ cd sync-server cargo test export_bindings cd - -# Both target directories contain only generated bindings — wipe and copy +# Wipe and copy generated bindings into the consuming workspace rm -f frontend/sync-client/src/services/types/*.ts -rm -f frontend/history-ui/src/lib/types/*.ts cp -r sync-server/bindings/* frontend/sync-client/src/services/types/ -cp -r sync-server/bindings/* frontend/history-ui/src/lib/types/ cd frontend npm run lint diff --git a/sync-server/Cargo.lock b/sync-server/Cargo.lock index 7f9efb39..bf085a4f 100644 --- a/sync-server/Cargo.lock +++ b/sync-server/Cargo.lock @@ -2181,7 +2181,6 @@ dependencies = [ "log", "rand 0.9.0", "reconcile-text", - "regex", "sanitize-filename", "serde", "serde_json", diff --git a/sync-server/Cargo.toml b/sync-server/Cargo.toml index 2fed9d9b..0d378a4b 100644 --- a/sync-server/Cargo.toml +++ b/sync-server/Cargo.toml @@ -26,7 +26,6 @@ sqlx = { version = "0.8.6", features = ["sqlite", "runtime-tokio", "uuid", "chro chrono = { version = "0.4.41", features = ["serde"] } rand = "0.9.0" sanitize-filename = "0.6.0" -regex = "1.12.2" clap = { version = "4.5.38", features = ["derive"] } futures = "0.3.31" serde_yaml = "0.9.34" @@ -49,15 +48,19 @@ rust_2018_idioms = { level = "warn", priority = -1 } missing_debug_implementations = "warn" [lints.clippy] +arithmetic_side_effects = "deny" await_holding_lock = "warn" dbg_macro = "warn" -empty_enum = "warn" +disallowed_macros = { level = "deny", priority = 1 } +empty_enums = "warn" enum_glob_use = "warn" +expect_used = "deny" exit = "warn" filter_map_next = "warn" fn_params_excessive_bools = "warn" if_let_mutex = "warn" imprecise_flops = "warn" +indexing_slicing = "deny" inefficient_to_string = "warn" linkedlist = "warn" lossy_float_literal = "warn" @@ -67,13 +70,19 @@ mem_forget = "warn" needless_borrow = "warn" needless_continue = "warn" option_option = "warn" +panic = "deny" +panic_in_result_fn = "deny" rest_pat_in_fully_bound_structs = "warn" str_to_string = "warn" suboptimal_flops = "warn" -todo = "warn" +todo = "deny" uninlined_format_args = "warn" +unimplemented = "deny" +unreachable = "deny" unnested_or_patterns = "warn" unused_self = "warn" +unwrap_in_result = "deny" +unwrap_used = "deny" verbose_file_reads = "warn" large_stack_arrays = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/13774 @@ -87,7 +96,7 @@ single_call_fn = { level = "allow", priority = 1 } similar_names = { level = "allow", priority = 1 } missing_docs_in_private_items = { level = "allow", priority = 1 } -pedantic = { level = "warn", priority = 0 } +pedantic = { level = "warn", priority = -1 } [package.metadata.cargo-machete] ignored = ["humantime-serde"] # only used in serde macro diff --git a/sync-server/build.rs b/sync-server/build.rs index 25c39362..d5068697 100644 --- a/sync-server/build.rs +++ b/sync-server/build.rs @@ -1,3 +1,4 @@ +// generated by `sqlx migrate build-script` fn main() { // trigger recompilation when a new migration is added println!("cargo:rerun-if-changed=migrations"); diff --git a/sync-server/clippy.toml b/sync-server/clippy.toml new file mode 100644 index 00000000..81c6e562 --- /dev/null +++ b/sync-server/clippy.toml @@ -0,0 +1,3 @@ +disallowed-macros = [ + { path = "std::eprintln", reason = "use log::info! or log::warn! instead" }, +] diff --git a/sync-server/src/app_state/cursors.rs b/sync-server/src/app_state/cursors.rs index e17fb4f7..130da680 100644 --- a/sync-server/src/app_state/cursors.rs +++ b/sync-server/src/app_state/cursors.rs @@ -7,16 +7,16 @@ use super::{ database::models::{DeviceId, VaultId}, websocket::{ broadcasts::Broadcasts, - models::{ - ClientCursors, CursorPositionFromServer, WebSocketServerMessage, - WebSocketServerMessageWithOrigin, - }, + models::{ClientCursors, CursorPositionFromServer, WebSocketServerMessage}, }, }; use crate::{ app_state::websocket::models::DocumentWithCursors, config::database_config::DatabaseConfig, + errors::SyncServerError, }; +const CURSOR_CLEANUP_INTERVAL: Duration = Duration::from_secs(1); + #[derive(Clone, Debug)] pub struct Cursors { config: DatabaseConfig, @@ -39,7 +39,7 @@ impl Cursors { user_name: String, device_id: &DeviceId, document_to_cursors: Vec, - ) { + ) -> Result<(), SyncServerError> { let mut vault_to_cursors = self.vault_to_cursors.lock().await; let all_device_cursors = vault_to_cursors @@ -54,7 +54,7 @@ impl Cursors { })); drop(vault_to_cursors); // Explicitly drop the lock before broadcasting to avoid deadlock - self.broadcast_cursors_for_vault(&vault_id).await; + self.broadcast_cursors_for_vault(&vault_id).await } pub async fn get_cursors(&self, vault_id: &VaultId) -> Vec { @@ -75,16 +75,18 @@ impl Cursors { tokio::spawn(async move { loop { tokio::select! { - () = tokio::time::sleep(Duration::from_secs(1)) => { - self.remove_expired_cursors().await; + () = tokio::time::sleep(CURSOR_CLEANUP_INTERVAL) => { + self.remove_expired_cursors().await?; } Ok(()) = shutdown.changed() => break, } } + + Ok::<(), SyncServerError>(()) }); } - async fn remove_expired_cursors(&self) { + async fn remove_expired_cursors(&self) -> Result<(), SyncServerError> { let changed_vaults: Vec = { let mut vault_to_cursors = self.vault_to_cursors.lock().await; @@ -104,11 +106,13 @@ impl Cursors { }; for vault_id in &changed_vaults { - self.broadcast_cursors_for_vault(vault_id).await; + self.broadcast_cursors_for_vault(vault_id).await?; } + + Ok(()) } - async fn broadcast_cursors_for_vault(&self, vault_id: &VaultId) { + async fn broadcast_cursors_for_vault(&self, vault_id: &VaultId) -> Result<(), SyncServerError> { let client_cursors: Vec = { let vault_to_cursors = self.vault_to_cursors.lock().await; vault_to_cursors @@ -118,16 +122,18 @@ impl Cursors { }; self.broadcasts.send_document_update( - vault_id.clone(), - WebSocketServerMessageWithOrigin::new(WebSocketServerMessage::CursorPositions( - CursorPositionFromServer { - clients: client_cursors, - }, - )), - ); + vault_id, + WebSocketServerMessage::CursorPositions(CursorPositionFromServer { + clients: client_cursors, + }), + ) } - pub async fn remove_cursors_of_device(&self, vault_id: &VaultId, device_id: &DeviceId) { + pub async fn remove_cursors_of_device( + &self, + vault_id: &VaultId, + device_id: &DeviceId, + ) -> Result<(), SyncServerError> { let changed = { let mut vault_to_cursors = self.vault_to_cursors.lock().await; @@ -145,8 +151,9 @@ impl Cursors { }; if changed { - self.broadcast_cursors_for_vault(vault_id).await; + self.broadcast_cursors_for_vault(vault_id).await?; } + Ok(()) } } diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index 1fa6d223..86f00d6d 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -12,6 +12,8 @@ use models::{ }; use sqlx::{ConnectOptions, Connection, sqlite::SqliteConnectOptions, types::chrono::Utc}; +use crate::errors::{SyncServerError, database_error, server_error}; + pub mod models; /// Sentinel error indicating the `SQLite` database is busy (`SQLITE_BUSY`). @@ -20,6 +22,24 @@ pub mod models; #[error("Database is busy")] pub struct WriteBusyError; +/// Detects whether a `sqlx::Error` indicates the database is currently +/// unavailable for a retryable reason: a `SQLITE_BUSY` from the engine, or +/// a `PoolTimedOut` from our short acquire timeout. Both should surface as +/// 429 so the client retries instead of treating it as a server fault. +pub fn is_sqlite_busy_error(err: &sqlx::Error) -> bool { + match err { + sqlx::Error::Database(db_err) => { + // SQLITE_BUSY base code is 5. Extended codes share base 5. + let busy_by_code = db_err + .code() + .is_some_and(|c| c.parse::().is_ok_and(|n| n & 0xFF == 5)); + busy_by_code || db_err.message().contains("database is locked") + } + sqlx::Error::PoolTimedOut => true, + _ => false, + } +} + use sqlx::{ Pool, Sqlite, pool::PoolConnection, sqlite::SqliteConnection, sqlite::SqlitePoolOptions, }; @@ -29,10 +49,14 @@ use uuid::fmt::Hyphenated; use super::websocket::{ broadcasts::Broadcasts, - models::{WebSocketServerMessage, WebSocketServerMessageWithOrigin, WebSocketVaultUpdate}, + models::{WebSocketServerMessage, WebSocketVaultUpdate}, }; use crate::config::database_config::DatabaseConfig; -use crate::consts::IDLE_POOL_TIMEOUT; +use crate::consts::{IDLE_POOL_TIMEOUT, POOL_ACQUIRE_TIMEOUT}; + +fn duration_millis_u64(duration: Duration) -> u64 { + u64::try_from(duration.as_millis()).unwrap_or(u64::MAX) +} /// Holds separate reader and writer pools for a single vault. /// The writer pool has exactly 1 connection so writes never compete @@ -83,22 +107,16 @@ impl WriteTransaction { pool: &Pool, write_guard: tokio::sync::OwnedMutexGuard<()>, ) -> Result { - let mut conn = pool - .acquire() - .await - .context("Cannot acquire connection for write transaction")?; + let mut conn = match pool.acquire().await { + Ok(conn) => conn, + Err(e) if is_sqlite_busy_error(&e) => return Err(WriteBusyError.into()), + Err(e) => { + return Err(anyhow::Error::from(e) + .context("Cannot acquire connection for write transaction")); + } + }; if let Err(e) = sqlx::query("BEGIN IMMEDIATE").execute(&mut *conn).await { - let is_busy = match &e { - sqlx::Error::Database(db_err) => { - // SQLITE_BUSY base code is 5. Extended codes share base 5. - let busy_by_code = db_err - .code() - .is_some_and(|c| c.parse::().is_ok_and(|n| n & 0xFF == 5)); - busy_by_code || db_err.message().contains("database is locked") - } - _ => false, - }; - if is_busy { + if is_sqlite_busy_error(&e) { return Err(WriteBusyError.into()); } return Err(e).context("Cannot begin immediate transaction"); @@ -109,25 +127,33 @@ impl WriteTransaction { }) } - pub async fn commit(mut self) -> Result<()> { + pub async fn commit(mut self) -> Result<(), SyncServerError> { if let Some(mut conn) = self.conn.take() { sqlx::query("COMMIT") .execute(&mut *conn) .await - .context("Failed to commit transaction")?; + .context("Failed to commit transaction") + .map_err(database_error)?; } Ok(()) } - pub async fn rollback(mut self) -> Result<()> { + pub async fn rollback(mut self) -> Result<(), SyncServerError> { if let Some(mut conn) = self.conn.take() { sqlx::query("ROLLBACK") .execute(&mut *conn) .await - .context("Failed to rollback transaction")?; + .context("Failed to rollback transaction") + .map_err(database_error)?; } Ok(()) } + + pub fn connection_mut(&mut self) -> Result<&mut SqliteConnection> { + self.conn + .as_deref_mut() + .context("WriteTransaction already consumed") + } } impl Drop for WriteTransaction { @@ -143,25 +169,6 @@ impl Drop for WriteTransaction { } } -impl std::ops::Deref for WriteTransaction { - type Target = SqliteConnection; - fn deref(&self) -> &Self::Target { - self.conn - .as_ref() - .expect("BUG: WriteTransaction dereferenced after being consumed") - .deref() - } -} - -impl std::ops::DerefMut for WriteTransaction { - fn deref_mut(&mut self) -> &mut Self::Target { - self.conn - .as_mut() - .expect("BUG: WriteTransaction dereferenced after being consumed") - .deref_mut() - } -} - /// Ensure the connection has no leftover open transaction (e.g. from a /// `WriteTransaction` that was dropped without commit/rollback). ROLLBACK /// is a harmless no-op if no transaction is active. @@ -182,7 +189,7 @@ fn rollback_before_acquire( impl Database { fn now_ms(&self) -> u64 { - self.epoch.elapsed().as_millis() as u64 + duration_millis_u64(self.epoch.elapsed()) } pub async fn try_new( @@ -274,10 +281,14 @@ impl Database { drop(init_conn); // Per-connection PRAGMAs shared by both reader and writer pools. - // journal_mode = WAL is a no-op on an already-WAL database. + // Database-level PRAGMAs (auto_vacuum, journal_mode) are deliberately + // omitted here: they require a write lock to verify or set, so issuing + // them on every new pool connection blocks behind any in-flight writer + // and can fail with SQLITE_BUSY just to open a connection. The init + // connection above set them once; the WAL mode persists in the database + // header, so subsequent opens pick it up automatically. let base_options = SqliteConnectOptions::new() .filename(file_name.clone()) - .journal_mode(sqlx::sqlite::SqliteJournalMode::Wal) .busy_timeout(Duration::from_secs(30)) .log_slow_statements(log::LevelFilter::Warn, Duration::from_secs(30)) // In WAL mode, NORMAL is safe: data survives OS crashes, only the @@ -301,6 +312,7 @@ impl Database { // Reader pool: multiple connections for concurrent reads. let reader = SqlitePoolOptions::new() .max_connections(config.max_connections_per_vault) + .acquire_timeout(POOL_ACQUIRE_TIMEOUT) .acquire_slow_threshold(Duration::from_secs(30)) // Disabled: the health-check query is subject to busy_timeout // and blocks all connection checkouts when a write is active, @@ -318,6 +330,7 @@ impl Database { // reader pool ensures writes never compete with reads for pool slots. let writer = SqlitePoolOptions::new() .max_connections(1) + .acquire_timeout(POOL_ACQUIRE_TIMEOUT) .acquire_slow_threshold(Duration::from_secs(30)) .test_before_acquire(false) .before_acquire(rollback_before_acquire) @@ -384,7 +397,10 @@ impl Database { Ok(self.get_vault_pools(vault).await?.reader) } - pub async fn create_write_transaction(&self, vault: &VaultId) -> Result { + pub async fn create_write_transaction( + &self, + vault: &VaultId, + ) -> Result { let write_lock = { let mut locks = self.write_locks.lock().await; locks @@ -393,8 +409,10 @@ impl Database { .clone() }; let write_guard = write_lock.lock_owned().await; - let pools = self.get_vault_pools(vault).await?; - WriteTransaction::new(&pools.writer, write_guard).await + let pools = self.get_vault_pools(vault).await.map_err(database_error)?; + WriteTransaction::new(&pools.writer, write_guard) + .await + .map_err(database_error) } /// Return the latest state of all documents in the vault, optionally @@ -406,7 +424,7 @@ impl Database { vault: &VaultId, up_to_vault_update_id: Option, connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { // `i64::MAX` makes the upper bound a no-op for callers that don't // care about an exact snapshot (they pass `None`). let upper = up_to_vault_update_id.unwrap_or(i64::MAX); @@ -414,7 +432,6 @@ impl Database { r#" select vault_update_id, - creation_vault_update_id, document_id as "document_id: Hyphenated", relative_path, updated_date as "updated_date: chrono::DateTime", @@ -433,7 +450,12 @@ impl Database { query.fetch_all(&mut *conn).await } else { query - .fetch_all(&self.get_connection_pool(vault).await?) + .fetch_all( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .context("Cannot fetch latest documents") @@ -448,10 +470,10 @@ impl Database { user_id: row.user_id, device_id: row.device_id, content_size: row.content_size.unwrap_or(0), - is_new_file: row.creation_vault_update_id == row.vault_update_id, }) .collect() }) + .map_err(database_error) } /// Return the latest state of all documents (including deleted) in the @@ -465,7 +487,7 @@ impl Database { vault_update_id: VaultUpdateId, up_to_vault_update_id: Option, connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { // `i64::MAX` makes the upper bound a no-op for callers that don't // care about an exact snapshot (they pass `None`). let upper = up_to_vault_update_id.unwrap_or(i64::MAX); @@ -475,19 +497,14 @@ impl Database { // cursor capture (under broadcast send-lock) and this query // (which runs after drop-lock) would expose a `vault_update_id // > cursor` row that the cursor filter then drops, removing - // the doc from the catch-up entirely. The post-cursor live - // broadcast then carries `is_new_file = false` (per real-time - // semantics it's an update of a previously-existing version), - // and the receiving client — which has no record of the doc — - // ignores it as stale, stranding the doc forever. Computing - // the snapshot from the documents table directly with the - // upper bound applied at the GROUP BY layer keeps the - // catch-up self-contained at exactly the cursor. + // the doc from the catch-up entirely. Computing the snapshot + // from the documents table directly with the upper bound + // applied at the GROUP BY layer keeps the catch-up + // self-contained at exactly the cursor. let query = sqlx::query!( r#" select d.vault_update_id, - d.creation_vault_update_id, d.document_id as "document_id: Hyphenated", d.relative_path, d.updated_date as "updated_date: chrono::DateTime", @@ -515,7 +532,12 @@ impl Database { query.fetch_all(&mut *conn).await } else { query - .fetch_all(&self.get_connection_pool(vault).await?) + .fetch_all( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .with_context(|| { @@ -532,27 +554,17 @@ impl Database { user_id: row.user_id, device_id: row.device_id, content_size: row.content_size.unwrap_or(0), - // For catch-up streams, "new file" means "new to this - // recipient" — the doc was created past the recipient's - // watermark. The catch-up only carries the doc's - // *latest* version (not its full history), so using - // `creation == latest` instead would mis-flag every - // doc that was created and then updated before the - // client reconnected, and the client's - // `processRemoteChange` would drop it as "stale - // RemoteChange for untracked, non-new document", - // silently leaking docs to clients catching up. - is_new_file: row.creation_vault_update_id > vault_update_id, }) .collect() }) + .map_err(database_error) } pub async fn get_max_update_id_in_vault( &self, vault: &VaultId, connection: Option<&mut SqliteConnection>, - ) -> Result { + ) -> Result { let query = sqlx::query!( r#" select coalesce(max(vault_update_id), 0) as max_vault_update_id @@ -564,11 +576,17 @@ impl Database { query.fetch_one(&mut *conn).await } else { query - .fetch_one(&self.get_connection_pool(vault).await?) + .fetch_one( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .map(|row| row.max_vault_update_id) .context("Cannot fetch max update id in vault") + .map_err(database_error) } pub async fn get_latest_non_deleted_document_by_path( @@ -576,7 +594,7 @@ impl Database { vault: &VaultId, relative_path: &str, connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { let query = sqlx::query_as!( StoredDocumentVersion, r#" @@ -605,10 +623,16 @@ impl Database { query.fetch_optional(&mut *conn).await } else { query - .fetch_optional(&self.get_connection_pool(vault).await?) + .fetch_optional( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .context("Cannot fetch latest document version") + .map_err(database_error) } /// Find a doc whose CREATE was authored by this device with @@ -638,7 +662,7 @@ impl Database { last_seen_vault_update_id: VaultUpdateId, content: &[u8], connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { let query = sqlx::query_as!( StoredDocumentVersion, r#" @@ -673,10 +697,16 @@ impl Database { query.fetch_optional(&mut *conn).await } else { query - .fetch_optional(&self.get_connection_pool(vault).await?) + .fetch_optional( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .context("Cannot fetch lost-create candidate") + .map_err(database_error) } pub async fn get_latest_document( @@ -684,7 +714,7 @@ impl Database { vault: &VaultId, document_id: &DocumentId, connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { let document_id = document_id.as_hyphenated(); let query = sqlx::query_as!( StoredDocumentVersion, @@ -710,10 +740,16 @@ impl Database { query.fetch_optional(&mut *conn).await } else { query - .fetch_optional(&self.get_connection_pool(vault).await?) + .fetch_optional( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .context("Cannot fetch latest document version") + .map_err(database_error) } pub async fn get_document_version( @@ -721,7 +757,7 @@ impl Database { vault: &VaultId, vault_update_id: VaultUpdateId, connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { let query = sqlx::query_as!( StoredDocumentVersion, r#" @@ -745,10 +781,16 @@ impl Database { query.fetch_optional(&mut *conn).await } else { query - .fetch_optional(&self.get_connection_pool(vault).await?) + .fetch_optional( + &self + .get_connection_pool(vault) + .await + .map_err(database_error)?, + ) .await } .context("Cannot fetch document version") + .map_err(database_error) } // inserting the document must be the last step of the transaction @@ -757,7 +799,7 @@ impl Database { vault_id: &VaultId, version: &StoredDocumentVersion, mut transaction: WriteTransaction, - ) -> Result<()> { + ) -> Result<(), SyncServerError> { let document_id = version.document_id.as_hyphenated(); let query = sqlx::query!( r#" @@ -793,32 +835,26 @@ impl Database { let _send_guard = self.broadcasts.acquire_send_lock(vault_id).await; query - .execute(&mut *transaction) + .execute(transaction.connection_mut().map_err(server_error)?) .await - .context("Cannot insert document version")?; + .context("Cannot insert document version") + .map_err(database_error)?; - transaction - .commit() - .await - .context("Failed to commit transaction")?; + transaction.commit().await?; - // For non-delete writes the originating device already has - // authoritative state from its HTTP response, so we tag the - // broadcast with `origin_device_id` and the send task in - // `websocket.rs` filters it out for that device. Deletes are - // delivered to *every* connected client including the author — - // the originator only removes the document from its sync queue - // once it receives this receipt. - let envelope = WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate { - document: version.clone().into(), - }); - let with_origin = if version.is_deleted { - WebSocketServerMessageWithOrigin::new(envelope) - } else { - WebSocketServerMessageWithOrigin::with_origin(version.device_id.clone(), envelope) - }; - self.broadcasts - .send_document_update(vault_id.clone(), with_origin); + // Broadcast every commit to every connected client, including + // the originator. The HTTP response is the originator's normal + // path to learn its own update, but if the response is lost + // (sync reset, dropped TCP) the broadcast is the only remaining + // delivery channel — and the client-side `parentVersionId` + // dedup absorbs the redundant message when the response made it + // through. + self.broadcasts.send_document_update( + vault_id, + WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate { + document: version.clone().into(), + }), + )?; Ok(()) } @@ -831,7 +867,7 @@ impl Database { let idle_pools: Vec<(VaultId, Arc)> = { let mut pools = self.connection_pools.lock().await; let now_ms = self.now_ms(); - let idle_threshold_ms = IDLE_POOL_TIMEOUT.as_millis() as u64; + let idle_threshold_ms = duration_millis_u64(IDLE_POOL_TIMEOUT); let vaults_to_remove: Vec = pools .iter() diff --git a/sync-server/src/app_state/database/models.rs b/sync-server/src/app_state/database/models.rs index 976cc7e5..708a5b99 100644 --- a/sync-server/src/app_state/database/models.rs +++ b/sync-server/src/app_state/database/models.rs @@ -46,14 +46,10 @@ pub struct DocumentVersionWithoutContent { #[ts(type = "number")] pub content_size: u64, - - /// True iff this is the first version of the document - pub is_new_file: bool, } impl From for DocumentVersionWithoutContent { fn from(value: StoredDocumentVersion) -> Self { - let is_new_file = value.creation_vault_update_id == value.vault_update_id; Self { vault_update_id: value.vault_update_id, document_id: value.document_id, @@ -63,7 +59,6 @@ impl From for DocumentVersionWithoutContent { user_id: value.user_id, device_id: value.device_id, content_size: value.content.len() as u64, - is_new_file, } } } @@ -83,7 +78,6 @@ pub struct DocumentVersion { pub device_id: DeviceId, } - impl From for DocumentVersion { fn from(value: StoredDocumentVersion) -> Self { Self { diff --git a/sync-server/src/app_state/websocket/broadcasts.rs b/sync-server/src/app_state/websocket/broadcasts.rs index b9e2ea39..45ce78cb 100644 --- a/sync-server/src/app_state/websocket/broadcasts.rs +++ b/sync-server/src/app_state/websocket/broadcasts.rs @@ -3,11 +3,15 @@ use std::{ sync::{Arc, Mutex as StdMutex}, }; -use log::{debug, info, warn}; +use log::{debug, warn}; use tokio::sync::{Mutex, broadcast}; -use super::models::{WebSocketServerMessage, WebSocketServerMessageWithOrigin}; -use crate::{app_state::database::models::VaultId, config::server_config::ServerConfig}; +use super::models::WebSocketServerMessage; +use crate::{ + app_state::database::models::VaultId, + config::server_config::ServerConfig, + errors::{SyncServerError, client_error, server_error}, +}; #[derive(Debug, Clone)] pub struct Broadcasts { @@ -17,11 +21,11 @@ pub struct Broadcasts { // this non-async lets `send_document_update` run without an `.await`, // so an axum handler that is cancelled between `transaction.commit()` // and the broadcast can never drop the notification mid-flight. - tx: Arc>>>, + tx: Arc>>>, send_locks: Arc>>>>, } -type TxMap = HashMap>; +type TxMap = HashMap>; impl Broadcasts { pub fn new(server_config: &ServerConfig) -> Self { @@ -60,88 +64,64 @@ impl Broadcasts { pub fn get_receiver( &self, - vault: VaultId, + vault: &VaultId, max_clients: usize, - ) -> Result, crate::errors::SyncServerError> - { + ) -> Result, SyncServerError> { let mut tx_map = self .tx .lock() - .expect("broadcasts.tx mutex poisoned — a previous holder panicked"); + .map_err(|_| server_error(anyhow::anyhow!("broadcasts.tx mutex poisoned")))?; - let count_before_prune = tx_map - .get(&vault) - .map_or(0, tokio::sync::broadcast::Sender::receiver_count); - let pruned = Self::prune_inactive_vaults(&mut tx_map); - let pruned_self = pruned.contains(&vault); + Self::prune_inactive_vaults(&mut tx_map); let sender = tx_map - .entry(vault.clone()) + .entry(vault.to_owned()) .or_insert_with(|| broadcast::channel(self.broadcast_channel_capacity).0); // Hold the lock across the count check *and* the subscribe so the // `max_clients` cap is atomic: two concurrent callers can't both // observe `receiver_count() < max_clients` and both subscribe. if sender.receiver_count() >= max_clients { - return Err(crate::errors::client_error(anyhow::anyhow!( + return Err(client_error(anyhow::anyhow!( "Vault has reached the maximum number of clients ({max_clients})" ))); } let receiver = sender.subscribe(); - let count_after = sender.receiver_count(); - info!( - "[BCAST] get_receiver vault={vault} count_before_prune={count_before_prune} pruned_self={pruned_self} pruned_total={} count_after_subscribe={count_after}", - pruned.len() - ); Ok(receiver) } /// Notify all clients (who are subscribed to the vault) about an update. /// Synchronous: safe to invoke from a handler between `commit()` and /// function return without worrying about task cancellation dropping - /// the broadcast mid-flight. Failures are logged, never propagated. - pub fn send_document_update(&self, vault: VaultId, document: WebSocketServerMessageWithOrigin) { - let vault_update_id = match &document.message { - WebSocketServerMessage::VaultUpdate(u) => Some(u.document.vault_update_id), - WebSocketServerMessage::CursorPositions(_) => None, - }; - let is_deleted = match &document.message { - WebSocketServerMessage::VaultUpdate(u) => Some(u.document.is_deleted), - WebSocketServerMessage::CursorPositions(_) => None, - }; - let mut tx_map = self - .tx - .lock() - .expect("broadcasts.tx mutex poisoned — a previous holder panicked"); - let count_before_prune = tx_map - .get(&vault) - .map_or(0, tokio::sync::broadcast::Sender::receiver_count); - let pruned = Self::prune_inactive_vaults(&mut tx_map); - let pruned_self = pruned.contains(&vault); + /// the broadcast mid-flight. Mutex poison is returned; send failures + /// are logged because they can happen when receivers disconnect. + pub fn send_document_update( + &self, + vault: &str, + document: WebSocketServerMessage, + ) -> Result<(), SyncServerError> { + let mut tx_map = self.tx.lock().map_err(|_| { + server_error(anyhow::anyhow!( + "broadcasts.tx mutex poisoned; skipping document update broadcast" + )) + })?; + Self::prune_inactive_vaults(&mut tx_map); let sender = tx_map - .entry(vault.clone()) + .entry(vault.to_owned()) .or_insert_with(|| broadcast::channel(self.broadcast_channel_capacity).0); let count_before_send = sender.receiver_count(); if count_before_send == 0 { - info!( - "[BCAST] send_document_update vault={vault} vuid={vault_update_id:?} is_deleted={is_deleted:?} count_before_prune={count_before_prune} pruned_self={pruned_self} count_before_send=0 SKIPPED" - ); debug!("Skipping broadcast, no clients connected for vault `{vault}`"); - return; + return Ok(()); } - let send_result = sender.send(document); - match &send_result { - Ok(n) => info!( - "[BCAST] send_document_update vault={vault} vuid={vault_update_id:?} is_deleted={is_deleted:?} count_before_prune={count_before_prune} pruned_self={pruned_self} count_before_send={count_before_send} SENT delivered_to={n}" - ), - Err(e) => warn!( - "[BCAST] send_document_update vault={vault} vuid={vault_update_id:?} is_deleted={is_deleted:?} count_before_prune={count_before_prune} pruned_self={pruned_self} count_before_send={count_before_send} FAILED err={e}" - ), + if let Err(e) = sender.send(document) { + warn!("Failed to send document update broadcast: {e}"); } + Ok(()) } } diff --git a/sync-server/src/app_state/websocket/models.rs b/sync-server/src/app_state/websocket/models.rs index eb6c956a..60d690cd 100644 --- a/sync-server/src/app_state/websocket/models.rs +++ b/sync-server/src/app_state/websocket/models.rs @@ -58,15 +58,6 @@ pub struct CursorPositionFromServer { pub clients: Vec, } -// One committed version. Non-delete updates are broadcast to every -// connected client *except* the device that authored them — that -// device already has the new state via its HTTP response. Deletes are -// broadcast to every client including the author: the author keeps -// the document in its sync queue until this receipt arrives so a late -// remote update can't sneak in between the HTTP response and the -// queue cleanup. The server also emits these one-at-a-time to catch -// up a freshly-connected client on versions committed while it was -// offline, in ascending `vault_update_id` order. #[derive(TS, Serialize, Clone, Debug)] #[serde(rename_all = "camelCase")] pub struct WebSocketVaultUpdate { @@ -88,29 +79,3 @@ pub enum WebSocketServerMessage { VaultUpdate(WebSocketVaultUpdate), CursorPositions(CursorPositionFromServer), } - -/// Broadcast envelope carrying the message plus the device that produced -/// it. The per-recipient send task compares `origin_device_id` against -/// its own device id to fill in `originates_from_self` before the message -/// is serialized on the wire. -#[derive(Clone, Debug)] -pub struct WebSocketServerMessageWithOrigin { - pub origin_device_id: Option, - pub message: WebSocketServerMessage, -} - -impl WebSocketServerMessageWithOrigin { - pub fn new(message: WebSocketServerMessage) -> Self { - Self { - origin_device_id: None, - message, - } - } - - pub fn with_origin(origin_device_id: DeviceId, message: WebSocketServerMessage) -> Self { - Self { - origin_device_id: Some(origin_device_id), - message, - } - } -} diff --git a/sync-server/src/app_state/websocket/utils.rs b/sync-server/src/app_state/websocket/utils.rs index d78360de..ae75e3e3 100644 --- a/sync-server/src/app_state/websocket/utils.rs +++ b/sync-server/src/app_state/websocket/utils.rs @@ -10,7 +10,7 @@ use crate::{ }, config::user_config::User, errors::{SyncServerError, client_error, server_error, unauthenticated_error}, - server::auth::auth, + server::auth::authenticate_for_vault, }; pub struct AuthenticatedWebSocketHandshake { @@ -30,7 +30,7 @@ pub fn get_authenticated_handshake( match message { WebSocketClientMessage::Handshake(handshake) => { - let user = auth(state, handshake.token.trim(), vault_id)?; + let user = authenticate_for_vault(state, handshake.token.trim(), vault_id)?; Ok(AuthenticatedWebSocketHandshake { handshake, user }) } WebSocketClientMessage::CursorPositions(_) => Err(unauthenticated_error( @@ -51,6 +51,9 @@ pub fn get_authenticated_handshake( /// vault send lock; commits past the cursor are then delivered solely /// through the broadcast channel (filtered by the same cursor on the /// receive side), so every committed update is delivered exactly once. +/// We could've used a read transaction but that would've meant all other +/// clients would need to wait for the new client to catch up before +/// sending any updates. pub async fn get_unseen_documents( state: &AppState, vault_id: &VaultId, @@ -62,13 +65,11 @@ pub async fn get_unseen_documents( .database .get_latest_documents_since(vault_id, update_id, Some(up_to_vault_update_id), None) .await - .map_err(server_error) } else { state .database .get_latest_documents(vault_id, Some(up_to_vault_update_id), None) .await - .map_err(server_error) } } diff --git a/sync-server/src/cli/color_when.rs b/sync-server/src/cli/color_when.rs index a3709b94..911cdeff 100644 --- a/sync-server/src/cli/color_when.rs +++ b/sync-server/src/cli/color_when.rs @@ -23,9 +23,10 @@ impl ColorWhen { impl std::fmt::Display for ColorWhen { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.to_possible_value() - .expect("no values are skipped") - .get_name() - .fmt(f) + f.write_str(match self { + Self::Always => "always", + Self::Auto => "auto", + Self::Never => "never", + }) } } diff --git a/sync-server/src/config/server_config.rs b/sync-server/src/config/server_config.rs index 715d216c..c436fbb0 100644 --- a/sync-server/src/config/server_config.rs +++ b/sync-server/src/config/server_config.rs @@ -71,6 +71,10 @@ impl ServerConfig { self.max_pending_websocket_connections > 0, "max_pending_websocket_connections must be greater than 0" ); + ensure!( + self.rate_limit_per_user_per_second != Some(0), + "rate_limit_per_user_per_second must be greater than 0 when set (use null to disable rate limiting)" + ); Ok(()) } diff --git a/sync-server/src/config/user_config.rs b/sync-server/src/config/user_config.rs index fd824f39..1d97758f 100644 --- a/sync-server/src/config/user_config.rs +++ b/sync-server/src/config/user_config.rs @@ -20,15 +20,7 @@ where let mut user_token_map = BiHashMap::new(); for user in &users { if let Some(existing_name) = user_token_map.get_by_right(&user.token) { - let redacted = if user.token.len() > 6 { - format!( - "{}...{}", - &user.token[..3], - &user.token[user.token.len() - 3..] - ) - } else { - "***".to_owned() - }; + let redacted = redact_token(&user.token); return Err(D::Error::custom(format!( "Duplicate user token found: `{redacted}` for users `{}` and `{}`. User tokens \ must be unique.", @@ -49,6 +41,23 @@ where Ok(users) } +fn redact_token(token: &str) -> String { + if token.chars().count() <= 6 { + return "***".to_owned(); + } + + let prefix = token.chars().take(3).collect::(); + let suffix = token + .chars() + .rev() + .take(3) + .collect::>() + .into_iter() + .rev() + .collect::(); + format!("{prefix}...{suffix}") +} + impl UserConfig { pub fn get_user(&self, token: &str) -> Option<&User> { self.user_configs diff --git a/sync-server/src/consts.rs b/sync-server/src/consts.rs index e03b848f..a88fe5ff 100644 --- a/sync-server/src/consts.rs +++ b/sync-server/src/consts.rs @@ -21,6 +21,10 @@ pub const DEFAULT_MAX_PENDING_WS_CONNECTIONS: usize = 128; pub const DEFAULT_LOG_DIRECTORY: &str = "logs"; pub const DEFAULT_LOG_ROTATION_INTERVAL: Duration = Duration::from_hours(24); pub const IDLE_POOL_TIMEOUT: Duration = Duration::from_mins(5); + +/// Fail fast on pool acquire so a transiently locked database surfaces as +/// a 429 in seconds, not after a 30s `busy_timeout`. Callers retry. +pub const POOL_ACQUIRE_TIMEOUT: Duration = Duration::from_secs(5); pub const GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10); pub const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10); diff --git a/sync-server/src/errors.rs b/sync-server/src/errors.rs index 892db36f..4f27598b 100644 --- a/sync-server/src/errors.rs +++ b/sync-server/src/errors.rs @@ -79,10 +79,7 @@ impl IntoResponse for SyncServerError { Self::InitError(_) | Self::ServerError(_) => { error!("{serialized}"); } - Self::ClientError(_) | Self::NotFound(_) => { - warn!("{serialized}"); - } - Self::TooManyRequests(_) => { + Self::ClientError(_) | Self::NotFound(_) | Self::TooManyRequests(_) => { warn!("{serialized}"); } Self::Unauthenticated(_) | Self::PermissionDeniedError(_) => {} @@ -166,15 +163,26 @@ pub fn too_many_requests_error(error: anyhow::Error) -> SyncServerError { SyncServerError::TooManyRequests(error) } -/// Maps a `create_write_transaction` error to 429 if the database is busy, -/// or 500 for all other failures. -pub fn write_transaction_error(error: anyhow::Error) -> SyncServerError { - if error - .downcast_ref::() - .is_some() - { +/// Maps a database-operation error to 429 if the database is busy or the +/// pool acquire timed out (both retryable), or 500 for all other failures. +pub fn database_error(error: anyhow::Error) -> SyncServerError { + if is_database_busy(&error) { too_many_requests_error(error) } else { server_error(error) } } + +fn is_database_busy(error: &anyhow::Error) -> bool { + if error + .downcast_ref::() + .is_some() + { + return true; + } + error.chain().any(|cause| { + cause + .downcast_ref::() + .is_some_and(crate::app_state::database::is_sqlite_busy_error) + }) +} diff --git a/sync-server/src/main.rs b/sync-server/src/main.rs index dc00d4d5..9d318d29 100644 --- a/sync-server/src/main.rs +++ b/sync-server/src/main.rs @@ -1,3 +1,19 @@ +#![cfg_attr( + test, + allow( + clippy::arithmetic_side_effects, + clippy::expect_used, + clippy::indexing_slicing, + clippy::panic, + clippy::panic_in_result_fn, + clippy::todo, + clippy::unimplemented, + clippy::unreachable, + clippy::unwrap_in_result, + clippy::unwrap_used + ) +)] + mod app_state; mod cli; mod config; @@ -14,7 +30,7 @@ use cli::args::Args; use config::Config; use consts::DEFAULT_CONFIG_PATH; use errors::{SyncServerError, init_error}; -use log::info; +use log::{error, info, warn}; use server::create_server; use tracing_appender::non_blocking::WorkerGuard; use tracing_subscriber::{EnvFilter, fmt::format, layer::SubscriberExt, util::SubscriberInitExt}; @@ -36,30 +52,63 @@ async fn main() -> ExitCode { .map_err(init_error) { Ok(config) => config, - Err(e) => { - eprintln!("{}", e.serialize()); - return ExitCode::FAILURE; + Err(error) => { + return exit_with_startup_error(&args, &error); } }; - let result = async { - config.validate().map_err(init_error)?; - // Hold the non-blocking writer guards until shutdown so the - // dedicated writer threads stay alive and flush queued log lines. - let _log_guards = set_up_logging(&args, &config.logging)?; - start_server(config).await + if let Err(error) = config.validate().map_err(init_error) { + return exit_with_startup_error(&args, &error); } - .await; - match result { + // Hold the non-blocking writer guards until shutdown so the dedicated + // writer threads stay alive and flush queued log lines. + let _log_guards = match set_up_logging(&args, &config.logging) { + Ok(log_guards) => log_guards, + Err(error) => { + return exit_with_startup_error(&args, &error); + } + }; + + match start_server(config).await { Ok(()) => ExitCode::SUCCESS, - Err(e) => { - eprintln!("{}", e.serialize()); + Err(error) => { + let serialized = error.serialize(); + warn!("{serialized}"); ExitCode::FAILURE } } } +fn exit_with_startup_error(args: &Args, err: &SyncServerError) -> ExitCode { + let _ = set_up_stderr_logging(args); + + let serialized = err.serialize(); + error!("{serialized}"); + + ExitCode::FAILURE +} + +fn set_up_stderr_logging(args: &Args) -> Result<(), SyncServerError> { + let env_filter = EnvFilter::builder() + .with_default_directive(tracing::Level::WARN.into()) + .from_env() + .context("Failed to create logging env filter") + .map_err(init_error)?; + + let stderr_layer = tracing_subscriber::fmt::layer() + .with_ansi(args.color.use_colors()) + .with_writer(std::io::stderr) + .event_format(format().compact()); + + tracing_subscriber::registry() + .with(env_filter) + .with(stderr_layer) + .try_init() + .context("Failed to initialise fallback tracing") + .map_err(init_error) +} + fn set_up_logging( args: &Args, logging_config: &config::logging_config::LoggingConfig, diff --git a/sync-server/src/server.rs b/sync-server/src/server.rs index 8f4f9a7a..44960bc6 100644 --- a/sync-server/src/server.rs +++ b/sync-server/src/server.rs @@ -4,8 +4,6 @@ mod delete_document; mod device_id_header; mod fetch_document_version; mod fetch_document_version_content; -mod fetch_latest_document_version; -mod fetch_latest_documents; mod index; mod ping; mod rate_limit; @@ -14,13 +12,14 @@ mod responses; mod update_document; mod websocket; -use anyhow::{Context as _, Result}; +use anyhow::{Context as _, Result, anyhow}; use auth::auth_middleware; use axum::{ Router, extract::{DefaultBodyLimit, Request}, http::{self, HeaderValue, Method}, middleware, + response::IntoResponse, routing::{IntoMakeService, delete, get, post, put}, }; use device_id_header::DEVICE_ID_HEADER_NAME; @@ -42,6 +41,7 @@ use crate::{ app_state::AppState, config::{Config, server_config::ServerConfig}, consts::GRACEFUL_SHUTDOWN_TIMEOUT, + errors::not_found_error, }; pub async fn create_server(config: Config) -> Result<()> { @@ -71,7 +71,13 @@ pub async fn create_server(config: Config) -> Result<()> { let app = app .layer(DefaultBodyLimit::disable()) .layer(RequestBodyLimitLayer::new( - app_state.config.server.max_body_size_mb * 1024 * 1024, + app_state + .config + .server + .max_body_size_mb + .checked_mul(1024) + .and_then(|kb| kb.checked_mul(1024)) + .context("max_body_size_mb is too large")?, )) .layer(TimeoutLayer::new(server_config.response_timeout)) .layer(cors_layer) @@ -95,6 +101,7 @@ pub async fn create_server(config: Config) -> Result<()> { .on_failure(DefaultOnFailure::new().level(Level::ERROR)), ) .with_state(app_state.clone()) + .fallback(handle_404) .into_make_service(); start_server(app, &server_config, app_state).await @@ -103,7 +110,7 @@ pub async fn create_server(config: Config) -> Result<()> { fn build_cors_layer(server_config: &ServerConfig) -> Result { let origins = &server_config.allowed_origins; - let cors = if origins.len() == 1 && origins[0] == "*" { + let cors = if origins.len() == 1 && origins.first().is_some_and(|origin| origin == "*") { info!("CORS: allowing all origins"); let header: HeaderValue = "*" .parse() @@ -131,18 +138,10 @@ fn build_cors_layer(server_config: &ServerConfig) -> Result { fn get_authed_routes(app_state: AppState) -> Router { Router::new() - .route( - "/vaults/:vault_id/documents", - get(fetch_latest_documents::fetch_latest_documents), - ) .route( "/vaults/:vault_id/documents", post(create_document::create_document), ) - .route( - "/vaults/:vault_id/documents/:document_id", - get(fetch_latest_document_version::fetch_latest_document_version), - ) .route( "/vaults/:vault_id/documents/:document_id/binary", put(update_document::update_binary), @@ -233,3 +232,7 @@ async fn shutdown_signal() { () = terminate => {}, } } + +async fn handle_404() -> impl IntoResponse { + not_found_error(anyhow!("Endpoint not found")) +} diff --git a/sync-server/src/server/auth.rs b/sync-server/src/server/auth.rs index 7fa45abd..90bdb205 100644 --- a/sync-server/src/server/auth.rs +++ b/sync-server/src/server/auth.rs @@ -34,7 +34,7 @@ pub async fn auth_middleware( .ok_or_else(|| unauthenticated_error(anyhow::anyhow!("Missing vault_id")))?, ); - let user = auth(&state, token, &vault_id)?; + let user = authenticate_for_vault(&state, token, &vault_id)?; req.extensions_mut().insert(user); @@ -50,7 +50,11 @@ pub fn authenticate(state: &AppState, token: &str) -> Result Result { +pub fn authenticate_for_vault( + state: &AppState, + token: &str, + vault_id: &VaultId, +) -> Result { let user = authenticate(state, token)?; if match user.vault_access { diff --git a/sync-server/src/server/create_document.rs b/sync-server/src/server/create_document.rs index d772e16a..0812b052 100644 --- a/sync-server/src/server/create_document.rs +++ b/sync-server/src/server/create_document.rs @@ -14,7 +14,7 @@ use crate::{ database::models::{StoredDocumentVersion, VaultId}, }, config::user_config::User, - errors::{SyncServerError, client_error, server_error, write_transaction_error}, + errors::{SyncServerError, client_error, server_error}, server::{responses::DocumentUpdateResponse, update_document}, utils::{ find_first_available_path::find_first_available_path, is_binary::is_binary, @@ -49,8 +49,7 @@ pub async fn create_document( let mut transaction = state .database .create_write_transaction(&vault_id) - .await - .map_err(write_transaction_error)?; + .await?; let sanitized_relative_path = sanitize_path(&request.relative_path).map_err(client_error)?; let new_content = request.content.contents.to_vec(); @@ -60,10 +59,9 @@ pub async fn create_document( .get_latest_non_deleted_document_by_path( &vault_id, &sanitized_relative_path, - Some(&mut *transaction), + Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .map_err(server_error)?; + .await?; if let Some(latest_version) = latest_version { // Only merge with an existing document the client couldn't have @@ -129,16 +127,13 @@ pub async fn create_document( &device_id.0, request.last_seen_vault_update_id, &new_content, - Some(&mut *transaction), + Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .map_err(server_error)? + .await? { info!( "Lost-create recovery: binding retry at `{sanitized_relative_path}` to existing doc {} (was at `{}`) in vault `{vault_id}` for device `{}`", - lost_create.document_id, - lost_create.relative_path, - device_id.0 + lost_create.document_id, lost_create.relative_path, device_id.0 ); return update_document::update_document( &sanitized_relative_path, @@ -159,9 +154,11 @@ pub async fn create_document( let last_update_id = state .database - .get_max_update_id_in_vault(&vault_id, Some(&mut *transaction)) - .await - .map_err(server_error)?; + .get_max_update_id_in_vault( + &vault_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) + .await?; let deduped_path = find_first_available_path( &vault_id, @@ -169,8 +166,7 @@ pub async fn create_document( &state.database, &mut transaction, ) - .await - .map_err(server_error)?; + .await?; if deduped_path != sanitized_relative_path { info!( @@ -178,7 +174,9 @@ pub async fn create_document( ); } - let new_vault_update_id = last_update_id + 1; + let new_vault_update_id = last_update_id + .checked_add(1) + .ok_or_else(|| server_error(anyhow::anyhow!("Vault update id overflow")))?; let new_version = StoredDocumentVersion { vault_update_id: new_vault_update_id, creation_vault_update_id: new_vault_update_id, @@ -195,8 +193,7 @@ pub async fn create_document( state .database .insert_document_version(&vault_id, &new_version, transaction) - .await - .map_err(server_error)?; + .await?; Ok(Json(DocumentUpdateResponse::FastForwardUpdate( new_version.into(), diff --git a/sync-server/src/server/delete_document.rs b/sync-server/src/server/delete_document.rs index 2ee6eac3..12e58f89 100644 --- a/sync-server/src/server/delete_document.rs +++ b/sync-server/src/server/delete_document.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, anyhow}; +use anyhow::anyhow; use axum::{ Extension, Json, extract::{Path, State}, @@ -16,7 +16,7 @@ use crate::{ }, }, config::user_config::User, - errors::{SyncServerError, not_found_error, server_error, write_transaction_error}, + errors::{SyncServerError, not_found_error, server_error}, utils::normalize::normalize, }; @@ -43,44 +43,42 @@ pub async fn delete_document( let mut transaction = state .database .create_write_transaction(&vault_id) - .await - .map_err(write_transaction_error)?; + .await?; let last_update_id = state .database - .get_max_update_id_in_vault(&vault_id, Some(&mut transaction)) - .await - .map_err(server_error)?; + .get_max_update_id_in_vault( + &vault_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) + .await?; let latest_version = state .database - .get_latest_document(&vault_id, &document_id, Some(&mut transaction)) - .await - .map_err(server_error)?; + .get_latest_document( + &vault_id, + &document_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) + .await?; let Some(latest_version) = latest_version else { - transaction - .rollback() - .await - .context("Failed to roll back transaction") - .map_err(server_error)?; + transaction.rollback().await?; return Err(not_found_error(anyhow!( "Document `{document_id}` not found in vault `{vault_id}`" ))); }; if latest_version.is_deleted { - transaction - .rollback() - .await - .context("Failed to roll back transaction") - .map_err(server_error)?; + transaction.rollback().await?; info!("Document `{document_id}` has already been deleted",); return Ok(Json(latest_version.into())); } - let new_vault_update_id = last_update_id + 1; + let new_vault_update_id = last_update_id + .checked_add(1) + .ok_or_else(|| server_error(anyhow!("Vault update id overflow")))?; let latest_relative_path = latest_version.relative_path; let latest_content = latest_version.content; let creation_vault_update_id = latest_version.creation_vault_update_id; @@ -101,8 +99,7 @@ pub async fn delete_document( state .database .insert_document_version(&vault_id, &new_version, transaction) - .await - .map_err(server_error)?; + .await?; Ok(Json(new_version.into())) } diff --git a/sync-server/src/server/fetch_document_version.rs b/sync-server/src/server/fetch_document_version.rs index 159cad3a..657cea81 100644 --- a/sync-server/src/server/fetch_document_version.rs +++ b/sync-server/src/server/fetch_document_version.rs @@ -11,7 +11,7 @@ use crate::{ AppState, database::models::{DocumentId, DocumentVersion, VaultId, VaultUpdateId}, }, - errors::{SyncServerError, client_error, not_found_error, server_error}, + errors::{SyncServerError, not_found_error}, utils::normalize::normalize, }; @@ -40,8 +40,7 @@ pub async fn fetch_document_version( let result = state .database .get_document_version(&vault_id, vault_update_id, None) - .await - .map_err(server_error)? + .await? .map_or_else( || { Err(not_found_error(anyhow!( @@ -52,7 +51,7 @@ pub async fn fetch_document_version( )?; if result.document_id != document_id { - return Err(client_error(anyhow!( + return Err(not_found_error(anyhow!( "Document with document id `{document_id}` does not have a version with id \ `{vault_update_id}`", ))); diff --git a/sync-server/src/server/fetch_document_version_content.rs b/sync-server/src/server/fetch_document_version_content.rs index a163b036..f888c866 100644 --- a/sync-server/src/server/fetch_document_version_content.rs +++ b/sync-server/src/server/fetch_document_version_content.rs @@ -11,7 +11,7 @@ use crate::{ AppState, database::models::{DocumentId, VaultId, VaultUpdateId}, }, - errors::{SyncServerError, client_error, not_found_error, server_error}, + errors::{SyncServerError, not_found_error}, utils::normalize::normalize, }; @@ -40,8 +40,7 @@ pub async fn fetch_document_version_content( let result = state .database .get_document_version(&vault_id, vault_update_id, None) - .await - .map_err(server_error)? + .await? .map_or_else( || { Err(not_found_error(anyhow!( @@ -52,7 +51,7 @@ pub async fn fetch_document_version_content( )?; if result.document_id != document_id { - return Err(client_error(anyhow!( + return Err(not_found_error(anyhow!( "Document with document id `{document_id}` does not have a version with id \ `{vault_update_id}`", ))); diff --git a/sync-server/src/server/fetch_latest_document_version.rs b/sync-server/src/server/fetch_latest_document_version.rs deleted file mode 100644 index a9973606..00000000 --- a/sync-server/src/server/fetch_latest_document_version.rs +++ /dev/null @@ -1,51 +0,0 @@ -use anyhow::anyhow; -use axum::{ - Json, - extract::{Path, State}, -}; -use log::debug; -use serde::Deserialize; - -use crate::{ - app_state::{ - AppState, - database::models::{DocumentId, DocumentVersion, VaultId}, - }, - errors::{SyncServerError, not_found_error, server_error}, - utils::normalize::normalize, -}; - -#[derive(Deserialize)] -pub struct FetchLatestDocumentVersionPathParams { - #[serde(deserialize_with = "normalize")] - vault_id: VaultId, - - document_id: DocumentId, -} - -#[axum::debug_handler] -pub async fn fetch_latest_document_version( - Path(FetchLatestDocumentVersionPathParams { - vault_id, - document_id, - }): Path, - State(state): State, -) -> Result, SyncServerError> { - debug!("Fetching latest document version for document `{document_id}` in vault `{vault_id}`"); - - let latest_version = state - .database - .get_latest_document(&vault_id, &document_id, None) - .await - .map_err(server_error)? - .map_or_else( - || { - Err(not_found_error(anyhow!( - "Document with id `{document_id}` not found", - ))) - }, - Ok, - )?; - - Ok(Json(latest_version.into())) -} diff --git a/sync-server/src/server/fetch_latest_documents.rs b/sync-server/src/server/fetch_latest_documents.rs deleted file mode 100644 index f1ca702d..00000000 --- a/sync-server/src/server/fetch_latest_documents.rs +++ /dev/null @@ -1,59 +0,0 @@ -use axum::{ - Json, - extract::{Path, Query, State}, -}; -use log::debug; -use serde::Deserialize; - -use super::responses::FetchLatestDocumentsResponse; -use crate::{ - app_state::{ - AppState, - database::models::{VaultId, VaultUpdateId}, - }, - errors::{SyncServerError, server_error}, - utils::normalize::normalize, -}; - -#[derive(Deserialize)] -pub struct FetchLatestDocumentsPathParams { - #[serde(deserialize_with = "normalize")] - vault_id: VaultId, -} - -#[derive(Deserialize)] -pub struct QueryParams { - since_update_id: Option, -} - -#[axum::debug_handler] -pub async fn fetch_latest_documents( - Path(FetchLatestDocumentsPathParams { vault_id }): Path, - Query(QueryParams { since_update_id }): Query, - State(state): State, -) -> Result, SyncServerError> { - debug!("Fetching latest documents in vault `{vault_id}` since update ID `{since_update_id:?}`"); - - let documents = if let Some(since_update_id) = since_update_id { - state - .database - .get_latest_documents_since(&vault_id, since_update_id, None, None) - .await - .map_err(server_error) - } else { - state - .database - .get_latest_documents(&vault_id, None, None) - .await - .map_err(server_error) - }?; - - Ok(Json(FetchLatestDocumentsResponse { - last_update_id: documents - .iter() - .map(|doc| doc.vault_update_id) - .max() - .unwrap_or(since_update_id.unwrap_or(0)), - latest_documents: documents, - })) -} diff --git a/sync-server/src/server/ping.rs b/sync-server/src/server/ping.rs index 31aa8acd..6740acae 100644 --- a/sync-server/src/server/ping.rs +++ b/sync-server/src/server/ping.rs @@ -9,7 +9,7 @@ use axum_extra::{ use log::debug; use serde::Deserialize; -use super::{auth::auth, responses::PingResponse}; +use super::{auth::authenticate_for_vault, responses::PingResponse}; use crate::{ app_state::{AppState, database::models::VaultId}, consts::SUPPORTED_API_VERSION, @@ -31,8 +31,9 @@ pub async fn ping( ) -> Result, SyncServerError> { debug!("Pinging vault `{vault_id}`"); - let is_authenticated = maybe_auth_header - .is_some_and(|auth_header| auth(&state, auth_header.token(), &vault_id).is_ok()); + let is_authenticated = maybe_auth_header.is_some_and(|auth_header| { + authenticate_for_vault(&state, auth_header.token(), &vault_id).is_ok() + }); Ok(Json(PingResponse { server_version: env!("CARGO_PKG_VERSION").to_owned(), diff --git a/sync-server/src/server/rate_limit.rs b/sync-server/src/server/rate_limit.rs index 7792a814..a6ef4d48 100644 --- a/sync-server/src/server/rate_limit.rs +++ b/sync-server/src/server/rate_limit.rs @@ -32,26 +32,23 @@ struct BucketState { impl RateLimiter { /// Create a new per-user rate limiter. - /// - /// # Panics - /// - /// Panics if `max_per_second` is 0. pub fn new(max_per_second: u64) -> Self { - assert!( - max_per_second > 0, - "max_per_second must be > 0 (set rate_limit_per_user_per_second to null in config to disable)" - ); - Self { max_per_second, buckets: Arc::new(Mutex::new(HashMap::new())), } } - fn get_or_create_bucket(&self, token: &str) -> Arc { - self.buckets + fn get_or_create_bucket( + &self, + token: &str, + ) -> std::result::Result, StatusCode> { + let mut buckets = self + .buckets .lock() - .expect("rate limiter lock poisoned") + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + Ok(buckets .entry(token.to_owned()) .or_insert_with(|| { Arc::new(TokenBucket { @@ -62,23 +59,26 @@ impl RateLimiter { max_tokens: self.max_per_second, }) }) - .clone() + .clone()) } } impl TokenBucket { - fn try_acquire(&self) -> bool { - let mut state = self.state.lock().expect("token bucket lock poisoned"); + fn try_acquire(&self) -> std::result::Result { + let mut state = self + .state + .lock() + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; let now = Instant::now(); if now.duration_since(state.last_refill).as_secs() >= 1 { state.tokens = self.max_tokens; state.last_refill = now; } if state.tokens > 0 { - state.tokens -= 1; - true + state.tokens = state.tokens.saturating_sub(1); + Ok(true) } else { - false + Ok(false) } } } @@ -88,13 +88,13 @@ pub async fn rate_limit_middleware( auth_header: Option>>, req: Request, next: Next, -) -> Result { +) -> std::result::Result { let Some(TypedHeader(auth)) = auth_header else { return Ok(next.run(req).await); }; - let bucket = limiter.get_or_create_bucket(auth.token()); - if bucket.try_acquire() { + let bucket = limiter.get_or_create_bucket(auth.token())?; + if bucket.try_acquire()? { Ok(next.run(req).await) } else { Err(StatusCode::TOO_MANY_REQUESTS) diff --git a/sync-server/src/server/responses.rs b/sync-server/src/server/responses.rs index 47b6e402..c07c054b 100644 --- a/sync-server/src/server/responses.rs +++ b/sync-server/src/server/responses.rs @@ -1,9 +1,7 @@ use serde::{self, Serialize}; use ts_rs::TS; -use crate::app_state::database::models::{ - DocumentVersion, DocumentVersionWithoutContent, VaultUpdateId, -}; +use crate::app_state::database::models::{DocumentVersion, DocumentVersionWithoutContent}; /// Response to a ping request. #[derive(TS, Debug, Clone, Serialize)] @@ -25,17 +23,6 @@ pub struct PingResponse { pub supported_api_version: u32, } -/// Response to a fetch latest documents request. -#[derive(TS, Debug, Clone, Serialize)] -#[serde(rename_all = "camelCase")] -#[ts(export)] -pub struct FetchLatestDocumentsResponse { - pub latest_documents: Vec, - - /// The update ID of the latest document in the response. - pub last_update_id: VaultUpdateId, -} - /// Response to a create/update document request. #[derive(TS, Debug, Clone, Serialize)] #[serde(tag = "type")] diff --git a/sync-server/src/server/update_document.rs b/sync-server/src/server/update_document.rs index 0145288c..7977c644 100644 --- a/sync-server/src/server/update_document.rs +++ b/sync-server/src/server/update_document.rs @@ -22,12 +22,10 @@ use crate::{ }, }, config::user_config::User, - errors::{ - SyncServerError, client_error, not_found_error, server_error, write_transaction_error, - }, + errors::{SyncServerError, client_error, not_found_error, server_error}, server::requests::UpdateBinaryDocumentVersion, utils::{ - find_first_available_path::find_first_available_path, is_binary::is_binary, + find_first_available_path::find_first_available_path, is_binary::as_non_binary_text, is_file_type_mergable::is_file_type_mergable, normalize::normalize, sanitize_path::sanitize_path, }, @@ -58,8 +56,7 @@ pub async fn update_binary( let transaction = state .database .create_write_transaction(&vault_id) - .await - .map_err(write_transaction_error)?; + .await?; update_document( &parent_document.relative_path, @@ -104,8 +101,7 @@ pub async fn update_text( let transaction = state .database .create_write_transaction(&vault_id) - .await - .map_err(write_transaction_error)?; + .await?; update_document( &parent_document.relative_path, @@ -131,8 +127,7 @@ async fn get_parent_document( let parent = state .database .get_document_version(vault_id, parent_version_id, None) - .await - .map_err(server_error)? + .await? .map_or_else( || { Err(not_found_error(anyhow!( @@ -173,15 +168,20 @@ pub async fn update_document( let last_update_id = state .database - .get_max_update_id_in_vault(&vault_id, Some(&mut transaction)) - .await - .map_err(server_error)?; + .get_max_update_id_in_vault( + &vault_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) + .await?; let latest_version = state .database - .get_latest_document(&vault_id, &document_id, Some(&mut transaction)) - .await - .map_err(server_error)? + .get_latest_document( + &vault_id, + &document_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) + .await? .map_or_else( || { Err(not_found_error(anyhow!( @@ -192,11 +192,7 @@ pub async fn update_document( )?; if latest_version.is_deleted { - transaction - .rollback() - .await - .context("Failed to roll back transaction") - .map_err(server_error)?; + transaction.rollback().await?; info!("Document `{document_id}` has been deleted, ignoring update to it",); return Ok(Json(DocumentUpdateResponse::FastForwardUpdate( @@ -214,75 +210,63 @@ pub async fn update_document( info!( "Document content is the same as the latest version for `{document_id}`, skipping update" ); - transaction - .rollback() - .await - .context("Failed to roll back transaction") - .map_err(server_error)?; + transaction.rollback().await?; return Ok(Json(DocumentUpdateResponse::FastForwardUpdate( latest_version.into(), ))); } - // For mergability, use whichever path the new version will live at — the - // requested rename target if the client sent one, otherwise the existing - // server-side path. + // For mergability, use whichever path the new version will live at: + // - the requested rename target if the client sent one + // - otherwise the existing server-side path. let mergable_check_path = sanitized_relative_path .as_deref() .unwrap_or(&latest_version.relative_path); - let are_all_participants_mergable = is_file_type_mergable( + + let mergeable_texts = if is_file_type_mergable( mergable_check_path, &state.config.server.mergeable_file_extensions, - ) && !is_binary(&parent_content) - && !is_binary(&latest_version.content) - && !is_binary(&content); - - let (merged_content, is_different_from_request_content) = if are_all_participants_mergable { - info!("Merging changes for document `{document_id}` in vault `{vault_id}`"); - let parent_text = str::from_utf8(&parent_content) - .context("Parent document content is not valid UTF-8") - .map_err(client_error)?; - let latest_text = str::from_utf8(&latest_version.content) - .context("Latest version content is not valid UTF-8") - .map_err(client_error)?; - let new_text = str::from_utf8(&content) - .context("New content is not valid UTF-8") - .map_err(client_error)?; - let parent_owned = parent_text.to_owned(); - let latest_owned = latest_text.to_owned(); - let new_owned = new_text.to_owned(); - let content_clone = content.clone(); - - let (merged, is_different) = tokio::task::spawn_blocking(move || { - let merged = reconcile( - &parent_owned, - &latest_owned.into(), - &new_owned.into(), - &*BuiltinTokenizer::Word, - ) - .apply() - .text() - .into_bytes(); - let is_different = merged != content_clone; - (merged, is_different) - }) - .await - .map_err(|e| server_error(anyhow::anyhow!("Reconcile task failed: {e}")))?; - - (merged, is_different) + ) { + as_non_binary_texts(&parent_content, &latest_version.content, &content) } else { - (content, false) // false means that the client doesn't need to refetch the file as we can ensure the remote and local versions are the same as LWW is the merging method for binary files + None }; + let are_all_participants_mergable = mergeable_texts.is_some(); - // Rename resolution: only apply the client's rename if (a) the client - // requested one (`sanitized_relative_path` is `Some`) and (b) the - // document's path hasn't changed since this client's parent version. - // If the parent and latest paths differ, another client already renamed - // the document — keep the latest path (first rename wins). Content - // changes from both clients are still merged correctly via the 3-way - // reconcile above, independent of which rename wins. A missing - // relative_path means "keep current path" (content-only edit). + let (merged_content, is_same_as_request) = + if let Some((parent_text, latest_text, new_text)) = mergeable_texts { + info!("Merging changes for document `{document_id}` in vault `{vault_id}`"); + + let parent_owned = parent_text.to_owned(); + let latest_owned = latest_text.to_owned(); + let new_owned = new_text.to_owned(); + let content_clone = content.clone(); + + let merged = tokio::task::spawn_blocking(move || { + + reconcile( + &parent_owned, + &latest_owned.into(), + &new_owned.into(), + &*BuiltinTokenizer::Word, + ) + .apply() + .text() + .into_bytes() + }) + .await + .map_err(|e| server_error(anyhow::anyhow!("Reconcile task failed: {e}")))?; + + let is_same = merged == content_clone; + (merged, is_same) + } else { + (content, true) // true means that the client doesn't need to refetch the file as we can ensure the remote and local versions are the same as LWW is the merging method for binary files + }; + + // First rename wins: apply the client's rename only if the doc's path + // hasn't changed since its parent version. Content from both clients + // still merges via the 3-way reconcile above let new_relative_path = match sanitized_relative_path.as_deref() { Some(requested) if parent_relative_path == latest_version.relative_path @@ -290,8 +274,7 @@ pub async fn update_document( { let new_path = find_first_available_path(&vault_id, requested, &state.database, &mut transaction) - .await - .map_err(server_error)?; + .await?; if new_path != requested { info!( @@ -306,7 +289,9 @@ pub async fn update_document( let new_version = StoredDocumentVersion { document_id, - vault_update_id: last_update_id + 1, + vault_update_id: last_update_id + .checked_add(1) + .ok_or_else(|| server_error(anyhow!("Vault update id overflow")))?, creation_vault_update_id: latest_version.creation_vault_update_id, relative_path: new_relative_path, content: merged_content, @@ -314,18 +299,29 @@ pub async fn update_document( is_deleted: false, user_id: user.name, device_id: device_id.0, - has_been_merged: are_all_participants_mergable && is_different_from_request_content, + has_been_merged: are_all_participants_mergable && !is_same_as_request, }; state .database .insert_document_version(&vault_id, &new_version, transaction) - .await - .map_err(server_error)?; + .await?; - Ok(Json(if is_different_from_request_content { - DocumentUpdateResponse::MergingUpdate(new_version.into()) - } else { + Ok(Json(if is_same_as_request { DocumentUpdateResponse::FastForwardUpdate(new_version.into()) + } else { + DocumentUpdateResponse::MergingUpdate(new_version.into()) })) } + +fn as_non_binary_texts<'a>( + parent_content: &'a [u8], + latest_content: &'a [u8], + new_content: &'a [u8], +) -> Option<(&'a str, &'a str, &'a str)> { + Some(( + as_non_binary_text(parent_content)?, + as_non_binary_text(latest_content)?, + as_non_binary_text(new_content)?, + )) +} diff --git a/sync-server/src/server/websocket.rs b/sync-server/src/server/websocket.rs index 6e1af0ba..936eb483 100644 --- a/sync-server/src/server/websocket.rs +++ b/sync-server/src/server/websocket.rs @@ -136,8 +136,7 @@ async fn websocket( // catch-up and in a contended-then-released broadcast is // delivered exactly once (via the catch-up). let send_guard = state.broadcasts.acquire_send_lock(&vault_id).await; - let mut broadcast_receiver = match state.broadcasts.get_receiver(vault_id.clone(), max_clients) - { + let mut broadcast_receiver = match state.broadcasts.get_receiver(&vault_id, max_clients) { Ok(receiver) => receiver, Err(err) => { drop(send_guard); @@ -163,8 +162,7 @@ async fn websocket( let cursor = state .database .get_max_update_id_in_vault(&vault_id, None) - .await - .map_err(server_error)?; + .await?; drop(send_guard); // Catch-up on versions committed while this client was offline, @@ -209,14 +207,24 @@ async fn websocket( loop { match broadcast_receiver.recv().await { Ok(update) => { - // Drop messages this device authored because the HTTP - // response already carried authoritative state back. - // Delete broadcasts are sent without an origin so the - // author also receives them — that's the receipt the - // client needs to drop the doc from its sync queue. - if Some(&device_id) == update.origin_device_id.as_ref() { - continue; - } + // Always deliver vault updates to the originating + // device too. The HTTP response is the *normal* path + // for the originator to learn its own update, and + // the client-side wire loop dedupes redundant + // broadcasts via the `parentVersionId` check. But + // when the response is lost mid-flight (sync reset, + // pause/resume, dropped TCP) the originator has no + // record of the doc; if the broadcast is also + // suppressed AND the next handshake's cursor was + // captured before the commit (cursor < vuid), the + // doc falls through both delivery paths and is + // stranded forever on the originator. Letting the + // self-broadcast through closes that window — the + // message processes as a remote create on the + // originator's reconnected WS and the file is + // restored. (Cursor messages still get the inner + // self-filter below; we drop our own cursor entries + // from the `clients` payload.) // Filter out vault updates already covered by the // catch-up snapshot. The handshake atomically @@ -229,13 +237,13 @@ async fn websocket( // Cursor messages aren't versioned and are always // forwarded. if let WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate { document }) = - &update.message + &update && document.vault_update_id <= cursor { continue; } - let message = match update.message { + let message = match update { WebSocketServerMessage::CursorPositions(CursorPositionFromServer { clients, }) => WebSocketServerMessage::CursorPositions(CursorPositionFromServer { @@ -244,7 +252,7 @@ async fn websocket( .filter(|client| client.device_id != device_id) .collect(), }), - WebSocketServerMessage::VaultUpdate(_) => update.message, + update @ WebSocketServerMessage::VaultUpdate(_) => update, }; send_update_over_websocket(&message, &mut sender).await?; @@ -307,7 +315,7 @@ async fn websocket( &device_id, docs, ) - .await; + .await?; } } } @@ -352,7 +360,7 @@ async fn websocket( state .cursors .remove_cursors_of_device(&vault_id, &authed_handshake.handshake.device_id) - .await; + .await?; match &result { Ok(()) => { diff --git a/sync-server/src/utils/dedup_paths.rs b/sync-server/src/utils/dedup_paths.rs index 0baf8ba8..0c9fd218 100644 --- a/sync-server/src/utils/dedup_paths.rs +++ b/sync-server/src/utils/dedup_paths.rs @@ -1,10 +1,3 @@ -use std::sync::LazyLock; - -use regex::Regex; - -static DEDUP_SUFFIX_REGEX: LazyLock = - LazyLock::new(|| Regex::new(r" \((\d+)\)$").expect("invalid regex")); - pub fn dedup_paths(path: &str) -> impl Iterator { let mut path_parts = path.split('/').collect::>(); let file_name = path_parts @@ -24,29 +17,19 @@ pub fn dedup_paths(path: &str) -> impl Iterator { let (stem, extension) = if is_simple_dotfile { (file_name.clone(), String::new()) } else { - // Regular file or dotfile with extension - let name_parts = file_name.rsplitn(2, '.').collect::>(); - let mut reverse_parts = name_parts.into_iter().rev(); - match (reverse_parts.next(), reverse_parts.next()) { - (Some(stem), maybe_extension) => ( - stem.to_owned(), - maybe_extension - .map(|ext| format!(".{ext}")) - .unwrap_or_default(), - ), - _ => unreachable!("Path must have at least one part"), + match file_name.rsplit_once('.') { + Some((stem, extension)) => (stem.to_owned(), format!(".{extension}")), + None => (file_name.clone(), String::new()), } }; - let start_number = DEDUP_SUFFIX_REGEX - .captures(&stem) - .and_then(|caps| caps.get(1)) - .and_then(|m| m.as_str().parse::().ok()) - .unwrap_or(0); + let (clean_stem, start_number) = strip_dedup_suffix(&stem); + let clean_stem = clean_stem.to_owned(); - let clean_stem = DEDUP_SUFFIX_REGEX.replace(&stem, "").to_string(); - - (start_number..).map(move |dedup_number| { + std::iter::successors(Some(start_number), |dedup_number| { + dedup_number.checked_add(1) + }) + .map(move |dedup_number| { if dedup_number == 0 { format!("{directory}{clean_stem}{extension}") } else { @@ -55,6 +38,20 @@ pub fn dedup_paths(path: &str) -> impl Iterator { }) } +fn strip_dedup_suffix(stem: &str) -> (&str, u64) { + let Some(without_closing_paren) = stem.strip_suffix(')') else { + return (stem, 0); + }; + let Some((clean_stem, number)) = without_closing_paren.rsplit_once(" (") else { + return (stem, 0); + }; + if number.is_empty() || !number.chars().all(|c| c.is_ascii_digit()) { + return (stem, 0); + } + + (clean_stem, number.parse::().unwrap_or(0)) +} + #[cfg(test)] mod test { use super::*; @@ -103,7 +100,7 @@ mod test { } #[test] - fn test_regex_capturing_group() { + fn test_dedup_suffix_parsing() { // Single digit in parentheses let mut deduped = dedup_paths("document (5).md"); assert_eq!(deduped.next(), Some("document (5).md".to_owned())); diff --git a/sync-server/src/utils/find_first_available_path.rs b/sync-server/src/utils/find_first_available_path.rs index eddd81d2..9e115eaf 100644 --- a/sync-server/src/utils/find_first_available_path.rs +++ b/sync-server/src/utils/find_first_available_path.rs @@ -1,20 +1,24 @@ -use crate::app_state::database::models::VaultId; +use crate::app_state::database::{WriteTransaction, models::VaultId}; +use crate::errors::{SyncServerError, server_error}; use crate::utils::dedup_paths::dedup_paths; -use anyhow::Result; +use anyhow::anyhow; use log::{debug, info}; -use sqlx::sqlite::SqliteConnection; pub async fn find_first_available_path( vault_id: &VaultId, sanitized_relative_path: &str, database: &crate::app_state::database::Database, - connection: &mut SqliteConnection, -) -> Result { + transaction: &mut WriteTransaction, +) -> Result { info!("Finding first available path for `{sanitized_relative_path}` in vault `{vault_id}`"); for candidate in dedup_paths(sanitized_relative_path) { debug!("Checking candidate path for deconflicting names: `{candidate}`"); if database - .get_latest_non_deleted_document_by_path(vault_id, &candidate, Some(connection)) + .get_latest_non_deleted_document_by_path( + vault_id, + &candidate, + Some(transaction.connection_mut().map_err(server_error)?), + ) .await? .is_none() { @@ -27,5 +31,7 @@ pub async fn find_first_available_path( ); } - unreachable!("dedup_paths produces infinite paths"); + Err(server_error(anyhow!( + "No available path candidates produced for `{sanitized_relative_path}` in vault `{vault_id}`" + ))) } diff --git a/sync-server/src/utils/is_binary.rs b/sync-server/src/utils/is_binary.rs index 09bfcf94..1c7e99b9 100644 --- a/sync-server/src/utils/is_binary.rs +++ b/sync-server/src/utils/is_binary.rs @@ -1,16 +1,22 @@ -/// Heuristically determine if the given data is a binary or a text file's -/// content. +/// Return the given data as UTF-8 text if it is not considered binary. /// /// Only text inputs can be reconciled using the crate's functions. #[must_use] -pub fn is_binary(data: &[u8]) -> bool { +pub fn as_non_binary_text(data: &[u8]) -> Option<&str> { if data.contains(&0) { // Even though the NUL character is valid in UTF-8, it's highly suspicious in // human-readable text. - return true; + return None; } - std::str::from_utf8(data).is_err() + std::str::from_utf8(data).ok() +} + +/// Heuristically determine if the given data is a binary or a text file's +/// content. +#[must_use] +pub fn is_binary(data: &[u8]) -> bool { + as_non_binary_text(data).is_none() } #[cfg(test)] @@ -23,4 +29,11 @@ mod tests { assert!(is_binary(&[0, 12])); assert!(!is_binary(b"hello")); } + + #[test] + fn test_as_non_binary_text() { + assert_eq!(as_non_binary_text(b"hello"), Some("hello")); + assert_eq!(as_non_binary_text(&[0, 12]), None); + assert_eq!(as_non_binary_text(&[0xff]), None); + } } diff --git a/sync-server/src/utils/rotating_file_writer.rs b/sync-server/src/utils/rotating_file_writer.rs index 1c5c86c5..1de3277a 100644 --- a/sync-server/src/utils/rotating_file_writer.rs +++ b/sync-server/src/utils/rotating_file_writer.rs @@ -2,11 +2,12 @@ use std::{ fs::{self, OpenOptions}, io::{self, Write}, path::{Path, PathBuf}, - sync::{Arc, Mutex}, + sync::{Arc, Mutex, MutexGuard}, time::{Duration, SystemTime, UNIX_EPOCH}, }; use chrono::NaiveDateTime; +use log::warn; use tracing_subscriber::fmt::MakeWriter; #[derive(Clone)] @@ -51,14 +52,14 @@ impl RotatingFileWriter { /// Parse timestamp from log filename and return as `SystemTime` fn parse_log_timestamp(filename: &str, file_prefix: &str) -> Option { // Expected format: {prefix}.{timestamp}.log where timestamp is %Y-%m-%d_%H-%M-%S - let prefix_len = file_prefix.len() + 1; // +1 for the dot + let prefix_len = file_prefix.len().checked_add(1)?; // +1 for the dot let timestamp_str = filename.get(prefix_len..filename.len().checked_sub(4)?)?; let dt = NaiveDateTime::parse_from_str(timestamp_str, "%Y-%m-%d_%H-%M-%S").ok()?; let timestamp = dt.and_utc(); let secs: u64 = timestamp.timestamp().try_into().ok()?; - Some(UNIX_EPOCH + Duration::from_secs(secs)) + UNIX_EPOCH.checked_add(Duration::from_secs(secs)) } fn find_latest_log_file(directory: &Path, file_prefix: &str) -> Option { @@ -85,7 +86,9 @@ impl RotatingFileWriter { Self::find_latest_log_file(directory, file_prefix) .and_then(|filename| Self::parse_log_timestamp(&filename, file_prefix)) .map_or_else(SystemTime::now, |last_rotation| { - last_rotation + rotation_duration + last_rotation + .checked_add(rotation_duration) + .unwrap_or_else(SystemTime::now) }) } @@ -93,6 +96,17 @@ impl RotatingFileWriter { SystemTime::now() >= inner.next_rotation_time } + fn lock_inner(&self) -> MutexGuard<'_, RotatingFileWriterInner> { + match self.inner.lock() { + Ok(inner) => inner, + Err(poisoned) => { + warn!("RotatingFileWriter mutex was poisoned, recovering"); + self.inner.clear_poison(); + poisoned.into_inner() + } + } + } + fn open_or_create_log_file(inner: &mut RotatingFileWriterInner) -> io::Result<()> { // If we haven't reached rotation time and there's an existing log file, reuse it if !Self::should_rotate(inner) @@ -124,7 +138,9 @@ impl RotatingFileWriter { .open(&filepath)?; inner.current_file = Some(file); - inner.next_rotation_time = SystemTime::now() + inner.rotation_duration; + inner.next_rotation_time = SystemTime::now() + .checked_add(inner.rotation_duration) + .unwrap_or_else(SystemTime::now); Ok(()) } @@ -132,10 +148,7 @@ impl RotatingFileWriter { impl Write for RotatingFileWriter { fn write(&mut self, buf: &[u8]) -> io::Result { - let mut inner = self.inner.lock().unwrap_or_else(|poisoned| { - eprintln!("RotatingFileWriter mutex was poisoned, recovering"); - poisoned.into_inner() - }); + let mut inner = self.lock_inner(); // Reset file handle after poison recovery so the next branch // re-opens a valid file rather than writing to a potentially @@ -154,10 +167,7 @@ impl Write for RotatingFileWriter { } fn flush(&mut self) -> io::Result<()> { - let mut inner = self.inner.lock().unwrap_or_else(|poisoned| { - eprintln!("RotatingFileWriter mutex was poisoned, recovering"); - poisoned.into_inner() - }); + let mut inner = self.lock_inner(); if let Some(ref mut file) = inner.current_file { file.flush() } else {