| … | … | @@ -86,6 +86,24 @@ export async function configureRepoActivityCache (storageDir) { |
| 86 | 86 | if (obj && obj.entries && typeof obj.entries === 'object') { |
| 87 | 87 | for (const [name, entry] of Object.entries(obj.entries)) { |
| 88 | 88 | if (entry && entry.data && typeof entry.mtime === 'number') { |
| 89 | // Self-heal: distrust a persisted zero. | |
| 90 | // | |
| 91 | // execGit used to turn ANY git failure into '', which parsed to | |
| 92 | // exactly { commits: 0, lastCommit: null } and got cached and | |
| 93 | // persisted as a real answer. The cache key is the pack-dir mtime, | |
| 94 | // so unless that repo's pack happens to change again the zero is | |
| 95 | // served for ever, across restarts. stortron carried one for | |
| 96 | // dc-terrain while `git log --all` returned 514 by hand. | |
| 97 | // | |
| 98 | // The write path is fixed above, but entries written by an older | |
| 99 | // build are still on disk, so drop them on load and recompute. | |
| 100 | // | |
| 101 | // A genuinely empty repo looks identical, and that is fine: the only | |
| 102 | // cost of dropping its entry is ONE cheap recompute that writes the | |
| 103 | // same zero straight back. Paying that on a handful of empty repos | |
| 104 | // per boot is worth not serving a stale zero for a real one. | |
| 105 | if (entry.data.commits === 0 && !entry.data.lastCommit) continue | |
| 106 | ||
| 89 | 107 | cache.set(name, { |
| 90 | 108 | cachedAt: entry.cachedAt || 0, |
| 91 | 109 | mtime: entry.mtime, |
| … | … | @@ -264,6 +282,22 @@ async function computeRepoDetails (name, repoPath, packMtime) { |
| 264 | 282 | commitStats(repoPath) |
| 265 | 283 | ]) |
| 266 | 284 | |
| 285 | // A FAILED walk is rendered as zeros for this pass but is NEVER cached. | |
| 286 | // | |
| 287 | // Caching it would make a transient git failure permanent: the cache key is | |
| 288 | // the pack-dir mtime, so if the repo's pack does not happen to change again | |
| 289 | // the zero is served for ever, across restarts, because it is persisted too. | |
| 290 | // dc-terrain sat at commits:0 on stortron for exactly this reason while the | |
| 291 | // identical `git log --all` returned 514 by hand; hyper-tps escaped only | |
| 292 | // because a later pull happened to move its pack mtime. | |
| 293 | // | |
| 294 | // Returning zeros keeps the page rendering (it degrades to a card without | |
| 295 | // counts, which is what enrichRepos already does on a throw) while the next | |
| 296 | // sweep retries. | |
| 297 | if (stats === null) { | |
| 298 | return { description, commits: 0, lastCommit: null, activity: new Array(BUCKETS).fill(0) } | |
| 299 | } | |
| 300 | ||
| 267 | 301 | const data = { description, ...stats } |
| 268 | 302 | cache.set(name, { cachedAt: Date.now(), mtime: packMtime, data }) |
| 269 | 303 | dirty = true |
| … | … | @@ -293,7 +327,11 @@ async function ensureCommitGraph (repoPath) { |
| 293 | 327 | // - %at within 365d = the activity buckets (== the old log --since=365d) |
| 294 | 328 | // %at is author time — matches each old per-metric call exactly. |
| 295 | 329 | async function commitStats (repoPath) { |
| 296 | const out = await execGit(repoPath, ['log', '--all', '--format=%at|%H']).catch(() => '') | |
| 330 | const out = await execGit(repoPath, ['log', '--all', '--format=%at|%H']) | |
| 331 | // null = the walk FAILED (non-zero exit / spawn error), which is NOT the | |
| 332 | // same as an empty repo returning ''. The caller must not cache a failure: | |
| 333 | // an empty-repo '' is a real answer, a failure is the absence of one. | |
| 334 | if (out === null) return null | |
| 297 | 335 | const buckets = new Array(BUCKETS).fill(0) |
| 298 | 336 | const now = Date.now() |
| 299 | 337 | const start = now - BUCKETS * DAY_MS |
| … | … | @@ -320,18 +358,57 @@ async function commitStats (repoPath) { |
| 320 | 358 | } |
| 321 | 359 | |
| 322 | 360 | /** |
| 323 | * Run git in a bare repo and capture stdout. Resolves with stdout text | |
| 324 | * (even on non-zero exit so callers can treat "no output" as "empty"). | |
| 361 | * Run git in a bare repo and capture stdout. | |
| 362 | * | |
| 363 | * Resolves with stdout text on success, or `null` on failure — a spawn error | |
| 364 | * or a non-zero exit. The distinction matters: this used to resolve with | |
| 365 | * whatever stdout it had captured REGARDLESS of exit code, with stderr routed | |
| 366 | * to 'ignore'. A failing `git log --all` therefore produced '', which parses | |
| 367 | * to { commits: 0, lastCommit: null, activity: all-zero } — byte-identical to | |
| 368 | * a legitimately empty repo — and got cached and persisted as a real answer. | |
| 369 | * | |
| 370 | * That is not hypothetical. stortron, 2026-09-02: the mirror was mid | |
| 371 | * `git fetch` into dc-terrain (a long-running pull; three other pulls started | |
| 372 | * AND finished inside its window) when the reconcile sweep walked the same | |
| 373 | * bare repo. git exited non-zero, the failure became commits:0, and it stuck — | |
| 374 | * the cache key is the pack-dir mtime, that fetch brought nothing new, so the | |
| 375 | * mtime never moved and the poison never expired. hyper-tps was hit by the | |
| 376 | * same race and only recovered by luck, when a later pull did change its pack. | |
| 377 | * | |
| 378 | * stderr is captured (bounded) and logged so the NEXT such failure names | |
| 379 | * itself instead of being unrecoverable after the fact. | |
| 325 | 380 | */ |
| 326 | 381 | function execGit (repoPath, args) { |
| 327 | 382 | return new Promise((resolve) => { |
| 328 | const proc = spawn('git', ['-C', repoPath, ...args], { | |
| 329 | stdio: ['ignore', 'pipe', 'ignore'] | |
| 330 | }) | |
| 383 | let proc | |
| 384 | try { | |
| 385 | proc = spawn('git', ['-C', repoPath, ...args], { | |
| 386 | stdio: ['ignore', 'pipe', 'pipe'] | |
| 387 | }) | |
| 388 | } catch { | |
| 389 | return resolve(null) | |
| 390 | } | |
| 331 | 391 | const chunks = [] |
| 392 | const errChunks = [] | |
| 393 | let errBytes = 0 | |
| 332 | 394 | proc.stdout.on('data', (c) => chunks.push(c)) |
| 333 | proc.on('error', () => resolve('')) | |
| 334 | proc.on('close', () => resolve(Buffer.concat(chunks).toString('utf8'))) | |
| 395 | if (proc.stderr) { | |
| 396 | proc.stderr.on('data', (c) => { | |
| 397 | // Bounded: a pathological repo must not buffer megabytes of stderr. | |
| 398 | if (errBytes < 8192) { errChunks.push(c); errBytes += c.length } | |
| 399 | }) | |
| 400 | } | |
| 401 | proc.on('error', () => resolve(null)) | |
| 402 | proc.on('close', (code) => { | |
| 403 | if (code !== 0) { | |
| 404 | const msg = Buffer.concat(errChunks).toString('utf8').trim().split('\n')[0] | |
| 405 | // eslint-disable-next-line no-console | |
| 406 | console.error('[repo-activity] git ' + args[0] + ' exited ' + code + | |
| 407 | ' in ' + repoPath + (msg ? ': ' + msg : ' (no stderr)')) | |
| 408 | return resolve(null) | |
| 409 | } | |
| 410 | resolve(Buffer.concat(chunks).toString('utf8')) | |
| 411 | }) | |
| 335 | 412 | }) |
| 336 | 413 | } |
| 337 | 414 |
| … | … | @@ -1,6 +1,6 @@ |
| 1 | 1 | { |
| 2 | 2 | "name": "pear-git-v3", |
| 3 | "version": "0.1.248", | |
| 3 | "version": "0.1.249", | |
| 4 | 4 | "description": "P2P git forge — multi-writer issue/org/release/peer-mirror daemon. Built bottom-up after the failed v2.", |
| 5 | 5 | "type": "module", |
| 6 | 6 | "main": "index.js", |
| … | … | @@ -0,0 +1,142 @@ |
| 1 | /** | |
| 2 | * A FAILED git walk must not be cached as "this repo has 0 commits". | |
| 3 | * | |
| 4 | * commitStats runs ONE `git log --all --format=%at|%H` through execGit, which | |
| 5 | * resolves on 'close' REGARDLESS of exit code, with stdio[2] set to 'ignore' | |
| 6 | * so stderr is discarded. A git failure therefore produces '' -- which parses | |
| 7 | * to { commits: 0, lastCommit: null, activity: all-zero }, byte-identical to a | |
| 8 | * legitimately empty repo. computeRepoDetails then caches that keyed on the | |
| 9 | * pack-dir mtime, and flushRepoActivityCache persists it, so a single | |
| 10 | * transient failure is served for the life of the pack. | |
| 11 | * | |
| 12 | * Production, stortron 2026-09-02: repo-activity-cache.json held | |
| 13 | * "dc-terrain": { cachedAt 11:07:39, mtime 10:59:12, | |
| 14 | * data: { commits: 0, lastCommit: null, activity: [0,...] } } | |
| 15 | * while `git -C <bare> log --all` -- the daemon's exact invocation -- returned | |
| 16 | * 514 commits, exit 0, empty stderr, and tabertron reported 514 for the same | |
| 17 | * repo at the same HEAD. The zero survived two daemon restarts. | |
| 18 | * | |
| 19 | * The trigger was unrecoverable precisely BECAUSE the exit code and stderr | |
| 20 | * were thrown away. That is the bug: a failure has to be distinguishable from | |
| 21 | * an empty repo, and must not be cached. | |
| 22 | */ | |
| 23 | ||
| 24 | import test from 'brittle' | |
| 25 | import { mkdtempSync, mkdirSync, writeFileSync, statSync } from '#node:fs' | |
| 26 | import { join } from '#node:path' | |
| 27 | import { tmpdir } from '#node:os' | |
| 28 | import { execFile } from '../lib/_compat-exec.js' | |
| 29 | ||
| 30 | import { | |
| 31 | configureRepoActivityCache, | |
| 32 | enrichRepos, | |
| 33 | getRepoActivityStats, | |
| 34 | resetRepoActivityCacheForTests | |
| 35 | } from '../lib/services/repo-activity.js' | |
| 36 | ||
| 37 | let n = 0 | |
| 38 | const tmpDir = () => mkdtempSync(join(tmpdir(), 'pgv3-activity-fail-' + (n++) + '-')) | |
| 39 | ||
| 40 | const statusFor = (name) => ({ repos: [{ name, branches: 0, packSize: 0, head: null, serving: true, lastUpdated: 0 }] }) | |
| 41 | ||
| 42 | test('a failed git walk is not cached as commits:0', async (t) => { | |
| 43 | t.timeout(60000) | |
| 44 | ||
| 45 | // A directory that is NOT a git repo — `git log --all` exits non-zero. | |
| 46 | const notARepo = join(tmpDir(), 'not-a-repo') | |
| 47 | mkdirSync(notARepo, { recursive: true }) | |
| 48 | writeFileSync(join(notARepo, 'placeholder'), 'x') | |
| 49 | ||
| 50 | resetRepoActivityCacheForTests() | |
| 51 | const reposMap = new Map([['broken', { path: notARepo }]]) | |
| 52 | ||
| 53 | const first = await enrichRepos(statusFor('broken'), reposMap) | |
| 54 | t.is(first[0].commits, 0, 'a failed walk still renders as 0 for display') | |
| 55 | const afterFirst = getRepoActivityStats().computes | |
| 56 | ||
| 57 | const second = await enrichRepos(statusFor('broken'), reposMap) | |
| 58 | t.is(second[0].commits, 0, 'still 0 on the second pass') | |
| 59 | ||
| 60 | t.is(getRepoActivityStats().computes, afterFirst + 1, | |
| 61 | 'the failure was RECOMPUTED, not served from cache — a transient git failure must not stick') | |
| 62 | t.is(getRepoActivityStats().cached, 0, | |
| 63 | 'nothing failed was written to the cache, so nothing can be persisted') | |
| 64 | }) | |
| 65 | ||
| 66 | test('a successful walk IS still cached (no over-correction)', async (t) => { | |
| 67 | t.timeout(60000) | |
| 68 | ||
| 69 | const repo = join(tmpDir(), 'real.git') | |
| 70 | await execFile('git', ['init', '--bare', '-q', repo]) | |
| 71 | ||
| 72 | // Give it a real commit so the walk returns something. | |
| 73 | const work = join(tmpDir(), 'work') | |
| 74 | mkdirSync(work, { recursive: true }) | |
| 75 | await execFile('git', ['init', '-q', work]) | |
| 76 | await execFile('git', ['-C', work, 'config', 'user.email', 't@t']) | |
| 77 | await execFile('git', ['-C', work, 'config', 'user.name', 't']) | |
| 78 | writeFileSync(join(work, 'f.txt'), 'hello') | |
| 79 | await execFile('git', ['-C', work, 'add', 'f.txt']) | |
| 80 | await execFile('git', ['-C', work, 'commit', '-qm', 'one']) | |
| 81 | await execFile('git', ['-C', work, 'push', '-q', repo, 'HEAD:refs/heads/master']) | |
| 82 | ||
| 83 | resetRepoActivityCacheForTests() | |
| 84 | const reposMap = new Map([['real', { path: repo }]]) | |
| 85 | ||
| 86 | const first = await enrichRepos(statusFor('real'), reposMap) | |
| 87 | t.is(first[0].commits, 1, 'counts the real commit') | |
| 88 | const afterFirst = getRepoActivityStats().computes | |
| 89 | ||
| 90 | const second = await enrichRepos(statusFor('real'), reposMap) | |
| 91 | t.is(second[0].commits, 1, 'same answer') | |
| 92 | t.is(getRepoActivityStats().computes, afterFirst, | |
| 93 | 'a SUCCESSFUL result is served from cache — caching still works') | |
| 94 | }) | |
| 95 | ||
| 96 | test('a zero-commit entry already on disk is distrusted at boot and recomputed', async (t) => { | |
| 97 | // The fix above stops NEW poison. An entry written by an older build is | |
| 98 | // still on disk and, because the cache key is the pack-dir mtime, would be | |
| 99 | // served for the life of that pack — across restarts, since it is persisted. | |
| 100 | // | |
| 101 | // So a persisted { commits: 0, lastCommit: null } is distrusted on load. It | |
| 102 | // is indistinguishable from a genuinely empty repo, and that is fine: the | |
| 103 | // only cost of dropping a real empty repo's entry is ONE cheap recompute | |
| 104 | // that writes the same zero straight back. | |
| 105 | t.timeout(60000) | |
| 106 | ||
| 107 | const storage = tmpDir() | |
| 108 | const repo = join(tmpDir(), 'poisoned.git') | |
| 109 | await execFile('git', ['init', '--bare', '-q', repo]) | |
| 110 | ||
| 111 | const work = join(tmpDir(), 'w2') | |
| 112 | mkdirSync(work, { recursive: true }) | |
| 113 | await execFile('git', ['init', '-q', work]) | |
| 114 | await execFile('git', ['-C', work, 'config', 'user.email', 't@t']) | |
| 115 | await execFile('git', ['-C', work, 'config', 'user.name', 't']) | |
| 116 | writeFileSync(join(work, 'a.txt'), 'x') | |
| 117 | await execFile('git', ['-C', work, 'add', 'a.txt']) | |
| 118 | await execFile('git', ['-C', work, 'commit', '-qm', 'one']) | |
| 119 | await execFile('git', ['-C', work, 'push', '-q', repo, 'HEAD:refs/heads/master']) | |
| 120 | ||
| 121 | // Hand-write the exact shape stortron had for dc-terrain: a real repo, a | |
| 122 | // zeroed entry, keyed on the CURRENT pack mtime so the key still matches. | |
| 123 | let packMtime = 0 | |
| 124 | try { packMtime = statSync(join(repo, 'objects', 'pack')).mtimeMs } catch {} | |
| 125 | writeFileSync(join(storage, 'repo-activity-cache.json'), JSON.stringify({ | |
| 126 | version: 1, | |
| 127 | entries: { | |
| 128 | poisoned: { | |
| 129 | cachedAt: Date.now(), | |
| 130 | mtime: packMtime, | |
| 131 | data: { description: '', commits: 0, lastCommit: null, activity: new Array(365).fill(0) } | |
| 132 | } | |
| 133 | } | |
| 134 | })) | |
| 135 | ||
| 136 | resetRepoActivityCacheForTests() | |
| 137 | await configureRepoActivityCache(storage) | |
| 138 | ||
| 139 | const out = await enrichRepos(statusFor('poisoned'), new Map([['poisoned', { path: repo }]])) | |
| 140 | t.is(out[0].commits, 1, | |
| 141 | 'the persisted zero was distrusted and recomputed from the real repo') | |
| 142 | }) |
| … | … | @@ -18,12 +18,13 @@ |
| 18 | 18 | |
| 19 | 19 | import test from 'brittle' |
| 20 | 20 | import process from '#node:process' |
| 21 | import { mkdtempSync, existsSync } from '#node:fs' | |
| 21 | import { mkdtempSync, existsSync, writeFileSync, mkdirSync } from '#node:fs' | |
| 22 | 22 | import { readFile } from '#node:fs/promises' |
| 23 | 23 | import { join } from '#node:path' |
| 24 | 24 | import { tmpdir } from '#node:os' |
| 25 | 25 | |
| 26 | 26 | import { startPearGit } from '../index.js' |
| 27 | import { execFile } from '../lib/_compat-exec.js' | |
| 27 | 28 | import { enrichRepos, resetRepoActivityCacheForTests } from '../lib/services/repo-activity.js' |
| 28 | 29 | |
| 29 | 30 | process.env.PGV3_USE_RAW_IDENTITY = '1' |
| … | … | @@ -48,11 +49,30 @@ test('close() flushes the pending repo-activity cache write', async (t) => { |
| 48 | 49 | const cacheFile = join(storage, 'repo-activity-cache.json') |
| 49 | 50 | t.absent(existsSync(cacheFile), 'no cache file before anything is enriched') |
| 50 | 51 | |
| 51 | // A plain directory is enough: getRepoDetails stats objects/pack (absent → | |
| 52 | // mtime 0), the git calls fail and resolve empty, and the zero-commit result | |
| 53 | // is still cached + marks the module dirty. That is the state we need — a | |
| 54 | // pending write — without paying for a real repo. | |
| 55 | const repoDir = tmp('pgv3-flush-repo-') | |
| 52 | // A REAL bare repo with a commit, because we need a cached entry and only a | |
| 53 | // SUCCESSFUL walk is cached now. | |
| 54 | // | |
| 55 | // This used to pass a plain directory and rely on the git calls failing: | |
| 56 | // execGit resolved '' on any non-zero exit, that parsed to commits:0, and | |
| 57 | // the zero was cached — which marked the module dirty for free. That was a | |
| 58 | // bug (a transient git failure became a permanent "0 commits" keyed on the | |
| 59 | // pack mtime, as seen on stortron's dc-terrain), so a failed walk is no | |
| 60 | // longer cached and this fixture no longer produces a pending write. | |
| 61 | // | |
| 62 | // The assertions below are unchanged — the point of this test is that | |
| 63 | // close() flushes without waiting for the debounce, not how the entry got | |
| 64 | // there. | |
| 65 | const repoDir = join(tmp('pgv3-flush-repo-'), 'r1.git') | |
| 66 | await execFile('git', ['init', '--bare', '-q', repoDir]) | |
| 67 | const workDir = tmp('pgv3-flush-work-') | |
| 68 | await execFile('git', ['init', '-q', workDir]) | |
| 69 | await execFile('git', ['-C', workDir, 'config', 'user.email', 't@t']) | |
| 70 | await execFile('git', ['-C', workDir, 'config', 'user.name', 't']) | |
| 71 | writeFileSync(join(workDir, 'f.txt'), 'hello') | |
| 72 | await execFile('git', ['-C', workDir, 'add', 'f.txt']) | |
| 73 | await execFile('git', ['-C', workDir, 'commit', '-qm', 'one']) | |
| 74 | await execFile('git', ['-C', workDir, 'push', '-q', repoDir, 'HEAD:refs/heads/master']) | |
| 75 | ||
| 56 | 76 | await enrichRepos({ repos: [{ name: 'r1' }] }, new Map([['r1', { path: repoDir }]])) |
| 57 | 77 | |
| 58 | 78 | // No sleep. Pre-fix the debounced write is still 1s out and unref'd, so the |