Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
# WHO CAN BE BLOCKED: gate modes choose *which* deterministic checks run, never
# *who* gets blocked. A hard block is always confirmed-contributor-gated.
#
# MANIFEST VALIDATION: any parse warning from this file (an unrecognized field, a
# malformed value that got dropped) is surfaced as a single grouped "Manifest
# validation" section in the unified PR review comment, so a misconfigured value
# fails clearly instead of silently falling back to its default. A fully valid
# manifest shows nothing here (byte-identical). Always on; not a config knob.
#
# NOTE: some capabilities (safety scanning, CI/full-file grounding, RAG,
# reputation control, the unified comment) are switched on at the deployment
# level by the operator's GITTENSORY_REVIEW_* feature flags AND a per-repo
Expand Down
6 changes: 6 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@
# WHO CAN BE BLOCKED: gate modes choose *which* deterministic checks run, never
# *who* gets blocked. A hard block is always confirmed-contributor-gated.
#
# MANIFEST VALIDATION: any parse warning from this file (an unrecognized field, a
# malformed value that got dropped) is surfaced as a single grouped "Manifest
# validation" section in the unified PR review comment, so a misconfigured value
# fails clearly instead of silently falling back to its default. A fully valid
# manifest shows nothing here (byte-identical). Always on; not a config knob.
#
# NOTE: some capabilities (safety scanning, CI/full-file grounding, RAG,
# reputation control, the unified comment) are switched on at the deployment
# level by the operator's GITTENSORY_REVIEW_* feature flags AND a per-repo
Expand Down
8 changes: 6 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9445,8 +9445,8 @@ async function maybePublishPrPublicSurface(
if (decision.willComment) {
// Maintainer review-content overrides from `.gittensory.yml` (footer text, row toggles, intro note).
// Cached, so this is a DB read after the settings resolution already loaded the manifest.
const reviewConfig = (await loadRepoFocusManifest(env, repoFullName))
.review;
const repoFocusManifestForComment = await loadRepoFocusManifest(env, repoFullName);
const reviewConfig = repoFocusManifestForComment.review;
// Duplicate-winner adjudication (#dup-winner): thread the flag into the public panel builders so the
// winner's hard-duplicate block is suppressed (they recompute the winner from their own open-only sibling
// list). Flag-OFF (default) ⇒ false ⇒ the panels are byte-identical to today.
Expand Down Expand Up @@ -9718,6 +9718,10 @@ async function maybePublishPrPublicSurface(
: {}),
maxFindingsCaps: reviewConfig.maxFindings,
commentVerbosity: reviewConfig.commentVerbosity,
// review-manifest validation (#2056): reuse the same manifest already loaded above for reviewConfig —
// unconditional (no manifest opt-in needed, a broken config should always fail clearly); no warnings
// ⇒ the bridge omits the section (byte-identical).
manifestWarnings: repoFocusManifestForComment.warnings,
});
} else {
deterministicBody = buildPublicPrIntelligenceComment(commentArgs);
Expand Down
27 changes: 26 additions & 1 deletion src/review/unified-comment-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import type { AdvisoryFinding } from "../types";
import type { GateCheckConclusion, GateCheckEvaluation } from "../rules/advisory";
import type { PublicPrPanelSignalRow } from "../signals/engine";
import { formatManifestValidationNotice } from "../signals/focus-manifest";
import type { CaptureRoute } from "./visual/capture";
// Single-source the panel marker from its canonical home (the upsert reads it there); re-export so existing
// importers of `PR_PANEL_COMMENT_MARKER` from this module keep working. The unified body MUST prepend this
Expand Down Expand Up @@ -327,6 +328,11 @@ export type UnifiedCommentBridgeArgs = {
* OFF (the processor passes this only when the manifest opts in — see `resolveReviewPromptOverrides`'s
* `commentVerbosity`). */
commentVerbosity?: "quiet" | "normal" | "detailed" | null | undefined;
/** The manifest's parse `warnings[]` (#2056) — when non-empty, a "Manifest validation" collapsible listing
* each grouped, deduped warning is appended, so an invalid/malformed `.gittensory.yml` value fails clearly
* instead of silently falling back to a default. No AI, no network. Absent/empty ⇒ no section
* (byte-identical) — always safe to pass the manifest's raw warnings unconditionally. */
manifestWarnings?: string[] | undefined;
/** Line-anchored AI findings, one entry per inline finding (review.finding_categories port). When present +
* non-empty, a "Finding categories" collapsible (a count per security/correctness/performance/maintainability/
* tests/style category) is appended. A finding missing its own `category` falls back to
Expand Down Expand Up @@ -492,6 +498,18 @@ export function buildChangedFilesSummaryCollapsible(files: ChangedFileSummaryInp
return { title: "Changed files", body };
}

/**
* Build the "Manifest validation" collapsible from a manifest's parse `warnings[]` (#2056) — grouped,
* deduped, so an invalid/malformed `.gittensory.yml` value fails clearly instead of silently falling back
* to a default. Returns null when there are no warnings, so the caller can unconditionally chain this
* alongside the other optional collapsibles (byte-identical when the manifest is fully valid).
*/
export function buildManifestValidationCollapsible(warnings: string[]): UnifiedCollapsible | null {
const notice = formatManifestValidationNotice(warnings);
if (notice === null) return null;
return { title: "Manifest validation", body: notice };
}

/** One impact-map entry — everything `buildImpactMapCollapsible` needs to render a row. Deliberately narrower
* than `ImpactMapEntry` (`src/review/impact-map.ts`) shape-wise (it IS that shape) so this bridge's import
* surface stays limited to what rendering actually reads. */
Expand Down Expand Up @@ -635,6 +653,13 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
const visibleRows = args.panelRows.filter((row) => args.reviewFields?.[row.key] !== false);
const signals = panelRowsToSignalRows(visibleRows);

// review-manifest validation (#2056): a broken/malformed .gittensory.yml value should fail clearly, so this
// is unconditional (no manifest opt-in) — prepended ahead of every content-shape summary since a config
// problem is more foundational than what changed. No warnings ⇒ extraCollapsibles is unchanged.
const manifestValidationCollapsible =
args.manifestWarnings && args.manifestWarnings.length > 0 ? buildManifestValidationCollapsible(args.manifestWarnings) : null;
const withManifestValidation =
manifestValidationCollapsible !== null ? [manifestValidationCollapsible, ...(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)
Expand All @@ -643,7 +668,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
? buildChangedFilesSummaryCollapsible(args.changedFilesSummary)
: null;
const withChangedFiles =
changedFilesCollapsible !== null ? [...(args.extraCollapsibles ?? []), changedFilesCollapsible] : args.extraCollapsibles;
changedFilesCollapsible !== null ? [...(withManifestValidation ?? []), changedFilesCollapsible] : withManifestValidation;
// 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) ⇒
Expand Down
19 changes: 19 additions & 0 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2956,6 +2956,25 @@ export function parseFocusManifestContent(content: string | null | undefined, so
return parseFocusManifest(parsed, source);
}

/**
* Format a manifest's parse `warnings[]` into one grouped, deduped, order-preserving notice for the review
* surface — an acceptance criterion of #1670: an invalid/malformed `.gittensory.yml` value should fail
* clearly instead of silently falling back to a default. Empty/no warnings ⇒ `null` (byte-identical, no
* notice). Pure; reuses the warnings every parser already accumulates rather than a parallel schema. (#2056)
*/
export function formatManifestValidationNotice(warnings: string[]): string | null {
const seen = new Set<string>();
const deduped: string[] = [];
for (const warning of warnings) {
const trimmed = warning.trim();
if (!trimmed || seen.has(trimmed)) continue;
seen.add(trimmed);
deduped.push(trimmed);
}
if (deduped.length === 0) return null;
return deduped.map((warning) => `- ${warning}`).join("\n");
}

function normalizePathForMatch(path: string): string {
return String(path).replace(/\\/g, "/").replace(/^\.\//, "").replace(/^\/+/, "").toLowerCase();
}
Expand Down
46 changes: 46 additions & 0 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
matchesManifestPath,
parseFocusManifest,
parseFocusManifestContent,
formatManifestValidationNotice,
resolveEffectiveSettings,
excludeReviewPaths,
applyReviewPathFilters,
Expand Down Expand Up @@ -4106,3 +4107,48 @@ describe("gate.expectedCiContexts (#selfhost-ci-verification)", () => {
expect(eff.expectedCiContexts).toBeUndefined();
});
});

describe("formatManifestValidationNotice (#2056)", () => {
it("returns null for an empty warnings array", () => {
expect(formatManifestValidationNotice([])).toBeNull();
});

it("returns null when every warning is blank/whitespace-only", () => {
expect(formatManifestValidationNotice(["", " ", "\n"])).toBeNull();
});

it("formats a single warning as a bullet line", () => {
expect(formatManifestValidationNotice(["Manifest field \"review.tone\" must be a string; ignoring it."])).toBe(
"- Manifest field \"review.tone\" must be a string; ignoring it.",
);
});

it("groups multiple distinct warnings, preserving order", () => {
const result = formatManifestValidationNotice(["first warning", "second warning", "third warning"]);
expect(result).toBe("- first warning\n- second warning\n- third warning");
});

it("dedupes identical warnings (case-sensitive, exact-match), keeping the first occurrence's position", () => {
const result = formatManifestValidationNotice(["dup warning", "unique warning", "dup warning"]);
expect(result).toBe("- dup warning\n- unique warning");
});

it("trims surrounding whitespace from each warning before formatting/deduping", () => {
const result = formatManifestValidationNotice([" padded warning ", "padded warning"]);
expect(result).toBe("- padded warning");
});

it("round-trips through a real malformed manifest's warnings", () => {
const malformed = parseFocusManifest({ review: { tone: 42, profile: "loud" } });
expect(malformed.warnings.length).toBeGreaterThan(0);
const notice = formatManifestValidationNotice(malformed.warnings);
expect(notice).not.toBeNull();
expect(notice).toContain("- ");
});

it("returns null for a fully-valid manifest with zero warnings (byte-identical, no notice)", () => {
const valid = parseFocusManifest({ review: { profile: "chill" } });
expect(valid.warnings).toEqual([]);
expect(formatManifestValidationNotice(valid.warnings)).toBeNull();
});
});
104 changes: 104 additions & 0 deletions test/unit/manifest-validation-collapsible.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { describe, expect, it } from "vitest";
import { buildManifestValidationCollapsible, buildUnifiedCommentBody } from "../../src/review/unified-comment-bridge";
import type { GateCheckEvaluation } from "../../src/rules/advisory";
import type { PublicPrPanelSignalRow } from "../../src/signals/engine";

function gate(over: Partial<GateCheckEvaluation> = {}): GateCheckEvaluation {
return {
enabled: true,
conclusion: "success",
title: "Gittensory Orb Review Agent passed",
summary: "No configured hard blocker was found.",
blockers: [],
warnings: [],
...over,
};
}

const panelRows: PublicPrPanelSignalRow[] = [
{ key: "gateResult", cells: ["Gate result", "✅ Passing", "No configured blocker found.", "No action."] },
];
const footer = "💰 Earn for open-source contributions. Checked by Gittensory.";

describe("buildManifestValidationCollapsible (#2056)", () => {
it("returns null for an empty warnings array", () => {
expect(buildManifestValidationCollapsible([])).toBeNull();
});

it("returns null when every warning is blank", () => {
expect(buildManifestValidationCollapsible(["", " "])).toBeNull();
});

it("builds a titled collapsible listing each grouped warning", () => {
const c = buildManifestValidationCollapsible(["bad review.tone value", "bad review.profile value"]);
expect(c).not.toBeNull();
expect(c?.title).toBe("Manifest validation");
expect(c?.body).toBe("- bad review.tone value\n- bad review.profile value");
});

it("dedupes identical warnings", () => {
const c = buildManifestValidationCollapsible(["same warning", "same warning"]);
expect(c?.body).toBe("- same warning");
});
});

describe("buildUnifiedCommentBody: manifest validation wiring (#2056)", () => {
it("appends a Manifest validation collapsible when manifestWarnings is non-empty", () => {
const body = buildUnifiedCommentBody({
gate: gate(),
panelRows,
readinessTotal: 88,
changedFiles: 1,
footerMarkdown: footer,
manifestWarnings: ["Manifest field \"review.tone\" must be a string; ignoring it."],
});
expect(body).toContain("<summary><b>Manifest validation</b></summary>");
expect(body).toContain("Manifest field \"review.tone\" must be a string; ignoring it.");
});

it("omits the section when manifestWarnings is empty or absent (byte-identical)", () => {
const withEmpty = buildUnifiedCommentBody({
gate: gate(),
panelRows,
readinessTotal: 88,
changedFiles: 1,
footerMarkdown: footer,
manifestWarnings: [],
});
const withoutField = buildUnifiedCommentBody({
gate: gate(),
panelRows,
readinessTotal: 88,
changedFiles: 1,
footerMarkdown: footer,
});
expect(withEmpty).toBe(withoutField);
expect(withEmpty).not.toContain("Manifest validation");
});

it("renders the Manifest validation section ahead of the Changed files section", () => {
const body = buildUnifiedCommentBody({
gate: gate(),
panelRows,
readinessTotal: 88,
changedFiles: 1,
footerMarkdown: footer,
manifestWarnings: ["a config warning"],
changedFilesSummary: [{ path: "src/a.ts", additions: 1, deletions: 0 }],
});
expect(body.indexOf("Manifest validation")).toBeLessThan(body.indexOf("Changed files"));
});

it("renders Changed files with no Manifest validation section when only manifestWarnings is absent", () => {
const body = buildUnifiedCommentBody({
gate: gate(),
panelRows,
readinessTotal: 88,
changedFiles: 1,
footerMarkdown: footer,
changedFilesSummary: [{ path: "src/a.ts", additions: 1, deletions: 0 }],
});
expect(body).toContain("Changed files");
expect(body).not.toContain("Manifest validation");
});
});