From 21ff4911bfc2a10ba15560c8d3c2daca1fb04793 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:57:11 +0800 Subject: [PATCH] refactor(setup): share publication byte verification Use one bounded positional byte comparator for client configuration and skill publication readback. Keep pathname identity, rollback, mode restoration, cleanup and retry authority in the existing publication owners. Preserve short-read progress, EOF, empty-file equality and original I/O errors. Extend existing byte-exact skill recovery coverage to empty originals. Validation: client configuration transaction, skill publication recovery and process-global rollback suites passed; static, docs and discovery checks passed. --- .../ClientConfigurationPublication.ts | 39 +++---------------- src/application/SkillFilePublication.ts | 27 ++----------- src/filesystem/fileHandleMatchesBytes.ts | 27 +++++++++++++ .../filesystem/setupSkillPublication.test.ts | 1 + 4 files changed, 37 insertions(+), 57 deletions(-) create mode 100644 src/filesystem/fileHandleMatchesBytes.ts diff --git a/src/application/ClientConfigurationPublication.ts b/src/application/ClientConfigurationPublication.ts index 230f50d81b..d46c4725e5 100644 --- a/src/application/ClientConfigurationPublication.ts +++ b/src/application/ClientConfigurationPublication.ts @@ -1,16 +1,9 @@ import { constants, type Stats } from "node:fs"; -import { - link, - lstat, - mkdtemp, - open, - realpath, - rename, - type FileHandle, -} from "node:fs/promises"; +import { link, lstat, mkdtemp, open, realpath, rename } from "node:fs/promises"; import { basename, dirname, join, resolve, sep } from "node:path"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { fileHandleMatchesBytes } from "../filesystem/fileHandleMatchesBytes.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import { @@ -160,7 +153,10 @@ export class ClientConfigurationPublication { !before.isFile() || named.isSymbolicLink() || !sameRegularFileState(before, named) || - !(await this.#hasPublishedBytes(handle)) || + !(await fileHandleMatchesBytes( + handle, + Buffer.from(this.contents, "utf8"), + )) || !sameRegularFileState(before, await handle.stat()) || !sameRegularFileState(before, await lstat(this.path)) ) @@ -185,29 +181,6 @@ export class ClientConfigurationPublication { if (!sameRegularFileState(await handle.stat(), await lstat(this.path))) throw new RegularFileChangedError(this.path); } - - async #hasPublishedBytes(handle: FileHandle): Promise { - const expected = Buffer.from(this.contents, "utf8"); - if ((await handle.stat()).size !== expected.length) return false; - const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, expected.length)); - for (let position = 0; position < expected.length;) { - const { bytesRead } = await handle.read( - chunk, - 0, - Math.min(chunk.length, expected.length - position), - position, - ); - if ( - bytesRead === 0 || - !chunk - .subarray(0, bytesRead) - .equals(expected.subarray(position, position + bytesRead)) - ) - return false; - position += bytesRead; - } - return true; - } } const assertRequestedConfigurationPath = async ( diff --git a/src/application/SkillFilePublication.ts b/src/application/SkillFilePublication.ts index 0cc4977381..b07a1dff6e 100644 --- a/src/application/SkillFilePublication.ts +++ b/src/application/SkillFilePublication.ts @@ -3,6 +3,7 @@ import { link, lstat, mkdtemp, open, realpath, rename } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { fileHandleMatchesBytes } from "../filesystem/fileHandleMatchesBytes.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import { @@ -163,7 +164,7 @@ export class SkillFilePublication { ) throw new RegularFileChangedError(this.path); await this.#assertNamed(before); - if (!(await this.#matches(this.bytes))) + if (!(await fileHandleMatchesBytes(this.owner.handle, this.bytes))) throw new RegularFileChangedError(this.path); if (!sameRegularFileState(before, await this.owner.handle.stat())) throw new RegularFileChangedError(this.path); @@ -203,7 +204,7 @@ export class SkillFilePublication { await this.owner.handle.chmod(original.mode); await this.owner.handle.sync(); await this.#assertNamed(await this.owner.handle.stat()); - if (!(await this.#matches(original.bytes))) + if (!(await fileHandleMatchesBytes(this.owner.handle, original.bytes))) throw new RegularFileChangedError(this.path); this.#restored = true; } @@ -222,28 +223,6 @@ export class SkillFilePublication { throw new RegularFileChangedError(this.path); } - async #matches(expected: Buffer): Promise { - if ((await this.owner.handle.stat()).size !== expected.length) return false; - const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, expected.length)); - for (let position = 0; position < expected.length;) { - const { bytesRead } = await this.owner.handle.read( - chunk, - 0, - Math.min(chunk.length, expected.length - position), - position, - ); - if ( - bytesRead === 0 || - !chunk - .subarray(0, bytesRead) - .equals(expected.subarray(position, position + bytesRead)) - ) - return false; - position += bytesRead; - } - return true; - } - async #assertSelected(): Promise { const current = await optionalState(this.path); if ( diff --git a/src/filesystem/fileHandleMatchesBytes.ts b/src/filesystem/fileHandleMatchesBytes.ts new file mode 100644 index 0000000000..3883e944d1 --- /dev/null +++ b/src/filesystem/fileHandleMatchesBytes.ts @@ -0,0 +1,27 @@ +import type { FileHandle } from "node:fs/promises"; + +/** Compare descriptor bytes without taking ownership or validating pathname identity. */ +export const fileHandleMatchesBytes = async ( + handle: Pick, + expected: Buffer, +): Promise => { + if ((await handle.stat()).size !== expected.length) return false; + const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, expected.length)); + for (let position = 0; position < expected.length;) { + const { bytesRead } = await handle.read( + chunk, + 0, + Math.min(chunk.length, expected.length - position), + position, + ); + if ( + bytesRead === 0 || + !chunk + .subarray(0, bytesRead) + .equals(expected.subarray(position, position + bytesRead)) + ) + return false; + position += bytesRead; + } + return true; +}; diff --git a/tests/boundary/filesystem/setupSkillPublication.test.ts b/tests/boundary/filesystem/setupSkillPublication.test.ts index 336d88e1b2..a25797eb39 100644 --- a/tests/boundary/filesystem/setupSkillPublication.test.ts +++ b/tests/boundary/filesystem/setupSkillPublication.test.ts @@ -123,6 +123,7 @@ describe("skill publication ownership", () => { describe("skill publication recovery", () => { it.each([ + Buffer.alloc(0), Buffer.from([0xff, 0xfe, 0x61, 0x00]), Buffer.from("\ufefforiginal BOM instructions\r\n"), ])(