Repository navigation
fix(desktop): atomic writes for all filesystem-backed stores #354
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ export type TauriApis = { | |
| exists: (path: string) => Promise<boolean>; | ||
| readDir: (path: string) => Promise<{ name?: string; isDirectory?: boolean }[]>; | ||
| remove: (path: string, opts?: { recursive?: boolean }) => Promise<void>; | ||
| rename: (oldPath: string, newPath: string) => Promise<void>; | ||
| open: (opts?: Record<string, unknown>) => Promise<string | null>; | ||
| save: (opts?: Record<string, unknown>) => Promise<string | null>; | ||
| appDataDir: () => Promise<string>; | ||
|
|
@@ -44,6 +45,7 @@ export async function loadTauriApis(): Promise<TauriApis> { | |
| exists: fsModule.exists, | ||
| readDir: fsModule.readDir as TauriApis['readDir'], | ||
| remove: fsModule.remove, | ||
| rename: fsModule.rename, | ||
| open: dialogModule.open as TauriApis['open'], | ||
| save: dialogModule.save as TauriApis['save'], | ||
| appDataDir: pathModule.appDataDir, | ||
|
|
@@ -78,6 +80,120 @@ export async function retryFs<T>(fn: () => Promise<T>, retries = 2, delayMs = 50 | |
| throw lastError; | ||
| } | ||
|
|
||
| // --- Atomic writes (write-temp-then-rename) --- | ||
| // QNBS-v3: write-temp-then-rename replaces prior direct writeTextFile/writeFile calls to the final path (crash/power-loss mid-write could truncate it); rename() replaces an existing destination per @tauri-apps/plugin-fs 2.5.1's own dist-js/index.d.ts docs, so readers only ever see the old or new complete file, never a partial one. | ||
|
|
||
| // QNBS-v3: crypto.randomUUID() is unsupported on some older WebKit still within this app's declared minimumSystemVersion — same feature-detected fallback as createMigrationOperationId() in encryptionMigrationOrchestrator.ts. | ||
| function createTempSuffix(): string { | ||
| if (typeof crypto.randomUUID === 'function') return crypto.randomUUID(); | ||
| const bytes = crypto.getRandomValues(new Uint8Array(16)); | ||
| return Array.from(bytes, (byte) => byte.toString(16).padStart(2, '0')).join(''); | ||
| } | ||
|
|
||
| // QNBS-v3: tracks temp files currently mid-write so cleanupOrphanedTempFiles' startup sweep never | ||
| // deletes one out from under an active save — a sweep that races a save started right after | ||
| // initialize() would otherwise see a brand-new (not orphaned) .tmp-* file and remove it. | ||
|
qnbs marked this conversation as resolved.
Outdated
|
||
| const activeTempPaths = new Set<string>(); | ||
|
|
||
| interface QueuedWrite { | ||
| fn: () => Promise<void>; | ||
| waiters: Array<{ resolve: () => void; reject: (reason: unknown) => void }>; | ||
| } | ||
|
|
||
| // QNBS-v3: per-path write coordination — at most one write in flight per path (serializes rename | ||
| // order so an older/slower save can never overwrite a newer one) plus at most one queued "next" | ||
| // write; a write arriving while one is already queued (not yet started) supersedes it in place | ||
| // instead of appending another link, so a burst of same-path saves (e.g. rapid autosave retries | ||
| // on a slow/locked disk) can't grow an unbounded promise chain or write every stale intermediate | ||
| // version — only the currently-running write and the single latest queued one ever execute. | ||
| const inFlightPaths = new Set<string>(); | ||
| const nextWrite = new Map<string, QueuedWrite>(); | ||
|
|
||
| function enqueueWrite(path: string, fn: () => Promise<void>): Promise<void> { | ||
| return new Promise<void>((resolve, reject) => { | ||
| const waiter = { resolve, reject }; | ||
| const queued = nextWrite.get(path); | ||
| if (queued) { | ||
| queued.fn = fn; | ||
| queued.waiters.push(waiter); | ||
| return; | ||
|
There was a problem hiding this comment. Suggestion: When multiple writes to the same path arrive while one is in flight, replacing Severity Level: Major
|
||
| } | ||
| if (inFlightPaths.has(path)) { | ||
| nextWrite.set(path, { fn, waiters: [waiter] }); | ||
| return; | ||
| } | ||
| void runQueuedWrite(path, fn, [waiter]); | ||
| }); | ||
| } | ||
|
|
||
| async function runQueuedWrite( | ||
| path: string, | ||
| fn: () => Promise<void>, | ||
| waiters: Array<{ resolve: () => void; reject: (reason: unknown) => void }>, | ||
| ): Promise<void> { | ||
| inFlightPaths.add(path); | ||
| try { | ||
| await fn(); | ||
| for (const waiter of waiters) waiter.resolve(); | ||
| } catch (err) { | ||
| for (const waiter of waiters) waiter.reject(err); | ||
| } finally { | ||
| inFlightPaths.delete(path); | ||
| } | ||
| const queued = nextWrite.get(path); | ||
| if (queued) { | ||
| nextWrite.delete(path); | ||
| void runQueuedWrite(path, queued.fn, queued.waiters); | ||
| } | ||
| } | ||
|
|
||
| async function atomicRename(apis: TauriApis, tmpPath: string, finalPath: string): Promise<void> { | ||
| try { | ||
| await retryFs(() => apis.rename(tmpPath, finalPath)); | ||
|
qnbs marked this conversation as resolved.
|
||
| } catch (err) { | ||
| // Best-effort cleanup of the orphaned temp file; the original write error is what matters. | ||
|
qodo-code-review[bot] marked this conversation as resolved.
|
||
| await apis.remove(tmpPath).catch(() => {}); | ||
| throw err; | ||
| } | ||
| } | ||
|
|
||
| // QNBS-v3: cleans up the orphaned temp file on a WRITE failure too, not just on rename failure — otherwise a failed temp write (e.g. disk full mid-write) leaves a uniquely-named orphan behind. | ||
| async function writeThenRename( | ||
| apis: TauriApis, | ||
| tmpPath: string, | ||
| finalPath: string, | ||
| write: () => Promise<void>, | ||
| ): Promise<void> { | ||
| activeTempPaths.add(tmpPath); | ||
| try { | ||
| try { | ||
| await write(); | ||
| } catch (err) { | ||
| await apis.remove(tmpPath).catch(() => {}); | ||
| throw err; | ||
| } | ||
| await atomicRename(apis, tmpPath, finalPath); | ||
| } finally { | ||
| activeTempPaths.delete(tmpPath); | ||
| } | ||
| } | ||
|
|
||
| export function writeTextFileAtomic(apis: TauriApis, path: string, content: string): Promise<void> { | ||
| return enqueueWrite(path, () => { | ||
| const tmpPath = `${path}.tmp-${createTempSuffix()}`; | ||
|
qnbs marked this conversation as resolved.
|
||
| return writeThenRename(apis, tmpPath, path, () => | ||
| retryFs(() => apis.writeTextFile(tmpPath, content)), | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| export function writeFileAtomic(apis: TauriApis, path: string, data: Uint8Array): Promise<void> { | ||
| return enqueueWrite(path, () => { | ||
| const tmpPath = `${path}.tmp-${createTempSuffix()}`; | ||
| return writeThenRename(apis, tmpPath, path, () => retryFs(() => apis.writeFile(tmpPath, data))); | ||
| }); | ||
| } | ||
|
|
||
| // --- LZ-String compression (mirrors dbService threshold and prefix) --- | ||
|
|
||
| const COMPRESS_THRESHOLD = 10_240; | ||
|
|
@@ -98,15 +214,7 @@ export function decompressData<T>(raw: string): T { | |
| } | ||
|
|
||
| // --- Crypto helpers --- | ||
| // QNBS-v3: PBKDF2-SHA-256 (600k iter, OWASP 2024 minimum) + random 32-byte salt, mirroring storageEncryptionService.ts#deriveKey, replacing a prior unsalted-SHA-256-of-public-material scheme (F-05/F-06); legacy (no `salt` field) payloads are treated as unreadable, not migrated — see decryptText below. | ||
|
|
||
| const PBKDF2_ITERATIONS = 600_000; // OWASP 2024 minimum for PBKDF2-HMAC-SHA-256 | ||
| const SALT_BYTE_LENGTH = 32; | ||
|
|
@@ -119,9 +227,7 @@ function bytesToBase64(bytes: Uint8Array): string { | |
| return btoa(bin); | ||
| } | ||
|
|
||
| // QNBS-v3: explicit Uint8Array<ArrayBuffer> return type — a bare `Uint8Array` annotation widens to `Uint8Array<ArrayBufferLike>` (includes SharedArrayBuffer), rejected by crypto.subtle as a BufferSource; same pattern as libraryBackupService.ts#copyToFixedBuffer. | ||
| function base64ToBytes(b64: string): Uint8Array<ArrayBuffer> { | ||
| const bin = atob(b64); | ||
| const out = new Uint8Array(bin.length); | ||
|
|
@@ -180,8 +286,7 @@ export async function decryptText( | |
| secretMaterial: string, | ||
| ): Promise<string> { | ||
| if (!payload.salt) { | ||
| // QNBS-v3: pre-2026-07-29 payloads have no salt field (unsalted single-SHA-256 scheme, F-05) — not migrated by design (locked decision); the caller treats this as "no key available". | ||
| throw new Error('Legacy unsalted key payload is no longer supported; re-enter the API key.'); | ||
| } | ||
| const salt = base64ToBytes(payload.salt); | ||
|
|
@@ -228,20 +333,55 @@ export function countProjectWords(projectData: unknown): number { | |
|
|
||
| // --- Base class: Tauri path resolution --- | ||
|
|
||
| // QNBS-v3: both atomicRename's and writeThenRename's cleanup only run when a JS promise actually rejects — a process kill (crash/power-loss) mid-write stops execution before either handler runs, leaving a uniquely-named `.tmp-*` orphan with no in-session path to reclaim it. Swept once at startup below. | ||
| const TEMP_FILE_PATTERN = /\.tmp-[0-9a-f-]+$/i; | ||
|
|
||
| export async function cleanupOrphanedTempFiles( | ||
| apis: TauriApis, | ||
| dir: string, | ||
| depth = 0, | ||
| ): Promise<void> { | ||
| if (depth > 6) return; // guard against an unexpectedly deep tree | ||
| let entries: { name?: string; isDirectory?: boolean }[]; | ||
| try { | ||
| entries = await apis.readDir(dir); | ||
| } catch { | ||
| return; // directory may not exist yet on a fresh install — nothing to clean up | ||
| } | ||
| await Promise.all( | ||
| entries.map(async (entry) => { | ||
| if (!entry.name) return; | ||
| const entryPath = await apis.join(dir, entry.name); | ||
| if (entry.isDirectory) { | ||
| await cleanupOrphanedTempFiles(apis, entryPath, depth + 1); | ||
| return; | ||
| } | ||
| if (TEMP_FILE_PATTERN.test(entry.name) && !activeTempPaths.has(entryPath)) { | ||
| await apis.remove(entryPath).catch(() => {}); | ||
| } | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| export class FsCore { | ||
| protected appDataPath: string | null = null; | ||
| protected lastAutoSnapshotTime = Date.now(); | ||
| protected readonly AUTO_SNAPSHOT_INTERVAL = 5 * 60 * 1000; // 5 minutes | ||
| protected readonly MAX_AUTO_SNAPSHOTS = 20; | ||
|
|
||
| async initialize(): Promise<void> { | ||
| let apis: TauriApis; | ||
| try { | ||
| apis = await loadTauriApis(); | ||
| this.appDataPath = await apis.appDataDir(); | ||
| } catch (error) { | ||
| logger.error('Failed to get app data directory:', error); | ||
| throw error; | ||
| } | ||
| // Fire-and-forget: never delays the caller waiting on this initialize() call. | ||
| cleanupOrphanedTempFiles(apis, this.appDataPath).catch((error) => { | ||
|
qnbs marked this conversation as resolved.
Outdated
|
||
| logger.warn('Failed to clean up orphaned temp files:', error); | ||
| }); | ||
| } | ||
|
|
||
| protected async ensureAppDataPath(): Promise<string> { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: These two independent atomic replacements do not provide an atomic commit for a binder asset. Concurrent saves, or a reader running between the two operations, can combine the new binary with old metadata or old binary with new metadata, causing
byteSizeand the returned payload to describe different versions. Publish a versioned asset directory or commit metadata only after both files are ready, and have readers resolve a single committed version. [race condition]Severity Level: Major⚠️
Prompt for AI Agent 🤖