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
28 changes: 28 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3301,6 +3301,11 @@ export function gateCheckPolicy(
confirmedContributor?: boolean,
slopRisk?: number | null,
authorHistory?: { mergedPrCount: number; closedUnmergedPrCount: number },
sizeContext?: {
changedFileCount: number;
changedLineCount: number;
guardrailHit: boolean;
},
) {
// `settings` is already the EFFECTIVE config (`.gittensory.yml` > DB > defaults), resolved upstream by
// resolveRepositorySettings, so the blocker modes here reflect the repo's config file directly.
Expand All @@ -3326,6 +3331,13 @@ export function gateCheckPolicy(
slopGateMinScore: settings.slopGateMinScore ?? null,
slopRisk: slopRisk ?? null,
confirmedContributor: confirmedContributorForPack,
// PR-size + guardrail manual-review HOLD (#gate-size / #gate-guardrail): the MODE comes from config; the
// thresholds default to 10 files / 500 lines (advisory.ts constants); the live counts + guardrail-hit come from
// the per-PR sizeContext threaded by the caller.
sizeGateMode: settings.sizeGateMode,
changedFileCount: sizeContext?.changedFileCount ?? null,
changedLineCount: sizeContext?.changedLineCount ?? null,
guardrailHit: sizeContext?.guardrailHit ?? false,
};
}

Expand Down Expand Up @@ -4546,12 +4558,28 @@ async function maybePublishPrPublicSurface(
pr.number,
);

// PR-size + guardrail manual-review HOLD (#gate-size / #gate-guardrail): compute the live change size + the
// guardrail-hit from the resolved files (getReviewFiles is memoized — no extra fetch) so the gate can HOLD an
// oversized or guardrail-touching PR (neutral → "manual" verdict), visible even in advisory/dry-run.
const sizeGateFiles = await getReviewFiles();
const gateSizeContext = {
changedFileCount: sizeGateFiles.length,
changedLineCount: sizeGateFiles.reduce(
(n, f) => n + f.additions + f.deletions,
0,
),
guardrailHit: isGuardrailHit(
changedPathsForGuardrail(sizeGateFiles),
await loadHardGuardrailGlobs(env, repoFullName),
),
};
const gatePolicy = gateCheckPolicy(
settings,
readiness.total,
confirmedContributor,
slopRisk,
authorHistory,
gateSizeContext,
);
gateEvaluation = gateEnabled
? evaluateGateCheck(advisory, gatePolicy)
Expand Down
64 changes: 64 additions & 0 deletions src/rules/advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ export type GateCheckPolicy = {
* regardless of confirmed status, which now affects only on-chain scoring). `undefined` = unresolved.
* (#gate-nonconfirmed) */
confirmedContributor?: boolean | undefined;
/** PR-size HOLD (#gate-size). When set (advisory/block), a PR with >= sizeGateMaxFiles changed files OR
* >= sizeGateMaxLines changed (added+deleted) lines that would OTHERWISE pass is HELD for manual review — a
* neutral gate → "manual" verdict, never auto-merged and never a hard failure. Defaults off; thresholds default
* to 10 files / 500 lines. This is a HOLD (advisory dry-run friendly), not a close. */
sizeGateMode?: GateRuleMode | undefined;
/** Aggregate change size, threaded from the resolved file list (changedLineCount = additions + deletions). */
changedFileCount?: number | null | undefined;
changedLineCount?: number | null | undefined;
/** True when the PR's diff trips a hard guardrail path (caller computes via loadHardGuardrailGlobs + isGuardrailHit).
* A guardrail hit HOLDS an otherwise-passing gate for manual review (neutral → "manual"), never auto-merged.
* Always-on (the guardrail globs default to the crucial/config-as-code/engine paths). (#gate-guardrail) */
guardrailHit?: boolean | undefined;
};

