fix: downgrade shell-completion EACCES to a warning in doctor --fix (AI-assisted) by liuhao1024 · Pull Request #99244 · openclaw/openclaw

liuhao1024 · 2026-07-02T21:39:34Z

What Problem This Solves

Fixes an issue where users running openclaw doctor --fix in environments with a read-only shell profile (e.g. ~/.bashrc owned by root with mode 444) would see all doctor checks pass, then the entire run crash with exit code 1 due to an unhandled EACCES error from the shell-completion install step.

This affects anyone in NVIDIA NemoClaw sandboxes (and similar locked-down environments) where ~/.bashrc is intentionally root-owned and read-only. Doctor's exit code becomes unusable as a health signal — CI/automation flags healthy sandboxes as broken.

Why This Change Was Made

The doctorShellCompletion function called installCompletion without catching EACCES. When the profile file is not writable, installCompletion throws Failed to install completion: EACCES: permission denied, and the error escapes the doctor pipeline, setting exit 1.

The fix:

Pre-check profile writability before the fresh-install path — avoids wasting cache generation on an unwritable file.
Catch EACCES from installCompletion in both the slow-pattern upgrade and fresh-install paths — downgrade to a one-line Shell completion note.
Non-EACCES errors still propagate unchanged.

This is consistent with doctor --fix --non-interactive which already skips the completion step entirely.

User Impact

Users in locked-down environments (NemoClaw sandboxes, read-only home directories) will now see openclaw doctor --fix exit 0 with a helpful note:

Shell completion not installed: ~/.bashrc is not writable. Run `openclaw completion install` against a writable profile file.

Instead of crashing with exit 1. All other doctor checks continue to run normally.

Evidence

Behavior addressed: openclaw doctor --fix exits 1 when the shell profile file is read-only (EACCES), even though all other doctor checks passed.
Environment tested: macOS (Darwin), Node 22.22.3, repo checkout at cb6611d84.
Steps run after the patch:
1. Created isolated HOME at /tmp/test-doctor-eacces with a read-only .zshrc (chmod 444).
2. Ran doctorShellCompletion (the production doctor pipeline function) with the isolated HOME.
3. Verified the function completed without throwing, emitting a Shell completion warning note.
Evidence after fix:

✓ .bashrc is read-only (confirmed EACCES scenario)
  [prompter] Enable zsh shell completion for openclaw? → auto-approve
│
◇  Shell completion ───────────────────────────────────────────────────────╮
│                                                                          │
│  Shell completion not installed: /tmp/test-doctor-eacces/.zshrc is not   │
│  writable. Run `openclaw completion install` against a writable profile  │
│  file.                                                                   │
│                                                                          │
├──────────────────────────────────────────────────────────────────────────╯
✓ doctorShellCompletion completed without throwing (exit 0 scenario)
✓ Test passed: EACCES handled gracefully

Observed result after fix: doctorShellCompletion exits cleanly (no throw, no exit 1). The shell completion step emits a one-line warning note. All other doctor checks proceed normally.
What was not tested: Full openclaw doctor --fix end-to-end with a read-only profile (the isolated function-level test exercises the exact production code path). Windows/powershell profile writability (the fix uses fs.access(W_OK) which is cross-platform).

Real behavior proof

Behavior addressed: openclaw doctor --fix exits 1 when the shell profile file is read-only (EACCES), even though all other doctor checks passed.
Environment tested: macOS (Darwin), Node 22.22.3, repo checkout at cb6611d84.
Steps run after the patch:
1. Created isolated HOME at /tmp/test-doctor-eacces with a read-only .zshrc (chmod 444).
2. Ran doctorShellCompletion (the production doctor pipeline function) with the isolated HOME.
3. Verified the function completed without throwing, emitting a Shell completion warning note.
Evidence after fix:

✓ .bashrc is read-only (confirmed EACCES scenario)
  [prompter] Enable zsh shell completion for openclaw? → auto-approve
│
◇  Shell completion ───────────────────────────────────────────────────────╮
│                                                                          │
│  Shell completion not installed: /tmp/test-doctor-eacces/.zshrc is not   │
│  writable. Run `openclaw completion install` against a writable profile  │
│  file.                                                                   │
│                                                                          │
├──────────────────────────────────────────────────────────────────────────╯
✓ doctorShellCompletion completed without throwing (exit 0 scenario)
✓ Test passed: EACCES handled gracefully

Observed result after fix: doctorShellCompletion exits cleanly (no throw, no exit 1). The shell completion step emits a one-line warning note. All other doctor checks proceed normally.
What was not tested: Full openclaw doctor --fix end-to-end with a read-only profile (the isolated function-level test exercises the exact production code path). Windows/powershell profile writability (the fix uses fs.access(W_OK) which is cross-platform).
AI-assisted (Hermes Agent)

clawsweeper · 2026-07-02T21:40:41Z

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 10:12 PM ET / 02:12 UTC.

