diff --git a/packages/loopover-miner/lib/discover-cli.ts b/packages/loopover-miner/lib/discover-cli.ts index bfa18dd5e..2f24631a2 100644 --- a/packages/loopover-miner/lib/discover-cli.ts +++ b/packages/loopover-miner/lib/discover-cli.ts @@ -33,7 +33,10 @@ import type { PortfolioQueueStore } from "./portfolio-queue.js"; import { initRankedCandidatesStore } from "./ranked-candidates.js"; import type { RankedCandidatesStore } from "./ranked-candidates.js"; import { extractContributionProfile } from "./contribution-profile-extract.js"; -import { initContributionProfileCache } from "./contribution-profile-cache.js"; +import { + initContributionProfileCache, + resolveContributionProfileCacheDbPath, +} from "./contribution-profile-cache.js"; import { filterCandidatesByProfiles } from "./contribution-profile-filter.js"; import type { ContributionProfile } from "./contribution-profile.js"; import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; @@ -139,7 +142,13 @@ export type RunDiscoverOptions = { * resolveContributionProfilesForDiscover; injectable so tests avoid the network. */ resolveContributionProfiles?: ( repoFullNames: string[], - ctx: { githubToken?: string; apiBaseUrl?: string; nowMs?: number }, + ctx: { + githubToken?: string; + apiBaseUrl?: string; + nowMs?: number; + env?: Record; + dryRun?: boolean; + }, ) => Promise>; }; @@ -457,8 +466,12 @@ export function renderDiscoverSummary(result: DiscoverResult): string { * repo's label taxonomy/docs unauthenticated (rate limits), so it safe-defaults to no eligibility filtering. * That also keeps callers that don't supply a token (the common CLI path, and every test) hermetic. * + * #10000: honour `ctx.env` for the cache DB path (same as every other store in runDiscover) and honour + * `ctx.dryRun` with the #9679 event-ledger discipline — never open/create a missing cache file on a dry run, + * and never `put` during a dry run even when an existing file is opened for reads. + * * @param {string[]} repoFullNames unique repos among the fanned-out candidates - * @param {{ githubToken?: string, apiBaseUrl?: string, nowMs?: number, initCache?: typeof initContributionProfileCache, extract?: typeof extractContributionProfile }} ctx + * @param {{ githubToken?: string, apiBaseUrl?: string, nowMs?: number, env?: Record, dryRun?: boolean, initCache?: typeof initContributionProfileCache, extract?: typeof extractContributionProfile }} ctx * @returns {Promise>} */ export async function resolveContributionProfilesForDiscover( @@ -467,6 +480,8 @@ export async function resolveContributionProfilesForDiscover( githubToken?: string; apiBaseUrl?: string; nowMs?: number; + env?: Record; + dryRun?: boolean; initCache?: unknown; extract?: unknown; } = {}, @@ -475,24 +490,33 @@ export async function resolveContributionProfilesForDiscover( if (!ctx.githubToken) return profiles; const initCache = (ctx.initCache as typeof initContributionProfileCache | undefined) ?? initContributionProfileCache; const extract = (ctx.extract as typeof extractContributionProfile | undefined) ?? extractContributionProfile; - const cache = initCache(); + const env = ctx.env ?? process.env; + const cacheDbPath = resolveContributionProfileCacheDbPath(env); + // #10000 / #9679: opening a not-yet-existing SQLite store is itself a write (mkdir + create + migrate). On a + // dry run only open when the file already exists; reads are fine, puts are not. + const openCache = !ctx.dryRun || existsSync(cacheDbPath); + const cache = openCache ? initCache(cacheDbPath) : null; try { for (const repoFullName of repoFullNames) { - const cached = cache.get(repoFullName, ctx.nowMs); - if (cached && !cached.stale) { - profiles.set(repoFullName, cached.profile); - continue; + if (cache) { + const cached = cache.get(repoFullName, ctx.nowMs); + if (cached && !cached.stale) { + profiles.set(repoFullName, cached.profile); + continue; + } } const profile = await extract(repoFullName, { githubToken: ctx.githubToken, // exactOptionalPropertyTypes: omit apiBaseUrl when unset (pre-existing optional-prop shape). ...(ctx.apiBaseUrl !== undefined ? { apiBaseUrl: ctx.apiBaseUrl } : {}), } as Parameters[1]); - cache.put(profile, ctx.nowMs); + if (cache && !ctx.dryRun) { + cache.put(profile, ctx.nowMs); + } profiles.set(repoFullName, profile); } } finally { - cache.close(); + cache?.close(); } return profiles; } @@ -580,6 +604,9 @@ export async function runDiscover(args: string[], options: RunDiscoverOptions = githubToken, ...(apiBaseUrl !== undefined ? { apiBaseUrl } : {}), ...(options.nowMs !== undefined ? { nowMs: options.nowMs } : {}), + // #10000: thread caller env + dryRun so the default resolver never writes the contribution-profile cache. + env: options.env ?? process.env, + dryRun: true, }); // RunDiscoverOptions.resolveContributionProfiles is typed as Map (pre-existing .d.ts); // the filter expects ContributionProfile values — same runtime objects. @@ -702,6 +729,9 @@ export async function runDiscover(args: string[], options: RunDiscoverOptions = githubToken, ...(apiBaseUrl !== undefined ? { apiBaseUrl } : {}), ...(options.nowMs !== undefined ? { nowMs: options.nowMs } : {}), + // #10000: same env threading as the dry-run path; dryRun omitted/false so cache opens + puts normally. + env: options.env ?? process.env, + dryRun: false, }); // RunDiscoverOptions.resolveContributionProfiles is typed as Map (pre-existing .d.ts); // the filter expects ContributionProfile values — same runtime objects. diff --git a/test/unit/miner-discover-cli.test.ts b/test/unit/miner-discover-cli.test.ts index ad7a6867e..5b06f45ea 100644 --- a/test/unit/miner-discover-cli.test.ts +++ b/test/unit/miner-discover-cli.test.ts @@ -125,6 +125,7 @@ afterEach(() => { if (previousConfigDir === undefined) delete process.env.LOOPOVER_MINER_CONFIG_DIR; else process.env.LOOPOVER_MINER_CONFIG_DIR = previousConfigDir; vi.restoreAllMocks(); + vi.unstubAllGlobals(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); @@ -1936,6 +1937,173 @@ describe("runDiscover onResult hook (#6522)", () => { expect(extract).not.toHaveBeenCalled(); }); + it("the default resolver re-extracts and puts when the cached row is stale", async () => { + const { resolveContributionProfilesForDiscover } = + await import("../../packages/loopover-miner/lib/discover-cli"); + const cache = { + get: vi.fn(() => ({ + profile: { ...trustworthyProfile }, + fetchedAt: "x", + stale: true, + })), + put: vi.fn(), + close: vi.fn(), + }; + const extract = vi.fn(async (repoFullName: string) => ({ + ...trustworthyProfile, + repoFullName, + })); + await resolveContributionProfilesForDiscover(["acme/widgets"], { + githubToken: "tok", + dryRun: false, + initCache: (() => cache) as never, + extract: extract as never, + }); + expect(extract).toHaveBeenCalledOnce(); + expect(cache.put).toHaveBeenCalledOnce(); + }); + + it("#10000: dryRun with no cache file extracts without opening or writing the store", async () => { + const { resolveContributionProfilesForDiscover } = + await import("../../packages/loopover-miner/lib/discover-cli"); + const { resolveContributionProfileCacheDbPath } = + await import("../../packages/loopover-miner/lib/contribution-profile-cache"); + const dir = mkdtempSync(join(tmpdir(), "miner-discover-profile-dry-miss-")); + roots.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const cachePath = resolveContributionProfileCacheDbPath(env); + expect(existsSync(cachePath)).toBe(false); + + const initCache = vi.fn(() => { + throw new Error("initCache must not open on a dry-run miss"); + }); + const extract = vi.fn(async (repoFullName: string) => ({ + ...trustworthyProfile, + repoFullName, + })); + + const profiles = await resolveContributionProfilesForDiscover(["acme/widgets"], { + githubToken: "tok", + dryRun: true, + env, + nowMs: NOW, + initCache: initCache as never, + extract: extract as never, + }); + + expect(initCache).not.toHaveBeenCalled(); + expect(extract).toHaveBeenCalledOnce(); + expect(profiles.get("acme/widgets")).toMatchObject({ repoFullName: "acme/widgets" }); + expect(existsSync(cachePath)).toBe(false); + }); + + it("#10000: dryRun with an existing fresh cache row reads it and never puts", async () => { + const { resolveContributionProfilesForDiscover } = + await import("../../packages/loopover-miner/lib/discover-cli"); + const { + initContributionProfileCache, + resolveContributionProfileCacheDbPath, + } = await import("../../packages/loopover-miner/lib/contribution-profile-cache"); + const dir = mkdtempSync(join(tmpdir(), "miner-discover-profile-dry-hit-")); + roots.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const cachePath = resolveContributionProfileCacheDbPath(env); + + const seed = initContributionProfileCache(cachePath); + seed.put(trustworthyProfile as never, NOW); + const before = seed.get("acme/widgets", NOW); + expect(before?.stale).toBe(false); + const fetchedAtBefore = before?.fetchedAt; + seed.close(); + + const extract = vi.fn(); + const profiles = await resolveContributionProfilesForDiscover(["acme/widgets"], { + githubToken: "tok", + dryRun: true, + env, + nowMs: NOW, + extract: extract as never, + }); + + expect(extract).not.toHaveBeenCalled(); + expect(profiles.get("acme/widgets")).toMatchObject({ repoFullName: "acme/widgets" }); + + const after = initContributionProfileCache(cachePath); + expect(after.get("acme/widgets", NOW)?.fetchedAt).toBe(fetchedAtBefore); + after.close(); + }); + + it("#10000: dryRun with an existing but stale/missing row extracts without put", async () => { + const { resolveContributionProfilesForDiscover } = + await import("../../packages/loopover-miner/lib/discover-cli"); + const { + initContributionProfileCache, + resolveContributionProfileCacheDbPath, + } = await import("../../packages/loopover-miner/lib/contribution-profile-cache"); + const dir = mkdtempSync(join(tmpdir(), "miner-discover-profile-dry-stale-")); + roots.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const cachePath = resolveContributionProfileCacheDbPath(env); + // Create the file (empty) so existsSync is true, then leave it with no fresh row. + initContributionProfileCache(cachePath).close(); + + const extract = vi.fn(async (repoFullName: string) => ({ + ...trustworthyProfile, + repoFullName, + })); + await resolveContributionProfilesForDiscover(["acme/widgets"], { + githubToken: "tok", + dryRun: true, + env, + nowMs: NOW, + extract: extract as never, + }); + expect(extract).toHaveBeenCalledOnce(); + + const after = initContributionProfileCache(cachePath); + expect(after.get("acme/widgets", NOW)).toBeNull(); + after.close(); + }); + + it("#10000: resolves the cache path from ctx.env and falls back to process.env when omitted", async () => { + const { resolveContributionProfilesForDiscover } = + await import("../../packages/loopover-miner/lib/discover-cli"); + const { resolveContributionProfileCacheDbPath } = + await import("../../packages/loopover-miner/lib/contribution-profile-cache"); + const dir = mkdtempSync(join(tmpdir(), "miner-discover-profile-env-")); + roots.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const expectedPath = resolveContributionProfileCacheDbPath(env); + const initCache = vi.fn(() => ({ + get: vi.fn(() => null), + put: vi.fn(), + close: vi.fn(), + })); + const extract = vi.fn(async (repoFullName: string) => ({ + ...trustworthyProfile, + repoFullName, + })); + + await resolveContributionProfilesForDiscover(["acme/widgets"], { + githubToken: "tok", + dryRun: false, + env, + initCache: initCache as never, + extract: extract as never, + }); + expect(initCache).toHaveBeenCalledWith(expectedPath); + + initCache.mockClear(); + const ambientPath = resolveContributionProfileCacheDbPath(process.env); + await resolveContributionProfilesForDiscover(["acme/widgets"], { + githubToken: "tok", + dryRun: false, + initCache: initCache as never, + extract: extract as never, + }); + expect(initCache).toHaveBeenCalledWith(ambientPath); + }); + describe("assignee-exclusion (#7040)", () => { it("excludes a candidate assigned to the repo's own owner, unconditionally enqueuing only the rest", async () => { const issues = [ @@ -2403,3 +2571,123 @@ describe("#9679: --dry-run makes zero event-ledger writes", () => { } }); }); + +describe("#10000: --dry-run makes zero contribution-profile-cache writes", () => { + const fanOut = vi.fn(async () => ({ + issues: [fanOutIssue({ issueNumber: 1, title: "candidate" })], + warnings: [], + rateLimitRemaining: 5000, + rateLimitResetAt: "2026-07-09T13:00:00.000Z", + })); + const enqueueOnce = () => + vi.fn(() => ({ enqueued: 1, skippedBelowMinRank: 0, skippedInvalid: 0, itemsAppended: 0 })); + + function stubProfileFetch() { + // extractContributionProfile never throws — a 404-ish response degrades to a low-confidence profile, + // which is enough to exercise the cache open/put path without hitting the network. + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response("Not Found", { status: 404, headers: { "content-type": "text/plain" } })), + ); + } + + it("REGRESSION: --dry-run creates no contribution-profile cache file when a GitHub token is present", async () => { + const { resolveContributionProfileCacheDbPath } = + await import("../../packages/loopover-miner/lib/contribution-profile-cache"); + vi.spyOn(console, "log").mockImplementation(() => undefined); + stubProfileFetch(); + + const dir = mkdtempSync(join(tmpdir(), "miner-discover-dryrun-nocache-")); + roots.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const cachePath = resolveContributionProfileCacheDbPath(env); + const ambientCachePath = resolveContributionProfileCacheDbPath(process.env); + expect(existsSync(cachePath)).toBe(false); + + const exitCode = await runDiscover(["acme/widgets", "--dry-run", "--json"], { + nowMs: NOW, + env, + githubToken: "t", + fetchCandidateIssuesWithSummary: fanOut, + enqueueRankedDiscovery: enqueueOnce() as never, + }); + expect(exitCode).toBe(0); + // Before the fix, resolveContributionProfilesForDiscover opened/created the cache whenever a token was set. + expect(existsSync(cachePath)).toBe(false); + expect(existsSync(ambientCachePath)).toBe(false); + }); + + it("a non-dry run with options.env creates the contribution-profile cache under that env, not process.env", async () => { + const { resolveContributionProfileCacheDbPath } = + await import("../../packages/loopover-miner/lib/contribution-profile-cache"); + vi.spyOn(console, "log").mockImplementation(() => undefined); + stubProfileFetch(); + + const dir = mkdtempSync(join(tmpdir(), "miner-discover-realrun-cache-env-")); + roots.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const cachePath = resolveContributionProfileCacheDbPath(env); + const ambientCachePath = resolveContributionProfileCacheDbPath(process.env); + expect(cachePath).not.toBe(ambientCachePath); + expect(existsSync(cachePath)).toBe(false); + + const exitCode = await runDiscover(["acme/widgets", "--json"], { + nowMs: NOW, + env, + githubToken: "t", + fetchCandidateIssuesWithSummary: fanOut, + initPortfolioQueue: () => tempQueueStore(), + initPolicyDocCache: () => tempPolicyDocCacheStore(), + initPolicyVerdictCache: () => tempPolicyVerdictCacheStore(), + initRankedCandidatesStore: () => tempRankedCandidatesStore(), + enqueueRankedDiscovery: enqueueOnce() as never, + }); + expect(exitCode).toBe(0); + // Before the fix, initCache() ignored options.env and wrote under the ambient process.env location. + expect(existsSync(cachePath)).toBe(true); + expect(existsSync(ambientCachePath)).toBe(false); + }); + + it("still threads an injected resolveContributionProfiles on both dry-run and real-run paths", async () => { + vi.spyOn(console, "log").mockImplementation(() => undefined); + const resolveContributionProfiles = vi.fn( + async (_repos: string[], _ctx?: Record) => new Map(), + ); + const dryDir = mkdtempSync(join(tmpdir(), "miner-discover-inject-dry-")); + const realDir = mkdtempSync(join(tmpdir(), "miner-discover-inject-real-")); + roots.push(dryDir, realDir); + + await runDiscover(["acme/widgets", "--dry-run", "--json"], { + nowMs: NOW, + githubToken: "t", + env: { LOOPOVER_MINER_CONFIG_DIR: dryDir }, + fetchCandidateIssuesWithSummary: fanOut, + resolveContributionProfiles, + enqueueRankedDiscovery: enqueueOnce() as never, + }); + expect(resolveContributionProfiles).toHaveBeenCalledOnce(); + expect(resolveContributionProfiles.mock.calls[0]?.[1]).toMatchObject({ + githubToken: "t", + dryRun: true, + }); + + resolveContributionProfiles.mockClear(); + await runDiscover(["acme/widgets", "--json"], { + nowMs: NOW, + githubToken: "t", + env: { LOOPOVER_MINER_CONFIG_DIR: realDir }, + fetchCandidateIssuesWithSummary: fanOut, + resolveContributionProfiles, + initPortfolioQueue: () => tempQueueStore(), + initPolicyDocCache: () => tempPolicyDocCacheStore(), + initPolicyVerdictCache: () => tempPolicyVerdictCacheStore(), + initRankedCandidatesStore: () => tempRankedCandidatesStore(), + enqueueRankedDiscovery: enqueueOnce() as never, + }); + expect(resolveContributionProfiles).toHaveBeenCalledOnce(); + expect(resolveContributionProfiles.mock.calls[0]?.[1]).toMatchObject({ + githubToken: "t", + dryRun: false, + }); + }); +});