From d3b51d45546c0e418ddcdaf19fba91ac284f7075 Mon Sep 17 00:00:00 2001 From: Kanav Jain Date: Mon, 12 Oct 2026 20:19:21 -0500 Subject: [PATCH] fix(cli): keep cron's state inside --cache-dir and quiet its first run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cron ran analysis without its cache dir, so the inner --since path read and overwrote ~/.wikihistory/observations/.json and printed a "Re-observation delta" against it — outside --cache-dir, and counting prior events outside the lookback window as resolved, which cron's own count does not. Cron now passes its cache dir through and asks analysis not to record an observation (runAnalyze's new { recordObservation: false }); cron keeps the only one. A page's first run logged the not-yet-written report with its ENOENT and full stack; analyze --since did the same for its first observation. readPriorJson is silent on a missing file and reports any other read or parse failure in one line, path and reason. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 + .../cli/src/__tests__/analyze-helpers.test.ts | 35 ++++++++++ .../cli/src/__tests__/cron-cache-dir.test.ts | 68 +++++++++++++++++++ packages/cli/src/__tests__/cron.test.ts | 62 ++++++++++++++--- packages/cli/src/commands/analyze-helpers.ts | 33 ++++++--- packages/cli/src/commands/analyze.ts | 3 +- packages/cli/src/commands/cron.ts | 38 ++++------- 7 files changed, 197 insertions(+), 44 deletions(-) create mode 100644 packages/cli/src/__tests__/analyze-helpers.test.ts create mode 100644 packages/cli/src/__tests__/cron-cache-dir.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 029b2e1..f0995ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed - **`refract verify --html` marked every event's proof "Merkle Root Match"**, including in a bundle that had failed verification. Each row now shows whether that proof hashes to the root it records, the page lists the checks that failed, and it states what `verify` checks and what it does not. The receipt and the command's output no longer say "cryptographically verified" or "integrity verified": the events are not rehashed and nothing in a bundle is signed. Revision IDs read from the bundle are HTML-escaped like the other fields. - **`--bundle` was described as a signed evidence bundle** in `refract export --help` and the CLI README. It carries a SHA-256 `bundleHash` of its own contents, which anyone who edits the bundle can recompute; nothing in Refract signs anything. +- **`refract cron --cache-dir` read and wrote outside the cache directory.** The analysis cron runs for each page recorded its own observation in `~/.wikihistory/observations/.json` whatever `--cache-dir` said, and printed a "Re-observation delta" against that file: on a first run into an empty cache directory it could report dozens of new and resolved events from an unrelated earlier run, and it counted prior events outside the lookback window as resolved, which cron's own count does not. Cron now keeps the only observation, under its cache directory, and passes that directory to the analysis. +- **A page's first `refract cron` run printed a stack trace.** The report file that does not exist until the first run finishes was logged as a read failure, with the ENOENT and its full stack. `refract analyze --since` did the same for the observation file on its first run. A missing file is now silent; a file that exists but cannot be read or parsed is reported in one line naming the path and the reason. ## 0.5.17 (2026-09-26) diff --git a/packages/cli/src/__tests__/analyze-helpers.test.ts b/packages/cli/src/__tests__/analyze-helpers.test.ts new file mode 100644 index 0000000..72c38b7 --- /dev/null +++ b/packages/cli/src/__tests__/analyze-helpers.test.ts @@ -0,0 +1,35 @@ +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { finalizeEvents } from "../commands/analyze-helpers.js"; + +describe("finalizeEvents", () => { + it("records a first --since observation without logging the missing prior file", () => { + const obsDir = mkdtempSync(join(tmpdir(), "analyze-obs-")); + const stderr = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + finalizeEvents("Earth", [], [], "2024-01-01T00:00:00Z", obsDir); + expect(stderr.mock.calls).toEqual([["First observation — no delta available."]]); + expect(existsSync(join(obsDir, "Earth.json"))).toBe(true); + } finally { + stderr.mockRestore(); + rmSync(obsDir, { recursive: true, force: true }); + } + }); + + it("neither prints nor writes an observation when the caller keeps its own", () => { + const obsDir = mkdtempSync(join(tmpdir(), "analyze-obs-")); + const stderr = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + finalizeEvents("Earth", [], [], "2024-01-01T00:00:00Z", obsDir, false); + expect(stderr).not.toHaveBeenCalled(); + expect(existsSync(join(obsDir, "Earth.json"))).toBe(false); + } finally { + stderr.mockRestore(); + rmSync(obsDir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/src/__tests__/cron-cache-dir.test.ts b/packages/cli/src/__tests__/cron-cache-dir.test.ts new file mode 100644 index 0000000..1c67fcf --- /dev/null +++ b/packages/cli/src/__tests__/cron-cache-dir.test.ts @@ -0,0 +1,68 @@ +import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { homedir, tmpdir } from "node:os"; +import { basename, join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { FAKE_API, installFakeMediaWiki } from "../../../../tests/support/fake-mediawiki.js"; + +// Every homedir() the CLI calls resolves to an empty temp directory, so a write +// to the default ~/.wikihistory lands where this test can see it and not in +// the real one. +vi.mock("node:os", async (importOriginal) => { + const os = await importOriginal(); + const { mkdtempSync } = await import("node:fs"); + const home = mkdtempSync(`${os.tmpdir()}/cron-home-`); + return { ...os, default: { ...os, homedir: () => home }, homedir: () => home }; +}); + +import { runCron } from "../commands/cron.js"; + +/** + * cron.test.ts replaces analysis with a mock, which cannot show what the real + * analysis writes. These run cron over the actual pipeline against the fake + * MediaWiki API. Analysis's --since path used to read and overwrite + * ~/.wikihistory/observations/.json whatever --cache-dir said, and print + * a delta against that file. + */ +describe("cron --cache-dir", () => { + const fakeHome = homedir(); + let restore: () => void; + + beforeAll(() => { + restore = installFakeMediaWiki(); + }); + + afterAll(() => { + restore(); + // Only ever the directory the mock made; never a real home. + if (basename(fakeHome).startsWith("cron-home-")) rmSync(fakeHome, { recursive: true, force: true }); + }); + + it("keeps a real analysis's state inside the cache dir, across two runs", async () => { + expect(basename(fakeHome)).toMatch(/^cron-home-/); + const cacheDir = mkdtempSync(join(tmpdir(), "cron-cache-")); + const pagesFile = join(mkdtempSync(join(tmpdir(), "cron-pages-")), "pages.txt"); + writeFileSync(pagesFile, "Earth\n", "utf-8"); + const stderr = vi.spyOn(console, "error").mockImplementation(() => {}); + const stdout = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + const first = await runCron(pagesFile, 720, FAKE_API, cacheDir); + const second = await runCron(pagesFile, 720, FAKE_API, cacheDir); + + expect(first.reports[0].deltaSummary).toBe("baseline established"); + expect(second.reports[0].deltaSummary).toBe("no changes"); + expect(readdirSync(fakeHome)).toEqual([]); + expect(existsSync(join(cacheDir, "observations", "Earth.json"))).toBe(true); + expect(existsSync(join(cacheDir, "reports", "Earth.json"))).toBe(true); + + const printed = stderr.mock.calls.map((args) => args.map(String).join(" ")).join("\n"); + expect(printed).not.toContain("Re-observation delta"); + expect(printed).not.toContain("ENOENT"); + } finally { + stderr.mockRestore(); + stdout.mockRestore(); + rmSync(cacheDir, { recursive: true, force: true }); + rmSync(join(pagesFile, ".."), { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/src/__tests__/cron.test.ts b/packages/cli/src/__tests__/cron.test.ts index 4116fe3..2a1dd51 100644 --- a/packages/cli/src/__tests__/cron.test.ts +++ b/packages/cli/src/__tests__/cron.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; @@ -130,8 +130,10 @@ describe("cron command", () => { false, undefined, undefined, + tmpDir, undefined, undefined, + { recordObservation: false }, ); rmSync(tmpDir, { recursive: true, force: true }); @@ -153,9 +155,11 @@ describe("cron command", () => { }); it("finds a new event even when analysis rewrites the observation file first", async () => { - // runAnalyze's --since path writes the events it just produced to the same - // observations/.json cron reads. Cron used to read it afterwards and - // diff the run against itself, so it never reported a new event. + // Cron now asks analysis not to record an observation, but when analysis + // did, it wrote the events it just produced to the same + // observations/.json. Cron read the file afterwards and diffed the + // run against itself, so it never reported a new event. Reading the prior + // first keeps that from coming back. const tmpDir = mkdtempSync(join(tmpdir(), "cron-test-")); const pagesFile = join(tmpDir, "pages.txt"); writeFileSync(pagesFile, "Earth\n", "utf-8"); @@ -239,13 +243,11 @@ describe("cron command", () => { vi.mocked(runAnalyze).mockResolvedValueOnce({ events: [seen], revisions: [] }); await runCron(pagesFile, undefined, undefined, tmpDir); - // A day with no edits. Analysis may still write an empty observation file. - vi.mocked(runAnalyze).mockImplementationOnce(async () => { - writeFileSync(obsFile, "[]"); - return { events: [], revisions: [] }; - }); + // A day with no edits. + vi.mocked(runAnalyze).mockResolvedValueOnce({ events: [], revisions: [] }); const quiet = await runCron(pagesFile, undefined, undefined, tmpDir); expect(quiet.reports[0].deltaSummary).toBe("no changes"); + expect(JSON.parse(readFileSync(obsFile, "utf-8"))).toEqual([seen]); const fresh = makeEvent({ eventType: "citation_removed", @@ -261,4 +263,46 @@ describe("cron command", () => { rmSync(tmpDir, { recursive: true, force: true }); }); + + it("writes nothing to stderr on a page's first run", async () => { + // No prior observation or report exists yet, which is what a first run is. + // The missing report was logged with its ENOENT and full stack. + const tmpDir = mkdtempSync(join(tmpdir(), "cron-test-")); + const pagesFile = join(tmpDir, "pages.txt"); + writeFileSync(pagesFile, "Earth\n", "utf-8"); + vi.mocked(runAnalyze).mockResolvedValueOnce({ events: [makeEvent()], revisions: [] }); + const stderr = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + const result = await runCron(pagesFile, undefined, undefined, tmpDir); + expect(result.reports[0].deltaSummary).toBe("baseline established"); + expect(stderr).not.toHaveBeenCalled(); + } finally { + stderr.mockRestore(); + rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("reports an unreadable prior report in one line, without a stack", async () => { + const tmpDir = mkdtempSync(join(tmpdir(), "cron-test-")); + const pagesFile = join(tmpDir, "pages.txt"); + writeFileSync(pagesFile, "Earth\n", "utf-8"); + const reportFile = join(tmpDir, "reports", "Earth.json"); + mkdirSync(join(tmpDir, "reports"), { recursive: true }); + writeFileSync(reportFile, "{ truncated"); + vi.mocked(runAnalyze).mockResolvedValueOnce({ events: [makeEvent()], revisions: [] }); + const stderr = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + await runCron(pagesFile, undefined, undefined, tmpDir); + expect(stderr).toHaveBeenCalledTimes(1); + const args = stderr.mock.calls[0]; + expect(args).toHaveLength(1); + expect(args[0]).toContain(`failed to read prior observation report ${reportFile}: `); + expect(args[0]).not.toContain("\n"); + } finally { + stderr.mockRestore(); + rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/packages/cli/src/commands/analyze-helpers.ts b/packages/cli/src/commands/analyze-helpers.ts index 5d35fd5..cc69d98 100644 --- a/packages/cli/src/commands/analyze-helpers.ts +++ b/packages/cli/src/commands/analyze-helpers.ts @@ -102,26 +102,41 @@ export function correlateTalkPages(sortedRevs: Revision[], talkRevs: Revision[]) return talkEvents; } -/** Observation diffing, schema version stamping, and semantic enrichment. */ +/** + * Reads JSON an earlier run wrote. A missing file is the first run, not an + * error, so it returns undefined silently; anything else it could not read or + * parse is reported in one line, path and reason, and also returns undefined. + */ +export function readPriorJson(file: string, failure: string): T | undefined { + try { + return JSON.parse(readFileSync(file, "utf-8")) as T; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + console.error(`refract: ${failure} ${file}: ${err instanceof Error ? err.message : String(err)}`); + } + return undefined; + } +} + +/** + * Observation diffing, schema version stamping, and semantic enrichment. + * `recordObservation: false` is for a caller that keeps its own observation + * state, as cron does: the delta is neither printed nor written. + */ export function finalizeEvents( pageTitle: string, events: EvidenceEvent[], _sortedRevs: Revision[], fromTimestamp: string | undefined, cacheDir: string | undefined, + recordObservation = true, ): EvidenceEvent[] { - if (fromTimestamp) { + if (fromTimestamp && recordObservation) { const obsDir = cacheDir ?? join(homedir(), ".wikihistory", "observations"); if (!existsSync(obsDir)) mkdirSync(obsDir, { recursive: true }); const obsFile = join(obsDir, `${pageTitle.replace(/[^a-zA-Z0-9_-]/g, "_")}.json`); - let priorEvents: EvidenceEvent[] = []; - try { - const raw = readFileSync(obsFile, "utf-8"); - priorEvents = JSON.parse(raw) as EvidenceEvent[]; - } catch (err) { - console.error("refract: analyze: failed to read prior observation file", err); - } + const priorEvents = readPriorJson(obsFile, "analyze: failed to read prior observation file") ?? []; const obsDiff = diffObservations(priorEvents, events); if (priorEvents.length > 0) { diff --git a/packages/cli/src/commands/analyze.ts b/packages/cli/src/commands/analyze.ts index 3235076..0a8ec6e 100644 --- a/packages/cli/src/commands/analyze.ts +++ b/packages/cli/src/commands/analyze.ts @@ -170,6 +170,7 @@ export async function runAnalyze( cacheDir?: string, auth?: AuthConfig, config?: AnalyzerConfig, + { recordObservation = true }: { recordObservation?: boolean } = {}, ): Promise<{ events: EvidenceEvent[]; revisions: Revision[] }> { if (pagesFile) { return runBatch(pagesFile, depth, fromRevId, toRevId, fromTimestamp, useCache, apiUrl, cacheDir, auth, config); @@ -223,7 +224,7 @@ export async function runAnalyze( ); events.push(...correlateTalkPages(sortedRevs, talkRevs)); - finalizeEvents(pageTitle, events, sortedRevs, fromTimestamp, cacheDir); + finalizeEvents(pageTitle, events, sortedRevs, fromTimestamp, cacheDir, recordObservation); return { events, revisions: sortedRevs }; } diff --git a/packages/cli/src/commands/cron.ts b/packages/cli/src/commands/cron.ts index 51a9265..edf5f51 100644 --- a/packages/cli/src/commands/cron.ts +++ b/packages/cli/src/commands/cron.ts @@ -7,6 +7,7 @@ import type { AuthConfig } from "@refract-org/ingestion"; import type { NotifyConfig } from "../notify.js"; import { sendNotifications } from "../notify.js"; import { buildObservationReport, runAnalyze } from "./analyze.js"; +import { readPriorJson } from "./analyze-helpers.js"; export interface CronReport { pageTitle: string; @@ -54,16 +55,6 @@ function mergeObservationReports(prior: ObservationReport | null, current: Obser }; } -function readPriorObservation(obsFile: string): EvidenceEvent[] { - if (!existsSync(obsFile)) return []; - try { - return JSON.parse(readFileSync(obsFile, "utf-8")) as EvidenceEvent[]; - } catch (err) { - console.error("refract: cron: failed to read prior observation file", err); - return []; - } -} - /** * Prior events this run could have seen again. Strictly after the window's * start: an event at the start itself is the edit into the first revision of @@ -106,14 +97,14 @@ export async function runCron( const safeName = title.replace(/[^a-zA-Z0-9_-]/g, "_"); const obsFile = join(obsDir, `${safeName}.json`); - // The prior observation is read before analysis runs, and cron writes its - // own afterwards. It used to be read after runAnalyze, whose --since path - // overwrites ~/.wikihistory/observations/.json with the events it - // just produced: with the default directory cron diffed the current run - // against itself and never found a new event, and with --cache-dir it read - // a directory nothing wrote, so every run was "baseline established". + // Cron keeps its own observation under baseDir and asks analysis not to + // record one. Analysis's --since path otherwise reads and overwrites + // ~/.wikihistory/observations/.json whatever --cache-dir says, and + // prints a delta against it that counts prior events outside this window + // as resolved. The prior is still read before analysis runs, so a write + // there can never make cron diff a run against itself. const hadPrior = existsSync(obsFile); - const priorEvents = readPriorObservation(obsFile); + const priorEvents = readPriorJson(obsFile, "cron: failed to read prior observation file") ?? []; let priorObservationAt: string | null = null; let fromTimestamp: string | undefined; @@ -140,8 +131,10 @@ export async function runCron( false, apiUrl, undefined, - undefined, + cacheDir, auth, + undefined, + { recordObservation: false }, ); const isFirstObservation = !hadPrior; @@ -177,14 +170,9 @@ export async function runCron( const pageId = revisions[0]?.pageId ?? 0; const currentReport = buildObservationReport(title, pageId, events, revisions); - let priorObservationReport: ObservationReport | null = null; const observationReportFile = join(reportsDir, `${safeName}.json`); - try { - const raw = readFileSync(observationReportFile, "utf-8"); - priorObservationReport = JSON.parse(raw) as ObservationReport; - } catch (err) { - console.error("refract: cron: failed to read prior observation report", err); - } + const priorObservationReport = + readPriorJson(observationReportFile, "cron: failed to read prior observation report") ?? null; const mergedReport = mergeObservationReports(priorObservationReport, currentReport); writeFileSync(observationReportFile, JSON.stringify(mergedReport, null, 2), "utf-8");