From 90cdab6aa4b9e3ae05929c6c319fbab352588be8 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 4 Jul 2026 22:17:20 -0700 Subject: [PATCH] feat(settings): add a read-only DB/config settings-drift diagnostic Adds computeSettingsDrift, a pure diff between a repo's DB-stored repository_settings row and the live effective settings resolveEffectiveSettings would actually apply for it, so a self-host operator can see which DB-stored fields a .gittensory.yml override is silently shadowing. Compares against a "no manifest" baseline rather than the raw DB row, so manifest-independent normalization (the shared contributor-blacklist merge) never misreports as drift. computeSettingsDriftForRepo wires this to the same DB + manifest reads resolveRepositorySettings already makes, so it is immediately callable with a real Env. Never touches the live resolve/review path itself -- purely diagnostic. A human-facing entry point (an MCP tool or CLI script) needs its own auth/data-access wiring and is left as a focused follow-up rather than bolted onto this PR. --- src/settings/settings-drift.ts | 65 ++++++++++++++++++ test/unit/settings-drift.test.ts | 114 +++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+) create mode 100644 src/settings/settings-drift.ts create mode 100644 test/unit/settings-drift.test.ts diff --git a/src/settings/settings-drift.ts b/src/settings/settings-drift.ts new file mode 100644 index 0000000000..482d3e9635 --- /dev/null +++ b/src/settings/settings-drift.ts @@ -0,0 +1,65 @@ +import { getGlobalContributorBlacklist, getRepositorySettings } from "../db/repositories"; +import { parseFocusManifest, resolveEffectiveSettings, type FocusManifest } from "../signals/focus-manifest"; +import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; +import type { RepositorySettings } from "../types"; + +// The "no manifest at all" baseline (parseFocusManifest(null) is this codebase's existing idiom for it, e.g. +// test/unit/focus-manifest.test.ts). Comparing against THIS instead of the raw dbSettings row isolates drift +// caused specifically by the manifest -- resolveEffectiveSettings also applies manifest-INDEPENDENT +// normalization (the shared contributor-blacklist merge below the gate block, and the requireLinkedIssue-implies- +// block downgrade), which would otherwise misreport as "shadowed by private config" for every repo with a +// global blacklist entry, even one with no manifest at all. +const NO_MANIFEST = parseFocusManifest(null); + +export type SettingsDriftEntry = { + field: keyof RepositorySettings; + dbValue: unknown; + effectiveValue: unknown; +}; + +// Local copy of the stableStringify helper already duplicated in review/ai-review-cache-input.ts and +// upstream/ruleset.ts for the same order-independent-equality purpose -- matches this codebase's existing +// pattern of a small per-module copy rather than a shared utility for a ~6-line function. +function stableStringify(value: unknown): string { + if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`; + if (value !== null && typeof value === "object") { + return `{${Object.entries(value as Record) + .sort(([left], [right]) => left.localeCompare(right)) + .map(([key, nested]) => `${JSON.stringify(key)}:${stableStringify(nested)}`) + .join(",")}}`; + } + return JSON.stringify(value); +} + +/** + * Diagnostic-only, read-only diff between a repo's DB-stored `repository_settings` row and the LIVE effective + * settings `resolveEffectiveSettings` would actually apply for it (config-as-code `.gittensory.yml` merged over + * the DB row). Every entry here is a DB-stored field whose value is silently shadowed by a manifest override -- + * useful for a self-host operator who changed something via the dashboard and can't tell why it isn't taking + * effect. PURE and never called from the live review/gate path itself (see resolveEffectiveSettings, + * settings/repository-settings.ts's resolveRepositorySettings) -- this can never affect what settings a PR + * review actually resolves to, only report on it after the fact. + */ +export function computeSettingsDrift( + dbSettings: RepositorySettings, + manifest: FocusManifest, + sharedContributorBlacklist: RepositorySettings["contributorBlacklist"] = [], +): SettingsDriftEntry[] { + const baseline = resolveEffectiveSettings(dbSettings, NO_MANIFEST, sharedContributorBlacklist); + const effective = resolveEffectiveSettings(dbSettings, manifest, sharedContributorBlacklist); + return (Object.keys(dbSettings) as (keyof RepositorySettings)[]) + .filter((field) => stableStringify(baseline[field]) !== stableStringify(effective[field])) + .map((field) => ({ field, dbValue: dbSettings[field], effectiveValue: effective[field] })); +} + +/** Same diff, but fetching the DB row, manifest, and shared contributor blacklist live for one repo -- the + * same three reads settings/repository-settings.ts's resolveRepositorySettings already makes, so this never + * introduces a new data-fetch path, only a read-only diagnostic view over the existing one. */ +export async function computeSettingsDriftForRepo(env: Env, repoFullName: string): Promise { + const [dbSettings, manifest, sharedContributorBlacklist] = await Promise.all([ + getRepositorySettings(env, repoFullName), + loadRepoFocusManifest(env, repoFullName), + getGlobalContributorBlacklist(env).catch(() => []), + ]); + return computeSettingsDrift(dbSettings, manifest, sharedContributorBlacklist); +} diff --git a/test/unit/settings-drift.test.ts b/test/unit/settings-drift.test.ts new file mode 100644 index 0000000000..675476cb19 --- /dev/null +++ b/test/unit/settings-drift.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it, vi } from "vitest"; +import * as repositories from "../../src/db/repositories"; +import { parseFocusManifest } from "../../src/signals/focus-manifest"; +import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; +import { computeSettingsDrift, computeSettingsDriftForRepo } from "../../src/settings/settings-drift"; +import type { RepositorySettings } from "../../src/types"; +import { createTestEnv } from "../helpers/d1"; + +function fields(entries: ReturnType): string[] { + return entries.map((entry) => entry.field).sort(); +} + +describe("computeSettingsDrift (#config-drift-audit)", () => { + it("reports no drift for an empty manifest", () => { + const dbSettings = { gittensorLabel: "gittensor", qualityGateMinScore: 50 } as RepositorySettings; + expect(computeSettingsDrift(dbSettings, parseFocusManifest(null))).toEqual([]); + }); + + it("reports a settings: override that differs from the DB value", () => { + const dbSettings = { gittensorLabel: "gittensor" } as RepositorySettings; + const manifest = parseFocusManifest({ settings: { gittensorLabel: "custom-label" } }); + const drift = computeSettingsDrift(dbSettings, manifest); + expect(drift).toEqual([{ field: "gittensorLabel", dbValue: "gittensor", effectiveValue: "custom-label" }]); + }); + + it("does NOT report drift when the manifest sets the SAME value already in the DB", () => { + const dbSettings = { gittensorLabel: "gittensor" } as RepositorySettings; + const manifest = parseFocusManifest({ settings: { gittensorLabel: "gittensor" } }); + expect(computeSettingsDrift(dbSettings, manifest)).toEqual([]); + }); + + it("reports a gate: override the same way as an equivalent settings: override", () => { + const dbSettings = { gateCheckMode: "off" } as RepositorySettings; + const manifest = parseFocusManifest({ gate: { enabled: true } }); + const drift = computeSettingsDrift(dbSettings, manifest); + expect(drift).toEqual([{ field: "gateCheckMode", dbValue: "off", effectiveValue: "enabled" }]); + }); + + it("detects array-valued drift (hardGuardrailGlobs) by content, not by reference", () => { + const dbSettings = { hardGuardrailGlobs: ["src/scoring/**"] } as RepositorySettings; + const sameContent = parseFocusManifest({ settings: { hardGuardrailGlobs: ["src/scoring/**"] } }); + expect(computeSettingsDrift(dbSettings, sameContent)).toEqual([]); + + const different = parseFocusManifest({ settings: { hardGuardrailGlobs: ["src/settings/**"] } }); + const drift = computeSettingsDrift(dbSettings, different); + expect(drift).toEqual([{ field: "hardGuardrailGlobs", dbValue: ["src/scoring/**"], effectiveValue: ["src/settings/**"] }]); + }); + + it("detects object-valued drift (typeLabels) from a sparse per-category manifest override", () => { + const dbSettings = { typeLabels: { bug: "bug", feature: "enhancement" } } as unknown as RepositorySettings; + const manifest = parseFocusManifest({ settings: { typeLabels: { bug: "defect" } } }); + const drift = computeSettingsDrift(dbSettings, manifest); + expect(drift).toEqual([ + { field: "typeLabels", dbValue: { bug: "bug", feature: "enhancement" }, effectiveValue: { bug: "defect", feature: "enhancement" } }, + ]); + }); + + it("does not report contributorBlacklist drift from the shared/global blacklist merge alone (manifest-independent normalization, not manifest shadowing)", () => { + const dbSettings = { contributorBlacklist: [] } as unknown as RepositorySettings; + // No manifest override at all, but a non-empty shared/global blacklist -- resolveEffectiveSettings ALWAYS + // merges this in regardless of manifest presence, so it must not be misreported as manifest-driven drift. + const drift = computeSettingsDrift(dbSettings, parseFocusManifest(null), [{ login: "GlobalBad", reason: "global" }]); + expect(drift.some((entry) => entry.field === "contributorBlacklist")).toBe(false); + }); + + it("still reports a MANIFEST-driven contributorBlacklist entry on top of an unrelated shared blacklist", () => { + const dbSettings = { contributorBlacklist: [] } as unknown as RepositorySettings; + const manifest = parseFocusManifest({ settings: { contributorBlacklist: [{ login: "ManifestBad" }] } }); + const drift = computeSettingsDrift(dbSettings, manifest, [{ login: "GlobalBad", reason: "global" }]); + const entry = drift.find((e) => e.field === "contributorBlacklist"); + expect(entry?.effectiveValue).toEqual(expect.arrayContaining([expect.objectContaining({ login: "ManifestBad" }), expect.objectContaining({ login: "GlobalBad" })])); + }); + + it("does not report drift for a DB field the manifest never touches, even when other fields drift", () => { + const dbSettings = { gittensorLabel: "gittensor", checkRunMode: "enabled" } as RepositorySettings; + const manifest = parseFocusManifest({ settings: { gittensorLabel: "custom-label" } }); + expect(fields(computeSettingsDrift(dbSettings, manifest))).toEqual(["gittensorLabel"]); + }); +}); + +describe("computeSettingsDriftForRepo — live DB + manifest fetch (#config-drift-audit)", () => { + it("reports no drift for a repo with DB settings only, no manifest", async () => { + const env = createTestEnv(); + const repo = "acme/no-manifest"; + await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, 'acme', 'no-manifest', 1, 1)").bind(repo).run(); + await repositories.upsertRepositorySettings(env, { repoFullName: repo, gittensorLabel: "gittensor" }); + expect(await computeSettingsDriftForRepo(env, repo)).toEqual([]); + }); + + it("reports drift for a repo whose manifest shadows a DB-stored field", async () => { + const env = createTestEnv(); + const repo = "acme/shadowed"; + await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, 'acme', 'shadowed', 1, 1)").bind(repo).run(); + await Promise.all([ + repositories.upsertRepositorySettings(env, { repoFullName: repo, gittensorLabel: "gittensor" }), + upsertRepoFocusManifest(env, repo, { settings: { gittensorLabel: "manifest-label" } }, "api_record"), + ]); + const drift = await computeSettingsDriftForRepo(env, repo); + expect(drift).toEqual([{ field: "gittensorLabel", dbValue: "gittensor", effectiveValue: "manifest-label" }]); + }); + + it("falls back to an empty shared blacklist (never throws) when the global blacklist read rejects", async () => { + const env = createTestEnv(); + const repo = "acme/blacklist-read-fails"; + await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, 'acme', 'blacklist-read-fails', 1, 1)").bind(repo).run(); + await repositories.upsertRepositorySettings(env, { repoFullName: repo, gittensorLabel: "gittensor" }); + const getGlobalSpy = vi.spyOn(repositories, "getGlobalContributorBlacklist").mockRejectedValue(new Error("transient DB issue")); + try { + await expect(computeSettingsDriftForRepo(env, repo)).resolves.toEqual([]); + } finally { + getGlobalSpy.mockRestore(); + } + }); +});