From 4511caacabb5fad7a19bea4ba8f877fac829ee78 Mon Sep 17 00:00:00 2001 From: shin-core <153108882+shin-core@users.noreply.github.com> Date: Wed, 29 Jul 2026 14:48:36 +0900 Subject: [PATCH] fix(api): declare 404 and 503 for the public README badge routes #8377 gave GET /v1/public/repos/:owner/:repo/badge.svg and .../badge.json a deliberate three-outcome split -- 404 (no public badge for this repo), 503 (the badge data could not be loaded, short-cached), 200 otherwise -- so a monitor can tell a repo with no badge apart from a transient backend failure. But the spec entries added in #9531 declared only 200, so the published contract these README-embedded routes are read against was missing exactly the two statuses the handler comment says a monitor must distinguish. Add 404 and 503 to both getRepoBadgeSvg and getRepoBadgeJson (their descriptions matching the handler's own distinction) and regenerate openapi.json. No handler behaviour, status, or cache header changes. Adds tests pinning which branch produces which status: the no-badge 404 and the loader-failure 503 each carry Cache-Control: public, max-age=300 and render the exact renderUnavailableBadgeSvg() body (svg) / unavailable shields payload (json), plus a buildOpenApiSpec() assertion that both operations declare 200/404/503. Closes #9710 --- apps/loopover-ui/public/openapi.json | 12 ++++++++++++ src/openapi/internal-and-public-route-specs.ts | 12 ++++++++++-- test/integration/api.test.ts | 12 ++++++++++-- test/unit/openapi.test.ts | 8 ++++++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index a84890ed11..ee022dac06 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -25897,6 +25897,12 @@ "responses": { "200": { "description": "Badge payload" + }, + "404": { + "description": "The repo has no public badge (unknown, private, uninstalled, or badgeEnabled off)" + }, + "503": { + "description": "The badge data could not be loaded (a transient loader failure, short-cached)" } }, "security": [] @@ -25930,6 +25936,12 @@ "responses": { "200": { "description": "SVG badge" + }, + "404": { + "description": "The repo has no public badge (unknown, private, uninstalled, or badgeEnabled off)" + }, + "503": { + "description": "The badge data could not be loaded (a transient loader failure, short-cached)" } }, "security": [] diff --git a/src/openapi/internal-and-public-route-specs.ts b/src/openapi/internal-and-public-route-specs.ts index 6c5a60ade4..8ffbf79ac0 100644 --- a/src/openapi/internal-and-public-route-specs.ts +++ b/src/openapi/internal-and-public-route-specs.ts @@ -234,7 +234,11 @@ const PUBLIC_ROUTES: SpecEntry[] = [ tags: ["Public"], summary: "Return a shields.io-compatible badge payload for a repo", auth: "public", - responses: { 200: { description: "Badge payload" } }, + responses: { + 200: { description: "Badge payload" }, + 404: { description: "The repo has no public badge (unknown, private, uninstalled, or badgeEnabled off)" }, + 503: { description: "The badge data could not be loaded (a transient loader failure, short-cached)" }, + }, }, { method: "get", @@ -243,7 +247,11 @@ const PUBLIC_ROUTES: SpecEntry[] = [ tags: ["Public"], summary: "Return a rendered SVG badge for a repo", auth: "public", - responses: { 200: { description: "SVG badge" } }, + responses: { + 200: { description: "SVG badge" }, + 404: { description: "The repo has no public badge (unknown, private, uninstalled, or badgeEnabled off)" }, + 503: { description: "The badge data could not be loaded (a transient loader failure, short-cached)" }, + }, }, { method: "get", diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 64538696a3..3ce8c35307 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -37,6 +37,7 @@ import { upsertAgentRecommendationOutcome, } from "../../src/db/repositories"; import { createApp } from "../../src/api/routes"; +import { renderUnavailableBadgeSvg } from "../../src/api/badge"; import { clearPublicRepoStatsCacheForTests } from "../../src/github/public"; import { getRepositoryCollaboratorPermission } from "../../src/github/app"; import { BURDEN_FORECAST_MAX_AGE_MS } from "../../src/services/burden-forecast"; @@ -324,7 +325,13 @@ describe("api routes", () => { await upsertRepositoryFromGitHub(env, { name: "not-opted-in", full_name: "acme/not-opted-in", private: false, owner: { login: "acme" }, default_branch: "main" }, 556); const notOptedIn = await app.request("/v1/public/repos/acme/not-opted-in/badge.svg", {}, env); expect(notOptedIn.status).toBe(404); - expect(await notOptedIn.text()).toContain("unavailable"); + // #9710: the "no public badge" 404 carries the SHORT cache (not the long stale-while-revalidate window) and + // renders the exact unavailable badge -- the same shape a monitor distinguishes from the 503 loader-failure. + expect(notOptedIn.headers.get("cache-control")).toBe("public, max-age=300"); + expect(await notOptedIn.text()).toBe(renderUnavailableBadgeSvg()); + const notOptedInJson = await app.request("/v1/public/repos/acme/not-opted-in/badge.json", {}, env); + expect(notOptedInJson.status).toBe(404); + expect(notOptedInJson.headers.get("cache-control")).toBe("public, max-age=300"); // Private repos stay unavailable even when installed and explicitly opted in. await upsertRepositoryFromGitHub(env, { name: "private", full_name: "acme/private", private: true, owner: { login: "acme" }, default_branch: "main" }, 558); @@ -358,7 +365,8 @@ describe("api routes", () => { expect(failedSvg.status).toBe(503); expect(failedSvg.headers.get("content-type")).toContain("image/svg+xml"); expect(failedSvg.headers.get("cache-control")).toBe("public, max-age=300"); - expect(await failedSvg.text()).toContain("unavailable"); + // #9710: the 503 loader-failure branch renders the exact same unavailable badge as the 404 branch. + expect(await failedSvg.text()).toBe(renderUnavailableBadgeSvg()); const failedJson = await app.request("/v1/public/repos/acme/badged/badge.json", {}, brokenEnv); expect(failedJson.status).toBe(503); diff --git a/test/unit/openapi.test.ts b/test/unit/openapi.test.ts index b566b441f9..721cf0b3ff 100644 --- a/test/unit/openapi.test.ts +++ b/test/unit/openapi.test.ts @@ -191,6 +191,14 @@ describe("OpenAPI contract", () => { expect(spec.paths["/v1/auth/github/token"]?.post?.security).toEqual([{ LoopOverSessionCookie: [] }]); }); + it("#9710: both public badge routes declare 200, 404, and 503 -- the statuses a monitor distinguishes", () => { + const spec = buildOpenApiSpec(); + for (const path of ["/v1/public/repos/{owner}/{repo}/badge.svg", "/v1/public/repos/{owner}/{repo}/badge.json"]) { + const responses = spec.paths[path]?.get?.responses ?? {}; + expect(Object.keys(responses).sort()).toEqual(expect.arrayContaining(["200", "404", "503"])); + } + }); + // #9303: selftune-override routes were live but undocumented; assert both paths, their HTTP methods, and the // response schema keys matching each route's MCP tool output shape (selftuneOverrideAuditOutputSchema / // clearSelftuneOverrideOutputSchema in src/mcp/server.ts).