Skip to content

Commit 4ec8ce7

Browse files
committed
fix(gate): default the three bare gateMode() calls in advisory.ts to advisory
recordGateScoreSignals and buildQualityGateWarning called gateMode() bare on an optional mode field. Since #9167 made gateMode's fallback fail-closed to "block", an absent slopGateMode resolved to "block" there while buildSlopGateBlocker (the pure evaluator it's documented to mirror) already defaulted the same absent value to "advisory" via gateMode(policy.slopGateMode ?? "advisory") -- so an unconfigured slop gate still wrote a slop_gate_score corpus signal labeled "mode block" that the gate itself never evaluated. Give all three call sites the same ?? "advisory" default and correct the gateMode doc comment, which claimed every caller already supplied that default when three sites in the same file didn't.
1 parent 79d7e03 commit 4ec8ce7

2 files changed

Lines changed: 29 additions & 5 deletions

File tree

src/rules/advisory.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,7 +1557,7 @@ export async function recordGateScoreSignals(
15571557
const occurredAt = nowIso();
15581558
const writes: Promise<void>[] = [];
15591559

1560-
const slopMode = gateMode(effective.slopGateMode);
1560+
const slopMode = gateMode(effective.slopGateMode ?? "advisory");
15611561
const slopRisk = normalizeScore(effective.slopRisk);
15621562
if (slopMode === "block" && slopRisk !== null) {
15631563
const slopMin = normalizeScore(effective.slopGateMinScore) ?? DEFAULT_SLOP_BLOCK_THRESHOLD;
@@ -1577,7 +1577,7 @@ export async function recordGateScoreSignals(
15771577
);
15781578
}
15791579

1580-
const qualityMode = gateMode(effective.qualityGateMode);
1580+
const qualityMode = gateMode(effective.qualityGateMode ?? "advisory");
15811581
const readinessScore = normalizeScore(effective.readinessScore);
15821582
const qualityMin = normalizeScore(effective.qualityGateMinScore);
15831583
if (qualityMode !== "off" && readinessScore !== null && qualityMin !== null) {
@@ -1601,7 +1601,7 @@ export async function recordGateScoreSignals(
16011601
}
16021602

16031603
function buildQualityGateWarning(policy: GateCheckPolicy): AdvisoryFinding | null {
1604-
if (gateMode(policy.qualityGateMode) === "off") return null;
1604+
if (gateMode(policy.qualityGateMode ?? "advisory") === "off") return null;
16051605
const score = normalizeScore(policy.readinessScore);
16061606
const minScore = normalizeScore(policy.qualityGateMinScore);
16071607
if (score === null || minScore === null || score >= minScore) return null;
@@ -1635,8 +1635,8 @@ function buildSlopGateBlocker(policy: GateCheckPolicy): AdvisoryFinding | null {
16351635
}
16361636

16371637
// #9167: fail CLOSED on a value that isn't one of the three real modes, matching the rest of this
1638-
// codebase's fail-closed defaults -- every legitimate caller already supplies its own `?? "advisory"`
1639-
// default before reaching here (see every `gateMode(policy.xGateMode ?? "advisory")` call site above), so
1638+
// codebase's fail-closed defaults -- every call site now supplies its own `?? "advisory"` default
1639+
// before reaching here (see every `gateMode(policy.xGateMode ?? "advisory")` call site above), so
16401640
// this branch is only ever reached for a truly malformed value (e.g. a caller that bypassed
16411641
// GateRuleMode's compile-time union via an untyped/JSON-decoded config). Previously coerced to
16421642
// "advisory" -- a fail-OPEN default in a codebase whose other defaults are carefully fail-closed. This is

test/unit/configured-gate-blocker-signals.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,16 @@ describe("recordGateScoreSignals (#8223)", () => {
331331
expect((await createSignalStore(env).queryRuleHistory("slop_gate_score", 0)).fired).toEqual([]);
332332
});
333333

334+
it("records NOTHING for slop when slopGateMode is unset — mirrors buildSlopGateBlocker's own advisory default (#10015)", async () => {
335+
// Regression: gateMode(effective.slopGateMode) used to bare-call gateMode without the `?? "advisory"`
336+
// default, so an unset slopGateMode fail-closed to "block" here while buildSlopGateBlocker (the pure
337+
// evaluator this capture is documented to mirror) resolved the same unset value to "advisory" and never
338+
// evaluated. That mismatch wrote a "mode block" signal for a threshold comparison the gate never made.
339+
const env = createTestEnv();
340+
await recordGateScoreSignals(env, { slopRisk: 72, slopGateMinScore: 60 }, "owner/repo", 7);
341+
expect((await createSignalStore(env).queryRuleHistory("slop_gate_score", 0)).fired).toEqual([]);
342+
});
343+
334344
it("fires quality_gate_score in advisory mode too — pass AND fail evaluations both leave corpus evidence", async () => {
335345
const env = createTestEnv();
336346
await recordGateScoreSignals(env, { qualityGateMode: "advisory", readinessScore: 80, qualityGateMinScore: 70 }, "owner/repo", 7);
@@ -349,6 +359,20 @@ describe("recordGateScoreSignals (#8223)", () => {
349359
expect((await createSignalStore(env).queryRuleHistory("quality_gate_score", 0)).fired).toEqual([]);
350360
});
351361

362+
it("still fires quality_gate_score when qualityGateMode is unset — the #10015 default fix doesn't change this path's outcome", async () => {
363+
// qualityMode resolves to "advisory" now instead of the old bare-call "block", but the firing condition
364+
// is only `!== "off"` either way, so an unset qualityGateMode still records — pinning that the fix is
365+
// scoped to the mismatch (slop's block-only firing condition) and doesn't regress quality's capture.
366+
const env = createTestEnv();
367+
await recordGateScoreSignals(env, { readinessScore: 40, qualityGateMinScore: 70 }, "owner/repo", 7);
368+
const history = await createSignalStore(env).queryRuleHistory("quality_gate_score", 0);
369+
expect(history.fired).toHaveLength(1);
370+
expect(history.fired[0]).toMatchObject({
371+
outcome: "below_threshold",
372+
metadata: { rawSignal: "public readiness score 40/100 vs threshold 70/100 (mode advisory)" },
373+
});
374+
});
375+
352376
it("degrades silently when the SignalStore write rejects — the call resolves normally", async () => {
353377
vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({
354378
recordRuleFired: async () => {

0 commit comments

Comments
 (0)