fix(doctor): shell completion install fails doctor when profile is read-only by rballiance · Pull Request #99540 · openclaw/openclaw

rballiance · 2026-07-03T12:52:58Z

When ~/.bashrc or other shell profile is not writable (e.g., read-only files in sandboxes with root-owned rc files), doctor --fix previously failed with exit code 1 due to unhandled permission errors. This change treats shell completion installation as optional, similar to how other doctor sub-steps degrade gracefully. On EACCES/EPERM/EROFS, a warning note is shown instead of throwing, and doctor exits 0. Non-permission errors (e.g., ENOSPC) are still re-thrown.

Fixes #99237

What Problem This Solves

Fixes an issue where users running openclaw doctor --fix in environments with read-only shell profiles would experience a hard exit with code 1, even though all other doctor checks completed successfully.

The affected workflow is the doctor --fix command, which runs multiple health checks and repairs. When shell completion installation attempts to write to a read-only ~/.bashrc file (common in containerized or sandboxed environments like NVIDIA NemoClaw), the entire doctor run fails instead of treating this as an optional convenience step.

Why This Change Was Made

Shell completion is a convenience feature, not a critical health check. The fix aligns behavior with doctor --fix --non-interactive, which already skips this step entirely. By wrapping installCompletion() calls in try-catch blocks that only handle permission errors (EACCES, EPERM, EROFS), the code now shows a warning note instead of throwing an error, allowing the doctor process to exit with code 0.

The implementation adds:

isProfileWriteError() helper function that detects permission errors (EACCES, EPERM, EROFS), including those wrapped in a cause chain
Graceful error handling in two locations within doctor-completion.ts:
- When upgrading slow dynamic completion patterns to cached completion
- When installing completion for the first time after user confirmation

User Impact

Users with read-only shell profiles will now see a clear warning message instead of a fatal error:
Shell completion not upgraded: /sandbox/.bashrc is not writable. Run openclaw completion --install against a writable profile file.

The doctor command will exit with code 0, allowing automation scripts and CI pipelines to complete successfully. Users can manually install completion later when they have write access to their shell profile.

Evidence

Test cases added: doctor-completion.test.ts now includes:

handles EACCES gracefully when installing completion - verifies permission errors are downgraded to warnings
re-throws non-permission errors from installCompletion - verifies ENOSPC and other errors are still re-thrown

All existing tests pass:

src/commands/doctor-completion.test.ts: 6 tests passed
src/flows/doctor-health-contributions.test.ts: 61 tests passed
src/cli/completion-runtime.test.ts + src/cli/update-cli.test.ts: 168 tests passed

After-fix terminal output (EACCES gracefully handled):
◇ Shell completion ────────────────────────────────────────────────╮
│ │
│ Your bash profile uses slow dynamic completion (source <(...)). │
│ Upgrading to cached completion for faster shell startup... │
│ │
├───────────────────────────────────────────────────────────────────╯
│
◇ Shell completion ─────────────────────────────────────────────────╮
│ │
│ Shell completion not upgraded: │
│ /tmp/openclaw-doctor-home-xZe3IJ/.bashrc is not writable. Run │
│ openclaw completion --install against a writable profile file. │
│ │
├────────────────────────────────────────────────────────────────────╯

Error coverage:

✅ EACCES (permission denied) - downgraded to warning
✅ EPERM (operation not permitted) - downgraded to warning
✅ EROFS (read-only filesystem) - downgraded to warning
✅ ENOSPC (no space left on device) - re-thrown as error

clawsweeper · 2026-07-03T12:59:32Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 8:30 PM ET / 00:30 UTC.

Summary
The PR makes doctor --fix treat shell-completion profile permission errors as non-fatal warnings while preserving unexpected installer failures, and adds regression coverage for both install and upgrade branches.

PR surface: Source +30, Tests +100. Total +130 across 3 files.

Reproducibility: yes. The linked issue gives a concrete read-only .bashrc reproduction, and current main source shows doctor calls the completion installer without a permission downgrade while the installer wraps profile write failures as fatal errors.

Review metrics: 1 noteworthy metric.

