Skip to content

Fix fan recovery, sampling races, network counters, and UI state - #96

Merged
gitar-bot[bot] merged 4 commits into
offyotto:mainfrom
OGSmokeStudios:fix/remaining-runtime-issues
Sep 19, 2026
Merged

gitar-bot[bot] merged 4 commits into
offyotto:mainfrom
OGSmokeStudios:fix/remaining-runtime-issues

Conversation

@OGSmokeStudios

Copy link
Copy Markdown
Contributor

Change

Fix ten fan-control, monitoring, and UI state findings from #94.

  • Supervise manual fan targets with per-client leases in the privileged helper. A persistent, authenticated XPC session owns each fan; a two-second heartbeat renews its 15-second lease. Disconnects expire ownership immediately, and the helper retries failed automatic restores. Requests and diagnostics now await asynchronous replies rather than blocking the UI thread. Timeouts invalidate the session, including writes whose reply never arrived.
  • Reject fan IDs outside the existing single-decimal-character SMC key format and validate reported fan counts before conversion, so IDs 10 and 11 cannot produce malformed five-character keys.
  • Keep CPU/network fallback state on the sampling queue. Reject late results after sampling stops or restarts, and preserve the current process-privacy state when publishing a monitor snapshot. Use 64-bit per-interface network counters with independent baselines for added/reset interfaces.
  • Reuse the dashboard controller to preserve its frame, honor the selected locale in battery/rescue formatting, expire weather cache fallback after the refresh interval, and reuse one desktop notification identifier per alert rule while retaining cooldown and history behavior.

Addresses #94 items 1, 4, 8, 11, 16, 17, 21, 25, 26 (stale weather), and 27. Other #94 findings and hardware-validation issues #49–#52 remain open.

Verification

  • Final macOS build, all 121 tests, and the optimized helper lifetime check pass in fork CI.
  • Regression coverage includes real asynchronous XPC through an anonymous test listener; timeout, disconnect and late-reply handling; simulated lease expiry/renewal/restore failures and separate owners; stop/restart sampling; 64-bit interface decoding and counter resets; locale formatting; weather expiry; and the original fan, disk-cache and Settings fixes.
  • All added/affected tests are registered in the Xcode target. The validation branch has identical app, helper, and test sources; its only extra changes enable the fork CI trigger and print the test summary.
  • git diff --check passes. The battery precision fixture avoids an exact rounding midpoint and specifies its locale, so it tests precision without assuming a different native rounding rule or the tester's language setting.

Compatibility and validation limits

  • Update the app and reinstall the bundled helper together. The app checks the helper's safety protocol before manual writes. Standalone CLI set is no longer supported because an exiting process cannot supervise an override; read and auto remain available.

  • No physical Apple Silicon fan, thermal, signing/install, sleep/wake, or manual visual validation was performed. Lease tests simulate hardware callbacks; the watchdog requires a running helper and responsive SMC calls and does not claim recovery from helper/system crashes. See runtime behavior and validation.

  • New battery phrases include English, Swedish, German, French, and Spanish; existing catalog translations are reused where available, and other missing translations fall back to English.

  • Tests cover the changed behavior.

  • User-facing changes are documented.

  • Security-sensitive changes were reviewed for privilege, XPC, signing, and SMC impact; existing client authorization remains enforced, and hardware-dependent behavior is explicitly unverified.

Comment thread Core-Monitor/BatteryDetailFormatter.swift
@gitar-bot

gitar-bot Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Code Review ✅ Approved 1 closed / 1 findings

🔴 High risk

Comprehensive fix for ten fan-control, monitoring, and UI state issues, including XPC lease supervision, fan ID validation, sampling race conditions, network counter handling, and locale-aware UI state. All 121 tests pass with strong regression coverage.

Consider adding "Charging" and "AC Power" entries to BatteryDetails.xcstrings to complete localization of power states across all supported languages, since these labels currently fall back to English in Swedish, German, French, and Spanish.

Auto-approved: No blocking issues found.
Please see Auto-approve Docs for details on setting custom approval criteria.

✅ 1 closed
Closed: Quality: Charging/AC Power battery states not localized

📄 Core-Monitor/BatteryDetailFormatter.swift:5-11 📄 Core-Monitor/BatteryDetails.xcstrings:1-15
powerStateDescription now routes "Charging", "AC Power", and "Battery Power" through localized(...), and it is displayed in PowerPage with the selected locale. However the new BatteryDetails.xcstrings catalog only contains "Battery Power" (plus the source/runtime phrases); it has no entry for "Charging" or "AC Power", and there is no other Localizable strings table in the project. As a result, in sv/de/fr/es a charging or plugged-in battery still shows English "Charging"/"AC Power" while an on-battery state is translated — an inconsistent, partially-localized power label that contradicts the PR's stated goal of following the selected locale.

Review coverage

Auto-approval Approved

Rules No rules evaluated

Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@gitar-bot
gitar-bot Bot enabled auto-merge (squash) September 19, 2026 09:39

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

@gitar-bot gitar-bot Bot added the gitar-approved Added by Gitar label Sep 19, 2026
@gitar-bot
gitar-bot Bot merged commit 42d11dc into offyotto:main Sep 19, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gitar-approved Added by Gitar

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants