fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix by yplongapp · Pull Request #99300 · openclaw/openclaw

yplongapp · 2026-07-03T01:37:49Z

What Problem This Solves

openclaw doctor --fix exits with code 1 when shell completion install fails on a read-only shell profile (EACCES). Shell completion setup is an optional step — a non-writable profile should not block the rest of the doctor run.

Observed error:

Error: Failed to install completion: EACCES: permission denied, open '/sandbox/.bashrc'

Why This Change Was Made

Wrap both installCompletion call sites in doctorShellCompletion in try-catch blocks. On EACCES (or any install failure), log a friendly message telling the user to run completion install manually after fixing permissions, then continue instead of crashing.

Changes

src/commands/doctor-completion.ts: Wrap two installCompletion calls in try-catch (+20/-4)
src/commands/doctor-completion.test.ts: Add regression test for EACCES handling (+31/-1)

Real behavior proof

Behavior addressed: doctor --fix no longer exits 1 when shell completion install fails on read-only profile.
Environment tested: Node 22.x on Linux
Steps run after the patch: Ran focused unit test and compilation check
Evidence after fix:

Test output (5/5 passed):

 ✓ shell completion health mapping > checks an explicit shell instead of the detected environment shell
 ✓ shell completion health mapping > reports slow dynamic shell completion with dry-run effects
 ✓ shell completion health mapping > reports missing completion cache with a dry-run cache effect
 ✓ shell completion health mapping > keeps healthy shell completion quiet
 ✓ doctorShellCompletion EACCES handling > logs a friendly message and does not throw when installCompletion fails with EACCES

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

Observed result: All 5 tests pass. The new EACCES test mocks installCompletion to reject with EACCES and verifies doctorShellCompletion resolves gracefully instead of throwing.
Not tested: Live EACCES scenario with read-only ~/.bashrc on a real system.

Fixes #99237

…p does not fail doctor --fix When shell profile files (e.g. ~/.bashrc) are read-only, installCompletion throws EACCES, which propagates up from doctorShellCompletion and causes the entire doctor --fix run to exit with code 1. Shell completion is an optional step — a failed install should not block the rest of doctor. Wrap both installCompletion call sites in try-catch so a non-writable profile logs a friendly message and continues instead of crashing the run. Fixes openclaw#99237

clawsweeper · 2026-07-03T01:48:22Z

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

Close as superseded: #99341 now covers the same doctor shell-completion EACCES bug with sufficient real terminal proof and stronger filesystem regression tests, while this branch still has mock-only proof and an ineffective regression test.

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:

canonical: fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix #99341 - It targets the same linked issue, is mergeable, and has sufficient real behavior proof plus stronger filesystem regression tests.
fixed_by_candidate: [Bug]: doctor --fix exits 1 when shell-completion install hits a read-only ~/.bashrc (EACCES) — optional step fails the whole doctor run #99237 - The issue reports the exact read-only shell profile doctor failure that the canonical PR is intended to fix.
same_root_cause: fix: downgrade shell-completion EACCES to a warning in doctor --fix (AI-assisted) #99244 - This open PR also targets the same doctor shell-completion EACCES failure but prior review found blockers.
same_root_cause: fix: soften doctor shell completion install failures #99309 - This open PR also closes the same issue but remains weaker than the proof-positive canonical candidate.

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

Canonical path: Review and land one canonical doctor-boundary fix, currently best represented by #99341, then let #99237 close on merge.

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

Review details

Best possible solution:

Review and land one canonical doctor-boundary fix, currently best represented by #99341, then let #99237 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 routes doctorShellCompletion to 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 stronger canonical path is the proof-positive overlapping PR with real filesystem coverage.

Security review:

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

AGENTS.md: found and applied where relevant.

What I checked:

Repository policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md exists under src/commands, so root read-beyond-diff and proof requirements govern this review. (AGENTS.md:1, c5afa92c1f8a)
Current main still has the bug path: Current main still awaits installCompletion directly in the slow-pattern doctor path, so profile-write failures can escape doctorShellCompletion. (src/commands/doctor-completion.ts:198, c5afa92c1f8a)
Installer wraps profile write failures: installCompletion writes the profile and wraps caught failures as Failed to install completion, matching the linked EACCES failure shape. (src/cli/completion-runtime.ts:303, c5afa92c1f8a)
Latest release still has the failure shape: v2026.6.11 also directly awaits installCompletion in doctorShellCompletion, so the underlying bug is not already shipped-fixed. (src/commands/doctor-completion.ts:198, e085fa1a3ffd)
This PR test can pass without exercising the install call: The added test calls doctorShellCompletion with nonInteractive true; in the common no-profile state current code returns before installCompletion, so the mocked rejection can remain unused. (src/commands/doctor-completion.test.ts:131, 10d0f3363062)
Superseding candidate has stronger proof and tests: fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix #99341 targets the same closing issue, is mergeable, carries proof:sufficient/ready labels, includes terminal proof from a real read-only profile run, and adds filesystem tests for both install paths. (src/commands/doctor-completion.test.ts:126, c9d0529d9766)

Likely related people:

shakkernerd: PR metadata and git history show the shell-completion doctor module and doctor integration came from the merged shell-completion feature work. (role: introduced behavior; confidence: high; commits: 3fae90386328, b9b86fb163e9; files: src/commands/doctor-completion.ts, src/commands/doctor.ts)
vincentkoc: Recent history touches the completion runtime and release/current-main state around the affected completion surfaces. (role: recent adjacent contributor; confidence: medium; commits: 0e54440ecc39, e085fa1a3ffd; files: src/cli/completion-runtime.ts, src/commands/doctor-completion.ts)
steipete: Recent doctor health/contribution history includes the doctor orchestration surface that calls doctorShellCompletion. (role: adjacent doctor flow contributor; confidence: medium; commits: 7d6d642cb825, 48aa076d12e7; files: src/flows/doctor-health-contributions.ts)

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

… handling Add a test that verifies doctorShellCompletion does not throw when installCompletion fails with EACCES. The test mocks installCompletion to reject with an EACCES error and confirms the function resolves gracefully with no thrown exception.

yplongapp · 2026-07-03T05:39:05Z

@clawsweeper re-review

clawsweeper · 2026-07-03T05:39:07Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-04T00:36:43Z

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 @yplongapp for working on this bug.

openclaw-barnacle Bot added commands Command implementations size: XS labels Jul 3, 2026

This was referenced Jul 3, 2026

fix: downgrade shell-completion EACCES to a warning in doctor --fix (AI-assisted) #99244

Closed

fix: soften doctor shell completion install failures #99309

Closed

lzyyzznl mentioned this pull request Jul 3, 2026

fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix #99341

Closed

6 tasks

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 3, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 3, 2026

steipete closed this Jul 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix#99300

fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix#99300
yplongapp wants to merge 2 commits into
openclaw:mainfrom
yplongapp:fix/doctor-completion-eacces-handling

yplongapp commented Jul 3, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

yplongapp commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

yplongapp commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

Changes

Real behavior proof

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yplongapp commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yplongapp commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading