Public: anyone can read this; the crew can change it.
A crew clone that fails on a broken mirror drops the mirror and clones plain, so one bad object never stops a run #5120
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5109-a-crew-clone-that-fails-on-a-broken-mirr → main 2 commits fork 2 threads · 2 unresolved
2 files
changed +71
−4
src/crew/mirror.ts +22 −3
| @@ -7,7 +7,8 @@ | ||
| 7 | 7 | * |
| 8 | 8 | * The clone borrows with `--reference-if-able` and `--dissociate`: it copies |
| 9 | 9 | * what it borrowed and keeps no link, so a deleted mirror breaks nothing. A |
| 10 | * mirror that cannot be fetched is skipped, and the clone is a plain one. | |
| 10 | * mirror that cannot be fetched is skipped, and the clone is a plain one. A | |
| 11 | * mirror whose objects git finds damaged is dropped, and the clone is plain. | |
| 11 | 12 | */ |
| 12 | 13 | |
| 13 | 14 | import { existsSync, mkdirSync, rmSync } from "node:fs"; |
| @@ -61,6 +62,12 @@ async function refresh( | ||
| 61 | 62 | return undefined; |
| 62 | 63 | } |
| 63 | 64 | |
| 65 | /** Git's wording for a damaged object store; a timeout kill or a dead origin says none of it. */ | |
| 66 | const damaged = (stderr: string): boolean => | |
| 67 | /bad object|corrupt|inflate|did not receive expected object|missing object|unable to parse|invalid object|checkout failed/i.test( | |
| 68 | stderr, | |
| 69 | ); | |
| 70 | ||
| 64 | 71 | const validId = (repoId: string): boolean => /^[A-Za-z0-9_-]+$/.test(repoId); |
| 65 | 72 | |
| 66 | 73 | /** |
| @@ -88,7 +95,8 @@ export function dropMirror(dataDir: string, repoId: string): void { | ||
| 88 | 95 | /** |
| 89 | 96 | * `git clone` of `url` into `dir`, borrowing from the repository's mirror |
| 90 | 97 | * when there is one. The clone runs inside the chain: a rebuild of the mirror |
| 91 | * cannot delete it while it is being read. | |
| 98 | * cannot delete it while it is being read. `dir` must be fresh: a fallback | |
| 99 | * removes whatever the failed clone left there. | |
| 92 | 100 | */ |
| 93 | 101 | export async function cloneWithMirror(input: { |
| 94 | 102 | dataDir: string; |
| @@ -110,9 +118,20 @@ export async function cloneWithMirror(input: { | ||
| 110 | 118 | return exclusive(input.repoId, async () => { |
| 111 | 119 | const mirror = await refresh(mirrorDir(input.dataDir, input.repoId), input.url, opts); |
outdated
Gate this on corruption. A timeout kill or an unreachable origin also reaches this line and deletes a good mirror, then repeats a full clone. |
||
| 112 | 120 | if (mirror === undefined) return plain(); |
| 113 | return runGit( | |
| 121 | const borrowed = await runGit( | |
| 114 | 122 | ["clone", "--quiet", "--reference-if-able", mirror, "--dissociate", "--", input.url, input.dir], |
| 115 | 123 | opts, |
| 116 | 124 | ); |
| 125 | if (borrowed.ok || input.signal?.aborted) return borrowed; | |
| 126 | // A timeout (SIGKILL, empty stderr) or an unreachable origin is not the mirror's fault. | |
| 127 | if (!damaged(borrowed.error.stderr)) return borrowed; | |
| 128 | // A damaged mirror costs one slower clone, never the run. | |
| 129 | log.warn("a clone from the mirror failed; dropping it and cloning plain", { | |
| 130 | repo: input.repoId, | |
| 131 | error: borrowed.error.stderr.split("\n")[0] ?? "", | |
| 132 | }); | |
| 133 | rmSync(input.dir, { recursive: true, force: true }); | |
| 134 | rmSync(mirror, { recursive: true, force: true }); | |
| 135 | return plain(); | |
| 117 | 136 | }); |
| 118 | 137 | } |
test/crew-mirror.test.ts +49 −1
| @@ -1,7 +1,7 @@ | ||
| 1 | 1 | /** A second clone borrows from the repository's mirror; a deleted mirror breaks nothing. */ |
| 2 | 2 | |
| 3 | 3 | import { afterAll, beforeAll, expect, test } from "bun:test"; |
| 4 | import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; | |
| 4 | import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; | |
| 5 | 5 | import { tmpdir } from "node:os"; |
| 6 | 6 | import { join } from "node:path"; |
| 7 | 7 | import { seedAdmin } from "./helpers/admin.ts"; |
| @@ -223,3 +223,51 @@ test("removing an account deletes its repositories' mirrors without waiting for | ||
| 223 | 223 | app.close(); |
| 224 | 224 | } |
| 225 | 225 | }); |
| 226 | ||
| 227 | test("a damaged mirror is dropped and the clone falls back to a plain one", async () => { | |
| 228 | expect((await mirrored(data(), "r7")).ok).toBe(true); | |
| 229 | const mirror = mirrorDir(data(), "r7"); | |
| 230 | // Loosen the objects, then garble the blob: a fetch that finds nothing new never reads it, the clone does. | |
| 231 | const packs = Bun.spawnSync(["sh", "-c", "ls objects/pack/*.pack"], { cwd: mirror }).stdout.toString().trim().split("\n"); | |
| 232 | for (const p of packs) { | |
| 233 | const moved = join(root, "moved.pack"); | |
| 234 | renameSync(join(mirror, p), moved); | |
| 235 | rmSync(join(mirror, p.replace(/\.pack$/, ".idx"))); | |
| 236 | expect(Bun.spawnSync(["git", "unpack-objects", "-q"], { cwd: mirror, stdin: readFileSync(moved) }).exitCode).toBe(0); | |
| 237 | } | |
| 238 | const blob = Bun.spawnSync(["git", "rev-parse", "HEAD:a.txt"], { cwd: origin }).stdout.toString().trim(); | |
| 239 | const loose = join(mirror, "objects", blob.slice(0, 2), blob.slice(2)); | |
| 240 | chmodSync(loose, 0o644); | |
| 241 | writeFileSync(loose, "garbage"); | |
| 242 | const dir = join(root, "c-damaged"); | |
| 243 | const cloned = await cloneWithMirror({ dataDir: data(), repoId: "r7", url: `file://${origin}`, dir, env: {} }); | |
| 244 | expect(cloned.ok).toBe(true); | |
| 245 | expect(readFileSync(join(dir, "a.txt"), "utf8")).toBe("hello\n"); | |
| 246 | expect(existsSync(mirror)).toBe(false); | |
| 247 | // rebuilt on the next call | |
| 248 | expect((await mirrored(data(), "r7")).ok).toBe(true); | |
| 249 | expect(existsSync(join(mirror, "HEAD"))).toBe(true); | |
| 250 | }); | |
| 251 | ||
| 252 | test("a clone that times out keeps a good mirror and does not clone again", async () => { | |
| 253 | expect((await mirrored(data(), "r8")).ok).toBe(true); | |
| 254 | // Fast for the mirror's fetch (the first conversation), slow for the clone after it. | |
| 255 | const script = join(root, "late-ssh"); | |
| 256 | writeFileSync( | |
| 257 | script, | |
| 258 | `#!/bin/sh\necho start >> "$SLOW_LOG"\n[ "$(wc -l < "$SLOW_LOG")" -gt 1 ] && sleep 2\neval "git \${2#git-}"\n`, | |
| 259 | ); | |
| 260 | chmodSync(script, 0o755); | |
| 261 | const logFile = join(root, `late-${throwaway++}.log`); | |
| 262 | writeFileSync(logFile, ""); | |
| 263 | const dir = join(root, "c-timeout"); | |
| 264 | const cloned = await cloneWithMirror({ | |
| 265 | dataDir: data(), repoId: "r8", url: `ssh://forge.test${origin}`, dir, | |
| 266 | env: { GIT_SSH_COMMAND: script, GIT_SSH_VARIANT: "simple", SLOW_LOG: logFile }, | |
| 267 | timeoutMs: 500, | |
| 268 | }); | |
| 269 | expect(cloned.ok).toBe(false); | |
| 270 | expect(existsSync(join(mirrorDir(data(), "r8"), "HEAD"))).toBe(true); | |
| 271 | // one fetch and one borrowed clone; no plain retry | |
| 272 | expect(readFileSync(logFile, "utf8").split("\n").filter(Boolean)).toHaveLength(2); | |
| 273 | }); |
new above
offline
checkout failedandcorruptare broad. A full disk or a bad path in the working tree also matches, and then a good mirror is thrown away for nothing. Anchor these to object wording.