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
7 changes: 7 additions & 0 deletions .loopover.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,13 @@ review:
# Bool or null. Default: null/true -- no change to today's watched-repo-set behavior.
# prReconciliation: false

# Per-repo FORCE-OFF for the active-review-tracking reconciliation sweep (#webhook-reorder-clobber) --
# `false` excludes just this repo's rows from the sweep's scan even though the operator's
# LOOPOVER_ACTIVE_REVIEW_RECONCILIATION flag is on. Deliberately FORCE-OFF-ONLY (no `true` override),
# mirroring `prReconciliation` immediately above. Bool or null. Default: null/true -- no change to today's
# scanned-row-set behavior.
# activeReviewReconciliation: false

# Repeat-false-positive suppression (#2179, part of #1964). Bool | null. Default: null/false — byte-identical
# (no suppression-store read, no matching). Also requires the operator's LOOPOVER_REVIEW_MEMORY env flag to
# be on -- this manifest field alone cannot enable it. When both are on, an advisory (non-blocking) AI finding
Expand Down
7 changes: 7 additions & 0 deletions config/examples/loopover.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,13 @@ review:
# Bool or null. Default: null/true -- no change to today's watched-repo-set behavior.
# prReconciliation: false

# Per-repo FORCE-OFF for the active-review-tracking reconciliation sweep (#webhook-reorder-clobber) --
# `false` excludes just this repo's rows from the sweep's scan even though the operator's
# LOOPOVER_ACTIVE_REVIEW_RECONCILIATION flag is on. Deliberately FORCE-OFF-ONLY (no `true` override),
# mirroring `prReconciliation` immediately above. Bool or null. Default: null/true -- no change to today's
# scanned-row-set behavior.
# activeReviewReconciliation: false

