Skip to content

Commit 073de61

Browse files
andriypolanskiandriy-polanskicursoragent
authored
feat(review): wire remaining configured gate blockers into signal tracking (#8104) (#8119)
Record RuleFiredEvent for every isConfiguredGateBlocker finding except linked_issue_scope_mismatch (#8101), and emit reversed HumanOverrideEvent on contributor reopen and owner reopen-then-merge when prior fires exist. Co-authored-by: Andriy Polanski <andriy.polanski@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 5553138 commit 073de61

5 files changed

Lines changed: 382 additions & 1 deletion

File tree

src/queue/processors.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ import {
195195
buildIssueAdvisory,
196196
buildPullRequestAdvisory,
197197
evaluateGateCheck,
198+
recordConfiguredGateBlockerSignals,
198199
resolveAiReviewLowConfidenceHold,
199200
} from "../rules/advisory";
200201
import { hasValidationNote, isTestPath } from "../signals/test-evidence";
@@ -10298,6 +10299,11 @@ async function maybePublishPrPublicSurface(
1029810299
let evaluation = shouldEvaluateGate
1029910300
? evaluateGateCheck(advisory, gatePolicy)
1030010301
: undefined;
10302+
// #8104: record RuleFiredEvent for every configured gate blocker except linked_issue_scope_mismatch
10303+
// (#8101). Same advisory+policy as evaluateGateCheck above so the filter stays in lock-step.
10304+
if (evaluation) {
10305+
await recordConfiguredGateBlockerSignals(env, advisory, gatePolicy, repoFullName, pr.number);
10306+
}
1030110307
// Deterministic content/registry surface lane (#1255) — flag-gated + per-repo allowlist, byte-identical when
1030210308
// off (evaluateWithSurfaceLane returns the generic evaluation unchanged and resolves no files). A metagraphed
1030310309
// registry-submission PR's surface verdict OVERRIDES the generic gate; the helper preserves a generic HARD

src/review/outcomes-wire.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ import { tryEnqueueDecisionPackRebuild } from "../services/decision-pack";
2929
import { incr } from "../selfhost/metrics";
3030
import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
3131
import type { GitHubWebhookPayload } from "../types";
32+
import {
33+
CONFIGURED_GATE_BLOCKER_SIGNAL_CODES,
34+
CONFIGURED_GATE_BLOCKER_SIGNAL_LOOKBACK_MS,
35+
} from "../rules/advisory";
3236
import { errorMessage, nowIso } from "../utils/json";
3337
import {
3438
applyAutoTune,
@@ -448,6 +452,33 @@ async function hasRecentOwnerReopenPendingReversal(env: Env, targetKey: string,
448452
}
449453
}
450454

455+
// #8104: when a reversal is recorded for a target that any configured-gate-blocker rule (except
456+
// linked_issue_scope_mismatch — #8101 owns that one) previously fired against, the human undoing of the bot
457+
// action IS the human judgment on those findings. Fixed 30-day lookback; candidate codes come from
458+
// CONFIGURED_GATE_BLOCKER_SIGNAL_CODES so the list cannot silently drift from isConfiguredGateBlocker.
459+
// Callers attach `.catch(() => undefined)`: a SignalStore failure (including a queryRuleHistory read error,
460+
// which deliberately propagates) must never affect whether the underlying reversal itself is recorded.
461+
async function recordConfiguredGateBlockerOverrides(env: Env, targetId: string): Promise<void> {
462+
const store = createSignalStore(env);
463+
const sinceMs = Date.now() - CONFIGURED_GATE_BLOCKER_SIGNAL_LOOKBACK_MS;
464+
await Promise.all(
465+
CONFIGURED_GATE_BLOCKER_SIGNAL_CODES.map(async (ruleId) => {
466+
try {
467+
const history = await store.queryRuleHistory(ruleId, sinceMs);
468+
if (!history.fired.some((event) => event.targetKey === targetId)) return;
469+
await store.recordHumanOverride({
470+
ruleId,
471+
targetKey: targetId,
472+
verdict: "reversed",
473+
occurredAt: nowIso(),
474+
});
475+
} catch {
476+
// Fail-open per code: one SignalStore reject must not skip the rest of the candidate list.
477+
}
478+
}),
479+
);
480+
}
481+
451482
// #8101: when a reversal is recorded for a target that a `linked_issue_scope_mismatch` finding fired
452483
// against (fixed 30-day lookback), the human undoing of the bot action IS the human judgment on that
453484
// finding — record a "reversed" HumanOverrideEvent in the shared calibration module (#7982) so the
@@ -537,6 +568,7 @@ export async function recordReversalSignals(
537568
detail: `Bot-closed PR #${pr.number} reopened by a contributor.`,
538569
metadata: { repoFullName, pullNumber: pr.number },
539570
}).catch(() => undefined);
571+
await recordConfiguredGateBlockerOverrides(env, targetId).catch(() => undefined); // #8104
540572
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
541573
return;
542574
}
@@ -561,6 +593,7 @@ export async function recordReversalSignals(
561593
detail: `Bot-closed PR #${pr.number} reopened and merged by the repo owner.`,
562594
metadata: { repoFullName, pullNumber: pr.number },
563595
}).catch(() => undefined);
596+
await recordConfiguredGateBlockerOverrides(env, targetId).catch(() => undefined); // #8104
564597
await recordLinkedIssueScopeMismatchOverride(env, targetId).catch(() => undefined); // #8101
565598
}
566599
const reverted = parseRevertedPrNumber(pr.body);

