Conversation
…vices services.list reported running=false for services that were actually running (e.g., systemd-journald). The root cause was in the batched "systemctl show" approach used by List(): template units (name@.service) cause systemctl show to fail with no stdout output and no blank-line separator. Since ParseServiceSystemDShow uses blank lines to delimit records, the service before a template and the service after it get merged into one record, silently losing the first service's data. Fix: Replace the batched "systemctl show" calls with a single "systemctl list-units --type service --all" command, which directly reports ActiveState for all loaded services. This uses the already- defined SYSTEMD_LIST_UNITS_REGEX and eliminates the template unit issue entirely. Also hardens ParseServiceSystemDShow with defense-in-depth: if a new Id= is encountered while building a record (missing separator), flush the current record first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
||
| var ( | ||
| SYSTEMD_LIST_UNITS_REGEX = regexp.MustCompile(`(?m)^(?:[^\S\n]{2}|●[^\S\n]|)(\S+)(?:[^\S\n])+(loaded|not-found|masked)(?:[^\S\n])+(\S+)(?:[^\S\n])+(\S+)(?:[^\S\n])+(.+)$`) | ||
| SYSTEMD_LIST_UNITS_REGEX = regexp.MustCompile(`(?m)^(?:[^\S\n]{2}|●[^\S\n]|)(\S+)(?:[^\S\n])+(\S+)(?:[^\S\n])+(\S+)(?:[^\S\n])+(\S+)(?:[^\S\n])+(.+)$`) |
There was a problem hiding this comment.
🔵 suggestion — The regex change from (loaded|not-found|masked) to (\S+) for the LOAD column is intentionally more permissive, which is fine since you only check for "not-found" in ParseSystemdListUnits. However, the old restricted pattern would reject malformed lines early. Consider adding a brief comment noting the LOAD column is intentionally matched broadly (e.g., # LOAD column: loaded, not-found, masked, error, etc.).
Contributor
Test Results5 426 tests +2 5 422 ✅ +2 2m 2s ⏱️ -37s Results for commit 49edba9. ± Comparison against base commit 2116076. This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both. |
syrull
approved these changes
Mar 13, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
services.listreportedrunning=falsefor services that were actually running (e.g.,systemd-journald), whileservice("systemd-journald").runningcorrectly returnedtruesystemctl showfails on template units (name@.service) with no stdout output and no blank-line separator, causing the parser to merge adjacent records and silently lose service datasystemctl showcalls with a singlesystemctl list-units --type service --allcommand, which directly reportsActiveStatefor all loaded servicesParseServiceSystemDShowto handle missing record separators (defense in depth)Test plan
TestParseSystemdListUnits— covers active/inactive/exited/not-found services, bullet prefix, non-service unit filteringTestParseServiceSystemDShowMergedRecords— verifies parser handles missing blank-line separatorsTestSystemDServiceManagerListUsesListUnits— verifies 2 commands issued, merge behavior, template units, unloaded servicesmql run local -c 'services.list.where(name == /journal/) { name running }'should showrunning=trueforsystemd-journald🤖 Generated with Claude Code