fix: batch-2 bug sweep — scheduler DST wedge, stale 429 masking, export route alias, rune-safe redaction, cutoff parity

cmd/odek/bug_sweep_b2_cutoff_test.go

@@ -0,0 +1,29 @@
+package main
+
+// Bug-sweep batch 2 — B7 cutoff-parity regression test.
+//
+// The real sweep computes day-based cutoffs with duration arithmetic
+// (maintenance daysAgo: now - N*24h), explicitly to avoid DST-sensitive
+// calendar arithmetic. The dry-run preview used time.AddDate, so after a
+// DST transition the previewed deletion set diverged from the sweep's by
+// up to an hour of files. The helper is now exported and shared, making
+// the divergence impossible by construction.
+//
+// RED observation: this file failed to compile before the fix —
+// maintenance.DaysAgo did not exist (capability-absent RED, same class as
+// the dry-run artifacts test in batch 1).
+
+import (
+	"testing"
+	"time"
+
+	"github.com/BackendStack21/odek/internal/maintenance"
+)
+
+func TestDaysAgo_MatchesSweepDurationMath(t *testing.T) {
+	now := time.Date(2026, 9, 1, 6, 0, 0, 0, time.UTC)
+	want := now.Add(-3 * 24 * time.Hour)
+	if got := maintenance.DaysAgo(now, 3); !got.Equal(want) {
+		t.Fatalf("DaysAgo(now, 3) = %v, want %v (pure duration arithmetic)", got, want)
+	}
+}

cmd/odek/bug_sweep_b2_export_test.go

@@ -0,0 +1,60 @@
+package main
+
+// Bug-sweep batch 2 — /export route alias regression test.
+//
+// RED-first: handleSessionByID stripped the /export suffix for ALL methods
+// while only GET dispatches to the export handler. DELETE /api/sessions/{id}/export
+// therefore deleted the session and POST .../export renamed it — destructive
+// aliases through a documented read-only route. (The sibling /plan guard is
+// GET-only for exactly this reason.)
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/BackendStack21/odek/internal/llm"
+)
+
+func TestSessionExportSuffix_NotAliasedForMutatingMethods(t *testing.T) {
+	store := newTestSessionStore(t)
+
+	sess, err := store.Create([]llm.Message{
+		{Role: "user", Content: "hello"},
+	}, "test-model", "greeting task")
+	if err != nil {
+		t.Fatalf("Create session: %v", err)
+	}
+
+	handler := handleSessionByID(store, nil, "")
+
+	// DELETE through the export URL must NOT delete the session.
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodDelete, "/api/sessions/"+sess.ID+"/export", nil)
+	req.Header.Set("X-Session-Token", sess.AuthToken)
+	handler(w, req)
+	if w.Code == http.StatusNoContent {
+		t.Fatalf("DELETE /export fell through to base-session delete (status 204) — destructive alias")
+	}
+	if _, err := store.Load(sess.ID); err != nil {
+		t.Fatalf("session %s was deleted through the /export URL alias: %v", sess.ID, err)
+	}
+
+	// POST through the export URL must NOT rename the session.
+	body := strings.NewReader(`{"name":"renamed-via-export"}`)
+	w2 := httptest.NewRecorder()
+	req2 := httptest.NewRequest(http.MethodPost, "/api/sessions/"+sess.ID+"/export", body)
+	req2.Header.Set("X-Session-Token", sess.AuthToken)
+	handler(w2, req2)
+	if w2.Code == http.StatusOK {
+		t.Fatalf("POST /export fell through to session rename (status 200) — destructive alias")
+	}
+	after, err := store.Load(sess.ID)
+	if err != nil {
+		t.Fatalf("session %s missing after POST /export: %v", sess.ID, err)
+	}
+	if after.Task == "renamed-via-export" {
+		t.Fatalf("session renamed through the /export URL alias")
+	}
+}

cmd/odek/bug_sweep_b2_redact_test.go

