fix(ui+http): timezone validator + system-configs 404 noise

[vanducng]

Summary

Two small quick-fixes that surfaced together on a fork dashboard but apply identically to upstream/dev (both files unchanged from upstream).

1. Timezone save rejected with "Invalid timezone"

Saving a cron job with `Asia/Ho_Chi_Minh` (or any IANA name picked from the dropdown) failed validator and showed an "Invalid timezone" toast.

Root cause: `isValidIanaTimezone` (in `ui/web/src/lib/timezone-utils.ts`) built a `Set` from `Intl.supportedValuesOf("timeZone")` then canonicalized each name via `new Intl.DateTimeFormat(...).resolvedOptions().timeZone`. Across browser/OS combos this drops or remaps some names — e.g. on Chrome 110+ `Asia/Saigon` canonicalizes to `Asia/Ho_Chi_Minh` but on older Safari/Firefox it stays `Asia/Saigon`. When the user's stored value happens to be the absent form, save fails despite the value being a valid IANA name.

Fix: validate via direct `Intl.DateTimeFormat(...)` try/catch. That's the canonical IANA validation path — accepts every name the browser's tz database understands, deprecated aliases included.

2. `/v1/system-configs/{key}` 404 console spam

`background-error-banner.tsx` polls `/v1/system-configs/alert.background.provider_error` to recover alert state after page refresh. When no alert exists (the common case), backend returned 404. The frontend's `.catch()` handled it semantically — but Chrome still logs "Failed to load resource" for every 404. Devtools console fills with hundreds of these entries.

Fix: `handleGet` returns 200 with empty `value` for missing keys. Settings-store "missing" == "empty" is the natural semantics here; the frontend's existing `if (res.value)` guard already handles both shapes. No behavior change for other callers.

Files changed

• `ui/web/src/lib/timezone-utils.ts` — replace Set-based validator with Intl try/catch
• `internal/http/system_configs.go` — `handleGet` returns 200 with empty value on miss

Test plan

• `go build ./...` + `go vet ./...` green
• `pnpm build` in `ui/web/` green
• Manual: open a cron-job detail → pick `Asia/Ho_Chi_Minh` → Save → no toast
• Manual: open dashboard with no active provider alert → console clean (no `/v1/system-configs/*` 404)

Notes

Tested in production on a downstream fork (dataplanelabs/goclaw v3.23.11) since 2026-05-25. Both fixes resolved the reported user-visible issues; no regressions observed across the trace/cron/alert paths.

[mrgoonie]

Backlog review: merge-candidate. The timezone validator change uses Intl directly, which handles canonical/deprecated IANA aliases better than the Set-based check, and the system-config missing-key behavior is intentionally fail-soft. go and web checks are passing.

[mrgoonie]

review-pr --fix --reply result

Verdict: Request changes until the prepared fix can be applied to the PR branch.

Summary: The timezone validator change is sound. The system-configs 404 noise fix needs one guard so missing config keys are treated as empty without masking real store failures.

Iterations: 1 review loop, 1 local fix attempt.
Important finding fixed locally: SystemConfigsHandler.handleGet treats every store.Get error as a missing key and returns 200 {"value":""}. That hides database, tenant-context, or store failures from callers. Expected behavior: missing key can be empty 200 for the alert poller, but non-not-found store errors should still return 500.

Prepared local commit: a1c20d65 fix: preserve system config store errors
Prepared fix scope: added store.ErrSystemConfigNotFound, returned it from PG/SQLite SystemConfigStore.Get no-row paths, made the HTTP handler fail-soft only for that sentinel, and added handler tests for missing vs store-failure responses.
Merge/conflict state: GitHub reports CLEAN.
CI state: Existing PR checks are green (go, web), but no new checks ran for the prepared fix because push was blocked.

Local verification run after the fix:

• go test ./internal/http ./internal/store/pg
• go test -tags sqliteonly ./internal/store/sqlitestore
• go build ./...
• go vet ./...
• go build -tags sqliteonly ./...
• pnpm install --frozen-lockfile in ui/web/
• pnpm build in ui/web/

External blocker: GitHub rejected pushing to dataplanelabs/goclaw:upstream-fix/timezone-validator-sysconfig-404 as mrgoonie (Permission denied). Branch owner or maintainer needs to apply the prepared store-error guard, or grant push access.

Unresolved blockers/questions: fork push permission blocked applying the prepared fix to the PR head.

[mrgoonie]

Summary: This is a useful pair of small fixes, but the system-config GET change currently swallows every store error and returns an empty successful config response.

Risk level: Medium

Mandatory gates:

• Duplicate/prior implementation: clear — repo search for timezone/system-configs found only this PR.
• Project standards: issue found — HTTP handler must not hide backend/tenant errors as successful API responses.
• Strategic necessity: clear value — the timezone validation fix and alert 404-noise reduction both address real UX friction.
• CI/checks: green (go, web)

Findings:

• Critical: none
• Important: internal/http/system_configs.go now treats any h.store.Get error as a missing value and returns 200 {"value":""}. The store can return non-not-found failures too, including tenant-context failures (system config get: tenant_id required) and database/query errors. Those should remain errors; otherwise the API silently masks real backend/auth/tenant bugs as an empty config. Please distinguish missing-key/no-rows from operational errors (for example by making the store expose a typed not-found error or by adding a specific GetOptional path for alert-state reads) and only return empty value for the intended missing-alert case.
• Suggestion: consider scoping the fail-soft behavior to alert.background.provider_error or another explicit optional-read path. Changing all /v1/system-configs/{key} misses from 404 to 200 is an API contract change that may be okay, but it should be intentional and tested.

Verdict: REQUEST_CHANGES