OverviewFilesCommitsIssuesDocsReleasesPeers

pear-git-v3 commits / 5590c1d

Land a locally-held artifact at its own version path
favela-android master-5d6d7e4 sat incomplete on stortron — the ACTIVE public backend — for a day, missing worklet.bundle (37 MB) and tree.json while retrying every few minutes. The retry was working. The hand-off at the end was not: [release-pull] error: favela-android/versions/master-5d6d7e4/worklet.bundle: ENOENT: no such file or directory, rename ".../worklet.bundle.pull-tmp" -> ".../worklet.bundle" The rename's SOURCE is missing, which is the whole tell. hyper-overlay short-circuits a content hash it already holds on disk and returns WHERE THOSE BYTES ARE, ignoring the caller's destPath: const localDisk = this._contentHashPaths.get(contentHash) if (localDisk) { ... return { destPath: localDisk, local: true, size: v.size } } So the temp file was never written, the unconditional rename failed, and every retry took the same branch — permanently, not transiently. The puller already had the right answer in hand and discarded it; it even logs `got.local`. It bites exactly the artifacts UNCHANGED between builds, because those are the ones already present under an earlier version. Verified on the real data rather than inferred from equal file sizes: master-5d6d7e4/worklet.bundle and master-53b10d2/worklet.bundle are both sha256 ca096cb2f6e7a907dbe2ada8ad27502156e252964b7295a0bf5eaa691f7aa43a. The fix honours `got.destPath`. The bytes DO have to appear at this version's path — downloads are served straight off disk by path (GET /releases/:repo/:version/:artifact -> createReadStream), so an artifact held only under another version is unreachable here. They do NOT have to be a second copy: a hardlink satisfies the path for free, release versions are immutable by design, and pgh's tree store already hardlinks for this dedup, so it is the established pattern rather than a new trick. Cross-device falls back to copyFileSync. It links to its own temp and renames over, rather than unlinking the destination first. That keeps the atomic replace the original rename provided and never removes a real artifact to make room for one that might still fail. WHY THE SUITE MISSED IT: release-puller.test.js's `fakeOverlay` always writes destPath and always returns `local: false`, and its comment states the contract as "give me this hash, write it to destPath" — which is precisely the belief under test. A fixture that shares an assumption with the code cannot falsify it. The new fixture implements the short-circuit instead, and reproduces the production error verbatim, arrow and all, before the fix. Four tests: the artifact lands at the new path; it is the SAME INODE as the copy it was linked from (the 37 MB is stored once); a genuinely remote artifact is still fetched over the wire (the control — "lands at the path" must not be satisfied by breaking the path that moves most bytes); and a local hit whose file vanished between the overlay's verify and the link is recorded as an error and reported partial, so it is retried rather than silently marked complete. local-hit 4/4, release-puller 20/20, release-pull-retry 9/9, release-download, release-index-serve-order 3/3, release-event-log 13/13. NOT DEPLOYED. Publishing is fleet-wide and the fix has to reach the active public backend, which needs an explicit restart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5590c1d5402bf18ff03ae115ac107eb28de7ee95
by plankatron <fleeky@gmail.com> · 2026-08-18T23:54:53.000Z

Changed files (3) +221 -2

