daemon/systemd: treat user-bus-unavailable is-enabled as disabled#34482
daemon/systemd: treat user-bus-unavailable is-enabled as disabled#34482yuweuii wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes Linux service detection in environments without a systemd user bus (e.g. non-interactive shells) by treating Key changes:
Confidence Score: 4/5
Last reviewed commit: 4f3f13d |
| function isSystemdUserUnavailable(detail: string): boolean { | ||
| if (!detail) { | ||
| return false; | ||
| } | ||
| const normalized = detail.toLowerCase(); | ||
| return ( | ||
| normalized.includes("failed to connect") || | ||
| normalized.includes("not been booted with systemd") || | ||
| normalized.includes("system has not been booted") || | ||
| normalized.includes("no such file or directory") || | ||
| normalized.includes("not supported") | ||
| ); | ||
| } |
There was a problem hiding this comment.
"no such file or directory" duplicated across helpers
isSystemdUserUnavailable includes "no such file or directory" (line 187), which is already present in isSystemctlMissing (line 156). Both functions return the same outcome (false) in every call site, so there is no behavioral difference today, but keeping the same string in two semantically distinct helpers can cause confusion: isSystemctlMissing treats the phrase as "the systemctl binary is absent", while isSystemdUserUnavailable treats it as "the user-bus socket path is absent".
Consider removing the overlap from isSystemdUserUnavailable, since a missing socket path message typically arrives as part of a "Failed to connect to bus: No such file or directory" string and is therefore already caught by the "failed to connect" check one line above.
| function isSystemdUserUnavailable(detail: string): boolean { | |
| if (!detail) { | |
| return false; | |
| } | |
| const normalized = detail.toLowerCase(); | |
| return ( | |
| normalized.includes("failed to connect") || | |
| normalized.includes("not been booted with systemd") || | |
| normalized.includes("system has not been booted") || | |
| normalized.includes("no such file or directory") || | |
| normalized.includes("not supported") | |
| ); | |
| } | |
| function isSystemdUserUnavailable(detail: string): boolean { | |
| if (!detail) { | |
| return false; | |
| } | |
| const normalized = detail.toLowerCase(); | |
| return ( | |
| normalized.includes("failed to connect") || | |
| normalized.includes("not been booted with systemd") || | |
| normalized.includes("system has not been booted") || | |
| normalized.includes("not supported") | |
| ); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/daemon/systemd.ts
Line: 178-190
Comment:
**`"no such file or directory"` duplicated across helpers**
`isSystemdUserUnavailable` includes `"no such file or directory"` (line 187), which is already present in `isSystemctlMissing` (line 156). Both functions return the same outcome (`false`) in every call site, so there is no behavioral difference today, but keeping the same string in two semantically distinct helpers can cause confusion: `isSystemctlMissing` treats the phrase as "the systemctl binary is absent", while `isSystemdUserUnavailable` treats it as "the user-bus socket path is absent".
Consider removing the overlap from `isSystemdUserUnavailable`, since a missing socket path message typically arrives as part of a `"Failed to connect to bus: No such file or directory"` string and is therefore already caught by the `"failed to connect"` check one line above.
```suggestion
function isSystemdUserUnavailable(detail: string): boolean {
if (!detail) {
return false;
}
const normalized = detail.toLowerCase();
return (
normalized.includes("failed to connect") ||
normalized.includes("not been booted with systemd") ||
normalized.includes("system has not been booted") ||
normalized.includes("not supported")
);
}
```
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
After the early This was a pre-existing quirk, but this PR adds the new helper call here and makes the structure more visible. Consider either collapsing the function to a single expression, or ensuring the conditional is meant to have a distinct outcome (e.g. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/daemon/systemd.ts
Line: 192-205
Comment:
**Dead-code branch in `isSystemdUserServiceAvailable`**
After the early `return true` for `res.code === 0`, every remaining code path returns `false` unconditionally. The `if (isSystemctlMissing(detail) || isSystemdUserUnavailable(detail))` block (lines 201–203) is therefore dead code — its `return false` is unreachable in any way that differs from the final `return false` on line 204.
This was a pre-existing quirk, but this PR adds the new helper call here and makes the structure more visible. Consider either collapsing the function to a single expression, or ensuring the conditional is meant to have a distinct outcome (e.g. `return true` for some "available but degraded" scenario):
```suggestion
export async function isSystemdUserServiceAvailable(): Promise<boolean> {
const res = await execSystemctl(["--user", "status"]);
if (res.code === 0) {
return true;
}
return false;
}
```
How can I resolve this? If you propose a fix, please make it concise. |
Summary
Fix Linux service checks when
systemctl --user is-enabledruns in environments without a user bus.Problem
Issue #34464 reports install/start flows failing with:
systemctl is-enabled unavailable: Failed to connect to busOn some shells (especially non-interactive / no user session bus), this should be treated as "service unavailable/disabled", not a hard failure.
Changes
src/daemon/systemd.tsisSystemdUserUnavailable(...)isSystemdServiceEnabled(...)returnfalsefor user-bus-unavailable statesisSystemdUserServiceAvailable(...)src/daemon/systemd.test.tsFailed to connect to busto resolvefalsePermission denied)Validation
corepack pnpm exec vitest run src/daemon/systemd.test.ts --reporter=dot