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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
node_modules/
node_modules
dist/
dist-ssr/
.output
Expand Down
5 changes: 5 additions & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ declare global {
/** Convergence (infra): Browser Rendering binding for visual (before/after screenshot) capture. Optional —
* absent ⇒ no visual capture. Unused until the per-module wiring chunk; an unbound deploy is inert. */
BROWSER?: Fetcher;
/** Convergence (infra): the shared REVIEW_CONFIG KV (reviewbot's per-repo config, keyed by repo slug).
* The converged auto-maintain path resolves each repo's `hardGuardrailGlobs` from it so guarded paths
* force MANUAL review (no auto-merge / auto-close). Optional — absent ⇒ the conservative
* DEFAULT_CRUCIAL_GUARDRAIL_GLOBS fallback applies (CI workflows + scripts still guarded). */
REVIEW_CONFIG?: KVNamespace;
/** TODO (convergence follow-up): a per-PR LOCK Durable Object (`SubmissionLock` mutex) is a separate,
* more-involved sub-task — it needs the ported DO class + its own migration tag, not just a binding here.
* Deliberately NOT declared in this chunk; the review path keeps its current concurrency behavior. */
Expand Down
15 changes: 15 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ import { buildReviewGroundingText, isGroundingEnabled } from "../review/groundin
import { buildReviewRagContext, isRagEnabled } from "../review/rag-wire";
import { isReputationEnabled, recordReputationOutcome, shouldSkipAiForReputation } from "../review/reputation-wire";
import { isConvergenceRepoAllowed } from "../review/cutover-gate";
import { loadHardGuardrailGlobs } from "../review/guardrail-config";
import { isOpsEnabled, runOpsAlerts } from "../review/ops-wire";
import { isSelfTuneEnabled, runSelfTune } from "../review/selftune-wire";
import { recordNativeGateDecision } from "../review/parity-wire";
Expand Down Expand Up @@ -480,12 +481,26 @@ async function maybeRunAgentMaintenance(
if (pr.state !== "open") return;
if (!gate) return;

// Convergence safety: feed the planner the PR's changed paths + the repo's hard-guardrail globs so guarded
// paths force manual review, and flag owner-authored PRs so they are never auto-closed (standing rule).
const [changedFiles, hardGuardrailGlobs] = await Promise.all([
listPullRequestFiles(env, repoFullName, pr.number),
loadHardGuardrailGlobs(env, repoFullName),
]);
const changedPaths = changedFiles.map((file) => file.path).filter((path) => path.length > 0);
const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : "";
const authorLogin = pr.authorLogin ?? "";
const authorIsOwner = authorLogin.length > 0 && authorLogin.toLowerCase() === repoOwner.toLowerCase();

const planned = planAgentMaintenanceActions({
conclusion: gate.conclusion,
blockerTitles: gate.blockers.map((blocker) => blocker.title),
autonomy: settings.autonomy,
autoMaintain: settings.autoMaintain,
slopGateMinScore: settings.slopGateMinScore,
changedPaths,
hardGuardrailGlobs,
authorIsOwner,
pr: {
mergeableState: pr.mergeableState,
reviewDecision: pr.reviewDecision,
Expand Down
36 changes: 36 additions & 0 deletions src/review/guardrail-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { JsonValue } from "../types";

// Per-repo hard-guardrail path globs (paths that force MANUAL review — no auto-merge / no auto-close).
//
// Convergence note: gittensory does not have its own per-repo guardrail config surface, but reviewbot already
// stores carefully-tuned globs per repo in the shared REVIEW_CONFIG KV (keyed by repo slug, e.g. "gittensory"
// / "awesome-claude" / "metagraphed"). That KV is the established home for private, runtime-editable operator
// tuning, so the converged auto-maintain path reads its guardrail globs from there too — no redeploy needed
// to retune, and the same KV survives reviewbot's decommission.

// Conservative cross-repo fallback when a repo has no KV-configured globs: CI workflows + build/policy scripts
// are universally sensitive (the awesome-claude #4196 incident class). Fail-SAFE — a config miss still guards
// these, it never opens the gate wide.
export const DEFAULT_CRUCIAL_GUARDRAIL_GLOBS = [".github/workflows/**", "scripts/**"];

function asNonEmptyStringArray(value: unknown): string[] | null {
if (!Array.isArray(value)) return null;
const out = value.filter((entry): entry is string => typeof entry === "string" && entry.length > 0);
return out.length > 0 ? out : null;
}

/**
* Resolve a repo's hard-guardrail path globs from the shared REVIEW_CONFIG KV (key = repo slug). Falls back to
* DEFAULT_CRUCIAL_GUARDRAIL_GLOBS when the binding / key / field is absent or malformed — fail-SAFE and never
* throws (the auto-maintain trigger is best-effort and must not be sunk by a config read).
*/
export async function loadHardGuardrailGlobs(env: Env, repoFullName: string): Promise<string[]> {
const slug = repoFullName.includes("/") ? repoFullName.slice(repoFullName.indexOf("/") + 1) : repoFullName;
if (!env.REVIEW_CONFIG) return DEFAULT_CRUCIAL_GUARDRAIL_GLOBS;
try {
const config = (await env.REVIEW_CONFIG.get(slug, "json")) as { hardGuardrailGlobs?: JsonValue } | null;
return asNonEmptyStringArray(config?.hardGuardrailGlobs) ?? DEFAULT_CRUCIAL_GUARDRAIL_GLOBS;
} catch {
return DEFAULT_CRUCIAL_GUARDRAIL_GLOBS;
}
}
18 changes: 15 additions & 3 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AgentActionClass, AutoMaintainPolicy, AutoMergeMethod, AutonomyPolicy } from "../types";
import type { GateCheckConclusion } from "../rules/advisory";
import { DEFAULT_AUTO_MAINTAIN_POLICY, autonomyRequiresApproval, isActingAutonomyLevel, resolveAutonomy } from "./autonomy";
import { changedPathsHittingGuardrail } from "../signals/change-guardrail";

// High-slop threshold default when a repo hasn't set slopGateMinScore (mirrors the gate's `high` band).
const DEFAULT_SLOP_GATE_MIN_SCORE = 60;
Expand Down Expand Up @@ -35,6 +36,14 @@ export type AgentActionPlanInput = {
// Optional so the trigger can pass raw repo settings; both fall back to conservative defaults here.
autoMaintain?: AutoMaintainPolicy | undefined;
slopGateMinScore?: number | null | undefined;
// Convergence safety (hard-guardrail port, #4196 incident class): the PR's changed paths + the repo's
// hard-guardrail globs. Any changed path matching a guardrail glob forces MANUAL review — gittensory will
// neither auto-merge, auto-approve, nor auto-close such a PR; it falls through to a human.
changedPaths: string[];
hardGuardrailGlobs: string[];
// True when the PR author is the repo owner (e.g. JSONbored). Standing rule: owner PRs are NEVER
// auto-closed. They may still auto-merge when clean + passing.
authorIsOwner: boolean;
pr: {
mergeableState?: string | null | undefined;
reviewDecision?: string | null | undefined;
Expand Down Expand Up @@ -75,6 +84,9 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne

const blocking = isBlocking(input.conclusion);
const passing = input.conclusion === "success";
// A changed path matching a hard guardrail forces manual review: suppress the irreversible dispositions
// (merge / close) AND the auto-approve that could later satisfy a merge. label + request_changes still run.
const guardrailHit = changedPathsHittingGuardrail(input.changedPaths, input.hardGuardrailGlobs).length > 0;

// 1) label — reflect the verdict bucket. After the neutral/skipped return above, a non-blocking verdict is
// necessarily `success`. Idempotent: skip if the PR already carries the label.
Expand All @@ -94,7 +106,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
reason: `${input.blockerTitles.length || 1} blocker(s)`,
reviewBody: `Gittensory requests changes — the gate is not yet satisfied:\n\n${summary}`,
});
} else if (passing && acting("approve") && input.pr.reviewDecision !== "APPROVED") {
} else if (passing && acting("approve") && !guardrailHit && input.pr.reviewDecision !== "APPROVED") {
actions.push({
actionClass: "approve",
requiresApproval: approval("approve"),
Expand All @@ -105,15 +117,15 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne

// 3) disposition — merge a clean, approved, passing PR; otherwise close clear noise. Mutually exclusive.
const mergeableClean = input.pr.mergeableState === "clean";
const canMerge = passing && acting("merge") && mergeableClean && approvalsSatisfied;
const canMerge = passing && acting("merge") && mergeableClean && approvalsSatisfied && !guardrailHit;
if (canMerge) {
actions.push({
actionClass: "merge",
requiresApproval: approval("merge"),
reason: `gate passed, mergeable, ${autoMaintain.requireApprovals} approval(s) satisfied`,
mergeMethod: autoMaintain.mergeMethod,
});
} else if (acting("close") && !passing) {
} else if (acting("close") && !passing && !guardrailHit && !input.authorIsOwner) {
const noiseReasons: string[] = [];
if (input.pr.slopRisk != null && input.pr.slopRisk >= slopGateMinScore) noiseReasons.push(`slop score ${input.pr.slopRisk} ≥ ${slopGateMinScore}`);
if ((input.pr.linkedDuplicateCount ?? 0) > 0) noiseReasons.push("duplicate of another open PR");
Expand Down
41 changes: 41 additions & 0 deletions src/signals/change-guardrail.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Convergence safety: the hard-guardrail path check for the auto-maintain layer (#778). Changed paths that
// match a repo's hardGuardrailGlobs force MANUAL review — gittensory must never auto-merge OR auto-close a PR
// that touches a guarded path (scoring / auth / CI workflows / policy scripts, etc.). Ported verbatim from
// reviewbot core/change-classifier.ts — the mechanism that prevents the awesome-claude #4196 incident class
// (a weakened policy script auto-merging because its path wasn't guarded). Pure + dependency-free.

/** Convert a path glob (`*` matches within a segment, `**` matches across `/`) to an anchored RegExp. */
function globToRegExp(glob: string): RegExp {
let re = "";
for (let i = 0; i < glob.length; i += 1) {
const c = glob.charAt(i);
if (c === "*") {
if (glob.charAt(i + 1) === "*") {
re += ".*";
i += 1;
if (glob.charAt(i + 1) === "/") i += 1; // `**/` also matches zero segments
} else {
re += "[^/]*";
}
} else if (/[.+?^${}()|[\]\\]/.test(c)) {
re += `\\${c}`;
} else {
re += c;
}
}
return new RegExp(`^${re}$`);
}

/** True if `path` matches any of the globs (`*` within a segment, `**` across `/`). */
export function matchesAny(path: string, globs: string[]): boolean {
return globs.some((g) => globToRegExp(g).test(path));
}

/**
* The changed paths (if any) that trip a hard guardrail. A non-empty result means the PR touches a guarded
* path and MUST fall through to a human — gittensory may neither auto-merge nor auto-close it. Pure.
*/
export function changedPathsHittingGuardrail(changedPaths: string[], hardGuardrailGlobs: string[]): string[] {
if (hardGuardrailGlobs.length === 0) return [];
return changedPaths.filter((path) => path.length > 0 && matchesAny(path, hardGuardrailGlobs));
}
56 changes: 53 additions & 3 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ function input(overrides: Partial<AgentActionPlanInput> & { conclusion: GateChec
autonomy: {},
autoMaintain: { requireApprovals: 1, mergeMethod: "squash" },
slopGateMinScore: 60,
changedPaths: [],
hardGuardrailGlobs: [],
authorIsOwner: false,
pr: { labels: [] },
...overrides,
};
Expand Down Expand Up @@ -73,11 +76,11 @@ describe("planAgentMaintenanceActions (#778)", () => {

it("applies conservative defaults when autoMaintain / slopGateMinScore are omitted", () => {
// no autoMaintain → requireApprovals defaults to 1 → a clean passing PR without APPROVED does NOT merge
expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge");
expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge");
// no slopGateMinScore → defaults to 60 → slopRisk 70 counts as noise and closes
expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, pr: { labels: [], slopRisk: 70 } }))).toContain("close");
expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, pr: { labels: [], slopRisk: 70 } }))).toContain("close");
// ...and slopRisk 50 is below the default → no close
expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, pr: { labels: [], slopRisk: 50 } }))).not.toContain("close");
expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, pr: { labels: [], slopRisk: 50 } }))).not.toContain("close");
});

