Skip to content
Merged
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
10 changes: 8 additions & 2 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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 } : {}) };
Expand Down
16 changes: 12 additions & 4 deletions src/review/guardrail-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,24 @@ 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);
return out.length > 0 ? out : 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<string[]> {
const slug = repoFullName.includes("/") ? repoFullName.slice(repoFullName.indexOf("/") + 1) : repoFullName;
Expand All @@ -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;
}
}
25 changes: 25 additions & 0 deletions test/unit/backfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
});
});

});
Expand Down
38 changes: 38 additions & 0 deletions test/unit/change-guardrail.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});
7 changes: 4 additions & 3 deletions test/unit/guardrail-config.test.ts
Original file line number Diff line number Diff line change
@@ -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<unknown>): Env {
return { REVIEW_CONFIG: { get } } as unknown as Env;
Expand Down Expand Up @@ -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 () => {
Expand Down
Loading