diff --git a/.claude/skills/contributing-to-loopover/SKILL.md b/.claude/skills/contributing-to-loopover/SKILL.md index daae0f124e..218d6ab8fa 100644 --- a/.claude/skills/contributing-to-loopover/SKILL.md +++ b/.claude/skills/contributing-to-loopover/SKILL.md @@ -236,7 +236,7 @@ npm audit --audit-level=moderate # the dependency-review job's local eq `db:schema-drift:check`, `selfhost:env-reference:check`, `selfhost:validate-observability`, `cf-typegen:check`, `typecheck`, `test:coverage`, `test:engine-parity`, `test:live-gate-parity`, `test:driver-parity`, the `@loopover/engine` workspace's own test run, `test:workers`, `build:mcp`, `test:mcp-pack`, -`build:miner`, `test:miner-pack`, `rees:test`, `ui:openapi:check`, `ui:openapi:settings-parity`, +`build:miner`, `test:miner-pack`, `rees:test`, `ui:openapi:check`, `ui:version-audit`, `docs:drift-check`, `manifest:drift-check`, `engine-parity:drift-check`, `command-reference:check`, `ui:lint`, `ui:typecheck`, `ui:test`, `ui:build`. If any step fails, fix it and re-run — do not push a red tree. (Full per-check table in `reference.md`; check `package.json`'s diff --git a/.claude/skills/contributing-to-loopover/reference.md b/.claude/skills/contributing-to-loopover/reference.md index 7c1b60ad0a..287306d15f 100644 --- a/.claude/skills/contributing-to-loopover/reference.md +++ b/.claude/skills/contributing-to-loopover/reference.md @@ -75,7 +75,7 @@ still compiles and packs cleanly) is unchanged. |---|---|---|---| | rees → test | `validate-code` → REES build, source-map validation, and tests (L601) | `npm run rees:test` | any failing test under `review-enrichment/` | | ui → openapi drift | `validate-code` → OpenAPI drift check (L702) | `npm run ui:openapi:check` | committed `openapi.json` is stale (run `npm run ui:openapi`) | -| ui → openapi settings-parity | `validate-code` → OpenAPI settings-parity check (L708) | `npm run ui:openapi:settings-parity` | `RepositorySettingsSchema` (src/openapi/schemas.ts) is missing a field the `RepositorySettings` type has | +| ui → openapi schema/type parity | folded into `typecheck` (#9531 retired the standalone script + CI step) | `npm run typecheck` | a hand-authored Zod schema in src/openapi/schemas.ts drifted from the TS type its handler serializes — tsc names the field, see `test/unit/openapi-schema-type-parity.test.ts` | | ui → version audit | `validate-code` → UI/MCP version audit (L714) | `npm run ui:version-audit` | stale MCP version strings / non-`@latest` install copy (hits npm registry) | | docs → drift | `validate-code` → Docs drift check (L355) | `npm run docs:drift-check` | a doc makes a claim the mechanical lint can verify is now false | | docs → command-reference | `validate-code` → Command reference drift check (L342) | `npm run command-reference:check` | committed command-reference doc is stale (run `npm run command-reference`) | diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 1de5e0d65b..4db371b436 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -7062,6 +7062,15 @@ }, "summary": { "type": "string" + }, + "predictedGate": { + "$ref": "#/components/schemas/PredictedGateVerdict" + }, + "dataQuality": { + "type": "object", + "additionalProperties": { + "nullable": true + } } }, "required": [ @@ -7088,7 +7097,9 @@ "prPacket", "nextActions", "workspaceIntelligence", - "summary" + "summary", + "predictedGate", + "dataQuality" ] }, "ScorePreviewResult": { @@ -16555,6 +16566,135 @@ "type": "string" } } + }, + "PredictedGateVerdict": { + "type": "object", + "properties": { + "predicted": { + "type": "boolean", + "enum": [ + true + ] + }, + "basis": { + "type": "string", + "enum": [ + "public_config" + ] + }, + "pack": { + "type": "string", + "enum": [ + "gittensor", + "oss-anti-slop" + ] + }, + "conclusion": { + "type": "string", + "enum": [ + "success", + "failure", + "action_required", + "neutral", + "skipped" + ] + }, + "title": { + "type": "string" + }, + "summary": { + "type": "string" + }, + "readinessScore": { + "type": "number", + "nullable": true + }, + "confirmedContributor": { + "type": "boolean" + }, + "blockers": { + "type": "array", + "items": { + "type": "object", + "properties": { + "code": { + "type": "string" + }, + "title": { + "type": "string" + }, + "detail": { + "type": "string" + }, + "action": { + "type": "string" + } + }, + "required": [ + "code", + "title", + "detail" + ] + } + }, + "warnings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "code": { + "type": "string" + }, + "title": { + "type": "string" + }, + "detail": { + "type": "string" + }, + "action": { + "type": "string" + } + }, + "required": [ + "code", + "title", + "detail" + ] + } + }, + "funnel": { + "type": "object", + "nullable": true, + "properties": { + "message": { + "type": "string" + }, + "registerUrl": { + "type": "string" + } + }, + "required": [ + "message", + "registerUrl" + ] + }, + "note": { + "type": "string" + } + }, + "required": [ + "predicted", + "basis", + "pack", + "conclusion", + "title", + "summary", + "readinessScore", + "blockers", + "warnings", + "funnel", + "note" + ] } }, "parameters": {}, diff --git a/packages/loopover-engine/src/predicted-gate.ts b/packages/loopover-engine/src/predicted-gate.ts index 5c6713f06f..eb7a084116 100644 --- a/packages/loopover-engine/src/predicted-gate.ts +++ b/packages/loopover-engine/src/predicted-gate.ts @@ -53,7 +53,11 @@ export type PredictedGateVerdict = { title: string; summary: string; readinessScore: number | null; - confirmedContributor: boolean | undefined; + /** Deliberately `undefined` under the `oss-anti-slop` pack, where confirmed status carries no meaning. + * Optional rather than required-but-undefined (#9531): the value is serialized straight into the + * branch-analysis response, so `JSON.stringify` drops the key outright for those repos and a client + * reading the published spec sees an absent field, not a null one. */ + confirmedContributor?: boolean | undefined; blockers: Array<{ code: string; title: string; detail: string; action?: string | undefined }>; warnings: Array<{ code: string; title: string; detail: string; action?: string | undefined }>; /** Opt-in conversion funnel (#694): present only under the `oss-anti-slop` pack — a non-Gittensor diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 9e3e4154d2..e3f34258b8 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -3152,6 +3152,36 @@ export const LocalWorkspaceIntelligenceSchema = z }) .openapi("LocalWorkspaceIntelligence"); +/** The pre-submission gate prediction `POST /v1/local/branch-analysis` returns (#9517), authored here so + * {@link LocalBranchAnalysisSchema} can finally declare the `predictedGate` field its route has always + * emitted (#9531). Held in exact parity with `PredictedGateVerdict` (packages/loopover-engine/src/predicted-gate.ts) + * by the compile-time assertion in test/unit/openapi-settings-schema-parity.test.ts. */ +const PredictedGateFindingSchema = z.object({ + code: z.string(), + title: z.string(), + detail: z.string(), + action: z.string().optional(), +}); + +export const PredictedGateVerdictSchema = z + .object({ + predicted: z.literal(true), + basis: z.literal("public_config"), + pack: z.enum(["gittensor", "oss-anti-slop"]), + conclusion: z.enum(["success", "failure", "action_required", "neutral", "skipped"]), + title: z.string(), + summary: z.string(), + readinessScore: z.number().nullable(), + // Absent, not null, under the `oss-anti-slop` pack -- see the field's doc comment on PredictedGateVerdict. + confirmedContributor: z.boolean().optional(), + blockers: z.array(PredictedGateFindingSchema), + warnings: z.array(PredictedGateFindingSchema), + /** Present only under the `oss-anti-slop` pack (#694); `null` under `gittensor`. */ + funnel: z.object({ message: z.string(), registerUrl: z.string() }).nullable(), + note: z.string(), + }) + .openapi("PredictedGateVerdict"); + export const LocalBranchAnalysisSchema = z .object({ login: z.string(), @@ -3255,6 +3285,12 @@ export const LocalBranchAnalysisSchema = z nextActions: z.array(RewardRiskActionSchema), workspaceIntelligence: LocalWorkspaceIntelligenceSchema, summary: z.string(), + // #9531: the route returns `{ ...analysis, predictedGate, dataQuality }` (src/api/routes.ts's + // POST /v1/local/branch-analysis) and this schema declared neither -- the published-spec lie #9531 + // called out by name. `dataQuality` uses the same opaque record shape every other schema here models + // it with; `predictedGate` gets a real schema, pinned to its type by a compile-time assertion. + predictedGate: PredictedGateVerdictSchema, + dataQuality: z.record(z.string(), z.unknown()), }) .openapi("LocalBranchAnalysis"); diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 25e4798440..c2c08cb898 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -96,6 +96,7 @@ import { PullRequestMaintainerPacketSchema, PullRequestReviewIntelligenceSchema, PullRequestReviewabilitySchema, + PredictedGateVerdictSchema, PreflightResultSchema, PublicRepoStatsSchema, PublicQualityMetricsSchema, @@ -201,6 +202,7 @@ export function buildOpenApiSpec() { registry.register("PreflightResult", PreflightResultSchema); registry.register("LocalDiffPreflightResult", LocalDiffPreflightResultSchema); registry.register("ReviewRiskExplanation", ReviewRiskExplanationSchema); + registry.register("PredictedGateVerdict", PredictedGateVerdictSchema); registry.register("LocalBranchAnalysis", LocalBranchAnalysisSchema); registry.register("MaintainerPacket", MaintainerPacketSchema); registry.register("MaintainerLaneReport", MaintainerLaneReportSchema); diff --git a/test/unit/openapi-settings-schema-parity.test.ts b/test/unit/openapi-schema-type-parity.test.ts similarity index 58% rename from test/unit/openapi-settings-schema-parity.test.ts rename to test/unit/openapi-schema-type-parity.test.ts index 6cdd475abb..31d4997dbc 100644 --- a/test/unit/openapi-settings-schema-parity.test.ts +++ b/test/unit/openapi-schema-type-parity.test.ts @@ -1,22 +1,26 @@ import { describe, expect, it } from "vitest"; import type { z } from "zod"; import { defaultRepositorySettings } from "../../src/db/repositories"; -import { RepoSettingsPreviewSchema, RepositorySettingsSchema } from "../../src/openapi/schemas"; +import { LocalBranchAnalysisSchema, PredictedGateVerdictSchema, RepoSettingsPreviewSchema, RepositorySettingsSchema } from "../../src/openapi/schemas"; +import type { PredictedGateVerdict } from "../../src/rules/predicted-gate"; import type { RepoSettingsPreview } from "../../src/signals/settings-preview"; import type { RepositorySettings } from "../../src/types"; -// #9531: RepositorySettingsSchema/RepoSettingsPreviewSchema (src/openapi/schemas.ts) are hand-authored Zod -// schemas, and ui:openapi:check only verifies the generated openapi.json matches THEM -- never that they -// match the TS types the handlers actually serialize. scripts/check-openapi-settings-parity.ts (#2556/#7011) -// closed that gap by regex-diffing the two KEY SETS out of the raw source text; the assertions below replace -// it and subsume it, because the compiler compares what a key-set diff structurally cannot: each field's -// optionality, nullability, and value type, in BOTH directions. Retiring the script also retires its own -// blind spots -- it could only ever see top-level names, and it re-parsed src/types.ts by brace-and-indent -// heuristics that any reformatting would have quietly broken. +// #9531: the schemas in src/openapi/schemas.ts are hand-authored Zod, and ui:openapi:check only verifies the +// generated openapi.json matches THEM -- never that they match the TS types the handlers actually serialize. +// scripts/check-openapi-settings-parity.ts (#2556/#7011) closed that gap for two of them by regex-diffing +// KEY SETS out of the raw source text; the assertions below replace it and subsume it, because the compiler +// compares what a key-set diff structurally cannot: each field's optionality, nullability, and value type, +// in BOTH directions. Retiring the script also retires its own blind spots -- it could only ever see +// top-level names, and it re-parsed src/types.ts by brace-and-indent heuristics that any reformatting would +// have quietly broken. // -// Three published-spec defects it had been unable to see, all found the moment these assertions compiled: -// the dead `autonomy` levels (#4620), `contributorBlacklist.githubId` (#9125), and `moderationRules`' -// missing `copycat` member -- see each field's own comment in src/openapi/schemas.ts. +// Six published-spec defects it had been unable to see, five found the moment these assertions compiled and +// one (`predictedGate`) named in advance by #9531 itself -- see each field's own comment in schemas.ts: +// the dead `autonomy` levels (#4620), the missing `contributorBlacklist.githubId` (#9125), `moderationRules`' +// missing `copycat` member (#1969), `expectedCiContexts`' undeclared null, the settings-preview's +// unreachable `aiReviewConfirmedContributorsOnly` null, and LocalBranchAnalysis' undeclared +// `predictedGate`/`dataQuality`. /** Exact type equality (not mutual assignability): the deferred-conditional identity trick, which compares * the two types structurally rather than checking each is assignable to the other -- the latter treats @@ -43,16 +47,20 @@ type AssertNoSchemaDrift<_Drifted extends never> = true; type RepositorySettingsParity = AssertNoSchemaDrift, RepositorySettings>>; /** GET /v1/repos/:owner/:repo/settings-preview serializes buildRepoSettingsPreview's RepoSettingsPreview. */ type RepoSettingsPreviewParity = AssertNoSchemaDrift, RepoSettingsPreview>>; +/** POST /v1/local/branch-analysis spreads buildPredictedGateVerdict's result in as `predictedGate`. */ +type PredictedGateVerdictParity = AssertNoSchemaDrift, PredictedGateVerdict>>; -// The two aliases are checked when tsc INSTANTIATES them, which only happens where they are referenced -- -// hence this anchor, rather than two dangling type declarations a later cleanup would read as dead. -const schemaParity: [RepositorySettingsParity, RepoSettingsPreviewParity] = [true, true]; +// The aliases are checked when tsc INSTANTIATES them, which only happens where they are referenced -- +// hence this anchor, rather than dangling type declarations a later cleanup would read as dead. +const schemaParity: [RepositorySettingsParity, RepoSettingsPreviewParity, PredictedGateVerdictParity] = [true, true, true]; -describe("RepositorySettingsSchema parity with the RepositorySettings type (#9531)", () => { - it("compiles the schema-vs-type drift assertions", () => { - expect(schemaParity).toEqual([true, true]); +describe("schema/type drift assertions (#9531)", () => { + it("compiles for every schema pinned above", () => { + expect(schemaParity).toEqual([true, true, true]); }); +}); +describe("RepositorySettingsSchema parity with the RepositorySettings type (#9531)", () => { // The assertions above are erased at runtime, so this pins the other half of the contract: that the schema // ACCEPTS a real payload. A field the schema requires but no read path actually populates would compile // fine (the type says required) and still reject every generated client -- only parsing catches that. @@ -101,6 +109,50 @@ describe("RepositorySettingsSchema parity with the RepositorySettings type (#953 }); }); +// #9531 named this one in advance: POST /v1/local/branch-analysis returns `{ ...analysis, predictedGate, +// dataQuality }`, and LocalBranchAnalysisSchema declared neither of the last two. The key-set diff could not +// have caught it -- it only ever looked at RepositorySettings and RepoSettingsPreview. +describe("LocalBranchAnalysisSchema declares the fields its route actually returns (#9531)", () => { + // The gittensor-pack shape: confirmed status is meaningful, and the conversion funnel is null. + const verdict: PredictedGateVerdict = { + predicted: true, + basis: "public_config", + pack: "gittensor", + conclusion: "action_required", + title: "Predicted: needs work", + summary: "One blocker, one warning.", + readinessScore: 42, + confirmedContributor: false, + blockers: [{ code: "missing_linked_issue", title: "No linked issue", detail: "Link an open issue.", action: "Add `Closes #123`." }], + warnings: [{ code: "size", title: "Large PR", detail: "18 files changed." }], + funnel: null, + note: "Predicted from public config.", + }; + + it("accepts a full predicted-gate verdict, per-finding action included", () => { + const parsed = PredictedGateVerdictSchema.safeParse(verdict); + expect(parsed.error?.issues ?? []).toEqual([]); + }); + + // Under oss-anti-slop, buildPredictedGateVerdict forces confirmedContributor to undefined, so the key is + // absent from the serialized response entirely -- and the funnel is present instead of null (#694). + it("accepts the oss-anti-slop shape: funnel present, confirmed status absent, no readiness score", () => { + const { confirmedContributor: _absent, ...ossAntiSlop } = verdict; + const parsed = PredictedGateVerdictSchema.safeParse({ ...ossAntiSlop, pack: "oss-anti-slop", readinessScore: null, funnel: { message: "Earn on Gittensor", registerUrl: "https://example.invalid/register" } }); + expect(parsed.error?.issues ?? []).toEqual([]); + }); + + it("rejects a verdict claiming a basis other than the public config it is predicted from", () => { + expect(PredictedGateVerdictSchema.safeParse({ ...verdict, basis: "private_settings" }).success).toBe(false); + expect(PredictedGateVerdictSchema.safeParse({ ...verdict, predicted: false }).success).toBe(false); + }); + + it("carries predictedGate and dataQuality on the branch-analysis response schema", () => { + expect(Object.keys(LocalBranchAnalysisSchema.shape)).toEqual(expect.arrayContaining(["predictedGate", "dataQuality"])); + expect(LocalBranchAnalysisSchema.shape.predictedGate.safeParse(verdict).success).toBe(true); + }); +}); + describe("defaultRepositorySettings (#9531)", () => { it("carries the caller's repo name and the built-in config-as-code defaults", () => { const settings = defaultRepositorySettings("acme/widgets");