Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions migrations/0075_global_contributor_blacklist.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- Shared/global contributor blacklist (#1425): singleton row consumed during settings resolution.
-- This acts like `global_agent_controls`: one row, id = 'singleton', to avoid repo-by-repo duplication.
CREATE TABLE IF NOT EXISTS global_contributor_blacklist (
id TEXT PRIMARY KEY,
contributor_blacklist_json TEXT NOT NULL DEFAULT '[]',
updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_by TEXT
);
INSERT OR IGNORE INTO global_contributor_blacklist (id, contributor_blacklist_json) VALUES ('singleton', '[]');
28 changes: 28 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,34 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
};
}

/** Read the singleton shared/global contributor blacklist (#1425). Missing table or malformed JSON are
* treated as an empty list so DB hiccups in this path default to no global blocks rather than halting
* processing. A singleton row (`id = 'singleton'`) makes this a global control plane just like
* `global_agent_controls`. */
export async function getGlobalContributorBlacklist(env: Env): Promise<RepositorySettings["contributorBlacklist"]> {
try {
const row = await env.DB.prepare("SELECT contributor_blacklist_json FROM global_contributor_blacklist WHERE id = 'singleton'").first<{
contributor_blacklist_json: string;
}>();
return parseContributorBlacklist(row?.contributor_blacklist_json ?? "[]");
} catch {
return [];
}
}

/** Upsert the singleton shared/global contributor blacklist (#1425). Input is normalized/validated once so
* malformed stored data never reaches execution. Returns the normalized persisted list for convenience/tests.
*/
export async function upsertGlobalContributorBlacklist(env: Env, input: { contributorBlacklist: unknown; updatedBy?: string | null }): Promise<RepositorySettings["contributorBlacklist"]> {
const normalized = normalizeContributorBlacklist(input.contributorBlacklist).entries;
await env.DB.prepare(
"INSERT INTO global_contributor_blacklist (id, contributor_blacklist_json, updated_at, updated_by) VALUES ('singleton', ?, CURRENT_TIMESTAMP, ?) ON CONFLICT(id) DO UPDATE SET contributor_blacklist_json = excluded.contributor_blacklist_json, updated_at = excluded.updated_at, updated_by = excluded.updated_by",
)
.bind(jsonString(normalized), input.updatedBy ?? null)
.run();
return normalized;
}

