Skip to content

chore(review): converge 9 hand-duplicated src/review/ files onto their @loopover/engine shims #6437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loopover-orb merged 1 commit into from
Jul 16, 2026
Merged

chore(review): converge 9 hand-duplicated src/review/ files onto their @loopover/engine shims #6437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 6 additions & 41 deletions src/review/advisory-ai-routing-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,6 @@
import type { AdvisoryAiRoutingConfig } from "../types";

export const DEFAULT_ADVISORY_AI_ROUTING: AdvisoryAiRoutingConfig = {
slop: false,
e2eTestGen: false,
planner: false,
summaries: false,
chatQa: false,
chatQaFrontierFallback: false,
intentRouting: false,
};

function normalizeField(value: unknown, field: keyof AdvisoryAiRoutingConfig, warnings: string[]): boolean {
if (value === undefined) return DEFAULT_ADVISORY_AI_ROUTING[field];
if (typeof value === "boolean") return value;
warnings.push(`settings.advisoryAiRouting.${field} must be a boolean; using the default "${DEFAULT_ADVISORY_AI_ROUTING[field]}".`);
return DEFAULT_ADVISORY_AI_ROUTING[field];
}

/**
* Normalize a raw `.loopover.yml settings.advisoryAiRouting` value into a typed config, fail-safe: any
* malformed field falls back to its own (false) default and pushes a warning rather than rejecting the
* whole block. Mirrors `normalizeUnlinkedIssueGuardrailConfig`'s per-field discipline.
*/
export function normalizeAdvisoryAiRoutingConfig(input: unknown, warnings: string[]): AdvisoryAiRoutingConfig {
if (input === undefined) return { ...DEFAULT_ADVISORY_AI_ROUTING };
if (typeof input !== "object" || input === null || Array.isArray(input)) {
warnings.push("settings.advisoryAiRouting must be an object; using the default (every capability off).");
return { ...DEFAULT_ADVISORY_AI_ROUTING };
}
const record = input as Record<string, unknown>;
return {
slop: normalizeField(record.slop, "slop", warnings),
e2eTestGen: normalizeField(record.e2eTestGen, "e2eTestGen", warnings),
planner: normalizeField(record.planner, "planner", warnings),
summaries: normalizeField(record.summaries, "summaries", warnings),
chatQa: normalizeField(record.chatQa, "chatQa", warnings),
chatQaFrontierFallback: normalizeField(record.chatQaFrontierFallback, "chatQaFrontierFallback", warnings),
intentRouting: normalizeField(record.intentRouting, "intentRouting", warnings),
};
}
// advisory-ai-routing-config, converged onto @loopover/engine (#6203). This src/ file was a hand-maintained twin
// of the engine copy; it is now a thin re-export shim so the single implementation lives at
// packages/loopover-engine/src/review/advisory-ai-routing-config.ts (imported via relative source path, not the
// published package, to match this repo's existing engine-consumption convention — see
// src/settings/auto-close-exempt.ts).
export * from "../../packages/loopover-engine/src/review/advisory-ai-routing-config";
93 changes: 5 additions & 88 deletions src/review/cla-check.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,88 +1,5 @@
import type { AdvisoryFinding } from "../types";

/** Finding code raised when `gate.claMode` is opted in (advisory/block) and neither configured detection method
* (the PR body consent phrase, or the named CLA-bot check-run) confirms consent. ALWAYS severity "warning" at
* generation time — mirrors `manifest_missing_tests`/`manifest_linked_issue_required` (focus-manifest.ts): a
* single finding code whose escalation to a hard blocker is decided entirely by the configured gate MODE
* (isConfiguredGateBlocker, src/rules/advisory.ts), not by this evaluator. */
export const CLA_CONSENT_MISSING_CODE = "cla_consent_missing";
/** Finding code emitted when check-run detection is the ONLY configured method and its conclusion could not be
* resolved (a transient fetch failure, not a resolved "no such check-run"). Mirrors `pre_merge_check_unresolved`
* (review/pre-merge-checks.ts): isEvaluationBlocker (advisory.ts) treats this as a NEUTRAL gate (HELD,
* re-evaluates automatically) — never silently skipping a hard requirement and never hard-closing the
* contributor on a transient resolution miss. */
export const CLA_CHECK_UNRESOLVED_CODE = "cla_check_unresolved";

