Skip to content

Doctor: expose write-config blocker findings#100093

Merged
giodl73-repo merged 6 commits into
openclaw:mainfrom
giodl73-repo:doctor-write-config-lint-findings
Jul 5, 2026
Merged

Doctor: expose write-config blocker findings#100093
giodl73-repo merged 6 commits into
openclaw:mainfrom
giodl73-repo:doctor-write-config-lint-findings

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • add default-disabled core/doctor/write-config lint findings for config write blockers
  • report Nix immutable config mode and unwritable config write directories/ancestors without attempting a write
  • keep real config writes in the existing doctor:write-config run path; no CLI, SDK, plugin, or config contract change
  • complete the tracked Doctor rule inventory: this is the 44th of 44 rule families with structured lint findings

Validation

  • corepack pnpm exec vitest run src/flows/doctor-health-contributions.test.ts --maxWorkers 1 --no-fileParallelism (81 tests)
  • corepack pnpm exec oxfmt --check src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • corepack pnpm exec oxlint --tsconfig config/tsconfig/oxlint.core.json src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • corepack pnpm tsgo:core
  • git diff --check

Real behavior proof

Source lint entrypoint with isolated OPENCLAW_CONFIG_PATH / OPENCLAW_STATE_DIR and OPENCLAW_NIX_MODE=1:

  • default doctor --lint --json path: checksRun=23, checksSkipped=24, no core/doctor/write-config finding emitted
  • selected lint path: --only core/doctor/write-config equivalent ran checksRun=1, checksSkipped=46, emitted core/doctor/write-config with requirement=mutable-config-write-path

Notes

  • The new check is opt-in via defaultEnabled: false; default lint behavior is unchanged.
  • This PR intentionally does not report "there is config normalization to write" as a lint finding. That remains separate from the write-path blocker diagnostics here.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jul 4, 2026
@giodl73-repo

giodl73-repo commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper please review this default-disabled Doctor write-config lint findings slice. It adds read-only blocker diagnostics only; no CLI, SDK, plugin, or config contract change.

@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: please review this default-disabled Doctor write-config lint findings slice. It adds read-only blocker diagnostics only; no CLI, SDK, plugin, or config contract change.

@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 12:14 PM ET / 16:14 UTC.

Summary
This PR adds a default-disabled core/doctor/write-config Doctor lint health check and regression tests for Nix mode plus config write-path blockers.

PR surface: Source +95, Tests +255. Total +350 across 2 files.

Reproducibility: not applicable. this is a feature PR adding a Doctor lint diagnostic rather than a bug report with a failing current-main reproduction path. The behavior is checkable from PR-head source, tests, and the PR body's copied lint output.

Review metrics: 1 noteworthy metric.

  • Doctor lint check surface: 1 added, 0 default-enabled. The new diagnostic affects explicit and --all lint runs while preserving ordinary default lint output.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] Explicit --only core/doctor/write-config and doctor --lint --all runs may now warn and return nonzero for existing Nix or config write-path blockers; ordinary default lint remains unchanged.

Maintainer options:

  1. Land The Opt-In Diagnostic (recommended)
    Preserve ordinary default lint behavior while allowing explicitly selected write-config lint coverage to fail on real blockers.
  2. Pause For The Stacked Audit
    Hold this PR if maintainers want one combined decision for the broader doctor --lint --all selection behavior in Doctor: audit lint default selection #100361.

Next step before merge

  • No automated repair remains; the protected label and intentional opt-in lint compatibility behavior need a maintainer landing decision.

Maintainer decision needed

  • Question: Should maintainers land this default-disabled write-config lint diagnostic even though explicit --only and --all lint runs may now return nonzero for existing config write blockers?
  • Rationale: Doctor/config lint behavior is compatibility-sensitive, and the protected maintainer label keeps the final landing decision with a human despite no blocking code finding.
  • Likely owner: vincentkoc — Current-main blame and recent config history connect this person to the Doctor/config write behavior that defines the compatibility boundary.
  • Options:
    • Land Accepted Opt-In Lint Behavior (recommended): Keep the check default-disabled and accept selected/all-check lint failures for real Nix or config write-path blockers.
    • Hold For Default-Selection Audit: Wait for Doctor: audit lint default selection #100361 if maintainers want to decide this slice together with the broader lint default policy.

Security
Cleared: The diff adds read-only Doctor diagnostics and tests without dependency, workflow, secret, package execution, or actual config-write changes.

