diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 54059580aa..5dc8b93840 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -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. @@ -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, }; } @@ -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) diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 14359007b1..47e6ba65f3 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -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 = { @@ -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 @@ -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", diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 8fc4b4a3e8..fa11eefa27 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -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; @@ -239,6 +240,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { slopMode: null, slopMinScore: null, slopAiAdvisory: null, + sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, @@ -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) : 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), @@ -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), @@ -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 || @@ -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 = {}; if (gate.slopMode !== null) slop.mode = gate.slopMode; @@ -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; diff --git a/src/types.ts b/src/types.ts index 8352cd5cf2..64dc108ea7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -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, diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 3d79fa3255..bac19015aa 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -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 }, @@ -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)", () => { @@ -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"); + }); +}); diff --git a/test/unit/gate-check-policy.test.ts b/test/unit/gate-check-policy.test.ts index 84aaceb3be..44c450617c 100644 --- a/test/unit/gate-check-policy.test.ts +++ b/test/unit/gate-check-policy.test.ts @@ -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"); + }); +});