Skip to content

Commit 78891b8

Browse files
authored
fix(gate): default the three bare gateMode() calls in advisory.ts to advisory (#10245)
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. Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>
1 parent 00e3ee9 commit 78891b8

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
@@ -1566,7 +1566,7 @@ export async function recordGateScoreSignals(
15661566
const occurredAt = nowIso();
15671567
const writes: Promise<void>[] = [];
15681568

1569-
const slopMode = gateMode(effective.slopGateMode);
1569+
const slopMode = gateMode(effective.slopGateMode ?? "advisory");
15701570
const slopRisk = normalizeScore(effective.slopRisk);
15711571
if (slopMode === "block" && slopRisk !== null) {
15721572
const slopMin = normalizeScore(effective.slopGateMinScore) ?? DEFAULT_SLOP_BLOCK_THRESHOLD;
@@ -1586,7 +1586,7 @@ export async function recordGateScoreSignals(
15861586
);
15871587
}
15881588

1589-
const qualityMode = gateMode(effective.qualityGateMode);
1589+
const qualityMode = gateMode(effective.qualityGateMode ?? "advisory");
15901590
const readinessScore = normalizeScore(effective.readinessScore);
15911591
const qualityMin = normalizeScore(effective.qualityGateMinScore);
15921592
if (qualityMode !== "off" && readinessScore !== null && qualityMin !== null) {
@@ -1610,7 +1610,7 @@ export async function recordGateScoreSignals(
16101610
}
16111611

16121612
function buildQualityGateWarning(policy: GateCheckPolicy): AdvisoryFinding | null {
1613-
if (gateMode(policy.qualityGateMode) === "off") return null;
1613+
if (gateMode(policy.qualityGateMode ?? "advisory") === "off") return null;
16141614
const score = normalizeScore(policy.readinessScore);
16151615
const minScore = normalizeScore(policy.qualityGateMinScore);
16161616
if (score === null || minScore === null || score >= minScore) return null;
@@ -1644,8 +1644,8 @@ function buildSlopGateBlocker(policy: GateCheckPolicy): AdvisoryFinding | null {
16441644
}
16451645

16461646
// #9167: fail CLOSED on a value that isn't one of the three real modes, matching the rest of this
1647-
// codebase's fail-closed defaults -- every legitimate caller already supplies its own `?? "advisory"`
1648-
// default before reaching here (see every `gateMode(policy.xGateMode ?? "advisory")` call site above), so
1647+
// codebase's fail-closed defaults -- every call site now supplies its own `?? "advisory"` default
1648+
// before reaching here (see every `gateMode(policy.xGateMode ?? "advisory")` call site above), so
16491649
// this branch is only ever reached for a truly malformed value (e.g. a caller that bypassed
16501650
// GateRuleMode's compile-time union via an untyped/JSON-decoded config). Previously coerced to
16511651
// "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)