Skip to content

fix: doctor --fix exits 1 when shell-completion install hits EACCES on read-only rc file#99436

Closed
lwy-2 wants to merge 1 commit into
openclaw:mainfrom
lwy-2:fix/doctor-completion-eacces-graceful
Closed

fix: doctor --fix exits 1 when shell-completion install hits EACCES on read-only rc file#99436
lwy-2 wants to merge 1 commit into
openclaw:mainfrom
lwy-2:fix/doctor-completion-eacces-graceful

Conversation

@lwy-2

@lwy-2 lwy-2 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where users running openclaw doctor --fix in an environment with a read-only shell rc file (e.g. ~/.bashrc with mode 444) see all doctor sections complete green, then get Error: Failed to install completion: EACCES: permission denied, open '/sandbox/.bashrc' and exit code 1 — even though shell completion is an optional feature and every other doctor sub-step degrades gracefully.

Closes #99237

Why This Change Was Made

The doctorShellCompletion function called installCompletion directly without error wrapping. When installCompletion threw (e.g. EACCES on a read-only rc file), the exception propagated through the doctor runner as a hard failure with exit code 1.

The fix introduces tryInstallCompletion, a wrapper that catches all errors from installCompletion, prints a user-facing warning, and returns false instead of throwing. Both call sites in doctorShellCompletion (slow-profile upgrade path and fresh-install path) now use the wrapper and only emit the success note when installation actually succeeded.

This is consistent with how other doctor sub-steps degrade: doctor --fix --non-interactive already skips completion entirely, so bare --fix should also be graceful.

User Impact

Users with read-only shell rc files can now run openclaw doctor --fix to completion (exit 0) instead of getting a hard failure. A one-line warning is printed explaining that completion was not installed and pointing to the manual command.

Evidence

1. Unit tests

All 5 tests pass, including the new EACCES degradation test:

$ node scripts/run-vitest.mjs src/commands/doctor-completion.test.ts

 RUN  v4.1.8

|  Shell completion                                                |
|  Shell completion not installed: EACCES: permission denied,      |
|  open '/sandbox/.bashrc'. Run `openclaw completion install`      |
|  against a writable rc file manually.                            |

 Test Files  1 passed (1)
      Tests  5 passed (5)

2. Format / Lint / Type checks

  • pnpm exec oxfmt --check — all matched files use correct format
  • node scripts/run-oxlint.mjs — exit 0
  • node scripts/run-tsgo.mjs — exit 0
  • git diff --check — no whitespace errors

3. Before / After

Before: openclaw doctor --fix exits 1 with Error: Failed to install completion: EACCES...
After: Prints a warning and exits 0.

4. Real behavior proof (test output)

The unit test output above confirms the warning is printed and no exception propagates. The test uses mocked installCompletion to throw the exact EACCES error from the issue repro, verifying the degradation path works end-to-end.

@lwy-2 @claude
fix: degrade shell-completion EACCES to warning instead of hard exit 1
c9835c1 
When doctor --fix runs shell-completion install against a read-only rc file
(~/.bashrc with mode 444), print a one-line warning and exit 0 instead of
throwing Error('Failed to install completion: EACCES...') and exiting 1.

Introduces tryInstallCompletion wrapper that catches installCompletion errors,
logs a user-facing note, and returns false. Both call sites in
doctorShellCompletion (slow-profile upgrade and fresh install) use the wrapper
and only emit success messages on actual success.

Closes openclaw#99237

Co-Authored-By: Claude <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the same doctor shell-completion EACCES bug is already owned by the proof-positive, mergeable #99341, while this branch has mock-only proof and a test type mismatch.

Root-cause cluster
Relationship: superseded
Canonical: #99341
Summary: This PR is an overlapping candidate for the same doctor shell-completion EACCES bug, and the canonical landing candidate is the proof-positive open PR.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Land one canonical doctor-boundary fix, currently #99341, then let the linked bug report close on merge.

So I’m closing this here and keeping the remaining discussion on #99341.

Review details

Best possible solution:

Land one canonical doctor-boundary fix, currently #99341, then let the linked bug report close on merge.

Do we have a high-confidence way to reproduce the issue?

Yes. The linked issue provides concrete EACCES logs, and current main still lets doctorShellCompletion call installCompletion without catching profile write failures.

Is this the best way to solve the issue?

No for this branch. The doctor boundary is the right fix location, but the proof-positive overlapping PR is already the stronger canonical landing path and this branch has a test type error.

Security review:

Security review cleared: The diff only changes local doctor completion error handling and tests, with no dependency, workflow, credential, or supply-chain surface added.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • shakkernerd: Commit search shows this person added the doctor shell-completion module and integrated doctorShellCompletion into the doctor command. (role: introduced behavior; confidence: high; commits: 3fae90386328, 5bd63b012c9c; files: src/commands/doctor-completion.ts, src/flows/doctor-health-contributions.ts)
  • giodl73-repo: Recent GitHub history shows repeated doctor health contribution work in the same doctor flow family, including shell-completion health findings. (role: recent area contributor; confidence: medium; commits: dc17412c3a95, 9238d9aeae8d, dce1b0a87afb; files: src/commands/doctor-completion.ts, src/flows/doctor-health-contributions.ts)
  • steipete: Completion runtime history shows adjacent CLI utility failure-handling work that preserved completion install error causes and made update completion refresh best-effort. (role: recent adjacent contributor; confidence: medium; commits: 0ec29289c686, d2ff1c31d63e; files: src/cli/completion-runtime.ts, src/commands/doctor-completion.ts, src/cli/update-cli/update-command.ts)
  • vincentkoc: Recent history touches shell-completion tests and completion runtime internals around the affected completion surfaces. (role: recent adjacent contributor; confidence: medium; commits: 120fd2f702bd, 0cf941344ca1; files: src/cli/completion-runtime.ts, src/commands/doctor-completion.ts)

Codex review notes: model internal, reasoning high; reviewed against 1fef99962edf.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 3, 2026
This was referenced Jul 3, 2026
Closed
Merged
@steipete

steipete commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Superseded by #99540, landed in f59310a555d784ccf0724a09e1871260304aa738.

The canonical fix narrows recovery to wrapped EACCES, EPERM, and EROFS profile errors instead of catching every install failure, so unrelated failures such as ENOSPC still stop doctor. It centralizes both install branches and includes built-CLI read-only-profile proof.

Thanks @lwy-2 for working on this bug.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

commands Command implementations P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: doctor --fix exits 1 when shell-completion install hits a read-only ~/.bashrc (EACCES) — optional step fails the whole doctor run

2 participants

@lwy-2 @steipete