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
5 changes: 4 additions & 1 deletion .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ gate:

# Quality / merge-readiness score gate (the PR-quality score).
readiness:
# off | advisory | block. Default: advisory.
# off | advisory. Default: advisory. Informational only — this dimension can
# never hard-block a PR, so "block" is not an accepted value (a config that
# still says "block" is downgraded to "advisory" with a warning). For an
# enforceable quality floor, use mergeReadiness or manifestPolicy below.
mode: advisory
# At/above this score the quality dimension passes.
# Number 0–100, or null for the engine default band. Default: null.
Expand Down
23 changes: 20 additions & 3 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ import { isRagEnabled } from "../review/rag-wire";
import { getPublicStats, isPublicStatsEnabled } from "../review/public-stats";
import { buildMaintainerQualityDashboard, isMaintainerQualityDataStale } from "../services/maintainer-quality-dashboard";
import { MAX_LOCAL_SCORER_WARNING_CHARS, MAX_LOCAL_SCORER_WARNING_COUNT } from "../signals/local-scorer-diagnostics";
import { compileFocusManifestPolicy, MAX_FOCUS_MANIFEST_BYTES } from "../signals/focus-manifest";
import { compileFocusManifestPolicy, MAX_FOCUS_MANIFEST_BYTES, normalizeReadinessGateMode } from "../signals/focus-manifest";
import { resolveRepositorySettings } from "../settings/repository-settings";
import { loadPublicRepoFocusManifest, loadRepoFocusManifest, upsertRepoFocusManifest } from "../signals/focus-manifest-loader";
import { buildRepoOnboardingPackPreviewForRepo } from "../services/repo-onboarding-pack";
Expand All @@ -265,6 +265,7 @@ import type {
PullRequestRecord,
RepoSyncSegmentRecord,
RepositoryRecord,
RepositorySettings,
} from "../types";
import { errorMessage, nowIso } from "../utils/json";

Expand Down Expand Up @@ -686,6 +687,21 @@ const maintainerSettingsSchema = z
})
.partial();

/** Readiness/quality can never hard-block a PR (buildQualityGateWarning is always advisory-severity;
* isConfiguredGateBlocker has no branch for it) — downgrade a settings-write's `qualityGateMode: "block"` to
* `"advisory"` here too, mirroring the same downgrade `.gittensory.yml`'s `gate.readiness.mode` /
* `settings.qualityGateMode` already get in normalizeReadinessGateMode, so the dashboard/API save path can
* never persist a value that implies enforcement it doesn't have (#2267). Callers check for `undefined`
* (a PATCH-style save that didn't touch this field) before calling — this only handles the defined case, so
* the return type never needs `undefined` under `exactOptionalPropertyTypes`. The warnings array is scratch;
* these routes have no warnings-response protocol. */
function downgradeQualityGateMode(mode: "off" | "advisory" | "block"): "off" | "advisory" {
/* v8 ignore next -- never null for a Zod-validated "off"|"advisory"|"block" input (normalizeReadinessGateMode's
"must be one of" branch only fires for a value outside that set); the fallback only satisfies the shared
parser's broader return type. */
return (normalizeReadinessGateMode(mode, "qualityGateMode", []) as "off" | "advisory" | null) ?? "advisory";
}

