From 3ab14948400bb2396896dd4e5f02cc2b250b0097 Mon Sep 17 00:00:00 2001 From: castrojo Date: Sat, 24 Oct 2026 20:18:54 -0400 Subject: [PATCH] fix(analytics): report failed refresh jobs and isolate write-error tests Keep the fetcher build-safe while requiring scheduled snapshots to be complete and generated during the current run. Publish unavailable state before failing the scheduled job. Run the write-failure CLI regression in a disposable workspace rather than against tracked data. Assisted-by: GPT-6.1 Sol via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../workflows/update-contributor-metrics.yml | 21 ++++- docs/skills/github-api-client.md | 7 ++ scripts/fetch-contributor-metrics.js | 20 +++++ scripts/fetch-contributor-metrics.test.js | 77 +++++++++++++++---- 4 files changed, 108 insertions(+), 17 deletions(-) diff --git a/.github/workflows/update-contributor-metrics.yml b/.github/workflows/update-contributor-metrics.yml index 2b8c4a09..acd83c25 100644 --- a/.github/workflows/update-contributor-metrics.yml +++ b/.github/workflows/update-contributor-metrics.yml @@ -59,9 +59,12 @@ jobs: NODE - name: Compute contributor metrics + id: collect env: GITHUB_TOKEN: ${{ github.token }} - run: node scripts/fetch-contributor-metrics.js --force + run: | + echo "startedAt=$(node -e 'console.log(Date.now())')" >> "$GITHUB_OUTPUT" + node scripts/fetch-contributor-metrics.js --force - name: Check for contributor metrics dataset changes id: check-changes @@ -99,3 +102,19 @@ jobs: --title "chore(analytics): refresh public contributor participation" \ --body "Refreshes the complete public GitHub participation snapshot. Known bots and imported fork commit history are excluded; failed collection preserves last-good history and marks unavailability. Requires maintainer approval before merge." fi + + # Publish the unavailable state first, then report a failed refresh honestly. + # The build-facing fetcher still exits zero and preserves the last-good data. + - name: Require a verified current refresh + env: + SCAN_STARTED_AT: ${{ steps.collect.outputs.startedAt }} + run: | + node <<'NODE' + const fs = require('node:fs'); + const { isVerifiedRefresh } = require('./scripts/fetch-contributor-metrics'); + const snapshot = JSON.parse(fs.readFileSync('static/data/contributor-metrics.json', 'utf8')); + if (!isVerifiedRefresh(snapshot, Number(process.env.SCAN_STARTED_AT), Date.now())) { + console.error('Scheduled contributor refresh did not produce a complete current snapshot. Last-good data is retained.'); + process.exitCode = 1; + } + NODE diff --git a/docs/skills/github-api-client.md b/docs/skills/github-api-client.md index 9cef2222..a2094276 100644 --- a/docs/skills/github-api-client.md +++ b/docs/skills/github-api-client.md @@ -157,6 +157,13 @@ instead of restating a token read or a header object. schema1 counts are never a last-good fallback. The offline fixture in `fetch-contributor-metrics.test.js` drives the actual injectable transport through two-page nested connections and dropped-page failures. + The fetcher remains build-safe (exit 0), but the scheduled refresh job must + finish red when it retained unavailable or stale data. Its final workflow + gate uses `isVerifiedRefresh` to require a complete snapshot timestamped inside + the current collection interval, after publishing any unavailable state for + review. A genuine measured zero passes. CLI write-failure tests run copied + modules and fixture seeds inside a disposable workspace; never point a + fault-injected child process at the tracked production dataset. ## Common Rationalizations diff --git a/scripts/fetch-contributor-metrics.js b/scripts/fetch-contributor-metrics.js index ac2514ec..7ebe1c1d 100755 --- a/scripts/fetch-contributor-metrics.js +++ b/scripts/fetch-contributor-metrics.js @@ -598,6 +598,25 @@ async function collectContributorMetrics({ } } +/** The scheduled job must distinguish a fresh scan from retained build-safe data. */ +function isVerifiedRefresh(snapshot, startedAt, finishedAt) { + const generatedAt = + typeof snapshot?.generatedAt === "string" + ? Date.parse(snapshot.generatedAt) + : NaN; + return ( + snapshot?.schemaVersion === 2 && + snapshot.unavailable === false && + snapshot.coverage?.complete === true && + Number.isFinite(startedAt) && + startedAt > 0 && + Number.isFinite(finishedAt) && + Number.isFinite(generatedAt) && + startedAt <= generatedAt && + generatedAt <= finishedAt + ); +} + async function main({ args = process.argv.slice(2), outputFile = OUTPUT_FILE, @@ -681,6 +700,7 @@ module.exports = { aggregateContributions, unavailableData, collectContributorMetrics, + isVerifiedRefresh, main, }; diff --git a/scripts/fetch-contributor-metrics.test.js b/scripts/fetch-contributor-metrics.test.js index d1a3812c..80baef75 100644 --- a/scripts/fetch-contributor-metrics.test.js +++ b/scripts/fetch-contributor-metrics.test.js @@ -11,6 +11,7 @@ const { FAILURE_REASON, aggregateContributions, collectContributorMetrics, + isVerifiedRefresh, main, } = require("./fetch-contributor-metrics.js"); @@ -822,47 +823,56 @@ test("TTL uses the seed timestamp rather than checkout mtime and rejects future } }); -test("the actual CLI exits zero without replacing history when the output cannot be written", (t) => { +test("the actual CLI exits zero without replacing isolated history when writes are denied", async (t) => { const dir = fs.mkdtempSync( path.join(os.tmpdir(), "contributor-write-failure-"), ); t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + const scripts = path.join(dir, "scripts"); + const lib = path.join(scripts, "lib"); + fs.mkdirSync(lib, { recursive: true }); + // Run an unmodified copy of the actual CLI in a disposable workspace. A future + // writer implementation can fail this test, but cannot overwrite a tracked seed. + for (const file of [ + "fetch-contributor-metrics.js", + "lib/request-queue.js", + "lib/seed-cache.js", + "lib/contributor-tracker.mjs", + ]) { + fs.copyFileSync(path.join(__dirname, file), path.join(scripts, file)); + } + const seed = path.join(dir, "static", "data", "contributor-metrics.json"); + fs.mkdirSync(path.dirname(seed), { recursive: true }); + const previous = await collect(apiFixture()); + const before = JSON.stringify(previous, null, 2) + "\n"; + fs.writeFileSync(seed, before); const preload = path.join(dir, "fail-write.cjs"); fs.writeFileSync( preload, ` - const fs = require('node:fs'); - const write = fs.writeFileSync; - fs.writeFileSync = (file, ...args) => { - if (String(file).endsWith('contributor-metrics.json.tmp')) throw new Error('sensitive-write-error'); - return write(file, ...args); + require('node:fs').writeFileSync = () => { + process.stderr.write('injected-write-denied\\n'); + throw new Error('sensitive-write-error'); }; `, ); - const seed = path.join( - __dirname, - "..", - "static", - "data", - "contributor-metrics.json", - ); - const before = fs.readFileSync(seed, "utf8"); const result = spawnSync( process.execPath, [ "--require", preload, - path.join(__dirname, "fetch-contributor-metrics.js"), + path.join(scripts, "fetch-contributor-metrics.js"), "--force", ], { + cwd: dir, encoding: "utf8", env: { ...process.env, GH_TOKEN: "", GITHUB_TOKEN: "" }, }, ); assert.equal(result.status, 0, result.stderr); assert.equal(fs.readFileSync(seed, "utf8"), before); - assert.match(result.stderr, /could not be written/); + assert.match(result.stderr, /injected-write-denied/); assert.doesNotMatch(result.stderr, /sensitive-write-error/); }); @@ -919,3 +929,38 @@ test("persistent throttling is bounded and retains the last verified snapshot", assert.equal(data.generatedAt, previous.generatedAt); assert.deepEqual(data.contributors, previous.contributors); }); + +test("scheduled refresh status rejects retained, incomplete, stale or future snapshots", async () => { + const verified = await collect(apiFixture()); + const stamp = Date.parse(AS_OF); + assert.equal(isVerifiedRefresh(verified, stamp - 1000, stamp + 1000), true); + for (const snapshot of [ + null, + { ...verified, unavailable: true }, + { ...verified, coverage: { ...verified.coverage, complete: false } }, + { ...verified, generatedAt: null }, + { ...verified, generatedAt: "not-a-date" }, + { ...verified, schemaVersion: 1 }, + ]) { + assert.equal( + isVerifiedRefresh(snapshot, stamp - 1000, stamp + 1000), + false, + ); + } + assert.equal(isVerifiedRefresh(verified, stamp + 1, stamp + 1000), false); + assert.equal(isVerifiedRefresh(verified, stamp - 1000, stamp - 1), false); + assert.equal(isVerifiedRefresh(verified, 0, stamp + 1000), false); + assert.equal(isVerifiedRefresh(verified, NaN, stamp + 1000), false); +}); + +test("a verified zero-contributor refresh is still a successful scheduled collection", async () => { + const empty = await aggregateContributions({ + repositories: [], + events: [], + asOf: AS_OF, + complete: true, + }); + const stamp = Date.parse(AS_OF); + assert.equal(empty.summary.totalContributors, 0); + assert.equal(isVerifiedRefresh(empty, stamp - 1000, stamp + 1000), true); +});