fix(systemd): handle missing DBUS env vars gracefully in isSystemdServiceEnabled#7730
Closed
briancolinger wants to merge 1 commit intoopenclaw:mainfrom
Closed
fix(systemd): handle missing DBUS env vars gracefully in isSystemdServiceEnabled#7730briancolinger wants to merge 1 commit intoopenclaw:mainfrom
briancolinger wants to merge 1 commit intoopenclaw:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+329
to
+336
| try { | ||
| await assertSystemdAvailable(); | ||
| } catch { | ||
| // If systemd user services are unavailable (e.g., missing DBUS_SESSION_BUS_ADDRESS | ||
| // or XDG_RUNTIME_DIR in SSH sessions, WSL, or certain terminals), the service | ||
| // cannot be enabled. | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
[P2] Catch-all hides unexpected assertSystemdAvailable failures
isSystemdServiceEnabled now returns false for any error from assertSystemdAvailable(), which includes cases beyond missing DBUS env vars (e.g., permission issues, transient exec errors). That can mask real problems and make debugging harder.
Consider narrowing this to known “systemd user services unavailable” errors (similar to isSystemdUserServiceAvailable()’s failed to connect / not been booted checks) or at least logging/including the error detail somewhere before returning false.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/daemon/systemd.ts
Line: 329:336
Comment:
[P2] Catch-all hides unexpected `assertSystemdAvailable` failures
`isSystemdServiceEnabled` now returns `false` for *any* error from `assertSystemdAvailable()`, which includes cases beyond missing DBUS env vars (e.g., permission issues, transient exec errors). That can mask real problems and make debugging harder.
Consider narrowing this to known “systemd user services unavailable” errors (similar to `isSystemdUserServiceAvailable()`’s `failed to connect` / `not been booted` checks) or at least logging/including the error detail somewhere before returning `false`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+62
to
+70
| execFileMock.mockImplementation((_cmd, args, _opts, cb) => { | ||
| callCount++; | ||
| if (callCount === 1) { | ||
| // First call: systemctl --user status (availability check) | ||
| cb(null, "", ""); | ||
| } else { | ||
| // Second call: systemctl --user is-enabled | ||
| cb(null, "enabled", ""); | ||
| } |
Contributor
There was a problem hiding this comment.
[P3] Unused args parameter in mock callback
In the execFileMock implementations, args is declared but never used (e.g. here and again in the “not enabled” test). This can trigger lint warnings and makes it look like the test is asserting on args when it isn’t.
You can rename it to _args (or remove it) to match the other tests’ style.
Also appears in: src/daemon/systemd-availability.test.ts:77-87.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/daemon/systemd-availability.test.ts
Line: 62:70
Comment:
[P3] Unused `args` parameter in mock callback
In the `execFileMock` implementations, `args` is declared but never used (e.g. here and again in the “not enabled” test). This can trigger lint warnings and makes it look like the test is asserting on args when it isn’t.
You can rename it to `_args` (or remove it) to match the other tests’ style.
Also appears in: src/daemon/systemd-availability.test.ts:77-87.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.…viceEnabled When $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR are not set (SSH sessions, WSL, certain terminals), systemctl --user commands fail. The isSystemdUserServiceAvailable function already handled this gracefully, but isSystemdServiceEnabled threw via assertSystemdAvailable. Now isSystemdServiceEnabled catches the error and returns false, consistent with the semantics that if systemd user services are unavailable, no service can be enabled. Fixes #1818
briancolinger
force-pushed
the
fix/ubuntu-systemd-dbus
branch
from
4dbace6 to
4230675
Compare
bob10042
pushed a commit
to bob10042/openclaw
that referenced
this pull request
Feb 14, 2026
- Update all documentation URLs from docs.anthropic.com to docs.claude.com to avoid redirects - Change Getting Started Guide URL from /getting-started to /quickstart path - Remove Discussions link as GitHub Discussions are not enabled for this repository Fixes openclaw#7730
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
18 tasks
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
When
$DBUS_SESSION_BUS_ADDRESSand$XDG_RUNTIME_DIRare not set (SSH sessions, WSL, certain terminals),systemctl --usercommands fail with:The
isSystemdUserServiceAvailablefunction already handled this error gracefully by returningfalse. However,isSystemdServiceEnabledwas callingassertSystemdAvailable()directly, which throws on this error.This caused the onboarding wizard to show error messages like:
Fix
Wrap the
assertSystemdAvailable()call inisSystemdServiceEnabledwith try-catch and returnfalsewhen it throws. This is semantically correct — if systemd user services are unavailable, no service can be enabled.Testing
Added 3 test cases to
systemd-availability.test.ts:All existing tests continue to pass.
Fixes #1818
Greptile Overview
Greptile Summary
This PR makes
isSystemdServiceEnabledresilient to environments wheresystemctl --usercan’t connect to the user bus (common in SSH/WSL when DBUS env vars aren’t set). The core change wrapsassertSystemdAvailable()in a try/catch and returnsfalseon failure, aligning behavior withisSystemdUserServiceAvailable().It also adds Vitest coverage for the new behavior, plus basic enabled/disabled cases, to prevent regressions in the onboarding wizard’s systemd checks.
Confidence Score: 4/5
assertSystemdAvailable()failures by silently returning false, which may reduce diagnosability in unexpected error scenarios.(5/5) You can turn off certain types of comments like style here!