Skip to content

fix(review): give shadow overrides the same expired-clear_at handling as live overrides - #10325

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/shadow-override-expired-clearat-10291
Aug 4, 2026
Merged

fix(review): give shadow overrides the same expired-clear_at handling as live overrides#10325
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/shadow-override-expired-clearat-10291

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

src/review/auto-apply.ts keeps two parallel tunable-override tables — LIVE (tunables_overrides) and SHADOW (tunables_overrides_shadow, a soak-gated staging area promoted to live once validated). Both carry an operator-settable clear_at (a temporary override's own expiration).

The LIVE pair correctly treats an already-lapsed clear_at as cleared:

export async function loadOverride(env, project, nowIso?) { return rowToOverride(await loadOverrideRow(env, project), nowIso); }
export async function writeLiveOverride(env, project, o, nowIso?) {
  const clearAt = existingRow && !clearAtIsExpired(existingRow.clear_at, nowIso) ? existingRow.clear_at : null; // dropped when expired
}

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 parameter and computed clearAt as existingRow?.clear_at ?? null — re-persisting a lapsed clear_at unconditionally, and calling rowToOverride(existingRow) with no nowIso so clearAtIsExpired always short-circuited to false during the merge.
  • loadShadowOverride had no nowIso parameter and called rowToOverride(row) with no second argument, so a shadow row with a lapsed clear_at was read back as still active.

Impact: loadShadowOverride feeds runAutoApplyRecommendations's promotion step and the gate-config/effective / live-gate-thresholds API + MCP routes. A shadow override whose clear_at has already lapsed was treated as active indefinitely, and a stale shadow tightening could be promoted to LIVE after its own operator-set expiration had passed — silently defeating the temporary-override-expiration semantics the live side already honours.

The fix

Thread an optional nowIso through both, mirroring the live pair exactly:

  • loadShadowOverride(env, project, nowIso?) passes nowIso into rowToOverride, so a lapsed clear_at reads as cleared.
  • writeShadowOverride(env, project, o, validatedUntilIso, nowIso?) computes clearAt via existingRow && !clearAtIsExpired(existingRow.clear_at, nowIso) ? existingRow.clear_at : null and passes nowIso into its own rowToOverride(existingRow, nowIso) merge read.

Both parameters are optional and additive, so every existing caller compiles and behaves exactly as before when nowIso is omitted (matching the live side's convention).

Unchanged: the live loadOverride/writeLiveOverride pair, the promotion/soak-gate decision logic, and every other behaviour. No migration (additive optional params only).

Tests (test/unit/auto-apply.test.ts)

  • writeShadowOverride drops an already-expired clear_at (and does not resurrect the expired floor) when nowIso is after it — mirroring the existing live-side "does NOT resurrect an ALREADY-EXPIRED override" test.
  • loadShadowOverride reads an expired-clear_at row as cleared (null) when nowIso is after it, and still reads it active when nowIso is omitted (the non-breaking, additive contract).
  • The existing "PRESERVES an existing clear_at across a shadow write" test (a FUTURE clear_at) still passes unchanged.
  • Both new assertions fail on main.

Validation

  • Diff coverage on src/review/auto-apply.ts is 100% line and branch (all three arms of the new existingRow && !clearAtIsExpired(...) guard).
  • npm run typecheck clean for these files; npm run engine-parity:drift-check passes (not a twin); npm run dead-exports:check clean; the suite (100 tests) green.
  • git diff --check clean; no schema/migration/generated-artifact change.

Closes #10291

… 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 JSONbored#10291
@shin-core
shin-core requested a review from JSONbored as a code owner August 4, 2026 03:02
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit 5b95330 into JSONbored:main Aug 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

review(auto-apply): give shadow overrides the same expired-clear_at handling as live overrides

2 participants