diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index e44afb3e7c..b401fdd288 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -9085,13 +9085,15 @@ "type": "integer", "nullable": true, "minimum": 0, - "exclusiveMinimum": true + "exclusiveMinimum": true, + "maximum": 100 }, "contributorOpenIssueCap": { "type": "integer", "nullable": true, "minimum": 0, - "exclusiveMinimum": true + "exclusiveMinimum": true, + "maximum": 100 }, "contributorCapLabel": { "type": "string", diff --git a/packages/gittensory-engine/src/focus-manifest.ts b/packages/gittensory-engine/src/focus-manifest.ts index 915a2ad831..1e2b0a351e 100644 --- a/packages/gittensory-engine/src/focus-manifest.ts +++ b/packages/gittensory-engine/src/focus-manifest.ts @@ -1273,6 +1273,14 @@ function normalizeOptionalPositiveInteger(value: JsonValue | undefined, field: s return null; } +const MAX_CONTRIBUTOR_OPEN_ITEM_CAP = 100; + +function normalizeOptionalContributorOpenItemCap(value: JsonValue | undefined, field: string, warnings: string[]): number | null { + const parsed = normalizeOptionalPositiveInteger(value, field, warnings); + if (parsed === null) return null; + return Math.min(parsed, MAX_CONTRIBUTOR_OPEN_ITEM_CAP); +} + const REVIEW_VISUAL_MAX_ROUTES_LIMIT = 5; function normalizeOptionalVisualMaxRoutes(value: JsonValue | undefined, warnings: string[]): number | null { @@ -1664,8 +1672,9 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) if (entries.length > 0) out.contributorBlacklist = entries; } // Per-contributor open PR/issue caps (#2270): discrete counts, not scores — reuse the same positive-integer - // normalizer as contentLane.maxAppendedEntries so a fractional/non-positive typo is dropped with a warning - // instead of configuring a nonsensical cap. UNLIKE contributorBlacklist above, an explicit yml `null` here is + // shape as contentLane.maxAppendedEntries so a fractional/non-positive typo is dropped with a warning + // instead of configuring a nonsensical cap. Valid counts clamp to the fixed live-verification budget. UNLIKE + // contributorBlacklist above, an explicit yml `null` here is // load-bearing (not the same as omitting the key): the documented `yml > DB > null` precedence means a // maintainer must be able to force a DB-configured cap back to "no cap" via `.gittensory.yml` without deleting // the DB row. `normalizeOptionalPositiveInteger` collapses "absent" and "null" to the same silent `null` @@ -1675,13 +1684,13 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) if (r.contributorOpenPrCap === null) { out.contributorOpenPrCap = null; } else { - const contributorOpenPrCap = normalizeOptionalPositiveInteger(r.contributorOpenPrCap, "settings.contributorOpenPrCap", warnings); + const contributorOpenPrCap = normalizeOptionalContributorOpenItemCap(r.contributorOpenPrCap, "settings.contributorOpenPrCap", warnings); if (contributorOpenPrCap !== null) out.contributorOpenPrCap = contributorOpenPrCap; } if (r.contributorOpenIssueCap === null) { out.contributorOpenIssueCap = null; } else { - const contributorOpenIssueCap = normalizeOptionalPositiveInteger(r.contributorOpenIssueCap, "settings.contributorOpenIssueCap", warnings); + const contributorOpenIssueCap = normalizeOptionalContributorOpenItemCap(r.contributorOpenIssueCap, "settings.contributorOpenIssueCap", warnings); if (contributorOpenIssueCap !== null) out.contributorOpenIssueCap = contributorOpenIssueCap; } // #label-scoping: same load-bearing-null idiom as blacklistLabel above. diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 7fed01f327..cf928e42b3 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -61,6 +61,7 @@ import { webhookEvents, } from "./schema"; import { DEFAULT_REVIEW_EVASION_LABEL, MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../settings/agent-actions"; +import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP } from "../types"; import type { Advisory, AdvisoryFinding, @@ -6757,12 +6758,12 @@ function normalizeQualityGateMinScore(value: number | null | undefined): number } // A per-contributor open-item cap (#2270) counts discrete open PRs/issues, not a 0-100 score, so unlike -// normalizeQualityGateMinScore it is neither clamped into a range nor rounded — a fractional or non-positive -// value is a malformed cap (there's no such thing as "allow 2.5 open PRs"), so it is dropped to null (no cap) -// rather than silently coerced into a nonsensical threshold. +// normalizeQualityGateMinScore it is not rounded — a fractional or non-positive value is a malformed cap +// (there's no such thing as "allow 2.5 open PRs"), so it is dropped to null (no cap). Valid counts are +// clamped to the fixed live-verification sample budget so the cap cannot exceed the rows enforcement sees. function normalizeOpenItemCap(value: number | null | undefined): number | null { if (typeof value !== "number" || !Number.isFinite(value) || !Number.isInteger(value) || value <= 0) return null; - return value; + return Math.min(value, MAX_CONTRIBUTOR_OPEN_ITEM_CAP); } function parsePublicSurface(value: string): RepositorySettings["publicSurface"] { diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index b362c265aa..472c8e20c3 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../settings/agent-actions"; +import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP } from "../types"; import { extendZodWithOpenApi } from "@asteasolutions/zod-to-openapi"; extendZodWithOpenApi(z); @@ -733,8 +734,8 @@ export const RepositorySettingsSchema = z autoMaintain: z.object({ requireApprovals: z.number().int(), mergeMethod: z.enum(["merge", "squash", "rebase"]) }).optional(), agentPaused: z.boolean().optional(), agentDryRun: z.boolean().optional(), - contributorOpenPrCap: z.number().int().positive().nullable().optional(), - contributorOpenIssueCap: z.number().int().positive().nullable().optional(), + contributorOpenPrCap: z.number().int().positive().max(MAX_CONTRIBUTOR_OPEN_ITEM_CAP).nullable().optional(), + contributorOpenIssueCap: z.number().int().positive().max(MAX_CONTRIBUTOR_OPEN_ITEM_CAP).nullable().optional(), contributorCapLabel: z.string().nullable().optional(), contributorCapCancelCi: z.boolean().nullable().optional(), reviewNagPolicy: z.enum(["off", "hold", "close"]).optional(), diff --git a/src/types.ts b/src/types.ts index aef8d21366..8d41b04ee9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -622,6 +622,8 @@ export type CombineStrategy = "single" | "consensus" | "synthesis"; * {@link CombineStrategy} for why the canonical definition lives here rather than `services/ai-review.ts`. */ export type OnMerge = "either" | "both"; +export const MAX_CONTRIBUTOR_OPEN_ITEM_CAP = 100; + export type RepositorySettings = { repoFullName: string; commentMode: "off" | "detected_contributors_only" | "all_prs"; @@ -850,11 +852,12 @@ export type RepositorySettings = { blacklistLabel?: string | null | undefined; /** Per-contributor open-PR cap (#2270, anti-abuse): the max PRs a single non-owner/admin/bot contributor may * have open on this repo at once. `null`/absent (default) = no cap, byte-identical to today. Layered like - * every other settings field (`.gittensory.yml` `settings.contributorOpenPrCap` > DB > `null`). Enforcement - * (closing the newest PR(s) over the cap) is a separate follow-up; this field only carries the threshold. */ + * every other settings field (`.gittensory.yml` `settings.contributorOpenPrCap` > DB > `null`). Capped at + * {@link MAX_CONTRIBUTOR_OPEN_ITEM_CAP} so the fixed live-verification sample can enforce the threshold. */ contributorOpenPrCap?: number | null | undefined; /** Per-contributor open-issue cap (#2270, anti-abuse): same shape and precedence as {@link contributorOpenPrCap}, - * applied to open issues instead of open PRs. `null`/absent (default) = no cap. */ + * applied to open issues instead of open PRs. `null`/absent (default) = no cap. Also capped at + * {@link MAX_CONTRIBUTOR_OPEN_ITEM_CAP}. */ contributorOpenIssueCap?: number | null | undefined; /** The label applied to a PR/issue closed for exceeding a per-contributor open-item cap (#2270). Same * configurable-with-fallback shape as {@link blacklistLabel} (including the explicit-`null`-closes-without-a- diff --git a/test/unit/ci-openapi-settings-parity.test.ts b/test/unit/ci-openapi-settings-parity.test.ts index 314ecab314..fbb9ed1a02 100644 --- a/test/unit/ci-openapi-settings-parity.test.ts +++ b/test/unit/ci-openapi-settings-parity.test.ts @@ -49,4 +49,9 @@ describe("OpenAPI settings-parity check (#2556)", () => { const schemaFields = new Set(Object.keys(RepositorySettingsSchema.shape)); expect(diffFieldSets(typeFields, schemaFields)).toEqual({ missingFromSchema: [], extraInSchema: [] }); }); + it("rejects contributor open caps above the enforcement sample budget", () => { + expect(() => RepositorySettingsSchema.partial().parse({ contributorOpenPrCap: 101 })).toThrow(); + expect(() => RepositorySettingsSchema.partial().parse({ contributorOpenIssueCap: 101 })).toThrow(); + expect(RepositorySettingsSchema.partial().parse({ contributorOpenPrCap: 100, contributorOpenIssueCap: 100 })).toMatchObject({ contributorOpenPrCap: 100, contributorOpenIssueCap: 100 }); + }); }); diff --git a/test/unit/data-spine.test.ts b/test/unit/data-spine.test.ts index 1777791413..39dd4ac6c8 100644 --- a/test/unit/data-spine.test.ts +++ b/test/unit/data-spine.test.ts @@ -312,6 +312,8 @@ describe("data spine repositories", () => { expect(await getRepositorySettings(env, "owner/caprepo")).toMatchObject({ contributorOpenPrCap: 2, contributorOpenIssueCap: 5 }); await upsertRepositorySettings(env, { repoFullName: "owner/caprepo", contributorOpenPrCap: 3, contributorOpenIssueCap: null }); expect(await getRepositorySettings(env, "owner/caprepo")).toMatchObject({ contributorOpenPrCap: 3, contributorOpenIssueCap: null }); // update persists + can clear + await upsertRepositorySettings(env, { repoFullName: "owner/caprepo", contributorOpenPrCap: 101, contributorOpenIssueCap: 150 }); + expect(await getRepositorySettings(env, "owner/caprepo")).toMatchObject({ contributorOpenPrCap: 100, contributorOpenIssueCap: 100 }); // clamps to the live-check sample budget // A cap must be a positive whole number: fractional, non-positive, and non-finite values are all // dropped to null rather than silently coerced (there's no such thing as "allow 2.5 open PRs"). await upsertRepositorySettings(env, { repoFullName: "owner/badcaprepo", contributorOpenPrCap: 2.5 as never }); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 7cc3b04f82..3d50643b24 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1905,8 +1905,12 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = const noOverride = resolveEffectiveSettings({ contributorOpenPrCap: 4, contributorOpenIssueCap: null } as unknown as RepositorySettings, parseFocusManifest({})); expect(noOverride.contributorOpenPrCap).toBe(4); expect(noOverride.contributorOpenIssueCap).toBeNull(); - // A cap is a discrete count, not a 0-100 score: fractional, non-positive, and non-numeric values are all - // dropped with a warning rather than silently coerced or clamped into range. + // A cap is a discrete count, not a score: over-budget valid integers clamp to the fixed enforcement + // sample, while fractional, non-positive, and non-numeric values are dropped with a warning. + const overBudget = parseFocusManifest({ settings: { contributorOpenPrCap: 101, contributorOpenIssueCap: 150 } }); + expect(overBudget.settings.contributorOpenPrCap).toBe(100); + expect(overBudget.settings.contributorOpenIssueCap).toBe(100); + const invalid = parseFocusManifest({ settings: { contributorOpenPrCap: 2.5, contributorOpenIssueCap: 0 } }); expect(invalid.settings.contributorOpenPrCap).toBeUndefined(); expect(invalid.settings.contributorOpenIssueCap).toBeUndefined();