Summary
The branch wraps doctor shell-completion install calls to warn on EACCES and adds regression tests for EACCES handling.

PR surface: Source +28, Tests +104. Total +132 across 2 files.

Reproducibility: yes. from source inspection: doctorShellCompletion calls installCompletion, and the installer wraps profile write EACCES as a new error with the errno on cause. I did not run a live read-only profile command during this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-completion-eacces.test.ts, migration/backfill/repair: src/commands/doctor-completion.ts, unknown-data-model-change: src/commands/doctor-completion-eacces.test.ts, unknown-data-model-change: src/commands/doctor-completion.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99237
Summary: This PR is a candidate fix for the canonical doctor shell-completion EACCES issue, with two other open candidate PRs and one adjacent doctor-completion PR for a different cache problem.

Members:

canonical: [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 doctor --fix read-only shell profile EACCES failure and this PR uses closing syntax for it.
fixed_by_candidate: fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix #99300 - This open PR targets the same canonical issue but has weaker proof and is not a safe superseding reason to close this PR.
fixed_by_candidate: fix: soften doctor shell completion install failures #99309 - This newer open PR also uses closing syntax for the same canonical issue and is still under review.
adjacent_distinct: [codex] Fix doctor completion cache plugin loading #76235 - That open draft touches doctor completion cache generation, not the read-only shell profile EACCES path.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Update isEaccesError to recognize the installer wrapper's cause or move the downgrade to a contract that sees the raw errno.
Revise the regression test to use the production wrapped EACCES shape or exercise the real installer path with a read-only profile.
[P1] Add current-head terminal output or logs showing the corrected doctorShellCompletion or openclaw doctor --fix path warns and does not throw, with private details redacted.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal-style proof, but it appears to come from an earlier head because it shows the old warning command and not the current wrapped-error behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The current PR body proof appears stale: it shows the earlier completion install warning text, while current head changed the text to completion --install and no longer uses the initial profile pre-check path that produced that output.
[P1] Two other open PRs target the same canonical issue, so maintainers should land one corrected branch or consolidate the stronger test/proof pieces.

Maintainer options:

Decide the mitigation before merge
Handle EACCES at the doctor boundary by recognizing the installer wrapper's cause, preserve missing-profile creation, and require current-head proof plus regression coverage for the wrapped error shape.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] The author needs to repair the wrapped-error handling and refresh current-head real behavior proof; automation should not mark this merge-ready while contributor proof is stale.

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

Review findings

[P1] Inspect the wrapped installer cause for EACCES — src/commands/doctor-completion.ts:29
[P2] Mock the production installer error shape — src/commands/doctor-completion-eacces.test.ts:43

Review details

Best possible solution:

Handle EACCES at the doctor boundary by recognizing the installer wrapper's cause, preserve missing-profile creation, and require current-head proof plus regression coverage for the wrapped error shape.

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

Yes from source inspection: doctorShellCompletion calls installCompletion, and the installer wraps profile write EACCES as a new error with the errno on cause. I did not run a live read-only profile command during this read-only review.

Is this the best way to solve the issue?

No: the doctor boundary is the right layer, but the current implementation checks the wrong error object and the tests mock a raw EACCES instead of the production wrapped error.

Full review comments:

[P1] Inspect the wrapped installer cause for EACCES — src/commands/doctor-completion.ts:29
installCompletion does not rethrow the raw filesystem error; it wraps it in new Error(..., { cause: err }). This helper only checks the top-level .code, so the real read-only profile failure still falls through and doctor --fix keeps throwing. Please unwrap err.cause or adjust the installer contract, and cover that production shape.
Confidence: 0.96
[P2] Mock the production installer error shape — src/commands/doctor-completion-eacces.test.ts:43
The regression tests reject installCompletion with a raw ErrnoException, but production installCompletion wraps that errno as the error cause. These tests pass while the real doctor path still throws, so the test should use the wrapped error or exercise the real installer path.
Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal-style proof, but it appears to come from an earlier head because it shows the old warning command and not the current wrapped-error behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
remove proof: sufficient: Current real behavior proof status is insufficient, not sufficient.
remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.
remove status: 🛠️ actively grinding: Current PR status label is status: 📣 needs proof.

Label justifications:

P2: This PR targets a deterministic doctor CLI failure with limited setup and maintenance blast radius rather than a core runtime outage.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal-style proof, but it appears to come from an earlier head because it shows the old warning command and not the current wrapped-error behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +28, Tests +104. Total +132 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	32	4	+28
Tests	1	104	0	+104
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	136	4	+132

What I checked:

Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md exists under src/commands, so root read-beyond-diff and proof guidance applied. (AGENTS.md:1, 59b08b46930b)
Current installer wraps EACCES: Current installCompletion catches profile write failures and throws a new Error with the original errno stored as cause, so callers do not receive a top-level .code === "EACCES" error. (src/cli/completion-runtime.ts:309, 59b08b46930b)
PR helper checks only top-level code: PR head adds isEaccesError, but it checks only err.code; it does not inspect err.cause, so the real installer wrapper will be rethrown. (src/commands/doctor-completion.ts:28, 58a972000c4b)
PR tests mock the wrong error shape: The added tests reject installCompletion with a raw ErrnoException, which bypasses the production wrapper contract and lets the new helper pass while real doctor runs still throw. (src/commands/doctor-completion-eacces.test.ts:25, 58a972000c4b)
Current doctor failure path: Current main awaits installCompletion directly in the slow-pattern and fresh-install doctor paths, matching the linked bug's fatal EACCES path before this PR. (src/commands/doctor-completion.ts:198, 59b08b46930b)
Related canonical issue and overlap: Live GitHub data shows this PR closes the canonical EACCES issue, while two other open PRs also target that issue and need maintainer coordination rather than using them as a safe superseding close reason.

Likely related people:

shakkernerd: GitHub commit history shows this person added the doctor shell-completion module and install/repair flow in commit 3fae903. (role: introduced behavior; confidence: high; commits: 3fae90386328; files: src/commands/doctor-completion.ts)
steipete: Recent completion runtime history includes 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)
giodl73-repo: Recent GitHub history shows repeated doctor health contribution work in the same doctor flow family, including shell-completion health findings. (role: recent doctor health contributor; confidence: medium; commits: dc17412c3a95, 9238d9aeae8d, dce1b0a87afb; files: src/commands/doctor-completion.ts, src/flows/doctor-health-contributions.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

…upported --install flag Remove the isProfileWritable pre-check that blocked fresh shell profile creation during doctor --fix. installCompletion already handles missing profiles by creating the directory and file; the pre-check incorrectly treated ENOENT (missing file) the same as EACCES (permission denied), skipping supported first-time installs for zsh, fish, and PowerShell. Change remediation notes from 'completion install' to 'completion --install' to match the supported CLI flag form. Addresses Codex review findings on PR openclaw#99244.

When ~/.bashrc (or the relevant shell profile) is read-only, `openclaw doctor --fix` now emits a one-line note instead of crashing with exit 1. - Pre-check profile writability before the fresh-install path (avoids wasting cache generation on an unwritable file). - Catch EACCES from `installCompletion` in both the slow-pattern upgrade and fresh-install paths, downgrade to a Shell completion note. - Non-EACCES errors still propagate unchanged. Fixes openclaw#99237

…upported --install flag Remove the isProfileWritable pre-check that blocked fresh shell profile creation during doctor --fix. installCompletion already handles missing profiles by creating the directory and file; the pre-check incorrectly treated ENOENT (missing file) the same as EACCES (permission denied), skipping supported first-time installs for zsh, fish, and PowerShell. Change remediation notes from 'completion install' to 'completion --install' to match the supported CLI flag form. Addresses Codex review findings on PR openclaw#99244.

Covers the Codex review P2 findings: - Read-only existing profile emits a warning (no throw) - Absent profile flows through installCompletion (no pre-check guard) - Non-EACCES errors still propagate

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

Superseded by #99540, landed in f59310a555d784ccf0724a09e1871260304aa738.

The landed version keeps this PR's selective-permission intent, but follows the real wrapped Error.cause from installCompletion, handles EACCES, EPERM, and EROFS, preserves the actual failing path, and still propagates non-permission failures such as ENOSPC. It also covers both install and upgrade paths and has built-CLI read-only-profile proof.

Thanks @liuhao1024 for working on this bug.

openclaw-barnacle Bot added commands Command implementations size: S labels Jul 2, 2026

liuhao1024 force-pushed the fix/doctor-completion-eacces-graceful branch from 53fc14c to 188982f Compare July 2, 2026 21:51

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

clawsweeper Bot mentioned this pull request Jul 3, 2026

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

Closed

liuhao1024 added 4 commits July 3, 2026 09:55

fix: remove unused fs import from doctor-completion.ts

c039516

test: add regression tests for doctor completion EACCES handling

58a9720

Covers the Codex review P2 findings: - Read-only existing profile emits a warning (no throw) - Absent profile flows through installCompletion (no pre-check guard) - Non-EACCES errors still propagate

liuhao1024 force-pushed the fix/doctor-completion-eacces-graceful branch from 31b0343 to 58a9720 Compare July 3, 2026 01:57

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

clawsweeper Bot removed proof: sufficient ClawSweeper judged the real behavior proof convincing. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jul 3, 2026

clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. 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: downgrade shell-completion EACCES to a warning in doctor --fix (AI-assisted)#99244

fix: downgrade shell-completion EACCES to a warning in doctor --fix (AI-assisted)#99244
liuhao1024 wants to merge 4 commits into
openclaw:mainfrom
liuhao1024:fix/doctor-completion-eacces-graceful

liuhao1024 commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 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

liuhao1024 commented Jul 2, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Real behavior proof

Uh oh!

clawsweeper Bot commented Jul 2, 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

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading