diff --git a/CHANGELOG.md b/CHANGELOG.md index e943880c..3afcdb19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,42 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Atomic writes for desktop filesystem storage.** Every `services/fs/*Store.ts` writer + (project, active-project marker, settings, API keys, snapshots, Codex, RAG vectors, binder + assets, images) previously wrote directly to its final path — a crash or power loss mid-write + could leave the file truncated/corrupted with no recovery path. New `writeTextFileAtomic`/ + `writeFileAtomic` helpers (`services/fs/fsCore.ts`) write to a temp sibling first, then + atomically rename over the final path, so a reader only ever sees the old complete file or the + new complete file, never a partial one. Also fixes a latent capability gap: `fs:allow-read-file`/ + `fs:allow-write-file`/`fs:allow-rename` were never declared in + `src-tauri/capabilities/default.json`, even though `assetFsStore.ts` already called the binary + `readFile`/`writeFile` commands they gate. **Review-loop follow-up fixes to the same change:** a + failed temp-file write (not just a failed rename) now also cleans up its orphaned temp file; + same-path writes now serialize in call order via a per-path queue, closing a race where two + overlapping saves could have an older write's rename land after a newer one and silently roll + the file back; `crypto.randomUUID()` now has a `getRandomValues()`-based fallback for WebKit + versions that predate it, matching the existing pattern in + `encryptionMigrationOrchestrator.ts#createMigrationOperationId`. A new startup sweep + (`cleanupOrphanedTempFiles`, run once per session, fire-and-forget) recursively removes + `.tmp-*` siblings left behind by a write interrupted by an actual process kill (crash/power + loss) rather than a caught JS error — neither `atomicRename`'s nor the temp-write failure + handler's in-session cleanup can ever see that case, since execution stops before either runs. + **Second round of review-loop follow-up fixes:** the startup sweep being fire-and-forget meant a + save started immediately after `initialize()` could create a brand-new (legitimate, not + orphaned) temp file that the still-running sweep would then delete out from under it — a + module-level `activeTempPaths` set, populated for the duration of each write, now tells the + sweep to skip any temp file currently being written rather than delaying every session's + startup by awaiting the sweep. The per-path write queue also didn't coalesce: a burst of + same-path writes arriving faster than disk I/O completes (e.g. rapid autosave retries on a slow + or temporarily locked disk) appended every call to a growing promise chain, writing every stale + intermediate version to disk instead of just the latest one. The queue now keeps at most the + currently-running write and a single latest-queued write per path — a write arriving while + another is already queued (not yet started) supersedes it in place instead of appending another + link, bounding both memory and wasted I/O regardless of how many same-path writes arrive in a + burst. + ## [1.27.0] — 2026-08-13 ### Added diff --git a/services/fs/assetFsStore.ts b/services/fs/assetFsStore.ts index 32ce52ca..996b1b12 100644 --- a/services/fs/assetFsStore.ts +++ b/services/fs/assetFsStore.ts @@ -6,10 +6,120 @@ import { logger } from '../logger'; import type { BinderAssetMeta, BinderAssetPayload } from '../storageBackend'; -import { retryFs, sanitizePathSegment } from './fsCore'; +import { retryFs, sanitizePathSegment, writeFileAtomic, writeTextFileAtomic } from './fsCore'; import { FsSnapshotStore } from './snapshotFsStore'; +interface BinderAssetManifest { + version: 1; + dataFile: string; + meta: BinderAssetMeta; +} + +function isBinderAssetMeta(value: unknown): value is BinderAssetMeta { + if (!value || typeof value !== 'object') return false; + const candidate = value as Partial; + return ( + typeof candidate.mimeType === 'string' && + typeof candidate.originalFileName === 'string' && + typeof candidate.byteSize === 'number' && + Number.isFinite(candidate.byteSize) && + candidate.byteSize >= 0 + ); +} + +function createBinderRevision(): string { + if (typeof crypto.randomUUID === 'function') return crypto.randomUUID(); + return Array.from(crypto.getRandomValues(new Uint8Array(16)), (byte) => + byte.toString(16).padStart(2, '0'), + ).join(''); +} + +function isBinderAssetManifest(value: unknown): value is BinderAssetManifest { + if (!value || typeof value !== 'object') return false; + const candidate = value as Partial; + return ( + candidate.version === 1 && + typeof candidate.dataFile === 'string' && + isBinderAssetMeta(candidate.meta) + ); +} + +const BINDER_REVISION_FILE_PATTERN = /^(.+)\.([0-9a-f]{32}|[0-9a-f-]{36})\.bin$/i; + export class FsAssetStore extends FsSnapshotStore { + private readonly binderOperationTails = new Map>(); + + private enqueueBinderOperation( + projectId: string, + assetId: string, + operation: () => Promise, + ): Promise { + const key = `${projectId}\u0000${assetId}`; + const previous = this.binderOperationTails.get(key) ?? Promise.resolve(); + const result = previous.catch(() => {}).then(operation); + const tail = result.then( + () => undefined, + () => undefined, + ); + this.binderOperationTails.set(key, tail); + void tail.then(() => { + if (this.binderOperationTails.get(key) === tail) this.binderOperationTails.delete(key); + }); + return result; + } + + override async initialize(): Promise { + await super.initialize(); + await this.cleanupOrphanedBinderRevisions(); + } + + private async cleanupOrphanedBinderRevisions(): Promise { + try { + const apis = await this.getApis(); + const appDataPath = await this.ensureAppDataPath(); + const projectsPath = await apis.join(appDataPath, 'projects'); + if (!(await apis.exists(projectsPath))) return; + const projects = await retryFs(() => apis.readDir(projectsPath)); + for (const project of projects) { + if (!project.name || !project.isDirectory) continue; + const binderPath = await apis.join(projectsPath, project.name, 'binder'); + if (!(await apis.exists(binderPath))) continue; + const entries = await retryFs(() => apis.readDir(binderPath)); + const committedFiles = new Set(); + const protectedAssets = new Set(); + for (const entry of entries) { + const metaName = entry.name; + if (!metaName?.endsWith('.meta.json')) continue; + const safeAsset = metaName.replace(/\.meta\.json$/, ''); + const metaFile = await apis.join(binderPath, metaName); + try { + const raw = JSON.parse(await retryFs(() => apis.readTextFile(metaFile))) as unknown; + if (raw && typeof raw === 'object' && ('version' in raw || 'dataFile' in raw)) { + const manifest = await this.readBinderManifest(apis, metaFile, safeAsset); + if (manifest) committedFiles.add(manifest.dataFile); + else protectedAssets.add(safeAsset); + } + } catch { + protectedAssets.add(safeAsset); + } + } + for (const entry of entries) { + const match = entry.name?.match(BINDER_REVISION_FILE_PATTERN); + if (!match) continue; + const [, safeAsset] = match; + if (!safeAsset || committedFiles.has(entry.name!) || protectedAssets.has(safeAsset)) + continue; + const revisionFile = await apis.join(binderPath, entry.name!); + await retryFs(() => apis.remove(revisionFile)).catch((error) => { + logger.warn('Failed to remove orphaned binder asset revision:', error); + }); + } + } + } catch (error) { + logger.warn('Failed to clean up orphaned binder asset revisions:', error); + } + } + // --- Image Store Methods --- async saveImage(id: string, base64Data: string): Promise { @@ -22,8 +132,8 @@ export class FsAssetStore extends FsSnapshotStore { } const imageFile = await apis.join(imagesPath, `${sanitizePathSegment(id, 'image')}.png`); - const cleanBase64 = base64Data.replace(/^data:image\/png;base64,/, ''); - await retryFs(() => apis.writeTextFile(imageFile, cleanBase64)); + // QNBS-v3: preserve the original data URL so JPEG/WebP uploads keep their MIME type; legacy raw base64 remains readable below. + await writeTextFileAtomic(apis, imageFile, base64Data); } async getImage(id: string): Promise { @@ -41,7 +151,9 @@ export class FsAssetStore extends FsSnapshotStore { } const base64Data = await retryFs(() => apis.readTextFile(imageFile)); - return `data:image/png;base64,${base64Data}`; + return base64Data.startsWith('data:image/') + ? base64Data + : `data:image/png;base64,${base64Data}`; } catch (error) { logger.error('Failed to load image:', error); return null; @@ -75,7 +187,29 @@ export class FsAssetStore extends FsSnapshotStore { const dir = await apis.join(appDataPath, 'projects', safeId, 'binder'); const binFile = await apis.join(dir, `${safeAsset}.bin`); const metaFile = await apis.join(dir, `${safeAsset}.meta.json`); - return { apis, dir, binFile, metaFile }; + return { apis, dir, binFile, metaFile, safeAsset }; + } + + private async readBinderManifest( + apis: Awaited>['apis'], + metaFile: string, + safeAsset: string, + ): Promise { + try { + const parsed = JSON.parse(await retryFs(() => apis.readTextFile(metaFile))) as unknown; + if (!isBinderAssetManifest(parsed)) return null; + if ( + !parsed.dataFile.startsWith(`${safeAsset}.`) || + !parsed.dataFile.endsWith('.bin') || + parsed.dataFile.includes('/') || + parsed.dataFile.includes('\\') + ) { + throw new Error('Binder asset manifest references an invalid data file'); + } + return parsed; + } catch { + return null; + } } async saveBinderAsset( @@ -83,25 +217,66 @@ export class FsAssetStore extends FsSnapshotStore { assetId: string, data: ArrayBuffer, meta: BinderAssetMeta, + ): Promise { + return this.enqueueBinderOperation(projectId, assetId, () => + this.saveBinderAssetLocked(projectId, assetId, data, meta), + ); + } + + private async saveBinderAssetLocked( + projectId: string, + assetId: string, + data: ArrayBuffer, + meta: BinderAssetMeta, ): Promise { const apis = await this.getApis(); - const { dir, binFile, metaFile } = await this.binderAssetPaths(projectId, assetId); + const { dir, binFile, metaFile, safeAsset } = await this.binderAssetPaths(projectId, assetId); if (!(await apis.exists(dir))) await apis.mkdir(dir, { recursive: true }); const metaOut: BinderAssetMeta = { ...meta, byteSize: data.byteLength }; - await retryFs(() => apis.writeFile(binFile, new Uint8Array(data))); - await retryFs(() => apis.writeTextFile(metaFile, JSON.stringify(metaOut))); + const prior = await this.readBinderManifest(apis, metaFile, safeAsset); + const dataFileName = `${safeAsset}.${createBinderRevision()}.bin`; + const dataFile = await apis.join(dir, dataFileName); + await writeFileAtomic(apis, dataFile, new Uint8Array(data)); + // QNBS-v3: publishing this manifest is the binder pair's commit point, so readers never combine new bytes with stale metadata. + try { + await writeTextFileAtomic( + apis, + metaFile, + JSON.stringify({ version: 1, dataFile: dataFileName, meta: metaOut }), + ); + } catch (error) { + // QNBS-v3: the revision is unreachable until its manifest commits, so failed publication must not leak a new binary on every retry. + await retryFs(() => apis.remove(dataFile)).catch((cleanupError) => { + logger.warn('Failed to remove unpublished binder asset revision:', cleanupError); + }); + throw error; + } + if (prior) { + const priorFile = await apis.join(dir, prior.dataFile); + if (priorFile !== dataFile && (await apis.exists(priorFile))) { + await retryFs(() => apis.remove(priorFile)).catch((error) => { + logger.warn('Failed to remove superseded binder asset revision:', error); + }); + } + } else if (await apis.exists(binFile)) { + await retryFs(() => apis.remove(binFile)).catch((error) => { + logger.warn('Failed to remove superseded legacy binder asset:', error); + }); + } } async getBinderAsset(projectId: string, assetId: string): Promise { try { const apis = await this.getApis(); - const { binFile, metaFile } = await this.binderAssetPaths(projectId, assetId); - if (!(await apis.exists(binFile)) || !(await apis.exists(metaFile))) return null; - const [bytes, metaRaw] = await Promise.all([ - retryFs(() => apis.readFile(binFile)), - retryFs(() => apis.readTextFile(metaFile)), - ]); - const meta = JSON.parse(metaRaw) as BinderAssetMeta; + const { binFile, metaFile, dir, safeAsset } = await this.binderAssetPaths(projectId, assetId); + if (!(await apis.exists(metaFile))) return null; + const manifest = await this.readBinderManifest(apis, metaFile, safeAsset); + const dataFile = manifest ? await apis.join(dir, manifest.dataFile) : binFile; + if (!(await apis.exists(dataFile))) return null; + const bytes = await retryFs(() => apis.readFile(dataFile)); + const meta = manifest + ? manifest.meta + : (JSON.parse(await retryFs(() => apis.readTextFile(metaFile))) as BinderAssetMeta); const copy = bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength); return { data: copy, meta }; } catch (error) { @@ -111,10 +286,20 @@ export class FsAssetStore extends FsSnapshotStore { } async deleteBinderAsset(projectId: string, assetId: string): Promise { + return this.enqueueBinderOperation(projectId, assetId, () => + this.deleteBinderAssetLocked(projectId, assetId), + ); + } + + private async deleteBinderAssetLocked(projectId: string, assetId: string): Promise { try { const apis = await this.getApis(); - const { binFile, metaFile } = await this.binderAssetPaths(projectId, assetId); - if (await apis.exists(binFile)) await retryFs(() => apis.remove(binFile)); + const { binFile, metaFile, dir, safeAsset } = await this.binderAssetPaths(projectId, assetId); + const manifest = await this.readBinderManifest(apis, metaFile, safeAsset); + const dataFile = manifest ? await apis.join(dir, manifest.dataFile) : binFile; + if (await apis.exists(dataFile)) await retryFs(() => apis.remove(dataFile)); + if (dataFile !== binFile && (await apis.exists(binFile))) + await retryFs(() => apis.remove(binFile)); if (await apis.exists(metaFile)) await retryFs(() => apis.remove(metaFile)); } catch (error) { logger.warn('deleteBinderAsset failed:', error); diff --git a/services/fs/codexFsStore.ts b/services/fs/codexFsStore.ts index 2176506a..ce7c0f75 100644 --- a/services/fs/codexFsStore.ts +++ b/services/fs/codexFsStore.ts @@ -6,7 +6,13 @@ import type { StoryCodex } from '../../types'; import { logger } from '../logger'; -import { compressData, decompressData, retryFs, sanitizePathSegment } from './fsCore'; +import { + compressData, + decompressData, + retryFs, + sanitizePathSegment, + writeTextFileAtomic, +} from './fsCore'; import { FsSettingsStore } from './settingsFsStore'; export class FsCodexStore extends FsSettingsStore { @@ -19,7 +25,8 @@ export class FsCodexStore extends FsSettingsStore { const codexDir = await apis.join(appDataPath, 'projects', safeId, 'codex'); if (!(await apis.exists(codexDir))) await apis.mkdir(codexDir, { recursive: true }); const codexFile = await apis.join(codexDir, 'codex.snap'); - await retryFs(() => apis.writeTextFile(codexFile, compressData(codex))); + // QNBS-v3: atomic write — a crash/power-loss mid-write must never leave codex.snap truncated. + await writeTextFileAtomic(apis, codexFile, compressData(codex)); } async getStoryCodex(projectId: string): Promise { @@ -58,7 +65,8 @@ export class FsCodexStore extends FsSettingsStore { const codexDir = await apis.join(appDataPath, 'projects', safeId, 'codex'); if (!(await apis.exists(codexDir))) await apis.mkdir(codexDir, { recursive: true }); const vectorsFile = await apis.join(codexDir, 'vectors.snap'); - await retryFs(() => apis.writeTextFile(vectorsFile, compressData(vectors))); + // QNBS-v3: atomic write — same crash-safety rationale as saveStoryCodex above. + await writeTextFileAtomic(apis, vectorsFile, compressData(vectors)); } async getRagVectors(projectId: string): Promise { diff --git a/services/fs/fsCore.ts b/services/fs/fsCore.ts index 9f09f012..d9180566 100644 --- a/services/fs/fsCore.ts +++ b/services/fs/fsCore.ts @@ -5,6 +5,7 @@ import LZString from 'lz-string'; import { logger } from '../logger'; +import { isTauriRuntime } from '../tauriRuntime'; // Dynamic imports for Tauri v2 plugin APIs — fail gracefully in browser export type TauriApis = { @@ -16,11 +17,16 @@ export type TauriApis = { exists: (path: string) => Promise; readDir: (path: string) => Promise<{ name?: string; isDirectory?: boolean }[]>; remove: (path: string, opts?: { recursive?: boolean }) => Promise; + rename: (oldPath: string, newPath: string) => Promise; open: (opts?: Record) => Promise; save: (opts?: Record) => Promise; appDataDir: () => Promise; join: (...parts: string[]) => Promise; - invoke: (cmd: string, args?: Record) => Promise; + invoke: ( + cmd: string, + args?: Record | Uint8Array, + options?: { headers: HeadersInit }, + ) => Promise; }; let tauriApis: TauriApis | null = null; @@ -44,6 +50,7 @@ export async function loadTauriApis(): Promise { 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 +85,102 @@ export async function retryFs(fn: () => Promise, 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 mid-write so the startup sweep never deletes one racing an active save right after initialize(). +const activeTempPaths = new Set(); + +// QNBS-v3: each caller owns a real ordered write result; coalescing pending writes made an earlier caller report success after a later payload was persisted instead. +const writeTails = new Map>(); +const pendingWriteCounts = new Map(); +const MAX_PENDING_WRITES_PER_PATH = 8; + +function enqueueWrite(path: string, fn: () => Promise): Promise { + const pendingCount = pendingWriteCounts.get(path) ?? 0; + if (pendingCount >= MAX_PENDING_WRITES_PER_PATH) { + return Promise.reject(new Error(`Atomic write backlog is full for ${path}`)); + } + pendingWriteCounts.set(path, pendingCount + 1); + const previous = writeTails.get(path) ?? Promise.resolve(); + const operation = previous.catch(() => {}).then(fn); + const tail = operation.catch(() => {}); + writeTails.set(path, tail); + void tail.then(() => { + const remaining = (pendingWriteCounts.get(path) ?? 1) - 1; + if (remaining > 0) pendingWriteCounts.set(path, remaining); + else pendingWriteCounts.delete(path); + if (writeTails.get(path) === tail) writeTails.delete(path); + }); + return operation; +} + +async function atomicRename(apis: TauriApis, tmpPath: string, finalPath: string): Promise { + try { + await retryFs(() => apis.rename(tmpPath, finalPath)); + } catch (err) { + // Best-effort cleanup of the orphaned temp file; the original write error is what matters. + 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, +): Promise { + 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 { + return enqueueWrite(path, () => { + if (isTauriRuntime()) { + return retryFs(() => writeFileDurablyInTauri(path, new TextEncoder().encode(content))); + } + const tmpPath = `${path}.tmp-${createTempSuffix()}`; + return writeThenRename(apis, tmpPath, path, () => + retryFs(() => apis.writeTextFile(tmpPath, content)), + ); + }); +} + +export function writeFileAtomic(apis: TauriApis, path: string, data: Uint8Array): Promise { + return enqueueWrite(path, () => { + if (isTauriRuntime()) return retryFs(() => writeFileDurablyInTauri(path, data)); + const tmpPath = `${path}.tmp-${createTempSuffix()}`; + return writeThenRename(apis, tmpPath, path, () => retryFs(() => apis.writeFile(tmpPath, data))); + }); +} + +async function writeFileDurablyInTauri(path: string, data: Uint8Array): Promise { + const { invoke } = await import('@tauri-apps/api/core'); + // QNBS-v3: raw IPC avoids expanding every binary byte into a JavaScript number-array element. + await invoke('worldscript_atomic_write', data, { + headers: { 'x-worldscript-path': encodeURIComponent(path) }, + }); +} + // --- LZ-String compression (mirrors dbService threshold and prefix) --- const COMPRESS_THRESHOLD = 10_240; @@ -98,15 +201,7 @@ export function decompressData(raw: string): T { } // --- Crypto helpers --- -// QNBS-v3: PBKDF2-SHA-256 (600k iter, OWASP 2024 minimum) + random 32-byte salt per encryption, -// mirroring services/storage/storageEncryptionService.ts#deriveKey. The prior scheme derived the -// key from a single unsalted SHA-256 digest of publicly-derivable material -// (`${appDataPath}|${provider}|WorldScriptStudio|v1` — anyone who can read the encrypted file -// already knows its own parent path and the provider from the filename), making it obfuscation, -// not encryption (F-05/F-06). No migration path for pre-existing `*_key.enc.json` files: a legacy -// payload (no `salt` field) is treated as unreadable — see decryptText below and -// FsSettingsStore#getApiKey, which already returns null on any decrypt failure so the caller -// naturally re-prompts for the key. +// 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 +214,7 @@ function bytesToBase64(bytes: Uint8Array): string { return btoa(bin); } -// QNBS-v3: explicit Uint8Array return type — a bare `Uint8Array` annotation widens to -// `Uint8Array` (includes SharedArrayBuffer), which crypto.subtle rejects as a -// BufferSource. Same pattern as services/libraryBackupService.ts#copyToFixedBuffer. +// QNBS-v3: explicit Uint8Array return type — a bare `Uint8Array` annotation widens to `Uint8Array` (includes SharedArrayBuffer), rejected by crypto.subtle as a BufferSource; same pattern as libraryBackupService.ts#copyToFixedBuffer. function base64ToBytes(b64: string): Uint8Array { const bin = atob(b64); const out = new Uint8Array(bin.length); @@ -180,8 +273,7 @@ export async function decryptText( secretMaterial: string, ): Promise { 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". + // 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,6 +320,36 @@ export function countProjectWords(projectData: unknown): number { // --- Base class: Tauri path resolution --- +// QNBS-v3: JS-generated names are separate from native names so the JavaScript sweep cannot delete a native temp while its native command is still writing it. +const TEMP_FILE_PATTERN = /\.tmp-[0-9a-f-]+$/i; + +export async function cleanupOrphanedTempFiles( + apis: TauriApis, + dir: string, + depth = 0, +): Promise { + 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(); @@ -235,13 +357,20 @@ export class FsCore { protected readonly MAX_AUTO_SNAPSHOTS = 20; async initialize(): Promise { + let apis: TauriApis; try { - const apis = await loadTauriApis(); + apis = await loadTauriApis(); this.appDataPath = await apis.appDataDir(); } catch (error) { logger.error('Failed to get app data directory:', error); throw error; } + await cleanupOrphanedTempFiles(apis, this.appDataPath).catch((error) => { + logger.warn('Failed to clean up orphaned temp files:', error); + }); + await apis.invoke('worldscript_cleanup_atomic_temps').catch((error) => { + logger.warn('Failed to clean up native orphaned temp files:', error); + }); } protected async ensureAppDataPath(): Promise { diff --git a/services/fs/projectFsStore.ts b/services/fs/projectFsStore.ts index cfcb5a08..0f59d538 100644 --- a/services/fs/projectFsStore.ts +++ b/services/fs/projectFsStore.ts @@ -10,7 +10,13 @@ import { logger } from '../logger'; import { parseImportedProjectJson } from '../projectImportSchema'; import { normalizeSaveProjectInputToStoryProject, type SaveProjectInput } from '../storageBackend'; import { FsAssetStore } from './assetFsStore'; -import { compressData, decompressData, retryFs, sanitizePathSegment } from './fsCore'; +import { + compressData, + decompressData, + retryFs, + sanitizePathSegment, + writeTextFileAtomic, +} from './fsCore'; export class FsProjectStore extends FsAssetStore { async saveProject(project: SaveProjectInput): Promise { @@ -36,7 +42,7 @@ export class FsProjectStore extends FsAssetStore { } const projectFile = await apis.join(projectPath, 'project.json'); - await retryFs(() => apis.writeTextFile(projectFile, compressData(flat))); + await writeTextFileAtomic(apis, projectFile, compressData(flat)); // QNBS-v3 (#332): documented best-effort abort — the project data above already saved; a failed marker write only degrades the next cold-boot's project selection, not worth failing this save over. await this.setActiveProjectId(projectId).catch((error) => { logger.warn('Failed to persist active-project marker (project save itself succeeded)', { @@ -54,7 +60,7 @@ export class FsProjectStore extends FsAssetStore { await apis.mkdir(configPath, { recursive: true }); } const markerFile = await apis.join(configPath, 'active-project-id.txt'); - await retryFs(() => apis.writeTextFile(markerFile, projectId)); + await writeTextFileAtomic(apis, markerFile, projectId); } /** diff --git a/services/fs/settingsFsStore.ts b/services/fs/settingsFsStore.ts index 09be76e4..2ea5fff0 100644 --- a/services/fs/settingsFsStore.ts +++ b/services/fs/settingsFsStore.ts @@ -10,7 +10,7 @@ import type { Settings } from '../../types'; import { logger } from '../logger'; import { normalizePersistedSettings } from '../storage/idbProjectStore'; import type { TauriApis } from './fsCore'; -import { decryptText, encryptText, FsCore, retryFs } from './fsCore'; +import { decryptText, encryptText, FsCore, retryFs, writeTextFileAtomic } from './fsCore'; export class FsSettingsStore extends FsCore { async saveSettings(settings: Settings): Promise { @@ -23,7 +23,7 @@ export class FsSettingsStore extends FsCore { } const settingsFile = await apis.join(configPath, 'settings.json'); - await retryFs(() => apis.writeTextFile(settingsFile, JSON.stringify(settings, null, 2))); + await writeTextFileAtomic(apis, settingsFile, JSON.stringify(settings, null, 2)); } async loadSettings(): Promise { @@ -38,9 +38,7 @@ export class FsSettingsStore extends FsCore { const content = await retryFs(() => apis.readTextFile(settingsFile)); const parsed = JSON.parse(content) as Record; - // QNBS-v3: reuse the same normalizer as the IDB path — older desktop settings files can - // predate newer required Settings fields (e.g. writingSurfaceStyle); an unchecked `as - // Settings` cast would let those fall through as undefined at runtime. + // QNBS-v3: reuse the same normalizer as the IDB path — older desktop settings files can predate newer required Settings fields (e.g. writingSurfaceStyle); an unchecked `as Settings` cast would let those fall through as undefined at runtime. return normalizePersistedSettings(parsed); } catch (error) { logger.error('Failed to load settings:', error); @@ -77,13 +75,11 @@ export class FsSettingsStore extends FsCore { `${appDataPath}|${provider}|WorldScriptStudio|v1`, ); const filePath = await apis.join(configPath, `${provider}_key.enc.json`); - await retryFs(() => apis.writeTextFile(filePath, JSON.stringify(encrypted))); + await writeTextFileAtomic(apis, filePath, JSON.stringify(encrypted)); } async getApiKey(provider: string): Promise { - // QNBS-v3: separate outer refs (rather than narrowing `apis`/`keyFile` from the try block) — - // TS's control-flow narrowing doesn't survive into the retryFs() closure below, and the catch - // block below still needs both for the legacy-payload cleanup path. + // QNBS-v3: separate outer refs (rather than narrowing `apis`/`keyFile` from the try block) — TS's control-flow narrowing doesn't survive into the retryFs() closure, and the catch block below still needs both for the legacy-payload cleanup path. let apisForCleanup: TauriApis | undefined; let keyFileForCleanup: string | undefined; try { @@ -97,10 +93,7 @@ export class FsSettingsStore extends FsCore { const payload = JSON.parse(content) as { iv: string; salt?: string; data: string }; return await decryptText(payload, `${appDataPath}|${provider}|WorldScriptStudio|v1`); } catch (error) { - // QNBS-v3: pre-2026-07-29 key files used an unsalted single-SHA-256 derivation (F-05/F-06, - // fixed in fsCore.ts) and are not migrated (locked decision — discard, not migrate). Remove - // the stale file so this doesn't retry on every call, and surface a one-time, explicit - // notification rather than a silent null return that looks identical to "no key ever set". + // QNBS-v3: pre-2026-07-29 key files (unsalted single-SHA-256, F-05/F-06) are not migrated (locked decision — discard); removing the stale file avoids retrying every call, and a one-time notification beats a silent null indistinguishable from "no key ever set". if (apisForCleanup && keyFileForCleanup) { try { await apisForCleanup.remove(keyFileForCleanup); diff --git a/services/fs/snapshotFsStore.ts b/services/fs/snapshotFsStore.ts index 9574fcac..f995271d 100644 --- a/services/fs/snapshotFsStore.ts +++ b/services/fs/snapshotFsStore.ts @@ -6,7 +6,13 @@ import type { ProjectSnapshot } from '../../types'; import { logger } from '../logger'; import { FsCodexStore } from './codexFsStore'; -import { compressData, countProjectWords, decompressData, retryFs } from './fsCore'; +import { + compressData, + countProjectWords, + decompressData, + retryFs, + writeTextFileAtomic, +} from './fsCore'; // Envelope stored in each snapshot file — outer shell is plain JSON, `data` field is compressed. interface SnapshotEnvelope { @@ -36,7 +42,8 @@ export class FsSnapshotStore extends FsCodexStore { data: compressData(data), }; const snapshotFile = await apis.join(snapshotsPath, `${id}.json`); - await retryFs(() => apis.writeTextFile(snapshotFile, JSON.stringify(envelope))); + // QNBS-v3: atomic write — a crash/power-loss mid-write must never leave a snapshot truncated. + await writeTextFileAtomic(apis, snapshotFile, JSON.stringify(envelope)); return id; } diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 83083f08..b252df9a 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -6699,6 +6699,7 @@ dependencies = [ "tauri-plugin-updater", "tauri-plugin-window-state", "tokio", + "windows-sys 0.61.2", ] [[package]] diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 1435ba94..6c7c363a 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -41,6 +41,9 @@ tokio = { version = "1", features = ["sync", "time", "process", "rt-multi-thread candle-core = { version = "0.11", optional = true } candle-nn = { version = "0.11", optional = true } +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem"] } + [profile.release] # QNBS-v3: Aggressive size + speed optimizations for desktop bundles opt-level = 3 diff --git a/src-tauri/capabilities/default.json b/src-tauri/capabilities/default.json index 0f7032d4..9e7c8b93 100644 --- a/src-tauri/capabilities/default.json +++ b/src-tauri/capabilities/default.json @@ -23,6 +23,30 @@ } ] }, + { + "identifier": "fs:allow-read-file", + "allow": [ + { + "path": "$APPDATA/**" + } + ] + }, + { + "identifier": "fs:allow-write-file", + "allow": [ + { + "path": "$APPDATA/**" + } + ] + }, + { + "identifier": "fs:allow-rename", + "allow": [ + { + "path": "$APPDATA/**" + } + ] + }, { "identifier": "fs:allow-mkdir", "allow": [ diff --git a/src-tauri/src/durable_fs.rs b/src-tauri/src/durable_fs.rs new file mode 100644 index 00000000..61bcdd58 --- /dev/null +++ b/src-tauri/src/durable_fs.rs @@ -0,0 +1,354 @@ +use std::{ + fs::{self, File, OpenOptions}, + io::Write, + path::{Path, PathBuf}, + process, + sync::{ + atomic::{AtomicU64, Ordering}, + LazyLock, Mutex, + }, + time::{SystemTime, UNIX_EPOCH}, +}; + +use tauri::{AppHandle, Manager}; + +#[cfg(windows)] +use std::os::windows::ffi::OsStrExt; + +#[cfg(windows)] +use windows_sys::Win32::Storage::FileSystem::{ + MoveFileExW, ReplaceFileW, MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, + REPLACEFILE_WRITE_THROUGH, +}; + +static NATIVE_TEMP_OPERATION_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); +static NATIVE_TEMP_SEQUENCE: AtomicU64 = AtomicU64::new(0); + +fn validated_destination(app: &AppHandle, requested: &str) -> Result<(PathBuf, PathBuf), String> { + let app_data = app + .path() + .app_data_dir() + .map_err(|error| format!("Could not resolve app-data directory: {error}"))? + .canonicalize() + .map_err(|error| format!("Could not canonicalize app-data directory: {error}"))?; + let destination = PathBuf::from(requested); + let parent = destination + .parent() + .ok_or_else(|| "Durable write target has no parent directory".to_owned())? + .canonicalize() + .map_err(|error| format!("Could not canonicalize durable write parent: {error}"))?; + if !parent.starts_with(&app_data) { + return Err("Durable writes are restricted to the application data directory".to_owned()); + } + let file_name = destination + .file_name() + .ok_or_else(|| "Durable write target has no file name".to_owned())?; + Ok((parent.join(file_name), parent)) +} + +fn create_sibling_temp(destination: &Path) -> Result<(PathBuf, File), String> { + let parent = destination + .parent() + .ok_or_else(|| "Durable write target has no parent directory".to_owned())?; + let name = destination + .file_name() + .ok_or_else(|| "Durable write target has no file name".to_owned())? + .to_string_lossy(); + let epoch_nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + let sequence = NATIVE_TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed); + for collision_offset in 0..32_u8 { + let candidate = parent.join(format!( + "{name}.native-tmp-{}-{epoch_nanos}-{}", + process::id(), + sequence + u64::from(collision_offset) + )); + match OpenOptions::new() + .write(true) + .create_new(true) + .open(&candidate) + { + Ok(file) => return Ok((candidate, file)), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(format!("Could not create durable temporary file: {error}")), + } + } + Err("Could not allocate a unique durable temporary file".to_owned()) +} + +#[cfg(unix)] +fn preserve_destination_permissions(destination: &Path, temporary: &Path) -> Result<(), String> { + if let Ok(metadata) = fs::metadata(destination) { + fs::set_permissions(temporary, metadata.permissions()).map_err(|error| { + format!("Could not preserve durable destination permissions: {error}") + })?; + } + Ok(()) +} + +#[cfg(not(unix))] +fn preserve_destination_permissions(_destination: &Path, _temporary: &Path) -> Result<(), String> { + Ok(()) +} + +#[cfg(unix)] +fn sync_parent_directory(parent: &Path) -> Result<(), String> { + File::open(parent) + .and_then(|directory| directory.sync_all()) + .map_err(|error| format!("Could not synchronize durable write directory: {error}")) +} + +#[cfg(not(unix))] +fn sync_parent_directory(_parent: &Path) -> Result<(), String> { + Ok(()) +} + +#[cfg(not(windows))] +fn publish_replacement(temporary: &Path, destination: &Path) -> Result<(), String> { + fs::rename(temporary, destination) + .map_err(|error| format!("Could not publish durable replacement: {error}")) +} + +#[cfg(windows)] +fn publish_replacement(temporary: &Path, destination: &Path) -> Result<(), String> { + let temporary_wide = temporary + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + let destination_wide = destination + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + if destination.exists() { + // QNBS-v3: ReplaceFileW preserves the existing destination's security descriptor when replacing an existing file. + let outcome = unsafe { + ReplaceFileW( + destination_wide.as_ptr(), + temporary_wide.as_ptr(), + std::ptr::null(), + REPLACEFILE_WRITE_THROUGH, + std::ptr::null_mut(), + std::ptr::null_mut(), + ) + }; + if outcome != 0 { + return Ok(()); + } + return Err(format!( + "Could not publish durable replacement: {}", + std::io::Error::last_os_error() + )); + } + // QNBS-v3: MoveFileExW publishes new destinations with write-through semantics; ReplaceFileW above handles existing destinations safely. + let outcome = unsafe { + MoveFileExW( + temporary_wide.as_ptr(), + destination_wide.as_ptr(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + }; + if outcome == 0 { + return Err(format!( + "Could not publish durable replacement: {}", + std::io::Error::last_os_error() + )); + } + Ok(()) +} + +fn durable_write(app: AppHandle, path: String, data: Vec) -> Result<(), String> { + let _operation_guard = NATIVE_TEMP_OPERATION_LOCK + .lock() + .map_err(|_| "Durable-write coordination lock is poisoned".to_owned())?; + let (destination, parent) = validated_destination(&app, &path)?; + let (temporary, mut file) = create_sibling_temp(&destination)?; + let result = (|| -> Result<(), String> { + preserve_destination_permissions(&destination, &temporary)?; + file.write_all(&data) + .map_err(|error| format!("Could not write durable temporary file: {error}"))?; + file.sync_all() + .map_err(|error| format!("Could not synchronize durable temporary file: {error}"))?; + drop(file); + publish_replacement(&temporary, &destination)?; + sync_parent_directory(&parent) + })(); + if result.is_err() { + let _ = fs::remove_file(&temporary); + } + result +} + +fn decode_percent_encoded_path(encoded: &str) -> Result { + let mut bytes = Vec::with_capacity(encoded.len()); + let raw = encoded.as_bytes(); + let mut index = 0; + while index < raw.len() { + if raw[index] == b'%' { + if index + 2 >= raw.len() { + return Err("Durable write path contains an incomplete percent escape".to_owned()); + } + let hex = std::str::from_utf8(&raw[index + 1..index + 3]) + .map_err(|_| "Durable write path contains invalid percent encoding".to_owned())?; + let byte = u8::from_str_radix(hex, 16) + .map_err(|_| "Durable write path contains invalid percent encoding".to_owned())?; + bytes.push(byte); + index += 3; + } else { + bytes.push(raw[index]); + index += 1; + } + } + String::from_utf8(bytes).map_err(|_| "Durable write path is not valid UTF-8".to_owned()) +} + +fn is_native_temp_file_name(name: &std::ffi::OsStr) -> bool { + let name = name.to_string_lossy(); + let Some((_, suffix)) = name.rsplit_once(".native-tmp-") else { + return false; + }; + let mut segments = suffix.split('-'); + let (Some(process_id), Some(epoch_nanos), Some(sequence), None) = ( + segments.next(), + segments.next(), + segments.next(), + segments.next(), + ) else { + return false; + }; + !process_id.is_empty() + && !epoch_nanos.is_empty() + && !sequence.is_empty() + && process_id.bytes().all(|byte| byte.is_ascii_digit()) + && epoch_nanos.bytes().all(|byte| byte.is_ascii_digit()) + && sequence.bytes().all(|byte| byte.is_ascii_digit()) +} + +#[tauri::command] +pub async fn worldscript_atomic_write( + app: AppHandle, + request: tauri::ipc::Request<'_>, +) -> Result<(), String> { + let encoded_path = request + .headers() + .get("x-worldscript-path") + .ok_or_else(|| "Durable write is missing its target path".to_owned())? + .to_str() + .map_err(|_| "Durable write target path header is invalid".to_owned())?; + let path = decode_percent_encoded_path(encoded_path)?; + let data = match request.body() { + tauri::ipc::InvokeBody::Raw(data) => data.clone(), + tauri::ipc::InvokeBody::Json(_) => { + return Err("Durable write requires a raw binary IPC body".to_owned()) + } + }; + tauri::async_runtime::spawn_blocking(move || durable_write(app, path, data)) + .await + .map_err(|error| format!("Durable write task failed: {error}"))? +} + +fn cleanup_native_temp_files(dir: &Path, depth: usize) -> Result<(), String> { + if depth > 6 || !dir.exists() { + return Ok(()); + } + for entry in fs::read_dir(dir) + .map_err(|error| format!("Could not read durable temp directory: {error}"))? + { + let entry = + entry.map_err(|error| format!("Could not inspect durable temp entry: {error}"))?; + let path = entry.path(); + let file_type = entry + .file_type() + .map_err(|error| format!("Could not inspect durable temp type: {error}"))?; + if file_type.is_dir() { + cleanup_native_temp_files(&path, depth + 1)?; + } else if file_type.is_file() && path.file_name().is_some_and(is_native_temp_file_name) { + fs::remove_file(&path) + .map_err(|error| format!("Could not remove native orphaned temp file: {error}"))?; + } + } + Ok(()) +} + +#[tauri::command] +pub async fn worldscript_cleanup_atomic_temps(app: AppHandle) -> Result<(), String> { + tauri::async_runtime::spawn_blocking(move || { + let _operation_guard = NATIVE_TEMP_OPERATION_LOCK + .lock() + .map_err(|_| "Durable-write coordination lock is poisoned".to_owned())?; + let app_data = app + .path() + .app_data_dir() + .map_err(|error| format!("Could not resolve app-data directory: {error}"))?; + cleanup_native_temp_files(&app_data, 0) + }) + .await + .map_err(|error| format!("Native temp cleanup task failed: {error}"))? +} + +#[cfg(test)] +mod tests { + use super::{ + create_sibling_temp, is_native_temp_file_name, publish_replacement, sync_parent_directory, + }; + use std::{ + fs, + io::Write, + path::PathBuf, + process, + time::{SystemTime, UNIX_EPOCH}, + }; + + fn temporary_directory() -> PathBuf { + let epoch_nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("test clock should be after the Unix epoch") + .as_nanos(); + std::env::temp_dir().join(format!( + "worldscript-durable-fs-{}-{epoch_nanos}", + process::id() + )) + } + + #[test] + fn replacement_preserves_only_the_complete_new_content() { + let parent = temporary_directory(); + fs::create_dir_all(&parent).expect("test directory should be created"); + let destination = parent.join("project.json"); + fs::write(&destination, b"old content").expect("old file should be written"); + + let (temporary, mut file) = + create_sibling_temp(&destination).expect("temp file should open"); + file.write_all(b"new content") + .expect("new content should be written"); + file.sync_all().expect("temp file should synchronize"); + drop(file); + + // QNBS-v3: exercises replacement of an existing destination, the platform-sensitive part of the durability contract. + publish_replacement(&temporary, &destination).expect("replacement should publish"); + sync_parent_directory(&parent).expect("parent directory should synchronize when supported"); + + assert_eq!( + fs::read(&destination).expect("published file should read"), + b"new content" + ); + assert!(!temporary.exists()); + fs::remove_dir_all(parent).expect("test directory should be removed"); + } + + #[test] + fn native_temp_cleanup_matches_only_generated_name_shape() { + assert!(is_native_temp_file_name( + "project.json.native-tmp-42-1723560000000000000-0".as_ref() + )); + assert!(!is_native_temp_file_name( + "notes.native-tmp-user-content.bin".as_ref() + )); + assert!(!is_native_temp_file_name( + "project.json.native-tmp-42-100-0-copy".as_ref() + )); + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index c4683af3..afd823dd 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1,4 +1,5 @@ mod commands; +mod durable_fs; mod lora; mod pandoc; @@ -158,6 +159,8 @@ pub fn run() { lora::set_lora_python_path, commands::task_supervisor::worldscript_task_supervisor_ping, commands::task_supervisor::worldscript_task_supervisor_submit, + durable_fs::worldscript_atomic_write, + durable_fs::worldscript_cleanup_atomic_temps, ]) .setup(|app| { if cfg!(debug_assertions) { diff --git a/tests/unit/services/fs/fsCore.test.ts b/tests/unit/services/fs/fsCore.test.ts index 284d2194..87234ea7 100644 --- a/tests/unit/services/fs/fsCore.test.ts +++ b/tests/unit/services/fs/fsCore.test.ts @@ -4,8 +4,22 @@ * sanitization, and word counting — no Tauri APIs required. */ -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const h = vi.hoisted(() => ({ + isTauri: false, + invoke: vi.fn(), +})); + +vi.mock('../../../../services/tauriRuntime', () => ({ + isTauriRuntime: () => h.isTauri, +})); + +vi.mock('@tauri-apps/api/core', () => ({ invoke: h.invoke })); + +import type { TauriApis } from '../../../../services/fs/fsCore'; import { + cleanupOrphanedTempFiles, compressData, countProjectWords, decompressData, @@ -13,8 +27,65 @@ import { encryptText, retryFs, sanitizePathSegment, + writeFileAtomic, + writeTextFileAtomic, } from '../../../../services/fs/fsCore'; +// QNBS-v3: entries are plain names, directories suffixed with '/' — just enough to model a nested tree for cleanupOrphanedTempFiles' recursive walk, independent of makeAtomicWriteFake above. +function makeDirTreeFake(tree: Record) { + const removed: string[] = []; + const apis: Pick = { + readDir: (dir) => { + const entries = tree[dir]; + if (!entries) return Promise.reject(new Error(`ENOENT ${dir}`)); + return Promise.resolve( + entries.map((name) => ({ + name: name.endsWith('/') ? name.slice(0, -1) : name, + isDirectory: name.endsWith('/'), + })), + ); + }, + remove: (p: string) => { + removed.push(p); + return Promise.resolve(); + }, + join: (...parts: string[]) => Promise.resolve(parts.join('/')), + }; + return { apis: apis as TauriApis, removed }; +} + +// QNBS-v3: minimal in-memory fake covering only what writeTextFileAtomic/writeFileAtomic use — a lighter-weight sibling of fsStores.test.ts's fuller FakeFs, scoped to this file's needs. +function makeAtomicWriteFake() { + const text = new Map(); + const bin = new Map(); + const apis: Pick = { + writeTextFile: (p, c) => { + text.set(p, c); + return Promise.resolve(); + }, + writeFile: (p, d) => { + bin.set(p, d); + return Promise.resolve(); + }, + rename: (oldPath, newPath) => { + if (text.has(oldPath)) { + text.set(newPath, text.get(oldPath) as string); + text.delete(oldPath); + } else if (bin.has(oldPath)) { + bin.set(newPath, bin.get(oldPath) as Uint8Array); + bin.delete(oldPath); + } + return Promise.resolve(); + }, + remove: (p) => { + text.delete(p); + bin.delete(p); + return Promise.resolve(); + }, + }; + return { apis: apis as TauriApis, text, bin }; +} + describe('retryFs', () => { it('returns on first success without retrying', async () => { const fn = vi.fn().mockResolvedValue('ok'); @@ -44,6 +115,365 @@ describe('retryFs', () => { }); }); +describe('writeTextFileAtomic / writeFileAtomic', () => { + afterEach(() => { + h.isTauri = false; + h.invoke.mockReset(); + }); + + // QNBS-v3: desktop writes cross the native boundary so the temp file and replacement metadata are synchronized before success returns. + it('uses the native durable command for text writes in Tauri', async () => { + const { apis } = makeAtomicWriteFake(); + h.isTauri = true; + h.invoke.mockResolvedValue(undefined); + + await writeTextFileAtomic(apis, '/app/project.json', 'content'); + + expect(h.invoke).toHaveBeenCalledWith( + 'worldscript_atomic_write', + new TextEncoder().encode('content'), + { headers: { 'x-worldscript-path': '%2Fapp%2Fproject.json' } }, + ); + }); + + it('uses the native durable command for binary writes in Tauri', async () => { + const { apis } = makeAtomicWriteFake(); + h.isTauri = true; + h.invoke.mockResolvedValue(undefined); + + await writeFileAtomic(apis, '/app/asset.bin', new Uint8Array([1, 2, 3])); + + expect(h.invoke).toHaveBeenCalledWith('worldscript_atomic_write', new Uint8Array([1, 2, 3]), { + headers: { 'x-worldscript-path': '%2Fapp%2Fasset.bin' }, + }); + }); + + it('propagates a native durable-write failure', async () => { + const { apis } = makeAtomicWriteFake(); + h.isTauri = true; + h.invoke.mockRejectedValue(new Error('disk full')); + + await expect(writeTextFileAtomic(apis, '/app/project.json', 'content')).rejects.toThrow( + 'disk full', + ); + }); + + it('retries a transient native durable-write failure', async () => { + const { apis } = makeAtomicWriteFake(); + h.isTauri = true; + h.invoke + .mockRejectedValueOnce(new Error('resource temporarily unavailable')) + .mockResolvedValueOnce(undefined); + + await expect( + writeTextFileAtomic(apis, '/app/project.json', 'content'), + ).resolves.toBeUndefined(); + expect(h.invoke).toHaveBeenCalledTimes(2); + }); + + it('writes content under the final path and leaves no temp file behind', async () => { + const { apis, text } = makeAtomicWriteFake(); + await writeTextFileAtomic(apis, '/app/project.json', '{"a":1}'); + expect(text.get('/app/project.json')).toBe('{"a":1}'); + expect([...text.keys()]).toEqual(['/app/project.json']); + }); + + it('binary variant writes content under the final path and leaves no temp file behind', async () => { + const { apis, bin } = makeAtomicWriteFake(); + const data = new Uint8Array([1, 2, 3]); + await writeFileAtomic(apis, '/app/asset.bin', data); + expect(bin.get('/app/asset.bin')).toEqual(data); + expect([...bin.keys()]).toEqual(['/app/asset.bin']); + }); + + // QNBS-v3: the core crash-safety guarantee — a failure after the temp write but before rename must never touch the final path, so a reader always sees the old or new complete file, never a partial/torn write. + it('leaves the original file untouched when the rename step fails after the temp write succeeds', async () => { + const { apis, text } = makeAtomicWriteFake(); + text.set('/app/project.json', '{"old":true}'); + apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + + await expect(writeTextFileAtomic(apis, '/app/project.json', '{"new":true}')).rejects.toThrow( + /locked/, + ); + expect(text.get('/app/project.json')).toBe('{"old":true}'); + }); + + it('cleans up the orphaned temp file when the rename step fails', async () => { + const { apis, text } = makeAtomicWriteFake(); + apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + const removeSpy = vi.spyOn(apis, 'remove'); + + await expect(writeTextFileAtomic(apis, '/app/project.json', '{"new":true}')).rejects.toThrow( + /locked/, + ); + + expect(removeSpy).toHaveBeenCalledWith(expect.stringContaining('/app/project.json.tmp-')); + expect([...text.keys()]).toEqual([]); + }); + + it('leaves the original file untouched when the temp-file write itself fails', async () => { + const { apis, text } = makeAtomicWriteFake(); + text.set('/app/project.json', '{"old":true}'); + apis.writeTextFile = () => Promise.reject(new Error('disk full')); + + await expect(writeTextFileAtomic(apis, '/app/project.json', '{"new":true}')).rejects.toThrow( + /disk full/, + ); + expect(text.get('/app/project.json')).toBe('{"old":true}'); + }); + + it('does not throw when best-effort cleanup of the orphaned temp file also fails', async () => { + const { apis } = makeAtomicWriteFake(); + apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + apis.remove = () => Promise.reject(new Error('ENOENT')); + + // The original rename error must still surface — the cleanup failure must not mask it or + // throw an unhandled rejection of its own. + await expect(writeTextFileAtomic(apis, '/app/project.json', '{"new":true}')).rejects.toThrow( + /locked/, + ); + }); + + // QNBS-v3: binary-path parity with the text-path failure-mode tests above. + it('binary: leaves the original file untouched when the rename step fails after the temp write succeeds', async () => { + const { apis, bin } = makeAtomicWriteFake(); + const original = new Uint8Array([9, 9, 9]); + bin.set('/app/asset.bin', original); + apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + + await expect( + writeFileAtomic(apis, '/app/asset.bin', new Uint8Array([1, 2, 3])), + ).rejects.toThrow(/locked/); + expect(bin.get('/app/asset.bin')).toEqual(original); + expect([...bin.keys()]).toEqual(['/app/asset.bin']); + }); + + it('binary: cleans up the orphaned temp file when the rename step fails', async () => { + const { apis, bin } = makeAtomicWriteFake(); + apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + const removeSpy = vi.spyOn(apis, 'remove'); + + await expect( + writeFileAtomic(apis, '/app/asset.bin', new Uint8Array([1, 2, 3])), + ).rejects.toThrow(/locked/); + + expect(removeSpy).toHaveBeenCalledWith(expect.stringContaining('/app/asset.bin.tmp-')); + expect([...bin.keys()]).toEqual([]); + }); + + it('binary: leaves the original file untouched when the temp-file write itself fails', async () => { + const { apis, bin } = makeAtomicWriteFake(); + const original = new Uint8Array([9, 9, 9]); + bin.set('/app/asset.bin', original); + apis.writeFile = () => Promise.reject(new Error('disk full')); + + await expect( + writeFileAtomic(apis, '/app/asset.bin', new Uint8Array([1, 2, 3])), + ).rejects.toThrow(/disk full/); + expect(bin.get('/app/asset.bin')).toEqual(original); + }); + + it('binary: does not throw when best-effort cleanup of the orphaned temp file also fails', async () => { + const { apis } = makeAtomicWriteFake(); + apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + apis.remove = () => Promise.reject(new Error('ENOENT')); + + await expect( + writeFileAtomic(apis, '/app/asset.bin', new Uint8Array([1, 2, 3])), + ).rejects.toThrow(/locked/); + }); + + // QNBS-v3: same-path writes serialize in call order, so an older (slower) save can never overwrite a newer save's already-committed content. + it('serializes concurrent writes to the same path in call order, not completion order', async () => { + const { apis, text } = makeAtomicWriteFake(); + const originalWriteTextFile = apis.writeTextFile; + let firstWriteStarted = false; + apis.writeTextFile = async (p, c) => { + if (p.includes('.tmp-') && c === 'first' && !firstWriteStarted) { + firstWriteStarted = true; + // Delay the first (older) write so it would finish AFTER the second one if unserialized. + await new Promise((resolve) => setTimeout(resolve, 20)); + } + return originalWriteTextFile(p, c); + }; + + const first = writeTextFileAtomic(apis, '/app/project.json', 'first'); + const second = writeTextFileAtomic(apis, '/app/project.json', 'second'); + await Promise.all([first, second]); + + // Second (newer) call must be the one that lands, even though the first call's temp write + // was artificially slower. + expect(text.get('/app/project.json')).toBe('second'); + }); + + // QNBS-v3: every caller must observe its own write outcome; replacing a pending payload made superseded callers receive false success. + it('executes every queued write in call order and resolves each caller only after its own payload commits', async () => { + const { apis, text } = makeAtomicWriteFake(); + const originalWriteTextFile = apis.writeTextFile; + const writtenContents: string[] = []; + let releaseFirst: (() => void) | undefined; + let resolveFirstStarted: (() => void) | undefined; + const firstStarted = new Promise((resolve) => { + resolveFirstStarted = resolve; + }); + + apis.writeTextFile = async (p, c) => { + writtenContents.push(c); + if (c === 'first') { + resolveFirstStarted?.(); + await new Promise((res) => { + releaseFirst = res; + }); + } + return originalWriteTextFile(p, c); + }; + + const first = writeTextFileAtomic(apis, '/app/project.json', 'first'); + await firstStarted; + + const second = writeTextFileAtomic(apis, '/app/project.json', 'second'); + const third = writeTextFileAtomic(apis, '/app/project.json', 'third'); + + releaseFirst?.(); + await Promise.all([first, second, third]); + + expect(writtenContents).toEqual(['first', 'second', 'third']); + expect(text.get('/app/project.json')).toBe('third'); + }); + + it('rejects new work when a same-path write backlog reaches its bounded limit', async () => { + const { apis } = makeAtomicWriteFake(); + let releaseFirst: (() => void) | undefined; + const originalWriteTextFile = apis.writeTextFile; + let isFirstWrite = true; + apis.writeTextFile = async (path, content) => { + if (!isFirstWrite) return originalWriteTextFile(path, content); + isFirstWrite = false; + await new Promise((resolve) => { + releaseFirst = resolve; + }); + return originalWriteTextFile(path, content); + }; + + const accepted = Array.from({ length: 8 }, (_, index) => + writeTextFileAtomic(apis, '/app/project.json', `save-${index}`), + ); + await expect(writeTextFileAtomic(apis, '/app/project.json', 'overflow')).rejects.toThrow( + 'backlog is full', + ); + + releaseFirst?.(); + await Promise.all(accepted); + }); +}); + +describe('cleanupOrphanedTempFiles', () => { + // QNBS-v3: reclaims .tmp-* siblings left by writes interrupted by a process kill, which neither atomicRename's nor writeThenRename's in-session cleanup ever sees. + it('removes .tmp-* files at the root and nested inside subdirectories', async () => { + const { apis, removed } = makeDirTreeFake({ + '/app': ['project.json.tmp-a1b2c3d4-1111-2222-3333-444444444444', 'config/', 'projects/'], + '/app/config': ['settings.json'], + '/app/projects': ['p1/'], + '/app/projects/p1': ['project.json', 'codex/'], + '/app/projects/p1/codex': ['codex.snap.tmp-deadbeefcafefeed00112233445566'], + }); + + await cleanupOrphanedTempFiles(apis, '/app'); + + expect(removed.sort()).toEqual( + [ + '/app/project.json.tmp-a1b2c3d4-1111-2222-3333-444444444444', + '/app/projects/p1/codex/codex.snap.tmp-deadbeefcafefeed00112233445566', + ].sort(), + ); + }); + + it('does not remove non-temp files', async () => { + const { apis, removed } = makeDirTreeFake({ '/app': ['project.json', 'settings.json'] }); + await cleanupOrphanedTempFiles(apis, '/app'); + expect(removed).toEqual([]); + }); + + it('does not throw when the root directory does not exist yet (fresh install)', async () => { + const { apis, removed } = makeDirTreeFake({}); + await expect(cleanupOrphanedTempFiles(apis, '/app')).resolves.toBeUndefined(); + expect(removed).toEqual([]); + }); + + // QNBS-v3: a save started right after initialize() creates a brand-new .tmp-* file that a still-running startup sweep must never treat as an orphan — without this, the sweep could delete a legitimately in-progress write out from under it. + it('does not delete a temp file that a same-session write is still actively writing', async () => { + const removed: string[] = []; + const text = new Map(); + let capturedTmpPath = ''; + let releaseWrite: (() => void) | undefined; + let resolveWriteStarted: (() => void) | undefined; + const writeStarted = new Promise((resolve) => { + resolveWriteStarted = resolve; + }); + + const apis: TauriApis = { + writeTextFile: async (p, c) => { + capturedTmpPath = p; + resolveWriteStarted?.(); + await new Promise((res) => { + releaseWrite = res; + }); + text.set(p, c); + }, + writeFile: () => Promise.reject(new Error('not used')), + readTextFile: () => Promise.reject(new Error('not used')), + readFile: () => Promise.reject(new Error('not used')), + mkdir: () => Promise.resolve(), + exists: () => Promise.resolve(false), + readDir: (dir) => { + if (dir !== '/app') return Promise.reject(new Error(`ENOENT ${dir}`)); + const name = capturedTmpPath.split('/').pop(); + return Promise.resolve(name ? [{ name, isDirectory: false }] : []); + }, + remove: (p) => { + removed.push(p); + return Promise.resolve(); + }, + rename: (oldPath, newPath) => { + if (text.has(oldPath)) { + text.set(newPath, text.get(oldPath) as string); + text.delete(oldPath); + } + return Promise.resolve(); + }, + open: () => Promise.resolve(null), + save: () => Promise.resolve(null), + appDataDir: () => Promise.resolve('/app'), + join: (...parts) => Promise.resolve(parts.join('/')), + invoke: () => Promise.resolve(undefined), + }; + + const writePromise = writeTextFileAtomic(apis, '/app/project.json', 'content'); + await writeStarted; + + await cleanupOrphanedTempFiles(apis, '/app'); + expect(removed).toEqual([]); + + releaseWrite?.(); + await writePromise; + expect(text.get('/app/project.json')).toBe('content'); + }); + + it('stops recursing past the depth guard instead of following an unexpectedly deep tree', async () => { + const tree: Record = { '/app': ['d/'] }; + let path = '/app'; + for (let i = 0; i < 10; i++) { + tree[path] = ['d/']; + path = `${path}/d`; + } + tree[path] = ['leaf.tmp-a1b2c3d4a1b2c3d4a1b2c3d4a1b2c3d4']; + const { apis, removed } = makeDirTreeFake(tree); + + await expect(cleanupOrphanedTempFiles(apis, '/app')).resolves.toBeUndefined(); + expect(removed).toEqual([]); // the leaf is past the depth guard, never reached + }); +}); + describe('compressData / decompressData', () => { it('round-trips small data uncompressed (plain JSON)', () => { const data = { a: 1, b: ['x', 'y'], c: 'hello' }; @@ -78,8 +508,7 @@ describe('encryptText / decryptText', () => { await expect(decryptText(payload, 'wrong-key')).rejects.toBeDefined(); }); - // QNBS-v3 (F-05/F-06 fix, 2026-07-29): regression guard for the PBKDF2 + random-salt derivation - // replacing the prior unsalted single-SHA-256 scheme. + // QNBS-v3 (F-05/F-06 fix, 2026-07-29): regression guard for the PBKDF2 + random-salt derivation replacing the prior unsalted single-SHA-256 scheme. it('produces a different ciphertext, iv, and salt on every encryption of the same secret+plaintext', async () => { const a = await encryptText('same plaintext', 'same-secret-material'); const b = await encryptText('same plaintext', 'same-secret-material'); diff --git a/tests/unit/services/fs/fsStores.test.ts b/tests/unit/services/fs/fsStores.test.ts index 596511f1..7256436e 100644 --- a/tests/unit/services/fs/fsStores.test.ts +++ b/tests/unit/services/fs/fsStores.test.ts @@ -8,13 +8,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { TauriApis } from '../../../../services/fs/fsCore'; -// QNBS-v3: typed via `unknown` (not `any`) so the mock factories see a non-null TauriApis; the -// real value is set in beforeEach before any mock is invoked. +// QNBS-v3: typed via `unknown` (not `any`) so the mock factories see a non-null TauriApis; the real value is set in beforeEach before any mock is invoked. const { fsHolder } = vi.hoisted(() => ({ fsHolder: { current: null as unknown as TauriApis } })); -// QNBS-v3: mock the @tauri-apps plugin modules so the REAL loadTauriApis assembles a TauriApis -// whose methods delegate to the per-test in-memory fake FS (memoization-safe — each call reads -// fsHolder.current). This exercises the real store logic AND loadTauriApis itself. +// QNBS-v3: mock the @tauri-apps plugin modules so the REAL loadTauriApis assembles a TauriApis whose methods delegate to the per-test in-memory fake FS (memoization-safe); exercises real store logic AND loadTauriApis itself. vi.mock('@tauri-apps/api/core', () => ({ invoke: (cmd: string, args?: Record) => fsHolder.current.invoke(cmd, args), })); @@ -27,6 +24,7 @@ vi.mock('@tauri-apps/plugin-fs', () => ({ exists: (p: string) => fsHolder.current.exists(p), readDir: (p: string) => fsHolder.current.readDir(p), remove: (p: string, opts?: { recursive?: boolean }) => fsHolder.current.remove(p, opts), + rename: (oldPath: string, newPath: string) => fsHolder.current.rename(oldPath, newPath), })); vi.mock('@tauri-apps/plugin-dialog', () => ({ open: (opts?: Record) => fsHolder.current.open(opts), @@ -95,6 +93,19 @@ function makeFakeFs(): FakeFs { for (const k of [...bin.keys()]) if (k.startsWith(`${p}/`)) bin.delete(k); return Promise.resolve(); }, + // QNBS-v3: atomic-write support (writeTextFileAtomic/writeFileAtomic) — a plain in-memory move from the temp key to the final key, mirroring a real filesystem rename. + rename: (oldPath: string, newPath: string) => { + if (text.has(oldPath)) { + text.set(newPath, text.get(oldPath) as string); + text.delete(oldPath); + } else if (bin.has(oldPath)) { + bin.set(newPath, bin.get(oldPath) as Uint8Array); + bin.delete(oldPath); + } else { + return Promise.reject(new Error(`ENOENT ${oldPath}`)); + } + return Promise.resolve(); + }, readDir: (p: string) => Promise.resolve(under(p).map((name) => ({ name, isDirectory: false }))), open: () => Promise.resolve(null), save: () => Promise.resolve(null), @@ -155,11 +166,35 @@ describe('FsProjectStore — projects', () => { expect(await store.getActiveProjectId()).toBe('p2'); }); + // QNBS-v3: empirical proof rename() replaces an existing project.json — a normal repeat save must succeed and reflect the new content. + it('saves the same project twice, with the second save replacing the first', async () => { + await store.saveProject(project as never); + expect((await store.loadProject('p1'))?.title).toBe('My Novel'); + + const updated = { ...project, title: 'Revised Novel' }; + await store.saveProject(updated as never); + expect((await store.loadProject('p1'))?.title).toBe('Revised Novel'); + }); + + // QNBS-v3: writeTextFileAtomic integration proof for the highest-stakes writer — an interrupted save (rename fails after the temp file has the new content) must never corrupt/truncate the previously-saved project.json. + it('leaves the previously-saved project.json intact when an interrupted save fails after the temp write', async () => { + await store.saveProject(project as never); + expect((await store.loadProject('p1'))?.title).toBe('My Novel'); + + fake.apis.rename = () => Promise.reject(new Error('EBUSY: file is locked')); + const updated = { ...project, title: 'Renamed Novel' }; + await expect(store.saveProject(updated as never)).rejects.toThrow(/locked/); + + const reloaded = await store.loadProject('p1'); + expect(reloaded?.title).toBe('My Novel'); + }); + // QNBS-v3 (#332): a rejected marker write is a documented best-effort abort — it must not fail the project save that already succeeded. it('still resolves saveProject and logs a warning when the active-project marker write rejects', async () => { const originalWriteTextFile = fake.apis.writeTextFile; + // QNBS-v3: writeTextFileAtomic writes to a temp sibling first — `.includes` (not `.endsWith`) still catches that path, failing before the rename step. fake.apis.writeTextFile = (p: string, c: string) => { - if (p.endsWith('active-project-id.txt')) return Promise.reject(new Error('disk full')); + if (p.includes('active-project-id.txt')) return Promise.reject(new Error('disk full')); return originalWriteTextFile(p, c); }; @@ -295,14 +330,24 @@ describe('FsAssetStore — images + binder assets', () => { expect(await store.getImage('char-1')).toBeNull(); }); + it('preserves the MIME type of non-PNG image data URLs while reading legacy raw base64', async () => { + await store.saveImage('world-1', 'data:image/webp;base64,V0VCUA=='); + expect(await store.getImage('world-1')).toBe('data:image/webp;base64,V0VCUA=='); + + fake.text.set('/app/images/legacy.png', 'TEVHQUNZ'); + expect(await store.getImage('legacy')).toBe('data:image/png;base64,TEVHQUNZ'); + }); + it('round-trips a binder binary asset with metadata', async () => { const data = new Uint8Array([1, 2, 3, 4]).buffer; await store.saveBinderAsset('p1', 'a1', data, { - name: 'doc.pdf', - mime: 'application/pdf', - } as never); + originalFileName: 'doc.pdf', + mimeType: 'application/pdf', + byteSize: 0, + }); const got = await store.getBinderAsset('p1', 'a1'); expect(got?.meta.byteSize).toBe(4); + expect(got?.meta.originalFileName).toBe('doc.pdf'); expect(new Uint8Array(got?.data as ArrayBuffer)).toEqual(new Uint8Array([1, 2, 3, 4])); expect(await store.listBinderAssetIds('p1')).toContain('a1'); @@ -310,6 +355,35 @@ describe('FsAssetStore — images + binder assets', () => { expect(await store.getBinderAsset('p1', 'a1')).toBeNull(); }); + it('keeps the previously committed binder pair readable when the new manifest cannot publish', async () => { + const original = new Uint8Array([1, 2, 3]).buffer; + await store.saveBinderAsset('p1', 'a1', original, { + originalFileName: 'before.pdf', + mimeType: 'application/pdf', + byteSize: 0, + }); + const committedFiles = [...fake.bin.keys()].sort(); + + const originalWriteTextFile = fake.apis.writeTextFile; + fake.apis.writeTextFile = (path, content) => { + if (path.includes('a1.meta.json.tmp-')) return Promise.reject(new Error('disk full')); + return originalWriteTextFile(path, content); + }; + + await expect( + store.saveBinderAsset('p1', 'a1', new Uint8Array([9, 9]).buffer, { + originalFileName: 'after.pdf', + mimeType: 'application/pdf', + byteSize: 0, + }), + ).rejects.toThrow('disk full'); + + const recovered = await store.getBinderAsset('p1', 'a1'); + expect(recovered?.meta.originalFileName).toBe('before.pdf'); + expect(new Uint8Array(recovered?.data as ArrayBuffer)).toEqual(new Uint8Array([1, 2, 3])); + expect([...fake.bin.keys()].sort()).toEqual(committedFiles); + }); + it('returns null/[] for missing binder assets', async () => { expect(await store.getBinderAsset('p1', 'missing')).toBeNull(); expect(await store.listBinderAssetIds('p1')).toEqual([]);