diff --git a/packages/loopover-miner/lib/ams-calibration.ts b/packages/loopover-miner/lib/ams-calibration.ts index 4fd47cdcc..ce2acd280 100644 --- a/packages/loopover-miner/lib/ams-calibration.ts +++ b/packages/loopover-miner/lib/ams-calibration.ts @@ -24,7 +24,7 @@ import { import { existsSync as fsExistsSync, readFileSync as fsReadFileSync } from "node:fs"; import { parseAmsPolicySpecContent } from "@loopover/engine"; import type { AppendEventInput, LedgerEntry } from "./event-ledger.js"; -import { resolveAmsPolicyConfigPath } from "./ams-policy.js"; +import { resolveLocalAmsPolicyReadPath } from "./ams-policy.js"; import { MINER_PR_OUTCOME_EVENT } from "./pr-outcome.js"; /** Event-ledger vocabulary for one persisted advisory min-rank backtest run (the AMS analog of ORB's @@ -304,8 +304,11 @@ export function buildAmsBacktestProposals( return proposals; } -/** Sync read of the operator's `.loopover-ams.yml` `minRankAutotuneEnabled` flag (#8187's gate one). The - * async resolveAmsPolicy wrapper exists for attempt-time policy; the calibration commands and discover's +/** Sync read of the operator's `minRankAutotuneEnabled` flag (#8187's gate one), probing the full documented + * {@link AMS_POLICY_SPEC_FILENAMES} discovery order via the SAME `resolveLocalAmsPolicyReadPath` resolver + * `resolveAmsPolicy` uses for attempt-time policy (#10009 -- this used to resolve the canonical filename only, + * silently ignoring the flag when an operator's policy lived at any of the other three documented candidates). + * The async resolveAmsPolicy wrapper exists for attempt-time policy; the calibration commands and discover's * consumption point need only this one boolean and must stay synchronous, so this reuses the same path * resolution + tolerant parser. Fail CLOSED: an unreadable policy file never enables autonomy. */ export function readMinRankAutotuneEnabled( @@ -313,9 +316,9 @@ export function readMinRankAutotuneEnabled( deps: { readFileSync?: typeof fsReadFileSync; existsSync?: typeof fsExistsSync } = {}, ): boolean { try { - const path = resolveAmsPolicyConfigPath(env); const exists = deps.existsSync ?? fsExistsSync; - if (!exists(path)) return false; + const path = resolveLocalAmsPolicyReadPath(env, exists); + if (path === null) return false; const read = deps.readFileSync ?? fsReadFileSync; return parseAmsPolicySpecContent(String(read(path, "utf8"))).spec.minRankAutotuneEnabled; } catch { diff --git a/packages/loopover-miner/lib/ams-policy.ts b/packages/loopover-miner/lib/ams-policy.ts index 896d45ec0..726b271af 100644 --- a/packages/loopover-miner/lib/ams-policy.ts +++ b/packages/loopover-miner/lib/ams-policy.ts @@ -93,8 +93,12 @@ function readLocalAmsPolicyContent(resolved: NormalizedAmsPolicyOptions): string /** Path of the AMS policy file to read: an explicit `LOOPOVER_MINER_AMS_POLICY_PATH` override (when present) points * at one exact file and bypasses discovery; otherwise the first {@link AMS_POLICY_SPEC_FILENAMES} candidate that - * exists in the operator config directory (first match wins), or null when none of them exist. */ -function resolveLocalAmsPolicyReadPath( + * exists in the operator config directory (first match wins), or null when none of them exist. + * + * Exported so every synchronous local reader (e.g. `readMinRankAutotuneEnabled` in ams-calibration.ts) shares + * this exact resolution instead of recreating its own probe loop over `AMS_POLICY_SPEC_FILENAMES` (#8863's fix, + * #10009's sibling gap). */ +export function resolveLocalAmsPolicyReadPath( env: Record, existsSync: (path: string) => boolean, ): string | null { diff --git a/test/unit/miner-ams-calibration.test.ts b/test/unit/miner-ams-calibration.test.ts index e44a7af01..b725740be 100644 --- a/test/unit/miner-ams-calibration.test.ts +++ b/test/unit/miner-ams-calibration.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; @@ -215,4 +215,52 @@ describe("readMinRankAutotuneEnabled (#8187 gate one)", () => { }), ).toBe(false); // fail CLOSED }); + + it("REGRESSION: the autotune flag is honoured from every documented AMS policy filename, not just the canonical one", () => { + // Before #10009, this resolved only AMS_POLICY_SPEC_FILENAMES[0] (.loopover-ams.yml) instead of sharing + // resolveLocalAmsPolicyReadPath, so candidates 2-4 were silently ignored even though resolveAmsPolicy + // (attempt-time) honoured them. + const candidates = [".loopover-ams.yml", ".github/loopover-ams.yml", ".loopover-ams.json", ".github/loopover-ams.json"]; + for (const candidate of candidates) { + const dir = mkdtempSync(join(tmpdir(), "miner-ams-policy-multi-")); + tempDirs.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const path = join(dir, candidate); + mkdirSync(join(path, ".."), { recursive: true }); + const isJson = candidate.endsWith(".json"); + writeFileSync(path, isJson ? JSON.stringify({ minRankAutotuneEnabled: true }) : "minRankAutotuneEnabled: true\n"); + expect(readMinRankAutotuneEnabled(env)).toBe(true); + } + }); + + it("first-match-wins: the canonical file's value is used even when a later candidate also exists", () => { + const dir = mkdtempSync(join(tmpdir(), "miner-ams-policy-precedence-")); + tempDirs.push(dir); + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + writeFileSync(join(dir, ".loopover-ams.yml"), "minRankAutotuneEnabled: false\n"); + mkdirSync(join(dir, ".github"), { recursive: true }); + writeFileSync(join(dir, ".github", "loopover-ams.yml"), "minRankAutotuneEnabled: true\n"); + expect(readMinRankAutotuneEnabled(env)).toBe(false); // canonical candidate wins, same precedence as resolveAmsPolicy + }); + + it("an explicit LOOPOVER_MINER_AMS_POLICY_PATH outside the config dir wins outright and skips discovery", () => { + const dir = mkdtempSync(join(tmpdir(), "miner-ams-policy-explicit-")); + tempDirs.push(dir); + const explicitPath = join(dir, "custom-policy.yml"); + writeFileSync(explicitPath, "minRankAutotuneEnabled: true\n"); + // A discovery candidate is ALSO present, but the explicit override must win without ever probing it. + mkdirSync(join(dir, "cfg"), { recursive: true }); + writeFileSync(join(dir, "cfg", ".loopover-ams.yml"), "minRankAutotuneEnabled: false\n"); + const env = { LOOPOVER_MINER_AMS_POLICY_PATH: explicitPath, LOOPOVER_MINER_CONFIG_DIR: join(dir, "cfg") }; + expect(readMinRankAutotuneEnabled(env)).toBe(true); + }); + + it("an explicit LOOPOVER_MINER_AMS_POLICY_PATH pointing at a nonexistent file returns false without probing discovery candidates", () => { + const dir = mkdtempSync(join(tmpdir(), "miner-ams-policy-explicit-missing-")); + tempDirs.push(dir); + // A discovery candidate exists in the config dir, but it must never be consulted once an explicit path is set. + writeFileSync(join(dir, ".loopover-ams.yml"), "minRankAutotuneEnabled: true\n"); + const env = { LOOPOVER_MINER_AMS_POLICY_PATH: join(dir, "nope.yml"), LOOPOVER_MINER_CONFIG_DIR: dir }; + expect(readMinRankAutotuneEnabled(env)).toBe(false); + }); }); diff --git a/test/unit/miner-discover-cli.test.ts b/test/unit/miner-discover-cli.test.ts index e025596f7..ad7a6867e 100644 --- a/test/unit/miner-discover-cli.test.ts +++ b/test/unit/miner-discover-cli.test.ts @@ -2338,6 +2338,41 @@ describe("#9679: --dry-run makes zero event-ledger writes", () => { } }); + it("REGRESSION (#10009): the earned min-rank override is also applied when the flag lives in .github/loopover-ams.yml", async () => { + const { mkdirSync, mkdtempSync, rmSync, writeFileSync } = await import("node:fs"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + const { initEventLedger, resolveEventLedgerDbPath } = await import("../../packages/loopover-miner/lib/event-ledger"); + const { MINER_AMS_MIN_RANK_APPLIED_EVENT } = await import("../../packages/loopover-miner/lib/ams-calibration"); + vi.spyOn(console, "log").mockImplementation(() => undefined); + + const dir = mkdtempSync(join(tmpdir(), "miner-discover-dryrun-override-github-")); + try { + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const ledger = initEventLedger(resolveEventLedgerDbPath(env)); + ledger.appendEvent({ type: MINER_AMS_MIN_RANK_APPLIED_EVENT, payload: { value: 0.2 } }); + ledger.close(); + // Non-canonical discovery candidate -- before #10009 readMinRankAutotuneEnabled only ever probed the + // canonical .loopover-ams.yml, so this flag was silently ignored here even though resolveAmsPolicy + // (attempt-time) already honoured it. + mkdirSync(join(dir, ".github"), { recursive: true }); + writeFileSync(join(dir, ".github", "loopover-ams.yml"), "minRankAutotuneEnabled: true\n"); + + const enqueueSpy = enqueueOnce(); + const exitCode = await runDiscover(["acme/widgets", "--dry-run", "--json"], { + nowMs: NOW, + env, + fetchCandidateIssuesWithSummary: fanOut, + enqueueRankedDiscovery: enqueueSpy as never, + }); + expect(exitCode).toBe(0); + const minRankSeen = ((enqueueSpy.mock.calls[0] as unknown[] | undefined)?.[1] as { minRankScore?: number } | undefined)?.minRankScore; + expect(minRankSeen).toBe(0.2); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("the non-dry-run path is unchanged: it still opens (and creates) the event ledger", async () => { const { mkdtempSync, rmSync, existsSync } = await import("node:fs"); const { tmpdir } = await import("node:os");