export async function upsertRepositorySettings(env: Env, settings: Partial<RepositorySettings> & { repoFullName: string }): Promise<RepositorySettings> {
const resolved: RepositorySettings = {
repoFullName: settings.repoFullName,
Expand Down
7 changes: 4 additions & 3 deletions src/settings/repository-settings.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getRepositorySettings } from "../db/repositories";
import { getGlobalContributorBlacklist, getRepositorySettings } from "../db/repositories";
import { loadOverride, type StorageEnv } from "../review/auto-apply";
import { resolveEffectiveSettings } from "../signals/focus-manifest";
import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
Expand Down Expand Up @@ -32,11 +32,12 @@ export function applySelfTuneOverrideToSettings(
* self-improvement loop is enabled (`GITTENSORY_REVIEW_SELFTUNE`, default OFF) — with the repo's promoted,
* soak-passed, tightening-only auto-tune override. Flag-OFF (default) ⇒ no override read, byte-identical to before. */
export async function resolveRepositorySettings(env: Env, repoFullName: string): Promise<RepositorySettings> {
const [dbSettings, manifest] = await Promise.all([
const [dbSettings, manifest, globalContributorBlacklist] = await Promise.all([
getRepositorySettings(env, repoFullName),
loadRepoFocusManifest(env, repoFullName),
getGlobalContributorBlacklist(env).catch(() => []),
]);
const effective = resolveEffectiveSettings(dbSettings, manifest);
const effective = resolveEffectiveSettings(dbSettings, manifest, globalContributorBlacklist);
if (!selfTuneFlagOn(env)) return effective;
// loadOverride is internally fail-safe (returns null on a DB blip), so this never breaks settings resolution.
const override = await loadOverride(env as unknown as StorageEnv, repoFullName);
Expand Down
9 changes: 7 additions & 2 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { parse as parseYaml } from "yaml";
import type { GatePolicyPack, GateRuleMode, JsonValue, RepositorySettings } from "../types";
import { normalizeAutonomyPolicy, normalizeAutoMaintainPolicy } from "../settings/autonomy";
import { normalizeContributorBlacklist } from "../settings/contributor-blacklist";
import { mergeContributorBlacklists, normalizeContributorBlacklist } from "../settings/contributor-blacklist";

export type FocusManifestSource = "repo_file" | "api_record" | "none";
export type FocusManifestLinkedIssuePolicy = "required" | "preferred" | "optional";
Expand Down Expand Up @@ -786,7 +786,11 @@ export function excludeReviewPaths<T extends { path: string }>(files: T[], exclu
* for its fields. This single resolver makes the whole gittensory configuration — gate on/off, blocker
* modes, comments, labels, surface, audience — controllable from the repo's `.gittensory.yml`.
*/
export function resolveEffectiveSettings(dbSettings: RepositorySettings, manifest: FocusManifest): RepositorySettings {
export function resolveEffectiveSettings(
dbSettings: RepositorySettings,
manifest: FocusManifest,
sharedContributorBlacklist: RepositorySettings["contributorBlacklist"] = [],
): RepositorySettings {
const effective: RepositorySettings = { ...dbSettings, ...manifest.settings };
const gate = manifest.gate;
if (gate.enabled !== null) effective.gateCheckMode = gate.enabled ? "enabled" : "off";
Expand All @@ -811,6 +815,7 @@ export function resolveEffectiveSettings(dbSettings: RepositorySettings, manifes
if (effective.requireLinkedIssue && effective.linkedIssueGateMode === "off") {
effective.linkedIssueGateMode = "block";
}
effective.contributorBlacklist = mergeContributorBlacklists(effective.contributorBlacklist ?? [], sharedContributorBlacklist);
return effective;
}

Expand Down
22 changes: 21 additions & 1 deletion test/unit/contributor-blacklist.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from "vitest";
import { findBlacklistEntry, isAuthorBlacklisted, mergeContributorBlacklists, normalizeContributorBlacklist } from "../../src/settings/contributor-blacklist";
import { getRepositorySettings, upsertRepositorySettings } from "../../src/db/repositories";
import { getGlobalContributorBlacklist, getRepositorySettings, upsertGlobalContributorBlacklist, upsertRepositorySettings } from "../../src/db/repositories";
import { createTestEnv } from "../helpers/d1";
import type { ContributorBlacklistEntry } from "../../src/types";

Expand All @@ -17,6 +17,26 @@ describe("contributor blacklist DB round-trip (#1425)", () => {
const settings = await getRepositorySettings(createTestEnv(), "owner/none");
expect(settings.contributorBlacklist).toEqual([]);
});

it("persists + resolves the shared/global blacklist singleton through DB", async () => {
const env = createTestEnv();
await upsertGlobalContributorBlacklist(env, { contributorBlacklist: [{ login: "global-bad-actor", reason: "global" }, { login: "-bad" }, { login: "Global-Owner", reason: "repo" }] });
const globalList = await getGlobalContributorBlacklist(env);
expect(globalList?.map((entry) => entry.login)).toEqual(["global-bad-actor", "Global-Owner"]);
expect(globalList?.[0]).toEqual({ login: "global-bad-actor", reason: "global" });
});

it("returns [] when singleton global row is missing", async () => {
const env = createTestEnv();
await env.DB.prepare("DELETE FROM global_contributor_blacklist WHERE id = 'singleton'").run();
expect(await getGlobalContributorBlacklist(env)).toEqual([]);
});

it("fails open to an empty list when the shared/global table is unavailable", async () => {
const env = createTestEnv();
await env.DB.prepare("DROP TABLE global_contributor_blacklist").run();
expect(await getGlobalContributorBlacklist(env)).toEqual([]);
});
});

describe("normalizeContributorBlacklist (#1425)", () => {
Expand Down
11 changes: 11 additions & 0 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,17 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
expect(noOverride.contributorBlacklist?.map((e) => e.login)).toEqual(["keep-me"]);
});

it("resolves contributor blacklist by unioning the shared/global list with effective per-repo settings", () => {
const manifest = parseFocusManifest({ settings: { contributorBlacklist: [{ login: "repo-only", reason: "manifest" }, { login: "Global-Repo", reason: "manifest-overrides-global" }] } });
const eff = resolveEffectiveSettings(
{ contributorBlacklist: [{ login: "global-repo", reason: "repo-db" }] } as unknown as RepositorySettings,
manifest,
[{ login: "global-repo", reason: "global" }, { login: "global-only", reason: "shared-only" }],
);
expect(eff.contributorBlacklist?.map((entry) => entry.login)).toEqual(["repo-only", "Global-Repo", "global-only"]);
expect(eff.contributorBlacklist?.find((entry) => entry.login === "Global-Repo")?.reason).toBe("manifest-overrides-global");
});

it("resolveEffectiveSettings overlays settings: over DB and lets gate: win for gate fields", () => {
const db = { commentMode: "off", gateCheckMode: "off", linkedIssueGateMode: "off", duplicatePrGateMode: "off", autoLabelEnabled: true } as unknown as RepositorySettings;
const eff = resolveEffectiveSettings(
Expand Down
37 changes: 34 additions & 3 deletions test/unit/selftune-readback.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { describe, expect, it } from "vitest";
import { upsertRepositorySettings } from "../../src/db/repositories";
import { describe, expect, it, vi } from "vitest";
import * as repositories from "../../src/db/repositories";
import { writeLiveOverride, type StorageEnv } from "../../src/review/auto-apply";
import { applySelfTuneOverrideToSettings, resolveRepositorySettings } from "../../src/settings/repository-settings";
import type { RepositorySettings } from "../../src/types";
import { createTestEnv } from "../helpers/d1";
import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader";

// The promoted override is ALWAYS a tightening (selftune-wire only ever populates the would-merge error side),
// so the read-back must only ever RAISE an existing readiness threshold — never create or lower one.
Expand Down Expand Up @@ -34,7 +35,7 @@ describe("resolveRepositorySettings — self-tune override overlay (flag-gated)"
const repo = "acme/widgets";
async function seed(env: Env): Promise<void> {
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, 'acme', 'widgets', 1, 1)").bind(repo).run();
await upsertRepositorySettings(env, { repoFullName: repo, qualityGateMinScore: 50 });
await repositories.upsertRepositorySettings(env, { repoFullName: repo, qualityGateMinScore: 50 });
await writeLiveOverride(env as unknown as StorageEnv, repo, { confidenceFloor: 0.7 });
}

Expand All @@ -49,4 +50,34 @@ describe("resolveRepositorySettings — self-tune override overlay (flag-gated)"
await seed(env);
expect((await resolveRepositorySettings(env, repo)).qualityGateMinScore).toBe(50);
});

it("merges shared/global blacklist entries with effective repo settings", async () => {
const env = createTestEnv();
const repoFullName = "acme/blacklist";
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, 'acme', 'blacklist', 1, 1)").bind(repoFullName).run();
await Promise.all([
repositories.upsertRepositorySettings(env, { repoFullName, qualityGateMinScore: 50 }),
repositories.upsertGlobalContributorBlacklist(env, { contributorBlacklist: [{ login: "GlobalBad", reason: "global" }] }),
upsertRepoFocusManifest(env, repoFullName, { settings: { contributorBlacklist: [{ login: "ManifestBad" }] } }, "api_record"),
]);

const settings = await resolveRepositorySettings(env, repoFullName);
expect(settings.contributorBlacklist?.map((entry) => entry.login)).toEqual(["ManifestBad", "GlobalBad"]);
});

it("uses fallback [] when shared/global blacklist read rejects", async () => {
const env = createTestEnv();
const repo = "acme/fallback";
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, 'acme', 'fallback', 1, 1)").bind(repo).run();

const getGlobalSpy = vi.spyOn(repositories, "getGlobalContributorBlacklist").mockRejectedValue(new Error("transient DB issue"));

try {
const settings = await resolveRepositorySettings(env, repo);
expect(settings.contributorBlacklist).toEqual([]);
expect(getGlobalSpy).toHaveBeenCalledOnce();
} finally {
getGlobalSpy.mockRestore();
}
});
});
Loading