Review details

Best possible solution:

Land the default-disabled diagnostic if maintainers accept selected/all lint failure for real write blockers, and leave the broader default-selection audit to #100361.

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

Not applicable: this is a feature PR adding a Doctor lint diagnostic rather than a bug report with a failing current-main reproduction path. The behavior is checkable from PR-head source, tests, and the PR body's copied lint output.

Is this the best way to solve the issue?

Yes, with maintainer acceptance of the opt-in policy: the implementation stays in the existing Doctor contribution boundary, keeps the check default-disabled, and mirrors config write directory requirements for a read-only diagnostic.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority Doctor lint improvement with limited blast radius and no blocking correctness finding.
  • merge-risk: 🚨 compatibility: The PR intentionally changes selected and --all Doctor lint behavior for existing setups with Nix or config write-path blockers.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-change live output showing default lint skips the new check and selected lint emits the expected write-config finding.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-change live output showing default lint skips the new check and selected lint emits the expected write-config finding.
Evidence reviewed

PR surface:

Source +95, Tests +255. Total +350 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 95 0 +95
Tests 1 255 0 +255
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 350 0 +350

What I checked:

Likely related people:

  • giodl73-repo: Recent merged history on src/flows/doctor-health-contributions.ts repeatedly adds default-disabled structured Doctor lint findings, matching this PR's pattern beyond this branch alone. (role: recent Doctor lint contributor; confidence: high; commits: 7bb686c14012, 219f27a5ec6b, 6011c9e16d21; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-health-contributions.test.ts, src/flows/doctor-lint-flow.ts)
  • vincentkoc: Local blame and recent config history connect this person to the current write-config contribution, lint selection, and config write boundary used for this review. (role: current-main area contributor; confidence: medium; commits: fd2e4da00651, 6daabd23f821, 767e8280ac7c; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-lint-flow.ts, src/config/io.ts)
  • yetval: Recent config history includes the fail-closed unreadable-config write rejection that defines part of the write-blocker boundary this diagnostic exposes. (role: adjacent config write behavior contributor; confidence: medium; commits: 14198836b2c6; files: src/config/io.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.
Review history (21 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-05T04:58:27.609Z sha 35a22b2 :: needs changes before merge. :: [P2] Require search permission for config parents
  • reviewed 2026-07-05T05:04:04.443Z sha 4f620ee :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T05:09:37.166Z sha 4f620ee :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T14:03:02.356Z sha 4f620ee :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T15:35:16.128Z sha b65aefa :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T15:54:51.865Z sha a09c8f6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T16:01:23.214Z sha a09c8f6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T16:08:31.895Z sha c1a0a58 :: needs maintainer review before merge. :: none

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper assist: This needs a full correctness review, not a lightweight context answer. The slice touches Doctor health-check behavior and tests, so use @clawsweeper review for line-level validation.

Evidence:

  • PR Doctor: expose write-config blocker findings #100093 is draft, cleanly mergeable, and changes 2 files with +177/-0.
  • Head commit is 5b22c21cbbc2175e60b29dda6c8d91c0768e66fa (doctor: add write-config lint findings).
  • The implementation adds collectWriteConfigHealthFindings in src/flows/doctor-health-contributions.ts, including Nix mode, unreadable file, unwritable file, and unwritable directory diagnostics.
  • Tests were added in src/flows/doctor-health-contributions.test.ts, including opt-in behavior via defaultEnabled: false and selected core/doctor/write-config findings.
  • PR body reports focused validation: vitest for doctor-health-contributions.test.ts, oxfmt, oxlint, tsgo:core, and git diff --check.

Suggested next action: Run @clawsweeper review on #100093 for the requested correctness review.

Source: #100093 (comment)
Assist reasoning: low.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 4, 2026
@giodl73-repo
giodl73-repo force-pushed the doctor-write-config-lint-findings branch from 5b22c21 to 8c3e9bc Compare July 5, 2026 02:47
@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper please re-review the rebased head. Scope is unchanged: default-disabled Doctor write-config blocker lint findings, now noted as the 44th of 44 tracked Doctor rule families with structured lint findings; no CLI, SDK, plugin, or config contract change.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: please re-review the rebased head. Scope is unchanged: default-disabled Doctor write-config blocker lint findings, now noted as the 44th of 44 tracked Doctor rule families with structured lint findings; no CLI, SDK, plugin, or config contract change.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper assist: This is a request for a full correctness re-review of the rebased PR head, so the right path is @clawsweeper review. The supplied context already shows a completed review on the rebased SHA with no findings, but lightweight assist cannot substitute for line-level review.

Evidence:

  • PR: Doctor: expose write-config blocker findings #100093
  • Current head SHA is 8c3e9bc50b90d55580241c05a4a98df13f6a3382, after a force-push at 2026-07-05T02:47:23Z.
  • Previous ClawSweeper review covered SHA 8c3e9bc50b90d55580241c05a4a98df13f6a3382 at 2026-07-05T02:59:04.244Z with no findings.
  • Scope shown in the PR remains 2 files, +177/-0: src/flows/doctor-health-contributions.ts and src/flows/doctor-health-contributions.test.ts.
  • PR body states the check is defaultEnabled: false, covers write-config blocker diagnostics, and does not change CLI, SDK, plugin, or config contracts.

Suggested next action: Run @clawsweeper review on #100093 for the requested rebased-head correctness review.

Source: #100093 (comment)
Assist reasoning: low.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 5, 2026
@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the P2 findings on the new head: missing config paths now check the nearest existing parent to match recursive config writes, and the unreachable unreadable-config diagnostic was removed from this slice. Focused validation passed: contribution Vitest 77 tests, changed-file oxfmt, changed-file oxlint, tsgo:core, and git diff --check.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the atomic-write parity P2 on the new head: existing and missing config paths now both check the containing directory / nearest existing parent, matching recursive mkdir plus atomic replacement semantics. Added regressions for read-only existing config in a writable directory and for an existing config whose directory is unwritable. Focused validation passed: contribution Vitest 78 tests, changed-file oxfmt, changed-file oxlint, tsgo:core, and git diff --check.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the latest P2 on the new head: the write-config detector now reports an existing file that blocks the config directory path before checking writability, matching recursive mkdir behavior. Added regression coverage for the file-path-component case. Focused validation passed: contribution Vitest 79 tests, changed-file oxfmt, changed-file oxlint, tsgo:core, and git diff --check.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the dangling-symlink P2 on the new head: nearest existing parent detection now treats lstat-visible path entries as blockers, while directory usability is still checked with stat. Added regression coverage for a dangling symlink blocking the config directory path. Focused validation passed: contribution Vitest 80 tests, changed-file oxfmt, changed-file oxlint, tsgo:core, and git diff --check.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the search-permission P2 on the new head: the write-config detector now requires both write and execute/search permission on the existing config directory or nearest parent, matching recursive mkdir traversal. Added regression coverage for a parent that fails the combined W_OK|X_OK check. Focused validation passed: contribution Vitest 81 tests, changed-file oxfmt, changed-file oxlint, tsgo:core, and git diff --check.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 5, 2026
@giodl73-repo
giodl73-repo marked this pull request as ready for review July 5, 2026 13:51
@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer note: accepting the opt-in lint compatibility tradeoff for this slice. The new core/doctor/write-config diagnostic remains default-disabled, so ordinary doctor --lint output is unchanged; --only core/doctor/write-config and doctor --lint --all may now report Nix/config write-path blockers and return a nonzero lint result when those blockers are present. That is intentional for operator-selected coverage and completes the tracked 44/44 Doctor rule-family lint inventory.

@giodl73-repo giodl73-repo mentioned this pull request Jul 5, 2026
Merged
@giodl73-repo
giodl73-repo force-pushed the doctor-write-config-lint-findings branch 2 times, most recently from b65aefa to a09c8f6 Compare July 5, 2026 15:49
@giodl73-repo
giodl73-repo force-pushed the doctor-write-config-lint-findings branch from a09c8f6 to c1a0a58 Compare July 5, 2026 16:04
@giodl73-repo
giodl73-repo merged commit b1f3986 into openclaw:main Jul 5, 2026
96 checks passed
@giodl73-repo

giodl73-repo commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@giodl73-repo
giodl73-repo deleted the doctor-write-config-lint-findings branch July 5, 2026 16:18
@github-actions github-actions Bot mentioned this pull request Jul 5, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 5, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 6, 2026
@giodl73-repo
Doctor: expose write-config blocker findings (openclaw#100093)
3a333bd 
* doctor: add write-config lint findings

* doctor: align write-config lint with config writes

* doctor: check config write directory permissions

* doctor: detect blocked config directory paths

* doctor: detect symlink config directory blockers

* doctor: require searchable config write parents
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

maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@giodl73-repo