Skip to content

Commit ea86691

Browse files
authored
refactor(openapi): replace the settings-parity script with a compile-time schema/type assertion (#9531) (#9585)
scripts/check-openapi-settings-parity.ts diffed the top-level KEY SETS of RepositorySettingsSchema/RepoSettingsPreviewSchema against the two TS types by re-parsing src/types.ts and src/signals/settings-preview.ts with brace-and-indent heuristics. It could not see a field's optionality, nullability, or value type, and any reformatting of either type would have silently broken its extraction. test/unit/openapi-settings-schema-parity.test.ts replaces it with an exact-equality type assertion in both directions, compared per key so a failure still names the drifted field. Three published-spec defects the key-set diff had been structurally unable to see turned up the moment it compiled: - autonomy advertised "suggest"/"propose", removed from AutonomyLevel by #4620 and dropped on persist ever since; - contributorBlacklist omitted the immutable githubId the API has returned since #9125; - moderationRules omitted "copycat", a real ModerationRuleType since #1969. Also: expectedCiContexts is declared and read as nullable but the schema never said so, and the settings-preview's aiReviewConfirmedContributorsOnly advertised a null the builder's `?? false` makes unreachable. Reaching parity required the types to state what the read paths already guarantee. The thirteen config-as-code fields both getRepositorySettings paths always populate become required; the no-row defaults move into defaultRepositorySettings() so there is one home for them, and upsertRepositorySettings echoes linkedIssueHardRules/synchronizeClosePolicy so PUT's response matches GET's shape. resolveEffectiveSettings now skips undefined-valued manifest keys before the spread, so config-as-code can no longer punch a hole in a resolved DB value -- "not overridden" is what an absent value means at every other layer.
1 parent 643383c commit ea86691

29 files changed

Lines changed: 329 additions & 305 deletions

.github/workflows/ci.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -790,12 +790,6 @@ jobs:
790790
- name: OpenAPI drift check
791791
if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.uiContract == 'true' }}
792792
run: npm run ui:openapi:check
793-
# #2556: RepositorySettingsSchema (hand-authored Zod) can silently drift from the actual
794-
# RepositorySettings TS type -- the OpenAPI drift check above only verifies the generated spec
795-
# matches the Zod schema, never that the schema matches the type the API actually serializes.
796-
- name: OpenAPI settings-parity check
797-
if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.uiContract == 'true' }}
798-
run: npm run ui:openapi:settings-parity
799793
# Checks apps/loopover-ui/src' known-latest MCP version string against the published package, so
800794
# its dependency is `ui` (the file it scans) + `mcp` (the package it checks against) -- NOT the
801795
# OpenAPI contract, which this script never reads.

apps/loopover-ui/public/openapi.json

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9206,7 +9206,8 @@
92069206
"type": "array",
92079207
"items": {
92089208
"type": "string"
9209-
}
9209+
},
9210+
"nullable": true
92109211
},
92119212
"advisoryCheckRuns": {
92129213
"type": "array",
@@ -9690,6 +9691,9 @@
96909691
},
96919692
"addedAt": {
96929693
"type": "string"
9694+
},
9695+
"githubId": {
9696+
"type": "integer"
96939697
}
96949698
},
96959699
"required": [
@@ -9704,8 +9708,6 @@
97049708
"type": "string",
97059709
"enum": [
97069710
"observe",
9707-
"suggest",
9708-
"propose",
97099711
"auto_with_approval",
97109712
"auto"
97119713
]
@@ -9714,8 +9716,6 @@
97149716
"type": "string",
97159717
"enum": [
97169718
"observe",
9717-
"suggest",
9718-
"propose",
97199719
"auto_with_approval",
97209720
"auto"
97219721
]
@@ -9724,8 +9724,6 @@
97249724
"type": "string",
97259725
"enum": [
97269726
"observe",
9727-
"suggest",
9728-
"propose",
97299727
"auto_with_approval",
97309728
"auto"
97319729
]
@@ -9734,8 +9732,6 @@
97349732
"type": "string",
97359733
"enum": [
97369734
"observe",
9737-
"suggest",
9738-
"propose",
97399735
"auto_with_approval",
97409736
"auto"
97419737
]
@@ -9744,8 +9740,6 @@
97449740
"type": "string",
97459741
"enum": [
97469742
"observe",
9747-
"suggest",
9748-
"propose",
97499743
"auto_with_approval",
97509744
"auto"
97519745
]
@@ -9754,8 +9748,6 @@
97549748
"type": "string",
97559749
"enum": [
97569750
"observe",
9757-
"suggest",
9758-
"propose",
97599751
"auto_with_approval",
97609752
"auto"
97619753
]
@@ -9764,8 +9756,6 @@
97649756
"type": "string",
97659757
"enum": [
97669758
"observe",
9767-
"suggest",
9768-
"propose",
97699759
"auto_with_approval",
97709760
"auto"
97719761
]
@@ -9774,8 +9764,6 @@
97749764
"type": "string",
97759765
"enum": [
97769766
"observe",
9777-
"suggest",
9778-
"propose",
97799767
"auto_with_approval",
97809768
"auto"
97819769
]
@@ -9784,8 +9772,6 @@
97849772
"type": "string",
97859773
"enum": [
97869774
"observe",
9787-
"suggest",
9788-
"propose",
97899775
"auto_with_approval",
97909776
"auto"
97919777
]
@@ -9954,7 +9940,8 @@
99549940
"contributor_cap",
99559941
"blacklist",
99569942
"review_nag",
9957-
"review_evasion"
9943+
"review_evasion",
9944+
"copycat"
99589945
]
99599946
}
99609947
},
@@ -10133,14 +10120,24 @@
1013310120
"closeOwnerAuthors",
1013410121
"autoLabelEnabled",
1013510122
"typeLabelsEnabled",
10123+
"linkedIssueLabelPropagation",
10124+
"linkedIssueHardRules",
1013610125
"gittensorLabel",
1013710126
"blacklistLabel",
1013810127
"createMissingLabel",
1013910128
"publicSurface",
1014010129
"includeMaintainerAuthors",
1014110130
"requireLinkedIssue",
1014210131
"backfillEnabled",
10143-
"commandAuthorization"
10132+
"commandAuthorization",
10133+
"contributorBlacklist",
10134+
"autonomy",
10135+
"moderationGateMode",
10136+
"fairnessAnalyticsMode",
10137+
"reviewEvasionProtection",
10138+
"draftPrClosePolicy",
10139+
"synchronizeClosePolicy",
10140+
"screenshotTableGate"
1014410141
]
1014510142
},
1014610143
"AutomationState": {
@@ -10991,8 +10988,7 @@
1099110988
"type": "boolean"
1099210989
},
1099310990
"aiReviewConfirmedContributorsOnly": {
10994-
"type": "boolean",
10995-
"nullable": true
10991+
"type": "boolean"
1099610992
},
1099710993
"commandAuthorization": {
1099810994
"type": "object",

package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
"ui:test": "npm run ui:kit:build && npm --workspace @loopover/ui run test && npm --workspace @loopover/ui-miner run test",
6464
"ui:openapi": "tsx scripts/write-ui-openapi.ts",
6565
"ui:openapi:check": "tsx scripts/write-ui-openapi.ts --check",
66-
"ui:openapi:settings-parity": "tsx scripts/check-openapi-settings-parity.ts",
6766
"cloudflare:schema": "tsx scripts/write-cloudflare-schema.ts",
6867
"ui:version-audit": "node --experimental-strip-types scripts/check-ui-mcp-version-copy.ts",
6968
"ui:version-audit:sync": "node --experimental-strip-types scripts/check-ui-mcp-version-copy.ts --write",
@@ -122,7 +121,7 @@
122121
"test:smoke:browser:install": "playwright install chromium",
123122
"test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts",
124123
"pretest:ci": "npm run check-node-version",
125-
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
124+
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
126125
"test:release": "npm run test:ci && npm run changelog:check",
127126
"test:release:mcp": "npm run test:ci",
128127
"test:watch": "vitest",

scripts/check-openapi-settings-parity.ts

Lines changed: 0 additions & 113 deletions
This file was deleted.

scripts/check-schema-drift.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
// drizzle-orm's getTableColumns (keyed by each column's .name -- the actual DB column name, not the JS
1616
// property name). Diff the two column-name sets per table.
1717
//
18-
// Run via `tsx` (not plain `node`) for the same reason as check-migrations.ts and
19-
// check-openapi-settings-parity.ts: this script imports src/db/schema.ts (a .ts module) directly, and a bare
18+
// Run via `tsx` (not plain `node`) for the same reason as check-migrations.ts: this script imports
19+
// src/db/schema.ts (a .ts module) directly, and a bare
2020
// `node` invocation can't resolve a `.ts` import without an experimental flag CI's pinned Node isn't
2121
// guaranteed to support.
2222
import { readdirSync, readFileSync } from "node:fs";

src/db/repositories.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -712,11 +712,16 @@ export async function listRepositories(env: Env): Promise<RepositoryRecord[]> {
712712
return rows.map(toRepositoryRecord);
713713
}
714714

715-
export async function getRepositorySettings(env: Env, fullName: string): Promise<RepositorySettings> {
716-
const db = getDb(env.DB);
717-
const [row] = await db.select().from(repositorySettings).where(eq(repositorySettings.repoFullName, fullName)).limit(1);
718-
if (!row) {
719-
return {
715+
/**
716+
* The complete built-in default settings for a repo LoopOver has no row for (#9531).
717+
*
718+
* Extracted from getRepositorySettings' no-row branch so it has ONE home: the read path returns it,
719+
* and the test fixtures that build full RepositorySettings literals spread it instead of each
720+
* hand-maintaining a private copy of every config-as-code default -- which is how ten of them came
721+
* to silently omit the same thirteen fields.
722+
*/
723+
export function defaultRepositorySettings(fullName: string): RepositorySettings {
724+
return {
720725
repoFullName: fullName,
721726
commentMode: "detected_contributors_only",
722727
publicAudienceMode: "oss_maintainer",
@@ -805,7 +810,13 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
805810
mergeTrainMode: "off",
806811
screenshotTableGate: { ...DEFAULT_SCREENSHOT_TABLE_GATE, whenLabels: [], whenPaths: [], requireViewports: [], requireThemes: [] },
807812
};
808-
}
813+
}
814+
815+
816+
export async function getRepositorySettings(env: Env, fullName: string): Promise<RepositorySettings> {
817+
const db = getDb(env.DB);
818+
const [row] = await db.select().from(repositorySettings).where(eq(repositorySettings.repoFullName, fullName)).limit(1);
819+
if (!row) return defaultRepositorySettings(fullName);
809820
return {
810821
repoFullName: row.repoFullName,
811822
// Config-as-code only (Batch A, loopover#6442): no DB column backs these 9 fields anymore -- the
@@ -1055,6 +1066,11 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
10551066
draftPrClosePolicy: "off" as const, // #6440/#draft-pr-close-policy: config-as-code only now -- any caller-supplied value is a silent no-op, configure via .loopover.yml settings.draftPrClosePolicy instead
10561067
mergeTrainMode: "off" as const,
10571068
screenshotTableGate: normalizeScreenshotTableGateConfig(settings.screenshotTableGate, []),
1069+
// #9531: both GET paths always return these two config-as-code-only fields, so RepositorySettings
1070+
// declares them required and this write-path echo must carry the same built-in defaults the read
1071+
// paths do -- omitting them made PUT's response a different shape from GET's for the same repo.
1072+
linkedIssueHardRules: { ...DEFAULT_LINKED_ISSUE_HARD_RULES, pointBearingLabels: [], maintainerOnlyLabels: [] },
1073+
synchronizeClosePolicy: "off" as const,
10581074
} satisfies RepositorySettings;
10591075
const db = getDb(env.DB);
10601076
await db

0 commit comments

Comments
 (0)