Skip to content
Open
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
2 changes: 2 additions & 0 deletions src/selfhost/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [
["loopover_active_review_reconciliation_terminalized_total", { help: "Orphaned active_review_tracking rows terminalized after a live GitHub check confirmed the PR is closed, by repo.", type: "counter" }],
["loopover_open_pr_reconciliation_missing_total", { help: "Open PRs found missing from local tracking during reconciliation, by repo.", type: "counter" }],
["loopover_orb_relay_malformed_events_total", { help: "Orb relay batch entries dropped for missing/mistyped required fields (deliveryId/eventName/rawBody).", type: "counter" }],
["loopover_orb_relay_multiple_live_enrollments_total", { help: "Forwarded orb events where more than one LIVE enrollment existed for the installation (a blue/green swap, or a secret rotated but not yet revoked) -- the winner is still elected deterministically, but the overlap is no longer silent (#9150).", type: "counter" }],
["loopover_orb_relay_register_total", { help: "Orb relay registration attempts, by mode and result (registered/recovered/failed).", type: "counter" }],
["loopover_pr_outcomes_total", { help: "Recorded PR gate outcomes, by decision.", type: "counter" }],
["loopover_close_audit_holdouts_total", { help: "Would-auto-close PRs diverted to human adjudication by the close-audit holdout (#8831).", type: "counter" }],
Expand All @@ -181,6 +182,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [
["loopover_rees_enrich_request_duration_seconds", { help: "REES /v1/enrich call duration in seconds, for calls that were actually attempted (excludes the auth-rejected circuit-breaker skip).", type: "histogram" }],
["loopover_metrics_sampler_errors_total", { help: "Scrape-time gauge sampler failures, by metric name -- a failing sampler previously emitted no series at all, silently. Any occurrence means that metric's value is currently invisible to Prometheus this scrape (see the sentinel gauges' own -1-on-failure convention).", type: "counter" }],
["loopover_review_source_fresh", { help: "1 when a review/ops/reputation source table has a row inside its own consumer's window, 0 when stale -- labeled by table and window_days. review_targets was silently orphaned by the 2026-06-22 convergence cutover for months before anyone noticed; this makes the next such orphaning loud instead.", type: "gauge" }],
["loopover_private_manifest_warnings_total", { help: "Private-manifest layer warnings (a malformed shared/global/repo config layer dropped during load), counted one per warning rather than one per load -- a sustained run means a mount is repeatedly serving truncated or invalid config (#9065).", type: "counter" }],
];
const metricMeta = new Map<string, MetricMeta>(DEFAULT_METRIC_META);

Expand Down
16 changes: 13 additions & 3 deletions test/unit/salvageability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,21 @@ describe("resolveAiReviewSalvageableHold", () => {
expect(hold!.comment).toContain("HELD with guidance");
});

it("defaults: no configured floor uses the gate default, and a confidence-less blocker counts as certainty (at/above floor)", () => {
// #9085 (landed via #9237): an absent confidence used to degrade to 1.0 -- maximum certainty -- so "the
// model said nothing" silently cleared even the default floor here. It now degrades to
// CONFIDENCE_WHEN_UNSTATED (0.5), which is sub-floor against the 0.93 gate default, so the low-confidence
// hold owns that case and this resolver stands down. #9085 renamed both sibling assertions in
// rules.test.ts but missed this third consumption site, leaving it asserting the pre-change semantics.
// Both calls are load-bearing for coverage: the first is the ONLY case in this file that exercises the
// `aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE` default arm (every other case passes an
// explicit floor), and the second is the only one exercising the `confidence ?? CONFIDENCE_WHEN_UNSTATED`
// nullish arm.
it("defaults: no configured floor uses the gate default (0.93), and a confidence-less blocker is sub-floor", () => {
expect(resolveAiReviewSalvageableHold(aiEval(0.99), { aiReviewSalvageabilityMinScore: 60 }, salv)).toBeDefined();

const noConfidence = aiEval(0.99);
delete (noConfidence.blockers[0] as { confidence?: number }).confidence;
const hold = resolveAiReviewSalvageableHold(noConfidence, { aiReviewSalvageabilityMinScore: 60 }, salv);
expect(hold).toBeDefined();
expect(resolveAiReviewSalvageableHold(noConfidence, { aiReviewSalvageabilityMinScore: 60 }, salv)).toBeUndefined();
});

it("knob unset (the default) or no score: never changes a disposition", () => {
Expand Down
9 changes: 7 additions & 2 deletions test/unit/selfhost-pg-retention.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ function makeRetentionPgPool(remaining: Record<string, number> = {}): MockPgPool
return { rows: [{ n: remaining[table] ?? 0 }], rowCount: 1 };
}

const deleteMatch = /^DELETE FROM (\w+) WHERE ctid IN \(SELECT ctid FROM \1 WHERE .*? LIMIT (\d+)\)$/i.exec(q);
// #9083 (via #9237) changed the emitted shape: retention now deletes by PRIMARY KEY (`id`, `delivery_id`,
// ...) with an explicit `ORDER BY <retention column>`, falling back to the physical row key only for the
// composite-key tables that have no single-column PK -- an index-backed range delete replacing a
// full-scan-per-batch semi-join. Group 2 captures whichever key column is in play, so BOTH paths (mapped
// PK and the `ctid` fallback) stay exercised by this mock rather than one silently ceasing to match.
const deleteMatch = /^DELETE FROM (\w+) WHERE (\w+) IN \(SELECT \2 FROM \1 WHERE .*? ORDER BY \w+ LIMIT (\d+)\)$/i.exec(q);
if (deleteMatch) {
const table = deleteMatch[1] as string;
const limit = Number(deleteMatch[2]);
const limit = Number(deleteMatch[3]);
const have = remaining[table] ?? 0;
const changes = Math.min(have, limit);
remaining[table] = have - changes;
Expand Down
28 changes: 24 additions & 4 deletions test/unit/worker-entry-boundary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,34 @@ describe("worker entry boundary", () => {
expect(forbidden, `worker entry must not reach agent-only modules: ${forbidden.join(", ")}`).toEqual([]);
});

it("does not reference pixelmatch, pngjs, visual-diff, gifenc, or sharp in worker-reachable source", () => {
// Scans MODULE SPECIFIERS, not raw file text. What this guards is the Worker BUNDLE: a Node-only dep can
// only get bundled by being imported (statically or dynamically), so parsing the same specifiers
// collectReachableSources already walks catches every real inclusion path. Grepping whole-file content
// instead produced a false positive the moment ordinary English prose contained one of these words --
// #9230 added the user-facing string "route(s) crossed the visual-diff threshold" to
// src/review/visual/visual-findings.ts (a file worker-reachable since #4120, importing none of these deps),
// and the raw-content regex failed a green tree over a sentence. Bending correct user-facing copy to dodge
// a test regex would have been the wrong repair; narrowing the check to what it actually means is the right
// one.
it("does not import pixelmatch, pngjs, visual-diff, gifenc, or sharp from worker-reachable source", () => {
const hits = collectReachableSources(WORKER_ENTRY)
.map((file) => {
const content = readFileSync(file, "utf8");
return FORBIDDEN_IDENTIFIERS.test(content) ? relativeToRoot(file) : null;
const offending = parseImportSpecifiers(file).filter((specifier) => FORBIDDEN_IDENTIFIERS.test(specifier));
return offending.length > 0 ? `${relativeToRoot(file)} (${offending.join(", ")})` : null;
})
.filter((entry): entry is string => entry !== null);
expect(hits, `worker-reachable files must not mention Node-only visual diff/GIF/image deps: ${hits.join(", ")}`).toEqual([]);
expect(hits, `worker-reachable files must not import Node-only visual diff/GIF/image deps: ${hits.join(", ")}`).toEqual([]);
});

// Proves the specifier-scoped check above is still DISCRIMINATING, not vacuously passing: the same regex
// must still flag a real dependency import, and must still ignore the same word in prose. Without this, a
// future edit that broke the matching entirely would look identical to a clean tree.
it("the forbidden-identifier check still flags a real import specifier and still ignores prose", () => {
expect(["sharp", "pixelmatch", "gifenc", "pngjs", "@foo/visual-diff"].every((specifier) => FORBIDDEN_IDENTIFIERS.test(specifier))).toBe(true);
expect(["./capture", "../../types", "node:fs", "hono"].some((specifier) => FORBIDDEN_IDENTIFIERS.test(specifier))).toBe(false);
// The exact #9230 prose that broke the old whole-file scan is not a module specifier, so it is correctly
// invisible to a specifier-scoped check -- while the bare dep name it contains still is not.
expect(parseImportSpecifiers(join(srcRoot, "review/visual/visual-findings.ts")).some((s) => FORBIDDEN_IDENTIFIERS.test(s))).toBe(false);
});

it("does not reference visual diff or GIF modules in the published MCP bin bundle", () => {
Expand Down