From c844f838993c94c3520582c8c1627762d56e0f7b Mon Sep 17 00:00:00 2001 From: shin-core <153108882+shin-core@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:01:42 +0900 Subject: [PATCH] fix(review): give shadow overrides the same expired-clear_at handling as live overrides auto-apply.ts's LIVE override read/write pair treats an already-lapsed clear_at as cleared: loadOverride threads nowIso into rowToOverride, and writeLiveOverride drops an expired clear_at rather than resurrecting it. The SHADOW pair only ported the "preserve the column" half of the stale-clear-at fix, not the "drop it once expired" half: writeShadowOverride had no nowIso param and re-persisted the existing row's clear_at unconditionally, and loadShadowOverride called rowToOverride with no nowIso. So a shadow override whose clear_at has lapsed was read back as active, and a stale shadow tightening could still be promoted to live after its operator-set expiry passed. Thread an optional nowIso through both, mirroring the live pair exactly: loadShadowOverride passes nowIso to rowToOverride, and writeShadowOverride computes clearAt via the same "existing row AND clear_at not expired, else null" rule writeLiveOverride uses, plus rowToOverride with nowIso on the merge read. Both params are optional, so every existing caller compiles and behaves exactly as before. The live pair, the promotion/soak-gate decision logic, and every other behaviour are unchanged. Closes #10291 --- src/review/auto-apply.ts | 16 +++++++++++----- test/unit/auto-apply.test.ts | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/review/auto-apply.ts b/src/review/auto-apply.ts index 33e196659..ad3f0661d 100644 --- a/src/review/auto-apply.ts +++ b/src/review/auto-apply.ts @@ -276,10 +276,14 @@ async function loadShadowOverrideRow(env: StorageEnv, project: string): Promise< /** 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) * 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 { +export async function writeShadowOverride(env: StorageEnv, project: string, o: TunableOverride, validatedUntilIso: string, nowIso?: string): Promise { const existingRow = await loadShadowOverrideRow(env, project); - const merged = mergeOverride(existingRow ? rowToOverride(existingRow) : null, o); - const clearAt = existingRow?.clear_at ?? null; + // #10291: mirror writeLiveOverride exactly. Thread nowIso through the merge read AND the clear_at + // preservation so an already-lapsed clear_at is DROPPED rather than resurrected — the shadow side only + // ported the "preserve the column" half of the #stale-clear-at-fix, not the "drop it once expired" half, + // so a stale shadow tightening could be promoted to live after its own operator-set expiry had passed. + const merged = mergeOverride(existingRow ? rowToOverride(existingRow, nowIso) : null, o); + const clearAt = existingRow && !clearAtIsExpired(existingRow.clear_at, nowIso) ? 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, clear_at) VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP, ?, ?)", @@ -289,10 +293,12 @@ export async function writeShadowOverride(env: StorageEnv, project: string, o: T } /** Load the pending shadow override for a project (null if none / DB error). */ -export async function loadShadowOverride(env: StorageEnv, project: string): Promise { +export async function loadShadowOverride(env: StorageEnv, project: string, nowIso?: string): Promise { const row = await loadShadowOverrideRow(env, project); if (!row) return null; - const override = rowToOverride(row); + // #10291: thread nowIso (mirroring loadOverride) so a shadow row whose clear_at has already lapsed is read + // back as cleared, not still-active — rowToOverride applies the same clearAtIsExpired rule the live read uses. + const override = rowToOverride(row, nowIso); return override ? { override, validatedUntil: row.validated_until } : null; } diff --git a/test/unit/auto-apply.test.ts b/test/unit/auto-apply.test.ts index 1328582fb..85a4a4d56 100644 --- a/test/unit/auto-apply.test.ts +++ b/test/unit/auto-apply.test.ts @@ -612,6 +612,25 @@ describe("writeShadowOverride / loadShadowOverride / deleteShadowOverride", () = 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"); }); + // #10291: the shadow pair only ported the "preserve the column" half of #stale-clear-at-fix, not the + // "drop it once expired" half — mirror the live-side "does NOT resurrect an ALREADY-EXPIRED override" test. + it("#10291: writeShadowOverride DROPS an already-expired clear_at (and does not resurrect the expired floor) when nowIso is passed", async () => { + const { env, tables } = fakeEnv(); + tables.shadow.set("g", { confidence_floor: 0.8, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z", clear_at: "2020-01-01T00:00:00Z" }); + await writeShadowOverride(env, "g", { scopeCap: { files: 3, lines: 100 } }, "2026-06-25T00:00:00Z", "2026-06-20T00:00:00Z"); + const row = tables.shadow.get("g"); + expect(row?.clear_at).toBeNull(); // the lapsed clear_at is dropped, not carried forward + expect(row?.confidence_floor).toBeNull(); // the expired floor is not resurrected into the merge + expect(row?.scope_cap_files).toBe(3); // the new write still applies normally + }); + it("#10291: loadShadowOverride reads an already-expired clear_at row as cleared when nowIso is after it", async () => { + const { env, tables } = fakeEnv(); + // Only a confidence_floor gated by an expired clear_at: rowToOverride drops it → the override is empty → null. + tables.shadow.set("g", { confidence_floor: 0.8, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z", clear_at: "2020-01-01T00:00:00Z" }); + expect(await loadShadowOverride(env, "g", "2026-06-20T00:00:00Z")).toBeNull(); + // Without nowIso (the pre-#10291 caller convention) the row is still read active — additive, non-breaking. + expect(await loadShadowOverride(env, "g")).not.toBeNull(); + }); 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" });