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
1,752 changes: 895 additions & 857 deletions apps/gittensory-ui/public/openapi.json

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,12 @@ export const RepositorySettingsSchema = z
aiReviewModel: z.string().nullable().optional(),
aiReviewAllAuthors: z.boolean(),
aiReviewCloseConfidence: z.number().nullable().optional(),
aiReviewCombine: z.enum(["single", "consensus", "synthesis"]).nullable().optional(),
aiReviewOnMerge: z.enum(["either", "both"]).nullable().optional(),
aiReviewReviewers: z
.array(z.object({ model: z.string(), fallback: z.string().nullable().optional() }))
.nullable()
.optional(),
closeOwnerAuthors: z.boolean(),
autoLabelEnabled: z.boolean(),
gittensorLabel: z.string(),
Expand Down
7 changes: 7 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5450,6 +5450,13 @@ export async function runAiReviewForAdvisory(
observability: { rag: ragTelemetry },
enrichment,
profile: args.reviewProfile ?? null,
// Per-repo dual-AI combine/onMerge/reviewers overrides (#2567), resolved by resolveEffectiveSettings from
// `.gittensory.yml gate.aiReview.*` onto `args.settings`. Absent ⇒ undefined ⇒ runGittensoryAiReview falls
// back to the operator's AI_REVIEW_PLAN (byte-identical to today). `onMerge` is clamped to the operator's
// floor INSIDE runGittensoryAiReview (resolveEffectiveAiReviewOnMerge), not here.
combine: args.settings.aiReviewCombine ?? undefined,
onMerge: args.settings.aiReviewOnMerge ?? undefined,
reviewers: args.settings.aiReviewReviewers ?? undefined,
securityFocus: args.reviewSecurityFocus === true,
// Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag,
// the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical.
Expand Down
48 changes: 38 additions & 10 deletions src/services/ai-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { errorMessage } from "../utils/json";
import type { ReviewProfile } from "../signals/focus-manifest";
import { isCodeFile } from "../signals/local-branch";
import { isTestPath } from "../signals/test-evidence";
import type { CombineStrategy, OnMerge } from "../types";

/**
* The best free Workers-AI model pair for review accuracy — two different families for independence,
Expand Down Expand Up @@ -79,17 +80,36 @@ export type AiReviewProviderKey = {
model?: string | null | undefined;
};

// `CombineStrategy` / `OnMerge` (#dual-ai-combiner) are defined in ../types.ts, not here, and re-exported for
// backward compat: both this file's own callers AND signals/focus-manifest.ts + types.ts's RepositorySettings
// need the type, but focus-manifest.ts/types.ts are imported by the UI workspace, which lacks the ambient
// Cloudflare Workers types (`Env`, `D1Database`, …) this file's runtime code depends on — a type-only
// `import("../services/ai-review")` reference from either would still drag this whole module graph into the UI's
// typecheck and break it (#2567 follow-up fix). See ../types.ts for the full doc comment.
export type { CombineStrategy, OnMerge } from "../types";

/**
* How the independent reviewer opinions are combined into ONE gate decision (#dual-ai-combiner):
* • `single` — one reviewer; its verdict IS the decision (a named blocker blocks).
* • `consensus` — two reviewers; block ONLY when BOTH name a blocker; lone blocker → split (hold). The
* historical cloud behavior — the default, so an unset `combine` is byte-identical.
* • `synthesis` — two reviewers run separately, then merge into ONE decision (no split/hold-on-disagree):
* `onMerge: either` blocks if EITHER flags a blocker; `both` only if all do.
* Resolve the EFFECTIVE `onMerge` rule for a review call, enforcing that a per-repo `.gittensory.yml
* gate.aiReview.onMerge` override (#2567) can only TIGHTEN the self-host operator's `AI_REVIEW_PLAN.onMerge`
* floor, never loosen it. `either` is the STRICTER rule (any one reviewer's blocker blocks/holds); `both` is
* more PERMISSIVE (requires every reviewer to agree before a blocker counts). So:
* - operator floor `either` + repo override `both` → CLAMPED to `either` (an attempted loosening).
* - operator floor `either` + repo override `either` → `either` (a no-op tightening).
* - operator floor `both` (or unset) → the repo override (or the operator's own value) wins
* unclamped — there is no stricter floor to violate.
* Returns the resolved value alongside whether a clamp fired, so the caller can log/surface it (a maintainer
* who configured a loosening override should see it was not honored, not have it silently ignored).
*/
export type CombineStrategy = "single" | "consensus" | "synthesis";
/** Synthesis merge rule — block if `either` reviewer flags a blocker, or only when `both` agree. */
export type OnMerge = "either" | "both";
export function resolveEffectiveAiReviewOnMerge(
repoOverride: OnMerge | null | undefined,
operatorFloor: OnMerge | null | undefined,
): { onMerge: OnMerge | null | undefined; clamped: boolean } {
if (repoOverride == null) return { onMerge: operatorFloor, clamped: false };
if (operatorFloor === "either" && repoOverride === "both") {
return { onMerge: "either", clamped: true };
}
return { onMerge: repoOverride, clamped: false };
}

export type GittensoryAiReviewInput = {
repoFullName: string;
Expand Down Expand Up @@ -1115,7 +1135,15 @@ export async function runGittensoryAiReview(
const secondaryFallback = secondary.fallback ?? secondary.model;
const combine: CombineStrategy =
input.combine ?? plan?.combine ?? "consensus";
const onMerge: OnMerge | null | undefined = input.onMerge ?? plan?.onMerge;
// `onMerge` is a per-repo REFINEMENT of the operator's plan, never a bypass (#2567): a repo can only TIGHTEN
// the operator's floor (never loosen `either` down to `both`). resolveEffectiveAiReviewOnMerge enforces the
// clamp; a fired clamp increments a metric so it is surfaced, not silently ignored (mirrors the
// gittensory_ai_review_inconclusive_total pattern below).
const onMergeResolution = resolveEffectiveAiReviewOnMerge(input.onMerge, plan?.onMerge);
const onMerge = onMergeResolution.onMerge;
if (onMergeResolution.clamped) {
incr("gittensory_ai_review_onmerge_clamped_total", { mode: input.mode });
}
const dual = combine !== "single" && (!configured || configured.length > 1);
const freeAiCalls =
(input.mode === "block" ? (dual ? 2 : 1) : 0) + (input.providerKey ? 0 : 1);
Expand Down
96 changes: 95 additions & 1 deletion src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ export type FocusManifestGateConfig = {
/** `gate.aiReview.closeConfidence` (#7): minimum calibrated AI-reviewer confidence (0-1) for an AI defect to BLOCK
* under `aiReview.mode: block`. null (unset) ⇒ the gate's 0.93 default. Clamped to [0,1] at parse time. */
aiReviewCloseConfidence: number | null;
/** `gate.aiReview.combine` (#2567): per-repo override of the self-host operator's `AI_REVIEW_PLAN.combine`
* boot default (single/consensus/synthesis). null (unset) ⇒ the operator's plan (or `consensus`). A
* REFINEMENT only — see {@link aiReviewOnMerge} for the operator-floor clamp `runGittensoryAiReview` applies
* to the paired `onMerge` field; `combine` itself is not floor-clamped (the three strategies are not ordered
* by strictness, so there is no single "loosening" direction to clamp). */
aiReviewCombine: import("../types").CombineStrategy | null;
/** `gate.aiReview.onMerge` (#2567): per-repo override of the `synthesis` merge rule. `either` is the STRICTER
* rule (any one reviewer's blocker blocks/holds); `both` is more PERMISSIVE (requires every reviewer to
* agree). null (unset) ⇒ the operator's `AI_REVIEW_PLAN.onMerge`. A repo may only TIGHTEN the operator's
* floor (never loosen `either` down to `both`) — `runGittensoryAiReview` enforces the clamp at resolve time,
* since only it can see both the per-repo value and the operator's plan. */
aiReviewOnMerge: import("../types").OnMerge | null;
/** `gate.aiReview.reviewers` (#2567): per-repo override of the named reviewer pair(s) to run, in place of the
* operator's `AI_REVIEW_PLAN.reviewers` (or the free Workers-AI pair when the operator configured none). null
* (unset) ⇒ the operator's plan. No operator floor applies to WHICH reviewers run (only `onMerge` gates
* strictness), so this always wins unclamped when set. */
aiReviewReviewers: ReadonlyArray<{ model: string; fallback?: string | null | undefined }> | null;
mergeReadiness: GateRuleMode | null;
manifestPolicy: GateRuleMode | null;
selfAuthoredLinkedIssue: GateRuleMode | null;
Expand Down Expand Up @@ -320,6 +337,9 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = {
aiReviewModel: null,
aiReviewAllAuthors: null,
aiReviewCloseConfidence: null,
aiReviewCombine: null,
aiReviewOnMerge: null,
aiReviewReviewers: null,
mergeReadiness: null,
manifestPolicy: null,
selfAuthoredLinkedIssue: null,
Expand Down Expand Up @@ -494,6 +514,49 @@ function normalizeOptionalConfidence(value: JsonValue | undefined, field: string
return Math.max(0, Math.min(1, value));
}

// A hard cap on `gate.aiReview.reviewers` entries — the combiner only ever addresses reviewer[0]/[1] (single runs
// one, consensus/synthesis run two), so anything beyond 2 is inert; capping at 4 leaves headroom without letting a
// hostile/huge manifest bloat the parsed config for no functional gain.
const MAX_AI_REVIEW_REVIEWERS = 4;

/** Normalize `gate.aiReview.reviewers` (#2567) — a list of `{ model, fallback? }` entries naming self-host
* providers (e.g. `claude-code`, `codex`) to run in place of the operator's `AI_REVIEW_PLAN.reviewers`. Each
* entry needs a non-empty string `model`; `fallback` is optional and, when present, must also be a non-empty
* string. Invalid entries are dropped with a warning rather than failing the whole list, mirroring the other
* manifest list parsers. Absent/empty/all-invalid ⇒ null (so the resolver's `??` fallback to the operator's
* plan is untouched). */
function normalizeOptionalReviewers(
value: JsonValue | undefined,
field: string,
warnings: string[],
): ReadonlyArray<{ model: string; fallback?: string | null | undefined }> | null {
if (value === undefined || value === null) return null;
if (!Array.isArray(value)) {
warnings.push(`Manifest gate field "${field}" must be a list of { model, fallback? }; ignoring it.`);
return null;
}
const out: Array<{ model: string; fallback?: string | null | undefined }> = [];
for (const [index, entry] of value.entries()) {
if (out.length >= MAX_AI_REVIEW_REVIEWERS) {
warnings.push(`Manifest gate field "${field}" is capped at ${MAX_AI_REVIEW_REVIEWERS} entries; dropping the rest.`);
break;
}
if (entry === null || typeof entry !== "object" || Array.isArray(entry)) {
warnings.push(`Manifest gate field "${field}[${index}]" must be a mapping with a "model" string; ignoring it.`);
continue;
}
const e = entry as Record<string, JsonValue>;
const model = typeof e.model === "string" ? e.model.trim() : "";
if (!model) {
warnings.push(`Manifest gate field "${field}[${index}].model" must be a non-empty string; ignoring the entry.`);
continue;
}
const fallback = typeof e.fallback === "string" && e.fallback.trim() ? e.fallback.trim() : undefined;
out.push(fallback ? { model, fallback } : { model });
}
return out.length > 0 ? out : null;
}

/**
* Parse the optional `gate:` mapping. Every field stays `null` when unset so the resolver can layer
* this OVER DB settings without clobbering. A nested `readiness: { mode, minScore }` block is accepted.
Expand Down Expand Up @@ -544,6 +607,9 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
aiReviewModel: normalizeOptionalString(aiReviewRecord?.model, "gate.aiReview.model", warnings),
aiReviewAllAuthors: normalizeOptionalBoolean(aiReviewRecord?.allAuthors, "gate.aiReview.allAuthors", warnings),
aiReviewCloseConfidence: normalizeOptionalConfidence(aiReviewRecord?.closeConfidence, "gate.aiReview.closeConfidence", warnings),
aiReviewCombine: normalizeOptionalEnum(aiReviewRecord?.combine, "gate.aiReview.combine", ["single", "consensus", "synthesis"] as const, warnings),
aiReviewOnMerge: normalizeOptionalEnum(aiReviewRecord?.onMerge, "gate.aiReview.onMerge", ["either", "both"] as const, warnings),
aiReviewReviewers: normalizeOptionalReviewers(aiReviewRecord?.reviewers, "gate.aiReview.reviewers", warnings),
mergeReadiness: normalizeOptionalGateMode(record.mergeReadiness, "gate.mergeReadiness", warnings),
manifestPolicy: normalizeOptionalGateMode(record.manifestPolicy, "gate.manifestPolicy", warnings),
selfAuthoredLinkedIssue: normalizeOptionalGateMode(record.selfAuthoredLinkedIssue, "gate.selfAuthoredLinkedIssue", warnings),
Expand Down Expand Up @@ -577,6 +643,9 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
gate.aiReviewModel !== null ||
gate.aiReviewAllAuthors !== null ||
gate.aiReviewCloseConfidence !== null ||
gate.aiReviewCombine !== null ||
gate.aiReviewOnMerge !== null ||
gate.aiReviewReviewers !== null ||
gate.mergeReadiness !== null ||
gate.manifestPolicy !== null ||
gate.selfAuthoredLinkedIssue !== null ||
Expand Down Expand Up @@ -613,14 +682,31 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue {
if (gate.slopAiAdvisory !== null) slop.aiAdvisory = gate.slopAiAdvisory;
out.slop = slop;
}
if (gate.aiReviewMode !== null || gate.aiReviewByok !== null || gate.aiReviewProvider !== null || gate.aiReviewModel !== null || gate.aiReviewAllAuthors !== null || gate.aiReviewCloseConfidence !== null) {
if (
gate.aiReviewMode !== null ||
gate.aiReviewByok !== null ||
gate.aiReviewProvider !== null ||
gate.aiReviewModel !== null ||
gate.aiReviewAllAuthors !== null ||
gate.aiReviewCloseConfidence !== null ||
gate.aiReviewCombine !== null ||
gate.aiReviewOnMerge !== null ||
gate.aiReviewReviewers !== null
) {
const aiReview: Record<string, JsonValue> = {};
if (gate.aiReviewMode !== null) aiReview.mode = gate.aiReviewMode;
if (gate.aiReviewByok !== null) aiReview.byok = gate.aiReviewByok;
if (gate.aiReviewProvider !== null) aiReview.provider = gate.aiReviewProvider;
if (gate.aiReviewModel !== null) aiReview.model = gate.aiReviewModel;
if (gate.aiReviewAllAuthors !== null) aiReview.allAuthors = gate.aiReviewAllAuthors;
if (gate.aiReviewCloseConfidence !== null) aiReview.closeConfidence = gate.aiReviewCloseConfidence;
if (gate.aiReviewCombine !== null) aiReview.combine = gate.aiReviewCombine;
if (gate.aiReviewOnMerge !== null) aiReview.onMerge = gate.aiReviewOnMerge;
if (gate.aiReviewReviewers !== null) {
aiReview.reviewers = gate.aiReviewReviewers.map((r) =>
r.fallback ? { model: r.model, fallback: r.fallback } : { model: r.model },
) as JsonValue;
}
out.aiReview = aiReview;
}
if (gate.mergeReadiness !== null) out.mergeReadiness = gate.mergeReadiness;
Expand Down Expand Up @@ -1272,6 +1358,14 @@ export function resolveEffectiveSettings(
if (gate.aiReviewModel !== null) effective.aiReviewModel = gate.aiReviewModel;
if (gate.aiReviewAllAuthors !== null) effective.aiReviewAllAuthors = gate.aiReviewAllAuthors;
if (gate.aiReviewCloseConfidence !== null) effective.aiReviewCloseConfidence = gate.aiReviewCloseConfidence;
// Dual-AI combine/onMerge/reviewers overrides (#2567) are projected onto `effective` unclamped here — they are
// a REFINEMENT of the operator's AI_REVIEW_PLAN, not a replacement for it, so the actual operator-floor clamp
// (onMerge can only TIGHTEN, never loosen) happens where both the per-repo value AND the operator's plan are
// visible: `resolveEffectiveAiReviewOnMerge` in services/ai-review.ts, called from the review call site. This
// resolver has no access to `env.AI_REVIEW_PLAN`, so it cannot itself enforce the floor.
if (gate.aiReviewCombine !== null) effective.aiReviewCombine = gate.aiReviewCombine;
if (gate.aiReviewOnMerge !== null) effective.aiReviewOnMerge = gate.aiReviewOnMerge;
if (gate.aiReviewReviewers !== null) effective.aiReviewReviewers = gate.aiReviewReviewers;
if (gate.mergeReadiness !== null) effective.mergeReadinessGateMode = gate.mergeReadiness;
if (gate.manifestPolicy !== null) effective.manifestPolicyGateMode = gate.manifestPolicy;
if (gate.selfAuthoredLinkedIssue !== null) effective.selfAuthoredLinkedIssueGateMode = gate.selfAuthoredLinkedIssue;
Expand Down
Loading
Loading