Skip to content

Commit 6d0e249

Browse files
committed
fix(gate): downgrade readiness gate mode block to advisory
gate.readiness.mode accepted the shared off/advisory/block tri-state, but buildQualityGateWarning always produces a warning-severity finding and isConfiguredGateBlocker has no branch for it — readiness/quality is intentionally informational-only and can never hard-block a PR. A maintainer setting mode: block believed a real quality floor was enforced when the effective behavior was silently advisory-only. Add normalizeReadinessGateMode to downgrade block to advisory at the config-as-code parse layer, with a deprecation warning pointing at mergeReadiness/manifestPolicy for an enforceable floor. The shared normalizeOptionalGateMode is untouched since other gate fields legitimately support block.
1 parent 6cef302 commit 6d0e249

4 files changed

Lines changed: 43 additions & 7 deletions

File tree

.gittensory.yml.example

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,10 @@ gate:
112112

113113
# Quality / merge-readiness score gate (the PR-quality score).
114114
readiness:
115-
# off | advisory | block. Default: advisory.
115+
# off | advisory. Default: advisory. Informational only — this dimension can
116+
# never hard-block a PR, so "block" is not an accepted value (a config that
117+
# still says "block" is downgraded to "advisory" with a warning). For an
118+
# enforceable quality floor, use mergeReadiness or manifestPolicy below.
116119
mode: advisory
117120
# At/above this score the quality dimension passes.
118121
# Number 0–100, or null for the engine default band. Default: null.

src/signals/focus-manifest.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,20 @@ function normalizeOptionalGateMode(value: JsonValue | undefined, field: string,
357357
return null;
358358
}
359359

