Skip to content

Commit 702b282

Browse files
committed
fix(review): key gate presentation on the evaluation signal, not check-run publish (#2852)
buildPublicPrIntelligenceComment and buildPublicPrPanelSignalRows derived their local gateEnabled flag from shouldPublishReviewCheck alone, so a reviewCheckMode: disabled repo with autonomy configured would silently downgrade a real, evaluated gate failure to "Advisory only" / "No action" in the public comment and panel row -- even though the same evaluation is authoritative for the autonomous merge/close decision. Both call sites now also treat a configured autonomy policy as enabling gate presentation, mirroring the shouldEvaluateGate policy signal in processors.ts, so the public surface never contradicts the decision the engine actually made.
1 parent 4e8822e commit 702b282

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

src/signals/engine.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { PREFLIGHT_LIMITS } from "./preflight-limits";
3333
import type { UnifiedCollapsible } from "../review/unified-comment";
3434
import { splitAiReviewNits } from "../review/ai-notes";
3535
import { GITTENSORY_GATE_CHECK_NAME, shouldPublishReviewCheck } from "../review/check-names";
36+
import { isAgentConfigured } from "../settings/autonomy";
3637
import { diffFilePriority } from "../review/review-diff";
3738

3839
export type ParticipationLane = "direct_pr" | "issue_discovery" | "split" | "inactive" | "unknown";
@@ -4278,7 +4279,11 @@ export function buildPublicPrIntelligenceComment(args: {
42784279
/* v8 ignore next -- Public findings may omit actions; public comment tests cover sanitized action inclusion. */
42794280
...(publicFindings.length > 0 ? publicFindings.flatMap((finding) => (finding.action ? [finding.action] : [])) : []),
42804281
].filter((step) => !containsPrivatePublicTerm(step));
4281-
const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode);
4282+
// #2852: gate presentation follows the SAME evaluation/policy signal as shouldEvaluateGate in
4283+
// processors.ts (published check-run OR autonomy needs a verdict) -- not the check-run publish flag
4284+
// alone, so a `reviewCheckMode: disabled` repo with autonomy configured still shows its real gate
4285+
// result in the public comment instead of silently downgrading to "no gate at all".
4286+
const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode) || isAgentConfigured(args.settings.autonomy);
42824287
const hardLinkedIssueBlock =
42834288
args.settings.linkedIssueGateMode === "block" && args.pr.linkedIssues.length === 0 && !hasClearNoIssueRationale(args.pr);
42844289
// Duplicate-winner adjudication (#dup-winner): when the flag is ON and this PR is the earliest observed
@@ -4519,7 +4524,10 @@ export function buildPublicPrPanelSignalRows(args: {
45194524
const readiness = buildPublicReadinessScore({ pr: args.pr, preflight: args.preflight, queueHealth: args.queueHealth, linkedDuplicatePrs: visibleLinkedDuplicatePrs, scopedOverlapCount });
45204525
const linkedIssueResult = linkedIssuePanelResult(args.pr);
45214526
const relatedWorkResult = relatedWorkPanelResult(visibleLinkedDuplicatePrs, scopedOverlapCount);
4522-
const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode);
4527+
// #2852: see the matching comment in buildPublicPrIntelligenceComment -- gate presentation must
4528+
// track whether a gate is actually evaluated (check-run published OR autonomy configured), not
4529+
// merely whether the check-run itself is published.
4530+
const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode) || isAgentConfigured(args.settings.autonomy);
45234531
const hardLinkedIssueBlock = args.settings.linkedIssueGateMode === "block" && args.pr.linkedIssues.length === 0 && !hasClearNoIssueRationale(args.pr);
45244532
// Duplicate-winner adjudication (#dup-winner): suppress the earliest known claimant's hard-duplicate block
45254533
// (see the comment builder). Sparse legacy rows fail closed; flag-OFF keeps legacy behavior.

test/unit/signals-coverage.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,44 @@ describe("signal coverage edge cases", () => {
831831
expect(duplicateBlocked.rows.find((r) => r.key === "gateResult")!.cells[1]).not.toBe(providedGate.cells[1]);
832832
});
833833

834+
it("REGRESSION (#2852): reviewCheckMode disabled + autonomy configured still surfaces a real blocking gate result, not 'Advisory only'", () => {
835+
// The gate presentation must key off whether a gate was actually EVALUATED (check-run published OR
836+
// autonomy configured), not merely whether the check-run itself is published -- otherwise a disabled-
837+
// check-run repo that still evaluates the gate for autonomous merge/close would silently hide a real
838+
// blocking verdict from the public comment/panel, contradicting reviews/comments "must still work".
839+
const directRepo = repo("owner/disabled-autonomy");
840+
const collisions = buildCollisionReport(directRepo.fullName, [], []);
841+
const baseArgs = {
842+
repo: directRepo,
843+
pr: pr(directRepo.fullName, 90, "Fix cache", { authorLogin: "miner", linkedIssues: [42], body: "Fixes #42" }),
844+
profile: buildContributorProfile("miner", { login: "miner", topLanguages: ["TypeScript"], source: "github" }, [], []),
845+
detection: { detected: true, source: "official_gittensor_api" as const, reason: "Confirmed.", priorPullRequests: 1, priorMergedPullRequests: 0, priorIssues: 0 },
846+
queueHealth: buildQueueHealth(directRepo, [], [], collisions),
847+
collisions,
848+
preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix cache", body: "Fixes #42", changedFiles: ["src/cache.ts"] }, directRepo, [], []),
849+
settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" as const, autonomy: { merge: "auto" as const } },
850+
gate: { conclusion: "failure" as const, summary: "A configured blocker fired." },
851+
};
852+
853+
const panel = buildPublicPrPanelSignalRows(baseArgs);
854+
const gateRow = panel.rows.find((r) => r.key === "gateResult")!;
855+
expect(gateRow.cells[1]).toBe("❌ Blocking");
856+
expect(gateRow.cells[2]).not.toBe("Advisory only.");
857+
expect(gateRow.cells[3]).not.toBe("No action.");
858+
859+
const comment = buildPublicPrIntelligenceComment(baseArgs);
860+
expect(comment).toContain("Gittensory Orb Review Agent is blocking merge");
861+
expect(comment).toContain("> [!CAUTION]");
862+
863+
// Sanity check: the SAME disabled-check-run repo WITHOUT autonomy configured correctly stays advisory-only
864+
// (no gate evaluation happens at all, so there is nothing real to surface).
865+
const noAutonomySettings = { ...baseArgs.settings, autonomy: {} };
866+
const noAutonomyPanel = buildPublicPrPanelSignalRows({ ...baseArgs, settings: noAutonomySettings, gate: undefined });
867+
const noAutonomyGateRow = noAutonomyPanel.rows.find((r) => r.key === "gateResult")!;
868+
expect(noAutonomyGateRow.cells[2]).toBe("Advisory only.");
869+
expect(noAutonomyGateRow.cells[3]).toBe("No action.");
870+
});
871+
834872
it("#dup-winner: panel hard-duplicate block is suppressed for the winner, kept for the loser, byte-identical when flag OFF", () => {
835873
const directRepo = repo("owner/dupwin");
836874
const dupIssue = issue(directRepo.fullName, 42, "Cache invalidation race");

0 commit comments

Comments
 (0)