# Repeat-false-positive suppression (#2179, part of #1964). Bool | null. Default: null/false — byte-identical
# (no suppression-store read, no matching). Also requires the operator's LOOPOVER_REVIEW_MEMORY env flag to
# be on -- this manifest field alone cannot enable it. When both are on, an advisory (non-blocking) AI finding
Expand Down
2 changes: 1 addition & 1 deletion packages/loopover-engine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
],
"scripts": {
"build": "tsc -p tsconfig.json",
"test": "npm run build && rm -rf dist-test && tsc -p tsconfig.test.json && node --test \"dist-test/**/*.test.js\""
"test": "npm run build && rm -rf dist-test .test.tsbuildinfo && tsc -p tsconfig.test.json && node --test \"dist-test/**/*.test.js\""
},
"dependencies": {
"@anthropic-ai/claude-agent-sdk": "^0.3.205",
Expand Down
24 changes: 20 additions & 4 deletions packages/loopover-engine/src/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,9 @@ export type FocusManifestPrReconciliationConfig = {
* (LOOPOVER_ACTIVE_REVIEW_RECONCILIATION), declared under top-level `activeReviewReconciliation:`
* (#webhook-reorder-clobber). Same shape and precedence as `prReconciliation:` above -- the sweep re-checks
* `active_review_tracking` rows a delayed webhook job left stuck "active" for a PR that already closed.
* Not present ⇒ the caller falls back to the LOOPOVER_ACTIVE_REVIEW_RECONCILIATION env var.
* Not present ⇒ the caller falls back to the LOOPOVER_ACTIVE_REVIEW_RECONCILIATION env var. Distinct from
* the per-repo FORCE-OFF under `review.activeReviewReconciliation` (which only excludes one repo's rows
* from the scan set once this fleet-wide gate is ON).
*/
export type FocusManifestActiveReviewReconciliationConfig = {
present: boolean;
Expand Down Expand Up @@ -793,6 +795,14 @@ export type FocusManifestReviewConfig = {
* above, for the identical reason. A manifest-load error fails OPEN (the repo stays watched). null/true
* (default, absent) ⇒ no change to today's watched-repo-set behavior. */
prReconciliation: boolean | null;
/** `review.activeReviewReconciliation` (#webhook-reorder-clobber): explicit per-repo FORCE-OFF for the
* active-review-tracking reconciliation sweep (`runActiveReviewReconciliation`,
* `src/review/active-review-reconciliation.ts`) — `false` excludes this repo's stale `active_review_tracking`
* rows from the sweep's scan even though the global `LOOPOVER_ACTIVE_REVIEW_RECONCILIATION` kill-switch is
* on. Deliberately FORCE-OFF-ONLY (no `true` override), mirroring `selftune`/`sweepWatchdog`/`prReconciliation`
* above, for the identical reason. A manifest-load error fails OPEN (the repo's rows stay eligible). null/true
* (default, absent) ⇒ no change to today's scanned-row-set behavior. */
activeReviewReconciliation: boolean | null;
/** `review.memory` (#2179, config slice of #1964): when true, gates repeat-false-positive SUPPRESSION —
* before an advisory (non-blocking) AI finding is surfaced in the unified review comment, it is matched
* against this repo's stored `review_suppression` signals (a maintainer's own past false-positive
Expand Down Expand Up @@ -1412,7 +1422,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, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, activeReviewReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null },
features: { ...EMPTY_FEATURES_CONFIG },
experimental: { ...EMPTY_EXPERIMENTAL_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
Expand Down Expand Up @@ -1453,7 +1463,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, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, activeReviewReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null },
features: { ...EMPTY_FEATURES_CONFIG },
experimental: { ...EMPTY_EXPERIMENTAL_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
Expand Down Expand Up @@ -2937,7 +2947,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, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, activeReviewReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: 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.`);
Expand Down Expand Up @@ -2984,6 +2994,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
const selftune = normalizeOptionalBoolean(r.selftune, "review.selftune", warnings);
const sweepWatchdog = normalizeOptionalBoolean(r.sweepWatchdog, "review.sweepWatchdog", warnings);
const prReconciliation = normalizeOptionalBoolean(r.prReconciliation, "review.prReconciliation", warnings);
const activeReviewReconciliation = normalizeOptionalBoolean(r.activeReviewReconciliation, "review.activeReviewReconciliation", warnings);
const reviewMemory = normalizeOptionalBoolean(r.memory, "review.memory", warnings);
const findingCategories = normalizeOptionalBoolean(r.finding_categories, "review.finding_categories", warnings);
const inlineCommentsPerCategory = normalizeOptionalNonNegativeInt(
Expand Down Expand Up @@ -3028,6 +3039,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
selftune !== null ||
sweepWatchdog !== null ||
prReconciliation !== null ||
activeReviewReconciliation !== null ||
reviewMemory !== null ||
findingCategories !== null ||
inlineCommentsPerCategory !== null ||
Expand Down Expand Up @@ -3069,6 +3081,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
selftune,
sweepWatchdog,
prReconciliation,
activeReviewReconciliation,
reviewMemory,
findingCategories,
inlineCommentsPerCategory,
Expand Down Expand Up @@ -3174,6 +3187,7 @@ function computeReviewConfigPresent(review: Omit<FocusManifestReviewConfig, "pre
review.selftune !== null ||
review.sweepWatchdog !== null ||
review.prReconciliation !== null ||
review.activeReviewReconciliation !== null ||
review.reviewMemory !== null ||
review.findingCategories !== null ||
review.inlineCommentsPerCategory !== null ||
Expand Down Expand Up @@ -3221,6 +3235,7 @@ export function overlayReviewConfig(
selftune: pickOverlayNullable(override.selftune, base.selftune),
sweepWatchdog: pickOverlayNullable(override.sweepWatchdog, base.sweepWatchdog),
prReconciliation: pickOverlayNullable(override.prReconciliation, base.prReconciliation),
activeReviewReconciliation: pickOverlayNullable(override.activeReviewReconciliation, base.activeReviewReconciliation),
reviewMemory: pickOverlayNullable(override.reviewMemory, base.reviewMemory),
findingCategories: pickOverlayNullable(override.findingCategories, base.findingCategories),
inlineCommentsPerCategory: pickOverlayNullable(override.inlineCommentsPerCategory, base.inlineCommentsPerCategory),
Expand Down Expand Up @@ -3796,6 +3811,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
if (review.selftune !== null) out.selftune = review.selftune;
if (review.sweepWatchdog !== null) out.sweepWatchdog = review.sweepWatchdog;
if (review.prReconciliation !== null) out.prReconciliation = review.prReconciliation;
if (review.activeReviewReconciliation !== null) out.activeReviewReconciliation = review.activeReviewReconciliation;
if (review.reviewMemory !== null) out.memory = review.reviewMemory;
if (review.findingCategories !== null) out.finding_categories = review.findingCategories;
if (review.inlineCommentsPerCategory !== null) out.inline_comments_per_category = review.inlineCommentsPerCategory;
Expand Down
6 changes: 6 additions & 0 deletions src/review/active-review-reconciliation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ export async function runActiveReviewReconciliation(env: Env, nowMs: number = Da
try {
const repo = await getRepository(env, row.repoFullName);
if (!repo || typeof repo.installationId !== "number") continue;
// Per-repo opt-out (#webhook-reorder-clobber): mirrors pr-reconciliation.ts's watchedRepos() FORCE-OFF
// exactly -- an explicit per-repo `review.activeReviewReconciliation: false` excludes just this repo's
// rows from the sweep even though the fleet-wide gate is on. A manifest-load error fails OPEN (the
// row stays eligible), matching the surrounding scan's own settings-blip fail-safe.
const manifest = await loadRepoFocusManifest(env, row.repoFullName).catch(() => null);
if (manifest?.review.activeReviewReconciliation === false) continue;
const token = (await createInstallationToken(env, repo.installationId).catch(() => undefined)) ?? env.GITHUB_PUBLIC_TOKEN;
const admissionKey = githubRateLimitAdmissionKeyForToken(env, token, repo.installationId);
const liveState = await fetchLivePullRequestState(env, row.repoFullName, row.pullNumber, token, admissionKey);
Expand Down
36 changes: 36 additions & 0 deletions test/unit/active-review-reconciliation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as repositoriesModule from "../../src/db/repositories";
import * as backfillModule from "../../src/github/backfill";
import { counterValue, resetMetrics } from "../../src/selfhost/metrics";
import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader";
import * as focusManifestLoaderModule from "../../src/signals/focus-manifest-loader";
import { createTestEnv } from "../helpers/d1";

const SELF_REPO = "JSONbored/loopover";
Expand Down Expand Up @@ -177,6 +178,41 @@ describe("runActiveReviewReconciliation (#webhook-reorder-clobber)", () => {
expect(liveSpy).not.toHaveBeenCalled();
});

it("REGRESSION: an explicit review.activeReviewReconciliation: false excludes an otherwise-eligible repo's rows from the scan entirely", async () => {
const env = createTestEnv();
await seedStaleActiveRow(env, "owner/opted-out", 9, 9507, STALE_ACTIVE_REVIEW_MIN_AGE_MS + 60_000);
await upsertRepoFocusManifest(env, "owner/opted-out", { review: { activeReviewReconciliation: false } });
const liveSpy = vi.spyOn(backfillModule, "fetchLivePullRequestState");

const reconciled = await runActiveReviewReconciliation(env);

expect(reconciled).toEqual([]);
expect(liveSpy).not.toHaveBeenCalled(); // opted-out before any GitHub call is ever spent
expect(await hasActiveReviewForHeadSha(env, "owner/opted-out", 9, "sha1")).toBe(true); // row untouched
});

it("an explicit review.activeReviewReconciliation: true is a no-op -- the repo's rows are scanned exactly as when unset", async () => {
const env = createTestEnv();
await seedStaleActiveRow(env, "owner/opted-in", 10, 9508, STALE_ACTIVE_REVIEW_MIN_AGE_MS + 60_000);
await upsertRepoFocusManifest(env, "owner/opted-in", { review: { activeReviewReconciliation: true } });
vi.spyOn(backfillModule, "fetchLivePullRequestState").mockResolvedValueOnce("closed");

const reconciled = await runActiveReviewReconciliation(env);

expect(reconciled).toEqual([{ repoFullName: "owner/opted-in", pullNumber: 10 }]);
});

it("fails OPEN on a manifest-load error for the row's own repo -- a config-read blip must never silently exclude a row from reconciliation", async () => {
const env = createTestEnv();
await seedStaleActiveRow(env, "owner/manifest-errors", 11, 9509, STALE_ACTIVE_REVIEW_MIN_AGE_MS + 60_000);
vi.spyOn(focusManifestLoaderModule, "loadRepoFocusManifest").mockRejectedValueOnce(new Error("manifest load failed"));
vi.spyOn(backfillModule, "fetchLivePullRequestState").mockResolvedValueOnce("closed");

const reconciled = await runActiveReviewReconciliation(env);

expect(reconciled).toEqual([{ repoFullName: "owner/manifest-errors", pullNumber: 11 }]);
});

it("fails safe per-row: an error on one row is logged and the scan continues to the next row", async () => {
const env = createTestEnv();
await seedStaleActiveRow(env, "owner/erroring-repo", 6, 9504, STALE_ACTIVE_REVIEW_MIN_AGE_MS + 60_000);
Expand Down
5 changes: 4 additions & 1 deletion test/unit/db-parsers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,10 @@ describe("database row parser hardening", () => {
number: 36, title: "PR", state: "open", user: { login: "bob" }, head: { sha: "a2" }, labels: [], updated_at: "2026-07-21T12:05:00.000Z",
});
expect(pushed.headSha).toBe("a2");
expect(pushed.headShaObservedAt).not.toBe(first.headShaObservedAt);
// Not asserting inequality against first.headShaObservedAt (both could land in the same millisecond in
// a fast test run, matching the same caveat the review-latency-metric tests above already document) --
// typeof "string" here plus the exact-preservation assertion below is the deterministic, non-flaky check.
expect(typeof pushed.headShaObservedAt).toBe("string");

// A delayed job for the OLDER `review_requested` event (before the push) finally dequeues, still carrying
// the STALE head "a1". Without resolvedHeadSha driving headShaChanged, this would look like "a2 -> a1", a
Expand Down
Loading