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
7 changes: 7 additions & 0 deletions packages/loopover-miner/bin/loopover-miner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { runMetrics } from "../lib/metrics-cli.js";
import { runPlanCli } from "../lib/plan-store-cli.js";
import { runClaimCli } from "../lib/claim-ledger-cli.js";
import { runPurge } from "../lib/purge-cli.js";
import { runDenyHooks } from "../lib/deny-hooks-cli.js";
import { runQueueCli } from "../lib/portfolio-queue-cli.js";
import { runOrbExportCli } from "../lib/orb-export.js";
import { runTenantCli } from "../lib/tenant-cli.js";
Expand Down Expand Up @@ -168,6 +169,12 @@ if (cliArgs[0] === "purge") {
process.exit(runPurge(cliArgs.slice(1)));
}

// `deny-hooks` (#8806) is strictly local + offline like `purge` above — it only opens the local synthesis
// store to list/refresh/approve the synthesized guardrail proposals buildAttemptDeps now enforces.
if (cliArgs[0] === "deny-hooks") {
process.exit(runDenyHooks(cliArgs.slice(1)));
}

const packageName = "@loopover/miner";
const packageVersion = resolveMinerVersion(process.env);
const upgradeCommand = resolveUpgradeCommand(packageName);
Expand Down
37 changes: 34 additions & 3 deletions packages/loopover-miner/lib/attempt-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ import { openWorktreeAllocator } from "./worktree-allocator.js";
import type { WorktreeAllocation, WorktreeAllocator } from "./worktree-allocator.js";
import { isValidRepoSegment } from "./repo-clone.js";
import { REJECTION_REASON_AI_USAGE_POLICY_BAN, REJECTION_REASON_OWN_SUBMISSION_REJECTED, resolveOwnOpenPrForIssue, resolveRejectionSignaled } from "./rejection-signal.js";
import { initDenyHookSynthesisStore } from "./deny-hook-synthesis.js";
import type { DenyRule } from "@loopover/engine";
import type { resolveRejectionSignaled as ResolveRejectionSignaledFn } from "./rejection-signal.js";
import { cleanupAttemptWorktree, prepareAttemptWorktree } from "./attempt-worktree.js";
import type {
Expand Down Expand Up @@ -262,14 +264,41 @@ export function parseAttemptArgs(args: string[]): ParsedAttemptArgs {
* constructProductionCodingAgentDriver's own contract) -- callers should report that clearly rather than
* silently falling back to a driver that could never run.
*/
/**
* #8806: maintainer-approved synthesized deny rules finally reach a live consumer. Pre-#8806 the synthesis
* store was written and reviewed but NEVER read at driver construction — every attempt fell back to
* DEFAULT_DENY_RULES, so an operator who approved a synthesized guardrail reasonably (and wrongly) believed
* future attempts respected it. Resolves the repo's effective rules (approved proposals merged over the
* defaults) for the driver's PreToolUse hooks. FAIL-OPEN to undefined (→ the pre-#8806 defaults) on any
* store failure — a guardrail read hiccup must never block an attempt, and the defaults are the historical
* floor, never nothing. `initStore` is an injection seam for tests.
*/
export function resolveAttemptHouseRulesConfig(
repoFullName: string | undefined,
initStore: typeof initDenyHookSynthesisStore = initDenyHookSynthesisStore,
): { rules: readonly DenyRule[]; repoFullName: string } | undefined {
if (!repoFullName) return undefined;
try {
const store = initStore();
try {
return { rules: store.resolveEffectiveRules(repoFullName), repoFullName };
} finally {
store.close();
}
} catch {
return undefined;
}
}

export function buildAttemptDeps(
env: Record<string, string | undefined>,
ledgers: { claimLedger: ClaimLedger; eventLedger: EventLedger; attemptLog: AttemptLog; governorLedger: GovernorLedger; nowMs: number },
ledgers: { claimLedger: ClaimLedger; eventLedger: EventLedger; attemptLog: AttemptLog; governorLedger: GovernorLedger; nowMs: number; repoFullName?: string },
): AttemptDeps {
const houseRulesConfig = resolveAttemptHouseRulesConfig(ledgers.repoFullName);
// AttemptDeps' claimLedger/callback parameter types are looser structural stubs than the real ledgers
// (pre-existing .d.ts drift on attempt-runner); cast preserves the same runtime wiring the .js had.
return {
driver: constructProductionCodingAgentDriver(env),
driver: constructProductionCodingAgentDriver(env, houseRulesConfig !== undefined ? { houseRulesConfig } : {}),
runSlopAssessment: (input) => runSlopAssessment(input as Parameters<typeof runSlopAssessment>[0]),
appendAttemptLogEvent: (event) => {
ledgers.attemptLog.appendAttemptLogEvent(event as Parameters<AttemptLog["appendAttemptLogEvent"]>[0]);
Expand Down Expand Up @@ -520,7 +549,9 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
let deps;
try {
const buildDeps = options.buildAttemptDeps ?? buildAttemptDeps;
deps = buildDeps(env, { claimLedger, eventLedger, attemptLog, governorLedger, nowMs });
// #8806: the target repo threads through so the driver's PreToolUse deny hooks carry the repo's
// maintainer-approved synthesized rules, not only DEFAULT_DENY_RULES.
deps = buildDeps(env, { claimLedger, eventLedger, attemptLog, governorLedger, nowMs, repoFullName: parsed.repoFullName });
} catch (error) {
const reason = describeCliError(error);
return reportCliFailure(
Expand Down
96 changes: 96 additions & 0 deletions packages/loopover-miner/lib/deny-hooks-cli.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// `loopover-miner deny-hooks` (#8806): the operator surface the deny-hook synthesis store (#5667) never
// had — which is WHY its guardrails never enforced: refreshProposals had no caller anywhere (no CLI, no
// wiring), so nothing ever populated or consumed the store outside tests. This module closes the operate
// half (list / approve / reject / refresh); buildAttemptDeps (#8806's other half) closes the enforce half
// by resolving the repo's effective rules into every coding-agent driver's PreToolUse hooks.
//
// `refresh` takes its blocker/path history from an explicit `--history <file.json>` (an array of
// `{ blockerCodes: string[], changedPaths: string[] }` records) — deliberately NOT auto-sourced: no local
// ledger carries both fields today (prediction-ledger has blockerCodes but no changedPaths), and inventing
// an implicit source here would hide that gap instead of documenting it. Auto-sourcing from the miner's own
// PR-outcome history is the tracked follow-up once a ledger records changed paths alongside blockers.
// Strictly local + offline (like `purge`/`queue`): only the local synthesis SQLite is touched.
import { readFileSync } from "node:fs";
import { initDenyHookSynthesisStore } from "./deny-hook-synthesis.js";

const USAGE = [
"Usage:",
" loopover-miner deny-hooks list <owner/repo> [--json]",
" loopover-miner deny-hooks refresh <owner/repo> --history <file.json> [--json]",
" loopover-miner deny-hooks approve <owner/repo> <proposal-id>",
" loopover-miner deny-hooks reject <owner/repo> <proposal-id>",
].join("\n");

type HistoryRecord = { blockerCodes: string[]; changedPaths: string[] };

function parseHistoryFile(path: string): HistoryRecord[] {
const parsed: unknown = JSON.parse(readFileSync(path, "utf8"));
if (!Array.isArray(parsed)) throw new Error("history file must be a JSON array of { blockerCodes, changedPaths } records");
return parsed as HistoryRecord[];
}

export function runDenyHooks(args: string[]): number {
const json = args.includes("--json");
const positional = args.filter((arg) => !arg.startsWith("--"));
const [subcommand, repoFullName, proposalId] = positional;
if (!subcommand || !repoFullName) {
console.error(USAGE);
return 2;
}
const store = initDenyHookSynthesisStore();
try {
switch (subcommand) {
case "list": {
const proposals = store.listProposals(repoFullName);
const effective = store.resolveEffectiveRules(repoFullName);
if (json) {
console.log(JSON.stringify({ repoFullName, proposals, effectiveRuleCount: effective.length }, null, 2));
} else if (proposals.length === 0) {
console.log(`No synthesized proposals for ${repoFullName} (${effective.length} effective rule(s), all defaults).`);
} else {
for (const proposal of proposals) {
console.log(`${proposal.id} [${proposal.status}] ${JSON.stringify(proposal.rule)}`);
}
console.log(`${effective.length} effective rule(s) including defaults — approved proposals enforce on the next attempt.`);
}
return 0;
}
case "refresh": {
const historyFlag = args.indexOf("--history");
const historyPath = historyFlag !== -1 ? args[historyFlag + 1] : undefined;
if (!historyPath) {
console.error("refresh requires --history <file.json>\n" + USAGE);
return 2;
}
const proposals = store.refreshProposals(repoFullName, parseHistoryFile(historyPath));
if (json) {
console.log(JSON.stringify({ repoFullName, proposals }, null, 2));
} else {
console.log(`${proposals.length} proposal(s) for ${repoFullName} — approve with: loopover-miner deny-hooks approve ${repoFullName} <id>`);
}
return 0;
}
case "approve":
case "reject": {
if (!proposalId) {
console.error(USAGE);
return 2;
}
store.setProposalStatus(repoFullName, proposalId, subcommand === "approve" ? "approved" : "rejected");
console.log(`${subcommand === "approve" ? "Approved" : "Rejected"} ${proposalId} for ${repoFullName}${subcommand === "approve" ? " — it enforces on the next attempt." : "."}`);
return 0;
}
default:
console.error(USAGE);
return 2;
}
} catch (error) {
// String(error) renders an Error as "Error: <message>" — every throw site here (store methods,
// readFileSync, JSON.parse, parseHistoryFile) throws real Errors, so a two-arm instanceof ternary
// would carry a permanently-unreachable branch.
console.error(String(error));
return 1;
} finally {
store.close();
}
}
34 changes: 33 additions & 1 deletion test/unit/miner-attempt-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { closeDefaultGovernorLedger, initGovernorLedger } from "../../packages/l
import { closeDefaultWorktreeAllocator, openWorktreeAllocator } from "../../packages/loopover-miner/lib/worktree-allocator.js";
import { closeDefaultPortfolioQueueStore } from "../../packages/loopover-miner/lib/portfolio-queue.js";
import { closeDefaultGovernorState } from "../../packages/loopover-miner/lib/governor-state.js";
import { buildAttemptDeps, parseAttemptArgs, runAttempt } from "../../packages/loopover-miner/lib/attempt-cli.js";
import { buildAttemptDeps, parseAttemptArgs, runAttempt, resolveAttemptHouseRulesConfig } from "../../packages/loopover-miner/lib/attempt-cli.js";
import type { RunAttemptOptions } from "../../packages/loopover-miner/lib/attempt-cli.js";
import type { RuleFiredEvent, SignalStore } from "../../packages/loopover-engine/src/calibration/signal-tracking.js";
import * as minerSentryModule from "../../packages/loopover-miner/lib/sentry.js";
Expand Down Expand Up @@ -2638,3 +2638,35 @@ describe("runAttempt: Neon branch-per-attempt DB fork (#7858)", () => {
expect(exitCode).toBe(7);
});
});

describe("resolveAttemptHouseRulesConfig (#8806)", () => {
it("resolves the repo's effective rules (approved proposals merged over defaults) and closes the store", () => {
const close = vi.fn();
const resolveEffectiveRules = vi.fn(() => [{ toolNamePattern: /Bash/, inputTokenPattern: /CHANGELOG\.md/ }]);
const config = resolveAttemptHouseRulesConfig("acme/widgets", (() => ({ resolveEffectiveRules, close })) as never);
expect(config?.repoFullName).toBe("acme/widgets");
expect(config?.rules).toHaveLength(1);
expect(resolveEffectiveRules).toHaveBeenCalledWith("acme/widgets");
expect(close).toHaveBeenCalled(); // no leaked store handle
});

it("FAIL-OPEN: a store failure (or no repo) resolves undefined — the pre-#8806 DEFAULT_DENY_RULES floor, never a blocked attempt", () => {
expect(resolveAttemptHouseRulesConfig(undefined)).toBeUndefined();
expect(
resolveAttemptHouseRulesConfig("acme/widgets", (() => {
throw new Error("store down");
}) as never),
).toBeUndefined();
// A resolve failure AFTER open still closes fail-open to undefined.
const close = vi.fn();
expect(
resolveAttemptHouseRulesConfig("acme/widgets", (() => ({
resolveEffectiveRules: () => {
throw new Error("read failed");
},
close,
})) as never),
).toBeUndefined();
expect(close).toHaveBeenCalled();
});
});
94 changes: 94 additions & 0 deletions test/unit/miner-deny-hooks-cli.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { mkdtempSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { runDenyHooks } from "../../packages/loopover-miner/lib/deny-hooks-cli.js";
import { initDenyHookSynthesisStore } from "../../packages/loopover-miner/lib/deny-hook-synthesis.js";
import { resolveAttemptHouseRulesConfig } from "../../packages/loopover-miner/lib/attempt-cli.js";

// #8806: the operate half of the deny-hook loop — refresh (explicit --history file) → approve → the
// attempt-side resolver picks the approved rule up. The end-to-end test below is the loop the audit found
// severed: pre-#8806 nothing invoked refreshProposals and nothing read resolveEffectiveRules.
describe("loopover-miner deny-hooks (#8806)", () => {
let configDir: string;
const savedEnv = { configDir: process.env.LOOPOVER_MINER_CONFIG_DIR, dbOverride: process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB };

beforeEach(() => {
configDir = mkdtempSync(join(tmpdir(), "miner-deny-hooks-cli-"));
process.env.LOOPOVER_MINER_CONFIG_DIR = configDir;
delete process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB;
});
afterEach(() => {
if (savedEnv.configDir === undefined) delete process.env.LOOPOVER_MINER_CONFIG_DIR;
else process.env.LOOPOVER_MINER_CONFIG_DIR = savedEnv.configDir;
if (savedEnv.dbOverride === undefined) delete process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB;
else process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB = savedEnv.dbOverride;
vi.restoreAllMocks();
});

function writeHistory(): string {
const path = join(configDir, "history.json");
writeFileSync(
path,
JSON.stringify([
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
]),
);
return path;
}

it("END-TO-END: refresh --history → approve → the attempt-side resolver enforces the approved rule", () => {
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
expect(runDenyHooks(["refresh", "acme/widgets", "--history", writeHistory(), "--json"])).toBe(0);
const { proposals } = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as { proposals: Array<{ id: string }> };
expect(proposals.length).toBeGreaterThan(0);

expect(runDenyHooks(["approve", "acme/widgets", proposals[0]!.id])).toBe(0);

// The enforce half: buildAttemptDeps' resolver (same default store path) now includes the approved rule.
const baseline = resolveAttemptHouseRulesConfig("other/repo");
const withApproved = resolveAttemptHouseRulesConfig("acme/widgets");
expect(withApproved?.rules.length).toBe((baseline?.rules.length ?? 0) + 1);
});

it("list renders proposals with status and the effective-rule count", () => {
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
runDenyHooks(["refresh", "acme/widgets", "--history", writeHistory()]);
expect(runDenyHooks(["list", "acme/widgets", "--json"])).toBe(0);
const payload = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as { proposals: unknown[]; effectiveRuleCount: number };
expect(payload.proposals.length).toBeGreaterThan(0);
expect(payload.effectiveRuleCount).toBeGreaterThan(0);
// Human output too (both list arms + the empty-repo arm).
expect(runDenyHooks(["list", "acme/widgets"])).toBe(0);
expect(runDenyHooks(["list", "empty/repo"])).toBe(0);
});

it("reject marks a proposal rejected — it never reaches the effective rules", () => {
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
runDenyHooks(["refresh", "acme/widgets", "--history", writeHistory(), "--json"]);
const { proposals } = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as { proposals: Array<{ id: string }> };
expect(runDenyHooks(["reject", "acme/widgets", proposals[0]!.id])).toBe(0);
const store = initDenyHookSynthesisStore();
try {
const baselineCount = store.resolveEffectiveRules("other/repo").length;
expect(store.resolveEffectiveRules("acme/widgets").length).toBe(baselineCount); // defaults only
} finally {
store.close();
}
});

it("usage/argument errors exit 2; a bad history file exits 1 with the parse error", () => {
const error = vi.spyOn(console, "error").mockImplementation(() => undefined);
vi.spyOn(console, "log").mockImplementation(() => undefined);
expect(runDenyHooks([])).toBe(2); // no subcommand
expect(runDenyHooks(["list"])).toBe(2); // no repo
expect(runDenyHooks(["refresh", "acme/widgets"])).toBe(2); // no --history
expect(runDenyHooks(["approve", "acme/widgets"])).toBe(2); // no proposal id
expect(runDenyHooks(["bogus", "acme/widgets"])).toBe(2); // unknown subcommand
const badPath = join(configDir, "bad.json");
writeFileSync(badPath, JSON.stringify({ not: "an array" }));
expect(runDenyHooks(["refresh", "acme/widgets", "--history", badPath])).toBe(1);
expect(error).toHaveBeenCalledWith(expect.stringContaining("JSON array"));
});
});
Loading