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

* fix(schedule): Next() infinite loop across DST fall-back

The hour-jump advanced via time.Date(...).Add(time.Hour). time.Date
resolves an ambiguous wall time to its FIRST occurrence, so across a
DST fall-back transition the hop could land back on the repeated wall
hour and stop advancing entirely when that hour is not in the hour
mask — an infinite loop that wedged the scheduler daemon and every
schedule add/list/next invocation. When the jump makes no progress,
fall back to plain duration arithmetic, which crosses the transition
by construction.

RED-first regression test: TestNext_DstFallBackRepeatedHourNotInMask
(observed hanging 5s before the fix; hermetic via time/tzdata).

* fix(llm): stale 429 state no longer masks the final failure

lastStatus/lastBody were set on non-200 responses and never cleared,
so a 429 early in the retry loop wrapped a LATER different failure in
RateLimitError on exhaustion — the exact type the serve turn handler
reads as 'provider throttled' (dead-prompt handling). A final
malformed-200 (buffered) or streaming failure after an earlier 429 now
reports its real cause. Fixed on both the buffered and streaming paths.

RED-first regression test: TestClient_Call_Stale429DoesNotMaskMalformed200.

* fix(subagents): redactGoal truncation is rune-safe

redactGoal sliced by bytes while the constant promises chars: a
multi-byte rune at the boundary was split, corrupting the goal text
with invalid UTF-8 exactly when the clamp engaged (long goals are the
normal case for real tasks). Now truncates on a rune boundary.

RED-first regression test: TestRedactGoal_TruncationIsRuneSafe.

* fix(serve): /export suffix stripped for GET only

handleSessionByID stripped the /export suffix for ALL methods while
only GET dispatches to the export handler — so DELETE
/api/sessions/{id}/export fell through to the base-session delete
(destroying the session through a documented read-only route) and
POST .../export renamed it. Mirrors the GET-only /plan guard, which
exists for exactly this reason.

RED-first regression test: TestSessionExportSuffix_NotAliasedForMutatingMethods.

* fix(cleanup): dry-run cutoffs share the sweep's DaysAgo math

The sweep computes day-based retention cutoffs with duration
arithmetic (N*24h) to avoid DST-sensitive calendar math; 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 (maintenance.DaysAgo) and shared, making
preview/sweep divergence impossible by construction.

RED observation: the regression test referenced the not-yet-existing
maintenance.DaysAgo (capability-absent compile RED), then passed.

Author: jkyberneees

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.