From fa3fd8eb3a7c76781defe61771d9f8f3418f2a3e Mon Sep 17 00:00:00 2001 From: Sergey Galuza Date: Sun, 23 Aug 2026 08:32:39 +0200 Subject: [PATCH] refactor(shared-manager): tighten canonical identity capture Build the canonical identity from the stat getCanonicalFile already takes, instead of a second lstat of the same inode. One syscall less, and mode, mtime and identity now describe the same moment rather than two adjacent ones. Assert in the adoption race tests that the foreign writer never fired. It writes only when the canonical path is observed empty, so a zero count states the invariant the fix establishes - the path is never left without a regular file - instead of only checking the final content. Built [OnSteroids](https://onsteroids.ai) --- .../shared-manager/diverged-file-adopter.ts | 16 ++++++---------- tests/unit/shared-manager.test.ts | 4 ++++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/management/shared-manager/diverged-file-adopter.ts b/src/management/shared-manager/diverged-file-adopter.ts index 1ac67e28..41dbde84 100644 --- a/src/management/shared-manager/diverged-file-adopter.ts +++ b/src/management/shared-manager/diverged-file-adopter.ts @@ -148,9 +148,7 @@ interface CanonicalIdentity { size: number; } -function readCanonicalIdentity(writePath: string): CanonicalIdentity | null { - const stats = getLstatSync(writePath); - if (!stats?.isFile()) return null; +function canonicalIdentityOf(stats: fs.Stats): CanonicalIdentity { return { ino: stats.ino, mtimeMs: stats.mtimeMs, size: stats.size }; } @@ -246,9 +244,7 @@ function publishCanonicalContent( descriptor = null; const currentStats = getLstatSync(writePath); - const current = currentStats?.isFile() - ? { ino: currentStats.ino, mtimeMs: currentStats.mtimeMs, size: currentStats.size } - : null; + const current = currentStats?.isFile() ? canonicalIdentityOf(currentStats) : null; if (!canonicalIdentityMatches(expected, current)) { throw Object.assign(new TypeError(`Canonical file changed during adoption: ${writePath}`), { code: 'EEXIST', @@ -297,12 +293,12 @@ function getCanonicalFile(canonicalPath: string): { }); } - // Identity first: a write landing between the two reads leaves us holding - // newer bytes than the identity describes, and publication fails closed. - const identity = readCanonicalIdentity(writePath); + // The identity describes the inode as of the stat above, taken before the + // content read: a write landing in between leaves us holding newer bytes + // than the identity describes, and publication fails closed. return { content: fs.readFileSync(writePath), - identity, + identity: canonicalIdentityOf(canonicalStats), mode: canonicalStats.mode & 0o777, mtimeMs: canonicalStats.mtimeMs, writePath, diff --git a/tests/unit/shared-manager.test.ts b/tests/unit/shared-manager.test.ts index a045c919..7b1e70ea 100644 --- a/tests/unit/shared-manager.test.ts +++ b/tests/unit/shared-manager.test.ts @@ -686,6 +686,10 @@ describe('SharedManager', () => { foreignWriter.restore(); } + // The writer only fires when the canonical path is observed empty, so + // zero writes is the invariant itself: adoption never left the path + // without a regular file. + expect(foreignWriter.placeholderWrites()).toBe(0); expect(fs.existsSync(canonicalPath)).toBe(true); expect(fs.readFileSync(canonicalPath, 'utf8')).not.toBe(placeholder); expect([divergedSettings, previousSettings]).toContainEqual(readJson(canonicalPath));