Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions packages/loopover-miner/lib/ams-calibration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -304,18 +304,21 @@ 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(
env: Record<string, string | undefined>,
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 {
Expand Down
8 changes: 6 additions & 2 deletions packages/loopover-miner/lib/ams-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string | undefined>,
existsSync: (path: string) => boolean,
): string | null {
Expand Down
50 changes: 49 additions & 1 deletion test/unit/miner-ams-calibration.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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);
});
});
35 changes: 35 additions & 0 deletions test/unit/miner-discover-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down