From ef4079a1755256073d4cbf2d78766fd69335acb3 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:07:53 -0700 Subject: [PATCH 1/4] fix(upstream): bound the raw-github fallback fetch with a timeout (#9165) fetchTrackedSource's raw-GitHub fallback was the only fetch in ruleset.ts without a timeout, so a stalled raw.githubusercontent.com connection hung the whole scheduled refreshUpstreamDrift job. Route it through the same timeoutFetch every other call site in the file already uses, and confirm the Promise.all fan-out degrades per-source rather than all-or-nothing. Sweep the rest of src/ for the same bare-fetch shape: three GitHub calls in github-oauth.ts (device-flow start/poll, web-OAuth code exchange) and one call each in linear-adapter.ts and registry/sync.ts had no timeout and are now bounded the same way. --- src/auth/github-oauth.ts | 10 ++++--- src/integrations/linear-adapter.ts | 5 ++++ src/registry/sync.ts | 6 +++++ src/upstream/ruleset.ts | 10 ++++++- test/unit/upstream-ruleset.test.ts | 42 ++++++++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/auth/github-oauth.ts b/src/auth/github-oauth.ts index 99d129a6e4..6efa98cd3d 100644 --- a/src/auth/github-oauth.ts +++ b/src/auth/github-oauth.ts @@ -42,7 +42,11 @@ type GitHubWebOAuthState = { export async function startGitHubDeviceFlow(env: Env): Promise { if (!env.GITHUB_OAUTH_CLIENT_ID) throw new Error("github_oauth_not_configured"); - const response = await fetch("https://github.com/login/device/code", { + // #9165 sweep: this file's OTHER GitHub calls (createSessionFromGitHubToken, verifyTokenBelongsToApp, + // refreshGitHubUserToken below) already route through timeoutFetch -- this bare fetch and the two below it + // were the only ones left with no timeout, so a stalled github.com connection during device-flow start/poll + // or the web-OAuth code exchange would have hung indefinitely instead of failing bounded. + const response = await timeoutFetch("https://github.com/login/device/code", { method: "POST", headers: { accept: "application/json", @@ -68,7 +72,7 @@ export async function startGitHubDeviceFlow(env: Env): Promise(apiKey: string, query: string, variables: Record method: "POST", headers: { "Content-Type": "application/json", Authorization: apiKey }, body: JSON.stringify({ query, variables }), + signal: AbortSignal.timeout(LINEAR_FETCH_TIMEOUT_MS), }); if (!response.ok) throw new Error(`Linear API HTTP ${response.status}`); const body = (await response.json()) as { data?: T } & LinearGraphQlErrorResponse; diff --git a/src/registry/sync.ts b/src/registry/sync.ts index 4027d99fac..467cb565bb 100644 --- a/src/registry/sync.ts +++ b/src/registry/sync.ts @@ -8,6 +8,11 @@ import type { RegistrySnapshot } from "../types"; import { errorMessage, jsonString, nowIso, repoParts } from "../utils/json"; import { normalizeRegistryPayload } from "./normalize"; +// #9165 sweep: this loop's `fallbackUrl` candidate is the same raw-GitHub-fallback shape as +// src/upstream/ruleset.ts's fetchTrackedSource -- a stalled connection to any candidate (API or raw fallback) +// hung this whole sequential probe with no bound, since `fetch()` has no default timeout. +const REGISTRY_SYNC_FETCH_TIMEOUT_MS = 12_000; + const API_CANDIDATES = [ "https://api.gittensor.io/repositories", "https://api.gittensor.io/api/repositories", @@ -40,6 +45,7 @@ export async function refreshRegistry(env: Env): Promise { accept: "application/json", "user-agent": PRODUCT_USER_AGENT, }, + signal: AbortSignal.timeout(REGISTRY_SYNC_FETCH_TIMEOUT_MS), }); if (!response.ok) { warnings.push(`Registry probe failed: ${url} (${response.status})`); diff --git a/src/upstream/ruleset.ts b/src/upstream/ruleset.ts index 5c33443001..dec422018b 100644 --- a/src/upstream/ruleset.ts +++ b/src/upstream/ruleset.ts @@ -107,6 +107,10 @@ export async function refreshUpstreamSourceSnapshots(env: Env): Promise raw-fallback -> a `status: "error"` snapshot on the previous?.parsed carry-forward), it never + // throws, so one stalled/failing TRACKED_SOURCES entry can never reject this Promise.all or block its + // siblings from completing. const snapshots = await Promise.all( TRACKED_SOURCES.map((source) => fetchTrackedSource(env, config, source, fetchedAt, commitSha, previousByKey.get(source.key))), ); @@ -517,7 +521,11 @@ async function fetchTrackedSource( } try { - const response = await fetch(rawUrl(config, source.path), { headers: githubHeaders({ token: env.GITHUB_PUBLIC_TOKEN, accept: "text/plain" }) }); + // #9165: this raw-GitHub fallback was the only fetch in this file with no timeout -- a stalled + // raw.githubusercontent.com connection (a real CDN failure mode, and exactly the case this fallback exists + // to handle) hung the whole scheduled refreshUpstreamDrift job indefinitely. timeoutFetch matches every + // other call site in this file (the primary contents-API read above, plus the drift-issue GitHub calls). + const response = await timeoutFetch(rawUrl(config, source.path), { headers: githubHeaders({ token: env.GITHUB_PUBLIC_TOKEN, accept: "text/plain" }) }); if (!response.ok) throw new Error(`${response.status} ${response.statusText}`); return sourceSnapshotFromContent({ config, diff --git a/test/unit/upstream-ruleset.test.ts b/test/unit/upstream-ruleset.test.ts index 43b01e4376..ef2506998c 100644 --- a/test/unit/upstream-ruleset.test.ts +++ b/test/unit/upstream-ruleset.test.ts @@ -149,6 +149,27 @@ describe("upstream ruleset drift tracking", () => { expect(stored).toHaveLength(6); }); + it("degrades a single hung/timed-out source to an error snapshot without blocking its siblings (#9165)", async () => { + // The raw-GitHub fallback used to be the only fetch in ruleset.ts with no timeout: a stalled + // raw.githubusercontent.com connection hung the entire scheduled refresh. This simulates exactly that -- + // the registry source's contents-API read fails (forcing the fallback) and its fallback fetch rejects the + // way AbortSignal.timeout does -- and asserts the fan-out still completes: the registry source degrades to + // an "error" snapshot instead of the whole Promise.all hanging, and every other source is unaffected. + const env = driftEnv({ GITHUB_PUBLIC_TOKEN: "token" }); + const files = fixtures("58", 0.01); + const failingPath = "gittensor/validator/weights/master_repositories.json"; + vi.stubGlobal("fetch", upstreamPartialTimeoutFetch(files, failingPath)); + + const sources = await refreshUpstreamSourceSnapshots(env); + const byKey = new Map(sources.map((source) => [source.sourceKey, source])); + + expect(byKey.get("registry")?.status).toBe("error"); + expect(byKey.get("registry")?.warnings).toEqual(expect.arrayContaining([expect.stringContaining("Raw fallback failed")])); + for (const key of ["constants", "programming_languages", "mirror_scoring", "issue_discovery_scan", "mirror_models"]) { + expect(byKey.get(key)?.status).toBe("fetched"); + } + }); + it("reuses previous snapshots on not-modified responses", async () => { const env = driftEnv({ GITHUB_PUBLIC_TOKEN: "token" }); vi.stubGlobal("fetch", upstreamFetch(fixtures("58", 0.01), { etag: "\"etag-58\"" })); @@ -1512,6 +1533,27 @@ function upstreamFetch(files: Record, options: { etag?: string } }; } +// One source's contents-API read fails (forcing the raw-GitHub fallback), and that source's fallback fetch +// rejects the way AbortSignal.timeout does on a real stalled connection -- every other source's contents-API +// read succeeds normally. Exercises the #9165 fix: the failing source degrades to an "error" snapshot instead +// of the Promise.all fan-out hanging on it. +function upstreamPartialTimeoutFetch(files: Record, failingPath: string) { + return async (input: RequestInfo | URL): Promise => { + const url = input.toString(); + if (url.includes("/commits/")) return Response.json({ sha: `commit-${scaleFrom(files)}` }); + if (url.includes(`/contents/${failingPath}`)) return new Response("server error", { status: 500 }); + if (url.endsWith(failingPath)) throw new DOMException("The operation was aborted due to timeout", "TimeoutError"); + const path = Object.keys(files).find((candidate) => url.includes(`/contents/${candidate}`)); + if (!path) return new Response("not found", { status: 404 }); + return Response.json({ + content: Buffer.from(files[path]!, "utf8").toString("base64"), + encoding: "base64", + sha: `blob-${path}-${scaleFrom(files)}`, + download_url: `https://raw.githubusercontent.com/entrius/gittensor/test/${path}`, + }); + }; +} + function upstreamNoCommitShaFetch(files: Record) { return async (input: RequestInfo | URL): Promise => { const url = input.toString(); From 30d8f87f4c1f59bf68f7c22a9d0d570d6d285b6b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:08:00 -0700 Subject: [PATCH 2/4] fix(selfhost): sample clock skew in broker mode and alert on staleness (#9156) recordClockSkewFromResponse's only call site is inside the local GitHub App JWT mint path, which a brokered self-host never reaches (mintInstallationToken returns from the broker branch first) -- so a brokered deployment's clock skew gauge read a hard 0, indistinguishable from "clock is fine". Sample it from fetchBrokeredInstallationToken's own response instead, on both success and failure. 0 is a real, reachable skew value (a perfectly synced clock), so it can't double as "never sampled" -- clockSkewSecondsSample() now returns NaN until the first real sample lands. The companion loopover_clock_skew_sample_age_seconds gauge (added in #7000 precisely so a stale reading is distinguishable from a fresh one) was wired to zero alert rules and zero dashboard panels. Add LoopoverClockSkewSampleStale (fires past 2h, tolerating normal ~hourly mint-timing jitter) and the matching Grafana stat panel. --- grafana/dashboards/selfhost.json | 36 +++++++++++++++++++++++++++++ prometheus/rules/alerts.yml | 18 +++++++++++++++ src/orb/broker-client.ts | 8 +++++++ src/selfhost/clock-skew.ts | 12 +++++++--- src/selfhost/metrics.ts | 4 ++-- test/unit/clock-skew.test.ts | 17 ++++++++++---- test/unit/orb-broker-client.test.ts | 31 ++++++++++++++++++++++++- 7 files changed, 116 insertions(+), 10 deletions(-) diff --git a/grafana/dashboards/selfhost.json b/grafana/dashboards/selfhost.json index 2913acf8d1..5320680090 100644 --- a/grafana/dashboards/selfhost.json +++ b/grafana/dashboards/selfhost.json @@ -2773,6 +2773,42 @@ } ] }, + { + "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, + "fieldConfig": { + "defaults": { + "color": { "mode": "thresholds" }, + "thresholds": { + "mode": "absolute", + "steps": [ + { "color": "green", "value": null }, + { "color": "yellow", "value": 7200 } + ] + }, + "unit": "s" + } + }, + "gridPos": { "h": 4, "w": 4, "x": 4, "y": 205 }, + "id": 206, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "auto", + "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, + "textMode": "auto" + }, + "description": "Seconds since the last successful clock-skew sample (#7000, #9156). Ages continuously from process start when never sampled (#9128) -- LoopoverClockSkewSampleStale fires past 7200s (2h). While this is high, the Clock Skew panel to the left cannot be trusted: it may be reading stale or never-measured data.", + "title": "Clock Skew Sample Age (#7000, LoopoverClockSkewSampleStale > 7200s)", + "type": "stat", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, + "expr": "(loopover_clock_skew_sample_age_seconds or gittensory_clock_skew_sample_age_seconds)", + "legendFormat": "sample age" + } + ] + }, { "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, "fieldConfig": { diff --git a/prometheus/rules/alerts.yml b/prometheus/rules/alerts.yml index 87db6c002e..acbb3fc144 100644 --- a/prometheus/rules/alerts.yml +++ b/prometheus/rules/alerts.yml @@ -672,6 +672,24 @@ groups: description: "Clock skew has exceeded 120s (sustained 2m), well past the point GitHub App JWT auth (\"Bad credentials\") is expected to start failing fleet-wide." runbook: "Same as LoopoverClockSkewWarning, but treat as urgent: fix NTP sync immediately (chronyc sources, chronyc makestep, restart chrony if every source stays at Reach: 0). Check for github_app_jwt_rejected logs to confirm auth impact." + - alert: LoopoverClockSkewSampleStale + # #9156: loopover_clock_skew_seconds' own companion staleness gauge (#7000) was referenced by ZERO + # alert rules until this one -- so LoopoverClockSkewWarning/Critical above could read a flattering + # "0" (or, pre-#9156, a hard 0 that never distinguished "fine" from "never measured") forever on a + # deployment whose sampling path never runs (a brokered self-host before #9156's broker-path fix, or + # any future regression that stops installation-token minting). The gauge ages continuously from + # process start when never sampled (#9128), so ONE threshold catches both "never sampled at all" and + # "was sampled once, then mint activity stalled". Installation tokens mint roughly hourly by design; + # a 2x margin tolerates normal mint-timing jitter without masking a real stall. + expr: loopover_clock_skew_sample_age_seconds > 7200 + for: 15m + labels: + severity: warning + annotations: + summary: "loopover has not sampled clock skew in over {{ $value | printf \"%.0f\" }}s" + description: "loopover_clock_skew_seconds has gone unmeasured for over 2h (sustained 15m) -- an installation-token mint (local App-JWT or brokered) hasn't happened recently, or the sampling call itself broke. While this fires, LoopoverClockSkewWarning/Critical cannot be trusted -- real drift could be going undetected." + runbook: "Confirm installation-token minting is still happening (look for github_installation_token_rejected / orb_broker_unavailable logs). If mint activity looks healthy but this stays stale, the sampling call itself (recordClockSkewFromResponse, src/selfhost/clock-skew.ts) may be broken -- check both requestInstallationTokenWithJwt (src/github/app.ts) and fetchBrokeredInstallationToken (src/orb/broker-client.ts)." + # ── Cloudflare D1 (central cloud) size + signal_snapshots dedup regression (#3810) ──── # loopover_d1_* metrics come from the OPT-IN Cloudflare Management API probe (src/selfhost/ # d1-size-probe.ts, CLOUDFLARE_D1_MONITOR_* env vars) -- absent/disabled reads -1 on every gauge below, diff --git a/src/orb/broker-client.ts b/src/orb/broker-client.ts index bec4ac755c..b5ed4e4495 100644 --- a/src/orb/broker-client.ts +++ b/src/orb/broker-client.ts @@ -7,6 +7,7 @@ // The signal is the ENROLLMENT SECRET's presence: a brokered self-host sets ORB_ENROLLMENT_SECRET (issued by the // operator), cloud never does — so this path is inert on cloud and the deploy is byte-identical there. +import { recordClockSkewFromResponse } from "../selfhost/clock-skew"; import { incr } from "../selfhost/metrics"; /** The Orb's hosted broker base; override (ORB_BROKER_URL) only to point at a private loopover deployment. */ @@ -72,6 +73,13 @@ export async function fetchBrokeredInstallationToken( ...(options.forceRefresh ? { body: JSON.stringify({ forceRefresh: true }) } : {}), signal: AbortSignal.timeout(BROKER_TIMEOUT_MS), }); + // #9156: this IS the JWT-mint-equivalent call in broker mode -- a brokered self-host holds no App private + // key, so requestInstallationTokenWithJwt's own clock-skew sample (src/github/app.ts, #3811) is UNREACHABLE + // by construction whenever isOrbBrokerMode(env) is true (mintInstallationToken's broker branch returns + // before ever reaching the JWT path). Sampled unconditionally (both success and failure responses carry a + // real `Date` header) so a brokered deployment's clock drift is measured at all, not silently left at the + // NaN "never sampled" sentinel forever. + recordClockSkewFromResponse(response); if (!response.ok) { throw new Error(`Orb broker token exchange failed (${response.status}).`); } diff --git a/src/selfhost/clock-skew.ts b/src/selfhost/clock-skew.ts index c92afba66c..e20c112336 100644 --- a/src/selfhost/clock-skew.ts +++ b/src/selfhost/clock-skew.ts @@ -7,7 +7,12 @@ // installation-token mint call that's ALREADY made whenever a token needs (re-)minting -- no new // outbound request, sampled at exactly the cadence the vulnerable code path itself runs. -let lastSkewSeconds = 0; +// #9156: NaN (not 0) until the first successful sample -- 0 is a REAL, legitimately-reachable skew value (a +// perfectly-synced clock), so it can never double as "unmeasured" without masking that exact healthy state +// forever. `abs(NaN)` and every other comparison against NaN evaluates false in PromQL, so an unmeasured +// gauge never spuriously satisfies (or fails) either LoopoverClockSkewWarning/Critical alert rule -- it just +// reads as no signal, exactly like the sample-age gauge already distinguishes via its own reference point. +let lastSkewSeconds = Number.NaN; // The wall-clock time (ms) of the last SUCCESSFUL sample, or null before the first one. Backs the staleness // signal below so an old sample can't silently look current if token-mint activity — the only thing that // refreshes lastSkewSeconds — stalls (#7000). @@ -36,7 +41,8 @@ export function recordClockSkewFromResponse(response: Response): void { lastSkewSampleAtMs = localMs; } -/** The most recently observed clock-skew sample in seconds (0 until the first successful sample). */ +/** The most recently observed clock-skew sample in seconds (#9156: `NaN` until the first successful sample -- + * see `lastSkewSeconds`'s own comment for why `0` cannot double as that sentinel). */ export function clockSkewSecondsSample(): number { return lastSkewSeconds; } @@ -56,7 +62,7 @@ export function clockSkewSampleAgeSeconds(): number { /** Test-only: reset the module-level sample between tests, including the #9128 boot-time reference (so a * test can control "time since load" precisely, matching resetPostHogForTest-style module resets elsewhere). */ export function resetClockSkewForTest(): void { - lastSkewSeconds = 0; + lastSkewSeconds = Number.NaN; lastSkewSampleAtMs = null; moduleLoadedAtMs = Date.now(); } diff --git a/src/selfhost/metrics.ts b/src/selfhost/metrics.ts index 2633446558..60feae4909 100644 --- a/src/selfhost/metrics.ts +++ b/src/selfhost/metrics.ts @@ -51,8 +51,8 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [ ["loopover_jobs_claimed_by_lane_total", { help: "Foreground jobs claimed via the backlog-vs-fresh-intake fairness lane.", type: "counter" }], ["loopover_github_rest_rate_limit_remaining", { help: "Newest observed GitHub REST rate-limit remaining count, by key scope.", type: "gauge" }], ["loopover_host_load_avg1_per_core", { help: "One-minute host load average normalized by CPU core count.", type: "gauge" }], - ["loopover_clock_skew_seconds", { help: "Clock skew in seconds between this process and GitHub's server time (positive = ahead), sampled from GitHub App JWT-mint response Date headers.", type: "gauge" }], - ["loopover_clock_skew_sample_age_seconds", { help: "Seconds since the last successful clock-skew sample (loopover_clock_skew_seconds); -1 when no sample has landed yet, so a stale reading is distinguishable from a fresh one.", type: "gauge" }], + ["loopover_clock_skew_seconds", { help: "Clock skew in seconds between this process and GitHub's server time (positive = ahead), sampled from a GitHub response Date header -- the App-JWT mint response locally, or the broker's own token-exchange response in brokered self-host mode (#9156). NaN (not 0, a real reachable skew value) until the first successful sample.", type: "gauge" }], + ["loopover_clock_skew_sample_age_seconds", { help: "Seconds since the last successful clock-skew sample (loopover_clock_skew_seconds); ages from process start when no sample has landed yet (#9128), so a stale/never-sampled reading is always distinguishable from a fresh one -- alert on this exceeding a threshold, not on an exact sentinel value.", type: "gauge" }], ["loopover_uptime_seconds", { help: "Self-host process uptime in seconds.", type: "gauge" }], ["loopover_backup_acknowledged", { help: "1 when SQLite backup is acknowledged or Postgres is in use; 0 when the boot backup advisory would fire.", type: "gauge" }], ["loopover_config_dir_empty_acknowledged", { help: "1 when LOOPOVER_REPO_CONFIG_DIR is unset, has entries, or is acknowledged; 0 when it's configured but the mounted directory is empty.", type: "gauge" }], diff --git a/test/unit/clock-skew.test.ts b/test/unit/clock-skew.test.ts index 1a852aeb49..1e4a59aaa5 100644 --- a/test/unit/clock-skew.test.ts +++ b/test/unit/clock-skew.test.ts @@ -5,8 +5,8 @@ beforeEach(() => resetClockSkewForTest()); afterEach(() => vi.useRealTimers()); describe("clock-skew", () => { - it("defaults to 0 before any sample is recorded", () => { - expect(clockSkewSecondsSample()).toBe(0); + it("defaults to NaN before any sample is recorded (#9156: 0 is a real, reachable skew value)", () => { + expect(clockSkewSecondsSample()).toBeNaN(); }); it("records a positive skew when the local clock is ahead of the response's Date header", () => { @@ -45,11 +45,20 @@ describe("clock-skew", () => { expect(clockSkewSecondsSample()).toBe(300); // unchanged, not reset to 0 }); - it("resetClockSkewForTest restores the sample to 0", () => { + it("resetClockSkewForTest restores the sample to NaN (#9156)", () => { recordClockSkewFromResponse(new Response(null, { headers: { date: new Date(Date.now() - 60_000).toUTCString() } })); - expect(clockSkewSecondsSample()).not.toBe(0); + expect(clockSkewSecondsSample()).not.toBeNaN(); resetClockSkewForTest(); + expect(clockSkewSecondsSample()).toBeNaN(); + }); + + it("a genuine zero-skew sample (perfectly synced clocks) reads as the real number 0, not NaN (#9156)", () => { + vi.useFakeTimers(); + const now = new Date("2026-07-06T12:00:00.000Z"); + vi.setSystemTime(now); + recordClockSkewFromResponse(new Response(null, { headers: { date: now.toUTCString() } })); expect(clockSkewSecondsSample()).toBe(0); + expect(Number.isNaN(clockSkewSecondsSample())).toBe(false); }); }); diff --git a/test/unit/orb-broker-client.test.ts b/test/unit/orb-broker-client.test.ts index 57ed202211..c19164934d 100644 --- a/test/unit/orb-broker-client.test.ts +++ b/test/unit/orb-broker-client.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { clockSkewSecondsSample, resetClockSkewForTest } from "../../src/selfhost/clock-skew"; import { counterValue, resetMetrics } from "../../src/selfhost/metrics"; import { createOrbRelayRegistrationState, @@ -115,6 +116,34 @@ describe("fetchBrokeredInstallationToken", () => { }); }); +describe("fetchBrokeredInstallationToken samples clock skew from the broker response (#9156)", () => { + afterEach(() => { + resetClockSkewForTest(); + vi.useRealTimers(); + }); + + it("records skew from a SUCCESSFUL broker response's Date header -- the broker-mode equivalent of the local JWT-mint sample, unreachable in broker mode", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-06T12:05:00.000Z")); + expect(clockSkewSecondsSample()).toBeNaN(); // never sampled yet + const fetchImpl = (async () => + Response.json( + { token: "ghs_x", installationId: 42, expiresAt: "2026-06-25T09:00:00Z" }, + { headers: { date: "Mon, 06 Jul 2026 12:00:00 GMT" } }, + )) as typeof fetch; + await fetchBrokeredInstallationToken({ ORB_ENROLLMENT_SECRET: "s" }, fetchImpl); + expect(clockSkewSecondsSample()).toBe(300); // 5 minutes ahead of the broker's clock + }); + + it("records skew even from a FAILED (non-OK) broker response -- sampling must not depend on the exchange succeeding", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-06T12:00:00.000Z")); + const fetchImpl = (async () => new Response("nope", { status: 403, headers: { date: "Mon, 06 Jul 2026 12:02:00 GMT" } })) as typeof fetch; + await expect(fetchBrokeredInstallationToken({ ORB_ENROLLMENT_SECRET: "s" }, fetchImpl)).rejects.toThrow(/403/); + expect(clockSkewSecondsSample()).toBe(-120); // 2 minutes behind, even though the exchange itself failed + }); +}); + describe("fetchBrokeredStoredSecret (#8202)", () => { it("exchanges the tenant secret token for a stored secret (default broker URL + Bearer token)", async () => { const { fetchImpl, calls } = captureFetch(Response.json({ secretValue: "postgres://tenant-acme:hunter2@neon/acme", secretType: "tenant_db_credential" })); From 16fa5ae15b37c9139e410cce3ed0b834578277c7 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:08:15 -0700 Subject: [PATCH 3/4] fix(review): repoint the reputation quality signal off orphaned review_targets (#9136) getSubmitterReputation, getSubmitterReputationAcrossInstall, and listSubmitterCohortRows read `review_targets` over a 90-day recency window -- but review_targets has had no live writer since the 2026-06-22 convergence cutover, so that window reads a shrinking set that goes permanently empty around 2026-09-20 (following #9015's partial fix and #9179's repoint of the anomaly-alerter's reversal/DLQ signals). Repointed onto the same live ledgers #9179 and getSubmitterCadence (#9015) already read: review_audit's pr_outcome rows (the realized outcome, not just the bot's own prediction) joined to pull_requests for the submitter's login, restricted to each target's LATEST pr_outcome row so a redelivered webhook can't double-count. ams-miner-cohort.ts required no changes -- it consumes listSubmitterCohortRows directly and picks the repoint up for free; its stale doc comments are updated to match. Before restoring the signal, give evaluateVisualVisionGate's and evaluateScreenshotTableVisionGate's `low_reputation` skip a compensating advisory finding: silently skipping the one extra AI check a low-reputation submitter's confirmed visual change would otherwise get reproduces #9015's "suspicion buys less scrutiny" shape. Remaining scope, tracked under #9136: ops.ts's computeAgentHealth (byStatus/byVerdict/failedRows/manualRate/stuckRetryable/failed) and computeCalibration still read review_targets directly. Deferred because review_targets' non-terminal states (queued/reviewing/error/error_retryable) and the attempt-exhausted 'failed' bucket have no live per-target equivalent post-cutover -- gate_decision's own `decision` column only ever records 'merge' | 'close' | 'hold', not the full status enum review_targets tracked. See the PR description for the full reader inventory. --- src/queue/processors.ts | 24 ++- src/review/ams-miner-cohort.ts | 14 +- src/review/ops.ts | 14 +- src/review/submitter-reputation.ts | 137 ++++++++++++------ src/review/visual/screenshot-table-vision.ts | 24 +++ src/review/visual/visual-findings.ts | 29 ++++ .../screenshot-table-vision-wiring.test.ts | 35 +++++ test/unit/screenshot-table-vision.test.ts | 18 +++ test/unit/submitter-reputation.test.ts | 132 +++++++++++++++++ test/unit/visual-findings.test.ts | 18 +++ test/unit/visual-vision-wiring.test.ts | 36 ++++- 11 files changed, 429 insertions(+), 52 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index a53b1b8c74..33a4cd3720 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -486,9 +486,11 @@ import { import { buildVisualBugAnalysisUserPrompt, buildVisualRegressionFindings, + buildVisualVisionSkippedForReputationFinding, buildVisualVisionUserPrompt, evaluateVisualVisionGate, parseVisualVisionResponse, + selectRoutesForVisualVision, VISUAL_BUG_ANALYSIS_SYSTEM_PROMPT, VISUAL_VISION_SYSTEM_PROMPT, } from "../review/visual/visual-findings"; @@ -641,6 +643,7 @@ import { DEFAULT_SCREENSHOT_CONTRACT_MESSAGE, DEFAULT_SCREENSHOT_TABLE_GATE, eva import { isSafeHttpUrl } from "../review/content-lane/safe-url"; import { buildScreenshotTableVisionFindings, + buildScreenshotTableVisionSkippedForReputationFinding, buildScreenshotTableVisionUserPrompt, evaluateScreenshotTableVisionGate, parseScreenshotTableVisionResponse, @@ -8774,7 +8777,18 @@ export async function runVisualVisionForAdvisory( providerKey: visionProviderKey, selfHostVisionAvailable, }); - if (!visionGate.run) return; + if (!visionGate.run) { + // #9136 compensating hold: a low-reputation skip on a route the pixel-diff threshold already confirmed + // changed must not go completely silent -- see buildVisualVisionSkippedForReputationFinding's own doc + // comment for why (the #9015 "suspicion buys less scrutiny" shape). Only reachable for low_reputation -- + // the other two skip reasons (byok_not_configured, no_confirmed_regression) have nothing to compensate + // for: either there was no confirmed regression at all, or vision simply isn't configured this run. + if (visionGate.reason === "low_reputation") { + const skippedFinding = buildVisualVisionSkippedForReputationFinding(selectRoutesForVisualVision(args.routes).length); + if (skippedFinding) args.advisory.findings.push(skippedFinding); + } + return; + } // evaluateVisualVisionGate only ever returns run:true when providerKey OR selfHostVisionAvailable (the // SAME two values resolved above) was truthy -- this is a defensive type-narrowing guard, not a reachable // false case: if neither is set here, the gate itself would already have returned run:false above. @@ -9111,6 +9125,14 @@ export async function runScreenshotTableVisionForAdvisory( // summary -- the caller's own "absent means omit" contract for the AI review prompt param. evidenceSummary = parseScreenshotTableVisionSummary(visionText); } + } else if (gate.reason === "low_reputation") { + // #9136 compensating hold: mirrors runVisualVisionForAdvisory's own low_reputation compensating + // finding above -- a low-reputation submitter's real (different-bytes) screenshot-table pair is + // EXACTLY the gaming case this check exists to catch, so skipping it silently reproduces #9015's + // "suspicion buys less scrutiny" shape. The other two skip reasons (byok_not_configured, + // no_image_pairs) have nothing to compensate for. + const skippedFinding = buildScreenshotTableVisionSkippedForReputationFinding(fetchedPairs.length); + if (skippedFinding) findings.push(skippedFinding); } } if (findings.length > 0) args.advisory.findings.push(...findings); diff --git a/src/review/ams-miner-cohort.ts b/src/review/ams-miner-cohort.ts index 3bf5210940..b6f9576831 100644 --- a/src/review/ams-miner-cohort.ts +++ b/src/review/ams-miner-cohort.ts @@ -1,9 +1,10 @@ // AMS-vs-human contributor-mix dashboard panel (#6488), per #6210's decided design: for one repo, classify each // recent submitter as "has AMS track-record data" (via the ORB/AMS reputation bridge's pull path, #6485/#6208) // vs. not, then compare acceptance rate / review-cycle count / time-to-merge / PR volume between the two -// cohorts. Reuses `submitter-reputation.ts`'s existing `review_targets`-based outcome classification for the -// metrics and `ams-reputation-bridge.ts`'s existing fail-safe, timeout-bounded pull for AMS membership — no new -// identity system, no new network path. +// cohorts. Reuses `submitter-reputation.ts`'s existing outcome classification (`listSubmitterCohortRows`, #9136: +// repointed off the orphaned `review_targets` table onto the live `review_audit`/`pull_requests` ledgers — this +// module needed no changes of its own to pick that up) for the metrics, and `ams-reputation-bridge.ts`'s +// existing fail-safe, timeout-bounded pull for AMS membership — no new identity system, no new network path. // // BOUNDED, NOT EXHAUSTIVE: classifying membership needs one live call per distinct submitter login (there is no // cached "is this login an AMS miner" flag anywhere yet). An unbounded per-dashboard-load fan-out over every @@ -33,8 +34,11 @@ export type AmsMinerCohortMetrics = { /** merged / (merged + closed) over the cohort's terminal rows, `null` when the cohort has no terminal rows * to divide by (never fabricated as 0, which would misleadingly read as "0% acceptance"). */ acceptanceRate: number | null; - /** Mean of each submitter's own average `attempt_count` (the gate's re-review counter, reused as the - * review-cycle-count proxy per #6488's requirements) — `null` when the cohort is empty. */ + /** Mean of each submitter's own average review-cycle-count proxy per #6488's requirements — `null` when + * the cohort is empty. Sourced from `pull_requests.merge_attempt_count` (#9136: the pre-repoint + * `review_targets.attempt_count`, the gate's own re-review counter, has no live equivalent; see + * `SubmitterCohortRow.avgAttemptCount`'s own doc comment in submitter-reputation.ts for the documented, + * narrower substitution this reuses). */ avgReviewCycleCount: number | null; /** Mean time-to-merge in ms across the cohort's MERGED rows only, `null` when the cohort has no merges. */ avgTimeToMergeMs: number | null; diff --git a/src/review/ops.ts b/src/review/ops.ts index 0b279b804a..90deb0efcb 100644 --- a/src/review/ops.ts +++ b/src/review/ops.ts @@ -547,9 +547,17 @@ export interface ReviewSourceFreshnessCheck { * window. `review_targets` was silently orphaned by the 2026-06-22 convergence cutover (no live writer * anywhere — see src/db/repo-identity-rename.ts / src/review/public-stats.ts's own comments) and nobody * noticed for months; this is the generalizable fix so the NEXT orphaning is loud, not silent. - * - review_targets: submitter-reputation.ts's own REPUTATION_WINDOW_DAYS (90) — this table's newest row - * is frozen at the 2026-06-22 cutover, so it will fall outside this window (and read permanently stale - * here) around 2026-09-20 if nothing else changes; see #9136 for the full scope decision on that module. + * - review_targets: submitter-reputation.ts (#9136) has SINCE been repointed off this table onto the live + * review_audit/pull_requests ledgers — its own REPUTATION_WINDOW_DAYS (90) no longer reads this table at + * all. The remaining live readers are THIS file's own computeAgentHealth (byStatus/byVerdict/failedRows/ + * manualRate/stuckRetryable/failed) and computeCalibration (mergedRows/closesByReasonRows/disputedRows) — + * deferred here because review_targets' non-terminal states (queued/reviewing/error/error_retryable) and + * the attempt-exhausted 'failed' bucket have no live per-target equivalent post-cutover: gate_decision's + * own `decision` column only ever records 'merge' | 'close' | 'hold' (parity.ts's GateAction), not the + * full status enum review_targets tracked. This table's newest row is frozen at the 2026-06-22 cutover, + * so ANY windowDays here eventually reads permanently stale — 90 is kept as a conservative, still-fires- + * eventually value, not because a live consumer still uses that exact number. See #9136 for the tracked + * remainder and the reasoning above. * - review_audit: this module's own ANOMALY_WINDOW (7 days) — IS live today (parity-wire.ts writes * 'gate_decision' rows, outcomes-wire.ts writes the outcome/reversal types), so this should read fresh * in steady state. If both writers ever stop, this is what catches the alerter going silently inert diff --git a/src/review/submitter-reputation.ts b/src/review/submitter-reputation.ts index b81b46b291..a81e997821 100644 --- a/src/review/submitter-reputation.ts +++ b/src/review/submitter-reputation.ts @@ -8,22 +8,32 @@ // REDESIGN (#reputation-redesign): the old signal was a raw close ratio over ALL-TIME submitter_stats counts, // which (a) trapped high-volume contributors who sometimes ship good PRs purely on a ratio, and (b) counted // merge-conflict closes (a rebase artifact, not quality) and OLD closes from a since-relaxed/over-strict bar. -// The new signal is QUALITY-weighted + RECENCY-aware: it reads review_targets (the source of truth) over a -// RECENT WINDOW, classifies each terminal outcome by its reasonCode, IGNORES conflict / out-of-band artifacts, -// and only brands "low" on CLEAR, RECENT, genuine abuse or serial quality-failure. The reputation signal ONLY -// routes to manual review (it NEVER closes), so we default GENEROUS to keep auto-merge flowing — old closes age -// out of the window and trapped contributors auto-correct with no migration. recordSubmissionOutcome still -// maintains submitter_stats for /stats, but the SIGNAL is now derived from review_targets. +// The new signal is QUALITY-weighted + RECENCY-aware: it classifies each terminal outcome by its reasonCode +// over a RECENT WINDOW, IGNORES conflict / out-of-band artifacts, and only brands "low" on CLEAR, RECENT, +// genuine abuse or serial quality-failure. The reputation signal ONLY routes to manual review (it NEVER +// closes), so we default GENEROUS to keep auto-merge flowing — old closes age out of the window and trapped +// contributors auto-correct with no migration. recordSubmissionOutcome still maintains submitter_stats for +// /stats, but the SIGNAL below is derived from the live ledgers (see #9136). +// +// #9136: the quality signal used to read `review_targets`, which stopped receiving writes at the 2026-06-22 +// self-host cutover (no live writer anywhere in this codebase — see src/db/repo-identity-rename.ts's own +// comment) — its newest row is frozen at the cutover date, so a 90-day recency window reads a shrinking set +// that goes permanently empty around 2026-09-20. Repointed onto the SAME live ledgers getSubmitterCadence +// (#9015) already reads, mirroring that repoint: `review_audit`'s `pr_outcome` rows (outcomes-wire.ts, +// written on every realized merge/close — the ground truth, not just the bot's own prediction) joined to +// `pull_requests` for the submitter's login (review_audit carries no author identity of its own), with the +// reasonCode pulled from that same target's latest `gate_decision` row (mirrors resolveDispositionReason's +// own lookup). A target can carry more than one `pr_outcome` row in practice (recordPrOutcome's webhook path +// has no existence check unlike recordTerminalActionOutcome's direct-write path, so a redelivered `closed` +// webhook can double-insert) — every query below reads only the LATEST `pr_outcome` per target_id so a +// duplicate never double-counts one PR's outcome. // // SELF-CONTAINED NATIVE PORT (reviewbot→loopover convergence): every type + helper this module needs is // defined HERE. No imports from reviewbot — the reviewbot `storage(env)` adapter is inlined as `env.DB`, and // the `Env` / `ReputationConfig` types are declared locally. The CLASSIFY/SIGNAL/COUNT logic is byte-faithful // to the reviewbot source (src/core/submitter-reputation.ts); the only deltas are mechanical guards for // loopover's stricter tsconfig (noUncheckedIndexedAccess / exactOptionalPropertyTypes), which don't change -// behavior. ADDITIVE + DORMANT: the DB-touching reads/writes assume the reviewbot D1 tables (review_targets, -// submitter_stats) — loopover does not yet have them, so getSubmitterReputation / recordSubmissionOutcome -// degrade fail-safe (neutral / no-op) until a later migration lands them. The PURE classifiers -// (classifyOutcome / countOutcomes / signalFromCounts) are usable immediately. +// behavior. // ── Inlined minimal deps (no reviewbot imports) ───────────────────────────────────────────────────────── @@ -292,6 +302,32 @@ export async function recordSubmissionOutcome(env: Env, project: string, submitt } } +// ── #9136 live-ledger source fragments (shared by every quality-signal query below) ────────────────────── +// +// review_audit carries no author identity of its own (see this file's header comment) -- `pr.number` is +// recovered from `po.target_id` (`project#number`, reviewAuditTargetId in outcomes-wire.ts) via string +// concatenation rather than a stored column, since `project` IS `pr.repo_full_name` for every native writer +// (parity-wire.ts / outcomes-wire.ts both pass `repoFullName` as `project`). +const PR_OUTCOME_JOIN = `JOIN pull_requests pr ON pr.repo_full_name = po.project AND (po.project || '#' || pr.number) = po.target_id`; + +// A target can carry more than one `pr_outcome` row in practice (recordPrOutcome's webhook path has no +// existence check unlike recordTerminalActionOutcome's direct-write path, so a redelivered `closed` webhook +// can double-insert) -- restrict to each target's LATEST `pr_outcome` row so a duplicate never double-counts. +const LATEST_PR_OUTCOME_FILTER = `po.event_type = 'pr_outcome' + AND po.created_at = ( + SELECT MAX(po2.created_at) FROM review_audit po2 + WHERE po2.target_id = po.target_id AND po2.event_type = 'pr_outcome' + )`; + +// The reasonCode a pr_outcome row's OWN bot decision recorded, if any -- a pr_outcome row never carries a +// summary itself (appendReviewAudit's default), only a gate_decision row does. Mirrors +// resolveDispositionReason's own "latest gate_decision summary for this target" lookup (outcomes-wire.ts). +const REASON_CODE_SUBQUERY = `( + SELECT gd.summary FROM review_audit gd + WHERE gd.target_id = po.target_id AND gd.event_type = 'gate_decision' AND gd.summary IS NOT NULL + ORDER BY gd.created_at DESC LIMIT 1 + )`; + /** Read a submitter's internal reputation. The SIGNAL is derived from review_targets over the recency window * (quality-weighted, conflict-excluded); the all-time counts come from submitter_stats for the /stats view. * Fail-safe → "neutral" on ANY error (it must never throw into the gate). */ @@ -328,12 +364,16 @@ export async function getSubmitterReputation(env: Env, project: string, submitte let signal: ReputationSignal = "neutral"; try { + // #9136: repointed off review_targets (frozen since the 2026-06-22 cutover) onto the live review_audit + // pr_outcome ledger -- see this file's header comment + the shared fragments above for the full shape. const result = await storage(env) .prepare( - `SELECT status, json_extract(decision_json, '$.reasonCode') AS reasonCode - FROM review_targets - WHERE project = ? AND submitter = ? AND terminal_at IS NOT NULL AND terminal_at >= datetime('now', ?) - ORDER BY terminal_at DESC LIMIT ?`, + `SELECT po.decision AS status, ${REASON_CODE_SUBQUERY} AS reasonCode + FROM review_audit po + ${PR_OUTCOME_JOIN} + WHERE po.project = ? AND LOWER(pr.author_login) = LOWER(?) AND ${LATEST_PR_OUTCOME_FILTER} + AND po.created_at >= datetime('now', ?) + ORDER BY po.created_at DESC LIMIT ?`, ) .bind(project, submitter, `-${cfg.windowDays} days`, REPUTATION_WINDOW_ROW_CAP) .all<{ status: string; reasonCode: string | null }>(); @@ -347,10 +387,15 @@ export async function getSubmitterReputation(env: Env, project: string, submitte return { ...agg, closeRate: decided > 0 ? agg.closed / decided : 0, signal }; } -/** One submitter's raw, recency-windowed terminal-outcome tally for a repo (#6488) — the same `review_targets` - * source {@link getSubmitterReputation} reads, but grouped by submitter instead of scoped to one. `avgAttemptCount` - * reuses `attempt_count` (the gate's own re-review counter) as the review-cycle-count proxy; `avgMergeMs` is - * `AVG(terminal_at - created_at)` over MERGED rows only (`null` when this submitter has no merges in the window). */ +/** One submitter's raw, recency-windowed terminal-outcome tally for a repo (#6488) — the same live-ledger + * source {@link getSubmitterReputation} reads, but grouped by submitter instead of scoped to one. + * `avgAttemptCount` (#9136): the pre-repoint source (`review_targets.attempt_count`, the gate's own + * re-review counter) has no live equivalent — `pull_requests.merge_attempt_count` is the closest LIVE + * per-PR retry counter, but it is narrower (only FAILED merge attempts — permission/check/conflict — reset + * per head SHA; see its own schema comment), not every re-review cycle. Documented substitution, not a + * fabricated value: still a genuine review-friction signal, just not byte-identical in meaning to the + * pre-cutover figure. `avgMergeMs` is `AVG(merged_at - created_at)` over MERGED rows only, sourced from + * `pull_requests`' own timestamps (`null` when this submitter has no merges in the window). */ export interface SubmitterCohortRow { submitter: string; submissions: number; @@ -361,21 +406,23 @@ export interface SubmitterCohortRow { } /** Per-submitter cohort tally for a repo over the recency window (#6488, AMS-vs-human dashboard comparison). - * Reuses the SAME `review_targets` terminal-row convention {@link getSubmitterReputation} does (terminal_at - * within `windowDays`), just grouped by submitter instead of read for one. Fail-safe: any read error degrades - * to an empty array (never throws — the caller treats that identically to "no activity in the window"). */ + * Reuses the SAME live review_audit pr_outcome ledger {@link getSubmitterReputation} does (#9136), just + * grouped by submitter instead of read for one. Fail-safe: any read error degrades to an empty array (never + * throws — the caller treats that identically to "no activity in the window"). */ export async function listSubmitterCohortRows(env: Env, project: string, windowDays: number = REPUTATION_WINDOW_DAYS): Promise { try { const result = await storage(env) .prepare( - `SELECT submitter, + `SELECT pr.author_login AS submitter, COUNT(*) AS submissions, - SUM(CASE WHEN status = 'merged' THEN 1 ELSE 0 END) AS merged, - SUM(CASE WHEN status = 'closed' THEN 1 ELSE 0 END) AS closed, - AVG(attempt_count) AS avgAttemptCount, - AVG(CASE WHEN status = 'merged' THEN (julianday(terminal_at) - julianday(created_at)) * 86400000 ELSE NULL END) AS avgMergeMs - FROM review_targets - WHERE project = ? AND submitter IS NOT NULL AND submitter != '' AND terminal_at IS NOT NULL AND terminal_at >= datetime('now', ?) + SUM(CASE WHEN po.decision = 'merged' THEN 1 ELSE 0 END) AS merged, + SUM(CASE WHEN po.decision = 'closed' THEN 1 ELSE 0 END) AS closed, + AVG(pr.merge_attempt_count) AS avgAttemptCount, + AVG(CASE WHEN po.decision = 'merged' AND pr.merged_at IS NOT NULL THEN (julianday(pr.merged_at) - julianday(pr.created_at)) * 86400000 ELSE NULL END) AS avgMergeMs + FROM review_audit po + ${PR_OUTCOME_JOIN} + WHERE po.project = ? AND pr.author_login IS NOT NULL AND pr.author_login != '' AND ${LATEST_PR_OUTCOME_FILTER} + AND po.created_at >= datetime('now', ?) GROUP BY submitter`, ) .bind(project, `-${windowDays} days`) @@ -394,16 +441,19 @@ export async function listSubmitterCohortRows(env: Env, project: string, windowD } /** Install-wide sibling of {@link getSubmitterReputation} (#4513): the SAME quality-weighted, recency-windowed - * signal derivation, but aggregated across EVERY repo `review_targets` has recorded for this installation_id - * (migrations/0050), not just one project. Closes a real blind spot: a fleet identity spreading thin across - * many repos in one self-hosted install never accumulates same-repo sample density for the per-project - * signal to ever leave "neutral," even while it burns full paid AI-review spend on every submission. Callers - * should reserve this for a CONFIRMED official Gittensor miner identity (this function does not itself check - * that) -- an ordinary contributor's reputation stays intentionally scoped per-repo. The all-time - * submitter_stats aggregate (submissions/merged/closed/manual, /stats-view only, not the signal) is NOT - * widened here: that table is keyed (project, submitter) with no installation_id column, and only the - * SIGNAL — not the display counts — gates the AI-spend decision. Fail-safe: any read error degrades to - * "neutral", identical to the per-project function. */ + * signal derivation, but aggregated across EVERY repo the live ledgers have recorded for this + * installation_id (the `repositories` table's own `installation_id` column), not just one project. Closes a + * real blind spot: a fleet identity spreading thin across many repos in one self-hosted install never + * accumulates same-repo sample density for the per-project signal to ever leave "neutral," even while it + * burns full paid AI-review spend on every submission. Callers should reserve this for a CONFIRMED official + * Gittensor miner identity (this function does not itself check that) -- an ordinary contributor's + * reputation stays intentionally scoped per-repo. The all-time submitter_stats aggregate + * (submissions/merged/closed/manual, /stats-view only, not the signal) is NOT widened here: that table is + * keyed (project, submitter) with no installation_id column, and only the SIGNAL — not the display counts — + * gates the AI-spend decision. Fail-safe: any read error degrades to "neutral", identical to the per-project + * function. #9136: repointed off review_targets onto the same live review_audit pr_outcome ledger as + * {@link getSubmitterReputation}, joined through `repositories` to resolve `project` (repo_full_name) → + * installation_id (review_audit itself carries no installation scope). */ export async function getSubmitterReputationAcrossInstall( env: Env, installationId: number, @@ -416,10 +466,13 @@ export async function getSubmitterReputationAcrossInstall( try { const result = await storage(env) .prepare( - `SELECT status, json_extract(decision_json, '$.reasonCode') AS reasonCode - FROM review_targets - WHERE installation_id = ? AND submitter = ? AND terminal_at IS NOT NULL AND terminal_at >= datetime('now', ?) - ORDER BY terminal_at DESC LIMIT ?`, + `SELECT po.decision AS status, ${REASON_CODE_SUBQUERY} AS reasonCode + FROM review_audit po + ${PR_OUTCOME_JOIN} + JOIN repositories r ON r.full_name = po.project + WHERE r.installation_id = ? AND LOWER(pr.author_login) = LOWER(?) AND ${LATEST_PR_OUTCOME_FILTER} + AND po.created_at >= datetime('now', ?) + ORDER BY po.created_at DESC LIMIT ?`, ) .bind(installationId, submitter, `-${cfg.windowDays} days`, REPUTATION_WINDOW_ROW_CAP) .all<{ status: string; reasonCode: string | null }>(); diff --git a/src/review/visual/screenshot-table-vision.ts b/src/review/visual/screenshot-table-vision.ts index 64843d606d..793e052fa7 100644 --- a/src/review/visual/screenshot-table-vision.ts +++ b/src/review/visual/screenshot-table-vision.ts @@ -72,6 +72,30 @@ export function evaluateScreenshotTableVisionGate(input: { return { run: true, pairCount }; } +/** The advisory finding code recorded when the gaming-detection vision check is SKIPPED for a low-reputation + * submitter on a real (different-bytes) image pair (#9136, mirrors visual-findings.ts's + * VISUAL_VISION_SKIPPED_LOW_REPUTATION_FINDING_CODE — a compensating signal for the `low_reputation` skip + * above). This is exactly the submitter this check exists to catch — a low-reputation submitter gaming the + * deterministic screenshot-table gate with a duplicated/unrelated image — so skipping it with no trace at + * all reproduces #9015's "suspicion buys less scrutiny" shape. `severity: "warning"` (never a blocker — this + * module stays STRICTLY ADVISORY, see the file header) puts it in `gate.warnings`. */ +export const SCREENSHOT_TABLE_VISION_SKIPPED_LOW_REPUTATION_FINDING_CODE = "screenshot_table_vision_skipped_low_reputation"; + +/** Build the compensating advisory finding for a low-reputation gaming-check skip (#9136). `imagePairCount` is + * how many real (different-bytes) pairs survived the caller's byte pre-check — the pairs vision would have + * looked at had reputation not skipped it. Returns `null` when there was nothing to compensate for (no real + * pairs at all). Pure + total, mirrors every other finding builder in this file. */ +export function buildScreenshotTableVisionSkippedForReputationFinding(imagePairCount: number): AdvisoryFinding | null { + if (imagePairCount <= 0) return null; + return { + code: SCREENSHOT_TABLE_VISION_SKIPPED_LOW_REPUTATION_FINDING_CODE, + severity: "warning", + title: "Screenshot-table gaming check skipped (low submitter reputation)", + detail: `${imagePairCount} real before/after image pair(s) were ready to check, but the AI gaming-detection vision check was skipped because this submitter's reputation signal is currently "low".`, + action: "Advisory only — manually verify the screenshot-table images against the stated change before deciding.", + }; +} + /** One vision observation the model reported for a specific table row (1-indexed among the pairs sent, not * the row's position in the PR body — the live caller has no cheap way to recover the original row number * once rows have been filtered down to real pairs, and the number only needs to disambiguate WHICH pair a diff --git a/src/review/visual/visual-findings.ts b/src/review/visual/visual-findings.ts index c905a3303b..403c6e6ac1 100644 --- a/src/review/visual/visual-findings.ts +++ b/src/review/visual/visual-findings.ts @@ -92,6 +92,35 @@ export function evaluateVisualVisionGate(input: { return { run: true, routes }; } +/** The advisory finding code recorded when vision scrutiny is SKIPPED for a low-reputation submitter on a + * route the pixel-diff threshold already confirmed changed (#9136, a compensating signal for the + * `low_reputation` skip above). Restoring the reputation signal (#9136) without this reproduces #9015's + * "suspicion buys less scrutiny" shape: the ONE extra check a low-reputation submitter's confirmed visual + * change would otherwise get was silently skipped with no trace anywhere. `severity: "warning"` (never a + * blocker — this module stays STRICTLY ADVISORY, see the file header) puts it in `gate.warnings` + * (`evaluateGateCheckCore`, src/rules/advisory.ts) so a maintainer reviewing the unified comment actually + * sees it, instead of the skip being completely invisible. */ +export const VISUAL_VISION_SKIPPED_LOW_REPUTATION_FINDING_CODE = "visual_vision_skipped_low_reputation"; + +/** Build the compensating advisory finding for a low-reputation vision skip (#9136). `confirmedRegressionRouteCount` + * is how many routes the pixel-diff threshold already flagged changed — the routes vision would have looked at + * had reputation not skipped it (the caller resolves this via {@link selectRoutesForVisualVision} BEFORE + * calling {@link evaluateVisualVisionGate}, since the gate itself short-circuits on reputation before ever + * computing that count). Returns `null` when there was nothing to compensate for — no confirmed regression at + * all means vision would have been skipped for `no_confirmed_regression` regardless of reputation, so a + * "skipped due to low reputation" finding would be misleading. Pure + total, mirrors every other finding + * builder in this file. */ +export function buildVisualVisionSkippedForReputationFinding(confirmedRegressionRouteCount: number): AdvisoryFinding | null { + if (confirmedRegressionRouteCount <= 0) return null; + return { + code: VISUAL_VISION_SKIPPED_LOW_REPUTATION_FINDING_CODE, + severity: "warning", + title: "Visual-regression vision check skipped (low submitter reputation)", + detail: `${confirmedRegressionRouteCount} route(s) crossed the visual-diff threshold, but the AI vision check was skipped because this submitter's reputation signal is currently "low".`, + action: "Advisory only — manually review the before/after captures for this PR before deciding.", + }; +} + /** One vision observation the model reported for a specific route — `path`/`body` already public-safe (see * {@link parseVisualVisionResponse}). `category` is only ever populated when the caller used * {@link VISUAL_BUG_ANALYSIS_SYSTEM_PROMPT} (`review.visual.bugAnalysis`) — the default prompt diff --git a/test/unit/screenshot-table-vision-wiring.test.ts b/test/unit/screenshot-table-vision-wiring.test.ts index 338d616b36..30f1c687d4 100644 --- a/test/unit/screenshot-table-vision-wiring.test.ts +++ b/test/unit/screenshot-table-vision-wiring.test.ts @@ -405,6 +405,41 @@ describe("runScreenshotTableVisionForAdvisory (#4366)", () => { expect(fetchCalls).not.toContain("https://api.anthropic.com/v1/messages"); }); + it("declines the AI call for a low-reputation submitter on a REAL (different-bytes) pair, leaving a compensating advisory finding (#9136)", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-key", model: null }); + vi.spyOn(submitterReputation, "getSubmitterReputation").mockResolvedValueOnce({ + submissions: 6, + merged: 0, + closed: 6, + manual: 0, + closeRate: 1, + signal: "low", + }); + // Genuinely different bytes -- survives the free byte pre-check and reaches the vision gate, unlike the + // byte-identical fixture above (which never reaches the reputation gate at all). + stubShotsAndProvider(null, { before: [1, 2, 3], after: [4, 5, 6] }); + const adv = findingsHolder(); + await runScreenshotTableVisionForAdvisory(env, { + mode: "live", + repoFullName, + pr, + prBody: tableBody(BEFORE_URL, AFTER_URL), + prTitle: "Redesign the nav bar", + author: "bob", + confirmedContributor: true, + settings: gateEnabledSettings(), + advisory: adv, + }); + // #9136: the vision call itself is still skipped (never spends), but a real pair going unverified for a + // low-reputation submitter must not be completely silent -- exactly the gaming case this check exists for. + expect(adv.findings).toEqual([ + expect.objectContaining({ code: "screenshot_table_vision_skipped_low_reputation", severity: "warning" }), + ]); + const fetchCalls = (globalThis.fetch as unknown as ReturnType).mock.calls.map((c: unknown[]) => String(c[0])); + expect(fetchCalls).not.toContain("https://api.anthropic.com/v1/messages"); + }); + it("runs via env.AI_VISION when no BYOK key is configured at all", async () => { const runMock = vi.fn(async () => ({ response: findingsResponse([{ pairIndex: 1, body: "Looks like a different app entirely." }]) })); const env = byokEnv(); diff --git a/test/unit/screenshot-table-vision.test.ts b/test/unit/screenshot-table-vision.test.ts index 6f0d037e3a..8c302bd0ff 100644 --- a/test/unit/screenshot-table-vision.test.ts +++ b/test/unit/screenshot-table-vision.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { buildScreenshotTableVisionFindings, + buildScreenshotTableVisionSkippedForReputationFinding, buildScreenshotTableVisionUserPrompt, evaluateScreenshotTableVisionGate, parseScreenshotTableVisionResponse, @@ -53,6 +54,23 @@ describe("evaluateScreenshotTableVisionGate", () => { }); }); +describe("buildScreenshotTableVisionSkippedForReputationFinding (#9136 compensating hold)", () => { + it("returns null when there was nothing to compensate for (no real image pairs)", () => { + expect(buildScreenshotTableVisionSkippedForReputationFinding(0)).toBeNull(); + }); + + it("builds a warning-severity advisory finding naming the pair count", () => { + const finding = buildScreenshotTableVisionSkippedForReputationFinding(1); + expect(finding).toEqual( + expect.objectContaining({ + code: "screenshot_table_vision_skipped_low_reputation", + severity: "warning", + detail: expect.stringContaining("1 real before/after image pair(s)"), + }), + ); + }); +}); + describe("buildScreenshotTableVisionUserPrompt", () => { it("includes the PR title and pair count when a title is given", () => { const prompt = buildScreenshotTableVisionUserPrompt("Redesign the nav bar", 2); diff --git a/test/unit/submitter-reputation.test.ts b/test/unit/submitter-reputation.test.ts index 35198bf27c..aa5c1faac5 100644 --- a/test/unit/submitter-reputation.test.ts +++ b/test/unit/submitter-reputation.test.ts @@ -162,6 +162,138 @@ describe("signalFromCounts — config-overridable thresholds (#private-config pa }); }); +describe("getSubmitterReputation / getSubmitterReputationAcrossInstall / listSubmitterCohortRows — live-ledger repoint (#9136)", () => { + it("query the live review_audit/pull_requests ledgers, not the frozen review_targets table", async () => { + const seenSql: string[] = []; + const spyEnv = (): Env => + ({ + DB: { + prepare: (sql: string) => { + seenSql.push(sql); + return { + bind: () => ({ + first: async () => null, + all: async () => ({ results: [] }), + }), + }; + }, + }, + }) as unknown as Env; + + await getSubmitterReputation(spyEnv(), "acme/widgets", "farmer99"); + await getSubmitterReputationAcrossInstall(spyEnv(), 123, "farmer99"); + await listSubmitterCohortRows(spyEnv(), "acme/widgets"); + + for (const sql of seenSql) expect(sql).not.toContain("review_targets"); + expect(seenSql.some((sql) => sql.includes("FROM review_audit") && sql.includes("pull_requests"))).toBe(true); + }); + + async function seedPullRequest(env: Env, args: { repo: string; number: number; author: string; state?: string; mergedAt?: string | null; createdAt?: string; mergeAttemptCount?: number }): Promise { + await env.DB.prepare( + `INSERT INTO pull_requests (repo_full_name, number, title, state, author_login, merged_at, created_at, merge_attempt_count) + VALUES (?, ?, 't', ?, ?, ?, ?, ?)`, + ) + .bind(args.repo, args.number, args.state ?? (args.mergedAt ? "closed" : "open"), args.author, args.mergedAt ?? null, args.createdAt ?? "2026-07-01T00:00:00.000Z", args.mergeAttemptCount ?? 0) + .run(); + } + + async function seedGateDecision(env: Env, args: { repo: string; number: number; decision: string; summary?: string | null; createdAt?: string }): Promise { + await env.DB.prepare( + `INSERT INTO review_audit (id, project, target_id, event_type, decision, source, summary, created_at) + VALUES (?, ?, ?, 'gate_decision', ?, 'gittensory-native', ?, ?)`, + ) + .bind(`gd:${args.repo}#${args.number}:${Math.random()}`, args.repo, `${args.repo}#${args.number}`, args.decision, args.summary ?? null, args.createdAt ?? "2026-07-10T00:00:00.000Z") + .run(); + } + + async function seedPrOutcome(env: Env, args: { repo: string; number: number; decision: "merged" | "closed"; createdAt?: string }): Promise { + await env.DB.prepare( + `INSERT INTO review_audit (id, project, target_id, event_type, decision, source, created_at) + VALUES (?, ?, ?, 'pr_outcome', ?, 'gittensory-native', ?)`, + ) + .bind(`po:${args.repo}#${args.number}:${Math.random()}`, args.repo, `${args.repo}#${args.number}`, args.decision, args.createdAt ?? "2026-07-11T00:00:00.000Z") + .run(); + } + + it("derives the quality signal from real review_audit pr_outcome rows joined to pull_requests", async () => { + const env = createTestEnv(); + // 1 genuine decline + 7 merges for "farmer99" -- a healthy contributor, must NOT be 'low' (success guard). + await seedPullRequest(env, { repo: "acme/widgets", number: 1, author: "Farmer99", mergedAt: "2026-07-05T00:00:00.000Z" }); + await seedGateDecision(env, { repo: "acme/widgets", number: 1, decision: "merge" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: 1, decision: "merged" }); + for (let n = 2; n <= 8; n++) { + await seedPullRequest(env, { repo: "acme/widgets", number: n, author: "farmer99", mergedAt: `2026-07-0${Math.min(n, 9)}T00:00:00.000Z` }); + await seedGateDecision(env, { repo: "acme/widgets", number: n, decision: "merge" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: n, decision: "merged" }); + } + await seedPullRequest(env, { repo: "acme/widgets", number: 9, author: "farmer99" }); + await seedGateDecision(env, { repo: "acme/widgets", number: 9, decision: "close", summary: "dual_review_declined" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: 9, decision: "closed" }); + + const rep = await getSubmitterReputation(env, "acme/widgets", "farmer99"); + expect(rep.signal).toBe("trusted"); // login match is case-insensitive (Farmer99 vs farmer99) + }); + + it("reads a submitter's login case-insensitively, and scopes strictly to the requested project", async () => { + const env = createTestEnv(); + await seedPullRequest(env, { repo: "acme/widgets", number: 1, author: "farmer99" }); + await seedGateDecision(env, { repo: "acme/widgets", number: 1, decision: "close", summary: "source_prompt_injection" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: 1, decision: "closed" }); + // A same-named submitter in a DIFFERENT repo must not leak into this project's signal. + await seedPullRequest(env, { repo: "other/repo", number: 1, author: "farmer99" }); + await seedGateDecision(env, { repo: "other/repo", number: 1, decision: "merge" }); + await seedPrOutcome(env, { repo: "other/repo", number: 1, decision: "merged" }); + + const rep = await getSubmitterReputation(env, "acme/widgets", "farmer99"); + expect(rep.signal).toBe("low"); // prompt-injection is a hard override, unaffected by minSample + }); + + it("only counts the LATEST pr_outcome row per target when a duplicate write exists", async () => { + const env = createTestEnv(); + await seedPullRequest(env, { repo: "acme/widgets", number: 1, author: "farmer99" }); + await seedGateDecision(env, { repo: "acme/widgets", number: 1, decision: "close", summary: "dual_review_declined" }); + // Simulate the recordPrOutcome-vs-recordTerminalActionOutcome double-write race (#9136): an EARLIER + // "closed" row and a LATER "merged" row for the SAME target. Only the latest should be counted. + await seedPrOutcome(env, { repo: "acme/widgets", number: 1, decision: "closed", createdAt: "2026-07-01T00:00:00.000Z" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: 1, decision: "merged", createdAt: "2026-07-02T00:00:00.000Z" }); + + const rep = await getSubmitterReputation(env, "acme/widgets", "farmer99"); + // If both rows were counted, this would be sample=2 (merged=1, closed=1). Counting only the LATEST + // ("merged") leaves a sample of 1 success and 0 fails -- below minSample, so 'neutral', not 'low'/'trusted'. + expect(rep.signal).toBe("neutral"); + }); + + it("getSubmitterReputationAcrossInstall aggregates across every repo the installation owns, via `repositories`", async () => { + const env = createTestEnv(); + await env.DB.prepare(`INSERT INTO repositories (full_name, owner, name, installation_id) VALUES (?, ?, ?, ?)`).bind("acme/widgets", "acme", "widgets", 555).run(); + await env.DB.prepare(`INSERT INTO repositories (full_name, owner, name, installation_id) VALUES (?, ?, ?, ?)`).bind("acme/other", "acme", "other", 555).run(); + for (const [repo, n] of [["acme/widgets", 1], ["acme/other", 1]] as const) { + await seedPullRequest(env, { repo, number: n, author: "farmer99" }); + await seedGateDecision(env, { repo, number: n, decision: "close", summary: "source_prompt_injection" }); + await seedPrOutcome(env, { repo, number: n, decision: "closed" }); + } + const rep = await getSubmitterReputationAcrossInstall(env, 555, "farmer99"); + expect(rep.signal).toBe("low"); + // A different installation_id must not see this submitter's activity. + expect((await getSubmitterReputationAcrossInstall(env, 999, "farmer99")).signal).toBe("neutral"); + }); + + it("listSubmitterCohortRows groups real ledger rows by submitter with pull_requests-sourced merge timing", async () => { + const env = createTestEnv(); + await seedPullRequest(env, { repo: "acme/widgets", number: 1, author: "alice", mergedAt: "2026-07-02T00:00:00.000Z", createdAt: "2026-07-01T00:00:00.000Z", mergeAttemptCount: 2 }); + await seedGateDecision(env, { repo: "acme/widgets", number: 1, decision: "merge" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: 1, decision: "merged" }); + await seedPullRequest(env, { repo: "acme/widgets", number: 2, author: "alice" }); + await seedGateDecision(env, { repo: "acme/widgets", number: 2, decision: "close", summary: "dual_review_declined" }); + await seedPrOutcome(env, { repo: "acme/widgets", number: 2, decision: "closed" }); + + const rows = await listSubmitterCohortRows(env, "acme/widgets"); + expect(rows).toEqual([ + expect.objectContaining({ submitter: "alice", submissions: 2, merged: 1, closed: 1, avgAttemptCount: 1, avgMergeMs: 86_400_000 }), + ]); + }); +}); + describe("getSubmitterReputation — recency window (#reputation-redesign)", () => { it("OLD closes outside the window are not in the query result → they don't count (auto-correct)", async () => { // Simulate the DB returning ONLY in-window rows (the SQL `terminal_at >= datetime('now', -90 days)` filters diff --git a/test/unit/visual-findings.test.ts b/test/unit/visual-findings.test.ts index edc9acfc01..f950c5fd12 100644 --- a/test/unit/visual-findings.test.ts +++ b/test/unit/visual-findings.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { buildVisualBugAnalysisUserPrompt, buildVisualRegressionFindings, + buildVisualVisionSkippedForReputationFinding, buildVisualVisionUserPrompt, evaluateVisualVisionGate, parseVisualVisionResponse, @@ -108,6 +109,23 @@ describe("evaluateVisualVisionGate", () => { }); }); +describe("buildVisualVisionSkippedForReputationFinding (#9136 compensating hold)", () => { + it("returns null when there was nothing to compensate for (no confirmed regression)", () => { + expect(buildVisualVisionSkippedForReputationFinding(0)).toBeNull(); + }); + + it("builds a warning-severity advisory finding naming the confirmed-regression route count", () => { + const finding = buildVisualVisionSkippedForReputationFinding(2); + expect(finding).toEqual( + expect.objectContaining({ + code: "visual_vision_skipped_low_reputation", + severity: "warning", + detail: expect.stringContaining("2 route(s)"), + }), + ); + }); +}); + describe("buildVisualVisionUserPrompt", () => { it("renders one bullet per route path", () => { const prompt = buildVisualVisionUserPrompt([{ path: "/pricing" }, { path: "/about" }]); diff --git a/test/unit/visual-vision-wiring.test.ts b/test/unit/visual-vision-wiring.test.ts index 5d0ddf4876..5811698b36 100644 --- a/test/unit/visual-vision-wiring.test.ts +++ b/test/unit/visual-vision-wiring.test.ts @@ -147,7 +147,7 @@ describe("runVisualVisionForAdvisory", () => { expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); - it("declines for a low-reputation submitter even with a confirmed regression and BYOK configured", async () => { + it("declines for a low-reputation submitter even with a confirmed regression and BYOK configured, but leaves a compensating advisory finding (#9136)", async () => { const env = byokEnv(); await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); // Reputation-signal derivation is submitter-reputation.ts's own concern (see submitter-reputation.test.ts); @@ -173,6 +173,40 @@ describe("runVisualVisionForAdvisory", () => { advisory: adv, routes: [route({ path: "/app", diffUrl: "https://x/loopover/shot?key=diff", beforeUrl: "https://x/loopover/shot?key=b", afterUrl: "https://x/loopover/shot?key=a" })], }); + // #9136: the vision call itself is still skipped (never spends), but the confirmed-regression skip must + // not be completely silent -- a compensating "warning"-severity advisory finding takes its place instead + // of the pre-#9136 empty findings array (the #9015 "suspicion buys less scrutiny" shape). + expect(fetchMock).not.toHaveBeenCalled(); + expect(adv.findings).toEqual([ + expect.objectContaining({ code: "visual_vision_skipped_low_reputation", severity: "warning" }), + ]); + }); + + it("does NOT leave a compensating finding for a low-reputation submitter with NO confirmed regression (#9136)", async () => { + const env = byokEnv(); + vi.spyOn(submitterReputation, "getSubmitterReputation").mockResolvedValueOnce({ + submissions: 6, + merged: 0, + closed: 6, + manual: 0, + closeRate: 1, + signal: "low", + }); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + mode: "live", + repoFullName, + pr, + author: "bob", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + // No diffUrl/diffUrlMobile -- routeHasConfirmedVisualRegression is false, so there is nothing a vision + // call would have looked at even absent the reputation skip. + routes: [route({ path: "/app" })], + }); expect(adv.findings).toEqual([]); expect(fetchMock).not.toHaveBeenCalled(); }); From f3d151a1962c8814718a63f0f80f57e8ae37b8bf Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 02:00:18 -0700 Subject: [PATCH 4/4] chore(cf): regenerate worker-configuration.d.ts after rebasing onto main --- worker-configuration.d.ts | 93 +++++++++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 19 deletions(-) diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 94c3666d0c..486ab2b33e 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,6 +1,6 @@ /* eslint-disable */ // Generated by Wrangler by running `wrangler types` (hash: 12ab7fcab3ef47d0e7c428770fc3c36a) -// Runtime types generated with workerd@1.20260701.1 2026-05-28 nodejs_compat +// Runtime types generated with workerd@1.20260722.1 2026-05-28 nodejs_compat interface __BaseEnv_Env { REVIEW_AUDIT: R2Bucket; VISUAL_CAPTURE_PUBLIC: R2Bucket; @@ -636,7 +636,7 @@ declare abstract class DurableObjectNamespace; jurisdiction(jurisdiction: DurableObjectJurisdiction): DurableObjectNamespace; } -type DurableObjectJurisdiction = "eu" | "fedramp" | "fedramp-high"; +type DurableObjectJurisdiction = "eu" | "fedramp" | "fedramp-high" | "us"; interface DurableObjectNamespaceNewUniqueIdOptions { jurisdiction?: DurableObjectJurisdiction; } @@ -12331,6 +12331,13 @@ interface ForwardableEmailMessage extends EmailMessage { * @returns A promise that resolves when the email message is replied. */ reply(message: EmailMessage): Promise; + /** + * Reply to the sender of this email message with a message built from the given + * fields. Threading headers (In-Reply-To/References) are set automatically. + * @param builder The reply message contents. + * @returns A promise that resolves when the email message is replied. + */ + reply(builder: EmailReplyMessageBuilder): Promise; } /** A file attachment for an email message */ type EmailAttachment = { @@ -12351,23 +12358,46 @@ interface EmailAddress { name: string; email: string; } +/** + * Recipient fields for `SendEmail.send()`. At least one of `to`, `cc`, or + * `bcc` must be provided. + */ +type EmailDestinations = { + to?: string | EmailAddress | (string | EmailAddress)[]; + cc?: string | EmailAddress | (string | EmailAddress)[]; + bcc?: string | EmailAddress | (string | EmailAddress)[]; +} & ({ + to: string | EmailAddress | (string | EmailAddress)[]; +} | { + cc: string | EmailAddress | (string | EmailAddress)[]; +} | { + bcc: string | EmailAddress | (string | EmailAddress)[]; +}); +/** + * Fields shared by all composed emails (no recipients). Used directly by + * `ForwardableEmailMessage.reply()`, which always replies to the original + * sender, and extended by `EmailMessageBuilder` for `SendEmail.send()`. + */ +interface EmailReplyMessageBuilder { + from: string | EmailAddress; + subject: string; + replyTo?: string | EmailAddress; + headers?: Record; + text?: string; + html?: string; + attachments?: EmailAttachment[]; +} +/** + * Fields for composing an email without constructing raw MIME, for + * `SendEmail.send()`. Requires at least one of `to`, `cc`, or `bcc`. + */ +type EmailMessageBuilder = EmailReplyMessageBuilder & EmailDestinations; /** * A binding that allows a Worker to send email messages. */ interface SendEmail { send(message: EmailMessage): Promise; - send(builder: { - from: string | EmailAddress; - to: string | EmailAddress | (string | EmailAddress)[]; - subject: string; - replyTo?: string | EmailAddress; - cc?: string | EmailAddress | (string | EmailAddress)[]; - bcc?: string | EmailAddress | (string | EmailAddress)[]; - headers?: Record; - text?: string; - html?: string; - attachments?: EmailAttachment[]; - }): Promise; + send(builder: EmailMessageBuilder): Promise; } declare abstract class EmailEvent extends ExtendableEvent { readonly message: ForwardableEmailMessage; @@ -13189,6 +13219,11 @@ declare namespace CloudflareWorkersModule { export type WorkflowDurationLabel = 'second' | 'minute' | 'hour' | 'day' | 'week' | 'month' | 'year'; export type WorkflowSleepDuration = `${number} ${WorkflowDurationLabel}${'s' | ''}` | number; export type WorkflowDelayDuration = WorkflowSleepDuration; + export type WorkflowDynamicDelayContext = { + ctx: WorkflowStepContext; + error: Error; + }; + export type WorkflowDelayFunction = (input: WorkflowDynamicDelayContext) => WorkflowDelayDuration | Promise; export type WorkflowTimeoutDuration = WorkflowSleepDuration; export type WorkflowRetentionDuration = WorkflowSleepDuration; export type WorkflowBackoff = 'constant' | 'linear' | 'exponential'; @@ -13196,7 +13231,7 @@ declare namespace CloudflareWorkersModule { export type WorkflowStepConfig = { retries?: { limit: number; - delay: WorkflowDelayDuration | number; + delay: WorkflowDelayDuration | number | WorkflowDelayFunction; backoff?: WorkflowBackoff; }; timeout?: WorkflowTimeoutDuration | number; @@ -13222,13 +13257,22 @@ declare namespace CloudflareWorkersModule { type: string; sensitive?: WorkflowStepSensitivity; }; - export type WorkflowStepContext = { + export type WorkflowStepContext = { step: { name: string; count: number; }; attempt: number; - config: WorkflowStepConfig; + config: { + retries?: { + limit: number; + backoff?: WorkflowBackoff; + } & (Delay extends WorkflowDelayFunction ? {} : { + delay: WorkflowDelayDuration | number; + }); + timeout?: WorkflowTimeoutDuration | number; + sensitive?: WorkflowStepSensitivity; + }; }; export type WorkflowRollbackContext = { ctx: WorkflowStepContext; @@ -13244,7 +13288,9 @@ declare namespace CloudflareWorkersModule { }; export abstract class WorkflowStep { do>(name: string, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; - do>(name: string, config: WorkflowStepConfig, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; + do, const C extends WorkflowStepConfig>(name: string, config: C, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; sleep: (name: string, duration: WorkflowSleepDuration) => Promise; sleepUntil: (name: string, timestamp: Date | number) => Promise; waitForEvent>(name: string, options: { @@ -14154,6 +14200,7 @@ declare namespace TailStream { readonly dispatchNamespace?: string; readonly entrypoint?: string; readonly executionModel: string; + readonly durableObjectId?: string; readonly scriptName?: string; readonly scriptTags?: string[]; readonly scriptVersion?: ScriptVersion; @@ -14708,6 +14755,13 @@ interface WorkflowError { code?: number; message: string; } +interface WorkflowInstanceTerminateOptions { + /** + * If true, run registered rollback handlers before terminating the instance. + * Only steps that registered rollback handlers are rolled back. + */ + rollback?: boolean; +} interface WorkflowInstanceRestartOptions { /** * Restart from a specific step. If omitted, the instance restarts from the beginning. @@ -14741,8 +14795,9 @@ declare abstract class WorkflowInstance { public resume(): Promise; /** * Terminate the instance. If it is errored, terminated or complete, an error will be thrown. + * @param options Options for termination, including whether registered rollback handlers should run. */ - public terminate(): Promise; + public terminate(options?: WorkflowInstanceTerminateOptions): Promise; /** * Restart the instance. Optionally restart from a specific step, preserving * cached results for all steps before it.