Skip to content

Commit 33bacda

Browse files
authored
feat(miner): wire deny-hook synthesis to a live consumer and give it an operator CLI (#8806) (#8817)
The synthesis store (#5667) was written and reviewable but NEVER read: refreshProposals had no caller anywhere (not even a CLI), and both driver construction sites passed no houseRulesConfig — so every attempt fell back to DEFAULT_DENY_RULES and a maintainer-approved synthesized guardrail never enforced anything. - Enforce half: buildAttemptDeps threads the target repo and resolves its effective rules (approved proposals merged over defaults) into the driver's PreToolUse hooks via the new exported resolveAttemptHouseRulesConfig. FAIL-OPEN to the pre-#8806 defaults on any store failure — a guardrail read hiccup never blocks an attempt, and the defaults are the historical floor, never nothing. - Operate half: `loopover-miner deny-hooks list|refresh|approve|reject` (strictly local + offline, mirroring `purge`). refresh takes an explicit --history <file.json> — deliberately not auto-sourced: no local ledger carries blockerCodes AND changedPaths together today (prediction-ledger lacks paths); the implicit-source gap is documented at the module header as the tracked follow-up rather than hidden behind an invented source. - End-to-end test pins the previously-severed loop: refresh → approve → the attempt-side resolver includes the approved rule.
1 parent d5ec15a commit 33bacda

5 files changed

Lines changed: 264 additions & 4 deletions

File tree

packages/loopover-miner/bin/loopover-miner.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { runMetrics } from "../lib/metrics-cli.js";
1616
import { runPlanCli } from "../lib/plan-store-cli.js";
1717
import { runClaimCli } from "../lib/claim-ledger-cli.js";
1818
import { runPurge } from "../lib/purge-cli.js";
19+
import { runDenyHooks } from "../lib/deny-hooks-cli.js";
1920
import { runQueueCli } from "../lib/portfolio-queue-cli.js";
2021
import { runOrbExportCli } from "../lib/orb-export.js";
2122
import { runTenantCli } from "../lib/tenant-cli.js";
@@ -168,6 +169,12 @@ if (cliArgs[0] === "purge") {
168169
process.exit(runPurge(cliArgs.slice(1)));
169170
}
170171

172+
// `deny-hooks` (#8806) is strictly local + offline like `purge` above — it only opens the local synthesis
173+
// store to list/refresh/approve the synthesized guardrail proposals buildAttemptDeps now enforces.
174+
if (cliArgs[0] === "deny-hooks") {
175+
process.exit(runDenyHooks(cliArgs.slice(1)));
176+
}
177+
171178
const packageName = "@loopover/miner";
172179
const packageVersion = resolveMinerVersion(process.env);
173180
const upgradeCommand = resolveUpgradeCommand(packageName);

packages/loopover-miner/lib/attempt-cli.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import { openWorktreeAllocator } from "./worktree-allocator.js";
4444
import type { WorktreeAllocation, WorktreeAllocator } from "./worktree-allocator.js";
4545
import { isValidRepoSegment } from "./repo-clone.js";
4646
import { REJECTION_REASON_AI_USAGE_POLICY_BAN, REJECTION_REASON_OWN_SUBMISSION_REJECTED, resolveOwnOpenPrForIssue, resolveRejectionSignaled } from "./rejection-signal.js";
47+
import { initDenyHookSynthesisStore } from "./deny-hook-synthesis.js";
48+
import type { DenyRule } from "@loopover/engine";
4749
import type { resolveRejectionSignaled as ResolveRejectionSignaledFn } from "./rejection-signal.js";
4850
import { cleanupAttemptWorktree, prepareAttemptWorktree } from "./attempt-worktree.js";
4951
import type {
@@ -262,14 +264,41 @@ export function parseAttemptArgs(args: string[]): ParsedAttemptArgs {
262264
* constructProductionCodingAgentDriver's own contract) -- callers should report that clearly rather than
263265
* silently falling back to a driver that could never run.
264266
*/
267+
/**
268+
* #8806: maintainer-approved synthesized deny rules finally reach a live consumer. Pre-#8806 the synthesis
269+
* store was written and reviewed but NEVER read at driver construction — every attempt fell back to
270+
* DEFAULT_DENY_RULES, so an operator who approved a synthesized guardrail reasonably (and wrongly) believed
271+
* future attempts respected it. Resolves the repo's effective rules (approved proposals merged over the
272+
* defaults) for the driver's PreToolUse hooks. FAIL-OPEN to undefined (→ the pre-#8806 defaults) on any
273+
* store failure — a guardrail read hiccup must never block an attempt, and the defaults are the historical
274+
* floor, never nothing. `initStore` is an injection seam for tests.
275+
*/
276+
export function resolveAttemptHouseRulesConfig(
277+
repoFullName: string | undefined,
278+
initStore: typeof initDenyHookSynthesisStore = initDenyHookSynthesisStore,
279+
): { rules: readonly DenyRule[]; repoFullName: string } | undefined {
280+
if (!repoFullName) return undefined;
281+
try {
282+
const store = initStore();
283+
try {
284+
return { rules: store.resolveEffectiveRules(repoFullName), repoFullName };
285+
} finally {
286+
store.close();
287+
}
288+
} catch {
289+
return undefined;
290+
}
291+
}
292+
265293
export function buildAttemptDeps(
266294
env: Record<string, string | undefined>,
267-
ledgers: { claimLedger: ClaimLedger; eventLedger: EventLedger; attemptLog: AttemptLog; governorLedger: GovernorLedger; nowMs: number },
295+
ledgers: { claimLedger: ClaimLedger; eventLedger: EventLedger; attemptLog: AttemptLog; governorLedger: GovernorLedger; nowMs: number; repoFullName?: string },
268296
): AttemptDeps {
297+
const houseRulesConfig = resolveAttemptHouseRulesConfig(ledgers.repoFullName);
269298
// AttemptDeps' claimLedger/callback parameter types are looser structural stubs than the real ledgers
270299
// (pre-existing .d.ts drift on attempt-runner); cast preserves the same runtime wiring the .js had.
271300
return {
272-
driver: constructProductionCodingAgentDriver(env),
301+
driver: constructProductionCodingAgentDriver(env, houseRulesConfig !== undefined ? { houseRulesConfig } : {}),
273302
runSlopAssessment: (input) => runSlopAssessment(input as Parameters<typeof runSlopAssessment>[0]),
274303
appendAttemptLogEvent: (event) => {
275304
ledgers.attemptLog.appendAttemptLogEvent(event as Parameters<AttemptLog["appendAttemptLogEvent"]>[0]);
@@ -520,7 +549,9 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
520549
let deps;
521550
try {
522551
const buildDeps = options.buildAttemptDeps ?? buildAttemptDeps;
523-
deps = buildDeps(env, { claimLedger, eventLedger, attemptLog, governorLedger, nowMs });
552+
// #8806: the target repo threads through so the driver's PreToolUse deny hooks carry the repo's
553+
// maintainer-approved synthesized rules, not only DEFAULT_DENY_RULES.
554+
deps = buildDeps(env, { claimLedger, eventLedger, attemptLog, governorLedger, nowMs, repoFullName: parsed.repoFullName });
524555
} catch (error) {
525556
const reason = describeCliError(error);
526557
return reportCliFailure(
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// `loopover-miner deny-hooks` (#8806): the operator surface the deny-hook synthesis store (#5667) never
2+
// had — which is WHY its guardrails never enforced: refreshProposals had no caller anywhere (no CLI, no
3+
// wiring), so nothing ever populated or consumed the store outside tests. This module closes the operate
4+
// half (list / approve / reject / refresh); buildAttemptDeps (#8806's other half) closes the enforce half
5+
// by resolving the repo's effective rules into every coding-agent driver's PreToolUse hooks.
6+
//
7+
// `refresh` takes its blocker/path history from an explicit `--history <file.json>` (an array of
8+
// `{ blockerCodes: string[], changedPaths: string[] }` records) — deliberately NOT auto-sourced: no local
9+
// ledger carries both fields today (prediction-ledger has blockerCodes but no changedPaths), and inventing
10+
// an implicit source here would hide that gap instead of documenting it. Auto-sourcing from the miner's own
11+
// PR-outcome history is the tracked follow-up once a ledger records changed paths alongside blockers.
12+
// Strictly local + offline (like `purge`/`queue`): only the local synthesis SQLite is touched.
13+
import { readFileSync } from "node:fs";
14+
import { initDenyHookSynthesisStore } from "./deny-hook-synthesis.js";
15+
16+
const USAGE = [
17+
"Usage:",
18+
" loopover-miner deny-hooks list <owner/repo> [--json]",
19+
" loopover-miner deny-hooks refresh <owner/repo> --history <file.json> [--json]",
20+
" loopover-miner deny-hooks approve <owner/repo> <proposal-id>",
21+
" loopover-miner deny-hooks reject <owner/repo> <proposal-id>",
22+
].join("\n");
23+
24+
type HistoryRecord = { blockerCodes: string[]; changedPaths: string[] };
25+
26+
function parseHistoryFile(path: string): HistoryRecord[] {
27+
const parsed: unknown = JSON.parse(readFileSync(path, "utf8"));
28+
if (!Array.isArray(parsed)) throw new Error("history file must be a JSON array of { blockerCodes, changedPaths } records");
29+
return parsed as HistoryRecord[];
30+
}
31+
32+
export function runDenyHooks(args: string[]): number {
33+
const json = args.includes("--json");
34+
const positional = args.filter((arg) => !arg.startsWith("--"));
35+
const [subcommand, repoFullName, proposalId] = positional;
36+
if (!subcommand || !repoFullName) {
37+
console.error(USAGE);
38+
return 2;
39+
}
40+
const store = initDenyHookSynthesisStore();
41+
try {
42+
switch (subcommand) {
43+
case "list": {
44+
const proposals = store.listProposals(repoFullName);
45+
const effective = store.resolveEffectiveRules(repoFullName);
46+
if (json) {
47+
console.log(JSON.stringify({ repoFullName, proposals, effectiveRuleCount: effective.length }, null, 2));
48+
} else if (proposals.length === 0) {
49+
console.log(`No synthesized proposals for ${repoFullName} (${effective.length} effective rule(s), all defaults).`);
50+
} else {
51+
for (const proposal of proposals) {
52+
console.log(`${proposal.id} [${proposal.status}] ${JSON.stringify(proposal.rule)}`);
53+
}
54+
console.log(`${effective.length} effective rule(s) including defaults — approved proposals enforce on the next attempt.`);
55+
}
56+
return 0;
57+
}
58+
case "refresh": {
59+
const historyFlag = args.indexOf("--history");
60+
const historyPath = historyFlag !== -1 ? args[historyFlag + 1] : undefined;
61+
if (!historyPath) {
62+
console.error("refresh requires --history <file.json>\n" + USAGE);
63+
return 2;
64+
}
65+
const proposals = store.refreshProposals(repoFullName, parseHistoryFile(historyPath));
66+
if (json) {
67+
console.log(JSON.stringify({ repoFullName, proposals }, null, 2));
68+
} else {
69+
console.log(`${proposals.length} proposal(s) for ${repoFullName} — approve with: loopover-miner deny-hooks approve ${repoFullName} <id>`);
70+
}
71+
return 0;
72+
}
73+
case "approve":
74+
case "reject": {
75+
if (!proposalId) {
76+
console.error(USAGE);
77+
return 2;
78+
}
79+
store.setProposalStatus(repoFullName, proposalId, subcommand === "approve" ? "approved" : "rejected");
80+
console.log(`${subcommand === "approve" ? "Approved" : "Rejected"} ${proposalId} for ${repoFullName}${subcommand === "approve" ? " — it enforces on the next attempt." : "."}`);
81+
return 0;
82+
}
83+
default:
84+
console.error(USAGE);
85+
return 2;
86+
}
87+
} catch (error) {
88+
// String(error) renders an Error as "Error: <message>" — every throw site here (store methods,
89+
// readFileSync, JSON.parse, parseHistoryFile) throws real Errors, so a two-arm instanceof ternary
90+
// would carry a permanently-unreachable branch.
91+
console.error(String(error));
92+
return 1;
93+
} finally {
94+
store.close();
95+
}
96+
}

test/unit/miner-attempt-cli.test.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { closeDefaultGovernorLedger, initGovernorLedger } from "../../packages/l
1515
import { closeDefaultWorktreeAllocator, openWorktreeAllocator } from "../../packages/loopover-miner/lib/worktree-allocator.js";
1616
import { closeDefaultPortfolioQueueStore } from "../../packages/loopover-miner/lib/portfolio-queue.js";
1717
import { closeDefaultGovernorState } from "../../packages/loopover-miner/lib/governor-state.js";
18-
import { buildAttemptDeps, parseAttemptArgs, runAttempt } from "../../packages/loopover-miner/lib/attempt-cli.js";
18+
import { buildAttemptDeps, parseAttemptArgs, runAttempt, resolveAttemptHouseRulesConfig } from "../../packages/loopover-miner/lib/attempt-cli.js";
1919
import type { RunAttemptOptions } from "../../packages/loopover-miner/lib/attempt-cli.js";
2020
import type { RuleFiredEvent, SignalStore } from "../../packages/loopover-engine/src/calibration/signal-tracking.js";
2121
import * as minerSentryModule from "../../packages/loopover-miner/lib/sentry.js";
@@ -2638,3 +2638,35 @@ describe("runAttempt: Neon branch-per-attempt DB fork (#7858)", () => {
26382638
expect(exitCode).toBe(7);
26392639
});
26402640
});
2641+
2642+
describe("resolveAttemptHouseRulesConfig (#8806)", () => {
2643+
it("resolves the repo's effective rules (approved proposals merged over defaults) and closes the store", () => {
2644+
const close = vi.fn();
2645+
const resolveEffectiveRules = vi.fn(() => [{ toolNamePattern: /Bash/, inputTokenPattern: /CHANGELOG\.md/ }]);
2646+
const config = resolveAttemptHouseRulesConfig("acme/widgets", (() => ({ resolveEffectiveRules, close })) as never);
2647+
expect(config?.repoFullName).toBe("acme/widgets");
2648+
expect(config?.rules).toHaveLength(1);
2649+
expect(resolveEffectiveRules).toHaveBeenCalledWith("acme/widgets");
2650+
expect(close).toHaveBeenCalled(); // no leaked store handle
2651+
});
2652+
2653+
it("FAIL-OPEN: a store failure (or no repo) resolves undefined — the pre-#8806 DEFAULT_DENY_RULES floor, never a blocked attempt", () => {
2654+
expect(resolveAttemptHouseRulesConfig(undefined)).toBeUndefined();
2655+
expect(
2656+
resolveAttemptHouseRulesConfig("acme/widgets", (() => {
2657+
throw new Error("store down");
2658+
}) as never),
2659+
).toBeUndefined();
2660+
// A resolve failure AFTER open still closes fail-open to undefined.
2661+
const close = vi.fn();
2662+
expect(
2663+
resolveAttemptHouseRulesConfig("acme/widgets", (() => ({
2664+
resolveEffectiveRules: () => {
2665+
throw new Error("read failed");
2666+
},
2667+
close,
2668+
})) as never),
2669+
).toBeUndefined();
2670+
expect(close).toHaveBeenCalled();
2671+
});
2672+
});
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { mkdtempSync, writeFileSync } from "node:fs";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5+
import { runDenyHooks } from "../../packages/loopover-miner/lib/deny-hooks-cli.js";
6+
import { initDenyHookSynthesisStore } from "../../packages/loopover-miner/lib/deny-hook-synthesis.js";
7+
import { resolveAttemptHouseRulesConfig } from "../../packages/loopover-miner/lib/attempt-cli.js";
8+
9+
// #8806: the operate half of the deny-hook loop — refresh (explicit --history file) → approve → the
10+
// attempt-side resolver picks the approved rule up. The end-to-end test below is the loop the audit found
11+
// severed: pre-#8806 nothing invoked refreshProposals and nothing read resolveEffectiveRules.
12+
describe("loopover-miner deny-hooks (#8806)", () => {
13+
let configDir: string;
14+
const savedEnv = { configDir: process.env.LOOPOVER_MINER_CONFIG_DIR, dbOverride: process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB };
15+
16+
beforeEach(() => {
17+
configDir = mkdtempSync(join(tmpdir(), "miner-deny-hooks-cli-"));
18+
process.env.LOOPOVER_MINER_CONFIG_DIR = configDir;
19+
delete process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB;
20+
});
21+
afterEach(() => {
22+
if (savedEnv.configDir === undefined) delete process.env.LOOPOVER_MINER_CONFIG_DIR;
23+
else process.env.LOOPOVER_MINER_CONFIG_DIR = savedEnv.configDir;
24+
if (savedEnv.dbOverride === undefined) delete process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB;
25+
else process.env.LOOPOVER_MINER_DENY_HOOK_SYNTHESIS_DB = savedEnv.dbOverride;
26+
vi.restoreAllMocks();
27+
});
28+
29+
function writeHistory(): string {
30+
const path = join(configDir, "history.json");
31+
writeFileSync(
32+
path,
33+
JSON.stringify([
34+
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
35+
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
36+
]),
37+
);
38+
return path;
39+
}
40+
41+
it("END-TO-END: refresh --history → approve → the attempt-side resolver enforces the approved rule", () => {
42+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
43+
expect(runDenyHooks(["refresh", "acme/widgets", "--history", writeHistory(), "--json"])).toBe(0);
44+
const { proposals } = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as { proposals: Array<{ id: string }> };
45+
expect(proposals.length).toBeGreaterThan(0);
46+
47+
expect(runDenyHooks(["approve", "acme/widgets", proposals[0]!.id])).toBe(0);
48+
49+
// The enforce half: buildAttemptDeps' resolver (same default store path) now includes the approved rule.
50+
const baseline = resolveAttemptHouseRulesConfig("other/repo");
51+
const withApproved = resolveAttemptHouseRulesConfig("acme/widgets");
52+
expect(withApproved?.rules.length).toBe((baseline?.rules.length ?? 0) + 1);
53+
});
54+
55+
it("list renders proposals with status and the effective-rule count", () => {
56+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
57+
runDenyHooks(["refresh", "acme/widgets", "--history", writeHistory()]);
58+
expect(runDenyHooks(["list", "acme/widgets", "--json"])).toBe(0);
59+
const payload = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as { proposals: unknown[]; effectiveRuleCount: number };
60+
expect(payload.proposals.length).toBeGreaterThan(0);
61+
expect(payload.effectiveRuleCount).toBeGreaterThan(0);
62+
// Human output too (both list arms + the empty-repo arm).
63+
expect(runDenyHooks(["list", "acme/widgets"])).toBe(0);
64+
expect(runDenyHooks(["list", "empty/repo"])).toBe(0);
65+
});
66+
67+
it("reject marks a proposal rejected — it never reaches the effective rules", () => {
68+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
69+
runDenyHooks(["refresh", "acme/widgets", "--history", writeHistory(), "--json"]);
70+
const { proposals } = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as { proposals: Array<{ id: string }> };
71+
expect(runDenyHooks(["reject", "acme/widgets", proposals[0]!.id])).toBe(0);
72+
const store = initDenyHookSynthesisStore();
73+
try {
74+
const baselineCount = store.resolveEffectiveRules("other/repo").length;
75+
expect(store.resolveEffectiveRules("acme/widgets").length).toBe(baselineCount); // defaults only
76+
} finally {
77+
store.close();
78+
}
79+
});
80+
81+
it("usage/argument errors exit 2; a bad history file exits 1 with the parse error", () => {
82+
const error = vi.spyOn(console, "error").mockImplementation(() => undefined);
83+
vi.spyOn(console, "log").mockImplementation(() => undefined);
84+
expect(runDenyHooks([])).toBe(2); // no subcommand
85+
expect(runDenyHooks(["list"])).toBe(2); // no repo
86+
expect(runDenyHooks(["refresh", "acme/widgets"])).toBe(2); // no --history
87+
expect(runDenyHooks(["approve", "acme/widgets"])).toBe(2); // no proposal id
88+
expect(runDenyHooks(["bogus", "acme/widgets"])).toBe(2); // unknown subcommand
89+
const badPath = join(configDir, "bad.json");
90+
writeFileSync(badPath, JSON.stringify({ not: "an array" }));
91+
expect(runDenyHooks(["refresh", "acme/widgets", "--history", badPath])).toBe(1);
92+
expect(error).toHaveBeenCalledWith(expect.stringContaining("JSON array"));
93+
});
94+
});

0 commit comments

Comments
 (0)