fix: batch-2 bug sweep — scheduler DST wedge, stale 429 masking, export route alias, rune-safe redaction, cutoff parity - #171
Merged
Conversation
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).
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.
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.
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.
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.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
odek | 7b03a1c | Commit Preview URL Branch Preview URL |
Sep 01 2026, 07:19 AM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch 2 of the bug-hunt sweep (5 bugs, every fix RED-first).
Bugs fixed
🔴
Schedule.Next()infinite loop across DST fall-back (internal/schedule) — the hour-jump advanced viatime.Date(...).Add(time.Hour).time.Dateresolves an ambiguous wall time to its FIRST occurrence, so across a fall-back transition the hop could land back on the repeated wall hour and stop advancing entirely when that hour ∉ the hour mask — an infinite loop wedging the scheduler daemon and everyschedule add/list/next. Falls back to duration arithmetic when the jump makes no progress. Reproduced: the RED test hung 5s pre-fix.Stale 429 state masked the real final failure (internal/llm) —
lastStatus/lastBodywere set on non-200 and never cleared: a 429 early in the retry loop wrapped a later malformed-200/streaming failure inRateLimitError— the exact type the serve turn handler reads as "provider throttled" (dead-prompt handling). Both buffered and streaming paths now reset the window on a 200./api/sessions/{id}/exportroute alias (serve.go) — the suffix was stripped for ALL methods while only GET dispatches to export:DELETE …/exportdeleted the session andPOST …/exportrenamed it — destructive aliases through a read-only route. Mirrors the GET-only/planguard.redactGoalbyte-truncation split UTF-8 runes (wire-v2 registry) — the constant promises chars; a multi-byte rune at the boundary was corrupted with invalid UTF-8 exactly when the clamp engaged. Now rune-safe.Dry-run cutoff math diverged from the sweep — sweep uses duration arithmetic (DST-safe), the preview used
AddDate; up to an hour of files could be deleted that the preview never listed.maintenance.DaysAgois now exported and shared.Verification
go vetclean;golangci-lint0 issues;-raceclean