diff --git a/.loopover.yml.example b/.loopover.yml.example index cd65173dc7..017b656c53 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -686,6 +686,12 @@ settings: # of this setting. Bool. Default: false. closeOwnerAuthors: false + # AI review, gate evaluation, and public-surface publish are skipped ENTIRELY for a PR/event genuinely + # triggered by a known automation bot (release-please, Renovate, Dependabot). "inherit" defers to the + # self-host operator's LOOPOVER_SKIP_AUTOMATION_BOT_PRS env default (itself default-ON); "off"/"enabled" + # override that default in either direction for this repo. inherit | off | enabled. Default: inherit. + # skipAutomationBotAuthors: inherit + # Hard manual-review path guardrails are config-as-code only. Safe by default (#3943): whatever you list # here is ADDED to a fixed, built-in invariant set (CI workflows/scripts, deploy config, config-as-code # files, core engine-decision paths — see DEFAULT_HARD_GUARDRAIL_GLOBS in src/review/guardrail-config.ts) diff --git a/config/examples/loopover.full.yml b/config/examples/loopover.full.yml index 6dc4672b75..5099ddd7a5 100644 --- a/config/examples/loopover.full.yml +++ b/config/examples/loopover.full.yml @@ -700,6 +700,12 @@ settings: # of this setting. Bool. Default: false. closeOwnerAuthors: false + # AI review, gate evaluation, and public-surface publish are skipped ENTIRELY for a PR/event genuinely + # triggered by a known automation bot (release-please, Renovate, Dependabot). "inherit" defers to the + # self-host operator's LOOPOVER_SKIP_AUTOMATION_BOT_PRS env default (itself default-ON); "off"/"enabled" + # override that default in either direction for this repo. inherit | off | enabled. Default: inherit. + # skipAutomationBotAuthors: inherit + # Hard manual-review path guardrails are config-as-code only. Safe by default (#3943): whatever you list # here is ADDED to a fixed, built-in invariant set (CI workflows/scripts, deploy config, config-as-code # files, core engine-decision paths — see DEFAULT_HARD_GUARDRAIL_GLOBS in src/review/guardrail-config.ts) diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index 1eeabda074..19e8954450 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -398,6 +398,7 @@ export type FocusManifestSettings = Partial< | "aiReviewAllAuthors" | "aiReviewConfirmedContributorsOnly" | "closeOwnerAuthors" + | "skipAutomationBotAuthors" | "autoLabelEnabled" | "typeLabelsEnabled" | "badgeEnabled" @@ -2219,6 +2220,11 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) if (commandRateLimitAiMaxPerWindow !== null) out.commandRateLimitAiMaxPerWindow = commandRateLimitAiMaxPerWindow; const commandRateLimitWindowHours = normalizeOptionalPositiveInteger(r.commandRateLimitWindowHours, "settings.commandRateLimitWindowHours", warnings); if (commandRateLimitWindowHours !== null) out.commandRateLimitWindowHours = commandRateLimitWindowHours; + // Automation-bot skip (#automation-bot-skip): per-repo override of the global LOOPOVER_SKIP_AUTOMATION_BOT_PRS + // default -- "inherit" defers to it, "off"/"enabled" override in either direction for this repo. Previously + // DB/dashboard-write-only despite RepositorySettings already carrying the field; this closes that gap. + const skipAutomationBotAuthors = normalizeOptionalEnum(r.skipAutomationBotAuthors, "settings.skipAutomationBotAuthors", ["inherit", "off", "enabled"] as const, warnings); + if (skipAutomationBotAuthors !== null) out.skipAutomationBotAuthors = skipAutomationBotAuthors; // Moderation-rules engine (#selfhost-mod-engine): per-repo override of the global moderation config. const moderationGateMode = normalizeOptionalEnum(r.moderationGateMode, "settings.moderationGateMode", ["inherit", "off", "enabled"] as const, warnings); if (moderationGateMode !== null) out.moderationGateMode = moderationGateMode; diff --git a/packages/loopover-engine/src/types/manifest-deps-types.ts b/packages/loopover-engine/src/types/manifest-deps-types.ts index 57f5ee0cf4..0abad01d3a 100644 --- a/packages/loopover-engine/src/types/manifest-deps-types.ts +++ b/packages/loopover-engine/src/types/manifest-deps-types.ts @@ -337,6 +337,11 @@ export type RepositorySettings = { * PRs are exempt from auto-close (merge or manual-hold only). Per-repo configurable so maintainers choose * rather than inheriting a hardwired opinion. */ closeOwnerAuthors: boolean; + /** AI review, gate evaluation, and public-surface publish are skipped entirely for a PR/event genuinely + * triggered by a known automation bot (release-please, Renovate, Dependabot). `"inherit"` defers to the + * self-host operator's `LOOPOVER_SKIP_AUTOMATION_BOT_PRS` global default (itself default-ON); `"off"`/ + * `"enabled"` override that default in either direction for this repo. */ + skipAutomationBotAuthors?: "inherit" | "off" | "enabled" | undefined; autoLabelEnabled: boolean; gittensorLabel: string; createMissingLabel: boolean; diff --git a/scripts/check-docs-drift.mjs b/scripts/check-docs-drift.mjs index 2a62945439..f43fdff636 100644 --- a/scripts/check-docs-drift.mjs +++ b/scripts/check-docs-drift.mjs @@ -90,22 +90,17 @@ export function extractRepositorySettingsFields(typesText) { } /** RepositorySettings fields deliberately excluded from the "every field must have SOME - * `.loopover.yml.example` mention" check below, for two distinct reasons -- flagging any as "undocumented" - * would be a false drift signal, not a real gap: - * - Not a maintainer-settable knob at all: `repoFullName` is the row's own identity key (set once at - * creation, the opposite of something a maintainer overrides via config); `createdAt`/`updatedAt` are - * DB-row bookkeeping timestamps. - * - `skipAutomationBotAuthors`: genuinely settable (global env default + per-repo `inherit`/`off`/`enabled` - * override, mirroring `moderationGateMode`'s shape), but DELIBERATELY not wired into the - * FocusManifest/`.loopover.yml` parsing path -- DB-only for now, confirmed as an intentional scope choice - * for this feature rather than an oversight. It is correctly absent from `FocusManifestSettings` (so the - * separate `.loopover.yml.example` field-exhaustiveness suite never expected a token for it either). */ -const NOT_YML_CONFIGURABLE_SETTINGS_FIELDS = new Set([ - "repoFullName", - "createdAt", - "updatedAt", - "skipAutomationBotAuthors", -]); + * `.loopover.yml.example` mention" check below -- flagging any as "undocumented" would be a false drift + * signal, not a real gap. Not a maintainer-settable knob at all: `repoFullName` is the row's own identity + * key (set once at creation, the opposite of something a maintainer overrides via config); `createdAt`/ + * `updatedAt` are DB-row bookkeeping timestamps. + * + * `skipAutomationBotAuthors` USED to live here too (DB-only, "confirmed as an intentional scope choice" per + * #4659 -- the same DB-only-escape-hatch shape `agentGlobalFreezeOverride` had, before that field was removed + * entirely for making the DB a second, independently-mutable source of truth alongside config). It is no + * longer DB-only: it's wired into `FocusManifestSettings`/`parseSettingsOverride` like every sibling + * `*GateMode`-shaped field, so it's held to the same yml-exhaustiveness bar as the rest of the surface now. */ +const NOT_YML_CONFIGURABLE_SETTINGS_FIELDS = new Set(["repoFullName", "createdAt", "updatedAt"]); /** RepositorySettings fields whose `.loopover.yml.example` documentation exists under a DIFFERENT, shorter * name than the field itself -- almost always because the yml groups several sibling fields under one named diff --git a/test/unit/check-docs-drift-script.test.ts b/test/unit/check-docs-drift-script.test.ts index 7200c34e70..26446fd813 100644 --- a/test/unit/check-docs-drift-script.test.ts +++ b/test/unit/check-docs-drift-script.test.ts @@ -485,19 +485,27 @@ describe("check-docs-drift script", () => { } }); - it("treats NOT_YML_CONFIGURABLE_SETTINGS_FIELDS members as excluded even with zero yml mention (repoFullName, createdAt, updatedAt, skipAutomationBotAuthors)", () => { + it("treats NOT_YML_CONFIGURABLE_SETTINGS_FIELDS members as excluded even with zero yml mention (repoFullName, createdAt, updatedAt)", () => { const files = baseFixtures(); files["src/types.ts"] = files["src/types.ts"]!.replace( "};", - " repoFullName: string;\n createdAt?: string | null | undefined;\n updatedAt?: string | null | undefined;\n skipAutomationBotAuthors?: \"inherit\" | \"off\" | \"enabled\" | undefined;\n};", + " repoFullName: string;\n createdAt?: string | null | undefined;\n updatedAt?: string | null | undefined;\n};", ); const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); - for (const field of ["repoFullName", "createdAt", "updatedAt", "skipAutomationBotAuthors"]) { + for (const field of ["repoFullName", "createdAt", "updatedAt"]) { expect(result.failures.find((failure) => failure.includes(field))).toBeUndefined(); } }); + it("no longer excludes skipAutomationBotAuthors -- it's wired into FocusManifestSettings now, so a zero-yml-mention regression must fail like any other field (#automation-bot-skip)", () => { + const files = baseFixtures(); + files["src/types.ts"] = files["src/types.ts"]!.replace("};", " skipAutomationBotAuthors?: \"inherit\" | \"off\" | \"enabled\" | undefined;\n};"); + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + expect(result.failures.find((failure) => failure.includes("skipAutomationBotAuthors"))).toBeDefined(); + }); + it("catches a FocusManifest field nested inside another config type with zero yml mention -- the exact review.visual.production_url shape (#4617)", () => { const files = baseFixtures(); // A SECOND VisualConfig-shaped leaf that the synthetic .loopover.yml.example never mentions. diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index fcba9bbfa0..48a6ab09fa 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -337,6 +337,7 @@ describe(".loopover.yml.example field-exhaustiveness (#1670)", () => { hardGuardrailGlobs: "hardGuardrailGlobs:", hardGuardrailGlobsOverridesInvariants: "hardGuardrailGlobsOverridesInvariants:", aiReviewConfirmedContributorsOnly: "aiReviewConfirmedContributorsOnly:", + skipAutomationBotAuthors: "skipAutomationBotAuthors:", manualReviewLabel: "manualReviewLabel:", readyToMergeLabel: "readyToMergeLabel:", changesRequestedLabel: "changesRequestedLabel:", @@ -2498,6 +2499,21 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(invalid.warnings.some((w) => /settings\.moderationGateMode/.test(w))).toBe(true); }); + it("parses + resolves skipAutomationBotAuthors from the settings: block, overlaying the DB (#automation-bot-skip)", () => { + const manifest = parseFocusManifest({ settings: { skipAutomationBotAuthors: "enabled" } }); + expect(manifest.settings.skipAutomationBotAuthors).toBe("enabled"); + // yml overlays (replaces) the DB-configured value. + const eff = resolveEffectiveSettings({ skipAutomationBotAuthors: "off" } as unknown as RepositorySettings, manifest); + expect(eff.skipAutomationBotAuthors).toBe("enabled"); + // Omitted in yml ⇒ the DB-configured value survives untouched. + const noOverride = resolveEffectiveSettings({ skipAutomationBotAuthors: "off" } as unknown as RepositorySettings, parseFocusManifest({})); + expect(noOverride.skipAutomationBotAuthors).toBe("off"); + // An invalid enum is dropped with a warning rather than silently coerced. + const invalid = parseFocusManifest({ settings: { skipAutomationBotAuthors: "sometimes" as never } }); + expect(invalid.settings.skipAutomationBotAuthors).toBeUndefined(); + expect(invalid.warnings.some((w) => /settings\.skipAutomationBotAuthors/.test(w))).toBe(true); + }); + it("moderationRules accepts review_evasion alongside the original three rule types (#review-evasion-protection)", () => { const manifest = parseFocusManifest({ settings: { moderationRules: ["review_evasion", "not-a-rule" as never] } }); expect(manifest.settings.moderationRules).toEqual(["review_evasion"]);