diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 9ec29b3fd8..4e9eab7305 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -58,7 +58,7 @@ import type { RepositorySettings, } from "../types"; import { errorMessage, nowIso, repoParts, strippedErrorMessage } from "../utils/json"; -import { createInstallationToken, getAppInstallation } from "./app"; +import { createInstallationToken, getAppInstallation, GITTENSORY_GATE_CHECK_NAME } from "./app"; type GitHubLabelPayload = { name: string; @@ -1985,6 +1985,11 @@ export async function fetchLiveCiAggregate( ).catch(() => undefined); if (!result) break; for (const run of result.data.check_runs ?? []) { + // The bot's OWN gate check is NOT "CI" to wait on. It posts the gate as in_progress and then concludes it + // AFTER reviewing — so counting it here self-deadlocks: the review waits for all CI to finish, the gate + // never finishes (it's pending until the review it is blocking runs), and the PR defers forever. Skip it. + // (#gate-self-deadlock — this was deferring green-CI PRs as "CI still running" indefinitely.) + if (run.name === GITTENSORY_GATE_CHECK_NAME) continue; total += 1; const conclusion = (run.conclusion ?? "").toLowerCase(); const status = (run.status ?? "").toLowerCase(); @@ -2011,9 +2016,10 @@ export async function fetchLiveCiAggregate( token, ).catch(() => undefined); for (const ctx of statusResult?.data.statuses ?? []) { + const name = ctx.context ?? "status"; + if (name === GITTENSORY_GATE_CHECK_NAME) continue; // never wait on the bot's own gate (see #gate-self-deadlock above) total += 1; const state = (ctx.state ?? "").toLowerCase(); - const name = ctx.context ?? "status"; if (state === "failure" || state === "error") { const summary = typeof ctx.description === "string" ? ctx.description.trim().slice(0, 200) : ""; const detail = { name, ...(summary ? { summary } : {}), ...(ctx.target_url ? { detailsUrl: ctx.target_url } : {}) }; diff --git a/src/review/guardrail-config.ts b/src/review/guardrail-config.ts index d89794183f..8d60693fd7 100644 --- a/src/review/guardrail-config.ts +++ b/src/review/guardrail-config.ts @@ -13,6 +13,13 @@ import type { JsonValue } from "../types"; // these, it never opens the gate wide. export const DEFAULT_CRUCIAL_GUARDRAIL_GLOBS = [".github/workflows/**", "scripts/**"]; +// A KV READ FAULT (binding present but the read threw — an outage/transient error) must fail CLOSED, NOT fall +// back to the narrow default: a config-read fault correlated with a contributor flood would otherwise silently +// shrink the guarded surface to CI+scripts and let crown-jewel edits (scoring/auth/rules/the gate) auto-merge. +// "**" matches every path (the glob engine maps ** -> .*), so this holds ALL PRs for human review until the +// config read recovers — fail-safe for the surface a flood most threatens. (#flood-readiness) +export const FAIL_CLOSED_GUARDRAIL_GLOBS = ["**"]; + function asNonEmptyStringArray(value: unknown): string[] | null { if (!Array.isArray(value)) return null; const out = value.filter((entry): entry is string => typeof entry === "string" && entry.length > 0); @@ -20,9 +27,10 @@ function asNonEmptyStringArray(value: unknown): string[] | null { } /** - * Resolve a repo's hard-guardrail path globs from the shared REVIEW_CONFIG KV (key = repo slug). Falls back to - * DEFAULT_CRUCIAL_GUARDRAIL_GLOBS when the binding / key / field is absent or malformed — fail-SAFE and never - * throws (the auto-maintain trigger is best-effort and must not be sunk by a config read). + * Resolve a repo's hard-guardrail path globs from the shared REVIEW_CONFIG KV (key = repo slug). Never throws + * (the auto-maintain trigger is best-effort). A legitimately-absent binding/key/field falls back to the narrow + * DEFAULT_CRUCIAL_GUARDRAIL_GLOBS so a freshly-installed repo can still operate; but a THROWN read (KV outage) + * fails CLOSED to FAIL_CLOSED_GUARDRAIL_GLOBS so a config fault can never open the gate during a flood. */ export async function loadHardGuardrailGlobs(env: Env, repoFullName: string): Promise { const slug = repoFullName.includes("/") ? repoFullName.slice(repoFullName.indexOf("/") + 1) : repoFullName; @@ -31,6 +39,6 @@ export async function loadHardGuardrailGlobs(env: Env, repoFullName: string): Pr const config = (await env.REVIEW_CONFIG.get(slug, "json")) as { hardGuardrailGlobs?: JsonValue } | null; return asNonEmptyStringArray(config?.hardGuardrailGlobs) ?? DEFAULT_CRUCIAL_GUARDRAIL_GLOBS; } catch { - return DEFAULT_CRUCIAL_GUARDRAIL_GLOBS; + return FAIL_CLOSED_GUARDRAIL_GLOBS; } } diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index 8a60253601..6e04aed959 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -2684,6 +2684,31 @@ describe("GitHub backfill", () => { expect(aggregate.failingDetails).toEqual([expect.objectContaining({ name: "unknown-required-status" })]); expect(aggregate.nonRequiredFailingDetails).toEqual([]); }); + + it("ignores the bot's OWN Gittensory Gate check so it never self-deadlocks (#gate-self-deadlock)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/check-runs?")) { + return Response.json({ + check_runs: [ + { name: "test", status: "completed", conclusion: "success" }, + // the bot's OWN gate, still in_progress (posted but not yet concluded). Counting it would defer + // the very review that concludes it — the self-deadlock that froze green-CI PRs as "CI pending". + { name: "Gittensory Gate", status: "in_progress", conclusion: null }, + ], + }); + } + if (url.includes("/status?")) return Response.json({ statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + // The gate is among the branch-protection required contexts, yet it MUST be excluded from the CI wait. + const aggregate = await fetchLiveCiAggregate(env, "JSONbored/metagraphed", "headsha", "public-token", new Set(["test", "Gittensory Gate"])); + + expect(aggregate.ciState).toBe("passed"); // would be "pending" if the in_progress gate were counted + expect(aggregate.failingDetails).toEqual([]); + }); }); }); diff --git a/test/unit/change-guardrail.test.ts b/test/unit/change-guardrail.test.ts index b18205d2c8..5c1a1e172c 100644 --- a/test/unit/change-guardrail.test.ts +++ b/test/unit/change-guardrail.test.ts @@ -30,3 +30,41 @@ describe("change-guardrail glob matching", () => { expect(changedPathsHittingGuardrail(["src/scoring/x.ts"], [])).toEqual([]); }); }); + +// #flood-readiness: the LIVE gittensory KV globs must guard crucial files that live OUTSIDE the dir-prefix +// guards (the awesome-claude #4196 class — a weakened sensitive file slipping through because its folder +// wasn't covered), while leaving clean non-crucial PRs auto-mergeable. Mirrors REVIEW_CONFIG["gittensory"]. +describe("hard-guardrail covers content-crucial files outside the dir-prefix guards", () => { + const GITTENSORY_GLOBS = [ + ".github/**", "scripts/**", "packages/**", "apps/gittensory-ui/**", + "src/scoring/**", "src/signals/**", "src/rules/**", "src/gittensor/**", "src/auth/**", + "src/upstream/**", "src/settings/**", "src/review/**", "src/services/**", "src/github/**", "src/config/**", + ]; + + it("guards crucial files in non-obvious folders (scoring/auth/rules/gate/reviewer)", () => { + for (const p of [ + "src/services/score-breakdown.ts", // scoring logic under services/ + "src/services/ai-review.ts", // the reviewer engine (#4196 class) + "src/settings/command-authorization.ts", // authorization under settings/ + "src/settings/agent-actions.ts", // the merge/close decision planner + "src/upstream/ruleset.ts", // rules under upstream/ + "src/upstream/unmodeled-scoring-drift.ts", // scoring drift under upstream/ + "src/review/guardrail-config.ts", // the guardrail loader itself + "src/github/backfill.ts", // CI aggregation that gates merges + "src/config/gittensory-repo-focus-manifest.ts", // scoring focus config + ]) { + expect(changedPathsHittingGuardrail([p], GITTENSORY_GLOBS)).toEqual([p]); + } + }); + + it("still lets clean non-crucial PRs auto-merge (infra/data/registry/docs/tests)", () => { + const nonCrucial = ["src/utils/json.ts", "src/db/repositories.ts", "src/registry/normalize.ts", "src/mcp/server.ts", "README.md", "docs/x.md", "test/unit/foo.test.ts"]; + expect(changedPathsHittingGuardrail(nonCrucial, GITTENSORY_GLOBS)).toEqual([]); + }); + + it("the fail-closed sentinel ['**'] guards every path (KV-outage hold-all)", () => { + for (const p of ["src/utils/json.ts", "README.md", "anything/at/all.txt"]) { + expect(matchesAny(p, ["**"])).toBe(true); + } + }); +}); diff --git a/test/unit/guardrail-config.test.ts b/test/unit/guardrail-config.test.ts index fdb2a91a6d..ee8902fd11 100644 --- a/test/unit/guardrail-config.test.ts +++ b/test/unit/guardrail-config.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { DEFAULT_CRUCIAL_GUARDRAIL_GLOBS, loadHardGuardrailGlobs } from "../../src/review/guardrail-config"; +import { DEFAULT_CRUCIAL_GUARDRAIL_GLOBS, FAIL_CLOSED_GUARDRAIL_GLOBS, loadHardGuardrailGlobs } from "../../src/review/guardrail-config"; function envWith(get: (key: string, type: string) => Promise): Env { return { REVIEW_CONFIG: { get } } as unknown as Env; @@ -28,14 +28,15 @@ describe("loadHardGuardrailGlobs", () => { expect(globs).toEqual(["scripts/**"]); }); - it("fails safe to the default when the KV read throws", async () => { + it("fails CLOSED (guard everything) when the KV read throws — an outage must never open the gate", async () => { const globs = await loadHardGuardrailGlobs( envWith(async () => { throw new Error("kv down"); }), "o/r", ); - expect(globs).toEqual(DEFAULT_CRUCIAL_GUARDRAIL_GLOBS); + expect(globs).toEqual(FAIL_CLOSED_GUARDRAIL_GLOBS); // ["**"] → every path held for human review + expect(globs).not.toEqual(DEFAULT_CRUCIAL_GUARDRAIL_GLOBS); }); it("uses the whole name as the slug when there is no owner prefix", async () => {