@@ -0,0 +1,33 @@
+package main
+
+// Bug-sweep batch 2 (fix/bug-hunt-b2) — B6 regression test.
+//
+// RED-first: redactGoal truncated by BYTES (goal[:2048]) while the constant
+// is named GoalChars — a multi-byte rune at the boundary was split,
+// corrupting the goal text with an invalid UTF-8 sequence exactly when the
+// clamp engaged (long goals are the normal case for real tasks).
+
+import (
+	"strings"
+	"testing"
+	"unicode/utf8"
+)
+
+func TestRedactGoal_TruncationIsRuneSafe(t *testing.T) {
+	// 2041 ASCII runes followed by multi-byte runes straddling byte 2048:
+	// each 🚀 is 4 bytes, so byte-index 2048 lands INSIDE the second
+	// rocket (bytes 2045-2048), splitting it mid-rune.
+	goal := strings.Repeat("a", 2041) + strings.Repeat("🚀", 100)
+
+	got := redactGoal(goal)
+
+	if !utf8.ValidString(got) {
+		t.Fatalf("redactGoal produced invalid UTF-8 (byte-sliced a multi-byte rune)")
+	}
+	if n := utf8.RuneCountInString(got); n > maxSubagentRegistryGoalChars {
+		t.Fatalf("redactGoal returned %d runes, want <= %d", n, maxSubagentRegistryGoalChars)
+	}
+	if !strings.HasPrefix(got, strings.Repeat("a", 2040)) {
+		t.Fatalf("redactGoal mangled the ASCII prefix")
+	}
+}

cmd/odek/cleanup.go

