diff --git a/.loopover.yml.example b/.loopover.yml.example index 03a5a2a899..3eaeb0f79a 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -924,10 +924,12 @@ settings: # Bool. Default: true. backfillEnabled: true - # Render a README status badge for the repo. Bool. Default: false. + # Render a README status badge for the repo. Bool. Default: false. Config-as-code only (Batch A + # follow-up, loopover#6442) -- no DB column or dashboard toggle. badgeEnabled: false - # Publish a public per-repo review-quality page. Bool. Default: false. + # Publish a public per-repo review-quality page. Bool. Default: false. Config-as-code only (Batch A + # follow-up, loopover#6442) -- no DB column or dashboard toggle. publicQualityMetrics: false # Per-repo kill-switch: when true, the agent does nothing on this repo. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 18027a34a7..752e5ee1c2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -284,10 +284,12 @@ Public GitHub surfaces: Config as code (`.loopover.yml`) — every repository setting is controllable from the config file: -- **`settings:`** is a partial of the repository settings: any behaviour a maintainer can toggle in the - dashboard can be set here as code — `commentMode`, `publicAudienceMode`, `publicSurface`, `checkRunMode`, - `reviewCheckMode`, the gate-blocker modes, `autoLabelEnabled`, `gittensorLabel`, `requireLinkedIssue`, - `backfillEnabled`, etc. +- **`settings:`** is a partial of the repository settings. Any behaviour a maintainer can toggle in the + dashboard can also be set here as code — `reviewCheckMode`, the gate-blocker modes, `autoLabelEnabled`, + `gittensorLabel`, `requireLinkedIssue`, etc. A subset is config-as-code **only** (no DB column, no + dashboard toggle — this file is their sole source): `commentMode`, `publicAudienceMode`, + `publicSignalLevel`, `checkRunMode`, `checkRunDetailLevel`, `publicSurface`, `includeMaintainerAuthors`, + `backfillEnabled`, `badgeEnabled`, `publicQualityMetrics`, `regateSweepOrderMode`. - **`gate:`** is a friendly typed alias for the gate subset — `enabled` (on/off), `linkedIssue`, `duplicates`, `readiness: { mode, minScore }` (each `off | advisory | block`). - **`review:`** customizes the public review-panel CONTENT: `footer: { text }` (custom lead copy — the diff --git a/apps/loopover-ui/content/docs/github-app.mdx b/apps/loopover-ui/content/docs/github-app.mdx index 5edead8245..d6ced9b2b0 100644 --- a/apps/loopover-ui/content/docs/github-app.mdx +++ b/apps/loopover-ui/content/docs/github-app.mdx @@ -141,7 +141,8 @@ gates every author identically, regardless of config. { const payload = buildMaintainerSettingsSavePayload(SETTINGS); expect(Object.keys(payload).sort()).toEqual([...MAINTAINER_SETTINGS_EDITABLE_KEYS].sort()); expect(payload.linkedIssueGateMode).toBe("advisory"); - expect(payload.badgeEnabled).toBe(false); + expect(payload.autoLabelEnabled).toBe(true); }); it("buildMaintainerSettingsSavePayload merges a partial patch over the base settings", () => { @@ -47,7 +45,7 @@ describe("maintainer-settings-editable (#2218)", () => { expect(payload.duplicatePrGateMode).toBe("block"); // Untouched fields pass through unchanged. expect(payload.qualityGateMode).toBe("advisory"); - expect(payload.badgeEnabled).toBe(false); + expect(payload.autoLabelEnabled).toBe(true); }); it("an empty patch object is a no-op (same as omitting it)", () => { diff --git a/apps/loopover-ui/src/lib/maintainer-settings-editable.ts b/apps/loopover-ui/src/lib/maintainer-settings-editable.ts index 83d3c6bfab..761bdcd3d0 100644 --- a/apps/loopover-ui/src/lib/maintainer-settings-editable.ts +++ b/apps/loopover-ui/src/lib/maintainer-settings-editable.ts @@ -36,8 +36,6 @@ export type MaintainerSettingsEditable = { // #6443: gittensorLabel/createMissingLabel removed -- no longer DB-backed, config-as-code only via // .loopover.yml's settings: block now (the dashboard can no longer write them). requireLinkedIssue: boolean; - badgeEnabled: boolean; - publicQualityMetrics: boolean; commandAuthorization: CommandAuthorization; autonomy: Partial>; autoMaintain: { requireApprovals: number; mergeMethod: AutoMergeMethod }; @@ -61,8 +59,6 @@ export const MAINTAINER_SETTINGS_EDITABLE_KEYS: Array; async function loadPublicRepoBadge(env: Env, owner: string, repo: string): Promise { const repository = await getRepository(env, `${owner}/${repo}`); if (!repository || repository.isPrivate || !repository.isInstalled) return null; - // Intentionally the raw DB row, not resolveRepositorySettings: this is an unauthenticated, high-frequency - // public route (a README-embedded badge image), so it deliberately trades honoring a yml-only `badgeEnabled` - // override for avoiding a manifest-cache lookup (and a possible cold-cache GitHub fetch) on every image load. - // `badgeEnabled` is normally set via the dashboard/API, which persists straight to this same DB row (#2912). - const settings = await getRepositorySettings(env, repository.fullName); + // badgeEnabled has no DB column anymore (Batch A follow-up, loopover#6442) -- config-as-code only, so + // this must read the resolved (manifest-overlaid) settings instead of the old raw-DB-row shortcut. An + // accepted perf tradeoff (a manifest-cache lookup, occasionally a cold-cache GitHub fetch, on this + // unauthenticated high-frequency README-badge route) in exchange for `.loopover.yml` being honored here. + const settings = await resolveRepositorySettings(env, repository.fullName); if (!settings.badgeEnabled) return null; const pullRequests = await listPullRequests(env, repository.fullName); return buildPublicRepoQuality(pullRequests); @@ -339,7 +339,7 @@ async function loadPublicRepoBadge(env: Env, owner: string, repo: string): Promi async function loadPublicRepoQualityMetrics(env: Env, owner: string, repo: string) { const repository = await getRepository(env, `${owner}/${repo}`); if (!repository || repository.isPrivate || !repository.isInstalled) return null; - const settings = await getRepositorySettings(env, repository.fullName); + const settings = await resolveRepositorySettings(env, repository.fullName); if (!settings.publicQualityMetrics) return null; return loadPublicQualityMetrics(env, repository.fullName); } @@ -683,8 +683,6 @@ const repositorySettingsSchema = z.object({ // #6443: gittensorLabel/blacklistLabel/createMissingLabel/contributorBlacklist removed -- no longer // DB-backed, config-as-code only via .loopover.yml's settings: block now. requireLinkedIssue: z.boolean().default(false), - badgeEnabled: z.boolean().default(false), - publicQualityMetrics: z.boolean().default(false), commandAuthorization: z .object({ default: z.array(z.enum(["maintainer", "collaborator", "pr_author", "confirmed_miner"])).max(4).optional(), @@ -721,8 +719,6 @@ const maintainerSettingsSchema = z autoLabelEnabled: z.boolean(), closeOwnerAuthors: z.boolean(), requireLinkedIssue: z.boolean(), - badgeEnabled: z.boolean(), - publicQualityMetrics: z.boolean(), agentPaused: z.boolean(), agentDryRun: z.boolean(), requireFreshRebaseWindowMinutes: z.number().int().positive().nullable(), @@ -4231,8 +4227,6 @@ export function createApp() { closeOwnerAuthors: parsed.data.closeOwnerAuthors, autoLabelEnabled: parsed.data.autoLabelEnabled, requireLinkedIssue: parsed.data.requireLinkedIssue, - badgeEnabled: parsed.data.badgeEnabled, - publicQualityMetrics: parsed.data.publicQualityMetrics, commandAuthorization: normalizeCommandAuthorizationPolicy(parsed.data.commandAuthorization).policy, }), ); diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 0ac73b5496..5fc5082950 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -705,8 +705,9 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise includeMaintainerAuthors: false, requireLinkedIssue: row.requireLinkedIssue, backfillEnabled: true, - badgeEnabled: row.badgeEnabled, - publicQualityMetrics: row.publicQualityMetrics, + // Config-as-code only (Batch A follow-up, loopover#6442): no DB column anymore, see migration 0158. + badgeEnabled: false, + publicQualityMetrics: false, agentPaused: row.agentPaused, agentDryRun: row.agentDryRun, commandAuthorization: parseCommandAuthorizationPolicy(row.commandAuthorizationJson), @@ -830,8 +831,10 @@ export async function upsertRepositorySettings(env: Env, settings: Partial { // Installed + opted in, with assessed merged PRs. await upsertRepositoryFromGitHub(env, { name: "badged", full_name: "acme/badged", private: false, owner: { login: "acme" }, default_branch: "main" }, 555); - await upsertRepositorySettings(env, { repoFullName: "acme/badged", badgeEnabled: true }); + await upsertRepoFocusManifest(env, "acme/badged", { settings: { badgeEnabled: true } }); await upsertPullRequestFromGitHub(env, "acme/badged", { number: 1, title: "Feature", state: "merged", created_at: "2026-06-01T00:00:00Z", merged_at: "2026-06-01T04:00:00Z", labels: [] }); await upsertPullRequestFromGitHub(env, "acme/badged", { number: 2, title: "Slop", state: "merged", created_at: "2026-06-02T00:00:00Z", merged_at: "2026-06-02T06:00:00Z", labels: [] }); await updatePullRequestSlopAssessment(env, "acme/badged", 1, { slopRisk: 0, slopBand: "clean" }); @@ -310,7 +310,7 @@ describe("api routes", () => { // 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); - await upsertRepositorySettings(env, { repoFullName: "acme/private", badgeEnabled: true }); + await upsertRepoFocusManifest(env, "acme/private", { settings: { badgeEnabled: true } }); await upsertPullRequestFromGitHub(env, "acme/private", { number: 1, title: "Secret", state: "merged", created_at: "2026-06-03T00:00:00Z", merged_at: "2026-06-03T02:00:00Z", labels: [] }); await updatePullRequestSlopAssessment(env, "acme/private", 1, { slopRisk: 0, slopBand: "clean" }); const privateSvg = await app.request("/v1/public/repos/acme/private/badge.svg", {}, env); @@ -322,7 +322,7 @@ describe("api routes", () => { // Opted in but NOT installed → unavailable. await upsertRepositoryFromGitHub(env, { name: "uninstalled", full_name: "acme/uninstalled", private: false, owner: { login: "acme" }, default_branch: "main" }); - await upsertRepositorySettings(env, { repoFullName: "acme/uninstalled", badgeEnabled: true }); + await upsertRepoFocusManifest(env, "acme/uninstalled", { settings: { badgeEnabled: true } }); const notInstalled = await app.request("/v1/public/repos/acme/uninstalled/badge.svg", {}, env); expect(notInstalled.status).toBe(404); @@ -337,7 +337,7 @@ describe("api routes", () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "quality", full_name: "acme/quality", private: false, owner: { login: "acme" }, default_branch: "main" }, 560); - await upsertRepositorySettings(env, { repoFullName: "acme/quality", publicQualityMetrics: true }); + await upsertRepoFocusManifest(env, "acme/quality", { settings: { publicQualityMetrics: true } }); await upsertPullRequestFromGitHub(env, "acme/quality", { number: 1, title: "Merged", state: "merged", created_at: "2026-06-01T00:00:00Z", merged_at: "2026-06-02T00:00:00Z", labels: [] }); await upsertPullRequestFromGitHub(env, "acme/quality", { number: 2, title: "Merged too", state: "merged", created_at: "2026-06-01T01:00:00Z", merged_at: "2026-06-02T01:00:00Z", labels: [] }); await upsertPullRequestFromGitHub(env, "acme/quality", { number: 3, title: "Closed", state: "closed", created_at: "2026-06-03T00:00:00Z", labels: [] }); @@ -371,13 +371,13 @@ describe("api routes", () => { // Private repos stay unavailable even when installed and explicitly opted in. await upsertRepositoryFromGitHub(env, { name: "quality-private", full_name: "acme/quality-private", private: true, owner: { login: "acme" }, default_branch: "main" }, 562); - await upsertRepositorySettings(env, { repoFullName: "acme/quality-private", publicQualityMetrics: true }); + await upsertRepoFocusManifest(env, "acme/quality-private", { settings: { publicQualityMetrics: true } }); const privateRes = await app.request("/v1/public/repos/acme/quality-private/quality", {}, env); expect(privateRes.status).toBe(404); // Opted in but NOT installed → unavailable. await upsertRepositoryFromGitHub(env, { name: "quality-uninstalled", full_name: "acme/quality-uninstalled", private: false, owner: { login: "acme" }, default_branch: "main" }); - await upsertRepositorySettings(env, { repoFullName: "acme/quality-uninstalled", publicQualityMetrics: true }); + await upsertRepoFocusManifest(env, "acme/quality-uninstalled", { settings: { publicQualityMetrics: true } }); const notInstalled = await app.request("/v1/public/repos/acme/quality-uninstalled/quality", {}, env); expect(notInstalled.status).toBe(404); @@ -387,7 +387,7 @@ describe("api routes", () => { await expect(unknown.json()).resolves.toMatchObject({ error: "not_found" }); }); - it("persists the publicQualityMetrics opt-in through the settings write endpoint (#2568)", async () => { + it("REGRESSION (Batch A follow-up, loopover#6442): ignores a publicQualityMetrics opt-in posted to the settings write endpoint (config-as-code only now)", async () => { const app = createApp(); const env = createTestEnv(); const response = await app.request( @@ -396,10 +396,11 @@ describe("api routes", () => { env, ); expect(response.status).toBe(200); - await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/quality", publicQualityMetrics: true }); + // publicQualityMetrics has no DB column anymore -- only .loopover.yml's settings: block can opt a repo in. + await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/quality", publicQualityMetrics: false }); }); - it("persists the badgeEnabled opt-in through the settings write endpoint (#541)", async () => { + it("REGRESSION (Batch A follow-up, loopover#6442): ignores a badgeEnabled opt-in posted to the settings write endpoint (config-as-code only now)", async () => { const app = createApp(); const env = createTestEnv(); const response = await app.request( @@ -408,7 +409,8 @@ describe("api routes", () => { env, ); expect(response.status).toBe(200); - await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/badged", badgeEnabled: true }); + // badgeEnabled has no DB column anymore -- only .loopover.yml's settings: block can opt a repo in. + await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/badged", badgeEnabled: false }); }); it("downgrades qualityGateMode: block to advisory through the internal settings write endpoint too (#2267)", async () => { diff --git a/test/integration/public-quality-metrics-route-error.test.ts b/test/integration/public-quality-metrics-route-error.test.ts index 4e230f2484..74c8d5ec59 100644 --- a/test/integration/public-quality-metrics-route-error.test.ts +++ b/test/integration/public-quality-metrics-route-error.test.ts @@ -7,19 +7,17 @@ vi.mock("../../src/services/public-quality-metrics", () => ({ import { createApp } from "../../src/api/routes"; import { createTestEnv } from "../helpers/d1"; -import { upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories"; +import { upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; describe("GET /v1/public/repos/:owner/:repo/quality — error path", () => { it("returns 503 when quality metrics computation throws", async () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "quality", full_name: "acme/quality", private: false, owner: { login: "acme" }, default_branch: "main" }, 560); - // NOTE: publicQualityMetrics intentionally stays DB-backed here (not moved to the focus manifest) - // because the route under test (`loadPublicRepoQualityMetrics` in src/api/routes.ts) reads - // `getRepositorySettings` directly -- the same deliberate raw-DB-row bypass documented on the sibling - // `loadPublicRepoBadge` helper -- and never consults `resolveRepositorySettings`/the manifest overlay. - // Moving this field to `upsertRepoFocusManifest` would make the route see publicQualityMetrics=false - // (404) instead of true (503 via the mocked throw), which is a real behavior difference, not a wiring bug. - await upsertRepositorySettings(env, { repoFullName: "acme/quality", publicQualityMetrics: true }); + // publicQualityMetrics has no DB column anymore (Batch A follow-up, loopover#6442) -- config-as-code + // only. loadPublicRepoQualityMetrics now reads resolveRepositorySettings (manifest-aware), so opt-in + // must go through the manifest, not a DB row. + await upsertRepoFocusManifest(env, "acme/quality", { settings: { publicQualityMetrics: true } }); const res = await createApp().request("/v1/public/repos/acme/quality/quality", {}, env); expect(res.status).toBe(503); diff --git a/test/integration/routes-errors.test.ts b/test/integration/routes-errors.test.ts index 685bd45342..c4a0b5eea0 100644 --- a/test/integration/routes-errors.test.ts +++ b/test/integration/routes-errors.test.ts @@ -1185,13 +1185,12 @@ describe("api route guards and error branches", () => { headers: internalHeaders(env), body: JSON.stringify({ gatePack: "oss-anti-slop", - badgeEnabled: true, }), }, env, ); expect(updated.status).toBe(200); - await expect(updated.json()).resolves.toMatchObject({ gatePack: "oss-anti-slop", badgeEnabled: true }); + await expect(updated.json()).resolves.toMatchObject({ gatePack: "oss-anti-slop" }); }); it("exposes and clears self-tune overrides for operators, rejecting unauthorized callers (#6168)", async () => { diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index db91fb34f2..4915f15a22 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -3039,16 +3039,16 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(eff.linkedIssueGateMode).toBe("block"); // gate: wins over settings: }); - it("wires settings.badgeEnabled into the manifest parser and lets it override the DB value (#2555)", () => { + it("wires settings.badgeEnabled into the manifest parser and lets it override the built-in default (#2555, Batch A follow-up loopover#6442)", () => { const parsedTrue = parseFocusManifest({ settings: { badgeEnabled: true } }); expect(parsedTrue.settings.badgeEnabled).toBe(true); expect(parsedTrue.warnings).toEqual([]); const parsedFalse = parseFocusManifest({ settings: { badgeEnabled: false } }); expect(parsedFalse.settings.badgeEnabled).toBe(false); - const db = { badgeEnabled: false } as unknown as RepositorySettings; - const eff = resolveEffectiveSettings(db, parseFocusManifest({ settings: { badgeEnabled: true } })); - expect(eff.badgeEnabled).toBe(true); // settings: override wins over the DB-stored value + const base = { badgeEnabled: false } as unknown as RepositorySettings; + const eff = resolveEffectiveSettings(base, parseFocusManifest({ settings: { badgeEnabled: true } })); + expect(eff.badgeEnabled).toBe(true); // settings: override wins over the built-in default (no DB column anymore) }); it("wires settings.includeMaintainerAuthors into the manifest parser and resolver (#2052)", () => { @@ -3073,15 +3073,15 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(reparsed.settings.includeMaintainerAuthors).toBe(true); }); - it("wires settings.publicQualityMetrics into the manifest parser and lets it override the DB value (#2568)", () => { + it("wires settings.publicQualityMetrics into the manifest parser and lets it override the built-in default (#2568, Batch A follow-up loopover#6442)", () => { const parsedTrue = parseFocusManifest({ settings: { publicQualityMetrics: true } }); expect(parsedTrue.settings.publicQualityMetrics).toBe(true); expect(parsedTrue.warnings).toEqual([]); const parsedFalse = parseFocusManifest({ settings: { publicQualityMetrics: false } }); expect(parsedFalse.settings.publicQualityMetrics).toBe(false); - const db = { publicQualityMetrics: false } as unknown as RepositorySettings; - const eff = resolveEffectiveSettings(db, parseFocusManifest({ settings: { publicQualityMetrics: true } })); + const base = { publicQualityMetrics: false } as unknown as RepositorySettings; + const eff = resolveEffectiveSettings(base, parseFocusManifest({ settings: { publicQualityMetrics: true } })); expect(eff.publicQualityMetrics).toBe(true); });