Skip to content
Closed
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
19 changes: 19 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,25 @@ gate:
# ----------------------------------------------------------------------------
# Everything a maintainer can toggle in the dashboard can be set here as code.
# All values shown are the safe defaults; delete any line to inherit it.
#
# Review output controls. These tune review output without changing the
# deterministic gate policy above. Omit the block to keep the byte-identical
# defaults.
review:
# Skip AI/public review output for matching PR author logins. Useful for dependency
# bump or release automation that already has separate policy/CI. This is a quiet
# skip, not a gate failure: when the Orb review check is enabled it is completed as
# "skipped" with an ignored-author reason.
#
# Glob list. `*` and `**` both match any run of characters; `**/name` also matches
# `name` at the root. Matching is case-insensitive against the GitHub login.
# Default: [] (every author remains review-eligible).
auto_review:
ignore_authors:
- "*[bot]"
- dependabot
- renovate

settings:
# Who receives the public PR comment.
# off | detected_contributors_only | all_prs. Default: detected_contributors_only.
Expand Down
3 changes: 3 additions & 0 deletions apps/gittensory-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -10162,6 +10162,7 @@
"surface_off",
"missing_author",
"bot_author",
"ignored_author",
"maintainer_author",
"miner_detection_unavailable",
"not_official_gittensor_miner",
Expand Down Expand Up @@ -10456,6 +10457,7 @@
"surface_off",
"missing_author",
"bot_author",
"ignored_author",
"maintainer_author",
"miner_detection_unavailable",
"not_official_gittensor_miner",
Expand Down Expand Up @@ -15715,6 +15717,7 @@
"surface_off",
"missing_author",
"bot_author",
"ignored_author",
"maintainer_author",
"miner_detection_unavailable",
"not_official_gittensor_miner"
Expand Down
3 changes: 3 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ const PR_VISIBILITY_SKIP_REASONS = [
"surface_off",
"missing_author",
"bot_author",
"ignored_author",
"maintainer_author",
"miner_detection_unavailable",
"not_official_gittensor_miner",
Expand Down Expand Up @@ -5365,6 +5366,8 @@ function skippedPrAuditRemediation(reason: string): string {
return "Retry after GitHub provides a resolvable pull request author.";
case "bot_author":
return "No action needed; bot-authored pull requests are intentionally kept quiet.";
case "ignored_author":
return "No action needed; the repository manifest explicitly skips review output for this author.";
case "maintainer_author":
return "Enable maintainer-authored PRs in repository settings only if those PRs should receive public GitHub App output.";
case "miner_detection_unavailable":
Expand Down
4 changes: 2 additions & 2 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ export const RepoSettingsPreviewSchema = z
willLabel: z.boolean(),
willCheckRun: z.boolean(),
skipped: z.boolean(),
skipReason: z.enum(["surface_off", "missing_author", "bot_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).nullable(),
skipReason: z.enum(["surface_off", "missing_author", "bot_author", "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).nullable(),
actions: z.array(z.enum(["skip", "comment", "label", "check_run", "none"])),
summary: z.string(),
}),
Expand Down Expand Up @@ -861,7 +861,7 @@ export const SkippedPrAuditExportSchema = z
filters: z.object({
repoFullName: z.string().nullable(),
reason: z
.enum(["surface_off", "missing_author", "bot_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"])
.enum(["surface_off", "missing_author", "bot_author", "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"])
.nullable(),
since: z.string().nullable(),
}),
Expand Down
2 changes: 1 addition & 1 deletion src/openapi/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ export function buildOpenApiSpec() {
param: { description: "Optional repository filter. Browser sessions must have control-panel access to this repo." },
example: "JSONbored/gittensory",
}),
reason: z.enum(["surface_off", "missing_author", "bot_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).optional().openapi({
reason: z.enum(["surface_off", "missing_author", "bot_author", "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).optional().openapi({
param: { description: "Optional PR skip reason filter." },
example: "not_official_gittensor_miner",
}),
Expand Down
55 changes: 54 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,13 +345,15 @@ import {
composeRepoReviewContext,
excludeReviewPaths,
resolveRepoEnrichmentToggles,
resolveReviewAutoReviewConfig,
resolveReviewPathInstructions,
resolveReviewPreMergeChecks,
resolveReviewPromptOverrides,
type FocusManifestFinding,
type ReviewPathInstruction,
type ReviewProfile,
} from "../signals/focus-manifest";
import { decideReviewEligibility } from "../review/review-eligibility";
import {
loadRepoFocusManifest,
loadRepoFocusManifests,
Expand Down Expand Up @@ -7043,6 +7045,58 @@ async function maybePublishPrPublicSurface(
// override) is gated on this, NOT on gateEnabled alone — gateEnabled stays scoped to the check-run PUBLISH
// calls (createOrUpdate*GateCheckRun), which must still never fire when reviewCheckMode is disabled.
const shouldEvaluateGate = gateEnabled || autonomyNeedsGateEvaluation;
const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null);
const autoReviewConfig = resolveReviewAutoReviewConfig(reviewManifest);
const reviewEligibility = decideReviewEligibility({
authorLogin: author,
ignoreAuthors: autoReviewConfig.ignoreAuthors,
});
if (!reviewEligibility.eligible) {
await auditPrVisibilitySkip(
env,
repoFullName,
pr.number,
author,
reviewEligibility.skipReason,
webhook.deliveryId,
);
if (gateEnabled) {
const gateCheckResult = await createOrUpdateSkippedGateCheckRun(
env,
installationId,
repoFullName,
advisory,
"Review skipped: ignored author.",
mode,
);
/* v8 ignore next -- permission-missing audit behavior mirrors the existing skipped-check path above. */
if (gateCheckResult?.kind === "permission_missing") {
await auditGateCheckPermissionMissing(
env,
author,
repoFullName,
pr.number,
webhook.deliveryId,
gateCheckResult.warning,
);
}
}
await recordAuditEvent(env, {
eventType: "github_app.pr_public_surface_skipped",
actor: author,
targetKey: `${repoFullName}#${pr.number}`,
outcome: "completed",
detail: reviewEligibility.skipReason,
metadata: {
deliveryId: webhook.deliveryId,
repoFullName,
matchedPattern: reviewEligibility.matchedPattern,
gateCheckMode: settings.gateCheckMode,
reviewCheckMode: settings.reviewCheckMode,
},
}).catch(() => undefined);
return undefined;
}
if (
!gateEnabled &&
!autonomyNeedsGateEvaluation &&
Expand Down Expand Up @@ -7788,7 +7842,6 @@ async function maybePublishPrPublicSurface(
agent: "dual-ai",
},
async () => {
const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null);
// `.gittensory.yml` review.profile + review.security_focus + review.path_instructions +
// review.exclude_paths (#review-profile / #review-security-focus / #review-path-instructions /
// #review-exclude-paths): resolve from the manifest (cached from settings resolution, so a cheap cache
Expand Down
57 changes: 57 additions & 0 deletions src/review/review-eligibility.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { matchesManifestPath } from "../signals/focus-manifest";

export type ReviewEligibilitySkipReason = "ignored_author";

export type ReviewEligibilityInput = {
authorLogin?: string | null | undefined;
ignoreAuthors?: readonly string[] | null | undefined;
};

export type ReviewEligibilityDecision =
| {
eligible: true;
skipReason: null;
matchedPattern: null;
}
| {
eligible: false;
skipReason: ReviewEligibilitySkipReason;
matchedPattern: string;
};

export const REVIEW_ELIGIBLE: ReviewEligibilityDecision = {
eligible: true,
skipReason: null,
matchedPattern: null,
};

function normalizeAuthorLogin(login: string | null | undefined): string {
return (login ?? "").trim();
}

/**
* Decide whether the auto-review pipeline should spend/reply for this PR author. This is intentionally narrower
* than the gate decision: ignored authors only suppress review/public output, never create a blocker.
*/
export function decideReviewEligibility(input: ReviewEligibilityInput): ReviewEligibilityDecision {
const author = normalizeAuthorLogin(input.authorLogin);
if (!author) return REVIEW_ELIGIBLE;

for (const pattern of input.ignoreAuthors ?? []) {
const trimmed = pattern.trim();
if (!trimmed) continue;
if (matchesManifestPath(author, trimmed)) {
return {
eligible: false,
skipReason: "ignored_author",
matchedPattern: trimmed,
};
}
}

return REVIEW_ELIGIBLE;
}

export function isIgnoredReviewAuthor(input: ReviewEligibilityInput): boolean {
return !decideReviewEligibility(input).eligible;
}
50 changes: 43 additions & 7 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ export type FocusManifestReviewConfig = {
footerText: string | null;
note: string | null;
fields: Partial<Record<ReviewFieldKey, boolean>>;
/** `review.auto_review.ignore_authors`: login glob list for authors whose PRs should stay quiet. This is an
* operator noise-control knob for dependency-bump and other automation accounts; empty (default) keeps review
* eligibility byte-identical. The runtime decision lives in review/review-eligibility.ts so the parser only
* owns normalization, capping, de-duping, and cache round-tripping. (#2060) */
autoReview: ReviewAutoReviewConfig;
/** `review.enrichment`: per-repo REES enrichment-analyzer toggles (analyzer name → on/off). Only known analyzer
* keys are kept (unknown keys warn + drop at parse). Empty (default, absent) ⇒ the operator's default analyzer
* set runs unchanged (byte-identical). (#2050) */
Expand Down Expand Up @@ -328,6 +333,10 @@ export type FocusManifestReviewConfig = {
preMergeChecks: PreMergeCheck[];
};

export type ReviewAutoReviewConfig = {
ignoreAuthors: string[];
};

/** One `review.path_instructions[]` entry: a manifest path glob + the public-safe instructions to apply when a
* changed file matches it. */
export type ReviewPathInstruction = { path: string; instructions: string };
Expand Down Expand Up @@ -487,7 +496,7 @@ const EMPTY_MANIFEST: FocusManifest = {
publicNotes: [],
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
review: { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -517,7 +526,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, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
review: { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -1410,13 +1419,14 @@ 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, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] };
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.`);
return empty;
}
const r = value as Record<string, JsonValue>;
const autoReview = parseReviewAutoReviewConfig(r.auto_review, warnings);
const footerRecord = r.footer !== null && typeof r.footer === "object" && !Array.isArray(r.footer) ? (r.footer as Record<string, JsonValue>) : undefined;
if (r.footer !== undefined && r.footer !== null && footerRecord === undefined) warnings.push(`Manifest "review.footer" must be a mapping; ignoring it.`);
const fieldsRecord = r.fields !== null && typeof r.fields === "object" && !Array.isArray(r.fields) ? (r.fields as Record<string, JsonValue>) : undefined;
Expand Down Expand Up @@ -1461,11 +1471,13 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
instructions !== null ||
excludePaths.length > 0 ||
preMergeChecks.length > 0 ||
autoReview.ignoreAuthors.length > 0 ||
Object.keys(fields).length > 0 ||
Object.keys(enrichmentAnalyzers).length > 0,
footerText,
note,
fields,
autoReview,
enrichmentAnalyzers,
profile,
securityFocus,
Expand All @@ -1477,6 +1489,18 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
};
}

function parseReviewAutoReviewConfig(value: JsonValue | undefined, warnings: string[]): ReviewAutoReviewConfig {
if (value === undefined || value === null) return { ignoreAuthors: [] };
if (typeof value !== "object" || Array.isArray(value)) {
warnings.push(`Manifest "review.auto_review" must be a mapping; ignoring it.`);
return { ignoreAuthors: [] };
}
const record = value as Record<string, JsonValue>;
return {
ignoreAuthors: parseManifestGlobList(record.ignore_authors, "review.auto_review.ignore_authors", warnings),
};
}

/** Parse `review.pre_merge_checks` — an array of DETERMINISTIC pre-merge assertions. Each entry needs a non-empty
* public-safe `name` and at least ONE assertion (`title_contains` / `description_contains` / `require_label`,
* each public-safe); `when_paths` (optional) gates the check to PRs touching a matching glob; `enforce` (default
Expand Down Expand Up @@ -1529,11 +1553,8 @@ function parseManifestGlobList(value: JsonValue | undefined, fieldLabel: string,
return [];
}
const out: string[] = [];
const seen = new Set<string>();
for (const [index, entry] of value.entries()) {
if (out.length >= MAX_PATH_INSTRUCTIONS) {
warnings.push(`Manifest "${fieldLabel}" is capped at ${MAX_PATH_INSTRUCTIONS} entries; dropping the rest.`);
break;
}
const glob = typeof entry === "string" ? entry.trim() : "";
if (!glob) {
warnings.push(`Manifest "${fieldLabel}[${index}]" must be a non-empty string; ignoring it.`);
Expand All @@ -1543,6 +1564,13 @@ function parseManifestGlobList(value: JsonValue | undefined, fieldLabel: string,
warnings.push(`Manifest "${fieldLabel}[${index}]" exceeds ${MAX_ITEM_LENGTH} chars; ignoring it.`);
continue;
}
const key = glob.toLowerCase();
if (seen.has(key)) continue;
if (out.length >= MAX_PATH_INSTRUCTIONS) {
warnings.push(`Manifest "${fieldLabel}" is capped at ${MAX_PATH_INSTRUCTIONS} entries; dropping the rest.`);
break;
}
seen.add(key);
out.push(glob);
}
return out;
Expand Down Expand Up @@ -1612,6 +1640,7 @@ function parseReviewProfile(value: JsonValue | undefined, warnings: string[]): R
export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue {
if (!review.present) return null;
const out: Record<string, JsonValue> = {};
if (review.autoReview.ignoreAuthors.length > 0) out.auto_review = { ignore_authors: [...review.autoReview.ignoreAuthors] };
if (review.footerText !== null) out.footer = { text: review.footerText };
if (review.note !== null) out.note = review.note;
if (review.profile !== null) out.profile = review.profile;
Expand Down Expand Up @@ -1672,6 +1701,13 @@ export function resolveReviewPreMergeChecks(manifest: FocusManifest | null): Pre
/** Resolve `review.enrichment` analyzer toggles from a possibly-null manifest (null = load failure ⇒ no toggles ⇒
* the operator's default analyzer set runs unchanged). Centralized so the enrichment caller threads them in one
* place with the null-manifest branch covered here (unit-tested) rather than inline in the processor. (#2050) */
/** Resolve `review.auto_review` from a possibly-null manifest (null = load failure => no ignored authors). The
* runtime eligibility check then fails open instead of suppressing review output on an ambiguous manifest read.
* (#2060) */
export function resolveReviewAutoReviewConfig(manifest: FocusManifest | null): ReviewAutoReviewConfig {
return { ignoreAuthors: manifest?.review.autoReview.ignoreAuthors ?? [] };
}

export function resolveEnrichmentAnalyzerToggles(manifest: FocusManifest | null): Partial<Record<ReesAnalyzerName, boolean>> {
return manifest?.review.enrichmentAnalyzers ?? {};
}
Expand Down
Loading
Loading