From 6d40097bcd71234887322f6e617a0d14bfa2faa0 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 9 May 2026 15:16:16 +0100 Subject: [PATCH 01/12] Clean up diff --- .../src/deterministic-agent.ts | 41 ++++++------- scripts/update-api-types.sh | 4 +- sync-server/Cargo.toml | 3 +- sync-server/clippy.toml | 4 ++ .../src/server/fetch_document_version.rs | 4 +- .../server/fetch_document_version_content.rs | 4 +- .../server/fetch_latest_document_version.rs | 51 ---------------- .../src/server/fetch_latest_documents.rs | 59 ------------------- sync-server/src/server/ping.rs | 7 ++- sync-server/src/server/responses.rs | 15 +---- sync-server/src/utils/rotating_file_writer.rs | 24 +++++--- 11 files changed, 52 insertions(+), 164 deletions(-) create mode 100644 sync-server/clippy.toml delete mode 100644 sync-server/src/server/fetch_latest_document_version.rs delete mode 100644 sync-server/src/server/fetch_latest_documents.rs diff --git a/frontend/deterministic-tests/src/deterministic-agent.ts b/frontend/deterministic-tests/src/deterministic-agent.ts index b32b01c2..9fb1eaa5 100644 --- a/frontend/deterministic-tests/src/deterministic-agent.ts +++ b/frontend/deterministic-tests/src/deterministic-agent.ts @@ -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,7 +138,7 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { this.nextCreateResponseDrop === undefined, `Client ${this.clientId} already has a create response drop armed` ); - let resolveDropped!: () => void; + let resolveDropped: () => void = () => {}; const dropped = new Promise((resolve) => { resolveDropped = resolve; }); @@ -461,23 +481,4 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { }; } - 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/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.toml b/sync-server/Cargo.toml index 2fed9d9b..c51460eb 100644 --- a/sync-server/Cargo.toml +++ b/sync-server/Cargo.toml @@ -51,7 +51,8 @@ missing_debug_implementations = "warn" [lints.clippy] await_holding_lock = "warn" dbg_macro = "warn" -empty_enum = "warn" +disallowed_macros = { level = "deny", priority = 1 } +empty_enums = "warn" enum_glob_use = "warn" exit = "warn" filter_map_next = "warn" diff --git a/sync-server/clippy.toml b/sync-server/clippy.toml new file mode 100644 index 00000000..2b275dbd --- /dev/null +++ b/sync-server/clippy.toml @@ -0,0 +1,4 @@ +disallowed-macros = [ + { path = "std::eprintln", reason = "use log::info! or log::warn! instead" }, + { path = "std::println", reason = "use log::info! or log::warn! instead" }, +] diff --git a/sync-server/src/server/fetch_document_version.rs b/sync-server/src/server/fetch_document_version.rs index 159cad3a..c30f1d76 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, server_error}, utils::normalize::normalize, }; @@ -52,7 +52,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..9fdd0ad8 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, server_error}, utils::normalize::normalize, }; @@ -52,7 +52,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/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/utils/rotating_file_writer.rs b/sync-server/src/utils/rotating_file_writer.rs index 1c5c86c5..da6d0d7d 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)] @@ -93,6 +94,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) @@ -132,10 +144,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 +163,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 { From 792f57dc7e05fda022f295f0f48f83c07128cf08 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 9 May 2026 15:28:43 +0100 Subject: [PATCH 02/12] Fix lints & format --- CLAUDE.md | 7 +- frontend/deterministic-tests/README.md | 25 +- frontend/deterministic-tests/src/cli.ts | 2 +- .../src/deterministic-agent.ts | 35 +- .../deterministic-tests/src/server-control.ts | 131 +- .../deterministic-tests/src/test-runner.ts | 8 +- ...-and-create-at-target-create-first.test.ts | 79 +- ...-and-create-at-target-rename-first.test.ts | 85 +- .../src/utils/assertable-state.ts | 20 - frontend/local-client-cli/src/args.test.ts | 1 - frontend/local-client-cli/src/cli.ts | 3 +- .../local-client-cli/src/node-filesystem.ts | 2 +- .../src/obsidian-file-system.ts | 4 - frontend/package-lock.json | 1257 +---------------- .../sync-client/src/services/sync-service.ts | 58 - .../types/FetchLatestDocumentsResponse.ts | 13 - frontend/sync-client/src/sync-client.ts | 33 +- .../offline-change-detector.test.ts | 37 +- .../offline-change-detector.ts | 37 +- .../src/sync-operations/reconciler.test.ts | 17 +- .../sync-operations/sync-event-queue.test.ts | 5 +- .../src/sync-operations/sync-event-queue.ts | 24 +- frontend/sync-client/src/tracing/logger.ts | 5 - .../src/utils/data-structures/locks.ts | 4 - frontend/test-client/src/agent/mock-agent.ts | 18 +- sync-server/clippy.toml | 1 - sync-server/src/app_state/cursors.rs | 2 +- sync-server/src/app_state/database.rs | 11 +- sync-server/src/app_state/database/models.rs | 1 - sync-server/src/app_state/websocket/utils.rs | 4 +- sync-server/src/errors.rs | 5 +- sync-server/src/main.rs | 61 +- sync-server/src/server.rs | 19 +- sync-server/src/server/auth.rs | 8 +- sync-server/src/server/create_document.rs | 4 +- sync-server/src/server/websocket.rs | 3 +- 36 files changed, 342 insertions(+), 1687 deletions(-) delete mode 100644 frontend/sync-client/src/services/types/FetchLatestDocumentsResponse.ts diff --git a/CLAUDE.md b/CLAUDE.md index 39161e39..ab91695c 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 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 9fb1eaa5..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( @@ -138,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"); } @@ -175,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; } @@ -324,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, @@ -480,5 +474,4 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem { return response; }; } - } 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-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/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/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/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/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..3a47152e 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -56,13 +56,7 @@ export class SyncClient { 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 expectedFsEvents: ExpectedFsEvents ) {} public get syncedDocumentCount(): number { @@ -172,7 +166,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 }; @@ -269,8 +263,7 @@ export class SyncClient { contentCache, serverConfig, syncService, - expectedFsEvents, - persistence + expectedFsEvents ); logger.info("SyncClient created successfully"); @@ -322,26 +315,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"); 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/sync-event-queue.test.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts index d2676011..aef7c5f7 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 @@ -307,7 +307,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..9cc986d9 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.ts @@ -220,9 +220,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 ( @@ -324,7 +322,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 +341,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. @@ -648,14 +646,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 { @@ -814,6 +804,7 @@ export class SyncEventQueue { 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++; @@ -866,6 +857,7 @@ export class SyncEventQueue { 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 +899,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/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/sync-server/clippy.toml b/sync-server/clippy.toml index 2b275dbd..81c6e562 100644 --- a/sync-server/clippy.toml +++ b/sync-server/clippy.toml @@ -1,4 +1,3 @@ disallowed-macros = [ { path = "std::eprintln", reason = "use log::info! or log::warn! instead" }, - { path = "std::println", 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..b729131f 100644 --- a/sync-server/src/app_state/cursors.rs +++ b/sync-server/src/app_state/cursors.rs @@ -118,7 +118,7 @@ impl Cursors { }; self.broadcasts.send_document_update( - vault_id.clone(), + vault_id, WebSocketServerMessageWithOrigin::new(WebSocketServerMessage::CursorPositions( CursorPositionFromServer { clients: client_cursors, diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index 1fa6d223..e774824b 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -34,6 +34,10 @@ use super::websocket::{ use crate::config::database_config::DatabaseConfig; use crate::consts::IDLE_POOL_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 /// with reads for pool slots. @@ -182,7 +186,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( @@ -817,8 +821,7 @@ impl Database { } else { WebSocketServerMessageWithOrigin::with_origin(version.device_id.clone(), envelope) }; - self.broadcasts - .send_document_update(vault_id.clone(), with_origin); + self.broadcasts.send_document_update(vault_id, with_origin); Ok(()) } @@ -831,7 +834,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..cf8f379c 100644 --- a/sync-server/src/app_state/database/models.rs +++ b/sync-server/src/app_state/database/models.rs @@ -83,7 +83,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/utils.rs b/sync-server/src/app_state/websocket/utils.rs index d78360de..a1e824b7 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( diff --git a/sync-server/src/errors.rs b/sync-server/src/errors.rs index 892db36f..ef0d017d 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(_) => {} diff --git a/sync-server/src/main.rs b/sync-server/src/main.rs index dc00d4d5..7cf2227c 100644 --- a/sync-server/src/main.rs +++ b/sync-server/src/main.rs @@ -14,7 +14,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 +36,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..35bcd4f6 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<()> { @@ -95,6 +95,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 @@ -131,18 +132,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 +226,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..afff662d 100644 --- a/sync-server/src/server/create_document.rs +++ b/sync-server/src/server/create_document.rs @@ -136,9 +136,7 @@ pub async fn create_document( { 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, diff --git a/sync-server/src/server/websocket.rs b/sync-server/src/server/websocket.rs index 6e1af0ba..2cf91d1d 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); From e5373ab2bb3d363fc987bcc2f5fad3392f22de0f Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 9 May 2026 16:27:48 +0100 Subject: [PATCH 03/12] Improve diff --- .../file-operations/file-operations.test.ts | 6 +- .../src/file-operations/file-operations.ts | 20 +-- .../sync-client/src/sync-operations/syncer.ts | 4 +- scripts/check.sh | 3 +- sync-server/Cargo.lock | 1 - sync-server/Cargo.toml | 14 +- sync-server/src/app_state/cursors.rs | 28 ++-- sync-server/src/app_state/database.rs | 32 ++--- .../src/app_state/websocket/broadcasts.rs | 52 ++++--- sync-server/src/cli/color_when.rs | 9 +- sync-server/src/config/server_config.rs | 4 + sync-server/src/config/user_config.rs | 27 ++-- sync-server/src/main.rs | 16 +++ sync-server/src/server.rs | 10 +- sync-server/src/server/create_document.rs | 13 +- sync-server/src/server/delete_document.rs | 15 +- sync-server/src/server/rate_limit.rs | 42 +++--- sync-server/src/server/update_document.rs | 129 ++++++++++-------- sync-server/src/server/websocket.rs | 4 +- sync-server/src/utils/dedup_paths.rs | 51 ++++--- .../src/utils/find_first_available_path.rs | 17 ++- sync-server/src/utils/is_binary.rs | 23 +++- sync-server/src/utils/rotating_file_writer.rs | 12 +- 23 files changed, 312 insertions(+), 220 deletions(-) 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..44b4fe7e 100644 --- a/frontend/sync-client/src/file-operations/file-operations.test.ts +++ b/frontend/sync-client/src/file-operations/file-operations.test.ts @@ -85,7 +85,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 +109,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 +119,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..b73bcec9 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -11,16 +11,6 @@ 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; @@ -68,7 +58,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`, @@ -84,7 +74,7 @@ export class FileOperations { this.expectedFsEvents.unexpectCreate(path); throw e; } - return { actualPath: path }; + return path; } /** @@ -220,9 +210,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)) { @@ -241,7 +231,7 @@ export class FileOperations { throw e; } await this.deletingEmptyParentDirectoriesOfDeletedFile(oldPath); - return { actualPath: newPath }; + return newPath; } private async deletingEmptyParentDirectoriesOfDeletedFile( diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 4e908600..483597fa 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -1103,7 +1103,7 @@ export class Syncer { remoteHash, localPath: target }); - const result = await this.operations.create( + const createdPath = await this.operations.create( target, remoteContent ); @@ -1112,7 +1112,7 @@ export class Syncer { ); localPath = liveRecord === undefined - ? result.actualPath + ? createdPath : liveRecord.localPath; await this.updateCache( remoteVersion.vaultUpdateId, 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/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 c51460eb..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,16 +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" 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" @@ -68,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 @@ -88,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/src/app_state/cursors.rs b/sync-server/src/app_state/cursors.rs index b729131f..6bde3613 100644 --- a/sync-server/src/app_state/cursors.rs +++ b/sync-server/src/app_state/cursors.rs @@ -15,6 +15,7 @@ use super::{ }; use crate::{ app_state::websocket::models::DocumentWithCursors, config::database_config::DatabaseConfig, + errors::SyncServerError, }; #[derive(Clone, Debug)] @@ -39,7 +40,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 +55,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 { @@ -76,15 +77,17 @@ impl Cursors { loop { tokio::select! { () = tokio::time::sleep(Duration::from_secs(1)) => { - self.remove_expired_cursors().await; + 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 +107,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 @@ -124,10 +129,14 @@ impl Cursors { 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 +154,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 e774824b..c9122538 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -5,7 +5,7 @@ use std::{ sync::atomic::{AtomicU64, Ordering}, }; -use anyhow::{Context as _, Result}; +use anyhow::{Context as _, Result, anyhow}; use log::info; use models::{ DocumentId, DocumentVersionWithoutContent, StoredDocumentVersion, VaultId, VaultUpdateId, @@ -132,6 +132,12 @@ impl WriteTransaction { } Ok(()) } + + pub fn connection_mut(&mut self) -> Result<&mut SqliteConnection> { + self.conn + .as_deref_mut() + .context("WriteTransaction already consumed") + } } impl Drop for WriteTransaction { @@ -147,25 +153,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. @@ -797,7 +784,7 @@ impl Database { let _send_guard = self.broadcasts.acquire_send_lock(vault_id).await; query - .execute(&mut *transaction) + .execute(transaction.connection_mut()?) .await .context("Cannot insert document version")?; @@ -821,7 +808,8 @@ impl Database { } else { WebSocketServerMessageWithOrigin::with_origin(version.device_id.clone(), envelope) }; - self.broadcasts.send_document_update(vault_id, with_origin); + self.broadcasts + .send_document_update(vault_id, with_origin)?; Ok(()) } diff --git a/sync-server/src/app_state/websocket/broadcasts.rs b/sync-server/src/app_state/websocket/broadcasts.rs index b9e2ea39..5dec6221 100644 --- a/sync-server/src/app_state/websocket/broadcasts.rs +++ b/sync-server/src/app_state/websocket/broadcasts.rs @@ -7,7 +7,11 @@ use log::{debug, info, warn}; use tokio::sync::{Mutex, broadcast}; use super::models::{WebSocketServerMessage, WebSocketServerMessageWithOrigin}; -use crate::{app_state::database::models::VaultId, config::server_config::ServerConfig}; +use crate::{ + app_state::database::models::VaultId, + config::server_config::ServerConfig, + errors::{SyncServerError, client_error, server_error}, +}; #[derive(Debug, Clone)] pub struct Broadcasts { @@ -60,30 +64,31 @@ 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) + .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); + let pruned_self = pruned + .iter() + .any(|pruned_vault| pruned_vault.as_str() == vault); 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})" ))); } @@ -100,8 +105,13 @@ impl Broadcasts { /// 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) { + /// 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: WebSocketServerMessageWithOrigin, + ) -> Result<(), SyncServerError> { let vault_update_id = match &document.message { WebSocketServerMessage::VaultUpdate(u) => Some(u.document.vault_update_id), WebSocketServerMessage::CursorPositions(_) => None, @@ -110,18 +120,21 @@ impl Broadcasts { 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 mut tx_map = self.tx.lock().map_err(|_| { + server_error(anyhow::anyhow!( + "broadcasts.tx mutex poisoned; skipping document update broadcast" + )) + })?; let count_before_prune = tx_map - .get(&vault) + .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); + let pruned_self = pruned + .iter() + .any(|pruned_vault| pruned_vault.as_str() == vault); 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(); @@ -131,7 +144,7 @@ impl Broadcasts { "[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); @@ -143,5 +156,6 @@ impl Broadcasts { "[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}" ), } + Ok(()) } } 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/main.rs b/sync-server/src/main.rs index 7cf2227c..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; diff --git a/sync-server/src/server.rs b/sync-server/src/server.rs index 35bcd4f6..44960bc6 100644 --- a/sync-server/src/server.rs +++ b/sync-server/src/server.rs @@ -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) @@ -104,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() diff --git a/sync-server/src/server/create_document.rs b/sync-server/src/server/create_document.rs index afff662d..cd70c4e2 100644 --- a/sync-server/src/server/create_document.rs +++ b/sync-server/src/server/create_document.rs @@ -60,7 +60,7 @@ 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)?; @@ -129,7 +129,7 @@ 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)? @@ -157,7 +157,10 @@ pub async fn create_document( let last_update_id = state .database - .get_max_update_id_in_vault(&vault_id, Some(&mut *transaction)) + .get_max_update_id_in_vault( + &vault_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) .await .map_err(server_error)?; @@ -176,7 +179,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, diff --git a/sync-server/src/server/delete_document.rs b/sync-server/src/server/delete_document.rs index 2ee6eac3..54360a3d 100644 --- a/sync-server/src/server/delete_document.rs +++ b/sync-server/src/server/delete_document.rs @@ -48,13 +48,20 @@ pub async fn delete_document( let last_update_id = state .database - .get_max_update_id_in_vault(&vault_id, Some(&mut transaction)) + .get_max_update_id_in_vault( + &vault_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) .await .map_err(server_error)?; let latest_version = state .database - .get_latest_document(&vault_id, &document_id, Some(&mut transaction)) + .get_latest_document( + &vault_id, + &document_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) .await .map_err(server_error)?; @@ -80,7 +87,9 @@ pub async fn delete_document( 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; 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/update_document.rs b/sync-server/src/server/update_document.rs index 0145288c..ac2a2987 100644 --- a/sync-server/src/server/update_document.rs +++ b/sync-server/src/server/update_document.rs @@ -27,7 +27,7 @@ use crate::{ }, 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, }, @@ -173,13 +173,20 @@ pub async fn update_document( let last_update_id = state .database - .get_max_update_id_in_vault(&vault_id, Some(&mut transaction)) + .get_max_update_id_in_vault( + &vault_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) .await .map_err(server_error)?; let latest_version = state .database - .get_latest_document(&vault_id, &document_id, Some(&mut transaction)) + .get_latest_document( + &vault_id, + &document_id, + Some(transaction.connection_mut().map_err(server_error)?), + ) .await .map_err(server_error)? .map_or_else( @@ -225,64 +232,56 @@ pub async fn update_document( ))); } - // 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 || { + let merged = reconcile( + &parent_owned, + &latest_owned.into(), + &new_owned.into(), + &*BuiltinTokenizer::Word, + ) + .apply() + .text() + .into_bytes(); + merged + }) + .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 @@ -306,7 +305,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,7 +315,7 @@ 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 @@ -323,9 +324,21 @@ pub async fn update_document( .await .map_err(server_error)?; - 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 2cf91d1d..1bf49dbf 100644 --- a/sync-server/src/server/websocket.rs +++ b/sync-server/src/server/websocket.rs @@ -306,7 +306,7 @@ async fn websocket( &device_id, docs, ) - .await; + .await?; } } } @@ -351,7 +351,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..97361240 100644 --- a/sync-server/src/utils/find_first_available_path.rs +++ b/sync-server/src/utils/find_first_available_path.rs @@ -1,20 +1,23 @@ -use crate::app_state::database::models::VaultId; +use crate::app_state::database::{WriteTransaction, models::VaultId}; use crate::utils::dedup_paths::dedup_paths; -use anyhow::Result; +use anyhow::{Result, 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, + 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()?), + ) .await? .is_none() { @@ -27,5 +30,7 @@ pub async fn find_first_available_path( ); } - unreachable!("dedup_paths produces infinite paths"); + Err(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 da6d0d7d..1de3277a 100644 --- a/sync-server/src/utils/rotating_file_writer.rs +++ b/sync-server/src/utils/rotating_file_writer.rs @@ -52,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 { @@ -86,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) }) } @@ -136,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(()) } From afa3b6dca3ab9777ae1cbc2b3920f2281532737a Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 9 May 2026 16:45:51 +0100 Subject: [PATCH 04/12] Improve diff --- sync-server/build.rs | 1 + sync-server/config-e2e.yml | 2 +- sync-server/src/app_state/cursors.rs | 4 +++- sync-server/src/app_state/websocket/models.rs | 9 --------- sync-server/src/app_state/websocket/utils.rs | 3 +++ 5 files changed, 8 insertions(+), 11 deletions(-) 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/config-e2e.yml b/sync-server/config-e2e.yml index 03b860b7..7b43ced9 100644 --- a/sync-server/config-e2e.yml +++ b/sync-server/config-e2e.yml @@ -1,5 +1,5 @@ database: - databases_directory_path: /host/tmp/vaultlink-e2e-databases + databases_directory_path: databases max_connections_per_vault: 8 cursor_timeout: 1m server: diff --git a/sync-server/src/app_state/cursors.rs b/sync-server/src/app_state/cursors.rs index 6bde3613..d3ea0602 100644 --- a/sync-server/src/app_state/cursors.rs +++ b/sync-server/src/app_state/cursors.rs @@ -18,6 +18,8 @@ use crate::{ errors::SyncServerError, }; +const CURSOR_CLEANUP_INTERVAL: Duration = Duration::from_secs(1); + #[derive(Clone, Debug)] pub struct Cursors { config: DatabaseConfig, @@ -76,7 +78,7 @@ impl Cursors { tokio::spawn(async move { loop { tokio::select! { - () = tokio::time::sleep(Duration::from_secs(1)) => { + () = tokio::time::sleep(CURSOR_CLEANUP_INTERVAL) => { self.remove_expired_cursors().await?; } Ok(()) = shutdown.changed() => break, diff --git a/sync-server/src/app_state/websocket/models.rs b/sync-server/src/app_state/websocket/models.rs index eb6c956a..8a8d42cc 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 { diff --git a/sync-server/src/app_state/websocket/utils.rs b/sync-server/src/app_state/websocket/utils.rs index a1e824b7..834684bb 100644 --- a/sync-server/src/app_state/websocket/utils.rs +++ b/sync-server/src/app_state/websocket/utils.rs @@ -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, From eb23f445d0cc34ee4dbdc7c7ce11b146b84b9c4c Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 9 May 2026 22:19:01 +0100 Subject: [PATCH 05/12] Fix path --- sync-server/config-e2e.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sync-server/config-e2e.yml b/sync-server/config-e2e.yml index 7b43ced9..9ba68682 100644 --- a/sync-server/config-e2e.yml +++ b/sync-server/config-e2e.yml @@ -1,5 +1,5 @@ database: - databases_directory_path: databases + databases_directory_path: /tmp/databases max_connections_per_vault: 8 cursor_timeout: 1m server: From 0329fc29f21ffcf36b171cdca1b7c79e254ce8f7 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 10 May 2026 15:08:40 +0100 Subject: [PATCH 06/12] Fix slow commit bug --- .../deterministic-tests/src/test-registry.ts | 4 +- .../disable-mid-create-then-delete.test.ts | 56 +++++++++++++++++++ scripts/clean-up.sh | 2 +- scripts/e2e.sh | 2 +- sync-server/src/app_state/database.rs | 21 +++---- sync-server/src/app_state/websocket/models.rs | 17 +----- sync-server/src/server/websocket.rs | 26 ++++++--- 7 files changed, 88 insertions(+), 40 deletions(-) create mode 100644 frontend/deterministic-tests/src/tests/disable-mid-create-then-delete.test.ts 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/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/scripts/clean-up.sh b/scripts/clean-up.sh index dcf400bb..267a1019 100755 --- a/scripts/clean-up.sh +++ b/scripts/clean-up.sh @@ -1,4 +1,4 @@ #!/bin/bash -rm -rf /host/tmp/vaultlink-e2e-databases +rm -rf /tmp/vaultlink-e2e-databases rm -rf logs diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 7ab8d90c..abc3dcd2 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -31,7 +31,7 @@ sleep 1 # Clean databases (uses tmpfs via /dev/shm for zero disk I/O) echo "Cleaning databases..." -rm -rf /host/tmp/vaultlink-e2e-databases +rm -rf /tmp/databases # Start the server in the background echo "Starting server..." diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index c9122538..1a2483a2 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -793,23 +793,18 @@ impl Database { .await .context("Failed to commit transaction")?; - // 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. + // 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. 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, with_origin)?; + .send_document_update(vault_id, WebSocketServerMessageWithOrigin::new(envelope))?; Ok(()) } diff --git a/sync-server/src/app_state/websocket/models.rs b/sync-server/src/app_state/websocket/models.rs index 8a8d42cc..ebe4018b 100644 --- a/sync-server/src/app_state/websocket/models.rs +++ b/sync-server/src/app_state/websocket/models.rs @@ -80,28 +80,13 @@ pub enum WebSocketServerMessage { 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, - } + Self { message } } } diff --git a/sync-server/src/server/websocket.rs b/sync-server/src/server/websocket.rs index 1bf49dbf..e2db6ca9 100644 --- a/sync-server/src/server/websocket.rs +++ b/sync-server/src/server/websocket.rs @@ -208,14 +208,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 From d8b6ec5b77447c09047c7cc50fc965d6e8893683 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 10 May 2026 15:16:40 +0100 Subject: [PATCH 07/12] Simplify --- sync-server/src/app_state/cursors.rs | 13 ++++--------- sync-server/src/app_state/database.rs | 13 +++++++------ sync-server/src/app_state/websocket/broadcasts.rs | 14 +++++++------- sync-server/src/app_state/websocket/models.rs | 11 ----------- sync-server/src/server/websocket.rs | 6 +++--- 5 files changed, 21 insertions(+), 36 deletions(-) diff --git a/sync-server/src/app_state/cursors.rs b/sync-server/src/app_state/cursors.rs index d3ea0602..130da680 100644 --- a/sync-server/src/app_state/cursors.rs +++ b/sync-server/src/app_state/cursors.rs @@ -7,10 +7,7 @@ use super::{ database::models::{DeviceId, VaultId}, websocket::{ broadcasts::Broadcasts, - models::{ - ClientCursors, CursorPositionFromServer, WebSocketServerMessage, - WebSocketServerMessageWithOrigin, - }, + models::{ClientCursors, CursorPositionFromServer, WebSocketServerMessage}, }, }; use crate::{ @@ -126,11 +123,9 @@ impl Cursors { self.broadcasts.send_document_update( vault_id, - WebSocketServerMessageWithOrigin::new(WebSocketServerMessage::CursorPositions( - CursorPositionFromServer { - clients: client_cursors, - }, - )), + WebSocketServerMessage::CursorPositions(CursorPositionFromServer { + clients: client_cursors, + }), ) } diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index 1a2483a2..ace07de3 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -29,7 +29,7 @@ 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; @@ -800,11 +800,12 @@ impl Database { // delivery channel — and the client-side `parentVersionId` // dedup absorbs the redundant message when the response made it // through. - let envelope = WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate { - document: version.clone().into(), - }); - self.broadcasts - .send_document_update(vault_id, WebSocketServerMessageWithOrigin::new(envelope))?; + self.broadcasts.send_document_update( + vault_id, + WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate { + document: version.clone().into(), + }), + )?; Ok(()) } diff --git a/sync-server/src/app_state/websocket/broadcasts.rs b/sync-server/src/app_state/websocket/broadcasts.rs index 5dec6221..0ef21e4e 100644 --- a/sync-server/src/app_state/websocket/broadcasts.rs +++ b/sync-server/src/app_state/websocket/broadcasts.rs @@ -6,7 +6,7 @@ use std::{ use log::{debug, info, warn}; use tokio::sync::{Mutex, broadcast}; -use super::models::{WebSocketServerMessage, WebSocketServerMessageWithOrigin}; +use super::models::WebSocketServerMessage; use crate::{ app_state::database::models::VaultId, config::server_config::ServerConfig, @@ -21,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 { @@ -66,7 +66,7 @@ impl Broadcasts { &self, vault: &VaultId, max_clients: usize, - ) -> Result, SyncServerError> { + ) -> Result, SyncServerError> { let mut tx_map = self .tx .lock() @@ -110,13 +110,13 @@ impl Broadcasts { pub fn send_document_update( &self, vault: &str, - document: WebSocketServerMessageWithOrigin, + document: WebSocketServerMessage, ) -> Result<(), SyncServerError> { - let vault_update_id = match &document.message { + let vault_update_id = match &document { WebSocketServerMessage::VaultUpdate(u) => Some(u.document.vault_update_id), WebSocketServerMessage::CursorPositions(_) => None, }; - let is_deleted = match &document.message { + let is_deleted = match &document { WebSocketServerMessage::VaultUpdate(u) => Some(u.document.is_deleted), WebSocketServerMessage::CursorPositions(_) => None, }; diff --git a/sync-server/src/app_state/websocket/models.rs b/sync-server/src/app_state/websocket/models.rs index ebe4018b..60d690cd 100644 --- a/sync-server/src/app_state/websocket/models.rs +++ b/sync-server/src/app_state/websocket/models.rs @@ -79,14 +79,3 @@ pub enum WebSocketServerMessage { VaultUpdate(WebSocketVaultUpdate), CursorPositions(CursorPositionFromServer), } - -#[derive(Clone, Debug)] -pub struct WebSocketServerMessageWithOrigin { - pub message: WebSocketServerMessage, -} - -impl WebSocketServerMessageWithOrigin { - pub fn new(message: WebSocketServerMessage) -> Self { - Self { message } - } -} diff --git a/sync-server/src/server/websocket.rs b/sync-server/src/server/websocket.rs index e2db6ca9..379f68fd 100644 --- a/sync-server/src/server/websocket.rs +++ b/sync-server/src/server/websocket.rs @@ -238,13 +238,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 { @@ -253,7 +253,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?; From ce995cdc33716a79f25bea32f9b1bf5237a3e7b3 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 10 May 2026 18:36:57 +0100 Subject: [PATCH 08/12] Remove isNewFile --- CLAUDE.md | 4 +-- ...chup-create-and-update-not-skipped.test.ts | 23 +++++++--------- .../types/DocumentVersionWithoutContent.ts | 4 --- .../sync-operations/sync-event-queue.test.ts | 1 - .../src/sync-operations/sync-event-queue.ts | 5 ++-- .../sync-client/src/sync-operations/syncer.ts | 11 +------- sync-server/src/app_state/database.rs | 26 +++---------------- sync-server/src/app_state/database/models.rs | 5 ---- 8 files changed, 18 insertions(+), 61 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ab91695c..2caab8dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,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 @@ -134,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/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/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/sync-operations/sync-event-queue.test.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts index aef7c5f7..9aadebb4 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 }; } 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 9cc986d9..66dcf1a4 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.ts @@ -618,9 +618,8 @@ 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(); } diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 483597fa..c51e7394 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -703,8 +703,7 @@ export class Syncer { if (response.isDeleted) { await this.processRemoteDelete(record.localPath, { ...response, - contentSize: 0, - isNewFile: false + contentSize: 0 }); return; } @@ -859,14 +858,6 @@ export class Syncer { return this.processRemoteUpdate(trackedRecord, remoteVersion); } - if (!remoteVersion.isNewFile) { - this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId; - this.logger.debug( - `Ignoring stale RemoteChange for untracked, non-new document ${remoteVersion.documentId}` - ); - return; - } - return this.processRemoteCreateForNewDocument(remoteVersion); } diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index ace07de3..b47263bc 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -405,7 +405,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", @@ -439,7 +438,6 @@ 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() }) @@ -466,19 +464,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", @@ -523,17 +516,6 @@ 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() }) diff --git a/sync-server/src/app_state/database/models.rs b/sync-server/src/app_state/database/models.rs index cf8f379c..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, } } } From 2d69d4b26db01b2b1048fe24e63a45b0ce7de5da Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Mon, 11 May 2026 20:25:37 +0100 Subject: [PATCH 09/12] Push down db error returning --- sync-server/src/app_state/database.rs | 161 +++++++++++++----- sync-server/src/app_state/websocket/utils.rs | 2 - sync-server/src/consts.rs | 4 + sync-server/src/errors.rs | 25 ++- sync-server/src/server/create_document.rs | 20 +-- sync-server/src/server/delete_document.rs | 28 +-- .../src/server/fetch_document_version.rs | 5 +- .../server/fetch_document_version_content.rs | 5 +- sync-server/src/server/update_document.rs | 37 ++-- sync-server/src/server/websocket.rs | 3 +- .../src/utils/find_first_available_path.rs | 11 +- 11 files changed, 172 insertions(+), 129 deletions(-) diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index b47263bc..86f00d6d 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -5,13 +5,15 @@ use std::{ sync::atomic::{AtomicU64, Ordering}, }; -use anyhow::{Context as _, Result, anyhow}; +use anyhow::{Context as _, Result}; use log::info; use models::{ DocumentId, DocumentVersionWithoutContent, StoredDocumentVersion, VaultId, VaultUpdateId, }; 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, }; @@ -32,7 +52,7 @@ use super::websocket::{ 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) @@ -87,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"); @@ -113,22 +127,24 @@ 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(()) } @@ -265,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 @@ -292,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, @@ -309,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) @@ -375,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 @@ -384,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 @@ -397,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); @@ -423,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") @@ -441,6 +473,7 @@ impl Database { }) .collect() }) + .map_err(database_error) } /// Return the latest state of all documents (including deleted) in the @@ -454,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); @@ -499,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(|| { @@ -519,13 +557,14 @@ impl Database { }) .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 @@ -537,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( @@ -549,7 +594,7 @@ impl Database { vault: &VaultId, relative_path: &str, connection: Option<&mut SqliteConnection>, - ) -> Result> { + ) -> Result, SyncServerError> { let query = sqlx::query_as!( StoredDocumentVersion, r#" @@ -578,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 @@ -611,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#" @@ -646,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( @@ -657,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, @@ -683,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( @@ -694,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#" @@ -718,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 @@ -730,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#" @@ -766,14 +835,12 @@ impl Database { let _send_guard = self.broadcasts.acquire_send_lock(vault_id).await; query - .execute(transaction.connection_mut()?) + .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?; // Broadcast every commit to every connected client, including // the originator. The HTTP response is the originator's normal diff --git a/sync-server/src/app_state/websocket/utils.rs b/sync-server/src/app_state/websocket/utils.rs index 834684bb..ae75e3e3 100644 --- a/sync-server/src/app_state/websocket/utils.rs +++ b/sync-server/src/app_state/websocket/utils.rs @@ -65,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/consts.rs b/sync-server/src/consts.rs index e03b848f..b92fb139 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 ef0d017d..4f27598b 100644 --- a/sync-server/src/errors.rs +++ b/sync-server/src/errors.rs @@ -163,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/server/create_document.rs b/sync-server/src/server/create_document.rs index cd70c4e2..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(); @@ -62,8 +61,7 @@ pub async fn create_document( &sanitized_relative_path, 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 @@ -131,8 +129,7 @@ pub async fn create_document( &new_content, 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 `{}`", @@ -161,8 +158,7 @@ pub async fn create_document( &vault_id, Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .map_err(server_error)?; + .await?; let deduped_path = find_first_available_path( &vault_id, @@ -170,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!( @@ -198,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 54360a3d..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,8 +43,7 @@ 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 @@ -52,8 +51,7 @@ pub async fn delete_document( &vault_id, Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .map_err(server_error)?; + .await?; let latest_version = state .database @@ -62,26 +60,17 @@ pub async fn delete_document( &document_id, Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .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())); @@ -110,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 c30f1d76..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, 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!( diff --git a/sync-server/src/server/fetch_document_version_content.rs b/sync-server/src/server/fetch_document_version_content.rs index 9fdd0ad8..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, 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!( diff --git a/sync-server/src/server/update_document.rs b/sync-server/src/server/update_document.rs index ac2a2987..a071d1e9 100644 --- a/sync-server/src/server/update_document.rs +++ b/sync-server/src/server/update_document.rs @@ -22,9 +22,7 @@ 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::as_non_binary_text, @@ -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!( @@ -177,8 +172,7 @@ pub async fn update_document( &vault_id, Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .map_err(server_error)?; + .await?; let latest_version = state .database @@ -187,8 +181,7 @@ pub async fn update_document( &document_id, Some(transaction.connection_mut().map_err(server_error)?), ) - .await - .map_err(server_error)? + .await? .map_or_else( || { Err(not_found_error(anyhow!( @@ -199,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( @@ -221,11 +210,7 @@ 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(), @@ -289,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!( @@ -321,8 +305,7 @@ pub async fn update_document( state .database .insert_document_version(&vault_id, &new_version, transaction) - .await - .map_err(server_error)?; + .await?; Ok(Json(if is_same_as_request { DocumentUpdateResponse::FastForwardUpdate(new_version.into()) diff --git a/sync-server/src/server/websocket.rs b/sync-server/src/server/websocket.rs index 379f68fd..936eb483 100644 --- a/sync-server/src/server/websocket.rs +++ b/sync-server/src/server/websocket.rs @@ -162,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, diff --git a/sync-server/src/utils/find_first_available_path.rs b/sync-server/src/utils/find_first_available_path.rs index 97361240..9e115eaf 100644 --- a/sync-server/src/utils/find_first_available_path.rs +++ b/sync-server/src/utils/find_first_available_path.rs @@ -1,6 +1,7 @@ use crate::app_state::database::{WriteTransaction, models::VaultId}; +use crate::errors::{SyncServerError, server_error}; use crate::utils::dedup_paths::dedup_paths; -use anyhow::{Result, anyhow}; +use anyhow::anyhow; use log::{debug, info}; pub async fn find_first_available_path( @@ -8,7 +9,7 @@ pub async fn find_first_available_path( sanitized_relative_path: &str, database: &crate::app_state::database::Database, transaction: &mut WriteTransaction, -) -> Result { +) -> 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}`"); @@ -16,7 +17,7 @@ pub async fn find_first_available_path( .get_latest_non_deleted_document_by_path( vault_id, &candidate, - Some(transaction.connection_mut()?), + Some(transaction.connection_mut().map_err(server_error)?), ) .await? .is_none() @@ -30,7 +31,7 @@ pub async fn find_first_available_path( ); } - Err(anyhow!( + Err(server_error(anyhow!( "No available path candidates produced for `{sanitized_relative_path}` in vault `{vault_id}`" - )) + ))) } From cd08cd80c71ce11114638c877f96246950073763 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Mon, 11 May 2026 20:49:46 +0100 Subject: [PATCH 10/12] Clean up logs --- .../src/app_state/websocket/broadcasts.rs | 44 +++---------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/sync-server/src/app_state/websocket/broadcasts.rs b/sync-server/src/app_state/websocket/broadcasts.rs index 0ef21e4e..45ce78cb 100644 --- a/sync-server/src/app_state/websocket/broadcasts.rs +++ b/sync-server/src/app_state/websocket/broadcasts.rs @@ -3,7 +3,7 @@ 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; @@ -72,13 +72,7 @@ impl Broadcasts { .lock() .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 - .iter() - .any(|pruned_vault| pruned_vault.as_str() == vault); + Self::prune_inactive_vaults(&mut tx_map); let sender = tx_map .entry(vault.to_owned()) @@ -94,11 +88,6 @@ impl Broadcasts { } 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) } @@ -112,26 +101,12 @@ impl Broadcasts { vault: &str, document: WebSocketServerMessage, ) -> Result<(), SyncServerError> { - let vault_update_id = match &document { - WebSocketServerMessage::VaultUpdate(u) => Some(u.document.vault_update_id), - WebSocketServerMessage::CursorPositions(_) => None, - }; - let is_deleted = match &document { - WebSocketServerMessage::VaultUpdate(u) => Some(u.document.is_deleted), - WebSocketServerMessage::CursorPositions(_) => None, - }; let mut tx_map = self.tx.lock().map_err(|_| { server_error(anyhow::anyhow!( "broadcasts.tx mutex poisoned; skipping document update broadcast" )) })?; - 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 - .iter() - .any(|pruned_vault| pruned_vault.as_str() == vault); + Self::prune_inactive_vaults(&mut tx_map); let sender = tx_map .entry(vault.to_owned()) @@ -140,21 +115,12 @@ impl Broadcasts { 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 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(()) } From 935ed9c8e792b31096f5d90d88ebde17ce6bf173 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Tue, 12 May 2026 22:18:10 +0100 Subject: [PATCH 11/12] Remove expected FS events --- .../file-operations/file-operations.test.ts | 4 +- .../src/file-operations/file-operations.ts | 33 +---- frontend/sync-client/src/sync-client.ts | 26 +--- .../src/sync-operations/expected-fs-events.ts | 138 ------------------ .../src/sync-operations/reconciler.ts | 70 +++++++-- .../sync-operations/sync-event-queue.test.ts | 39 +++-- .../src/sync-operations/sync-event-queue.ts | 22 ++- .../sync-client/src/sync-operations/syncer.ts | 26 +++- scripts/clean-up.sh | 2 +- scripts/e2e.sh | 2 +- sync-server/config-e2e.yml | 2 +- sync-server/src/consts.rs | 2 +- sync-server/src/server/update_document.rs | 6 +- 13 files changed, 129 insertions(+), 243 deletions(-) delete mode 100644 frontend/sync-client/src/sync-operations/expected-fs-events.ts 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 44b4fe7e..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 }; } diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index b73bcec9..efd1d5b2 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -9,7 +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"; export class FileOperations { private readonly fs: SafeFileSystemOperations; @@ -18,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); @@ -67,13 +65,7 @@ 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; - } + await this.fs.write(path, this.toNativeLineEndings(newContent)); return path; } @@ -95,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( @@ -165,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` @@ -178,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`); @@ -223,13 +202,7 @@ 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 newPath; } diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index 3a47152e..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,8 +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 syncService: SyncService ) {} public get syncedDocumentCount(): number { @@ -209,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 ); @@ -262,8 +257,7 @@ export class SyncClient { fileChangeNotifier, contentCache, serverConfig, - syncService, - expectedFsEvents + syncService ); logger.info("SyncClient created successfully"); @@ -378,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); } @@ -395,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 @@ -409,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); } @@ -540,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/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 9aadebb4..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 @@ -247,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 () => { 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 66dcf1a4..5fb861e0 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.ts @@ -145,9 +145,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); @@ -267,6 +267,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, @@ -279,7 +289,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); @@ -796,7 +806,7 @@ 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 && @@ -849,7 +859,7 @@ 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 && diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index c51e7394..14a990d0 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}` diff --git a/scripts/clean-up.sh b/scripts/clean-up.sh index 267a1019..dcf400bb 100755 --- a/scripts/clean-up.sh +++ b/scripts/clean-up.sh @@ -1,4 +1,4 @@ #!/bin/bash -rm -rf /tmp/vaultlink-e2e-databases +rm -rf /host/tmp/vaultlink-e2e-databases rm -rf logs diff --git a/scripts/e2e.sh b/scripts/e2e.sh index abc3dcd2..7ab8d90c 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -31,7 +31,7 @@ sleep 1 # Clean databases (uses tmpfs via /dev/shm for zero disk I/O) echo "Cleaning databases..." -rm -rf /tmp/databases +rm -rf /host/tmp/vaultlink-e2e-databases # Start the server in the background echo "Starting server..." diff --git a/sync-server/config-e2e.yml b/sync-server/config-e2e.yml index 9ba68682..03b860b7 100644 --- a/sync-server/config-e2e.yml +++ b/sync-server/config-e2e.yml @@ -1,5 +1,5 @@ database: - databases_directory_path: /tmp/databases + databases_directory_path: /host/tmp/vaultlink-e2e-databases max_connections_per_vault: 8 cursor_timeout: 1m server: diff --git a/sync-server/src/consts.rs b/sync-server/src/consts.rs index b92fb139..a88fe5ff 100644 --- a/sync-server/src/consts.rs +++ b/sync-server/src/consts.rs @@ -23,7 +23,7 @@ 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. +/// 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/server/update_document.rs b/sync-server/src/server/update_document.rs index a071d1e9..7977c644 100644 --- a/sync-server/src/server/update_document.rs +++ b/sync-server/src/server/update_document.rs @@ -244,7 +244,8 @@ pub async fn update_document( let content_clone = content.clone(); let merged = tokio::task::spawn_blocking(move || { - let merged = reconcile( + + reconcile( &parent_owned, &latest_owned.into(), &new_owned.into(), @@ -252,8 +253,7 @@ pub async fn update_document( ) .apply() .text() - .into_bytes(); - merged + .into_bytes() }) .await .map_err(|e| server_error(anyhow::anyhow!("Reconcile task failed: {e}")))?; From 36695e93619876960ffc0ac9ecde729b2da75552 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Thu, 14 May 2026 20:58:14 +0100 Subject: [PATCH 12/12] Fix deletions --- .../src/sync-operations/sync-event-queue.ts | 24 +++++++++++++++++++ .../sync-client/src/sync-operations/syncer.ts | 17 +++++++++++++ frontend/test-client/src/cli.ts | 2 +- scripts/e2e.sh | 17 +------------ 4 files changed, 43 insertions(+), 17 deletions(-) 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 5fb861e0..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, @@ -605,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 @@ -634,6 +653,10 @@ export class SyncEventQueue { 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 @@ -739,6 +762,7 @@ export class SyncEventQueue { this.byDocId.clear(); this._byLocalPath.clear(); this._pendingServerDeletes.clear(); + this._deletedDocumentIds.clear(); this._lastSeenUpdateId.reset(); await this.save(); } diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 14a990d0..adc34217 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -876,6 +876,23 @@ export class Syncer { return this.processRemoteUpdate(trackedRecord, remoteVersion); } + // 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( + `Discarding stale remote update for tombstoned ${remoteVersion.documentId} at ${remoteVersion.relativePath}` + ); + return; + } + return this.processRemoteCreateForNewDocument(remoteVersion); } 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/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