From 249ecde89cbf589d3f67fbabf73e275b5ab64a44 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:46:27 -0700 Subject: [PATCH 1/2] fix(guardrail): fail-closed on KV outage + broaden crucial-path coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flood-readiness (emission weights rose on metagraphed + gittensory; a contributor wave is imminent). Two guardrail hardenings: 1. Fail-CLOSED on a KV read fault. loadHardGuardrailGlobs previously fell back to the narrow default (.github/workflows + scripts) on a THROWN read, so a KV outage correlated with a flood would silently shrink the guarded surface and let crown-jewel edits (scoring/auth/rules/the gate) auto-merge. A thrown read now returns FAIL_CLOSED_GUARDRAIL_GLOBS = ['**'] → every PR is held for human review until the read recovers. A legitimately-absent key still uses the narrow default so a freshly-installed repo can operate. 2. Broadened the LIVE per-repo KV globs (done out-of-band, effective immediately): - gittensory: added src/upstream/**, src/settings/**, src/review/**, src/services/**, src/github/**, src/config/** — the gate/decision/reviewer/ scoring/auth engine that lived OUTSIDE the old dir-prefix guards (e.g. score-breakdown.ts, command-authorization.ts, agent-actions.ts, ai-review.ts, ruleset.ts). This is the awesome-claude #4196 incident class. - metagraphed: added src/** (the old config guarded specific .mjs files but NOT src/mcp-server.mjs — the file PR #1528 touched). Tests: change-guardrail asserts the crucial out-of-dir files now hit the guardrail while infra/data/registry/docs/tests still auto-merge; guardrail-config asserts the outage path fails closed. Full suite green (3489). --- src/review/guardrail-config.ts | 16 +++++++++---- test/unit/change-guardrail.test.ts | 38 ++++++++++++++++++++++++++++++ test/unit/guardrail-config.test.ts | 7 +++--- 3 files changed, 54 insertions(+), 7 deletions(-) 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/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 () => { From 520a1262e3e78685b19e1a1ee9e02446eca29911 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:15:59 -0700 Subject: [PATCH 2/2] fix(review): break the Gittensory Gate self-deadlock that froze reviews under load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ROOT CAUSE of the stall (diagnosed from the LIVE audit_events ledger, not the frozen review_targets/review_audit): the Gittensory Gate is a REQUIRED check, and fetchLiveCiAggregate counted it like any other CI. A review posts the gate as in_progress, then prReadyForReview waits for ALL required CI to finish — but the gate never finishes (it's only concluded by the very review being deferred). So every green-CI PR deferred forever as 'CI still running' (38 review_deferred_ci_pending in 30 min; metagraphed #1532 deferred 4x with all checks green). Fix: fetchLiveCiAggregate now SKIPS the bot's own GITTENSORY_GATE_CHECK_NAME in both the check-runs and commit-status loops, so the bot waits on real CI only and never on its own gate. Test: a green 'test' check + an in_progress 'Gittensory Gate' (gate in the required set) now reports ciState 'passed', not 'pending'. --- src/github/backfill.ts | 10 ++++++++-- test/unit/backfill.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) 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/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([]); + }); }); });