// Maintainer BYOK provider key. Write-only: the key is encrypted at rest and never returned. A loose
// prefix check catches the common provider/key mismatch (e.g. pasting an OpenAI key under Anthropic)
// without coupling to exact provider key formats: Anthropic keys start with `sk-ant-`; OpenAI keys
Expand Down Expand Up @@ -2105,7 +2121,8 @@ export function createApp() {
const parsed = maintainerSettingsSchema.safeParse(body);
if (!parsed.success) return c.json({ error: "invalid_repository_settings", issues: parsed.error.issues }, 400);
const current = await getRepositorySettings(c.env, fullName);
const changes = Object.fromEntries(Object.entries(parsed.data).filter(([, value]) => value !== undefined));
const changes = Object.fromEntries(Object.entries(parsed.data).filter(([, value]) => value !== undefined)) as Partial<RepositorySettings>;
if (changes.qualityGateMode !== undefined) changes.qualityGateMode = downgradeQualityGateMode(changes.qualityGateMode);
const updated = await upsertRepositorySettings(c.env, { ...current, ...changes, repoFullName: fullName });
await recordAuditEvent(c.env, {
eventType: "repo.settings_updated",
Expand Down Expand Up @@ -3424,7 +3441,7 @@ export function createApp() {
gatePack: parsed.data.gatePack,
linkedIssueGateMode: parsed.data.linkedIssueGateMode,
duplicatePrGateMode: parsed.data.duplicatePrGateMode,
qualityGateMode: parsed.data.qualityGateMode,
qualityGateMode: downgradeQualityGateMode(parsed.data.qualityGateMode),
qualityGateMinScore: parsed.data.qualityGateMinScore,
aiReviewMode: parsed.data.aiReviewMode,
aiReviewByok: parsed.data.aiReviewByok,
Expand Down
30 changes: 28 additions & 2 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,21 @@ function normalizeOptionalGateMode(value: JsonValue | undefined, field: string,
return null;
}

/** `gate.readiness.mode` (and its `settings.qualityGateMode` alias below) is documented and parsed as the shared
* off/advisory/block tri-state, but buildQualityGateWarning (src/rules/advisory.ts) always produces a
* warning-severity finding — never a blocker — and isConfiguredGateBlocker has no branch for it: readiness/
* quality is intentionally informational-only and can never hard-block a PR. Without this, a maintainer who
* sets `mode: block` believes a real quality floor is enforced when the effective behavior is silently
* advisory-only (#2267). Downgrade "block" to "advisory" here, with a clear deprecation warning, so the parsed
* config always matches what the gate actually does. Exported so the settings-write API routes (the
* dashboard/API path for the SAME `qualityGateMode` field) can apply the identical downgrade before persisting. */
export function normalizeReadinessGateMode(value: JsonValue | undefined, field: string, warnings: string[]): GateRuleMode | null {
const mode = normalizeOptionalGateMode(value, field, warnings);
if (mode !== "block") return mode;
warnings.push(`Manifest gate field "${field}" no longer accepts "block" — readiness/quality is informational-only and can never hard-block a PR; downgrading to "advisory". Use gate.manifestPolicy or another enforceable gate for a real quality floor.`);
return "advisory";
}

function normalizeOptionalBoolean(value: JsonValue | undefined, field: string, warnings: string[]): boolean | null {
if (value === undefined || value === null) return null;
if (typeof value === "boolean") return value;
Expand Down Expand Up @@ -422,7 +437,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
pack: normalizeOptionalEnum(record.pack, "gate.pack", ["gittensor", "oss-anti-slop"] as const, warnings),
linkedIssue: normalizeOptionalGateMode(record.linkedIssue, "gate.linkedIssue", warnings),
duplicates: normalizeOptionalGateMode(record.duplicates, "gate.duplicates", warnings),
readinessMode: normalizeOptionalGateMode(readinessRecord?.mode, "gate.readiness.mode", warnings),
readinessMode: normalizeReadinessGateMode(readinessRecord?.mode, "gate.readiness.mode", warnings),
readinessMinScore: normalizeOptionalScore(readinessRecord?.minScore, "gate.readiness.minScore", warnings),
slopMode: normalizeOptionalGateMode(slopRecord?.mode, "gate.slop.mode", warnings),
slopMinScore: normalizeOptionalScore(slopRecord?.minScore, "gate.slop.minScore", warnings),
Expand Down Expand Up @@ -584,7 +599,10 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[])
if (duplicatePrGateMode !== null) out.duplicatePrGateMode = duplicatePrGateMode;
const selfAuthoredLinkedIssueGateMode = normalizeOptionalGateMode(r.selfAuthoredLinkedIssueGateMode, "settings.selfAuthoredLinkedIssueGateMode", warnings);
if (selfAuthoredLinkedIssueGateMode !== null) out.selfAuthoredLinkedIssueGateMode = selfAuthoredLinkedIssueGateMode;
const qualityGateMode = normalizeOptionalGateMode(r.qualityGateMode, "settings.qualityGateMode", warnings);
// Same tri-state field as gate.readiness.mode above (the friendly gate alias overlays onto it in
// resolveEffectiveSettings) — apply the identical "block" → "advisory" downgrade here too, so a maintainer
// setting `settings.qualityGateMode: block` directly hits the same deprecation warning (#2267).
const qualityGateMode = normalizeReadinessGateMode(r.qualityGateMode, "settings.qualityGateMode", warnings);
if (qualityGateMode !== null) out.qualityGateMode = qualityGateMode;
const qualityGateMinScore = normalizeOptionalScore(r.qualityGateMinScore, "settings.qualityGateMinScore", warnings);
if (qualityGateMinScore !== null) out.qualityGateMinScore = qualityGateMinScore;
Expand Down Expand Up @@ -983,6 +1001,14 @@ export function resolveEffectiveSettings(
if (effective.requireLinkedIssue && effective.linkedIssueGateMode === "off") {
effective.linkedIssueGateMode = "block";
}
// Readiness/quality can never hard-block a PR (buildQualityGateWarning is always advisory-severity;
// isConfiguredGateBlocker has no branch for it). The write-time guards (the settings.qualityGateMode /
// gate.readiness.mode parsers above, and the settings-write API routes) stop a NEW "block" value from being
// introduced, but a repo whose DB row already has quality_gate_mode = "block" from before those guards
// existed would still resolve to it here. Downgrade it at this single resolver too, so the EFFECTIVE settings
// the gate/review pipeline AND the settings-preview dashboard read (both call this function) can never carry
// a value that implies enforcement it doesn't have, regardless of when or where it was written (#2267).
if (effective.qualityGateMode === "block") effective.qualityGateMode = "advisory";
effective.contributorBlacklist = mergeContributorBlacklists(effective.contributorBlacklist ?? [], sharedContributorBlacklist);
return effective;
}
Expand Down
20 changes: 19 additions & 1 deletion test/integration/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,20 @@ describe("api routes", () => {
await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/badged", badgeEnabled: true });
});

it("downgrades qualityGateMode: block to advisory through the internal settings write endpoint too (#2267)", async () => {
// Readiness/quality can never hard-block a PR — the internal full-settings write path (used by tooling,
// not just the maintainer dashboard) gets the identical downgrade so it can't persist "block" either.
const app = createApp();
const env = createTestEnv();
const response = await app.request(
"/v1/internal/repos/acme/readiness-block/settings",
{ method: "POST", headers: { authorization: `Bearer ${env.INTERNAL_JOB_TOKEN}`, "content-type": "application/json" }, body: JSON.stringify({ qualityGateMode: "block" }) },
env,
);
expect(response.status).toBe(200);
await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/readiness-block", qualityGateMode: "advisory" });
});

it("rejects invalid public GitHub repo stats paths before calling GitHub", async () => {
const app = createApp();
const env = createTestEnv();
Expand Down Expand Up @@ -2274,7 +2288,10 @@ describe("api routes", () => {
method: "PUT",
headers: ownerHeaders,
// #773/#774/#776: the agent-layer config is settable here; the DB layer drops an unknown action class.
body: JSON.stringify({ gateCheckMode: "enabled", slopGateMode: "block", slopGateMinScore: 55, autonomy: { merge: "auto_with_approval", deploy: "auto" }, autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" }, agentPaused: true, agentDryRun: true }),
// #2267: qualityGateMode: "block" is downgraded to "advisory" on write — readiness/quality can never
// hard-block a PR, so the dashboard/API save path can't persist a value implying enforcement it doesn't
// have. slopGateMode: "block" is a DIFFERENT, legitimately-blockable dimension and is left untouched.
body: JSON.stringify({ gateCheckMode: "enabled", slopGateMode: "block", slopGateMinScore: 55, qualityGateMode: "block", autonomy: { merge: "auto_with_approval", deploy: "auto" }, autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" }, agentPaused: true, agentDryRun: true }),
},
ownerEnv,
);
Expand All @@ -2283,6 +2300,7 @@ describe("api routes", () => {
gateCheckMode: "enabled",
slopGateMode: "block",
slopGateMinScore: 55,
qualityGateMode: "advisory", // #2267: downgraded, not persisted as "block"
autonomy: { merge: "auto_with_approval" }, // unknown action class dropped by the DB normalizer
autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" },
agentPaused: true, // #776 kill-switch
Expand Down
50 changes: 46 additions & 4 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,9 +780,11 @@ describe("public-safe invariant", () => {

describe("parseFocusManifest gate config", () => {
it("parses a full gate section including the readiness block", () => {
const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "block", minScore: 70 } } });
// readiness.mode uses "advisory" here (not "block") — readiness/quality can never hard-block (#2267);
// the block→advisory deprecation-downgrade behavior itself is covered separately below.
const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "advisory", minScore: 70 } } });
expect(m.present).toBe(true);
expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "block", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null });
expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null });
});

it("parses gate.mergeReadiness + gate.firstTimeContributorGrace, round-trips them, and warns on bad values (#822)", () => {
Expand Down Expand Up @@ -892,6 +894,21 @@ describe("parseFocusManifest gate config", () => {
expect(m.warnings.some((w) => /gate\.readiness\.mode/.test(w))).toBe(true);
});

it("downgrades gate.readiness.mode: block to advisory with a deprecation warning (#2267)", () => {
// readiness/quality is informational-only (buildQualityGateWarning always produces a warning-severity
// finding; isConfiguredGateBlocker has no branch for it) — a config that says "block" is downgraded
// rather than silently accepted, so the parsed config always matches what the gate actually does.
const m = parseFocusManifest({ gate: { readiness: { mode: "block" } } });
expect(m.gate.readinessMode).toBe("advisory");
expect(m.gate.present).toBe(true);
expect(m.warnings.some((w) => /gate\.readiness\.mode.*no longer accepts "block"/.test(w))).toBe(true);
// Genuinely invalid values still take the ORIGINAL "must be one of" warning path, unchanged.
const bad = parseFocusManifest({ gate: { readiness: { mode: "sometimes" } } });
expect(bad.gate.readinessMode).toBeNull();
expect(bad.warnings.some((w) => /gate\.readiness\.mode.*must be one of/.test(w))).toBe(true);
expect(bad.warnings.some((w) => /no longer accepts "block"/.test(w))).toBe(false);
});

it("clamps and rounds the readiness minScore to 0-100", () => {
expect(parseFocusManifest({ gate: { readiness: { minScore: 250 } } }).gate.readinessMinScore).toBe(100);
expect(parseFocusManifest({ gate: { readiness: { minScore: -10 } } }).gate.readinessMinScore).toBe(0);
Expand All @@ -915,9 +932,9 @@ describe("parseFocusManifest gate config", () => {
});

it("parses the gate section from YAML content", () => {
const m = parseFocusManifestContent("gate:\n duplicates: block\n readiness:\n mode: block\n minScore: 80\n", "repo_file");
const m = parseFocusManifestContent("gate:\n duplicates: block\n readiness:\n mode: advisory\n minScore: 80\n", "repo_file");
expect(m.gate.duplicates).toBe("block");
expect(m.gate.readinessMode).toBe("block");
expect(m.gate.readinessMode).toBe("advisory");
expect(m.gate.readinessMinScore).toBe(80);
});

Expand Down Expand Up @@ -1072,6 +1089,20 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
expect(parseFocusManifest({ settings: { commentMode: "off" } }).present).toBe(true);
});

it("downgrades settings.qualityGateMode: block to advisory with a deprecation warning, same as gate.readiness.mode (#2267)", () => {
// The generic settings: override is the SAME dashboard/API-facing qualityGateMode field, read through a
// different manifest path than gate.readiness.mode — it must get the identical downgrade, not just a
// "must be one of" pass-through, or a maintainer using this path keeps the false-enforcement belief.
const m = parseFocusManifest({ settings: { qualityGateMode: "block" } });
expect(m.settings.qualityGateMode).toBe("advisory");
expect(m.warnings.some((w) => /settings\.qualityGateMode.*no longer accepts "block"/.test(w))).toBe(true);
// Genuinely invalid values still take the ORIGINAL "must be one of" warning path, unchanged.
const bad = parseFocusManifest({ settings: { qualityGateMode: "sometimes" } });
expect(bad.settings.qualityGateMode).toBeUndefined();
expect(bad.warnings.some((w) => /settings\.qualityGateMode.*must be one of/.test(w))).toBe(true);
expect(bad.warnings.some((w) => /no longer accepts "block"/.test(w))).toBe(false);
});

it("round-trips settings through settingsOverrideToJson and serializes empty as null", () => {
const original = parseFocusManifest({ settings: { commentMode: "all_prs", qualityGateMinScore: 40 } });
const reparsed = parseFocusManifest({ settings: settingsOverrideToJson(original.settings) });
Expand Down Expand Up @@ -1152,6 +1183,17 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
);
expect(eff.linkedIssueGateMode).toBe("block");
});

it("REGRESSION: downgrades a pre-existing DB qualityGateMode: block to advisory, even with no gate.readiness.mode override (#2267)", () => {
// Simulates a repo whose DB row already has quality_gate_mode = "block" from before the write-time guards
// (the settings.qualityGateMode parser, the settings-write API routes) existed — the dashboard/API path's
// "still survives" loophole this resolver-level guard closes for good, regardless of source or vintage.
const db = { qualityGateMode: "block" } as unknown as RepositorySettings;
expect(resolveEffectiveSettings(db, parseFocusManifest(null)).qualityGateMode).toBe("advisory");
// A non-"block" value is untouched — the downgrade only ever fires for "block".
const dbAdvisory = { qualityGateMode: "advisory" } as unknown as RepositorySettings;
expect(resolveEffectiveSettings(dbAdvisory, parseFocusManifest(null)).qualityGateMode).toBe("advisory");
});
});

describe("parseFocusManifest review config", () => {
Expand Down
Loading
Loading