it("closes clear noise (high slop or duplicate) on a non-passing verdict, and never closes a passing PR", () => {
Expand Down Expand Up @@ -112,4 +115,51 @@ describe("planAgentMaintenanceActions (#778)", () => {
);
expect(classes(plan)).toEqual(["label", "approve", "merge"]);
});

describe("hard-guardrail: a changed path matching a guardrail glob forces manual review", () => {
const guarded = { changedPaths: ["src/scoring/model.ts"], hardGuardrailGlobs: ["src/scoring/**", "scripts/**"] };

it("does NOT auto-merge a clean+approved+passing PR that touches a guarded path", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, ...guarded, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })));
expect(plan).not.toContain("merge");
});

it("does NOT auto-close a noisy failing PR that touches a guarded path", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], ...guarded, pr: { labels: [], slopRisk: 95 } })));
expect(plan).not.toContain("close");
});

it("does NOT auto-approve a passing PR that touches a guarded path (so it can't later satisfy a merge)", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { approve: "auto" }, ...guarded, pr: { labels: [] } })));
expect(plan).not.toContain("approve");
});

it("still labels a guarded PR (the reversible action is unaffected — it just falls to a human)", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", merge: "auto" }, ...guarded, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })));
expect(plan).toContain("label");
expect(plan).not.toContain("merge");
});

