From 34b8782e49099278de9668cec9f0db7229ef4749 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Thu, 3 Sep 2026 05:34:00 +0000 Subject: [PATCH] fix(native-playground): adopt a hard-linked catalog winner while its staging link is still present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hard-link publication leaves the sidecar doubly linked between the winner's link() and the unlink of its .stage- file. A loser (or any reader) arriving in that window hit the nlink !== 1 guard in #readSidecar and failed with "Native Playground catalog snapshot is invalid." — the main CI flake on the #364 merge run (Node 24). Account for the extra link by identity: a same-epoch staging sibling sharing dev/ino, or the open handle reporting nlink 1 once the staging file is gone. Other extra hard links stay rejected. Adds a deterministic gate-ordered reproduction and an aliasing regression test. --- .changeset/native-catalog-linked-winner.md | 5 ++ .../playground/native-playground-service.ts | 43 +++++++--- .../tests/native-playground-service.test.ts | 85 ++++++++++++++++++- 3 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 .changeset/native-catalog-linked-winner.md diff --git a/.changeset/native-catalog-linked-winner.md b/.changeset/native-catalog-linked-winner.md new file mode 100644 index 000000000..055177c56 --- /dev/null +++ b/.changeset/native-catalog-linked-winner.md @@ -0,0 +1,5 @@ +--- +"agent-bundle": patch +--- + +Native Playground catalog readers no longer reject a sidecar that a concurrent publisher has just hard-linked into place but not yet released its staging file for. Hard-link publication legitimately leaves the sidecar doubly linked until the winner unlinks its `.stage-` file; a loser (or any reader) arriving inside that window previously failed with `Native Playground catalog snapshot is invalid.` instead of adopting the winner. The extra link is now accounted for by identity — exactly one same-epoch staging sibling shares the sidecar's dev/ino, or the still-open handle reports a single link once the staging file is gone — and any other extra hard link stays rejected as aliasing. diff --git a/packages/agent-bundle/src/dev/playground/native-playground-service.ts b/packages/agent-bundle/src/dev/playground/native-playground-service.ts index 3ff4423dc..ff48b256c 100644 --- a/packages/agent-bundle/src/dev/playground/native-playground-service.ts +++ b/packages/agent-bundle/src/dev/playground/native-playground-service.ts @@ -1,6 +1,6 @@ -import { constants } from 'node:fs'; -import { link, lstat, mkdir, mkdtemp, open, realpath, rename, rm } from 'node:fs/promises'; -import { dirname, isAbsolute, join, relative, resolve } from 'node:path'; +import { constants, type Stats } from 'node:fs'; +import { link, lstat, mkdir, mkdtemp, open, readdir, realpath, rename, rm, type FileHandle } from 'node:fs/promises'; +import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { digest, stableJson } from '../../core/digest.ts'; import { hasExactOwnKeys, isJsonRecord, parseJsonWithoutDuplicateKeys, snapshotStrictJsonValue, type JsonValue } from '../../core/strict-json.ts'; @@ -24,7 +24,7 @@ import { safeDevWireText } from '../logs/dev-log-service.ts'; import type { ArtifactEpoch } from '../types.ts'; import { workspaceDiff, type WorkspaceDiff } from '../../eval/workspace-diff.ts'; import { isErrno } from '../../core/errors.ts'; -import { isInsideOrEqual } from '../../core/paths.ts'; +import { isInsideOrEqual, sameFile } from '../../core/paths.ts'; export type { NativePlaygroundHost } from './native-playground-types.ts'; @@ -1017,12 +1017,10 @@ export class NativePlaygroundService { const file = await open(path, catalogOpenFlags); try { const metadata = await file.stat(); - if ( - !metadata.isFile() || - metadata.nlink < 1 || - (!allowMultipleLinks && metadata.nlink !== 1) || - metadata.size > maximumCatalogSnapshotBytes - ) { + if (!metadata.isFile() || metadata.nlink < 1 || metadata.size > maximumCatalogSnapshotBytes) { + throw new Error('Native Playground catalog snapshot is invalid.'); + } + if (!allowMultipleLinks && metadata.nlink !== 1 && !(await this.#stagingLinkAccountsFor(file, path, metadata))) { throw new Error('Native Playground catalog snapshot is invalid.'); } const buffer = Buffer.allocUnsafe(maximumCatalogSnapshotBytes + 1); @@ -1048,6 +1046,31 @@ export class NativePlaygroundService { } } + /** + * Hard-link publication leaves a freshly linked sidecar doubly linked until + * the winner releases its staging file. A concurrent reader must adopt that + * winner rather than reject it, so the extra link is accounted for by + * identity: exactly one staging sibling of this epoch shares the sidecar's + * dev/ino, or the staging link was released while the directory was being + * listed and the still-open handle now reports a single link. Any other + * extra link is hostile aliasing and stays rejected. + */ + async #stagingLinkAccountsFor(file: FileHandle, path: string, metadata: Stats): Promise { + if (metadata.nlink !== 2) return false; + const directory = dirname(path); + const stagingPrefix = `.${basename(path, '.json')}.stage-`; + for (const entry of await readdir(directory)) { + if (!entry.startsWith(stagingPrefix)) continue; + try { + const staged = await lstat(join(directory, entry)); + if (staged.isFile() && sameFile(staged, metadata)) return true; + } catch (error) { + if (!isErrno(error, 'ENOENT')) throw error; + } + } + return (await file.stat()).nlink === 1; + } + async #persistSnapshot( reference: NativePlaygroundEpochReference, snapshot: PersistedCatalogSnapshot, diff --git a/packages/agent-bundle/tests/native-playground-service.test.ts b/packages/agent-bundle/tests/native-playground-service.test.ts index 614427798..3b5f0e690 100644 --- a/packages/agent-bundle/tests/native-playground-service.test.ts +++ b/packages/agent-bundle/tests/native-playground-service.test.ts @@ -1,4 +1,4 @@ -import { link, mkdir, mkdtemp, open, readFile, readdir, rename, rm, symlink, writeFile } from 'node:fs/promises'; +import { link, mkdir, mkdtemp, open, readFile, readdir, rename, rm, stat, symlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -1473,6 +1473,89 @@ it('fsyncs durable catalog publication, validates a no-replace winner, and retai } }); +it('adopts a linked winner while its staging link is still present instead of rejecting the doubly linked sidecar', async () => { + const root = await mkdtemp(join(tmpdir(), 'agent-bundle-native-playground-linked-winner-')); + const catalogDirectory = join(root, 'catalog'); + const reference = epoch('epoch-linked-winner', join(root, 'artifact')); + const sidecar = join(catalogDirectory, `${reference.epoch.id}.json`); + let winnerStaging: string | undefined; + let signalLinked!: () => void; + const linked = new Promise((resolvePromise) => { signalLinked = resolvePromise; }); + let releaseWinnerCleanup!: () => void; + const winnerCleanup = new Promise((resolvePromise) => { releaseWinnerCleanup = resolvePromise; }); + const storage: NativePlaygroundCatalogStorage = { + link: async (source, destination) => { + await link(source, destination); + winnerStaging = String(source); + signalLinked(); + }, + mkdir, + open, + remove: async (path, options) => { + if (String(path) === winnerStaging) await winnerCleanup; + await rm(path, options); + }, + }; + const serviceFor = (): NativePlaygroundService => new NativePlaygroundService({ + catalogDirectory, + catalogStorage: storage, + discover: async () => suite(), + inspectArtifact: async (candidate) => Object.freeze({ + binding: Object.freeze({ manifestPath: 'agent-bundle.manifest.json', source: 'explicit' as const, targetDigests: candidate.epoch.targetDigests }), + root: candidate.root, + }), + planFixture: async () => fixturePlan, + projectRoot: '/project', + }); + const winner = serviceFor(); + const loser = serviceFor(); + try { + const winning = winner.catalog(reference); + await linked; + // The winner has linked its staging file into place but has not released + // it yet: the published sidecar is legitimately doubly linked here. + expect((await stat(sidecar)).nlink).toBe(2); + + const losing = await loser.catalog(reference); + expect((await stat(sidecar)).nlink).toBe(2); + releaseWinnerCleanup(); + expect(await winning).toEqual(losing); + expect((await stat(sidecar)).nlink).toBe(1); + expect((await readdir(catalogDirectory)).filter((name) => name.includes('.stage-'))).toEqual([]); + await Promise.all([winner.close(), loser.close()]); + } finally { + releaseWinnerCleanup(); + await rm(root, { force: true, recursive: true }); + } +}); + +it('still rejects a persisted catalog aliased by a hard link that is not an epoch staging file', async () => { + const root = await mkdtemp(join(tmpdir(), 'agent-bundle-native-playground-aliased-catalog-')); + const catalogDirectory = join(root, 'catalog'); + const reference = epoch('epoch-aliased-catalog', join(root, 'artifact')); + const sidecar = join(catalogDirectory, `${reference.epoch.id}.json`); + const serviceFor = (discover: () => Promise): NativePlaygroundService => new NativePlaygroundService({ + catalogDirectory, + discover, + planFixture: async () => fixturePlan, + projectRoot: '/project', + }); + try { + const writer = serviceFor(async () => suite()); + await writer.catalog(reference); + await writer.close(); + for (const alias of ['alias.json', '.epoch-other.stage-alias', `.${reference.epoch.id}.staged`]) { + await link(sidecar, join(catalogDirectory, alias)); + const reader = serviceFor(async () => { throw new Error('An aliased catalog must not fall back to discovery.'); }); + await expect(reader.catalog(reference)).rejects.toThrow('catalog snapshot is invalid'); + await reader.close(); + await rm(join(catalogDirectory, alias)); + } + } finally { + await rm(root, { force: true, recursive: true }); + } +}); + it('drains a gated catalog discovery before close and never publishes it after close begins', async () => { const root = await mkdtemp(join(tmpdir(), 'agent-bundle-native-playground-catalog-close-')); try {