@@ -135,14 +135,14 @@ func collectCleanupCandidates(home string, cfg maintenance.Config) cleanupCandid
 	var c cleanupCandidates
 
 	if cfg.SessionsMaxAgeDays > 0 {
-		c.sessions = sessionCandidates(home, now.AddDate(0, 0, -cfg.SessionsMaxAgeDays))
+		c.sessions = sessionCandidates(home, maintenance.DaysAgo(now, cfg.SessionsMaxAgeDays))
 	}
 	if cfg.AuditMaxAgeDays > 0 {
-		c.audit = filesOlderThan(filepath.Join(home, "sessions", "audit"), now.AddDate(0, 0, -cfg.AuditMaxAgeDays), false)
+		c.audit = filesOlderThan(filepath.Join(home, "sessions", "audit"), maintenance.DaysAgo(now, cfg.AuditMaxAgeDays), false)
 	}
 	if cfg.PlansMaxAgeDays > 0 {
 		// Plans may be nested per chat (plans/chat<id>/), so walk recursively.
-		c.plans = filesOlderThan(filepath.Join(home, "plans"), now.AddDate(0, 0, -cfg.PlansMaxAgeDays), true)
+		c.plans = filesOlderThan(filepath.Join(home, "plans"), maintenance.DaysAgo(now, cfg.PlansMaxAgeDays), true)
 	}
 	if cfg.ArtifactsMaxAgeHours > 0 {
 		// Duration-based cutoff, mirroring sweepArtifacts exactly.

cmd/odek/serve.go

@@ -2411,8 +2411,13 @@ func handleSessionByID(store *session.Store, trustedProxies []string, wsToken st
 		id := strings.TrimPrefix(r.URL.Path, "/api/sessions/")
 		// /api/sessions/{id}/export — transcript download (md|json). Shares
 		// the GET auth path below (rate limit + session token).
+		// /export is a GET-only surface: the suffix is stripped for GET
+		// requests only, mirroring the /plan guard below. Stripping it for
+		// every method let DELETE /api/sessions/{id}/export fall through to
+		// the base-session delete (destroying the session through a
+		// read-only route) and POST .../export rename it.
 		exportFormat := ""
-		if strings.HasSuffix(id, "/export") {
+		if r.Method == http.MethodGet && strings.HasSuffix(id, "/export") {
 			id = strings.TrimSuffix(id, "/export")
 			exportFormat = r.URL.Query().Get("format")
 		}

cmd/odek/subagent_registry.go

@@ -358,8 +358,12 @@ func newSubagentTelemetryRelay(send func(v any) error, runKey string) func(taskI
 // but model-controlled text — same treatment as the log relay).
 func redactGoal(goal string) string {
 	goal = redact.RedactSecrets(goal)
-	if len(goal) > maxSubagentRegistryGoalChars {
-		goal = goal[:maxSubagentRegistryGoalChars]
+	// Rune-safe truncation: the constant promises chars, and a byte slice
+	// at the boundary split multi-byte runes, corrupting the goal text with
+	// invalid UTF-8 exactly when the clamp engaged (long goals are the
+	// normal case for real tasks).
+	if r := []rune(goal); len(r) > maxSubagentRegistryGoalChars {
+		goal = string(r[:maxSubagentRegistryGoalChars])
 	}
 	return goal
 }

internal/llm/client.go

@@ -618,6 +618,12 @@ func (c *Client) postChatWithRetry(ctx context.Context, reqBytes []byte) ([]byte
 		// A 200 with an unparseable body or zero choices is often a transient
 		// gateway/proxy artifact during an incident — retry it through the
 		// same budget instead of aborting the turn on the first bad body.
+		// A 200 response resets the stale-status window: lastStatus/
+		// lastBody classify the error of the FINAL attempt, and a 429
+		// earlier in the loop must not mask a malformed-200 exhaustion —
+		// serve reads RateLimitError as "provider throttled".
+		lastStatus = http.StatusOK
+		lastBody = ""
 		if err := validateCompletionBody(respBytes); err != nil {
 			lastErr = err
 			continue

internal/llm/client_stale_retry_test.go

@@ -0,0 +1,45 @@
+package llm
+
+// Bug-sweep batch 2 — stale retry state regression test.
+//
+// RED-first: lastStatus/lastBody were set on non-200 responses and never
+// cleared, so a 429 early in the retry loop masked the REAL final failure:
+// a later malformed-200 exhaustion was wrapped in RateLimitError — the
+// exact type the serve turn handler reads as "provider throttled".
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestClient_Call_Stale429DoesNotMaskMalformed200(t *testing.T) {
+	stubRetrySleep(t) // full retry budget without real backoff sleeps
+
+	n := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		n++
+		if n == 1 {
+			w.WriteHeader(http.StatusTooManyRequests)
+			w.Write([]byte(`{"error":"rate limited"}`))
+			return
+		}
+		// 200 with a body that fails completion-body validation: the final
+		// failure is NOT a rate limit.
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"unexpected":true}`))
+	}))
+	defer server.Close()
+
+	c := New(server.URL, "sk-test", "test-model", "", 0, 0)
+	_, err := c.Call(context.Background(), []Message{{Role: "user", Content: "hi"}}, nil, nil)
+	if err == nil {
+		t.Fatal("expected an error (malformed completion body)")
+	}
+	var rle *RateLimitError
+	if errors.As(err, &rle) {
+		t.Fatalf("final malformed-200 exhaustion misreported as RateLimitError (stale 429 state): %v", err)
+	}
+}

internal/llm/stream.go

@@ -246,6 +246,10 @@ func (c *Client) postChatStream(ctx context.Context, reqBytes []byte, cb func(De
 			return nil, false, lastErr
 		}
 
+		// 200 resets the stale-status window (see buffered Call): a 429
+		// earlier in the retry loop must not mask a streaming failure.
+		lastStatus = http.StatusOK
+		lastBody = ""
 		res, emitted, err := readSSE(ctx, reqCtx, cancelReq, resp.Body, cb)
 		resp.Body.Close()
 		if err != nil {

internal/maintenance/maintenance.go

@@ -216,6 +216,16 @@ func Start(ctx context.Context, home string, cfg Config) {
 	}()
 }
 
+// DaysAgo returns the cutoff time for a day-based retention policy at the
+// given instant: pure duration arithmetic (N*24h), NOT calendar AddDate —
+// the two diverge by an hour across DST transitions. Exported so the cleanup
+// dry-run preview (cmd/odek) computes its candidate cutoffs with the same
+// math as the sweep; preview and deletion set can no longer disagree about
+// what "3 days old" means.
+func DaysAgo(now time.Time, days int) time.Time {
+	return now.Add(-time.Duration(days) * 24 * time.Hour)
+}
+
 // daysAgo returns the cutoff time for a day-based retention policy. Duration
 // arithmetic (instead of AddDate) avoids DST-sensitive behaviour where a
 // "day" isn't always 24 hours.

internal/schedule/cronexpr.go

@@ -293,8 +293,18 @@ func (s *Schedule) Next(after time.Time) time.Time {
 			continue
 		}
 		if s.hour&(1<<uint(t.Hour())) == 0 {
-			// Jump to the top of the next hour.
-			t = time.Date(t.Year(), t.Month(), t.Day(), t.Hour(), 0, 0, 0, s.loc).Add(time.Hour)
+			// Jump to the top of the next hour. time.Date resolves an
+			// ambiguous wall time to its FIRST occurrence, so across a DST
+			// fall-back this +1h hop can land back on the repeated wall
+			// hour and stop advancing entirely — an infinite loop that
+			// wedges the scheduler. When the jump made no progress, fall
+			// back to plain duration arithmetic, which crosses the
+			// transition by construction.
+			next := time.Date(t.Year(), t.Month(), t.Day(), t.Hour(), 0, 0, 0, s.loc).Add(time.Hour)
+			if !next.After(t) {
+				next = t.Add(time.Hour)
+			}
+			t = next
 			continue
 		}
 		if s.minute&(1<<uint(t.Minute())) == 0 {

internal/schedule/cronexpr_dst_test.go

@@ -0,0 +1,55 @@
+package schedule
+
+// Bug-sweep batch 2 — DST fall-back regression test.
+//
+// RED-first: Next() jumped hours via time.Date(...).Add(time.Hour). Across
+// a DST fall-back, time.Date resolves an ambiguous wall time to its FIRST
+// occurrence, so when the repeated wall hour is not in the hour mask the
+// hop can stop advancing entirely — an infinite loop that wedges the
+// scheduler (and `schedule add/list/next`, which validate through Next).
+
+import (
+	"testing"
+	"time"
+
+	// Hermetic zone database: the test must not depend on the host's
+	// tzdata installation.
+	_ "time/tzdata"
+)
+
+func TestNext_DstFallBackRepeatedHourNotInMask(t *testing.T) {
+	loc, err := time.LoadLocation("America/New_York")
+	if err != nil {
+		t.Skipf("tzdata unavailable: %v", err)
+	}
+	s, err := ParseInLocation("0 3 * * *", loc)
+	if err != nil {
+		t.Fatalf("ParseInLocation: %v", err)
+	}
+
+	// 2026-11-01: 02:00 EDT falls back to 01:00 EST — wall hour 01 occurs
+	// twice. Starting before the transition with hour 01 ∉ {03}: the scan
+	// must cross the repeated hour and land on 03:00 EST.
+	after := time.Date(2026, 11, 1, 0, 30, 0, 0, loc)
+	want := time.Date(2026, 11, 1, 3, 0, 0, 0, loc)
+
+	type result struct {
+		t time.Time
+	}
+	ch := make(chan result, 1)
+	go func() { ch <- result{s.Next(after)} }()
+
+	select {
+	case r := <-ch:
+		if r.t.IsZero() {
+			t.Fatal("Next returned zero time (no match within horizon)")
+		}
+		if !r.t.Equal(want) {
+			t.Fatalf("Next = %v, want %v (first firing strictly after %v)", r.t, want, after)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("Next did not return within 5s — infinite loop across DST fall-back " +
+			"(time.Date pins the ambiguous wall time to its first occurrence, so the " +
+			"+1h hour-jump stops advancing)")
+	}
+}