From 89ca299e5e4263d40a01e68a1e7d5b6c29b88202 Mon Sep 17 00:00:00 2001 From: cervantesh <11169707+cervantesh@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:34:58 -0400 Subject: [PATCH 1/5] docs: define filesystem crash durability --- docs/adr/0001-filesystem-crash-durability.md | 95 ++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 docs/adr/0001-filesystem-crash-durability.md diff --git a/docs/adr/0001-filesystem-crash-durability.md b/docs/adr/0001-filesystem-crash-durability.md new file mode 100644 index 0000000..5730d18 --- /dev/null +++ b/docs/adr/0001-filesystem-crash-durability.md @@ -0,0 +1,95 @@ +# ADR 0001: Filesystem crash-durability levels + +## Status + +Accepted + +## Date + +2026-07-16 + +## Context + +`FilesystemPiClient` already writes through a same-directory temporary file, calls `fsync` on that file, and atomically renames it over the destination. This provides atomic visibility and a useful file-data durability step, but it does not make the rename durable because the parent directory is not synchronized. Directory synchronization support also varies by operating system and filesystem: for example, the Node.js `fsync` operation on an opened directory reports `EPERM` on Windows. + +Atomic visibility and crash durability are separate guarantees. The public API needs to let callers trade durability for latency without silently changing the behavior existing callers receive. + +## Decision + +### Public levels and defaults + +The package exports three `FilesystemDurability` levels through `FILESYSTEM_DURABILITY_LEVELS`: + +- `none`: write and atomically rename without an explicit sync; +- `file`: sync the temporary file before rename; +- `file-and-parent-directory`: sync the temporary file before rename, then sync the destination's direct parent directory after rename. + +`DEFAULT_FILESYSTEM_DURABILITY` is `file`, preserving the behavior existing `new FilesystemPiClient()` callers receive today. + +`FilesystemPiClientConfig` selects the durability level. It also selects `unsupportedDirectorySync` as either `degrade` or `strict`; the default is `degrade`. + +### Ordering + +The write sequence is: + +1. create and write the same-directory temporary file; +2. apply the preserved destination mode to the temporary file, when one exists; +3. perform the selected final temporary-file sync (`file` and `file-and-parent-directory` only); +4. close the temporary-file handle; +5. revalidate the destination and atomically rename; +6. for `file-and-parent-directory`, sync the destination's direct parent directory. + +Applying mode before the final file sync includes the mode metadata in the best available file durability boundary. + +### Unsupported parent-directory synchronization + +Capability is detected from the actual open/sync operation on the destination's parent rather than from an operating-system allowlist. Known unsupported-operation error codes are classified explicitly. Windows `EPERM` from directory sync is also classified as unsupported; permission errors not identified as an unsupported capability remain real failures. + +- `degrade`: a classified unsupported directory-sync result completes successfully at file durability. The caller chose best-effort parent-directory durability and must not interpret success as confirmation that the rename survived a crash on that filesystem. +- `strict`: the same classified result throws an explicit durability error. +- Any unclassified directory open/sync failure throws regardless of policy. + +No parent-directory capability result is cached globally because different paths can reside on filesystems with different behavior. + +### Post-rename failures + +Parent-directory sync occurs after rename, so any failure from that step happens after the destination has been committed and is visible. The operation throws a `FilesystemDurabilityError` with `destinationVisible: true`; it does not roll back, delete, or restore the visible destination. Retrying the original edit blindly is unsafe. Callers must inspect/re-read the destination and decide whether another durability attempt or edit is appropriate. + +In `degrade` mode, only a classified unsupported-capability result is absorbed. Other post-rename failures still throw `FilesystemDurabilityError` and carry the original error as `cause`. + +### Created-directory scope + +`file-and-parent-directory` synchronizes exactly the destination's direct parent once. If recursive directory creation was needed, the level does not claim to synchronize every newly created ancestor or the ancestor entries that make the new path reachable. Callers requiring crash-durable directory-tree creation must provision and synchronize that hierarchy separately before editing. + +### Test seams + +Protected filesystem-operation seams remain available for deterministic ordering and failure injection. They are testability hooks, not additional public durability guarantees. + +## Consequences + +### Positive + +- Existing callers retain file-sync behavior. +- Callers can choose lower latency or stronger rename durability explicitly. +- Unsupported filesystems and Windows have deterministic degrade/strict behavior. +- Post-commit errors cannot be mistaken for pre-commit failures. +- Mode preservation is ordered before the final file sync. + +### Negative + +- `file-and-parent-directory` adds a directory open and sync after every successful rename. +- `degrade` success cannot prove parent-directory durability; strict mode is required when lack of that capability must be surfaced. +- Newly created directory hierarchies remain outside the guarantee. +- Filesystem and storage hardware may still weaken or ignore sync semantics beyond what Node.js can observe. + +## Alternatives considered + +1. **Always synchronize the parent directory.** Rejected because it changes current latency and introduces unsupported-platform failures for all callers. +2. **Make no-sync the default.** Rejected because it weakens current behavior. +3. **Reject parent-directory mode on Windows by platform name.** Rejected because support is an operation/filesystem capability and platform allowlists become inaccurate. +4. **Synchronize all recursively created ancestors.** Rejected because it expands a file-edit operation into directory-tree provisioning and makes the level's cost and scope less predictable. +5. **Roll back after a parent sync failure.** Rejected because rename has already committed; a rollback would be a second mutation with its own failure and concurrency risks. + +## Reversal signals + +Revisit this decision if Node.js provides a portable directory-sync capability API, if callers require crash-durable recursive path creation, or if measurements show the configuration surface cannot express required durability guarantees clearly. From 14fdff72c52794ef2183aa81693abaad3e307627 Mon Sep 17 00:00:00 2001 From: cervantesh <11169707+cervantesh@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:34:58 -0400 Subject: [PATCH 2/5] feat: add configurable filesystem durability --- CHANGELOG.md | 7 + README.md | 27 ++- SECURITY.md | 2 + benchmarks/core.mjs | 82 +++++++-- dist/src/filesystem-client.d.ts | 33 ++++ dist/src/filesystem-client.js | 100 ++++++++++- dist/test/filesystem-client.test.js | 199 +++++++++++++++++++++- docs/ARCHITECTURE.md | 12 +- docs/EXAMPLES.md | 12 +- docs/OPERATIONS.md | 13 ++ docs/PERFORMANCE.md | 22 +-- src/filesystem-client.ts | 141 ++++++++++++++- test/filesystem-client.test.ts | 255 +++++++++++++++++++++++++++- 13 files changed, 867 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1177383..eb9c2b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,18 +11,25 @@ - package provenance/license verification and immutable GitHub release artifact creation; - weekly Dependabot and CodeQL scanning workflows. - deterministic filesystem race fixtures for changed content, permission mode, replacement identity, deletion, and missing-to-created destinations. +- exported filesystem durability levels, defaults, configuration, and post-rename `FilesystemDurabilityError` recovery metadata; +- deterministic ordering/failure seams and cross-platform tests for file sync, parent-directory capability, strict/degraded behavior, visibility, and cleanup. +- repository ADR defining crash-durability scope and failure semantics. ### Changed - filesystem edits now classify paths before decoding and use same-directory atomic replacement with mode, BOM, newline, and cleanup guarantees; - package contents exclude compiled tests and include complete repository provenance metadata. - filesystem atomic replacement now performs best-effort optimistic destination revalidation with identity, permission-mode, and SHA-256 byte-digest evidence, returning `[E_CONCURRENT_DESTINATION]` while preserving detected concurrent state and cleaning temporary files. +- callers can select `none`, `file`, or `file-and-parent-directory` durability through `FilesystemPiClient`; the default remains file sync. +- preserved destination mode is now applied before the final temporary-file sync, and parent-directory sync capability is detected from the actual filesystem operation. ### Security - unsafe binary, image, special-file, symlink, null-byte, and lossy UTF-8 rewrites are rejected before writing; - security reporting, sensitive-diagnostic handling, dependency review, and recovery responsibilities are documented. - same-size, coarse-timestamp, and permission-only destination changes are no longer silently overwritten when detected before replacement; documentation explicitly records the residual check-to-rename race and makes no compare-and-swap guarantee. +- post-rename sync failures now explicitly report that the destination is visible but crash durability is unconfirmed, preventing unsafe blind retry/rollback assumptions; +- documentation distinguishes atomic visibility, file durability, parent-directory durability, and the unsupported scope of recursively created ancestors. ## 0.1.0 diff --git a/README.md b/README.md index 8bac6fb..bf047d6 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ Provide one dependable cross-platform edit core where every edit applies to the - File-kind and text-loading helpers for text/binary/empty-file handling. - Text and newline normalization used by cross-platform edit tools. - A local filesystem Pi-style client for tests and retry-oriented tooling. +- Selectable filesystem crash-durability levels with the existing file-sync behavior retained by default. - TypeScript declarations and ESM output under `dist/`. ## Quickstart @@ -44,6 +45,23 @@ const client = new FilesystemPiClient(); const rendered = await client.read({ path: 'src/file.ts' }); ``` +Choose durability explicitly only when the default file sync is not the desired trade-off: + +```ts +import { FILESYSTEM_DURABILITY_LEVELS, FilesystemPiClient } from 'pi-anchor-edit-core'; + +const fastest = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.NONE, +}); + +const renameDurable = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', +}); +``` + +The levels are `none`, `file`, and `file-and-parent-directory`; `DEFAULT_FILESYSTEM_DURABILITY` is `file`. Parent-directory sync support is detected from the real operation. Use `strict` when unsupported capability must throw; the default `degrade` policy completes at file durability when the operation is known to be unsupported. + Copy anchors verbatim from `rendered`; they are opaque observations. ### 4. Apply a verified edit @@ -93,6 +111,10 @@ The destination changed after the filesystem adapter loaded it and before atomic This is best-effort optimistic detection, not compare-and-swap. A residual race remains between the final revalidation and `rename`, so a successful edit is not a guarantee that no concurrent writer intervened. +### `[E_DIRECTORY_SYNC_UNSUPPORTED]` / `[E_DURABILITY_UNCONFIRMED]` + +These `FilesystemDurabilityError` failures occur after rename. The edited destination is already visible (`destinationVisible === true`), but requested parent-directory crash durability was not confirmed. Do not blindly replay the edit or attempt to restore the old file. Re-read the destination before deciding how to recover. A classified unsupported operation is absorbed only when `unsupportedDirectorySync: 'degrade'`; other sync failures always throw. + ## Development ```bash @@ -108,15 +130,16 @@ npm run benchmark ### Supported matrix -CI runs Node.js 22 on Ubuntu, Windows, and macOS, plus the Node.js 24 compatibility job on Ubuntu. Capability-sensitive symlink and permission assertions report a specific diagnostic when the host cannot provide that feature; unrelated filesystem and CRLF assertions continue to run. The thresholded coverage command enforces at least 85% line coverage and 75% branch coverage. +CI runs Node.js 22 on Ubuntu, Windows, and macOS, plus the Node.js 24 compatibility job on Ubuntu. Capability-sensitive symlink and permission assertions report a specific diagnostic when the host cannot provide that feature; unrelated filesystem and CRLF assertions continue to run. Durability tests assert successful real parent-directory sync on hosted Linux/macOS and Windows `EPERM` degradation/strict classification, in addition to deterministic injected failure fixtures. The thresholded coverage command enforces at least 85% line coverage and 75% branch coverage. ## Documentation - [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) — components, control flow, and invariants. +- [`docs/adr/0001-filesystem-crash-durability.md`](docs/adr/0001-filesystem-crash-durability.md) — durability levels, ordering, capability degradation, and post-rename failure contract. - [`docs/BRANCH_PROTECTION.md`](docs/BRANCH_PROTECTION.md) — required review/check policy and read-only verification. - [`docs/OPERATIONS.md`](docs/OPERATIONS.md) — classified errors, filesystem recovery, and safe escalation. - [`docs/EXAMPLES.md`](docs/EXAMPLES.md) — parsing, editing, recovery, and adapter examples. -- [`docs/PERFORMANCE.md`](docs/PERFORMANCE.md) — reproducible hash baseline. +- [`docs/PERFORMANCE.md`](docs/PERFORMANCE.md) — reproducible hash and filesystem durability benchmarks. - [`docs/PRODUCT.md`](docs/PRODUCT.md) — vision and success metrics. - [`docs/RELEASING.md`](docs/RELEASING.md) — package verification, immutable tags, release creation, and recovery. - [`CONTRIBUTING.md`](CONTRIBUTING.md) — contributor workflow. diff --git a/SECURITY.md b/SECURITY.md index b5a9d11..b144677 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -23,6 +23,8 @@ We aim to acknowledge a private report within 3 business days, provide an initia Anchor diagnostics can quote nearby file content in `>>> LINE#HASH:content` retry lines. Treat all diagnostics as potentially sensitive. Redact or replace them with synthetic examples before sharing, and never send raw diagnostics to telemetry by default. +Filesystem sync is a durability control, not an authorization or confidentiality boundary. `none` intentionally omits explicit syncs; the default `file` level does not make the directory rename crash-durable; and `file-and-parent-directory` depends on operating-system, filesystem, mount, virtualization, and hardware behavior. A degraded unsupported directory sync confirms only file durability. Strict post-rename failures leave the new destination visible and must not trigger blind replay or rollback. + See [`docs/OPERATIONS.md`](docs/OPERATIONS.md) for safe recovery actions and [`docs/RELEASING.md`](docs/RELEASING.md) for immutable release recovery. ## Dependency and scanning maintenance diff --git a/benchmarks/core.mjs b/benchmarks/core.mjs index 52aa5a7..7290569 100644 --- a/benchmarks/core.mjs +++ b/benchmarks/core.mjs @@ -1,30 +1,82 @@ import { performance } from 'node:perf_hooks'; -import { computeLineHash } from '../dist/src/index.js'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + FILESYSTEM_DURABILITY_LEVELS, + FilesystemPiClient, + computeLineHash, +} from '../dist/src/index.js'; -const samples = []; -const rounds = 100; -const operationsPerRound = 10_000; +function summarize(samples) { + const sorted = [...samples].sort((left, right) => left - right); + return { + meanMilliseconds: sorted.reduce((sum, value) => sum + value, 0) / sorted.length, + p95Milliseconds: sorted[Math.ceil(sorted.length * 0.95) - 1], + p99Milliseconds: sorted[Math.ceil(sorted.length * 0.99) - 1], + }; +} + +const hashSamples = []; +const hashRounds = 100; +const operationsPerHashRound = 10_000; -for (let round = 0; round < rounds; round += 1) { +for (let round = 0; round < hashRounds; round += 1) { const start = performance.now(); - for (let index = 0; index < operationsPerRound; index += 1) { + for (let index = 0; index < operationsPerHashRound; index += 1) { computeLineHash(index + 1, `const value${index} = ${index};`); } - samples.push((performance.now() - start) / operationsPerRound); + hashSamples.push((performance.now() - start) / operationsPerHashRound); } -samples.sort((a, b) => a - b); -const mean = samples.reduce((sum, value) => sum + value, 0) / samples.length; -const p99 = samples[Math.ceil(samples.length * 0.99) - 1]; +const directory = await mkdtemp(join(tmpdir(), 'pi-anchor-edit-core-benchmark-')); +const editRounds = 25; +const durability = {}; + +try { + for (const level of Object.values(FILESYSTEM_DURABILITY_LEVELS)) { + const path = join(directory, `${level}.txt`); + await writeFile(path, 'value-a'); + const client = new FilesystemPiClient({ + durability: level, + unsupportedDirectorySync: 'degrade', + }); + let from = 'value-a'; + let to = 'value-b'; + const samples = []; + + for (let round = 0; round < editRounds + 3; round += 1) { + const start = performance.now(); + await client.edit({ + path, + edits: [{ op: 'replace_text', oldText: from, newText: to }], + }); + const elapsed = performance.now() - start; + [from, to] = [to, from]; + if (round >= 3) samples.push(elapsed); + } + + durability[level] = summarize(samples); + } +} finally { + await rm(directory, { recursive: true, force: true }); +} console.log( JSON.stringify( { - operation: 'computeLineHash', - rounds, - operationsPerRound, - meanMilliseconds: mean, - p99Milliseconds: p99, + runtime: { node: process.version, platform: process.platform, arch: process.arch }, + computeLineHash: { + rounds: hashRounds, + operationsPerRound: operationsPerHashRound, + ...summarize(hashSamples), + }, + filesystemAtomicEdit: { + roundsPerLevel: editRounds, + includes: 'load, classify, transform, temp write, selected syncs, revalidation, and rename', + unsupportedDirectorySync: 'degrade', + durability, + }, }, null, 2, diff --git a/dist/src/filesystem-client.d.ts b/dist/src/filesystem-client.d.ts index cbebdb1..dabcf9b 100644 --- a/dist/src/filesystem-client.d.ts +++ b/dist/src/filesystem-client.d.ts @@ -1,7 +1,40 @@ +import type { FileHandle } from 'node:fs/promises'; import type { EditParams, PiClient, ReadParams } from './types.js'; +export declare const FILESYSTEM_DURABILITY_LEVELS: { + readonly NONE: "none"; + readonly FILE: "file"; + readonly FILE_AND_PARENT_DIRECTORY: "file-and-parent-directory"; +}; +export type FilesystemDurability = typeof FILESYSTEM_DURABILITY_LEVELS[keyof typeof FILESYSTEM_DURABILITY_LEVELS]; +export declare const DEFAULT_FILESYSTEM_DURABILITY: FilesystemDurability; +export declare const UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS: { + readonly DEGRADE: "degrade"; + readonly STRICT: "strict"; +}; +export type UnsupportedDirectorySyncBehavior = typeof UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS[keyof typeof UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS]; +export declare const DEFAULT_UNSUPPORTED_DIRECTORY_SYNC_BEHAVIOR: UnsupportedDirectorySyncBehavior; +export type FilesystemPiClientConfig = { + durability?: FilesystemDurability; + unsupportedDirectorySync?: UnsupportedDirectorySyncBehavior; +}; +export type FilesystemDurabilityErrorCode = 'E_DIRECTORY_SYNC_UNSUPPORTED' | 'E_DURABILITY_UNCONFIRMED'; +export declare class FilesystemDurabilityError extends Error { + readonly code: FilesystemDurabilityErrorCode; + readonly destinationPath: string; + readonly durability: FilesystemDurability; + readonly destinationVisible = true; + constructor(code: FilesystemDurabilityErrorCode, destinationPath: string, durability: FilesystemDurability, cause: unknown); +} export declare class FilesystemPiClient implements PiClient { + private readonly durability; + private readonly unsupportedDirectorySync; + constructor(config?: FilesystemPiClientConfig); protected beforeDestinationRevalidation(_destinationPath: string): Promise; + protected applyTemporaryFileMode(temporaryPath: string, mode: number): Promise; + protected synchronizeTemporaryFile(handle: FileHandle): Promise; protected replaceTemporaryFile(temporaryPath: string, destinationPath: string): Promise; + protected synchronizeParentDirectory(parentPath: string): Promise; + private synchronizeParentAfterRename; private observeDestination; private atomicWrite; read({ path, offset, limit }: ReadParams): Promise; diff --git a/dist/src/filesystem-client.js b/dist/src/filesystem-client.js index a8bc1d0..1468c69 100644 --- a/dist/src/filesystem-client.js +++ b/dist/src/filesystem-client.js @@ -5,6 +5,33 @@ import { formatAnchors } from './anchors.js'; import { loadFileKindAndText } from './file-kind.js'; import { applyHashlineEdits, resolveEditAnchors } from './hashline.js'; import { detectLineEnding, normalizeToLF, restoreLineEndings } from './text.js'; +export const FILESYSTEM_DURABILITY_LEVELS = { + NONE: 'none', + FILE: 'file', + FILE_AND_PARENT_DIRECTORY: 'file-and-parent-directory', +}; +export const DEFAULT_FILESYSTEM_DURABILITY = FILESYSTEM_DURABILITY_LEVELS.FILE; +export const UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS = { + DEGRADE: 'degrade', + STRICT: 'strict', +}; +export const DEFAULT_UNSUPPORTED_DIRECTORY_SYNC_BEHAVIOR = UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE; +export class FilesystemDurabilityError extends Error { + code; + destinationPath; + durability; + destinationVisible = true; + constructor(code, destinationPath, durability, cause) { + const detail = code === 'E_DIRECTORY_SYNC_UNSUPPORTED' + ? 'parent-directory synchronization is unsupported' + : 'parent-directory synchronization failed'; + super(`[${code}] ${destinationPath} was replaced and is visible, but ${detail}; crash durability is not confirmed. Re-read before retrying.`, { cause }); + this.code = code; + this.destinationPath = destinationPath; + this.durability = durability; + this.name = 'FilesystemDurabilityError'; + } +} function splitLines(text) { return text.length === 0 ? [] : text.split(/\r?\n/); } @@ -108,11 +135,73 @@ async function loadText(path) { throw new Error(`[E_BINARY_FILE] Refusing to read binary file (${loaded.description}): ${path}`); } } +const UNSUPPORTED_DIRECTORY_SYNC_CODES = new Set([ + 'EBADF', + 'EISDIR', + 'EINVAL', + 'ENOSYS', + 'ENOTSUP', + 'EOPNOTSUPP', +]); +function isUnsupportedDirectorySyncError(error) { + const { code, syscall } = error ?? {}; + return (typeof code === 'string' && UNSUPPORTED_DIRECTORY_SYNC_CODES.has(code)) + || (process.platform === 'win32' && code === 'EPERM' && syscall === 'fsync'); +} +function isFilesystemDurability(value) { + return Object.values(FILESYSTEM_DURABILITY_LEVELS).includes(value); +} +function isUnsupportedDirectorySyncBehavior(value) { + return Object.values(UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS) + .includes(value); +} export class FilesystemPiClient { + durability; + unsupportedDirectorySync; + constructor(config = {}) { + const durability = config.durability ?? DEFAULT_FILESYSTEM_DURABILITY; + const unsupportedDirectorySync = config.unsupportedDirectorySync + ?? DEFAULT_UNSUPPORTED_DIRECTORY_SYNC_BEHAVIOR; + if (!isFilesystemDurability(durability)) { + throw new TypeError(`Unsupported filesystem durability level: ${String(durability)}`); + } + if (!isUnsupportedDirectorySyncBehavior(unsupportedDirectorySync)) { + throw new TypeError(`Unsupported directory-sync behavior: ${String(unsupportedDirectorySync)}`); + } + this.durability = durability; + this.unsupportedDirectorySync = unsupportedDirectorySync; + } async beforeDestinationRevalidation(_destinationPath) { } + async applyTemporaryFileMode(temporaryPath, mode) { + await chmod(temporaryPath, mode); + } + async synchronizeTemporaryFile(handle) { + await handle.sync(); + } async replaceTemporaryFile(temporaryPath, destinationPath) { await rename(temporaryPath, destinationPath); } + async synchronizeParentDirectory(parentPath) { + const handle = await open(parentPath, 'r'); + try { + await handle.sync(); + } + finally { + await handle.close().catch(() => undefined); + } + } + async synchronizeParentAfterRename(parentPath, destinationPath) { + try { + await this.synchronizeParentDirectory(parentPath); + } + catch (error) { + const unsupported = isUnsupportedDirectorySyncError(error); + if (unsupported && this.unsupportedDirectorySync === UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE) { + return; + } + throw new FilesystemDurabilityError(unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', destinationPath, this.durability, error); + } + } async observeDestination(path) { let pathBefore; try { @@ -168,17 +257,22 @@ export class FilesystemPiClient { try { handle = await open(temporaryPath, 'wx', mode ?? 0o666); await handle.writeFile(content, 'utf8'); - await handle.sync(); + if (mode !== undefined) + await this.applyTemporaryFileMode(temporaryPath, mode); + if (this.durability !== FILESYSTEM_DURABILITY_LEVELS.NONE) { + await this.synchronizeTemporaryFile(handle); + } await handle.close(); handle = undefined; - if (mode !== undefined) - await chmod(temporaryPath, mode); await this.beforeDestinationRevalidation(path); const currentObservation = await this.observeDestination(path); if (!sameObservation(observation, currentObservation)) throw concurrentDestinationError(path); // Best-effort only: the destination can still change after this check and before rename. await this.replaceTemporaryFile(temporaryPath, path); + if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { + await this.synchronizeParentAfterRename(parent, path); + } } finally { await handle?.close().catch(() => undefined); diff --git a/dist/test/filesystem-client.test.js b/dist/test/filesystem-client.test.js index fa4aa5f..fdc6f84 100644 --- a/dist/test/filesystem-client.test.js +++ b/dist/test/filesystem-client.test.js @@ -3,7 +3,7 @@ import test from 'node:test'; import { chmod, link, lstat, mkdtemp, readFile, readdir, rename, rm, stat, symlink, utimes, writeFile, } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { basename, join } from 'node:path'; -import { FilesystemPiClient, loadFileKindAndText } from '../src/index.js'; +import { DEFAULT_FILESYSTEM_DURABILITY, FILESYSTEM_DURABILITY_LEVELS, FilesystemDurabilityError, FilesystemPiClient, loadFileKindAndText, } from '../src/index.js'; async function fixture(name = 'file.txt') { const dir = await mkdtemp(join(tmpdir(), 'pi-anchor-edit-core-')); return { dir, path: join(dir, name) }; @@ -28,6 +28,49 @@ class RevalidationRaceClient extends FilesystemPiClient { await this.race(destinationPath); } } +class OperationRecordingClient extends FilesystemPiClient { + operations = []; + synchronizedParents = []; + constructor(config = {}) { + super(config); + } + async applyTemporaryFileMode(temporaryPath, mode) { + this.operations.push('mode'); + await super.applyTemporaryFileMode(temporaryPath, mode); + } + async synchronizeTemporaryFile(handle) { + this.operations.push('file-sync'); + await super.synchronizeTemporaryFile(handle); + } + async replaceTemporaryFile(temporaryPath, destinationPath) { + this.operations.push('rename'); + await super.replaceTemporaryFile(temporaryPath, destinationPath); + } + async synchronizeParentDirectory(parentPath) { + this.operations.push('parent-sync'); + this.synchronizedParents.push(parentPath); + } +} +class FailingFileSyncClient extends FilesystemPiClient { + async synchronizeTemporaryFile() { + const error = new Error('simulated file sync failure'); + error.code = 'EIO'; + throw error; + } +} +class FailingDirectorySyncClient extends FilesystemPiClient { + failureCode; + constructor(config, failureCode) { + super(config); + this.failureCode = failureCode; + } + async synchronizeParentDirectory() { + const error = new Error(`simulated directory sync failure: ${this.failureCode}`); + error.code = this.failureCode; + error.syscall = 'fsync'; + throw error; + } +} function expectedConcurrentDestinationError(path) { return `[E_CONCURRENT_DESTINATION] Refusing to replace ${path}: destination changed after it was loaded. Re-read and retry with current anchors.`; } @@ -87,6 +130,160 @@ test('preserves CRLF, UTF-8 BOM, and an existing permission mode', async (t) => t.diagnostic('permission-bit assertion unavailable on Windows; CRLF/BOM assertions still ran'); } }); +test('exports durability levels and preserves file sync as the default', async () => { + assert.deepEqual(FILESYSTEM_DURABILITY_LEVELS, { + NONE: 'none', + FILE: 'file', + FILE_AND_PARENT_DIRECTORY: 'file-and-parent-directory', + }); + assert.equal(DEFAULT_FILESYSTEM_DURABILITY, FILESYSTEM_DURABILITY_LEVELS.FILE); + const { path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new OperationRecordingClient(); + const preview = await client.read({ path }); + await client.edit({ path, edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }] }); + assert.deepEqual(client.operations, ['mode', 'file-sync', 'rename']); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); +}); +test('supports no-sync, file, and file-and-parent-directory operation sequences', async () => { + const cases = [ + { + durability: FILESYSTEM_DURABILITY_LEVELS.NONE, + expected: ['mode', 'rename'], + }, + { + durability: FILESYSTEM_DURABILITY_LEVELS.FILE, + expected: ['mode', 'file-sync', 'rename'], + }, + { + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + expected: ['mode', 'file-sync', 'rename', 'parent-sync'], + }, + ]; + for (const { durability, expected } of cases) { + const { path } = await fixture(`${durability}.txt`); + await writeFile(path, 'one\ntwo'); + const client = new OperationRecordingClient({ durability }); + const preview = await client.read({ path }); + await client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }); + assert.deepEqual(client.operations, expected, durability); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + } +}); +test('syncs only the direct parent when recursive directory creation was needed', async () => { + const { dir, path } = await fixture('one/two/new.txt'); + const client = new OperationRecordingClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }); + await client.edit({ path, edits: [{ op: 'prepend', lines: ['created'] }] }); + assert.deepEqual(client.operations, ['file-sync', 'rename', 'parent-sync']); + assert.deepEqual(client.synchronizedParents, [join(dir, 'one', 'two')]); + assert.equal(await readFile(path, 'utf8'), 'created'); + await assertNoTemporaryFiles(join(dir, 'one', 'two')); +}); +test('default policy degrades a classified unsupported parent sync to file durability', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectorySyncClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }, 'EINVAL'); + const preview = await client.read({ path }); + await client.edit({ path, edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }] }); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); +test('strict unsupported parent sync reports that the renamed destination is visible', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectorySyncClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', + }, 'ENOTSUP'); + const preview = await client.read({ path }); + await assert.rejects(() => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), (error) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DIRECTORY_SYNC_UNSUPPORTED'); + assert.equal(error.destinationVisible, true); + assert.equal(error.destinationPath, path); + assert.equal(error.durability, FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY); + assert.equal(error.cause.code, 'ENOTSUP'); + return true; + }); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); +test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectorySyncClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'degrade', + }, 'EIO'); + const preview = await client.read({ path }); + await assert.rejects(() => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), (error) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DURABILITY_UNCONFIRMED'); + assert.equal(error.destinationVisible, true); + assert.match(error.message, /was replaced and is visible/); + return true; + }); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); +test('pre-rename file sync failure preserves the original and cleans the temporary file', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingFileSyncClient(); + const preview = await client.read({ path }); + await assert.rejects(() => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), /simulated file sync failure/); + assert.equal(await readFile(path, 'utf8'), 'one\ntwo'); + await assertNoTemporaryFiles(dir); +}); +test('real directory sync capability has explicit hosted-platform behavior', async (t) => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', + }); + const preview = await client.read({ path }); + const edit = () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }); + if (process.platform === 'win32') { + await assert.rejects(edit, (error) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DIRECTORY_SYNC_UNSUPPORTED'); + assert.equal(error.cause.code, 'EPERM'); + return true; + }); + t.diagnostic('Windows directory fsync reported EPERM and strict mode surfaced it'); + } + else { + await edit(); + } + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); +test('rejects invalid runtime durability configuration', () => { + assert.throws(() => new FilesystemPiClient({ durability: 'invalid' }), /Unsupported filesystem durability level/); + assert.throws(() => new FilesystemPiClient({ + unsupportedDirectorySync: 'invalid', + }), /Unsupported directory-sync behavior/); +}); test('replacement failure leaves the original intact and removes the temporary file', async () => { const client = new FailingReplacementClient(); const { dir, path } = await fixture(); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4f1508a..5cb8f19 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -19,7 +19,9 @@ 4. Operations are validated for overlap, uniqueness, and invalid rendered prefixes. 5. The engine produces new content or throws a classified actionable error. 6. The filesystem adapter classifies the path before decoding, captures destination existence, file identity, byte length, permission mode, and a SHA-256 digest, and preserves newline/BOM/mode behavior. -7. Immediately before atomic replacement, it re-observes the destination and aborts with `[E_CONCURRENT_DESTINATION]` if existence, identity, length, permission mode, or digest differs. +7. The adapter writes a same-directory temporary file, applies preserved mode, and performs the configured file sync unless `none` was selected. +8. Immediately before atomic replacement, it re-observes the destination and aborts with `[E_CONCURRENT_DESTINATION]` if existence, identity, length, permission mode, or digest differs; otherwise it renames the temporary file. +9. `file-and-parent-directory` synchronizes the destination's direct parent after rename, degrading only classified unsupported capability under the configured policy. ## Invariants @@ -33,3 +35,11 @@ - Temporary files are created beside the destination so replacement stays on the same filesystem, and are removed after success or handled failure. - Revalidation uses permission-mode and byte-digest evidence rather than size or timestamps alone, so permission-only changes, same-size content changes, and changes hidden by coarse timestamp resolution are detected. - The concurrency guard is optimistic and best-effort, not compare-and-swap: a destination can still change in the residual interval after revalidation and before `rename`. The library makes no false CAS guarantee. +- Atomic visibility and crash durability are separate: `none` performs no explicit sync, `file` syncs the temporary file and remains the default, and `file-and-parent-directory` additionally syncs the direct parent after rename. +- Preserved mode is applied before the selected final file sync. Parent synchronization never precedes rename. +- A parent-sync error is post-commit: the destination is visible and is not rolled back. `FilesystemDurabilityError.destinationVisible` records this recovery boundary. +- Parent sync covers only the direct parent. Recursively created ancestors are not included in the durability claim. + +## Architecture decisions + +- [`ADR 0001: Filesystem crash-durability levels`](adr/0001-filesystem-crash-durability.md) defines the exported levels/default, ordering, unsupported capability behavior, post-rename failures, and created-directory scope. diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index a01ae55..fe4ffc8 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -30,10 +30,18 @@ Catch the error, parse it with `parseStaleAnchorError`, and retry using the exac ## Use the filesystem adapter ```ts -import { FilesystemPiClient } from 'pi-anchor-edit-core'; +import { FILESYSTEM_DURABILITY_LEVELS, FilesystemPiClient } from 'pi-anchor-edit-core'; +// Existing behavior: temporary-file fsync, then atomic rename. const client = new FilesystemPiClient(); + +// Require an explicit error if the post-rename parent-directory sync is unsupported. +const strictDurabilityClient = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', +}); + const rendered = await client.read({ path: 'src/file.ts' }); ``` -The adapter rejects unsupported binary edits and preserves detected newline style. +The adapter rejects unsupported binary edits and preserves detected newline style. If `FilesystemDurabilityError` is thrown, inspect `destinationVisible`: parent sync runs after rename, so the destination must be re-read rather than blindly retried. diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index 2267227..9aa1cf2 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -6,6 +6,12 @@ Authorize the caller-selected path, keep a recoverable copy when content is impo The filesystem adapter performs same-directory temporary-file replacement. It records destination existence, file identity, byte length, permission mode, and a SHA-256 byte digest when loading, then revalidates immediately before replacement. A detected content change, permission-mode change, destination replacement, deletion, or missing-to-created race preserves the concurrent state and removes the adapter's temporary file. Symbolic links are rejected. Editing one name in a hard-link set replaces only that directory entry, so sibling hard links retain the old inode and content. +## Durability selection + +`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to sync the direct parent after rename. The latter does not synchronize recursively created ancestor entries. + +Parent-directory support is detected by attempting the real operation. `unsupportedDirectorySync: 'degrade'` (default) absorbs only a classified unsupported-capability error and completes at file durability. `strict` throws instead. Unclassified I/O or permission failures always throw. See [`ADR 0001`](adr/0001-filesystem-crash-durability.md). + ## Classified errors | Error prefix | Meaning | Safe caller action | @@ -23,6 +29,8 @@ The filesystem adapter performs same-directory temporary-file replacement. It re | `[E_DECODE_LOSS]` | UTF-8 decoding would replace invalid bytes. | Do not rewrite. Determine the real encoding and use an encoding-aware conversion with an explicit backup. | | `[E_UNSUPPORTED_FILE]` | The path is a directory, symbolic link, or unsupported special file. | Resolve and authorize a regular-file path explicitly; do not weaken the classifier or follow the link implicitly. | | `[E_CONCURRENT_DESTINATION]` | The destination's existence, identity, length, permission mode, or byte digest changed after it was loaded and before replacement. | Preserve the concurrent destination, re-read it, reassess intent, and retry only with current anchors. | +| `[E_DIRECTORY_SYNC_UNSUPPORTED]` | Strict parent-directory sync was unsupported after rename. The destination is already visible. | Do not replay blindly. Re-read the destination; choose whether file durability is acceptable or provision a supported filesystem. | +| `[E_DURABILITY_UNCONFIRMED]` | Parent-directory sync failed after rename for a reason other than a classified unsupported capability. The destination is already visible. | Preserve the visible destination, investigate the underlying `cause`, and re-read before any further mutation. | `Operation aborted` means the supplied abort signal was already cancelled; leave content unchanged, determine whether the caller still wants the operation, then re-read before retrying. @@ -32,6 +40,10 @@ Node filesystem errors such as `EACCES`, `EPERM`, `EROFS`, `ENOSPC`, `EMFILE`, a The optimistic guard deliberately compares permission mode and a byte digest rather than trusting size and timestamps, so permission-only changes, same-size edits, and coarse-time metadata collisions are detected. It is still a best-effort check, not an atomic compare-and-swap: another writer can change the destination after revalidation and before `rename`. Callers must not treat a successful edit as proof that no writer raced in that residual interval. +`FilesystemDurabilityError` always represents a post-rename boundary and exposes `destinationVisible: true`, the destination path, requested durability, stable error code, and original `cause`. Cleanup removes any temporary-path residue but never removes or rolls back the renamed destination. A strict Windows parent sync normally reports `E_DIRECTORY_SYNC_UNSUPPORTED` with an `EPERM` cause; degrade mode treats that specific capability result as file durability. + +A successful sync reports only what the operating system and storage stack make observable. Hardware, network filesystems, virtualized filesystems, and mount options can weaken persistence guarantees. + ## Escalation data A safe report contains: @@ -39,6 +51,7 @@ A safe report contains: - package version, Node version, operating system, and filesystem type; - the error prefix or Node error code; - whether the destination existed and whether link capability was involved; and +- selected durability and unsupported-directory-sync behavior; - a minimal synthetic fixture with no proprietary text. Never attach real credentials, tokens, private paths, customer data, complete file content, or unredacted stale-anchor output. Report suspected security impact only through the private route in [`SECURITY.md`](../SECURITY.md). diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md index f284dc4..9eee645 100644 --- a/docs/PERFORMANCE.md +++ b/docs/PERFORMANCE.md @@ -1,22 +1,24 @@ # Performance Baseline -Build and run the hash benchmark: +Build and run both the pure hash and end-to-end durability benchmarks: ```bash npm run benchmark ``` -The fixture runs `computeLineHash` over deterministic TypeScript-like lines and reports mean and p99 milliseconds per operation. It excludes filesystem I/O and higher-level Pi tool serialization. +The benchmark prints machine-readable JSON. The hash fixture runs `computeLineHash` over deterministic TypeScript-like lines. The filesystem fixture performs 25 measured edits per durability level after three warmups and includes loading, classification, transformation, same-directory temporary-file write, selected syncs, destination revalidation, and rename. `file-and-parent-directory` uses `unsupportedDirectorySync: 'degrade'` so the same command runs on Windows and filesystems without directory fsync. -## Initial local result +## Local result -Measured 2026-07-11 on Windows with Node 24.18.0: +Measured 2026-07-16 on Windows x64 with Node 24.18.0. Values are milliseconds per operation and are a local comparison, not a cross-host SLO. -- rounds: 100 -- operations per round: 10,000 -- mean: 0.001019 ms per line -- p99: 0.001282 ms per line +| Operation / durability | Mean | p95 | p99 | +|---|---:|---:|---:| +| `computeLineHash` (100 × 10,000 operations) | 0.001030 | 0.001097 | 0.001349 | +| Filesystem edit: `none` | 2.484 | 2.954 | 3.110 | +| Filesystem edit: `file` (default) | 5.324 | 6.457 | 6.622 | +| Filesystem edit: `file-and-parent-directory` | 5.586 | 6.663 | 6.983 | -The result is comfortably below the 0.1 ms target. This is a local pure-hash baseline, not an end-to-end filesystem edit SLO. +On this Windows host, file sync added about 2.84 ms mean latency over `none`. Attempting parent-directory sync added about 0.26 ms over `file`; Windows returned the classified unsupported `EPERM`, so the configured degrade policy completed at file durability. This does **not** measure the cost of a successful directory fsync. Linux/macOS and different filesystems/storage can differ substantially. -The initial target in [`PRODUCT.md`](PRODUCT.md) is p99 below 0.1 ms per line. Record Node, OS, rounds, and operations per round when publishing updated results. +The pure hash result remains comfortably below the 0.1 ms target in [`PRODUCT.md`](PRODUCT.md). No end-to-end edit budget is enforced yet. When publishing updated results, record Node version, OS/filesystem, rounds, selected unsupported-directory policy, whether parent sync succeeded or degraded, and the complete JSON output. diff --git a/src/filesystem-client.ts b/src/filesystem-client.ts index fb0e411..ea88af8 100644 --- a/src/filesystem-client.ts +++ b/src/filesystem-client.ts @@ -1,4 +1,5 @@ import { chmod, lstat, mkdir, open, readFile, rename, rm } from 'node:fs/promises'; +import type { FileHandle } from 'node:fs/promises'; import { createHash, randomUUID } from 'node:crypto'; import { basename, dirname, join } from 'node:path'; import { formatAnchors } from './anchors.js'; @@ -7,6 +8,58 @@ import { applyHashlineEdits, resolveEditAnchors, type HashlineToolEdit } from '. import { detectLineEnding, normalizeToLF, restoreLineEndings } from './text.js'; import type { EditParams, PiClient, ReadParams } from './types.js'; +export const FILESYSTEM_DURABILITY_LEVELS = { + NONE: 'none', + FILE: 'file', + FILE_AND_PARENT_DIRECTORY: 'file-and-parent-directory', +} as const; + +export type FilesystemDurability = + typeof FILESYSTEM_DURABILITY_LEVELS[keyof typeof FILESYSTEM_DURABILITY_LEVELS]; + +export const DEFAULT_FILESYSTEM_DURABILITY: FilesystemDurability = + FILESYSTEM_DURABILITY_LEVELS.FILE; + +export const UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS = { + DEGRADE: 'degrade', + STRICT: 'strict', +} as const; + +export type UnsupportedDirectorySyncBehavior = + typeof UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS[keyof typeof UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS]; + +export const DEFAULT_UNSUPPORTED_DIRECTORY_SYNC_BEHAVIOR: UnsupportedDirectorySyncBehavior = + UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE; + +export type FilesystemPiClientConfig = { + durability?: FilesystemDurability; + unsupportedDirectorySync?: UnsupportedDirectorySyncBehavior; +}; + +export type FilesystemDurabilityErrorCode = + | 'E_DIRECTORY_SYNC_UNSUPPORTED' + | 'E_DURABILITY_UNCONFIRMED'; + +export class FilesystemDurabilityError extends Error { + readonly destinationVisible = true; + + constructor( + readonly code: FilesystemDurabilityErrorCode, + readonly destinationPath: string, + readonly durability: FilesystemDurability, + cause: unknown, + ) { + const detail = code === 'E_DIRECTORY_SYNC_UNSUPPORTED' + ? 'parent-directory synchronization is unsupported' + : 'parent-directory synchronization failed'; + super( + `[${code}] ${destinationPath} was replaced and is visible, but ${detail}; crash durability is not confirmed. Re-read before retrying.`, + { cause }, + ); + this.name = 'FilesystemDurabilityError'; + } +} + function splitLines(text: string): string[] { return text.length === 0 ? [] : text.split(/\r?\n/); } @@ -129,13 +182,90 @@ async function loadText(path: string): Promise { } +const UNSUPPORTED_DIRECTORY_SYNC_CODES = new Set([ + 'EBADF', + 'EISDIR', + 'EINVAL', + 'ENOSYS', + 'ENOTSUP', + 'EOPNOTSUPP', +]); + +function isUnsupportedDirectorySyncError(error: unknown): boolean { + const { code, syscall } = (error as NodeJS.ErrnoException | undefined) ?? {}; + return (typeof code === 'string' && UNSUPPORTED_DIRECTORY_SYNC_CODES.has(code)) + || (process.platform === 'win32' && code === 'EPERM' && syscall === 'fsync'); +} + +function isFilesystemDurability(value: unknown): value is FilesystemDurability { + return Object.values(FILESYSTEM_DURABILITY_LEVELS).includes(value as FilesystemDurability); +} + +function isUnsupportedDirectorySyncBehavior(value: unknown): value is UnsupportedDirectorySyncBehavior { + return Object.values(UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS) + .includes(value as UnsupportedDirectorySyncBehavior); +} + export class FilesystemPiClient implements PiClient { + private readonly durability: FilesystemDurability; + private readonly unsupportedDirectorySync: UnsupportedDirectorySyncBehavior; + + constructor(config: FilesystemPiClientConfig = {}) { + const durability = config.durability ?? DEFAULT_FILESYSTEM_DURABILITY; + const unsupportedDirectorySync = config.unsupportedDirectorySync + ?? DEFAULT_UNSUPPORTED_DIRECTORY_SYNC_BEHAVIOR; + if (!isFilesystemDurability(durability)) { + throw new TypeError(`Unsupported filesystem durability level: ${String(durability)}`); + } + if (!isUnsupportedDirectorySyncBehavior(unsupportedDirectorySync)) { + throw new TypeError( + `Unsupported directory-sync behavior: ${String(unsupportedDirectorySync)}`, + ); + } + this.durability = durability; + this.unsupportedDirectorySync = unsupportedDirectorySync; + } + protected async beforeDestinationRevalidation(_destinationPath: string): Promise {} + protected async applyTemporaryFileMode(temporaryPath: string, mode: number): Promise { + await chmod(temporaryPath, mode); + } + + protected async synchronizeTemporaryFile(handle: FileHandle): Promise { + await handle.sync(); + } + protected async replaceTemporaryFile(temporaryPath: string, destinationPath: string): Promise { await rename(temporaryPath, destinationPath); } + protected async synchronizeParentDirectory(parentPath: string): Promise { + const handle = await open(parentPath, 'r'); + try { + await handle.sync(); + } finally { + await handle.close().catch(() => undefined); + } + } + + private async synchronizeParentAfterRename(parentPath: string, destinationPath: string): Promise { + try { + await this.synchronizeParentDirectory(parentPath); + } catch (error) { + const unsupported = isUnsupportedDirectorySyncError(error); + if (unsupported && this.unsupportedDirectorySync === UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE) { + return; + } + throw new FilesystemDurabilityError( + unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', + destinationPath, + this.durability, + error, + ); + } + } + private async observeDestination(path: string): Promise { let pathBefore: Awaited>; try { @@ -193,20 +323,25 @@ export class FilesystemPiClient implements PiClient { const parent = dirname(path); await mkdir(parent, { recursive: true }); const temporaryPath = join(parent, `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); - let handle: Awaited> | undefined; + let handle: FileHandle | undefined; try { handle = await open(temporaryPath, 'wx', mode ?? 0o666); await handle.writeFile(content, 'utf8'); - await handle.sync(); + if (mode !== undefined) await this.applyTemporaryFileMode(temporaryPath, mode); + if (this.durability !== FILESYSTEM_DURABILITY_LEVELS.NONE) { + await this.synchronizeTemporaryFile(handle); + } await handle.close(); handle = undefined; - if (mode !== undefined) await chmod(temporaryPath, mode); await this.beforeDestinationRevalidation(path); const currentObservation = await this.observeDestination(path); if (!sameObservation(observation, currentObservation)) throw concurrentDestinationError(path); // Best-effort only: the destination can still change after this check and before rename. await this.replaceTemporaryFile(temporaryPath, path); + if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { + await this.synchronizeParentAfterRename(parent, path); + } } finally { await handle?.close().catch(() => undefined); await rm(temporaryPath, { force: true }).catch(() => undefined); diff --git a/test/filesystem-client.test.ts b/test/filesystem-client.test.ts index 1378ad9..d2e3e7a 100644 --- a/test/filesystem-client.test.ts +++ b/test/filesystem-client.test.ts @@ -16,7 +16,15 @@ import { } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { basename, join } from 'node:path'; -import { FilesystemPiClient, loadFileKindAndText } from '../src/index.js'; +import { + DEFAULT_FILESYSTEM_DURABILITY, + FILESYSTEM_DURABILITY_LEVELS, + FilesystemDurabilityError, + FilesystemPiClient, + loadFileKindAndText, + type FilesystemPiClientConfig, +} from '../src/index.js'; +import type { FileHandle } from 'node:fs/promises'; async function fixture(name = 'file.txt'): Promise<{ dir: string; path: string }> { const dir = await mkdtemp(join(tmpdir(), 'pi-anchor-edit-core-')); @@ -45,6 +53,59 @@ class RevalidationRaceClient extends FilesystemPiClient { } } +class OperationRecordingClient extends FilesystemPiClient { + readonly operations: string[] = []; + readonly synchronizedParents: string[] = []; + + constructor(config: FilesystemPiClientConfig = {}) { + super(config); + } + + protected override async applyTemporaryFileMode(temporaryPath: string, mode: number): Promise { + this.operations.push('mode'); + await super.applyTemporaryFileMode(temporaryPath, mode); + } + + protected override async synchronizeTemporaryFile(handle: FileHandle): Promise { + this.operations.push('file-sync'); + await super.synchronizeTemporaryFile(handle); + } + + protected override async replaceTemporaryFile( + temporaryPath: string, + destinationPath: string, + ): Promise { + this.operations.push('rename'); + await super.replaceTemporaryFile(temporaryPath, destinationPath); + } + + protected override async synchronizeParentDirectory(parentPath: string): Promise { + this.operations.push('parent-sync'); + this.synchronizedParents.push(parentPath); + } +} + +class FailingFileSyncClient extends FilesystemPiClient { + protected override async synchronizeTemporaryFile(): Promise { + const error = new Error('simulated file sync failure') as NodeJS.ErrnoException; + error.code = 'EIO'; + throw error; + } +} + +class FailingDirectorySyncClient extends FilesystemPiClient { + constructor(config: FilesystemPiClientConfig, private readonly failureCode: string) { + super(config); + } + + protected override async synchronizeParentDirectory(): Promise { + const error = new Error(`simulated directory sync failure: ${this.failureCode}`) as NodeJS.ErrnoException; + error.code = this.failureCode; + error.syscall = 'fsync'; + throw error; + } +} + function expectedConcurrentDestinationError(path: string): string { return `[E_CONCURRENT_DESTINATION] Refusing to replace ${path}: destination changed after it was loaded. Re-read and retry with current anchors.`; } @@ -118,6 +179,198 @@ test('preserves CRLF, UTF-8 BOM, and an existing permission mode', async (t) => } }); +test('exports durability levels and preserves file sync as the default', async () => { + assert.deepEqual(FILESYSTEM_DURABILITY_LEVELS, { + NONE: 'none', + FILE: 'file', + FILE_AND_PARENT_DIRECTORY: 'file-and-parent-directory', + }); + assert.equal(DEFAULT_FILESYSTEM_DURABILITY, FILESYSTEM_DURABILITY_LEVELS.FILE); + + const { path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new OperationRecordingClient(); + const preview = await client.read({ path }); + await client.edit({ path, edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }] }); + + assert.deepEqual(client.operations, ['mode', 'file-sync', 'rename']); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); +}); + +test('supports no-sync, file, and file-and-parent-directory operation sequences', async () => { + const cases = [ + { + durability: FILESYSTEM_DURABILITY_LEVELS.NONE, + expected: ['mode', 'rename'], + }, + { + durability: FILESYSTEM_DURABILITY_LEVELS.FILE, + expected: ['mode', 'file-sync', 'rename'], + }, + { + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + expected: ['mode', 'file-sync', 'rename', 'parent-sync'], + }, + ] as const; + + for (const { durability, expected } of cases) { + const { path } = await fixture(`${durability}.txt`); + await writeFile(path, 'one\ntwo'); + const client = new OperationRecordingClient({ durability }); + const preview = await client.read({ path }); + await client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }); + assert.deepEqual(client.operations, expected, durability); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + } +}); + +test('syncs only the direct parent when recursive directory creation was needed', async () => { + const { dir, path } = await fixture('one/two/new.txt'); + const client = new OperationRecordingClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }); + + await client.edit({ path, edits: [{ op: 'prepend', lines: ['created'] }] }); + + assert.deepEqual(client.operations, ['file-sync', 'rename', 'parent-sync']); + assert.deepEqual(client.synchronizedParents, [join(dir, 'one', 'two')]); + assert.equal(await readFile(path, 'utf8'), 'created'); + await assertNoTemporaryFiles(join(dir, 'one', 'two')); +}); + +test('default policy degrades a classified unsupported parent sync to file durability', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectorySyncClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }, 'EINVAL'); + const preview = await client.read({ path }); + + await client.edit({ path, edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }] }); + + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); + +test('strict unsupported parent sync reports that the renamed destination is visible', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectorySyncClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', + }, 'ENOTSUP'); + const preview = await client.read({ path }); + + await assert.rejects( + () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), + (error: unknown) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DIRECTORY_SYNC_UNSUPPORTED'); + assert.equal(error.destinationVisible, true); + assert.equal(error.destinationPath, path); + assert.equal(error.durability, FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY); + assert.equal((error.cause as NodeJS.ErrnoException).code, 'ENOTSUP'); + return true; + }, + ); + + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); + +test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectorySyncClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'degrade', + }, 'EIO'); + const preview = await client.read({ path }); + + await assert.rejects( + () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), + (error: unknown) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DURABILITY_UNCONFIRMED'); + assert.equal(error.destinationVisible, true); + assert.match(error.message, /was replaced and is visible/); + return true; + }, + ); + + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); + +test('pre-rename file sync failure preserves the original and cleans the temporary file', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingFileSyncClient(); + const preview = await client.read({ path }); + + await assert.rejects( + () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), + /simulated file sync failure/, + ); + + assert.equal(await readFile(path, 'utf8'), 'one\ntwo'); + await assertNoTemporaryFiles(dir); +}); + +test('real directory sync capability has explicit hosted-platform behavior', async (t) => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', + }); + const preview = await client.read({ path }); + const edit = () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }); + + if (process.platform === 'win32') { + await assert.rejects(edit, (error: unknown) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DIRECTORY_SYNC_UNSUPPORTED'); + assert.equal((error.cause as NodeJS.ErrnoException).code, 'EPERM'); + return true; + }); + t.diagnostic('Windows directory fsync reported EPERM and strict mode surfaced it'); + } else { + await edit(); + } + + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); +}); + +test('rejects invalid runtime durability configuration', () => { + assert.throws( + () => new FilesystemPiClient({ durability: 'invalid' as FilesystemPiClientConfig['durability'] }), + /Unsupported filesystem durability level/, + ); + assert.throws( + () => new FilesystemPiClient({ + unsupportedDirectorySync: 'invalid' as FilesystemPiClientConfig['unsupportedDirectorySync'], + }), + /Unsupported directory-sync behavior/, + ); +}); + test('replacement failure leaves the original intact and removes the temporary file', async () => { const client = new FailingReplacementClient(); const { dir, path } = await fixture(); From 24a646e67b8e423bc220baf2f37e0addc8a4e955 Mon Sep 17 00:00:00 2001 From: cervantesh <11169707+cervantesh@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:48:25 -0400 Subject: [PATCH 3/5] fix: pin parent directory before rename --- CHANGELOG.md | 2 +- README.md | 2 +- SECURITY.md | 2 +- dist/src/filesystem-client.d.ts | 9 ++- dist/src/filesystem-client.js | 55 +++++++++---- dist/test/filesystem-client.test.js | 41 +++++++++- docs/ARCHITECTURE.md | 8 +- docs/EXAMPLES.md | 4 +- docs/OPERATIONS.md | 8 +- docs/adr/0001-filesystem-crash-durability.md | 19 +++-- src/filesystem-client.ts | 84 ++++++++++++++------ test/filesystem-client.test.ts | 52 +++++++++++- 12 files changed, 213 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb9c2b8..2107b30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ - package contents exclude compiled tests and include complete repository provenance metadata. - filesystem atomic replacement now performs best-effort optimistic destination revalidation with identity, permission-mode, and SHA-256 byte-digest evidence, returning `[E_CONCURRENT_DESTINATION]` while preserving detected concurrent state and cleaning temporary files. - callers can select `none`, `file`, or `file-and-parent-directory` durability through `FilesystemPiClient`; the default remains file sync. -- preserved destination mode is now applied before the final temporary-file sync, and parent-directory sync capability is detected from the actual filesystem operation. +- preserved destination mode is now applied before the final temporary-file sync, and parent-directory capability is detected from the actual filesystem operation using a pre-rename pinned handle. ### Security diff --git a/README.md b/README.md index bf047d6..1521ed6 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ This is best-effort optimistic detection, not compare-and-swap. A residual race ### `[E_DIRECTORY_SYNC_UNSUPPORTED]` / `[E_DURABILITY_UNCONFIRMED]` -These `FilesystemDurabilityError` failures occur after rename. The edited destination is already visible (`destinationVisible === true`), but requested parent-directory crash durability was not confirmed. Do not blindly replay the edit or attempt to restore the old file. Re-read the destination before deciding how to recover. A classified unsupported operation is absorbed only when `unsupportedDirectorySync: 'degrade'`; other sync failures always throw. +`FilesystemDurabilityError` distinguishes the commit boundary. If opening the parent fails before rename, `destinationVisible === false` and the original destination remains unchanged. If syncing the pinned parent handle fails after rename, `destinationVisible === true`; the edited destination is already visible but crash durability was not confirmed. Do not blindly replay or roll back a visible edit—re-read first. A classified unsupported operation is absorbed only when `unsupportedDirectorySync: 'degrade'`; other failures always throw. ## Development diff --git a/SECURITY.md b/SECURITY.md index b144677..e16abc6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -23,7 +23,7 @@ We aim to acknowledge a private report within 3 business days, provide an initia Anchor diagnostics can quote nearby file content in `>>> LINE#HASH:content` retry lines. Treat all diagnostics as potentially sensitive. Redact or replace them with synthetic examples before sharing, and never send raw diagnostics to telemetry by default. -Filesystem sync is a durability control, not an authorization or confidentiality boundary. `none` intentionally omits explicit syncs; the default `file` level does not make the directory rename crash-durable; and `file-and-parent-directory` depends on operating-system, filesystem, mount, virtualization, and hardware behavior. A degraded unsupported directory sync confirms only file durability. Strict post-rename failures leave the new destination visible and must not trigger blind replay or rollback. +Filesystem sync is a durability control, not an authorization or confidentiality boundary. `none` intentionally omits explicit syncs; the default `file` level does not make the directory rename crash-durable; and `file-and-parent-directory` depends on operating-system, filesystem, mount, virtualization, and hardware behavior. A degraded unsupported directory sync confirms only file durability. Strict failures expose `destinationVisible`; visible post-rename failures must not trigger blind replay or rollback. See [`docs/OPERATIONS.md`](docs/OPERATIONS.md) for safe recovery actions and [`docs/RELEASING.md`](docs/RELEASING.md) for immutable release recovery. diff --git a/dist/src/filesystem-client.d.ts b/dist/src/filesystem-client.d.ts index dabcf9b..f3f31e3 100644 --- a/dist/src/filesystem-client.d.ts +++ b/dist/src/filesystem-client.d.ts @@ -22,8 +22,8 @@ export declare class FilesystemDurabilityError extends Error { readonly code: FilesystemDurabilityErrorCode; readonly destinationPath: string; readonly durability: FilesystemDurability; - readonly destinationVisible = true; - constructor(code: FilesystemDurabilityErrorCode, destinationPath: string, durability: FilesystemDurability, cause: unknown); + readonly destinationVisible: boolean; + constructor(code: FilesystemDurabilityErrorCode, destinationPath: string, durability: FilesystemDurability, destinationVisible: boolean, cause: unknown); } export declare class FilesystemPiClient implements PiClient { private readonly durability; @@ -33,7 +33,10 @@ export declare class FilesystemPiClient implements PiClient { protected applyTemporaryFileMode(temporaryPath: string, mode: number): Promise; protected synchronizeTemporaryFile(handle: FileHandle): Promise; protected replaceTemporaryFile(temporaryPath: string, destinationPath: string): Promise; - protected synchronizeParentDirectory(parentPath: string): Promise; + protected openParentDirectoryForSync(parentPath: string): Promise; + protected synchronizeParentDirectory(handle: FileHandle, _parentPath: string): Promise; + private handleDirectorySyncFailure; + private openParentBeforeRename; private synchronizeParentAfterRename; private observeDestination; private atomicWrite; diff --git a/dist/src/filesystem-client.js b/dist/src/filesystem-client.js index 1468c69..91e6450 100644 --- a/dist/src/filesystem-client.js +++ b/dist/src/filesystem-client.js @@ -20,15 +20,22 @@ export class FilesystemDurabilityError extends Error { code; destinationPath; durability; - destinationVisible = true; - constructor(code, destinationPath, durability, cause) { + destinationVisible; + constructor(code, destinationPath, durability, destinationVisible, cause) { const detail = code === 'E_DIRECTORY_SYNC_UNSUPPORTED' ? 'parent-directory synchronization is unsupported' : 'parent-directory synchronization failed'; - super(`[${code}] ${destinationPath} was replaced and is visible, but ${detail}; crash durability is not confirmed. Re-read before retrying.`, { cause }); + const boundary = destinationVisible + ? `${destinationPath} was replaced and is visible` + : `${destinationPath} was not replaced`; + const recovery = destinationVisible + ? 'Crash durability is not confirmed. Re-read before retrying.' + : 'The original destination is unchanged.'; + super(`[${code}] ${boundary}, but ${detail}. ${recovery}`, { cause }); this.code = code; this.destinationPath = destinationPath; this.durability = durability; + this.destinationVisible = destinationVisible; this.name = 'FilesystemDurabilityError'; } } @@ -181,25 +188,34 @@ export class FilesystemPiClient { async replaceTemporaryFile(temporaryPath, destinationPath) { await rename(temporaryPath, destinationPath); } - async synchronizeParentDirectory(parentPath) { - const handle = await open(parentPath, 'r'); + async openParentDirectoryForSync(parentPath) { + return open(parentPath, 'r'); + } + async synchronizeParentDirectory(handle, _parentPath) { + await handle.sync(); + } + handleDirectorySyncFailure(error, destinationPath, destinationVisible) { + const unsupported = isUnsupportedDirectorySyncError(error); + if (unsupported && this.unsupportedDirectorySync === UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE) { + return; + } + throw new FilesystemDurabilityError(unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', destinationPath, this.durability, destinationVisible, error); + } + async openParentBeforeRename(parentPath, destinationPath) { try { - await handle.sync(); + return await this.openParentDirectoryForSync(parentPath); } - finally { - await handle.close().catch(() => undefined); + catch (error) { + this.handleDirectorySyncFailure(error, destinationPath, false); + return undefined; } } - async synchronizeParentAfterRename(parentPath, destinationPath) { + async synchronizeParentAfterRename(handle, parentPath, destinationPath) { try { - await this.synchronizeParentDirectory(parentPath); + await this.synchronizeParentDirectory(handle, parentPath); } catch (error) { - const unsupported = isUnsupportedDirectorySyncError(error); - if (unsupported && this.unsupportedDirectorySync === UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE) { - return; - } - throw new FilesystemDurabilityError(unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', destinationPath, this.durability, error); + this.handleDirectorySyncFailure(error, destinationPath, true); } } async observeDestination(path) { @@ -254,6 +270,7 @@ export class FilesystemPiClient { await mkdir(parent, { recursive: true }); const temporaryPath = join(parent, `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); let handle; + let parentHandle; try { handle = await open(temporaryPath, 'wx', mode ?? 0o666); await handle.writeFile(content, 'utf8'); @@ -264,18 +281,22 @@ export class FilesystemPiClient { } await handle.close(); handle = undefined; + if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { + parentHandle = await this.openParentBeforeRename(parent, path); + } await this.beforeDestinationRevalidation(path); const currentObservation = await this.observeDestination(path); if (!sameObservation(observation, currentObservation)) throw concurrentDestinationError(path); // Best-effort only: the destination can still change after this check and before rename. await this.replaceTemporaryFile(temporaryPath, path); - if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { - await this.synchronizeParentAfterRename(parent, path); + if (parentHandle !== undefined) { + await this.synchronizeParentAfterRename(parentHandle, parent, path); } } finally { await handle?.close().catch(() => undefined); + await parentHandle?.close().catch(() => undefined); await rm(temporaryPath, { force: true }).catch(() => undefined); } } diff --git a/dist/test/filesystem-client.test.js b/dist/test/filesystem-client.test.js index fdc6f84..7eb681b 100644 --- a/dist/test/filesystem-client.test.js +++ b/dist/test/filesystem-client.test.js @@ -42,13 +42,17 @@ class OperationRecordingClient extends FilesystemPiClient { this.operations.push('file-sync'); await super.synchronizeTemporaryFile(handle); } + async openParentDirectoryForSync(parentPath) { + this.operations.push('parent-open'); + this.synchronizedParents.push(parentPath); + return super.openParentDirectoryForSync(parentPath); + } async replaceTemporaryFile(temporaryPath, destinationPath) { this.operations.push('rename'); await super.replaceTemporaryFile(temporaryPath, destinationPath); } - async synchronizeParentDirectory(parentPath) { + async synchronizeParentDirectory(_handle, _parentPath) { this.operations.push('parent-sync'); - this.synchronizedParents.push(parentPath); } } class FailingFileSyncClient extends FilesystemPiClient { @@ -71,6 +75,14 @@ class FailingDirectorySyncClient extends FilesystemPiClient { throw error; } } +class FailingDirectoryOpenClient extends FilesystemPiClient { + async openParentDirectoryForSync() { + const error = new Error('simulated directory open failure'); + error.code = 'EISDIR'; + error.syscall = 'open'; + throw error; + } +} function expectedConcurrentDestinationError(path) { return `[E_CONCURRENT_DESTINATION] Refusing to replace ${path}: destination changed after it was loaded. Re-read and retry with current anchors.`; } @@ -157,7 +169,7 @@ test('supports no-sync, file, and file-and-parent-directory operation sequences' }, { durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, - expected: ['mode', 'file-sync', 'rename', 'parent-sync'], + expected: ['mode', 'file-sync', 'parent-open', 'rename', 'parent-sync'], }, ]; for (const { durability, expected } of cases) { @@ -179,7 +191,7 @@ test('syncs only the direct parent when recursive directory creation was needed' durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, }); await client.edit({ path, edits: [{ op: 'prepend', lines: ['created'] }] }); - assert.deepEqual(client.operations, ['file-sync', 'rename', 'parent-sync']); + assert.deepEqual(client.operations, ['file-sync', 'parent-open', 'rename', 'parent-sync']); assert.deepEqual(client.synchronizedParents, [join(dir, 'one', 'two')]); assert.equal(await readFile(path, 'utf8'), 'created'); await assertNoTemporaryFiles(join(dir, 'one', 'two')); @@ -218,6 +230,27 @@ test('strict unsupported parent sync reports that the renamed destination is vis assert.equal(await readFile(path, 'utf8'), 'one\npatched'); await assertNoTemporaryFiles(dir); }); +test('strict unsupported parent open fails before rename and cleans the temporary file', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectoryOpenClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', + }); + const preview = await client.read({ path }); + await assert.rejects(() => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), (error) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DIRECTORY_SYNC_UNSUPPORTED'); + assert.equal(error.destinationVisible, false); + assert.match(error.message, /was not replaced/); + return true; + }); + assert.equal(await readFile(path, 'utf8'), 'one\ntwo'); + await assertNoTemporaryFiles(dir); +}); test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { const { dir, path } = await fixture(); await writeFile(path, 'one\ntwo'); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5cb8f19..e62f4f2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -20,8 +20,8 @@ 5. The engine produces new content or throws a classified actionable error. 6. The filesystem adapter classifies the path before decoding, captures destination existence, file identity, byte length, permission mode, and a SHA-256 digest, and preserves newline/BOM/mode behavior. 7. The adapter writes a same-directory temporary file, applies preserved mode, and performs the configured file sync unless `none` was selected. -8. Immediately before atomic replacement, it re-observes the destination and aborts with `[E_CONCURRENT_DESTINATION]` if existence, identity, length, permission mode, or digest differs; otherwise it renames the temporary file. -9. `file-and-parent-directory` synchronizes the destination's direct parent after rename, degrading only classified unsupported capability under the configured policy. +8. For `file-and-parent-directory`, it opens and retains a handle to the direct parent before replacement. It then re-observes the destination and aborts with `[E_CONCURRENT_DESTINATION]` if existence, identity, length, permission mode, or digest differs; otherwise it renames the temporary file. +9. `file-and-parent-directory` synchronizes the retained parent handle after rename, so path replacement cannot redirect the sync; only classified unsupported capability degrades under the configured policy. ## Invariants @@ -36,8 +36,8 @@ - Revalidation uses permission-mode and byte-digest evidence rather than size or timestamps alone, so permission-only changes, same-size content changes, and changes hidden by coarse timestamp resolution are detected. - The concurrency guard is optimistic and best-effort, not compare-and-swap: a destination can still change in the residual interval after revalidation and before `rename`. The library makes no false CAS guarantee. - Atomic visibility and crash durability are separate: `none` performs no explicit sync, `file` syncs the temporary file and remains the default, and `file-and-parent-directory` additionally syncs the direct parent after rename. -- Preserved mode is applied before the selected final file sync. Parent synchronization never precedes rename. -- A parent-sync error is post-commit: the destination is visible and is not rolled back. `FilesystemDurabilityError.destinationVisible` records this recovery boundary. +- Preserved mode is applied before the selected final file sync. The direct parent is opened before rename and the retained handle is synchronized afterward. +- A parent-open error is pre-commit (`destinationVisible: false`); a parent-sync error is post-commit (`destinationVisible: true`) and is never rolled back. - Parent sync covers only the direct parent. Recursively created ancestors are not included in the durability claim. ## Architecture decisions diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index fe4ffc8..2175270 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -35,7 +35,7 @@ import { FILESYSTEM_DURABILITY_LEVELS, FilesystemPiClient } from 'pi-anchor-edit // Existing behavior: temporary-file fsync, then atomic rename. const client = new FilesystemPiClient(); -// Require an explicit error if the post-rename parent-directory sync is unsupported. +// Require an explicit error if opening or syncing the parent is unsupported. const strictDurabilityClient = new FilesystemPiClient({ durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, unsupportedDirectorySync: 'strict', @@ -44,4 +44,4 @@ const strictDurabilityClient = new FilesystemPiClient({ const rendered = await client.read({ path: 'src/file.ts' }); ``` -The adapter rejects unsupported binary edits and preserves detected newline style. If `FilesystemDurabilityError` is thrown, inspect `destinationVisible`: parent sync runs after rename, so the destination must be re-read rather than blindly retried. +The adapter rejects unsupported binary edits and preserves detected newline style. If `FilesystemDurabilityError` is thrown, inspect `destinationVisible`: `false` preserves the original before rename; `true` means the pinned parent failed to sync after rename, so re-read rather than blindly retry. diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index 9aa1cf2..331850b 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -8,7 +8,7 @@ The filesystem adapter performs same-directory temporary-file replacement. It re ## Durability selection -`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to sync the direct parent after rename. The latter does not synchronize recursively created ancestor entries. +`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to open and pin the direct parent before rename and sync that retained handle afterward. The latter does not synchronize recursively created ancestor entries. Parent-directory support is detected by attempting the real operation. `unsupportedDirectorySync: 'degrade'` (default) absorbs only a classified unsupported-capability error and completes at file durability. `strict` throws instead. Unclassified I/O or permission failures always throw. See [`ADR 0001`](adr/0001-filesystem-crash-durability.md). @@ -29,8 +29,8 @@ Parent-directory support is detected by attempting the real operation. `unsuppor | `[E_DECODE_LOSS]` | UTF-8 decoding would replace invalid bytes. | Do not rewrite. Determine the real encoding and use an encoding-aware conversion with an explicit backup. | | `[E_UNSUPPORTED_FILE]` | The path is a directory, symbolic link, or unsupported special file. | Resolve and authorize a regular-file path explicitly; do not weaken the classifier or follow the link implicitly. | | `[E_CONCURRENT_DESTINATION]` | The destination's existence, identity, length, permission mode, or byte digest changed after it was loaded and before replacement. | Preserve the concurrent destination, re-read it, reassess intent, and retry only with current anchors. | -| `[E_DIRECTORY_SYNC_UNSUPPORTED]` | Strict parent-directory sync was unsupported after rename. The destination is already visible. | Do not replay blindly. Re-read the destination; choose whether file durability is acceptable or provision a supported filesystem. | -| `[E_DURABILITY_UNCONFIRMED]` | Parent-directory sync failed after rename for a reason other than a classified unsupported capability. The destination is already visible. | Preserve the visible destination, investigate the underlying `cause`, and re-read before any further mutation. | +| `[E_DIRECTORY_SYNC_UNSUPPORTED]` | Strict parent-directory open or sync was unsupported. `destinationVisible` identifies whether rename occurred. | If false, the original remains unchanged. If true, do not replay blindly: re-read and decide whether file durability is acceptable or provision a supported filesystem. | +| `[E_DURABILITY_UNCONFIRMED]` | Parent-directory open/sync failed for an unclassified reason. `destinationVisible` identifies whether rename occurred. | Preserve/re-read a visible destination; otherwise investigate the underlying `cause` before retrying. | `Operation aborted` means the supplied abort signal was already cancelled; leave content unchanged, determine whether the caller still wants the operation, then re-read before retrying. @@ -40,7 +40,7 @@ Node filesystem errors such as `EACCES`, `EPERM`, `EROFS`, `ENOSPC`, `EMFILE`, a The optimistic guard deliberately compares permission mode and a byte digest rather than trusting size and timestamps, so permission-only changes, same-size edits, and coarse-time metadata collisions are detected. It is still a best-effort check, not an atomic compare-and-swap: another writer can change the destination after revalidation and before `rename`. Callers must not treat a successful edit as proof that no writer raced in that residual interval. -`FilesystemDurabilityError` always represents a post-rename boundary and exposes `destinationVisible: true`, the destination path, requested durability, stable error code, and original `cause`. Cleanup removes any temporary-path residue but never removes or rolls back the renamed destination. A strict Windows parent sync normally reports `E_DIRECTORY_SYNC_UNSUPPORTED` with an `EPERM` cause; degrade mode treats that specific capability result as file durability. +`FilesystemDurabilityError` exposes the destination path, requested durability, stable error code, original `cause`, and commit boundary. `destinationVisible: false` means parent open failed before rename and cleanup preserved the original; `true` means the retained parent handle failed to sync after rename, so cleanup never removes or rolls back the visible destination. A strict Windows parent sync normally reports `E_DIRECTORY_SYNC_UNSUPPORTED` with an `EPERM`/`fsync` cause; degrade mode treats that specific capability result as file durability. A successful sync reports only what the operating system and storage stack make observable. Hardware, network filesystems, virtualized filesystems, and mount options can weaken persistence guarantees. diff --git a/docs/adr/0001-filesystem-crash-durability.md b/docs/adr/0001-filesystem-crash-durability.md index 5730d18..74561ab 100644 --- a/docs/adr/0001-filesystem-crash-durability.md +++ b/docs/adr/0001-filesystem-crash-durability.md @@ -36,17 +36,20 @@ The write sequence is: 2. apply the preserved destination mode to the temporary file, when one exists; 3. perform the selected final temporary-file sync (`file` and `file-and-parent-directory` only); 4. close the temporary-file handle; -5. revalidate the destination and atomically rename; -6. for `file-and-parent-directory`, sync the destination's direct parent directory. +5. for `file-and-parent-directory`, open and retain a handle to the destination's direct parent; +6. revalidate the destination and atomically rename; +7. for `file-and-parent-directory`, sync the retained parent handle and close it. Applying mode before the final file sync includes the mode metadata in the best available file durability boundary. +Opening the parent before rename pins the directory that receives the replacement. Reopening the parent path afterward could synchronize a different directory if another process renamed or replaced that path. + ### Unsupported parent-directory synchronization -Capability is detected from the actual open/sync operation on the destination's parent rather than from an operating-system allowlist. Known unsupported-operation error codes are classified explicitly. Windows `EPERM` from directory sync is also classified as unsupported; permission errors not identified as an unsupported capability remain real failures. +Capability is detected from the actual open/sync operation on the destination's parent rather than from an operating-system allowlist. Known unsupported-operation error codes are classified explicitly. Windows `EPERM` from the `fsync` syscall is also classified as unsupported; permission errors from opening the directory remain real failures. -- `degrade`: a classified unsupported directory-sync result completes successfully at file durability. The caller chose best-effort parent-directory durability and must not interpret success as confirmation that the rename survived a crash on that filesystem. -- `strict`: the same classified result throws an explicit durability error. +- `degrade`: a classified unsupported directory open or sync result completes successfully at file durability. The caller chose best-effort parent-directory durability and must not interpret success as confirmation that the rename survived a crash on that filesystem. +- `strict`: the same classified result throws an explicit durability error. If opening the parent failed, this occurs before rename with `destinationVisible: false`; if sync failed, it occurs after rename with `destinationVisible: true`. - Any unclassified directory open/sync failure throws regardless of policy. No parent-directory capability result is cached globally because different paths can reside on filesystems with different behavior. @@ -55,6 +58,8 @@ No parent-directory capability result is cached globally because different paths Parent-directory sync occurs after rename, so any failure from that step happens after the destination has been committed and is visible. The operation throws a `FilesystemDurabilityError` with `destinationVisible: true`; it does not roll back, delete, or restore the visible destination. Retrying the original edit blindly is unsafe. Callers must inspect/re-read the destination and decide whether another durability attempt or edit is appropriate. +The retained handle prevents a path replacement between rename and sync from redirecting synchronization to a different directory. + In `degrade` mode, only a classified unsupported-capability result is absorbed. Other post-rename failures still throw `FilesystemDurabilityError` and carry the original error as `cause`. ### Created-directory scope @@ -72,12 +77,12 @@ Protected filesystem-operation seams remain available for deterministic ordering - Existing callers retain file-sync behavior. - Callers can choose lower latency or stronger rename durability explicitly. - Unsupported filesystems and Windows have deterministic degrade/strict behavior. -- Post-commit errors cannot be mistaken for pre-commit failures. +- Pre-commit and post-commit durability errors are distinguished by `destinationVisible`. - Mode preservation is ordered before the final file sync. ### Negative -- `file-and-parent-directory` adds a directory open and sync after every successful rename. +- `file-and-parent-directory` adds a directory open before rename and a sync afterward. - `degrade` success cannot prove parent-directory durability; strict mode is required when lack of that capability must be surfaced. - Newly created directory hierarchies remain outside the guarantee. - Filesystem and storage hardware may still weaken or ignore sync semantics beyond what Node.js can observe. diff --git a/src/filesystem-client.ts b/src/filesystem-client.ts index ea88af8..c14b169 100644 --- a/src/filesystem-client.ts +++ b/src/filesystem-client.ts @@ -41,21 +41,23 @@ export type FilesystemDurabilityErrorCode = | 'E_DURABILITY_UNCONFIRMED'; export class FilesystemDurabilityError extends Error { - readonly destinationVisible = true; - constructor( readonly code: FilesystemDurabilityErrorCode, readonly destinationPath: string, readonly durability: FilesystemDurability, + readonly destinationVisible: boolean, cause: unknown, ) { const detail = code === 'E_DIRECTORY_SYNC_UNSUPPORTED' ? 'parent-directory synchronization is unsupported' : 'parent-directory synchronization failed'; - super( - `[${code}] ${destinationPath} was replaced and is visible, but ${detail}; crash durability is not confirmed. Re-read before retrying.`, - { cause }, - ); + const boundary = destinationVisible + ? `${destinationPath} was replaced and is visible` + : `${destinationPath} was not replaced`; + const recovery = destinationVisible + ? 'Crash durability is not confirmed. Re-read before retrying.' + : 'The original destination is unchanged.'; + super(`[${code}] ${boundary}, but ${detail}. ${recovery}`, { cause }); this.name = 'FilesystemDurabilityError'; } } @@ -240,29 +242,56 @@ export class FilesystemPiClient implements PiClient { await rename(temporaryPath, destinationPath); } - protected async synchronizeParentDirectory(parentPath: string): Promise { - const handle = await open(parentPath, 'r'); + protected async openParentDirectoryForSync(parentPath: string): Promise { + return open(parentPath, 'r'); + } + + protected async synchronizeParentDirectory( + handle: FileHandle, + _parentPath: string, + ): Promise { + await handle.sync(); + } + + private handleDirectorySyncFailure( + error: unknown, + destinationPath: string, + destinationVisible: boolean, + ): void { + const unsupported = isUnsupportedDirectorySyncError(error); + if (unsupported && this.unsupportedDirectorySync === UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE) { + return; + } + throw new FilesystemDurabilityError( + unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', + destinationPath, + this.durability, + destinationVisible, + error, + ); + } + + private async openParentBeforeRename( + parentPath: string, + destinationPath: string, + ): Promise { try { - await handle.sync(); - } finally { - await handle.close().catch(() => undefined); + return await this.openParentDirectoryForSync(parentPath); + } catch (error) { + this.handleDirectorySyncFailure(error, destinationPath, false); + return undefined; } } - private async synchronizeParentAfterRename(parentPath: string, destinationPath: string): Promise { + private async synchronizeParentAfterRename( + handle: FileHandle, + parentPath: string, + destinationPath: string, + ): Promise { try { - await this.synchronizeParentDirectory(parentPath); + await this.synchronizeParentDirectory(handle, parentPath); } catch (error) { - const unsupported = isUnsupportedDirectorySyncError(error); - if (unsupported && this.unsupportedDirectorySync === UNSUPPORTED_DIRECTORY_SYNC_BEHAVIORS.DEGRADE) { - return; - } - throw new FilesystemDurabilityError( - unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', - destinationPath, - this.durability, - error, - ); + this.handleDirectorySyncFailure(error, destinationPath, true); } } @@ -324,6 +353,7 @@ export class FilesystemPiClient implements PiClient { await mkdir(parent, { recursive: true }); const temporaryPath = join(parent, `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); let handle: FileHandle | undefined; + let parentHandle: FileHandle | undefined; try { handle = await open(temporaryPath, 'wx', mode ?? 0o666); @@ -334,16 +364,20 @@ export class FilesystemPiClient implements PiClient { } await handle.close(); handle = undefined; + if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { + parentHandle = await this.openParentBeforeRename(parent, path); + } await this.beforeDestinationRevalidation(path); const currentObservation = await this.observeDestination(path); if (!sameObservation(observation, currentObservation)) throw concurrentDestinationError(path); // Best-effort only: the destination can still change after this check and before rename. await this.replaceTemporaryFile(temporaryPath, path); - if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { - await this.synchronizeParentAfterRename(parent, path); + if (parentHandle !== undefined) { + await this.synchronizeParentAfterRename(parentHandle, parent, path); } } finally { await handle?.close().catch(() => undefined); + await parentHandle?.close().catch(() => undefined); await rm(temporaryPath, { force: true }).catch(() => undefined); } } diff --git a/test/filesystem-client.test.ts b/test/filesystem-client.test.ts index d2e3e7a..5978426 100644 --- a/test/filesystem-client.test.ts +++ b/test/filesystem-client.test.ts @@ -71,6 +71,12 @@ class OperationRecordingClient extends FilesystemPiClient { await super.synchronizeTemporaryFile(handle); } + protected override async openParentDirectoryForSync(parentPath: string): Promise { + this.operations.push('parent-open'); + this.synchronizedParents.push(parentPath); + return super.openParentDirectoryForSync(parentPath); + } + protected override async replaceTemporaryFile( temporaryPath: string, destinationPath: string, @@ -79,9 +85,11 @@ class OperationRecordingClient extends FilesystemPiClient { await super.replaceTemporaryFile(temporaryPath, destinationPath); } - protected override async synchronizeParentDirectory(parentPath: string): Promise { + protected override async synchronizeParentDirectory( + _handle: FileHandle, + _parentPath: string, + ): Promise { this.operations.push('parent-sync'); - this.synchronizedParents.push(parentPath); } } @@ -106,6 +114,15 @@ class FailingDirectorySyncClient extends FilesystemPiClient { } } +class FailingDirectoryOpenClient extends FilesystemPiClient { + protected override async openParentDirectoryForSync(): Promise { + const error = new Error('simulated directory open failure') as NodeJS.ErrnoException; + error.code = 'EISDIR'; + error.syscall = 'open'; + throw error; + } +} + function expectedConcurrentDestinationError(path: string): string { return `[E_CONCURRENT_DESTINATION] Refusing to replace ${path}: destination changed after it was loaded. Re-read and retry with current anchors.`; } @@ -209,7 +226,7 @@ test('supports no-sync, file, and file-and-parent-directory operation sequences' }, { durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, - expected: ['mode', 'file-sync', 'rename', 'parent-sync'], + expected: ['mode', 'file-sync', 'parent-open', 'rename', 'parent-sync'], }, ] as const; @@ -235,7 +252,7 @@ test('syncs only the direct parent when recursive directory creation was needed' await client.edit({ path, edits: [{ op: 'prepend', lines: ['created'] }] }); - assert.deepEqual(client.operations, ['file-sync', 'rename', 'parent-sync']); + assert.deepEqual(client.operations, ['file-sync', 'parent-open', 'rename', 'parent-sync']); assert.deepEqual(client.synchronizedParents, [join(dir, 'one', 'two')]); assert.equal(await readFile(path, 'utf8'), 'created'); await assertNoTemporaryFiles(join(dir, 'one', 'two')); @@ -284,6 +301,33 @@ test('strict unsupported parent sync reports that the renamed destination is vis await assertNoTemporaryFiles(dir); }); +test('strict unsupported parent open fails before rename and cleans the temporary file', async () => { + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new FailingDirectoryOpenClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + unsupportedDirectorySync: 'strict', + }); + const preview = await client.read({ path }); + + await assert.rejects( + () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), + (error: unknown) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DIRECTORY_SYNC_UNSUPPORTED'); + assert.equal(error.destinationVisible, false); + assert.match(error.message, /was not replaced/); + return true; + }, + ); + + assert.equal(await readFile(path, 'utf8'), 'one\ntwo'); + await assertNoTemporaryFiles(dir); +}); + test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { const { dir, path } = await fixture(); await writeFile(path, 'one\ntwo'); From 45f4a3aabab8cad5545ea4e24b4e8d0a99f57d7e Mon Sep 17 00:00:00 2001 From: cervantesh <11169707+cervantesh@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:02:20 -0400 Subject: [PATCH 4/5] fix: verify parent identity around rename --- CHANGELOG.md | 2 +- README.md | 2 +- dist/src/filesystem-client.d.ts | 1 + dist/src/filesystem-client.js | 44 ++++++++++++--- dist/test/filesystem-client.test.js | 49 +++++++++++++++- docs/ARCHITECTURE.md | 6 +- docs/OPERATIONS.md | 6 +- docs/PERFORMANCE.md | 10 ++-- docs/adr/0001-filesystem-crash-durability.md | 9 +-- src/filesystem-client.ts | 57 +++++++++++++++---- test/filesystem-client.test.ts | 59 +++++++++++++++++++- 11 files changed, 206 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2107b30..e89b197 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ - package contents exclude compiled tests and include complete repository provenance metadata. - filesystem atomic replacement now performs best-effort optimistic destination revalidation with identity, permission-mode, and SHA-256 byte-digest evidence, returning `[E_CONCURRENT_DESTINATION]` while preserving detected concurrent state and cleaning temporary files. - callers can select `none`, `file`, or `file-and-parent-directory` durability through `FilesystemPiClient`; the default remains file sync. -- preserved destination mode is now applied before the final temporary-file sync, and parent-directory capability is detected from the actual filesystem operation using a pre-rename pinned handle. +- preserved destination mode is now applied before the final temporary-file sync, and parent-directory capability uses a retained pre-rename handle with identity checks around rename. ### Security diff --git a/README.md b/README.md index 1521ed6..e5c6954 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ This is best-effort optimistic detection, not compare-and-swap. A residual race ### `[E_DIRECTORY_SYNC_UNSUPPORTED]` / `[E_DURABILITY_UNCONFIRMED]` -`FilesystemDurabilityError` distinguishes the commit boundary. If opening the parent fails before rename, `destinationVisible === false` and the original destination remains unchanged. If syncing the pinned parent handle fails after rename, `destinationVisible === true`; the edited destination is already visible but crash durability was not confirmed. Do not blindly replay or roll back a visible edit—re-read first. A classified unsupported operation is absorbed only when `unsupportedDirectorySync: 'degrade'`; other failures always throw. +`FilesystemDurabilityError` distinguishes the commit boundary. If parent preparation/identity verification fails before rename, `destinationVisible === false`; if the pinned parent fails identity verification or sync after rename, `destinationVisible === true` and crash durability was not confirmed. Do not blindly replay or roll back a committed edit—re-read first. A classified unsupported operation is absorbed only when `unsupportedDirectorySync: 'degrade'`; parent changes and other failures always throw. ## Development diff --git a/dist/src/filesystem-client.d.ts b/dist/src/filesystem-client.d.ts index f3f31e3..d9fafd1 100644 --- a/dist/src/filesystem-client.d.ts +++ b/dist/src/filesystem-client.d.ts @@ -37,6 +37,7 @@ export declare class FilesystemPiClient implements PiClient { protected synchronizeParentDirectory(handle: FileHandle, _parentPath: string): Promise; private handleDirectorySyncFailure; private openParentBeforeRename; + private verifyPinnedParent; private synchronizeParentAfterRename; private observeDestination; private atomicWrite; diff --git a/dist/src/filesystem-client.js b/dist/src/filesystem-client.js index 91e6450..3d5a2cd 100644 --- a/dist/src/filesystem-client.js +++ b/dist/src/filesystem-client.js @@ -202,17 +202,39 @@ export class FilesystemPiClient { throw new FilesystemDurabilityError(unsupported ? 'E_DIRECTORY_SYNC_UNSUPPORTED' : 'E_DURABILITY_UNCONFIRMED', destinationPath, this.durability, destinationVisible, error); } async openParentBeforeRename(parentPath, destinationPath) { + let handle; try { - return await this.openParentDirectoryForSync(parentPath); + handle = await this.openParentDirectoryForSync(parentPath); + const stats = await handle.stat({ bigint: true }); + if (!stats.isDirectory()) { + const error = new Error(`Parent path is no longer a directory: ${parentPath}`); + error.code = 'E_PARENT_DIRECTORY_CHANGED'; + throw error; + } + return { handle, dev: stats.dev, ino: stats.ino }; } catch (error) { + await handle?.close().catch(() => undefined); this.handleDirectorySyncFailure(error, destinationPath, false); return undefined; } } - async synchronizeParentAfterRename(handle, parentPath, destinationPath) { + async verifyPinnedParent(parentSync, parentPath, destinationPath, destinationVisible) { + let cause; + try { + const stats = await lstat(parentPath, { bigint: true }); + if (stats.isDirectory() && sameIdentity(parentSync, stats)) + return; + cause = new Error(`Parent directory changed during replacement: ${parentPath}`); + } + catch (error) { + cause = error; + } + throw new FilesystemDurabilityError('E_DURABILITY_UNCONFIRMED', destinationPath, this.durability, destinationVisible, cause); + } + async synchronizeParentAfterRename(parentSync, parentPath, destinationPath) { try { - await this.synchronizeParentDirectory(handle, parentPath); + await this.synchronizeParentDirectory(parentSync.handle, parentPath); } catch (error) { this.handleDirectorySyncFailure(error, destinationPath, true); @@ -270,7 +292,7 @@ export class FilesystemPiClient { await mkdir(parent, { recursive: true }); const temporaryPath = join(parent, `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); let handle; - let parentHandle; + let parentSync; try { handle = await open(temporaryPath, 'wx', mode ?? 0o666); await handle.writeFile(content, 'utf8'); @@ -282,21 +304,25 @@ export class FilesystemPiClient { await handle.close(); handle = undefined; if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { - parentHandle = await this.openParentBeforeRename(parent, path); + parentSync = await this.openParentBeforeRename(parent, path); } await this.beforeDestinationRevalidation(path); const currentObservation = await this.observeDestination(path); if (!sameObservation(observation, currentObservation)) throw concurrentDestinationError(path); - // Best-effort only: the destination can still change after this check and before rename. + if (parentSync !== undefined) { + await this.verifyPinnedParent(parentSync, parent, path, false); + } + // Best-effort only: the destination or parent can still change during rename. await this.replaceTemporaryFile(temporaryPath, path); - if (parentHandle !== undefined) { - await this.synchronizeParentAfterRename(parentHandle, parent, path); + if (parentSync !== undefined) { + await this.verifyPinnedParent(parentSync, parent, path, true); + await this.synchronizeParentAfterRename(parentSync, parent, path); } } finally { await handle?.close().catch(() => undefined); - await parentHandle?.close().catch(() => undefined); + await parentSync?.handle.close().catch(() => undefined); await rm(temporaryPath, { force: true }).catch(() => undefined); } } diff --git a/dist/test/filesystem-client.test.js b/dist/test/filesystem-client.test.js index 7eb681b..0994709 100644 --- a/dist/test/filesystem-client.test.js +++ b/dist/test/filesystem-client.test.js @@ -1,8 +1,8 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { chmod, link, lstat, mkdtemp, readFile, readdir, rename, rm, stat, symlink, utimes, writeFile, } from 'node:fs/promises'; +import { chmod, link, lstat, mkdir, mkdtemp, readFile, readdir, rename, rm, stat, symlink, utimes, writeFile, } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { basename, join } from 'node:path'; +import { basename, dirname, join } from 'node:path'; import { DEFAULT_FILESYSTEM_DURABILITY, FILESYSTEM_DURABILITY_LEVELS, FilesystemDurabilityError, FilesystemPiClient, loadFileKindAndText, } from '../src/index.js'; async function fixture(name = 'file.txt') { const dir = await mkdtemp(join(tmpdir(), 'pi-anchor-edit-core-')); @@ -83,6 +83,25 @@ class FailingDirectoryOpenClient extends FilesystemPiClient { throw error; } } +class ParentReplacementDuringRenameClient extends FilesystemPiClient { + movedParent; + async replaceTemporaryFile(temporaryPath, destinationPath) { + const parent = dirname(destinationPath); + const movedParent = `${parent}-moved`; + this.movedParent = movedParent; + await rename(parent, movedParent); + await mkdir(parent); + await link(join(movedParent, basename(destinationPath)), destinationPath); + const movedTemporaryPath = join(movedParent, basename(temporaryPath)); + await link(movedTemporaryPath, temporaryPath); + try { + await super.replaceTemporaryFile(temporaryPath, destinationPath); + } + finally { + await rm(movedTemporaryPath, { force: true }); + } + } +} function expectedConcurrentDestinationError(path) { return `[E_CONCURRENT_DESTINATION] Refusing to replace ${path}: destination changed after it was loaded. Re-read and retry with current anchors.`; } @@ -251,6 +270,32 @@ test('strict unsupported parent open fails before rename and cleans the temporar assert.equal(await readFile(path, 'utf8'), 'one\ntwo'); await assertNoTemporaryFiles(dir); }); +test('reports unconfirmed durability when the parent changes during rename', async (t) => { + if (process.platform === 'win32') { + t.skip('Windows does not permit renaming the pinned open parent directory'); + return; + } + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new ParentReplacementDuringRenameClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }); + const preview = await client.read({ path }); + await assert.rejects(() => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), (error) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DURABILITY_UNCONFIRMED'); + assert.equal(error.destinationVisible, true); + return true; + }); + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); + if (client.movedParent !== undefined) { + await rm(client.movedParent, { recursive: true, force: true }); + } +}); test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { const { dir, path } = await fixture(); await writeFile(path, 'one\ntwo'); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e62f4f2..6b635c0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -20,8 +20,8 @@ 5. The engine produces new content or throws a classified actionable error. 6. The filesystem adapter classifies the path before decoding, captures destination existence, file identity, byte length, permission mode, and a SHA-256 digest, and preserves newline/BOM/mode behavior. 7. The adapter writes a same-directory temporary file, applies preserved mode, and performs the configured file sync unless `none` was selected. -8. For `file-and-parent-directory`, it opens and retains a handle to the direct parent before replacement. It then re-observes the destination and aborts with `[E_CONCURRENT_DESTINATION]` if existence, identity, length, permission mode, or digest differs; otherwise it renames the temporary file. -9. `file-and-parent-directory` synchronizes the retained parent handle after rename, so path replacement cannot redirect the sync; only classified unsupported capability degrades under the configured policy. +8. For `file-and-parent-directory`, it opens and retains a handle to the direct parent before replacement. It re-observes the destination and verifies the parent path still has the retained identity before rename. +9. After rename it verifies parent identity again, then synchronizes the retained handle. A mismatch throws unconfirmed durability instead of syncing an unrelated directory; only classified unsupported capability degrades under policy. ## Invariants @@ -36,7 +36,7 @@ - Revalidation uses permission-mode and byte-digest evidence rather than size or timestamps alone, so permission-only changes, same-size content changes, and changes hidden by coarse timestamp resolution are detected. - The concurrency guard is optimistic and best-effort, not compare-and-swap: a destination can still change in the residual interval after revalidation and before `rename`. The library makes no false CAS guarantee. - Atomic visibility and crash durability are separate: `none` performs no explicit sync, `file` syncs the temporary file and remains the default, and `file-and-parent-directory` additionally syncs the direct parent after rename. -- Preserved mode is applied before the selected final file sync. The direct parent is opened before rename and the retained handle is synchronized afterward. +- Preserved mode is applied before the selected final file sync. Parent identity is checked around rename, and only the retained matching handle is synchronized afterward. - A parent-open error is pre-commit (`destinationVisible: false`); a parent-sync error is post-commit (`destinationVisible: true`) and is never rolled back. - Parent sync covers only the direct parent. Recursively created ancestors are not included in the durability claim. diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index 331850b..d669b74 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -8,7 +8,7 @@ The filesystem adapter performs same-directory temporary-file replacement. It re ## Durability selection -`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to open and pin the direct parent before rename and sync that retained handle afterward. The latter does not synchronize recursively created ancestor entries. +`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to retain the direct parent before rename, verify its path identity around rename, and sync that handle afterward. The latter does not synchronize recursively created ancestor entries. Parent-directory support is detected by attempting the real operation. `unsupportedDirectorySync: 'degrade'` (default) absorbs only a classified unsupported-capability error and completes at file durability. `strict` throws instead. Unclassified I/O or permission failures always throw. See [`ADR 0001`](adr/0001-filesystem-crash-durability.md). @@ -30,7 +30,7 @@ Parent-directory support is detected by attempting the real operation. `unsuppor | `[E_UNSUPPORTED_FILE]` | The path is a directory, symbolic link, or unsupported special file. | Resolve and authorize a regular-file path explicitly; do not weaken the classifier or follow the link implicitly. | | `[E_CONCURRENT_DESTINATION]` | The destination's existence, identity, length, permission mode, or byte digest changed after it was loaded and before replacement. | Preserve the concurrent destination, re-read it, reassess intent, and retry only with current anchors. | | `[E_DIRECTORY_SYNC_UNSUPPORTED]` | Strict parent-directory open or sync was unsupported. `destinationVisible` identifies whether rename occurred. | If false, the original remains unchanged. If true, do not replay blindly: re-read and decide whether file durability is acceptable or provision a supported filesystem. | -| `[E_DURABILITY_UNCONFIRMED]` | Parent-directory open/sync failed for an unclassified reason. `destinationVisible` identifies whether rename occurred. | Preserve/re-read a visible destination; otherwise investigate the underlying `cause` before retrying. | +| `[E_DURABILITY_UNCONFIRMED]` | Parent-directory open/sync failed or parent identity changed around rename. `destinationVisible` identifies whether rename occurred. | Preserve/re-read a visible destination; otherwise investigate the underlying `cause` before retrying. | `Operation aborted` means the supplied abort signal was already cancelled; leave content unchanged, determine whether the caller still wants the operation, then re-read before retrying. @@ -40,7 +40,7 @@ Node filesystem errors such as `EACCES`, `EPERM`, `EROFS`, `ENOSPC`, `EMFILE`, a The optimistic guard deliberately compares permission mode and a byte digest rather than trusting size and timestamps, so permission-only changes, same-size edits, and coarse-time metadata collisions are detected. It is still a best-effort check, not an atomic compare-and-swap: another writer can change the destination after revalidation and before `rename`. Callers must not treat a successful edit as proof that no writer raced in that residual interval. -`FilesystemDurabilityError` exposes the destination path, requested durability, stable error code, original `cause`, and commit boundary. `destinationVisible: false` means parent open failed before rename and cleanup preserved the original; `true` means the retained parent handle failed to sync after rename, so cleanup never removes or rolls back the visible destination. A strict Windows parent sync normally reports `E_DIRECTORY_SYNC_UNSUPPORTED` with an `EPERM`/`fsync` cause; degrade mode treats that specific capability result as file durability. +`FilesystemDurabilityError` exposes the destination path, requested durability, stable error code, original `cause`, and commit boundary. `destinationVisible: false` means the durability step failed before rename; `true` means rename returned before parent sync or identity confirmation failed, so cleanup never removes or rolls back the committed destination. A strict Windows parent sync normally reports `E_DIRECTORY_SYNC_UNSUPPORTED` with an `EPERM`/`fsync` cause; degrade mode treats that specific capability result as file durability. A successful sync reports only what the operating system and storage stack make observable. Hardware, network filesystems, virtualized filesystems, and mount options can weaken persistence guarantees. diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md index 9eee645..d0932a5 100644 --- a/docs/PERFORMANCE.md +++ b/docs/PERFORMANCE.md @@ -14,11 +14,11 @@ Measured 2026-07-16 on Windows x64 with Node 24.18.0. Values are milliseconds pe | Operation / durability | Mean | p95 | p99 | |---|---:|---:|---:| -| `computeLineHash` (100 × 10,000 operations) | 0.001030 | 0.001097 | 0.001349 | -| Filesystem edit: `none` | 2.484 | 2.954 | 3.110 | -| Filesystem edit: `file` (default) | 5.324 | 6.457 | 6.622 | -| Filesystem edit: `file-and-parent-directory` | 5.586 | 6.663 | 6.983 | +| `computeLineHash` (100 × 10,000 operations) | 0.001060 | 0.001160 | 0.001401 | +| Filesystem edit: `none` | 2.659 | 3.466 | 3.596 | +| Filesystem edit: `file` (default) | 5.236 | 6.284 | 6.640 | +| Filesystem edit: `file-and-parent-directory` | 5.954 | 7.000 | 7.517 | -On this Windows host, file sync added about 2.84 ms mean latency over `none`. Attempting parent-directory sync added about 0.26 ms over `file`; Windows returned the classified unsupported `EPERM`, so the configured degrade policy completed at file durability. This does **not** measure the cost of a successful directory fsync. Linux/macOS and different filesystems/storage can differ substantially. +On this Windows host, file sync added about 2.58 ms mean latency over `none`. Parent-handle open plus the identity checks and unsupported sync attempt added about 0.72 ms over `file`; Windows returned the classified unsupported `EPERM`, so the configured degrade policy completed at file durability. This does **not** measure the cost of a successful directory fsync. Linux/macOS and different filesystems/storage can differ substantially. The pure hash result remains comfortably below the 0.1 ms target in [`PRODUCT.md`](PRODUCT.md). No end-to-end edit budget is enforced yet. When publishing updated results, record Node version, OS/filesystem, rounds, selected unsupported-directory policy, whether parent sync succeeded or degraded, and the complete JSON output. diff --git a/docs/adr/0001-filesystem-crash-durability.md b/docs/adr/0001-filesystem-crash-durability.md index 74561ab..f8a1ee4 100644 --- a/docs/adr/0001-filesystem-crash-durability.md +++ b/docs/adr/0001-filesystem-crash-durability.md @@ -37,12 +37,13 @@ The write sequence is: 3. perform the selected final temporary-file sync (`file` and `file-and-parent-directory` only); 4. close the temporary-file handle; 5. for `file-and-parent-directory`, open and retain a handle to the destination's direct parent; -6. revalidate the destination and atomically rename; -7. for `file-and-parent-directory`, sync the retained parent handle and close it. +6. revalidate the destination and verify that the retained handle still matches the parent path; +7. atomically rename; +8. verify the parent identity again, then sync the retained handle and close it. Applying mode before the final file sync includes the mode metadata in the best available file durability boundary. -Opening the parent before rename pins the directory that receives the replacement. Reopening the parent path afterward could synchronize a different directory if another process renamed or replaced that path. +Opening the parent before rename pins a candidate directory, while identity checks immediately before and after rename prevent a replaced parent path from making the operation sync an unrelated directory and falsely report success. Reopening the parent path only afterward would not provide either property. ### Unsupported parent-directory synchronization @@ -58,7 +59,7 @@ No parent-directory capability result is cached globally because different paths Parent-directory sync occurs after rename, so any failure from that step happens after the destination has been committed and is visible. The operation throws a `FilesystemDurabilityError` with `destinationVisible: true`; it does not roll back, delete, or restore the visible destination. Retrying the original edit blindly is unsafe. Callers must inspect/re-read the destination and decide whether another durability attempt or edit is appropriate. -The retained handle prevents a path replacement between rename and sync from redirecting synchronization to a different directory. +The retained handle plus pre/post-rename identity checks prevent a path replacement from redirecting synchronization or producing a false durability success. A detected mismatch throws `E_DURABILITY_UNCONFIRMED`; `destinationVisible` reports whether rename had already returned. In `degrade` mode, only a classified unsupported-capability result is absorbed. Other post-rename failures still throw `FilesystemDurabilityError` and carry the original error as `cause`. diff --git a/src/filesystem-client.ts b/src/filesystem-client.ts index c14b169..021c323 100644 --- a/src/filesystem-client.ts +++ b/src/filesystem-client.ts @@ -72,6 +72,7 @@ type DestinationObservation = | { state: 'unstable' }; type LoadedText = { text: string; mode?: number; observation: DestinationObservation }; +type ParentDirectorySync = { handle: FileHandle; dev: bigint; ino: bigint }; const CONCURRENT_DESTINATION_ERROR = 'E_CONCURRENT_DESTINATION'; @@ -274,22 +275,54 @@ export class FilesystemPiClient implements PiClient { private async openParentBeforeRename( parentPath: string, destinationPath: string, - ): Promise { + ): Promise { + let handle: FileHandle | undefined; try { - return await this.openParentDirectoryForSync(parentPath); + handle = await this.openParentDirectoryForSync(parentPath); + const stats = await handle.stat({ bigint: true }); + if (!stats.isDirectory()) { + const error = new Error(`Parent path is no longer a directory: ${parentPath}`) as NodeJS.ErrnoException; + error.code = 'E_PARENT_DIRECTORY_CHANGED'; + throw error; + } + return { handle, dev: stats.dev, ino: stats.ino }; } catch (error) { + await handle?.close().catch(() => undefined); this.handleDirectorySyncFailure(error, destinationPath, false); return undefined; } } + private async verifyPinnedParent( + parentSync: ParentDirectorySync, + parentPath: string, + destinationPath: string, + destinationVisible: boolean, + ): Promise { + let cause: unknown; + try { + const stats = await lstat(parentPath, { bigint: true }); + if (stats.isDirectory() && sameIdentity(parentSync, stats)) return; + cause = new Error(`Parent directory changed during replacement: ${parentPath}`); + } catch (error) { + cause = error; + } + throw new FilesystemDurabilityError( + 'E_DURABILITY_UNCONFIRMED', + destinationPath, + this.durability, + destinationVisible, + cause, + ); + } + private async synchronizeParentAfterRename( - handle: FileHandle, + parentSync: ParentDirectorySync, parentPath: string, destinationPath: string, ): Promise { try { - await this.synchronizeParentDirectory(handle, parentPath); + await this.synchronizeParentDirectory(parentSync.handle, parentPath); } catch (error) { this.handleDirectorySyncFailure(error, destinationPath, true); } @@ -353,7 +386,7 @@ export class FilesystemPiClient implements PiClient { await mkdir(parent, { recursive: true }); const temporaryPath = join(parent, `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); let handle: FileHandle | undefined; - let parentHandle: FileHandle | undefined; + let parentSync: ParentDirectorySync | undefined; try { handle = await open(temporaryPath, 'wx', mode ?? 0o666); @@ -365,19 +398,23 @@ export class FilesystemPiClient implements PiClient { await handle.close(); handle = undefined; if (this.durability === FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY) { - parentHandle = await this.openParentBeforeRename(parent, path); + parentSync = await this.openParentBeforeRename(parent, path); } await this.beforeDestinationRevalidation(path); const currentObservation = await this.observeDestination(path); if (!sameObservation(observation, currentObservation)) throw concurrentDestinationError(path); - // Best-effort only: the destination can still change after this check and before rename. + if (parentSync !== undefined) { + await this.verifyPinnedParent(parentSync, parent, path, false); + } + // Best-effort only: the destination or parent can still change during rename. await this.replaceTemporaryFile(temporaryPath, path); - if (parentHandle !== undefined) { - await this.synchronizeParentAfterRename(parentHandle, parent, path); + if (parentSync !== undefined) { + await this.verifyPinnedParent(parentSync, parent, path, true); + await this.synchronizeParentAfterRename(parentSync, parent, path); } } finally { await handle?.close().catch(() => undefined); - await parentHandle?.close().catch(() => undefined); + await parentSync?.handle.close().catch(() => undefined); await rm(temporaryPath, { force: true }).catch(() => undefined); } } diff --git a/test/filesystem-client.test.ts b/test/filesystem-client.test.ts index 5978426..c4ac9ef 100644 --- a/test/filesystem-client.test.ts +++ b/test/filesystem-client.test.ts @@ -4,6 +4,7 @@ import { chmod, link, lstat, + mkdir, mkdtemp, readFile, readdir, @@ -15,7 +16,7 @@ import { writeFile, } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { basename, join } from 'node:path'; +import { basename, dirname, join } from 'node:path'; import { DEFAULT_FILESYSTEM_DURABILITY, FILESYSTEM_DURABILITY_LEVELS, @@ -123,6 +124,29 @@ class FailingDirectoryOpenClient extends FilesystemPiClient { } } +class ParentReplacementDuringRenameClient extends FilesystemPiClient { + movedParent: string | undefined; + + protected override async replaceTemporaryFile( + temporaryPath: string, + destinationPath: string, + ): Promise { + const parent = dirname(destinationPath); + const movedParent = `${parent}-moved`; + this.movedParent = movedParent; + await rename(parent, movedParent); + await mkdir(parent); + await link(join(movedParent, basename(destinationPath)), destinationPath); + const movedTemporaryPath = join(movedParent, basename(temporaryPath)); + await link(movedTemporaryPath, temporaryPath); + try { + await super.replaceTemporaryFile(temporaryPath, destinationPath); + } finally { + await rm(movedTemporaryPath, { force: true }); + } + } +} + function expectedConcurrentDestinationError(path: string): string { return `[E_CONCURRENT_DESTINATION] Refusing to replace ${path}: destination changed after it was loaded. Re-read and retry with current anchors.`; } @@ -328,6 +352,39 @@ test('strict unsupported parent open fails before rename and cleans the temporar await assertNoTemporaryFiles(dir); }); +test('reports unconfirmed durability when the parent changes during rename', async (t) => { + if (process.platform === 'win32') { + t.skip('Windows does not permit renaming the pinned open parent directory'); + return; + } + + const { dir, path } = await fixture(); + await writeFile(path, 'one\ntwo'); + const client = new ParentReplacementDuringRenameClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }); + const preview = await client.read({ path }); + + await assert.rejects( + () => client.edit({ + path, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }), + (error: unknown) => { + assert.ok(error instanceof FilesystemDurabilityError); + assert.equal(error.code, 'E_DURABILITY_UNCONFIRMED'); + assert.equal(error.destinationVisible, true); + return true; + }, + ); + + assert.equal(await readFile(path, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(dir); + if (client.movedParent !== undefined) { + await rm(client.movedParent, { recursive: true, force: true }); + } +}); + test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { const { dir, path } = await fixture(); await writeFile(path, 'one\ntwo'); From b8d133a8139cd64e9077ef6d588aaf58ae9c3ab8 Mon Sep 17 00:00:00 2001 From: cervantesh <11169707+cervantesh@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:11:48 -0400 Subject: [PATCH 5/5] fix: follow symlinked parent identity --- SECURITY.md | 2 +- dist/src/filesystem-client.js | 4 +-- dist/test/filesystem-client.test.js | 30 ++++++++++++++++++ docs/ARCHITECTURE.md | 2 +- docs/OPERATIONS.md | 2 +- docs/adr/0001-filesystem-crash-durability.md | 2 +- src/filesystem-client.ts | 4 +-- test/filesystem-client.test.ts | 32 ++++++++++++++++++++ 8 files changed, 70 insertions(+), 8 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index e16abc6..18162b9 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -19,7 +19,7 @@ We aim to acknowledge a private report within 3 business days, provide an initia ## Trust boundary -`pi-anchor-edit-core` reads and mutates caller-selected local paths. The caller is responsible for authorization, path selection, backups, and preventing untrusted users from choosing sensitive targets. The filesystem adapter rejects symbolic links, special files, directories, images, binary/null-byte content, and invalid UTF-8 rewrites; it does not create a sandbox or establish that a path is safe to edit. +`pi-anchor-edit-core` reads and mutates caller-selected local paths. The caller is responsible for authorization, path selection, parent-path components, backups, and preventing untrusted users from choosing sensitive targets. The filesystem adapter rejects a symbolic link at the destination, plus special files, directories, images, binary/null-byte content, and invalid UTF-8 rewrites; parent-directory components follow normal OS resolution and may be symlinks. It does not create a sandbox or establish that a path is safe to edit. Anchor diagnostics can quote nearby file content in `>>> LINE#HASH:content` retry lines. Treat all diagnostics as potentially sensitive. Redact or replace them with synthetic examples before sharing, and never send raw diagnostics to telemetry by default. diff --git a/dist/src/filesystem-client.js b/dist/src/filesystem-client.js index 3d5a2cd..5e22534 100644 --- a/dist/src/filesystem-client.js +++ b/dist/src/filesystem-client.js @@ -1,4 +1,4 @@ -import { chmod, lstat, mkdir, open, readFile, rename, rm } from 'node:fs/promises'; +import { chmod, lstat, mkdir, open, readFile, rename, rm, stat } from 'node:fs/promises'; import { createHash, randomUUID } from 'node:crypto'; import { basename, dirname, join } from 'node:path'; import { formatAnchors } from './anchors.js'; @@ -222,7 +222,7 @@ export class FilesystemPiClient { async verifyPinnedParent(parentSync, parentPath, destinationPath, destinationVisible) { let cause; try { - const stats = await lstat(parentPath, { bigint: true }); + const stats = await stat(parentPath, { bigint: true }); if (stats.isDirectory() && sameIdentity(parentSync, stats)) return; cause = new Error(`Parent directory changed during replacement: ${parentPath}`); diff --git a/dist/test/filesystem-client.test.js b/dist/test/filesystem-client.test.js index 0994709..f2c3ad8 100644 --- a/dist/test/filesystem-client.test.js +++ b/dist/test/filesystem-client.test.js @@ -296,6 +296,36 @@ test('reports unconfirmed durability when the parent changes during rename', asy await rm(client.movedParent, { recursive: true, force: true }); } }); +test('file-and-parent-directory follows and verifies a symlinked parent target', async (t) => { + const { dir } = await fixture(); + const realParent = join(dir, 'real-parent'); + const linkedParent = join(dir, 'linked-parent'); + const realPath = join(realParent, 'file.txt'); + const linkedPath = join(linkedParent, 'file.txt'); + await mkdir(realParent); + await writeFile(realPath, 'one\ntwo'); + try { + await symlink(realParent, linkedParent, process.platform === 'win32' ? 'junction' : 'dir'); + } + catch (error) { + const code = error.code; + if (code === 'EPERM' || code === 'EACCES' || code === 'ENOSYS') { + t.skip(`directory-symlink capability unavailable: ${code}`); + return; + } + throw error; + } + const client = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }); + const preview = await client.read({ path: linkedPath }); + await client.edit({ + path: linkedPath, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }); + assert.equal(await readFile(realPath, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(realParent); +}); test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { const { dir, path } = await fixture(); await writeFile(path, 'one\ntwo'); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6b635c0..5f459cd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -21,7 +21,7 @@ 6. The filesystem adapter classifies the path before decoding, captures destination existence, file identity, byte length, permission mode, and a SHA-256 digest, and preserves newline/BOM/mode behavior. 7. The adapter writes a same-directory temporary file, applies preserved mode, and performs the configured file sync unless `none` was selected. 8. For `file-and-parent-directory`, it opens and retains a handle to the direct parent before replacement. It re-observes the destination and verifies the parent path still has the retained identity before rename. -9. After rename it verifies parent identity again, then synchronizes the retained handle. A mismatch throws unconfirmed durability instead of syncing an unrelated directory; only classified unsupported capability degrades under policy. +9. After rename it verifies the followed parent-target identity again, then synchronizes the retained handle. Symlinked parents are accepted while their target stays stable; a mismatch throws unconfirmed durability instead of syncing an unrelated directory. ## Invariants diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index d669b74..a135c1b 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -8,7 +8,7 @@ The filesystem adapter performs same-directory temporary-file replacement. It re ## Durability selection -`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to retain the direct parent before rename, verify its path identity around rename, and sync that handle afterward. The latter does not synchronize recursively created ancestor entries. +`new FilesystemPiClient()` preserves the historical `file` durability default: write, apply preserved mode, sync the temporary file, then rename. Set `durability` to `none` to omit explicit syncs or `file-and-parent-directory` to retain the direct parent before rename, verify its followed-target identity around rename, and sync that handle afterward. A stable symlinked parent target is supported; retargeting fails unconfirmed. Recursively created ancestor entries are not synchronized. Parent-directory support is detected by attempting the real operation. `unsupportedDirectorySync: 'degrade'` (default) absorbs only a classified unsupported-capability error and completes at file durability. `strict` throws instead. Unclassified I/O or permission failures always throw. See [`ADR 0001`](adr/0001-filesystem-crash-durability.md). diff --git a/docs/adr/0001-filesystem-crash-durability.md b/docs/adr/0001-filesystem-crash-durability.md index f8a1ee4..abc0202 100644 --- a/docs/adr/0001-filesystem-crash-durability.md +++ b/docs/adr/0001-filesystem-crash-durability.md @@ -43,7 +43,7 @@ The write sequence is: Applying mode before the final file sync includes the mode metadata in the best available file durability boundary. -Opening the parent before rename pins a candidate directory, while identity checks immediately before and after rename prevent a replaced parent path from making the operation sync an unrelated directory and falsely report success. Reopening the parent path only afterward would not provide either property. +Opening the parent before rename pins a candidate directory, while followed-target identity checks immediately before and after rename prevent a replaced or retargeted parent path from making the operation sync an unrelated directory and falsely report success. A symlinked parent is supported when its current target still matches the retained handle. Reopening the parent path only afterward would not provide either property. ### Unsupported parent-directory synchronization diff --git a/src/filesystem-client.ts b/src/filesystem-client.ts index 021c323..15360b1 100644 --- a/src/filesystem-client.ts +++ b/src/filesystem-client.ts @@ -1,4 +1,4 @@ -import { chmod, lstat, mkdir, open, readFile, rename, rm } from 'node:fs/promises'; +import { chmod, lstat, mkdir, open, readFile, rename, rm, stat } from 'node:fs/promises'; import type { FileHandle } from 'node:fs/promises'; import { createHash, randomUUID } from 'node:crypto'; import { basename, dirname, join } from 'node:path'; @@ -301,7 +301,7 @@ export class FilesystemPiClient implements PiClient { ): Promise { let cause: unknown; try { - const stats = await lstat(parentPath, { bigint: true }); + const stats = await stat(parentPath, { bigint: true }); if (stats.isDirectory() && sameIdentity(parentSync, stats)) return; cause = new Error(`Parent directory changed during replacement: ${parentPath}`); } catch (error) { diff --git a/test/filesystem-client.test.ts b/test/filesystem-client.test.ts index c4ac9ef..95ab4f5 100644 --- a/test/filesystem-client.test.ts +++ b/test/filesystem-client.test.ts @@ -385,6 +385,38 @@ test('reports unconfirmed durability when the parent changes during rename', asy } }); +test('file-and-parent-directory follows and verifies a symlinked parent target', async (t) => { + const { dir } = await fixture(); + const realParent = join(dir, 'real-parent'); + const linkedParent = join(dir, 'linked-parent'); + const realPath = join(realParent, 'file.txt'); + const linkedPath = join(linkedParent, 'file.txt'); + await mkdir(realParent); + await writeFile(realPath, 'one\ntwo'); + try { + await symlink(realParent, linkedParent, process.platform === 'win32' ? 'junction' : 'dir'); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES' || code === 'ENOSYS') { + t.skip(`directory-symlink capability unavailable: ${code}`); + return; + } + throw error; + } + + const client = new FilesystemPiClient({ + durability: FILESYSTEM_DURABILITY_LEVELS.FILE_AND_PARENT_DIRECTORY, + }); + const preview = await client.read({ path: linkedPath }); + await client.edit({ + path: linkedPath, + edits: [{ op: 'replace', pos: secondAnchor(preview), lines: ['patched'] }], + }); + + assert.equal(await readFile(realPath, 'utf8'), 'one\npatched'); + await assertNoTemporaryFiles(realParent); +}); + test('unclassified post-rename sync failure reports visible but unconfirmed durability', async () => { const { dir, path } = await fixture(); await writeFile(path, 'one\ntwo');