Permission error boundary: 3 downgraded, 1 preserved fatal. The core safety claim is that EACCES, EPERM, and EROFS become doctor warnings while ENOSPC and other unexpected failures still block.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-completion.test.ts, migration/backfill/repair: src/commands/doctor-completion.ts, migration/backfill/repair: src/scripts/test-projects.test.ts, unknown-data-model-change: src/commands/doctor-completion.test.ts, unknown-data-model-change: src/commands/doctor-completion.ts. Migration or upgrade compatibility proof is recorded; maintainers should verify it before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99237
Summary: The PR is a candidate fix for the linked doctor shell-completion permission bug; sibling PRs target the same remaining work.

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 PR body uses closing syntax for the bug report, and the source path matches the reported doctor failure.
fixed_by_candidate: fix: downgrade shell-completion EACCES to a warning in doctor --fix (AI-assisted) #99244 - This open sibling PR also targets the same linked issue with a doctor shell-completion permission downgrade.
fixed_by_candidate: fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix #99300 - This open sibling PR also targets the same linked issue with a doctor shell-completion permission downgrade.
fixed_by_candidate: fix: soften doctor shell completion install failures #99309 - This open sibling PR also targets the same linked issue with a doctor shell-completion permission downgrade.
fixed_by_candidate: fix(doctor): catch EACCES on shell completion install so optional step does not fail doctor --fix #99341 - This open sibling PR also targets the same linked issue with a doctor shell-completion permission downgrade.
2 more in the report.

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Risk before merge

[P1] Several open sibling PRs target the same linked bug, so maintainers should land one canonical implementation and then close or supersede the rest.
Exact-head broad CI still had pending jobs in the last status snapshot; no code repair is indicated, but merge should wait for required checks.

Maintainer options:

Decide the mitigation before merge
Land one canonical doctor-boundary fix for the linked bug after exact-head CI finishes, then close the sibling duplicate PRs and linked issue through the normal merge flow.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair is needed; maintainer action is to select and merge one canonical fix after exact-head checks finish.

Security
Cleared: The diff changes local doctor CLI TypeScript and tests only, with no dependency, workflow, credential, package-resolution, or secret-handling surface added.

Review details

Best possible solution:

Land one canonical doctor-boundary fix for the linked bug after exact-head CI finishes, then close the sibling duplicate PRs and linked issue through the normal merge flow.

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

Yes. The linked issue gives a concrete read-only .bashrc reproduction, and current main source shows doctor calls the completion installer without a permission downgrade while the installer wraps profile write failures as fatal errors.

Is this the best way to solve the issue?

Yes. The patch keeps explicit completion installation strict and downgrades only profile permission failures in the doctor-owned optional repair path, with non-permission errors still rethrown.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a normal-priority doctor CLI bug fix for a bounded filesystem permission failure in locked-down environments.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and maintainer follow-up include after-fix terminal/Testbox proof showing real openclaw doctor --fix --yes exits 0 with a read-only .bashrc warning and leaves the profile unchanged.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and maintainer follow-up include after-fix terminal/Testbox proof showing real openclaw doctor --fix --yes exits 0 with a read-only .bashrc warning and leaves the profile unchanged.

Evidence reviewed

PR surface:

Source +30, Tests +100. Total +130 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	34	4	+30
Tests	2	110	10	+100
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	144	14	+130

What I checked:

Repository policy: Root AGENTS.md and the relevant shared test-helper guide were read; they required whole-path PR review, best-fix judgment, and checking the shared-helper boundary. (AGENTS.md:1, 24dba4b44a5b)
Current main failure path: On current main, doctorShellCompletion calls installCompletion directly in both slow-pattern upgrade and fresh install paths, so installer profile write errors can escape doctor. (src/commands/doctor-completion.ts:198, 24dba4b44a5b)
Installer wraps filesystem errors: installCompletion writes the resolved shell profile and wraps failures as Failed to install completion with the original filesystem error in cause, matching the linked EACCES report. (src/cli/completion-runtime.ts:303, 24dba4b44a5b)
PR implementation: PR head adds installCompletionForDoctor, unwraps cause-chain errno values, downgrades only EACCES, EPERM, and EROFS, and rethrows every other installer failure. (src/commands/doctor-completion.ts:53, 1a055eeb251b)
Regression coverage: PR head covers EACCES, EPERM, and EROFS for both doctor install and upgrade paths, and confirms ENOSPC is still rethrown. (src/commands/doctor-completion.test.ts:170, 1a055eeb251b)
Sibling surfaces: The setup wizard and explicit completion CLI still call installCompletion directly, while update flow already catches completion refresh failures separately, so the PR keeps the new downgrade doctor-scoped. (src/wizard/setup.completion.ts:71, 24dba4b44a5b)

Likely related people:

