Skip to content
27 changes: 27 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1850,6 +1850,26 @@ export function applyPrecisionBreakers(
return closeHoldOnly ? downgradeCloseToHold(afterMerge, true) : afterMerge;
}

/** PURE: which precision-breaker directions actually rewrote the plan — i.e. `planned` had a merge/close that
* `breakerOnPlan` (the post-{@link applyPrecisionBreakers} result) no longer has. Extracted from the call site
* so the bounded-cardinality observability counter (#terminal-outcome-audit) is unit-tested directly, the same
* way applyPrecisionBreakers itself is. Returns at most one entry per direction, in a stable merge-then-close
* order; empty on the common (not-engaged, or nothing downgraded) path. */
export function precisionBreakerDowngradeDirections(planned: PlannedAgentAction[], breakerOnPlan: PlannedAgentAction[]): Array<"merge" | "close"> {
// Reference identity, not "is the class still present anywhere in the array": downgradeMergeToHold /
// downgradeCloseToHold both filter() the input (preserving object identity for every KEPT action) and only
// ever push brand-new label actions, so a specific planned action survives iff the SAME object reference is
// still in breakerOnPlan. A coarse `!breakerOnPlan.some(actionClass === "close")` check would miss a downgrade
// when a plan carries TWO close actions and only one (the heuristic one) is dropped — the surviving
// deterministic close keeps that check from ever firing even though the breaker did rewrite the plan (gate
// review finding, round 2).
const kept = new Set(breakerOnPlan);
const directions: Array<"merge" | "close"> = [];
if (planned.some((action) => action.actionClass === "merge" && !kept.has(action))) directions.push("merge");
if (planned.some((action) => action.actionClass === "close" && !kept.has(action))) directions.push("close");
return directions;
}

/**
* Historical compatibility helper for callers/tests that still need to know whether branch-protection contexts
* were readable. The disposition planner no longer uses this to soften red CI: any visible completed red
Expand Down Expand Up @@ -2358,6 +2378,13 @@ async function runAgentMaintenancePlanAndExecute(
await isHoldOnly(env, repoFullName),
await isCloseHoldOnly(env, repoFullName),
);
// Observability (#terminal-outcome-audit): a bounded-cardinality counter (direction only — no repo/PR/reason
// text) so an operator can see, at a glance, how much of the plan a breaker is currently rewriting, without
// re-deriving it from individual PR audit rows. Fires only when the breaker actually changed something —
// the common (not-engaged) path increments nothing, matching every other breaker log in this codebase.
for (const direction of precisionBreakerDowngradeDirections(planned, breakerOnPlan)) {
incr("gittensory_precision_breaker_downgrades_total", { direction });
}
if (breakerOnPlan.length === 0) return;

// #2552 (gate review finding, round 2): force a fresh rebase + CI recheck when the base has advanced within
Expand Down
81 changes: 65 additions & 16 deletions src/review/outcomes-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
maybeAutoClearHoldOnly,
} from "./auto-tune";
import { computeGateEval } from "./parity";
import { GITTENSORY_NATIVE_SOURCE } from "./parity-wire";

/** PURE: parse the PR number an "Reverts #N / Reverts owner/repo#N" body refers to (GitHub's revert PRs).
* Mirrors reviewbot runtime.ts parseRevertedPrNumber. Returns undefined when the body isn't a revert. */
Expand Down Expand Up @@ -104,6 +105,46 @@ export async function isCloseHoldOnly(
}
}

/** Every project currently holding a PER-PROJECT (not `:global`) `holdonly:`/`closehold:` flag. Used ONLY to
* widen the auto-clear tick's candidate set beyond `report.rows` (#autoclear-deadlock) — the eval report only
* contains a project once it has a fresh DECIDED sample in the window, but a breaker that is suppressing every
* merge/close for a project stops that project from producing new decided samples at all, so a project with no
* OTHER (e.g. merge-side) activity can silently never reappear in `report.rows` and its stuck flag would never
* be reconsidered. `:global` is deliberately excluded here (mirrors {@link shouldAutoClear}: a human-set global
* freeze is never auto-cleared, so it must never enter an auto-clear candidate set). Fail-open (empty) on a DB
* error, matching every other flag read in this module. */
async function listEngagedProjectScopes(env: Env): Promise<{ holdonly: string[]; closehold: string[] }> {
try {
const res = await env.DB.prepare(
"SELECT key, value FROM system_flags WHERE key LIKE 'holdonly:%' OR key LIKE 'closehold:%'",
).all<{ key: string; value: string }>();
const holdonly: string[] = [];
const closehold: string[] = [];
for (const row of res.results ?? []) {
if (!flagTruthy(row.value)) continue;
const [prefix, ...rest] = row.key.split(":");
const project = rest.join(":");
if (!project || project === "global") continue;
// The SQL WHERE clause above only ever matches a "holdonly:" or "closehold:" key, so prefix can never be
// anything else here — a plain else (not another === check) so there is no unreachable branch to cover.
// If the WHERE clause ever grows a third prefix, this must go back to an explicit `else if (prefix ===
// "closehold")` (with a new branch/test for the resulting default case) so an unrecognized prefix is
// never silently miscategorized as closehold.
if (prefix === "holdonly") holdonly.push(project);
else closehold.push(project);
}
return { holdonly, closehold };
} catch (error) {
console.warn(
JSON.stringify({
ev: "flags_read_error",
message: errorMessage(error).slice(0, 120),
}),
);
return { holdonly: [], closehold: [] };
}
}