360+
/** `gate.readiness.mode` is documented and parsed as the shared off/advisory/block tri-state, but
361+
* buildQualityGateWarning (src/rules/advisory.ts) always produces a warning-severity finding — never a
362+
* blocker — and isConfiguredGateBlocker has no branch for it: readiness/quality is intentionally
363+
* informational-only and can never hard-block a PR. Without this, a maintainer who sets `mode: block`
364+
* believes a real quality floor is enforced when the effective behavior is silently advisory-only (#2267).
365+
* Downgrade "block" to "advisory" here, with a clear deprecation warning, so the parsed config always
366+
* matches what the gate actually does. */
367+
function normalizeReadinessGateMode(value: JsonValue | undefined, field: string, warnings: string[]): GateRuleMode | null {
368+
const mode = normalizeOptionalGateMode(value, field, warnings);
369+
if (mode !== "block") return mode;
370+
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.`);
371+
return "advisory";
372+
}
373+
360374
function normalizeOptionalBoolean(value: JsonValue | undefined, field: string, warnings: string[]): boolean | null {
361375
if (value === undefined || value === null) return null;
362376
if (typeof value === "boolean") return value;
@@ -422,7 +436,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
422436
pack: normalizeOptionalEnum(record.pack, "gate.pack", ["gittensor", "oss-anti-slop"] as const, warnings),
423437
linkedIssue: normalizeOptionalGateMode(record.linkedIssue, "gate.linkedIssue", warnings),
424438
duplicates: normalizeOptionalGateMode(record.duplicates, "gate.duplicates", warnings),
425-
readinessMode: normalizeOptionalGateMode(readinessRecord?.mode, "gate.readiness.mode", warnings),
439+
readinessMode: normalizeReadinessGateMode(readinessRecord?.mode, "gate.readiness.mode", warnings),
426440
readinessMinScore: normalizeOptionalScore(readinessRecord?.minScore, "gate.readiness.minScore", warnings),
427441
slopMode: normalizeOptionalGateMode(slopRecord?.mode, "gate.slop.mode", warnings),
428442
slopMinScore: normalizeOptionalScore(slopRecord?.minScore, "gate.slop.minScore", warnings),

test/unit/focus-manifest.test.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -780,9 +780,11 @@ describe("public-safe invariant", () => {
780780

781781
describe("parseFocusManifest gate config", () => {
782782
it("parses a full gate section including the readiness block", () => {
783-
const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "block", minScore: 70 } } });
783+
// readiness.mode uses "advisory" here (not "block") — readiness/quality can never hard-block (#2267);
784+
// the block→advisory deprecation-downgrade behavior itself is covered separately below.
785+
const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "advisory", minScore: 70 } } });
784786
expect(m.present).toBe(true);
785-
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 });
787+
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 });
786788
});
787789

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

897+
it("downgrades gate.readiness.mode: block to advisory with a deprecation warning (#2267)", () => {
898+
// readiness/quality is informational-only (buildQualityGateWarning always produces a warning-severity
899+
// finding; isConfiguredGateBlocker has no branch for it) — a config that says "block" is downgraded
900+
// rather than silently accepted, so the parsed config always matches what the gate actually does.
901+
const m = parseFocusManifest({ gate: { readiness: { mode: "block" } } });
902+
expect(m.gate.readinessMode).toBe("advisory");
903+
expect(m.gate.present).toBe(true);
904+
expect(m.warnings.some((w) => /gate\.readiness\.mode.*no longer accepts "block"/.test(w))).toBe(true);
905+
// Genuinely invalid values still take the ORIGINAL "must be one of" warning path, unchanged.
906+
const bad = parseFocusManifest({ gate: { readiness: { mode: "sometimes" } } });
907+
expect(bad.gate.readinessMode).toBeNull();
908+
expect(bad.warnings.some((w) => /gate\.readiness\.mode.*must be one of/.test(w))).toBe(true);
909+
expect(bad.warnings.some((w) => /no longer accepts "block"/.test(w))).toBe(false);
910+
});
911+
895912
it("clamps and rounds the readiness minScore to 0-100", () => {
896913
expect(parseFocusManifest({ gate: { readiness: { minScore: 250 } } }).gate.readinessMinScore).toBe(100);
897914
expect(parseFocusManifest({ gate: { readiness: { minScore: -10 } } }).gate.readinessMinScore).toBe(0);
@@ -915,9 +932,9 @@ describe("parseFocusManifest gate config", () => {
915932
});
916933

917934
it("parses the gate section from YAML content", () => {
918-
const m = parseFocusManifestContent("gate:\n duplicates: block\n readiness:\n mode: block\n minScore: 80\n", "repo_file");
935+
const m = parseFocusManifestContent("gate:\n duplicates: block\n readiness:\n mode: advisory\n minScore: 80\n", "repo_file");
919936
expect(m.gate.duplicates).toBe("block");
920-
expect(m.gate.readinessMode).toBe("block");
937+
expect(m.gate.readinessMode).toBe("advisory");
921938
expect(m.gate.readinessMinScore).toBe(80);
922939
});
923940

test/unit/gate-check-policy.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,14 @@ describe(".gittensory.yml settings override (resolveEffectiveSettings)", () => {
5050
it("overlays the friendly gate: alias over DB settings (incl. gate.enabled -> gateCheckMode)", () => {
5151
const eff = resolveEffectiveSettings(
5252
settings({ gateCheckMode: "enabled", linkedIssueGateMode: "advisory", duplicatePrGateMode: "block", qualityGateMode: "off", qualityGateMinScore: 10 }),
53+
// readiness.mode: "block" is downgraded to "advisory" at parse time (#2267) — readiness/quality can
54+
// never hard-block, so this exercises the SAME downgrade flowing through resolveEffectiveSettings.
5355
parseFocusManifest({ gate: { enabled: false, linkedIssue: "block", duplicates: "off", readiness: { mode: "block", minScore: 70 } } }),
5456
);
5557
expect(eff.gateCheckMode).toBe("off"); // gate.enabled: false disables from config
5658
expect(eff.linkedIssueGateMode).toBe("block");
5759
expect(eff.duplicatePrGateMode).toBe("off");
58-
expect(eff.qualityGateMode).toBe("block");
60+
expect(eff.qualityGateMode).toBe("advisory");
5961
expect(eff.qualityGateMinScore).toBe(70);
6062
});
6163

0 commit comments

Comments
 (0)