Skip to content

daemon/systemd: treat user-bus-unavailable is-enabled as disabled#34482

Closed
yuweuii wants to merge 1 commit intoopenclaw:mainfrom
yuweuii:fix/34464-systemctl-is-enabled-unavailable
Closed

daemon/systemd: treat user-bus-unavailable is-enabled as disabled#34482
yuweuii wants to merge 1 commit intoopenclaw:mainfrom
yuweuii:fix/34464-systemctl-is-enabled-unavailable

Conversation

@yuweuii
Copy link
Copy Markdown
Contributor

@yuweuii yuweuii commented Mar 4, 2026

Summary

Fix Linux service checks when systemctl --user is-enabled runs in environments without a user bus.

Problem

Issue #34464 reports install/start flows failing with:

systemctl is-enabled unavailable: Failed to connect to bus

On 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.ts
    • add isSystemdUserUnavailable(...)
    • make isSystemdServiceEnabled(...) return false for user-bus-unavailable states
    • reuse the same helper in isSystemdUserServiceAvailable(...)
  • src/daemon/systemd.test.ts
    • update expectation for Failed to connect to bus to resolve false
    • keep hard failure behavior for true non-state errors (e.g. Permission denied)
  • changelog fragment added.

Validation

  • corepack pnpm exec vitest run src/daemon/systemd.test.ts --reporter=dot
  • Result: 20 tests passed.

@openclaw-barnacle openclaw-barnacle bot added gateway Gateway runtime size: S labels Mar 4, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR fixes Linux service detection in environments without a systemd user bus (e.g. non-interactive shells) by treating "Failed to connect to bus" errors from systemctl --user is-enabled as a "disabled/unavailable" state (false) rather than a thrown exception. The change is well-scoped and the test suite is updated to cover both the new soft-failure path and the retained hard-failure path for unrelated errors (e.g. "Permission denied").

Key changes:

  • New isSystemdUserUnavailable(detail) helper centralises all user-bus–unavailability string patterns.
  • isSystemdServiceEnabled now returns false (instead of throwing) when the user bus cannot be reached.
  • isSystemdUserServiceAvailable is refactored to reuse the same helper, replacing a chain of inline includes checks.
  • Two style issues to consider:
    • "no such file or directory" appears in both isSystemdUserUnavailable (new) and the existing isSystemctlMissing, creating semantic overlap between "systemctl binary absent" and "user-bus socket absent". The socket-absent case is already covered by "failed to connect", so the duplicate entry in isSystemdUserUnavailable could be removed to keep the helpers semantically distinct.
    • In isSystemdUserServiceAvailable, all code paths after the res.code === 0 early-return resolve to false, making the if (isSystemctlMissing || isSystemdUserUnavailable) block dead code. The function body could be simplified.

Confidence Score: 4/5

  • Safe to merge; the fix is correct and well-tested, with only minor style/redundancy concerns.
  • The core logic change is correct and directly addresses the reported issue. Tests cover both the new soft-failure path and the retained hard-failure path. The two flagged items (duplicate string in helper and dead-code branch in isSystemdUserServiceAvailable) are pre-existing or low-risk style issues that do not affect runtime correctness.
  • src/daemon/systemd.ts — minor redundancy and dead code in isSystemdUserServiceAvailable are worth cleaning up before or after merging.

Last reviewed commit: 4f3f13d

Comment on lines +178 to +190
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")
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"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.

Suggested change
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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 4, 2026

Additional Comments (1)

src/daemon/systemd.ts
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):

export async function isSystemdUserServiceAvailable(): Promise<boolean> {
  const res = await execSystemctl(["--user", "status"]);
  if (res.code === 0) {
    return true;
  }
  return false;
}
Prompt To Fix With AI
This 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.

@vincentkoc
Copy link
Copy Markdown
Member

Good catch and thanks for the implementation work. The same is-enabled failure path is now resolved in merged #33634 and #34884, so I am closing this as a duplicate/superseded branch. Reopen path is available if current main still fails in your environment.

@vincentkoc vincentkoc closed this Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants