refactor(openapi): replace the settings-parity script with a compile-time schema/type assertion (#9531) - #9585
Merged
Conversation
…time schema/type assertion (#9531) 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.
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-28 19:49:05 UTC
Review summary Nits — 7 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | f9f2a63 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 07:28 PM |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9585 +/- ##
==========================================
- Coverage 89.83% 89.83% -0.01%
==========================================
Files 875 878 +3
Lines 110958 111011 +53
Branches 26413 26421 +8
==========================================
+ Hits 99680 99727 +47
Misses 9992 9992
- Partials 1286 1292 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Bundle ReportChanges will increase total bundle size by 120.84kB (1.58%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/check-openapi-settings-parity.ts(fix(openapi): add 11 missing RepositorySettings fields + a structural-drift CI check #2556/check-openapi-settings-parity.mjs only checks RepositorySettingsSchema, not RepoSettingsPreviewSchema despite its own header naming both #7011) — the last hand-rolled parity checker for the settings schemas — and replaces it with a compile-time assertion that subsumes it. This is the#9531deliverable of the same name; the route↔spec ratchet itself landed in feat(api): drive the route-spec ratchet to zero and delete the baseline (#9531) #9581.RepositorySettingsSchema/RepoSettingsPreviewSchemaagainst the two TS types by re-parsingsrc/types.tsandsrc/signals/settings-preview.tswith brace-and-indent regex 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.tsasserts exact type equality in both directions between each schema'sz.infer<>and the type the handler actually serializes. The comparison is per key (throughPick, so optionality is part of it), so a failure still names the drifted field the way the script's error message did — verified by injecting a drift and reading tsc's output.npm run typecheckis now the enforcement point, and it coverstest/**already.Published-spec defects the key-set diff was structurally unable to see
All five turned up the moment the assertion compiled. Every one of them is a spec-vs-payload mismatch a generated client (including
@loopover/mcp) would have been wrong about:autonomy"suggest"/"propose", removed fromAutonomyLevelby #4620 (both were behaviorally identical toobservefrom day one) and dropped on persist ever sincecontributorBlacklist[].githubIdmoderationRules"copycat", a realModerationRuleTypesince #1969expectedCiContexts| nulland read as a distinct value from absent (focus-manifest.ts's!== nullgate), but the schema never saidnullable— unlike its identically-shaped siblingadvisoryCheckRunssettings-preview.aiReviewConfirmedContributorsOnlynullthatbuildRepoSettingsPreview's?? falsemakes unreachableMaking the types state what the read paths already guarantee
Reaching parity meant closing the gap between what the settings types declare and what both
getRepositorySettingspaths always return:typeLabelsEnabled,linkedIssueLabelPropagation,linkedIssueHardRules,commandAuthorization,contributorBlacklist,blacklistLabel,autonomy,moderationGateMode,fairnessAnalyticsMode,reviewEvasionProtection,draftPrClosePolicy,synchronizeClosePolicy,screenshotTableGate) become required onRepositorySettings— adjudicated one at a time against both DB read paths, which have always populated all 13.getRepositorySettings' inline branch intodefaultRepositorySettings(fullName), so there is one home for them; the 13 test fixtures that hand-maintained private copies of those defaults now spread it instead, which is how ten of them came to silently omit the same fields.upsertRepositorySettingsechoeslinkedIssueHardRules/synchronizeClosePolicy, so PUT's response matches GET's shape for the same repo — it did not before.resolveEffectiveSettingsnow skips undefined-valued manifest keys before the spread.manifest.settingsis aPartial<>, so an explicitly-undefinedkey was indistinguishable from an absent one to the type system but not to the spread, which overwrote the resolved DB value withundefined. "Not overridden" is what an absent value means at every other layer of that resolver.RepoSettingsPreview.autoProjectMilestoneMatch{,Backend}become optional rather than required-but-possibly-undefined: both are copied straight from settings, soJSON.stringifydrops the key and the response genuinely omits the field.Closes #9531.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm audit --audit-level=moderatereports 11 pre-existing high-severity advisories from thewrangler→miniflaretransitive chain underpackages/discovery-index. This PR changes no dependency and does not touchpackage-lock.json, so the result is identical onmain.build:mcp/test:mcp-packwere not run: the diff touches nopackages/loopover-mcp/**source. The MCP client consumes the generatedopenapi.json, whose regeneration is verified byui:openapi:checkabove.npm run test:coverage, then the changed lines were checked againstcoverage/lcov.infodirectly: 0 uncovered and 0 partially-covered changed lines insrc/db/repositories.tsandsrc/signals/focus-manifest.ts(the only changed files with executable statements — the rest are Zod declarations and type-only edits). The fulltest/unit+test/contractrun is green on this base: 23,574 passed, 6 skipped.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — no visible UI, frontend, or docs change. The only
apps/loopover-ui/**file touched is the generatedpublic/openapi.json.Notes
RepositorySettingsSchemais a response schema only (spec.tsregisters it and uses it for the200onGET /v1/repos/:owner/:repo/settings), so promoting fields from optional to required tightens what the spec promises about a payload without tightening any request body..readonly()markers added toexpectedCiContexts/advisoryCheckRuns(matching theirReadonlyArraydeclarations) are erased in the emitted JSON Schema — they add noreadOnly: true, as theopenapi.jsondiff shows.npm run ui:openapi:settings-parityand itstest:ci/ci.ymlentries are removed along with the script. Four comments elsewhere that cited the script as the local precedent fortsx-over-nodeor for source-of-truth diffing are updated rather than left pointing at a deleted file.