export type ClaCheckConfig = {
/** Public-safe-filtered consent phrase a maintainer requires somewhere in the PR body (case-insensitive
* substring match), e.g. "I have read and agree to the CLA". `null` ⇒ phrase-match detection is not configured. */
consentPhrase: string | null;
/** Name of a separate CLA-bot check-run this repo also runs (e.g. "CLA Assistant Lite"). When set, a
* `success`/`neutral` conclusion for a check-run with this exact name (case-insensitive) also satisfies
* consent. `null` ⇒ check-run detection is not configured. */
checkRunName: string | null;
};

/**
* Evaluate `.loopover.yml gate.claMode` + `gate.cla` (consentPhrase / checkRunName) against a PR —
* DETERMINISTICALLY, mirroring the pre-merge-checks title/description phrase-match pattern (review/pre-merge-checks.ts)
* exactly: a case-insensitive substring match against already-resolved PR data, no AI judgment. Consent is
* satisfied when EITHER configured method holds (an "either" contract, not "all", because a repo may only be able
* to detect ONE method for a given PR — e.g. no check-run data was resolved): the PR body contains
* `consentPhrase`, OR a check-run named `checkRunName` concluded `success`/`neutral`. When NEITHER method is
* configured (both null), there is nothing to evaluate — no finding (byte-identical, matches `pre_merge_checks`'
* empty-checks behavior).
*
* `checkRunConclusion` is `undefined` when the caller could not resolve check-run data at all (a transient
* fetch failure, or the predicted-gate metadata-only path, which never sees live check-runs) — that is NOT the
* same as a resolved-but-absent check-run (`null`, "no check-run with this name exists"). When check-run
* detection is configured and its conclusion is unresolved (a transient fetch failure, or "not yet run"), this
* HOLDS (`cla_check_unresolved`) instead of failing closed — exactly like an unresolved changed-file set HOLDS
* a path-gated pre-merge check rather than silently skipping (auto-merge bypass) or hard-closing on a
* transient miss. This applies EVEN WHEN `consentPhrase` is ALSO configured but not (yet) satisfied: per the
* "either method holds ⇒ satisfied" contract above, an unresolved check-run might still satisfy consent, so
* deciding purely from a not-yet-satisfied phrase would hard-fail a PR the check-run could have saved (#2564
* gate-review finding). A hold only degrades to a hard `cla_consent_missing` once EVERY configured method has
* been definitively resolved and none of them is satisfied. Pure + side-effect-free; the caller pushes the
* finding into the advisory before the gate evaluates.
*/
export function evaluateClaCheck(
config: ClaCheckConfig,
ctx: { body?: string | null | undefined; checkRunConclusion?: string | null | undefined },
): AdvisoryFinding[] {
// A blank/whitespace-only consentPhrase is treated as unset (null), mirroring the config-as-code path's
// normalizeOptionalString (packages/loopover-engine/src/focus-manifest.ts): otherwise `"".includes("")` (or
// any body `.includes("")`) is unconditionally true, silently satisfying consent for every PR — the DB-backed
// dashboard `claConsentPhrase` field has no non-empty validation and reaches here via `?? null` unchanged (#5838).
const consentPhrase = config.consentPhrase !== null && config.consentPhrase.trim().length > 0 ? config.consentPhrase : null;
if (consentPhrase === null && config.checkRunName === null) return []; // nothing configured ⇒ no finding
const phraseSatisfied = consentPhrase !== null && (ctx.body ?? "").toLowerCase().includes(consentPhrase.toLowerCase());
const checkRunSatisfied = config.checkRunName !== null && (ctx.checkRunConclusion === "success" || ctx.checkRunConclusion === "neutral");
if (phraseSatisfied || checkRunSatisfied) return [];
// A configured check-run whose conclusion is unresolved: cannot confirm OR deny consent via that method, so
// HOLD rather than fail closed — regardless of whether consentPhrase is ALSO configured (a not-yet-satisfied
// phrase does not mean consent is definitively absent while the check-run could still satisfy it).
if (config.checkRunName !== null && ctx.checkRunConclusion === undefined) {
return [
{
code: CLA_CHECK_UNRESOLVED_CODE,
severity: "warning",
title: `CLA check held — "${config.checkRunName}" not resolved`,
detail: `LoopOver could not resolve the "${config.checkRunName}" check-run's conclusion for this PR; the gate is held and re-evaluates automatically.`,
action: "No action needed — the gate re-evaluates once the check-run's conclusion is available.",
},
];
}
const missing: string[] = [];
if (consentPhrase !== null) missing.push(`the PR description must contain "${consentPhrase}"`);
if (config.checkRunName !== null) missing.push(`the "${config.checkRunName}" check must pass`);
return [
{
code: CLA_CONSENT_MISSING_CODE,
severity: "warning",
title: "CLA consent not confirmed",
detail: `This PR does not confirm contributor license agreement consent: ${missing.join(" or ")}.`,
action: "Add the required CLA consent phrase to the PR description, or complete the CLA check, then re-run the gate.",
},
];
}
// cla-check, converged onto @loopover/engine (#6203). This src/ file was a hand-maintained twin of the engine
// copy; it is now a thin re-export shim so the single implementation lives at
// packages/loopover-engine/src/review/cla-check.ts (imported via relative source path, not the published
// package, to match this repo's existing engine-consumption convention — see src/settings/auto-close-exempt.ts).
export * from "../../packages/loopover-engine/src/review/cla-check";
74 changes: 6 additions & 68 deletions src/review/enrichment-analyzer-names.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,68 +1,6 @@
// Canonical REES enrichment-analyzer name registry (#2050). The single source of truth for the analyzer keys that
// both the operator `REES_ANALYZERS` env list and the per-repo `.loopover.yml` `review.enrichment` toggles are
// validated against. A leaf module with no imports, so the review wiring and the signals-layer manifest parser can
// share it without a heavy or circular dependency.

