From ce8d1b578b0e32b02e5db9014cc7984f95c63b4c Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:45:19 +0000 Subject: [PATCH] fix(ci): guard RepoSettingsPreviewSchema against settings-preview drift The settings-parity check's own header names both RepositorySettingsSchema and RepoSettingsPreviewSchema as hand-authored Zod schemas needing drift protection, but main() only diffed RepositorySettingsSchema against the RepositorySettings type -- RepoSettingsPreviewSchema had no check at all. Add a second pure extraction (extractRepoSettingsPreviewFieldNames) + diff wired into main(), covering RepoSettingsPreviewSchema.settings against buildRepoSettingsPreview's return shape (the named RepoSettingsPreview type in src/signals/settings-preview.ts). It brace-matches the nested settings block so the inline commandAuthorization members don't leak in, and fails CI with the same actionable message as the existing check. This surfaced a real drift: the schema has always declared autoProjectMilestoneMatch/autoProjectMilestoneMatchBackend (since #3290) but the builder never populated them, so the endpoint never returned the two fields its own OpenAPI spec documents. Wire them into the builder's return shape (they are plain RepositorySettings fields) so the endpoint honors the spec and the new parity check passes; the schema and generated openapi.json are unchanged. Closes #7011 --- scripts/check-openapi-settings-parity.d.mts | 4 ++ scripts/check-openapi-settings-parity.mjs | 73 +++++++++++++++++--- src/signals/settings-preview.ts | 4 ++ test/unit/ci-openapi-settings-parity.test.ts | 48 ++++++++++++- 4 files changed, 118 insertions(+), 11 deletions(-) diff --git a/scripts/check-openapi-settings-parity.d.mts b/scripts/check-openapi-settings-parity.d.mts index daef8b00c2..12450bb099 100644 --- a/scripts/check-openapi-settings-parity.d.mts +++ b/scripts/check-openapi-settings-parity.d.mts @@ -1,5 +1,9 @@ export const TYPES_PATH: string; +export const SETTINGS_PREVIEW_PATH: string; + export function extractRepositorySettingsFieldNames(source: string): Set; +export function extractRepoSettingsPreviewFieldNames(source: string): Set; + export function diffFieldSets(typeFields: Set, schemaFields: Set): { missingFromSchema: string[]; extraInSchema: string[] }; diff --git a/scripts/check-openapi-settings-parity.mjs b/scripts/check-openapi-settings-parity.mjs index 0f1238dcf2..60e07ef284 100644 --- a/scripts/check-openapi-settings-parity.mjs +++ b/scripts/check-openapi-settings-parity.mjs @@ -7,11 +7,20 @@ // know about a field the spec doesn't mention. This is a structural key-set diff, not a value/type check. import { readFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; -import { RepositorySettingsSchema } from "../src/openapi/schemas.ts"; +import { RepositorySettingsSchema, RepoSettingsPreviewSchema } from "../src/openapi/schemas.ts"; export const TYPES_PATH = "src/types.ts"; const TYPE_START = "export type RepositorySettings = {"; +// #7011: RepoSettingsPreviewSchema.settings (the nested settings object of the settings-preview response) is a +// second hand-authored Zod schema this check's header names -- its source of truth is buildRepoSettingsPreview's +// return shape, whose named return type is RepoSettingsPreview (src/signals/settings-preview.ts). A field added +// to (or dropped from) that builder without a matching schema edit would otherwise drift with no CI signal, +// exactly as for RepositorySettings above. +export const SETTINGS_PREVIEW_PATH = "src/signals/settings-preview.ts"; +const PREVIEW_TYPE_START = "export type RepoSettingsPreview = {"; +const PREVIEW_SETTINGS_START = " settings: {"; + /** Pure: extract the top-level field names of the `RepositorySettings` type from raw source text. Every * field is a primitive/union/type-alias reference (never an inline nested object literal), so this never * needs to track brace depth -- verified by direct inspection of the type at the time this check was added. */ @@ -27,6 +36,33 @@ export function extractRepositorySettingsFieldNames(source) { return names; } +/** Pure: extract the field names of the nested `settings` object of the `RepoSettingsPreview` type from raw + * source text. Unlike RepositorySettings, this block nests one inline object literal (`commandAuthorization`), + * so we brace-match to bound the settings body precisely, then take only its direct children (4-space indent); + * the nested members sit deeper and never match the anchor. */ +export function extractRepoSettingsPreviewFieldNames(source) { + const typeIndex = source.indexOf(PREVIEW_TYPE_START); + if (typeIndex === -1) throw new Error(`Could not find "${PREVIEW_TYPE_START}" in the given source.`); + const blockIndex = source.indexOf(PREVIEW_SETTINGS_START, typeIndex); + if (blockIndex === -1) throw new Error(`Could not find the "settings" block of RepoSettingsPreview in the given source.`); + const openBrace = blockIndex + PREVIEW_SETTINGS_START.length - 1; + let depth = 0; + let endIndex = -1; + for (let i = openBrace; i < source.length; i++) { + if (source[i] === "{") depth++; + else if (source[i] === "}" && --depth === 0) { + endIndex = i; + break; + } + } + if (endIndex === -1) throw new Error(`Could not find the closing "}" for the RepoSettingsPreview settings block in the given source.`); + const body = source.slice(openBrace + 1, endIndex); + const fieldPattern = /^ {4}(\w+)\??:/gm; + const names = new Set(); + for (const match of body.matchAll(fieldPattern)) names.add(match[1]); + return names; +} + /** Pure: diff two field-name sets, returning the sorted asymmetric differences. */ export function diffFieldSets(typeFields, schemaFields) { return { @@ -36,22 +72,41 @@ export function diffFieldSets(typeFields, schemaFields) { } function main() { + let failed = false; + const typeFields = extractRepositorySettingsFieldNames(readFileSync(TYPES_PATH, "utf8")); const schemaFields = new Set(Object.keys(RepositorySettingsSchema.shape)); - const { missingFromSchema, extraInSchema } = diffFieldSets(typeFields, schemaFields); + const repo = diffFieldSets(typeFields, schemaFields); + if (repo.missingFromSchema.length > 0 || repo.extraInSchema.length > 0) { + if (repo.missingFromSchema.length > 0) { + console.error(`RepositorySettingsSchema (src/openapi/schemas.ts) is missing field(s) present on the RepositorySettings type: ${repo.missingFromSchema.join(", ")}`); + } + if (repo.extraInSchema.length > 0) { + console.error(`RepositorySettingsSchema (src/openapi/schemas.ts) declares field(s) not present on the RepositorySettings type: ${repo.extraInSchema.join(", ")}`); + } + console.error("Update src/openapi/schemas.ts, then run: npm run ui:openapi"); + failed = true; + } else { + console.log(`RepositorySettingsSchema matches the RepositorySettings type (${typeFields.size} fields).`); + } - if (missingFromSchema.length > 0 || extraInSchema.length > 0) { - if (missingFromSchema.length > 0) { - console.error(`RepositorySettingsSchema (src/openapi/schemas.ts) is missing field(s) present on the RepositorySettings type: ${missingFromSchema.join(", ")}`); + const previewTypeFields = extractRepoSettingsPreviewFieldNames(readFileSync(SETTINGS_PREVIEW_PATH, "utf8")); + const previewSchemaFields = new Set(Object.keys(RepoSettingsPreviewSchema.shape.settings.shape)); + const preview = diffFieldSets(previewTypeFields, previewSchemaFields); + if (preview.missingFromSchema.length > 0 || preview.extraInSchema.length > 0) { + if (preview.missingFromSchema.length > 0) { + console.error(`RepoSettingsPreviewSchema.settings (src/openapi/schemas.ts) is missing field(s) present on buildRepoSettingsPreview's return shape (RepoSettingsPreview.settings, src/signals/settings-preview.ts): ${preview.missingFromSchema.join(", ")}`); } - if (extraInSchema.length > 0) { - console.error(`RepositorySettingsSchema (src/openapi/schemas.ts) declares field(s) not present on the RepositorySettings type: ${extraInSchema.join(", ")}`); + if (preview.extraInSchema.length > 0) { + console.error(`RepoSettingsPreviewSchema.settings (src/openapi/schemas.ts) declares field(s) not present on buildRepoSettingsPreview's return shape (RepoSettingsPreview.settings, src/signals/settings-preview.ts): ${preview.extraInSchema.join(", ")}`); } console.error("Update src/openapi/schemas.ts, then run: npm run ui:openapi"); - process.exit(1); + failed = true; + } else { + console.log(`RepoSettingsPreviewSchema.settings matches buildRepoSettingsPreview's return shape (${previewTypeFields.size} fields).`); } - console.log(`RepositorySettingsSchema matches the RepositorySettings type (${typeFields.size} fields).`); + if (failed) process.exit(1); } // Guard so importing this module for its pure exports (tests) never triggers the file-read/exit side effects. diff --git a/src/signals/settings-preview.ts b/src/signals/settings-preview.ts index 680659d199..d89cbddcd0 100644 --- a/src/signals/settings-preview.ts +++ b/src/signals/settings-preview.ts @@ -234,6 +234,8 @@ export type RepoSettingsPreview = { checkRunDetailLevel: RepositorySettings["checkRunDetailLevel"]; regateSweepOrderMode: RepositorySettings["regateSweepOrderMode"]; reviewCheckMode: RepositorySettings["reviewCheckMode"]; + autoProjectMilestoneMatch: RepositorySettings["autoProjectMilestoneMatch"]; + autoProjectMilestoneMatchBackend: RepositorySettings["autoProjectMilestoneMatchBackend"]; gatePack: RepositorySettings["gatePack"]; linkedIssueGateMode: RepositorySettings["linkedIssueGateMode"]; duplicatePrGateMode: RepositorySettings["duplicatePrGateMode"]; @@ -368,6 +370,8 @@ export function buildRepoSettingsPreview(args: { checkRunDetailLevel: settings.checkRunDetailLevel, regateSweepOrderMode: settings.regateSweepOrderMode, reviewCheckMode: settings.reviewCheckMode, + autoProjectMilestoneMatch: settings.autoProjectMilestoneMatch, + autoProjectMilestoneMatchBackend: settings.autoProjectMilestoneMatchBackend, gatePack: settings.gatePack, linkedIssueGateMode: settings.linkedIssueGateMode, duplicatePrGateMode: settings.duplicatePrGateMode, diff --git a/test/unit/ci-openapi-settings-parity.test.ts b/test/unit/ci-openapi-settings-parity.test.ts index fbb9ed1a02..497834ebc7 100644 --- a/test/unit/ci-openapi-settings-parity.test.ts +++ b/test/unit/ci-openapi-settings-parity.test.ts @@ -1,7 +1,7 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -import { diffFieldSets, extractRepositorySettingsFieldNames, TYPES_PATH } from "../../scripts/check-openapi-settings-parity.mjs"; -import { RepositorySettingsSchema } from "../../src/openapi/schemas"; +import { diffFieldSets, extractRepoSettingsPreviewFieldNames, extractRepositorySettingsFieldNames, SETTINGS_PREVIEW_PATH, TYPES_PATH } from "../../scripts/check-openapi-settings-parity.mjs"; +import { RepoSettingsPreviewSchema, RepositorySettingsSchema } from "../../src/openapi/schemas"; // #2556: RepositorySettingsSchema (hand-authored Zod) can silently drift from the RepositorySettings TS // type -- this is the structural-diff guard closing that gap. ui:openapi:check only verified the generated @@ -55,3 +55,47 @@ describe("OpenAPI settings-parity check (#2556)", () => { expect(RepositorySettingsSchema.partial().parse({ contributorOpenPrCap: 100, contributorOpenIssueCap: 100 })).toMatchObject({ contributorOpenPrCap: 100, contributorOpenIssueCap: 100 }); }); }); + +// #7011: the header comment names RepoSettingsPreviewSchema too, but main() only guarded RepositorySettings. +// This block covers the added second check -- RepoSettingsPreviewSchema.settings against buildRepoSettingsPreview's +// return shape (the RepoSettingsPreview.settings type), mirroring the RepositorySettings coverage above. +describe("OpenAPI settings-preview parity check (#7011)", () => { + it("extracts only the direct field names of the nested settings block, skipping nested and sibling fields", () => { + const source = [ + "export type RepoSettingsPreview = {", + " repoFullName: string;", + " settings: {", + " publicSurface: RepositorySettings[\"publicSurface\"];", + " qualityGateMinScore?: number | null | undefined;", + " commandAuthorization: {", + " defaultAllowed: CommandAuthorizationRole[];", + " commandOverrides: Array<{ command: string; allowedRoles: CommandAuthorizationRole[] }>;", + " };", + " };", + " commandAuthorizationPreview: {", + " commandName: string;", + " };", + "};", + ].join("\n"); + const fields = extractRepoSettingsPreviewFieldNames(source); + expect(fields).toEqual(new Set(["publicSurface", "qualityGateMinScore", "commandAuthorization"])); + }); + + it("throws when the RepoSettingsPreview type start marker is missing", () => { + expect(() => extractRepoSettingsPreviewFieldNames("export type Unrelated = { settings: { a: string } };")).toThrow(/Could not find/); + }); + + it("throws when the settings block is missing from the type", () => { + expect(() => extractRepoSettingsPreviewFieldNames("export type RepoSettingsPreview = {\n repoFullName: string;\n};")).toThrow(/settings/); + }); + + it("throws when the settings block is never closed", () => { + expect(() => extractRepoSettingsPreviewFieldNames("export type RepoSettingsPreview = {\n settings: {\n publicSurface: string;")).toThrow(/closing/); + }); + + it("the real RepoSettingsPreview type and RepoSettingsPreviewSchema.settings are in parity (regression guard)", () => { + const previewFields = extractRepoSettingsPreviewFieldNames(readFileSync(SETTINGS_PREVIEW_PATH, "utf8")); + const schemaFields = new Set(Object.keys(RepoSettingsPreviewSchema.shape.settings.shape)); + expect(diffFieldSets(previewFields, schemaFields)).toEqual({ missingFromSchema: [], extraInSchema: [] }); + }); +});