From 47bf0303f737448fb6a0e69b5bc2da2c40944d64 Mon Sep 17 00:00:00 2001 From: bohdansolovie <153934212+bohdansolovie@users.noreply.github.com> Date: Mon, 6 Jul 2026 08:14:07 +0200 Subject: [PATCH 1/2] feat(review): add read-only auto-merge conditions table via review.auto_merge_summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaces CI, gate, merge-state, and linked-issue readiness as a collapsible summary in the unified comment when opted in — display-only, default off. Fixes #2051 --- .gittensory.yml.example | 8 + config/examples/gittensory.full.yml | 8 + src/queue/processors.ts | 3 + src/review/unified-comment-bridge.ts | 115 +++++++++++- src/review/unified-comment.ts | 27 +++ src/signals/focus-manifest.ts | 20 ++- .../auto-merge-summary-collapsible.test.ts | 129 ++++++++++++++ test/unit/focus-manifest.test.ts | 27 ++- test/unit/queue.test.ts | 165 ++++++++++++++++++ test/unit/signals-coverage.test.ts | 2 +- 10 files changed, 492 insertions(+), 12 deletions(-) create mode 100644 test/unit/auto-merge-summary-collapsible.test.ts diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 88497ddc07..e59c19d69b 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -410,6 +410,10 @@ review: # finding_categories: false # When true, inline findings are tagged with a category label (requires inline_comments). + # Read-only auto-merge conditions table (#2051): when enabled, the unified review comment includes a + # collapsible summary of CI/gate/merge-state/linked-issue readiness. Does NOT change merge decisions. + # auto_merge_summary: false + # Deterministic label suggestions (#2045). Each rule SUGGESTS a non-scoring label when a PR matches ALL of the # `when` criteria it sets (at least one is required): when_paths (any changed path matches a glob), title_contains, # description_contains (both case-insensitive). Suggestions are advisory; they are auto-applied only when the repo's @@ -783,6 +787,10 @@ settings: # # compact "review effort: N/5 (~M min)" chip -- a deterministic, no-AI complexity/time estimate from the # # changed files' added-line volume and file-type mix. Bool or null. Default: null/false. # effort_score: false +# # When true, the unified review comment gains a read-only "Auto-merge conditions" collapsible: a table of +# # which merge prerequisites currently pass/fail (CI green, gate passing, mergeable-clean, valid linked issue). +# # Surface only — does NOT change the merge/close decision. Bool or null. Default: null/false. +# auto_merge_summary: false # # When true, an inline finding is ALSO tagged with a category (security/correctness/performance/ # # maintainability/tests/style) -- the AI reviewer self-categorizes, with a deterministic path/keyword # # fallback for whatever it omits. Only takes effect when inline_comments is already on. Bool or null. diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 78ef7c062b..75287eb446 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -423,6 +423,10 @@ review: # finding_categories: false # When true, inline findings are tagged with a category label (requires inline_comments). + # Read-only auto-merge conditions table (#2051): when enabled, the unified review comment includes a + # collapsible summary of CI/gate/merge-state/linked-issue readiness. Does NOT change merge decisions. + # auto_merge_summary: false + # Deterministic label suggestions (#2045). Each rule SUGGESTS a non-scoring label when a PR matches ALL of the # `when` criteria it sets (at least one is required): when_paths (any changed path matches a glob), title_contains, # description_contains (both case-insensitive). Suggestions are advisory; they are auto-applied only when the repo's @@ -796,6 +800,10 @@ settings: # # compact "review effort: N/5 (~M min)" chip -- a deterministic, no-AI complexity/time estimate from the # # changed files' added-line volume and file-type mix. Bool or null. Default: null/false. # effort_score: false +# # When true, the unified review comment gains a read-only "Auto-merge conditions" collapsible: a table of +# # which merge prerequisites currently pass/fail (CI green, gate passing, mergeable-clean, valid linked issue). +# # Surface only — does NOT change the merge/close decision. Bool or null. Default: null/false. +# auto_merge_summary: false # # When true, an inline finding is ALSO tagged with a category (security/correctness/performance/ # # maintainability/tests/style) -- the AI reviewer self-categorizes, with a deterministic path/keyword # # fallback for whatever it omits. Only takes effect when inline_comments is already on. Bool or null. diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 9f50092797..b53dfb8dcc 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7684,6 +7684,7 @@ async function maybePublishPrPublicSurface( let suggestionsEnabledForReview = false; let changedFilesSummaryEnabledForReview = false; let effortScoreEnabledForReview = false; + let autoMergeSummaryEnabledForReview = false; let findingCategoriesEnabledForReview = false; let minFindingSeverityForReview: ReviewFindingSeverity | null = null; let aiReviewExpected = false; @@ -8190,6 +8191,7 @@ async function maybePublishPrPublicSurface( changedFilesSummaryEnabledForReview = deterministicReviewOverrides.changedFilesSummary; effortScoreEnabledForReview = deterministicReviewOverrides.effortScore; minFindingSeverityForReview = deterministicReviewOverrides.minFindingSeverity; + autoMergeSummaryEnabledForReview = deterministicReviewOverrides.autoMergeSummary; const aiReviewWillRun = !authorBlacklisted && !isFrozenForManualReview && @@ -9425,6 +9427,7 @@ async function maybePublishPrPublicSurface( ), } : {}), + ...(autoMergeSummaryEnabledForReview ? { autoMergeSummary: true } : {}), ...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length ? { findingCategories: aiReview.inlineFindings } : {}), diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 5b7faf1f48..7fa117d13d 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -31,7 +31,9 @@ import { classifyChangedFile, type ReviewFileClass } from "./changed-files-class import { classifyFindingCategory, FINDING_CATEGORIES, type FindingCategory } from "./finding-category-classify"; import { buildUnifiedReviewInput, + buildAutoMergeSummaryCollapsible, renderUnifiedReviewComment, + type AutoMergeConditionRow, type DualReviewNote, type MergeReadiness, type ReviewNotes, @@ -325,6 +327,11 @@ export type UnifiedCommentBridgeArgs = { * `classifyFindingCategory` — never omitted from the count. Default OFF (the processor passes this only when * the manifest opts in — see `resolveReviewPromptOverrides`'s `findingCategories`). (#1958) */ findingCategories?: FindingCategoryInput[] | undefined; + /** When true, append a read-only "Auto-merge conditions" collapsible built from the ALREADY-computed gate, + * merge-readiness, and panel signal rows passed into this call. Display-only — does NOT change the merge/close + * decision. Default OFF (the processor passes this only when the manifest opts in — see + * `resolveReviewPromptOverrides`'s `autoMergeSummary`). (#2051) */ + autoMergeSummary?: boolean | undefined; /** The disposition holds this PR for owner review because its diff touches a hard-guardrail path — so an * otherwise-ready comment renders "held for review" instead of "safe to merge". (#guarded-hold-comment) */ heldForReview?: boolean | undefined; @@ -515,6 +522,99 @@ export function buildFindingCategoryCollapsible(findings: FindingCategoryInput[] return { title: "Finding categories", body }; } +/** Map a legacy panel result cell's leading status icon (✅/⚠️/❌) → an auto-merge condition state. */ +function panelResultToConditionState(resultCell: string): AutoMergeConditionRow["state"] { + return rowState(resultCell); +} + +/** Strip the leading status icon from a panel result cell for human-readable evidence text. */ +function panelResultEvidence(resultCell: string, detailCell: string): string { + const detail = detailCell.trim(); + const result = rowResultText(resultCell); + if (detail && result) return `${result} — ${detail}`; + return detail || result || "No details."; +} + +/** + * Derive the four standard auto-merge condition rows from ALREADY-computed readiness signals — the gate + * evaluation, merge-readiness facts, and legacy panel rows the caller already resolved. Pure + read-only: + * does NOT call `deriveUnifiedStatus` or any merge/close decision path. (#2051) + */ +export function deriveAutoMergeConditionsFromSignals(args: { + gate: GateCheckEvaluation; + mergeReadiness?: MergeReadiness | undefined; + panelRows: PublicPrPanelSignalRow[]; +}): AutoMergeConditionRow[] { + const linkedIssueRow = args.panelRows.find((row) => row.key === "linkedIssue"); + const gateResultRow = args.panelRows.find((row) => row.key === "gateResult"); + + const ciState = args.mergeReadiness?.ciState; + const ciRow: AutoMergeConditionRow = { + condition: "CI green", + state: ciState === "passed" ? "ok" : ciState === "failed" ? "fail" : "warn", + evidence: + ciState === "passed" + ? "All required CI checks are green." + : ciState === "failed" + ? args.mergeReadiness?.failingChecks?.length + ? `Failing: ${args.mergeReadiness.failingChecks.join(", ")}.` + : "CI checks are failing." + : ciState === "unverified" + ? "CI is pending or unverified." + : "CI state was not resolved.", + }; + + const gateEnabled = args.gate.enabled; + const gateConclusion = args.gate.conclusion; + const gateRow: AutoMergeConditionRow = gateResultRow + ? { + condition: "Gate passing", + state: panelResultToConditionState(gateResultRow.cells[1] ?? ""), + evidence: panelResultEvidence(gateResultRow.cells[1] ?? "", gateResultRow.cells[2] ?? ""), + } + : { + condition: "Gate passing", + state: !gateEnabled ? "warn" : gateConclusion === "success" ? "ok" : gateConclusion === "failure" ? "fail" : "warn", + evidence: !gateEnabled + ? "Gate is advisory-only (not enforcing)." + : gateConclusion === "success" + ? "No configured hard blocker found." + : gateConclusion === "failure" + ? "Repo-configured hard blocker found." + : gateConclusion === "action_required" + ? "Install/config needs attention." + : "Gate is not blocking this PR.", + }; + + const mergeState = args.mergeReadiness?.mergeStateLabel?.trim().toLowerCase(); + const mergeRow: AutoMergeConditionRow = { + condition: "Mergeable / clean", + state: + mergeState === "dirty" || mergeState === "behind" + ? "fail" + : mergeState + ? "ok" + : "warn", + evidence: mergeState + ? `GitHub merge state: ${args.mergeReadiness?.mergeStateLabel}.` + : "Merge state was not resolved.", + }; + + const linkedIssueRowResult: AutoMergeConditionRow = linkedIssueRow + ? { + condition: "Valid linked issue", + state: panelResultToConditionState(linkedIssueRow.cells[1] ?? ""), + evidence: panelResultEvidence(linkedIssueRow.cells[1] ?? "", linkedIssueRow.cells[2] ?? ""), + } + : { + condition: "Valid linked issue", + state: "warn", + evidence: "Linked issue signal was not resolved.", + }; + + return [ciRow, gateRow, mergeRow, linkedIssueRowResult]; +} + /** * Build the unified PR-review comment body from gittensory's live data. Returns a string that STARTS with * the panel marker (so the existing upsert updates in place) followed by the rendered unified comment. @@ -569,6 +669,19 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string const visibleRows = args.panelRows.filter((row) => args.reviewFields?.[row.key] !== false); const signals = panelRowsToSignalRows(visibleRows); + // review.auto_merge_summary (#2051): read-only conditions table from signals the caller already computed. + const autoMergeCollapsible = args.autoMergeSummary + ? buildAutoMergeSummaryCollapsible( + deriveAutoMergeConditionsFromSignals({ + gate: args.gate, + mergeReadiness: args.mergeReadiness, + panelRows: args.panelRows, + }), + ) + : null; + const withAutoMerge = + autoMergeCollapsible !== null ? [autoMergeCollapsible, ...(args.extraCollapsibles ?? [])] : args.extraCollapsibles; + // review.changed_files_summary port: when the manifest opts in, the processor hands us every changed file's // path + deltas here; append the grouped "Changed files" collapsible ahead of the visual preview (structure // before pixels). Flag-OFF (the processor passes undefined) ⇒ extraCollapsibles is unchanged. (#1957) @@ -577,7 +690,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string ? buildChangedFilesSummaryCollapsible(args.changedFilesSummary) : null; const withChangedFiles = - changedFilesCollapsible !== null ? [...(args.extraCollapsibles ?? []), changedFilesCollapsible] : args.extraCollapsibles; + changedFilesCollapsible !== null ? [...(withAutoMerge ?? []), changedFilesCollapsible] : withAutoMerge; // review.finding_categories port: when the manifest opts in, the processor hands us this review's line-anchored // AI findings here; append the "Finding categories" collapsible right after Changed files (both are structural // review-shape summaries, ahead of the visual preview). Flag-OFF (the processor passes undefined) ⇒ diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index de3632463c..2dc258612b 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -222,6 +222,33 @@ export interface UnifiedCommentContext { reviewedAt?: string | number | Date | undefined; } +/** One read-only auto-merge condition row — pre-computed by the bridge from already-resolved readiness + * signals; this renderer only formats them, never re-derives a merge/close decision. (#2051) */ +export type AutoMergeConditionRow = { + condition: string; + state: UnifiedSignalRow["state"]; + evidence: string; +}; + +/** Build the "Auto-merge conditions" collapsible from pre-computed condition rows. Pure + display-only — + * does NOT call `deriveUnifiedStatus` or any merge/close decision path. Returns null when empty. (#2051) */ +export function buildAutoMergeSummaryCollapsible(conditions: AutoMergeConditionRow[]): UnifiedCollapsible | null { + if (conditions.length === 0) return null; + const lines = conditions.map((row) => { + const label = escapePublicHtmlAngles(row.condition); + const evidence = escapePublicHtmlAngles(row.evidence); + return `| ${label} | ${SIGNAL_ICON[row.state]} | ${evidence} |`; + }); + const body = [ + "| Condition | Status | Evidence |", + "|---|---|---|", + ...lines, + "", + "_Read-only summary of conditions that must pass before auto-merge. Does not change the merge decision._", + ].join("\n"); + return { title: "Auto-merge conditions", body }; +} + const STATUS_META: Record = { ready: { alert: "TIP", square: "🟩", icon: "✅" }, advisory: { alert: "NOTE", square: "🟦", icon: "💡" }, diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index e96b52a971..6b20e3c377 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -353,6 +353,11 @@ export type FocusManifestReviewConfig = { * configured level are suppressed from inline comments — never from gate blockers. null (default, absent) ⇒ every * finding shown = byte-identical behavior. (#2048) */ minFindingSeverity: ReviewFindingSeverity | null; + /** `review.auto_merge_summary`: when true, the unified review comment gains a read-only collapsible table of + * which auto-merge conditions currently pass/fail (CI green, gate passing, mergeable-clean, valid linked + * issue). Surface only; does NOT change the merge/close DECISION. null/false (default, absent) = no table = + * byte-identical behavior. (#2051) */ + autoMergeSummary: boolean | null; /** `review.path_instructions`: per-path natural-language guidance handed to the AI reviewer when the PR's * changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. (#review-path-instructions) */ pathInstructions: ReviewPathInstruction[]; @@ -684,7 +689,7 @@ const EMPTY_MANIFEST: FocusManifest = { publicNotes: [], gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, autoMergeSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -714,7 +719,7 @@ function emptyManifest(source: FocusManifestSource, warnings: string[] = []): Fo warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, autoMergeSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -1665,7 +1670,7 @@ function parsePublicSafeText(value: JsonValue | undefined, field: string, warnin * throws; invalid/unsafe values are dropped with warnings. */ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): FocusManifestReviewConfig { - const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }; + const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, autoMergeSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }; if (value === undefined || value === null) return empty; if (typeof value !== "object" || Array.isArray(value)) { warnings.push(`Manifest field "review" must be a mapping; ignoring it.`); @@ -1712,6 +1717,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo REVIEW_FINDING_SEVERITY_LADDER, warnings, ); + const autoMergeSummary = normalizeOptionalBoolean(r.auto_merge_summary, "review.auto_merge_summary", warnings); const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings); const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings); const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings); @@ -1735,6 +1741,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo effortScore !== null || findingCategories !== null || minFindingSeverity !== null || + autoMergeSummary !== null || pathInstructions.length > 0 || instructions !== null || excludePaths.length > 0 || @@ -1764,6 +1771,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo effortScore, findingCategories, minFindingSeverity, + autoMergeSummary, pathInstructions, instructions, excludePaths, @@ -2208,6 +2216,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue if (review.effortScore !== null) out.effort_score = review.effortScore; if (review.findingCategories !== null) out.finding_categories = review.findingCategories; if (review.minFindingSeverity !== null) out.min_finding_severity = review.minFindingSeverity; + if (review.autoMergeSummary !== null) out.auto_merge_summary = review.autoMergeSummary; if (review.instructions !== null) out.instructions = review.instructions; if (review.pathInstructions.length > 0) out.path_instructions = review.pathInstructions.map((entry) => ({ path: entry.path, instructions: entry.instructions })); if (review.excludePaths.length > 0) out.exclude_paths = [...review.excludePaths]; @@ -2423,7 +2432,7 @@ export function composeManifestReviewInstructions(instructions: string | null, t * failure). A null manifest yields the byte-identical defaults. Centralized so the AI-review caller threads them * in one place with the null-manifest branch covered here (unit-tested) rather than inline in the processor. * (#review-profile / #review-tone / #review-security-focus / #review-path-instructions / #review-exclude-paths / #2043 / #selfhost-ai-model-override / #1956) */ -export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; tone: string | null; securityFocus: boolean; inlineComments: boolean; suggestions: boolean; changedFilesSummary: boolean; effortScore: boolean; findingCategories: boolean; minFindingSeverity: ReviewFindingSeverity | null; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[]; pathFilters: string[]; selfHostAiModel: SelfHostAiModelConfig } { +export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; tone: string | null; securityFocus: boolean; inlineComments: boolean; suggestions: boolean; changedFilesSummary: boolean; effortScore: boolean; findingCategories: boolean; minFindingSeverity: ReviewFindingSeverity | null; autoMergeSummary: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[]; pathFilters: string[]; selfHostAiModel: SelfHostAiModelConfig } { // inlineComments resolves to a strict boolean — true ONLY when the manifest explicitly set review.inline_comments: // true; null/false/absent ⇒ false. The caller ANDs this per-repo toggle with the operator flag + cutover allowlist. // securityFocus resolves the same way — true ONLY when the manifest explicitly set review.security_focus: true. @@ -2435,7 +2444,8 @@ export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { // (never touches the AI prompt) and only needs the unified-comment convergence feature to be on. // findingCategories resolves the same way (#1958) — like suggestions, the caller further ANDs it with the // already-resolved inlineComments gate, since a category has nothing to categorize without an inline finding. - return { profile: manifest?.review.profile ?? null, tone: manifest?.review.tone ?? null, securityFocus: manifest?.review.securityFocus === true, inlineComments: manifest?.review.inlineComments === true, suggestions: manifest?.review.suggestions === true, changedFilesSummary: manifest?.review.changedFilesSummary === true, effortScore: manifest?.review.effortScore === true, findingCategories: manifest?.review.findingCategories === true, minFindingSeverity: manifest?.review.minFindingSeverity ?? null, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) }; + // autoMergeSummary resolves the same way (#2051) — read-only display; never touches the merge/close decision. + return { profile: manifest?.review.profile ?? null, tone: manifest?.review.tone ?? null, securityFocus: manifest?.review.securityFocus === true, inlineComments: manifest?.review.inlineComments === true, suggestions: manifest?.review.suggestions === true, changedFilesSummary: manifest?.review.changedFilesSummary === true, effortScore: manifest?.review.effortScore === true, findingCategories: manifest?.review.findingCategories === true, minFindingSeverity: manifest?.review.minFindingSeverity ?? null, autoMergeSummary: manifest?.review.autoMergeSummary === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) }; } /** Resolve `review.pre_merge_checks` from a possibly-null manifest (null = load failure ⇒ no checks). Centralized diff --git a/test/unit/auto-merge-summary-collapsible.test.ts b/test/unit/auto-merge-summary-collapsible.test.ts new file mode 100644 index 0000000000..8dca757669 --- /dev/null +++ b/test/unit/auto-merge-summary-collapsible.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it, vi } from "vitest"; +import { + buildUnifiedCommentBody, + deriveAutoMergeConditionsFromSignals, +} from "../../src/review/unified-comment-bridge"; +import * as unifiedComment from "../../src/review/unified-comment"; +import type { GateCheckEvaluation } from "../../src/rules/advisory"; +import type { PublicPrPanelSignalRow } from "../../src/signals/engine"; + +function gate(over: Partial = {}): GateCheckEvaluation { + return { + enabled: true, + conclusion: "success", + title: "Gittensory Orb Review Agent passed", + summary: "No configured hard blocker was found.", + blockers: [], + warnings: [], + ...over, + }; +} + +const panelRowsPassing: PublicPrPanelSignalRow[] = [ + { key: "linkedIssue", cells: ["Linked issue", "✅ Linked", "#42", "No action."] }, + { key: "gateResult", cells: ["Gate result", "✅ Passing", "No configured blocker found.", "No action."] }, +]; + +const footer = "💰 Earn for open-source contributions. Checked by Gittensory."; + +describe("buildAutoMergeSummaryCollapsible", () => { + it("renders a four-row read-only conditions table", () => { + const conditions = deriveAutoMergeConditionsFromSignals({ + gate: gate(), + mergeReadiness: { ciState: "passed", mergeStateLabel: "clean" }, + panelRows: panelRowsPassing, + }); + const c = unifiedComment.buildAutoMergeSummaryCollapsible(conditions); + expect(c).not.toBeNull(); + expect(c?.title).toBe("Auto-merge conditions"); + expect(c?.body).toContain("| Condition | Status | Evidence |"); + expect(c?.body).toContain("| CI green | ✅ |"); + expect(c?.body).toContain("| Gate passing | ✅ |"); + expect(c?.body).toContain("| Mergeable / clean | ✅ |"); + expect(c?.body).toContain("| Valid linked issue | ✅ |"); + expect(c?.body).toContain("Does not change the merge decision"); + }); + + it("returns null for an empty condition list", () => { + expect(unifiedComment.buildAutoMergeSummaryCollapsible([])).toBeNull(); + }); +}); + +describe("deriveAutoMergeConditionsFromSignals", () => { + it("maps failing CI, gate failure, dirty merge state, and missing linked issue to fail/warn states", () => { + const rows = deriveAutoMergeConditionsFromSignals({ + gate: gate({ conclusion: "failure" }), + mergeReadiness: { + ciState: "failed", + mergeStateLabel: "dirty", + failingChecks: ["codecov/patch"], + }, + panelRows: [ + { key: "linkedIssue", cells: ["Linked issue", "⚠️ Missing", "No linked issue or no-issue rationale found.", "Explain no-issue PR."] }, + { key: "gateResult", cells: ["Gate result", "❌ Blocking", "Repo-configured hard blocker found.", "Fix blocker."] }, + ], + }); + expect(rows.map((row) => row.state)).toEqual(["fail", "fail", "fail", "warn"]); + expect(rows[0]?.evidence).toContain("codecov/patch"); + expect(rows[2]?.evidence).toContain("dirty"); + }); + + it("falls back to gate fields when the gateResult panel row is absent", () => { + const rows = deriveAutoMergeConditionsFromSignals({ + gate: gate({ enabled: false, conclusion: "skipped" }), + mergeReadiness: { ciState: "unverified" }, + panelRows: [], + }); + expect(rows.find((row) => row.condition === "Gate passing")?.state).toBe("warn"); + expect(rows.find((row) => row.condition === "CI green")?.state).toBe("warn"); + expect(rows.find((row) => row.condition === "Mergeable / clean")?.state).toBe("warn"); + expect(rows.find((row) => row.condition === "Valid linked issue")?.state).toBe("warn"); + }); + + it("does not invoke deriveUnifiedStatus — display-only derivation from pre-computed signals (#2051)", () => { + const spy = vi.spyOn(unifiedComment, "deriveUnifiedStatus"); + deriveAutoMergeConditionsFromSignals({ + gate: gate(), + mergeReadiness: { ciState: "passed", mergeStateLabel: "clean" }, + panelRows: panelRowsPassing, + }); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); +}); + +describe("buildUnifiedCommentBody autoMergeSummary wiring (#2051)", () => { + const base = { + gate: gate(), + panelRows: panelRowsPassing, + readinessTotal: 90, + changedFiles: 2, + mergeReadiness: { ciState: "passed" as const, mergeStateLabel: "clean" }, + footerMarkdown: footer, + }; + + it("appends the Auto-merge conditions section when autoMergeSummary is on", () => { + const body = buildUnifiedCommentBody({ ...base, autoMergeSummary: true }); + expect(body).toContain("Auto-merge conditions"); + expect(body).toContain("| CI green | ✅ |"); + expect(body).toContain("| Valid linked issue | ✅ |"); + }); + + it("does NOT add the section when autoMergeSummary is absent (flag-OFF parity)", () => { + const body = buildUnifiedCommentBody(base); + expect(body).not.toContain("Auto-merge conditions"); + }); + + it("coexists with Changed files and Visual preview collapsibles", () => { + const body = buildUnifiedCommentBody({ + ...base, + autoMergeSummary: true, + changedFilesSummary: [{ path: "src/app.ts", additions: 1, deletions: 0 }], + beforeAfter: [{ path: "/", afterUrl: "https://api.example.dev/gittensory/shot?key=gittensory/shots/x.png" }], + }); + expect(body).toContain("Auto-merge conditions"); + expect(body).toContain("Changed files"); + expect(body).toContain("Visual preview"); + expect(body.indexOf("Auto-merge conditions")).toBeLessThan(body.indexOf("Changed files")); + }); +}); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 40178044e1..b6fac4bffd 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -356,6 +356,7 @@ describe(".gittensory.yml.example field-exhaustiveness (#1670)", () => { effortScore: "effort_score:", findingCategories: "finding_categories:", minFindingSeverity: "min_finding_severity:", + autoMergeSummary: "auto_merge_summary:", pathInstructions: "path_instructions:", instructions: "instructions:", excludePaths: "exclude_paths:", @@ -766,7 +767,7 @@ describe("compileFocusManifestPolicy", () => { publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, autoMergeSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null }, contentLane: { present: false, entryFileGlob: null, providerFileGlob: null, artifactGlob: null, collectionField: null, maxAppendedEntries: null, duplicateKeyFields: [], validatorId: null }, repoDocGeneration: { present: false, enabled: false, scope: ["agents"], allowOverwriteExisting: false, refreshIntervalDays: 7 }, @@ -2870,10 +2871,10 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { }); it("resolveReviewPromptOverrides: non-null manifest passes the config through; null manifest → defaults", () => { - const manifest = parseFocusManifest({ review: { profile: "chill", security_focus: true, inline_comments: true, suggestions: true, changed_files_summary: true, effort_score: true, finding_categories: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", exclude_paths: ["**/*.lock"], path_filters: ["src/**", "!src/generated/**"] } }); - expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", tone: null, securityFocus: true, inlineComments: true, suggestions: true, changedFilesSummary: true, effortScore: true, findingCategories: true, minFindingSeverity: null, pathInstructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", excludePaths: ["**/*.lock"], pathFilters: ["src/**", "!src/generated/**"], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); - // A null manifest (load failure) yields the byte-identical defaults; inline comments + suggestions + changed-files summary + effort score + finding categories + security focus default OFF. - expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, effortScore: false, findingCategories: false, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); + const manifest = parseFocusManifest({ review: { profile: "chill", security_focus: true, inline_comments: true, suggestions: true, changed_files_summary: true, effort_score: true, finding_categories: true, auto_merge_summary: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", exclude_paths: ["**/*.lock"], path_filters: ["src/**", "!src/generated/**"] } }); + expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", tone: null, securityFocus: true, inlineComments: true, suggestions: true, changedFilesSummary: true, effortScore: true, findingCategories: true, minFindingSeverity: null, autoMergeSummary: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", excludePaths: ["**/*.lock"], pathFilters: ["src/**", "!src/generated/**"], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); + // A null manifest (load failure) yields the byte-identical defaults; inline comments + suggestions + changed-files summary + effort score + finding categories + auto-merge summary + security focus default OFF. + expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, effortScore: false, findingCategories: false, minFindingSeverity: null, autoMergeSummary: false, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); // An explicit false / absent toggle both resolve to the strict-boolean false. expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { inline_comments: false } })).inlineComments).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).inlineComments).toBe(false); @@ -2884,6 +2885,8 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { effort_score: false } })).effortScore).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).effortScore).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { finding_categories: false } })).findingCategories).toBe(false); + expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { auto_merge_summary: false } })).autoMergeSummary).toBe(false); + expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).autoMergeSummary).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).findingCategories).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { security_focus: false } })).securityFocus).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).securityFocus).toBe(false); @@ -2986,6 +2989,20 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { expect(resolveReviewPromptOverrides(major).minFindingSeverity).toBe("major"); expect(resolveReviewPromptOverrides(parseFocusManifest({})).minFindingSeverity).toBeNull(); }); + + it("parses review.auto_merge_summary (default OFF), marks present, round-trips, and warns on a non-boolean (#2051)", () => { + expect(parseFocusManifest({ review: { auto_merge_summary: true } }).review.autoMergeSummary).toBe(true); + const on = parseFocusManifest({ review: { auto_merge_summary: true } }); + expect(on.review.present).toBe(true); + expect(parseFocusManifest({ review: reviewConfigToJson(on.review) }).review).toEqual(on.review); + const off = parseFocusManifest({ review: { auto_merge_summary: false } }); + expect(off.review.autoMergeSummary).toBe(false); + expect(off.review.present).toBe(true); + expect(parseFocusManifest({ review: {} }).review.autoMergeSummary).toBeNull(); + const bad = parseFocusManifest({ review: { auto_merge_summary: "yes" } }); + expect(bad.review.autoMergeSummary).toBeNull(); + expect(bad.warnings.some((w) => /review\.auto_merge_summary.*must be a boolean/.test(w))).toBe(true); + }); }); describe("review.exclude_paths (#review-exclude-paths)", () => { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 055d8e66fa..e3f1b40ec4 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -16285,6 +16285,171 @@ describe("queue processors", () => { } }); + // #2051: with the unified comment on AND `.gittensory.yml` opting into `review.auto_merge_summary`, the rendered + // comment gains the read-only "Auto-merge conditions" collapsible from already-computed gate/CI/merge/linked-issue + // signals — no AI, no decision-path re-derivation. + it("renders the Auto-merge conditions table when review.auto_merge_summary is on in .gittensory.yml", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_UNIFIED_COMMENT: "1" }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "detected_contributors_only", + publicAudienceMode: "gittensor_only", + publicSignalLevel: "standard", + publicSurface: "comment_and_label", + autoLabelEnabled: false, + checkRunMode: "off", + checkRunDetailLevel: "minimal", + gateCheckMode: "enabled", + backfillEnabled: true, + privateTrustEnabled: true, + autonomy: { update_branch: "auto" }, + }); + let postedBody = ""; + const calls = { comments: 0, gateChecks: 0 }; + let gateFinalized = false; + let failedPostGateMint = false; + const liveCiSpy = vi + .spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl") + .mockRejectedValueOnce(new Error("transient CI read failed")) + .mockResolvedValue({ + ciState: "passed", + hasPending: false, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") { + return Response.json([ + { + uid: 7, + githubUsername: "oktofeesh1", + githubId: "123", + totalPrs: 4, + totalMergedPrs: 3, + totalOpenPrs: 1, + totalClosedPrs: 0, + totalOpenIssues: 0, + totalClosedIssues: 0, + totalSolvedIssues: 0, + totalValidSolvedIssues: 0, + isEligible: true, + credibility: 1, + eligibleRepoCount: 1, + hotkey: "must-not-leak", + }, + ]); + } + if (url === "https://api.gittensor.io/miners/123") { + return Response.json({ + repositories: [ + { + repositoryFullName: "JSONbored/gittensory", + totalPrs: "4", + totalMergedPrs: "3", + totalOpenPrs: "1", + totalClosedPrs: "0", + totalOpenIssues: "0", + totalClosedIssues: "0", + isEligible: true, + credibility: "1.000000", + }, + ], + }); + } + if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]); + if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] }); + if (url.endsWith("/users/oktofeesh1")) return Response.json({ login: "oktofeesh1", public_repos: 2, followers: 1 }); + if (url.includes("/users/oktofeesh1/repos")) return Response.json([{ language: "TypeScript" }]); + if (url === "https://raw.githubusercontent.com/JSONbored/gittensory/HEAD/.gittensory.yml") { + return new Response("review:\n auto_merge_summary: true\n"); + } + if (url.includes("/access_tokens")) { + if (gateFinalized && !failedPostGateMint) { + failedPostGateMint = true; + return new Response("mint failed", { status: 500 }); + } + return Response.json({ token: "installation-token", expires_at: "2026-05-28T00:04:00.000Z" }); + } + if (url.includes("/pulls/3/files")) + return Response.json([{ filename: "src/cache.ts", additions: 3, deletions: 1, status: "modified" }]); + if (/\/pulls\/3(?:\?|$)/.test(url)) return Response.json({ number: 3, mergeable_state: "clean" }); + if (url.includes("/check-runs") && method === "GET") return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") { + calls.gateChecks += 1; + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string }; + if (body.status !== "in_progress" || body.conclusion) { + gateFinalized = true; + clearInstallationTokenCacheForTest(); + } + return Response.json({ id: 902 }, { status: 201 }); + } + if (url.includes("/check-runs/902") && method === "PATCH") { + calls.gateChecks += 1; + gateFinalized = true; + clearInstallationTokenCacheForTest(); + return Response.json({ id: 902 }); + } + if (url.includes("/issues/3/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/3/comments") && method === "POST") { + calls.comments += 1; + postedBody = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 1, html_url: "https://github.com/comment/1" }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + + try { + await processJob(env, { + type: "github-webhook", + deliveryId: "pr-unified-comment-auto-merge-summary", + eventName: "pull_request", + payload: { + action: "synchronize", + installation: { + id: 123, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", checks: "write" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { + number: 3, + title: "Fix webhook duplicate delivery again", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "unified790" }, + labels: [{ name: "bug" }], + body: "Fixes #1\n\nValidation: npm test", + }, + }, + }); + + expect(calls.comments).toBe(2); + expect(postedBody).toContain(""); + expect(postedBody).toContain("Auto-merge conditions"); + expect(postedBody).toContain("| CI green | ✅ |"); + expect(postedBody).toContain("| Gate passing | ✅ |"); + expect(postedBody).toContain("| Valid linked issue |"); + expect(postedBody).toContain("Does not change the merge decision"); + } finally { + liveCiSpy.mockRestore(); + } + }); + // #1955: the review-effort minutes persisted onto the public-stats audit event (independent of // review.effort_score, which only gates the unified-comment CHIP) must never block the publish itself when the // estimator throws — the publish still completes and simply omits `reviewEffortMinutes` from the event metadata diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index ef1877d937..a22dcf9439 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -1127,7 +1127,7 @@ describe("signal coverage edge cases", () => { collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), settings: gateSettings, - review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false }, linkedIssueSatisfaction: null }, + review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, autoMergeSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false }, linkedIssueSatisfaction: null }, aiReview: { notes: "The change is focused.\n\n**Nits (2)**\n- Add a test for the edge case.\n- Keep the validator helper scoped." }, }); expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead From d79fd27f522d6272b467ae9f18a6d38712e7aa10 Mon Sep 17 00:00:00 2001 From: bohdansolovie <153934212+bohdansolovie@users.noreply.github.com> Date: Mon, 6 Jul 2026 08:32:13 +0200 Subject: [PATCH 2/2] fix(review): only mark explicit clean merge state ok in auto-merge summary Conservative merge-row mapping: dirty/behind fail, clean passes, all other GitHub merge states warn instead of a misleading green check. --- src/review/unified-comment-bridge.ts | 8 +++---- .../auto-merge-summary-collapsible.test.ts | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 7fa117d13d..babc4ab8e9 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -590,10 +590,10 @@ export function deriveAutoMergeConditionsFromSignals(args: { const mergeRow: AutoMergeConditionRow = { condition: "Mergeable / clean", state: - mergeState === "dirty" || mergeState === "behind" - ? "fail" - : mergeState - ? "ok" + mergeState === "clean" + ? "ok" + : mergeState === "dirty" || mergeState === "behind" + ? "fail" : "warn", evidence: mergeState ? `GitHub merge state: ${args.mergeReadiness?.mergeStateLabel}.` diff --git a/test/unit/auto-merge-summary-collapsible.test.ts b/test/unit/auto-merge-summary-collapsible.test.ts index 8dca757669..394158946a 100644 --- a/test/unit/auto-merge-summary-collapsible.test.ts +++ b/test/unit/auto-merge-summary-collapsible.test.ts @@ -80,6 +80,29 @@ describe("deriveAutoMergeConditionsFromSignals", () => { expect(rows.find((row) => row.condition === "Valid linked issue")?.state).toBe("warn"); }); + it("marks only an explicit clean merge state as ok — blocked/unstable/draft stay warn (#2051)", () => { + for (const label of ["blocked", "unstable", "unmergeable", "draft", "unknown"]) { + const rows = deriveAutoMergeConditionsFromSignals({ + gate: gate(), + mergeReadiness: { ciState: "passed", mergeStateLabel: label }, + panelRows: panelRowsPassing, + }); + expect(rows.find((row) => row.condition === "Mergeable / clean")?.state).toBe("warn"); + } + const clean = deriveAutoMergeConditionsFromSignals({ + gate: gate(), + mergeReadiness: { ciState: "passed", mergeStateLabel: "clean" }, + panelRows: panelRowsPassing, + }); + expect(clean.find((row) => row.condition === "Mergeable / clean")?.state).toBe("ok"); + const dirty = deriveAutoMergeConditionsFromSignals({ + gate: gate(), + mergeReadiness: { ciState: "passed", mergeStateLabel: "dirty" }, + panelRows: panelRowsPassing, + }); + expect(dirty.find((row) => row.condition === "Mergeable / clean")?.state).toBe("fail"); + }); + it("does not invoke deriveUnifiedStatus — display-only derivation from pre-computed signals (#2051)", () => { const spy = vi.spyOn(unifiedComment, "deriveUnifiedStatus"); deriveAutoMergeConditionsFromSignals({