fix(fs-local): bound overwrite contextual diff bases
Rebuild of the fs-overwrite-diff-bound branch on current master. Adds the diffBasisMaxBytes Config field (10 MiB default, capped by runtime allocation/decode limits), gates both overwrite sides, and reads the prior basis from the bounded opened descriptor in cancellation-aware chunks; any post-stat size change returns a null basis. Also pins the one-extra-byte growth probe with a regression and drops the now-covered v8 ignore.
This commit is contained in:
@@ -15,6 +15,8 @@ import { FsError, FsTargetKey, FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
import { copyFileDaclWin32, replaceFileWin32 } from './win32.ts'
|
||||
|
||||
const BINARY_SAMPLE_BYTES = 8192
|
||||
// Bound one non-abortable FileHandle.read so cancellation is observed between chunks.
|
||||
const DIFF_BASIS_READ_CHUNK_BYTES = 64 * 1024
|
||||
|
||||
function isENOENT(error: unknown): boolean {
|
||||
return error instanceof Error && 'code' in error && error.code === 'ENOENT'
|
||||
@@ -70,9 +72,8 @@ function versionOf(info: BigIntStats): FsVersion {
|
||||
}
|
||||
|
||||
/**
|
||||
* Test seam: lets specs pin the atomic-write temp names (to prove
|
||||
* exclusive-open behavior without a name race) and observe the staged temp
|
||||
* file before it is renamed over the target.
|
||||
* Test seam: lets specs pin the atomic-write temp names (to prove exclusive-open behavior without
|
||||
* a name race), override native boundaries, and observe the staged temp file before publication.
|
||||
*/
|
||||
export interface FsIoInternals {
|
||||
/** Override the host platform for native-publication unit coverage. */
|
||||
@@ -564,17 +565,53 @@ export async function readForEdit(
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort overwrite diff basis. Binary or invalid UTF-8 returns `null` so the write still
|
||||
* succeeds and presentation falls back to a whole-file diff.
|
||||
* Best-effort overwrite diff basis. Binary, invalid UTF-8, or a file at/above the byte limit
|
||||
* returns `null` so the write still succeeds and presentation falls back to a whole-file diff.
|
||||
* The bound is enforced on the opened descriptor rather than a prior path stat, so concurrent
|
||||
* external replacement or size changes cannot make this helper buffer more than `maxBytes`.
|
||||
* @param absolutePath - the file to read (typically a target key); it must exist.
|
||||
* @param maxBytes - exclusive upper bound for bytes held as the contextual-diff basis.
|
||||
* @param signal - aborts the read (`FS_ABORTED`).
|
||||
* @returns the LF-normalized text, or null for a binary or non-UTF-8 file.
|
||||
* @returns the LF-normalized text, or null for a non-regular, at/above-limit, binary, non-UTF-8,
|
||||
* or descriptor-size-changed file.
|
||||
*/
|
||||
export async function readTextForDiff(absolutePath: string, signal?: AbortSignal): Promise<string | null> {
|
||||
const buffer = await readFileAbortable(absolutePath, 'read', signal)
|
||||
if (buffer.includes(0)) return null
|
||||
export async function readTextForDiff(
|
||||
absolutePath: string,
|
||||
maxBytes: number,
|
||||
signal?: AbortSignal,
|
||||
): Promise<string | null> {
|
||||
throwIfAborted(signal, 'read')
|
||||
const handle = await open(absolutePath, 'r')
|
||||
let buffer: Buffer
|
||||
let total = 0
|
||||
let openedSize = 0
|
||||
try {
|
||||
return normalizeLineEndings(new TextDecoder('utf-8', { fatal: true }).decode(buffer))
|
||||
throwIfAborted(signal, 'read')
|
||||
const info = await handle.stat()
|
||||
throwIfAborted(signal, 'read')
|
||||
/* v8 ignore next -- requires a post-preflight replacement with a non-file;
|
||||
* direct coverage is not portable to Windows. */
|
||||
if (!info.isFile()) return null
|
||||
if (info.size >= maxBytes) return null
|
||||
openedSize = info.size
|
||||
// One extra byte detects growth after stat without retaining per-read backing buffers.
|
||||
buffer = Buffer.allocUnsafe(openedSize + 1)
|
||||
while (total < buffer.length) {
|
||||
throwIfAborted(signal, 'read')
|
||||
const length = Math.min(buffer.length - total, DIFF_BASIS_READ_CHUNK_BYTES)
|
||||
const { bytesRead } = await handle.read(buffer, total, length, null)
|
||||
if (bytesRead === 0) break
|
||||
total += bytesRead
|
||||
}
|
||||
} finally {
|
||||
await handle.close()
|
||||
}
|
||||
throwIfAborted(signal, 'read')
|
||||
if (total !== openedSize) return null
|
||||
const basis = buffer.subarray(0, total)
|
||||
if (basis.includes(0)) return null
|
||||
try {
|
||||
return normalizeLineEndings(new TextDecoder('utf-8', { fatal: true }).decode(basis))
|
||||
} catch (error: unknown) {
|
||||
/* v8 ignore next 2 -- TextDecoder({fatal}) only throws TypeError on invalid bytes; any other throw is an unreachable runtime fault. */
|
||||
if (!(error instanceof TypeError)) throw error
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
*/
|
||||
|
||||
import { Context } from 'cordis'
|
||||
import { constants as bufferConstants } from 'node:buffer'
|
||||
import { resolve } from 'node:path'
|
||||
import z from 'schemastery'
|
||||
import { FileSystem, FsError, FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
@@ -38,9 +39,19 @@ import type { FsIoInternals } from './fsio.ts'
|
||||
export interface Config {
|
||||
/** Base directory for relative paths. Defaults to `process.cwd()`. */
|
||||
cwd?: string
|
||||
/**
|
||||
* Exclusive UTF-8 byte limit on each overwrite-diff side, capped by the
|
||||
* runtime's safe allocation/decode maximum. Defaults to 10 MiB.
|
||||
*/
|
||||
diffBasisMaxBytes?: number
|
||||
}
|
||||
|
||||
type ResolvedConfig = Required<Config>
|
||||
const DEFAULT_DIFF_BASIS_MAX_BYTES = 10 * 1024 * 1024
|
||||
const MAX_DIFF_BASIS_BYTES = Math.min(
|
||||
bufferConstants.MAX_LENGTH,
|
||||
bufferConstants.MAX_STRING_LENGTH,
|
||||
)
|
||||
|
||||
/**
|
||||
* The host-filesystem backend. Reads resolve relative paths from {@link Config.cwd}
|
||||
@@ -51,11 +62,12 @@ type ResolvedConfig = Required<Config>
|
||||
export class LocalFileSystem extends FileSystem {
|
||||
static Config: z<Config> = z.object({
|
||||
cwd: z.string().default(process.cwd()),
|
||||
diffBasisMaxBytes: z.number().default(DEFAULT_DIFF_BASIS_MAX_BYTES),
|
||||
})
|
||||
|
||||
/** Validated config (schemastery applied the defaults before construction). */
|
||||
readonly config: ResolvedConfig
|
||||
/** Test seam forwarded to fsio (force streaming path, pin temp names). */
|
||||
/** Test seam forwarded to fsio for atomic-publication boundaries. */
|
||||
internals: FsIoInternals = {}
|
||||
/** Per-targetKey tail promise: serializes mutating ops so the read→guard→write
|
||||
* window can't interleave, making concurrent writes/edits deterministically
|
||||
@@ -64,7 +76,13 @@ export class LocalFileSystem extends FileSystem {
|
||||
|
||||
constructor(ctx: Context, config: Config) {
|
||||
super(ctx)
|
||||
this.config = config as ResolvedConfig
|
||||
const resolved = config as ResolvedConfig
|
||||
if (!Number.isSafeInteger(resolved.diffBasisMaxBytes)
|
||||
|| resolved.diffBasisMaxBytes <= 0
|
||||
|| resolved.diffBasisMaxBytes > MAX_DIFF_BASIS_BYTES) {
|
||||
throw new Error(`fs-local: diffBasisMaxBytes must be a positive safe integer no greater than ${MAX_DIFF_BASIS_BYTES}`)
|
||||
}
|
||||
this.config = resolved
|
||||
}
|
||||
|
||||
/** Run `op` with exclusive access to `targetKey` (FIFO per key). */
|
||||
@@ -150,9 +168,16 @@ export class LocalFileSystem extends FileSystem {
|
||||
}
|
||||
// No expectation means an unconditional but still atomic write.
|
||||
|
||||
// Preserve prior text for contextual diffs; null falls back to a whole-file diff.
|
||||
// TODO(overwrite-diff-bound): cap this UI-only pre-read for large files.
|
||||
const before = existing ? await readTextForDiff(target.targetKey, signal) : null
|
||||
// Capture an optional contextual-diff basis before the write. The bounded
|
||||
// reader checks the opened file itself, so an external replacement after
|
||||
// `probe()` cannot turn this best-effort presentation read into an
|
||||
// unbounded allocation. Either side at/above the configured limit yields
|
||||
// `before: null`; consumers retain their whole-file fallback.
|
||||
const diffable = existing !== null
|
||||
&& Buffer.byteLength(content, 'utf8') < this.config.diffBasisMaxBytes
|
||||
const before = diffable
|
||||
? await readTextForDiff(target.targetKey, this.config.diffBasisMaxBytes, signal)
|
||||
: null
|
||||
await writeFileAtomic(target.targetKey, content, existing?.mode, signal, this.internals)
|
||||
const after = await probe(target.targetKey)
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user