export const REES_ANALYZER_NAMES = [
"dependency",
"dependencyDiff",
"lockfileDrift",
"secret",
"license",
"installScript",
"heavyDependency",
"hardcodedUrl",
"actionPin",
"eol",
"redos",
"provenance",
"codeowners",
"secretLog",
"assetWeight",
"typosquat",
"commitSignature",
"iacMisconfig",
"nativeBuild",
"history",
"docCommentDrift",
"duplication",
"duplicationDelta",
"churnHotspot",
"blameLink",
"approvalIntegrity",
"ciCheckSignals",
"undocumentedExport",
"staleBranch",
"commitHygiene",
"pendingReviewRequests",
"testRatio",
"migrationSafety",
"looseRange",
"terminology",
"todoMarker",
"magicNumber",
"conflictMarker",
"debugLeftover",
"sizeSmell",
"floatingPromise",
"deepNesting",
"errorSwallow",
"complexity",
"complexityDelta",
"unsafeAny",
"a11y",
"i18n",
"unusedExport",
"exhaustiveness",
"flakyTest",
"commitLint",
"apiBreak",
"deprecatedDep",
"revertRecurrence",
"coverageDelta",
"callerImpact",
] as const;

export type ReesAnalyzerName = (typeof REES_ANALYZER_NAMES)[number];

export const REES_ANALYZER_NAME_SET: ReadonlySet<string> = new Set<string>(REES_ANALYZER_NAMES);
// enrichment-analyzer-names, converged onto @loopover/engine (#6203). This src/ file was a hand-maintained twin
// of the engine copy; it is now a thin re-export shim so the single implementation lives at
// packages/loopover-engine/src/review/enrichment-analyzer-names.ts (imported via relative source path, not the
// published package, to match this repo's existing engine-consumption convention — see
// src/settings/auto-close-exempt.ts).
export * from "../../packages/loopover-engine/src/review/enrichment-analyzer-names";
82 changes: 5 additions & 77 deletions src/review/guardrail-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,77 +1,5 @@
import type { RepositorySettings } from "../types";

