Skip to content

Commit 5b95330

Browse files
authored
fix(review): give shadow overrides the same expired-clear_at handling as live overrides (#10325)
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
1 parent b73575f commit 5b95330

2 files changed

Lines changed: 30 additions & 5 deletions

File tree

src/review/auto-apply.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,14 @@ async function loadShadowOverrideRow(env: StorageEnv, project: string): Promise<
276276
/** Write a recommended override to the SHADOW queue with a future validated_until (the soak deadline). MERGED
277277
* over any existing shadow row so a partial write never erases a prior queued tunable. (#partial-overwrite-fix)
278278
* Preserves any existing clear_at rather than silently nulling it via INSERT OR REPLACE (#stale-clear-at-fix). */
279-
export async function writeShadowOverride(env: StorageEnv, project: string, o: TunableOverride, validatedUntilIso: string): Promise<void> {
279+
export async function writeShadowOverride(env: StorageEnv, project: string, o: TunableOverride, validatedUntilIso: string, nowIso?: string): Promise<void> {
280280
const existingRow = await loadShadowOverrideRow(env, project);
281-
const merged = mergeOverride(existingRow ? rowToOverride(existingRow) : null, o);
282-
const clearAt = existingRow?.clear_at ?? null;
281+
// #10291: mirror writeLiveOverride exactly. Thread nowIso through the merge read AND the clear_at
282+
// preservation so an already-lapsed clear_at is DROPPED rather than resurrected — the shadow side only
283+
// ported the "preserve the column" half of the #stale-clear-at-fix, not the "drop it once expired" half,
284+
// so a stale shadow tightening could be promoted to live after its own operator-set expiry had passed.
285+
const merged = mergeOverride(existingRow ? rowToOverride(existingRow, nowIso) : null, o);
286+
const clearAt = existingRow && !clearAtIsExpired(existingRow.clear_at, nowIso) ? existingRow.clear_at : null;
283287
await storage(env)
284288
.prepare(
285289
"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
289293
}
290294

291295
/** Load the pending shadow override for a project (null if none / DB error). */
292-
export async function loadShadowOverride(env: StorageEnv, project: string): Promise<ShadowOverride | null> {
296+
export async function loadShadowOverride(env: StorageEnv, project: string, nowIso?: string): Promise<ShadowOverride | null> {
293297
const row = await loadShadowOverrideRow(env, project);
294298
if (!row) return null;
295-
const override = rowToOverride(row);
299+
// #10291: thread nowIso (mirroring loadOverride) so a shadow row whose clear_at has already lapsed is read
300+
// back as cleared, not still-active — rowToOverride applies the same clearAtIsExpired rule the live read uses.
301+
const override = rowToOverride(row, nowIso);
296302
return override ? { override, validatedUntil: row.validated_until } : null;
297303
}
298304

test/unit/auto-apply.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,25 @@ describe("writeShadowOverride / loadShadowOverride / deleteShadowOverride", () =
612612
await writeShadowOverride(env, "g", { confidenceFloor: 0.95 }, "2026-06-25T00:00:00Z");
613613
expect(tables.shadow.get("g")?.clear_at).toBe("2099-01-01T00:00:00Z");
614614
});
615+
// #10291: the shadow pair only ported the "preserve the column" half of #stale-clear-at-fix, not the
616+
// "drop it once expired" half — mirror the live-side "does NOT resurrect an ALREADY-EXPIRED override" test.
617+
it("#10291: writeShadowOverride DROPS an already-expired clear_at (and does not resurrect the expired floor) when nowIso is passed", async () => {
618+
const { env, tables } = fakeEnv();
619+
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" });
620+
await writeShadowOverride(env, "g", { scopeCap: { files: 3, lines: 100 } }, "2026-06-25T00:00:00Z", "2026-06-20T00:00:00Z");
621+
const row = tables.shadow.get("g");
622+
expect(row?.clear_at).toBeNull(); // the lapsed clear_at is dropped, not carried forward
623+
expect(row?.confidence_floor).toBeNull(); // the expired floor is not resurrected into the merge
624+
expect(row?.scope_cap_files).toBe(3); // the new write still applies normally
625+
});
626+
it("#10291: loadShadowOverride reads an already-expired clear_at row as cleared when nowIso is after it", async () => {
627+
const { env, tables } = fakeEnv();
628+
// Only a confidence_floor gated by an expired clear_at: rowToOverride drops it → the override is empty → null.
629+
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" });
630+
expect(await loadShadowOverride(env, "g", "2026-06-20T00:00:00Z")).toBeNull();
631+
// Without nowIso (the pre-#10291 caller convention) the row is still read active — additive, non-breaking.
632+
expect(await loadShadowOverride(env, "g")).not.toBeNull();
633+
});
615634
it("loadShadowOverride returns null when the row maps to an EMPTY override (rowToOverride → null arm)", async () => {
616635
const { env, tables } = fakeEnv();
617636
tables.shadow.set("g", { confidence_floor: null, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-25T00:00:00Z" });

0 commit comments

Comments
 (0)