export type GateCheckEvaluation = {
Expand Down Expand Up @@ -378,6 +390,40 @@ export function formatCheckRunOutput(
return annotations.length > 0 ? { title, summary, text, annotations } : { title, summary, text };
}

const SIZE_HOLD_DEFAULT_MAX_FILES = 10;
const SIZE_HOLD_DEFAULT_MAX_LINES = 500;

/** Oversized-PR manual-review HOLD finding (#gate-size), or null when the size gate is off or the PR is within both
* thresholds. A HOLD (→ neutral gate → "manual" verdict), never a hard blocker, so it is dry-run/advisory friendly. */
function buildSizeHoldFinding(policy: GateCheckPolicy): AdvisoryFinding | null {
if (!policy.sizeGateMode || policy.sizeGateMode === "off") return null;
const files = policy.changedFileCount ?? 0;
const lines = policy.changedLineCount ?? 0;
if (
files < SIZE_HOLD_DEFAULT_MAX_FILES &&
lines < SIZE_HOLD_DEFAULT_MAX_LINES
)
return null;
return {
code: "oversized_pr",
severity: "warning",
title: "Large change — held for manual review",
detail: `This PR changes ${files} file(s) / ${lines} line(s) (hold threshold: ${SIZE_HOLD_DEFAULT_MAX_FILES} files or ${SIZE_HOLD_DEFAULT_MAX_LINES} lines).`,
action: "Split this into smaller, focused PRs, or a maintainer reviews and merges it manually.",
};
}

/** Guardrail-path manual-review HOLD finding (#gate-guardrail). A HOLD (neutral gate), never a hard blocker. */
function buildGuardrailHoldFinding(): AdvisoryFinding {
return {
code: "guardrail_hold",
severity: "warning",
title: "Touches a guarded path — held for manual review",
detail: "This PR changes a guardrail-protected path, so it is held for a maintainer to review and merge manually.",
action: "A maintainer must review and merge this change.",
};
}

export function evaluateGateCheck(advisoryResult: Advisory, policy: GateCheckPolicy = {}): GateCheckEvaluation {
const warnings = advisoryResult.findings.filter((finding) => finding.severity === "warning");
// App/infra state (repo not synced yet, PR not cached): gittensory cannot evaluate this PR yet, so the
Expand Down Expand Up @@ -440,6 +486,24 @@ export function evaluateGateCheck(advisoryResult: Advisory, policy: GateCheckPol
warnings,
};
}
// Manual-review HOLD (#gate-size / #gate-guardrail): a PR that would otherwise PASS but is oversized or touches
// a guarded path is HELD for a human (neutral → "manual" verdict) rather than auto-approved — never a failure,
// so neutral never blocks the merge (dry-run/advisory friendly) and a contributor PR is never auto-closed for size.
const sizeHold = buildSizeHoldFinding(effective);
const guardrailHold = effective.guardrailHit ? buildGuardrailHoldFinding() : null;
const holds = [sizeHold, guardrailHold].filter(
(f): f is AdvisoryFinding => f !== null,
);
if (holds.length > 0) {
return {
enabled: true,
conclusion: "neutral",
title: "Gittensory Gate — held for manual review",
summary: holds.map((h) => sanitizeForCheckRun(h.title)).join("; "),
blockers: [],
warnings: [...warnings, ...holds],
};
}
return {
enabled: true,
conclusion: "success",
Expand Down
11 changes: 11 additions & 0 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type FocusManifestGateConfig = {
slopMode: GateRuleMode | null;
slopMinScore: number | null;
slopAiAdvisory: boolean | null;
sizeMode: GateRuleMode | null;
aiReviewMode: GateRuleMode | null;
aiReviewByok: boolean | null;
aiReviewProvider: "anthropic" | "openai" | null;
Expand Down Expand Up @@ -239,6 +240,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = {
slopMode: null,
slopMinScore: null,
slopAiAdvisory: null,
sizeMode: null,
aiReviewMode: null,
aiReviewByok: null,
aiReviewProvider: null,
Expand Down Expand Up @@ -382,6 +384,11 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
if (slop !== undefined && slop !== null && slopRecord === undefined) {
warnings.push(`Manifest gate field "gate.slop" must be a mapping; ignoring it.`);
}
const size = record.size;
const sizeRecord = size !== null && typeof size === "object" && !Array.isArray(size) ? (size as Record<string, JsonValue>) : undefined;
if (size !== undefined && size !== null && sizeRecord === undefined) {
warnings.push(`Manifest gate field "gate.size" must be a mapping; ignoring it.`);
}
const gate: FocusManifestGateConfig = {
present: false,
enabled: normalizeOptionalBoolean(record.enabled, "gate.enabled", warnings),
Expand All @@ -393,6 +400,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
slopMode: normalizeOptionalGateMode(slopRecord?.mode, "gate.slop.mode", warnings),
slopMinScore: normalizeOptionalScore(slopRecord?.minScore, "gate.slop.minScore", warnings),
slopAiAdvisory: normalizeOptionalBoolean(slopRecord?.aiAdvisory, "gate.slop.aiAdvisory", warnings),
sizeMode: normalizeOptionalGateMode(sizeRecord?.mode, "gate.size.mode", warnings),
aiReviewMode: normalizeOptionalGateMode(aiReviewRecord?.mode, "gate.aiReview.mode", warnings),
aiReviewByok: normalizeOptionalBoolean(aiReviewRecord?.byok, "gate.aiReview.byok", warnings),
aiReviewProvider: normalizeOptionalEnum(aiReviewRecord?.provider, "gate.aiReview.provider", ["anthropic", "openai"] as const, warnings),
Expand All @@ -413,6 +421,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
gate.slopMode !== null ||
gate.slopMinScore !== null ||
gate.slopAiAdvisory !== null ||
gate.sizeMode !== null ||
gate.aiReviewMode !== null ||
gate.aiReviewByok !== null ||
gate.aiReviewProvider !== null ||
Expand Down Expand Up @@ -442,6 +451,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue {
if (gate.readinessMinScore !== null) readiness.minScore = gate.readinessMinScore;
out.readiness = readiness;
}
if (gate.sizeMode !== null) out.size = { mode: gate.sizeMode };
if (gate.slopMode !== null || gate.slopMinScore !== null || gate.slopAiAdvisory !== null) {
const slop: Record<string, JsonValue> = {};
if (gate.slopMode !== null) slop.mode = gate.slopMode;
Expand Down Expand Up @@ -915,6 +925,7 @@ export function resolveEffectiveSettings(dbSettings: RepositorySettings, manifes
if (gate.duplicates !== null) effective.duplicatePrGateMode = gate.duplicates;
if (gate.readinessMode !== null) effective.qualityGateMode = gate.readinessMode;
if (gate.readinessMinScore !== null) effective.qualityGateMinScore = gate.readinessMinScore;
if (gate.sizeMode !== null) effective.sizeGateMode = gate.sizeMode;
if (gate.slopMode !== null) effective.slopGateMode = gate.slopMode;
if (gate.slopMinScore !== null) effective.slopGateMinScore = gate.slopMinScore;
if (gate.slopAiAdvisory !== null) effective.slopAiAdvisory = gate.slopAiAdvisory;
Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,10 @@ export type RepositorySettings = {
* score + warnings in context; `block` = ALSO hard-block when slopRisk >= slopGateMinScore (deterministic
* only, applies to every author like every blocker). Default `off` — opt-in via .gittensory.yml. */
slopGateMode: GateRuleMode;
/** PR-size manual-review HOLD (#gate-size). `off` (default/absent) = no size hold; `advisory`/`block` = a PR with
* >= 10 changed files OR >= 500 changed (added+deleted) lines that would otherwise pass is HELD for manual review
* (neutral gate → "manual" verdict), never auto-merged and never a hard failure. Opt-in via `gate.size.mode`. */
sizeGateMode?: GateRuleMode | undefined;
/** Merge-readiness gate (#merge-readiness). `off`/`advisory`/`block`. No min-score. Default `off`. */
mergeReadinessGateMode: GateRuleMode;
/** Focus-manifest policy gate (#555). When `block`, the focus manifest's declared policy (blocked paths,
Expand Down
17 changes: 15 additions & 2 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ describe("compileFocusManifestPolicy", () => {
issueDiscoveryPolicy: "neutral",
maintainerNotes: [],
publicNotes: ["Keep PRs focused.", "Maximize your reward payout"],
gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, firstTimeContributorGrace: null },
gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, firstTimeContributorGrace: null },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null },
Expand Down Expand Up @@ -766,7 +766,7 @@ describe("parseFocusManifest gate config", () => {
it("parses a full gate section including the readiness block", () => {
const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "block", minScore: 70 } } });
expect(m.present).toBe(true);
expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "block", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, firstTimeContributorGrace: null });
expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "block", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, firstTimeContributorGrace: null });
});

it("parses gate.mergeReadiness + gate.firstTimeContributorGrace, round-trips them, and warns on bad values (#822)", () => {
Expand Down Expand Up @@ -1408,3 +1408,16 @@ describe("composeRepoReviewContext (#review-skills)", () => {
expect(out.length).toBeLessThanOrEqual(16_000);
});
});

describe("gate.size manual-review hold config (#gate-size)", () => {
it("parses gate.size.mode, warns on a non-mapping size, and round-trips via gateConfigToJson", () => {
const m = parseFocusManifest({ gate: { size: { mode: "advisory" } } });
expect(m.gate.sizeMode).toBe("advisory");
expect(m.gate.present).toBe(true);
const bad = parseFocusManifest({ gate: { size: "nope" } });
expect(bad.gate.sizeMode).toBeNull();
expect(bad.warnings.some((w) => w.includes("gate.size"))).toBe(true);
const round = parseFocusManifest({ gate: gateConfigToJson(m.gate) });
expect(round.gate.sizeMode).toBe("advisory");
});
});
25 changes: 25 additions & 0 deletions test/unit/gate-check-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,3 +577,28 @@ describe("buildAuthorizedPrActionAdvisory self-authored parity (#self-authored-p
expect(advisory.findings.some((finding) => finding.code === "self_authored_linked_issue")).toBe(false);
});
});

describe("size + guardrail manual-review HOLD (#gate-size / #gate-guardrail)", () => {
const clean = (): Advisory => ({ ...missingIssueAdvisory(), findings: [] });
it("holds (neutral) an oversized PR; passes under thresholds; off/unset = no hold", () => {
expect(evaluateGateCheck(clean(), { sizeGateMode: "advisory", changedFileCount: 12, changedLineCount: 10 }).conclusion).toBe("neutral"); // > 10 files
expect(evaluateGateCheck(clean(), { sizeGateMode: "advisory", changedFileCount: 2, changedLineCount: 600 }).conclusion).toBe("neutral"); // > 500 lines
expect(evaluateGateCheck(clean(), { sizeGateMode: "advisory", changedFileCount: 9, changedLineCount: 499 }).conclusion).toBe("success"); // under both thresholds
expect(evaluateGateCheck(clean(), { sizeGateMode: "off", changedFileCount: 50, changedLineCount: 9000 }).conclusion).toBe("success"); // gate off ⇒ no hold
expect(evaluateGateCheck(clean(), { changedFileCount: 50, changedLineCount: 9000 }).conclusion).toBe("success"); // mode unset ⇒ no hold
expect(evaluateGateCheck(clean(), { sizeGateMode: "advisory" }).conclusion).toBe("success"); // no counts ⇒ 0 ⇒ no hold
});
it("holds (neutral) on a guardrail hit, surfacing the hold finding in warnings", () => {
const out = evaluateGateCheck(clean(), { guardrailHit: true });
expect(out.conclusion).toBe("neutral");
expect(out.warnings.map((w) => w.code)).toContain("guardrail_hold");
});
it("a real hard blocker WINS over a size/guardrail hold (failure, not neutral)", () => {
const out = evaluateGateCheck(missingIssueAdvisory(), { linkedIssueGateMode: "block", sizeGateMode: "advisory", changedFileCount: 50, changedLineCount: 9000, guardrailHit: true });
expect(out.conclusion).toBe("failure");
});
it("resolveEffectiveSettings maps gate.size.mode → sizeGateMode", () => {
const eff = resolveEffectiveSettings(settings({}), parseFocusManifest({ gate: { size: { mode: "advisory" } } }));
expect(eff.sizeGateMode).toBe("advisory");
});
});
Loading