lib/services/release-puller.js +38 -1
……@@ -255,7 +255,44 @@ export class ReleasePuller {
255255 }
256256 // fetchFile verifies incrementally against contentHash and rejects on
257257 // mismatch, so arriving here means the bytes are already verified.
258 fs.renameSync(tmp, abs)
258 //
259 // But NOT necessarily at `tmp`. hyper-overlay short-circuits a content
260 // hash it already holds on disk and returns WHERE THOSE BYTES ARE,
261 // ignoring destPath entirely:
262 //
263 // const localDisk = this._contentHashPaths.get(contentHash)
264 // if (localDisk) { ... return { destPath: localDisk, local: true, ... } }
265 //
266 // Renaming `tmp` unconditionally therefore failed with ENOENT on the
267 // SOURCE — a temp file that was never written — and since every retry
268 // takes the same branch, the version stayed incomplete forever. It bit
269 // favela-android master-5d6d7e4 on the ACTIVE public backend for a day.
270 //
271 // The bytes still have to APPEAR at this version's path: downloads are
272 // served straight off disk by path (GET /releases/:repo/:version/:path),
273 // so an artifact held only under another version is unreachable here.
274 // They do not have to be a second copy — a hardlink satisfies the path
275 // and costs nothing, which matters because the artifacts that take this
276 // branch are exactly the ones UNCHANGED between builds.
277 const landed = got.destPath || tmp
278 if (landed === tmp) {
279 fs.renameSync(tmp, abs)
280 } else if (landed !== abs) {
281 // Link to our own temp and rename over, rather than unlinking `abs`
282 // first: that keeps the atomic-replace the rename above provides, and
283 // never removes a real artifact to make room for one that might fail.
284 const linkTmp = abs + '.pull-link'
285 try { fs.unlinkSync(linkTmp) } catch {}
286 try {
287 fs.linkSync(landed, linkTmp)
288 } catch (err) {
289 // Cross-device (or a filesystem without links): fall back to bytes.
290 if (err.code !== 'EXDEV' && err.code !== 'EPERM') throw err
291 fs.copyFileSync(landed, linkTmp)
292 }
293 fs.renameSync(linkTmp, abs)
294 try { fs.unlinkSync(tmp) } catch {}
295 }
259296
260297 if (replaceSet.has(f.path)) this.stats.replaced++
261298 this.stats.fetched++
package.json +1 -1
……@@ -1,6 +1,6 @@
11{
22 "name": "pear-git-v3",
3 "version": "0.1.224",
3 "version": "0.1.225",
44 "description": "P2P git forge — multi-writer issue/org/release/peer-mirror daemon. Built bottom-up after the failed v2.",
55 "type": "module",
66 "main": "index.js",
test/release-puller-local-hit.test.js +182 -0
……@@ -0,0 +1,182 @@
1// ReleasePuller — when the overlay ALREADY holds the bytes, under another path.
2//
3// This is the case that stranded favela-android master-5d6d7e4 on stortron, the
4// ACTIVE public backend, for a whole day. Measured there, not imagined:
5//
6// [release-pull] error: favela-android/versions/master-5d6d7e4/worklet.bundle:
7// ENOENT: no such file or directory,
8// rename ".../master-5d6d7e4/worklet.bundle.pull-tmp"
9// -> ".../master-5d6d7e4/worklet.bundle"
10//
11// The rename's SOURCE is missing, which is the tell. `fetchFile` never wrote it,
12// because hyper-overlay short-circuits a content hash it already has on disk:
13//
14// const localDisk = this._contentHashPaths.get(contentHash)
15// if (localDisk) {
16// const v = await verifyOnDisk(localDisk, contentHash)
17// if (v.ok) return { destPath: localDisk, local: true, size: v.size }
18// }
19//
20// It returns WHERE THE BYTES ALREADY ARE and ignores the caller's destPath. The
21// puller assumed destPath was always written, renamed a temp that never existed,
22// and failed — forever, since every retry takes the same branch.
23//
24// It bites exactly the artifacts that are UNCHANGED between builds, because those
25// are the ones already present under an earlier version. Confirmed on the real
26// data: master-5d6d7e4/worklet.bundle and master-53b10d2/worklet.bundle are both
27// sha256 ca096cb2f6e7a907dbe2ada8ad27502156e252964b7295a0bf5eaa691f7aa43a.
28//
29// WHY THE EXISTING SUITE MISSED IT: `fakeOverlay` in release-puller.test.js
30// always writes destPath and always returns `local: false`. Its comment states
31// the contract as "give me this hash, write it to destPath" — which is the
32// assumption under test here, and it is not what hyper-overlay promises. A
33// fixture that encodes the same belief as the code cannot falsify it.
34//
35// The file DOES have to appear at the new path: downloads are served straight off
36// disk by path (`GET /releases/:repo/:version/:artifact` -> createReadStream), so
37// bytes sitting only under another version are unreachable at this one's URL.
38// It does NOT have to be a second copy of those bytes — see the hardlink test.
39
40import test from 'brittle'
41import { mkdtempSync, readFileSync, existsSync, statSync } from '#node:fs'
42import { join } from '#node:path'
43import { tmpdir } from '#node:os'
44import b4a from 'b4a'
45
46import { ReleaseManager } from '../lib/release-manager.js'
47import { ReleasePuller } from '../lib/services/release-puller.js'
48
49function tmpDir (p) { return mkdtempSync(join(tmpdir(), p)) }
50
51// The overlay's REAL behaviour for content it already holds: hand back the
52// existing path, set `local`, and do NOT touch destPath.
53function localHitOverlay (byHashLocal, publisherDir, index) {
54 const remote = new Map()
55 for (const f of index.files) remote.set(f.hash, join(publisherDir, f.path))
56 return {
57 localHits: [],
58 async fetchFile (hash, opts = {}) {
59 const already = byHashLocal.get(hash)
60 if (already) {
61 this.localHits.push(hash)
62 const size = statSync(already).size
63 // Deliberately does not write opts.destPath. This is the contract.
64 return { destPath: already, local: true, size }
65 }
66 const src = remote.get(hash)
67 if (!src) return null
68 const buf = readFileSync(src)
69 const { writeFileSync } = await import('#node:fs')
70 writeFileSync(opts.destPath, buf)
71 return { destPath: opts.destPath, local: false, size: buf.length }
72 }
73 }
74}
75
76async function scenario () {
77 // A publisher with two versions whose artifact is byte-identical, which is the
78 // normal case for a build input that did not change between commits.
79 const pubDir = tmpDir('rplh-pub-')
80 const pub = new ReleaseManager({ releaseDir: pubDir })
81 const payload = b4a.from('WORKLET-BUNDLE-BYTES')
82 await pub.publish({
83 repo: 'demo', version: 'v1',
84 artifacts: [{ path: 'worklet.bundle', data: payload }]
85 })
86 await pub.publish({
87 repo: 'demo', version: 'v2',
88 artifacts: [{ path: 'worklet.bundle', data: payload }]
89 })
90 const pubIndex = JSON.parse(readFileSync(join(pubDir, '_index.json'), 'utf8'))
91
92 // The peer already replicated v1, so it holds those exact bytes on disk.
93 const peerDir = tmpDir('rplh-peer-')
94 const peerRm = new ReleaseManager({ releaseDir: peerDir })
95 await peerRm.publish({
96 repo: 'demo', version: 'v1',
97 artifacts: [{ path: 'worklet.bundle', data: payload }]
98 })
99 const existing = join(peerDir, 'demo/versions/v1/worklet.bundle')
100
101 const v2 = pubIndex.files.find(f => f.path === 'demo/versions/v2/worklet.bundle')
102 const byHashLocal = new Map([[v2.hash, existing]])
103
104 const overlay = localHitOverlay(byHashLocal, pubDir, pubIndex)
105 const logs = []
106 const puller = new ReleasePuller({
107 releaseDir: peerDir, overlay, releases: peerRm, log: (l) => logs.push(l)
108 })
109 const client = { call: async () => pubIndex }
110 return { puller, client, peerDir, existing, payload, logs, overlay }
111}
112
113test('a locally-held artifact still lands at the new version path', async (t) => {
114 // THE test. Before the fix this failed with
115 // ENOENT ... rename "worklet.bundle.pull-tmp" -> "worklet.bundle"
116 // and the version stayed permanently incomplete on the mirror.
117 const { puller, client, peerDir, payload, logs } = await scenario()
118
119 const res = await puller.pullFrom(client)
120 const target = join(peerDir, 'demo/versions/v2/worklet.bundle')
121
122 t.ok(existsSync(target),
123 'the artifact must exist AT THIS VERSION\'S PATH — downloads are served by ' +
124 'path, so bytes under another version are unreachable here:\n ' +
125 logs.filter(l => l.includes('error')).join('\n '))
126 if (!existsSync(target)) return // the read below would abort the whole suite
127 t.alike(readFileSync(target), payload, 'and it is the right bytes')
128 t.is(puller.stats.errors.length, 0,
129 'no errors: ' + JSON.stringify(puller.stats.errors))
130 t.ok(res.ok, 'the pull reports success')
131 t.not(res.partial, true, 'and is not partial, so nothing is re-queued forever')
132})
133
134test('it is a hardlink, not a second copy of the bytes', async (t) => {
135 // The artifacts this fails on are precisely the ones that are UNCHANGED between
136 // builds, so they are exactly the ones a link makes free. Release versions are
137 // immutable by design and both paths are on one filesystem, and pgh's own tree
138 // store already hardlinks for this dedup — so this is the established pattern
139 // here, not a new trick.
140 const { puller, client, peerDir, existing } = await scenario()
141 await puller.pullFrom(client)
142
143 const target = join(peerDir, 'demo/versions/v2/worklet.bundle')
144 t.is(statSync(target).ino, statSync(existing).ino,
145 'same inode — the 37 MB is stored once, not twice')
146})
147
148test('a genuinely remote artifact is still fetched normally', async (t) => {
149 // The control. Without it, "lands at the path" could be satisfied by a fix that
150 // broke the ordinary remote path, which is the one that moves most bytes.
151 const { puller, client, peerDir, overlay } = await scenario()
152 await puller.pullFrom(client)
153
154 t.ok(existsSync(join(peerDir, 'demo/versions/v2/manifest.json')),
155 'the manifest, which the peer did NOT already hold, came over the wire')
156 t.is(overlay.localHits.length, 1,
157 'exactly one artifact took the local-hit branch; the rest were real fetches')
158})
159
160test('a local hit whose file vanished mid-pull is an error, not a crash', async (t) => {
161 // The overlay verifies on disk before reporting a local hit, but the file can
162 // still go between that check and the link. Fail closed and RECORD it, so the
163 // retry path sees a partial rather than a version silently marked complete.
164 const { peerDir } = await scenario()
165 const pubDir = tmpDir('rplh-pub2-')
166 const pub = new ReleaseManager({ releaseDir: pubDir })
167 await pub.publish({
168 repo: 'demo', version: 'v3',
169 artifacts: [{ path: 'gone.bin', data: b4a.from('X') }]
170 })
171 const idx = JSON.parse(readFileSync(join(pubDir, '_index.json'), 'utf8'))
172 const overlay = {
173 async fetchFile (hash, opts = {}) {
174 return { destPath: join(peerDir, 'does/not/exist.bin'), local: true, size: 1 }
175 }
176 }
177 const puller = new ReleasePuller({ releaseDir: peerDir, overlay })
178 const res = await puller.pullFrom({ call: async () => idx })
179
180 t.ok(puller.stats.errors.length > 0, 'the failure is recorded')
181 t.ok(res.partial || !res.ok, 'and reported as partial/failed so it is retried')
182})