/** A live FlagStore over system_flags for the circuit-breaker (applyAutoTune / maybeAutoClearHoldOnly +
* applyCloseAutoTune / maybeAutoClearCloseHoldOnly). */
export function createFlagStore(env: Env): FlagStore {
Expand Down Expand Up @@ -448,7 +489,14 @@ const BREAKER_EVAL_WINDOW_DAYS = 90;

/**
* One precision-circuit-breaker tick, run on the scheduled (selftune) cron. Reads the gate-eval confusion
* matrix over gittensory's OWN recorded pr_outcome/gate_decision rows, then engages/clears BOTH breakers:
* matrix over gittensory's OWN recorded pr_outcome/gate_decision rows -- SCOPED to `source: 'gittensory-native'`
* (#autoclear-deadlock / stale-source): review_audit can also carry historical `gate_decision` rows from the
* pre-convergence reviewbot engine (source='reviewbot'), which stopped running once a repo converged and so
* never grows. Reading across ALL sources (the pre-fix behavior) let a permanently-frozen legacy prediction set
* dominate a project's measured precision forever, with no way for it to ever reflect the LIVE gate's actual
* behavior -- exactly the scenario that leaves a breaker stuck: precision can never "recover" against data that
* never changes. Scoping to the live source makes the loop honest: it judges (and can only re-engage on) what
* THIS instance's own gate has actually predicted. It then engages/clears BOTH breakers:
* • MERGE: ENGAGES holdonly:<project> for any repo whose merge precision dropped below the floor over a real
* sample (applyAutoTune) — the would-MERGE → HOLD downgrade then kicks in on the next merge path; AUTO-CLEARS
* an auto-engaged breaker once its cooldown elapsed AND precision recovered (maybeAutoClearHoldOnly).
Expand All @@ -466,6 +514,7 @@ export async function runSelfTuneBreaker(env: Env): Promise<void> {
const report: GateEvalReport = await computeGateEval(env, {
days: BREAKER_EVAL_WINDOW_DAYS,
nowMs,
source: GITTENSORY_NATIVE_SOURCE,
});
const flags = createFlagStore(env);
const engaged = await applyAutoTune(flags, report);
Expand Down Expand Up @@ -507,22 +556,22 @@ export async function runSelfTuneBreaker(env: Env): Promise<void> {
}),
);
}
// Auto-clear any auto-engaged breaker (merge AND close) that has cooled down + recovered (one per repo in the report).
for (const row of report.rows) {
if (await maybeAutoClearHoldOnly(flags, report, row.project, nowMs)) {
console.log(
JSON.stringify({ ev: "breaker_auto_cleared", project: row.project }),
);
// Auto-clear any auto-engaged breaker (merge AND close) that has cooled down + recovered. Candidates are the
// UNION of report.rows (projects with a fresh decided sample) and every project currently holding a
// per-project flag (#autoclear-deadlock) — a project whose breaker is suppressing 100% of its merges/closes
// stops producing new decided samples for THAT action class and can drop out of report.rows entirely, which
// would otherwise strand its flag engaged forever regardless of how long the cooldown has elapsed.
const engagedScopes = await listEngagedProjectScopes(env);
const mergeClearCandidates = new Set([...report.rows.map((row) => row.project), ...engagedScopes.holdonly]);
const closeClearCandidates = new Set([...report.rows.map((row) => row.project), ...engagedScopes.closehold]);
for (const project of mergeClearCandidates) {
if (await maybeAutoClearHoldOnly(flags, report, project, nowMs)) {
console.log(JSON.stringify({ ev: "breaker_auto_cleared", project }));
}
if (
await maybeAutoClearCloseHoldOnly(flags, report, row.project, nowMs)
) {
console.log(
JSON.stringify({
ev: "close_breaker_auto_cleared",
project: row.project,
}),
);
}
for (const project of closeClearCandidates) {
if (await maybeAutoClearCloseHoldOnly(flags, report, project, nowMs)) {
console.log(JSON.stringify({ ev: "close_breaker_auto_cleared", project }));
}
}
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions src/selfhost/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [
["gittensory_regate_ai_skipped_current_total", { help: "Regate requests skipped because AI state is current.", type: "counter" }],
["gittensory_public_surface_publish_skipped_current_total", { help: "Public surface publishes skipped because state is current.", type: "counter" }],
["gittensory_gate_decisions_total", { help: "Gate decisions by conclusion.", type: "counter" }],
["gittensory_precision_breaker_downgrades_total", { help: "Would-merge/would-close actions downgraded to a human hold by an accuracy circuit-breaker, by breaker direction.", type: "counter" }],
["gittensory_reviews_published_total", { help: "Published review comments.", type: "counter" }],
["gittensory_github_branch_protection_permission_denied_total", { help: "GitHub branch-protection reads denied by permissions.", type: "counter" }],
["gittensory_github_pr_files_fetch_total", { help: "GitHub pull-request file fetch attempts.", type: "counter" }],
Expand Down
2 changes: 2 additions & 0 deletions src/services/agent-action-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,8 @@ export function actionParams(action: PlannedAgentAction): AgentPendingActionPara
// Round-trip the CI dependency separately from closeKind: closeKind is intentionally broad (gate-verdict /
// duplicate / slop / CI) for the close-precision breaker, but only red-CI closes need the live-CI guard.
...(action.closeRequiresCiState !== undefined ? { closeRequiresCiState: action.closeRequiresCiState } : {}),
// Round-trip the concrete-evidence tag so the breaker's exemption still applies when a staged close accepts.
...(action.closeConcreteEvidence !== undefined ? { closeConcreteEvidence: action.closeConcreteEvidence } : {}),
};
}

Expand Down
Loading
Loading