fix: use systemctl to list units - #1238
Conversation
Replace scanning of unit-file directories with `systemctl list-units`, so units are reported regardless of where their files live. Parsing now lives in `SystemdUnit.parseListUnits`, and the sort comparator is a top-level `_compareUnits` function.
`getUnits` now returns a `SystemdRefreshResult` so the page can toast when system units fail to load or when the user manager is unavailable, instead of silently showing an incomplete list. The initial load moves to the view so first-load failures are surfaced too.
Covers normal parsing, multi-word descriptions, scope tagging, skipped unit types/states, empty/error output, and lines with too few columns.
Detect system-scope listing failures the same way as user scope (non-empty output that parses to no units), so hosts without a working systemctl get an error toast rather than a silently empty page. The shared heuristic is extracted into `_listFailed`.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds scoped Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/view/page/systemd.dart (1)
36-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
_refreshinto an Actions extension per repo convention.The new
_refreshmethod is defined directly on the State class. As per coding guidelines,lib/view/**/*.dartfiles should "Split UI into Widget build, Actions, Utils usingextension onto achieve this pattern." Consider moving_refreshinto a dedicatedextension _SystemdPageActions on _SystemdPageState { ... }block for consistency with the rest of the codebase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/view/page/systemd.dart` around lines 36 - 47, Move the _refresh method out of _SystemdPageState and into a dedicated extension named _SystemdPageActions on _SystemdPageState, preserving its existing refresh, mounted-check, and result-handling behavior.Source: Path instructions
lib/data/provider/systemd.dart (1)
65-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared per-scope fetch helper.
System-scope fetch/parse (Lines 74-80) is handled inline in the outer
try, while user-scope fetch/parse (Lines 84-92) has its own nestedtry/catchwith a distinct log label. The outer catch'sdprint('Parse systemd', ...)(Line 100) is also reachable by system-sideexecForOutputfailures, not just parse failures, which is a bit misleading for debugging. A small shared helper (e.g. returning(List<SystemdUnit>, bool failed)per scope) would unify error handling/logging for both scopes and remove the asymmetry.♻️ Proposed refactor
+ Future<(List<SystemdUnit>, bool)> _fetchScope( + SshClient client, + SystemdUnitScope scope, + ) async { + try { + final raw = await client.execForOutput(scope.listUnitsCmd); + final units = SystemdUnit.parseListUnits(raw, scope); + return (units, _listFailed(raw, units)); + } catch (e, s) { + dprint('Systemd ${scope.name} units', e, s); + return (<SystemdUnit>[], true); + } + } + Future<SystemdRefreshResult> getUnits() async { state = state.copyWith(isBusy: true); try { final client = _si.client; if (client == null) return SystemdRefreshResult.systemFailed; - final systemRaw = - await client.execForOutput(SystemdUnitScope.system.listUnitsCmd); - final systemUnits = - SystemdUnit.parseListUnits(systemRaw, SystemdUnitScope.system); - if (_listFailed(systemRaw, systemUnits)) { - return SystemdRefreshResult.systemFailed; - } - - var userUnits = <SystemdUnit>[]; - var userFailed = false; - try { - final userRaw = - await client.execForOutput(SystemdUnitScope.user.listUnitsCmd); - userUnits = SystemdUnit.parseListUnits(userRaw, SystemdUnitScope.user); - userFailed = _listFailed(userRaw, userUnits); - } catch (e, s) { - dprint('Systemd user units', e, s); - userFailed = true; - } + final (systemUnits, systemFailed) = + await _fetchScope(client, SystemdUnitScope.system); + if (systemFailed) return SystemdRefreshResult.systemFailed; + + final (userUnits, userFailed) = + await _fetchScope(client, SystemdUnitScope.user); final units = [...userUnits, ...systemUnits]..sort(_compareUnits); state = state.copyWith(units: units); return userFailed ? SystemdRefreshResult.userFailed : SystemdRefreshResult.ok;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/data/provider/systemd.dart` around lines 65 - 106, Extract the per-scope fetch, parse, failure detection, and error logging from getUnits into a shared helper that accepts a SystemdUnitScope and returns the parsed units plus a failure flag. Use the helper for both system and user scopes, preserving system failure as an immediate result while allowing user failure to return partial results, and replace the misleading outer parse-specific catch with scope-appropriate logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/data/provider/systemd.dart`:
- Around line 65-106: Extract the per-scope fetch, parse, failure detection, and
error logging from getUnits into a shared helper that accepts a SystemdUnitScope
and returns the parsed units plus a failure flag. Use the helper for both system
and user scopes, preserving system failure as an immediate result while allowing
user failure to return partial results, and replace the misleading outer
parse-specific catch with scope-appropriate logging.
In `@lib/view/page/systemd.dart`:
- Around line 36-47: Move the _refresh method out of _SystemdPageState and into
a dedicated extension named _SystemdPageActions on _SystemdPageState, preserving
its existing refresh, mounted-check, and result-handling behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: becfb148-8ac8-429e-b526-b19d29e7c875
📒 Files selected for processing (4)
lib/data/model/server/systemd.dartlib/data/provider/systemd.dartlib/view/page/systemd.darttest/systemd_test.dart
3 participants
On NixOS the systemd tab is empty, as listing unit files is done through
find -type f ..., but on NixOS these are symlinks to the nix store. Usingsystemctlavoids any discrepancies and ensures future systemd changes are followed. A specific error for missing user units or no systemd connection is also added.Summary by CodeRabbit
New Features
systemctl list-unitsoutput, including unit type, scope, state, and multi-word descriptions.Bug Fixes
Tests