From 52597051e34c931e05af593224c233d9b7081342 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 02:20:46 -0700 Subject: [PATCH] fix(review): enforce miner-scoped breakers --- src/queue/processors.ts | 16 ++++++++++++++-- src/review/outcomes-wire.ts | 19 ++++++++++++++++--- test/unit/outcomes-wire.test.ts | 22 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 0be0623a4d..2a6c4eddbd 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2796,10 +2796,18 @@ async function runAgentMaintenancePlanAndExecute( // Each read is independent and fail-open (isHoldOnly / isCloseHoldOnly read false until a breaker actually // engages), so the common path is byte-identical (both downgrades return the plan unchanged). The chaining is // extracted into the pure applyPrecisionBreakers below so it is unit-tested directly. + const breakerMinerAuthored = pr.authorLogin + ? ( + await getCachedOfficialMinerDetection(env, pr.authorLogin, { + targetKey: `${repoFullName}#${pr.number}`, + deliveryId, + }) + ).status === "confirmed" + : false; const breakerOnPlan = applyPrecisionBreakers( planned, - await isHoldOnly(env, repoFullName), - await isCloseHoldOnly(env, repoFullName), + await isHoldOnly(env, repoFullName, breakerMinerAuthored), + await isCloseHoldOnly(env, repoFullName, breakerMinerAuthored), { manualReviewLabel: settings.manualReviewLabel, readyToMergeLabel: settings.readyToMergeLabel, @@ -2846,6 +2854,10 @@ async function runAgentMaintenancePlanAndExecute( conclusion: gate.conclusion, action: disposition.actionClass, reasonCode: disposition.blockerClass === "none" ? gate.conclusion : disposition.blockerClass, + // #2352: this row is the ACTUAL autonomous disposition that the precision breaker evaluates, so preserve + // the same miner-authored scope as the gate-check audit row below. Omitting it defaults to non-miner and can + // erase a prior miner-authored prediction for the same head. + minerAuthored: breakerMinerAuthored, }); // #2349 (PR 1): additive per-contributor calibration data, gated identically to recordNativeGateDecision // above -- see src/review/contributor-calibration.ts's doc comment. Currently write-only; nothing reads diff --git a/src/review/outcomes-wire.ts b/src/review/outcomes-wire.ts index fd7c3ea26d..c1a1c7f825 100644 --- a/src/review/outcomes-wire.ts +++ b/src/review/outcomes-wire.ts @@ -60,14 +60,22 @@ function flagTruthy(v: string | null | undefined): boolean { /** Is auto-merge disabled (would-merge → hold) for this project (or globally)? Fail-OPEN (false) on a DB error. * This is the read the merge path consults to downgrade a would-MERGE into a HOLD. */ -export async function isHoldOnly(env: Env, project: string): Promise { +export async function isHoldOnly( + env: Env, + project: string, + minerAuthored = false, +): Promise { try { const res = await env.DB.prepare( "SELECT key, value FROM system_flags", ).all<{ key: string; value: string }>(); const set = new Set(); for (const r of res.results ?? []) if (flagTruthy(r.value)) set.add(r.key); - return set.has("holdonly:global") || set.has(`holdonly:${project}`); + return ( + set.has("holdonly:global") || + set.has(`holdonly:${project}`) || + (minerAuthored && set.has(`holdonly:${minerBreakerScope(project)}`)) + ); } catch (error) { console.warn( JSON.stringify({ @@ -86,6 +94,7 @@ export async function isHoldOnly(env: Env, project: string): Promise { export async function isCloseHoldOnly( env: Env, project: string, + minerAuthored = false, ): Promise { try { const res = await env.DB.prepare( @@ -93,7 +102,11 @@ export async function isCloseHoldOnly( ).all<{ key: string; value: string }>(); const set = new Set(); for (const r of res.results ?? []) if (flagTruthy(r.value)) set.add(r.key); - return set.has("closehold:global") || set.has(`closehold:${project}`); + return ( + set.has("closehold:global") || + set.has(`closehold:${project}`) || + (minerAuthored && set.has(`closehold:${minerBreakerScope(project)}`)) + ); } catch (error) { console.warn( JSON.stringify({ diff --git a/test/unit/outcomes-wire.test.ts b/test/unit/outcomes-wire.test.ts index 63153e0e54..d4f360602c 100644 --- a/test/unit/outcomes-wire.test.ts +++ b/test/unit/outcomes-wire.test.ts @@ -526,6 +526,17 @@ describe("isHoldOnly + createFlagStore (system_flags, migration 0054)", () => { expect(await isHoldOnly(env, "any/repo")).toBe(true); }); + it("enforces a miner-scoped holdonly flag only for confirmed miner-authored PRs", async () => { + const env = createTestEnv(); + const flags = createFlagStore(env); + await flags.setFlag("holdonly:owner/repo:miner", true); + + expect(await isHoldOnly(env, "owner/repo")).toBe(false); + expect(await isHoldOnly(env, "owner/repo", false)).toBe(false); + expect(await isHoldOnly(env, "owner/repo", true)).toBe(true); + expect(await isHoldOnly(env, "owner/other", true)).toBe(false); + }); + it("flagSetAt round-trips the updated_at and is null when unset", async () => { const env = createTestEnv(); const flags = createFlagStore(env); @@ -552,6 +563,17 @@ describe("isCloseHoldOnly + createFlagStore.isCloseHoldOnly (closehold:, expect(await isCloseHoldOnly(env, "any/repo")).toBe(true); }); + it("enforces a miner-scoped closehold flag only for confirmed miner-authored PRs", async () => { + const env = createTestEnv(); + const flags = createFlagStore(env); + await flags.setFlag("closehold:owner/repo:miner", true); + + expect(await isCloseHoldOnly(env, "owner/repo")).toBe(false); + expect(await isCloseHoldOnly(env, "owner/repo", false)).toBe(false); + expect(await isCloseHoldOnly(env, "owner/repo", true)).toBe(true); + expect(await isCloseHoldOnly(env, "owner/other", true)).toBe(false); + }); + it("createFlagStore.isCloseHoldOnly reads the per-project closehold key (not the global one)", async () => { const env = createTestEnv(); const flags = createFlagStore(env);