From 85a4e480a859e35e0f33d3172ed4ce56bfa81676 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 30 Jul 2026 10:35:06 -0700 Subject: [PATCH] fix(contract): restate MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES so the generated schemas compile main is currently failing `contract:api-schemas:check` for every PR. #9738 added `.max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES)` to the settings schema in src/openapi/schemas.ts. gen-contract-api-schemas.ts copies that schema verbatim into @loopover/contract, which cannot import the Worker's src/ -- so a referenced constant has to be restated in the contract's own limits.ts. That step was missed, leaving both paths broken: regenerating emits a file referencing a name it never imports (TS2304, contract build fails), and NOT regenerating leaves the drift check red. The generator's doc calls this "the loud failure this wants", and it worked -- it just needs the constant it was asking for. Also pins the three restated Worker bounds against their originals. limits.ts says they are "pinned against their originals like every other entry here", but PREFLIGHT_LIMITS was the only group with a meta-test doing that; the single constants were restated on trust. Nothing at compile time relates the two copies (that is the whole reason the contract is a zod-only leaf), so a one-sided edit would surface as a client-side validation error rather than a build failure. The compile-time failure only catches a MISSING constant, never a drifted VALUE. --- packages/loopover-contract/src/api-schemas.ts | 4 +- packages/loopover-contract/src/limits.ts | 2 + test/unit/contract-limits-pinned.test.ts | 46 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/unit/contract-limits-pinned.test.ts diff --git a/packages/loopover-contract/src/api-schemas.ts b/packages/loopover-contract/src/api-schemas.ts index db2abf7660..39122d833d 100644 --- a/packages/loopover-contract/src/api-schemas.ts +++ b/packages/loopover-contract/src/api-schemas.ts @@ -10,7 +10,7 @@ import { z } from "zod"; import { checkBeforeStartSchema, slopRiskSchema, validateFocusManifestSchema, validateLinkedIssueSchema } from "./api-requests.js"; import { AGENT_ACTION_CLASSES, AUTONOMY_LEVELS } from "./enums.js"; -import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP, MAX_REVIEW_NAG_COOLDOWN_DAYS } from "./limits.js"; +import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP, MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES, MAX_REVIEW_NAG_COOLDOWN_DAYS } from "./limits.js"; export const FindingSchema = z .object({ @@ -465,6 +465,8 @@ export const RepositorySettingsSchema = z gateDryRun: z.boolean().optional(), premergeContentRecheck: z.boolean().optional(), requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(), + // #9738: non-negative, not positive -- 0 is the documented way to turn the window off. + priorityEligibilityWindowMinutes: z.number().int().min(0).max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES).nullable().optional(), staleBaseAheadByThreshold: z.number().int().positive().nullable().optional(), mergeReadinessGateMode: z.enum(["off", "advisory", "block"]), manifestPolicyGateMode: z.enum(["off", "advisory", "block"]), diff --git a/packages/loopover-contract/src/limits.ts b/packages/loopover-contract/src/limits.ts index 64e87df132..6c9ec4b1e7 100644 --- a/packages/loopover-contract/src/limits.ts +++ b/packages/loopover-contract/src/limits.ts @@ -121,3 +121,5 @@ export const PUBLIC_SURFACE_SKIP_REASONS = [ export const MAX_CONTRIBUTOR_OPEN_ITEM_CAP = 100; /** src/settings/agent-actions.ts -- keeps the review-nag lookback from overflowing Date arithmetic. */ export const MAX_REVIEW_NAG_COOLDOWN_DAYS = 365; +/** src/review/priority-eligibility-window.ts (#9738) -- one day, the longest an eligibility hold may last. */ +export const MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES = 24 * 60; diff --git a/test/unit/contract-limits-pinned.test.ts b/test/unit/contract-limits-pinned.test.ts new file mode 100644 index 0000000000..aeb6547ec1 --- /dev/null +++ b/test/unit/contract-limits-pinned.test.ts @@ -0,0 +1,46 @@ +// The Worker-side bounds that @loopover/contract restates must equal their originals (#9773 follow-up). +// +// WHY THIS EXISTS. limits.ts says these entries are "pinned against their originals like every other entry +// here" -- but PREFLIGHT_LIMITS was the only group with a meta-test actually doing the pinning. The three +// single constants were restated on trust, which is the same thing as not being pinned. +// +// The failure this catches is quiet and one-sided: the contract package cannot import the Worker's `src/` +// (it is a zod-only leaf, which is the property every other surface depends on), so nothing at compile time +// relates the two copies. Raise a bound on the Worker side alone and the published schema keeps rejecting +// input the server would now accept; lower it alone and the schema accepts input the server then rejects or +// truncates. Either way the mismatch surfaces as a confusing client-side validation error rather than as a +// build failure. +// +// This is not a hypothetical. gen-contract-api-schemas.ts copies these schemas verbatim, so a copied schema +// referencing a constant that has NOT been restated here emits a file that does not compile -- the +// generator's own doc calls that "the loud failure this wants". #9738 added +// `.max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES)` to the settings schema without adding the constant, which +// left `main` failing `contract:api-schemas:check` for every PR: regenerating produced an uncompilable file, +// and not regenerating left the check red. The constant is now restated; this test is what keeps the VALUES +// together from here on, which the compile-time failure alone never did. +import { describe, expect, it } from "vitest"; + +import { + MAX_CONTRIBUTOR_OPEN_ITEM_CAP as CONTRACT_MAX_CONTRIBUTOR_OPEN_ITEM_CAP, + MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES as CONTRACT_MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES, + MAX_REVIEW_NAG_COOLDOWN_DAYS as CONTRACT_MAX_REVIEW_NAG_COOLDOWN_DAYS, +} from "../../packages/loopover-contract/src/limits"; +import { MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../../src/settings/agent-actions"; +import { MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES } from "../../src/review/priority-eligibility-window"; +import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP } from "../../src/types"; + +describe("@loopover/contract restated Worker bounds stay pinned to their originals", () => { + // One case per constant rather than a table, so a failure names the specific bound that drifted and the + // file it has to be reconciled with. + it("MAX_CONTRIBUTOR_OPEN_ITEM_CAP matches src/types.ts", () => { + expect(CONTRACT_MAX_CONTRIBUTOR_OPEN_ITEM_CAP).toBe(MAX_CONTRIBUTOR_OPEN_ITEM_CAP); + }); + + it("MAX_REVIEW_NAG_COOLDOWN_DAYS matches src/settings/agent-actions.ts", () => { + expect(CONTRACT_MAX_REVIEW_NAG_COOLDOWN_DAYS).toBe(MAX_REVIEW_NAG_COOLDOWN_DAYS); + }); + + it("MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES matches src/review/priority-eligibility-window.ts", () => { + expect(CONTRACT_MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES).toBe(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES); + }); +});