Shakker: All-refs history shows commit 3fae903 added the doctor shell-completion check module that owns the implicated doctor repair path. (role: introduced behavior; confidence: high; commits: 3fae90386328; files: src/commands/doctor-completion.ts, src/commands/doctor-completion.test.ts)
Peter Steinberger: History shows recent doctor orchestration/refactor work around src/flows/doctor-health-contributions.ts and adjacent CLI/doctor changes in this area. (role: recent adjacent contributor; confidence: medium; commits: 7d6d642cb825, 155162a8cd80, 48aa076d12e7; files: src/flows/doctor-health-contributions.ts, src/commands/doctor-completion.ts, src/cli/completion-runtime.ts)
Vincent Koc: History ties Vincent Koc to the shared temp-dir helper now used by the test and adjacent completion/runtime cleanup commits. (role: test helper and adjacent CLI contributor; confidence: medium; commits: f53fdb688df8, ce50b97c8661, 0e54440ecc39; files: test/helpers/temp-dir.ts, src/cli/completion-runtime.ts)
joelnishanth: Local blame attributes the current checked-out implicated lines to broad commit 39b703d, though that commit is low-signal for original feature ownership. (role: current source commit author; confidence: low; commits: 39b703dfcf0f; files: src/commands/doctor-completion.ts, src/cli/completion-runtime.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.

steipete · 2026-07-04T00:01:30Z

Maintainer rewrite + validation complete at 1a055eeb25.

What improved:

centralized install/upgrade handling in one doctor-owned helper;
unwraps the real installCompletion cause and downgrades only EACCES, EPERM, and EROFS;
preserves ENOSPC and every other unexpected failure;
reports the filesystem error's actual profile path, including bash's .bash_profile fallback;
table-driven coverage for both install and upgrade branches, all three permission codes, and non-permission rethrow;
migrated new temp fixtures to the repository cleanup tracker.

Proof:

node scripts/run-vitest.mjs src/commands/doctor-completion.test.ts — 11/11 passed.
node scripts/run-vitest.mjs src/scripts/test-projects.test.ts src/commands/doctor-completion.test.ts — importer routing 83/83 and doctor 11/11 passed.
Testbox-through-Crabbox tbx_01kwn5t8e8tqf3ympqtnr8bkpd / Actions 28688138246 — full source build, then real openclaw doctor --fix --yes as the Testbox user with a chmod 444 .bashrc; doctor exited 0, emitted the read-only warning, and left the profile unchanged.
Testbox-through-Crabbox tbx_01kwn83pc4aeta51t78yrhewne / Actions 28689151643 — final three-file patch-tree pnpm check:changed passed against current main, with test types, lint, importer routing, import-cycle, and database guards green.
Fresh Codex autoreviews — full branch clean at 0.98; final importer-routing delta clean at 0.99.

GitHub CI on the exact pushed SHA remains the final merge gate. No provider key is relevant to this filesystem-only doctor path.

Treat wrapped shell-profile permission failures as an optional doctor repair, while preserving non-permission failures and the actual failing profile path. Cover install and upgrade flows across EACCES, EPERM, and EROFS. Fixes openclaw#99237 Co-authored-by: Ceng Xin <[email protected]>

steipete · 2026-07-04T00:35:38Z

Merged via squash.

Prepared head SHA: 1a055eeb251b3fe7fb2a01152bf9b7b84a3dd6cf
Landed commit: f59310a555d784ccf0724a09e1871260304aa738

Treat wrapped shell-profile permission failures as an optional doctor repair, while preserving non-permission failures and the actual failing profile path. Cover install and upgrade flows across EACCES, EPERM, and EROFS. Fixes openclaw#99237 Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added commands Command implementations size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 3, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 3, 2026

steipete self-assigned this Jul 3, 2026

steipete force-pushed the fix/issue-99237-readonly-shell-completion branch from 7f964da to 3cbb06f Compare July 4, 2026 00:01

steipete force-pushed the fix/issue-99237-readonly-shell-completion branch from 3cbb06f to b8a2eb2 Compare July 4, 2026 00:09

clawsweeper Bot added the status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. label Jul 4, 2026

steipete force-pushed the fix/issue-99237-readonly-shell-completion branch from b8a2eb2 to 1a055ee Compare July 4, 2026 00:26

steipete merged commit f59310a into openclaw:main Jul 4, 2026
96 checks passed

sandl99 mentioned this pull request Jul 6, 2026

[DGX Spark][Agent&Skills] openclaw doctor --fix exits 1 due to EACCES installing shell completion to read-only /sandbox/.bashrc NVIDIA/NemoClaw#6048

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(doctor): shell completion install fails doctor when profile is read-only#99540

fix(doctor): shell completion install fails doctor when profile is read-only#99540
steipete merged 1 commit into
openclaw:mainfrom
rballiance:fix/issue-99237-readonly-shell-completion

rballiance commented Jul 3, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

rballiance 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

User Impact

Evidence

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rballiance commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

steipete commented Jul 4, 2026 •

edited

Loading