Skip to content

Commit 83d8b40

Browse files
authored
fix(review): preserve generic gate warnings on the surface-close hold path (#10100)
applySurfaceGate's fourth merge path returned the surface evaluation bare when the generic gate held with no blockers, dropping every warning-carried hold reason (size, guardrail, secret-scan) instead of unioning them like the three sibling paths already do. Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>
1 parent 3b2dd1e commit 83d8b40

2 files changed

Lines changed: 33 additions & 8 deletions

File tree

src/review/content-lane-wire.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ export function applySurfaceGate(
139139
if (generic.blockers.length === 0 && generic.conclusion === "success") return surface; // generic was clean → surface stands
140140
if (generic.blockers.length === 0) {
141141
if (surface.conclusion === "success") return generic;
142-
return surface;
142+
return { ...surface, warnings: [...generic.warnings, ...surface.warnings] };
143143
}
144144
// #3907: opt-in escape hatch from guard #3 below. Default (null/undefined/"advisory") preserves today's
145145
// behavior byte-identically. "gate" skips the override entirely, so an AI-judgment-only failure falls

test/unit/content-lane-wire.test.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,39 @@ describe("applySurfaceGate", () => {
9292

9393
expect(applySurfaceGate(genericHold, surfaceMerge)).toBe(genericHold);
9494
});
95-
it("lets a surface hard failure override a generic warning-only hold", () => {
96-
const genericHold = gate({
97-
conclusion: "neutral",
98-
blockers: [],
99-
warnings: [{ code: "oversized_pr", title: "Large change", severity: "warning", detail: "large" }],
100-
});
95+
it("lets a surface hard failure override a generic warning-only hold, preserving the generic gate's hold warnings", () => {
96+
const oversizedPrFinding: AdvisoryFinding = { code: "oversized_pr", title: "Large change", severity: "warning", detail: "large" };
97+
const genericHold = gate({ conclusion: "neutral", blockers: [], warnings: [oversizedPrFinding] });
98+
99+
const out = applySurfaceGate(genericHold, surfaceClose);
100+
expect(out?.conclusion).toBe("failure");
101+
expect(out?.blockers).toEqual(surfaceClose.blockers);
102+
expect(out?.warnings).toEqual([oversizedPrFinding]);
103+
});
104+
it("the unchanged sub-case: a success generic with warnings still returns the surface verbatim", () => {
105+
const genericSuccess = gate({ conclusion: "success", blockers: [], warnings: [{ code: "quality_readiness_low", title: "Readiness is low", severity: "warning", detail: "" }] });
106+
expect(applySurfaceGate(genericSuccess, surfaceClose)).toBe(surfaceClose);
107+
});
108+
it("the unchanged sub-case: a holding generic against a success surface still returns the generic verbatim", () => {
109+
const oversizedPrFinding: AdvisoryFinding = { code: "oversized_pr", title: "Large change", severity: "warning", detail: "large" };
110+
const genericHold = gate({ conclusion: "neutral", blockers: [], warnings: [oversizedPrFinding] });
111+
const surfaceMerge = gate({ conclusion: "success", title: "Surface", summary: "valid entry" });
112+
expect(applySurfaceGate(genericHold, surfaceMerge)).toBe(genericHold);
113+
});
114+
it("REGRESSION (#10011): preserves the generic gate's hold warnings when a non-success surface verdict overrides it", () => {
115+
const oversizedPrFinding: AdvisoryFinding = { code: "oversized_pr", title: "Large change", severity: "warning", detail: "large" };
116+
const genericHold = gate({ conclusion: "neutral", blockers: [], warnings: [oversizedPrFinding] });
117+
118+
const manual = surfaceVerdictToGate({ verdict: "manual", summary: "auth declared" }).evaluation;
119+
const outManual = applySurfaceGate(genericHold, manual);
120+
expect(outManual?.conclusion).toBe("neutral");
121+
expect(outManual?.warnings.map((w) => w.code)).toEqual(["oversized_pr", "surface_lane_manual"]);
101122

102-
expect(applySurfaceGate(genericHold, surfaceClose)).toBe(surfaceClose);
123+
const close = surfaceVerdictToGate({ verdict: "close", summary: "bad entry" }).evaluation;
124+
const outClose = applySurfaceGate(genericHold, close);
125+
expect(outClose?.conclusion).toBe("failure");
126+
expect(outClose?.blockers.map((b) => b.code)).toEqual(["surface_lane_reject"]);
127+
expect(outClose?.warnings.map((w) => w.code)).toEqual(["oversized_pr"]);
103128
});
104129
it("PRESERVES a generic hard blocker over a surface merge (a committed secret can never merge)", () => {
105130
const secret: AdvisoryFinding = { code: "secret_leak", title: "Secret", severity: "critical", detail: "leaked key" };

0 commit comments

Comments
 (0)