diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 2a4121a79..30f4df2b5 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -22079,6 +22079,9 @@ "202": { "description": "Job queued" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -22102,6 +22105,9 @@ "202": { "description": "Job queued" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -22125,6 +22131,9 @@ "202": { "description": "Job queued" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -22212,7 +22221,7 @@ }, "/v1/internal/jobs/build-contributor-evidence": { "post": { - "summary": "Queue a contributor evidence build job", + "summary": "Queue a job to build contributor evidence", "responses": { "202": { "description": "Job queued" @@ -22221,9 +22230,10 @@ "description": "Invalid internal token" } }, - "operationId": "postInternalJobsBuildContributorEvidence", + "operationId": "queueBuildContributorEvidenceJob", "tags": [ - "Internal" + "Internal", + "Jobs" ], "security": [ { @@ -22257,7 +22267,7 @@ }, "/v1/internal/jobs/build-burden-forecasts": { "post": { - "summary": "Queue a burden forecast build job", + "summary": "Queue a job to build burden forecasts", "responses": { "202": { "description": "Job queued" @@ -22266,9 +22276,10 @@ "description": "Invalid internal token" } }, - "operationId": "postInternalJobsBuildBurdenForecasts", + "operationId": "queueBuildBurdenForecastsJob", "tags": [ - "Internal" + "Internal", + "Jobs" ], "security": [ { @@ -22325,7 +22336,7 @@ }, "/v1/internal/jobs/repair-data-fidelity": { "post": { - "summary": "Queue a data fidelity repair job", + "summary": "Queue a job to repair data fidelity", "responses": { "202": { "description": "Job queued" @@ -22334,9 +22345,10 @@ "description": "Invalid internal token" } }, - "operationId": "postInternalJobsRepairDataFidelity", + "operationId": "queueRepairDataFidelityJob", "tags": [ - "Internal" + "Internal", + "Jobs" ], "security": [ { @@ -25026,6 +25038,9 @@ "200": { "description": "Job ran inline and returned its result" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -25049,6 +25064,9 @@ "202": { "description": "Job queued" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -25072,6 +25090,9 @@ "200": { "description": "Job ran inline and returned its result" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -25141,6 +25162,9 @@ "200": { "description": "Job ran inline and returned its result" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -25187,6 +25211,9 @@ "200": { "description": "Job ran inline and returned its result" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -25210,6 +25237,9 @@ "200": { "description": "Job ran inline and returned its result" }, + "400": { + "description": "Malformed job request" + }, "401": { "description": "Invalid internal token" } @@ -25315,21 +25345,21 @@ "Internal", "Jobs" ], - "summary": "Run the job to index repository content for retrieval", + "summary": "Queue a job to index repository content for retrieval", "security": [ { "LoopOverBearer": [] } ], "responses": { - "200": { - "description": "Job ran inline and returned its result" - }, - "400": { - "description": "Malformed job request" + "202": { + "description": "Job queued" }, "401": { "description": "Invalid internal token" + }, + "404": { + "description": "Retrieval is not enabled on this deployment" } } } @@ -25341,21 +25371,24 @@ "Internal", "Jobs" ], - "summary": "Run the job to re-run the gate for one pull request", + "summary": "Queue a job to re-run the gate for one pull request", "security": [ { "LoopOverBearer": [] } ], "responses": { - "200": { - "description": "Job ran inline and returned its result" + "202": { + "description": "Job queued" }, "400": { "description": "Malformed job request" }, "401": { "description": "Invalid internal token" + }, + "404": { + "description": "The repository is not installed" } } } diff --git a/src/openapi/internal-and-public-route-specs.ts b/src/openapi/internal-and-public-route-specs.ts index b35d50f0f..8fc1d4f77 100644 --- a/src/openapi/internal-and-public-route-specs.ts +++ b/src/openapi/internal-and-public-route-specs.ts @@ -30,28 +30,60 @@ type SpecEntry = { const INTERNAL_AUTH = { 401: { description: "Invalid internal token" } }; const QUEUED = { 202: { description: "Job queued" }, ...INTERNAL_AUTH }; const RAN = { 200: { description: "Job ran inline and returned its result" }, ...INTERNAL_AUTH }; +/** Attached PER ENTRY, never folded into QUEUED/RAN: only some job routes validate a body, and widening + * the shared constants would attach a 400 to every route that happily accepts any body (#9706). */ +const MALFORMED = { 400: { description: "Malformed job request" } }; -/** `[path segment, operationId stem, human summary]` for the jobs that have BOTH forms. */ -const JOB_PAIRS: ReadonlyArray = [ - ["refresh-registry", "RefreshRegistry", "refresh the Gittensor registry snapshot"], - ["refresh-scoring-model", "RefreshScoringModel", "refresh the active scoring model"], - ["refresh-upstream-drift", "RefreshUpstreamDrift", "recompute upstream ruleset drift"], - ["file-upstream-drift-issues", "FileUpstreamDriftIssues", "file issues for open upstream drift"], - ["build-contributor-decision-packs", "BuildContributorDecisionPacks", "rebuild contributor decision packs"], - ["refresh-contributor-activity", "RefreshContributorActivity", "refresh cached contributor activity"], - ["generate-signal-snapshots", "GenerateSignalSnapshots", "generate signal snapshots"], - ["generate-weekly-value-report", "GenerateWeeklyValueReport", "generate the weekly value report"], - ["generate-review-recap", "GenerateReviewRecap", "generate the maintainer review recap"], - ["backfill-registered-repos", "BackfillRegisteredRepos", "backfill registered repository records"], - ["backfill-repo-segment", "BackfillRepoSegment", "backfill repository segment assignments"], - ["backfill-pr-details", "BackfillPrDetails", "backfill pull-request detail rows"], - ["rollup-product-usage", "RollupProductUsage", "roll up product usage counters"], +/** + * A job that exists in BOTH forms: a bare POST that ENQUEUES onto the durable queue, and a `/run` sibling + * that executes inline. `queueValidates`/`runValidates` say which form rejects a malformed body -- the two + * are genuinely independent (build-contributor-decision-packs validates `login` only on `/run`), so a + * single flag would have published a 400 the bare form never returns. + */ +type JobPair = { segment: string; stem: string; summary: string; queueValidates?: boolean; runValidates?: boolean }; + +const JOB_PAIRS: readonly JobPair[] = [ + { segment: "refresh-registry", stem: "RefreshRegistry", summary: "refresh the Gittensor registry snapshot" }, + { segment: "refresh-scoring-model", stem: "RefreshScoringModel", summary: "refresh the active scoring model" }, + { segment: "refresh-upstream-drift", stem: "RefreshUpstreamDrift", summary: "recompute upstream ruleset drift" }, + { segment: "file-upstream-drift-issues", stem: "FileUpstreamDriftIssues", summary: "file issues for open upstream drift" }, + { segment: "build-contributor-decision-packs", stem: "BuildContributorDecisionPacks", summary: "rebuild contributor decision packs", runValidates: true }, + { segment: "refresh-contributor-activity", stem: "RefreshContributorActivity", summary: "refresh cached contributor activity", queueValidates: true, runValidates: true }, + { segment: "generate-signal-snapshots", stem: "GenerateSignalSnapshots", summary: "generate signal snapshots" }, + { segment: "generate-weekly-value-report", stem: "GenerateWeeklyValueReport", summary: "generate the weekly value report" }, + { segment: "generate-review-recap", stem: "GenerateReviewRecap", summary: "generate the maintainer review recap", queueValidates: true, runValidates: true }, + { segment: "backfill-registered-repos", stem: "BackfillRegisteredRepos", summary: "backfill registered repository records" }, + { segment: "backfill-repo-segment", stem: "BackfillRepoSegment", summary: "backfill repository segment assignments", queueValidates: true, runValidates: true }, + { segment: "backfill-pr-details", stem: "BackfillPrDetails", summary: "backfill pull-request detail rows", queueValidates: true, runValidates: true }, + { segment: "rollup-product-usage", stem: "RollupProductUsage", summary: "roll up product usage counters" }, ]; -/** Jobs that exist only in the bare (enqueue-or-run) form. */ -const SINGLE_JOBS: ReadonlyArray = [ - ["rag-index", "RunRagIndex", "index repository content for retrieval"], - ["regate-pr", "RegatePullRequest", "re-run the gate for one pull request"], +/** + * Jobs that exist only in the bare form. + * + * Every one of them ENQUEUES and answers 202 -- so the summary reads "Queue a job to", and the success + * status is 202 rather than the 200 this table used to publish, which was unreachable for all five. + * Responses vary per entry because the handlers do: rag-index 404s when retrieval is disabled and never + * validates a body at all (an unparseable one becomes `{}`), while regate-pr does both. + */ +type SingleJob = { segment: string; operationId: string; summary: string; responses: Record }; + +const SINGLE_JOBS: readonly SingleJob[] = [ + { + segment: "rag-index", + operationId: "RunRagIndex", + summary: "index repository content for retrieval", + responses: { ...QUEUED, 404: { description: "Retrieval is not enabled on this deployment" } }, + }, + { + segment: "regate-pr", + operationId: "RegatePullRequest", + summary: "re-run the gate for one pull request", + responses: { ...QUEUED, ...MALFORMED, 404: { description: "The repository is not installed" } }, + }, + { segment: "build-contributor-evidence", operationId: "queueBuildContributorEvidenceJob", summary: "build contributor evidence", responses: QUEUED }, + { segment: "build-burden-forecasts", operationId: "queueBuildBurdenForecastsJob", summary: "build burden forecasts", responses: QUEUED }, + { segment: "repair-data-fidelity", operationId: "queueRepairDataFidelityJob", summary: "repair data fidelity", responses: QUEUED }, ]; /** @@ -69,7 +101,7 @@ const RUN_ONLY_JOBS: ReadonlyArray = [ function jobRoutes(): SpecEntry[] { const entries: SpecEntry[] = []; - for (const [segment, stem, summary] of JOB_PAIRS) { + for (const { segment, stem, summary, queueValidates, runValidates } of JOB_PAIRS) { entries.push({ method: "post", path: `/v1/internal/jobs/${segment}`, @@ -77,7 +109,7 @@ function jobRoutes(): SpecEntry[] { tags: ["Internal", "Jobs"], summary: `Queue a job to ${summary}`, auth: "internal", - responses: QUEUED, + responses: queueValidates ? { ...QUEUED, ...MALFORMED } : QUEUED, }); entries.push({ method: "post", @@ -86,7 +118,7 @@ function jobRoutes(): SpecEntry[] { tags: ["Internal", "Jobs"], summary: `Run the job to ${summary} inline, bypassing the queue`, auth: "internal", - responses: RAN, + responses: runValidates ? { ...RAN, ...MALFORMED } : RAN, }); } for (const [segment, stem, summary] of RUN_ONLY_JOBS) { @@ -100,15 +132,15 @@ function jobRoutes(): SpecEntry[] { responses: RAN, }); } - for (const [segment, operationId, summary] of SINGLE_JOBS) { + for (const { segment, operationId, summary, responses } of SINGLE_JOBS) { entries.push({ method: "post", path: `/v1/internal/jobs/${segment}`, operationId, tags: ["Internal", "Jobs"], - summary: `Run the job to ${summary}`, + summary: `Queue a job to ${summary}`, auth: "internal", - responses: { ...RAN, 400: { description: "Malformed job request" } }, + responses, }); } return entries; @@ -546,33 +578,6 @@ const SILENT_401: SpecEntry[] = [ auth: "public", responses: AUTH_FLOW_RESPONSES, }, - { - method: "post", - path: "/v1/internal/jobs/build-contributor-evidence", - operationId: "postInternalJobsBuildContributorEvidence", - tags: ["Internal"], - summary: "Queue a contributor evidence build job", - auth: "internal", - responses: QUEUED, - }, - { - method: "post", - path: "/v1/internal/jobs/build-burden-forecasts", - operationId: "postInternalJobsBuildBurdenForecasts", - tags: ["Internal"], - summary: "Queue a burden forecast build job", - auth: "internal", - responses: QUEUED, - }, - { - method: "post", - path: "/v1/internal/jobs/repair-data-fidelity", - operationId: "postInternalJobsRepairDataFidelity", - tags: ["Internal"], - summary: "Queue a data fidelity repair job", - auth: "internal", - responses: QUEUED, - }, ]; export function registerInternalAndPublicRouteSpecs(registry: OpenAPIRegistry): void { diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index a790dc65f..89dec4d96 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -165,6 +165,17 @@ function loopOperationMeta(method: string, path: string, tag: string): { operati return { operationId: `${method}${stem}`, tags: [tag] }; } +/** + * Every module that contributes route specs, as ONE list (#9706). + * + * Exported so the duplicate-registration guard consumes the same list buildOpenApiSpec does. The guard + * exists because the ratchet compares SETS and is therefore blind to a path registered twice -- both + * directions of its diff still balance, while zod-to-openapi silently keeps the last registration. A guard + * that had to restate this list would go quiet the moment a new registrar was added, which is the rot the + * anti-rot guards in this repo keep finding. + */ +export const SPEC_REGISTRARS: ReadonlyArray<(registry: OpenAPIRegistry) => void> = [registerOrbAndControlRouteSpecs, registerInternalAndPublicRouteSpecs]; + export function buildOpenApiSpec() { const registry = new OpenAPIRegistry(); registry.register("Health", HealthSchema); @@ -2207,92 +2218,11 @@ export function buildOpenApiSpec() { }, }); } - // Instance subscription-CLI credentials (#9543). The response NEVER carries the credential itself -- - // only configured/last4/timestamps -- so the whole surface is safe to describe publicly. - registry.registerPath({ - method: "post", - path: "/v1/internal/jobs/refresh-registry", - operationId: "postInternalJobsRefreshRegistry", - tags: ["Internal"], - summary: "Queue a registry refresh job", - responses: { - 202: { description: "Registry refresh queued" }, - 401: { description: "Invalid internal token" }, - }, - }); - registry.registerPath({ - method: "post", - path: "/v1/internal/jobs/backfill-registered-repos", - operationId: "postInternalJobsBackfillRegisteredRepos", - tags: ["Internal"], - summary: "Queue a registered-repository backfill job", - responses: { - 202: { description: "Registered repo backfill queued" }, - 401: { description: "Invalid internal token" }, - }, - }); - registry.registerPath({ - method: "post", - path: "/v1/internal/jobs/backfill-repo-segment", - operationId: "postInternalJobsBackfillRepoSegment", - tags: ["Internal"], - summary: "Queue a repository segment backfill job", - responses: { - 202: { description: "Repository segment backfill queued" }, - 400: { description: "Invalid segment request" }, - 401: { description: "Invalid internal token" }, - }, - }); - registry.registerPath({ - method: "post", - path: "/v1/internal/jobs/backfill-pr-details", - operationId: "postInternalJobsBackfillPrDetails", - tags: ["Internal"], - summary: "Queue an open pull request detail backfill job", - responses: { - 202: { description: "Open PR detail backfill queued" }, - 400: { description: "Invalid PR detail backfill request" }, - 401: { description: "Invalid internal token" }, - }, - }); - registry.registerPath({ - method: "post", - path: "/v1/internal/jobs/generate-review-recap", - operationId: "postInternalJobsGenerateReviewRecap", - tags: ["Internal"], - summary: "Queue a maintainer review recap digest job", - responses: { - 202: { description: "Maintainer review recap digest queued (#1963)" }, - 400: { description: "Missing repoFullName" }, - 401: { description: "Invalid internal token" }, - }, - }); - for (const [path, summary] of [ - ["/v1/internal/jobs/refresh-scoring-model", "Queue a scoring model refresh job"], - ["/v1/internal/jobs/refresh-upstream-drift", "Queue an upstream drift refresh job"], - ["/v1/internal/jobs/file-upstream-drift-issues", "Queue a job that files upstream drift issues"], - ["/v1/internal/jobs/build-contributor-decision-packs", "Queue a contributor decision pack build job"], - ["/v1/internal/jobs/generate-signal-snapshots", "Queue a signal snapshot generation job"], - ["/v1/internal/jobs/generate-weekly-value-report", "Queue a weekly value report job"], - ] as const) { - registry.registerPath({ - method: "post", - path, - ...loopOperationMeta("post", path, "Internal"), - summary, - responses: { - 202: { description: "Internal job queued" }, - 401: { description: "Invalid internal token" }, - }, - }); - } - // #9531: the ORB ingress, the control-panel app surface, and the per-repo key/settings routes. // Registered from their own module rather than inline here because each entry declares an auth // level that DERIVES its security stanza, instead of having one bolted on afterwards by // applySecurityMetadata's path-prefix guesswork. - registerOrbAndControlRouteSpecs(registry); - registerInternalAndPublicRouteSpecs(registry); + for (const register of SPEC_REGISTRARS) register(registry); const generator = new OpenApiGeneratorV3(registry.definitions); const document = generator.generateDocument({ diff --git a/test/unit/route-spec-ratchet.test.ts b/test/unit/route-spec-ratchet.test.ts index ddca09389..a41a1cf2b 100644 --- a/test/unit/route-spec-ratchet.test.ts +++ b/test/unit/route-spec-ratchet.test.ts @@ -13,7 +13,8 @@ // exception, which is the point -- a new unspecced route fails immediately. import { describe, expect, it } from "vitest"; import { createApp } from "../../src/api/routes"; -import { buildOpenApiSpec } from "../../src/openapi/spec"; +import { SPEC_REGISTRARS, buildOpenApiSpec } from "../../src/openapi/spec"; +import { OpenAPIRegistry } from "@asteasolutions/zod-to-openapi"; import { diffRoutesAgainstSpec, isNonOperationRoute, @@ -85,3 +86,89 @@ describe("route↔spec ratchet", () => { expect(listSpecRouteKeys(buildOpenApiSpec() as never).length).toBeGreaterThan(200); }); }); + +// #9706: the ratchet compares SETS, so it is structurally blind to a path registered twice -- both +// directions of its diff still balance. @asteasolutions/zod-to-openapi silently keeps the last +// registration, which is how `POST /v1/internal/jobs/backfill-pr-details` came to publish one table's +// operation while a second, more accurate one (declaring the 400 the handler really returns) was +// discarded with nothing anywhere reporting it. +describe("no operation is registered twice (#9706)", () => { + it("contributes each method+path exactly once", () => { + const registry = new OpenAPIRegistry(); + for (const register of SPEC_REGISTRARS) register(registry); + + const seen = new Map(); + for (const definition of registry.definitions) { + if (definition.type !== "route") continue; + const key = `${definition.route.method.toUpperCase()} ${definition.route.path}`; + seen.set(key, (seen.get(key) ?? 0) + 1); + } + expect([...seen].filter(([, count]) => count > 1).map(([key]) => key)).toEqual([]); + }); + + it("declares every /v1/internal/ operation as bearer-gated", () => { + // requiresApiToken returns false for this family -- it has its own middleware -- so the legacy + // fill-in left any entry that did not declare `auth` with no stanza at all. + const paths = buildOpenApiSpec().paths as Record>; + const wrong: string[] = []; + for (const [path, item] of Object.entries(paths)) { + if (!path.startsWith("/v1/internal/")) continue; + for (const method of ["get", "post", "put", "patch", "delete"] as const) { + const operation = item[method]; + if (!operation) continue; + if (JSON.stringify(operation.security) !== JSON.stringify([{ LoopOverBearer: [] }])) wrong.push(`${method.toUpperCase()} ${path}`); + } + } + expect(wrong).toEqual([]); + }); +}); + +// #9706: the published status has to be the one the handler returns. Every job route enqueues and +// answers 202; the SINGLE_JOBS table published a 200 that was unreachable for all five of them. +describe("job operations publish what their handlers actually return (#9706)", () => { + const jobOperation = (path: string) => (buildOpenApiSpec().paths as Record; summary?: string } }>)[path]?.post; + + it.each(["rag-index", "regate-pr", "build-contributor-evidence", "build-burden-forecasts", "repair-data-fidelity"])( + "%s is queued (202), not run (200)", + (segment) => { + const operation = jobOperation(`/v1/internal/jobs/${segment}`); + expect(Object.keys(operation!.responses!)).toContain("202"); + expect(Object.keys(operation!.responses!)).not.toContain("200"); + expect(operation!.summary).toMatch(/^Queue a job to /); + }, + ); + + it("rag-index declares a 404 and NO 400 — an unparseable body becomes {}, it is never rejected", () => { + const responses = Object.keys(jobOperation("/v1/internal/jobs/rag-index")!.responses!); + expect(responses).toContain("404"); + expect(responses).not.toContain("400"); + }); + + it("regate-pr declares both — it validates its body AND 404s an uninstalled repo", () => { + const responses = Object.keys(jobOperation("/v1/internal/jobs/regate-pr")!.responses!); + expect(responses).toEqual(expect.arrayContaining(["400", "404"])); + }); + + it.each([ + "backfill-repo-segment", + "backfill-repo-segment/run", + "backfill-pr-details", + "backfill-pr-details/run", + "build-contributor-decision-packs/run", + "refresh-contributor-activity", + "refresh-contributor-activity/run", + "generate-review-recap", + "generate-review-recap/run", + ])("%s validates its body, so it declares a 400", (segment) => { + expect(Object.keys(jobOperation(`/v1/internal/jobs/${segment}`)!.responses!)).toContain("400"); + }); + + it.each(["refresh-registry", "build-contributor-decision-packs", "generate-signal-snapshots", "rollup-product-usage"])( + "%s accepts any body, so it must NOT claim a 400", + (segment) => { + // The other arm of the per-entry override, and the reason it is per-entry: widening the shared + // QUEUED constant would have attached a 400 to every one of these. + expect(Object.keys(jobOperation(`/v1/internal/jobs/${segment}`)!.responses!)).not.toContain("400"); + }, + ); +});