// This is a Set-membership guardrail list (order doesn't matter, unlike the loaders' priority-ordered candidate
// lists): a contributor PR touching the canonical `.loopover.*` config file gets hard-guardrail protection.
export const CONFIG_AS_CODE_GUARDRAIL_GLOBS = [
".loopover.yml",
".loopover.yaml",
".loopover.json",
".github/loopover.yml",
".github/loopover.yaml",
".github/loopover.json",
"**/codecov.yml",
"**/codecov.yaml",
"**/.codecov.yml",
];

export const WORKFLOW_AND_RUNTIME_GUARDRAIL_GLOBS = [
".github/workflows/**",
"scripts/**",
"wrangler.jsonc",
"src/selfhost/**",
];

export const ENGINE_DECISION_GUARDRAIL_GLOBS = [
"src/rules/**",
"src/services/**",
"src/settings/agent-actions.ts",
"src/settings/agent-execution.ts",
"src/settings/agent-sweep.ts",
"src/settings/autonomy.ts",
"src/queue/**",
"src/github/pr-actions.ts",
"src/github/app.ts",
"src/github/backfill.ts",
// #4197: writes a real commit onto a CONTRIBUTOR's own PR branch (not a branch loopover owns) — the same
// guardrail tier as pr-actions.ts/app.ts for the same reason, a new GitHub-write surface.
"src/github/e2e-test-commit.ts",
"src/scoring/**",
"src/auth/**",
"src/review/safety.ts",
"src/review/guardrail-config.ts",
"src/review/cutover-gate.ts",
"src/review/linked-issue-hard-rules.ts",
"src/review/outcomes-wire.ts",
];

// Default, safe-by-default invariant set (restored by #3943 after the original pure-config-as-code design
// let a `.loopover.yml` edit silently remove its own guardrail protection). Repo settings can only ADD to
// this set UNLESS the repo explicitly opts in via `hardGuardrailGlobsOverridesInvariants` (below).
export const DEFAULT_HARD_GUARDRAIL_GLOBS = [
...CONFIG_AS_CODE_GUARDRAIL_GLOBS,
...WORKFLOW_AND_RUNTIME_GUARDRAIL_GLOBS,
...ENGINE_DECISION_GUARDRAIL_GLOBS,
];

/**
* Resolve hard-guardrail path globs from the already-effective repo settings.
*
* Safe by default (#3943): `DEFAULT_HARD_GUARDRAIL_GLOBS` is an invariant floor, and a repo's configured
* `hardGuardrailGlobs` is ADDED to it (deduplicated), never allowed to shrink it — so an ordinary
* `.loopover.yml` edit (even a careless or malicious one) can only ever widen guardrail protection.
*
* Full self-hoster control, opt-in (config-as-code mandate): a repo that explicitly sets
* `hardGuardrailGlobsOverridesInvariants: true` takes complete ownership of its guardrail list —
* `hardGuardrailGlobs` is then used EXACTLY as given (including an explicit `[]` to disable path guardrails
* entirely), REPLACING rather than adding to the built-in floor. This is deliberately a second, explicit
* field rather than reusing `hardGuardrailGlobs: []`'s presence/absence, so opting out of the safety net is
* always a conscious, separately-visible decision in the config file, not a side effect of trimming a list.
*/
export function resolveHardGuardrailGlobs(
settings: Pick<RepositorySettings, "hardGuardrailGlobs" | "hardGuardrailGlobsOverridesInvariants"> | null | undefined,
): string[] {
const configured = settings?.hardGuardrailGlobs;
const configuredList = Array.isArray(configured) ? configured : [];
if (settings?.hardGuardrailGlobsOverridesInvariants === true) return [...configuredList];
return Array.from(new Set([...DEFAULT_HARD_GUARDRAIL_GLOBS, ...configuredList]));
}
// guardrail-config, converged onto @loopover/engine (#6203). This src/ file was a hand-maintained twin of the
// engine copy; it is now a thin re-export shim so the single implementation lives at
// packages/loopover-engine/src/review/guardrail-config.ts (imported via relative source path, not the published
// package, to match this repo's existing engine-consumption convention — see src/settings/auto-close-exempt.ts).
export * from "../../packages/loopover-engine/src/review/guardrail-config";
Loading