it("still auto-merges when the changed paths do NOT match any guardrail glob", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, changedPaths: ["docs/readme.md", "src/ui/button.tsx"], hardGuardrailGlobs: ["src/scoring/**", "scripts/**"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })));
expect(plan).toContain("merge");
});
});

describe("owner-PR guard: never auto-close the repo owner's own PRs", () => {
it("does NOT auto-close a noisy failing PR authored by the repo owner", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsOwner: true, pr: { labels: [], slopRisk: 95 } })));
expect(plan).not.toContain("close");
});

it("DOES auto-close the same noisy PR when the author is not the owner", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsOwner: false, pr: { labels: [], slopRisk: 95 } })));
expect(plan).toContain("close");
});

it("still auto-merges a clean+approved owner PR (the guard blocks only close, never merge)", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, authorIsOwner: true, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })));
expect(plan).toContain("merge");
});
});
});
32 changes: 32 additions & 0 deletions test/unit/change-guardrail.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { describe, expect, it } from "vitest";
import { changedPathsHittingGuardrail, matchesAny } from "../../src/signals/change-guardrail";

describe("change-guardrail glob matching", () => {
it("`**` matches across path separators (a guarded dir guards its whole subtree)", () => {
expect(matchesAny("scripts/foo/bar.sh", ["scripts/**"])).toBe(true);
expect(matchesAny("scripts/build.mjs", ["scripts/**"])).toBe(true);
expect(matchesAny(".github/workflows/ci.yml", [".github/workflows/**"])).toBe(true);
expect(matchesAny("src/scoring/deep/nested/model.ts", ["src/scoring/**"])).toBe(true);
});

it("`**/` also matches zero segments (the dir root itself)", () => {
expect(matchesAny("packages/index.ts", ["packages/**"])).toBe(true);
});

it("`*` matches only within a single segment", () => {
expect(matchesAny("src/auth.ts", ["src/*.ts"])).toBe(true);
expect(matchesAny("src/auth/session.ts", ["src/*.ts"])).toBe(false);
});

it("does not match unrelated paths", () => {
expect(matchesAny("docs/readme.md", ["scripts/**", "src/scoring/**"])).toBe(false);
expect(matchesAny("src/ui/button.tsx", ["src/scoring/**", "src/auth/**"])).toBe(false);
});

it("changedPathsHittingGuardrail returns the offending paths (empty globs ⇒ no hits)", () => {
const globs = ["src/scoring/**", "scripts/**"];
expect(changedPathsHittingGuardrail(["docs/a.md", "src/scoring/x.ts", "scripts/y.mjs"], globs)).toEqual(["src/scoring/x.ts", "scripts/y.mjs"]);
expect(changedPathsHittingGuardrail(["docs/a.md", "src/ui/b.tsx"], globs)).toEqual([]);
expect(changedPathsHittingGuardrail(["src/scoring/x.ts"], [])).toEqual([]);
});
});
Loading
Loading