Skip to content

fix: edge-case bugs + dead-code/IPv6/toolchain cleanup + tests - #6

Merged
aloks98 merged 10 commits into
masterfrom
fix/runtime-config-version
May 30, 2026
Merged

fix: edge-case bugs + dead-code/IPv6/toolchain cleanup + tests#6
aloks98 merged 10 commits into
masterfrom
fix/runtime-config-version

Conversation

@aloks98

@aloks98 aloks98 commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Addresses the findings from the codebase review. Builds on the merged dashboard PR (#5). All Go and frontend tests pass; go vet and svelte-check are clean.

Bug fixes

  1. formatRuntime dropped days (web/src/lib/utils.ts) — intervalToDuration buckets ≥24h into days, previously ignored, so a 25h runtime rendered as 1h. Now rolls days/months/years into the hours count.
  2. LoadConfig swallowed config-read errors (config.go) — an unreadable explicit -config path silently fell back to defaults (incl. a different data dir). Now fatal; a genuinely missing file is still tolerated.
  3. compareVersions poisoned by non-numeric segments (settings) — 1.2.3-beta → NaN made every suffixed tag report "no update". Now parses the leading integer per segment.

Cleanup / consistency

  1. Removed dead Go FormatRuntime — no callers.
  2. Surface status_label on the dashboard — GetStatusLabel's output was computed but never rendered; now shown (humanized) next to the raw NUT code.
  3. Accept IPv6 device addresses — the form's IPv4-only regex rejected IPv6 even though the backend (net.ParseIP) and the online-status probe handle it. Now validates via zod ipv4()/ipv6().
  4. Aligned toolchain — single Go version across go.mod/Dockerfile/CI (1.25, raised from 1.22 — see #11); Node 24 in CI to match Docker; CI reads pinned pnpm from web/package.json instead of latest; Alpine bumped to 3.21.

Robustness / UX

  1. Release-check failure backoff (api.go) — the latest-release cache only stored successes, so a GitHub outage/403 made every poll re-hit GitHub. Now a failed fetch starts a 5m backoff and serves the last good value (stale) meanwhile; 503 fast-fail if no prior value.
  2. UPS poll cadence (dashboard) — backend UPS cache refreshes every 15s but the dashboard polled every 30s (≤45s lag). UPS query now polls at 15s; device status keeps its matching 30s interval.
  3. ICMP liveness with TCP fallback (ping.go) — device status only TCP-connected to 5 ports, so a host with none of them open showed offline. Now tries an ICMP echo first (unprivileged ping socket where allowed, raw socket otherwise) and falls back to the TCP probe when ICMP isn't permitted or unanswered. Adds golang.org/x/net (raises the Go floor to 1.25). systemd already grants CAP_NET_RAW; Docker --cap-add NET_RAW documented (optional — degrades to TCP).
  4. Interface-aware directed broadcast (wol.go) — isLikelyBroadcast only matched x.x.x.255, so a /16 or /23 WoL broadcast target wasn't sent with SO_BROADCAST. Now computes each local network's real broadcast from its netmask, with the /24 heuristic as fallback.

Tests (new)

  1. Go (wol_test.go, nut_test.go, config_test.go, api_test.go) — MAC/WoL packet, broadcastAddr, SSRF/command-injection host & name guards, status-label edge cases, LoadConfig paths (regression for fix(docker): use Node 22.14 + corepack via packageManager field #2), release-cache backoff/stale state machine (#9).
    Frontend (web/src/lib/utils.test.ts, Vitest) — formatRuntime/compareVersions/formatWattage, regression guards for Code review fixes + design system overhaul #1 and ci(release): drop linux/arm/v7 from Docker multi-arch build #3. CI runs pnpm test.

Verification

  • go build ./..., go vet ./..., go test ./... ✓
  • pnpm check (svelte-check) — 0 errors/warnings ✓ · pnpm test — 7/7 ✓
  • ICMP path smoke-tested against loopback (alive) and TEST-NET (offline via ICMP→TCP fallback)

🤖 Generated with Claude Code

@aloks98 @claude
fix: correct runtime formatting, config errors, and version compare
f603cef 
- formatRuntime: roll days/months/years into the hours count so runtimes
  ≥24h no longer drop a full day (25h rendered as "1h").
- LoadConfig: fail on an unreadable explicit -config path instead of
  silently falling back to defaults (which use a different data dir).
- compareVersions: parse the leading integer per segment so a pre-release
  tag (e.g. 1.2.3-beta) no longer poisons the comparison with NaN and
  suppresses the update banner.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98
aloks98 force-pushed the fix/runtime-config-version branch from be4594f to f603cef Compare May 30, 2026 09:03
aloks98 and others added 5 commits May 30, 2026 14:34
@aloks98 @claude
refactor: drop dead FormatRuntime, surface status_label on dashboard
1d72e8a 
- Remove the unused Go FormatRuntime helper (the frontend formats runtime
  itself; the function had no callers).
- Render the humanized status_label on the dashboard status footer instead
  of only the raw NUT code, so GetStatusLabel's output is no longer dead
  data. Raw code is kept (mono) for grepping; the label is skipped when it
  equals the raw code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
fix: accept IPv6 device addresses in the form schema
e48deb6 
The device form rejected IPv6 IPs via an IPv4-only regex, even though the
backend validates with net.ParseIP and the online-status probe handles IPv6
via net.JoinHostPort. Validate against zod's ipv4()/ipv6() so a valid IPv6
address can be saved and edited.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
ci: align Go/Node toolchain across go.mod, Docker, and CI
5f553ef 
Dev, CI, and release binaries were built with different toolchains: go.mod
and CI pinned Go 1.22 while the Dockerfile already moved to Node 24, and CI
built the frontend on Node 22 with a floating pnpm@latest.

- Go 1.24 everywhere (go.mod directive, Dockerfile builder, both workflows).
- Node 24 in CI to match the Docker frontend stage.
- CI installs pnpm from web/package.json's packageManager field instead of
  `latest`, so the pinned version is the single source of truth.
- Bump Alpine base to 3.21 (builder + runtime) for current security patches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
test: add Go unit tests for pure logic
d32a357 
Cover the previously-untested pure functions, including a regression guard
for the LoadConfig error-handling fix (unreadable explicit path now errors;
missing path is tolerated):

- wol: ValidateMAC, NormalizeMAC, ParseMAC, BuildMagicPacket, isLikelyBroadcast
- nut: ValidateUPSHost, ValidateUPSName (SSRF / command-injection guards),
  GetStatusLabel (incl. the DISCHRG-contains-CHRG edge case)
- config: LoadConfig defaults/file/env/error paths, validateAppData ID rewrite

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
test: add Vitest coverage for runtime and version formatting
fced007 
Add Vitest and unit tests for the frontend pure helpers, with regression
guards for the two bugs fixed earlier:

- formatRuntime: ≥24h runtimes no longer drop a day (25h → "25h", not "1h").
- compareVersions: a pre-release suffix compares as its release base instead
  of poisoning the result with NaN.

Extract compareVersions from settings/+page.svelte into utils.ts so it's
importable/testable, and run `pnpm test` in CI.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 aloks98 changed the title fix: runtime formatting, config errors, and version compare fix: edge-case bugs + dead-code/IPv6/toolchain cleanup + tests May 30, 2026
aloks98 and others added 4 commits May 30, 2026 14:53
@aloks98 @claude
fix: back off and serve stale on GitHub release-check failures
5440f24 
The release-latest cache only stored successes, so when GitHub was down or
returned a 403 rate-limit, every poll re-hit GitHub — amplifying the very
rate-limiting the cache exists to prevent. Now a failed fetch starts a 5m
backoff window during which the last good value is served (stale) instead of
contacting GitHub again; with no prior value it fails fast with 503.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
fix: poll UPS status at 15s to match backend cache refresh
a0cde64 
The backend UPS cache refreshes every 15s but the dashboard polled every
30s, letting the displayed state lag reality by up to ~45s. Align the UPS
query's refetchInterval to 15s. (Device status keeps its 30s interval, which
already matches the device cache.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
fix: detect directed broadcast via interface netmask, not just /24
cf26c04 
isLikelyBroadcast only treated x.x.x.255 as a broadcast, so a /16 or /23
directed-broadcast WoL target wasn't sent with SO_BROADCAST. Compute each
local IPv4 network's real broadcast address and match against it; keep the
/24 .255 heuristic as a fallback for targets not on a directly-attached
network. Adds a broadcastAddr unit test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98 @claude
feat: use ICMP echo for device liveness, fall back to TCP probe
690895c 
Device online-status previously only TCP-connected to ports 80/443/22/3389/
445, so a host that's up but exposes none of them showed as offline. Now try
an ICMP echo first (true liveness regardless of open ports) and fall back to
the TCP probe when ICMP isn't permitted or the host doesn't answer ICMP but
may still serve TCP. ICMP uses the unprivileged datagram socket where allowed
(ping_group_range) and a raw socket otherwise.

- Add golang.org/x/net (icmp/ipv4); this raises the Go floor to 1.25, so bump
  go.mod, the Dockerfile builder, and both CI workflows to 1.25.
- The systemd unit already grants CAP_NET_RAW; correct its comment (it's for
  ICMP, not WoL broadcast, which uses SO_BROADCAST).
- Document --cap-add NET_RAW for Docker (run + compose); it's optional and
  degrades to the TCP probe without it.
- README: UPS refresh is now 15s (matches the dashboard poll change).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aloks98
aloks98 merged commit 4f39059 into master May 30, 2026
2 checks passed
@aloks98
aloks98 deleted the fix/runtime-config-version branch May 30, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@aloks98