From 948c3d7a31a5a8349edfc9740334a4b461c379f2 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 3 Oct 2026 21:00:51 +0000 Subject: [PATCH 1/6] fix(server): failed checkpoints no longer leave temporary packs in the repository --- apps/server/src/vcs/GitVcsDriver.test.ts | 149 ++++++++++++++++++++++- apps/server/src/vcs/GitVcsDriver.ts | 123 ++++++++++++++++--- 2 files changed, 255 insertions(+), 17 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index 6b89205012812..07fb09356f536 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -15,6 +15,7 @@ import { ChildProcessSpawner } from "effect/unstable/process"; import { assert, it } from "@effect/vitest"; import { CheckpointRef, GitCommandError, VcsProcessExitError } from "@t3tools/contracts"; +import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; import * as ServerConfig from "../config.ts"; import * as CheckpointStore from "../checkpointing/CheckpointStore.ts"; import * as ProcessRunner from "../processRunner.ts"; @@ -511,10 +512,150 @@ it.effect("checkpoint recovery preserves interruption and removes the private in assert.isTrue(Exit.isFailure(exit) && Cause.hasInterruptsOnly(exit.cause)); assert.isDefined(privateIndex); assert.isFalse(yield* fs.exists(privateIndex!)); + assert.isFalse(yield* fs.exists((yield* Path.Path).dirname(privateIndex!))); assert.isFalse(yield* driver.checkpoints.hasCheckpointRef({ cwd, checkpointRef })); }).pipe(Effect.scoped, Effect.provide(GitContractLayer)), ); +// These tests rely on POSIX FIFOs, shell clean filters, and permission bits. +const windowsHost = HostProcessPlatform.defaultValue() === "win32"; + +const makeLargeFileFixture = Effect.fn("makeLargeFileFixture")(function* ( + driver: Effect.Success>, + cwd: string, +) { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const fixture = yield* makeCheckpointFixture(driver, cwd); + // Above core.bigFileThreshold, `git add` streams the file into a temporary pack. + yield* fixture.git(["config", "core.bigFileThreshold", "1k"]); + yield* fs.writeFile( + path.join(cwd, "a-large.bin"), + Uint8Array.from({ length: 8192 }, (_, index) => (index * 7919) % 251), + ); + const objects = path.join(cwd, ".git", "objects"); + const listObjectStore = Effect.gen(function* () { + const pack = yield* fs.readDirectory(path.join(objects, "pack")); + const quarantines = (yield* fs.readDirectory(objects)).filter((entry) => + entry.startsWith("tmp_objdir-"), + ); + return [...pack.map((entry) => `pack/${entry}`), ...quarantines].toSorted(); + }); + return { ...fixture, objects, listObjectStore }; +}); + +it.effect.skipIf(windowsHost)( + "concurrent checkpoint captures publish their objects with shared permissions", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const driver = yield* GitVcsDriver.makeVcsDriverShape(); + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-publish-" }); + const { git, objects, listObjectStore } = yield* makeLargeFileFixture(driver, cwd); + yield* git(["config", "core.sharedRepository", "group"]); + const existingDirectories = new Set(yield* fs.readDirectory(objects)); + const refs = ["first", "second"].map((name) => + CheckpointRef.make(`refs/t3/checkpoints/${name}`), + ); + + yield* Effect.forEach( + refs, + (checkpointRef) => driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }), + { concurrency: "unbounded" }, + ); + + const expected = (yield* git(["hash-object", "a-large.bin"])).stdout; + for (const checkpointRef of refs) { + assert.strictEqual( + (yield* git(["rev-parse", `${checkpointRef}:a-large.bin`])).stdout, + expected, + ); + } + yield* git(["fsck", "--strict", "--no-dangling"]); + assert.isFalse((yield* listObjectStore).some((entry) => entry.includes("tmp_"))); + const createdDirectories = (yield* fs.readDirectory(objects)).filter( + (entry) => !existingDirectories.has(entry), + ); + assert.isNotEmpty(createdDirectories); + for (const entry of createdDirectories) { + // Other group members' Git processes must be able to add objects here. + const { mode } = yield* fs.stat(path.join(objects, entry)); + assert.strictEqual(mode & 0o2070, 0o2070, entry); + } + }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), +); + +it.effect.skipIf(windowsHost)( + "a killed checkpoint capture leaves none of its objects in the repository", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const processRunner = yield* ProcessRunner.ProcessRunner; + const driver = yield* GitVcsDriver.makeVcsDriverShape(); + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-killed-" }); + const gates = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-gates-" }); + const { git, checkpointRef, objects, listObjectStore } = yield* makeLargeFileFixture( + driver, + cwd, + ); + const existingRef = CheckpointRef.make("refs/t3/checkpoints/existing"); + yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef: existingRef }); + const existingCommit = (yield* git(["rev-parse", existingRef])).stdout; + // A later file blocks in its clean filter while Git holds the large file's open pack. + const started = path.join(gates, "started"); + const hold = path.join(gates, "hold"); + yield* processRunner.run({ command: "mkfifo", args: [started, hold], cwd: gates }); + const filter = path.join(gates, "filter.sh"); + yield* fs.writeFileString( + filter, + `#!/bin/sh\necho started > '${started}'\ncat '${hold}' > /dev/null\ncat\n`, + ); + yield* fs.chmod(filter, 0o755); + yield* fs.writeFileString( + path.join(cwd, ".git", "info", "attributes"), + "z-gated.txt filter=gate\n", + ); + yield* git(["config", "filter.gate.clean", `'${filter}'`]); + yield* fs.writeFileString(path.join(cwd, "z-gated.txt"), "gated\n"); + // Stand-ins for a concurrent Git command's pack and another capture's quarantine. + yield* fs.writeFileString(path.join(objects, "pack", "tmp_pack_concurrent"), "concurrent"); + yield* fs.makeDirectory(path.join(objects, "tmp_objdir-t3-checkpoint-concurrent")); + const originalIndex = yield* fs.readFile(path.join(cwd, ".git", "index")); + const objectStore = yield* listObjectStore; + + for (const kill of ["interrupt", "timeout"] as const) { + const capture = yield* driver.checkpoints + .captureCheckpoint({ cwd, checkpointRef }) + .pipe(Effect.forkScoped); + yield* fs.readFileString(started); + if (kill === "interrupt") yield* Fiber.interrupt(capture); + else yield* TestClock.adjust("30 seconds"); + const exit = yield* Fiber.await(capture); + assert.isTrue(Exit.isFailure(exit)); + if (Exit.isFailure(exit)) { + if (kill === "interrupt") assert.isTrue(Cause.hasInterruptsOnly(exit.cause)); + else { + const error = Cause.findErrorOption(exit.cause); + assert.isTrue(error._tag === "Some" && error.value._tag === "VcsProcessTimeoutError"); + } + } + assert.deepEqual(yield* listObjectStore, objectStore); + } + + assert.deepEqual(yield* fs.readFile(path.join(cwd, ".git", "index")), originalIndex); + assert.strictEqual((yield* git(["rev-parse", existingRef])).stdout, existingCommit); + assert.isFalse(yield* driver.checkpoints.hasCheckpointRef({ cwd, checkpointRef })); + yield* git(["config", "--unset", "filter.gate.clean"]); + yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }); + const captured = yield* git(["rev-parse", `${checkpointRef}:a-large.bin`]); + const expected = yield* git(["hash-object", "a-large.bin"]); + assert.strictEqual(captured.stdout, expected.stdout); + yield* git(["fsck", "--strict", "--no-dangling"]); + }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), +); + it.effect("checkpoint capture does not rerun clean filters for unchanged indexed files", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; @@ -1129,8 +1270,11 @@ it.effect("GitVcsDriver forwards execute env to the VCS process", () => { it.effect("GitVcsDriver flushes checkpoint objects and refs to disk before publishing them", () => { const observedArgs: ReadonlyArray[] = []; + let objectDirectory = ""; return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + objectDirectory = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-checkpoint-fsync-" }); const driver = yield* GitVcsDriver.makeVcsDriverShape(); yield* driver.checkpoints.captureCheckpoint({ @@ -1163,6 +1307,7 @@ it.effect("GitVcsDriver flushes checkpoint objects and refs to disk before publi "commit0000", ]); }).pipe( + Effect.scoped, Effect.provide( Layer.mergeAll( NodeServices.layer, @@ -1174,8 +1319,8 @@ it.effect("GitVcsDriver flushes checkpoint objects and refs to disk before publi ? "tree0000\n" : input.args.includes("commit-tree") ? "commit0000\n" - : input.args.includes("--git-common-dir") - ? ".git\n" + : input.args.includes("--git-path") && input.args.includes("objects") + ? `${objectDirectory}\n` : ""; return { exitCode: ChildProcessSpawner.ExitCode(0), diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 99220a8bf78f6..332a73718bbeb 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -32,6 +32,7 @@ import { type VcsStatusResult, type WorktreeSubmodules, } from "@t3tools/contracts"; +import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; import { makeGitVcsDriverCore, PATCH_RENDER_PREFIX_ARGS, @@ -414,6 +415,21 @@ export class GitVcsDriver extends Context.Service< const WORKSPACE_FILES_MAX_OUTPUT_BYTES = 16 * 1024 * 1024; const CHECKPOINT_RECOVERY_MAX_CANDIDATES = 64; const CHECKPOINT_RECOVERY_TIMEOUT = "5 seconds"; +// A pack becomes visible through its index, so publish the index last. +const PACK_PUBLISH_ORDER = ["keep", "pack", "rev", "mtimes", "promisor", "idx"]; + +/** Publication order of a quarantined object file, or undefined for anything that is not one. */ +const quarantinedObjectRank = (entry: string) => { + if (/^[0-9a-f]{2}\/[0-9a-f]{38,62}$/.test(entry)) return PACK_PUBLISH_ORDER.length; + const extension = /^pack\/pack-[0-9a-f]{40,64}\.([a-z]+)$/.exec(entry)?.[1]; + const rank = extension === undefined ? -1 : PACK_PUBLISH_ORDER.indexOf(extension); + return rank === -1 ? undefined : rank; +}; + +// Git C-unquotes alternates that start with a quote, keeping path delimiters and backslashes literal. +const quoteAlternateObjectDirectory = (directory: string) => + `"${directory.replace(/[\\"]/g, "\\$&")}"`; + const GIT_CHECK_IGNORE_MAX_STDIN_BYTES = 256 * 1024; const CHECKPOINT_DIFF_MAX_OUTPUT_BYTES = 10_000_000; const WORKSPACE_GIT_HARDENED_CONFIG_ARGS = [ @@ -775,15 +791,55 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( }), ); - const resolveGitCommonDir = (cwd: string) => + const resolveObjectDirectory = (cwd: string) => Effect.gen(function* () { const result = yield* execute({ - operation: "GitVcsDriver.checkpoints.resolveGitCommonDir", + operation: "GitVcsDriver.checkpoints.resolveObjectDirectory", cwd, - args: ["rev-parse", "--git-common-dir"], + args: ["rev-parse", "--git-path", "objects"], }); - const gitCommonDir = result.stdout.trim(); - return path.isAbsolute(gitCommonDir) ? gitCommonDir : path.resolve(cwd, gitCommonDir); + return path.resolve(cwd, result.stdout.trim()); + }); + + // Moves a successful capture's new objects into the repository the way Git's own + // quarantine migration does: never replacing an object, and each pack before its index. + const publishQuarantinedObjects = (quarantine: string, objectDirectory: string) => + Effect.gen(function* () { + const objectFiles = (yield* fileSystem.readDirectory(quarantine, { recursive: true })) + .flatMap((entry) => { + const rank = quarantinedObjectRank(entry.replaceAll("\\", "/")); + return rank === undefined ? [] : [{ entry, rank }]; + }) + .toSorted((left, right) => left.rank - right.rank); + const readyDirectories = new Set(); + for (const { entry } of objectFiles) { + const directory = path.dirname(entry); + if (!readyDirectories.has(directory)) { + const target = path.join(objectDirectory, directory); + // Git created the quarantine's directory with core.sharedRepository permissions. + yield* fileSystem.makeDirectory(target).pipe( + Effect.andThen(fileSystem.stat(path.join(quarantine, directory))), + Effect.flatMap((info) => fileSystem.chmod(target, info.mode & 0o7777)), + Effect.catchIf( + (error) => error.reason._tag === "AlreadyExists", + () => Effect.void, + ), + ); + readyDirectories.add(directory); + } + const source = path.join(quarantine, entry); + const target = path.join(objectDirectory, entry); + // Objects are content-addressed, so an existing file already holds these bytes. + yield* fileSystem + .link(source, target) + .pipe( + Effect.catch((error) => + error.reason._tag === "AlreadyExists" + ? Effect.void + : fileSystem.rename(source, target), + ), + ); + } }); // Git renames loose objects and refs into place without fsync by default, so @@ -806,14 +862,25 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( "-c", "sparse.expectFilesOutsideOfPatterns=false", ]; - const gitCommonDir = yield* resolveGitCommonDir(input.cwd); - const tempIndexPath = path.join( - gitCommonDir, - `t3-checkpoint-index-${NodeCrypto.randomUUID()}`, + const objectDirectory = yield* resolveObjectDirectory(input.cwd); + // New objects go to a private directory until the capture succeeds, so a failed or + // interrupted `git add` cannot leave its temporary packs in the repository. Git names + // its own quarantines tmp_objdir-*, which lets `git gc` reclaim one a crash left behind. + const quarantine = path.join( + objectDirectory, + `tmp_objdir-t3-checkpoint-${NodeCrypto.randomUUID()}`, ); + const tempIndexPath = path.join(quarantine, "index"); const commitEnv: NodeJS.ProcessEnv = { ...process.env, GIT_INDEX_FILE: tempIndexPath, + GIT_OBJECT_DIRECTORY: quarantine, + GIT_ALTERNATE_OBJECT_DIRECTORIES: [ + quoteAlternateObjectDirectory(objectDirectory), + ...(process.env.GIT_ALTERNATE_OBJECT_DIRECTORIES + ? [process.env.GIT_ALTERNATE_OBJECT_DIRECTORIES] + : []), + ].join((yield* HostProcessPlatform) === "win32" ? ";" : ":"), GIT_AUTHOR_NAME: "T3 Code", GIT_AUTHOR_EMAIL: "t3code@users.noreply.github.com", GIT_COMMITTER_NAME: "T3 Code", @@ -827,7 +894,24 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( { discard: true }, ); + const storageError = (cause: { readonly message: string }) => + new VcsProcessExitError({ + operation, + command: "git checkpoint storage", + cwd: input.cwd, + exitCode: 0, + detail: `Could not store checkpoint objects: ${cause.message}`, + }); + const removeQuarantine = fileSystem + .remove(quarantine, { recursive: true, force: true }) + .pipe( + Effect.catch((error) => + Effect.logWarning("checkpoint temporary objects were not removed", { error }), + ), + ); + yield* Effect.gen(function* () { + yield* fileSystem.makeDirectory(quarantine).pipe(Effect.mapError(storageError)); const headExists = yield* hasHeadCommit(input.cwd); const sparseConfig = yield* execute({ operation, @@ -1058,12 +1142,21 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( }); } - yield* execute({ - operation, - cwd: input.cwd, - args: [...durableWrite, "update-ref", input.checkpointRef, commitOid], - }); - }).pipe(Effect.ensuring(cleanupTempIndex)); + // Both steps are quick. An interruption between a pack and its index would strand a pack + // that gc never removes, and one before the ref would strand the published objects. + yield* Effect.uninterruptible( + Effect.gen(function* () { + yield* publishQuarantinedObjects(quarantine, objectDirectory).pipe( + Effect.mapError(storageError), + ); + yield* execute({ + operation, + cwd: input.cwd, + args: [...durableWrite, "update-ref", input.checkpointRef, commitOid], + }); + }), + ); + }).pipe(Effect.ensuring(removeQuarantine)); }), hasCheckpointRef: (input) => From 45c6ee598f28678d7f4c9994364cbd7daf6feca3 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 4 Oct 2026 00:07:13 +0000 Subject: [PATCH 2/6] fix(server): a failed checkpoint publication no longer strands a pack without its index --- apps/server/src/vcs/GitVcsDriver.test.ts | 41 ++++++++++++++++++++++++ apps/server/src/vcs/GitVcsDriver.ts | 36 ++++++++++++++------- 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index 07fb09356f536..edc9da5319c2c 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -586,6 +586,47 @@ it.effect.skipIf(windowsHost)( }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), ); +it.effect.skipIf(windowsHost)( + "a failed checkpoint publication leaves no pack without its index", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-publish-fail-" }); + const { checkpointRef, listObjectStore } = yield* makeLargeFileFixture( + yield* GitVcsDriver.makeVcsDriverShape(), + cwd, + ); + const objectStore = yield* listObjectStore; + const diskFull = (method: string, target: string) => + Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method, + pathOrDescriptor: target, + description: "ENOSPC", + }), + ); + const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fs, + link: (source, target) => + target.endsWith(".idx") ? diskFull("link", target) : fs.link(source, target), + rename: (source, target) => + target.endsWith(".idx") ? diskFull("rename", target) : fs.rename(source, target), + }), + ); + + const exit = yield* driver.checkpoints + .captureCheckpoint({ cwd, checkpointRef }) + .pipe(Effect.exit); + + assert.isTrue(Exit.isFailure(exit)); + // Git ignores a pack without an index, and gc never removes one. + assert.deepEqual(yield* listObjectStore, objectStore); + }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), +); + it.effect.skipIf(windowsHost)( "a killed checkpoint capture leaves none of its objects in the repository", () => diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 332a73718bbeb..9971fb88cf8c4 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -803,8 +803,9 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( // Moves a successful capture's new objects into the repository the way Git's own // quarantine migration does: never replacing an object, and each pack before its index. - const publishQuarantinedObjects = (quarantine: string, objectDirectory: string) => - Effect.gen(function* () { + const publishQuarantinedObjects = (quarantine: string, objectDirectory: string) => { + const createdPackFiles: Array = []; + return Effect.gen(function* () { const objectFiles = (yield* fileSystem.readDirectory(quarantine, { recursive: true })) .flatMap((entry) => { const rank = quarantinedObjectRank(entry.replaceAll("\\", "/")); @@ -830,17 +831,28 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( const source = path.join(quarantine, entry); const target = path.join(objectDirectory, entry); // Objects are content-addressed, so an existing file already holds these bytes. - yield* fileSystem - .link(source, target) - .pipe( - Effect.catch((error) => - error.reason._tag === "AlreadyExists" - ? Effect.void - : fileSystem.rename(source, target), - ), - ); + const created = yield* fileSystem.link(source, target).pipe( + Effect.as(true), + Effect.catch((error) => + error.reason._tag === "AlreadyExists" + ? Effect.succeed(false) + : fileSystem.rename(source, target).pipe(Effect.as(true)), + ), + ); + if (created && directory === "pack") createdPackFiles.push(target); } - }); + }).pipe( + // Git ignores a pack without its index, and gc never removes one. Loose objects that were + // already published stay as ordinary unreachable objects, which gc does remove. + Effect.tapError(() => + Effect.forEach( + createdPackFiles, + (file) => fileSystem.remove(file, { force: true }).pipe(Effect.ignore), + { discard: true }, + ), + ), + ); + }; // Git renames loose objects and refs into place without fsync by default, so // an unclean restart can leave 0-byte files under refs/t3/** that break every From f3bc42514072562aee499fdb8d8eacbc12f08cc1 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 4 Oct 2026 00:11:27 +0000 Subject: [PATCH 3/6] fix(server): a failed checkpoint publication keeps packs another capture may use --- apps/server/src/vcs/GitVcsDriver.test.ts | 90 ++++++++++++++---------- apps/server/src/vcs/GitVcsDriver.ts | 17 +++-- 2 files changed, 66 insertions(+), 41 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index edc9da5319c2c..03cfeac56fa42 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -586,46 +586,64 @@ it.effect.skipIf(windowsHost)( }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), ); -it.effect.skipIf(windowsHost)( - "a failed checkpoint publication leaves no pack without its index", - () => - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-publish-fail-" }); - const { checkpointRef, listObjectStore } = yield* makeLargeFileFixture( - yield* GitVcsDriver.makeVcsDriverShape(), - cwd, - ); - const objectStore = yield* listObjectStore; - const diskFull = (method: string, target: string) => - Effect.fail( - PlatformError.systemError({ - _tag: "Unknown", - module: "FileSystem", - method, - pathOrDescriptor: target, - description: "ENOSPC", +for (const failing of ["index", "loose object"] as const) { + it.effect.skipIf(windowsHost)( + `a checkpoint publication that fails on its ${failing} keeps every pack Git can read`, + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-publish-fail-" }); + const { git, checkpointRef, listObjectStore } = yield* makeLargeFileFixture( + yield* GitVcsDriver.makeVcsDriverShape(), + cwd, + ); + const objectStore = yield* listObjectStore; + const fails = (target: string) => + failing === "index" + ? target.endsWith(".idx") + : /^[0-9a-f]{2}$/.test(path.basename(path.dirname(target))); + const diskFull = (method: string, target: string) => + Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method, + pathOrDescriptor: target, + description: "ENOSPC", + }), + ); + const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fs, + link: (source, target) => + fails(target) ? diskFull("link", target) : fs.link(source, target), + rename: (source, target) => + fails(target) ? diskFull("rename", target) : fs.rename(source, target), }), ); - const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe( - Effect.provideService(FileSystem.FileSystem, { - ...fs, - link: (source, target) => - target.endsWith(".idx") ? diskFull("link", target) : fs.link(source, target), - rename: (source, target) => - target.endsWith(".idx") ? diskFull("rename", target) : fs.rename(source, target), - }), - ); - const exit = yield* driver.checkpoints - .captureCheckpoint({ cwd, checkpointRef }) - .pipe(Effect.exit); + const exit = yield* driver.checkpoints + .captureCheckpoint({ cwd, checkpointRef }) + .pipe(Effect.exit); - assert.isTrue(Exit.isFailure(exit)); - // Git ignores a pack without an index, and gc never removes one. - assert.deepEqual(yield* listObjectStore, objectStore); - }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), -); + assert.isTrue(Exit.isFailure(exit)); + const published = (yield* listObjectStore).filter((entry) => !objectStore.includes(entry)); + if (failing === "index") { + // Git ignores a pack without its index, and gc never removes one. + assert.deepEqual(published, []); + } else { + // Once its index is published, another capture may already use the pack. + assert.deepEqual( + published.map((entry) => path.extname(entry)), + [".idx", ".pack"], + ); + const blob = (yield* git(["hash-object", "a-large.bin"])).stdout.trim(); + yield* git(["cat-file", "-e", blob]); + } + }).pipe(Effect.scoped, Effect.provide(GitCaptureContractLayer)), + ); +} it.effect.skipIf(windowsHost)( "a killed checkpoint capture leaves none of its objects in the repository", diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 9971fb88cf8c4..7337a6ee53e06 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -804,7 +804,8 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( // Moves a successful capture's new objects into the repository the way Git's own // quarantine migration does: never replacing an object, and each pack before its index. const publishQuarantinedObjects = (quarantine: string, objectDirectory: string) => { - const createdPackFiles: Array = []; + // Files this capture created for packs whose index is not published yet. + const incompletePacks = new Map>(); return Effect.gen(function* () { const objectFiles = (yield* fileSystem.readDirectory(quarantine, { recursive: true })) .flatMap((entry) => { @@ -839,14 +840,20 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( : fileSystem.rename(source, target).pipe(Effect.as(true)), ), ); - if (created && directory === "pack") createdPackFiles.push(target); + if (directory === "pack") { + const pack = path.basename(entry, path.extname(entry)); + // A published index makes the pack visible, so other captures may already use it. + if (entry.endsWith(".idx")) incompletePacks.delete(pack); + else if (created) + incompletePacks.set(pack, [...(incompletePacks.get(pack) ?? []), target]); + } } }).pipe( - // Git ignores a pack without its index, and gc never removes one. Loose objects that were - // already published stay as ordinary unreachable objects, which gc does remove. + // Git ignores a pack without its index, and gc never removes one. Complete packs and + // loose objects that were already published are unreachable objects that gc removes. Effect.tapError(() => Effect.forEach( - createdPackFiles, + [...incompletePacks.values()].flat(), (file) => fileSystem.remove(file, { force: true }).pipe(Effect.ignore), { discard: true }, ), From f1e62f7b1b759c21b085bf48e70118b4a76c8bd1 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Wed, 14 Oct 2026 05:04:33 +0000 Subject: [PATCH 4/6] fix(server): checkpoint publication never renames over an existing object --- apps/server/src/vcs/GitVcsDriver.test.ts | 37 ++++++++++++++++++++++++ apps/server/src/vcs/GitVcsDriver.ts | 13 +++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index e5c6f3ad86fc6..272bc5aa509fc 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -586,6 +586,43 @@ it.effect.skipIf(windowsHost)( }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), ); +it.effect.skipIf(windowsHost)("checkpoint publication works without hard links", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-no-links-" }); + const { git, checkpointRef, listObjectStore } = yield* makeLargeFileFixture( + yield* GitVcsDriver.makeVcsDriverShape(), + cwd, + ); + const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fs, + link: (_source, target) => + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "link", + pathOrDescriptor: target, + description: "EPERM", + }), + ), + }), + ); + + yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }); + + const expected = (yield* git(["hash-object", "a-large.bin"])).stdout; + assert.strictEqual( + (yield* git(["rev-parse", `${checkpointRef}:a-large.bin`])).stdout, + expected, + ); + assert.isTrue((yield* listObjectStore).some((entry) => entry.endsWith(".idx"))); + assert.isFalse((yield* listObjectStore).some((entry) => entry.includes("tmp_"))); + yield* git(["fsck", "--strict", "--no-dangling"]); + }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), +); + for (const failing of ["index", "loose object"] as const) { it.effect.skipIf(windowsHost)( `a checkpoint publication that fails on its ${failing} keeps every pack Git can read`, diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 2dadd6b513d45..3dab5586e8dcf 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -846,13 +846,22 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( } const source = path.join(quarantine, entry); const target = path.join(objectDirectory, entry); - // Objects are content-addressed, so an existing file already holds these bytes. + // Objects are content-addressed, so an existing file already holds these bytes. Without + // hard links, Git renames instead, unless the object already exists. const created = yield* fileSystem.link(source, target).pipe( Effect.as(true), Effect.catch((error) => error.reason._tag === "AlreadyExists" ? Effect.succeed(false) - : fileSystem.rename(source, target).pipe(Effect.as(true)), + : fileSystem + .exists(target) + .pipe( + Effect.flatMap((exists) => + exists + ? Effect.succeed(false) + : fileSystem.rename(source, target).pipe(Effect.as(true)), + ), + ), ), ); if (directory === "pack") { From be5ec20b5f05968b18acf020b679bd1c3da30ccf Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 15:03:41 +0000 Subject: [PATCH 5/6] test(server): cover checkpoint cleanup and concurrent pack adoption --- apps/server/src/vcs/GitVcsDriver.test.ts | 237 ++++++++++++----------- 1 file changed, 119 insertions(+), 118 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index 2472762ecf894..0fcea5b44d8ec 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -555,18 +555,14 @@ const makeLargeFileFixture = Effect.fn("makeLargeFileFixture")(function* ( Uint8Array.from({ length: 8192 }, (_, index) => (index * 7919) % 251), ); const objects = path.join(cwd, ".git", "objects"); - const listObjectStore = Effect.gen(function* () { - const pack = yield* fs.readDirectory(path.join(objects, "pack")); - const quarantines = (yield* fs.readDirectory(objects)).filter((entry) => - entry.startsWith("tmp_objdir-"), - ); - return [...pack.map((entry) => `pack/${entry}`), ...quarantines].toSorted(); - }); + const listObjectStore = fs + .readDirectory(objects, { recursive: true }) + .pipe(Effect.map((entries) => entries.toSorted())); return { ...fixture, objects, listObjectStore }; }); it.effect.skipIf(windowsHost)( - "concurrent checkpoint captures publish their objects with shared permissions", + "concurrent checkpoint captures stay readable with shared permissions and without hard links", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -580,18 +576,34 @@ it.effect.skipIf(windowsHost)( CheckpointRef.make(`refs/t3/checkpoints/${name}`), ); + const noLinksDriver = yield* GitVcsDriver.makeVcsDriverShape().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fs, + link: (_source, target) => + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "link", + pathOrDescriptor: target, + description: "EPERM", + }), + ), + }), + ); yield* Effect.forEach( refs, - (checkpointRef) => driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }), + (checkpointRef, index) => + (index === 0 ? driver : noLinksDriver).checkpoints.captureCheckpoint({ + cwd, + checkpointRef, + }), { concurrency: "unbounded" }, ); - const expected = (yield* git(["hash-object", "a-large.bin"])).stdout; + const expected = yield* fs.readFileString(path.join(cwd, "a-large.bin")); for (const checkpointRef of refs) { - assert.strictEqual( - (yield* git(["rev-parse", `${checkpointRef}:a-large.bin`])).stdout, - expected, - ); + assert.strictEqual((yield* git(["show", `${checkpointRef}:a-large.bin`])).stdout, expected); } yield* git(["fsck", "--strict", "--no-dangling"]); assert.isFalse((yield* listObjectStore).some((entry) => entry.includes("tmp_"))); @@ -607,104 +619,70 @@ it.effect.skipIf(windowsHost)( }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), ); -it.effect.skipIf(windowsHost)("checkpoint publication works without hard links", () => - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-no-links-" }); - const { git, checkpointRef, listObjectStore } = yield* makeLargeFileFixture( - yield* GitVcsDriver.makeVcsDriverShape(), - cwd, - ); - const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe( - Effect.provideService(FileSystem.FileSystem, { - ...fs, - link: (_source, target) => - Effect.fail( - PlatformError.systemError({ - _tag: "PermissionDenied", - module: "FileSystem", - method: "link", - pathOrDescriptor: target, - description: "EPERM", - }), - ), - }), - ); - - yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }); - - const expected = (yield* git(["hash-object", "a-large.bin"])).stdout; - assert.strictEqual( - (yield* git(["rev-parse", `${checkpointRef}:a-large.bin`])).stdout, - expected, - ); - assert.isTrue((yield* listObjectStore).some((entry) => entry.endsWith(".idx"))); - assert.isFalse((yield* listObjectStore).some((entry) => entry.includes("tmp_"))); - yield* git(["fsck", "--strict", "--no-dangling"]); - }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), -); - -for (const failing of ["index", "loose object"] as const) { - it.effect.skipIf(windowsHost)( - `a checkpoint publication that fails on its ${failing} keeps every pack Git can read`, - () => - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-publish-fail-" }); - const { git, checkpointRef, listObjectStore } = yield* makeLargeFileFixture( - yield* GitVcsDriver.makeVcsDriverShape(), - cwd, - ); - const objectStore = yield* listObjectStore; - const fails = (target: string) => - failing === "index" - ? target.endsWith(".idx") - : /^[0-9a-f]{2}$/.test(path.basename(path.dirname(target))); - const diskFull = (method: string, target: string) => - Effect.fail( - PlatformError.systemError({ - _tag: "Unknown", - module: "FileSystem", - method, - pathOrDescriptor: target, - description: "ENOSPC", - }), - ); - const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe( - Effect.provideService(FileSystem.FileSystem, { - ...fs, - link: (source, target) => - fails(target) ? diskFull("link", target) : fs.link(source, target), - rename: (source, target) => - fails(target) ? diskFull("rename", target) : fs.rename(source, target), +it.effect( + "failed checkpoint publication preserves an identical pack adopted by another capture", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const driver = yield* GitVcsDriver.makeVcsDriverShape(); + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-checkpoint-publish-race-" }); + const { git, checkpointRef, listObjectStore } = yield* makeLargeFileFixture(driver, cwd); + const installingIndex = yield* Deferred.make(); + const failInstall = yield* Deferred.make(); + const diskFull = (method: string, target: string) => + Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method, + pathOrDescriptor: target, + description: "ENOSPC", }), ); + const failingDriver = yield* GitVcsDriver.makeVcsDriverShape().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fs, + link: (source, target) => + target.endsWith(".idx") ? diskFull("link", target) : fs.link(source, target), + rename: (source, target) => + target.endsWith(".idx") + ? Deferred.succeed(installingIndex, undefined).pipe( + Effect.andThen(Deferred.await(failInstall)), + Effect.andThen(diskFull("rename", target)), + ) + : fs.rename(source, target), + }), + ); + const failedCapture = yield* failingDriver.checkpoints + .captureCheckpoint({ cwd, checkpointRef }) + .pipe(Effect.exit, Effect.forkScoped); + yield* Effect.addFinalizer(() => Deferred.succeed(failInstall, undefined)); + yield* Deferred.await(installingIndex); - const exit = yield* driver.checkpoints - .captureCheckpoint({ cwd, checkpointRef }) - .pipe(Effect.exit); + const adoptedRef = CheckpointRef.make("refs/t3/checkpoints/adopted"); + yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef: adoptedRef }); + const expected = yield* fs.readFileString(path.join(cwd, "a-large.bin")); + assert.strictEqual((yield* git(["show", `${adoptedRef}:a-large.bin`])).stdout, expected); - assert.isTrue(Exit.isFailure(exit)); - const published = (yield* listObjectStore).filter((entry) => !objectStore.includes(entry)); - if (failing === "index") { - // Git ignores a pack without its index, and gc never removes one. - assert.deepEqual(published, []); - } else { - // Once its index is published, another capture may already use the pack. - assert.deepEqual( - published.map((entry) => path.extname(entry)), - [".idx", ".pack"], - ); - const blob = (yield* git(["hash-object", "a-large.bin"])).stdout.trim(); - yield* git(["cat-file", "-e", blob]); - } - }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), - ); -} + yield* Deferred.succeed(failInstall, undefined); + assert.isTrue(Exit.isFailure(yield* Fiber.join(failedCapture))); + assert.isFalse(yield* driver.checkpoints.hasCheckpointRef({ cwd, checkpointRef })); + const adopted = yield* driver.execute({ + operation: "checkpoint-test", + cwd, + args: ["show", `${adoptedRef}:a-large.bin`], + allowNonZeroExit: true, + }); + assert.strictEqual(adopted.exitCode, 0, adopted.stderr); + assert.strictEqual(adopted.stdout, expected); + assert.isFalse((yield* listObjectStore).some((entry) => entry.includes("tmp_"))); + yield* git(["fsck", "--strict", "--no-dangling"]); + }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), +); it.effect.skipIf(windowsHost)( - "a killed checkpoint capture leaves none of its objects in the repository", + "failed, interrupted, and timed-out captures clean up only their own temporary objects", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -719,7 +697,12 @@ it.effect.skipIf(windowsHost)( ); const existingRef = CheckpointRef.make("refs/t3/checkpoints/existing"); yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef: existingRef }); - const existingCommit = (yield* git(["rev-parse", existingRef])).stdout; + const existingRefs = (yield* git(["show-ref"])).stdout; + yield* fs.writeFileString( + path.join(cwd, "a-large.bin"), + "new packed contents\n".repeat(1024), + ); + yield* fs.writeFileString(path.join(cwd, "b-loose.txt"), "new loose object\n"); // A later file blocks in its clean filter while Git holds the large file's open pack. const started = path.join(gates, "started"); const hold = path.join(gates, "hold"); @@ -727,7 +710,7 @@ it.effect.skipIf(windowsHost)( const filter = path.join(gates, "filter.sh"); yield* fs.writeFileString( filter, - `#!/bin/sh\necho started > '${started}'\ncat '${hold}' > /dev/null\ncat\n`, + `#!/bin/sh\necho started > '${started}'\ncat '${hold}' > /dev/null\nexit 1\n`, ); yield* fs.chmod(filter, 0o755); yield* fs.writeFileString( @@ -735,6 +718,7 @@ it.effect.skipIf(windowsHost)( "z-gated.txt filter=gate\n", ); yield* git(["config", "filter.gate.clean", `'${filter}'`]); + yield* git(["config", "filter.gate.required", "true"]); yield* fs.writeFileString(path.join(cwd, "z-gated.txt"), "gated\n"); // Stand-ins for a concurrent Git command's pack and another capture's quarantine. yield* fs.writeFileString(path.join(objects, "pack", "tmp_pack_concurrent"), "concurrent"); @@ -742,33 +726,50 @@ it.effect.skipIf(windowsHost)( const originalIndex = yield* fs.readFile(path.join(cwd, ".git", "index")); const objectStore = yield* listObjectStore; - for (const kill of ["interrupt", "timeout"] as const) { + for (const outcome of ["failure", "interrupt", "timeout"] as const) { const capture = yield* driver.checkpoints .captureCheckpoint({ cwd, checkpointRef }) .pipe(Effect.forkScoped); yield* fs.readFileString(started); - if (kill === "interrupt") yield* Fiber.interrupt(capture); + assert.isTrue( + (yield* listObjectStore).some((entry) => /tmp_pack_(?!concurrent)/.test(entry)), + ); + if (outcome === "failure") yield* fs.writeFileString(hold, "release\n"); + else if (outcome === "interrupt") yield* Fiber.interrupt(capture); else yield* TestClock.adjust("30 seconds"); const exit = yield* Fiber.await(capture); assert.isTrue(Exit.isFailure(exit)); if (Exit.isFailure(exit)) { - if (kill === "interrupt") assert.isTrue(Cause.hasInterruptsOnly(exit.cause)); + if (outcome === "interrupt") assert.isTrue(Cause.hasInterruptsOnly(exit.cause)); else { const error = Cause.findErrorOption(exit.cause); - assert.isTrue(error._tag === "Some" && error.value._tag === "VcsProcessTimeoutError"); + assert.isTrue(error._tag === "Some"); + if (error._tag === "Some") + assert.strictEqual( + error.value._tag, + outcome === "timeout" ? "VcsProcessTimeoutError" : "VcsProcessExitError", + ); } } assert.deepEqual(yield* listObjectStore, objectStore); + assert.deepEqual(yield* fs.readFile(path.join(cwd, ".git", "index")), originalIndex); + assert.strictEqual((yield* git(["show-ref"])).stdout, existingRefs); + assert.strictEqual( + yield* fs.readFileString(path.join(objects, "pack", "tmp_pack_concurrent")), + "concurrent", + ); + assert.isFalse(yield* driver.checkpoints.hasCheckpointRef({ cwd, checkpointRef })); } - assert.deepEqual(yield* fs.readFile(path.join(cwd, ".git", "index")), originalIndex); - assert.strictEqual((yield* git(["rev-parse", existingRef])).stdout, existingCommit); - assert.isFalse(yield* driver.checkpoints.hasCheckpointRef({ cwd, checkpointRef })); yield* git(["config", "--unset", "filter.gate.clean"]); + yield* git(["config", "--unset", "filter.gate.required"]); yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }); - const captured = yield* git(["rev-parse", `${checkpointRef}:a-large.bin`]); - const expected = yield* git(["hash-object", "a-large.bin"]); - assert.strictEqual(captured.stdout, expected.stdout); + const captured = yield* git(["show", `${checkpointRef}:a-large.bin`]); + assert.strictEqual(captured.stdout, yield* fs.readFileString(path.join(cwd, "a-large.bin"))); + assert.strictEqual( + (yield* git(["show", `${checkpointRef}:b-loose.txt`])).stdout, + "new loose object\n", + ); yield* git(["fsck", "--strict", "--no-dangling"]); }).pipe(Effect.scoped, Effect.provide(layerGitCaptureContract)), ); From f6d50da52d28867194f9174df559ddfe33200146 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 15:12:01 +0000 Subject: [PATCH 6/6] fix(server): preserve checkpoint packs adopted by concurrent captures --- apps/server/src/vcs/GitVcsDriver.ts | 134 +++++++++++++--------------- packages/contracts/src/vcs.ts | 1 + 2 files changed, 65 insertions(+), 70 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 7dd82bcbfe36c..d83317e7202bd 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -795,74 +795,55 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( return path.resolve(cwd, result.stdout.trim()); }); - // Moves a successful capture's new objects into the repository the way Git's own - // quarantine migration does: never replacing an object, and each pack before its index. - const publishQuarantinedObjects = (quarantine: string, objectDirectory: string) => { - // Files this capture created for packs whose index is not published yet. - const incompletePacks = new Map>(); - return Effect.gen(function* () { - const objectFiles = (yield* fileSystem.readDirectory(quarantine, { recursive: true })) - .flatMap((entry) => { - const rank = quarantinedObjectRank(entry.replaceAll("\\", "/")); - return rank === undefined ? [] : [{ entry, rank }]; - }) - .toSorted((left, right) => left.rank - right.rank); - const readyDirectories = new Set(); - for (const { entry } of objectFiles) { - const directory = path.dirname(entry); - if (!readyDirectories.has(directory)) { - const target = path.join(objectDirectory, directory); - // Git created the quarantine's directory with core.sharedRepository permissions. - yield* fileSystem.makeDirectory(target).pipe( - Effect.andThen(fileSystem.stat(path.join(quarantine, directory))), - Effect.flatMap((info) => fileSystem.chmod(target, info.mode & 0o7777)), - Effect.catchIf( - (error) => error.reason._tag === "AlreadyExists", - () => Effect.void, - ), - ); - readyDirectories.add(directory); - } - const source = path.join(quarantine, entry); - const target = path.join(objectDirectory, entry); - // Objects are content-addressed, so an existing file already holds these bytes. Without - // hard links, Git renames instead, unless the object already exists. - const created = yield* fileSystem.link(source, target).pipe( - Effect.as(true), + // Published files are shared immediately: another capture can adopt a pack before we + // install its index. Even on failure, only the private quarantine is ours to remove. + const publishQuarantinedObjects = Effect.fnUntraced(function* ( + quarantine: string, + objectDirectory: string, + ) { + const objectFiles = (yield* fileSystem.readDirectory(quarantine, { recursive: true })) + .flatMap((entry) => { + const rank = quarantinedObjectRank(entry.replaceAll("\\", "/")); + return rank === undefined ? [] : [{ entry, rank }]; + }) + .toSorted((left, right) => left.rank - right.rank); + const readyDirectories = new Set(); + for (const { entry } of objectFiles) { + const directory = path.dirname(entry); + if (!readyDirectories.has(directory)) { + const target = path.join(objectDirectory, directory); + // Git created the quarantine's directory with core.sharedRepository permissions. + yield* fileSystem.makeDirectory(target).pipe( + Effect.andThen(fileSystem.stat(path.join(quarantine, directory))), + Effect.flatMap((info) => fileSystem.chmod(target, info.mode & 0o7777)), + Effect.catchIf( + (error) => error.reason._tag === "AlreadyExists", + () => Effect.void, + ), + ); + readyDirectories.add(directory); + } + const source = path.join(quarantine, entry); + const target = path.join(objectDirectory, entry); + // Objects are content-addressed, so an existing file already holds these bytes. Without + // hard links, Git renames instead, unless the object already exists. + yield* fileSystem + .link(source, target) + .pipe( Effect.catch((error) => error.reason._tag === "AlreadyExists" - ? Effect.succeed(false) + ? Effect.void : fileSystem .exists(target) .pipe( Effect.flatMap((exists) => - exists - ? Effect.succeed(false) - : fileSystem.rename(source, target).pipe(Effect.as(true)), + exists ? Effect.void : fileSystem.rename(source, target), ), ), ), ); - if (directory === "pack") { - const pack = path.basename(entry, path.extname(entry)); - // A published index makes the pack visible, so other captures may already use it. - if (entry.endsWith(".idx")) incompletePacks.delete(pack); - else if (created) - incompletePacks.set(pack, [...(incompletePacks.get(pack) ?? []), target]); - } - } - }).pipe( - // Git ignores a pack without its index, and gc never removes one. Complete packs and - // loose objects that were already published are unreachable objects that gc removes. - Effect.tapError(() => - Effect.forEach( - [...incompletePacks.values()].flat(), - (file) => fileSystem.remove(file, { force: true }).pipe(Effect.ignore), - { discard: true }, - ), - ), - ); - }; + } + }); // Git renames loose objects and refs into place without fsync by default, so // an unclean restart can leave 0-byte files under refs/t3/** that break every @@ -920,14 +901,6 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( { discard: true }, ); - const storageError = (cause: { readonly message: string }) => - new VcsProcessExitError({ - operation, - command: "git checkpoint storage", - cwd: input.cwd, - exitCode: 0, - detail: `Could not store checkpoint objects: ${cause.message}`, - }); const removeQuarantine = fileSystem .remove(quarantine, { recursive: true, force: true }) .pipe( @@ -937,7 +910,19 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( ); yield* Effect.gen(function* () { - yield* fileSystem.makeDirectory(quarantine).pipe(Effect.mapError(storageError)); + yield* fileSystem.makeDirectory(quarantine).pipe( + Effect.mapError( + (cause) => + new VcsProcessExitError({ + operation, + command: "git checkpoint storage", + cwd: input.cwd, + exitCode: 0, + detail: "Could not create checkpoint object storage.", + cause, + }), + ), + ); const headExists = yield* hasHeadCommit(input.cwd); const sparseConfig = yield* execute({ operation, @@ -1168,12 +1153,21 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( }); } - // Both steps are quick. An interruption between a pack and its index would strand a pack - // that gc never removes, and one before the ref would strand the published objects. + // Finish publishing objects and their ref together when cancellation arrives. yield* Effect.uninterruptible( Effect.gen(function* () { yield* publishQuarantinedObjects(quarantine, objectDirectory).pipe( - Effect.mapError(storageError), + Effect.mapError( + (cause) => + new VcsProcessExitError({ + operation, + command: "git checkpoint storage", + cwd: input.cwd, + exitCode: 0, + detail: "Could not publish checkpoint objects.", + cause, + }), + ), ); yield* execute({ operation, diff --git a/packages/contracts/src/vcs.ts b/packages/contracts/src/vcs.ts index f92e4fc1d5c48..f5848d4b710d3 100644 --- a/packages/contracts/src/vcs.ts +++ b/packages/contracts/src/vcs.ts @@ -122,6 +122,7 @@ export class VcsProcessExitError extends Schema.TaggedError retryable: Schema.optional(Schema.Boolean), stderrLength: Schema.optional(NonNegativeInt), stderrTruncated: Schema.optional(Schema.Boolean), + cause: Schema.optional(Schema.Defect()), }, ) { override get message(): string {