src/rules/advisory.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { nowIso } from "../utils/json";
3737
import { LOOPOVER_GATE_CHECK_NAME } from "../review/check-names";
3838
import { CLA_CHECK_UNRESOLVED_CODE, CLA_CONSENT_MISSING_CODE } from "../review/cla-check";
3939
import { REVIEW_THREAD_BLOCKER_CODE } from "../review/review-thread-findings";
40+
import { createSignalStore } from "../review/signal-tracking-wire";
4041
import { labelMatchesPattern } from "../scoring/preview";
4142

4243
export type GateCheckConclusion = "success" | "failure" | "action_required" | "neutral" | "skipped";
@@ -164,6 +165,29 @@ export type GateCheckEvaluation = {
164165
// `ai_review_inconclusive` is deliberately EXCLUDED — that is a "could not review" HOLD, not a false defect.
165166
export const AI_JUDGMENT_BLOCKER_CODES = new Set<string>(["ai_consensus_defect", "ai_review_split"]);
166167

168+
/**
169+
* Every finding code `isConfiguredGateBlocker` can return true for, EXCEPT `linked_issue_scope_mismatch`
170+
* (#8104). That one code is wired by #8101 at its own upstream push / reversal sites — including it here
171+
* would double-count fired/reversed history. Keep this list in sync with `isConfiguredGateBlocker`'s body.
172+
*/
173+
export const CONFIGURED_GATE_BLOCKER_SIGNAL_CODES: readonly string[] = Object.freeze([
174+
"missing_linked_issue",
175+
"duplicate_pr_risk",
176+
...AI_JUDGMENT_BLOCKER_CODES,
177+
REVIEW_THREAD_BLOCKER_CODE,
178+
"secret_leak",
179+
"pre_merge_check_required",
180+
"manifest_missing_tests",
181+
"manifest_linked_issue_required",
182+
"self_authored_linked_issue",
183+
"content_lane_deliverable_missing",
184+
"lockfile_tamper_risk",
185+
CLA_CONSENT_MISSING_CODE,
186+
]);
187+
188+
/** Fixed lookback for reversal→HumanOverrideEvent pairing (#8104) — 30 days in milliseconds. */
189+
export const CONFIGURED_GATE_BLOCKER_SIGNAL_LOOKBACK_MS = 30 * 24 * 60 * 60 * 1000;
190+
167191
/** True when the gate FAILED *solely* because of AI-judgment blockers (every blocker is an AI-judgment code).
168192
* An empty blocker list is NOT an AI-judgment-only failure. PURE. */
169193
export function isAiJudgmentOnlyFailure(evaluation: GateCheckEvaluation): boolean {
@@ -612,6 +636,10 @@ function evaluateGateCheckCore(advisoryResult: Advisory, policy: GateCheckPolicy
612636
// pass/fail. Readiness/quality stays advisory-only.
613637
const effective = applyMergeReadinessGate(policy);
614638
const configuredBlockers = advisoryResult.findings.filter((finding) => isConfiguredGateBlocker(finding, effective));
639+
// #8104: every configured blocker except linked_issue_scope_mismatch (#8101) records a RuleFiredEvent in
640+
// the shared calibration module. evaluateGateCheckCore stays sync/pure (engine parity twin); the env-bearing
641+
// caller awaits {@link recordConfiguredGateBlockerSignals} with the same advisory+policy so this filter and
642+
// the recording loop stay in lock-step.
615643
const qualityWarning = buildQualityGateWarning(effective);
616644
const slopBlocker = buildSlopGateBlocker(effective);
617645
const blockers = [...configuredBlockers, ...(slopBlocker ? [slopBlocker] : [])];
@@ -1025,6 +1053,41 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli
10251053
return false;
10261054
}
10271055

1056+
/**
1057+
* Record a {@link RuleFiredEvent} for every finding that `isConfiguredGateBlocker` would put into
1058+
* `configuredBlockers`, excluding `linked_issue_scope_mismatch` (#8104 / complements #8101). Call from the
1059+
* env-bearing gate path immediately after {@link evaluateGateCheck} with the SAME advisory + policy so the
1060+
* filter matches `evaluateGateCheckCore`'s own. Best-effort: a SignalStore failure never throws and never
1061+
* affects the gate verdict.
1062+
*/
1063+
export async function recordConfiguredGateBlockerSignals(
1064+
env: Env,
1065+
advisoryResult: Advisory,
1066+
policy: GateCheckPolicy,
1067+
repoFullName: string,
1068+
prNumber: number,
1069+
): Promise<void> {
1070+
const effective = applyMergeReadinessGate(policy);
1071+
const configuredBlockers = advisoryResult.findings.filter((finding) => isConfiguredGateBlocker(finding, effective));
1072+
const store = createSignalStore(env);
1073+
const targetKey = `${repoFullName}#${prNumber}`;
1074+
const occurredAt = nowIso();
1075+
await Promise.all(
1076+
configuredBlockers.map((finding) => {
1077+
if (finding.code === "linked_issue_scope_mismatch") return Promise.resolve();
1078+
return store
1079+
.recordRuleFired({
1080+
ruleId: finding.code,
1081+
targetKey,
1082+
outcome: finding.severity ?? "blocker",
1083+
occurredAt,
1084+
...(finding.confidence !== undefined ? { metadata: { confidence: finding.confidence } } : {}),
1085+
})
1086+
.catch(() => undefined);
1087+
}),
1088+
);
1089+
}
1090+
10281091
function buildQualityGateWarning(policy: GateCheckPolicy): AdvisoryFinding | null {
10291092
if (gateMode(policy.qualityGateMode) === "off") return null;
10301093
const score = normalizeScore(policy.readinessScore);
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { afterEach, describe, expect, it, vi } from "vitest";
2+
import {
3+
recordConfiguredGateBlockerSignals,
4+
type GateCheckPolicy,
5+
} from "../../src/rules/advisory";
6+
import * as signalTrackingWire from "../../src/review/signal-tracking-wire";
7+
import { createSignalStore } from "../../src/review/signal-tracking-wire";
8+
import type { Advisory, AdvisoryFinding } from "../../src/types";
9+
import { createTestEnv } from "../helpers/d1";
10+
11+
function finding(over: Partial<AdvisoryFinding> & Pick<AdvisoryFinding, "code">): AdvisoryFinding {
12+
return {
13+
title: over.title ?? over.code,
14+
severity: over.severity ?? "warning",
15+
detail: over.detail ?? `${over.code} detail`,
16+
action: over.action ?? "fix it",
17+
...over,
18+
};
19+
}
20+
21+
function advisory(findings: AdvisoryFinding[]): Advisory {
22+
return {
23+
id: "advisory-8104",
24+
targetType: "pull_request",
25+
targetKey: "owner/repo#7",
26+
repoFullName: "owner/repo",
27+
pullNumber: 7,
28+
headSha: "abc",
29+
conclusion: "neutral",
30+
severity: "warning",
31+
title: "advisory",
32+
summary: `${findings.length} finding(s)`,
33+
findings,
34+
generatedAt: "2026-07-22T00:00:00.000Z",
35+
};
36+
}
37+
38+
const blockAi: GateCheckPolicy = { aiReviewGateMode: "block" };
39+
const blockLinked: GateCheckPolicy = { linkedIssueGateMode: "block" };
40+
const blockSatisfaction: GateCheckPolicy = { linkedIssueSatisfactionGateMode: "block" };
41+
42+
describe("recordConfiguredGateBlockerSignals (#8104)", () => {
43+
afterEach(() => {
44+
vi.restoreAllMocks();
45+
});
46+
47+
it("records a fired signal for ai_consensus_defect when it is a configured gate blocker", async () => {
48+
const env = createTestEnv();
49+
await recordConfiguredGateBlockerSignals(
50+
env,
51+
advisory([finding({ code: "ai_consensus_defect", confidence: 0.95 })]),
52+
blockAi,
53+
"owner/repo",
54+
7,
55+
);
56+
const history = await createSignalStore(env).queryRuleHistory("ai_consensus_defect", 0);
57+
expect(history.fired).toHaveLength(1);
58+
expect(history.fired[0]).toMatchObject({
59+
ruleId: "ai_consensus_defect",
60+
targetKey: "owner/repo#7",
61+
outcome: "warning",
62+
metadata: { confidence: 0.95 },
63+
});
64+
});
65+
66+
it("records a fired signal for ai_review_split when it is a configured gate blocker", async () => {
67+
const env = createTestEnv();
68+
await recordConfiguredGateBlockerSignals(
69+
env,
70+
advisory([finding({ code: "ai_review_split", severity: "critical" })]),
71+
blockAi,
72+
"owner/repo",
73+
7,
74+
);
75+
const history = await createSignalStore(env).queryRuleHistory("ai_review_split", 0);
76+
expect(history.fired).toHaveLength(1);
77+
expect(history.fired[0]).toMatchObject({
78+
ruleId: "ai_review_split",
79+
targetKey: "owner/repo#7",
80+
outcome: "critical",
81+
});
82+
expect(history.fired[0]?.metadata).toBeUndefined();
83+
});
84+
85+
it("records a fired signal for a deterministic code (secret_leak)", async () => {
86+
const env = createTestEnv();
87+
await recordConfiguredGateBlockerSignals(
88+
env,
89+
advisory([finding({ code: "secret_leak", severity: "critical" })]),
90+
{},
91+
"owner/repo",
92+
7,
93+
);
94+
const history = await createSignalStore(env).queryRuleHistory("secret_leak", 0);
95+
expect(history.fired).toHaveLength(1);
96+
expect(history.fired[0]).toMatchObject({
97+
ruleId: "secret_leak",
98+
targetKey: "owner/repo#7",
99+
outcome: "critical",
100+
});
101+
});
102+
103+
it("records a fired signal for missing_linked_issue when linkedIssueGateMode is block", async () => {
104+
const env = createTestEnv();
105+
await recordConfiguredGateBlockerSignals(
106+
env,
107+
advisory([finding({ code: "missing_linked_issue" })]),
108+
blockLinked,
109+
"owner/repo",
110+
7,
111+
);
112+
expect((await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired).toHaveLength(1);
113+
});
114+
115+
it("records NO fired signal for linked_issue_scope_mismatch even when it is a configured blocker (#8101 owns it)", async () => {
116+
const env = createTestEnv();
117+
await recordConfiguredGateBlockerSignals(
118+
env,
119+
advisory([finding({ code: "linked_issue_scope_mismatch" }), finding({ code: "secret_leak", severity: "critical" })]),
120+
blockSatisfaction,
121+
"owner/repo",
122+
7,
123+
);
124+
expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]);
125+
expect((await createSignalStore(env).queryRuleHistory("secret_leak", 0)).fired).toHaveLength(1);
126+
});
127+
128+
it("records NO fired signal when isConfiguredGateBlocker returns false", async () => {
129+
const env = createTestEnv();
130+
// missing_linked_issue defaults to advisory — not a configured blocker.
131+
await recordConfiguredGateBlockerSignals(
132+
env,
133+
advisory([finding({ code: "missing_linked_issue" })]),
134+
{ linkedIssueGateMode: "advisory" },
135+
"owner/repo",
136+
7,
137+
);
138+
expect((await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired).toEqual([]);
139+
});
140+
141+
it("uses outcome 'blocker' when finding.severity is missing (nullish coalescing arm)", async () => {
142+
const env = createTestEnv();
143+
const noSeverity = finding({ code: "secret_leak" });
144+
delete (noSeverity as { severity?: AdvisoryFinding["severity"] }).severity;
145+
await recordConfiguredGateBlockerSignals(env, advisory([noSeverity]), {}, "owner/repo", 7);
146+
expect((await createSignalStore(env).queryRuleHistory("secret_leak", 0)).fired[0]?.outcome).toBe("blocker");
147+
});
148+
149+
it("degrades silently when the SignalStore write rejects: nothing throws", async () => {
150+
vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({
151+
recordRuleFired: async () => {
152+
throw new Error("signal store down");
153+
},
154+
recordHumanOverride: async () => undefined,
155+
queryRuleHistory: async () => ({ fired: [], overrides: [] }),
156+
});
157+
await expect(
158+
recordConfiguredGateBlockerSignals(
159+
createTestEnv(),
160+
advisory([finding({ code: "secret_leak", severity: "critical" })]),
161+
{},
162+
"owner/repo",
163+
7,
164+
),
165+
).resolves.toBeUndefined();
166+
});
167+
});

0 commit comments

Comments
 (0)