From 7464fa700b55bc8fa5508c6c9f5bda7248ef29d0 Mon Sep 17 00:00:00 2001
From: Minseong Choi <me@flyemoji.moe>
Date: Wed, 1 Jul 2026 18:26:42 +0900
Subject: [PATCH] fix(updates): tag Window JSON so the persisted maintenance
 window round-trips

The admin API persists the auto-update maintenance window as lowercase
JSON {"start","end"} (platform_settings key "update_window"), but
updates.Window had no json tags, so it marshaled/unmarshaled with
capitalized keys. The natural decode the update runner will use --
json.Unmarshal(stored, &updates.Window{}) -- would therefore miss every
key and silently yield the zero Window. That fails closed (a zero window
Contains nothing, so notify-only, never a rogue apply), so it is safe but
a latent silent-zero trap for the not-yet-built runner.

Add json:"start"/json:"end" to updates.Window so the obvious decode is
correct by construction; value time.Time treats a stored null as a no-op,
so a cleared/never-set window still decodes to the zero Window. Nothing
in the package serialized Window before, so this changes no existing
behavior.

Guarded by a cross-package contract test in internal/api that marshals
the real api.updateWindow DTO and unmarshals it into updates.Window --
asserting the interval survives (Contains(mid) is true) and that an empty
window decodes to the fail-closed zero Window -- so the two shapes cannot
drift apart silently.
---
 internal/api/handlers_updates_shape_test.go | 58 +++++++++++++++++++++
 internal/updates/plan.go                    |  9 +++-
 2 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100644 internal/api/handlers_updates_shape_test.go

diff --git a/internal/api/handlers_updates_shape_test.go b/internal/api/handlers_updates_shape_test.go
new file mode 100644
index 0000000..b1d3bb7
--- /dev/null
+++ b/internal/api/handlers_updates_shape_test.go
@@ -0,0 +1,58 @@
+package api
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"felis.lolicon.best/internal/updates"
+)
+
+// TestUpdateWindowStorageShapeDecodesIntoCoreWindow is a cross-package contract
+// guard. The maintenance window THIS package persists (api.updateWindow, lowercase
+// {"start","end"}) must decode straight into the update decision core's
+// updates.Window — because the (INTEGRATION-ONLY) `felis update` runner reads the
+// stored bytes back into a Window. The test marshals the REAL api DTO rather than a
+// hand-written JSON literal (which would drift silently if either shape changed) and
+// unmarshals into updates.Window, proving the on-disk bytes the admin API writes are
+// exactly what the runner will read back — no silent zero-window from a key-casing
+// mismatch. updates.Window carries json:"start"/json:"end" tags precisely so this
+// holds; without them the natural Unmarshal would zero every field.
+func TestUpdateWindowStorageShapeDecodesIntoCoreWindow(t *testing.T) {
+	start := time.Date(2026, 8, 1, 2, 0, 0, 0, time.UTC)
+	end := time.Date(2026, 8, 1, 4, 0, 0, 0, time.UTC)
+
+	raw, err := json.Marshal(updateWindow{Start: &start, End: &end})
+	if err != nil {
+		t.Fatalf("marshal api window: %v", err)
+	}
+
+	var win updates.Window
+	if err := json.Unmarshal(raw, &win); err != nil {
+		t.Fatalf("unmarshal into updates.Window: %v", err)
+	}
+	if !win.Start.Equal(start) || !win.End.Equal(end) {
+		t.Fatalf("decoded window = {%s,%s}, want {%s,%s}", win.Start, win.End, start, end)
+	}
+	mid := time.Date(2026, 8, 1, 3, 0, 0, 0, time.UTC)
+	if !win.Contains(mid) {
+		t.Fatalf("Contains(%s) = false, want true — the window did not survive the round-trip", mid)
+	}
+
+	// A cleared/never-set window ({null,null}) must decode to the zero Window, which
+	// fails closed (Contains always false) — never a spurious open apply slot.
+	rawEmpty, err := json.Marshal(updateWindow{})
+	if err != nil {
+		t.Fatalf("marshal empty api window: %v", err)
+	}
+	var empty updates.Window
+	if err := json.Unmarshal(rawEmpty, &empty); err != nil {
+		t.Fatalf("unmarshal empty into updates.Window: %v", err)
+	}
+	if !empty.Start.IsZero() || !empty.End.IsZero() {
+		t.Fatalf("empty window decoded to non-zero {%s,%s}", empty.Start, empty.End)
+	}
+	if empty.Contains(mid) {
+		t.Fatal("zero window Contains returned true, want false (must fail closed)")
+	}
+}
diff --git a/internal/updates/plan.go b/internal/updates/plan.go
index 984393c..363de29 100644
--- a/internal/updates/plan.go
+++ b/internal/updates/plan.go
@@ -34,9 +34,14 @@ const (
 // on top. A zero Window (both ends zero) is "unset" and Contains always returns
 // false, so a Scheduled component with no window set can never auto-apply — it holds
 // at notify until a human actually schedules a time.
+// The json tags are load-bearing across a package boundary: the maintenance window
+// is persisted by internal/api (platform_settings key "update_window") as lowercase
+// {"start","end"}, and the update runner reads those bytes back into a Window. With
+// value (not pointer) time.Time, a stored null unmarshals as a no-op, so a cleared
+// or never-set window decodes to the zero Window — Contains false, fails closed.
 type Window struct {
-	Start time.Time
-	End   time.Time
+	Start time.Time `json:"start"`
+	End   time.Time `json:"end"`
 }
 
 // Contains reports whether now is inside the window. An unset (zero) or inverted