diff --git a/src/review/auto-apply.ts b/src/review/auto-apply.ts index 9ad3099171..4e13594e72 100644 --- a/src/review/auto-apply.ts +++ b/src/review/auto-apply.ts @@ -18,7 +18,7 @@ // caller passes the already-computed recommendations + eval row into runAutoApplyRecommendations here. // The host wires those at cutover; the AutoApplyDeps interface below is the seam. -import type { OverridePayload, TuningRec } from "./auto-tune"; +import { RISK_MERGE_PRECISION, type OverridePayload, type TuningRec } from "./auto-tune"; // ── Inline minimal D1 storage seam + helper (matches the runtime's env.DB calls, no CF type dependency) ── @@ -61,10 +61,16 @@ interface OverrideRow { clear_at: string | null; } +/** PURE: is a row's clear_at in the past relative to nowIso? (Skipped — not expired — when either is unset.) + * Extracted so writeLiveOverride's clear_at-preservation logic uses the exact same rule as rowToOverride. */ +function clearAtIsExpired(clearAt: string | null, nowIso?: string): boolean { + return !!(clearAt && nowIso && clearAt <= nowIso); +} + /** PURE: a D1 row → a validated TunableOverride (or null when empty/expired/invalid). Unit-testable. */ export function rowToOverride(row: OverrideRow | null, nowIso?: string): TunableOverride | null { if (!row) return null; - if (row.clear_at && nowIso && row.clear_at <= nowIso) return null; // past clear_at → treated as cleared + if (clearAtIsExpired(row.clear_at, nowIso)) return null; // past clear_at → treated as cleared const o: TunableOverride = {}; if (typeof row.confidence_floor === "number" && row.confidence_floor >= 0 && row.confidence_floor <= 1) { o.confidenceFloor = row.confidence_floor; @@ -140,7 +146,11 @@ export const SHADOW_PROMOTION_MIN_DECIDED = 10; export const SHADOW_SOAK_MS = 24 * 60 * 60 * 1000; /** PURE promotion gate: promote a SHADOW override → LIVE only when it is (1) strictly tightening vs the live - * config, (2) backed by >= SHADOW_PROMOTION_MIN_DECIDED decided samples, and (3) SOAKED past validated_until. + * config, (2) backed by >= SHADOW_PROMOTION_MIN_DECIDED decided samples, (3) SOAKED past validated_until, and + * (4) the tightening is STILL warranted by the project's freshly-measured merge precision. Without (4) a + * 24h-old snapshot's verdict is applied blind to what happened since — a transient bad batch of outcomes that + * has since fully recovered would still get permanently promoted, because (1)-(3) only compare against the + * UNCHANGED live config, never re-derive whether the tightening is still justified. (#stale-shadow-promotion-fix) * Returns {promote, reason} so the cron can log why it did / didn't. (#276 evaluation-gated promotion) */ export function evaluateShadowPromotion(args: { override: TunableOverride; @@ -149,6 +159,10 @@ export function evaluateShadowPromotion(args: { decided: number; validatedUntilIso: string | null; nowIso: string; + /** The project's freshly-recomputed merge precision as of THIS tick (not the stale value the shadow rec was + * originally computed from). null/undefined when unavailable — the freshness check is then skipped rather + * than blocking promotion (fail toward the existing, already-verified soak+evidence gate). */ + currentMergePrecision?: number | null; }): { promote: boolean; reason: string } { if (!isStrictlyTightening(args.override, args.liveFloor, args.liveScopeCap)) { return { promote: false, reason: "not strictly tightening vs live config" }; @@ -159,6 +173,12 @@ export function evaluateShadowPromotion(args: { if (!args.validatedUntilIso || args.nowIso < args.validatedUntilIso) { return { promote: false, reason: `still soaking${args.validatedUntilIso ? ` until ${args.validatedUntilIso}` : ""}` }; } + if (args.currentMergePrecision != null && args.currentMergePrecision >= RISK_MERGE_PRECISION) { + return { + promote: false, + reason: `underlying merge precision recovered (${args.currentMergePrecision} >= ${RISK_MERGE_PRECISION}) — tightening no longer warranted`, + }; + } return { promote: true, reason: "tightening + evidence + soaked" }; } @@ -169,28 +189,38 @@ function newAuditId(): string { return `ova_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 10)}`; } -/** Load the active LIVE override for a project (null if none / expired / DB error). clear_at in the past = - * cleared. Fail-safe: a DB blip yields no override, never a blocked review. */ -export async function loadOverride(env: StorageEnv, project: string, nowIso?: string): Promise { - let row: OverrideRow | null; +/** Internal: raw row fetch shared by loadOverride + writeLiveOverride, so a write can preserve the existing + * clear_at column (loadOverride's public return, TunableOverride, doesn't carry clear_at). Fail-safe: null on + * a DB blip. */ +async function loadOverrideRow(env: StorageEnv, project: string): Promise { try { - row = await storage(env) + return await storage(env) .prepare("SELECT confidence_floor, scope_cap_files, scope_cap_lines, clear_at FROM tunables_overrides WHERE project = ?") .bind(project) .first(); } catch { return null; // fail-safe: no override on a DB blip } - return rowToOverride(row, nowIso); +} + +/** Load the active LIVE override for a project (null if none / expired / DB error). clear_at in the past = + * cleared. Fail-safe: a DB blip yields no override, never a blocked review. */ +export async function loadOverride(env: StorageEnv, project: string, nowIso?: string): Promise { + return rowToOverride(await loadOverrideRow(env, project), nowIso); } /** Write the LIVE override for a project, MERGED over any existing row (partial writes are additive, never - * destructive). Used by the apply path (force) + shadow promotion. */ -export async function writeLiveOverride(env: StorageEnv, project: string, o: TunableOverride): Promise { - const merged = mergeOverride(await loadOverride(env, project), o); + * destructive). Used by the apply path (force) + shadow promotion. Preserves any existing clear_at (an + * operator's temporary-override expiration) rather than silently nulling it via INSERT OR REPLACE, UNLESS + * that clear_at has itself already lapsed, in which case it is dropped rather than resurrected — nowIso is + * passed into the internal re-read for exactly this reason (#stale-clear-at-fix). */ +export async function writeLiveOverride(env: StorageEnv, project: string, o: TunableOverride, nowIso?: string): Promise { + const existingRow = await loadOverrideRow(env, project); + const merged = mergeOverride(rowToOverride(existingRow, nowIso), o); + const clearAt = existingRow && !clearAtIsExpired(existingRow.clear_at, nowIso) ? existingRow.clear_at : null; await storage(env) - .prepare("INSERT OR REPLACE INTO tunables_overrides (project, confidence_floor, scope_cap_files, scope_cap_lines, applied_at) VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP)") - .bind(project, merged.confidenceFloor ?? null, merged.scopeCap?.files ?? null, merged.scopeCap?.lines ?? null) + .prepare("INSERT OR REPLACE INTO tunables_overrides (project, confidence_floor, scope_cap_files, scope_cap_lines, applied_at, clear_at) VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP, ?)") + .bind(project, merged.confidenceFloor ?? null, merged.scopeCap?.files ?? null, merged.scopeCap?.lines ?? null, clearAt) .run(); } @@ -204,32 +234,41 @@ export interface ShadowOverride { validatedUntil: string | null; } +/** Internal: raw row fetch shared by loadShadowOverride + writeShadowOverride, so a write can preserve the + * existing clear_at column (ShadowOverride, loadShadowOverride's public return, doesn't carry clear_at). + * Fail-safe: null on a DB blip. */ +async function loadShadowOverrideRow(env: StorageEnv, project: string): Promise<(OverrideRow & { validated_until: string | null }) | null> { + try { + return await storage(env) + .prepare("SELECT confidence_floor, scope_cap_files, scope_cap_lines, validated_until, clear_at FROM tunables_overrides_shadow WHERE project = ?") + .bind(project) + .first(); + } catch { + return null; + } +} + /** Write a recommended override to the SHADOW queue with a future validated_until (the soak deadline). MERGED - * over any existing shadow row so a partial write never erases a prior queued tunable. (#partial-overwrite-fix) */ + * over any existing shadow row so a partial write never erases a prior queued tunable. (#partial-overwrite-fix) + * Preserves any existing clear_at rather than silently nulling it via INSERT OR REPLACE (#stale-clear-at-fix). */ export async function writeShadowOverride(env: StorageEnv, project: string, o: TunableOverride, validatedUntilIso: string): Promise { - const existing = await loadShadowOverride(env, project); - const merged = mergeOverride(existing?.override ?? null, o); + const existingRow = await loadShadowOverrideRow(env, project); + const merged = mergeOverride(existingRow ? rowToOverride(existingRow) : null, o); + const clearAt = existingRow?.clear_at ?? null; await storage(env) .prepare( - "INSERT OR REPLACE INTO tunables_overrides_shadow (project, confidence_floor, scope_cap_files, scope_cap_lines, applied_at, validated_until) VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP, ?)", + "INSERT OR REPLACE INTO tunables_overrides_shadow (project, confidence_floor, scope_cap_files, scope_cap_lines, applied_at, validated_until, clear_at) VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP, ?, ?)", ) - .bind(project, merged.confidenceFloor ?? null, merged.scopeCap?.files ?? null, merged.scopeCap?.lines ?? null, validatedUntilIso) + .bind(project, merged.confidenceFloor ?? null, merged.scopeCap?.files ?? null, merged.scopeCap?.lines ?? null, validatedUntilIso, clearAt) .run(); } /** Load the pending shadow override for a project (null if none / DB error). */ export async function loadShadowOverride(env: StorageEnv, project: string): Promise { - try { - const row = await storage(env) - .prepare("SELECT confidence_floor, scope_cap_files, scope_cap_lines, validated_until FROM tunables_overrides_shadow WHERE project = ?") - .bind(project) - .first(); - if (!row) return null; - const override = rowToOverride(row); - return override ? { override, validatedUntil: row.validated_until } : null; - } catch { - return null; - } + const row = await loadShadowOverrideRow(env, project); + if (!row) return null; + const override = rowToOverride(row); + return override ? { override, validatedUntil: row.validated_until } : null; } /** Delete a project's shadow override (after promotion, or on clear). */ @@ -237,15 +276,18 @@ export async function deleteShadowOverride(env: StorageEnv, project: string): Pr await storage(env).prepare("DELETE FROM tunables_overrides_shadow WHERE project = ?").bind(project).run(); } -/** Record one override-lifecycle event to the dedicated (target-free) audit table. Fail-safe. (#279) */ +/** Record one override-lifecycle event to the dedicated (target-free) audit table. Fail-safe: a write error + * never breaks the apply path, but it IS surfaced at error level (this is the operator's ONLY visibility + * into an autonomous config change — a silently-dropped log line here defeats that entirely). */ export async function recordOverrideAudit(env: StorageEnv, project: string, eventType: string, detail: Record): Promise { try { await storage(env) .prepare("INSERT INTO override_audit (id, project, event_type, detail) VALUES (?, ?, ?, ?)") .bind(newAuditId(), project, eventType, JSON.stringify(detail)) .run(); - } catch { - /* telemetry must never break the apply path */ + } catch (error) { + // telemetry must never break the apply path, but it must not be silent either. + console.error(JSON.stringify({ level: "error", event: "override_audit_write_failed", project, eventType, message: String(error).slice(0, 160) })); } } @@ -280,13 +322,16 @@ export async function applyOverrideRecommendation( opts: { force: boolean; soakMs: number; nowMs: number }, ): Promise { if (opts.force) { - await writeLiveOverride(env, project, payload); + // Audit BEFORE the mutation: recordOverrideAudit is itself fail-safe (a swallowed D1 blip must never break + // the apply path), so writing it first means the worst case is an audit row for a write that then fails — + // never a live config change with zero audit trail. (#audit-before-write-fix) await recordOverrideAudit(env, project, "override_applied", { override: payload, force: true }); + await writeLiveOverride(env, project, payload, new Date(opts.nowMs).toISOString()); return { ok: true, applied: true, reason: `force-applied ${describeOverride(payload)}` }; } const validatedUntil = new Date(opts.nowMs + opts.soakMs).toISOString(); - await writeShadowOverride(env, project, payload, validatedUntil); await recordOverrideAudit(env, project, "override_shadowed", { override: payload, validatedUntil }); + await writeShadowOverride(env, project, payload, validatedUntil); return { ok: true, applied: false, shadowed: true, validatedUntil, reason: `shadow-queued ${describeOverride(payload)} until ${validatedUntil}` }; } @@ -312,6 +357,11 @@ export interface AutoApplyContext { baseScopeCap?: { files: number; lines: number }; /** This project's decided-sample count from the gate eval (drives the promotion evidence gate). */ decided: number; + /** This project's freshly-computed merge precision from THIS tick's gate eval (the same field + * computeTuningRecommendations reads). Threaded into evaluateShadowPromotion so a shadow-queued tightening + * cannot be promoted once the precision that originally warranted it has since recovered. Optional/nullable + * because a project can have no would-merge samples yet (GateEvalRow.mergePrecision is null in that case). */ + mergePrecision?: number | null; /** The tuning advisor's recommendations for this project (only ones with an overridePayload are applied). */ recs: TuningRec[]; /** Current wall-clock (ms) — injected for determinism in tests. */ @@ -354,11 +404,14 @@ export async function runAutoApplyRecommendations(env: StorageEnv, ctx: AutoAppl decided: ctx.decided, validatedUntilIso: shadow.validatedUntil, nowIso, + ...(ctx.mergePrecision !== undefined ? { currentMergePrecision: ctx.mergePrecision } : {}), }); if (gate.promote) { - await writeLiveOverride(env, ctx.project, shadow.override); - await deleteShadowOverride(env, ctx.project); + // Audit BEFORE the mutation — see applyOverrideRecommendation's force branch for why this ordering + // matters. (#audit-before-write-fix) await recordOverrideAudit(env, ctx.project, "override_promoted", { override: shadow.override, reason: gate.reason }); + await writeLiveOverride(env, ctx.project, shadow.override, nowIso); + await deleteShadowOverride(env, ctx.project); console.log(JSON.stringify({ event: "auto_apply_promoted", project: ctx.project, override: describeOverride(shadow.override) })); } else { console.log(JSON.stringify({ event: "auto_apply_hold", project: ctx.project, reason: gate.reason })); diff --git a/src/review/auto-tune.ts b/src/review/auto-tune.ts index a668cc968b..d6cf730dd4 100644 --- a/src/review/auto-tune.ts +++ b/src/review/auto-tune.ts @@ -283,7 +283,10 @@ export interface TuningRec { const MIN_DECIDED = 10; const READY_MERGE_PRECISION = 0.95; const READY_CLOSE_PRECISION = 0.9; -const RISK_MERGE_PRECISION = 0.9; +// Exported (not just a local const) so auto-apply.ts's shadow-promotion gate can refuse to promote a stale +// tightening recommendation once the project's own freshly-measured precision has recovered back above this +// same bar the recommendation was originally computed against (#stale-shadow-promotion-fix). +export const RISK_MERGE_PRECISION = 0.9; // The tighten TARGET for a merge-precision failure: raise the floor to the known-good "ready" bar. It is a // project-agnostic, principled target — the apply path raises ONLY if it is above the project's current floor, // so an already-strict project is never affected (and a higher target can't add a bad auto-merge). (#275) diff --git a/test/unit/auto-apply.test.ts b/test/unit/auto-apply.test.ts index c1f9ca0768..20b6cf64b9 100644 --- a/test/unit/auto-apply.test.ts +++ b/test/unit/auto-apply.test.ts @@ -115,13 +115,32 @@ describe("evaluateShadowPromotion (#276 — tighten-only + evidence + soak gate) it("refuses when validated_until is unset (never soaked)", () => { expect(evaluateShadowPromotion({ ...base, validatedUntilIso: null }).promote).toBe(false); }); + + // (#stale-shadow-promotion-fix) The audited failure: a shadow tightening queued while precision was bad + // (0.2) must NOT be promoted once 24h later the project's OWN freshly-measured precision has recovered back + // above the risk floor (0.9) that originally triggered it — even though it is still strictly tightening vs + // the (unchanged) live config, has plenty of evidence, and has fully soaked. + it("refuses promotion once the underlying merge precision has recovered above the risk floor", () => { + const r = evaluateShadowPromotion({ ...base, currentMergePrecision: 0.92 }); + expect(r.promote).toBe(false); + expect(r.reason).toMatch(/recovered/); + }); + it("still promotes when the current merge precision is still below the risk floor", () => { + expect(evaluateShadowPromotion({ ...base, currentMergePrecision: 0.5 }).promote).toBe(true); + }); + it("still promotes when currentMergePrecision is omitted (freshness check is skipped, not blocking)", () => { + expect(evaluateShadowPromotion(base).promote).toBe(true); + }); + it("a currentMergePrecision exactly AT the risk floor still refuses (>= boundary)", () => { + expect(evaluateShadowPromotion({ ...base, currentMergePrecision: 0.9 }).promote).toBe(false); + }); }); // ── A tiny in-memory D1-shaped store for the store/orchestration tests (the deferred infra seam) ───────── type Tables = { live: Map; - shadow: Map; + shadow: Map; audit: Array<{ project: string; event_type: string; detail: string | null; created_at: string }>; }; @@ -147,11 +166,11 @@ function fakeEnv(): { env: StorageEnv; tables: Tables } { }, async run(): Promise { if (sql.startsWith("INSERT OR REPLACE INTO tunables_overrides_shadow")) { - const [project, cf, scf, scl, vu] = bound as [string, number | null, number | null, number | null, string | null]; - tables.shadow.set(project, { confidence_floor: cf, scope_cap_files: scf, scope_cap_lines: scl, validated_until: vu }); + const [project, cf, scf, scl, vu, clearAt] = bound as [string, number | null, number | null, number | null, string | null, string | null]; + tables.shadow.set(project, { confidence_floor: cf, scope_cap_files: scf, scope_cap_lines: scl, validated_until: vu, clear_at: clearAt ?? null }); } else if (sql.startsWith("INSERT OR REPLACE INTO tunables_overrides")) { - const [project, cf, scf, scl] = bound as [string, number | null, number | null, number | null]; - tables.live.set(project, { confidence_floor: cf, scope_cap_files: scf, scope_cap_lines: scl, clear_at: null }); + const [project, cf, scf, scl, clearAt] = bound as [string, number | null, number | null, number | null, string | null]; + tables.live.set(project, { confidence_floor: cf, scope_cap_files: scf, scope_cap_lines: scl, clear_at: clearAt ?? null }); } else if (sql.startsWith("DELETE FROM tunables_overrides_shadow")) { tables.shadow.delete(bound[0] as string); } else if (sql.startsWith("DELETE FROM tunables_overrides")) { @@ -215,6 +234,44 @@ describe("applyOverrideRecommendation (#277 — force vs shadow-soak)", () => { expect(tables.shadow.get("g")?.confidence_floor).toBe(0.95); expect(tables.live.has("g")).toBe(false); }); + + // (#audit-before-write-fix) The audited failure: a transient D1 blip on the LIVE write must not leave the + // apply path with zero audit trail. Proven here by making ONLY the live-table write throw while the audit + // INSERT succeeds — if the ordering is right (audit-first), the audit row exists despite the write failing. + it("records the audit row BEFORE the live write, so a write failure still leaves an audit trail", async () => { + const audit: Array<{ eventType: string }> = []; + const env: StorageEnv = { + DB: { + prepare: (sql: string) => { + const stmt = { + bind(...vals: unknown[]) { + return { + async first() { + return null; + }, + async run() { + if (sql.startsWith("INSERT INTO override_audit")) { + audit.push({ eventType: vals[2] as string }); + return {}; + } + if (sql.startsWith("INSERT OR REPLACE INTO tunables_overrides") && !sql.includes("_shadow")) { + throw new Error("d1 write blip"); // the LIVE write fails + } + return {}; + }, + async all() { + return { results: [] }; + }, + }; + }, + }; + return stmt as unknown as ReturnType; + }, + }, + }; + await expect(applyOverrideRecommendation(env, "g", { confidenceFloor: 0.95 }, { force: true, soakMs: 1000, nowMs: 0 })).rejects.toThrow("d1 write blip"); + expect(audit.some((a) => a.eventType === "override_applied")).toBe(true); + }); }); describe("runAutoApplyRecommendations (#278 — closes the loop: queue tightening → soak → promote)", () => { @@ -276,6 +333,26 @@ describe("runAutoApplyRecommendations (#278 — closes the loop: queue tightenin expect(tables.live.has("g")).toBe(false); expect(tables.shadow.has("g")).toBe(true); }); + + // (#stale-shadow-promotion-fix) The exact audited scenario: a shadow tightening was queued (and has since + // soaked past its deadline) when the project's precision was 0.2. By the time the cron re-ticks, the ctx the + // host passes in carries the FRESHLY-recomputed precision (0.92 — recovered above the 0.9 risk floor). The + // promotion must be refused even though the soak/evidence/tightening checks would all otherwise pass. + it("refuses to promote a stale shadow tightening once the project's precision has since recovered", async () => { + const { env, tables } = fakeEnv(); + tables.shadow.set("g", { confidence_floor: 0.95, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z" }); + await runAutoApplyRecommendations(env, ctx({ recs: [], mergePrecision: 0.92 })); + expect(tables.live.has("g")).toBe(false); // NOT promoted + expect(tables.shadow.has("g")).toBe(true); // stays queued rather than being silently dropped + }); + + it("still promotes a soaked shadow override when the fresh precision has NOT recovered", async () => { + const { env, tables } = fakeEnv(); + tables.shadow.set("g", { confidence_floor: 0.95, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z" }); + await runAutoApplyRecommendations(env, ctx({ recs: [], mergePrecision: 0.5 })); + expect(tables.live.get("g")?.confidence_floor).toBe(0.95); + expect(tables.shadow.has("g")).toBe(false); + }); }); // ──────────────────────────────────────────────────────────────────────────────────────────────────────── @@ -439,6 +516,30 @@ describe("writeLiveOverride / deleteLiveOverride (D1 writes)", () => { await deleteLiveOverride(env, "g"); expect(tables.live.has("g")).toBe(false); }); + + // (#stale-clear-at-fix) Previously the INSERT OR REPLACE column list omitted clear_at entirely, so SQLite's + // REPLACE (delete-then-insert) unconditionally nulled any existing operator-set expiration on every write. + it("PRESERVES an existing (non-expired) clear_at across a write instead of silently nulling it", async () => { + const { env, tables } = fakeEnv(); + tables.live.set("g", { confidence_floor: 0.9, scope_cap_files: null, scope_cap_lines: null, clear_at: "2099-01-01T00:00:00Z" }); + await writeLiveOverride(env, "g", { confidenceFloor: 0.95 }); + expect(tables.live.get("g")?.clear_at).toBe("2099-01-01T00:00:00Z"); + expect(tables.live.get("g")?.confidence_floor).toBe(0.95); + }); + + // (#stale-clear-at-fix) Previously the internal loadOverride() re-read inside writeLiveOverride never passed + // nowIso, so rowToOverride's `row.clear_at && nowIso && ...` guard short-circuited and an ALREADY-EXPIRED + // override was merged back in as still active. Passing nowIso through fixes both: the expired floor is not + // resurrected, and the stale clear_at itself is dropped rather than carried forward. + it("does NOT resurrect an ALREADY-EXPIRED override (or its stale clear_at) when nowIso is passed", async () => { + const { env, tables } = fakeEnv(); + tables.live.set("g", { confidence_floor: 0.8, scope_cap_files: null, scope_cap_lines: null, clear_at: "2020-01-01T00:00:00Z" }); + await writeLiveOverride(env, "g", { scopeCap: { files: 3, lines: 100 } }, "2026-06-20T00:00:00Z"); + const row = tables.live.get("g"); + expect(row?.confidence_floor).toBeNull(); // the expired floor is NOT resurrected + expect(row?.clear_at).toBeNull(); // the lapsed clear_at is not carried forward either + expect(row?.scope_cap_files).toBe(3); // the new write still applies normally + }); }); describe("writeShadowOverride / loadShadowOverride / deleteShadowOverride", () => { @@ -446,12 +547,14 @@ describe("writeShadowOverride / loadShadowOverride / deleteShadowOverride", () = const { env, tables } = fakeEnv(); tables.shadow.set("g", { confidence_floor: null, scope_cap_files: 5, scope_cap_lines: 200, validated_until: "2026-06-19T00:00:00Z" }); await writeShadowOverride(env, "g", { confidenceFloor: 0.95 }, "2026-06-25T00:00:00Z"); - expect(tables.shadow.get("g")).toEqual({ confidence_floor: 0.95, scope_cap_files: 5, scope_cap_lines: 200, validated_until: "2026-06-25T00:00:00Z" }); + // clear_at: null is now asserted explicitly (previously the INSERT OR REPLACE column list dropped clear_at + // entirely, so the written row never carried it — the fix now writes it through on every shadow write). + expect(tables.shadow.get("g")).toEqual({ confidence_floor: 0.95, scope_cap_files: 5, scope_cap_lines: 200, validated_until: "2026-06-25T00:00:00Z", clear_at: null }); }); it("writes a fresh shadow row when none exists (existing?.override ?? null arm)", async () => { const { env, tables } = fakeEnv(); await writeShadowOverride(env, "g", { scopeCap: { files: 2, lines: 40 } }, "2026-06-25T00:00:00Z"); - expect(tables.shadow.get("g")).toEqual({ confidence_floor: null, scope_cap_files: 2, scope_cap_lines: 40, validated_until: "2026-06-25T00:00:00Z" }); + expect(tables.shadow.get("g")).toEqual({ confidence_floor: null, scope_cap_files: 2, scope_cap_lines: 40, validated_until: "2026-06-25T00:00:00Z", clear_at: null }); }); it("loadShadowOverride returns the override + validatedUntil when present", async () => { const { env, tables } = fakeEnv(); @@ -462,6 +565,13 @@ describe("writeShadowOverride / loadShadowOverride / deleteShadowOverride", () = const { env } = fakeEnv(); expect(await loadShadowOverride(env, "missing")).toBeNull(); }); + // (#stale-clear-at-fix) The shadow table has the same column-drop bug as the live table. + it("PRESERVES an existing clear_at across a shadow write instead of silently nulling it", async () => { + const { env, tables } = fakeEnv(); + tables.shadow.set("g", { confidence_floor: 0.9, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z", clear_at: "2099-01-01T00:00:00Z" }); + await writeShadowOverride(env, "g", { confidenceFloor: 0.95 }, "2026-06-25T00:00:00Z"); + expect(tables.shadow.get("g")?.clear_at).toBe("2099-01-01T00:00:00Z"); + }); it("loadShadowOverride returns null when the row maps to an EMPTY override (rowToOverride → null arm)", async () => { const { env, tables } = fakeEnv(); tables.shadow.set("g", { confidence_floor: null, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-25T00:00:00Z" }); @@ -484,8 +594,15 @@ describe("recordOverrideAudit / listOverrideAudit", () => { await recordOverrideAudit(env, "g", "override_applied", { force: true }); expect(tables.audit.at(-1)).toMatchObject({ project: "g", event_type: "override_applied", detail: JSON.stringify({ force: true }) }); }); - it("SWALLOWS a DB error (telemetry must never break the apply path)", async () => { - await expect(recordOverrideAudit(throwingEnv(), "g", "x", {})).resolves.toBeUndefined(); + it("SWALLOWS a DB error but logs it at error level (telemetry never breaks the apply path, but is never silent)", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + await expect(recordOverrideAudit(throwingEnv(), "g", "x", {})).resolves.toBeUndefined(); + const errLog = errorSpy.mock.calls.map((c) => String(c[0])).find((s) => s.includes("override_audit_write_failed")); + expect(errLog).toBeTruthy(); + } finally { + errorSpy.mockRestore(); + } }); it("lists audit rows newest-first, mapped to the public shape", async () => { const { env } = fakeEnv(); @@ -619,6 +736,34 @@ describe("runAutoApplyRecommendations — remaining branches", () => { } }); + // (#audit-before-write-fix) Same ordering guarantee as applyOverrideRecommendation's force branch, but for + // the PROMOTION path: audits before writing live, so a write failure mid-promotion still leaves a trail. + it("promotion audits BEFORE writing live, so a write failure still records the promotion attempt", async () => { + const { env: baseEnv, tables } = fakeEnv(); + tables.shadow.set("g", { confidence_floor: 0.95, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z" }); + const env: StorageEnv = { + DB: { + prepare: (sql: string) => { + if (sql.startsWith("INSERT OR REPLACE INTO tunables_overrides") && !sql.includes("_shadow")) { + return { + bind: () => ({ + async run() { + throw new Error("d1 write blip"); // the LIVE write fails during promotion + }, + }), + } as unknown as ReturnType; + } + return baseEnv.DB.prepare(sql); + }, + }, + }; + await runAutoApplyRecommendations(env, ctx({ recs: [] })); // fails safe: throws are caught, never rethrown + expect(tables.audit.some((a) => a.event_type === "override_promoted")).toBe(true); + // the write (and the subsequent delete) never completed, so the shadow row is still queued + expect(tables.shadow.has("g")).toBe(true); + expect(tables.live.has("g")).toBe(false); + }); + it("breaks after the FIRST tightening rec (only one pending soak at a time)", async () => { const { env, tables } = fakeEnv(); const recs: TuningRec[] = [